refactor: declare each command's flags exactly once

CLAUDE.md

@@ -69,13 +69,12 @@ e2e/                 black-box tests driving the real binary as a subprocess
 3. Add `cmd/frob.go` — a thin adapter that parses flags and calls
    `mutate("frob", asJSON, func(env stack.Env, s *stack.State) (*stack.OpResult, error) { return stack.Frobnicate(env, s, arg) })`,
    self-registering via `init()` → `register(&Command{...})`. Add a `--json` flag.
-   If `frob` parses any flag beyond `--json`, also add a `frobFlagSet()` constructor
-   to `cmd/flagsets.go` declaring those flags, set `NewFlagSet: frobFlagSet` in its
-   `register(&Command{...})`, and list `frob`'s flags in `TestCommandFlagsMatchExpected`
-   (`cmd/execute_test.go`). `Run` parses the flags either way; this wiring keeps
-   `st help --json` (and the declared-flags contract in `docs/AGENT.md`) reporting
-   exactly what `Run` accepts — skip it and `make ci` fails with "update flagsets.go
-   or this table".
+   If `frob` parses any flag beyond `--json`, declare those flags once in
+   `cmd/flagsets.go`: a `frobOpts` struct and `newFrobFlags(o *frobOpts)` (the single
+   declaration site), plus a no-arg `frobFlagSet()` wrapper. `runFrob` calls
+   `fs := newFrobFlags(&o)` and reads `o.<field>`; `register` sets
+   `NewFlagSet: frobFlagSet`. `Run` and `st help --json` then build the flags from
+   the same constructor, so the declared-flags contract in `docs/AGENT.md` can't drift.
 4. If it has interesting CLI output, add a golden test (`cmd/golden_test.go`,
    regenerate with `go test ./cmd -run Golden -update`).
 5. `make ci`. Adding the command shifts the help golden — regenerate it deliberately.

CONTRIBUTING.md

@@ -42,10 +42,10 @@ interface (JSON, exit codes).
 3. **Adapter** — `cmd/frob.go`, a thin wrapper that self-registers and calls
    `mutate("frob", asJSON, func(env stack.Env, s *stack.State) (*stack.OpResult, error) { return stack.Frobnicate(env, s, arg) })`.
    Add a `--json` flag (see `docs/AGENT.md` — every command speaks JSON). For any
-   flag beyond `--json`, also add a `frobFlagSet()` to `cmd/flagsets.go`, set
-   `NewFlagSet: frobFlagSet` in `register()`, and list `frob`'s flags in
-   `TestCommandFlagsMatchExpected` — this keeps `help --json` reporting exactly
-   what `Run` accepts (or `make ci` fails with "update flagsets.go or this table").
+   flag beyond `--json`, declare it once in `cmd/flagsets.go` (a `frobOpts` struct +
+   `newFrobFlags(o)` constructor + a no-arg `frobFlagSet()` wrapper); `runFrob` reads
+   `o.<field>` and `register` sets `NewFlagSet: frobFlagSet`, so `help --json` reports
+   exactly what `Run` parses, from the same declaration.
 4. **Golden output** (optional) — `cmd/golden_test.go`; regenerate with
    `go test ./cmd -run Golden -update`.
 5. `make ci`. Adding a command shifts the `--help` golden — regenerate it

cmd/create.go

@@ -2,7 +2,6 @@ package cmd
 
 import (
 	"errors"
-	"fmt"
 
 	"stacked/internal/stack"
 )
@@ -19,21 +18,12 @@ func init() {
 }
 
 func runCreate(args []string) error {
-	var asJSON bool
-	fs := newFlagSet("create", &asJSON)
-	fs.Usage = func() {
-		fmt.Fprintln(fs.Output(), usageLine("create"))
-		fs.PrintDefaults()
-	}
-	var message string
-	fs.StringVar(&message, "m", "", "commit message for the new branch")
-	fs.StringVar(&message, "message", "", "commit message for the new branch")
-	var all bool
-	fs.BoolVar(&all, "a", false, "stage all changes before committing")
-	fs.BoolVar(&all, "all", false, "stage all changes before committing")
+	var o createOpts
+	fs := newCreateFlags(&o)
 	if err := parseArgs(fs, args); err != nil {
 		return err
 	}
+	asJSON, message, all := o.asJSON, o.message, o.all
 	rest := fs.Args()
 	if len(rest) != 1 {
 		usageUnlessJSON(fs, args)

cmd/delete.go

@@ -2,7 +2,6 @@ package cmd
 
 import (
 	"errors"
-	"fmt"
 
 	"stacked/internal/stack"
 )
@@ -19,18 +18,12 @@ func init() {
 }
 
 func runDelete(args []string) error {
-	var asJSON bool
-	fs := newFlagSet("delete", &asJSON)
-	fs.Usage = func() {
-		fmt.Fprintln(fs.Output(), usageLine("delete"))
-		fs.PrintDefaults()
-	}
-	var force bool
-	fs.BoolVar(&force, "f", false, "force delete the branch even if not fully merged")
-	fs.BoolVar(&force, "force", false, "force delete the branch even if not fully merged")
+	var o deleteOpts
+	fs := newDeleteFlags(&o)
 	if err := parseArgs(fs, args); err != nil {
 		return err
 	}
+	asJSON, force := o.asJSON, o.force
 	rest := fs.Args()
 	if len(rest) != 1 {
 		usageUnlessJSON(fs, args)

cmd/execute_test.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"slices"
 	"strings"
 	"testing"
 
@@ -314,41 +313,6 @@ func TestHelpReportsOnlyRealFlags(t *testing.T) {
 	}
 }
 
-// TestCommandFlagsMatchExpected pins each command's reported flag set to a
-// maintained table, so a change to flagsets.go (or a new command) that drifts the
-// help-reported flags fails loudly. TestHelpReportsOnlyRealFlags covers the
-// over-report direction (a listed flag the command rejects); this pins the set
-// itself. Every command also reports --json; completion has none.
-func TestCommandFlagsMatchExpected(t *testing.T) {
-	expected := map[string][]string{
-		"create":  {"a", "all", "json", "m", "message"},
-		"modify":  {"a", "all", "commit", "json", "m", "message"},
-		"delete":  {"f", "force", "json"},
-		"init":    {"json", "trunk"},
-		"track":   {"json", "parent"},
-		"submit":  {"dry-run", "json", "remote"},
-		"sync":    {"dry-run", "json", "no-delete", "remote"},
-		"restack": {"dry-run", "json"},
-		"squash":  {"json", "m", "message"},
-	}
-	for _, c := range registry {
-		got := make([]string, 0)
-		for _, f := range commandFlags(c) { // VisitAll order is lexicographical
-			got = append(got, f.Name)
-		}
-		want, ok := expected[c.Name]
-		if !ok {
-			want = []string{"json"} // every command without its own flag set has exactly --json
-			if c.Name == "completion" {
-				want = []string{} // completion declares no flags
-			}
-		}
-		if !slices.Equal(got, want) {
-			t.Errorf("commandFlags(%q) = %v, want %v (update flagsets.go or this table)", c.Name, got, want)
-		}
-	}
-}
-
 // TestHelpJSONIncludesFlags pins the structured flag list: help <cmd> --json
 // reports each declared flag with its type.
 func TestHelpJSONIncludesFlags(t *testing.T) {

cmd/flagsets.go

@@ -1,86 +1,162 @@
 package cmd
 
-import "flag"
-
-// Introspection flag sets for `help --json`. Each declares exactly the flags its
-// command's Run parses (Run still builds and binds its own set; these mirror it
-// for introspection only). They are gathered here so the full set is reviewable
-// at a glance, and TestHelpReportsOnlyRealFlags asserts every flag listed here is
-// actually accepted by the command — so help can never advertise a flag the
-// command rejects. Commands whose only flag is --json need no entry (help derives
-// that from newFlagSet); completion has none.
-
-func createFlagSet() *flag.FlagSet {
-	var asJSON bool
-	fs := newFlagSet("create", &asJSON)
-	fs.String("m", "", "commit message for the new branch")
-	fs.String("message", "", "commit message for the new branch")
-	fs.Bool("a", false, "stage all changes before committing")
-	fs.Bool("all", false, "stage all changes before committing")
+import (
+	"flag"
+	"fmt"
+)
+
+// Each command with flags beyond --json declares them ONCE here: newXxxFlags
+// binds a *xxxOpts that its Run reads, and the no-arg xxxFlagSet() wrapper feeds
+// the same declaration to help introspection (Command.NewFlagSet). So `help
+// --json` reports exactly what Run parses, by construction — no second
+// declaration to drift. Aliases (-m/--message, -a/--all, -f/--force) bind to one
+// field so command-line last-wins is preserved. (completion has no flags;
+// commands whose only flag is --json need no entry — help derives that from
+// newFlagSet.)
+
+// withDefaults gives a command's flag set a usage line plus its flag defaults,
+// the help body the five flag-rich commands print on -h.
+func withDefaults(fs *flag.FlagSet, name string) *flag.FlagSet {
+	fs.Usage = func() {
+		fmt.Fprintln(fs.Output(), usageLine(name))
+		fs.PrintDefaults()
+	}
 	return fs
 }
 
-func modifyFlagSet() *flag.FlagSet {
-	var asJSON bool
-	fs := newFlagSet("modify", &asJSON)
-	fs.String("m", "", "commit message")
-	fs.String("message", "", "commit message")
-	fs.Bool("a", true, "stage all tracked changes before amending/committing")
-	fs.Bool("all", true, "stage all tracked changes before amending/committing")
-	fs.Bool("commit", false, "create a new commit instead of amending the tip")
-	return fs
+type createOpts struct {
+	asJSON  bool
+	message string
+	all     bool
 }
 
-func deleteFlagSet() *flag.FlagSet {
-	var asJSON bool
-	fs := newFlagSet("delete", &asJSON)
-	fs.Bool("f", false, "force delete the branch even if not fully merged")
-	fs.Bool("force", false, "force delete the branch even if not fully merged")
-	return fs
+func newCreateFlags(o *createOpts) *flag.FlagSet {
+	fs := newFlagSet("create", &o.asJSON)
+	fs.StringVar(&o.message, "m", "", "commit message for the new branch")
+	fs.StringVar(&o.message, "message", "", "commit message for the new branch")
+	fs.BoolVar(&o.all, "a", false, "stage all changes before committing")
+	fs.BoolVar(&o.all, "all", false, "stage all changes before committing")
+	return withDefaults(fs, "create")
 }
 
-func initFlagSet() *flag.FlagSet {
-	var asJSON bool
-	fs := newFlagSet("init", &asJSON)
-	fs.String("trunk", "", "name of the trunk branch (default: detected)")
-	return fs
+func createFlagSet() *flag.FlagSet { return newCreateFlags(&createOpts{}) }
+
+type modifyOpts struct {
+	asJSON  bool
+	message string
+	all     bool
+	commit  bool
 }
 
-func trackFlagSet() *flag.FlagSet {
-	var asJSON bool
-	fs := newFlagSet("track", &asJSON)
-	fs.String("parent", "", "parent branch (trunk or a tracked branch)")
-	return fs
+func newModifyFlags(o *modifyOpts) *flag.FlagSet {
+	fs := newFlagSet("modify", &o.asJSON)
+	fs.StringVar(&o.message, "m", "", "commit message")
+	fs.StringVar(&o.message, "message", "", "commit message")
+	// Default to staging all changes so a bare invocation (and the "amend" alias)
+	// behaves like "stage all + amend"; only -a=false disables it.
+	fs.BoolVar(&o.all, "a", true, "stage all tracked changes before amending/committing")
+	fs.BoolVar(&o.all, "all", true, "stage all tracked changes before amending/committing")
+	fs.BoolVar(&o.commit, "commit", false, "create a new commit instead of amending the tip")
+	return withDefaults(fs, "modify")
+}
+
+func modifyFlagSet() *flag.FlagSet { return newModifyFlags(&modifyOpts{}) }
+
+type deleteOpts struct {
+	asJSON bool
+	force  bool
+}
+
+func newDeleteFlags(o *deleteOpts) *flag.FlagSet {
+	fs := newFlagSet("delete", &o.asJSON)
+	fs.BoolVar(&o.force, "f", false, "force delete the branch even if not fully merged")
+	fs.BoolVar(&o.force, "force", false, "force delete the branch even if not fully merged")
+	return withDefaults(fs, "delete")
 }
 
-func submitFlagSet() *flag.FlagSet {
-	var asJSON bool
-	fs := newFlagSet("submit", &asJSON)
-	fs.String("remote", "origin", "remote to push to")
-	fs.Bool("dry-run", false, "print what would be pushed without pushing")
+func deleteFlagSet() *flag.FlagSet { return newDeleteFlags(&deleteOpts{}) }
+
+type initOpts struct {
+	asJSON bool
+	trunk  string
+}
+
+func newInitFlags(o *initOpts) *flag.FlagSet {
+	fs := newFlagSet("init", &o.asJSON)
+	fs.StringVar(&o.trunk, "trunk", "", "name of the trunk branch (default: detected)")
+	return withDefaults(fs, "init")
+}
+
+func initFlagSet() *flag.FlagSet { return newInitFlags(&initOpts{}) }
+
+type trackOpts struct {
+	asJSON bool
+	parent string
+}
+
+func newTrackFlags(o *trackOpts) *flag.FlagSet {
+	fs := newFlagSet("track", &o.asJSON)
+	fs.StringVar(&o.parent, "parent", "", "parent branch (trunk or a tracked branch)")
+	return withDefaults(fs, "track")
+}
+
+func trackFlagSet() *flag.FlagSet { return newTrackFlags(&trackOpts{}) }
+
+type submitOpts struct {
+	asJSON bool
+	remote string
+	dryRun bool
+}
+
+func newSubmitFlags(o *submitOpts) *flag.FlagSet {
+	fs := newFlagSet("submit", &o.asJSON)
+	fs.StringVar(&o.remote, "remote", "origin", "remote to push to")
+	fs.BoolVar(&o.dryRun, "dry-run", false, "print what would be pushed without pushing")
 	return fs
 }
 
-func syncFlagSet() *flag.FlagSet {
-	var asJSON bool
-	fs := newFlagSet("sync", &asJSON)
-	fs.Bool("no-delete", false, "do not delete merged branches")
-	fs.String("remote", "origin", "remote to fetch and fast-forward from")
-	fs.Bool("dry-run", false, "show what would be pruned/restacked without changing anything")
+func submitFlagSet() *flag.FlagSet { return newSubmitFlags(&submitOpts{}) }
+
+type syncOpts struct {
+	asJSON   bool
+	noDelete bool
+	remote   string
+	dryRun   bool
+}
+
+func newSyncFlags(o *syncOpts) *flag.FlagSet {
+	fs := newFlagSet("sync", &o.asJSON)
+	fs.BoolVar(&o.noDelete, "no-delete", false, "do not delete merged branches")
+	fs.StringVar(&o.remote, "remote", "origin", "remote to fetch and fast-forward from")
+	fs.BoolVar(&o.dryRun, "dry-run", false, "show what would be pruned/restacked without changing anything")
 	return fs
 }
 
-func restackFlagSet() *flag.FlagSet {
-	var asJSON bool
-	fs := newFlagSet("restack", &asJSON)
-	fs.Bool("dry-run", false, "show what would be restacked without changing anything")
+func syncFlagSet() *flag.FlagSet { return newSyncFlags(&syncOpts{}) }
+
+type restackOpts struct {
+	asJSON bool
+	dryRun bool
+}
+
+func newRestackFlags(o *restackOpts) *flag.FlagSet {
+	fs := newFlagSet("restack", &o.asJSON)
+	fs.BoolVar(&o.dryRun, "dry-run", false, "show what would be restacked without changing anything")
 	return fs
 }
 
-func squashFlagSet() *flag.FlagSet {
-	var asJSON bool
-	fs := newFlagSet("squash", &asJSON)
-	fs.String("m", "", "commit message for the squashed commit")
-	fs.String("message", "", "commit message for the squashed commit")
+func restackFlagSet() *flag.FlagSet { return newRestackFlags(&restackOpts{}) }
+
+type squashOpts struct {
+	asJSON  bool
+	message string
+}
+
+func newSquashFlags(o *squashOpts) *flag.FlagSet {
+	fs := newFlagSet("squash", &o.asJSON)
+	fs.StringVar(&o.message, "m", "", "commit message for the squashed commit")
+	fs.StringVar(&o.message, "message", "", "commit message for the squashed commit")
 	return fs
 }
+
+func squashFlagSet() *flag.FlagSet { return newSquashFlags(&squashOpts{}) }

cmd/init.go

@@ -23,20 +23,15 @@ func init() {
 // the trunk branch from the --trunk flag, falling back to the remote default
 // branch, the current branch, and finally "main".
 func runInit(args []string) error {
-	var asJSON bool
-	fs := newFlagSet("init", &asJSON)
-	var trunk string
-	fs.StringVar(&trunk, "trunk", "", "name of the trunk branch (default: detected)")
-	fs.Usage = func() {
-		fmt.Fprintln(fs.Output(), usageLine("init"))
-		fs.PrintDefaults()
-	}
+	var o initOpts
+	fs := newInitFlags(&o)
 	if err := parseFlagSet(fs, args); err != nil {
 		return err
 	}
 	if err := rejectArgs("init", fs.Args()); err != nil {
 		return err
 	}
+	asJSON, trunk := o.asJSON, o.trunk
 
 	// Ensure we are inside a git repository.
 	if _, err := git.RepoRoot(); err != nil {