fix(cli): bring top-level help lines in line with their parsers and pin the drift

[arcaven]

Several top-level sideshow --help lines omitted flags their verbs accept, so a user could not find a flag from help, including one that an error hint tells them to pass. This brings those lines in line with their parsers and adds tests that fail when a help line or a parser drifts from the verb's usage string, so the drift stops coming back.

Closes #195. Help text and tests only; the parsers' behavior is unchanged. Two usage strings change in wording (listed below).

What changed:

• The help lines named in top-level usage lines omit flags their verbs accept (enable, adopt conversion, adopt migrate) #195: enable gains --override-stale-lock; the adopt conversion line gains <pack>[@<ver>], --scope and --allow-version-change; the migrate line gains --commit-consent and --override-stale-lock. Review of this PR added two more: coexist gains --sideshow-active, and project init gains --user-name and --dry-run. The long entries wrap, as the doctor entry does.
• Each verb's usage string is a named constant (enableUsage, disableUsage, activateUsage, deactivateUsage, coexistCheckUsage, coexistUsage, projectInitUsage; adoptUsage already was). Enable and disable shared one string, so disable advertised --scope and @<version>, which its help line and per-verb help do not; activate and deactivate shared one that omitted --agent. Each verb now has its own, so two error messages change: disable's drops [@<version>] and [--scope ...], and activate's gains [--agent <name>].
• The parser functions the runners call are named (parseEnableArgs, parseDisableArgs, parseActivate, parseDeactivate), and --agent is allowed by the verb inside parseActivateArgs, not by an argument a caller passes. The test reaches each parser through the same code its runner runs.

The pin (cmd/sideshow/usage_drift_test.go), read from the usage strings, with adoptUsage split by mode at its --migrate-user-scope group:

1. Every flag a usage string names appears in the matching --help entry. Covers enable, disable, activate, deactivate, coexist-check, coexist, project init, and the adopt conversion, migrate and finish entries. The conversion entry must also show [@<ver>].
2. The parser accepts every flag its usage names, with a dummy value where the flag takes one; 25 runs in scratch HOME, config and working directories. For the four parse-only verbs (enable, disable, activate, deactivate), any error fails the run. For the commands that run in full, an error that names the flag fails it, since later steps fail for unrelated reasons in scratch space. A migrate flag runs beside --migrate-user-scope, because --yes alone is refused on purpose.

Known limits. The first two are named in the test comment; the third is only here:

• A flag the parser accepts but no usage string names stays unpinned by both checks. Only a flag registry that parsers, usage strings and help lines all read from would close that; a registry is not part of this change.
• The project init parser ignores flags it does not know (this predates this PR), so check 2 cannot see a flag dropped from it. Check 1 still holds its help entry.
• A runner that called the wrong parser function would not be seen; the test calls each verb's parser function directly.

Red/green: the first pin was red on the three drifted entries (6e2c4fa) and green after the help edit (1a4b2d0). Review found that the first parser check let two mutants of the activate path through, because it called the activate parser with its own allowAgent and treated any error other than "unknown flag" as acceptance. The follow-up (6788895 refactor, 036acd5 test, 46fa325 fix) pins the help lines for coexist and project init red-first; the stricter parser check passes on arrival and is held by the mutants below.

Mutants, each compiled and killed by a named test: enable help line drops --override-stale-lock; migrate line drops --commit-consent and --yes; conversion line drops @<ver>; coexist line drops --sideshow-active; project init entry drops --dry-run; disable usage names a flag its help line lacks; the adopt, enable, activate and coexist-check parsers each stop accepting one named flag; the coexist parser drops --sideshow-active; activate treats --agent as refused because allowAgent is false; the --agent case refuses unconditionally; the enable parser returns an error that does not say "unknown flag"; adopt refuses --yes with an error that names it. Two survive by design: activateUsage dropping --agent, and the project init parser dropping --user-name; both are the limits above.

Gates, each run on its own: build, vet, go test -count=1 ./..., golangci-lint (0 issues), gofumpt -l (empty).

Opportunities, not touched: install, use and project unregister carry inline usage strings outside the approved verb list.

Closes #195

[arcavenai]

Requesting changes on one point. The parser check misses a real regression, so sideshow activate --agent can break with every test green. The help-line half of the change is right, and I would approve it as it stands; details below.

The blocker: check 2 is wider open than its known limit says. Two one-line mutants of cmd/sideshow/activate.go pass go test -count=1 ./cmd/sideshow:

• runActivate calls parseActivateArgs("activate", args, false) instead of true (line 17).
• The --agent case refuses unconditionally (if true { for if !allowAgent {).

Either way, sideshow activate demo --agent x fails with --agent applies to activate only, although the usage string and the help line both name --agent.

TestUsageDrift_EveryParserAcceptsEveryFlagItsUsageNames misses this for two reasons:

• It calls parseActivateArgs with its own allowAgent, not with the argument the verb passes, so the runner's call is never exercised.
• It counts any error that lacks the text "unknown flag" as acceptance, so a parser can reject a named flag in other words and pass.

The comment and body name only the other direction: a flag the parser accepts but no usage names. This direction, a named flag rejected with a different message, is not admitted.

A fix that stays inside this PR:

• For the four parse-only verbs, require err == nil. I checked that all 8 of their flag runs return nil today with the test's dummy values, so this is safe now.
• Reach parseActivateArgs through the same argument runActivate and runDeactivate pass. A shared table or a func value would do.
• For the full runAdopt and runCoexistCheck runs, where other errors are expected, also fail when the error names the flag.

Criterion check, for the record.

• Gaps from #194 are closed. The conversion entry now shows <pack>[@<ver>], --scope and --allow-version-change. The migrate entry shows --commit-consent and --override-stale-lock. The top-level enable line shows --override-stale-lock.

• Two top-level lines still omit flags their own usage strings name. The coexist line omits --sideshow-active (coexist.go:23). The project init line omits --user-name and --dry-run (main.go:787). The body lists these as outside the approved verbs. They are two one-line help edits if you want them here. (install's flags appear in its Install options block.)

• 905a4f4 is behavior-neutral apart from two messages. I diffed every --help, <verb> --help, bare <verb>, <verb> --bogus and <verb> demo --bogus output for 16 verbs, main against 905a4f4. Only these two pairs differ, each printed by a bare call and by --bogus:

  • disable: usage: sideshow disable <pack>[@<version>] [--repo <path>] [--scope local|project] [--override-stale-lock] becomes usage: sideshow disable <pack> [--repo <path>] [--override-stale-lock]
  • activate: usage: sideshow activate <pack> [--repo <path>] becomes usage: sideshow activate <pack> [--repo <path>] [--agent <name>]

  The parser code changes only in the branch that prints usage.

• The four split verbs, run through their parsers.

  • enable: --repo, --scope and --override-stale-lock each parse with no error.
  • disable: --repo and --override-stale-lock each parse with no error.
  • activate: --repo and --agent each parse with no error.
  • deactivate: --repo parses with no error, and --agent is refused with "--agent applies to activate only".

• Red and stability. At 6e2c4fa the pin fails on exactly the drifted entries:

  • enable's --override-stale-lock;
  • the conversion entry's --scope, --allow-version-change and [@<ver>];
  • the migrate entry's --override-stale-lock and --commit-consent.

  With head's tests and 905a4f4's main.go, 2 tests fail, so the regex change in the green commit does not mask the drift. Head passes go test -count=1 ./....

• Mutants that are killed. The 9 listed are killed. So are four I added:

  • a fake [--fake] in enableUsage, which fails both checks;
  • [--repo <path>] dropped from the deactivate help line;
  • [--repo <path>] dropped from the adopt conversion entry;
  • --scope added to disableUsage.

• The by-design survivor (activateUsage drops --agent) is what the stated limit describes. The limit is broader than stated only in the direction above.

• Other verbs. Help for verbs the PR does not touch is unchanged apart from the three edited entries, which every verb's fallback help shows. CI on 1a4b2d0 passes.

On the split you asked about. Giving disable and deactivate their own usage strings is right. parseVerbArgs still accepts --scope and @<version> for disable, but Disable ignores both: the scope comes from the ledger row (settings := settingsFile(opts.RepoDir, bindings.RepoScope(row.SettingsScope))), and opts.Scope is only validated. So the old usage advertised two flags that do nothing. The new error wording is more accurate, and I found nothing that parses it. Rejecting those no-op flags in the parser would be a behavior change, and the known limit already covers it.

Seat: reviewer / reviewer

[arcavenai]

Approving at 46fa325. My round-1 blocker is closed. The parser check now reaches each verb through the function its runner calls, and it fails on any error from a parse-only verb, so the activate --agent regressions I posted fail a test. The two help lines left over from round 1 now name their flags.

Round-1 mutants at head.

• allowAgent forced false: killed by TestUsageDrift_EveryParserAcceptsEveryFlagItsUsageNames.
• The --agent case refusing unconditionally (if true || !allowAgent {): killed by the same test.
• parseActivate passing "deactivate": killed by the same test.

Call sites. Each runner calls a named parser, and the test calls that same function:

• runEnable calls parseEnableArgs and runDisable calls parseDisableArgs (enable.go:17,25).
• runActivate calls parseActivate and runDeactivate calls parseDeactivate (activate.go:17,25).
• parseActivateArgs decides --agent from the verb, and nothing else calls it.
• coexist, coexist-check, project init and adopt run their whole runner.

6788895 is a pure refactor. I diffed 71 outputs between 1a4b2d0 and 6788895: --help, a bare sideshow, and for 16 verbs <verb> --help, a bare <verb>, <verb> --bogus and <verb> demo --bogus, plus activate/deactivate demo --agent x and three project init forms. They are byte-identical. Between 6788895 and 46fa325, the only change is the two edited entries, the coexist line and the project init line, in the 16 outputs that print the top-level help.

The new flags, run.

• coexist ck --sideshow-active exits 0, while coexist ck --bogus exits 1 with unknown flag: --bogus.
• project init ck --dry-run, --user-name Ada --dry-run and --user-name=Ada --dry-run each print the dry-run plan and leave the repo holding only .git.

The stated survivors, re-derived.

• activateUsage drops --agent: check 2 reads its flag list from that string, so the dropped flag is never tried. This is the accepted-but-unnamed limit, as stated.
• The project init parser: its switch has no default case, so an unknown flag is ignored. That is true at 46fa325, on main, and at 6dc5d2c (#152), so this PR did not introduce it. Check 2 cannot see a flag dropped from it. Other tests in the package do catch two of the drops: dropping the space form --user-name fails 6 TestProjectInit_* tests, and dropping --dry-run fails 2. Dropping only the --user-name= form passes everything, and the usage string does not name that form. So in practice the survivor is narrower than the comment, not broader.

The named residual. A runner calling the wrong parser survives as stated: runActivate calling parseDeactivate, or runDisable calling parseEnableArgs. It is in the body. The body says both limits are named in the test comment, but the comment names only the project init gap.

Red and mutants.

• Red: at 036acd5 the help pin fails on exactly coexist --sideshow-active, project init --user-name and project init --dry-run.

• Head: go test -count=1 ./... passes.

• Listed mutants: each is killed by its named test. That includes the enable parser returning an error that does not say "unknown flag", and adopt refusing --yes with an error that names it.

• My extra mutants:

  • killed: the coexist parser dropping --sideshow-active, [--sideshow-active] dropped from the coexist help line, [--dry-run] dropped from the project init entry;
  • survives: the coexist parser refusing --sideshow-active with an error that does not name the flag.

  That survivor follows from the rule the test comment states: for a full-run verb, only an error naming the flag counts. It is the rule I suggested in round 1, and every full-run parser here names the flag in its rejections today. Non-blocking.

CI on 46fa325 passes.

Seat: reviewer / reviewer