diff --git a/.changeset/patch-add-safe-output-actions.md b/.changeset/patch-add-safe-output-actions.md new file mode 100644 index 0000000000..664ce396aa --- /dev/null +++ b/.changeset/patch-add-safe-output-actions.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Add support for `safe-outputs.actions` to mount custom GitHub Actions as once-callable safe output tools, including compile-time action metadata resolution and generated guarded workflow steps. diff --git a/.github/aw/actions-lock.json b/.github/aw/actions-lock.json index 6ca9158024..743652c946 100644 --- a/.github/aw/actions-lock.json +++ b/.github/aw/actions-lock.json @@ -1,5 +1,10 @@ { "entries": { + "actions-ecosystem/action-add-labels@v1": { + "repo": "actions-ecosystem/action-add-labels", + "version": "v1", + "sha": "18f1af5e3544586314bbe15c0273249c770b2daf" + }, "actions/ai-inference@v2.0.7": { "repo": "actions/ai-inference", "version": "v2.0.7", diff --git a/.github/workflows/smoke-codex.lock.yml b/.github/workflows/smoke-codex.lock.yml index 264381e207..fbd43c9e6d 100644 --- a/.github/workflows/smoke-codex.lock.yml +++ b/.github/workflows/smoke-codex.lock.yml @@ -27,7 +27,7 @@ # - shared/gh.md # - shared/reporting.md # -# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"57064b51acff99a53ad08aada3ddf9626cdf25a44c87298d5df01726d62273e5","strict":true} +# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"f0c83e64ff01b5e650c92aaaf519cf17c35ab121d84d633cf4da7cb48488592c","strict":true} name: "Smoke Codex" "on": @@ -446,7 +446,22 @@ jobs: "remove_labels": " CONSTRAINTS: Only these labels can be removed: [smoke]." }, "repo_params": {}, - "dynamic_tools": [] + "dynamic_tools": [ + { + "description": "Add the 'smoked' label to the current pull request (can only be called once)", + "inputSchema": { + "additionalProperties": true, + "properties": { + "payload": { + "description": "JSON-encoded payload to pass to the action", + "type": "string" + } + }, + "type": "object" + }, + "name": "add_smoked_label" + } + ] } GH_AW_SAFE_OUTPUTS_TOOLS_META_EOF cat > ${RUNNER_TEMP}/gh-aw/safeoutputs/validation.json << 'GH_AW_SAFE_OUTPUTS_VALIDATION_EOF' @@ -1530,6 +1545,7 @@ jobs: GH_AW_ALLOWED_DOMAINS: "*.githubusercontent.com,127.0.0.1,172.30.0.1,::1,api.openai.com,api.snapcraft.io,app.renovatebot.com,appveyor.com,archive.ubuntu.com,azure.archive.ubuntu.com,badgen.net,cdn.playwright.dev,circleci.com,codacy.com,codeclimate.com,codecov.io,codeload.github.com,coveralls.io,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,deepsource.io,docs.github.com,drone.io,github-cloud.githubusercontent.com,github-cloud.s3.amazonaws.com,github.blog,github.com,github.githubassets.com,go.dev,golang.org,goproxy.io,host.docker.internal,img.shields.io,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,lfs.github.com,localhost,objects.githubusercontent.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,openai.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,pkg.go.dev,playwright.download.prss.microsoft.com,ppa.launchpad.net,proxy.golang.org,raw.githubusercontent.com,readthedocs.io,readthedocs.org,renovatebot.com,s.symcb.com,s.symcd.com,security.ubuntu.com,semaphoreci.com,shields.io,snyk.io,sonarcloud.io,sonarqube.com,storage.googleapis.com,sum.golang.org,travis-ci.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com,www.googleapis.com" GITHUB_SERVER_URL: ${{ github.server_url }} GITHUB_API_URL: ${{ github.api_url }} + GH_AW_SAFE_OUTPUT_ACTIONS: "{\"add_smoked_label\":\"add_smoked_label\"}" GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"add_comment\":{\"hide_older_comments\":true,\"max\":2},\"add_labels\":{\"allowed\":[\"smoke-codex\"]},\"create_issue\":{\"close_older_issues\":true,\"expires\":2,\"labels\":[\"automation\",\"testing\"],\"max\":1},\"hide_comment\":{\"max\":5},\"missing_data\":{},\"missing_tool\":{},\"noop\":{\"max\":1,\"report-as-issue\":\"true\"},\"remove_labels\":{\"allowed\":[\"smoke\"]},\"unassign_from_user\":{\"allowed\":[\"githubactionagent\"],\"max\":1}}" with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} @@ -1538,6 +1554,14 @@ jobs: setupGlobals(core, github, context, exec, io); const { main } = require('${{ runner.temp }}/gh-aw/actions/safe_output_handler_manager.cjs'); await main(); + - name: Add the 'smoked' label to the current pull request + id: action_add_smoked_label + if: steps.process_safe_outputs.outputs.action_add_smoked_label_payload != '' + uses: actions-ecosystem/action-add-labels@18f1af5e3544586314bbe15c0273249c770b2daf # v1 + env: + GITHUB_TOKEN: ${{ github.token }} + with: + payload: ${{ steps.process_safe_outputs.outputs.action_add_smoked_label_payload }} - name: Upload safe output items if: always() uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 diff --git a/.github/workflows/smoke-codex.md b/.github/workflows/smoke-codex.md index 2b8f27305b..cd0316b115 100644 --- a/.github/workflows/smoke-codex.md +++ b/.github/workflows/smoke-codex.md @@ -62,6 +62,12 @@ safe-outputs: run-started: "🔮 The ancient spirits stir... [{workflow_name}]({run_url}) awakens to divine this {event_type}..." run-success: "✨ The prophecy is fulfilled... [{workflow_name}]({run_url}) has completed its mystical journey. The stars align. 🌟" run-failure: "🌑 The shadows whisper... [{workflow_name}]({run_url}) {status}. The oracle requires further meditation..." + actions: + add-smoked-label: + uses: actions-ecosystem/action-add-labels@v1 + description: Add the 'smoked' label to the current pull request + env: + GITHUB_TOKEN: ${{ github.token }} timeout-minutes: 15 checkout: - fetch-depth: 2 @@ -99,6 +105,7 @@ If all tests pass: - Use the `add_labels` safe-output tool to add the label `smoke-codex` to the pull request - Use the `remove_labels` safe-output tool to remove the label `smoke` from the pull request - Use the `unassign_from_user` safe-output tool to unassign the user `githubactionagent` from the pull request (this is a fictitious user used for testing) +- Use the `add_smoked_label` safe-output action tool to add the label `smoked` to the pull request (call it with `{"labels": "smoked"}`) **Important**: If no action is needed after completing your analysis, you **MUST** call the `noop` safe-output tool with a brief explanation. Failing to call any safe-output tool is the most common cause of safe-output workflow failures. diff --git a/actions/setup/js/safe_output_action_handler.cjs b/actions/setup/js/safe_output_action_handler.cjs new file mode 100644 index 0000000000..3359e685e6 --- /dev/null +++ b/actions/setup/js/safe_output_action_handler.cjs @@ -0,0 +1,119 @@ +// @ts-check +/// + +/** + * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction + */ + +const { replaceTemporaryIdReferences } = require("./temporary_id.cjs"); +const { getErrorMessage } = require("./error_helpers.cjs"); +const { sanitizeContent } = require("./sanitize_content.cjs"); + +/** + * Internal safe-output message fields that should not be forwarded as action inputs. + * These fields are part of the safe-output messaging protocol and are not user-defined. + * Maintained as an explicit set so future protocol fields can be added without silently + * forwarding them to external action `with:` inputs. + * @type {Set} + */ +const INTERNAL_MESSAGE_FIELDS = new Set(["type"]); + +/** + * Main handler factory for a custom safe output action. + * + * Each configured safe-output action gets its own instance of this factory function, + * invoked with a config that includes `action_name` (the normalized tool name). + * + * The handler: + * 1. Enforces that the action is called at most once (per the spec). + * 2. Applies temporary ID substitutions to all string-valued fields in the payload. + * 3. Exports the processed payload as a step output named `action__payload`. + * + * The compiler generates a corresponding GitHub Actions step with: + * if: steps.process_safe_outputs.outputs.action__payload != '' + * uses: + * with: + * : ${{ fromJSON(steps.process_safe_outputs.outputs.action__payload). }} + * + * @type {HandlerFactoryFunction} + */ +async function main(config = {}) { + const actionName = config.action_name || "unknown_action"; + const outputKey = `action_${actionName}_payload`; + + core.info(`Custom action handler initialized: action_name=${actionName}, output_key=${outputKey}`); + + // Track whether this action has been called (enforces once-only constraint) + let called = false; + + /** + * Handler function that processes a single tool call for this action. + * Applies temporary ID substitutions and exports the payload as a step output. + * + * @param {Object} message - The tool call message from the agent output + * @param {Object} resolvedTemporaryIds - Map of temp IDs to resolved values (plain object) + * @param {Map} temporaryIdMap - Live map of temporary IDs (for substitution) + * @returns {Promise} Result with success/error status + */ + return async function handleCustomAction(message, resolvedTemporaryIds, temporaryIdMap = new Map()) { + // Enforce once-only constraint + if (called) { + const error = `Action "${actionName}" can only be called once per workflow run`; + core.warning(error); + return { + success: false, + error, + }; + } + called = true; + + try { + core.info(`Processing custom action: ${actionName}`); + + // Build the processed payload by: + // 1. Applying temporary ID reference substitutions to string fields + // 2. Redacting (sanitizing) all string fields via sanitizeContent() before export. + // This prevents prompt-injected content from leaking URLs, mentions, or harmful + // content into external action inputs. + const processedInputs = {}; + for (const [key, value] of Object.entries(message)) { + // Skip internal safe-output messaging fields that are not action inputs. + // Maintained as an explicit set to allow future additions without silently + // forwarding new internal fields to external action steps. + if (INTERNAL_MESSAGE_FIELDS.has(key)) { + continue; + } + + if (typeof value === "string") { + // Apply temporary ID reference substitution (e.g., "aw_abc1" → "42"), then + // sanitize to redact malicious URLs, neutralize bot-trigger phrases, and + // escape @mentions that could cause unintended notifications in the action. + const substituted = replaceTemporaryIdReferences(value, temporaryIdMap); + processedInputs[key] = sanitizeContent(substituted); + } else { + processedInputs[key] = value; + } + } + + // Export the processed payload as a step output + const payloadJSON = JSON.stringify(processedInputs); + core.setOutput(outputKey, payloadJSON); + core.info(`✓ Custom action "${actionName}": exported payload as "${outputKey}" (${payloadJSON.length} bytes)`); + + return { + success: true, + action_name: actionName, + payload: payloadJSON, + }; + } catch (error) { + const errorMessage = getErrorMessage(error); + core.error(`Failed to process custom action "${actionName}": ${errorMessage}`); + return { + success: false, + error: `Failed to process custom action "${actionName}": ${errorMessage}`, + }; + } + }; +} + +module.exports = { main }; diff --git a/actions/setup/js/safe_output_action_handler.test.cjs b/actions/setup/js/safe_output_action_handler.test.cjs new file mode 100644 index 0000000000..1b214221a2 --- /dev/null +++ b/actions/setup/js/safe_output_action_handler.test.cjs @@ -0,0 +1,117 @@ +// @ts-check + +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { main } from "./safe_output_action_handler.cjs"; + +describe("safe_output_action_handler", () => { + beforeEach(() => { + // Provide a mock global `core` matching the @actions/core API surface used by the handler. + global.core = { + info: vi.fn(), + debug: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + setOutput: vi.fn(), + setFailed: vi.fn(), + }; + }); + + describe("main() — factory", () => { + it("should return a function (the handler) when called", async () => { + const handler = await main({ action_name: "my_action" }); + expect(typeof handler).toBe("function"); + }); + + it("should use 'unknown_action' when config is not provided", async () => { + const handler = await main(); + expect(typeof handler).toBe("function"); + }); + }); + + describe("handler — basic payload export", () => { + it("should export the payload as a step output and return success", async () => { + const handler = await main({ action_name: "add_label" }); + + const message = { type: "add_label", labels: "bug" }; + const result = await handler(message, {}, new Map()); + + expect(result.success).toBe(true); + expect(result.action_name).toBe("add_label"); + + // 'type' (an INTERNAL_MESSAGE_FIELDS member) must be stripped from the exported payload + const exported = JSON.parse(global.core.setOutput.mock.calls[0][1]); + expect(exported).toHaveProperty("labels", "bug"); + expect(exported).not.toHaveProperty("type"); + }); + + it("should use the correct output key format action__payload", async () => { + const handler = await main({ action_name: "my_action" }); + await handler({ type: "my_action", key: "value" }, {}, new Map()); + + expect(global.core.setOutput).toHaveBeenCalledWith("action_my_action_payload", expect.any(String)); + }); + + it("should pass through non-string values without sanitization", async () => { + const handler = await main({ action_name: "my_action" }); + const message = { type: "my_action", count: 42, active: true }; + + await handler(message, {}, new Map()); + + const exported = JSON.parse(global.core.setOutput.mock.calls[0][1]); + expect(exported.count).toBe(42); + expect(exported.active).toBe(true); + }); + }); + + describe("handler — once-only enforcement", () => { + it("should return an error on the second call", async () => { + const handler = await main({ action_name: "my_action" }); + const message = { labels: "bug" }; + + const first = await handler(message, {}, new Map()); + expect(first.success).toBe(true); + + const second = await handler(message, {}, new Map()); + expect(second.success).toBe(false); + expect(second.error).toContain("can only be called once"); + + // setOutput should only be called once (for the first invocation) + expect(global.core.setOutput).toHaveBeenCalledTimes(1); + }); + }); + + describe("handler — INTERNAL_MESSAGE_FIELDS filtering", () => { + it("should strip 'type' from the exported payload", async () => { + const handler = await main({ action_name: "my_action" }); + await handler({ type: "my_action", title: "hello" }, {}, new Map()); + + const exported = JSON.parse(global.core.setOutput.mock.calls[0][1]); + expect(exported).not.toHaveProperty("type"); + expect(exported).toHaveProperty("title", "hello"); + }); + }); + + describe("handler — temporaryIdMap substitution", () => { + it("should substitute temporary ID references in string values", async () => { + const handler = await main({ action_name: "my_action" }); + // The temporary ID pattern is "#aw_XXXX" (3-12 alphanumeric chars with #aw_ prefix) + const temporaryIdMap = new Map([["aw_abc1", { repo: "owner/repo", number: 99 }]]); + await handler({ type: "my_action", body: "Fixes #aw_abc1" }, {}, temporaryIdMap); + + const exported = JSON.parse(global.core.setOutput.mock.calls[0][1]); + // replaceTemporaryIdReferences should replace '#aw_abc1' with the issue reference + expect(exported.body).not.toContain("#aw_abc1"); + }); + }); + + describe("handler — empty payload", () => { + it("should handle an empty message (only internal fields)", async () => { + const handler = await main({ action_name: "my_action" }); + const result = await handler({ type: "my_action" }, {}, new Map()); + + expect(result.success).toBe(true); + const exported = JSON.parse(global.core.setOutput.mock.calls[0][1]); + expect(exported).toEqual({}); + }); + }); +}); diff --git a/actions/setup/js/safe_output_handler_manager.cjs b/actions/setup/js/safe_output_handler_manager.cjs index b7a4fea575..e3a696e814 100644 --- a/actions/setup/js/safe_output_handler_manager.cjs +++ b/actions/setup/js/safe_output_handler_manager.cjs @@ -20,7 +20,7 @@ const { getIssuesToAssignCopilot } = require("./create_issue.cjs"); const { createReviewBuffer } = require("./pr_review_buffer.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { createManifestLogger, ensureManifestExists, extractCreatedItemFromResult } = require("./safe_output_manifest.cjs"); -const { loadCustomSafeOutputJobTypes, loadCustomSafeOutputScriptHandlers } = require("./safe_output_helpers.cjs"); +const { loadCustomSafeOutputJobTypes, loadCustomSafeOutputScriptHandlers, loadCustomSafeOutputActionHandlers } = require("./safe_output_helpers.cjs"); const { emitSafeOutputActionOutputs } = require("./safe_outputs_action_outputs.cjs"); /** @@ -194,6 +194,35 @@ async function loadHandlers(config, prReviewBuffer) { } } + // Load custom action handlers from GH_AW_SAFE_OUTPUT_ACTIONS + // These are GitHub Actions configured in safe-outputs.actions. The handler applies + // temporary ID substitutions to the payload and exports `action__payload` outputs + // that compiler-generated `uses:` steps consume. + const customActionHandlers = loadCustomSafeOutputActionHandlers(); + if (customActionHandlers.size > 0) { + core.info(`Loading ${customActionHandlers.size} custom action handler(s): ${[...customActionHandlers.keys()].join(", ")}`); + const actionHandlerPath = require("path").join(__dirname, "safe_output_action_handler.cjs"); + for (const [actionType, actionName] of customActionHandlers) { + try { + const actionModule = require(actionHandlerPath); + if (actionModule && typeof actionModule.main === "function") { + const handlerConfig = { action_name: actionName, ...(config[actionType] || {}) }; + const messageHandler = await actionModule.main(handlerConfig); + if (typeof messageHandler !== "function") { + core.warning(`✗ Custom action handler ${actionType} main() did not return a function (got ${typeof messageHandler}) — this handler will be skipped`); + } else { + messageHandlers.set(actionType, messageHandler); + core.info(`✓ Loaded and initialized custom action handler for: ${actionType}`); + } + } else { + core.warning(`Custom action handler module does not export a main function — skipping ${actionType}`); + } + } catch (error) { + core.warning(`Failed to load custom action handler for ${actionType}: ${getErrorMessage(error)} — this handler will be skipped`); + } + } + } + core.info(`Loaded ${messageHandlers.size} handler(s)`); return messageHandlers; } diff --git a/actions/setup/js/safe_output_helpers.cjs b/actions/setup/js/safe_output_helpers.cjs index 6fe9dc01c3..47967926ee 100644 --- a/actions/setup/js/safe_output_helpers.cjs +++ b/actions/setup/js/safe_output_helpers.cjs @@ -6,6 +6,9 @@ * Provides common validation and target resolution logic */ +const { getErrorMessage } = require("./error_helpers.cjs"); +const { matchesSimpleGlob } = require("./glob_pattern_helpers.cjs"); + /** * Parse a comma-separated list of allowed items from environment variable * @param {string|undefined} envValue - Environment variable value @@ -250,7 +253,6 @@ function loadCustomSafeOutputJobTypes() { return new Set(jobTypes); } catch (error) { if (typeof core !== "undefined") { - const { getErrorMessage } = require("./error_helpers.cjs"); core.warning(`Failed to parse GH_AW_SAFE_OUTPUT_JOBS: ${getErrorMessage(error)}`); } return new Set(); @@ -317,7 +319,6 @@ function extractAssignees(message) { * @returns {boolean} True if username matches the blocked pattern */ function matchesBlockedPattern(username, pattern) { - const { matchesSimpleGlob } = require("./glob_pattern_helpers.cjs"); return matchesSimpleGlob(username, pattern); } @@ -356,19 +357,49 @@ function loadCustomSafeOutputScriptHandlers() { return scriptHandlers; } catch (error) { if (typeof core !== "undefined") { - const { getErrorMessage } = require("./error_helpers.cjs"); core.warning(`Failed to parse GH_AW_SAFE_OUTPUT_SCRIPTS: ${getErrorMessage(error)}`); } return new Map(); } } +/** + * Load custom safe output action handlers from environment variable. + * These are GitHub Actions configured in safe-outputs.actions that are processed + * by compiler-injected `uses:` steps after the handler manager exports their payloads. + * The handler manager processes the tool call, applies temporary ID substitutions, + * and exports `action__payload` outputs that the injected steps consume. + * @returns {Map} Map of normalized action type name to action name (for handler config) + */ +function loadCustomSafeOutputActionHandlers() { + const safeOutputActionsEnv = process.env.GH_AW_SAFE_OUTPUT_ACTIONS; + if (!safeOutputActionsEnv) { + return new Map(); + } + + try { + const safeOutputActions = JSON.parse(safeOutputActionsEnv); + // The environment variable is a map of normalized action names to themselves + const actionHandlers = new Map(Object.entries(safeOutputActions)); + if (typeof core !== "undefined") { + core.debug(`Loaded ${actionHandlers.size} custom safe output action handler(s): ${[...actionHandlers.keys()].join(", ")}`); + } + return actionHandlers; + } catch (error) { + if (typeof core !== "undefined") { + core.warning(`Failed to parse GH_AW_SAFE_OUTPUT_ACTIONS: ${getErrorMessage(error)}`); + } + return new Map(); + } +} + module.exports = { parseAllowedItems, parseMaxCount, resolveTarget, loadCustomSafeOutputJobTypes, loadCustomSafeOutputScriptHandlers, + loadCustomSafeOutputActionHandlers, resolveIssueNumber, extractAssignees, matchesBlockedPattern, diff --git a/actions/setup/js/safe_output_helpers.test.cjs b/actions/setup/js/safe_output_helpers.test.cjs index 6cc63b2291..9e68fe46f9 100644 --- a/actions/setup/js/safe_output_helpers.test.cjs +++ b/actions/setup/js/safe_output_helpers.test.cjs @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach } from "vitest"; +import { describe, it, expect, beforeEach, vi } from "vitest"; describe("safe_output_helpers", () => { let helpers; @@ -658,4 +658,49 @@ describe("safe_output_helpers", () => { } }); }); + + describe("loadCustomSafeOutputActionHandlers", () => { + beforeEach(() => { + delete process.env.GH_AW_SAFE_OUTPUT_ACTIONS; + global.core = { + info: vi.fn(), + debug: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + }; + }); + + it("should return empty map when GH_AW_SAFE_OUTPUT_ACTIONS is not set", () => { + const result = helpers.loadCustomSafeOutputActionHandlers(); + expect(result).toBeInstanceOf(Map); + expect(result.size).toBe(0); + }); + + it("should parse and return action handlers map from env var", () => { + process.env.GH_AW_SAFE_OUTPUT_ACTIONS = JSON.stringify({ + add_smoked_label: "add_smoked_label", + notify_slack: "notify_slack", + }); + + const result = helpers.loadCustomSafeOutputActionHandlers(); + expect(result).toBeInstanceOf(Map); + expect(result.size).toBe(2); + expect(result.get("add_smoked_label")).toBe("add_smoked_label"); + expect(result.get("notify_slack")).toBe("notify_slack"); + }); + + it("should return empty map when env var contains invalid JSON", () => { + process.env.GH_AW_SAFE_OUTPUT_ACTIONS = "not-valid-json"; + const result = helpers.loadCustomSafeOutputActionHandlers(); + expect(result).toBeInstanceOf(Map); + expect(result.size).toBe(0); + }); + + it("should return empty map when env var contains empty object", () => { + process.env.GH_AW_SAFE_OUTPUT_ACTIONS = JSON.stringify({}); + const result = helpers.loadCustomSafeOutputActionHandlers(); + expect(result).toBeInstanceOf(Map); + expect(result.size).toBe(0); + }); + }); }); diff --git a/actions/setup/js/types/handler-factory.d.ts b/actions/setup/js/types/handler-factory.d.ts index b40b336a27..848817b2a2 100644 --- a/actions/setup/js/types/handler-factory.d.ts +++ b/actions/setup/js/types/handler-factory.d.ts @@ -67,9 +67,10 @@ type HandlerResult = HandlerSuccessResult | HandlerErrorResult; * * @param message - The safe output message to process * @param resolvedTemporaryIds - Map of temporary IDs that have been resolved to actual issue/PR/discussion numbers + * @param temporaryIdMap - Live Map of temporary IDs for in-flight substitutions (may be used by action handlers) * @returns Promise resolving to result with success status and details */ -type MessageHandlerFunction = (message: any, resolvedTemporaryIds: ResolvedTemporaryIds) => Promise; +type MessageHandlerFunction = (message: any, resolvedTemporaryIds: ResolvedTemporaryIds, temporaryIdMap?: Map) => Promise; /** * Main factory function signature for safe output handlers diff --git a/go.mod b/go.mod index eebfb2ef10..0972db58ec 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/sourcegraph/conc v0.3.0 github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.11.1 + go.yaml.in/yaml/v3 v3.0.4 golang.org/x/crypto v0.48.0 golang.org/x/mod v0.33.0 golang.org/x/term v0.40.0 @@ -102,7 +103,6 @@ require ( go.opentelemetry.io/otel/metric v1.39.0 // indirect go.opentelemetry.io/otel/trace v1.39.0 // indirect go.uber.org/multierr v1.11.0 // indirect - go.yaml.in/yaml/v3 v3.0.4 // indirect go.yaml.in/yaml/v4 v4.0.0-rc.4 // indirect golang.org/x/exp v0.0.0-20240909161429-701f63a606c0 // indirect golang.org/x/net v0.51.0 // indirect diff --git a/pkg/cli/compile_integration_test.go b/pkg/cli/compile_integration_test.go index b38477598f..659ea65eea 100644 --- a/pkg/cli/compile_integration_test.go +++ b/pkg/cli/compile_integration_test.go @@ -1127,3 +1127,270 @@ Test workflow to verify actions-lock.json path handling when compiling from subd t.Logf("Integration test passed - actions-lock.json created at correct location") } + +// TestCompileSafeOutputsActions verifies that a workflow with safe-outputs.actions +// compiles successfully and produces the expected output in the lock file: +// - GH_AW_SAFE_OUTPUT_ACTIONS env var on the process_safe_outputs step +// - An injected action step with the correct id and if-condition +func TestCompileSafeOutputsActions(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + testWorkflow := `--- +name: Test Safe Output Actions +on: + workflow_dispatch: +permissions: + contents: read + pull-requests: read +engine: copilot +safe-outputs: + actions: + add-label: + uses: actions-ecosystem/action-add-labels@v1 + description: Add a label to the current PR + env: + GITHUB_TOKEN: ${{ github.token }} +--- + +# Test Safe Output Actions + +When done, call add_label with the appropriate label. +` + testWorkflowPath := filepath.Join(setup.workflowsDir, "test-safe-output-actions.md") + if err := os.WriteFile(testWorkflowPath, []byte(testWorkflow), 0644); err != nil { + t.Fatalf("Failed to write test workflow file: %v", err) + } + + cmd := exec.Command(setup.binaryPath, "compile", testWorkflowPath) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output)) + } + + lockFilePath := filepath.Join(setup.workflowsDir, "test-safe-output-actions.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContentStr := string(lockContent) + + // Verify GH_AW_SAFE_OUTPUT_ACTIONS is emitted on the process_safe_outputs step + if !strings.Contains(lockContentStr, "GH_AW_SAFE_OUTPUT_ACTIONS") { + t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUT_ACTIONS\nLock file content:\n%s", lockContentStr) + } + + // Verify the injected action step id + if !strings.Contains(lockContentStr, "id: action_add_label") { + t.Errorf("Lock file should contain step 'id: action_add_label'\nLock file content:\n%s", lockContentStr) + } + + // Verify the injected action step if-condition + if !strings.Contains(lockContentStr, "action_add_label_payload") { + t.Errorf("Lock file should contain 'action_add_label_payload' in the step if-condition\nLock file content:\n%s", lockContentStr) + } + + // Verify the env block is included in the action step + if !strings.Contains(lockContentStr, "GITHUB_TOKEN") { + t.Errorf("Lock file should contain GITHUB_TOKEN env var in the action step\nLock file content:\n%s", lockContentStr) + } + + // Verify the handler manager step is present (required to process action payloads) + if !strings.Contains(lockContentStr, "safe_output_handler_manager.cjs") { + t.Errorf("Lock file should contain the safe_output_handler_manager.cjs step\nLock file content:\n%s", lockContentStr) + } +} + +// TestCompileSafeOutputsActionsMultiple verifies that multiple actions in safe-outputs.actions +// all generate separate action steps and all appear in GH_AW_SAFE_OUTPUT_ACTIONS. +func TestCompileSafeOutputsActionsMultiple(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + testWorkflow := `--- +name: Test Multiple Safe Output Actions +on: + workflow_dispatch: +permissions: + contents: read + issues: read + pull-requests: read +engine: copilot +safe-outputs: + actions: + add-bug-label: + uses: actions-ecosystem/action-add-labels@v1 + description: Add a bug label + env: + GITHUB_TOKEN: ${{ github.token }} + close-issue: + uses: peter-evans/close-issue@v3 + description: Close the issue +--- + +# Test Multiple Safe Output Actions + +Call add_bug_label or close_issue as appropriate. +` + testWorkflowPath := filepath.Join(setup.workflowsDir, "test-multi-safe-output-actions.md") + if err := os.WriteFile(testWorkflowPath, []byte(testWorkflow), 0644); err != nil { + t.Fatalf("Failed to write test workflow file: %v", err) + } + + cmd := exec.Command(setup.binaryPath, "compile", testWorkflowPath) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output)) + } + + lockFilePath := filepath.Join(setup.workflowsDir, "test-multi-safe-output-actions.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContentStr := string(lockContent) + + // Both action steps must be present + if !strings.Contains(lockContentStr, "id: action_add_bug_label") { + t.Errorf("Lock file should contain step 'id: action_add_bug_label'\nLock file content:\n%s", lockContentStr) + } + if !strings.Contains(lockContentStr, "id: action_close_issue") { + t.Errorf("Lock file should contain step 'id: action_close_issue'\nLock file content:\n%s", lockContentStr) + } + + // Both payloads must appear in if-conditions + if !strings.Contains(lockContentStr, "action_add_bug_label_payload") { + t.Errorf("Lock file should contain 'action_add_bug_label_payload'\nLock file content:\n%s", lockContentStr) + } + if !strings.Contains(lockContentStr, "action_close_issue_payload") { + t.Errorf("Lock file should contain 'action_close_issue_payload'\nLock file content:\n%s", lockContentStr) + } + + // GH_AW_SAFE_OUTPUT_ACTIONS must mention both tools + if !strings.Contains(lockContentStr, "add_bug_label") { + t.Errorf("Lock file should contain 'add_bug_label' in GH_AW_SAFE_OUTPUT_ACTIONS\nLock file content:\n%s", lockContentStr) + } + if !strings.Contains(lockContentStr, "close_issue") { + t.Errorf("Lock file should contain 'close_issue' in GH_AW_SAFE_OUTPUT_ACTIONS\nLock file content:\n%s", lockContentStr) + } +} + +// TestCompileSafeOutputsActionsCombinedWithBuiltin verifies that safe-outputs.actions +// can be used alongside built-in safe-output handlers (add-comment, create-issue, etc.) +// without compilation errors. +func TestCompileSafeOutputsActionsCombinedWithBuiltin(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + testWorkflow := `--- +name: Combined Safe Outputs +on: + workflow_dispatch: +permissions: + contents: read + issues: read + pull-requests: read +engine: copilot +safe-outputs: + add-comment: + max: 1 + add-labels: + allowed: [bug, enhancement] + actions: + apply-fix: + uses: actions-ecosystem/action-add-labels@v1 + description: Apply fix label + env: + GITHUB_TOKEN: ${{ github.token }} +--- + +# Combined Safe Outputs + +Use add_comment, add_labels, or apply_fix as appropriate. +` + testWorkflowPath := filepath.Join(setup.workflowsDir, "test-combined-safe-outputs.md") + if err := os.WriteFile(testWorkflowPath, []byte(testWorkflow), 0644); err != nil { + t.Fatalf("Failed to write test workflow file: %v", err) + } + + cmd := exec.Command(setup.binaryPath, "compile", testWorkflowPath) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output)) + } + + lockFilePath := filepath.Join(setup.workflowsDir, "test-combined-safe-outputs.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContentStr := string(lockContent) + + // Verify both built-in and action tools are present + if !strings.Contains(lockContentStr, "GH_AW_SAFE_OUTPUT_ACTIONS") { + t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUT_ACTIONS\nLock file content:\n%s", lockContentStr) + } + if !strings.Contains(lockContentStr, "id: action_apply_fix") { + t.Errorf("Lock file should contain step 'id: action_apply_fix'\nLock file content:\n%s", lockContentStr) + } + // Verify built-in handler config is still present + if !strings.Contains(lockContentStr, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") { + t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG\nLock file content:\n%s", lockContentStr) + } +} + +// TestCompileSafeOutputsActionsOnlyNoBuiltin verifies that a workflow with only +// safe-outputs.actions (no built-in handlers) still compiles correctly and emits +// the safe_outputs job. +func TestCompileSafeOutputsActionsOnlyNoBuiltin(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + testWorkflow := `--- +name: Actions Only Safe Outputs +on: + workflow_dispatch: +permissions: + contents: read + pull-requests: read +engine: copilot +safe-outputs: + actions: + pin-pr: + uses: actions-ecosystem/action-add-labels@v1 + description: Pin the PR +--- + +# Actions Only Safe Outputs + +Call pin_pr to pin the pull request. +` + testWorkflowPath := filepath.Join(setup.workflowsDir, "test-actions-only-safe-outputs.md") + if err := os.WriteFile(testWorkflowPath, []byte(testWorkflow), 0644); err != nil { + t.Fatalf("Failed to write test workflow file: %v", err) + } + + cmd := exec.Command(setup.binaryPath, "compile", testWorkflowPath) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output)) + } + + lockFilePath := filepath.Join(setup.workflowsDir, "test-actions-only-safe-outputs.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContentStr := string(lockContent) + + // Verify the safe_outputs job is created + if !strings.Contains(lockContentStr, "safe_outputs") { + t.Errorf("Lock file should contain a 'safe_outputs' job\nLock file content:\n%s", lockContentStr) + } + if !strings.Contains(lockContentStr, "GH_AW_SAFE_OUTPUT_ACTIONS") { + t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUT_ACTIONS\nLock file content:\n%s", lockContentStr) + } + if !strings.Contains(lockContentStr, "id: action_pin_pr") { + t.Errorf("Lock file should contain step 'id: action_pin_pr'\nLock file content:\n%s", lockContentStr) + } +} diff --git a/pkg/cli/workflows/test-copilot-safe-output-actions.md b/pkg/cli/workflows/test-copilot-safe-output-actions.md new file mode 100644 index 0000000000..87c1dc8a46 --- /dev/null +++ b/pkg/cli/workflows/test-copilot-safe-output-actions.md @@ -0,0 +1,23 @@ +--- +on: + workflow_dispatch: +permissions: + contents: read + pull-requests: read +engine: copilot +safe-outputs: + actions: + add-smoked-label: + uses: actions-ecosystem/action-add-labels@v1 + description: Add the 'smoked' label to the current pull request + env: + GITHUB_TOKEN: ${{ github.token }} +--- + +# Test Safe Output Actions + +This workflow demonstrates `safe-outputs.actions`, which mounts a GitHub Action +as a once-callable MCP tool. + +When done, call `add_smoked_label` with `{"labels": "smoked"}` to add the label +to the current pull request. diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 4516461853..8e323c1a5d 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -7833,6 +7833,37 @@ } ] ] + }, + "actions": { + "type": "object", + "description": "Custom GitHub Actions to mount as once-callable MCP tools. Each action is resolved at compile time to derive its input schema from action.yml, and a guarded `uses:` step is injected in the safe_outputs job. Action names containing dashes will be automatically normalized to underscores (e.g., 'add-smoked-label' becomes 'add_smoked_label').", + "patternProperties": { + "^[a-zA-Z_][a-zA-Z0-9_-]*$": { + "type": "object", + "description": "Configuration for a custom safe output action.", + "properties": { + "uses": { + "type": "string", + "description": "The GitHub Action to use. Supports owner/repo@ref, owner/repo/subdir@ref, or ./local/path.", + "examples": ["actions-ecosystem/action-add-labels@v1", "owner/repo@v1", "owner/repo/subdir@v1"] + }, + "description": { + "type": "string", + "description": "Optional description override for the MCP tool. Defaults to the action's description from action.yml." + }, + "env": { + "type": "object", + "description": "Additional environment variables to set on the injected action step. Useful for passing secrets like GITHUB_TOKEN.", + "additionalProperties": { + "type": "string" + }, + "examples": [{ "GITHUB_TOKEN": "${{ github.token }}" }] + } + }, + "required": ["uses"], + "additionalProperties": false + } + } } }, "additionalProperties": false diff --git a/pkg/workflow/compiler_jobs.go b/pkg/workflow/compiler_jobs.go index 88a35cb1f7..160d0285b2 100644 --- a/pkg/workflow/compiler_jobs.go +++ b/pkg/workflow/compiler_jobs.go @@ -198,6 +198,14 @@ func (c *Compiler) buildJobs(data *WorkflowData, markdownPath string) error { // Extract lock filename for timestamp check lockFilename := filepath.Base(stringutil.MarkdownToLockFile(markdownPath)) + // Resolve custom safe-output actions early so that tool schemas (derived from action.yml) + // are available when buildMainJobWrapper → generateMCPSetup → generateToolsMetaJSON → + // generateDynamicTools runs. Without this early resolution the dynamic_tools entry for + // each action tool would have an empty schema because Inputs/ActionDescription are nil. + if data.SafeOutputs != nil && len(data.SafeOutputs.Actions) > 0 { + c.resolveAllActions(data, markdownPath) + } + // Build pre-activation and activation jobs _, activationJobCreated, err := c.buildPreActivationAndActivationJobs(data, frontmatter, lockFilename) if err != nil { diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index 08eb2b577f..147fe78ca0 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -149,7 +149,8 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa data.SafeOutputs.AutofixCodeScanningAlert != nil || data.SafeOutputs.MissingTool != nil || data.SafeOutputs.MissingData != nil || - len(data.SafeOutputs.Scripts) > 0 // Custom scripts run in the handler loop + len(data.SafeOutputs.Scripts) > 0 || // Custom scripts run in the handler loop + len(data.SafeOutputs.Actions) > 0 // Custom actions need handler to export their payloads // Note: All project-related operations are now handled by the unified handler. // The project handler manager has been removed. @@ -222,6 +223,26 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa outputs["create_agent_session_session_url"] = "${{ steps.create_agent_session.outputs.session_url }}" } + // 5. Custom action steps — compiler-generated steps for each configured safe-output action. + // These steps run after the handler manager, which processes the agent payload and exports + // a JSON payload output for each action tool call. Each step is guarded by an `if:` condition + // that checks whether the handler manager exported a payload for this action. + if len(data.SafeOutputs.Actions) > 0 { + // resolveAllActions was already called early in buildJobs (before generateToolsMetaJSON) + // so action configs already have Inputs/ActionDescription populated. We only call it + // again here as a safety net in case compileSafeOutputsJob is called independently. + c.resolveAllActions(data, markdownPath) + + actionStepYAML := c.buildActionSteps(data) + steps = append(steps, actionStepYAML...) + + // Register each action as having a handler manager output + for actionName := range data.SafeOutputs.Actions { + normalizedName := stringutil.NormalizeSafeOutputIdentifier(actionName) + safeOutputStepNames = append(safeOutputStepNames, "action_"+normalizedName) + } + } + // The outputs and permissions are configured in the handler manager section above if data.SafeOutputs.AddReviewer != nil { outputs["add_reviewer_reviewers_added"] = "${{ steps.process_safe_outputs.outputs.reviewers_added }}" diff --git a/pkg/workflow/compiler_safe_outputs_steps.go b/pkg/workflow/compiler_safe_outputs_steps.go index 438360066d..3ebd872716 100644 --- a/pkg/workflow/compiler_safe_outputs_steps.go +++ b/pkg/workflow/compiler_safe_outputs_steps.go @@ -349,6 +349,13 @@ func (c *Compiler) buildHandlerManagerStep(data *WorkflowData) []string { consolidatedSafeOutputsStepsLog.Print("Added GH_AW_SAFE_OUTPUT_SCRIPTS env var for custom script handlers") } + // Add GH_AW_SAFE_OUTPUT_ACTIONS so the handler manager can load custom action handlers. + // The env var maps normalized action names to themselves (reserved for future extensibility). + if customActionsJSON := buildCustomSafeOutputActionsJSON(data); customActionsJSON != "" { + steps = append(steps, fmt.Sprintf(" GH_AW_SAFE_OUTPUT_ACTIONS: %q\n", customActionsJSON)) + consolidatedSafeOutputsStepsLog.Print("Added GH_AW_SAFE_OUTPUT_ACTIONS env var for custom action handlers") + } + // Add custom safe output env vars c.addCustomSafeOutputEnvVars(&steps, data) diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index 4071614425..7cf92efc34 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -503,6 +503,7 @@ type SafeOutputsConfig struct { IDToken *string `yaml:"id-token,omitempty"` // Override id-token permission: "write" to force-add, "none" to disable auto-detection ConcurrencyGroup string `yaml:"concurrency-group,omitempty"` // Concurrency group for the safe-outputs job (cancel-in-progress is always false) Environment string `yaml:"environment,omitempty"` // Override the GitHub deployment environment for the safe-outputs job (defaults to the top-level environment: field) + Actions map[string]*SafeOutputActionConfig `yaml:"actions,omitempty"` // Custom GitHub Actions mounted as safe output tools (resolved at compile time) AutoInjectedCreateIssue bool `yaml:"-"` // Internal: true when create-issues was automatically injected by the compiler (not user-configured) } diff --git a/pkg/workflow/data/action_pins.json b/pkg/workflow/data/action_pins.json index 6ca9158024..743652c946 100644 --- a/pkg/workflow/data/action_pins.json +++ b/pkg/workflow/data/action_pins.json @@ -1,5 +1,10 @@ { "entries": { + "actions-ecosystem/action-add-labels@v1": { + "repo": "actions-ecosystem/action-add-labels", + "version": "v1", + "sha": "18f1af5e3544586314bbe15c0273249c770b2daf" + }, "actions/ai-inference@v2.0.7": { "repo": "actions/ai-inference", "version": "v2.0.7", diff --git a/pkg/workflow/safe_outputs_actions.go b/pkg/workflow/safe_outputs_actions.go new file mode 100644 index 0000000000..9f76c3881c --- /dev/null +++ b/pkg/workflow/safe_outputs_actions.go @@ -0,0 +1,545 @@ +package workflow + +import ( + "context" + "encoding/base64" + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/stringutil" + "go.yaml.in/yaml/v3" +) + +var safeOutputActionsLog = logger.New("workflow:safe_outputs_actions") + +// SafeOutputActionConfig holds configuration for a single custom safe output action. +// Each configured action is resolved at compile time to get its inputs from action.yml, +// and is mounted as an MCP tool that the AI agent can call once per workflow run. +type SafeOutputActionConfig struct { + Uses string `yaml:"uses"` + Description string `yaml:"description,omitempty"` // optional override of the action's description + Env map[string]string `yaml:"env,omitempty"` // additional environment variables for the injected step + + // Computed at compile time (not from frontmatter): + ResolvedRef string `yaml:"-"` // Pinned action reference (e.g., "owner/repo@sha # v1") + Inputs map[string]*ActionYAMLInput `yaml:"-"` // Inputs parsed from action.yml + ActionDescription string `yaml:"-"` // Description from action.yml +} + +// ActionYAMLInput holds an input definition parsed from a GitHub Action's action.yml. +type ActionYAMLInput struct { + Description string `yaml:"description,omitempty"` + Required bool `yaml:"required,omitempty"` + Default string `yaml:"default,omitempty"` +} + +// actionYAMLFile is the parsed structure of a GitHub Action's action.yml. +type actionYAMLFile struct { + Name string `yaml:"name"` + Description string `yaml:"description"` + Inputs map[string]*ActionYAMLInput `yaml:"inputs"` +} + +// actionRef holds the parsed components of a GitHub Action `uses` field. +type actionRef struct { + // Repo is the GitHub repository slug (e.g., "owner/repo"). + Repo string + // Subdir is the sub-directory within the repository (e.g., "path/to/action"). + // Empty string means the action.yml is at the repository root. + Subdir string + // Ref is the git ref (tag, SHA, or branch) to checkout (e.g., "v1", "main"). + Ref string + // IsLocal is true when the `uses` value is a local path (e.g., "./path/to/action"). + IsLocal bool + // LocalPath is the filesystem path (only set when IsLocal is true). + LocalPath string +} + +// parseActionsConfig parses the safe-outputs.actions section from a raw frontmatter map. +// It returns a map of action names to their configurations. +func parseActionsConfig(actionsMap map[string]any) map[string]*SafeOutputActionConfig { + if actionsMap == nil { + return nil + } + + result := make(map[string]*SafeOutputActionConfig) + for actionName, actionValue := range actionsMap { + actionConfigMap, ok := actionValue.(map[string]any) + if !ok { + safeOutputActionsLog.Printf("Warning: action %q config is not a map, skipping", actionName) + continue + } + + actionConfig := &SafeOutputActionConfig{} + + if uses, ok := actionConfigMap["uses"].(string); ok { + actionConfig.Uses = uses + } + if description, ok := actionConfigMap["description"].(string); ok { + actionConfig.Description = description + } + if envMap, ok := actionConfigMap["env"].(map[string]any); ok { + actionConfig.Env = make(map[string]string, len(envMap)) + for k, v := range envMap { + if vStr, ok := v.(string); ok { + actionConfig.Env[k] = vStr + } + } + } + + if actionConfig.Uses == "" { + safeOutputActionsLog.Printf("Warning: action %q is missing required 'uses' field, skipping", actionName) + continue + } + + result[actionName] = actionConfig + } + + return result +} + +// parseActionUsesField parses a GitHub Action `uses` field into its components. +// Supported formats: +// - "owner/repo@ref" -> repo root action +// - "owner/repo/subdir@ref" -> sub-directory action +// - "./local/path" -> local filesystem action +func parseActionUsesField(uses string) (*actionRef, error) { + if strings.HasPrefix(uses, "./") || strings.HasPrefix(uses, "../") { + return &actionRef{IsLocal: true, LocalPath: uses}, nil + } + + // External action: split on "@" to get ref + atIdx := strings.LastIndex(uses, "@") + if atIdx < 0 { + return nil, fmt.Errorf("invalid action ref %q: missing @ref suffix", uses) + } + + refStr := uses[atIdx+1:] + repoAndPath := uses[:atIdx] + + // Split repo from subdir: first two path segments are owner/repo + parts := strings.SplitN(repoAndPath, "/", 3) + if len(parts) < 2 { + return nil, fmt.Errorf("invalid action ref %q: expected owner/repo format", uses) + } + + repo := parts[0] + "/" + parts[1] + var subdir string + if len(parts) == 3 { + subdir = parts[2] + } + + return &actionRef{ + Repo: repo, + Subdir: subdir, + Ref: refStr, + }, nil +} + +// fetchAndParseActionYAML resolves the inputs and description from the action.yml +// for each configured action. Results are stored in the action config's computed fields. +// This function should be called before tool generation and step generation. +func (c *Compiler) fetchAndParseActionYAML(actionName string, config *SafeOutputActionConfig, markdownPath string, data *WorkflowData) { + if config.Uses == "" { + return + } + + ref, err := parseActionUsesField(config.Uses) + if err != nil { + safeOutputActionsLog.Printf("Warning: failed to parse uses field %q for action %q: %v", config.Uses, actionName, err) + return + } + + var actionYAML *actionYAMLFile + var resolvedRef string + + if ref.IsLocal { + actionYAML, err = readLocalActionYAML(ref.LocalPath, markdownPath) + if err != nil { + safeOutputActionsLog.Printf("Warning: failed to read local action.yml for %q at %s: %v", actionName, ref.LocalPath, err) + } + resolvedRef = config.Uses // local paths stay as-is + } else { + // Pin the action ref and fetch the action.yml + pinned, pinErr := GetActionPinWithData(ref.Repo, ref.Ref, data) + var fetchRef string + if pinErr != nil { + safeOutputActionsLog.Printf("Warning: failed to pin action %q (%s@%s): %v", actionName, ref.Repo, ref.Ref, pinErr) + // Fall back to using the original ref + resolvedRef = config.Uses + fetchRef = ref.Ref + } else { + resolvedRef = pinned + // Extract the pinned SHA from the reference (format: "repo@sha # tag") + // and use it to fetch action.yml so the schema matches the exact pinned version. + if sha := extractSHAFromPinnedRef(pinned); sha != "" { + fetchRef = sha + } else { + fetchRef = ref.Ref + } + } + + actionYAML, err = fetchRemoteActionYAML(ref.Repo, ref.Subdir, fetchRef) + if err != nil { + safeOutputActionsLog.Printf("Warning: failed to fetch action.yml for %q (%s): %v", actionName, config.Uses, err) + } + } + + config.ResolvedRef = resolvedRef + + if actionYAML != nil { + config.Inputs = actionYAML.Inputs + config.ActionDescription = actionYAML.Description + } +} + +// extractSHAFromPinnedRef parses the SHA from a pinned action reference string. +// The format produced by formatActionReference is "repo@sha # version". +// Returns the SHA string, or empty string if it cannot be parsed. +func extractSHAFromPinnedRef(pinned string) string { + // Find the @ separator between repo and sha + _, afterAt, found := strings.Cut(pinned, "@") + if !found { + return "" + } + // Strip the "# version" comment + if commentIdx := strings.Index(afterAt, " #"); commentIdx >= 0 { + afterAt = strings.TrimSpace(afterAt[:commentIdx]) + } + // Validate it looks like a full SHA (40 hex chars) + if isValidFullSHA(afterAt) { + return afterAt + } + return "" +} + +// fetchRemoteActionYAML fetches and parses action.yml from a GitHub repository. +// It tries both action.yml and action.yaml filenames. +func fetchRemoteActionYAML(repo, subdir, ref string) (*actionYAMLFile, error) { + for _, filename := range []string{"action.yml", "action.yaml"} { + var contentPath string + if subdir != "" { + contentPath = subdir + "/" + filename + } else { + contentPath = filename + } + + apiPath := fmt.Sprintf("/repos/%s/contents/%s?ref=%s", repo, contentPath, ref) + safeOutputActionsLog.Printf("Fetching action YAML from: %s", apiPath) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + cmd := ExecGHContext(ctx, "api", apiPath, "--jq", ".content") + output, err := cmd.Output() + cancel() + if err != nil { + safeOutputActionsLog.Printf("Failed to fetch %s from %s@%s: %v", filename, repo, ref, err) + continue + } + + // GitHub API returns base64-encoded content with embedded newlines (line-wrapping every ~76 chars). + // The `gh api --jq .content` output is a raw string value (no surrounding quotes). + // We strip all whitespace (newlines and spaces) from the base64 string before decoding. + b64Content := strings.Map(func(r rune) rune { + if r == '\n' || r == '\r' || r == ' ' { + return -1 // remove character + } + return r + }, strings.TrimSpace(string(output))) + decoded, decErr := base64.StdEncoding.DecodeString(b64Content) + if decErr != nil { + safeOutputActionsLog.Printf("Failed to decode content for %s: %v", contentPath, decErr) + continue + } + + actionYAML, parseErr := parseActionYAMLContent(decoded) + if parseErr != nil { + safeOutputActionsLog.Printf("Failed to parse %s: %v", contentPath, parseErr) + continue + } + + return actionYAML, nil + } + + return nil, fmt.Errorf("could not find action.yml or action.yaml in %s@%s (subdir=%q)", repo, ref, subdir) +} + +// readLocalActionYAML reads and parses a local action.yml file. +func readLocalActionYAML(localPath, markdownPath string) (*actionYAMLFile, error) { + baseDir := filepath.Dir(markdownPath) + + // Strip leading "./" from the local path + cleanPath := strings.TrimPrefix(localPath, "./") + actionDir := filepath.Join(baseDir, cleanPath) + + for _, filename := range []string{"action.yml", "action.yaml"} { + fullPath := filepath.Join(actionDir, filename) + content, err := os.ReadFile(fullPath) + if err != nil { + continue + } + return parseActionYAMLContent(content) + } + + return nil, fmt.Errorf("could not find action.yml or action.yaml at %s", actionDir) +} + +// parseActionYAMLContent parses raw action.yml YAML content. +func parseActionYAMLContent(content []byte) (*actionYAMLFile, error) { + var parsed actionYAMLFile + if err := yaml.Unmarshal(content, &parsed); err != nil { + return nil, fmt.Errorf("failed to parse action YAML: %w", err) + } + return &parsed, nil +} + +// isGitHubExpressionDefault returns true if the input's default value is a GitHub Actions +// expression (e.g., "${{ github.token }}" or "${{ github.event.pull_request.number }}"). +// Such inputs should not be included in the MCP tool schema or the generated `with:` block +// so that GitHub Actions can apply the defaults naturally rather than having them overridden +// with an empty string from a missing JSON key in the agent payload. +func isGitHubExpressionDefault(input *ActionYAMLInput) bool { + if input == nil { + return false + } + return strings.HasPrefix(strings.TrimSpace(input.Default), "${{") +} + +// generateActionToolDefinition creates an MCP tool definition for a custom safe output action. +// The tool name is the normalized action name. Inputs are derived from the action.yml. +func generateActionToolDefinition(actionName string, config *SafeOutputActionConfig) map[string]any { + normalizedName := stringutil.NormalizeSafeOutputIdentifier(actionName) + + description := config.Description + if description == "" { + description = config.ActionDescription + } + if description == "" { + description = fmt.Sprintf("Run the %s action", actionName) + } + // Append once-only constraint to description + description += " (can only be called once)" + + // When action.yml could not be fetched at compile time (Inputs == nil), generate a + // permissive fallback schema so the agent can still call the tool. The runtime step + // passes the raw payload through a single `payload` input rather than individual fields. + if config.Inputs == nil { + inputSchema := map[string]any{ + "type": "object", + "properties": map[string]any{ + "payload": map[string]any{ + "type": "string", + "description": "JSON-encoded payload to pass to the action", + }, + }, + "additionalProperties": true, + } + return map[string]any{ + "name": normalizedName, + "description": description, + "inputSchema": inputSchema, + } + } + + inputSchema := map[string]any{ + "type": "object", + "properties": make(map[string]any), + "additionalProperties": false, + } + + var requiredFields []string + properties := inputSchema["properties"].(map[string]any) + + // Sort for deterministic output + inputNames := make([]string, 0, len(config.Inputs)) + for k := range config.Inputs { + inputNames = append(inputNames, k) + } + sort.Strings(inputNames) + + for _, inputName := range inputNames { + inputDef := config.Inputs[inputName] + // Skip inputs whose defaults are GitHub expression (e.g. "${{ github.token }}"). + // These are implementation details (authentication, context values) that the agent + // should not provide — GitHub Actions will apply the defaults automatically. + if isGitHubExpressionDefault(inputDef) { + continue + } + property := map[string]any{ + "type": "string", + } + if inputDef.Description != "" { + property["description"] = inputDef.Description + } + if inputDef.Default != "" { + property["default"] = inputDef.Default + } + if inputDef.Required { + requiredFields = append(requiredFields, inputName) + } + properties[inputName] = property + } + + if len(requiredFields) > 0 { + sort.Strings(requiredFields) + inputSchema["required"] = requiredFields + } + + return map[string]any{ + "name": normalizedName, + "description": description, + "inputSchema": inputSchema, + } +} + +// buildCustomSafeOutputActionsJSON builds a JSON mapping of normalized action names +// used for the GH_AW_SAFE_OUTPUT_ACTIONS env var of the handler manager step. +// This allows the handler manager to load and dispatch messages to action handlers. +// The map value is the normalized action name (same as key) for future extensibility. +func buildCustomSafeOutputActionsJSON(data *WorkflowData) string { + if data.SafeOutputs == nil || len(data.SafeOutputs.Actions) == 0 { + return "" + } + + actionMapping := make(map[string]string, len(data.SafeOutputs.Actions)) + for actionName := range data.SafeOutputs.Actions { + normalizedName := stringutil.NormalizeSafeOutputIdentifier(actionName) + actionMapping[normalizedName] = normalizedName + } + + // Sort keys for deterministic output + keys := make([]string, 0, len(actionMapping)) + for k := range actionMapping { + keys = append(keys, k) + } + sort.Strings(keys) + + ordered := make(map[string]string, len(keys)) + for _, k := range keys { + ordered[k] = actionMapping[k] + } + + jsonBytes, err := json.Marshal(ordered) + if err != nil { + safeOutputActionsLog.Printf("Warning: failed to marshal custom safe output actions: %v", err) + return "" + } + return string(jsonBytes) +} + +// actionOutputKey returns the step output key for a given normalized action name. +// The handler exports this key and the compiler uses it in step conditions and with: blocks. +func actionOutputKey(normalizedName string) string { + return "action_" + normalizedName + "_payload" +} + +// buildActionSteps generates the YAML steps for all configured safe output actions. +// Each step: +// - Is guarded by an `if:` condition checking the payload output from process_safe_outputs +// - Uses the resolved action reference +// - Has a `with:` block populated from parsed payload output via fromJSON +func (c *Compiler) buildActionSteps(data *WorkflowData) []string { + if data.SafeOutputs == nil || len(data.SafeOutputs.Actions) == 0 { + return nil + } + + // Sort action names for deterministic output + actionNames := make([]string, 0, len(data.SafeOutputs.Actions)) + for name := range data.SafeOutputs.Actions { + actionNames = append(actionNames, name) + } + sort.Strings(actionNames) + + var steps []string + + for _, actionName := range actionNames { + config := data.SafeOutputs.Actions[actionName] + normalizedName := stringutil.NormalizeSafeOutputIdentifier(actionName) + outputKey := actionOutputKey(normalizedName) + + // Determine the action reference to use in the step + actionRef := config.ResolvedRef + if actionRef == "" { + // Fall back to original uses value if resolution failed + actionRef = config.Uses + } + + // Display name: prefer the user description, then action description, then action name + displayName := config.Description + if displayName == "" { + displayName = config.ActionDescription + } + if displayName == "" { + displayName = actionName + } + + steps = append(steps, fmt.Sprintf(" - name: %s\n", displayName)) + steps = append(steps, fmt.Sprintf(" id: action_%s\n", normalizedName)) + steps = append(steps, fmt.Sprintf(" if: steps.process_safe_outputs.outputs.%s != ''\n", outputKey)) + steps = append(steps, fmt.Sprintf(" uses: %s\n", actionRef)) + + // Build optional env: block for per-action environment variables + if len(config.Env) > 0 { + steps = append(steps, " env:\n") + envKeys := make([]string, 0, len(config.Env)) + for k := range config.Env { + envKeys = append(envKeys, k) + } + sort.Strings(envKeys) + for _, envKey := range envKeys { + steps = append(steps, fmt.Sprintf(" %s: %s\n", envKey, config.Env[envKey])) + } + } + + // Build the with: block + if len(config.Inputs) > 0 { + // Filter to only inputs that the agent should provide (exclude those with GitHub + // expression defaults like "${{ github.token }}" — GitHub Actions applies them naturally). + agentInputNames := make([]string, 0, len(config.Inputs)) + for k, v := range config.Inputs { + if !isGitHubExpressionDefault(v) { + agentInputNames = append(agentInputNames, k) + } + } + sort.Strings(agentInputNames) + + if len(agentInputNames) > 0 { + steps = append(steps, " with:\n") + for _, inputName := range agentInputNames { + steps = append(steps, fmt.Sprintf(" %s: ${{ fromJSON(steps.process_safe_outputs.outputs.%s).%s }}\n", + inputName, outputKey, inputName)) + } + } + } else { + // When inputs couldn't be resolved, pass the raw payload as a single input + steps = append(steps, " with:\n") + steps = append(steps, fmt.Sprintf(" payload: ${{ steps.process_safe_outputs.outputs.%s }}\n", outputKey)) + } + } + + return steps +} + +// resolveAllActions fetches action.yml for all configured actions and populates +// the computed fields (ResolvedRef, Inputs, ActionDescription) in each config. +// This should be called once during compilation before tool generation and step generation. +func (c *Compiler) resolveAllActions(data *WorkflowData, markdownPath string) { + if data.SafeOutputs == nil || len(data.SafeOutputs.Actions) == 0 { + return + } + + safeOutputActionsLog.Printf("Resolving %d custom safe output action(s)", len(data.SafeOutputs.Actions)) + for actionName, config := range data.SafeOutputs.Actions { + if config.ResolvedRef != "" { + // Already resolved (e.g., called multiple times) + continue + } + c.fetchAndParseActionYAML(actionName, config, markdownPath, data) + safeOutputActionsLog.Printf("Resolved action %q: ref=%q, inputs=%d", actionName, config.ResolvedRef, len(config.Inputs)) + } +} diff --git a/pkg/workflow/safe_outputs_actions_test.go b/pkg/workflow/safe_outputs_actions_test.go new file mode 100644 index 0000000000..e9687edcb1 --- /dev/null +++ b/pkg/workflow/safe_outputs_actions_test.go @@ -0,0 +1,635 @@ +//go:build !integration + +package workflow + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestParseActionsConfig verifies parsing of safe-outputs.actions configuration +func TestParseActionsConfig(t *testing.T) { + actionsMap := map[string]any{ + "my-tool": map[string]any{ + "uses": "owner/repo@v1", + "description": "My custom tool", + }, + "another-tool": map[string]any{ + "uses": "owner/other-repo/subdir@v2", + }, + } + + result := parseActionsConfig(actionsMap) + + require.NotNil(t, result, "Should return non-nil result") + require.Len(t, result, 2, "Should have two actions") + + myTool, exists := result["my-tool"] + require.True(t, exists, "Should have my-tool action") + assert.Equal(t, "owner/repo@v1", myTool.Uses, "Uses should match") + assert.Equal(t, "My custom tool", myTool.Description, "Description should match") + + anotherTool, exists := result["another-tool"] + require.True(t, exists, "Should have another-tool action") + assert.Equal(t, "owner/other-repo/subdir@v2", anotherTool.Uses, "Uses should match") + assert.Empty(t, anotherTool.Description, "Description should be empty") +} + +// TestParseActionsConfigMissingUses verifies actions without uses are skipped +func TestParseActionsConfigMissingUses(t *testing.T) { + actionsMap := map[string]any{ + "invalid-tool": map[string]any{ + "description": "No uses field", + }, + "valid-tool": map[string]any{ + "uses": "owner/repo@v1", + }, + } + + result := parseActionsConfig(actionsMap) + + require.Len(t, result, 1, "Should only have the valid action") + _, exists := result["invalid-tool"] + assert.False(t, exists, "Should not have invalid-tool (missing uses)") + _, exists = result["valid-tool"] + assert.True(t, exists, "Should have valid-tool") +} + +// TestParseActionsConfigEmpty verifies empty map returns nil or empty +func TestParseActionsConfigEmpty(t *testing.T) { + result := parseActionsConfig(map[string]any{}) + assert.Empty(t, result, "Should return empty result for empty map") + + result = parseActionsConfig(nil) + assert.Nil(t, result, "Should return nil for nil input") +} + +// TestParseActionUsesField verifies parsing of different uses field formats +func TestParseActionUsesField(t *testing.T) { + tests := []struct { + name string + uses string + wantRepo string + wantSubdir string + wantRef string + wantLocal bool + wantErr bool + }{ + { + name: "root action", + uses: "owner/repo@v1", + wantRepo: "owner/repo", + wantRef: "v1", + }, + { + name: "sub-directory action", + uses: "owner/repo/subpath@v2", + wantRepo: "owner/repo", + wantSubdir: "subpath", + wantRef: "v2", + }, + { + name: "deep sub-directory action", + uses: "owner/repo/path/to/action@v3", + wantRepo: "owner/repo", + wantSubdir: "path/to/action", + wantRef: "v3", + }, + { + name: "local path", + uses: "./local/path", + wantLocal: true, + }, + { + name: "local path with parent", + uses: "../parent/path", + wantLocal: true, + }, + { + name: "missing ref", + uses: "owner/repo", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ref, err := parseActionUsesField(tt.uses) + if tt.wantErr { + assert.Error(t, err, "Should return error") + return + } + require.NoError(t, err, "Should not return error") + assert.Equal(t, tt.wantLocal, ref.IsLocal, "IsLocal mismatch") + if tt.wantLocal { + return + } + assert.Equal(t, tt.wantRepo, ref.Repo, "Repo mismatch") + assert.Equal(t, tt.wantSubdir, ref.Subdir, "Subdir mismatch") + assert.Equal(t, tt.wantRef, ref.Ref, "Ref mismatch") + }) + } +} + +// TestGenerateActionToolDefinition verifies tool definition generation +func TestGenerateActionToolDefinition(t *testing.T) { + config := &SafeOutputActionConfig{ + Uses: "owner/repo@v1", + Description: "My custom action tool", + ResolvedRef: "owner/repo@abc123 # v1", + ActionDescription: "Default description from action.yml", + Inputs: map[string]*ActionYAMLInput{ + "title": { + Description: "The title input", + Required: true, + }, + "body": { + Description: "The body input", + Required: false, + Default: "default body", + }, + }, + } + + tool := generateActionToolDefinition("my-tool", config) + + require.NotNil(t, tool, "Tool definition should not be nil") + assert.Equal(t, "my_tool", tool["name"], "Tool name should be normalized") + assert.Equal(t, "My custom action tool (can only be called once)", tool["description"], "Description should use user override + constraint") + + schema, ok := tool["inputSchema"].(map[string]any) + require.True(t, ok, "inputSchema should be a map") + assert.Equal(t, "object", schema["type"]) + assert.Equal(t, false, schema["additionalProperties"]) + + properties, ok := schema["properties"].(map[string]any) + require.True(t, ok, "properties should be a map") + assert.Len(t, properties, 2, "Should have 2 properties") + + titleProp, ok := properties["title"].(map[string]any) + require.True(t, ok, "title property should exist") + assert.Equal(t, "string", titleProp["type"]) + assert.Equal(t, "The title input", titleProp["description"]) + + bodyProp, ok := properties["body"].(map[string]any) + require.True(t, ok, "body property should exist") + assert.Equal(t, "string", bodyProp["type"]) + assert.Equal(t, "default body", bodyProp["default"]) + + required, ok := schema["required"].([]string) + require.True(t, ok, "required should be a []string") + assert.Equal(t, []string{"title"}, required, "Only title should be required") +} + +// TestGenerateActionToolDefinitionFallbackDescription verifies description fallback order +func TestGenerateActionToolDefinitionFallbackDescription(t *testing.T) { + tests := []struct { + name string + config *SafeOutputActionConfig + wantDescription string + }{ + { + name: "user description takes precedence", + config: &SafeOutputActionConfig{ + Description: "User description", + ActionDescription: "Action description", + }, + wantDescription: "User description (can only be called once)", + }, + { + name: "action description used when no user description", + config: &SafeOutputActionConfig{ + ActionDescription: "Action description", + }, + wantDescription: "Action description (can only be called once)", + }, + { + name: "fallback to action name", + config: &SafeOutputActionConfig{}, + wantDescription: "Run the my-action action (can only be called once)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tool := generateActionToolDefinition("my-action", tt.config) + assert.Equal(t, tt.wantDescription, tool["description"], "Description should match expected") + }) + } +} + +// TestGenerateActionToolDefinitionNoInputs verifies the fallback schema when resolution failed (Inputs == nil) +func TestGenerateActionToolDefinitionNoInputs(t *testing.T) { + config := &SafeOutputActionConfig{ + Uses: "owner/repo@v1", + // Inputs is nil (action.yml resolution failed) + } + + tool := generateActionToolDefinition("my-tool", config) + + schema, ok := tool["inputSchema"].(map[string]any) + require.True(t, ok, "inputSchema should be a map") + // Fallback schema should be permissive (additionalProperties: true) so agent can still call the tool + assert.Equal(t, true, schema["additionalProperties"], "Fallback schema should allow additional properties") + // Should have a payload property to hint to the agent what to send + properties, ok := schema["properties"].(map[string]any) + require.True(t, ok, "properties should be a map") + assert.Contains(t, properties, "payload", "Fallback schema should include a 'payload' property") + assert.Nil(t, schema["required"], "No required fields in fallback schema") +} + +// TestBuildCustomSafeOutputActionsJSON verifies JSON generation for GH_AW_SAFE_OUTPUT_ACTIONS +func TestBuildCustomSafeOutputActionsJSON(t *testing.T) { + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + Actions: map[string]*SafeOutputActionConfig{ + "my-tool": {Uses: "owner/repo@v1"}, + "another-tool": {Uses: "owner/repo2@v2"}, + }, + }, + } + + jsonStr := buildCustomSafeOutputActionsJSON(data) + require.NotEmpty(t, jsonStr, "Should return non-empty JSON") + + var result map[string]string + err := json.Unmarshal([]byte(jsonStr), &result) + require.NoError(t, err, "Should be valid JSON") + + assert.Equal(t, "my_tool", result["my_tool"], "Normalized name should be in map") + assert.Equal(t, "another_tool", result["another_tool"], "Normalized name should be in map") +} + +// TestBuildCustomSafeOutputActionsJSONEmpty verifies empty result when no actions +func TestBuildCustomSafeOutputActionsJSONEmpty(t *testing.T) { + assert.Empty(t, buildCustomSafeOutputActionsJSON(&WorkflowData{SafeOutputs: &SafeOutputsConfig{}}), "Should return empty for no actions") + assert.Empty(t, buildCustomSafeOutputActionsJSON(&WorkflowData{SafeOutputs: nil}), "Should return empty for nil SafeOutputs") +} + +// TestBuildActionSteps verifies step generation for configured actions +func TestBuildActionSteps(t *testing.T) { + compiler := NewCompiler() + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + Actions: map[string]*SafeOutputActionConfig{ + "my-tool": { + Uses: "owner/repo@v1", + Description: "My tool description", + ResolvedRef: "owner/repo@abc123 # v1", + Inputs: map[string]*ActionYAMLInput{ + "title": {Required: true}, + "body": {Required: false}, + }, + }, + }, + }, + } + + steps := compiler.buildActionSteps(data) + require.NotEmpty(t, steps, "Should generate steps") + + fullYAML := strings.Join(steps, "") + assert.Contains(t, fullYAML, "My tool description", "Should use description as step name") + assert.Contains(t, fullYAML, "id: action_my_tool", "Should have step ID") + assert.Contains(t, fullYAML, "steps.process_safe_outputs.outputs.action_my_tool_payload != ''", "Should have if condition") + assert.Contains(t, fullYAML, "uses: owner/repo@abc123 # v1", "Should use resolved ref") + assert.Contains(t, fullYAML, "title:", "Should have title input") + assert.Contains(t, fullYAML, "body:", "Should have body input") + assert.Contains(t, fullYAML, "fromJSON(steps.process_safe_outputs.outputs.action_my_tool_payload).title", "Should use fromJSON for title") + assert.Contains(t, fullYAML, "fromJSON(steps.process_safe_outputs.outputs.action_my_tool_payload).body", "Should use fromJSON for body") +} + +// TestBuildActionStepsFallbackPayload verifies step uses payload when inputs unknown +func TestBuildActionStepsFallbackPayload(t *testing.T) { + compiler := NewCompiler() + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + Actions: map[string]*SafeOutputActionConfig{ + "unknown-inputs-tool": { + Uses: "owner/repo@v1", + ResolvedRef: "owner/repo@abc123 # v1", + // No Inputs: action.yml couldn't be fetched + }, + }, + }, + } + + steps := compiler.buildActionSteps(data) + require.NotEmpty(t, steps, "Should generate steps even without inputs") + + fullYAML := strings.Join(steps, "") + assert.Contains(t, fullYAML, "payload:", "Should use payload as single with: key when inputs unknown") + assert.Contains(t, fullYAML, "steps.process_safe_outputs.outputs.action_unknown_inputs_tool_payload", "Should reference payload output") +} + +// TestBuildActionStepsEmpty verifies no steps when no actions +func TestBuildActionStepsEmpty(t *testing.T) { + compiler := NewCompiler() + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{}, + } + assert.Nil(t, compiler.buildActionSteps(data), "Should return nil for empty actions") +} + +// TestExtractSafeOutputsConfigIncludesActions verifies extractSafeOutputsConfig handles actions +func TestExtractSafeOutputsConfigIncludesActions(t *testing.T) { + compiler := NewCompiler() + frontmatter := map[string]any{ + "safe-outputs": map[string]any{ + "actions": map[string]any{ + "my-action": map[string]any{ + "uses": "owner/repo@v1", + "description": "My action", + }, + }, + }, + } + + config := compiler.extractSafeOutputsConfig(frontmatter) + + require.NotNil(t, config, "Should extract config") + require.Len(t, config.Actions, 1, "Should have 1 action") + + action, exists := config.Actions["my-action"] + require.True(t, exists, "Should have my-action") + assert.Equal(t, "owner/repo@v1", action.Uses, "Uses should match") + assert.Equal(t, "My action", action.Description, "Description should match") +} + +// TestHandlerManagerStepIncludesActionsEnvVar verifies GH_AW_SAFE_OUTPUT_ACTIONS env var +func TestHandlerManagerStepIncludesActionsEnvVar(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{}, + Actions: map[string]*SafeOutputActionConfig{ + "my-action": {Uses: "owner/repo@v1"}, + }, + }, + } + + steps := compiler.buildHandlerManagerStep(workflowData) + fullYAML := strings.Join(steps, "") + + assert.Contains(t, fullYAML, "GH_AW_SAFE_OUTPUT_ACTIONS", "Should include GH_AW_SAFE_OUTPUT_ACTIONS env var") + assert.Contains(t, fullYAML, "my_action", "Should include normalized action name") +} + +// TestHandlerManagerStepNoActionsEnvVar verifies GH_AW_SAFE_OUTPUT_ACTIONS absent when no actions +func TestHandlerManagerStepNoActionsEnvVar(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{}, + }, + } + + steps := compiler.buildHandlerManagerStep(workflowData) + fullYAML := strings.Join(steps, "") + + assert.NotContains(t, fullYAML, "GH_AW_SAFE_OUTPUT_ACTIONS", "Should not include GH_AW_SAFE_OUTPUT_ACTIONS when no actions") +} + +// TestHasAnySafeOutputEnabledWithActions verifies Actions are detected as enabled +func TestHasAnySafeOutputEnabledWithActions(t *testing.T) { + config := &SafeOutputsConfig{ + Actions: map[string]*SafeOutputActionConfig{ + "my-action": {Uses: "owner/repo@v1"}, + }, + } + assert.True(t, hasAnySafeOutputEnabled(config), "Should detect actions as enabled safe outputs") +} + +// TestHasNonBuiltinSafeOutputsEnabledWithActions verifies Actions count as non-builtin +func TestHasNonBuiltinSafeOutputsEnabledWithActions(t *testing.T) { + config := &SafeOutputsConfig{ + Actions: map[string]*SafeOutputActionConfig{ + "my-action": {Uses: "owner/repo@v1"}, + }, + } + assert.True(t, hasNonBuiltinSafeOutputsEnabled(config), "Actions should count as non-builtin safe outputs") +} + +// TestActionOutputKey verifies the output key naming convention +func TestActionOutputKey(t *testing.T) { + assert.Equal(t, "action_my_tool_payload", actionOutputKey("my_tool")) + assert.Equal(t, "action_another_action_payload", actionOutputKey("another_action")) +} + +// TestParseActionsConfigWithEnv verifies parsing of env field in actions config +func TestParseActionsConfigWithEnv(t *testing.T) { + actionsMap := map[string]any{ + "my-tool": map[string]any{ + "uses": "owner/repo@v1", + "env": map[string]any{ + "MY_VAR": "my-value", + "OTHER_VAR": "other-value", + }, + }, + } + + result := parseActionsConfig(actionsMap) + require.Len(t, result, 1, "Should have one action") + + myTool := result["my-tool"] + require.NotNil(t, myTool, "Should have my-tool") + require.Len(t, myTool.Env, 2, "Should have 2 env vars") + assert.Equal(t, "my-value", myTool.Env["MY_VAR"], "MY_VAR should match") + assert.Equal(t, "other-value", myTool.Env["OTHER_VAR"], "OTHER_VAR should match") +} + +// TestBuildActionStepsWithEnv verifies that env vars are emitted in generated steps +func TestBuildActionStepsWithEnv(t *testing.T) { + compiler := NewCompiler() + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + Actions: map[string]*SafeOutputActionConfig{ + "my-tool": { + Uses: "owner/repo@v1", + ResolvedRef: "owner/repo@abc123 # v1", + Env: map[string]string{ + "MY_SECRET": "${{ secrets.MY_SECRET }}", + "MY_VAR": "static-value", + }, + Inputs: map[string]*ActionYAMLInput{ + "title": {Required: true}, + }, + }, + }, + }, + } + + steps := compiler.buildActionSteps(data) + require.NotEmpty(t, steps, "Should generate steps") + + fullYAML := strings.Join(steps, "") + assert.Contains(t, fullYAML, "env:", "Should have env block") + assert.Contains(t, fullYAML, "MY_SECRET: ${{ secrets.MY_SECRET }}", "Should have MY_SECRET env var") + assert.Contains(t, fullYAML, "MY_VAR: static-value", "Should have MY_VAR env var") + // Env block should appear before with: block + envIdx := strings.Index(fullYAML, "env:") + withIdx := strings.Index(fullYAML, "with:") + assert.Less(t, envIdx, withIdx, "env: should appear before with:") +} + +// TestBuildActionStepsNoEnv verifies that no env block is emitted when Env is empty +func TestBuildActionStepsNoEnv(t *testing.T) { + compiler := NewCompiler() + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + Actions: map[string]*SafeOutputActionConfig{ + "my-tool": { + Uses: "owner/repo@v1", + ResolvedRef: "owner/repo@abc123 # v1", + Inputs: map[string]*ActionYAMLInput{ + "title": {Required: true}, + }, + }, + }, + }, + } + + steps := compiler.buildActionSteps(data) + fullYAML := strings.Join(steps, "") + assert.NotContains(t, fullYAML, "env:", "Should not have env block when Env is nil/empty") +} + +// TestExtractSafeOutputsConfigIncludesActionsWithEnv verifies env is parsed via extractSafeOutputsConfig +func TestExtractSafeOutputsConfigIncludesActionsWithEnv(t *testing.T) { + compiler := NewCompiler() + frontmatter := map[string]any{ + "safe-outputs": map[string]any{ + "actions": map[string]any{ + "my-action": map[string]any{ + "uses": "owner/repo@v1", + "env": map[string]any{ + "TOKEN": "${{ secrets.MY_TOKEN }}", + }, + }, + }, + }, + } + + config := compiler.extractSafeOutputsConfig(frontmatter) + + require.NotNil(t, config, "Should extract config") + action := config.Actions["my-action"] + require.NotNil(t, action, "Should have my-action") + require.Len(t, action.Env, 1, "Should have 1 env var") + assert.Equal(t, "${{ secrets.MY_TOKEN }}", action.Env["TOKEN"], "TOKEN env var should match") +} + +// TestIsGitHubExpressionDefault verifies detection of GitHub expression defaults +func TestIsGitHubExpressionDefault(t *testing.T) { + tests := []struct { + name string + input *ActionYAMLInput + expected bool + }{ + {"nil input", nil, false}, + {"no default", &ActionYAMLInput{}, false}, + {"static default", &ActionYAMLInput{Default: "latest"}, false}, + {"github token expression", &ActionYAMLInput{Default: "${{ github.token }}"}, true}, + {"pr number expression", &ActionYAMLInput{Default: "${{ github.event.pull_request.number }}"}, true}, + {"expression with leading whitespace", &ActionYAMLInput{Default: " ${{ github.token }}"}, true}, + {"partial expression no closing", &ActionYAMLInput{Default: "${{ incomplete"}, true}, // starts with ${{ + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, isGitHubExpressionDefault(tt.input), "isGitHubExpressionDefault result should match") + }) + } +} + +// TestBuildActionStepsSkipsGitHubExpressionDefaultInputs verifies inputs with ${{ defaults are excluded +func TestBuildActionStepsSkipsGitHubExpressionDefaultInputs(t *testing.T) { + compiler := NewCompiler() + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + Actions: map[string]*SafeOutputActionConfig{ + "add-labels": { + Uses: "actions-ecosystem/action-add-labels@v1", + ResolvedRef: "actions-ecosystem/action-add-labels@abc123 # v1", + Inputs: map[string]*ActionYAMLInput{ + "github_token": {Required: true, Default: "${{ github.token }}"}, + "number": {Required: false, Default: "${{ github.event.pull_request.number }}"}, + "labels": {Required: true}, + }, + }, + }, + }, + } + + steps := compiler.buildActionSteps(data) + fullYAML := strings.Join(steps, "") + + // Only 'labels' should be in with: block + assert.Contains(t, fullYAML, "labels:", "labels should be in with: block") + assert.NotContains(t, fullYAML, "github_token:", "github_token should be excluded (has ${{ default})") + assert.NotContains(t, fullYAML, "number:", "number should be excluded (has ${{ default})") +} + +// TestGenerateActionToolDefinitionSkipsGitHubExpressionInputs verifies schema omits ${{ inputs +func TestGenerateActionToolDefinitionSkipsGitHubExpressionInputs(t *testing.T) { + config := &SafeOutputActionConfig{ + Uses: "actions-ecosystem/action-add-labels@v1", + Inputs: map[string]*ActionYAMLInput{ + "github_token": {Required: true, Default: "${{ github.token }}"}, + "number": {Required: false, Default: "${{ github.event.pull_request.number }}"}, + "labels": {Required: true, Description: "Labels to add"}, + }, + } + + tool := generateActionToolDefinition("add-labels", config) + + schema, ok := tool["inputSchema"].(map[string]any) + require.True(t, ok, "inputSchema should be a map") + properties, ok := schema["properties"].(map[string]any) + require.True(t, ok, "properties should be a map") + + assert.Contains(t, properties, "labels", "labels should be in schema") + assert.NotContains(t, properties, "github_token", "github_token should be excluded from schema") + assert.NotContains(t, properties, "number", "number should be excluded from schema") +} + +// TestExtractSHAFromPinnedRef verifies SHA extraction from pinned action references +func TestExtractSHAFromPinnedRef(t *testing.T) { + tests := []struct { + name string + pinned string + expected string + }{ + { + name: "standard pinned reference", + pinned: "actions-ecosystem/action-add-labels@" + strings.Repeat("a", 40) + " # v1", + expected: strings.Repeat("a", 40), + }, + { + name: "no @ separator", + pinned: "no-at-sign", + expected: "", + }, + { + name: "non-SHA after @", + pinned: "owner/repo@v1 # v1", + expected: "", + }, + { + name: "empty string", + pinned: "", + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractSHAFromPinnedRef(tt.pinned) + assert.Equal(t, tt.expected, result, "extractSHAFromPinnedRef result should match") + }) + } +} diff --git a/pkg/workflow/safe_outputs_config.go b/pkg/workflow/safe_outputs_config.go index d0c124a7ff..70146ca9fc 100644 --- a/pkg/workflow/safe_outputs_config.go +++ b/pkg/workflow/safe_outputs_config.go @@ -539,6 +539,14 @@ func (c *Compiler) extractSafeOutputsConfig(frontmatter map[string]any) *SafeOut } } + // Handle actions (custom GitHub Actions mounted as safe output tools) + if actions, exists := outputMap["actions"]; exists { + if actionsMap, ok := actions.(map[string]any); ok { + config.Actions = parseActionsConfig(actionsMap) + safeOutputsConfigLog.Printf("Configured %d custom safe-output action(s)", len(config.Actions)) + } + } + // Handle app configuration for GitHub App token minting if app, exists := outputMap["github-app"]; exists { if appMap, ok := app.(map[string]any); ok { diff --git a/pkg/workflow/safe_outputs_state.go b/pkg/workflow/safe_outputs_state.go index 267c163251..a8efdd51e9 100644 --- a/pkg/workflow/safe_outputs_state.go +++ b/pkg/workflow/safe_outputs_state.go @@ -84,6 +84,12 @@ func hasAnySafeOutputEnabled(safeOutputs *SafeOutputsConfig) bool { return true } + // Check Actions separately as it's a map + if len(safeOutputs.Actions) > 0 { + safeOutputReflectionLog.Printf("Found %d custom actions enabled", len(safeOutputs.Actions)) + return true + } + // Use reflection to check all pointer fields val := reflect.ValueOf(safeOutputs).Elem() for fieldName := range safeOutputFieldMapping { @@ -136,6 +142,11 @@ func hasNonBuiltinSafeOutputsEnabled(safeOutputs *SafeOutputsConfig) bool { return true } + // Custom actions are always non-builtin + if len(safeOutputs.Actions) > 0 { + return true + } + // Check non-builtin pointer fields using the pre-computed list val := reflect.ValueOf(safeOutputs).Elem() for _, fieldName := range nonBuiltinSafeOutputFieldNames { diff --git a/pkg/workflow/safe_outputs_tools_filtering.go b/pkg/workflow/safe_outputs_tools_filtering.go index 08f83190da..b8e9a53155 100644 --- a/pkg/workflow/safe_outputs_tools_filtering.go +++ b/pkg/workflow/safe_outputs_tools_filtering.go @@ -249,8 +249,28 @@ func generateFilteredToolsJSON(data *WorkflowData, markdownPath string) (string, } } + // Add custom action tools from SafeOutputs.Actions. + // Action resolution (action.yml fetch + pinning) is done once in buildJobs before + // generateToolsMetaJSON/generateFilteredToolsJSON are called, so Inputs and + // ActionDescription are already populated here. + if len(data.SafeOutputs.Actions) > 0 { + safeOutputsConfigLog.Printf("Adding %d custom action tools", len(data.SafeOutputs.Actions)) + + actionNames := make([]string, 0, len(data.SafeOutputs.Actions)) + for actionName := range data.SafeOutputs.Actions { + actionNames = append(actionNames, actionName) + } + sort.Strings(actionNames) + + for _, actionName := range actionNames { + actionConfig := data.SafeOutputs.Actions[actionName] + customTool := generateActionToolDefinition(actionName, actionConfig) + filteredTools = append(filteredTools, customTool) + } + } + if safeOutputsConfigLog.Enabled() { - safeOutputsConfigLog.Printf("Filtered %d tools from %d total tools (including %d custom jobs, %d custom scripts)", len(filteredTools), len(allTools), len(data.SafeOutputs.Jobs), len(data.SafeOutputs.Scripts)) + safeOutputsConfigLog.Printf("Filtered %d tools from %d total tools (including %d custom jobs, %d custom scripts, %d custom actions)", len(filteredTools), len(allTools), len(data.SafeOutputs.Jobs), len(data.SafeOutputs.Scripts), len(data.SafeOutputs.Actions)) } // Add dynamic dispatch_workflow tools @@ -763,6 +783,26 @@ func generateDynamicTools(data *WorkflowData, markdownPath string) ([]map[string } } + // Add custom action tools from SafeOutputs.Actions + // Each configured action is exposed as an MCP tool with schema derived from action.yml. + // The compiler resolves action.yml at compile time; if resolution fails the tool is still + // added with an empty inputSchema so the agent can still attempt to call it. + if len(data.SafeOutputs.Actions) > 0 { + safeOutputsConfigLog.Printf("Adding %d custom action tools to dynamic tools", len(data.SafeOutputs.Actions)) + + actionNames := make([]string, 0, len(data.SafeOutputs.Actions)) + for actionName := range data.SafeOutputs.Actions { + actionNames = append(actionNames, actionName) + } + sort.Strings(actionNames) + + for _, actionName := range actionNames { + actionConfig := data.SafeOutputs.Actions[actionName] + customTool := generateActionToolDefinition(actionName, actionConfig) + dynamicTools = append(dynamicTools, customTool) + } + } + // Add dynamic dispatch_workflow tools if data.SafeOutputs.DispatchWorkflow != nil && len(data.SafeOutputs.DispatchWorkflow.Workflows) > 0 { safeOutputsConfigLog.Printf("Adding %d dispatch_workflow tools", len(data.SafeOutputs.DispatchWorkflow.Workflows))