From 9237444a936d5a90d2aa0120d94d898da6314d90 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 18:49:31 +0000 Subject: [PATCH 1/9] Initial plan From 8781afd123043c0da9e76560f846b1fab40f7146 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 19:05:15 +0000 Subject: [PATCH 2/9] Add experimental workflow edit command Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- cmd/gh-aw/main.go | 7 +- pkg/cli/edit_command.go | 327 +++++++++++++++++++++++++++++++++++ pkg/cli/edit_command_test.go | 81 +++++++++ 3 files changed, 412 insertions(+), 3 deletions(-) create mode 100644 pkg/cli/edit_command.go create mode 100644 pkg/cli/edit_command_test.go diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index 9ae1ed08df3..ed20a246648 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -553,7 +553,7 @@ func runCompileCmd(cmd *cobra.Command, args []string) error { } type commandSet struct { - addCmd, addWizardCmd, updateCmd, deployCmd, trialCmd, initCmd, statusCmd, listCmd *cobra.Command + addCmd, addWizardCmd, editCmd, updateCmd, deployCmd, trialCmd, initCmd, statusCmd, listCmd *cobra.Command mcpCmd, logsCmd, auditCmd, viewCmd, healthCmd, outcomesCmd, mcpServerCmd, prCmd, secretsCmd *cobra.Command fixCmd, upgradeCmd, completionCmd, hashCmd, projectCmd, doctorCmd, checksCmd, validateCmd, lintCmd *cobra.Command domainsCmd, experimentsCmd, forecastCmd, gradersCmd, modelsCmd, envCmd *cobra.Command @@ -708,6 +708,7 @@ func createCommandSet() commandSet { cmds := commandSet{ addCmd: cli.NewAddCommand(validateEngine), addWizardCmd: cli.NewAddWizardCommand(validateEngine), + editCmd: cli.NewEditCommand(), updateCmd: cli.NewUpdateCommand(validateEngine), deployCmd: cli.NewDeployCommand(validateEngine), trialCmd: cli.NewTrialCommand(validateEngine), @@ -845,7 +846,7 @@ func configureOtherCommandFlags() { func assignCommandGroups(cmds commandSet) { cmds.initCmd.GroupID, newCmd.GroupID, cmds.addCmd.GroupID, cmds.addWizardCmd.GroupID = "setup", "setup", "setup", "setup" - removeCmd.GroupID, cmds.updateCmd.GroupID, cmds.deployCmd.GroupID, cmds.upgradeCmd.GroupID = "setup", "setup", "setup", "setup" + removeCmd.GroupID, cmds.editCmd.GroupID, cmds.updateCmd.GroupID, cmds.deployCmd.GroupID, cmds.upgradeCmd.GroupID = "setup", "setup", "setup", "setup", "setup" cmds.secretsCmd.GroupID, cmds.envCmd.GroupID, cmds.doctorCmd.GroupID = "setup", "setup", "setup" compileCmd.GroupID, cmds.validateCmd.GroupID, cmds.lintCmd.GroupID = "development", "development", "development" cmds.mcpCmd.GroupID, cmds.fixCmd.GroupID, cmds.domainsCmd.GroupID = "development", "development", "development" @@ -859,7 +860,7 @@ func assignCommandGroups(cmds commandSet) { func addCommandsToRoot(cmds commandSet) { rootCmd.AddCommand( - compileCmd, cmds.addCmd, cmds.addWizardCmd, cmds.updateCmd, cmds.deployCmd, cmds.upgradeCmd, cmds.trialCmd, newCmd, cmds.initCmd, + compileCmd, cmds.addCmd, cmds.addWizardCmd, cmds.editCmd, cmds.updateCmd, cmds.deployCmd, cmds.upgradeCmd, cmds.trialCmd, newCmd, cmds.initCmd, runCmd, removeCmd, cmds.statusCmd, cmds.listCmd, enableCmd, disableCmd, cmds.logsCmd, cmds.auditCmd, cmds.viewCmd, cmds.healthCmd, cmds.outcomesCmd, cmds.checksCmd, cmds.mcpCmd, cmds.mcpServerCmd, cmds.prCmd, versionCmd, cmds.secretsCmd, cmds.fixCmd, cmds.validateCmd, cmds.lintCmd, cmds.completionCmd, cmds.hashCmd, cmds.projectCmd, cmds.doctorCmd, diff --git a/pkg/cli/edit_command.go b/pkg/cli/edit_command.go new file mode 100644 index 00000000000..a73bc02bd1d --- /dev/null +++ b/pkg/cli/edit_command.go @@ -0,0 +1,327 @@ +package cli + +import ( + "context" + "errors" + "fmt" + "os" + "slices" + "strings" + + "github.com/github/gh-aw/pkg/parser" + "github.com/goccy/go-yaml" + "github.com/spf13/cobra" +) + +// NewEditCommand creates the experimental command for changing workflow frontmatter. +func NewEditCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "edit [path: value]", + Short: "Experimental: edit workflow frontmatter and recompile", + Long: `Experimental: edit schema-validated workflow frontmatter and recompile its generated file. + +The workflow-id may be a workflow name, a Markdown filename, or a path. Changes are +validated before writing. Workflows managed by a source: declaration cannot be edited.`, + Example: ` gh aw edit repo-assist "max-turns: 20" + gh aw edit repo-assist --schedule 6h + gh aw edit repo-assist --set model=small --unset engine.model + gh aw edit repo-assist --add imports=shared/common.md`, + Args: cobra.RangeArgs(1, 2), + RunE: runEditCommand, + } + cmd.Flags().StringArray("set", nil, "Set a frontmatter path (path=value)") + cmd.Flags().StringArray("unset", nil, "Remove a frontmatter path") + cmd.Flags().StringArray("add", nil, "Append a value to a list (path=value)") + cmd.Flags().StringArray("remove", nil, "Remove a value from a list (path=value)") + cmd.Flags().StringArray("add-import", nil, "Append a workflow import path") + cmd.Flags().String("schedule", "", "Set a schedule using a duration, fuzzy schedule, or cron expression; use off to remove it") + cmd.Flags().Bool("dry-run", false, "Validate changes without writing or compiling") + cmd.Flags().StringP("dir", "d", "", "Workflow directory (default: $GH_AW_WORKFLOWS_DIR or .github/workflows)") + cmd.ValidArgsFunction = CompleteWorkflowNames + RegisterDirFlagCompletion(cmd, "dir") + return cmd +} + +func runEditCommand(cmd *cobra.Command, args []string) error { + workflowPath, err := resolveWorkflowFileInDir(args[0], false, flagString(cmd, "dir")) + if err != nil { + return err + } + content, err := os.ReadFile(workflowPath) + if err != nil { + return fmt.Errorf("read workflow: %w", err) + } + parsed, err := parser.ExtractFrontmatterFromContent(string(content)) + if err != nil { + return err + } + if _, managed := parsed.Frontmatter["source"]; managed { + return errors.New("cannot edit a source-managed workflow; update its source or pin, then run gh aw update") + } + + changes, err := editChangesFromCommand(cmd, args) + if err != nil { + return err + } + if len(changes) == 0 { + return errors.New("provide an assignment or an edit flag") + } + for _, change := range changes { + if err := applyEditChange(parsed.Frontmatter, change); err != nil { + return err + } + } + if err := parser.ValidateMainWorkflowFrontmatterWithSchemaAndLocation(parsed.Frontmatter, workflowPath); err != nil { + return fmt.Errorf("invalid edited workflow: %w", err) + } + updated, err := replaceFrontmatter(string(content), parsed.Frontmatter) + if err != nil { + return err + } + if flagBool(cmd, "dry-run") { + fmt.Fprint(cmd.OutOrStdout(), updated) + return nil + } + + return writeAndCompileEditedWorkflow(workflowPath, content, updated) +} + +func writeAndCompileEditedWorkflow(workflowPath string, content []byte, updated string) error { + lockPath := strings.TrimSuffix(workflowPath, ".md") + ".lock.yml" + previousLock, lockErr := os.ReadFile(lockPath) + lockExisted := lockErr == nil + if lockErr != nil && !os.IsNotExist(lockErr) { + return fmt.Errorf("read generated workflow: %w", lockErr) + } + if err := os.WriteFile(workflowPath, []byte(updated), 0o644); err != nil { + return fmt.Errorf("write workflow: %w", err) + } + if err := compileWorkflow(context.Background(), workflowPath, false, true, ""); err != nil { + _ = os.WriteFile(workflowPath, content, 0o644) + if lockExisted { + _ = os.WriteFile(lockPath, previousLock, 0o644) + } else { + _ = os.Remove(lockPath) + } + return fmt.Errorf("compile edited workflow: %w", err) + } + return nil +} + +type editChange struct { + kind, path string + value any +} + +func editChangesFromCommand(cmd *cobra.Command, args []string) ([]editChange, error) { + var changes []editChange + if len(args) == 2 { + change, err := parseEditAssignment(args[1], ":") + if err != nil { + return nil, err + } + changes = append(changes, change) + } + for _, name := range []string{"set", "add", "remove"} { + for _, assignment := range flagStrings(cmd, name) { + change, err := parseEditAssignment(assignment, "=") + if err != nil { + return nil, err + } + change.kind = name + changes = append(changes, change) + } + } + for _, path := range flagStrings(cmd, "unset") { + changes = append(changes, editChange{kind: "unset", path: path}) + } + for _, importPath := range flagStrings(cmd, "add-import") { + changes = append(changes, editChange{kind: "add", path: "imports", value: importPath}) + } + if schedule := flagString(cmd, "schedule"); schedule != "" { + change, err := scheduleChange(schedule) + if err != nil { + return nil, err + } + changes = append(changes, change) + } + return changes, nil +} + +func isCompactSchedule(schedule string) bool { + if len(schedule) < 2 { + return false + } + unit := schedule[len(schedule)-1] + if !strings.ContainsRune("hdwm", rune(unit)) { + return false + } + for _, char := range schedule[:len(schedule)-1] { + if char < '0' || char > '9' { + return false + } + } + return true +} + +func scheduleChange(schedule string) (editChange, error) { + schedule = strings.TrimSpace(strings.ToLower(schedule)) + if schedule == "off" { + return editChange{kind: "unset", path: "on.schedule"}, nil + } + if schedule == "weekdays" { + schedule = "daily on weekdays" + } else if isCompactSchedule(schedule) { + schedule = "every " + schedule + } + cron, _, err := parser.ParseSchedule(schedule) + if err != nil { + return editChange{}, fmt.Errorf("invalid schedule: %w", err) + } + return editChange{kind: "set", path: "on.schedule", value: []any{map[string]any{"cron": cron}}}, nil +} + +func parseEditAssignment(assignment, separator string) (editChange, error) { + path, rawValue, ok := strings.Cut(assignment, separator) + path, rawValue = strings.TrimSpace(path), strings.TrimSpace(rawValue) + if !ok || path == "" || rawValue == "" { + return editChange{}, fmt.Errorf("invalid assignment %q; expected path%svalue", assignment, separator) + } + var value map[string]any + if err := yaml.Unmarshal([]byte("value: "+rawValue), &value); err != nil { + return editChange{}, fmt.Errorf("parse value for %q: %w", path, err) + } + return editChange{kind: "set", path: path, value: value["value"]}, nil +} + +func applyEditChange(frontmatter map[string]any, change editChange) error { + change, err := normalizeEditChange(frontmatter, change) + if err != nil { + return err + } + parent, key, err := editChangeParent(frontmatter, change.path) + if err != nil { + return err + } + return applyEditChangeToParent(parent, key, change) +} + +func normalizeEditChange(frontmatter map[string]any, change editChange) (editChange, error) { + if change.kind == "set" && change.path == "on.schedule" { + if schedule, ok := change.value.(string); ok { + var err error + change, err = scheduleChange(schedule) + if err != nil { + return editChange{}, err + } + } + } + if change.path == "imports" && (change.kind == "add" || change.kind == "remove") { + if _, objectImports := frontmatter["imports"].(map[string]any); objectImports { + change.path = "imports.aw" + } + } + return change, nil +} + +func editChangeParent(frontmatter map[string]any, changePath string) (map[string]any, string, error) { + path := strings.Split(changePath, ".") + if slices.Contains(path, "") { + return nil, "", fmt.Errorf("invalid frontmatter path %q", changePath) + } + parent := frontmatter + for _, part := range path[:len(path)-1] { + child, ok := parent[part].(map[string]any) + if !ok { + if part == "on" { + switch triggers := parent[part].(type) { + case string: + child = map[string]any{triggers: nil} + case []any: + child = make(map[string]any, len(triggers)) + for _, trigger := range triggers { + name, ok := trigger.(string) + if !ok { + return nil, "", fmt.Errorf("cannot edit %q because on contains a non-string trigger", changePath) + } + child[name] = nil + } + } + if child != nil { + parent[part] = child + parent = child + continue + } + } + if parent[part] != nil { + return nil, "", fmt.Errorf("cannot edit %q because %q is not an object", changePath, part) + } + child = map[string]any{} + parent[part] = child + } + parent = child + } + return parent, path[len(path)-1], nil +} + +func applyEditChangeToParent(parent map[string]any, key string, change editChange) error { + switch change.kind { + case "set": + parent[key] = change.value + case "unset": + delete(parent, key) + case "add": + values, ok := parent[key].([]any) + if !ok && parent[key] != nil { + return fmt.Errorf("cannot add to %q because it is not a list", change.path) + } + if !slices.ContainsFunc(values, func(value any) bool { return fmt.Sprint(value) == fmt.Sprint(change.value) }) { + parent[key] = append(values, change.value) + } + case "remove": + values, ok := parent[key].([]any) + if !ok { + return fmt.Errorf("cannot remove from %q because it is not a list", change.path) + } + parent[key] = slices.DeleteFunc(values, func(value any) bool { return fmt.Sprint(value) == fmt.Sprint(change.value) }) + default: + return fmt.Errorf("unsupported edit operation %q", change.kind) + } + return nil +} + +func replaceFrontmatter(content string, frontmatter map[string]any) (string, error) { + encoded, err := yaml.MarshalWithOptions(frontmatter, yaml.Indent(2), yaml.IndentSequence(true)) + if err != nil { + return "", fmt.Errorf("encode frontmatter: %w", err) + } + start := strings.IndexByte(content, '\n') + if start < 0 || strings.TrimSpace(content[:start]) != "---" { + return "", errors.New("workflow must begin with YAML frontmatter") + } + end := strings.Index(content[start+1:], "\n---") + if end < 0 { + return "", errors.New("frontmatter not properly closed") + } + end += start + 1 + bodyStart := end + len("\n---") + if bodyStart < len(content) && content[bodyStart] == '\n' { + bodyStart++ + } + return "---\n" + string(encoded) + "---\n" + content[bodyStart:], nil +} + +func flagString(cmd *cobra.Command, name string) string { + value, _ := cmd.Flags().GetString(name) + return value +} + +func flagStrings(cmd *cobra.Command, name string) []string { + value, _ := cmd.Flags().GetStringArray(name) + return value +} + +func flagBool(cmd *cobra.Command, name string) bool { + value, _ := cmd.Flags().GetBool(name) + return value +} diff --git a/pkg/cli/edit_command_test.go b/pkg/cli/edit_command_test.go new file mode 100644 index 00000000000..5ee7ac1fa91 --- /dev/null +++ b/pkg/cli/edit_command_test.go @@ -0,0 +1,81 @@ +package cli + +import ( + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestEditChangesSupportAssignmentsSchedulesAndImports(t *testing.T) { + t.Parallel() + cmd := NewEditCommand() + require.NoError(t, cmd.Flags().Set("set", "max-turns=20")) + require.NoError(t, cmd.Flags().Set("schedule", "6h")) + require.NoError(t, cmd.Flags().Set("add-import", "shared/common.md")) + + changes, err := editChangesFromCommand(cmd, []string{"workflow", "model: small"}) + require.NoError(t, err) + frontmatter := map[string]any{"on": "workflow_dispatch"} + for _, change := range changes { + require.NoError(t, applyEditChange(frontmatter, change)) + } + + assert.Equal(t, "small", frontmatter["model"]) + assert.Equal(t, uint64(20), frontmatter["max-turns"]) + assert.Equal(t, []any{"shared/common.md"}, frontmatter["imports"]) + assert.Equal(t, map[string]any{ + "workflow_dispatch": nil, + "schedule": []any{map[string]any{"cron": "FUZZY:HOURLY/6 * * *"}}, + }, frontmatter["on"]) +} + +func TestEditCommandDryRunPreservesWorkflowFile(t *testing.T) { + t.Parallel() + dir := t.TempDir() + workflowPath := dir + "/workflow.md" + original := "---\non: workflow_dispatch\n---\n# Workflow\n" + require.NoError(t, os.WriteFile(workflowPath, []byte(original), 0o644)) + + cmd := NewEditCommand() + cmd.SetArgs([]string{workflowPath, "max-turns: 20", "--dry-run"}) + var output strings.Builder + cmd.SetOut(&output) + require.NoError(t, cmd.Execute()) + + assert.Contains(t, output.String(), "max-turns: 20") + content, err := os.ReadFile(workflowPath) + require.NoError(t, err) + assert.Equal(t, original, string(content)) +} + +func TestEditChangesAddImportsToObjectForm(t *testing.T) { + t.Parallel() + frontmatter := map[string]any{"imports": map[string]any{"aw": []any{"shared/base.md"}}} + require.NoError(t, applyEditChange(frontmatter, editChange{ + kind: "add", path: "imports", value: "shared/extra.md", + })) + assert.Equal(t, []any{"shared/base.md", "shared/extra.md"}, frontmatter["imports"].(map[string]any)["aw"]) +} + +func TestEditAssignmentParsesScheduleShorthands(t *testing.T) { + t.Parallel() + change, err := parseEditAssignment("on.schedule: weekdays", ":") + require.NoError(t, err) + frontmatter := map[string]any{"on": "workflow_dispatch"} + require.NoError(t, applyEditChange(frontmatter, change)) + assert.Equal(t, []any{map[string]any{"cron": "FUZZY:DAILY_WEEKDAYS * * *"}}, frontmatter["on"].(map[string]any)["schedule"]) +} + +func TestEditCommandRejectsSourceManagedWorkflow(t *testing.T) { + t.Parallel() + dir := t.TempDir() + workflowPath := dir + "/workflow.md" + require.NoError(t, os.WriteFile(workflowPath, []byte("---\nsource: owner/repo@v1\non: workflow_dispatch\n---\n"), 0o644)) + + cmd := NewEditCommand() + cmd.SetArgs([]string{workflowPath, "max-turns: 20"}) + assert.ErrorContains(t, cmd.Execute(), "source-managed") +} From 3f81f226b01ba20638f0dfb3ee1c81b38622c9c9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 19:08:00 +0000 Subject: [PATCH 3/9] Preserve edit frontmatter boundaries Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/edit_command.go | 50 +++++++++++++++++++++--------------- pkg/cli/edit_command_test.go | 8 ++++++ 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/pkg/cli/edit_command.go b/pkg/cli/edit_command.go index a73bc02bd1d..dccde1e4f20 100644 --- a/pkg/cli/edit_command.go +++ b/pkg/cli/edit_command.go @@ -43,7 +43,7 @@ validated before writing. Workflows managed by a source: declaration cannot be e } func runEditCommand(cmd *cobra.Command, args []string) error { - workflowPath, err := resolveWorkflowFileInDir(args[0], false, flagString(cmd, "dir")) + workflowPath, err := resolveWorkflowFileInDir(args[0], false, editFlagString(cmd, "dir")) if err != nil { return err } @@ -78,7 +78,7 @@ func runEditCommand(cmd *cobra.Command, args []string) error { if err != nil { return err } - if flagBool(cmd, "dry-run") { + if editFlagBool(cmd, "dry-run") { fmt.Fprint(cmd.OutOrStdout(), updated) return nil } @@ -123,7 +123,7 @@ func editChangesFromCommand(cmd *cobra.Command, args []string) ([]editChange, er changes = append(changes, change) } for _, name := range []string{"set", "add", "remove"} { - for _, assignment := range flagStrings(cmd, name) { + for _, assignment := range editFlagStrings(cmd, name) { change, err := parseEditAssignment(assignment, "=") if err != nil { return nil, err @@ -132,13 +132,13 @@ func editChangesFromCommand(cmd *cobra.Command, args []string) ([]editChange, er changes = append(changes, change) } } - for _, path := range flagStrings(cmd, "unset") { + for _, path := range editFlagStrings(cmd, "unset") { changes = append(changes, editChange{kind: "unset", path: path}) } - for _, importPath := range flagStrings(cmd, "add-import") { + for _, importPath := range editFlagStrings(cmd, "add-import") { changes = append(changes, editChange{kind: "add", path: "imports", value: importPath}) } - if schedule := flagString(cmd, "schedule"); schedule != "" { + if schedule := editFlagString(cmd, "schedule"); schedule != "" { change, err := scheduleChange(schedule) if err != nil { return nil, err @@ -295,33 +295,43 @@ func replaceFrontmatter(content string, frontmatter map[string]any) (string, err if err != nil { return "", fmt.Errorf("encode frontmatter: %w", err) } - start := strings.IndexByte(content, '\n') - if start < 0 || strings.TrimSpace(content[:start]) != "---" { + firstLineEnd := strings.IndexByte(content, '\n') + if firstLineEnd < 0 || strings.TrimSpace(content[:firstLineEnd]) != "---" { return "", errors.New("workflow must begin with YAML frontmatter") } - end := strings.Index(content[start+1:], "\n---") - if end < 0 { - return "", errors.New("frontmatter not properly closed") - } - end += start + 1 - bodyStart := end + len("\n---") - if bodyStart < len(content) && content[bodyStart] == '\n' { - bodyStart++ + bodyStart := firstLineEnd + 1 + for lineStart := bodyStart; lineStart <= len(content); { + lineEnd := strings.IndexByte(content[lineStart:], '\n') + if lineEnd < 0 { + lineEnd = len(content) + } else { + lineEnd += lineStart + } + if strings.TrimSpace(content[lineStart:lineEnd]) == "---" { + if lineEnd < len(content) { + lineEnd++ + } + return "---\n" + string(encoded) + "---\n" + content[lineEnd:], nil + } + if lineEnd == len(content) { + break + } + lineStart = lineEnd + 1 } - return "---\n" + string(encoded) + "---\n" + content[bodyStart:], nil + return "", errors.New("frontmatter not properly closed") } -func flagString(cmd *cobra.Command, name string) string { +func editFlagString(cmd *cobra.Command, name string) string { value, _ := cmd.Flags().GetString(name) return value } -func flagStrings(cmd *cobra.Command, name string) []string { +func editFlagStrings(cmd *cobra.Command, name string) []string { value, _ := cmd.Flags().GetStringArray(name) return value } -func flagBool(cmd *cobra.Command, name string) bool { +func editFlagBool(cmd *cobra.Command, name string) bool { value, _ := cmd.Flags().GetBool(name) return value } diff --git a/pkg/cli/edit_command_test.go b/pkg/cli/edit_command_test.go index 5ee7ac1fa91..c4ace53f938 100644 --- a/pkg/cli/edit_command_test.go +++ b/pkg/cli/edit_command_test.go @@ -69,6 +69,14 @@ func TestEditAssignmentParsesScheduleShorthands(t *testing.T) { assert.Equal(t, []any{map[string]any{"cron": "FUZZY:DAILY_WEEKDAYS * * *"}}, frontmatter["on"].(map[string]any)["schedule"]) } +func TestReplaceFrontmatterPreservesBodySeparators(t *testing.T) { + t.Parallel() + content := "---\non: workflow_dispatch\n---\n# Workflow\n\n---\nBody\n" + updated, err := replaceFrontmatter(content, map[string]any{"on": "push"}) + require.NoError(t, err) + assert.Contains(t, updated, "---\n# Workflow\n\n---\nBody\n") +} + func TestEditCommandRejectsSourceManagedWorkflow(t *testing.T) { t.Parallel() dir := t.TempDir() From de5e98cada238e37842a903f636bf6a885ef6187 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 19:10:25 +0000 Subject: [PATCH 4/9] Clarify edit lock file errors Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/edit_command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cli/edit_command.go b/pkg/cli/edit_command.go index dccde1e4f20..d7cdd97f3ba 100644 --- a/pkg/cli/edit_command.go +++ b/pkg/cli/edit_command.go @@ -91,7 +91,7 @@ func writeAndCompileEditedWorkflow(workflowPath string, content []byte, updated previousLock, lockErr := os.ReadFile(lockPath) lockExisted := lockErr == nil if lockErr != nil && !os.IsNotExist(lockErr) { - return fmt.Errorf("read generated workflow: %w", lockErr) + return fmt.Errorf("read generated lock file: %w", lockErr) } if err := os.WriteFile(workflowPath, []byte(updated), 0o644); err != nil { return fmt.Errorf("write workflow: %w", err) From 7fd2c0a6845b1272214bd9f7b1b9eb17f7c50f95 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 19:29:18 +0000 Subject: [PATCH 5/9] Extend specialized workflow edit flags Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/edit_command.go | 47 +++++++++++++++--------------------- pkg/cli/edit_command_test.go | 31 +++++++++++++++++++++--- 2 files changed, 48 insertions(+), 30 deletions(-) diff --git a/pkg/cli/edit_command.go b/pkg/cli/edit_command.go index d7cdd97f3ba..0ff9ca13cfe 100644 --- a/pkg/cli/edit_command.go +++ b/pkg/cli/edit_command.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/github/gh-aw/pkg/parser" + "github.com/github/gh-aw/pkg/stringutil" "github.com/goccy/go-yaml" "github.com/spf13/cobra" ) @@ -23,9 +24,10 @@ func NewEditCommand() *cobra.Command { The workflow-id may be a workflow name, a Markdown filename, or a path. Changes are validated before writing. Workflows managed by a source: declaration cannot be edited.`, Example: ` gh aw edit repo-assist "max-turns: 20" - gh aw edit repo-assist --schedule 6h + gh aw edit repo-assist --schedule "every 6h" gh aw edit repo-assist --set model=small --unset engine.model - gh aw edit repo-assist --add imports=shared/common.md`, + gh aw edit repo-assist --add-import shared/common.md + gh aw edit repo-assist --add-skill shared/review`, Args: cobra.RangeArgs(1, 2), RunE: runEditCommand, } @@ -34,7 +36,10 @@ validated before writing. Workflows managed by a source: declaration cannot be e cmd.Flags().StringArray("add", nil, "Append a value to a list (path=value)") cmd.Flags().StringArray("remove", nil, "Remove a value from a list (path=value)") cmd.Flags().StringArray("add-import", nil, "Append a workflow import path") - cmd.Flags().String("schedule", "", "Set a schedule using a duration, fuzzy schedule, or cron expression; use off to remove it") + cmd.Flags().StringArray("remove-import", nil, "Remove a workflow import path") + cmd.Flags().StringArray("add-skill", nil, "Append a workflow skill") + cmd.Flags().StringArray("remove-skill", nil, "Remove a workflow skill") + cmd.Flags().String("schedule", "", "Set a schedule using a fuzzy schedule or cron expression; use off to remove it") cmd.Flags().Bool("dry-run", false, "Validate changes without writing or compiling") cmd.Flags().StringP("dir", "d", "", "Workflow directory (default: $GH_AW_WORKFLOWS_DIR or .github/workflows)") cmd.ValidArgsFunction = CompleteWorkflowNames @@ -87,7 +92,7 @@ func runEditCommand(cmd *cobra.Command, args []string) error { } func writeAndCompileEditedWorkflow(workflowPath string, content []byte, updated string) error { - lockPath := strings.TrimSuffix(workflowPath, ".md") + ".lock.yml" + lockPath := stringutil.MarkdownToLockFile(workflowPath) previousLock, lockErr := os.ReadFile(lockPath) lockExisted := lockErr == nil if lockErr != nil && !os.IsNotExist(lockErr) { @@ -138,6 +143,15 @@ func editChangesFromCommand(cmd *cobra.Command, args []string) ([]editChange, er for _, importPath := range editFlagStrings(cmd, "add-import") { changes = append(changes, editChange{kind: "add", path: "imports", value: importPath}) } + for _, importPath := range editFlagStrings(cmd, "remove-import") { + changes = append(changes, editChange{kind: "remove", path: "imports", value: importPath}) + } + for _, skill := range editFlagStrings(cmd, "add-skill") { + changes = append(changes, editChange{kind: "add", path: "skills", value: skill}) + } + for _, skill := range editFlagStrings(cmd, "remove-skill") { + changes = append(changes, editChange{kind: "remove", path: "skills", value: skill}) + } if schedule := editFlagString(cmd, "schedule"); schedule != "" { change, err := scheduleChange(schedule) if err != nil { @@ -148,32 +162,11 @@ func editChangesFromCommand(cmd *cobra.Command, args []string) ([]editChange, er return changes, nil } -func isCompactSchedule(schedule string) bool { - if len(schedule) < 2 { - return false - } - unit := schedule[len(schedule)-1] - if !strings.ContainsRune("hdwm", rune(unit)) { - return false - } - for _, char := range schedule[:len(schedule)-1] { - if char < '0' || char > '9' { - return false - } - } - return true -} - func scheduleChange(schedule string) (editChange, error) { - schedule = strings.TrimSpace(strings.ToLower(schedule)) - if schedule == "off" { + schedule = strings.TrimSpace(schedule) + if strings.EqualFold(schedule, "off") { return editChange{kind: "unset", path: "on.schedule"}, nil } - if schedule == "weekdays" { - schedule = "daily on weekdays" - } else if isCompactSchedule(schedule) { - schedule = "every " + schedule - } cron, _, err := parser.ParseSchedule(schedule) if err != nil { return editChange{}, fmt.Errorf("invalid schedule: %w", err) diff --git a/pkg/cli/edit_command_test.go b/pkg/cli/edit_command_test.go index c4ace53f938..7a20c49cff1 100644 --- a/pkg/cli/edit_command_test.go +++ b/pkg/cli/edit_command_test.go @@ -13,8 +13,9 @@ func TestEditChangesSupportAssignmentsSchedulesAndImports(t *testing.T) { t.Parallel() cmd := NewEditCommand() require.NoError(t, cmd.Flags().Set("set", "max-turns=20")) - require.NoError(t, cmd.Flags().Set("schedule", "6h")) + require.NoError(t, cmd.Flags().Set("schedule", "every 6h")) require.NoError(t, cmd.Flags().Set("add-import", "shared/common.md")) + require.NoError(t, cmd.Flags().Set("add-skill", "shared/review")) changes, err := editChangesFromCommand(cmd, []string{"workflow", "model: small"}) require.NoError(t, err) @@ -26,6 +27,7 @@ func TestEditChangesSupportAssignmentsSchedulesAndImports(t *testing.T) { assert.Equal(t, "small", frontmatter["model"]) assert.Equal(t, uint64(20), frontmatter["max-turns"]) assert.Equal(t, []any{"shared/common.md"}, frontmatter["imports"]) + assert.Equal(t, []any{"shared/review"}, frontmatter["skills"]) assert.Equal(t, map[string]any{ "workflow_dispatch": nil, "schedule": []any{map[string]any{"cron": "FUZZY:HOURLY/6 * * *"}}, @@ -57,12 +59,35 @@ func TestEditChangesAddImportsToObjectForm(t *testing.T) { require.NoError(t, applyEditChange(frontmatter, editChange{ kind: "add", path: "imports", value: "shared/extra.md", })) - assert.Equal(t, []any{"shared/base.md", "shared/extra.md"}, frontmatter["imports"].(map[string]any)["aw"]) + require.NoError(t, applyEditChange(frontmatter, editChange{ + kind: "remove", path: "imports", value: "shared/base.md", + })) + assert.Equal(t, []any{"shared/extra.md"}, frontmatter["imports"].(map[string]any)["aw"]) +} + +func TestEditChangesRemoveImportsAndSkills(t *testing.T) { + t.Parallel() + cmd := NewEditCommand() + require.NoError(t, cmd.Flags().Set("remove-import", "shared/base.md")) + require.NoError(t, cmd.Flags().Set("remove-skill", "shared/base")) + + changes, err := editChangesFromCommand(cmd, []string{"workflow"}) + require.NoError(t, err) + frontmatter := map[string]any{ + "imports": []any{"shared/base.md", "shared/extra.md"}, + "skills": []any{"shared/base", "shared/review"}, + } + for _, change := range changes { + require.NoError(t, applyEditChange(frontmatter, change)) + } + + assert.Equal(t, []any{"shared/extra.md"}, frontmatter["imports"]) + assert.Equal(t, []any{"shared/review"}, frontmatter["skills"]) } func TestEditAssignmentParsesScheduleShorthands(t *testing.T) { t.Parallel() - change, err := parseEditAssignment("on.schedule: weekdays", ":") + change, err := parseEditAssignment("on.schedule: daily on weekdays", ":") require.NoError(t, err) frontmatter := map[string]any{"on": "workflow_dispatch"} require.NoError(t, applyEditChange(frontmatter, change)) From 4c2976e8a65bbe14ea843b45a9c12dff9cef2503 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 20:23:50 +0000 Subject: [PATCH 6/9] Add draft ADR for schema-validated workflow frontmatter edit command Documents the architectural decision to introduce `gh aw edit` as a dedicated, schema-validated mutation path for workflow frontmatter, including alternatives considered and consequences. Co-Authored-By: Claude Sonnet 4.6 --- ...dated-workflow-frontmatter-edit-command.md | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 docs/adr/55475-schema-validated-workflow-frontmatter-edit-command.md diff --git a/docs/adr/55475-schema-validated-workflow-frontmatter-edit-command.md b/docs/adr/55475-schema-validated-workflow-frontmatter-edit-command.md new file mode 100644 index 00000000000..110202f6ba6 --- /dev/null +++ b/docs/adr/55475-schema-validated-workflow-frontmatter-edit-command.md @@ -0,0 +1,45 @@ +# ADR-55475: Schema-Validated Workflow Frontmatter Edit Command + +**Date**: 2026-08-24 +**Status**: Draft +**Deciders**: pelikhan, copilot-swe-agent + +--- + +### Context + +Workflow definitions in this project are Markdown files with YAML frontmatter that controls execution parameters (e.g., `max-turns`, `model`, `on.schedule`, `imports`). To change any of these parameters today, users must manually edit the raw Markdown file and then separately run `gh aw compile` to regenerate the corresponding `.lock.yml` file. Manual editing bypasses schema validation entirely, meaning invalid frontmatter values are only detected at compile time—after the file has already been written to disk. The need for an explicit, validated, and atomic path for mutating workflow frontmatter is the driving problem this PR addresses. + +### Decision + +We will add a new `gh aw edit` CLI command (`pkg/cli/edit_command.go`) that provides schema-validated, programmatic mutation of workflow frontmatter with automatic recompilation. The command accepts typed flag-based mutations (`--set`, `--unset`, `--add`, `--remove`, `--schedule`, `--add-import`, `--add-skill`) and a positional `path: value` shorthand. It validates the resulting frontmatter against the workflow schema before writing and immediately recompiles the `.lock.yml`; on compilation failure it rolls back both files to their prior state. + +### Alternatives Considered + +#### Alternative 1: Direct file editing + manual compile + +Users continue to edit YAML frontmatter by hand and run `gh aw compile` separately. This requires no new code but provides no schema validation at edit time, allows invalid frontmatter to be committed before compile, and leaves the lock file in an inconsistent state when a user forgets to recompile. It was rejected because it does not address the safety problem. + +#### Alternative 2: Extend `gh aw update` with frontmatter mutation flags + +Add mutation flags to the existing `update` command. The `update` command is semantically about syncing a workflow from a `source:` declaration. Mixing configuration mutation into the same command would create a confusing API where the same command both fetches external content and edits local state. It was rejected to preserve the clarity of the existing command model. + +### Consequences + +#### Positive +- Schema validation runs before any bytes are written to disk, preventing invalid frontmatter values from ever reaching the repository. +- Automatic recompilation keeps `.lock.yml` atomically in sync with the edited workflow file. +- Transactional rollback: if recompilation fails, both the workflow source file and the lock file are restored to their pre-edit state. +- Fuzzy schedule shorthands (`1h`, `weekdays`, etc.) are normalized to cron expressions at edit time, providing a user-friendly schedule API. + +#### Negative +- Source-managed workflows (those with a `source:` key in their frontmatter) are explicitly rejected by the command; users must edit the upstream source or pin/unpin and then run `gh aw update` instead. +- YAML re-serialization via `go-yaml` may alter key ordering, indentation, or whitespace in the frontmatter beyond the intended change, potentially producing noisy diffs. + +#### Neutral +- The command is labelled "Experimental" in its `Short` and `Long` help text, signalling that its interface may change before stabilization. +- The command is registered in the `setup` group alongside `add`, `update`, and `remove`, maintaining consistency with the existing command taxonomy. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From 8d95230f2fce0d5452bc4d1ed9e2dad0533167e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 20:24:03 +0000 Subject: [PATCH 7/9] Add workflow edit integration tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/edit_command.go | 4 +- pkg/cli/edit_command_integration_test.go | 128 +++++++++++++++++++++++ pkg/cli/edit_command_test.go | 7 +- 3 files changed, 132 insertions(+), 7 deletions(-) create mode 100644 pkg/cli/edit_command_integration_test.go diff --git a/pkg/cli/edit_command.go b/pkg/cli/edit_command.go index 0ff9ca13cfe..ad9ef3af39d 100644 --- a/pkg/cli/edit_command.go +++ b/pkg/cli/edit_command.go @@ -167,11 +167,11 @@ func scheduleChange(schedule string) (editChange, error) { if strings.EqualFold(schedule, "off") { return editChange{kind: "unset", path: "on.schedule"}, nil } - cron, _, err := parser.ParseSchedule(schedule) + _, _, err := parser.ParseSchedule(schedule) if err != nil { return editChange{}, fmt.Errorf("invalid schedule: %w", err) } - return editChange{kind: "set", path: "on.schedule", value: []any{map[string]any{"cron": cron}}}, nil + return editChange{kind: "set", path: "on.schedule", value: schedule}, nil } func parseEditAssignment(assignment, separator string) (editChange, error) { diff --git a/pkg/cli/edit_command_integration_test.go b/pkg/cli/edit_command_integration_test.go new file mode 100644 index 00000000000..81012c4ad4e --- /dev/null +++ b/pkg/cli/edit_command_integration_test.go @@ -0,0 +1,128 @@ +//go:build integration + +package cli + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/github/gh-aw/pkg/parser" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestEditCommandIntegrationMutations(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + workflowPath := filepath.Join(setup.workflowsDir, "edit.md") + require.NoError(t, os.MkdirAll(filepath.Join(setup.workflowsDir, "shared"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(setup.workflowsDir, "shared", "base.md"), []byte("# Base\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(setup.workflowsDir, "shared", "extra.md"), []byte("# Extra\n"), 0o644)) + require.NoError(t, os.WriteFile(workflowPath, []byte(editIntegrationWorkflow), 0o644)) + + requireEditSucceeds(t, setup, "edit", "edit", "max-turns: 20") + requireEditSucceeds(t, setup, "edit", "edit.md", "--schedule", "daily on weekdays") + requireEditSucceeds(t, setup, "edit", workflowPath, "--add-import", "shared/extra.md") + requireEditSucceeds(t, setup, "edit", "edit", "--remove-import", "shared/base.md") + requireEditSucceeds(t, setup, "edit", "edit", "--add-skill", "owner/repo/skills/review@0123456789012345678901234567890123456789") + requireEditSucceeds(t, setup, "edit", "edit", "--remove-skill", "owner/repo/skills/base@0123456789012345678901234567890123456789") + requireEditSucceeds(t, setup, "edit", "edit", "--set", "model=small", "--unset", "description") + requireEditSucceeds(t, setup, "edit", "edit", "--add", "labels=one", "--remove", "labels=base") + + content, err := os.ReadFile(workflowPath) + require.NoError(t, err) + frontmatter, err := parser.ExtractFrontmatterFromContent(string(content)) + require.NoError(t, err) + assert.Equal(t, uint64(20), frontmatter.Frontmatter["max-turns"]) + assert.Equal(t, "small", frontmatter.Frontmatter["model"]) + assert.NotContains(t, frontmatter.Frontmatter, "description") + assert.Equal(t, []any{"shared/extra.md"}, frontmatter.Frontmatter["imports"]) + assert.Equal(t, []any{"owner/repo/skills/review@0123456789012345678901234567890123456789"}, frontmatter.Frontmatter["skills"]) + assert.Equal(t, []any{"one"}, frontmatter.Frontmatter["labels"]) + assert.Equal(t, map[string]any{ + "schedule": "daily on weekdays", + "workflow_dispatch": nil, + }, frontmatter.Frontmatter["on"]) + + lockContent, err := os.ReadFile(filepath.Join(setup.workflowsDir, "edit.lock.yml")) + require.NoError(t, err) + assert.Contains(t, string(lockContent), "Edit Integration Workflow") +} + +func TestEditCommandIntegrationDryRunAndFailureSafety(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + workflowPath := filepath.Join(setup.workflowsDir, "edit.md") + require.NoError(t, os.WriteFile(workflowPath, []byte(editIntegrationWorkflow), 0o644)) + original, err := os.ReadFile(workflowPath) + require.NoError(t, err) + + output := requireEditSucceeds(t, setup, "edit", "edit", "--set", "max-turns=20", "--dry-run") + assert.Contains(t, output, "max-turns: 20") + content, err := os.ReadFile(workflowPath) + require.NoError(t, err) + assert.Equal(t, original, content) + _, err = os.Stat(filepath.Join(setup.workflowsDir, "edit.lock.yml")) + assert.ErrorIs(t, err, os.ErrNotExist) + + requireEditSucceeds(t, setup, "edit", "edit", "--set", "max-turns=10") + beforeFailure, err := os.ReadFile(workflowPath) + require.NoError(t, err) + lockPath := filepath.Join(setup.workflowsDir, "edit.lock.yml") + beforeFailureLock, err := os.ReadFile(lockPath) + require.NoError(t, err) + + output = requireEditFails(t, setup, "edit", "edit", "--add-import", "shared/missing.md") + assert.Contains(t, output, "compile edited workflow") + content, err = os.ReadFile(workflowPath) + require.NoError(t, err) + assert.Equal(t, beforeFailure, content) + lockContent, err := os.ReadFile(lockPath) + require.NoError(t, err) + assert.Equal(t, beforeFailureLock, lockContent) + + require.NoError(t, os.WriteFile(workflowPath, []byte("---\nsource: owner/repo@v1\non: workflow_dispatch\n---\n# Managed\n"), 0o644)) + beforeManaged, err := os.ReadFile(workflowPath) + require.NoError(t, err) + output = requireEditFails(t, setup, "edit", "edit", "--set", "max-turns=20") + assert.Contains(t, output, "source-managed") + content, err = os.ReadFile(workflowPath) + require.NoError(t, err) + assert.Equal(t, beforeManaged, content) +} + +func requireEditSucceeds(t *testing.T, setup *integrationTestSetup, args ...string) string { + t.Helper() + command := exec.Command(setup.binaryPath, args...) + command.Dir = setup.tempDir + output, err := command.CombinedOutput() + require.NoError(t, err, "gh aw %v failed:\n%s", args, output) + return string(output) +} + +func requireEditFails(t *testing.T, setup *integrationTestSetup, args ...string) string { + t.Helper() + command := exec.Command(setup.binaryPath, args...) + command.Dir = setup.tempDir + output, err := command.CombinedOutput() + require.Error(t, err, "gh aw %v unexpectedly succeeded:\n%s", args, output) + return string(output) +} + +const editIntegrationWorkflow = `--- +description: Edit integration workflow +labels: [base] +on: + workflow_dispatch: +skills: + - owner/repo/skills/base@0123456789012345678901234567890123456789 +engine: claude +--- +# Edit Integration Workflow + +Test workflow. +` diff --git a/pkg/cli/edit_command_test.go b/pkg/cli/edit_command_test.go index 7a20c49cff1..37e8263cfd2 100644 --- a/pkg/cli/edit_command_test.go +++ b/pkg/cli/edit_command_test.go @@ -28,10 +28,7 @@ func TestEditChangesSupportAssignmentsSchedulesAndImports(t *testing.T) { assert.Equal(t, uint64(20), frontmatter["max-turns"]) assert.Equal(t, []any{"shared/common.md"}, frontmatter["imports"]) assert.Equal(t, []any{"shared/review"}, frontmatter["skills"]) - assert.Equal(t, map[string]any{ - "workflow_dispatch": nil, - "schedule": []any{map[string]any{"cron": "FUZZY:HOURLY/6 * * *"}}, - }, frontmatter["on"]) + assert.Equal(t, map[string]any{"workflow_dispatch": nil, "schedule": "every 6h"}, frontmatter["on"]) } func TestEditCommandDryRunPreservesWorkflowFile(t *testing.T) { @@ -91,7 +88,7 @@ func TestEditAssignmentParsesScheduleShorthands(t *testing.T) { require.NoError(t, err) frontmatter := map[string]any{"on": "workflow_dispatch"} require.NoError(t, applyEditChange(frontmatter, change)) - assert.Equal(t, []any{map[string]any{"cron": "FUZZY:DAILY_WEEKDAYS * * *"}}, frontmatter["on"].(map[string]any)["schedule"]) + assert.Equal(t, "daily on weekdays", frontmatter["on"].(map[string]any)["schedule"]) } func TestReplaceFrontmatterPreservesBodySeparators(t *testing.T) { From 502dfcd297d884f98d80ae0cde0335edefc6df5e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 21:02:49 +0000 Subject: [PATCH 8/9] Preserve trigger shape and harden edit writes Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- ...dated-workflow-frontmatter-edit-command.md | 4 +- pkg/cli/edit_command.go | 194 +++++++++++++----- pkg/cli/edit_command_integration_test.go | 9 + pkg/cli/edit_command_test.go | 111 +++++++++- 4 files changed, 260 insertions(+), 58 deletions(-) diff --git a/docs/adr/55475-schema-validated-workflow-frontmatter-edit-command.md b/docs/adr/55475-schema-validated-workflow-frontmatter-edit-command.md index 110202f6ba6..4b2cc7cc3e2 100644 --- a/docs/adr/55475-schema-validated-workflow-frontmatter-edit-command.md +++ b/docs/adr/55475-schema-validated-workflow-frontmatter-edit-command.md @@ -30,11 +30,11 @@ Add mutation flags to the existing `update` command. The `update` command is sem - Schema validation runs before any bytes are written to disk, preventing invalid frontmatter values from ever reaching the repository. - Automatic recompilation keeps `.lock.yml` atomically in sync with the edited workflow file. - Transactional rollback: if recompilation fails, both the workflow source file and the lock file are restored to their pre-edit state. -- Fuzzy schedule shorthands (`1h`, `weekdays`, etc.) are normalized to cron expressions at edit time, providing a user-friendly schedule API. +- Fuzzy schedule expressions (`daily`, `every 6h`, `daily on weekdays`, etc.) are validated with the shared schedule parser at edit time, providing a user-friendly schedule API. #### Negative - Source-managed workflows (those with a `source:` key in their frontmatter) are explicitly rejected by the command; users must edit the upstream source or pin/unpin and then run `gh aw update` instead. -- YAML re-serialization via `go-yaml` may alter key ordering, indentation, or whitespace in the frontmatter beyond the intended change, potentially producing noisy diffs. +- YAML re-serialization via `go-yaml` may alter key ordering, indentation, comments, or whitespace in the frontmatter beyond the intended change, potentially producing noisy diffs. Edits that change nothing are detected and skip writing entirely, so no-op edits never rewrite a workflow. #### Neutral - The command is labelled "Experimental" in its `Short` and `Long` help text, signalling that its interface may change before stabilization. diff --git a/pkg/cli/edit_command.go b/pkg/cli/edit_command.go index ad9ef3af39d..af97c88213f 100644 --- a/pkg/cli/edit_command.go +++ b/pkg/cli/edit_command.go @@ -5,11 +5,14 @@ import ( "errors" "fmt" "os" + "path/filepath" + "reflect" "slices" "strings" "github.com/github/gh-aw/pkg/parser" "github.com/github/gh-aw/pkg/stringutil" + "github.com/github/gh-aw/pkg/workflow" "github.com/goccy/go-yaml" "github.com/spf13/cobra" ) @@ -22,7 +25,10 @@ func NewEditCommand() *cobra.Command { Long: `Experimental: edit schema-validated workflow frontmatter and recompile its generated file. The workflow-id may be a workflow name, a Markdown filename, or a path. Changes are -validated before writing. Workflows managed by a source: declaration cannot be edited.`, +validated before writing. Workflows managed by a source: declaration cannot be edited. + +Edits that change nothing leave the workflow untouched. When frontmatter does change it is +re-serialized, so YAML comments, key ordering, and quoting styles are not preserved.`, Example: ` gh aw edit repo-assist "max-turns: 20" gh aw edit repo-assist --schedule "every 6h" gh aw edit repo-assist --set model=small --unset engine.model @@ -71,10 +77,17 @@ func runEditCommand(cmd *cobra.Command, args []string) error { if len(changes) == 0 { return errors.New("provide an assignment or an edit flag") } + edited := false for _, change := range changes { - if err := applyEditChange(parsed.Frontmatter, change); err != nil { + applied, err := applyEditChange(parsed.Frontmatter, change) + if err != nil { return err } + edited = edited || applied + } + if !edited { + fmt.Fprintln(cmd.OutOrStdout(), "workflow already matches the requested changes") + return nil } if err := parser.ValidateMainWorkflowFrontmatterWithSchemaAndLocation(parsed.Frontmatter, workflowPath); err != nil { return fmt.Errorf("invalid edited workflow: %w", err) @@ -88,31 +101,63 @@ func runEditCommand(cmd *cobra.Command, args []string) error { return nil } - return writeAndCompileEditedWorkflow(workflowPath, content, updated) + return writeAndCompileEditedWorkflow(cmd.Context(), workflowPath, content, updated) } -func writeAndCompileEditedWorkflow(workflowPath string, content []byte, updated string) error { +func writeAndCompileEditedWorkflow(ctx context.Context, workflowPath string, content []byte, updated string) error { lockPath := stringutil.MarkdownToLockFile(workflowPath) previousLock, lockErr := os.ReadFile(lockPath) lockExisted := lockErr == nil if lockErr != nil && !os.IsNotExist(lockErr) { return fmt.Errorf("read generated lock file: %w", lockErr) } - if err := os.WriteFile(workflowPath, []byte(updated), 0o644); err != nil { + if err := writeFileAtomically(workflowPath, []byte(updated)); err != nil { return fmt.Errorf("write workflow: %w", err) } - if err := compileWorkflow(context.Background(), workflowPath, false, true, ""); err != nil { - _ = os.WriteFile(workflowPath, content, 0o644) - if lockExisted { - _ = os.WriteFile(lockPath, previousLock, 0o644) - } else { - _ = os.Remove(lockPath) - } - return fmt.Errorf("compile edited workflow: %w", err) + if err := compileWorkflow(ctx, workflowPath, false, true, ""); err != nil { + return errors.Join(fmt.Errorf("compile edited workflow: %w", err), restoreEditedWorkflow(workflowPath, content, lockPath, previousLock, lockExisted)) } return nil } +// restoreEditedWorkflow puts the workflow and its generated file back to their pre-edit state. +func restoreEditedWorkflow(workflowPath string, content []byte, lockPath string, previousLock []byte, lockExisted bool) error { + var errs []error + if err := writeFileAtomically(workflowPath, content); err != nil { + errs = append(errs, fmt.Errorf("restore workflow: %w", err)) + } + if lockExisted { + if err := writeFileAtomically(lockPath, previousLock); err != nil { + errs = append(errs, fmt.Errorf("restore generated lock file: %w", err)) + } + } else if err := os.Remove(lockPath); err != nil && !os.IsNotExist(err) { + errs = append(errs, fmt.Errorf("remove generated lock file: %w", err)) + } + return errors.Join(errs...) +} + +// writeFileAtomically writes content through a sibling temporary file so a failed +// write never truncates the destination. +func writeFileAtomically(path string, content []byte) error { + file, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".*") + if err != nil { + return err + } + tempPath := file.Name() + defer func() { _ = os.Remove(tempPath) }() + if _, err := file.Write(content); err != nil { + _ = file.Close() + return err + } + if err := file.Close(); err != nil { + return err + } + if err := os.Chmod(tempPath, 0o644); err != nil { + return err + } + return os.Rename(tempPath, path) +} + type editChange struct { kind, path string value any @@ -187,14 +232,20 @@ func parseEditAssignment(assignment, separator string) (editChange, error) { return editChange{kind: "set", path: path, value: value["value"]}, nil } -func applyEditChange(frontmatter map[string]any, change editChange) error { +// applyEditChange applies a change and reports whether the frontmatter was modified. +func applyEditChange(frontmatter map[string]any, change editChange) (bool, error) { change, err := normalizeEditChange(frontmatter, change) if err != nil { - return err + return false, err } - parent, key, err := editChangeParent(frontmatter, change.path) + parent, key, err := editChangeParent(frontmatter, change.path, change.kind != "unset") if err != nil { - return err + return false, err + } + if parent == nil { + // The target does not exist, so an unset is a no-op and the original + // frontmatter representation is preserved. + return false, nil } return applyEditChangeToParent(parent, key, change) } @@ -217,7 +268,10 @@ func normalizeEditChange(frontmatter map[string]any, change editChange) (editCha return change, nil } -func editChangeParent(frontmatter map[string]any, changePath string) (map[string]any, string, error) { +// editChangeParent walks changePath and returns the map holding its last segment. +// When create is false, a missing or non-object ancestor yields a nil parent so the +// caller can treat the change as a no-op instead of rewriting the frontmatter. +func editChangeParent(frontmatter map[string]any, changePath string, create bool) (map[string]any, string, error) { path := strings.Split(changePath, ".") if slices.Contains(path, "") { return nil, "", fmt.Errorf("invalid frontmatter path %q", changePath) @@ -226,30 +280,20 @@ func editChangeParent(frontmatter map[string]any, changePath string) (map[string for _, part := range path[:len(path)-1] { child, ok := parent[part].(map[string]any) if !ok { - if part == "on" { - switch triggers := parent[part].(type) { - case string: - child = map[string]any{triggers: nil} - case []any: - child = make(map[string]any, len(triggers)) - for _, trigger := range triggers { - name, ok := trigger.(string) - if !ok { - return nil, "", fmt.Errorf("cannot edit %q because on contains a non-string trigger", changePath) - } - child[name] = nil - } - } - if child != nil { - parent[part] = child - parent = child - continue - } + if !create { + return nil, "", nil } - if parent[part] != nil { + if parent[part] == nil { + child = map[string]any{} + } else if part == "on" { + expanded, err := expandTriggers(parent[part]) + if err != nil { + return nil, "", fmt.Errorf("cannot edit %q: %w", changePath, err) + } + child = expanded + } else { return nil, "", fmt.Errorf("cannot edit %q because %q is not an object", changePath, part) } - child = map[string]any{} parent[part] = child } parent = child @@ -257,30 +301,86 @@ func editChangeParent(frontmatter map[string]any, changePath string) (map[string return parent, path[len(path)-1], nil } -func applyEditChangeToParent(parent map[string]any, key string, change editChange) error { +// expandTriggers converts the shorthand forms accepted by the on: field into the +// equivalent trigger object so a trigger can be edited without changing semantics. +func expandTriggers(triggers any) (map[string]any, error) { + switch value := triggers.(type) { + case map[string]any: + return value, nil + case string: + return expandTriggerString(value) + case []any: + expanded := make(map[string]any, len(value)) + for _, trigger := range value { + name, ok := trigger.(string) + if !ok { + return nil, errors.New("on contains a non-string trigger") + } + expanded[name] = nil + } + return expanded, nil + } + return nil, fmt.Errorf("on is not a trigger object, got %T", triggers) +} + +func expandTriggerString(triggers string) (map[string]any, error) { + triggers = strings.TrimSpace(triggers) + if _, _, err := parser.ParseSchedule(triggers); err == nil { + return map[string]any{"schedule": triggers, "workflow_dispatch": nil}, nil + } + trigger, err := workflow.ParseTriggerShorthand(triggers) + if err != nil { + return nil, fmt.Errorf("on shorthand %q is not a recognized trigger: %w", triggers, err) + } + if trigger != nil { + if len(trigger.Conditions) > 0 { + return nil, fmt.Errorf("expand the on shorthand %q into its object form before editing it", triggers) + } + return trigger.ToYAMLMap(), nil + } + if strings.ContainsAny(triggers, " \t/") { + return nil, fmt.Errorf("expand the on shorthand %q into its object form before editing it", triggers) + } + return map[string]any{triggers: nil}, nil +} + +// applyEditChangeToParent applies a change to parent and reports whether it modified it. +func applyEditChangeToParent(parent map[string]any, key string, change editChange) (bool, error) { switch change.kind { case "set": + existing, exists := parent[key] + if exists && reflect.DeepEqual(existing, change.value) { + return false, nil + } parent[key] = change.value case "unset": + if _, exists := parent[key]; !exists { + return false, nil + } delete(parent, key) case "add": values, ok := parent[key].([]any) if !ok && parent[key] != nil { - return fmt.Errorf("cannot add to %q because it is not a list", change.path) + return false, fmt.Errorf("cannot add to %q because it is not a list", change.path) } - if !slices.ContainsFunc(values, func(value any) bool { return fmt.Sprint(value) == fmt.Sprint(change.value) }) { - parent[key] = append(values, change.value) + if slices.ContainsFunc(values, func(value any) bool { return reflect.DeepEqual(value, change.value) }) { + return false, nil } + parent[key] = append(values, change.value) case "remove": values, ok := parent[key].([]any) if !ok { - return fmt.Errorf("cannot remove from %q because it is not a list", change.path) + return false, fmt.Errorf("cannot remove from %q because it is not a list", change.path) } - parent[key] = slices.DeleteFunc(values, func(value any) bool { return fmt.Sprint(value) == fmt.Sprint(change.value) }) + remaining := slices.DeleteFunc(slices.Clone(values), func(value any) bool { return reflect.DeepEqual(value, change.value) }) + if len(remaining) == len(values) { + return false, nil + } + parent[key] = remaining default: - return fmt.Errorf("unsupported edit operation %q", change.kind) + return false, fmt.Errorf("unsupported edit operation %q", change.kind) } - return nil + return true, nil } func replaceFrontmatter(content string, frontmatter map[string]any) (string, error) { diff --git a/pkg/cli/edit_command_integration_test.go b/pkg/cli/edit_command_integration_test.go index 81012c4ad4e..ec46b879935 100644 --- a/pkg/cli/edit_command_integration_test.go +++ b/pkg/cli/edit_command_integration_test.go @@ -85,6 +85,15 @@ func TestEditCommandIntegrationDryRunAndFailureSafety(t *testing.T) { require.NoError(t, err) assert.Equal(t, beforeFailureLock, lockContent) + shorthandPath := filepath.Join(setup.workflowsDir, "shorthand.md") + shorthand := "---\n# keep this comment\non: push\nengine: copilot\n---\n# Shorthand Workflow\n" + require.NoError(t, os.WriteFile(shorthandPath, []byte(shorthand), 0o644)) + output = requireEditSucceeds(t, setup, "edit", "shorthand", "--schedule", "off") + assert.Contains(t, output, "already matches") + content, err = os.ReadFile(shorthandPath) + require.NoError(t, err) + assert.Equal(t, shorthand, string(content)) + require.NoError(t, os.WriteFile(workflowPath, []byte("---\nsource: owner/repo@v1\non: workflow_dispatch\n---\n# Managed\n"), 0o644)) beforeManaged, err := os.ReadFile(workflowPath) require.NoError(t, err) diff --git a/pkg/cli/edit_command_test.go b/pkg/cli/edit_command_test.go index 37e8263cfd2..05b7ae54def 100644 --- a/pkg/cli/edit_command_test.go +++ b/pkg/cli/edit_command_test.go @@ -21,7 +21,7 @@ func TestEditChangesSupportAssignmentsSchedulesAndImports(t *testing.T) { require.NoError(t, err) frontmatter := map[string]any{"on": "workflow_dispatch"} for _, change := range changes { - require.NoError(t, applyEditChange(frontmatter, change)) + mustApplyEditChange(t, frontmatter, change) } assert.Equal(t, "small", frontmatter["model"]) @@ -53,12 +53,8 @@ func TestEditCommandDryRunPreservesWorkflowFile(t *testing.T) { func TestEditChangesAddImportsToObjectForm(t *testing.T) { t.Parallel() frontmatter := map[string]any{"imports": map[string]any{"aw": []any{"shared/base.md"}}} - require.NoError(t, applyEditChange(frontmatter, editChange{ - kind: "add", path: "imports", value: "shared/extra.md", - })) - require.NoError(t, applyEditChange(frontmatter, editChange{ - kind: "remove", path: "imports", value: "shared/base.md", - })) + mustApplyEditChange(t, frontmatter, editChange{kind: "add", path: "imports", value: "shared/extra.md"}) + mustApplyEditChange(t, frontmatter, editChange{kind: "remove", path: "imports", value: "shared/base.md"}) assert.Equal(t, []any{"shared/extra.md"}, frontmatter["imports"].(map[string]any)["aw"]) } @@ -75,7 +71,7 @@ func TestEditChangesRemoveImportsAndSkills(t *testing.T) { "skills": []any{"shared/base", "shared/review"}, } for _, change := range changes { - require.NoError(t, applyEditChange(frontmatter, change)) + mustApplyEditChange(t, frontmatter, change) } assert.Equal(t, []any{"shared/extra.md"}, frontmatter["imports"]) @@ -87,7 +83,7 @@ func TestEditAssignmentParsesScheduleShorthands(t *testing.T) { change, err := parseEditAssignment("on.schedule: daily on weekdays", ":") require.NoError(t, err) frontmatter := map[string]any{"on": "workflow_dispatch"} - require.NoError(t, applyEditChange(frontmatter, change)) + mustApplyEditChange(t, frontmatter, change) assert.Equal(t, "daily on weekdays", frontmatter["on"].(map[string]any)["schedule"]) } @@ -109,3 +105,100 @@ func TestEditCommandRejectsSourceManagedWorkflow(t *testing.T) { cmd.SetArgs([]string{workflowPath, "max-turns: 20"}) assert.ErrorContains(t, cmd.Execute(), "source-managed") } + +// mustApplyEditChange applies a change and asserts that it modified the frontmatter. +func mustApplyEditChange(t *testing.T, frontmatter map[string]any, change editChange) { + t.Helper() + applied, err := applyEditChange(frontmatter, change) + require.NoError(t, err) + assert.True(t, applied, "expected the change to modify the frontmatter") +} + +func TestEditChangeKeepsShorthandTriggersWhenScheduleIsAbsent(t *testing.T) { + t.Parallel() + for name, triggers := range map[string]any{ + "string": "push", + "list": []any{"push", "workflow_dispatch"}, + "map": map[string]any{"push": nil}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + frontmatter := map[string]any{"on": triggers} + applied, err := applyEditChange(frontmatter, editChange{kind: "unset", path: "on.schedule"}) + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, triggers, frontmatter["on"]) + }) + } +} + +func TestEditChangeExpandsScheduleAndTriggerShorthands(t *testing.T) { + t.Parallel() + frontmatter := map[string]any{"on": "daily"} + mustApplyEditChange(t, frontmatter, editChange{kind: "set", path: "on.schedule", value: "every 6h"}) + assert.Equal(t, map[string]any{"schedule": "every 6h", "workflow_dispatch": nil}, frontmatter["on"]) + + frontmatter = map[string]any{"on": []any{"push", "workflow_dispatch"}} + mustApplyEditChange(t, frontmatter, editChange{kind: "set", path: "on.schedule", value: "every 6h"}) + assert.Equal(t, map[string]any{"push": nil, "workflow_dispatch": nil, "schedule": "every 6h"}, frontmatter["on"]) +} + +func TestEditChangeRejectsUnexpandableTriggerShorthand(t *testing.T) { + t.Parallel() + frontmatter := map[string]any{"on": "/bot"} + _, err := applyEditChange(frontmatter, editChange{kind: "set", path: "on.schedule", value: "every 6h"}) + require.ErrorContains(t, err, "object form") + assert.Equal(t, "/bot", frontmatter["on"]) +} + +func TestEditChangesAreNoOpsWhenValuesAlreadyMatch(t *testing.T) { + t.Parallel() + frontmatter := map[string]any{"max-turns": 20, "imports": []any{"shared/common.md"}} + applied, err := applyEditChange(frontmatter, editChange{kind: "set", path: "max-turns", value: 20}) + require.NoError(t, err) + assert.False(t, applied) + + applied, err = applyEditChange(frontmatter, editChange{kind: "add", path: "imports", value: "shared/common.md"}) + require.NoError(t, err) + assert.False(t, applied) + + applied, err = applyEditChange(frontmatter, editChange{kind: "remove", path: "imports", value: "shared/other.md"}) + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, []any{"shared/common.md"}, frontmatter["imports"]) +} + +func TestEditCommandLeavesWorkflowUnchangedForNoOpEdits(t *testing.T) { + t.Parallel() + dir := t.TempDir() + workflowPath := dir + "/workflow.md" + original := "---\n# keep this comment\non: push\n---\n# Workflow\n" + require.NoError(t, os.WriteFile(workflowPath, []byte(original), 0o644)) + + cmd := NewEditCommand() + cmd.SetArgs([]string{workflowPath, "--schedule", "off"}) + var output strings.Builder + cmd.SetOut(&output) + require.NoError(t, cmd.Execute()) + + assert.Contains(t, output.String(), "already matches") + content, err := os.ReadFile(workflowPath) + require.NoError(t, err) + assert.Equal(t, original, string(content)) +} + +func TestWriteFileAtomicallyReplacesContent(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := dir + "/workflow.md" + require.NoError(t, os.WriteFile(path, []byte("old"), 0o644)) + require.NoError(t, writeFileAtomically(path, []byte("new"))) + + content, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, "new", string(content)) + + entries, err := os.ReadDir(dir) + require.NoError(t, err) + assert.Len(t, entries, 1) +} From 6260e69c90a69cdc41f68b4355cf1559258232ad Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 21:04:45 +0000 Subject: [PATCH 9/9] Preserve file permissions on atomic edit writes Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/edit_command.go | 6 +++++- pkg/cli/edit_command_test.go | 12 ++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/pkg/cli/edit_command.go b/pkg/cli/edit_command.go index af97c88213f..0c40024b85e 100644 --- a/pkg/cli/edit_command.go +++ b/pkg/cli/edit_command.go @@ -152,7 +152,11 @@ func writeFileAtomically(path string, content []byte) error { if err := file.Close(); err != nil { return err } - if err := os.Chmod(tempPath, 0o644); err != nil { + mode := os.FileMode(0o644) + if info, err := os.Stat(path); err == nil { + mode = info.Mode().Perm() + } + if err := os.Chmod(tempPath, mode); err != nil { return err } return os.Rename(tempPath, path) diff --git a/pkg/cli/edit_command_test.go b/pkg/cli/edit_command_test.go index 05b7ae54def..a227dc80513 100644 --- a/pkg/cli/edit_command_test.go +++ b/pkg/cli/edit_command_test.go @@ -202,3 +202,15 @@ func TestWriteFileAtomicallyReplacesContent(t *testing.T) { require.NoError(t, err) assert.Len(t, entries, 1) } + +func TestWriteFileAtomicallyPreservesPermissions(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := dir + "/workflow.md" + require.NoError(t, os.WriteFile(path, []byte("old"), 0o600)) + require.NoError(t, writeFileAtomically(path, []byte("new"))) + + info, err := os.Stat(path) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o600), info.Mode().Perm()) +}