docs: state the four-code exit ladder in the README

[Kiran01bm]

Why

pg-sprite exits with one of four codes: 0 (executed through an online-safe path), 1 (failed), 2 (refused, nothing committed), 3 (executed without online safety, once a migrate flag reaches it). That code is the only thing a shell gate in CI reads. Until now the full ladder was written down in one place, docs/lock-budgeted-passthrough.md, a design doc most users never open. The README said "exit code 2" in passing and nothing about 1 or 3, so a CI author had to guess whether a gate on != 0 was safe and what exit 2 promised about the table.

What

The README and docs/cli-output-examples.md gain an ## Exit codes section with the same four-row table and three rules: gate on non-zero, refusal is one code across migrate, diff, and pull, and exit 2 means nothing committed (a budget-cancelled attempt did run and rolled back). Exit 3 is marked reserved: executor.ExecuteAcceptedBlocking ships as a library primitive and no CLI flag reaches it yet. Seven other docs that glossed exit 2 as "nothing ran" now say "nothing committed", and the passthrough doc's rollout plan records that the ladder is surfaced.

A new test, TestExitCodeLadderDocsListEveryCode in pkg/verdict/docs_test.go, requires a | <code> | row for every exit code in each page that states the ladder. Adding an exit-code constant without a row, or dropping a row, fails the test.

How

Docs and one test only. No behaviour changes, so demo/tour.sh and the generated regions of docs/capabilities.md are untouched (make check-capabilities passes). The table wording was checked against cmd/pg-sprite/main.go (error-to-code mapping), pkg/migrate/migrate.go (--force acknowledgement mismatch is a plain error, exit 1), and pkg/verdict (exit constants). I did not add named constants for exits 0 and 1; they are the shell conventions the entry point inherits from kong, and the test lists them as literals with a comment saying so.

Risk

Low. Documentation plus a doc-pinning unit test.

Testing

Negative check of the new test, not run by CI: removed the exit-3 row from the README and ran SKIP_INTEGRATION=1 go test ./pkg/verdict/ -run TestExitCodeLadder; it failed with README.md is missing an exit-code ladder row for exit 3. Restored the row; the test passes.

Bigger picture

This is step 3 of the rollout plan in docs/lock-budgeted-passthrough.md. Step 4, the --accept-blocking flag on migrate, is what makes exit 3 reachable from the CLI; the capabilities matrix row for that tier stays "exit 2" until then.

Generated with Amp (Claude)

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[aparajon]

🤖 Adversarial review 1/2 — does the guard actually guard? cb436fbb, 9 files, +212/−24.

The interesting half of this PR is pkg/verdict/docs_test.go: a docs PR that pins itself. So I attacked the pin rather than the prose — 15 mutations across the three ladder tables and the constant block, 9 caught, 5 survived, 1 panic.

What it catches, and it is more than the commit message claims. The assertion is assert.Equal on the two sets, so it is bidirectional: deleting any row from any of the three docs fails, and so does adding a row for a code that does not exist (I added | 4 | to the README: (len=4) vs (len=5)). Renumbering either constant fails. An iota-derived constant fails with the intended message rather than silently. And moving the constants out of verdict.go fails closed, exactly as the two require.Contains guards at docs_test.go:75-76 intend:

--- FAIL: TestExitCodeLadderDocsListEveryCode
    Error: map[int]struct {}{0:{}, 1:{}} does not contain 2
    Messages: the walker recognises the declared constants

Three things it does not catch, and one crash.

---

1. The walker is file-scoped, so a new code in a sibling file of the same package is invisible

docs_test.go:49 parses one filename:

file, err := parser.ParseFile(fset, "verdict.go", nil, parser.SkipObjectResolution)

The mutation pair is the whole finding:

Mutation	Result
ExitCodeQuarantined = 4 added inside verdict.go	CAUGHT — docs don't list 4
ExitCodeQuarantined = 4 added in pkg/verdict/exitcode_extra.go	SURVIVED — suite green, docs silently incomplete

Same package, same const, same ExitCode prefix; the only difference is which file the next author opened. The require.Contains pair protects against the existing two constants moving — it cannot protect against a third one arriving, because it is hardcoded to the two that exist today, and a guard whose coverage is a hardcoded list of what it already knows about is the shape that rots.

This matters more than a normal test gap because the value proposition here is "derive the ladder from the declared constants" (the commit subject). Deriving it from one file's declared constants is a weaker claim than the test's name makes, and nothing in the file says so.

parser.ParseDir over ., skipping _test.go files, closes it in about four lines and makes the name true. That also retires the require.Contains pair, or demotes it to a non-empty check.

2. An implicit-repetition const spec panics instead of failing

docs_test.go:67 indexes vs.Values[i] after ranging vs.Names. Those two lengths are equal for every spec that has values. Drop the value from the second spec — legal Go, and the idiomatic way to write a repeated const:

const (
	ExitCodeRefused = 2
	ExitCodeAcceptedBlocking      // implicit repetition
)

panic: runtime error: index out of range [0] with length 0

The *ast.BasicLit type assertion right below it already handles the non-literal value case with a clear require.Truef message, so the non-literal shape was clearly considered — the absent shape was not. if i >= len(vs.Values) { continue }, or a require.Lenf naming the spec, turns a stack trace in a test called …DocsListEveryCode back into a sentence.

3. emit maps one outcome out of four, and the one it silently drops is exit 3

This is outside the diff but it is the behavioral half of the contract the PR documents, and I do not think it can be left for step 4 without a note here.

internal/cli/migrate.go:108-123 is where a verdict becomes a process status, and it tests exactly one outcome:

if v.Outcome == verdict.OutcomeRefused {
	return verdict.ErrRefused
}
return nil

OutcomeFailed is safe, and deliberately so — it only reaches emit on the runErr != nil path, which returns runErr afterwards, and the comment above it says so. But OutcomeExecutedWithoutOnlineSafety arriving at migrate.go:60 returns nil → exit 0, not exit 3. Exit 0 is the code this PR's own README reserves for "online-safe", and verdict.go:23-24 says the point of exit 3 is that it "cannot be confused with online-safe exit 0."

Latent today, and the PR is right that it is: WithAcceptedBlocking has no production caller — only pkg/verdict/verdict_test.go and internal/cli/docs_test.go construct that outcome, so no invocation can reach emit with it. The reservation is honest.

But consider how step 4 fails. --accept-blocking wires the flag, migrate.Run returns the accepted-blocking verdict with runErr == nil, and the process exits 0 while the JSON says executed-without-online-safety. No test fails, because this PR pinned doc↔constant agreement and nothing anywhere pins constant↔behavior. A three-line emit case plus a table test over the four outcomes would land that half now, while the contract is the thing being written down.

4. A fourth table in docs/ says "exit 3" and means the opposite

docs/optimistic-attempt.md:221 — "Exit inventory", untouched by this PR:

| Exit | Outcome | Typed as | DDL executed? |
| 3a | Lock not granted within lock_timeout  | ... | No — nothing executed     |
| 3b | Statement ran past statement_timeout  | ... | No — rolled back cleanly  |

That column is a scenario index, not a process status — row 7 gives itself away by saying "exit code 1" in its own cell. But it is headed Exit, it is numbered 1–7, it sits in the same directory, and it collides on the single most safety-relevant cell in the new ladder: exit 3 is the only code meaning "something committed that we do not vouch for", and this page tells a reader exit 3 means nothing executed. Rows 1, 2, 4 and 5 are equally inverted (Exit 1 = success, Exit 4/Exit 5 = refusals that really exit 2).

The new test cannot reach it — exitCodeLadderHeader is "| Exit | Meaning |" and this header is "| Exit | Outcome |", so it is neither in exitCodeLadderDocs nor matchable if it were. Renaming that column to Case or Path is a one-word change and it is this PR's job, because this PR is the one that makes Exit a defined term in the doc set.

---

Smaller, folded together — all three are scope calls I would keep, not asks. documentedExitCodes parses only the first cell, so a row may state the wrong meaning and pass (I swapped the meanings of 0 and 2 in cli-output-examples.md: green). It unions every matching table on a page rather than taking the first, so a second abridged table on the same page passes (also green). And exitCodeLadderDocs is hand-maintained, so a new page stating the ladder is unpinned (green). All three are the natural boundary of "pin completeness, not content", which is the right scope — worth knowing they are the boundary. I checked the three tables against each other by hand and they agree today, including the genuinely subtle bit: the README's "an optimistic attempt that exceeded its statement budget … still exits 2" and lock-budgeted-passthrough.md's "on this path only, a statement-budget cancellation is also a failure" are different paths, not a contradiction, and the README already forward-references the reason.

Everything else I checked is clean. All eleven #exit-codes links resolve to real headings (including the new ### Exit codes at lock-budgeted-passthrough.md:304). executor.ExecuteAcceptedBlocking is really exported and shipped, as the README claims. The README's lint/suggest exit claims match the code: lint returns ErrLintFindings on error-severity findings and nothing maps it to 2 or 3, so exit 1; suggest returns nil on any script it parses. checkForceAck returns a plain error, so a mismatched acknowledgement is exit 1 as documented. gofmt -l, go vet, and go test ./pkg/verdict/ ./internal/cli/ are all clean.

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 Review 2/2 — two lenses: OSS adoption, and integration ease for importers.

Lens 1: OSS adoption

Putting the ladder in the README is the single highest-leverage thing in this PR. The exit status is what a CI author needs before anything else — before the JSON schema, before refusal classes, before the execution model — and until now it was assembled from a parenthetical in cli-output-examples.md, a sentence in execution-model.md, and a design doc most evaluators will never open. A stranger writing their first .github/workflows/schema.yml against pg-sprite now has one table and three rules on the landing page, and "gate on non-zero" is stated as the default rather than left to be inferred.

The reserved framing is the part I would keep exactly as written. Saying plainly that exit 3 ships as a documented code with no invocation that produces it — and naming executor.ExecuteAcceptedBlocking as the primitive that does exist — is more useful to an outsider than quietly omitting the row and adding it later. It tells them the contract is stable before they depend on it. I verified the claim holds: WithAcceptedBlocking has no production caller in the tree.

The adoption hazard is the homonym, not the ladder. Covered as finding 4 in 1/2, but the adoption framing is a different argument for the same one-word fix: an evaluator's first move on a new repo is grep -ri "exit code", and that returns two tables in docs/ whose Exit columns disagree on what 1, 2, 3, 4 and 5 mean. From the inside, "Exit inventory" obviously means exit paths. From the outside, on day one, it reads as a second, more detailed ladder that contradicts the README — and the reader has no basis to pick a winner. This PR is what makes Exit a defined term in the doc set, so it is also what creates the collision.

One structural note for a first-time contributor: which table is canonical is never stated. The ladder now exists verbatim in three places, and the PR is careful about their roles — lock-budgeted-passthrough.md now says its section "holds the reasoning behind the two cells that are not obvious", which is exactly right. But the README and cli-output-examples.md are near-identical full restatements with no declared precedence, and a contributor who fixes a wording bug in one has no signal that two more copies exist. One clause in the two non-canonical tables ("restated from the README") would do it, and it costs less than the test does.

Lens 2: integration ease for schemabot and other orchestrators

No exported API changed; the diff is docs plus one test file. Nothing an importer compiles against moves. So the interesting question is not what breaks — nothing does — but whether this PR's contract is the one an embedder actually consumes. It is not, and that is worth being explicit about in the README.

I measured it against the real consumer. schemabot imports ten pg-sprite packages:

pkg/dbconn  pkg/diffplan  pkg/executor  pkg/plan  pkg/planner
pkg/preflight  pkg/progress  pkg/router  pkg/schemadiff  pkg/statement

pkg/verdict is not among them, and schemabot never execs the binary. It branches on 75 exported pkg/executor symbols — 30 executor.Code* outcome codes and 18 executor.Err* sentinels. So the entire four-code ladder this PR documents, pins, and cross-links through six pages is invisible to the one orchestrator that exists today.

That is not a criticism of the scope — the ladder is a CLI contract and it should live on the CLI's page. It is a gap in where the README leaves an evaluating embedder. ## Exit codes is the only section on the landing page that names an outcome contract, so someone assessing pg-sprite as a library reads it as the outcome contract and then has to discover on their own that their surface is executor.Code and errors.As. The fix already exists in the tree, one directory down: docs/execution-model.md:180 says "Library callers get the same facts typed", which is precisely the sentence the README's new section is missing. Adding it as a fourth bullet — one line, pointing at execution-model.md — makes the README honest about having two audiences instead of one.

The reservation is genuinely useful to an embedder, and for a reason the docs undersell. Because ExecuteAcceptedBlocking is exported and shipped while the flag is not, an orchestrator can adopt the accepted-blocking path now, on its own policy surface (an operator command, a PR label, an approval gate) without waiting for step 4 or ever touching the CLI. docs/cli-output-examples.md now says this — "the executor primitive and the verdict shape are shipped; the flag that reaches them is not" — but it says it inside a CLI example, framed as an apology for a non-runnable invocation. Said once from the library's side it is an adoption lever rather than a caveat.

One forward-looking note that belongs to the importer, not to this PR. When step 4 lands, the exit-3 seam and the library seam diverge: the CLI gets a new process status, and an embedder gets a verdict outcome it may not have a branch for. schemabot's 30-code switch has no arm for accepted-blocking today. A default: that treats an unrecognized outcome as success is the embedder-side version of finding 3 in 1/2 — same failure, different surface. Worth a sentence in step 4's docs telling embedders to audit their outcome switch, since the exit-code ladder will not warn them.

Approving. Nothing here blocks: the guard works in every direction I could test it, the prose claims check out against the code, and the ladder is a real improvement to the landing page. The two I would fix before merge are both one-liners — the sibling-file gap in the walker (finding 1) and the Exit inventory column name (finding 4).

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 Approved. The pin works in every direction I could push it — 15 mutations, 9 caught: every row deletion across all three ladder tables, both renumberings, a doc listing a code that does not exist, and moving the constants out of verdict.go (which fails closed with the intended message). Four findings in the review comments, none blocking. The two worth a follow-up commit are both one-liners: the walker parses verdict.go by name, so a fifth exit code declared in a sibling file of the same package leaves the suite green and the docs silently incomplete — the identical constant added to verdict.go is caught, which is the whole asymmetry — and docs/optimistic-attempt.md's "Exit inventory" column is headed Exit while meaning scenario index, so it tells a reader exit 3 means nothing executed, the exact inverse of the cell this PR defines. Also flagged: vs.Values[i] panics rather than fails on an implicit-repetition const spec, and emit maps only OutcomeRefused, so the accepted-blocking outcome would exit 0 rather than 3 the moment step 4 wires the flag.

This stamp was left by Claude Code (claude-opus-5).

[Kiran01bm]

🤖 Adversarial review response — created by Kiran's code review agent (Amp, Claude) — block/pg-sprite pull/109, follow-up commit

All four findings and both structural notes are fixed in one follow-up commit; the ladder is now pinned constant↔behavior as well as constant↔docs, and the three mutations that survived or crashed the guard now fail it with a sentence.

#	Finding	Status	Explanation
1	C1-F3 — emit maps only OutcomeRefused; an accepted-blocking verdict would return nil and exit 0 once the flag is wired	Fixed	emit now switches on the outcome: OutcomeRefused → ErrRefused, OutcomeExecutedWithoutOnlineSafety → ErrAcceptedBlocking, everything else nil, with the doc comment stating that a failed verdict returns nil here because its caller returns the run error. A table test in internal/cli drives all four outcomes through emit in JSON mode and asserts the sentinel each reaches the entry point as, plus that the verdict was printed first. Removing the new case fails the executed-without-online-safety subtest. Taken now rather than deferred because it is the behavioral half of the contract this change writes down, and nothing else in the tree would have caught the exit-0 regression.
2	C1-F1 — the walker parses verdict.go by name, so an ExitCode* constant in a sibling file of pkg/verdict is invisible	Fixed	The walker lists the package directory and parses every non-test .go file, so the file an author opens no longer matters. parser.ParseDir was the obvious four-liner but is deprecated in the module's Go version, so the loop is os.ReadDir plus parser.ParseFile per file. The two hardcoded require.Contains guards are retired in favour of a single non-empty check ("the walker found at least one declared ExitCode constant"), which is the property that actually matters and does not rot. Re-ran the review's mutation pair: ExitCodeQuarantined = 4 in a sibling file now fails with len=5 vs len=4; moving both existing constants wholesale into a new sibling file stays green, as it should.
3	C1-F2 — vs.Values[i] panics on an implicit-repetition const spec	Fixed	The per-spec walk is extracted to collectExitCodes, which asserts i < len(vs.Values) by constant name before indexing. The implicit-repetition mutation now fails with ExitCodeAcceptedBlocking states its value explicitly instead of an index-out-of-range trace.
4	C1-F4, C2 — docs/optimistic-attempt.md "Exit inventory" is headed Exit while meaning scenario index, so it reads as a second ladder that inverts exit 3	Fixed	The column is now Case, and the section opens with one paragraph saying the case numbers are not process exit codes and pointing at the README ladder for the status each row actually exits with (0 success, 2 refusal, 1 failed sequence). The section heading and its two in-page anchors are unchanged so existing links resolve.
5	C2 — the README and cli-output-examples.md ladders are near-identical with no declared canonical	Fixed	The README is named as the canonical statement. cli-output-examples.md now says its table is restated from the README and that a wording fix lands there first; lock-budgeted-passthrough.md says the same and names its own table as the restatement with this path's cells spelled out.
6	C2 — the README's ## Exit codes is the only outcome contract on the landing page, and an evaluating embedder reads it as theirs	Fixed	A fourth bullet, "Library callers get the same facts typed, not as a status", says an orchestrator importing pkg/executor branches on the verdict outcome, errors.As to the executor's typed errors, and executor.OutcomeCode, that the exit code is the CLI's rendering of those facts, and links docs/execution-model.md#how-a-failure-is-reported. The lead-in now reads "Three rules follow from the table, and one note for embedders".
7	C1 — first-cell-only parse, union of tables on a page, hand-maintained doc list	No action	Agreed these are the boundary of "pin completeness, not content", and the review's recommendation is to keep them. Unchanged.
8	C2 — embedders should audit their outcome switch when the accepted-blocking flag lands	No action	Belongs to the change that wires the flag, as the review says; noted for that change's docs.
9	C1 — anchors, ExecuteAcceptedBlocking export, lint/suggest/checkForceAck exit claims, gofmt/vet/tests	No action	Verified correct by the review; unchanged.

Source: block/pg-sprite#109, review comments 5644188857 and 5644189217 and review 5185610700 at head 6ce84e26