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. diff --git a/.github/workflows/daily-choice-test.lock.yml b/.github/workflows/daily-choice-test.lock.yml index 81b8b12ff8a..c5cf89b96e2 100644 --- a/.github/workflows/daily-choice-test.lock.yml +++ b/.github/workflows/daily-choice-test.lock.yml @@ -1060,6 +1060,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/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." diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 33b61993573..dcc0936b137 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 @@ -4722,6 +4737,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, @@ -4837,6 +4857,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 @@ -4886,6 +4911,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, @@ -5006,6 +5036,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, @@ -5087,6 +5122,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, @@ -5159,6 +5199,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 @@ -5216,6 +5261,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, @@ -5347,6 +5397,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, @@ -5441,6 +5496,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, @@ -5612,6 +5672,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, @@ -5680,6 +5745,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 @@ -5744,6 +5814,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 @@ -5799,6 +5874,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 @@ -5834,6 +5914,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 @@ -5883,6 +5968,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 @@ -5917,6 +6007,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 @@ -5988,6 +6083,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 @@ -6055,6 +6155,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 @@ -6106,6 +6211,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 @@ -6153,6 +6263,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 @@ -6238,6 +6353,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 @@ -6306,6 +6426,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 @@ -6369,6 +6494,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 @@ -6434,6 +6564,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 @@ -6502,6 +6637,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 @@ -6563,6 +6703,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 @@ -6718,6 +6863,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 @@ -6776,6 +6926,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 @@ -6826,6 +6981,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"], @@ -6879,6 +7039,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"], @@ -6938,6 +7103,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 @@ -6995,6 +7165,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 @@ -7040,6 +7215,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 @@ -7100,6 +7280,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 @@ -7142,6 +7327,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..8d5d316f697 100644 --- a/pkg/workflow/compiler_safe_outputs_env.go +++ b/pkg/workflow/compiler_safe_outputs_env.go @@ -13,93 +13,20 @@ 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 handler is configured. Staged mode is independent of target-repo. + if !c.trialMode && data.SafeOutputs.Staged && hasAnySafeOutputEnabled(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 - } - - // 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 - } - - // 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 - } - - // 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 - } - } - - // 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 - } - - // 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 stagedFlagAdded { - _ = stagedFlagAdded // Mark as used for linter - } - - // Note: Most handlers read from the config.json file, so we may not need all env vars here + // Note: All handler configuration is read from the config.json file at runtime. } diff --git a/pkg/workflow/compiler_safe_outputs_env_test.go b/pkg/workflow/compiler_safe_outputs_env_test.go index 90c4157df83..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" @@ -132,7 +133,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 +142,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 +224,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 +253,7 @@ func TestStagedFlagWithTargetRepo(t *testing.T) { SafeOutputs: &SafeOutputsConfig{ Staged: true, CreateIssues: &CreateIssuesConfig{ - TargetRepoSlug: tt.targetRepoSlug, + TargetRepoSlug: tt.targetRepo, }, }, } @@ -395,11 +398,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 +417,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 +429,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 +450,7 @@ func TestStagedFlagPrecedence(t *testing.T) { SafeOutputs: &SafeOutputsConfig{ Staged: tt.staged, CreateIssues: &CreateIssuesConfig{ - TargetRepoSlug: tt.targetRepoSlug, + TargetRepoSlug: tt.targetRepo, }, }, } @@ -486,8 +491,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,6 +517,60 @@ 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 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) { + 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())) + + compiler := NewCompiler() + workflowData := &WorkflowData{ + Name: "Test Workflow", + SafeOutputs: so, + } + + var steps []string + compiler.addAllSafeOutputConfigEnvVars(&steps, workflowData) + stepsContent := strings.Join(steps, "") + + 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") + }) }