From ab3300cca51399901a3109805122c7d754bdfc40 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:10:40 +0000 Subject: [PATCH 1/4] Initial plan From 97301d91f24fe079367afb9e634d42d75889bede Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:20:12 +0000 Subject: [PATCH 2/4] Soft-skip no-context add_comment/update_pull_request Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/safe_outputs_handlers.cjs | 76 +------------------ .../setup/js/safe_outputs_handlers.test.cjs | 58 ++++++-------- 2 files changed, 27 insertions(+), 107 deletions(-) diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index da0c183bcfe..db9a43df8b4 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -31,9 +31,6 @@ const { lstatGuard } = require("./symlink_guard.cjs"); const { validateValueAgainstSchema } = require("./mcp_scripts_validation.cjs"); const { resolveDataSchema } = require("./data_schema_normalizer.cjs"); -/** PR event names used for target:triggering context validation across all safe-output handlers. */ -const PR_EVENT_NAMES = new Set(["pull_request", "pull_request_target", "pull_request_review", "pull_request_review_comment"]); - /** * Resolve effective event name and payload from an invocation context, * falling back to the raw GitHub Actions context. @@ -1902,42 +1899,6 @@ function createHandlers(server, appendSafeOutput, config = {}) { ); } - // Reject target:triggering early when no explicit item number and no issue/PR/discussion context. - // Per Safe Outputs Specification MCE1: provides actionable feedback before writing to NDJSON. - // Mirrors update_issue validation; explicit item_number bypasses this check because the - // downstream handler resolves explicit numbers before falling back to triggering context. - const effectiveAddCommentTarget = addCommentConfig.target || "triggering"; - const hasExplicitItemNumber = args?.item_number != null || args?.issue_number != null || args?.["pr-number"] != null; - if (effectiveAddCommentTarget === "triggering" && !hasExplicitItemNumber) { - /** @type {any} */ - let invocationContext = null; - try { - invocationContext = resolveInvocationContext(context); - } catch (err) { - // A validation error (e.g. disallowed target_repo / SEC-005) is a real failure — surface it. - const errMsg = getErrorMessage(err); - if (errMsg.startsWith(ERR_VALIDATION)) { - return buildIntentErrorResponse(errMsg); - } - // Unexpected structural error: skip validation and let downstream handle gracefully. - } - if (invocationContext != null) { - const { effectiveEventName, effectivePayload } = resolveEffectiveContext(invocationContext, context); - const isIssueCommentOnPR = effectiveEventName === "issue_comment" && Boolean(effectivePayload?.issue?.pull_request); - const isIssueContext = effectiveEventName === "issues" || (effectiveEventName === "issue_comment" && !isIssueCommentOnPR); - const isPRContext = PR_EVENT_NAMES.has(effectiveEventName) || isIssueCommentOnPR; - const isDiscussionContext = effectiveEventName === "discussion" || effectiveEventName === "discussion_comment"; - if (!isIssueContext && !isPRContext && !isDiscussionContext) { - return buildIntentErrorResponse( - `add_comment requires an issue, pull request, or discussion context but the workflow is running on a "${effectiveEventName}" event. ` + - `The add-comment handler uses target: triggering which only applies when an issue, pull request, or discussion triggered the workflow. ` + - `To report results from this workflow, use create_discussion or create_issue instead. ` + - `If you need to comment on a specific item, provide an explicit item_number.` - ); - } - } - } - // Build the entry with a temporary_id const entry = { ...(args || {}), type: "add_comment" }; const wildcardTargetValidationError = validateWildcardTargetRequirement(entry); @@ -2255,9 +2216,9 @@ function createHandlers(server, appendSafeOutput, config = {}) { * to provide immediate feedback to the LLM before recording to NDJSON. * Uses hasUpdatePullRequestFields to validate that at least one of 'title', 'body', * or 'update_branch' is provided before recording to NDJSON. - * Rejects `target: triggering` (the default) when the workflow has no pull request context - * (e.g. on schedule or push events), so the agent receives an actionable error - * instead of a downstream Process Safe Outputs failure. + * For `target: triggering` in non-PR contexts (e.g. schedule/workflow_dispatch without + * aw_context), this MCP-phase handler still records the output. The downstream runtime + * handler resolves context and soft-skips those entries instead of hard-failing the run. */ const updatePullRequestHandler = args => { if (!hasUpdatePullRequestFields(args)) { @@ -2267,37 +2228,6 @@ function createHandlers(server, appendSafeOutput, config = {}) { }; } - const updatePRConfig = getSafeOutputsToolConfig(config, "update_pull_request"); - const effectivePRTarget = updatePRConfig.target || "triggering"; - if (effectivePRTarget === "triggering") { - /** @type {any} */ - let invocationContext = null; - try { - invocationContext = resolveInvocationContext(context); - } catch (err) { - // A validation error (e.g. disallowed target_repo / SEC-005) is a real failure — surface it. - const errMsg = getErrorMessage(err); - if (errMsg.startsWith(ERR_VALIDATION)) { - return buildIntentErrorResponse(errMsg); - } - // Unexpected structural error: skip validation and let downstream handle gracefully. - } - if (invocationContext != null) { - const { effectiveEventName, effectivePayload } = resolveEffectiveContext(invocationContext, context); - const isIssueCommentOnPR = effectiveEventName === "issue_comment" && Boolean(effectivePayload?.issue?.pull_request); - const isPRContext = PR_EVENT_NAMES.has(effectiveEventName) || isIssueCommentOnPR; - - if (!isPRContext) { - return buildIntentErrorResponse( - `update_pull_request requires a pull request context but the workflow is running on a "${effectiveEventName}" event. ` + - `The update-pull-request handler uses target: triggering which only applies when a pull request triggered the workflow. ` + - `To report results from this workflow, use create_discussion or create_issue instead. ` + - `If you need to update a specific pull request, the workflow must configure update-pull-request: target: '*' and you must supply pull_request_number.` - ); - } - } - } - return defaultHandler("update_pull_request")(args || {}); }; diff --git a/actions/setup/js/safe_outputs_handlers.test.cjs b/actions/setup/js/safe_outputs_handlers.test.cjs index a444d4d71c4..56483cea066 100644 --- a/actions/setup/js/safe_outputs_handlers.test.cjs +++ b/actions/setup/js/safe_outputs_handlers.test.cjs @@ -2281,33 +2281,29 @@ describe("safe_outputs_handlers", () => { } }); - it("should return intent error when target is triggering (default) and not in issue/PR/discussion context", () => { + it("should record entry when target is triggering (default) and not in issue/PR/discussion context", () => { const savedContext = global.context; global.context = { ...global.context, eventName: "push", payload: {} }; try { const result = handlers.addCommentHandler({ body: "A real comment body that is substantive" }); - expect(result.isError).toBe(true); + expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); - expect(responseData.result).toBe("error"); - expect(responseData.error).toContain("add_comment"); - expect(responseData.error).toContain('"push"'); - expect(mockAppendSafeOutput).not.toHaveBeenCalled(); + expect(responseData.result).toBe("success"); + expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "add_comment" })); } finally { global.context = savedContext; } }); - it("should return intent error on schedule event with default target", () => { + it("should record entry on schedule event with default target", () => { const savedContext = global.context; global.context = { ...global.context, eventName: "schedule", payload: {} }; try { const result = handlers.addCommentHandler({ body: "A real comment body that is substantive" }); - expect(result.isError).toBe(true); + expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); - expect(responseData.result).toBe("error"); - expect(responseData.error).toContain('"schedule"'); - expect(responseData.error).toContain("create_discussion"); - expect(mockAppendSafeOutput).not.toHaveBeenCalled(); + expect(responseData.result).toBe("success"); + expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "add_comment" })); } finally { global.context = savedContext; } @@ -2389,7 +2385,7 @@ describe("safe_outputs_handlers", () => { } }); - it("should return intent error on workflow_dispatch with no event_name override", () => { + it("should record entry on workflow_dispatch with no event_name override", () => { const savedContext = global.context; global.context = { ...global.context, @@ -2398,11 +2394,10 @@ describe("safe_outputs_handlers", () => { }; try { const result = handlers.addCommentHandler({ body: "A real comment body that is substantive" }); - expect(result.isError).toBe(true); + expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); - expect(responseData.result).toBe("error"); - expect(responseData.error).toContain('"workflow_dispatch"'); - expect(mockAppendSafeOutput).not.toHaveBeenCalled(); + expect(responseData.result).toBe("success"); + expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "add_comment" })); } finally { global.context = savedContext; } @@ -3063,33 +3058,29 @@ describe("safe_outputs_handlers", () => { } }); - it("should return intent error when target is triggering (default) and not in PR context", () => { + it("should record entry when target is triggering (default) and not in PR context", () => { const savedContext = global.context; global.context = { ...global.context, eventName: "push", payload: {} }; try { const result = handlers.updatePullRequestHandler({ title: "Update title" }); - expect(result.isError).toBe(true); + expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); - expect(responseData.result).toBe("error"); - expect(responseData.error).toContain("update_pull_request"); - expect(responseData.error).toContain('"push"'); - expect(mockAppendSafeOutput).not.toHaveBeenCalled(); + expect(responseData.result).toBe("success"); + expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "update_pull_request", title: "Update title" })); } finally { global.context = savedContext; } }); - it("should return intent error on schedule event with default target", () => { + it("should record entry on schedule event with default target", () => { const savedContext = global.context; global.context = { ...global.context, eventName: "schedule", payload: {} }; try { const result = handlers.updatePullRequestHandler({ body: "Report" }); - expect(result.isError).toBe(true); + expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); - expect(responseData.result).toBe("error"); - expect(responseData.error).toContain('"schedule"'); - expect(responseData.error).toContain("create_discussion"); - expect(mockAppendSafeOutput).not.toHaveBeenCalled(); + expect(responseData.result).toBe("success"); + expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "update_pull_request", body: "Report" })); } finally { global.context = savedContext; } @@ -3134,7 +3125,7 @@ describe("safe_outputs_handlers", () => { } }); - it("should return intent error on workflow_dispatch with no event_name override", () => { + it("should record entry on workflow_dispatch with no event_name override", () => { const savedContext = global.context; global.context = { ...global.context, @@ -3143,11 +3134,10 @@ describe("safe_outputs_handlers", () => { }; try { const result = handlers.updatePullRequestHandler({ title: "No context title" }); - expect(result.isError).toBe(true); + expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); - expect(responseData.result).toBe("error"); - expect(responseData.error).toContain('"workflow_dispatch"'); - expect(mockAppendSafeOutput).not.toHaveBeenCalled(); + expect(responseData.result).toBe("success"); + expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "update_pull_request", title: "No context title" })); } finally { global.context = savedContext; } From 704795fbf07a34f7a4db6018122f770681c0601b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:01:13 +0000 Subject: [PATCH 3/4] Restore SEC-005 validation to addCommentHandler and updatePullRequestHandler Keep resolveInvocationContext + ERR_VALIDATION check in both handlers to enforce target_repo allowlist (SEC-005) at MCP-phase, while still allowing no-context events to be enqueued for runtime soft-skip. Add test cases verifying disallowed target_repo is rejected at MCP-phase for both add_comment and update_pull_request. Update addCommentHandler JSDoc to document delegated runtime soft-skip behavior and preserved SEC-005 enforcement. Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- actions/setup/js/safe_outputs_handlers.cjs | 34 +++++++++++++- .../setup/js/safe_outputs_handlers.test.cjs | 46 +++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index db9a43df8b4..439cd8477b4 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -1866,8 +1866,13 @@ function createHandlers(server, appendSafeOutput, config = {}) { * Handler for add_comment tool * Spec cross-reference: Safe Output Outcome Evaluation §3 (`add_comment`). * Per Safe Outputs Specification MCE1: Enforces constraints during tool invocation - * to provide immediate feedback to the LLM before recording to NDJSON - * Also auto-generates a temporary_id if not provided and returns it to the agent + * to provide immediate feedback to the LLM before recording to NDJSON. + * Also auto-generates a temporary_id if not provided and returns it to the agent. + * + * Context resolution is delegated to runtime: when `target: triggering` resolves + * to a context with no issue, PR, or discussion (e.g. push, schedule), the runtime + * handler soft-skips the entry instead of failing the safe-outputs pass. + * SEC-005 (`target_repo` allowlist) is still enforced here at MCP-phase. */ const addCommentHandler = args => { // Validate comment constraints before appending to safe outputs @@ -1899,6 +1904,18 @@ function createHandlers(server, appendSafeOutput, config = {}) { ); } + // Enforce SEC-005: reject disallowed cross-repository target_repo overrides on workflow_dispatch. + // No-context events (push, schedule, etc.) are soft-skipped at runtime; only validation errors are surfaced here. + try { + resolveInvocationContext(context); + } catch (err) { + const errMsg = getErrorMessage(err); + if (errMsg.startsWith(ERR_VALIDATION)) { + return buildIntentErrorResponse(errMsg); + } + // Unexpected structural error: let downstream handle gracefully. + } + // Build the entry with a temporary_id const entry = { ...(args || {}), type: "add_comment" }; const wildcardTargetValidationError = validateWildcardTargetRequirement(entry); @@ -2219,6 +2236,7 @@ function createHandlers(server, appendSafeOutput, config = {}) { * For `target: triggering` in non-PR contexts (e.g. schedule/workflow_dispatch without * aw_context), this MCP-phase handler still records the output. The downstream runtime * handler resolves context and soft-skips those entries instead of hard-failing the run. + * SEC-005 (`target_repo` allowlist) is still enforced here at MCP-phase. */ const updatePullRequestHandler = args => { if (!hasUpdatePullRequestFields(args)) { @@ -2228,6 +2246,18 @@ function createHandlers(server, appendSafeOutput, config = {}) { }; } + // Enforce SEC-005: reject disallowed cross-repository target_repo overrides on workflow_dispatch. + // No-context events (push, schedule, etc.) are soft-skipped at runtime; only validation errors are surfaced here. + try { + resolveInvocationContext(context); + } catch (err) { + const errMsg = getErrorMessage(err); + if (errMsg.startsWith(ERR_VALIDATION)) { + return buildIntentErrorResponse(errMsg); + } + // Unexpected structural error: let downstream handle gracefully. + } + return defaultHandler("update_pull_request")(args || {}); }; diff --git a/actions/setup/js/safe_outputs_handlers.test.cjs b/actions/setup/js/safe_outputs_handlers.test.cjs index 56483cea066..b060aa15336 100644 --- a/actions/setup/js/safe_outputs_handlers.test.cjs +++ b/actions/setup/js/safe_outputs_handlers.test.cjs @@ -2402,6 +2402,29 @@ describe("safe_outputs_handlers", () => { global.context = savedContext; } }); + + it("should return intent error when workflow_dispatch has disallowed target_repo (SEC-005)", () => { + const savedContext = global.context; + global.context = { + ...global.context, + eventName: "workflow_dispatch", + payload: { + inputs: { + target_repo: "other-owner/other-repo", + }, + }, + }; + try { + const result = handlers.addCommentHandler({ body: "A real comment body that is substantive" }); + expect(result.isError).toBe(true); + const responseData = JSON.parse(result.content[0].text); + expect(responseData.result).toBe("error"); + expect(responseData.error).toContain("other-owner/other-repo"); + expect(mockAppendSafeOutput).not.toHaveBeenCalled(); + } finally { + global.context = savedContext; + } + }); }); describe("createIssueHandler", () => { @@ -3143,6 +3166,29 @@ describe("safe_outputs_handlers", () => { } }); + it("should return intent error when workflow_dispatch has disallowed target_repo (SEC-005)", () => { + const savedContext = global.context; + global.context = { + ...global.context, + eventName: "workflow_dispatch", + payload: { + inputs: { + target_repo: "other-owner/other-repo", + }, + }, + }; + try { + const result = handlers.updatePullRequestHandler({ title: "PR update with disallowed repo" }); + expect(result.isError).toBe(true); + const responseData = JSON.parse(result.content[0].text); + expect(responseData.result).toBe("error"); + expect(responseData.error).toContain("other-owner/other-repo"); + expect(mockAppendSafeOutput).not.toHaveBeenCalled(); + } finally { + global.context = savedContext; + } + }); + it("should write entry when issue_comment fires on a PR (PR context for update_pull_request)", () => { const savedContext = global.context; global.context = { From ec346493e5be35729502754252a21f5771d0e6dd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:00:00 +0000 Subject: [PATCH 4/4] Add hint feedback to safe-output success response on no-context enqueue Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/safe_outputs_handlers.cjs | 84 ++++++++++++++++--- .../setup/js/safe_outputs_handlers.test.cjs | 24 ++++++ 2 files changed, 98 insertions(+), 10 deletions(-) diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index 439cd8477b4..abd49949298 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -31,6 +31,9 @@ const { lstatGuard } = require("./symlink_guard.cjs"); const { validateValueAgainstSchema } = require("./mcp_scripts_validation.cjs"); const { resolveDataSchema } = require("./data_schema_normalizer.cjs"); +/** PR event names used for target:triggering context detection across add_comment and update_pull_request handlers. */ +const PR_EVENT_NAMES = new Set(["pull_request", "pull_request_target", "pull_request_review", "pull_request_review_comment"]); + /** * Resolve effective event name and payload from an invocation context, * falling back to the raw GitHub Actions context. @@ -1872,6 +1875,8 @@ function createHandlers(server, appendSafeOutput, config = {}) { * Context resolution is delegated to runtime: when `target: triggering` resolves * to a context with no issue, PR, or discussion (e.g. push, schedule), the runtime * handler soft-skips the entry instead of failing the safe-outputs pass. + * When no triggering context is detected, a hint is included in the success response + * so the agent can recover (e.g. switch to create_issue or create_discussion). * SEC-005 (`target_repo` allowlist) is still enforced here at MCP-phase. */ const addCommentHandler = args => { @@ -1904,10 +1909,14 @@ function createHandlers(server, appendSafeOutput, config = {}) { ); } - // Enforce SEC-005: reject disallowed cross-repository target_repo overrides on workflow_dispatch. - // No-context events (push, schedule, etc.) are soft-skipped at runtime; only validation errors are surfaced here. + // Enforce SEC-005 and detect no-context events for hint generation. + // A validation error (e.g. disallowed target_repo) is surfaced immediately. + // No-context events (push, schedule, etc.) are soft-skipped at runtime; a hint is + // included in the success response so the agent can recover. + /** @type {ReturnType | null} */ + let addCommentInvocationContext = null; try { - resolveInvocationContext(context); + addCommentInvocationContext = resolveInvocationContext(context); } catch (err) { const errMsg = getErrorMessage(err); if (errMsg.startsWith(ERR_VALIDATION)) { @@ -1916,6 +1925,27 @@ function createHandlers(server, appendSafeOutput, config = {}) { // Unexpected structural error: let downstream handle gracefully. } + // Detect whether the current event has no issue/PR/discussion context so a hint + // can be included in the success response (agent feedback, not a hard failure). + const effectiveAddCommentTarget = addCommentConfig.target || "triggering"; + const hasExplicitItemNumber = args?.item_number != null || args?.issue_number != null || args?.["pr-number"] != null; + /** @type {string | null} */ + let addCommentContextHint = null; + if (effectiveAddCommentTarget === "triggering" && !hasExplicitItemNumber && addCommentInvocationContext != null) { + const { effectiveEventName, effectivePayload } = resolveEffectiveContext(addCommentInvocationContext, context); + const isIssueCommentOnPR = effectiveEventName === "issue_comment" && Boolean(effectivePayload?.issue?.pull_request); + const isIssueContext = effectiveEventName === "issues" || (effectiveEventName === "issue_comment" && !isIssueCommentOnPR); + const isPRContext = PR_EVENT_NAMES.has(effectiveEventName) || isIssueCommentOnPR; + const isDiscussionContext = effectiveEventName === "discussion" || effectiveEventName === "discussion_comment"; + if (!isIssueContext && !isPRContext && !isDiscussionContext) { + addCommentContextHint = + `add_comment targets 'triggering' context but the workflow is running on a '${effectiveEventName}' event. ` + + `This entry will be skipped at runtime if no issue, pull request, or discussion triggered the workflow. ` + + `To ensure output is always recorded, use create_issue or create_discussion instead. ` + + `To comment on a specific item, provide an explicit item_number.`; + } + } + // Build the entry with a temporary_id const entry = { ...(args || {}), type: "add_comment" }; const wildcardTargetValidationError = validateWildcardTargetRequirement(entry); @@ -1941,7 +1971,9 @@ function createHandlers(server, appendSafeOutput, config = {}) { // Append to safe outputs appendSafeOutputCounted(entry); - // Return the temporary_id to the agent so it can reference this comment + // Return the temporary_id to the agent so it can reference this comment. + // Include a hint when the entry targets 'triggering' context in a no-context event so + // the agent can recover (e.g. add create_issue as a fallback) before the run ends. return { content: [ { @@ -1950,6 +1982,7 @@ function createHandlers(server, appendSafeOutput, config = {}) { result: "success", temporary_id: entry.temporary_id, comment: `#${entry.temporary_id}`, + ...(addCommentContextHint ? { hint: addCommentContextHint } : {}), }), }, ], @@ -2234,8 +2267,10 @@ function createHandlers(server, appendSafeOutput, config = {}) { * Uses hasUpdatePullRequestFields to validate that at least one of 'title', 'body', * or 'update_branch' is provided before recording to NDJSON. * For `target: triggering` in non-PR contexts (e.g. schedule/workflow_dispatch without - * aw_context), this MCP-phase handler still records the output. The downstream runtime - * handler resolves context and soft-skips those entries instead of hard-failing the run. + * aw_context), this MCP-phase handler still records the output. A hint is included in + * the success response so the agent can recover (e.g. switch to create_issue) before + * the run ends. The downstream runtime handler resolves context and soft-skips those + * entries instead of hard-failing the run. * SEC-005 (`target_repo` allowlist) is still enforced here at MCP-phase. */ const updatePullRequestHandler = args => { @@ -2246,10 +2281,14 @@ function createHandlers(server, appendSafeOutput, config = {}) { }; } - // Enforce SEC-005: reject disallowed cross-repository target_repo overrides on workflow_dispatch. - // No-context events (push, schedule, etc.) are soft-skipped at runtime; only validation errors are surfaced here. + // Enforce SEC-005 and detect no-PR-context events for hint generation. + // A validation error (e.g. disallowed target_repo) is surfaced immediately. + // No-context events (push, schedule, etc.) are soft-skipped at runtime; a hint is + // included in the success response so the agent can recover. + /** @type {ReturnType | null} */ + let updatePRInvocationContext = null; try { - resolveInvocationContext(context); + updatePRInvocationContext = resolveInvocationContext(context); } catch (err) { const errMsg = getErrorMessage(err); if (errMsg.startsWith(ERR_VALIDATION)) { @@ -2258,7 +2297,32 @@ function createHandlers(server, appendSafeOutput, config = {}) { // Unexpected structural error: let downstream handle gracefully. } - return defaultHandler("update_pull_request")(args || {}); + // Detect whether the current event has no PR context so a hint can be included in + // the success response (agent feedback, not a hard failure). + const updatePRConfig = getSafeOutputsToolConfig(config, "update_pull_request"); + const effectivePRTarget = updatePRConfig.target || "triggering"; + /** @type {string | null} */ + let updatePRContextHint = null; + if (effectivePRTarget === "triggering" && updatePRInvocationContext != null) { + const { effectiveEventName, effectivePayload } = resolveEffectiveContext(updatePRInvocationContext, context); + const isIssueCommentOnPR = effectiveEventName === "issue_comment" && Boolean(effectivePayload?.issue?.pull_request); + const isPRContext = PR_EVENT_NAMES.has(effectiveEventName) || isIssueCommentOnPR; + if (!isPRContext) { + updatePRContextHint = + `update_pull_request targets 'triggering' context but the workflow is running on a '${effectiveEventName}' event. ` + + `This entry will be skipped at runtime if no pull request triggered the workflow. ` + + `To report results from this workflow, use create_issue or create_discussion instead. ` + + `To update a specific pull request, configure 'update-pull-request: target: \"*\"' in the workflow and provide 'pull_request_number'.`; + } + } + + const updatePRResponse = defaultHandler("update_pull_request")(args || {}); + if (updatePRContextHint && !updatePRResponse.isError) { + const responseData = JSON.parse(updatePRResponse.content[0].text); + responseData.hint = updatePRContextHint; + updatePRResponse.content[0].text = JSON.stringify(responseData); + } + return updatePRResponse; }; return { diff --git a/actions/setup/js/safe_outputs_handlers.test.cjs b/actions/setup/js/safe_outputs_handlers.test.cjs index b060aa15336..fd4794c46e5 100644 --- a/actions/setup/js/safe_outputs_handlers.test.cjs +++ b/actions/setup/js/safe_outputs_handlers.test.cjs @@ -2289,6 +2289,9 @@ describe("safe_outputs_handlers", () => { expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); expect(responseData.result).toBe("success"); + expect(responseData.hint).toBeDefined(); + expect(responseData.hint).toContain("push"); + expect(responseData.hint).toContain("create_issue"); expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "add_comment" })); } finally { global.context = savedContext; @@ -2303,6 +2306,9 @@ describe("safe_outputs_handlers", () => { expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); expect(responseData.result).toBe("success"); + expect(responseData.hint).toBeDefined(); + expect(responseData.hint).toContain("schedule"); + expect(responseData.hint).toContain("create_issue"); expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "add_comment" })); } finally { global.context = savedContext; @@ -2317,6 +2323,7 @@ describe("safe_outputs_handlers", () => { expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); expect(responseData.result).toBe("success"); + expect(responseData.hint).toBeUndefined(); expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "add_comment" })); } finally { global.context = savedContext; @@ -2335,6 +2342,7 @@ describe("safe_outputs_handlers", () => { expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); expect(responseData.result).toBe("success"); + expect(responseData.hint).toBeUndefined(); expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "add_comment" })); } finally { global.context = savedContext; @@ -2352,6 +2360,7 @@ describe("safe_outputs_handlers", () => { expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); expect(responseData.result).toBe("success"); + expect(responseData.hint).toBeUndefined(); expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "add_comment", item_number: 42 })); } finally { global.context = savedContext; @@ -2379,6 +2388,7 @@ describe("safe_outputs_handlers", () => { expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); expect(responseData.result).toBe("success"); + expect(responseData.hint).toBeUndefined(); expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "add_comment" })); } finally { global.context = savedContext; @@ -2397,6 +2407,9 @@ describe("safe_outputs_handlers", () => { expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); expect(responseData.result).toBe("success"); + expect(responseData.hint).toBeDefined(); + expect(responseData.hint).toContain("workflow_dispatch"); + expect(responseData.hint).toContain("create_issue"); expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "add_comment" })); } finally { global.context = savedContext; @@ -3089,6 +3102,9 @@ describe("safe_outputs_handlers", () => { expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); expect(responseData.result).toBe("success"); + expect(responseData.hint).toBeDefined(); + expect(responseData.hint).toContain("push"); + expect(responseData.hint).toContain("create_issue"); expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "update_pull_request", title: "Update title" })); } finally { global.context = savedContext; @@ -3103,6 +3119,9 @@ describe("safe_outputs_handlers", () => { expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); expect(responseData.result).toBe("success"); + expect(responseData.hint).toBeDefined(); + expect(responseData.hint).toContain("schedule"); + expect(responseData.hint).toContain("create_issue"); expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "update_pull_request", body: "Report" })); } finally { global.context = savedContext; @@ -3142,6 +3161,7 @@ describe("safe_outputs_handlers", () => { expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); expect(responseData.result).toBe("success"); + expect(responseData.hint).toBeUndefined(); expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "update_pull_request" })); } finally { global.context = savedContext; @@ -3160,6 +3180,9 @@ describe("safe_outputs_handlers", () => { expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); expect(responseData.result).toBe("success"); + expect(responseData.hint).toBeDefined(); + expect(responseData.hint).toContain("workflow_dispatch"); + expect(responseData.hint).toContain("create_issue"); expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "update_pull_request", title: "No context title" })); } finally { global.context = savedContext; @@ -3201,6 +3224,7 @@ describe("safe_outputs_handlers", () => { expect(result.isError).toBeUndefined(); const responseData = JSON.parse(result.content[0].text); expect(responseData.result).toBe("success"); + expect(responseData.hint).toBeUndefined(); expect(mockAppendSafeOutput).toHaveBeenCalledWith(expect.objectContaining({ type: "update_pull_request" })); } finally { global.context = savedContext;