From 81d43dba08df5d0404d3907aeafcc496d04862bd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:07:15 +0000 Subject: [PATCH 1/5] Initial plan From abc811bff4a2c140d0351cdd1cabf7bccb6d8dff Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:29:32 +0000 Subject: [PATCH 2/5] Honor fix dry-run for dispatcher artifacts Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/copilot_agents.go | 38 +++++++++---- pkg/cli/fix_command.go | 6 +- pkg/cli/fix_command_test.go | 110 +++++++++++++++++++++++++++++++++++- pkg/cli/init.go | 4 +- pkg/cli/upgrade_command.go | 4 +- 5 files changed, 143 insertions(+), 19 deletions(-) diff --git a/pkg/cli/copilot_agents.go b/pkg/cli/copilot_agents.go index 94d047f0ad2..94de79be087 100644 --- a/pkg/cli/copilot_agents.go +++ b/pkg/cli/copilot_agents.go @@ -38,7 +38,7 @@ var listAgenticWorkflowsMarkdownFiles = fetchAgenticWorkflowsMarkdownFiles // ensureAgenticWorkflowsDispatcher ensures that .github/skills/agentic-workflows/SKILL.md // exists and contains the routing instructions loaded by the Agentic Workflows agent. -func ensureAgenticWorkflowsDispatcher(verbose bool, skipInstructions bool) error { +func ensureAgenticWorkflowsDispatcher(verbose bool, skipInstructions bool, write bool) error { copilotAgentsLog.Print("Ensuring agentic workflows dispatcher skill") if skipInstructions { @@ -54,10 +54,6 @@ func ensureAgenticWorkflowsDispatcher(verbose bool, skipInstructions bool) error targetDir := filepath.Join(gitRoot, ".github", "skills", "agentic-workflows") targetPath := filepath.Join(targetDir, "SKILL.md") - if err := fileutil.EnsureParentDir(targetPath, constants.DirPermPublic); err != nil { - return fmt.Errorf("failed to create .github/skills/agentic-workflows directory: %w", err) - } - skillContent, err := buildAgenticWorkflowsSkillContent() if err != nil { copilotAgentsLog.Printf("Failed to build dispatcher skill: %v", err) @@ -80,6 +76,19 @@ func ensureAgenticWorkflowsDispatcher(verbose bool, skipInstructions bool) error return nil } + if !write { + action := "update" + if existingContent == "" { + action = "create" + } + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Would %s dispatcher skill: %s", action, targetPath))) + return nil + } + + if err := fileutil.EnsureParentDir(targetPath, constants.DirPermPublic); err != nil { + return fmt.Errorf("failed to create .github/skills/agentic-workflows directory: %w", err) + } + // Skill files are committed repository instructions, so keep them world-readable. if err := os.WriteFile(targetPath, []byte(skillContent), constants.FilePermPublic); err != nil { copilotAgentsLog.Printf("Failed to write dispatcher skill: %s, error: %v", targetPath, err) @@ -102,7 +111,7 @@ func ensureAgenticWorkflowsDispatcher(verbose bool, skipInstructions bool) error } // ensureAgenticWorkflowsAgent ensures that .github/agents/agentic-workflows.md contains the custom agent. -func ensureAgenticWorkflowsAgent(verbose bool) error { +func ensureAgenticWorkflowsAgent(verbose bool, write bool) error { copilotAgentsLog.Print("Ensuring agentic workflows custom agent") gitRoot, err := gitutil.FindGitRoot() @@ -113,10 +122,6 @@ func ensureAgenticWorkflowsAgent(verbose bool) error { targetDir := filepath.Join(gitRoot, ".github", "agents") targetPath := filepath.Join(targetDir, "agentic-workflows.md") - if err := fileutil.EnsureParentDir(targetPath, constants.DirPermPublic); err != nil { - return fmt.Errorf("failed to create .github/agents directory: %w", err) - } - existingContent := "" if content, err := os.ReadFile(targetPath); err == nil { existingContent = string(content) @@ -136,6 +141,19 @@ func ensureAgenticWorkflowsAgent(verbose bool) error { return nil } + if !write { + action := "update" + if existingContent == "" { + action = "create" + } + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Would %s Agentic Workflows custom agent: %s", action, targetPath))) + return nil + } + + if err := fileutil.EnsureParentDir(targetPath, constants.DirPermPublic); err != nil { + return fmt.Errorf("failed to create .github/agents directory: %w", err) + } + if err := os.WriteFile(targetPath, []byte(agenticWorkflowsAgentContent), constants.FilePermPublic); err != nil { return fmt.Errorf("failed to write Agentic Workflows custom agent: %w", err) } diff --git a/pkg/cli/fix_command.go b/pkg/cli/fix_command.go index 8d1c948ef1d..4250ef3785c 100644 --- a/pkg/cli/fix_command.go +++ b/pkg/cli/fix_command.go @@ -57,6 +57,8 @@ all steps and additionally: 5. Delete deprecated .github/aw/schemas/agentic-workflow.json file if it exists. 6. Delete old template files from previous versions if present. 7. Delete old workflow-specific .agent.md files from .github/agents/ if present. + 8. Refresh the dispatcher skill and agent at .github/skills/agentic-workflows/SKILL.md + and .github/agents/agentic-workflows.md when they differ from the bundled templates. ` + WorkflowIDExplanation, Example: ` ` + string(constants.CLIExtensionPrefix) + ` fix # Check all workflows (dry-run) @@ -198,11 +200,11 @@ func runFixCommand(workflowIDs []string, write bool, verbose bool, workflowDir s fixLog.Print("Updating prompt and skill files") // Update dispatcher skill - if err := ensureAgenticWorkflowsDispatcher(verbose, false); err != nil { + if err := ensureAgenticWorkflowsDispatcher(verbose, false, write); err != nil { fixLog.Printf("Failed to update dispatcher skill: %v", err) fmt.Fprintf(os.Stderr, "%s\n", console.FormatWarningMessage(fmt.Sprintf("Warning: Failed to update dispatcher skill: %v", err))) } - if err := ensureAgenticWorkflowsAgent(verbose); err != nil { + if err := ensureAgenticWorkflowsAgent(verbose, write); err != nil { fixLog.Printf("Failed to update agentic workflows custom agent: %v", err) fmt.Fprintf(os.Stderr, "%s\n", console.FormatWarningMessage(fmt.Sprintf("Warning: Failed to update agentic workflows custom agent: %v", err))) } diff --git a/pkg/cli/fix_command_test.go b/pkg/cli/fix_command_test.go index b134118ab8f..3691766182a 100644 --- a/pkg/cli/fix_command_test.go +++ b/pkg/cli/fix_command_test.go @@ -3,6 +3,7 @@ package cli import ( + "context" "io" "os" "os/exec" @@ -691,15 +692,33 @@ This is a test workflow with mcp-scripts mode field. } } -func TestFixCommand_UpdatesPromptAndAgentFiles(t *testing.T) { +func stubAgenticWorkflowsMarkdownFilesForTest(t *testing.T) { + t.Helper() + + original := listAgenticWorkflowsMarkdownFiles + listAgenticWorkflowsMarkdownFiles = func(context.Context) ([]string, error) { + return []string{ + "create-agentic-workflow.md", + "update-agentic-workflow.md", + }, nil + } + t.Cleanup(func() { + listAgenticWorkflowsMarkdownFiles = original + }) +} + +func TestFixCommand_DryRunDoesNotUpdatePromptAndAgentFiles(t *testing.T) { if _, err := exec.LookPath("git"); err != nil { // Skip when git isn't available in the test environment. t.Skip("Git not available") } + stubAgenticWorkflowsMarkdownFilesForTest(t) // Create a temporary directory for test files tmpDir := t.TempDir() workflowFile := filepath.Join(tmpDir, "test-workflow.md") + skillPath := filepath.Join(tmpDir, ".github", "skills", "agentic-workflows", "SKILL.md") + agentPath := filepath.Join(tmpDir, ".github", "agents", "agentic-workflows.md") // Save and restore original directory originalDir, err := os.Getwd() @@ -734,6 +753,19 @@ This is a test workflow. ` require.NoError(t, os.WriteFile(workflowFile, []byte(content), 0644), "Failed to create test file") + require.NoError(t, os.MkdirAll(filepath.Dir(skillPath), 0755), "Failed to create skill directory") + require.NoError(t, os.MkdirAll(filepath.Dir(agentPath), 0755), "Failed to create agent directory") + + originalSkill := "local skill change\n" + originalAgent := "local agent change\n" + require.NoError(t, os.WriteFile(skillPath, []byte(originalSkill), 0644), "Failed to create skill file") + require.NoError(t, os.WriteFile(agentPath, []byte(originalAgent), 0644), "Failed to create agent file") + + originalStderr := os.Stderr + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stderr = w + t.Cleanup(func() { os.Stderr = originalStderr }) // Run fix command (which refreshes the generated skill and agent files) config := FixConfig{ @@ -743,15 +775,87 @@ This is a test workflow. WorkflowDir: tmpDir, } + require.NoError(t, RunFix(config), "RunFix failed") + require.NoError(t, w.Close()) + + outputBytes, err := io.ReadAll(r) + require.NoError(t, err) + output := string(outputBytes) + + skillContent, err := os.ReadFile(skillPath) + require.NoError(t, err, "Expected skill file to remain in place after dry-run") + assert.Equal(t, originalSkill, string(skillContent)) + + agentContent, err := os.ReadFile(agentPath) + require.NoError(t, err, "Expected agent file to remain in place after dry-run") + assert.Equal(t, originalAgent, string(agentContent)) + + assert.Contains(t, output, "Would update dispatcher skill: "+skillPath) + assert.Contains(t, output, "Would update Agentic Workflows custom agent: "+agentPath) +} + +func TestFixCommand_WriteUpdatesPromptAndAgentFiles(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("Git not available") + } + stubAgenticWorkflowsMarkdownFilesForTest(t) + + tmpDir := t.TempDir() + workflowFile := filepath.Join(tmpDir, "test-workflow.md") + skillPath := filepath.Join(tmpDir, ".github", "skills", "agentic-workflows", "SKILL.md") + agentPath := filepath.Join(tmpDir, ".github", "agents", "agentic-workflows.md") + + originalDir, err := os.Getwd() + require.NoError(t, err, "Failed to get current directory") + t.Cleanup(func() { + if chdirErr := os.Chdir(originalDir); chdirErr != nil { + t.Errorf("Failed to restore current directory: %v", chdirErr) + } + }) + + require.NoError(t, os.Chdir(tmpDir), "Failed to change to temp directory") + require.NoError(t, exec.Command("git", "init").Run(), "Failed to initialize git repo") + require.NoError(t, exec.Command("git", "config", "user.name", "Test User").Run(), "Failed to configure git user.name") + require.NoError(t, exec.Command("git", "config", "user.email", "test@example.com").Run(), "Failed to configure git user.email") + + content := `--- +on: + workflow_dispatch: + +permissions: + contents: read +--- + +# Test Workflow + +This is a test workflow. +` + + require.NoError(t, os.WriteFile(workflowFile, []byte(content), 0644), "Failed to create test file") + require.NoError(t, os.MkdirAll(filepath.Dir(skillPath), 0755), "Failed to create skill directory") + require.NoError(t, os.MkdirAll(filepath.Dir(agentPath), 0755), "Failed to create agent directory") + require.NoError(t, os.WriteFile(skillPath, []byte("local skill change\n"), 0644), "Failed to create skill file") + require.NoError(t, os.WriteFile(agentPath, []byte("local agent change\n"), 0644), "Failed to create agent file") + + config := FixConfig{ + WorkflowIDs: []string{"test-workflow"}, + Write: true, + Verbose: false, + WorkflowDir: tmpDir, + } + require.NoError(t, RunFix(config), "RunFix failed") - _, err = os.Stat(filepath.Join(tmpDir, ".github", "skills", "agentic-workflows", "SKILL.md")) + skillContent, err := os.ReadFile(skillPath) require.NoError(t, err, "Expected generated skill file to exist after RunFix") + assert.Contains(t, string(skillContent), "# Agentic Workflows Router") + assert.NotContains(t, string(skillContent), "local skill change") - agentContent, err := os.ReadFile(filepath.Join(tmpDir, ".github", "agents", "agentic-workflows.md")) + agentContent, err := os.ReadFile(agentPath) require.NoError(t, err, "Expected generated agent file to exist after RunFix") assert.Contains(t, string(agentContent), ".github/aw/create-agentic-workflow.md") assert.NotContains(t, string(agentContent), ".github/skills/agentic-workflows/SKILL.md") + assert.NotContains(t, string(agentContent), "local agent change") } func TestFixCommand_GrepToolRemoval(t *testing.T) { diff --git a/pkg/cli/init.go b/pkg/cli/init.go index 31915616001..62ad34b32dc 100644 --- a/pkg/cli/init.go +++ b/pkg/cli/init.go @@ -90,7 +90,7 @@ func InitRepository(opts InitOptions) error { if copilotArtifactsEnabled { if opts.Skill { initLog.Print("Writing agentic workflows dispatcher skill") - if err := ensureAgenticWorkflowsDispatcher(opts.Verbose, false); err != nil { + if err := ensureAgenticWorkflowsDispatcher(opts.Verbose, false, true); err != nil { initLog.Printf("Failed to write dispatcher skill: %v", err) return fmt.Errorf("failed to write dispatcher skill: %w", err) } @@ -102,7 +102,7 @@ func InitRepository(opts InitOptions) error { } if opts.Agent { initLog.Print("Writing agentic workflows custom agent") - if err := ensureAgenticWorkflowsAgent(opts.Verbose); err != nil { + if err := ensureAgenticWorkflowsAgent(opts.Verbose, true); err != nil { initLog.Printf("Failed to write agentic workflows custom agent: %v", err) return fmt.Errorf("failed to write agentic workflows custom agent: %w", err) } diff --git a/pkg/cli/upgrade_command.go b/pkg/cli/upgrade_command.go index 348b7d4801c..02f99a7d62d 100644 --- a/pkg/cli/upgrade_command.go +++ b/pkg/cli/upgrade_command.go @@ -421,11 +421,11 @@ func runUpgradeCommand(opts upgradeOptions) error { // updateCopilotArtifacts updates the dispatcher skill and related Copilot setup artifacts. func updateCopilotArtifacts(ctx context.Context, verbose bool) error { // Update dispatcher skill - if err := ensureAgenticWorkflowsDispatcher(verbose, false); err != nil { + if err := ensureAgenticWorkflowsDispatcher(verbose, false, true); err != nil { upgradeLog.Printf("Failed to update dispatcher skill: %v", err) return fmt.Errorf("failed to update dispatcher skill: %w", err) } - if err := ensureAgenticWorkflowsAgent(verbose); err != nil { + if err := ensureAgenticWorkflowsAgent(verbose, true); err != nil { upgradeLog.Printf("Failed to update Agentic Workflows custom agent: %v", err) return fmt.Errorf("failed to update Agentic Workflows custom agent: %w", err) } From f1dcb2a0a11e13b44b225d003ebd116d9e52a4e4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:45:37 +0000 Subject: [PATCH 3/5] Add runtime dry-run write guards Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/copilot_agents.go | 30 +++++++++++++++++++----------- pkg/cli/fix_command_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/pkg/cli/copilot_agents.go b/pkg/cli/copilot_agents.go index 94de79be087..8cdb290cfcf 100644 --- a/pkg/cli/copilot_agents.go +++ b/pkg/cli/copilot_agents.go @@ -85,12 +85,7 @@ func ensureAgenticWorkflowsDispatcher(verbose bool, skipInstructions bool, write return nil } - if err := fileutil.EnsureParentDir(targetPath, constants.DirPermPublic); err != nil { - return fmt.Errorf("failed to create .github/skills/agentic-workflows directory: %w", err) - } - - // Skill files are committed repository instructions, so keep them world-readable. - if err := os.WriteFile(targetPath, []byte(skillContent), constants.FilePermPublic); err != nil { + if err := writeGeneratedRepositoryInstructionFile(targetPath, []byte(skillContent), write, ".github/skills/agentic-workflows directory", "dispatcher skill"); err != nil { copilotAgentsLog.Printf("Failed to write dispatcher skill: %s, error: %v", targetPath, err) return fmt.Errorf("failed to write dispatcher skill: %w", err) } @@ -150,11 +145,7 @@ func ensureAgenticWorkflowsAgent(verbose bool, write bool) error { return nil } - if err := fileutil.EnsureParentDir(targetPath, constants.DirPermPublic); err != nil { - return fmt.Errorf("failed to create .github/agents directory: %w", err) - } - - if err := os.WriteFile(targetPath, []byte(agenticWorkflowsAgentContent), constants.FilePermPublic); err != nil { + if err := writeGeneratedRepositoryInstructionFile(targetPath, []byte(agenticWorkflowsAgentContent), write, ".github/agents directory", "Agentic Workflows custom agent"); err != nil { return fmt.Errorf("failed to write Agentic Workflows custom agent: %w", err) } @@ -173,6 +164,23 @@ func ensureAgenticWorkflowsAgent(verbose bool, write bool) error { return nil } +func writeGeneratedRepositoryInstructionFile(targetPath string, content []byte, write bool, parentDirDescription string, artifactDescription string) error { + if !write { + return fmt.Errorf("internal error: refusing to write %s without --write: %s", artifactDescription, targetPath) + } + + if err := fileutil.EnsureParentDir(targetPath, constants.DirPermPublic); err != nil { + return fmt.Errorf("failed to create %s: %w", parentDirDescription, err) + } + + // Repository instruction files are committed artifacts, so keep them world-readable. + if err := os.WriteFile(targetPath, content, constants.FilePermPublic); err != nil { + return err + } + + return nil +} + func buildAgenticWorkflowsAgentContent(gitRoot string) (string, error) { return agenticWorkflowsAgentTemplate, nil } diff --git a/pkg/cli/fix_command_test.go b/pkg/cli/fix_command_test.go index 3691766182a..1a440ac868f 100644 --- a/pkg/cli/fix_command_test.go +++ b/pkg/cli/fix_command_test.go @@ -707,6 +707,41 @@ func stubAgenticWorkflowsMarkdownFilesForTest(t *testing.T) { }) } +func TestWriteGeneratedRepositoryInstructionFile_RefusesDryRun(t *testing.T) { + tmpDir := t.TempDir() + targetPath := filepath.Join(tmpDir, ".github", "skills", "agentic-workflows", "SKILL.md") + originalContent := []byte("existing content\n") + + require.NoError(t, os.MkdirAll(filepath.Dir(targetPath), 0755)) + require.NoError(t, os.WriteFile(targetPath, originalContent, 0644)) + + err := writeGeneratedRepositoryInstructionFile( + targetPath, + []byte("new content\n"), + false, + ".github/skills/agentic-workflows directory", + "dispatcher skill", + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "refusing to write dispatcher skill without --write") + + content, readErr := os.ReadFile(targetPath) + require.NoError(t, readErr) + assert.Equal(t, string(originalContent), string(content)) + + missingPath := filepath.Join(tmpDir, ".github", "agents", "nested", "agentic-workflows.md") + err = writeGeneratedRepositoryInstructionFile( + missingPath, + []byte("new content\n"), + false, + ".github/agents directory", + "Agentic Workflows custom agent", + ) + require.Error(t, err) + assert.NoFileExists(t, missingPath) + assert.NoDirExists(t, filepath.Dir(missingPath)) +} + func TestFixCommand_DryRunDoesNotUpdatePromptAndAgentFiles(t *testing.T) { if _, err := exec.LookPath("git"); err != nil { // Skip when git isn't available in the test environment. From a12ae9d4273dd0425ea37e3e90360d16697bb2e7 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 12:00:35 +0000 Subject: [PATCH 4/5] docs(adr): add draft ADR-47269 for dry-run gate on dispatcher artifacts Co-Authored-By: Claude Sonnet 4.6 --- ...or-fix-dry-run-for-dispatcher-artifacts.md | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 docs/adr/47269-honor-fix-dry-run-for-dispatcher-artifacts.md diff --git a/docs/adr/47269-honor-fix-dry-run-for-dispatcher-artifacts.md b/docs/adr/47269-honor-fix-dry-run-for-dispatcher-artifacts.md new file mode 100644 index 00000000000..f2007934b97 --- /dev/null +++ b/docs/adr/47269-honor-fix-dry-run-for-dispatcher-artifacts.md @@ -0,0 +1,44 @@ +# ADR-47269: Honor `gh aw fix` Dry-Run for Dispatcher Artifacts + +**Date**: 2026-07-22 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +The `gh aw fix` command documents two modes: dry-run (default, no file mutations) and write mode (`--write`). However, the dispatcher artifact helpers — `ensureAgenticWorkflowsDispatcher` and `ensureAgenticWorkflowsAgent` — unconditionally created parent directories and wrote files regardless of the `write` flag. This violated the advertised contract: running `gh aw fix` without `--write` would still mutate `.github/skills/agentic-workflows/SKILL.md` and `.github/agents/agentic-workflows.md` when they drifted from the bundled templates. Fixing #47141, the core requirement is that dry-run must leave all files byte-for-byte unchanged while still reporting what would change. + +### Decision + +We will thread a `write bool` parameter through `ensureAgenticWorkflowsDispatcher` and `ensureAgenticWorkflowsAgent`, and extract a shared `writeGeneratedRepositoryInstructionFile` helper that hard-errors if called with `write=false`. In dry-run mode the ensure functions compare existing content against the generated template and emit a `Would create/update …` message to stderr, then return without touching the filesystem. The `init` and `upgrade` callers always pass `write=true`, so their behavior is unchanged. + +### Alternatives Considered + +#### Alternative 1: Skip dispatcher refresh entirely in dry-run + +Guard the two `ensure*` calls at the `runFixCommand` call site with `if write { ... }`. This is simpler — no parameter threading, no new helper. However, it silently swallows drift: users running dry-run would not learn that their dispatcher files are stale, defeating the purpose of a check-all-workflows diagnostic mode. Rejected because drift reporting in dry-run is explicitly desirable. + +#### Alternative 2: Accept a no-op write path without a hard guard + +Allow `writeGeneratedRepositoryInstructionFile` to accept `write=false` and simply return `nil` silently. This avoids an "internal error" return but makes it easy to introduce future callers that forget to check `write` before calling the helper, silently skipping writes. Rejected because the explicit error acts as a compile-time-equivalent invariant: if a future caller passes `write=false` it immediately surfaces rather than silently no-oping. + +### Consequences + +#### Positive +- Dry-run mode now correctly reports drift (`Would create/update dispatcher skill: …`) without mutating any files. +- The `writeGeneratedRepositoryInstructionFile` helper centralises `EnsureParentDir` + `WriteFile` + error formatting, reducing duplication across the two ensure functions. +- Both sides of the contract are explicitly covered by tests (`TestFixCommand_DryRunDoesNotUpdatePromptAndAgentFiles`, `TestFixCommand_WriteUpdatesPromptAndAgentFiles`, `TestWriteGeneratedRepositoryInstructionFile_RefusesDryRun`). + +#### Negative +- All existing callers of the two `ensure*` functions required a mechanical signature update (adding the `write` argument), increasing the diff surface area slightly. +- The `writeGeneratedRepositoryInstructionFile` helper returns an error on `write=false`; this path should never be reached in production, but it adds a code path that cannot be tested through the normal call graph without deliberate misuse. + +#### Neutral +- `init` and `upgrade` callers now explicitly pass `true` for `write`, making the write intent visible at call sites rather than implied by function design. +- CLI help text for `gh aw fix` was updated to list dispatcher refresh as a write-mode action, bringing documentation into sync with behavior. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From 3694b7d402eb5ef5a8886e165e9bde64138b1a77 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 12:53:15 +0000 Subject: [PATCH 5/5] fix(cli): tighten dry-run dispatcher drift reporting and read/write guards Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/copilot_agents.go | 34 +++++++++++++------- pkg/cli/fix_command.go | 2 +- pkg/cli/fix_command_test.go | 63 +++++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 13 deletions(-) diff --git a/pkg/cli/copilot_agents.go b/pkg/cli/copilot_agents.go index 8cdb290cfcf..9cc3bdd9227 100644 --- a/pkg/cli/copilot_agents.go +++ b/pkg/cli/copilot_agents.go @@ -60,10 +60,9 @@ func ensureAgenticWorkflowsDispatcher(verbose bool, skipInstructions bool, write return fmt.Errorf("failed to build dispatcher skill: %w", err) } - // Check if the file already exists and matches the downloaded content - existingContent := "" - if content, err := os.ReadFile(targetPath); err == nil { - existingContent = string(content) + existingContent, fileExists, err := readExistingRepositoryInstructionFile(targetPath, "dispatcher skill") + if err != nil { + return err } // Check if content matches the downloaded template @@ -78,7 +77,7 @@ func ensureAgenticWorkflowsDispatcher(verbose bool, skipInstructions bool, write if !write { action := "update" - if existingContent == "" { + if !fileExists { action = "create" } fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Would %s dispatcher skill: %s", action, targetPath))) @@ -90,7 +89,7 @@ func ensureAgenticWorkflowsDispatcher(verbose bool, skipInstructions bool, write return fmt.Errorf("failed to write dispatcher skill: %w", err) } - if existingContent == "" { + if !fileExists { copilotAgentsLog.Printf("Created dispatcher skill: %s", targetPath) if verbose { fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Created dispatcher skill: "+targetPath)) @@ -117,9 +116,9 @@ func ensureAgenticWorkflowsAgent(verbose bool, write bool) error { targetDir := filepath.Join(gitRoot, ".github", "agents") targetPath := filepath.Join(targetDir, "agentic-workflows.md") - existingContent := "" - if content, err := os.ReadFile(targetPath); err == nil { - existingContent = string(content) + existingContent, fileExists, err := readExistingRepositoryInstructionFile(targetPath, "Agentic Workflows custom agent") + if err != nil { + return err } agenticWorkflowsAgentContent, err := buildAgenticWorkflowsAgentContent(gitRoot) @@ -138,7 +137,7 @@ func ensureAgenticWorkflowsAgent(verbose bool, write bool) error { if !write { action := "update" - if existingContent == "" { + if !fileExists { action = "create" } fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Would %s Agentic Workflows custom agent: %s", action, targetPath))) @@ -149,7 +148,7 @@ func ensureAgenticWorkflowsAgent(verbose bool, write bool) error { return fmt.Errorf("failed to write Agentic Workflows custom agent: %w", err) } - if existingContent == "" { + if !fileExists { copilotAgentsLog.Printf("Created Agentic Workflows custom agent: %s", targetPath) if verbose { fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Created Agentic Workflows custom agent: "+targetPath)) @@ -175,12 +174,23 @@ func writeGeneratedRepositoryInstructionFile(targetPath string, content []byte, // Repository instruction files are committed artifacts, so keep them world-readable. if err := os.WriteFile(targetPath, content, constants.FilePermPublic); err != nil { - return err + return fmt.Errorf("failed to write %s: %w", artifactDescription, err) } return nil } +func readExistingRepositoryInstructionFile(targetPath string, artifactDescription string) (string, bool, error) { + content, err := os.ReadFile(targetPath) + if err == nil { + return string(content), true, nil + } + if os.IsNotExist(err) { + return "", false, nil + } + return "", false, fmt.Errorf("failed to read existing %s: %w", artifactDescription, err) +} + func buildAgenticWorkflowsAgentContent(gitRoot string) (string, error) { return agenticWorkflowsAgentTemplate, nil } diff --git a/pkg/cli/fix_command.go b/pkg/cli/fix_command.go index 4250ef3785c..f69f8d3d9b2 100644 --- a/pkg/cli/fix_command.go +++ b/pkg/cli/fix_command.go @@ -270,7 +270,7 @@ func runFixCommand(workflowIDs []string, write bool, verbose bool, workflowDir s fmt.Fprintf(os.Stderr, " gh aw fix %s --write\n", strings.TrimSuffix(wf.File, ".md")) } } else if totalGuidedErrors == 0 && totalProcessingErrors == 0 { - fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("✓ No fixes needed")) + fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("✓ No workflow fixes needed")) } } diff --git a/pkg/cli/fix_command_test.go b/pkg/cli/fix_command_test.go index 1a440ac868f..18d55d58406 100644 --- a/pkg/cli/fix_command_test.go +++ b/pkg/cli/fix_command_test.go @@ -827,6 +827,7 @@ This is a test workflow. assert.Contains(t, output, "Would update dispatcher skill: "+skillPath) assert.Contains(t, output, "Would update Agentic Workflows custom agent: "+agentPath) + assert.Contains(t, output, "✓ No workflow fixes needed") } func TestFixCommand_WriteUpdatesPromptAndAgentFiles(t *testing.T) { @@ -893,6 +894,68 @@ This is a test workflow. assert.NotContains(t, string(agentContent), "local agent change") } +func TestFixCommand_DryRunTreatsExistingEmptyDispatcherArtifactsAsUpdates(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("Git not available") + } + stubAgenticWorkflowsMarkdownFilesForTest(t) + + tmpDir := t.TempDir() + workflowFile := filepath.Join(tmpDir, "test-workflow.md") + skillPath := filepath.Join(tmpDir, ".github", "skills", "agentic-workflows", "SKILL.md") + agentPath := filepath.Join(tmpDir, ".github", "agents", "agentic-workflows.md") + + originalDir, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { + if chdirErr := os.Chdir(originalDir); chdirErr != nil { + t.Errorf("Failed to restore current directory: %v", chdirErr) + } + }) + + require.NoError(t, os.Chdir(tmpDir)) + require.NoError(t, exec.Command("git", "init").Run()) + require.NoError(t, exec.Command("git", "config", "user.name", "Test User").Run()) + require.NoError(t, exec.Command("git", "config", "user.email", "test@example.com").Run()) + + content := `--- +on: + workflow_dispatch: + +permissions: + contents: read +--- +` + require.NoError(t, os.WriteFile(workflowFile, []byte(content), 0644)) + require.NoError(t, os.MkdirAll(filepath.Dir(skillPath), 0755)) + require.NoError(t, os.MkdirAll(filepath.Dir(agentPath), 0755)) + require.NoError(t, os.WriteFile(skillPath, []byte{}, 0644)) + require.NoError(t, os.WriteFile(agentPath, []byte{}, 0644)) + + originalStderr := os.Stderr + r, w, err := os.Pipe() + require.NoError(t, err) + // NOTE: this test redirects os.Stderr and must not run in parallel. + os.Stderr = w + t.Cleanup(func() { os.Stderr = originalStderr }) + + require.NoError(t, RunFix(FixConfig{ + WorkflowIDs: []string{"test-workflow"}, + Write: false, + WorkflowDir: tmpDir, + })) + + require.NoError(t, w.Close()) + outputBytes, err := io.ReadAll(r) + require.NoError(t, err) + output := string(outputBytes) + + assert.Contains(t, output, "Would update dispatcher skill: "+skillPath) + assert.Contains(t, output, "Would update Agentic Workflows custom agent: "+agentPath) + assert.NotContains(t, output, "Would create dispatcher skill:") + assert.NotContains(t, output, "Would create Agentic Workflows custom agent:") +} + func TestFixCommand_GrepToolRemoval(t *testing.T) { // Create a temporary directory for test files tmpDir := t.TempDir()