From 4e19aa199f8d31d5a206896a3193fc339dbf82a4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 15:28:26 +0000 Subject: [PATCH 1/7] fix: add staged support to all safe-output handlers in schema and compiler - Add `staged` field to all 38 handler schemas in main_workflow_schema.json (previously only close-pull-request and push-to-pull-request-branch had it; all schemas have `additionalProperties: false` so missing staged caused parser rejection) - Refactor compiler_safe_outputs_env.go: replace per-handler staged flag logic with a single hasSafeOutputWithoutTargetRepo() helper that covers all 40 handlers, fixing the bug where handlers like create-discussion, autofix-code-scanning-alert, noop, etc. never triggered the GH_AW_SAFE_OUTPUTS_STAGED env var - Add TestStagedFlagForAllHandlerTypes covering previously untested handler types Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/daily-choice-test.lock.yml | 1 + pkg/parser/schemas/main_workflow_schema.json | 190 ++++++++++++++++ pkg/workflow/compiler_safe_outputs_env.go | 202 +++++++++++------- .../compiler_safe_outputs_env_test.go | 194 +++++++++++++++++ 4 files changed, 513 insertions(+), 74 deletions(-) diff --git a/.github/workflows/daily-choice-test.lock.yml b/.github/workflows/daily-choice-test.lock.yml index 7b4c152d6ad..f1616bcff4f 100644 --- a/.github/workflows/daily-choice-test.lock.yml +++ b/.github/workflows/daily-choice-test.lock.yml @@ -1057,6 +1057,7 @@ jobs: GITHUB_API_URL: ${{ github.api_url }} GH_AW_SAFE_OUTPUT_JOBS: "{\"test_environment\":\"\"}" GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"missing_data\":{},\"missing_tool\":{}}" + GH_AW_SAFE_OUTPUTS_STAGED: "true" with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} script: | diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 096121391a2..39d26863606 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -4464,6 +4464,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false, @@ -4536,6 +4541,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -4586,6 +4596,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -4691,6 +4706,11 @@ }, "additionalProperties": false } + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false, @@ -4806,6 +4826,11 @@ }, "additionalProperties": false } + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -4855,6 +4880,11 @@ "description": "Target project URL for status update operations. This is required in the configuration for documentation purposes. Agent messages MUST explicitly include the project field in their output - the configured value is not used as a fallback. Must be a valid GitHub Projects v2 URL.", "pattern": "^https://github\\.com/(users|orgs)/([^/]+|<[A-Z_]+>)/projects/(\\d+|<[A-Z_]+>)$", "examples": ["https://github.com/orgs/myorg/projects/123", "https://github.com/users/username/projects/456"] + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false, @@ -4975,6 +5005,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false, @@ -5056,6 +5091,11 @@ "target-repo": { "type": "string", "description": "Target repository in format 'owner/repo' for cross-repository operations. Takes precedence over trial target repo settings." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false, @@ -5128,6 +5168,11 @@ "type": "boolean", "description": "Controls whether AI-generated footer is added when updating the discussion body. When false, the visible footer content is omitted. Defaults to true. Only applies when 'body' is enabled.", "default": true + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -5185,6 +5230,11 @@ "type": "string" }, "description": "List of additional repositories in format 'owner/repo' that issues can be closed in. When specified, the agent can use a 'repo' field in the output to specify which repository to close the issue in. The target repository (current or target-repo) is always implicitly allowed." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false, @@ -5316,6 +5366,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false, @@ -5410,6 +5465,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false, @@ -5581,6 +5641,11 @@ "type": "string" }, "description": "List of glob patterns for files to exclude from the patch. Each pattern is passed to `git format-patch` as a `:(exclude)` magic pathspec, so matching files are stripped by git at generation time and will not appear in the commit. Excluded files are also not subject to `allowed-files` or `protected-files` checks. Supports * (any characters except /) and ** (any characters including /)." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false, @@ -5649,6 +5714,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -5713,6 +5783,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -5768,6 +5843,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -5803,6 +5883,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -5852,6 +5937,11 @@ "type": "string" }, "description": "List of additional repositories in format 'owner/repo' that code scanning alerts can be created in. When specified, the agent can use a 'repo' field in the output to specify which repository to create the alert in. The target repository (current or target-repo) is always implicitly allowed." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -5886,6 +5976,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -5957,6 +6052,11 @@ "type": "string" }, "description": "List of additional repositories in format 'owner/repo' that labels can be added to. When specified, the agent can use a 'repo' field in the output to specify which repository to add labels to. The target repository (current or target-repo) is always implicitly allowed." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -6024,6 +6124,11 @@ "type": "string" }, "description": "List of additional repositories in format 'owner/repo' that labels can be removed from. When specified, the agent can use a 'repo' field in the output to specify which repository to remove labels from. The target repository (current or target-repo) is always implicitly allowed." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -6075,6 +6180,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -6122,6 +6232,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -6207,6 +6322,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -6275,6 +6395,11 @@ "type": "string" }, "description": "List of allowed repositories in format 'owner/repo' for cross-repository user assignment operations. Use with 'repo' field in tool calls." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -6338,6 +6463,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -6403,6 +6533,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -6471,6 +6606,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -6532,6 +6672,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -6687,6 +6832,11 @@ "discussions": { "type": "boolean", "description": "Controls whether the workflow requests discussions:write permission for hide-comment. Default: true (includes discussions:write). Set to false if your GitHub App lacks Discussions permission to prevent 422 errors during token generation." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -6745,6 +6895,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -6795,6 +6950,11 @@ "target-ref": { "type": "string", "description": "Git ref (branch, tag, or SHA) to use when dispatching the workflow. For workflow_call relay scenarios this is auto-injected by the compiler from needs.activation.outputs.target_ref. Overrides the caller's GITHUB_REF." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "required": ["workflows"], @@ -6848,6 +7008,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token passed to called workflows. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "required": ["workflows"], @@ -6907,6 +7072,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -6964,6 +7134,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -7009,6 +7184,11 @@ "type": "boolean", "description": "Controls whether noop runs are reported as issue comments (default: true). Set to false to disable posting to the no-op runs issue.", "default": true + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -7069,6 +7249,11 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false @@ -7111,6 +7296,11 @@ "type": "boolean", "description": "Controls whether AI-generated footer is added when updating the release body. When false, the visible footer content is omitted. Defaults to true.", "default": true + }, + "staged": { + "type": "boolean", + "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", + "examples": [true, false] } }, "additionalProperties": false diff --git a/pkg/workflow/compiler_safe_outputs_env.go b/pkg/workflow/compiler_safe_outputs_env.go index dc631f1cc07..78c12f39591 100644 --- a/pkg/workflow/compiler_safe_outputs_env.go +++ b/pkg/workflow/compiler_safe_outputs_env.go @@ -13,93 +13,147 @@ func (c *Compiler) addAllSafeOutputConfigEnvVars(steps *[]string, data *Workflow return } - // Track if we've already added staged flag to avoid duplicates - stagedFlagAdded := false + // Add the global staged env var once if staged mode is enabled, not in trial mode, + // and at least one configured handler targets the current repository. + if !c.trialMode && data.SafeOutputs.Staged && hasSafeOutputWithoutTargetRepo(data.SafeOutputs) { + *steps = append(*steps, " GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\n") + compilerSafeOutputsEnvLog.Print("Added staged flag") + } - // Create Issue env vars - target-repo, allowed_labels and allowed_repos now in config object + // Check if copilot is in create-issue assignees - if so, output issues for assign_to_agent job if data.SafeOutputs.CreateIssues != nil { - cfg := data.SafeOutputs.CreateIssues - compilerSafeOutputsEnvLog.Print("Processing create-issue env vars") - // Add staged flag if needed (but not if target-repo is specified or we're in trial mode) - if !c.trialMode && data.SafeOutputs.Staged && !stagedFlagAdded && cfg.TargetRepoSlug == "" { - *steps = append(*steps, " GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\n") - stagedFlagAdded = true - compilerSafeOutputsEnvLog.Print("Added staged flag for create-issue") - } - // Check if copilot is in assignees - if so, we'll output issues for assign_to_agent job - if hasCopilotAssignee(cfg.Assignees) { + if hasCopilotAssignee(data.SafeOutputs.CreateIssues.Assignees) { *steps = append(*steps, " GH_AW_ASSIGN_COPILOT: \"true\"\n") compilerSafeOutputsEnvLog.Print("Copilot assignment requested - will output issues_to_assign_copilot") } } - // Add Comment - all config now in handler config JSON - if data.SafeOutputs.AddComments != nil { - cfg := data.SafeOutputs.AddComments - // Add staged flag if needed (but not if target-repo is specified or we're in trial mode) - if !c.trialMode && data.SafeOutputs.Staged && !stagedFlagAdded && cfg.TargetRepoSlug == "" { - *steps = append(*steps, " GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\n") - stagedFlagAdded = true - } - // All add_comment configuration (target, target-repo, hide_older_comments, max) is now in handler config JSON - } + // Note: All handler configuration is read from the config.json file at runtime. +} - // Add Labels - all config now in handler config JSON - if data.SafeOutputs.AddLabels != nil { - cfg := data.SafeOutputs.AddLabels - // Add staged flag if needed (but not if target-repo is specified or we're in trial mode) - if !c.trialMode && data.SafeOutputs.Staged && !stagedFlagAdded && cfg.TargetRepoSlug == "" { - *steps = append(*steps, " GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\n") - stagedFlagAdded = true - } - // All add_labels configuration (allowed, max, target) is now in handler config JSON +// hasSafeOutputWithoutTargetRepo returns true if any configured safe output handler +// targets the current repository (i.e., has no target-repo specified). +// Handlers without a target-repo field always target the current repo. +func hasSafeOutputWithoutTargetRepo(so *SafeOutputsConfig) bool { + // Handlers without a target-repo field always target the current repo. + if so.AutofixCodeScanningAlert != nil { + return true } - - // Remove Labels - all config now in handler config JSON - if data.SafeOutputs.RemoveLabels != nil { - cfg := data.SafeOutputs.RemoveLabels - // Add staged flag if needed (but not if target-repo is specified or we're in trial mode) - if !c.trialMode && data.SafeOutputs.Staged && !stagedFlagAdded && cfg.TargetRepoSlug == "" { - *steps = append(*steps, " GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\n") - stagedFlagAdded = true - } - // All remove_labels configuration (allowed, max, target) is now in handler config JSON + if so.UploadAssets != nil { + return true } - - // Update Issue env vars - if data.SafeOutputs.UpdateIssues != nil { - cfg := data.SafeOutputs.UpdateIssues - // Add staged flag if needed (but not if target-repo is specified or we're in trial mode) - if !c.trialMode && data.SafeOutputs.Staged && !stagedFlagAdded && cfg.TargetRepoSlug == "" { - *steps = append(*steps, " GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\n") - stagedFlagAdded = true - } + if so.UpdateProjects != nil { + return true } - - // Update Discussion env vars - if data.SafeOutputs.UpdateDiscussions != nil { - cfg := data.SafeOutputs.UpdateDiscussions - // Add staged flag if needed (but not if target-repo is specified or we're in trial mode) - if !c.trialMode && data.SafeOutputs.Staged && !stagedFlagAdded && cfg.TargetRepoSlug == "" { - *steps = append(*steps, " GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\n") - stagedFlagAdded = true - } - // All update configuration (target, allow_title, allow_body, allow_labels) is now in handler config JSON + if so.CreateProjects != nil { + return true } - - // Create Pull Request env vars - if data.SafeOutputs.CreatePullRequests != nil { - // Add staged flag if needed - if !c.trialMode && data.SafeOutputs.Staged && !stagedFlagAdded { - *steps = append(*steps, " GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\n") - stagedFlagAdded = true - } - // Note: base_branch and max_patch_size are now in handler config JSON + if so.CreateProjectStatusUpdates != nil { + return true } - - if stagedFlagAdded { - _ = stagedFlagAdded // Mark as used for linter + if so.CallWorkflow != nil { + return true + } + if so.MissingTool != nil { + return true + } + if so.MissingData != nil { + return true + } + if so.NoOp != nil { + return true } - // Note: Most handlers read from the config.json file, so we may not need all env vars here + // Handlers with a target-repo field qualify only when no cross-repo target is specified. + if so.CreateIssues != nil && so.CreateIssues.TargetRepoSlug == "" { + return true + } + if so.CreateDiscussions != nil && so.CreateDiscussions.TargetRepoSlug == "" { + return true + } + if so.CloseDiscussions != nil && so.CloseDiscussions.TargetRepoSlug == "" { + return true + } + if so.CloseIssues != nil && so.CloseIssues.TargetRepoSlug == "" { + return true + } + if so.ClosePullRequests != nil && so.ClosePullRequests.TargetRepoSlug == "" { + return true + } + if so.MarkPullRequestAsReadyForReview != nil && so.MarkPullRequestAsReadyForReview.TargetRepoSlug == "" { + return true + } + if so.AddComments != nil && so.AddComments.TargetRepoSlug == "" { + return true + } + if so.CreatePullRequests != nil && so.CreatePullRequests.TargetRepoSlug == "" { + return true + } + if so.CreatePullRequestReviewComments != nil && so.CreatePullRequestReviewComments.TargetRepoSlug == "" { + return true + } + if so.SubmitPullRequestReview != nil && so.SubmitPullRequestReview.TargetRepoSlug == "" { + return true + } + if so.ReplyToPullRequestReviewComment != nil && so.ReplyToPullRequestReviewComment.TargetRepoSlug == "" { + return true + } + if so.ResolvePullRequestReviewThread != nil && so.ResolvePullRequestReviewThread.TargetRepoSlug == "" { + return true + } + if so.CreateCodeScanningAlerts != nil && so.CreateCodeScanningAlerts.TargetRepoSlug == "" { + return true + } + if so.AddLabels != nil && so.AddLabels.TargetRepoSlug == "" { + return true + } + if so.RemoveLabels != nil && so.RemoveLabels.TargetRepoSlug == "" { + return true + } + if so.AddReviewer != nil && so.AddReviewer.TargetRepoSlug == "" { + return true + } + if so.AssignMilestone != nil && so.AssignMilestone.TargetRepoSlug == "" { + return true + } + if so.AssignToAgent != nil && so.AssignToAgent.TargetRepoSlug == "" { + return true + } + if so.AssignToUser != nil && so.AssignToUser.TargetRepoSlug == "" { + return true + } + if so.UnassignFromUser != nil && so.UnassignFromUser.TargetRepoSlug == "" { + return true + } + if so.UpdateIssues != nil && so.UpdateIssues.TargetRepoSlug == "" { + return true + } + if so.UpdatePullRequests != nil && so.UpdatePullRequests.TargetRepoSlug == "" { + return true + } + if so.UpdateDiscussions != nil && so.UpdateDiscussions.TargetRepoSlug == "" { + return true + } + if so.UpdateRelease != nil && so.UpdateRelease.TargetRepoSlug == "" { + return true + } + if so.PushToPullRequestBranch != nil && so.PushToPullRequestBranch.TargetRepoSlug == "" { + return true + } + if so.HideComment != nil && so.HideComment.TargetRepoSlug == "" { + return true + } + if so.SetIssueType != nil && so.SetIssueType.TargetRepoSlug == "" { + return true + } + if so.DispatchWorkflow != nil && so.DispatchWorkflow.TargetRepoSlug == "" { + return true + } + if so.CreateAgentSessions != nil && so.CreateAgentSessions.TargetRepoSlug == "" { + return true + } + if so.LinkSubIssue != nil && so.LinkSubIssue.TargetRepoSlug == "" { + return true + } + return false } diff --git a/pkg/workflow/compiler_safe_outputs_env_test.go b/pkg/workflow/compiler_safe_outputs_env_test.go index 90c4157df83..8c72e260ada 100644 --- a/pkg/workflow/compiler_safe_outputs_env_test.go +++ b/pkg/workflow/compiler_safe_outputs_env_test.go @@ -515,3 +515,197 @@ func TestAddLabelsTargetRepoStagedBehavior(t *testing.T) { // Should not add staged flag when target-repo is set assert.NotContains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_STAGED") } + +// TestStagedFlagForAllHandlerTypes tests that the staged flag is emitted for all safe output handler types. +func TestStagedFlagForAllHandlerTypes(t *testing.T) { + tests := []struct { + name string + safeOutputs *SafeOutputsConfig + expectFlag bool + }{ + // Handlers without a target-repo field always qualify + { + name: "autofix-code-scanning-alert with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + AutofixCodeScanningAlert: &AutofixCodeScanningAlertConfig{}, + }, + expectFlag: true, + }, + { + name: "upload-asset with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + UploadAssets: &UploadAssetsConfig{}, + }, + expectFlag: true, + }, + { + name: "update-project with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + UpdateProjects: &UpdateProjectConfig{}, + }, + expectFlag: true, + }, + { + name: "create-project with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + CreateProjects: &CreateProjectsConfig{}, + }, + expectFlag: true, + }, + { + name: "create-project-status-update with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + CreateProjectStatusUpdates: &CreateProjectStatusUpdateConfig{}, + }, + expectFlag: true, + }, + { + name: "call-workflow with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + CallWorkflow: &CallWorkflowConfig{}, + }, + expectFlag: true, + }, + { + name: "noop with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + NoOp: &NoOpConfig{}, + }, + expectFlag: true, + }, + { + name: "missing-tool with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + MissingTool: &MissingToolConfig{}, + }, + expectFlag: true, + }, + { + name: "missing-data with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + MissingData: &MissingDataConfig{}, + }, + expectFlag: true, + }, + // Handlers with target-repo: qualify when no target-repo is set + { + name: "create-discussion without target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + CreateDiscussions: &CreateDiscussionsConfig{}, + }, + expectFlag: true, + }, + { + name: "create-discussion with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + CreateDiscussions: &CreateDiscussionsConfig{ + TargetRepoSlug: "org/other", + }, + }, + expectFlag: false, + }, + { + name: "close-issue without target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + CloseIssues: &CloseIssuesConfig{}, + }, + expectFlag: true, + }, + { + name: "close-pull-request without target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + ClosePullRequests: &ClosePullRequestsConfig{}, + }, + expectFlag: true, + }, + { + name: "mark-pr-ready-for-review without target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + MarkPullRequestAsReadyForReview: &MarkPullRequestAsReadyForReviewConfig{}, + }, + expectFlag: true, + }, + { + name: "create-pull-request-review-comment without target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + CreatePullRequestReviewComments: &CreatePullRequestReviewCommentsConfig{}, + }, + expectFlag: true, + }, + { + name: "submit-pull-request-review without target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + SubmitPullRequestReview: &SubmitPullRequestReviewConfig{}, + }, + expectFlag: true, + }, + { + name: "dispatch-workflow without target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + DispatchWorkflow: &DispatchWorkflowConfig{}, + }, + expectFlag: true, + }, + { + name: "dispatch-workflow with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + DispatchWorkflow: &DispatchWorkflowConfig{ + TargetRepoSlug: "org/other", + }, + }, + expectFlag: false, + }, + // Only cross-repo handlers configured: no staged flag + { + name: "only cross-repo handlers configured", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + CreateIssues: &CreateIssuesConfig{ + TargetRepoSlug: "org/repo", + }, + CreateDiscussions: &CreateDiscussionsConfig{ + TargetRepoSlug: "org/other", + }, + }, + expectFlag: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{ + Name: "Test Workflow", + SafeOutputs: tt.safeOutputs, + } + + var steps []string + compiler.addAllSafeOutputConfigEnvVars(&steps, workflowData) + stepsContent := strings.Join(steps, "") + + if tt.expectFlag { + assert.Contains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_STAGED", "Expected staged flag to be present") + } else { + assert.NotContains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_STAGED", "Expected staged flag to be absent") + } + }) + } +} From 52450e46b22aa1f0b81aa3df3623d81d58eaa0f1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 15:32:51 +0000 Subject: [PATCH 2/7] fix: clean up comments per code review feedback Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_safe_outputs_env.go | 4 ++-- pkg/workflow/compiler_safe_outputs_env_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/workflow/compiler_safe_outputs_env.go b/pkg/workflow/compiler_safe_outputs_env.go index 78c12f39591..33c3184e521 100644 --- a/pkg/workflow/compiler_safe_outputs_env.go +++ b/pkg/workflow/compiler_safe_outputs_env.go @@ -33,9 +33,9 @@ func (c *Compiler) addAllSafeOutputConfigEnvVars(steps *[]string, data *Workflow // hasSafeOutputWithoutTargetRepo returns true if any configured safe output handler // targets the current repository (i.e., has no target-repo specified). -// Handlers without a target-repo field always target the current repo. +// Handlers without a target-repo field always target the current repo, +// while handlers with a target-repo field qualify only when no cross-repo target is set. func hasSafeOutputWithoutTargetRepo(so *SafeOutputsConfig) bool { - // Handlers without a target-repo field always target the current repo. if so.AutofixCodeScanningAlert != nil { return true } diff --git a/pkg/workflow/compiler_safe_outputs_env_test.go b/pkg/workflow/compiler_safe_outputs_env_test.go index 8c72e260ada..3102666b54e 100644 --- a/pkg/workflow/compiler_safe_outputs_env_test.go +++ b/pkg/workflow/compiler_safe_outputs_env_test.go @@ -596,7 +596,7 @@ func TestStagedFlagForAllHandlerTypes(t *testing.T) { }, expectFlag: true, }, - // Handlers with target-repo: qualify when no target-repo is set + // Handlers with a target-repo field: qualify when no cross-repo target is set { name: "create-discussion without target-repo", safeOutputs: &SafeOutputsConfig{ From 026fc8df97d7a9ff7f061eac72890b6f15ad07a1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 16:56:56 +0000 Subject: [PATCH 3/7] fix: staged mode is independent of target-repo, add test cases for all handler types Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_safe_outputs_env.go | 75 ++-- .../compiler_safe_outputs_env_test.go | 393 ++++++++++++++++-- 2 files changed, 387 insertions(+), 81 deletions(-) diff --git a/pkg/workflow/compiler_safe_outputs_env.go b/pkg/workflow/compiler_safe_outputs_env.go index 33c3184e521..2e92edd7272 100644 --- a/pkg/workflow/compiler_safe_outputs_env.go +++ b/pkg/workflow/compiler_safe_outputs_env.go @@ -14,8 +14,8 @@ func (c *Compiler) addAllSafeOutputConfigEnvVars(steps *[]string, data *Workflow } // Add the global staged env var once if staged mode is enabled, not in trial mode, - // and at least one configured handler targets the current repository. - if !c.trialMode && data.SafeOutputs.Staged && hasSafeOutputWithoutTargetRepo(data.SafeOutputs) { + // and at least one handler is configured. Staged mode is independent of target-repo. + if !c.trialMode && data.SafeOutputs.Staged && hasSafeOutputConfigured(data.SafeOutputs) { *steps = append(*steps, " GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\n") compilerSafeOutputsEnvLog.Print("Added staged flag") } @@ -31,11 +31,10 @@ func (c *Compiler) addAllSafeOutputConfigEnvVars(steps *[]string, data *Workflow // Note: All handler configuration is read from the config.json file at runtime. } -// hasSafeOutputWithoutTargetRepo returns true if any configured safe output handler -// targets the current repository (i.e., has no target-repo specified). -// Handlers without a target-repo field always target the current repo, -// while handlers with a target-repo field qualify only when no cross-repo target is set. -func hasSafeOutputWithoutTargetRepo(so *SafeOutputsConfig) bool { +// hasSafeOutputConfigured returns true if any safe output handler is configured. +// Staged mode is independent of target-repo: it activates whenever staged is set +// and at least one handler is present. +func hasSafeOutputConfigured(so *SafeOutputsConfig) bool { if so.AutofixCodeScanningAlert != nil { return true } @@ -63,96 +62,94 @@ func hasSafeOutputWithoutTargetRepo(so *SafeOutputsConfig) bool { if so.NoOp != nil { return true } - - // Handlers with a target-repo field qualify only when no cross-repo target is specified. - if so.CreateIssues != nil && so.CreateIssues.TargetRepoSlug == "" { + if so.CreateIssues != nil { return true } - if so.CreateDiscussions != nil && so.CreateDiscussions.TargetRepoSlug == "" { + if so.CreateDiscussions != nil { return true } - if so.CloseDiscussions != nil && so.CloseDiscussions.TargetRepoSlug == "" { + if so.CloseDiscussions != nil { return true } - if so.CloseIssues != nil && so.CloseIssues.TargetRepoSlug == "" { + if so.CloseIssues != nil { return true } - if so.ClosePullRequests != nil && so.ClosePullRequests.TargetRepoSlug == "" { + if so.ClosePullRequests != nil { return true } - if so.MarkPullRequestAsReadyForReview != nil && so.MarkPullRequestAsReadyForReview.TargetRepoSlug == "" { + if so.MarkPullRequestAsReadyForReview != nil { return true } - if so.AddComments != nil && so.AddComments.TargetRepoSlug == "" { + if so.AddComments != nil { return true } - if so.CreatePullRequests != nil && so.CreatePullRequests.TargetRepoSlug == "" { + if so.CreatePullRequests != nil { return true } - if so.CreatePullRequestReviewComments != nil && so.CreatePullRequestReviewComments.TargetRepoSlug == "" { + if so.CreatePullRequestReviewComments != nil { return true } - if so.SubmitPullRequestReview != nil && so.SubmitPullRequestReview.TargetRepoSlug == "" { + if so.SubmitPullRequestReview != nil { return true } - if so.ReplyToPullRequestReviewComment != nil && so.ReplyToPullRequestReviewComment.TargetRepoSlug == "" { + if so.ReplyToPullRequestReviewComment != nil { return true } - if so.ResolvePullRequestReviewThread != nil && so.ResolvePullRequestReviewThread.TargetRepoSlug == "" { + if so.ResolvePullRequestReviewThread != nil { return true } - if so.CreateCodeScanningAlerts != nil && so.CreateCodeScanningAlerts.TargetRepoSlug == "" { + if so.CreateCodeScanningAlerts != nil { return true } - if so.AddLabels != nil && so.AddLabels.TargetRepoSlug == "" { + if so.AddLabels != nil { return true } - if so.RemoveLabels != nil && so.RemoveLabels.TargetRepoSlug == "" { + if so.RemoveLabels != nil { return true } - if so.AddReviewer != nil && so.AddReviewer.TargetRepoSlug == "" { + if so.AddReviewer != nil { return true } - if so.AssignMilestone != nil && so.AssignMilestone.TargetRepoSlug == "" { + if so.AssignMilestone != nil { return true } - if so.AssignToAgent != nil && so.AssignToAgent.TargetRepoSlug == "" { + if so.AssignToAgent != nil { return true } - if so.AssignToUser != nil && so.AssignToUser.TargetRepoSlug == "" { + if so.AssignToUser != nil { return true } - if so.UnassignFromUser != nil && so.UnassignFromUser.TargetRepoSlug == "" { + if so.UnassignFromUser != nil { return true } - if so.UpdateIssues != nil && so.UpdateIssues.TargetRepoSlug == "" { + if so.UpdateIssues != nil { return true } - if so.UpdatePullRequests != nil && so.UpdatePullRequests.TargetRepoSlug == "" { + if so.UpdatePullRequests != nil { return true } - if so.UpdateDiscussions != nil && so.UpdateDiscussions.TargetRepoSlug == "" { + if so.UpdateDiscussions != nil { return true } - if so.UpdateRelease != nil && so.UpdateRelease.TargetRepoSlug == "" { + if so.UpdateRelease != nil { return true } - if so.PushToPullRequestBranch != nil && so.PushToPullRequestBranch.TargetRepoSlug == "" { + if so.PushToPullRequestBranch != nil { return true } - if so.HideComment != nil && so.HideComment.TargetRepoSlug == "" { + if so.HideComment != nil { return true } - if so.SetIssueType != nil && so.SetIssueType.TargetRepoSlug == "" { + if so.SetIssueType != nil { return true } - if so.DispatchWorkflow != nil && so.DispatchWorkflow.TargetRepoSlug == "" { + if so.DispatchWorkflow != nil { return true } - if so.CreateAgentSessions != nil && so.CreateAgentSessions.TargetRepoSlug == "" { + if so.CreateAgentSessions != nil { return true } - if so.LinkSubIssue != nil && so.LinkSubIssue.TargetRepoSlug == "" { + if so.LinkSubIssue != nil { return true } return false diff --git a/pkg/workflow/compiler_safe_outputs_env_test.go b/pkg/workflow/compiler_safe_outputs_env_test.go index 3102666b54e..41fdac04622 100644 --- a/pkg/workflow/compiler_safe_outputs_env_test.go +++ b/pkg/workflow/compiler_safe_outputs_env_test.go @@ -132,7 +132,8 @@ func TestAddAllSafeOutputConfigEnvVars(t *testing.T) { }, }, { - name: "target-repo specified does not add staged flag", + // staged is independent of target-repo: staged flag is emitted even when target-repo is set + name: "target-repo specified still adds staged flag", safeOutputs: &SafeOutputsConfig{ Staged: true, CreateIssues: &CreateIssuesConfig{ @@ -140,8 +141,8 @@ func TestAddAllSafeOutputConfigEnvVars(t *testing.T) { TitlePrefix: "[Test] ", }, }, - checkNotContains: []string{ - "GH_AW_SAFE_OUTPUTS_STAGED", + checkContains: []string{ + "GH_AW_SAFE_OUTPUTS_STAGED: \"true\"", }, }, } @@ -222,22 +223,23 @@ func TestNoEnvVarsWhenNoSafeOutputs(t *testing.T) { assert.Empty(t, steps) } -// TestStagedFlagWithTargetRepo tests staged flag behavior with target-repo +// TestStagedFlagWithTargetRepo tests that staged flag is emitted regardless of target-repo func TestStagedFlagWithTargetRepo(t *testing.T) { tests := []struct { - name string - targetRepoSlug string - shouldAddFlag bool + name string + targetRepo string + shouldAddFlag bool }{ { - name: "no target-repo", - targetRepoSlug: "", - shouldAddFlag: true, + name: "no target-repo", + targetRepo: "", + shouldAddFlag: true, }, { - name: "with target-repo", - targetRepoSlug: "org/repo", - shouldAddFlag: false, + // staged is independent of target-repo + name: "with target-repo", + targetRepo: "org/repo", + shouldAddFlag: true, }, } @@ -250,7 +252,7 @@ func TestStagedFlagWithTargetRepo(t *testing.T) { SafeOutputs: &SafeOutputsConfig{ Staged: true, CreateIssues: &CreateIssuesConfig{ - TargetRepoSlug: tt.targetRepoSlug, + TargetRepoSlug: tt.targetRepo, }, }, } @@ -395,11 +397,11 @@ func TestEnvVarFormatting(t *testing.T) { // TestStagedFlagPrecedence tests staged flag behavior across different configurations func TestStagedFlagPrecedence(t *testing.T) { tests := []struct { - name string - staged bool - trialMode bool - targetRepoSlug string - expectFlag bool + name string + staged bool + trialMode bool + targetRepo string + expectFlag bool }{ { name: "staged true, no trial, no target-repo", @@ -414,10 +416,11 @@ func TestStagedFlagPrecedence(t *testing.T) { expectFlag: false, }, { - name: "staged true, target-repo set", - staged: true, - targetRepoSlug: "org/repo", - expectFlag: false, + // staged is independent of target-repo + name: "staged true, target-repo set", + staged: true, + targetRepo: "org/repo", + expectFlag: true, }, { name: "staged false", @@ -425,11 +428,12 @@ func TestStagedFlagPrecedence(t *testing.T) { expectFlag: false, }, { - name: "staged true, trial mode and target-repo", - staged: true, - trialMode: true, - targetRepoSlug: "org/repo", - expectFlag: false, + // trial mode suppresses staged regardless of target-repo + name: "staged true, trial mode and target-repo", + staged: true, + trialMode: true, + targetRepo: "org/repo", + expectFlag: false, }, } @@ -445,7 +449,7 @@ func TestStagedFlagPrecedence(t *testing.T) { SafeOutputs: &SafeOutputsConfig{ Staged: tt.staged, CreateIssues: &CreateIssuesConfig{ - TargetRepoSlug: tt.targetRepoSlug, + TargetRepoSlug: tt.targetRepo, }, }, } @@ -486,8 +490,8 @@ func TestAddCommentsTargetRepoStagedBehavior(t *testing.T) { stepsContent := strings.Join(steps, "") - // Should not add staged flag when target-repo is set - assert.NotContains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_STAGED") + // staged is independent of target-repo: flag is emitted even with target-repo set + assert.Contains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_STAGED") } // TestAddLabelsTargetRepoStagedBehavior tests staged flag behavior for add_labels with target-repo @@ -512,8 +516,8 @@ func TestAddLabelsTargetRepoStagedBehavior(t *testing.T) { stepsContent := strings.Join(steps, "") - // Should not add staged flag when target-repo is set - assert.NotContains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_STAGED") + // staged is independent of target-repo: flag is emitted even with target-repo set + assert.Contains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_STAGED") } // TestStagedFlagForAllHandlerTypes tests that the staged flag is emitted for all safe output handler types. @@ -523,7 +527,7 @@ func TestStagedFlagForAllHandlerTypes(t *testing.T) { safeOutputs *SafeOutputsConfig expectFlag bool }{ - // Handlers without a target-repo field always qualify + // Handlers without a target-repo field { name: "autofix-code-scanning-alert with staged", safeOutputs: &SafeOutputsConfig{ @@ -596,7 +600,7 @@ func TestStagedFlagForAllHandlerTypes(t *testing.T) { }, expectFlag: true, }, - // Handlers with a target-repo field: qualify when no cross-repo target is set + // Handlers with a target-repo field: staged is emitted regardless of target-repo value { name: "create-discussion without target-repo", safeOutputs: &SafeOutputsConfig{ @@ -613,7 +617,7 @@ func TestStagedFlagForAllHandlerTypes(t *testing.T) { TargetRepoSlug: "org/other", }, }, - expectFlag: false, + expectFlag: true, }, { name: "close-issue without target-repo", @@ -671,20 +675,325 @@ func TestStagedFlagForAllHandlerTypes(t *testing.T) { TargetRepoSlug: "org/other", }, }, - expectFlag: false, + expectFlag: true, + }, + // Handlers previously missing from test coverage + { + name: "close-discussion with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + CloseDiscussions: &CloseDiscussionsConfig{}, + }, + expectFlag: true, }, - // Only cross-repo handlers configured: no staged flag { - name: "only cross-repo handlers configured", + name: "close-discussion with target-repo", safeOutputs: &SafeOutputsConfig{ Staged: true, - CreateIssues: &CreateIssuesConfig{ - TargetRepoSlug: "org/repo", + CloseDiscussions: &CloseDiscussionsConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, }, - CreateDiscussions: &CreateDiscussionsConfig{ + }, + expectFlag: true, + }, + { + name: "add-reviewer with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + AddReviewer: &AddReviewerConfig{}, + }, + expectFlag: true, + }, + { + name: "add-reviewer with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + AddReviewer: &AddReviewerConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, + }, + }, + expectFlag: true, + }, + { + name: "assign-milestone with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + AssignMilestone: &AssignMilestoneConfig{}, + }, + expectFlag: true, + }, + { + name: "assign-milestone with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + AssignMilestone: &AssignMilestoneConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, + }, + }, + expectFlag: true, + }, + { + name: "assign-to-agent with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + AssignToAgent: &AssignToAgentConfig{}, + }, + expectFlag: true, + }, + { + name: "assign-to-agent with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + AssignToAgent: &AssignToAgentConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, + }, + }, + expectFlag: true, + }, + { + name: "assign-to-user with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + AssignToUser: &AssignToUserConfig{}, + }, + expectFlag: true, + }, + { + name: "assign-to-user with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + AssignToUser: &AssignToUserConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, + }, + }, + expectFlag: true, + }, + { + name: "unassign-from-user with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + UnassignFromUser: &UnassignFromUserConfig{}, + }, + expectFlag: true, + }, + { + name: "unassign-from-user with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + UnassignFromUser: &UnassignFromUserConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, + }, + }, + expectFlag: true, + }, + { + name: "remove-labels with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + RemoveLabels: &RemoveLabelsConfig{}, + }, + expectFlag: true, + }, + { + name: "remove-labels with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + RemoveLabels: &RemoveLabelsConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, + }, + }, + expectFlag: true, + }, + { + name: "create-code-scanning-alerts with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + CreateCodeScanningAlerts: &CreateCodeScanningAlertsConfig{}, + }, + expectFlag: true, + }, + { + name: "create-code-scanning-alerts with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + CreateCodeScanningAlerts: &CreateCodeScanningAlertsConfig{ TargetRepoSlug: "org/other", }, }, + expectFlag: true, + }, + { + name: "create-agent-session with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + CreateAgentSessions: &CreateAgentSessionConfig{}, + }, + expectFlag: true, + }, + { + name: "create-agent-session with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + CreateAgentSessions: &CreateAgentSessionConfig{ + TargetRepoSlug: "org/other", + }, + }, + expectFlag: true, + }, + { + name: "link-sub-issue with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + LinkSubIssue: &LinkSubIssueConfig{}, + }, + expectFlag: true, + }, + { + name: "link-sub-issue with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + LinkSubIssue: &LinkSubIssueConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, + }, + }, + expectFlag: true, + }, + { + name: "hide-comment with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + HideComment: &HideCommentConfig{}, + }, + expectFlag: true, + }, + { + name: "hide-comment with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + HideComment: &HideCommentConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, + }, + }, + expectFlag: true, + }, + { + name: "set-issue-type with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + SetIssueType: &SetIssueTypeConfig{}, + }, + expectFlag: true, + }, + { + name: "set-issue-type with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + SetIssueType: &SetIssueTypeConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, + }, + }, + expectFlag: true, + }, + { + name: "reply-to-pull-request-review-comment with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + ReplyToPullRequestReviewComment: &ReplyToPullRequestReviewCommentConfig{}, + }, + expectFlag: true, + }, + { + name: "reply-to-pull-request-review-comment with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + ReplyToPullRequestReviewComment: &ReplyToPullRequestReviewCommentConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, + }, + }, + expectFlag: true, + }, + { + name: "resolve-pull-request-review-thread with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + ResolvePullRequestReviewThread: &ResolvePullRequestReviewThreadConfig{}, + }, + expectFlag: true, + }, + { + name: "resolve-pull-request-review-thread with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + ResolvePullRequestReviewThread: &ResolvePullRequestReviewThreadConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, + }, + }, + expectFlag: true, + }, + { + name: "push-to-pull-request-branch with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + PushToPullRequestBranch: &PushToPullRequestBranchConfig{}, + }, + expectFlag: true, + }, + { + name: "push-to-pull-request-branch with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + PushToPullRequestBranch: &PushToPullRequestBranchConfig{ + TargetRepoSlug: "org/other", + }, + }, + expectFlag: true, + }, + { + name: "update-pull-request with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + UpdatePullRequests: &UpdatePullRequestsConfig{}, + }, + expectFlag: true, + }, + { + name: "update-pull-request with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + UpdatePullRequests: &UpdatePullRequestsConfig{ + UpdateEntityConfig: UpdateEntityConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, + }, + }, + }, + expectFlag: true, + }, + { + name: "update-release with staged", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + UpdateRelease: &UpdateReleaseConfig{}, + }, + expectFlag: true, + }, + { + name: "update-release with target-repo", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + UpdateRelease: &UpdateReleaseConfig{ + UpdateEntityConfig: UpdateEntityConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, + }, + }, + }, + expectFlag: true, + }, + // Staged not emitted when no handlers are configured + { + name: "no handlers configured", + safeOutputs: &SafeOutputsConfig{ + Staged: true, + }, expectFlag: false, }, } From 1b120cef8ae1ba959830a5e7b70020ac0d3388bc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 20:08:53 +0000 Subject: [PATCH 4/7] test: add integration tests and workflow files for staged safe-outputs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/compile_integration_test.go | 234 ++++++++++++++++++ pkg/cli/workflows/test-staged-add-comment.md | 16 ++ .../test-staged-create-discussion.md | 18 ++ pkg/cli/workflows/test-staged-create-issue.md | 17 ++ pkg/cli/workflows/test-staged-cross-repo.md | 19 ++ 5 files changed, 304 insertions(+) create mode 100644 pkg/cli/workflows/test-staged-add-comment.md create mode 100644 pkg/cli/workflows/test-staged-create-discussion.md create mode 100644 pkg/cli/workflows/test-staged-create-issue.md create mode 100644 pkg/cli/workflows/test-staged-cross-repo.md diff --git a/pkg/cli/compile_integration_test.go b/pkg/cli/compile_integration_test.go index 968c87486e0..b38477598f1 100644 --- a/pkg/cli/compile_integration_test.go +++ b/pkg/cli/compile_integration_test.go @@ -802,6 +802,240 @@ This workflow tests that invalid schedule strings in array format fail compilati t.Logf("Integration test passed - invalid schedule in array format correctly failed compilation\nOutput: %s", outputStr) } +// TestCompileStagedSafeOutputsCreateIssue verifies that a workflow with staged: true +// and a create-issue handler compiles without error and emits GH_AW_SAFE_OUTPUTS_STAGED. +// Prior to the schema fix, staged was not listed in the create-issue schema +// (additionalProperties: false), so the frontmatter validator would reject the workflow. +func TestCompileStagedSafeOutputsCreateIssue(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + testWorkflow := `--- +name: Staged Create Issue +on: + workflow_dispatch: +permissions: read-all +engine: copilot +safe-outputs: + staged: true + create-issue: + title-prefix: "[staged] " + max: 1 +--- + +Verify staged safe-outputs with create-issue. +` + testWorkflowPath := filepath.Join(setup.workflowsDir, "staged-create-issue.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, "staged-create-issue.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContentStr := string(lockContent) + + if !strings.Contains(lockContentStr, `GH_AW_SAFE_OUTPUTS_STAGED: "true"`) { + t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\nLock file content:\n%s", lockContentStr) + } +} + +// TestCompileStagedSafeOutputsAddComment verifies that a workflow with staged: true +// and an add-comment handler compiles and emits GH_AW_SAFE_OUTPUTS_STAGED. +// Prior to the schema fix, staged was not listed in the add-comment handler schema. +func TestCompileStagedSafeOutputsAddComment(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + testWorkflow := `--- +name: Staged Add Comment +on: + workflow_dispatch: +permissions: read-all +engine: copilot +safe-outputs: + staged: true + add-comment: + max: 1 +--- + +Verify staged safe-outputs with add-comment. +` + testWorkflowPath := filepath.Join(setup.workflowsDir, "staged-add-comment.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, "staged-add-comment.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContentStr := string(lockContent) + + if !strings.Contains(lockContentStr, `GH_AW_SAFE_OUTPUTS_STAGED: "true"`) { + t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\nLock file content:\n%s", lockContentStr) + } +} + +// TestCompileStagedSafeOutputsCreateDiscussion verifies that a workflow with staged: true +// and a create-discussion handler compiles and emits GH_AW_SAFE_OUTPUTS_STAGED. +// Prior to the schema fix, staged was not listed in the create-discussion handler schema. +func TestCompileStagedSafeOutputsCreateDiscussion(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + testWorkflow := `--- +name: Staged Create Discussion +on: + workflow_dispatch: +permissions: + contents: read +engine: copilot +safe-outputs: + staged: true + create-discussion: + max: 1 + category: general +--- + +Verify staged safe-outputs with create-discussion. +` + testWorkflowPath := filepath.Join(setup.workflowsDir, "staged-create-discussion.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, "staged-create-discussion.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContentStr := string(lockContent) + + if !strings.Contains(lockContentStr, `GH_AW_SAFE_OUTPUTS_STAGED: "true"`) { + t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\nLock file content:\n%s", lockContentStr) + } +} + +// TestCompileStagedSafeOutputsWithTargetRepo verifies that staged: true emits +// GH_AW_SAFE_OUTPUTS_STAGED even when a target-repo is specified on the handler. +// Staged mode is independent of target-repo. +func TestCompileStagedSafeOutputsWithTargetRepo(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + testWorkflow := `--- +name: Staged Cross-Repo +on: + workflow_dispatch: +permissions: read-all +engine: copilot +safe-outputs: + staged: true + create-issue: + title-prefix: "[cross-repo staged] " + max: 1 + target-repo: org/other-repo +--- + +Verify that staged mode is independent of target-repo. +` + testWorkflowPath := filepath.Join(setup.workflowsDir, "staged-cross-repo.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, "staged-cross-repo.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContentStr := string(lockContent) + + // staged is independent of target-repo: env var must be present + if !strings.Contains(lockContentStr, `GH_AW_SAFE_OUTPUTS_STAGED: "true"`) { + t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUTS_STAGED: \"true\" even with target-repo set\nLock file content:\n%s", lockContentStr) + } +} + +// TestCompileStagedSafeOutputsMultipleHandlers verifies that staged: true with +// multiple handler types compiles and emits GH_AW_SAFE_OUTPUTS_STAGED exactly once. +// Previously, adding staged to most handler types caused a schema validation error. +func TestCompileStagedSafeOutputsMultipleHandlers(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + testWorkflow := `--- +name: Staged Multiple Handlers +on: + workflow_dispatch: +permissions: read-all +engine: copilot +safe-outputs: + staged: true + create-issue: + title-prefix: "[staged] " + max: 1 + add-comment: + max: 2 + add-labels: + allowed: + - bug + update-issue: +--- + +Verify staged safe-outputs with multiple handler types. +` + testWorkflowPath := filepath.Join(setup.workflowsDir, "staged-multi-handler.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, "staged-multi-handler.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContentStr := string(lockContent) + + if !strings.Contains(lockContentStr, `GH_AW_SAFE_OUTPUTS_STAGED: "true"`) { + t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\nLock file content:\n%s", lockContentStr) + } +} + // TestCompileFromSubdirectoryCreatesActionsLockAtRoot tests that actions-lock.json // is created at the repository root when compiling from a subdirectory func TestCompileFromSubdirectoryCreatesActionsLockAtRoot(t *testing.T) { diff --git a/pkg/cli/workflows/test-staged-add-comment.md b/pkg/cli/workflows/test-staged-add-comment.md new file mode 100644 index 00000000000..60776522b81 --- /dev/null +++ b/pkg/cli/workflows/test-staged-add-comment.md @@ -0,0 +1,16 @@ +--- +on: + workflow_dispatch: +permissions: read-all +engine: copilot +safe-outputs: + staged: true + add-comment: + max: 1 +--- + +# Test Staged Add Comment + +Verify that `staged: true` works together with the `add-comment` handler. + +Add a comment to issue #1 saying "Staged test comment." diff --git a/pkg/cli/workflows/test-staged-create-discussion.md b/pkg/cli/workflows/test-staged-create-discussion.md new file mode 100644 index 00000000000..69e8ac2fede --- /dev/null +++ b/pkg/cli/workflows/test-staged-create-discussion.md @@ -0,0 +1,18 @@ +--- +on: + workflow_dispatch: +permissions: + contents: read +engine: copilot +safe-outputs: + staged: true + create-discussion: + max: 1 + category: general +--- + +# Test Staged Create Discussion + +Verify that `staged: true` works together with the `create-discussion` handler. + +Create a discussion titled "Staged test discussion" with the body "This discussion was created in staged mode." diff --git a/pkg/cli/workflows/test-staged-create-issue.md b/pkg/cli/workflows/test-staged-create-issue.md new file mode 100644 index 00000000000..a74deda5438 --- /dev/null +++ b/pkg/cli/workflows/test-staged-create-issue.md @@ -0,0 +1,17 @@ +--- +on: + workflow_dispatch: +permissions: read-all +engine: copilot +safe-outputs: + staged: true + create-issue: + title-prefix: "[staged] " + max: 1 +--- + +# Test Staged Create Issue + +Verify that `staged: true` works together with the `create-issue` handler. + +Create an issue titled "Staged test issue" with the body "This issue was created in staged mode." diff --git a/pkg/cli/workflows/test-staged-cross-repo.md b/pkg/cli/workflows/test-staged-cross-repo.md new file mode 100644 index 00000000000..5fb597faa50 --- /dev/null +++ b/pkg/cli/workflows/test-staged-cross-repo.md @@ -0,0 +1,19 @@ +--- +on: + workflow_dispatch: +permissions: read-all +engine: copilot +safe-outputs: + staged: true + create-issue: + title-prefix: "[cross-repo staged] " + max: 1 + target-repo: org/other-repo +--- + +# Test Staged Create Issue Cross-Repo + +Verify that `staged: true` is emitted even when a `target-repo` is configured. +`staged` mode is independent of the `target-repo` setting. + +Create an issue in the target repository titled "Cross-repo staged test issue." From 0f140f38d7fcb50eb0829cba9b49f0c69168620f Mon Sep 17 00:00:00 2001 From: Peli de Halleux Date: Tue, 17 Mar 2026 14:03:37 -0700 Subject: [PATCH 5/7] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- pkg/workflow/compiler_safe_outputs_env.go | 32 ++--------------------- 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/pkg/workflow/compiler_safe_outputs_env.go b/pkg/workflow/compiler_safe_outputs_env.go index 2e92edd7272..3ba5288509c 100644 --- a/pkg/workflow/compiler_safe_outputs_env.go +++ b/pkg/workflow/compiler_safe_outputs_env.go @@ -35,36 +35,8 @@ func (c *Compiler) addAllSafeOutputConfigEnvVars(steps *[]string, data *Workflow // Staged mode is independent of target-repo: it activates whenever staged is set // and at least one handler is present. func hasSafeOutputConfigured(so *SafeOutputsConfig) bool { - if so.AutofixCodeScanningAlert != nil { - return true - } - if so.UploadAssets != nil { - return true - } - if so.UpdateProjects != nil { - return true - } - if so.CreateProjects != nil { - return true - } - if so.CreateProjectStatusUpdates != nil { - return true - } - if so.CallWorkflow != nil { - return true - } - if so.MissingTool != nil { - return true - } - if so.MissingData != nil { - return true - } - if so.NoOp != nil { - return true - } - if so.CreateIssues != nil { - return true - } + return hasAnySafeOutputEnabled(so) +} if so.CreateDiscussions != nil { return true } From a84b101fb3c6866bf92e4a9bad73bc7ff6b3acae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 21:12:17 +0000 Subject: [PATCH 6/7] refactor: remove hasSafeOutputConfigured wrapper, drive test from safeOutputFieldMapping Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_safe_outputs_env.go | 98 +--- .../compiler_safe_outputs_env_test.go | 524 ++---------------- 2 files changed, 41 insertions(+), 581 deletions(-) diff --git a/pkg/workflow/compiler_safe_outputs_env.go b/pkg/workflow/compiler_safe_outputs_env.go index 3ba5288509c..8d5d316f697 100644 --- a/pkg/workflow/compiler_safe_outputs_env.go +++ b/pkg/workflow/compiler_safe_outputs_env.go @@ -15,7 +15,7 @@ func (c *Compiler) addAllSafeOutputConfigEnvVars(steps *[]string, data *Workflow // Add the global staged env var once if staged mode is enabled, not in trial mode, // and at least one handler is configured. Staged mode is independent of target-repo. - if !c.trialMode && data.SafeOutputs.Staged && hasSafeOutputConfigured(data.SafeOutputs) { + if !c.trialMode && data.SafeOutputs.Staged && hasAnySafeOutputEnabled(data.SafeOutputs) { *steps = append(*steps, " GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\n") compilerSafeOutputsEnvLog.Print("Added staged flag") } @@ -30,99 +30,3 @@ func (c *Compiler) addAllSafeOutputConfigEnvVars(steps *[]string, data *Workflow // Note: All handler configuration is read from the config.json file at runtime. } - -// hasSafeOutputConfigured returns true if any safe output handler is configured. -// Staged mode is independent of target-repo: it activates whenever staged is set -// and at least one handler is present. -func hasSafeOutputConfigured(so *SafeOutputsConfig) bool { - return hasAnySafeOutputEnabled(so) -} - if so.CreateDiscussions != nil { - return true - } - if so.CloseDiscussions != nil { - return true - } - if so.CloseIssues != nil { - return true - } - if so.ClosePullRequests != nil { - return true - } - if so.MarkPullRequestAsReadyForReview != nil { - return true - } - if so.AddComments != nil { - return true - } - if so.CreatePullRequests != nil { - return true - } - if so.CreatePullRequestReviewComments != nil { - return true - } - if so.SubmitPullRequestReview != nil { - return true - } - if so.ReplyToPullRequestReviewComment != nil { - return true - } - if so.ResolvePullRequestReviewThread != nil { - return true - } - if so.CreateCodeScanningAlerts != nil { - return true - } - if so.AddLabels != nil { - return true - } - if so.RemoveLabels != nil { - return true - } - if so.AddReviewer != nil { - return true - } - if so.AssignMilestone != nil { - return true - } - if so.AssignToAgent != nil { - return true - } - if so.AssignToUser != nil { - return true - } - if so.UnassignFromUser != nil { - return true - } - if so.UpdateIssues != nil { - return true - } - if so.UpdatePullRequests != nil { - return true - } - if so.UpdateDiscussions != nil { - return true - } - if so.UpdateRelease != nil { - return true - } - if so.PushToPullRequestBranch != nil { - return true - } - if so.HideComment != nil { - return true - } - if so.SetIssueType != nil { - return true - } - if so.DispatchWorkflow != nil { - return true - } - if so.CreateAgentSessions != nil { - return true - } - if so.LinkSubIssue != nil { - return true - } - return false -} diff --git a/pkg/workflow/compiler_safe_outputs_env_test.go b/pkg/workflow/compiler_safe_outputs_env_test.go index 41fdac04622..75e4849eee8 100644 --- a/pkg/workflow/compiler_safe_outputs_env_test.go +++ b/pkg/workflow/compiler_safe_outputs_env_test.go @@ -3,6 +3,7 @@ package workflow import ( + "reflect" "strings" "testing" @@ -520,501 +521,56 @@ func TestAddLabelsTargetRepoStagedBehavior(t *testing.T) { assert.Contains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_STAGED") } -// TestStagedFlagForAllHandlerTypes tests that the staged flag is emitted for all safe output handler types. +// TestStagedFlagForAllHandlerTypes tests that the staged flag is emitted for every handler type +// registered in safeOutputFieldMapping. The test cases are generated via reflection so the test +// stays complete automatically when new handler types are added to safeOutputFieldMapping. func TestStagedFlagForAllHandlerTypes(t *testing.T) { - tests := []struct { - name string - safeOutputs *SafeOutputsConfig - expectFlag bool - }{ - // Handlers without a target-repo field - { - name: "autofix-code-scanning-alert with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - AutofixCodeScanningAlert: &AutofixCodeScanningAlertConfig{}, - }, - expectFlag: true, - }, - { - name: "upload-asset with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - UploadAssets: &UploadAssetsConfig{}, - }, - expectFlag: true, - }, - { - name: "update-project with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - UpdateProjects: &UpdateProjectConfig{}, - }, - expectFlag: true, - }, - { - name: "create-project with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - CreateProjects: &CreateProjectsConfig{}, - }, - expectFlag: true, - }, - { - name: "create-project-status-update with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - CreateProjectStatusUpdates: &CreateProjectStatusUpdateConfig{}, - }, - expectFlag: true, - }, - { - name: "call-workflow with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - CallWorkflow: &CallWorkflowConfig{}, - }, - expectFlag: true, - }, - { - name: "noop with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - NoOp: &NoOpConfig{}, - }, - expectFlag: true, - }, - { - name: "missing-tool with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - MissingTool: &MissingToolConfig{}, - }, - expectFlag: true, - }, - { - name: "missing-data with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - MissingData: &MissingDataConfig{}, - }, - expectFlag: true, - }, - // Handlers with a target-repo field: staged is emitted regardless of target-repo value - { - name: "create-discussion without target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - CreateDiscussions: &CreateDiscussionsConfig{}, - }, - expectFlag: true, - }, - { - name: "create-discussion with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - CreateDiscussions: &CreateDiscussionsConfig{ - TargetRepoSlug: "org/other", - }, - }, - expectFlag: true, - }, - { - name: "close-issue without target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - CloseIssues: &CloseIssuesConfig{}, - }, - expectFlag: true, - }, - { - name: "close-pull-request without target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - ClosePullRequests: &ClosePullRequestsConfig{}, - }, - expectFlag: true, - }, - { - name: "mark-pr-ready-for-review without target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - MarkPullRequestAsReadyForReview: &MarkPullRequestAsReadyForReviewConfig{}, - }, - expectFlag: true, - }, - { - name: "create-pull-request-review-comment without target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - CreatePullRequestReviewComments: &CreatePullRequestReviewCommentsConfig{}, - }, - expectFlag: true, - }, - { - name: "submit-pull-request-review without target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - SubmitPullRequestReview: &SubmitPullRequestReviewConfig{}, - }, - expectFlag: true, - }, - { - name: "dispatch-workflow without target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - DispatchWorkflow: &DispatchWorkflowConfig{}, - }, - expectFlag: true, - }, - { - name: "dispatch-workflow with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - DispatchWorkflow: &DispatchWorkflowConfig{ - TargetRepoSlug: "org/other", - }, - }, - expectFlag: true, - }, - // Handlers previously missing from test coverage - { - name: "close-discussion with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - CloseDiscussions: &CloseDiscussionsConfig{}, - }, - expectFlag: true, - }, - { - name: "close-discussion with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - CloseDiscussions: &CloseDiscussionsConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - expectFlag: true, - }, - { - name: "add-reviewer with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - AddReviewer: &AddReviewerConfig{}, - }, - expectFlag: true, - }, - { - name: "add-reviewer with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - AddReviewer: &AddReviewerConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - expectFlag: true, - }, - { - name: "assign-milestone with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - AssignMilestone: &AssignMilestoneConfig{}, - }, - expectFlag: true, - }, - { - name: "assign-milestone with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - AssignMilestone: &AssignMilestoneConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - expectFlag: true, - }, - { - name: "assign-to-agent with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - AssignToAgent: &AssignToAgentConfig{}, - }, - expectFlag: true, - }, - { - name: "assign-to-agent with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - AssignToAgent: &AssignToAgentConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - expectFlag: true, - }, - { - name: "assign-to-user with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - AssignToUser: &AssignToUserConfig{}, - }, - expectFlag: true, - }, - { - name: "assign-to-user with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - AssignToUser: &AssignToUserConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - expectFlag: true, - }, - { - name: "unassign-from-user with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - UnassignFromUser: &UnassignFromUserConfig{}, - }, - expectFlag: true, - }, - { - name: "unassign-from-user with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - UnassignFromUser: &UnassignFromUserConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - expectFlag: true, - }, - { - name: "remove-labels with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - RemoveLabels: &RemoveLabelsConfig{}, - }, - expectFlag: true, - }, - { - name: "remove-labels with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - RemoveLabels: &RemoveLabelsConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - expectFlag: true, - }, - { - name: "create-code-scanning-alerts with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - CreateCodeScanningAlerts: &CreateCodeScanningAlertsConfig{}, - }, - expectFlag: true, - }, - { - name: "create-code-scanning-alerts with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - CreateCodeScanningAlerts: &CreateCodeScanningAlertsConfig{ - TargetRepoSlug: "org/other", - }, - }, - expectFlag: true, - }, - { - name: "create-agent-session with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - CreateAgentSessions: &CreateAgentSessionConfig{}, - }, - expectFlag: true, - }, - { - name: "create-agent-session with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - CreateAgentSessions: &CreateAgentSessionConfig{ - TargetRepoSlug: "org/other", - }, - }, - expectFlag: true, - }, - { - name: "link-sub-issue with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - LinkSubIssue: &LinkSubIssueConfig{}, - }, - expectFlag: true, - }, - { - name: "link-sub-issue with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - LinkSubIssue: &LinkSubIssueConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - expectFlag: true, - }, - { - name: "hide-comment with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - HideComment: &HideCommentConfig{}, - }, - expectFlag: true, - }, - { - name: "hide-comment with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - HideComment: &HideCommentConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - expectFlag: true, - }, - { - name: "set-issue-type with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - SetIssueType: &SetIssueTypeConfig{}, - }, - expectFlag: true, - }, - { - name: "set-issue-type with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - SetIssueType: &SetIssueTypeConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - expectFlag: true, - }, - { - name: "reply-to-pull-request-review-comment with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - ReplyToPullRequestReviewComment: &ReplyToPullRequestReviewCommentConfig{}, - }, - expectFlag: true, - }, - { - name: "reply-to-pull-request-review-comment with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - ReplyToPullRequestReviewComment: &ReplyToPullRequestReviewCommentConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - expectFlag: true, - }, - { - name: "resolve-pull-request-review-thread with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - ResolvePullRequestReviewThread: &ResolvePullRequestReviewThreadConfig{}, - }, - expectFlag: true, - }, - { - name: "resolve-pull-request-review-thread with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - ResolvePullRequestReviewThread: &ResolvePullRequestReviewThreadConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - expectFlag: true, - }, - { - name: "push-to-pull-request-branch with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - PushToPullRequestBranch: &PushToPullRequestBranchConfig{}, - }, - expectFlag: true, - }, - { - name: "push-to-pull-request-branch with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - PushToPullRequestBranch: &PushToPullRequestBranchConfig{ - TargetRepoSlug: "org/other", - }, - }, - expectFlag: true, - }, - { - name: "update-pull-request with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - UpdatePullRequests: &UpdatePullRequestsConfig{}, - }, - expectFlag: true, - }, - { - name: "update-pull-request with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - UpdatePullRequests: &UpdatePullRequestsConfig{ - UpdateEntityConfig: UpdateEntityConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - }, - expectFlag: true, - }, - { - name: "update-release with staged", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - UpdateRelease: &UpdateReleaseConfig{}, - }, - expectFlag: true, - }, - { - name: "update-release with target-repo", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - UpdateRelease: &UpdateReleaseConfig{ - UpdateEntityConfig: UpdateEntityConfig{ - SafeOutputTargetConfig: SafeOutputTargetConfig{TargetRepoSlug: "org/other"}, - }, - }, - }, - expectFlag: true, - }, - // Staged not emitted when no handlers are configured - { - name: "no handlers configured", - safeOutputs: &SafeOutputsConfig{ - Staged: true, - }, - expectFlag: false, - }, - } + soType := reflect.TypeFor[SafeOutputsConfig]() + + // One sub-test per field registered in safeOutputFieldMapping. + // Each sub-test sets staged:true + that one handler, and verifies the env var is emitted. + for fieldName := range safeOutputFieldMapping { + t.Run(fieldName, func(t *testing.T) { + f, ok := soType.FieldByName(fieldName) + require.True(t, ok, "safeOutputFieldMapping references field %q which does not exist in SafeOutputsConfig", fieldName) + + so := &SafeOutputsConfig{Staged: true} + soVal := reflect.ValueOf(so).Elem() + field := soVal.FieldByName(fieldName) + require.True(t, field.IsValid(), "Field %q not found in SafeOutputsConfig", fieldName) + require.Equal(t, reflect.Pointer, field.Kind(), "Expected pointer field for %q, got %v", fieldName, f.Type) + + // Set the field to a zero-valued instance of the target struct. + field.Set(reflect.New(field.Type().Elem())) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { compiler := NewCompiler() workflowData := &WorkflowData{ Name: "Test Workflow", - SafeOutputs: tt.safeOutputs, + SafeOutputs: so, } var steps []string compiler.addAllSafeOutputConfigEnvVars(&steps, workflowData) stepsContent := strings.Join(steps, "") - if tt.expectFlag { - assert.Contains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_STAGED", "Expected staged flag to be present") - } else { - assert.NotContains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_STAGED", "Expected staged flag to be absent") - } + assert.Contains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_STAGED", + "Expected staged flag to be emitted for handler %q", fieldName) }) } + + // Verify the flag is not emitted when staged is set but no handler is configured. + t.Run("no handlers configured", func(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{ + Name: "Test Workflow", + SafeOutputs: &SafeOutputsConfig{Staged: true}, + } + + var steps []string + compiler.addAllSafeOutputConfigEnvVars(&steps, workflowData) + stepsContent := strings.Join(steps, "") + + assert.NotContains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_STAGED", + "Staged flag should not be emitted when no handlers are configured") + }) } From d32bd60a999e00275bfd862988c1734d55778e80 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 17 Mar 2026 22:05:55 +0000 Subject: [PATCH 7/7] Add changeset [skip-ci] --- .changeset/patch-fix-safe-outputs-staged-handlers.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/patch-fix-safe-outputs-staged-handlers.md diff --git a/.changeset/patch-fix-safe-outputs-staged-handlers.md b/.changeset/patch-fix-safe-outputs-staged-handlers.md new file mode 100644 index 00000000000..9616f405dba --- /dev/null +++ b/.changeset/patch-fix-safe-outputs-staged-handlers.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Fix safe-outputs staged mode so it validates and compiles correctly for all handler types, including cross-repo configurations.