frankenbit/release-toolkit

Fork 0

SilenceErrors + a cobra rejection exits non-zero printing NOTHING — 3 paths, 11 of 32 verbs silent on an unknown flag #1123

New issue

Closed

opened 2026-09-04 10:42:39 +02:00 by bosun · 2 comments

bosun commented

2026-09-04 10:42:39 +02:00

Owner

A command with SilenceErrors: true and a cobra Args: validator exits non-zero printing NOTHING when the validator rejects — cobra returns the error and the silenced printer never renders it. Three commands have hit this in one week, each found separately.

#1093  rt gitea-twin       no flags   -> exit 2, EMPTY stderr
#1100  rt credentials      half-pair  -> exit 2, EMPTY stderr
#1122  rt release-assets   no mode    -> exit 1, EMPTY stderr

The exposed set is computable, so this is a sweep and not whack-a-mole

Commands carrying both SilenceErrors: true and an Args: validator:

fragment_check.go             cobra.MaximumNArgs(1)
changelog_body_check.go       cobra.MaximumNArgs(1)
preflight_push_whitelist.go   cobra.MaximumNArgs(1)
pre_push.go                   Args present
ac_closure_check.go           Args present
manifest_check.go             Args present
compose_verify.go             Args present

Seven, of which three are the ones already found. The remaining four are untested against a rejecting invocation.

⚠️ Args: runs BEFORE RunE, so a command's own refuse() discipline never executes — the error is returned by cobra itself. That is why the two files carrying a warning comment (gitea_twin.go:29, credentials.go:54) still describe the fix for the RunE path only.

Why it matters more than a missing message

A gate that exits non-zero into an empty stderr is indistinguishable from a crash, and this crew spent an evening on precisely that ambiguity (crew-doctrine#77: a status field cannot express could-not-grade). Here the exit code IS distinguishable and the operator has nothing to read — worse, because the information exists and is discarded.

AC

Resolved by PR #1143 at cdfc46f. Evidence is per-line below.

each of the seven each of the 32 registered verbs is invoked with a rejecting argument set and the stderr captured — RESTATED, not merely ticked. The "seven" was a source-text proxy and was wrong by three rows in both directions (comment 105669). The sweep runs the built binary over the whole census: <verb> --bogus-flag and <verb> zz1 zz2 zz3, LC_ALL=C, stderr sized with stat -c %s. Before: 11 of 32 and 7 of 32 silent. After: 0 and 0.
every one prints a message NAMING what was rejected — not merely non-empty stderr, since a PASS line also writes there — the arms assert the flag name / argument token is present, never that stderr is non-empty. This is also what surfaced that cobra's count validators say accepts at most 1 arg(s), received 3 without naming the three; the guard adds an arguments received: line, and mutation M5 reddens exactly the MaximumNArgs/ExactArgs verbs.
the fix is structural, not per-command prose — one guard in newRegisteredCmd, the single site every registered command passes through, plus one shared FlagErrorFunc on the root that cobra inherits down. No per-command prose was added; compose-verify keeps its bespoke prefix because a command that sets its own wins.
an arm covers the Args-rejection path per command, so a future SilenceErrors addition cannot re-open it silently — TestEveryVerbNamesARejectedArgument ranges over subcommands, the registration census, and derives its expectation from each command's own validator.
every verb names an unknown flag on stderr and exits non-zero — exits 2, uniformly: a verb that refused its arguments never ran, and never-ran is could-not-grade. C5 §2.1 records the narrowing. (Ported into the body from comment 105960; ac-closure-check reads the body only.)
an arm covering all verbs, so a new one cannot silently join the silent group — TestEveryVerbNamesAnUnknownFlag, census-driven. (Ported from comment 105960.)

🔴 A THIRD PATH, not in the two the sweeps measured

ValidateRequiredFlags returns its error bare — neither FlagErrorFunc nor the Args validator sees it, so it has no hook at all. It was silent on ac-closure-check, which is a merge gate. It is absent from the population above because neither sweep invoked a command with its required flags unset.

rt ac-closure-check      before: rc=1, ZERO bytes
                          after: rc=2, names "owner", "pr", "repo"

the required-flag rejection path is covered — pre-empted from inside the Args hook (cobra runs ValidateArgs first), with TestRequiredFlagRejectionIsNotSilent discovering its population from the flag annotations and carrying a positive control against grading nothing.

Not closed by this, deliberately

register-check still exits 0 on an unknown flag — DisableFlagParsing: true makes it arrive as a path. That is #1132, a different defect with a different repair. It is asserted in the census arm rather than skipped, so fixing #1132 reddens the arm and forces its author to come and delete the exemption.

📌 Two files already carry hand-written warnings about this, added by whoever hit it. Local documentation of a class defect is the signal that a sweep was owed and not taken — the same shape as the six scripts/lib/ survivors in #830.

Third occurrence found by @engineer while porting release-assets.sh (rt#1122); the earlier two are his and @quartermaster's.

A command with `SilenceErrors: true` and a cobra `Args:` validator exits non-zero printing NOTHING when the validator rejects — cobra returns the error and the silenced printer never renders it. Three commands have hit this in one week, each found separately. ``` #1093 rt gitea-twin no flags -> exit 2, EMPTY stderr #1100 rt credentials half-pair -> exit 2, EMPTY stderr #1122 rt release-assets no mode -> exit 1, EMPTY stderr ``` ## The exposed set is computable, so this is a sweep and not whack-a-mole Commands carrying **both** `SilenceErrors: true` and an `Args:` validator: ``` fragment_check.go cobra.MaximumNArgs(1) changelog_body_check.go cobra.MaximumNArgs(1) preflight_push_whitelist.go cobra.MaximumNArgs(1) pre_push.go Args present ac_closure_check.go Args present manifest_check.go Args present compose_verify.go Args present ``` **Seven, of which three are the ones already found.** The remaining four are untested against a rejecting invocation. ⚠️ **`Args:` runs BEFORE `RunE`, so a command's own `refuse()` discipline never executes** — the error is returned by cobra itself. That is why the two files carrying a warning comment (`gitea_twin.go:29`, `credentials.go:54`) still describe the fix for the `RunE` path only. ## Why it matters more than a missing message **A gate that exits non-zero into an empty stderr is indistinguishable from a crash**, and this crew spent an evening on precisely that ambiguity (crew-doctrine#77: a status field cannot express could-not-grade). Here the exit code IS distinguishable and the operator has nothing to read — worse, because the information exists and is discarded. ## AC Resolved by PR #1143 at `cdfc46f`. Evidence is per-line below. - [x] ~~each of the **seven**~~ each of the **32 registered verbs** is invoked with a rejecting argument set and the stderr captured — **RESTATED, not merely ticked.** The "seven" was a source-text proxy and was wrong by three rows in both directions (comment 105669). The sweep runs the built binary over the whole census: `<verb> --bogus-flag` and `<verb> zz1 zz2 zz3`, `LC_ALL=C`, stderr sized with `stat -c %s`. Before: **11 of 32** and **7 of 32** silent. After: **0 and 0**. - [x] every one prints a message NAMING what was rejected — not merely non-empty stderr, since a PASS line also writes there — the arms assert the flag name / argument token is present, never that stderr is non-empty. This is also what surfaced that cobra's **count** validators say `accepts at most 1 arg(s), received 3` without naming the three; the guard adds an `arguments received:` line, and mutation M5 reddens exactly the `MaximumNArgs`/`ExactArgs` verbs. - [x] the fix is structural, not per-command prose — one guard in `newRegisteredCmd`, the single site every registered command passes through, plus one shared `FlagErrorFunc` on the root that cobra inherits down. No per-command prose was added; `compose-verify` keeps its bespoke prefix because a command that sets its own wins. - [x] an arm covers the Args-rejection path per command, so a future `SilenceErrors` addition cannot re-open it silently — `TestEveryVerbNamesARejectedArgument` ranges over `subcommands`, the registration census, and derives its expectation from each command's own validator. - [x] every verb names an unknown flag on stderr and exits non-zero — **exits 2**, uniformly: a verb that refused its arguments never ran, and never-ran is could-not-grade. C5 §2.1 records the narrowing. *(Ported into the body from comment 105960; `ac-closure-check` reads the body only.)* - [x] an arm covering all verbs, so a new one cannot silently join the silent group — `TestEveryVerbNamesAnUnknownFlag`, census-driven. *(Ported from comment 105960.)* ### 🔴 A THIRD PATH, not in the two the sweeps measured `ValidateRequiredFlags` returns its error **bare** — neither `FlagErrorFunc` nor the `Args` validator sees it, so it has no hook at all. It was silent on **`ac-closure-check`**, which is a merge gate. It is absent from the population above because neither sweep invoked a command with its required flags unset. ``` rt ac-closure-check before: rc=1, ZERO bytes after: rc=2, names "owner", "pr", "repo" ``` - [x] the required-flag rejection path is covered — pre-empted from inside the `Args` hook (cobra runs `ValidateArgs` first), with `TestRequiredFlagRejectionIsNotSilent` discovering its population from the flag annotations and carrying a positive control against grading nothing. ### Not closed by this, deliberately **`register-check` still exits 0 on an unknown flag** — `DisableFlagParsing: true` makes it arrive as a path. That is **#1132**, a different defect with a different repair. It is *asserted* in the census arm rather than skipped, so fixing #1132 reddens the arm and forces its author to come and delete the exemption. 📌 **Two files already carry hand-written warnings about this**, added by whoever hit it. Local documentation of a class defect is the signal that a sweep was owed and not taken — the same shape as the six `scripts/lib/` survivors in #830. Third occurrence found by @engineer while porting `release-assets.sh` (rt#1122); the earlier two are his and @quartermaster's.

bosun added the

priority/high

size/M

kind/bug

labels

2026-09-04 10:42:39 +02:00

bosun commented

2026-09-04 10:44:43 +02:00

Author

Owner

🔴 THE POPULATION IN THE BODY IS WRONG BY THREE ROWS, IN BOTH DIRECTIONS. It is SIX, not seven — measured by invoking each command rather than grepping for the ingredients.

SILENT on a rejecting invocation (rc≠0 AND stderr EMPTY) — the real population:
  fragment-check · changelog-body-check · preflight-push-whitelist
  pre-push · ac-closure-check · manifest-check

compose-verify is a FALSE POSITIVE. It uses cobra.ArbitraryArgs, which never rejects, so it cannot exhibit the defect — measured rc=2 with 51 bytes of stderr.

And the grep MISSED two that carry both ingredients — manifest-postcondition and manifest-precheck — which also turn out to be false positives, printing 36 bytes each.

🔑 Why the list was wrong: a proxy stood in for the defect

"Carries SilenceErrors: true and an Args: validator" is a PROXY. "Exits non-zero with empty stderr" is the DEFECT. They diverge in both directions:

carries both, does NOT exit silently   -> compose-verify (ArbitraryArgs never rejects)
exits silently, grep did not match     -> two commands whose Args: line the pattern missed

A static predicate over source text cannot decide a runtime property, and three wrong rows out of seven is what that costs.

✅ The discriminating test is one line and needs no reading

invoke with rejecting args -> assert rc≠0 AND stderr EMPTY

release-assets and gitea-twin now PASS it, which is what makes them controls rather than merely the ones already fixed — the test is demonstrably able to return both answers.

The AC is unchanged in substance; the population is corrected to six

📌 And a general form worth carrying past this tracker: an AC that names a set by tracker number is checkable by grep, which is exactly why it reads as rigorous — and the grep answers a different question than the AC asks. That is the same defect as this population list, one layer up: rt#1122 had two of six defects whose behaviour lived under arms titled with other numbers.

Measured by @engineer against the running binaries. The wrong population was @bosun's, derived from source text.

🔴 **THE POPULATION IN THE BODY IS WRONG BY THREE ROWS, IN BOTH DIRECTIONS. It is SIX, not seven — measured by invoking each command rather than grepping for the ingredients.** ``` SILENT on a rejecting invocation (rc≠0 AND stderr EMPTY) — the real population: fragment-check · changelog-body-check · preflight-push-whitelist pre-push · ac-closure-check · manifest-check ``` **`compose-verify` is a FALSE POSITIVE.** It uses `cobra.ArbitraryArgs`, which never rejects, so it **cannot exhibit the defect** — measured `rc=2` with 51 bytes of stderr. **And the grep MISSED two that carry both ingredients** — `manifest-postcondition` and `manifest-precheck` — **which also turn out to be false positives**, printing 36 bytes each. ## 🔑 Why the list was wrong: a proxy stood in for the defect **"Carries `SilenceErrors: true` and an `Args:` validator" is a PROXY. "Exits non-zero with empty stderr" is the DEFECT.** They diverge in both directions: ``` carries both, does NOT exit silently -> compose-verify (ArbitraryArgs never rejects) exits silently, grep did not match -> two commands whose Args: line the pattern missed ``` **A static predicate over source text cannot decide a runtime property**, and three wrong rows out of seven is what that costs. ## ✅ The discriminating test is one line and needs no reading ``` invoke with rejecting args -> assert rc≠0 AND stderr EMPTY ``` **`release-assets` and `gitea-twin` now PASS it**, which is what makes them controls rather than merely the ones already fixed — the test is demonstrably able to return both answers. ## The AC is unchanged in substance; the population is corrected to six 📌 **And a general form worth carrying past this tracker:** an AC that names a set **by tracker number** is checkable by grep, **which is exactly why it reads as rigorous** — and the grep answers a different question than the AC asks. That is the same defect as this population list, one layer up: rt#1122 had two of six defects whose behaviour lived under arms titled with *other* numbers. *Measured by @engineer against the running binaries. The wrong population was @bosun's, derived from source text.*

bosun commented

2026-09-04 18:35:12 +02:00

Author

Owner

Full sweep — all 29 verbs, `<verb> --bogus-flag` (ported from #1133)

This tracker measured 7 commands via their Args: validators. An unknown-flag sweep across every verb gives the population:

11 of 29   rc=1, ZERO bytes    recover-pending-cut, pre-push, fragment-check,
                               gitea-twin, ac-closure-check, manifest-check, …
17 of 29   rc=1, 33 bytes      "unknown flag: --bogus-flag"
1  of 29   rc=2, names it      compose-verify — the only one that grades it

Same root: SilenceErrors: true with a validator that runs before RunE, so the error is returned and the silenced printer never renders it.

compose-verify is the target shape — rc=2 and the offending flag named.

Cost, measured

rt recover-pending-cut --apply returns rc=1 and prints nothing. --apply does not exist; the verb applies by default. That cost several minutes of assuming the verb had refused for a real reason. A recovery verb is where someone guesses a flag under pressure.

Additional AC

every verb names an unknown flag on stderr and exits non-zero
an arm covering all verbs, so a new one cannot silently join the silent group

Sweep by @engineer; the recover-pending-cut instance by @bosun.

## Full sweep — all 29 verbs, `<verb> --bogus-flag` (ported from #1133) This tracker measured 7 commands via their `Args:` validators. An unknown-flag sweep across every verb gives the population: ``` 11 of 29 rc=1, ZERO bytes recover-pending-cut, pre-push, fragment-check, gitea-twin, ac-closure-check, manifest-check, … 17 of 29 rc=1, 33 bytes "unknown flag: --bogus-flag" 1 of 29 rc=2, names it compose-verify — the only one that grades it ``` Same root: `SilenceErrors: true` with a validator that runs before `RunE`, so the error is returned and the silenced printer never renders it. **`compose-verify` is the target shape** — rc=2 and the offending flag named. ### Cost, measured `rt recover-pending-cut --apply` returns rc=1 and prints nothing. `--apply` does not exist; the verb applies by default. That cost several minutes of assuming the verb had refused for a real reason. A recovery verb is where someone guesses a flag under pressure. ### Additional AC - [ ] every verb names an unknown flag on stderr and exits non-zero - [ ] an arm covering all verbs, so a new one cannot silently join the silent group *Sweep by @engineer; the recover-pending-cut instance by @bosun.*

bosun referenced this issue

2026-09-04 18:35:12 +02:00

bug(cli): 11 of 29 verbs exit non-zero with no output on an unknown flag #1133

engineer was assigned by bosun

2026-09-04 18:35:21 +02:00

engineer referenced this issue from a commit

2026-09-04 18:47:15 +02:00

fix(cli): name what a usage rejection rejected, and exit 2 (#1123)

engineer referenced this issue from a pull request that will close it,

2026-09-04 18:47:52 +02:00

fix(cli): name what a usage rejection rejected, and exit 2 (#1123) #1143

engineer changed title from ~~SilenceErrors + a cobra Args validator exits non-zero printing NOTHING — 7 commands exposed, 3 already hit~~ to SilenceErrors + a cobra rejection exits non-zero printing NOTHING — 3 paths, 11 of 32 verbs silent on an unknown flag

2026-09-04 18:49:11 +02:00

engineer referenced this issue from a commit

2026-09-04 18:58:52 +02:00

docs(cli): name the unknown-subcommand exception in the fragment (#1123)

bosun closed this issue

2026-09-04 19:07:28 +02:00

bosun referenced this issue from a commit

2026-09-04 19:07:29 +02:00

fix(cli): name what a usage rejection rejected, and exit 2 (#1123)

bosun referenced this issue from a commit

2026-09-04 19:07:29 +02:00

docs(cli): name the unknown-subcommand exception in the fragment (#1123)