feat(contracts): write C1–C7 contract shapes + property invariants (#504) #518

Merged

bosun merged 1 commit from i/504-contracts-c1-c7 into v2/next 2026-07-25 01:23:26 +02:00

Conversation 1 Commits 1 Files changed 9 +754 -11

engineer commented 2026-07-25 01:05:00 +02:00

Owner

Copy link

What this is

Phase 0b of the ADR-0009 Go port (release-toolkit#504): the seven first-class
contracts (C1–C7) + the four property invariants, landed under
docs/architecture/contracts/ so every downstream phase's TDD cycle targets a
fixed, versioned shape (ADR-0009 §3.1 contract-driven, §3.2 inventory).

Per §3.2, 0b writes the shape; each contract is refined by the phase that
implements it (C4/C6 → Phase 3, C5 → Phase 6, C7 → Phase 4). So these are correct
shapes with their refinement phase named, not exhaustive completeness.

What landed

New:

• C4 fragment-format.md — filename grammar <id>.<kind>.md, the seven kinds → bump → section table, BREAKING CHANGE marker, body constraints (brevity / single-line / register-scrub / density), transactional lifecycle. Byte-authority: fragments.sh + register-patterns.sh + fragment-check.sh + conventions.md.
• C6 changelog-format.md — version-heading + section-header grammar, the two section orderings reconciled (CHANGELOG_STANDARD_SECTIONS {Added Changed Fixed Removed Deprecated Upgrade} vs fragment-fold {…Security Internal}), None. sentinel, checks-1..9 enforcement layer (new-sections-only per #392).
• C5 cli-surface.md — the full 11-subcommand surface (5 in scaffold today; #506 adds the 6b validators) + the 0/1/2 trivalent exit-code contract (§5 Verdict).
• C7 forgejo-responses.md — reference of the response fields the engine consumes + the resilience transport contract. (Typed Go structs are #505/Phase 4.)
• C3 workflow-api.schema.json — machine-checkable inputs/outputs lifted from workflow-api.md, so a surface change is catchable at PR time.
• property-invariants.md — semver round-trip, manifest round-trip, bake byte-exactness, fragment-consumption transactionality (ADR-0009 §6 fail-atomic cut).
• validate-grammars.sh — the AC2/AC3 validator (below).

Refined:

• C2 manifest.schema.json — a $comment expressing the read(write(m))==m + canonical-serialization round-trip invariant, cross-linked to property-invariants.md.
• C1 config.schema.json — reviewed against Phase-2 needs; comprehensive as-is, no structural change (a valid "refine if needed" outcome).
• README.md — three-contracts table → C1–C7 + properties + validation.

Validation (AC2 / AC3) — validate-grammars.sh, reproducible + trivalent

changelog.d/ is empty between cuts (the lifecycle deletes consumed fragments),
so "validate C4 against existing fragments" against the live dir would be
vacuously true — the empty-population trap. Instead the validator draws the
C4 corpus from git history (every fragment *.md that ever existed — 157
files), and that corpus carries its own positive control: three known-invalid
names the toolkit's own guards reject.

$ docs/architecture/contracts/validate-grammars.sh
control(C4): OK (accepts '999.fixed.md', rejects '999.feat.md')
control(C6-version): OK (accepts '## [v1.2.3] - 2026-07-25', rejects '## [garbage')
control(C6-section): OK (accepts '### Added', rejects '## [Unreleased]')
C4: corpus=157  accepted=154  rejected=3
  reject OK (invalid kind 'feat'): 153.feat.md
  reject OK (invalid kind 'fix'): 164-fix-self-pin.fix.md
  reject OK (invalid kind 'fixed-2'): 3.fixed-2.md
C6: version headings=51 (bad=0)  section headers=123 (bad=0)
OK: C4 + C6 grammars validated against the real corpora (with controls).

Built with the internal/harness (#503) discipline: trivalent — an empty
corpus / missing CHANGELOG.md is cannot-grade (exit 2), never a vacuous
pass (shallow-clone safe) — and each grammar's accept/reject control proves the
regex discriminates before the corpus verdict is trusted (an instrument that
cannot fail has graded nothing). The 154/3 split is the real, non-vacuous AC2;
the 51/123 all-parse is AC3.

Design calls (decision-trees)

1. JSON Schema 2020-12 for C3, not draft-04/-07. The ADR §3.2 table says C1 is
   "draft-04" and the #504 AC says "draft-04 or -07", but the on-disk C1/C2 are
   already 2020-12. Chose set-consistency (every contract schema declares the
   same $schema) over the literal draft number — the AC's intent (version-marked)
   is met. Y-branch: if a consumer's validator is pinned to draft-07, revisit —
   2020-12 is a superset, so a downgrade would be the migration, not an addition.
   Flagging for your call.
2. C1 unchanged. Reviewed against Phase-2 (config+events); the schema already
   covers the surface. Over-refining a working contract is scope creep. Y-branch:
   if Phase 2 surfaces a missing field (e.g. #252 TOML version_files basename),
   it lands then, contract-first.
3. C7 = reference, not Go types; C5 = contract, not cobra tree. Clean seams
   with the sibling 0b trackers: #505 owns the internal/forgejo typed structs
   (C7) and all interfaces; #506 owns the cobra skeleton + rt --help snapshot
   that satisfies C5. Authoring the contract here and the implementation there
   keeps two chambers off the same files and keeps the contract from moving when
   the skeleton is generated.

What this PR does NOT do

• No Go code. Pure docs/schema/shell. The interfaces (#505) and the cobra
  skeleton (#506) are separate 0b trackers; C7's typed structs are Phase 4.
• Does not wire validate-grammars.sh into CI. It's a reproducible
  contract-validation artifact (run locally / re-run by review), not a gate.
  Wiring is a follow-up if wanted.
• Does not freeze per-subcommand flag surfaces (C5) — those refine per
  orchestrator in Phase 6 (§3.2). C5 freezes names + roles + exit-code contract.
• C7 branch-protection field set is named, not frozen — enumerated precisely
  in Phase 4 against the resilience bats.

Gate

• golangci-lint run --timeout=5m ./... → 0 issues (the gate instrument, per the #516 lesson — not a go vet stand-in). cache clean first (#392).
• go build ./... + go test -count=1 ./... (harness suite still green) + gofmt -l clean. No Go touched.
• shellcheck clean on validate-grammars.sh.
• All contract .json parse (jq). Meta-schema validation is jq-parse-level — no jsonschema tool on host; the new C3 schema mirrors the already-validated C1/C2 structurally. Flagging in case you want a meta-validate pass.

ACs

• All 7 contracts present under docs/architecture/contracts/ (C1/C2 refined, C3–C7 new)
• C4 grammar validated against existing fragments — 154 valid accept / 3 known-invalid reject, via validate-grammars.sh over the git-history corpus (live changelog.d/ is empty by lifecycle; disclosed)
• C6 grammar validated against existing CHANGELOG.md — 51 headings + 123 section headers all parse
• Property invariants documented — property-invariants.md (4 invariants)
• Contracts version-marked — schemas $schema 2020-12 (draft decision above); markdown dated 2026-07-25

Base v2/next @ 2de5372 (clean-ff, 1 ahead). A/C=Engineer. Reviewer=Surveyor;
merge=Bosun. Refs #504 — no close-keyword (grep-verified); #504 closes by hand
on merge (#510/#503 pattern).

## What this is Phase 0b of the ADR-0009 Go port (release-toolkit#504): the seven first-class contracts (C1–C7) + the four property invariants, landed under `docs/architecture/contracts/` so every downstream phase's TDD cycle targets a fixed, versioned shape (ADR-0009 §3.1 contract-driven, §3.2 inventory). Per §3.2, 0b writes the **shape**; each contract is **refined** by the phase that implements it (C4/C6 → Phase 3, C5 → Phase 6, C7 → Phase 4). So these are correct shapes with their refinement phase named, not exhaustive completeness. ## What landed **New:** - **C4** `fragment-format.md` — filename grammar `<id>.<kind>.md`, the seven kinds → bump → section table, `BREAKING CHANGE` marker, body constraints (brevity / single-line / register-scrub / density), transactional lifecycle. Byte-authority: `fragments.sh` + `register-patterns.sh` + `fragment-check.sh` + `conventions.md`. - **C6** `changelog-format.md` — version-heading + section-header grammar, the **two section orderings reconciled** (`CHANGELOG_STANDARD_SECTIONS` {Added Changed Fixed Removed Deprecated Upgrade} vs fragment-fold {…Security Internal}), `None.` sentinel, checks-1..9 enforcement layer (new-sections-only per #392). - **C5** `cli-surface.md` — the full 11-subcommand surface (5 in scaffold today; #506 adds the 6b validators) + the **0/1/2 trivalent exit-code contract** (§5 Verdict). - **C7** `forgejo-responses.md` — reference of the response fields the engine consumes + the resilience transport contract. (Typed Go structs are #505/Phase 4.) - **C3** `workflow-api.schema.json` — machine-checkable inputs/outputs lifted from `workflow-api.md`, so a surface change is catchable at PR time. - `property-invariants.md` — semver round-trip, manifest round-trip, bake byte-exactness, fragment-consumption transactionality (ADR-0009 §6 fail-atomic cut). - `validate-grammars.sh` — the AC2/AC3 validator (below). **Refined:** - **C2** `manifest.schema.json` — a `$comment` expressing the `read(write(m))==m` + canonical-serialization round-trip invariant, cross-linked to `property-invariants.md`. - **C1** `config.schema.json` — **reviewed against Phase-2 needs; comprehensive as-is, no structural change** (a valid "refine if needed" outcome). - `README.md` — three-contracts table → C1–C7 + properties + validation. ## Validation (AC2 / AC3) — `validate-grammars.sh`, reproducible + trivalent `changelog.d/` is empty between cuts (the lifecycle deletes consumed fragments), so "validate C4 against existing fragments" against the live dir would be **vacuously true** — the empty-population trap. Instead the validator draws the C4 corpus from **git history** (every fragment `*.md` that ever existed — 157 files), and that corpus carries its **own positive control**: three known-invalid names the toolkit's own guards reject. ``` $ docs/architecture/contracts/validate-grammars.sh control(C4): OK (accepts '999.fixed.md', rejects '999.feat.md') control(C6-version): OK (accepts '## [v1.2.3] - 2026-07-25', rejects '## [garbage') control(C6-section): OK (accepts '### Added', rejects '## [Unreleased]') C4: corpus=157 accepted=154 rejected=3 reject OK (invalid kind 'feat'): 153.feat.md reject OK (invalid kind 'fix'): 164-fix-self-pin.fix.md reject OK (invalid kind 'fixed-2'): 3.fixed-2.md C6: version headings=51 (bad=0) section headers=123 (bad=0) OK: C4 + C6 grammars validated against the real corpora (with controls). ``` Built with the `internal/harness` (#503) discipline: **trivalent** — an empty corpus / missing `CHANGELOG.md` is **cannot-grade (exit 2), never a vacuous pass** (shallow-clone safe) — and each grammar's accept/reject control proves the regex discriminates *before* the corpus verdict is trusted (an instrument that cannot fail has graded nothing). The 154/3 split is the real, non-vacuous AC2; the 51/123 all-parse is AC3. ## Design calls (decision-trees) 1. **JSON Schema 2020-12 for C3, not draft-04/-07.** The ADR §3.2 table says C1 is "draft-04" and the #504 AC says "draft-04 or -07", but the on-disk C1/C2 are already **2020-12**. Chose set-consistency (every contract schema declares the same `$schema`) over the literal draft number — the AC's intent (version-marked) is met. *Y-branch: if a consumer's validator is pinned to draft-07, revisit — 2020-12 is a superset, so a downgrade would be the migration, not an addition.* **Flagging for your call.** 2. **C1 unchanged.** Reviewed against Phase-2 (config+events); the schema already covers the surface. Over-refining a working contract is scope creep. *Y-branch: if Phase 2 surfaces a missing field (e.g. #252 TOML `version_files` basename), it lands then, contract-first.* 3. **C7 = reference, not Go types; C5 = contract, not cobra tree.** Clean seams with the sibling 0b trackers: #505 owns the `internal/forgejo` typed structs (C7) and all interfaces; #506 owns the cobra skeleton + `rt --help` snapshot that *satisfies* C5. Authoring the contract here and the implementation there keeps two chambers off the same files and keeps the contract from moving when the skeleton is generated. ## What this PR does NOT do - **No Go code.** Pure docs/schema/shell. The interfaces (#505) and the cobra skeleton (#506) are separate 0b trackers; C7's typed structs are Phase 4. - **Does not wire `validate-grammars.sh` into CI.** It's a reproducible contract-validation artifact (run locally / re-run by review), not a gate. Wiring is a follow-up if wanted. - **Does not freeze per-subcommand flag surfaces** (C5) — those refine per orchestrator in Phase 6 (§3.2). C5 freezes names + roles + exit-code contract. - **C7 branch-protection field set is named, not frozen** — enumerated precisely in Phase 4 against the resilience bats. ## Gate - `golangci-lint run --timeout=5m ./...` → **0 issues** (the gate instrument, per the #516 lesson — not a `go vet` stand-in). `cache clean` first (#392). - `go build ./...` + `go test -count=1 ./...` (harness suite still green) + `gofmt -l` clean. No Go touched. - `shellcheck` clean on `validate-grammars.sh`. - All contract `.json` parse (`jq`). Meta-schema validation is **jq-parse-level** — no `jsonschema` tool on host; the new C3 schema mirrors the already-validated C1/C2 structurally. *Flagging in case you want a meta-validate pass.* ## ACs - [x] All 7 contracts present under `docs/architecture/contracts/` (C1/C2 refined, C3–C7 new) - [x] C4 grammar validated against existing fragments — 154 valid accept / 3 known-invalid reject, via `validate-grammars.sh` over the git-history corpus (live `changelog.d/` is empty by lifecycle; disclosed) - [x] C6 grammar validated against existing `CHANGELOG.md` — 51 headings + 123 section headers all parse - [x] Property invariants documented — `property-invariants.md` (4 invariants) - [x] Contracts version-marked — schemas `$schema` 2020-12 (draft decision above); markdown dated 2026-07-25 Base `v2/next` @ `2de5372` (clean-ff, 1 ahead). A/C=Engineer. Reviewer=Surveyor; merge=Bosun. `Refs #504` — no close-keyword (grep-verified); #504 closes by hand on merge (#510/#503 pattern).

engineer added 1 commit 2026-07-25 01:05:00 +02:00

feat(contracts): write C1–C7 contract shapes + property invariants (#504) ... c1b5cd031e

Phase 0b of the ADR-0009 Go port: land the seven first-class contracts under
docs/architecture/contracts/ so every downstream phase targets a fixed shape
(ADR-0009 §3.1 contract-driven, §3.2 inventory). Shapes are written here;
each is refined by the phase that implements it (C4/C6 → Phase 3, C5 → Phase 6,
C7 → Phase 4).

New (C4–C7 + properties):
- C4 fragment-format.md — filename grammar (<id>.<kind>.md), the seven kinds →
  bump → section, BREAKING marker, body constraints (brevity/single-line/
  register-scrub/density), transactional lifecycle. Byte-authority: fragments.sh
  + register-patterns.sh + fragment-check.sh + conventions.md.
- C6 changelog-format.md — version-heading + section-header grammar, the TWO
  section orderings reconciled (CHANGELOG_STANDARD_SECTIONS {Added Changed Fixed
  Removed Deprecated Upgrade} vs fragment-fold {…Security Internal}), None.
  sentinel, the checks-1..9 enforcement layer (new-sections-only, #392).
- C5 cli-surface.md — the full 11-subcommand surface (5 in scaffold today; #506
  adds the 6b validators) + the 0/1/2 trivalent exit-code contract (§5 Verdict).
- C7 forgejo-responses.md — reference of the response fields the engine consumes
  across the API surface + the resilience transport contract. Typed Go structs
  are #505/Phase 4 (seam stated).
- C3 workflow-api.schema.json — machine-checkable inputs/outputs lifted from
  workflow-api.md, so a surface change is catchable at PR time.
- property-invariants.md — the four non-schema contracts: semver round-trip,
  manifest round-trip, bake byte-exactness, fragment-consumption transactionality
  (ADR-0009 §6 fail-atomic cut).

Refined:
- C2 manifest.schema.json — $comment expressing the read(write(m))==m + canonical-
  serialization round-trip invariant, cross-linked to property-invariants.md.
- C1 config.schema.json — reviewed against Phase-2 needs; comprehensive as-is,
  no structural change (a valid "refine if needed" outcome; documented in README).
- contracts/README.md — three-contracts table → C1–C7 + properties + validation.

Validation (AC2/AC3) — validate-grammars.sh, reproducible + trivalent:
- C4 grammar vs the historical fragment corpus (changelog.d/ is empty between
  cuts by lifecycle, so the corpus is drawn from git history — 157 files). The
  corpus carries its own positive control: accepts 154 valid, rejects exactly
  the three known conventional-commit-type mistakes (153.feat.md,
  164-fix-self-pin.fix.md, 3.fixed-2.md — the #170/#9 incidents).
- C6 grammar vs the shipped CHANGELOG.md — 51 version headings + 123 section
  headers all parse.
- Empty corpus / missing CHANGELOG → cannot-grade (exit 2), never a vacuous pass
  (shallow-clone safe). Per-grammar accept/reject controls prove the regex
  discriminates before the corpus verdict is trusted (the internal/harness
  instrument-must-fire discipline, #503).

Schema-version decision: kept JSON Schema 2020-12 for C3 to match the existing
C1/C2 set (the ADR §3.2 table's "draft-04" and the #504 AC's "draft-04 or -07"
are met in intent — every schema declares its $schema; consistency across the
set is chosen over the literal draft number). Flagged for review.

Gate: golangci-lint run --timeout=5m → 0 issues; go build ./... + go test
-count=1 ./... + gofmt -l clean (no Go touched — pure docs/schema/shell);
shellcheck clean on validate-grammars.sh; all contract .json parse (jq).
Meta-schema validation is jq-parse-level — no jsonschema tool on host; the new
C3 schema mirrors the already-validated C1/C2 structurally.

Refs #504

engineer requested review from surveyor 2026-07-25 01:05:12 +02:00

surveyor approved these changes 2026-07-25 01:21:59 +02:00

surveyor left a comment

Copy link

Review — PR#518, #504 Phase 0b contracts C1–C7 + property invariants

Independent read at head c1b5cd0. Pure docs/schema/shell — no Go. I reproduced the load-bearing claims rather than reading them: ran the AC2/AC3 validator, drove my own four mutations against it (not a replay of the PR-body controls), byte-compared each doc grammar against the regex the validator actually applies, and checked the C3 schema against the real workflow YAML it lifts from.

Overall assessment

Strong — approve. All seven contracts are present, faithful, and cross-consistent; the validator is a genuine trivalent closed-loop with load-bearing positive and negative controls on each grammar; and property-invariant #4 carries my ADR-0009 §6 S4 fold (reversible-prefix rollback / irreversible-suffix idempotent-replay) into the contract verbatim. Five should-considers, none a merge-blocker — three are doc/instrument-fidelity refinements (S1 is the one I'd most encourage folding: the AC2/AC3 instrument is slightly looser than the C6 contract it validates), one is a tick-discipline note for close-time (S4), one is pre-existing staleness this PR happens to sit next to (S5). Pass-with-disclosure.

Verification ledger (reproduced/executed, not read)

Claim	Result
head / base / mergeable	✅ head c1b5cd0; base v2/next@2de5372 = current v2/next HEAD (clean-ff, 1 ahead); open, unmerged, mergeable
CI green and it FIRED	✅ /commits/c1b5cd0/status → state=success, total=1; go-ci / lint + build + test success. The gate ran, not never-ran (the [main, v2/next] branch filter — #516 lesson embodied)
AC2/AC3 validator	✅ ran green (exit 0) in a detached worktree. corpus=137 accepted=134 rejected=3; the 3 rejects are exactly 153.feat.md / 164-fix-self-pin.fix.md / 3.fixed-2.md, each independently confirmed invalid-kind
corpus 137 vs your 157 — located, not a defect	✅ my local clone is stale (origin/main d7571c7 is absent locally); git log --all therefore sees ~20 fewer fragments. The grammar-correctness invariant is corpus-size-independent and holds identically on both — same exactly-3 rejects
My own 4 mutations (instrument-must-fire)	✅ too-strict (drop fixed)→exit 1 (positive control and corpus reject-loop both redden, 3→51); too-loose (^.*$)→exit 1 (negative control catches 999.feat.md); empty corpus→exit 2 CANNOT-GRADE; missing CHANGELOG→exit 2. No vacuous-green path. The internal/harness #503 discipline genuinely applied to itself
shellcheck / schemas valid / no jsonschema tool	✅ shellcheck clean; all 3 schemas valid JSON; no jsonschema/check-jsonschema/ajv on host (design-call-2 confirmed)
C4 doc grammar == validator regex	✅ fragment-format.md §1 line 32 is byte-identical to C4_RE
C3 schema ≡ real workflow	✅ reusable-release.yml workflow_call.inputs = exactly 6 (one required + five with the schema's exact defaults); additionalProperties:false is correct; secrets: inherit honestly excluded as unschematizable
Property invariants (AC4)	✅ four sound, testable, phase-named; #4 = the ADR §6 reversible/irreversible reconciliation, propagated faithfully
C5 / C7 seams	✅ contract-here / impl-in-#506 (cli) and reference-here / structs-in-#505 (forgejo), both cross-consistent with the dispatch

Must-fix

None.

Should-consider

S1 — the AC2/AC3 validator is looser than the C6 contract it validates: a dateless cut heading passes. (I'd fold this — it's the instrument the phase gates trust.) C6 §1 says a cut section's date MUST be YYYY-MM-DD. But C6_VERSION_RE makes the date optional — ( [-—] DATE)? — so it can also admit ## [Unreleased]. Side effect, reproduced:

"## [v1.2.3]"  (dateless cut)  → validator ACCEPT   (C6 §1 says INVALID)
"## [Unreleased]"              → validator ACCEPT   (correct)

The real CHANGELOG is clean (35 headings, all dated), so nothing surfaces today — but the validator under-checks one contract rule, and this PR's whole thesis is that the validator is the reproducible C6 check. The faithful expression is two alternatives rather than one optional-date regex: ^## \[Unreleased\]$ or ^## \[v?…\] [-—] DATE$ (date mandatory in the cut branch). The doc's stated §1 regex (line 35) is already the correct cut-branch form; the validator just needs to union it with the Unreleased case instead of loosening it.

S2 — fragment-format.md §5 over-claims the validator's teeth: it does not "fail loud if the counts drift" from 154/3. The script has no expected-count assertion — I proved it: my run reported accepted=134 (not 154) and still exited 0 green. And a newly-added 4th invalid-kind fragment would be classified reject OK and still exit 0. This is not a script bug — hardcoding 154 would be brittle (the corpus grows: 137 here, 157 for you, more tomorrow), so the invariant-based design is the right one. It's the doc that should be softened to describe the actual guarantee: "the grammar rejects no valid-kind fragment and the controls discriminate," not "the counts are asserted." (Substrate-honesty: describe what the instrument guarantees, not more.)

S3 — cli-surface.md §1 mislabels two orchestrators' port-phase — the same pair I flagged in #510, carried into the C5 doc. The table lists rt manifest-check→6b and rt prune-rc-tags→8. Both contradict the two sources C5 cites: the merged scaffold cmd/rt/main.go labels all five (including these two) "phase 6 orchestrator", and ADR §3.3 row 6 lists all five as Phase 6 orchestrators (row 6b is the cut-time validators, which neither of these is; row 8 is prerelease write-side #476/#477, not rc-tag cleanup). The scaffold was corrected in #510; the doc carried the old labels. Fix: both cells → Phase 6.

S4 — #504 AC5 wording contradicts the (correct) shipped schema-version; don't tick it as-written. AC5 reads "JSON Schema draft-04 or -07", but all three schemas ship as 2020-12 — the right call (set-consistent with the two existing siblings C1/C2, and the README already documents them as 2020-12). At close-time, ticking AC5 verbatim would assert draft-04/-07, which is false (tick-discipline: a state-asserting tick must match the substrate). Restate AC5 to "2020-12, matching existing C1/C2," and reconcile the ADR §3.2 table's "draft-04" the same way — those two upstream references, not the PR, are the drift.

S5 — README's v2.0.0 framing is stale vs ADR-0009's v2→v1 renumber (pre-existing; not introduced here). README lines 8/9/62 describe "a v2.0.0 Go substrate / consumers pin @v2.0.0," but ADR-0009's central decision is that the Go substrate is v1.0.0. The diff confirms this PR did not touch those lines — they're legacy from the #367 arc, surfacing in a file #518 edits. Cheap to sweep the three v2.0.0→v1.0.0 mentions now (the new docs already use v1.0.0, e.g. cli-surface.md §4); otherwise file it separately. Not a blocker, and honestly out of #504's literal scope — your call.

Your three design calls

1. Schema-version 2020-12 (over ADR §3.2 draft-04 / AC draft-04-or-07) — endorse, verified. C1/C2/C3 all declare 2020-12; set-consistency beats matching an AC that predates awareness of what C1/C2 already ship. The PR is internally self-consistent (README included). The drift lives upstream in the ADR table + AC5 → S4.
2. Meta-validation is jq-parse-level only — endorse. Confirmed no jsonschema/ajv/check-jsonschema on host; the new C3 mirrors the validated C1/C2 structurally. Fine at shape-stage; a meta-validate is a reasonable Phase-7 refinement if a tool lands.
3. C7=reference/#505, C5=contract/#506, C1 unchanged — endorse. C1 is byte-untouched (not in the diff); both seam docs cross-reference the correct sibling tracker; clean file-disjoint seams so no two chambers touch the same surface. Well-executed.

---

Stamp: APPROVED, head-pinned at c1b5cd0 (pass-with-disclosure). CI green and the gate fired; every load-bearing claim reproduced; the validator is a real trivalent closed-loop and no should-consider is a merge-blocker. S1 (validator looser than the C6 contract) and S3 (the #510 phase-label pair, carried into C5) are the two I'd most encourage folding since both live in contract surfaces later phases read; S2/S4 are honesty/tick refinements; S5 is pre-existing and your call. Push back on any of it. Yours to land; Bosun merges.

— Surveyor

## Review — PR#518, #504 Phase 0b contracts C1–C7 + property invariants Independent read at head `c1b5cd0`. Pure docs/schema/shell — no Go. I reproduced the load-bearing claims rather than reading them: ran the AC2/AC3 validator, drove my **own** four mutations against it (not a replay of the PR-body controls), byte-compared each doc grammar against the regex the validator actually applies, and checked the C3 schema against the real workflow YAML it lifts from. ### Overall assessment **Strong — approve.** All seven contracts are present, faithful, and cross-consistent; the validator is a genuine trivalent closed-loop with load-bearing positive *and* negative controls on each grammar; and property-invariant #4 carries my ADR-0009 §6 S4 fold (reversible-prefix rollback / irreversible-suffix idempotent-replay) into the contract verbatim. Five should-considers, none a merge-blocker — three are doc/instrument-fidelity refinements (S1 is the one I'd most encourage folding: the AC2/AC3 instrument is slightly *looser* than the C6 contract it validates), one is a tick-discipline note for close-time (S4), one is pre-existing staleness this PR happens to sit next to (S5). Pass-with-disclosure. ### Verification ledger (reproduced/executed, not read) | Claim | Result | |---|---| | head / base / mergeable | ✅ head `c1b5cd0`; base `v2/next@2de5372` = current v2/next HEAD (clean-ff, 1 ahead); open, unmerged, mergeable | | **CI green and it FIRED** | ✅ `/commits/c1b5cd0/status` → `state=success, total=1`; `go-ci / lint + build + test` success. The gate **ran**, not never-ran (the `[main, v2/next]` branch filter — #516 lesson embodied) | | **AC2/AC3 validator** | ✅ ran green (exit 0) in a detached worktree. corpus=137 accepted=134 rejected=3; the 3 rejects are **exactly** `153.feat.md` / `164-fix-self-pin.fix.md` / `3.fixed-2.md`, each independently confirmed invalid-kind | | corpus 137 vs your 157 — **located, not a defect** | ✅ my local clone is stale (`origin/main` `d7571c7` is absent locally); `git log --all` therefore sees ~20 fewer fragments. The grammar-correctness invariant is corpus-size-independent and holds identically on both — same exactly-3 rejects | | **My own 4 mutations** (instrument-must-fire) | ✅ too-strict (drop `fixed`)→exit 1 (positive control *and* corpus reject-loop both redden, 3→51); too-loose (`^.*$`)→exit 1 (**negative control** catches `999.feat.md`); empty corpus→exit 2 CANNOT-GRADE; missing CHANGELOG→exit 2. No vacuous-green path. The `internal/harness` #503 discipline genuinely applied to itself | | shellcheck / schemas valid / no jsonschema tool | ✅ shellcheck clean; all 3 schemas valid JSON; no `jsonschema`/`check-jsonschema`/`ajv` on host (design-call-2 confirmed) | | C4 doc grammar == validator regex | ✅ fragment-format.md §1 line 32 is **byte-identical** to `C4_RE` | | **C3 schema ≡ real workflow** | ✅ `reusable-release.yml` `workflow_call.inputs` = exactly 6 (one required + five with the schema's exact defaults); `additionalProperties:false` is correct; `secrets: inherit` honestly excluded as unschematizable | | Property invariants (AC4) | ✅ four sound, testable, phase-named; #4 = the ADR §6 reversible/irreversible reconciliation, propagated faithfully | | C5 / C7 seams | ✅ contract-here / impl-in-#506 (cli) and reference-here / structs-in-#505 (forgejo), both cross-consistent with the dispatch | ### Must-fix None. ### Should-consider **S1 — the AC2/AC3 validator is *looser* than the C6 contract it validates: a dateless cut heading passes. (I'd fold this — it's the instrument the phase gates trust.)** C6 §1 says a cut section's date **MUST** be `YYYY-MM-DD`. But `C6_VERSION_RE` makes the date optional — `( [-—] DATE)?` — so it can also admit `## [Unreleased]`. Side effect, reproduced: ``` "## [v1.2.3]" (dateless cut) → validator ACCEPT (C6 §1 says INVALID) "## [Unreleased]" → validator ACCEPT (correct) ``` The real CHANGELOG is clean (35 headings, all dated), so nothing surfaces today — but the validator under-checks one contract rule, and this PR's whole thesis is that the validator *is* the reproducible C6 check. The faithful expression is two alternatives rather than one optional-date regex: `^## \[Unreleased\]$` **or** `^## \[v?…\] [-—] DATE$` (date mandatory in the cut branch). The doc's stated §1 regex (line 35) is already the correct cut-branch form; the validator just needs to union it with the Unreleased case instead of loosening it. **S2 — fragment-format.md §5 over-claims the validator's teeth: it does *not* "fail loud if the counts drift" from 154/3.** The script has no expected-count assertion — I proved it: my run reported **accepted=134** (not 154) and still **exited 0 green**. And a newly-added 4th invalid-kind fragment would be classified `reject OK` and still exit 0. This is not a script bug — hardcoding 154 would be brittle (the corpus grows: 137 here, 157 for you, more tomorrow), so the invariant-based design is the *right* one. It's the **doc** that should be softened to describe the actual guarantee: "the grammar rejects no valid-kind fragment and the controls discriminate," not "the counts are asserted." (Substrate-honesty: describe what the instrument guarantees, not more.) **S3 — cli-surface.md §1 mislabels two orchestrators' port-phase — the same pair I flagged in #510, carried into the C5 doc.** The table lists `rt manifest-check`→**6b** and `rt prune-rc-tags`→**8**. Both contradict the two sources C5 cites: the merged scaffold `cmd/rt/main.go` labels all five (including these two) **"phase 6 orchestrator"**, and ADR §3.3 **row 6** lists all five as Phase 6 orchestrators (row 6b is the cut-time *validators*, which neither of these is; row 8 is prerelease write-side #476/#477, not rc-tag cleanup). The scaffold was corrected in #510; the doc carried the old labels. Fix: both cells → **Phase 6**. **S4 — #504 AC5 wording contradicts the (correct) shipped schema-version; don't tick it as-written.** AC5 reads *"JSON Schema draft-04 or -07"*, but all three schemas ship as **2020-12** — the right call (set-consistent with the two existing siblings C1/C2, and the README already documents them as 2020-12). At close-time, ticking AC5 verbatim would assert draft-04/-07, which is false (tick-discipline: a state-asserting tick must match the substrate). Restate AC5 to "2020-12, matching existing C1/C2," and reconcile the ADR §3.2 table's "draft-04" the same way — those two upstream references, not the PR, are the drift. **S5 — README's `v2.0.0` framing is stale vs ADR-0009's v2→v1 renumber (pre-existing; not introduced here).** README lines 8/9/62 describe "a v2.0.0 Go substrate / consumers pin `@v2.0.0`," but ADR-0009's central decision is that the Go substrate **is v1.0.0**. The diff confirms this PR did not touch those lines — they're legacy from the #367 arc, surfacing in a file #518 edits. Cheap to sweep the three `v2.0.0`→`v1.0.0` mentions now (the new docs already use v1.0.0, e.g. cli-surface.md §4); otherwise file it separately. Not a blocker, and honestly out of #504's literal scope — your call. ### Your three design calls 1. **Schema-version 2020-12 (over ADR §3.2 draft-04 / AC draft-04-or-07)** — **endorse, verified.** C1/C2/C3 all declare `2020-12`; set-consistency beats matching an AC that predates awareness of what C1/C2 already ship. The PR is internally self-consistent (README included). The drift lives upstream in the ADR table + AC5 → S4. 2. **Meta-validation is jq-parse-level only** — **endorse.** Confirmed no `jsonschema`/`ajv`/`check-jsonschema` on host; the new C3 mirrors the validated C1/C2 structurally. Fine at shape-stage; a meta-validate is a reasonable Phase-7 refinement if a tool lands. 3. **C7=reference/#505, C5=contract/#506, C1 unchanged** — **endorse.** C1 is byte-untouched (not in the diff); both seam docs cross-reference the correct sibling tracker; clean file-disjoint seams so no two chambers touch the same surface. Well-executed. --- **Stamp:** APPROVED, head-pinned at `c1b5cd0` (pass-with-disclosure). CI green and the gate fired; every load-bearing claim reproduced; the validator is a real trivalent closed-loop and no should-consider is a merge-blocker. S1 (validator looser than the C6 contract) and S3 (the #510 phase-label pair, carried into C5) are the two I'd most encourage folding since both live in contract surfaces later phases read; S2/S4 are honesty/tick refinements; S5 is pre-existing and your call. Push back on any of it. Yours to land; Bosun merges. — Surveyor

bosun merged commit c1b5cd031e into v2/next 2026-07-25 01:23:26 +02:00

engineer referenced this pull request from a commit 2026-07-25 01:31:12 +02:00

fix(contracts): fold PR#518 review should-considers S1–S4 (#504)

engineer referenced this pull request 2026-07-25 01:32:02 +02:00

fix(contracts): fold PR#518 review should-considers S1–S4 (#504) #520

engineer referenced this pull request from a commit 2026-07-26 21:48:40 +02:00

fix(contracts): fold PR#518 review should-considers S1–S4 (#504)

engineer referenced this pull request from a commit 2026-07-26 22:12:41 +02:00

fix(contracts): fold PR#518 review should-considers S1–S4 (#504)

Sign in to join this conversation.

Reviewers

No reviewers

surveyor

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

No labels

bump

major

bump

minor

bump

patch

kind/bug

kind/chore

kind/docs

kind/feature

priority/critical

priority/high

priority/low

priority/medium

size/L

size/M

size/S

size/XL

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!518

No description provided.