diff --git a/.claude/skills/release-pipeline/SKILL.md b/.claude/skills/release-pipeline/SKILL.md index b780d6fb..314db893 100644 --- a/.claude/skills/release-pipeline/SKILL.md +++ b/.claude/skills/release-pipeline/SKILL.md @@ -201,6 +201,12 @@ archive's sha256 to match the published one — the archive is reproducible by c (sorted entries, pinned timestamps, `gzip -n`), so a mismatch means the published bytes came from different input and must not be indexed. Publish a `+rebuild.N` tag in that case. +`complete` also covers a tag a later rebuild has taken over, including one whose run failed +before it published anything — re-running it would otherwise publish a stale archive and open a +catalog pull request moving the entry backwards (ADR-0005). The notice names the tag consumers +resolve to now. If you cut a `+rebuild.N` instead of re-running a failed job, the abandoned tag +stays abandoned; re-running it is a safe no-op. + ## Invariants not to break - GA versions only; no pre-releases. diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b8006459..07d2862e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -133,13 +133,16 @@ jobs: echo "mode=$mode" >> "$GITHUB_OUTPUT" case "$mode" in complete) - # Name the entry instead of asserting it is this tag. `complete` - # also covers a tag a later rebuild has superseded, for which - # "recorded in catalog.json" would be false and the operator's - # real question is which tag consumers resolve to now. + # Name the entry rather than asserting anything about this tag. + # `complete` also covers a tag a later rebuild has superseded, and + # such a tag need not be published at all — its run may have failed + # before `gh release create` (ADR-0005) — so neither "recorded in + # catalog.json" nor "is published" holds for every case reaching + # here. What the operator actually needs is the tag consumers + # resolve to now. current="$(jq -r --arg p "$PROJECT" --arg v "$VERSION" \ '.projects[$p][$v].tag' "$RUNNER_TEMP/catalog.json")" - echo "::notice::$TAG is published and catalog.json resolves $PROJECT $VERSION to $current; nothing to do. Correcting its archive needs a +rebuild.N tag." ;; + echo "::notice::catalog.json resolves $PROJECT $VERSION to $current; $TAG has nothing left to do. Correcting its archive needs a +rebuild.N tag." ;; register) echo "::notice::$TAG is published but catalog.json does not record it; completing registration only." ;; esac diff --git a/.please/docs/decisions/0003-idempotent-release-recovery.md b/.please/docs/decisions/0003-idempotent-release-recovery.md index aa54ac02..15aa073c 100644 --- a/.please/docs/decisions/0003-idempotent-release-recovery.md +++ b/.please/docs/decisions/0003-idempotent-release-recovery.md @@ -67,7 +67,8 @@ still outstanding and completes only those. | no | a newer rebuild of the pair | `publish`* | | no | this tag | refuse | - \* The one row this decision leaves as it found it — see Negative. + \* The one row this decision leaves as it found it — see Negative. **Amended by ADR-0005**, + which classifies that row `complete` and removes the asterisk. `publish` runs both phases. `register` skips publication and runs registration only. `complete` is a successful no-op. The last row cannot arise from an interrupted run — @@ -149,7 +150,8 @@ still outstanding and completes only those. it then registers it and moves the entry backwards. That path predates this decision — the guard it replaced keyed on the release existing, so it never fired here either — and closing it means deciding whether a superseded tag should still be publishable at all, which is a - separate question from recovery. Tracked as a follow-up. + separate question from recovery. Tracked as a follow-up. **Closed by ADR-0005** (issue #9), + which answers that question no and classifies the row `complete`. ### Neutral @@ -202,4 +204,6 @@ still outstanding and completes only those. - `scripts/release-mode.ts` — the entry point the workflow calls. - `scripts/lib/catalog-update.ts` — the tag-immutability rules this decision leaves intact. - `ARCHITECTURE.md` § "Release Invariants" — updated by this decision. +- ADR-0005 (`0005-supersession-on-the-publish-path.md`) — amends the decision table's fifth + row and closes the Negative bullet above. - Issue #6, and the Greptile review thread on PR #5 that raised it. diff --git a/.please/docs/decisions/0005-supersession-on-the-publish-path.md b/.please/docs/decisions/0005-supersession-on-the-publish-path.md new file mode 100644 index 00000000..69ab1490 --- /dev/null +++ b/.please/docs/decisions/0005-supersession-on-the-publish-path.md @@ -0,0 +1,142 @@ +# ADR-0005: Classify a Superseded Tag as `complete` Whether or Not It Was Published + +## Status + +Accepted — 2026-09-16 + +## Context + +ADR-0003 derives what a release run still owes from two observable facts — whether the GitHub +Release exists, and which tag `catalog.json` records for this `(project, version)` on the +default branch — and folds one more case into them: a catalog that has moved past this tag to a +*newer* `+rebuild.N` owes nothing, because registering the older tag is a legal catalog write +(`catalog-update.ts` repoints an entry at any rebuild of its own pair, deliberately recording +the rebuild last published rather than the highest one) that walks consumers backwards onto a +superseded archive. + +That check sat inside the branch for a release that already exists. Row 5 of the decision +table — release absent, catalog naming a newer rebuild — was therefore left as `publish*`, with +the asterisk pointing at a Negative bullet explaining why: closing it means answering a question +recovery did not raise, namely whether a superseded tag should still be publishable at all. + +The row is reachable, and `publish` is the damaging answer. `release.yml` gates registration on +`mode != 'complete'`, so `publish` runs *both* phases: + +1. `boot-4.1.1+rebuild.1` is tagged; its run fails during build, so no release is created. +2. The operator cuts `boot-4.1.1+rebuild.2` rather than re-running; it publishes and registers. +3. Someone re-runs the failed `rebuild.1` job, or re-pushes its tag. +4. `releaseExists === false` → `publish` → `rebuild.1` is published, and the catalog pull + request that follows repoints the entry from `rebuild.2` back to `rebuild.1`, dragging + `released_at` backwards with it. + +Publication itself is harmless — `rebuild.1` is a tag of its own and no published asset is +replaced. The catalog write is the damaging half, and it lands as a pull request titled +`chore(catalog): record boot-4.1.1+rebuild.1`, which reads as routine to whoever merges it. + +Step 1 is not hypothetical. Of the 47 Boot rebuild releases cut on 2026-09-16, four runs — +`boot-3.3.6`, `boot-3.3.7`, `boot-3.5.6` and `boot-3.5.7`, all `+rebuild.1` — were cancelled at +the Build step's 30-minute timeout, before any release existed. Had a `+rebuild.2` been cut for +any of them instead of a re-run, step 4 was one stale job re-run away. + +This supersedes ADR-0003's row 5 and closes the Negative bullet that recorded it; the rest of +ADR-0003 stands. + +## Decision + +**Supersession is decided before publication is considered.** `classifyRelease` asks whether the +catalog has moved past this tag first, and only then splits on whether the release exists: + +| Release exists | Catalog entry for this `(project, version)` | Mode | +| -------------- | ------------------------------------------- | ---------- | +| no | absent, or a tag this one supersedes | `publish` | +| yes | absent, or a tag this one supersedes | `register` | +| yes | this tag | `complete` | +| yes | a newer rebuild of the pair | `complete` | +| no | a newer rebuild of the pair | `complete` | +| no | this tag | refuse | + +Only the fifth row changes. `register` still proves the published bytes before indexing them, +`publish` still runs both phases, and the refusal for a catalog entry whose release is gone is +unchanged — it is checked first, since "the catalog names *this* tag" and "the catalog names a +*newer* rebuild" cannot both hold. + +**It stays in `classifyRelease`, not in `applyEntry`.** `applyEntry`'s permissiveness about +suffix ordering is the deliberate design ADR-0003 relied on and this decision leaves intact: the +catalog records the rebuild last published, so a genuine republication of `rebuild.1` after +`rebuild.2` — an operator correcting a bad `rebuild.2` by re-publishing the known-good +predecessor — must remain expressible. What is wrong is not the write; it is a *run* making it +by accident. Ordering is a fact about which run owes what, so it belongs where the other two +facts are already weighed. + +**No fourth mode.** The modes name what a run still owes (ADR-0003), and a run the catalog has +moved past owes nothing whichever phases it has left. The workflow's `complete` notice names +the tag the catalog actually resolves to, so an operator can still tell the cases apart, and it +no longer claims `$TAG is published` — which this row makes false. + +## Consequences + +### Positive + +- **The last backwards-catalog path is closed.** Both halves of the hazard ADR-0003 identified + now answer the same way, so no sequence of failed runs, rebuilds and re-runs can open a + pull request moving an entry onto a superseded archive. +- **Re-running a stale failed job stays a no-op rather than becoming an error.** The idempotence + ADR-0003 chose — re-running the tag is always the right instinct — now holds on this row too. +- **The rule got simpler, not larger.** Supersession was already implemented; moving it ahead of + the `releaseExists` split removed a branch rather than adding one, and the decision table + loses its footnote. + +### Negative + +- **A superseded tag can no longer be published at all.** Back-filling an older rebuild as an + archived artifact — publishing `rebuild.1`'s bytes for provenance after `rebuild.2` has taken + over the entry — is now a `complete` no-op. Nothing in the pipeline needs this today: the + catalog is the only way consumers discover a release (ARCHITECTURE.md), so an archive it does + not name is not reachable, and every one of the 155 published versions is the entry's own tag. + If it ever matters, the fourth mode rejected below is what to add. +- **Two facts no longer fully separate the modes.** `releaseExists` is now ignored on the + superseded rows, so the mode is not a function of the two booleans alone but of the tag + ordinals as well. That was already true for row 4; this makes it true for a second row. + +### Neutral + +- **The `complete` notice changed wording** to stop asserting that `$TAG` is published, since + the new row is reached precisely when it is not. +- **No workflow gating changed.** `publish` and `register` still key on the same step + conditions; the only difference is which runs reach them. + +## Alternatives Considered + +- **Publish but do not register — a fourth mode running phase 1 and skipping phase 2.** The only + option that preserves publishing a superseded tag, and the one to revisit if the Negative + above ever bites. Rejected now: it adds a mode whose name has to explain itself against the + "name what the run owes" rule, plus a third `if:` shape in `release.yml`, to serve a use case + that has never arisen in 155 releases. Speculative structure on the release path is the code + least likely to be correct when it finally runs. + +- **Refuse, naming the newer rebuild.** Loudest, and it would catch an operator re-pushing a tag + by mistake. Rejected: it makes a re-run of a stale failed job an error rather than a no-op, + which is exactly the reflex ADR-0003 set out to make safe. The `complete` notice already names + the tag the catalog resolves to, so the operator gets the same information without a red run. + +- **Enforce suffix ordering in `applyEntry`.** It would block the bad write at the last possible + moment, covering any future caller rather than this one path. Rejected: it removes a + capability the catalog's design states on purpose — the entry records the rebuild last + published, not the highest — and it would fail the run *after* a release has been published, + leaving the half-done state this whole area exists to avoid. + +- **Do nothing; rely on the pull-request merge gate.** Rejected: the gate is real but weak. The + pull request is titled `chore(catalog): record ` like every other one, and its diff shows + a tag and a timestamp changing, which is what a legitimate rebuild registration also looks + like. + +## Related + +- ADR-0003 (`0003-idempotent-release-recovery.md`) — the recovery decision this amends; its + decision table row 5 and the matching § Consequences → Negative bullet are superseded here. +- `scripts/lib/release-state.ts` — `classifyRelease`, the pure decision. +- `scripts/release-mode.ts` — the entry point `release.yml` calls. +- `scripts/lib/catalog-update.ts` — `applyEntry`, the permissive repoint this decision + deliberately leaves alone. +- `.github/workflows/release.yml` — the `complete` notice this reworded. +- Issue #9 — where the gap was filed, with the three options weighed above. diff --git a/.please/docs/decisions/index.md b/.please/docs/decisions/index.md index 9295e490..57b02820 100644 --- a/.please/docs/decisions/index.md +++ b/.please/docs/decisions/index.md @@ -8,3 +8,4 @@ | [0002](./0002-antora-as-a-library.md) | Drive Antora as a Library and Emit Markdown from the Asciidoctor AST | 2026-09-11 | Accepted | | [0003](./0003-idempotent-release-recovery.md) | Recover a Published-but-Unregistered Release by Re-running the Tag | 2026-09-12 | Accepted | | [0004](./0004-synthesize-3x-component.md) | Reconstruct the Generated Component for Spring Boot 3.x | 2026-09-12 | Accepted | +| [0005](./0005-supersession-on-the-publish-path.md) | Classify a Superseded Tag as `complete` Whether or Not It Was Published | 2026-09-16 | Accepted | diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 507da8ae..209b5eae 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -64,7 +64,7 @@ Conversion pipeline. Each top-level file is an executable Bun/TypeScript script | `convert.ts` | Pipeline entry — drive Antora's pipeline modules over a fetched tree and emit one Markdown file per page plus `_index.md`. | | `package-release.ts` | Pipeline entry — produce a reproducible `tar.gz` + `manifest.json` + SHA-256 checksum. | | `update-catalog.ts` | Pipeline entry — record a published `(project, version) → tag` in `catalog.json`. | -| `release-mode.ts` | Pipeline entry — report whether a tag still owes publication, registration, or nothing (ADR-0003). | +| `release-mode.ts` | Pipeline entry — report whether a tag still owes publication, registration, or nothing (ADR-0003, ADR-0005). | | `promote-markdown.ts` | Pipeline entry — copy a converted tree into the committed `markdown///`. | | `detect-upstream-versions.ts` | Tooling entry — list GA versions upstream has released that the catalog does not carry. Read-only; feeds the nightly issues and the build matrix. | | `lib/catalog-schema.ts` | zod schema for `catalog.json`. Owns the public catalog shape; changes require an ADR. | diff --git a/scripts/lib/release-state.ts b/scripts/lib/release-state.ts index bc71e308..50a1ab2b 100644 --- a/scripts/lib/release-state.ts +++ b/scripts/lib/release-state.ts @@ -21,6 +21,13 @@ * own pair — that walks consumers backwards onto a superseded archive. A run * whose tag the catalog has moved past owes nothing. * + * That last verdict does not depend on publication having happened. A tag whose + * run failed *before* `gh release create` is superseded the same way — by the + * rebuild cut in its place — and re-running it publishes an archive nobody + * asked for and then makes exactly that backwards catalog write. So supersession + * is decided before the two facts are split, not on the recovery path alone + * (ADR-0005). + * * Pure — the caller observes both facts and applies the verdict. */ @@ -32,7 +39,8 @@ * skip publication and record it, once the published bytes are shown to * match this run's rebuild. * - `complete` — the run is a no-op, because it owes nothing: both phases are - * done, or the catalog has already moved past this tag to a newer rebuild. + * done, or the catalog has already moved past this tag to a newer rebuild — + * whether or not this tag was ever published. * The modes are named after what a run still owes rather than after the state * it found (ADR-0003), so both cases are the same instruction to the caller. */ @@ -98,24 +106,24 @@ function isSupersededBy(tag: string, catalogTag: string): boolean { export function classifyRelease(state: ReleaseState): ReleaseMode { const registered = state.catalogTag === state.tag - if (!state.releaseExists) { - if (registered) { + if (registered) { + if (!state.releaseExists) { throw new Error( `catalog.json records tag "${state.tag}" but no such release exists. ` + 'A release is never recorded before it is published, so the release was ' + 'deleted or the catalog was edited by hand; repair the catalog before re-running.', ) } - return 'publish' - } - - if (registered) return 'complete' + } - // Published, unregistered, and the catalog has already moved on: the entry - // this run would write is the one a later rebuild replaced on purpose. + // The catalog has already moved on: the entry this run would write is the one + // a later rebuild replaced on purpose. Asked before publication is considered, + // because publication is not what makes the write wrong — both phases run + // together on the `publish` path, so a superseded tag that never published + // reaches the same backwards catalog write by a longer route. if (state.catalogTag !== null && isSupersededBy(state.tag, state.catalogTag)) return 'complete' - return 'register' + return state.releaseExists ? 'register' : 'publish' } diff --git a/tests/unit/release-state.test.ts b/tests/unit/release-state.test.ts index b961ab82..9d6d117a 100644 --- a/tests/unit/release-state.test.ts +++ b/tests/unit/release-state.test.ts @@ -74,6 +74,17 @@ const CASES: readonly Case[] = [ catalogTag: 'boot-4.1.1+rebuild.2', expected: 'complete', }, + { + name: 'is complete when a newer rebuild superseded a tag that was never published', + why: 'The rebuild.1 run failed before `gh release create`, so rebuild.2 was cut and ' + + 'completed both phases instead. Re-running the stale rebuild.1 job owes nothing: ' + + 'publishing it would be harmless on its own, but the catalog write that follows ' + + 'repoints the entry back onto it and drags `released_at` backwards with it.', + tag: 'boot-4.1.1+rebuild.1', + releaseExists: false, + catalogTag: 'boot-4.1.1+rebuild.2', + expected: 'complete', + }, { name: 'is complete when a rebuild has superseded the base tag', why: 'The same supersession seen from ordinal 0: the base tag is published and ' @@ -83,6 +94,15 @@ const CASES: readonly Case[] = [ catalogTag: 'boot-4.1.1+rebuild.1', expected: 'complete', }, + { + name: 'is complete when a rebuild superseded a base tag that was never published', + why: 'The same supersession at ordinal 0: the original tag never got a release, and ' + + 'the rebuild cut in its place is what consumers resolve to now.', + tag: 'boot-4.1.1', + releaseExists: false, + catalogTag: 'boot-4.1.1+rebuild.1', + expected: 'complete', + }, { name: 'orders rebuild suffixes numerically, not lexicographically', why: '"10" < "9" as text, which would read the catalog as being behind and '