Align safe-output no-context behavior: allow MCP enqueue, rely on runtime soft-skip

actions/setup/js/safe_outputs_handlers.cjs

@@ -31,7 +31,7 @@ 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. */
+/** 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"]);
 
 /**
@@ -1869,8 +1869,15 @@ 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.
+   * 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 => {
     // Validate comment constraints before appending to safe outputs
@@ -1902,39 +1909,40 @@ 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.
+    // 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<typeof resolveInvocationContext> | null} */
+    let addCommentInvocationContext = null;
+    try {
+      addCommentInvocationContext = resolveInvocationContext(context);
+    } catch (err) {
+      const errMsg = getErrorMessage(err);
+      if (errMsg.startsWith(ERR_VALIDATION)) {
+        return buildIntentErrorResponse(errMsg);
+      }
+      // 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;
-    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.`
-          );
-        }
+    /** @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.`;
       }
     }
 
@@ -1963,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: [
         {
@@ -1972,6 +1982,7 @@ function createHandlers(server, appendSafeOutput, config = {}) {
             result: "success",
             temporary_id: entry.temporary_id,
             comment: `#${entry.temporary_id}`,
+            ...(addCommentContextHint ? { hint: addCommentContextHint } : {}),
           }),
         },
       ],
@@ -2255,9 +2266,12 @@ 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. 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 => {
     if (!hasUpdatePullRequestFields(args)) {
@@ -2267,38 +2281,48 @@ 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.
+    // 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<typeof resolveInvocationContext> | null} */
+    let updatePRInvocationContext = null;
+    try {
+      updatePRInvocationContext = resolveInvocationContext(context);
+    } catch (err) {
+      const errMsg = getErrorMessage(err);
+      if (errMsg.startsWith(ERR_VALIDATION)) {
+        return buildIntentErrorResponse(errMsg);
       }
-      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;
+      // Unexpected structural error: let downstream handle gracefully.
+    }
 
-        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.`
-          );
-        }
+    // 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'.`;
       }
     }
 
-    return defaultHandler("update_pull_request")(args || {});
+    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 {