Skip to content

feat: gate the release on a structural surface diff#127

Open
REPPL wants to merge 3 commits into
feat/itd-73-p1-impact-derivationfrom
feat/itd-73-p2-surface-guardrail
Open

feat: gate the release on a structural surface diff#127
REPPL wants to merge 3 commits into
feat/itd-73-p1-impact-derivationfrom
feat/itd-73-p2-surface-guardrail

Conversation

@REPPL

@REPPL REPPL commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Phase 2 of the derived-versioning + auto-changelog + plugin-distribution programme
(plan: .abcd/development/plans/2026-07-21-derived-version-and-changelog.md; spec: spc-10).

Third of five stacked PRs. Based on feat/itd-73-p1-impact-derivation (#125) — GitHub
retargets it to main as the stack merges.

A mislabelled release can no longer ship a compatibility lie. A removed or renamed command or
flag, a newly-required flag, or a dropped manifest entry fails the cut unless the cut
declares a breaking intent — and the failure names the surface that changed. Additions,
help-text edits and reordering are explicitly not breaks.

Why a new emitter rather than reusing GenerateReference

Plan outcome 5, and it is not a preference. GenerateReference emits Markdown, returns
early on cmd.Hidden
— so the operator-internal hook subtree is invisible to it — and
carries neither per-flag required-ness nor the manifest surface. Reusing it would have baked
those blind spots into a compatibility gate. What is reused is the shared NewRootCommand()
tree.

The difference is measurable in the committed snapshot: 51 commands, including all 5 hidden
hook entries
, plus each flag's type and required-ness and 16 declared manifest keys. The
generated docs page mentions hook once, incidentally.

Types and the diff live in internal/core/surface (cobra-free, per the transport-agnostic
core boundary); the walk lives in internal/surface/cli. The generator and the drift test
share one render path so they cannot disagree — the same shape cmd/abcd-gen-cli-ref and
reference_test.go already use.

The trap this phase had to avoid

The committed surface.json is held equal to the working tree by its own drift test. So a
guardrail comparing "the committed baseline" against "a fresh walk" would compare a thing to
itself, never fire, and be a green light wearing a guardrail's clothes.

The baseline is the snapshot as of the last release tag, read out of git. A tag carrying
no snapshot refuses rather than passing — v0.3.0 predates this file, so cuts refuse until
a release contains the baseline. That is the deliberate fail-closed direction: the first cut
is the highest-risk one and must not sail through unguarded. The clean-cutover manual roll is
what puts the baseline into a tag, and the refusal message says so.

STOP condition — not hit

Phase 2's STOP is a non-deterministic surface walk. Determinism is proven at three levels: 50
in-process renders byte-identical, 10 fresh generator processes yielding a single sha256, and
go generate twice leaving no diff.

Worth noting why it holds: cobra already sorts subcommands, so the live walk is stable even
with our sort removed — a mutation check confirmed the live drift test still passed without
it, and only the core unit test caught the regression. The sort in canonical() is therefore
the guarantee that does not depend on a cobra default. The manifest's array-position ordering
defect was fixed inside the extractor, not patched downstream.

Each detector was mutation-checked: hiding the hook subtree, hardwiring required-ness to
false, removing the sort, and deleting a manifest key each make a named test fail.

What the two independent reviews found

The ruthless reviewer returned FIX_FIRST; the security reviewer returned SHIP. All fixed:

  1. FIX_FIRST — a breaking record on the REMOVED side satisfied "was this break
    declared?"
    A superseded intent leaving shipped/ carries whatever label the last
    release gave it, read from the anchor tag's immutable tree — so it could silently authorise
    an undeclared removal in this cut. Now RecordSet.DeclaresBreak() judges the added
    side only. Impact() deliberately still spans both, because the version bump must account
    for every record in the cut; the split is documented on both.
  2. Both reviewers independently flagged the same hole: the "current" surface was walked
    from the cobra tree compiled into the running binary, so a stale binary yields a false
    PASS. The guard now reads the committed snapshot at HEAD and refuses unless it encodes
    byte-equal to the walked surface, naming both remedies.
  3. Decode accepted trailing garbage after the snapshot while its comment claimed strictness.
  4. A stray untracked scratch test was removed.

Behavioural change phase 3 inherits: finding 2 adds a fourth refusal — a run against a
tree whose regenerated surface.json is not yet committed now refuses instead of passing.
Intended fail-closed, consistent with the no-baseline refusal.

Verification

gofmt -l . silent · go vet ./... · go build ./... · go test ./... · go test -race on
surface/changelog/cli/fsutil · go run ./cmd/record-lint exit 0 (pre-existing WARNs only) ·
regeneration idempotent.

CHANGELOG.md, go.mod, go.sum, .claude-plugin/** and both .github/workflows/*.yml are
untouched.

Incidental

  • cmd/abcd-gen-cli-ref and cmd/abcd-gen-surface were each walking up to go.mod
    separately; consolidated into a shared fsutil.ModuleRoot. Two walks diverging would send
    the two drift-checked artefacts to different directories.
  • iss-111 is captured on this branch: phase 1 armed two impact blockers, but neither write
    path (abcd capture resolve, seedDraft) can set the field. Captured, not fixed — the flag
    surface is a design call.

REPPL added 3 commits July 21, 2026 18:09
Phase 1 added two blockers requiring an explicit `impact` before a record can
move to shipped/ or resolved/, but neither write path can set the field:
`abcd capture resolve` writes only the resolution, and `seedDraft` on the intent
side emits no impact key. The gates are armed against a value the tool cannot
produce, so the field is hand-edited until this is closed.

Captured rather than fixed here — the flag surface is a design call, not an
integration fix, and phase 2 is the surface guardrail.

Assisted-by: Claude:claude-opus-4-8
Phase 2 of the derived-versioning programme (itd-73 / spc-10). A mislabelled
release can no longer ship a compatibility lie: a removed or renamed command or
flag, a newly-required flag, or a dropped manifest entry fails the cut unless the
cut declares a `breaking` intent, and the failure names the surface that changed.

THE SNAPSHOT
A new structured JSON emitter over the shared NewRootCommand() tree, not over
GenerateReference's Markdown walker. That walker returns early on cmd.Hidden, so
the operator-internal `hook` subtree is invisible to it, and it carries neither
per-flag required-ness nor the manifest surface — reusing it would have baked
those blind spots into a compatibility gate. What is reused is the root command
itself. The snapshot captures all 51 commands including the 5 hidden `hook`
entries, each flag's type and required-ness, and 16 declared manifest keys.

Types and diff live in internal/core/surface (cobra-free); the walk lives in
internal/surface/cli. The generator and the drift test share one render path, so
they cannot disagree — the same shape cmd/abcd-gen-cli-ref and reference_test.go
already use for the CLI reference page.

WHAT THE BASELINE IS
The committed surface.json is held equal to the working tree by its own drift
test, so comparing the committed file against a fresh walk would compare a thing
to itself and never fire. The baseline is the snapshot AS OF THE LAST RELEASE TAG,
read via git. A tag carrying no snapshot refuses rather than passing: v0.3.0
predates this file, so cuts refuse until a release contains the baseline, which is
the deliberate fail-closed direction for the first and highest-risk cut.

DETERMINISM
Proven at three levels: 50 in-process renders byte-identical, 10 fresh generator
processes producing one sha256, and a second `go generate` leaving no diff. Cobra
already sorts subcommands, so the live walk is stable even without our sort — the
sort in canonical() is the guarantee that does not depend on that default, and the
manifest's array-position ordering defect was fixed inside the extractor rather
than patched downstream. Each detector was mutation-checked: hiding the hook
subtree, hardwiring required-ness false, removing the sort, and deleting a
manifest key each make a named test fail.

Assisted-by: Claude:claude-opus-4-8
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.

1 participant