Skip to content

feat(ship)!: three-verdict gates, evidence-backed checkpoints, and pre-checkpoint hooks that actually run - #38

Merged
teragrid merged 4 commits into
mainfrom
refactor/gate-verdict-three-states
Aug 7, 2026
Merged

teragrid merged 4 commits into
mainfrom
refactor/gate-verdict-three-states

Conversation

@teragrid

@teragrid teragrid commented Aug 6, 2026

Copy link
Copy Markdown
Owner

M3 of the failure-mode work. HookResult.Passed bool becomes Verdict{Unknown, Pass, Fail}.

Why a bool was the bug

A bool forces every gate that could not check — artefact missing, tool not installed, config it cannot parse — to answer either pass or fail. Gate authors almost always picked pass, because failing a build over something that is not the user's fault is obviously wrong.

So "I did not verify this" and "I verified this and it is fine" became the same value, and the caller could not tell them apart. Every instance of that in forge has been the same bug: a green checkpoint standing on a check that never ran.

VerdictUnknown is the zero value, on purpose

A handler that forgets to set a verdict yields unverified — honest — rather than a false pass.

Pinned by TestVerdict_UnknownIsTheZeroValue: if VerdictPass ever became iota's first value, every incomplete handler in the package would silently start reporting success. That is the kind of regression that produces no diff noise and no test failure anywhere else.

Unverified gates annotate the checkpoint UNVERIFIED[…] and never escalate it. They are suppressed on an already-failed checkpoint, which has a real error to show and does not need unverified notes piled on top.

What it fixed

spec-code-alignment-gate reported PASS on projects it had never examined — the gap M2's mutation table found on its first run. auditSlug() returns early when spec.md is absent, skipping every check, and the gate fell through to pass. forge ship --from=code on a project whose spec was never written got a green alignment gate that verified nothing.

It now returns VerdictUnknown. The gate did not find the project acceptable; it found it unexaminable. Different facts.

self-review-gate reported PASS after scanning zero files. Same shape, found by the same test.

Nine gates in total returned Passed: true when their artefact was missing (// no spec file yet, // no ADR file → nothing to check, …). All now name the missing file. Enforced going forward by TestGateMutation_NoGateReportsCleanOnAnEmptyProject.

What it found and deliberately did NOT fix

PhasePreCheckpoint hooks are registered but never invoked.

self-review-gate is declared, listed in defaultHooks(), documented in the package header, and covered by tests — and has never executed. runWithOptions calls runHooks for PhasePostCheckpoint and PhasePostPipeline only; there is no PhasePreCheckpoint call site anywhere in the package.

This is the failure mode one level above a gate that checks nothing: a gate that never runs at all. Everything about it is correct — handler, tests, docs — and none of it was ever reachable. Counting it among forge's quality gates has been inaccurate since it was written.

Not wired in here on purpose: turning on a gate that has never fired will flag artefacts in projects that have been shipping happily, and that belongs in a release with a changelog entry, not inside a test file. TestPreCheckpointHooks_AreRegisteredButNeverRun fails the moment it is wired in, so it becomes a decision someone makes rather than a fact nobody knows.

This needs a call before the next release.

Full suite green; go vet and golangci-lint clean.

🤖 Generated with Claude Code

vietking and others added 2 commits August 6, 2026 22:47
M3 of the failure-mode work. HookResult.Passed bool becomes
Verdict{Unknown, Pass, Fail}.

A bool forced every gate that could not check — artefact missing, tool not
installed, config it cannot parse — to answer either pass or fail. Gate authors
almost always picked pass, because failing a build over something that is not
the user's fault is obviously wrong. So "I did not verify this" and "I verified
this and it is fine" became the same value, and the caller could not tell them
apart. Every instance of that failure in forge has been the same bug: a green
checkpoint standing on a check that never ran.

VerdictUnknown is deliberately the zero value. A handler that forgets to set a
verdict yields "unverified", which is honest, rather than a false pass. Pinned
by TestVerdict_UnknownIsTheZeroValue: if VerdictPass ever became iota's first
value, every incomplete handler in the package would silently start reporting
success.

Unverified gates annotate the checkpoint UNVERIFIED[...] and never escalate it.
They are suppressed on an already-failed checkpoint, which has a real error to
show and does not need "unverified" notes piled on top.

Nine gates returned Passed:true when their artefact was missing (// no spec
file yet, // no ADR file → nothing to check, ...). All now return
VerdictUnknown naming the missing file. Enforced going forward by
TestGateMutation_NoGateReportsCleanOnAnEmptyProject.

## What this fixed

spec-code-alignment-gate reported PASS on projects it had never examined — the
gap M2's mutation table found on its first run. auditSlug() returns early when
spec.md is absent, skipping every check, and the gate fell through to pass:
`forge ship --from=code` on a project whose spec was never written got a green
alignment gate that verified nothing. It now returns VerdictUnknown. The gate
did not find the project acceptable; it found it unexaminable, and those are
different facts.

self-review-gate reported PASS after scanning zero files. Same shape, found by
the same test.

## What this found and did not fix

PhasePreCheckpoint hooks are registered but never invoked. self-review-gate is
declared, listed in defaultHooks(), documented in the package header, and
covered by tests — and has never executed. runWithOptions calls runHooks for
PhasePostCheckpoint and PhasePostPipeline only; there is no PhasePreCheckpoint
call site anywhere in the package.

That is the failure mode one level above a gate that checks nothing: a gate
that never runs at all. Everything about it is correct — handler, tests, docs —
and none of it was ever reachable. Counting it among forge's quality gates has
been inaccurate since it was written.

Not wired in here on purpose. Turning on a gate that has never fired will flag
artefacts in projects that have been shipping happily, and that belongs in a
release with a changelog entry, not inside a test file.
TestPreCheckpointHooks_AreRegisteredButNeverRun fails the moment it is wired
in, so the change becomes a decision someone makes rather than a fact nobody
knows.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…checkpoint hooks

M1 of the failure-mode work, plus the pre-checkpoint phase M3 turned up.

## M1 — a green checkpoint has to rest on something

Checkpoint.Status is a plain string that any of twenty-odd code paths could set
to "ok", and nothing ever required the code setting it to say why. Because
forge is the actor in all of those paths, the thing implicitly asserted was
always true — "I wrote the file", "I ran the generator", "I completed the
step". Those are facts about forge's behaviour, not about whether the change is
sound:

    spec.md written    != spec.md is complete
    test file written  != the test will ever run
    the gate returned  != the gate examined anything
    the checkpoint ran != the checkpoint verified anything

Checkpoints now carry Evidence tagged by source. SourceExternalTool (a scanner,
test runner, linter or git was asked and answered) and SourceReadBack (forge
re-read the artefact from disk and re-validated it, judging it as it would
judge a stranger's) are independent. SourceForgeClaim — forge asserting its own
success — is recorded, reported, and never sufficient alone.

Enforcement is at the reporting boundary rather than via a private field or a
mandatory Pass(evidence) setter. Routing every assignment through a setter
would invite SourceForgeClaim boilerplate that satisfies the compiler and
nothing else. A checkpoint reaching "ok" with no independent evidence is
downgraded to "warning" and annotated UNVERIFIED.

This cannot break a working pipeline. The claim a downgrade makes is "nobody
checked" — a reason to withhold confidence, not to block a release. res.Ready
keys on "fail", which this policy never produces.

The gates turned out to be the evidence system already: a hook returning
VerdictPass has read an artefact off disk and re-validated it, which *is*
read-back evidence — it was simply never recorded as the basis for the status.
Wiring that up gave most checkpoints real evidence without touching a single
Status = "ok" line. Only VerdictUnknown contributes nothing, which is what M3
was for. The ship checkpoint has no post-checkpoint gates, so it records its
own scanner and hygiene results explicitly.

Checkpoint markers gain an Evidence: line. The marker is the durable record —
what `forge ship status` reads and what someone opens months later to ask "was
this actually checked?" — and recording a status without its basis left that
question unanswerable.

## Pre-checkpoint hooks now run

self-review-gate was declared, listed in defaultHooks(), documented in the
package header, covered by tests — and had never executed, because
runWithOptions only ever called runHooks for the two later phases. Counting it
among forge's quality gates was inaccurate from the day it was written.

Wired in as advisory: findings annotate the checkpoint and downgrade ok to
warning, but do not fail it. Every project using forge has been shipping
without this gate, so switching it on as a blocker would break builds over
artefacts that were acceptable yesterday. HookConfig.Strict is the opt-in for
making it stop a run, as with every other hook.

All three pre-checkpoint concerns are routed through one beforeCheckpoint()
helper alongside the snapshot and the agent-mode checkpoint marker, so a future
checkpoint cannot pick up two of the three and silently miss the third.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@teragrid teragrid changed the title refactor(ship)!: quality gates get a third verdict — could not check feat(ship)!: three-verdict gates, evidence-backed checkpoints, and pre-checkpoint hooks that actually run Aug 6, 2026
@teragrid

teragrid commented Aug 6, 2026

Copy link
Copy Markdown
Owner Author

Update: M1 and the pre-checkpoint activation are now in this PR

M1 — a green checkpoint has to rest on something

Checkpoint.Status was a plain string any of ~20 code paths could set to ok, with nothing requiring the code setting it to say why. And since forge is the actor in all of those paths, the thing implicitly asserted was always true:

forge verified what mattered
spec.md written spec.md is complete
test file written the test will ever run
the gate returned the gate examined anything
the checkpoint ran the checkpoint verified anything

Checkpoints now carry Evidence tagged by source. SourceExternalTool and SourceReadBack are independent; SourceForgeClaim is recorded, reported, and never sufficient alone. A checkpoint reaching ok with no independent evidence is downgraded to warning + UNVERIFIED[…].

Enforcement is at the reporting boundary, not a mandatory setter. Routing every assignment through Pass(evidence) would just invite SourceForgeClaim boilerplate that satisfies the compiler and nothing else.

It cannot break a working pipeline. The claim a downgrade makes is nobody checked — a reason to withhold confidence, not to block a release. res.Ready keys on fail, which this policy never produces.

The gates were already the evidence system

A hook returning VerdictPass has read an artefact off disk and re-validated it — that is read-back evidence, it was just never recorded as the basis for the status. Wiring that up gave most checkpoints real evidence without touching a single Status = "ok" line. Only VerdictUnknown contributes nothing, which is exactly what M3 was for.

Checkpoint markers gain an Evidence: line — the marker is what someone opens months later to ask was this actually checked?, and a status without its basis left that unanswerable.

Pre-checkpoint hooks now run

The gap M3 surfaced, closed. Wired in as advisory: annotate and downgrade ok → warning, never fail. Every project has been shipping without this gate, so switching it on as a blocker would break builds over artefacts that were fine yesterday. HookConfig.Strict is the opt-in, same as every other hook.

All three pre-checkpoint concerns now route through one beforeCheckpoint() helper, so a future checkpoint cannot pick up two of three and silently miss the third.

Full suite green; go vet and golangci-lint clean.

vietking and others added 2 commits August 7, 2026 01:13
The M1-30 public-API delta gate failed this PR for removing
HookResult.Passed without a BREAKING.md entry, and it was right to.

cmdship is an internal/ package, so nothing outside this module can import it
and no external consumer can break. Recorded anyway: the gate treats the
module's own API as worth tracking, and a policy waived the first time it is
inconvenient stops being a policy.

Includes the migration for the three constructors, since the third one —
gateUnknown, for a gate that could not check — is the whole point of the
change rather than a mechanical rename.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Version decided before tagging rather than after the first bug report — the
lesson written into BREAKING.md after the 1.8.2 mis-versioning.

MINOR is the right component under the Default-behaviour changes tier: the
previous behaviour stays available (HookConfig.Strict for the newly-activated
pre-checkpoint gate), every new failure names its gate and its opt-out at the
point of failure, nothing changes silently, and the commits carry the
Conventional Commits `!` marker. Nothing was removed from the CLI surface.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@teragrid
teragrid merged commit 9e61ed7 into main Aug 7, 2026
26 checks passed
@teragrid
teragrid deleted the refactor/gate-verdict-three-states branch August 7, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants