fix(pr): exit status must agree with the failure the command just printed

[laynepenney]

run_pr_create collected per-repo failures into all_failed_repos, printed
Failed to create N PR(s): with every one of them, and then returned Ok(()).
Anything gating on the exit status — a shell script, CI, an agent deciding
whether to continue — read that as success while the command was saying the
opposite on stdout.

The predicate was never missing. The --json branch already computed
success: !created.is_empty() && failed.is_empty(). The truth was computed and
then discarded, which is the same shape as the merge base in #917: the right
value in hand, one line from where it was needed, dropped.

What changed

• The human-output path now returns an error when all_failed_repos is
  non-empty, naming the count and the repos.
• --json keeps exit 0 and carries pass/fail in the body. That is this repo's
  shipped convention rather than a carve-out: gr verify --json returns Ok
  before its own exit(1) (verify.rs:101), and docs/PLAN-verify.md gives the
  reason — a caller who asked for JSON is parsing the body by construction, and
  a non-zero exit makes a set -e script die before it can read the answer it
  asked for.
• Both sites that serialize the --json payload now build it with
  pr_create_json_payload: the terminal branch at create.rs:470 and the
  empty-branch_groups early return at :213. There were two, and they
  disagreed. See below.
  Enumerated rather than asserted, because "both" is exactly the universal that
  four gate rounds have blocked this PR for: run_pr_create spans create.rs:110-543 and
  contains three to_string_pretty calls — :213, :455, :470. :455 is
  not a payload site; it serializes StateFile to .gitgrip/state.json on disk
  and no caller reads it on stdout. Two payload sites, three serializations, and
  the sentence now says which.
• Two witnesses assert on the bytes the shipped binary prints, one per route.

A behaviour change, stated plainly rather than folded in

gr pr create --json on a workspace where nothing is ahead previously emitted
"success": true. It now emits "success": false, with prs and failed both
empty as before, and the exit status is unchanged at 0.

This is not a new opinion about what a no-op means. Production already held
both opinions: the terminal --json branch computed success: false from
zero-created/zero-failed, while the empty-branch_groups early return
hardcoded "success": true for that identical state. Both shipped. Which
answer a caller received depended on how far the command happened to get, so no
consumer could rely on either. This change makes the two routes agree by
routing both through the one function that computes the value.

Whether a no-op ought to read success: false is a separate question
(#804/#836/#839) and is deliberately not settled here. What is settled is that
production must not answer it two different ways at once. Reviewers who would
rather see the divergence resolved the other way — both routes reporting true
— should say so; that is a one-line change to the same single site, and it is
the reason this section exists instead of a quieter diff.

Four gate rounds, one defect — and four more instances outside the rounds

Each round fixed a real thing and left the same shape one layer out. Recording
that is the useful part of this PR.

v1 — a test name instead of a body. The --json test never inspected the
payload, so mutating success to unconditional true survived every test —
while the test was named
json_mode_reports_the_failure_in_the_body_and_still_exits_zero. Both reviewers
blocked it independently.

v2 — a construction site instead of a wire. The payload was extracted to one
function and pinned on serialized text, and the body claimed the assertion ran
"through the same serde_json::to_string_pretty call the command makes" and
that run_pr_create "cannot route around the construction site". Both sentences
were false: the unit called the helper and to_string_pretty itself. Sentinel
proved it by leaving the helper correct and setting result.success = true at
the production call site — all six witnesses stayed green while a caller would
receive process success AND payload success after the platform refused.

v3 — one return enumerated instead of all of them. A subprocess witness
closed that wire, and the body then claimed the payload was "built in one
function" and that a no-op payload was pinned success: false "regardless".
Sentinel and Atlas independently found the second serialization site at the
empty-branch_groups early return, which made both sentences false again. The
helper's own no-op unit could not see it, because it calls the helper directly —
the same helper-not-use shape this PR had already named twice.

v4 — one witness read instead of both. The body said both subprocess
witnesses "guard the harness before asserting any content ... they require exit
0 and parseable output first." That was true of witness (5) and false of witness
(4), which parsed stdout, asserted success: false and the repo name, and only
then read out.status. Atlas blocked on it in r2. So a stale binary printing
nothing would have failed witness (4) on a content assertion — the exact
absent-versus-could-not-look collapse the sentence claimed to prevent, inside the
witness that names it. Fixed at 1b50436: both witnesses now run the identical
sequence — exit status, parseable output, route control, content — so "both" is
checkable by reading them side by side rather than by trusting the sentence. The
trailing exit assertion is not duplicated; the one guard serves as the harness
check and as the exit-0 half of the --json convention, asserted on that run
rather than inferred from another.

And a fifth instance, in the commit that fixed the fourth. 1b50436's
message says it "corrects a citation repeated in the source comment and the PR
body". It corrected one of three sites. A class sweep for file.rs:NNN across
the touched surfaces found create.rs:473 fixed, tests/test_pr_create_exit_status.rs:141
still reading verify.rs:99, and this body still reading it too. Filed forward
as 4ba0455 rather than amended, so the miss stays legible. The correct anchor
is verify.rs:101, the return Ok(()) inside the --json branch; :99 is the
struct literal's closing brace, and the exit(1) it returns before is at :132.

The general form, and it did not stop at the code. A witness that calls a
helper directly pins the helper, not its use. And a universal — "built in one
function", "both witnesses guard first", "corrects a citation", "both
serialization sites", "(1)–(5) survive" — is a claim about every instance,
which can only be held by enumerating them.

Asserting one after checking one is how this PR has now been wrong eight
times: a test name (v1), a body sentence (v2), a body sentence again (v3), a
body sentence again (v4), a commit message claiming a class sweep it had not
done (4ba0455), a count of serialization sites that was two of three, a
mutation-table survives-column whose range grew as witnesses were added without
the row being re-run, and a claim that a guard existed when no test guarded it
at all (see F1). The last three were found at this freeze.

The trajectory is the point. Rounds 1 through 3 moved the assertion one step
closer each time to what a caller actually observes, and a reviewer moved it
every time. Rounds 4 onward are about prose and tables, which no test can go
red over — so they are caught by someone reading, or by a sweep, or not at all.
The last three were caught here, before the freeze, by enumerating and by
going to look for a guard I had claimed — rather than by being more careful. That is the only difference I can point to that is
evidence of anything: the fix for a universal is an enumeration written into the
prose, not a resolution to check harder.

Scope, deliberately narrow

The umbrella issue covers nine verbs. This is one of them and no more. The
exit status of a zero-created/zero-failed run is unchanged at 0; that is the
zero-match question tracked separately. The only behaviour change is the JSON
success value described above, and it is a divergence repair rather than a
new semantic.

Witnesses

tests/test_pr_create_exit_status.rs — behaviour and wiring:

1. a reported failure must not exit zero — with a control that the creation was
   actually attempted;
2. a run with no failures still exits zero — the discriminating control, without
   which "always error" and "error only on failure" are indistinguishable;
3. --json still exits zero on a failed run;
4. the shipped binary prints success: false after a platform failure —
   subprocess, with a received-requests control;
5. the shipped binary's early no-op return serializes through the same
   helper — subprocess, with a route control described below.

src/cli/commands/pr/create.rs::json_payload_tests — payload field controls:

6. a failed run serializes success: false and names the repo and the reason;
7. a clean run serializes success: true — the discriminating control, without
   which a hardcoded false would satisfy (6);
8. a no-op run is not reported as success.

Both subprocess witnesses guard the harness before asserting any content, and
as of 1b50436 they do so in the same order — exit status, parseable output,
route control, content — so the claim is checkable by reading them side by side.
The guard exists because cargo test --test <name> does not reliably rebuild
the binary cargo_bin invokes, and a stale or half-written binary yields empty
stdout whose failure reads exactly like a missing feature. Absent and
could-not-look must not share a failure — one run during development did
exactly that, and until the v4 gate this sentence was true of only one of the
two witnesses it covered.

Witness (5)'s route control is worth naming, because the obvious one does not
discriminate: empty prs and failed do not identify the early return, since
the terminal branch reports the same arrays for a run that built groups and
created nothing. The control is instead the one line only the early return can
print — the same workspace in human mode must say "No repositories have changes
to create PRs for."

Mutation evidence

Re-measured in full at this head, 4ba0455. Every row below was run today;
none is carried from an earlier freeze. Each mutation was applied to the
committed tree, the binary rebuilt, both witness sets run, and the tree restored
under a trap … EXIT INT TERM whose restore is verified by re-hashing the
source against a baseline fingerprint taken before the first mutation
(a215232cf7f407cf, matched after all six).

mutation	dies	survives
never bail	(1)	(2)–(8)
always bail	(2)	(1), (3)–(8)
remove the --json early return	(3), (4)	(1), (2), (5)–(8)
success forced to true in the helper	(4), (5), (6), (8)	(7), (1)–(3)
result.success = true at the production call site	(4)	(1)–(3), (5)–(8)
the early return's hardcoded success: true restored	(5)	(1)–(4), (6)–(8)

The last two rows are the reviewers' own mutations, reproduced. That the helper
units survive both is correct: they are field controls, and only a subprocess
witness can tell the two production routes apart.

Two rows of this table were wrong in v4, and re-running is what found it

The bolded cells are corrections to the v4 body, not new behaviour. v4 recorded
remove the --json early return as killing only (3), and success forced to true in the helper as killing only (6) and (8) with "(1)–(5)" in its survives
column. Both understate the suite. Measured today, with per-witness output
rather than pass/fail counts:

• M3 kills (3) and (4) — removing the early return sends --json down the
  human path, which bails, so the subprocess witness fails its exit-status guard.
  (1), (2), (5) and all three units pass.
• M4 kills (4), (5), (6) and (8) — forcing success in the helper
  propagates to the bytes the shipped binary prints, so both subprocess
  witnesses see it. (7) and (1)–(3) pass.

The direction is the safe one and the shape is not. The old table claimed
fewer witnesses die than actually do, so nothing shipped weaker than
advertised. But (1)–(5) is a range that grew as witnesses (4) and (5) were
added across v3 and v4 without the row being re-run — a universal asserted over
a set that changed underneath it, which is this PR's own recurring defect for the
seventh time, in the one table whose whole purpose is to be checked. I cannot
reconstruct which freeze the row went stale at; only that the range was extended
rather than re-measured.

I re-ran it because 1b50436 reordered witness (4)'s assertions and a mutation
table is a claim about the code as it stands. That specific hypothesis was
wrong — never-bail and always-bail land exactly where v4 said, and the
reordering moved nothing. The rows that had rotted were stale for a different
reason than the one that made me look. Recording that because the reasoning was
worth acting on even though its prediction was not: the rule is re-measure a
carried table, not re-measure when you can name the thing that would have
changed it.

Verification

• cargo test --all: exit 0, 1196 passed / 0 failed. Eight of those are the
  witnesses above.
• cargo fmt --all --check: exit 0.
• cargo clippy --all-targets --all-features: exit 0. Counted as
  grep -c '^warning:' identically on both sides, base in a detached worktree
  at origin/dev: 155 → 156, delta +1, attributed. The one added line is
  the per-target summary for the new integration test target; that target's 48
  warnings are all dead_code, confirmed by code via --message-format=json,
  and are the standard mod common pattern where every integration binary
  compiles all shared helpers and warns about the ones it does not use.
• Note on comparability: v2's body quoted 2194 → 2242 under a different counting
  command. These figures are internally consistent and measured identically on
  both sides; they are not comparable to v2's.

F1, and F1 committed a second time

The doc-comment finding carried non-blocking from the #917 gate is fixed here as
r1 ruled: that patch left the whole squash-incident doc block attached to
pr_base_or_stored_target, so verify_merge_commit_parents — the function the
block is actually about — was left undocumented.

Then the payload hoist reproduced the same defect inside this branch. Moving
the JSON construction above run_pr_create inserted sixty lines between
#[allow(clippy::too_many_arguments)] and the function it governs, so the
attribute and the function's doc comment both landed on the new struct — the
version that has a behavioural consequence rather than only a rustdoc one.

It was caught by measuring the clippy delta, not by reading exit 0: chasing
two unattributed warnings found too_many_arguments firing twice because the
attribute had stopped suppressing anything. cargo clippy exited 0 the whole
time and said nothing. Both blocks now sit adjacent to the functions they govern
— #[allow(clippy::too_many_arguments)] at create.rs:109, run_pr_create at
:110.

Correction to the v4 body, and it is the eighth instance of this PR's own
defect. v4 said that adjacency was "asserted by line ordering with a negative
control proving the assertion can report WRONG." There is no such assertion.
I went looking for it at this freeze and it does not exist — not in the range,
not in the tree, and no test anywhere reads source to check ordering. Measured
by mutation: detaching the attribute from run_pr_create leaves cargo test --all at exit 0 with zero failures. Nothing guards it.

What is true is weaker and worth stating exactly. The same mutation moves the
clippy warning count 156 → 157 and makes too_many_arguments name the
function — but cargo clippy still exits 0, so the signal is visible only to
someone counting warnings, which is the manual step recorded under Verification
above. So this is a detectable regression, not a witnessed one, and it sits in
the same class as N3's unwitnessed guarantee rather than in the mutation table.

I am not adding a source-reading adjacency test at freeze time: the real
instrument is the clippy delta, it is already a measured step, and a brittle new
witness introduced at the freeze would need its own round. Naming the gap is the
honest option, and a reviewer who wants the witness instead should say so.
Recording the shape too, because it is the sharpest instance yet — the false
claim was that a guard existed, and it sat inside the paragraph congratulating
itself for measuring rather than reading exit 0.

Premium boundary

grip is OSS. This is CLI exit-status semantics: no identity resolution, no org
context, no workspace-identity derivation.

Ref #886 — closes at promotion