Honor gh aw fix dry-run for dispatcher agent and skill refresh

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.*

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,20 +54,15 @@ 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)
 		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
@@ -80,13 +75,21 @@ func ensureAgenticWorkflowsDispatcher(verbose bool, skipInstructions bool) error
 		return nil
 	}
 
-	// Skill files are committed repository instructions, so keep them world-readable.
-	if err := os.WriteFile(targetPath, []byte(skillContent), constants.FilePermPublic); err != nil {
+	if !write {
+		action := "update"
+		if !fileExists {
+			action = "create"
+		}
+		fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Would %s dispatcher skill: %s", action, targetPath)))
+		return 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)
 	}
 
-	if existingContent == "" {
+	if !fileExists {
 		copilotAgentsLog.Printf("Created dispatcher skill: %s", targetPath)
 		if verbose {
 			fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Created dispatcher skill: "+targetPath))
@@ -102,7 +105,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,13 +116,9 @@ 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)
+	existingContent, fileExists, err := readExistingRepositoryInstructionFile(targetPath, "Agentic Workflows custom agent")
+	if err != nil {
+		return err
 	}
 
 	agenticWorkflowsAgentContent, err := buildAgenticWorkflowsAgentContent(gitRoot)
@@ -136,11 +135,20 @@ func ensureAgenticWorkflowsAgent(verbose bool) error {
 		return nil
 	}
 
-	if err := os.WriteFile(targetPath, []byte(agenticWorkflowsAgentContent), constants.FilePermPublic); err != nil {
+	if !write {
+		action := "update"
+		if !fileExists {
+			action = "create"
+		}
+		fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Would %s Agentic Workflows custom agent: %s", action, targetPath)))
+		return 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)
 	}
 
-	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))
@@ -155,6 +163,34 @@ func ensureAgenticWorkflowsAgent(verbose 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 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
 }

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)))
 	}
@@ -268,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"))
 		}
 	}