Skip to content

Commit 9758a19

Browse files
CopilotpelikhanCopilotgithub-actions[bot]
authored
fix: ensure safe-outputs staged mode works for all handler types (#21414)
* 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> * fix: clean up comments per code review feedback Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * 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> * test: add integration tests and workflow files for staged safe-outputs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * refactor: remove hasSafeOutputConfigured wrapper, drive test from safeOutputFieldMapping Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Add changeset [skip-ci] --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 30b2873 commit 9758a19

10 files changed

+601
-115
lines changed

‎.changeset/patch-fix-safe-outputs-staged-handlers.md‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎.github/workflows/daily-choice-test.lock.yml‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/cli/compile_integration_test.go‎

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,240 @@ This workflow tests that invalid schedule strings in array format fail compilati
802802
t.Logf("Integration test passed - invalid schedule in array format correctly failed compilation\nOutput: %s", outputStr)
803803
}
804804

805+
// TestCompileStagedSafeOutputsCreateIssue verifies that a workflow with staged: true
806+
// and a create-issue handler compiles without error and emits GH_AW_SAFE_OUTPUTS_STAGED.
807+
// Prior to the schema fix, staged was not listed in the create-issue schema
808+
// (additionalProperties: false), so the frontmatter validator would reject the workflow.
809+
func TestCompileStagedSafeOutputsCreateIssue(t *testing.T) {
810+
setup := setupIntegrationTest(t)
811+
defer setup.cleanup()
812+
813+
testWorkflow := `---
814+
name: Staged Create Issue
815+
on:
816+
workflow_dispatch:
817+
permissions: read-all
818+
engine: copilot
819+
safe-outputs:
820+
staged: true
821+
create-issue:
822+
title-prefix: "[staged] "
823+
max: 1
824+
---
825+
826+
Verify staged safe-outputs with create-issue.
827+
`
828+
testWorkflowPath := filepath.Join(setup.workflowsDir, "staged-create-issue.md")
829+
if err := os.WriteFile(testWorkflowPath, []byte(testWorkflow), 0644); err != nil {
830+
t.Fatalf("Failed to write test workflow file: %v", err)
831+
}
832+
833+
cmd := exec.Command(setup.binaryPath, "compile", testWorkflowPath)
834+
output, err := cmd.CombinedOutput()
835+
if err != nil {
836+
t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output))
837+
}
838+
839+
lockFilePath := filepath.Join(setup.workflowsDir, "staged-create-issue.lock.yml")
840+
lockContent, err := os.ReadFile(lockFilePath)
841+
if err != nil {
842+
t.Fatalf("Failed to read lock file: %v", err)
843+
}
844+
lockContentStr := string(lockContent)
845+
846+
if !strings.Contains(lockContentStr, `GH_AW_SAFE_OUTPUTS_STAGED: "true"`) {
847+
t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\nLock file content:\n%s", lockContentStr)
848+
}
849+
}
850+
851+
// TestCompileStagedSafeOutputsAddComment verifies that a workflow with staged: true
852+
// and an add-comment handler compiles and emits GH_AW_SAFE_OUTPUTS_STAGED.
853+
// Prior to the schema fix, staged was not listed in the add-comment handler schema.
854+
func TestCompileStagedSafeOutputsAddComment(t *testing.T) {
855+
setup := setupIntegrationTest(t)
856+
defer setup.cleanup()
857+
858+
testWorkflow := `---
859+
name: Staged Add Comment
860+
on:
861+
workflow_dispatch:
862+
permissions: read-all
863+
engine: copilot
864+
safe-outputs:
865+
staged: true
866+
add-comment:
867+
max: 1
868+
---
869+
870+
Verify staged safe-outputs with add-comment.
871+
`
872+
testWorkflowPath := filepath.Join(setup.workflowsDir, "staged-add-comment.md")
873+
if err := os.WriteFile(testWorkflowPath, []byte(testWorkflow), 0644); err != nil {
874+
t.Fatalf("Failed to write test workflow file: %v", err)
875+
}
876+
877+
cmd := exec.Command(setup.binaryPath, "compile", testWorkflowPath)
878+
output, err := cmd.CombinedOutput()
879+
if err != nil {
880+
t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output))
881+
}
882+
883+
lockFilePath := filepath.Join(setup.workflowsDir, "staged-add-comment.lock.yml")
884+
lockContent, err := os.ReadFile(lockFilePath)
885+
if err != nil {
886+
t.Fatalf("Failed to read lock file: %v", err)
887+
}
888+
lockContentStr := string(lockContent)
889+
890+
if !strings.Contains(lockContentStr, `GH_AW_SAFE_OUTPUTS_STAGED: "true"`) {
891+
t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\nLock file content:\n%s", lockContentStr)
892+
}
893+
}
894+
895+
// TestCompileStagedSafeOutputsCreateDiscussion verifies that a workflow with staged: true
896+
// and a create-discussion handler compiles and emits GH_AW_SAFE_OUTPUTS_STAGED.
897+
// Prior to the schema fix, staged was not listed in the create-discussion handler schema.
898+
func TestCompileStagedSafeOutputsCreateDiscussion(t *testing.T) {
899+
setup := setupIntegrationTest(t)
900+
defer setup.cleanup()
901+
902+
testWorkflow := `---
903+
name: Staged Create Discussion
904+
on:
905+
workflow_dispatch:
906+
permissions:
907+
contents: read
908+
engine: copilot
909+
safe-outputs:
910+
staged: true
911+
create-discussion:
912+
max: 1
913+
category: general
914+
---
915+
916+
Verify staged safe-outputs with create-discussion.
917+
`
918+
testWorkflowPath := filepath.Join(setup.workflowsDir, "staged-create-discussion.md")
919+
if err := os.WriteFile(testWorkflowPath, []byte(testWorkflow), 0644); err != nil {
920+
t.Fatalf("Failed to write test workflow file: %v", err)
921+
}
922+
923+
cmd := exec.Command(setup.binaryPath, "compile", testWorkflowPath)
924+
output, err := cmd.CombinedOutput()
925+
if err != nil {
926+
t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output))
927+
}
928+
929+
lockFilePath := filepath.Join(setup.workflowsDir, "staged-create-discussion.lock.yml")
930+
lockContent, err := os.ReadFile(lockFilePath)
931+
if err != nil {
932+
t.Fatalf("Failed to read lock file: %v", err)
933+
}
934+
lockContentStr := string(lockContent)
935+
936+
if !strings.Contains(lockContentStr, `GH_AW_SAFE_OUTPUTS_STAGED: "true"`) {
937+
t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\nLock file content:\n%s", lockContentStr)
938+
}
939+
}
940+
941+
// TestCompileStagedSafeOutputsWithTargetRepo verifies that staged: true emits
942+
// GH_AW_SAFE_OUTPUTS_STAGED even when a target-repo is specified on the handler.
943+
// Staged mode is independent of target-repo.
944+
func TestCompileStagedSafeOutputsWithTargetRepo(t *testing.T) {
945+
setup := setupIntegrationTest(t)
946+
defer setup.cleanup()
947+
948+
testWorkflow := `---
949+
name: Staged Cross-Repo
950+
on:
951+
workflow_dispatch:
952+
permissions: read-all
953+
engine: copilot
954+
safe-outputs:
955+
staged: true
956+
create-issue:
957+
title-prefix: "[cross-repo staged] "
958+
max: 1
959+
target-repo: org/other-repo
960+
---
961+
962+
Verify that staged mode is independent of target-repo.
963+
`
964+
testWorkflowPath := filepath.Join(setup.workflowsDir, "staged-cross-repo.md")
965+
if err := os.WriteFile(testWorkflowPath, []byte(testWorkflow), 0644); err != nil {
966+
t.Fatalf("Failed to write test workflow file: %v", err)
967+
}
968+
969+
cmd := exec.Command(setup.binaryPath, "compile", testWorkflowPath)
970+
output, err := cmd.CombinedOutput()
971+
if err != nil {
972+
t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output))
973+
}
974+
975+
lockFilePath := filepath.Join(setup.workflowsDir, "staged-cross-repo.lock.yml")
976+
lockContent, err := os.ReadFile(lockFilePath)
977+
if err != nil {
978+
t.Fatalf("Failed to read lock file: %v", err)
979+
}
980+
lockContentStr := string(lockContent)
981+
982+
// staged is independent of target-repo: env var must be present
983+
if !strings.Contains(lockContentStr, `GH_AW_SAFE_OUTPUTS_STAGED: "true"`) {
984+
t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUTS_STAGED: \"true\" even with target-repo set\nLock file content:\n%s", lockContentStr)
985+
}
986+
}
987+
988+
// TestCompileStagedSafeOutputsMultipleHandlers verifies that staged: true with
989+
// multiple handler types compiles and emits GH_AW_SAFE_OUTPUTS_STAGED exactly once.
990+
// Previously, adding staged to most handler types caused a schema validation error.
991+
func TestCompileStagedSafeOutputsMultipleHandlers(t *testing.T) {
992+
setup := setupIntegrationTest(t)
993+
defer setup.cleanup()
994+
995+
testWorkflow := `---
996+
name: Staged Multiple Handlers
997+
on:
998+
workflow_dispatch:
999+
permissions: read-all
1000+
engine: copilot
1001+
safe-outputs:
1002+
staged: true
1003+
create-issue:
1004+
title-prefix: "[staged] "
1005+
max: 1
1006+
add-comment:
1007+
max: 2
1008+
add-labels:
1009+
allowed:
1010+
- bug
1011+
update-issue:
1012+
---
1013+
1014+
Verify staged safe-outputs with multiple handler types.
1015+
`
1016+
testWorkflowPath := filepath.Join(setup.workflowsDir, "staged-multi-handler.md")
1017+
if err := os.WriteFile(testWorkflowPath, []byte(testWorkflow), 0644); err != nil {
1018+
t.Fatalf("Failed to write test workflow file: %v", err)
1019+
}
1020+
1021+
cmd := exec.Command(setup.binaryPath, "compile", testWorkflowPath)
1022+
output, err := cmd.CombinedOutput()
1023+
if err != nil {
1024+
t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output))
1025+
}
1026+
1027+
lockFilePath := filepath.Join(setup.workflowsDir, "staged-multi-handler.lock.yml")
1028+
lockContent, err := os.ReadFile(lockFilePath)
1029+
if err != nil {
1030+
t.Fatalf("Failed to read lock file: %v", err)
1031+
}
1032+
lockContentStr := string(lockContent)
1033+
1034+
if !strings.Contains(lockContentStr, `GH_AW_SAFE_OUTPUTS_STAGED: "true"`) {
1035+
t.Errorf("Lock file should contain GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\nLock file content:\n%s", lockContentStr)
1036+
}
1037+
}
1038+
8051039
// TestCompileFromSubdirectoryCreatesActionsLockAtRoot tests that actions-lock.json
8061040
// is created at the repository root when compiling from a subdirectory
8071041
func TestCompileFromSubdirectoryCreatesActionsLockAtRoot(t *testing.T) {

‎pkg/cli/workflows/test-staged-add-comment.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
on:
3+
workflow_dispatch:
4+
permissions: read-all
5+
engine: copilot
6+
safe-outputs:
7+
staged: true
8+
add-comment:
9+
max: 1
10+
---
11+
12+
# Test Staged Add Comment
13+
14+
Verify that `staged: true` works together with the `add-comment` handler.
15+
16+
Add a comment to issue #1 saying "Staged test comment."

‎pkg/cli/workflows/test-staged-create-discussion.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
on:
3+
workflow_dispatch:
4+
permissions:
5+
contents: read
6+
engine: copilot
7+
safe-outputs:
8+
staged: true
9+
create-discussion:
10+
max: 1
11+
category: general
12+
---
13+
14+
# Test Staged Create Discussion
15+
16+
Verify that `staged: true` works together with the `create-discussion` handler.
17+
18+
Create a discussion titled "Staged test discussion" with the body "This discussion was created in staged mode."

‎pkg/cli/workflows/test-staged-create-issue.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
on:
3+
workflow_dispatch:
4+
permissions: read-all
5+
engine: copilot
6+
safe-outputs:
7+
staged: true
8+
create-issue:
9+
title-prefix: "[staged] "
10+
max: 1
11+
---
12+
13+
# Test Staged Create Issue
14+
15+
Verify that `staged: true` works together with the `create-issue` handler.
16+
17+
Create an issue titled "Staged test issue" with the body "This issue was created in staged mode."

‎pkg/cli/workflows/test-staged-cross-repo.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
on:
3+
workflow_dispatch:
4+
permissions: read-all
5+
engine: copilot
6+
safe-outputs:
7+
staged: true
8+
create-issue:
9+
title-prefix: "[cross-repo staged] "
10+
max: 1
11+
target-repo: org/other-repo
12+
---
13+
14+
# Test Staged Create Issue Cross-Repo
15+
16+
Verify that `staged: true` is emitted even when a `target-repo` is configured.
17+
`staged` mode is independent of the `target-repo` setting.
18+
19+
Create an issue in the target repository titled "Cross-repo staged test issue."

0 commit comments

Comments
 (0)