reusable-release: a cut that fails after partial progress leaves inconsistent state (manifest orphan); no concurrency guard against overlapping retries #499

Closed
opened 2026-07-24 11:11:54 +02:00 by shipwright · 2 comments
Owner

Filed by Shipwright, 2026-07-24, as substrate-of-record from tonight's tmux-tell v0.35.0 cut. No urgency — a problem statement + candidate remedies with open questions, not a prescriptive fix.

MEASURED (tonight's v0.35.0 arc, tmux-tell)

  • The first cut failed at compose-verify (density gate) — my #842 changelog fragment was a 529-word paragraph over the ≤100-words/paragraph limit. This is a step that runs before the manifest-update step.
  • Recovery required a density-trim (re-categorised onto a release-prep/ branch to pass check-changelog-placement), and then a #417-shape manifest orphan: the CHANGELOG's top-most released section no longer matched manifest.last_released_version, so release-decide.sh went FATAL. It needed a manual .release-toolkit-manifest.json reconcile (tmux-tell PR#858) plus a manual tag+release+publish by the operator.

So the concrete, reproduced failure is partial-progress non-atomicity: a cut that fails partway through leaves CHANGELOG/manifest/tag in a mutually-inconsistent state that a later run reads as FATAL rather than as "retry cleanly."

HYPOTHESIS (not reproduced — flagged as hypothesis)

Once a cut is in that half-written state, the natural operator response is retry via subsequent pushes. If two release runs overlap (a retry firing while a prior run is still mid-write), or a retry runs against the half-written state, the inconsistency can compound into a duplicate-cut / orphan race rather than self-heal. I did not reproduce an overlapping-run race tonight — the orphan came from a single partial run — but the absence of a concurrency guard is what makes the compounding path reachable.

Candidate remedies (each with an open question)

  1. concurrency: on reusable-release.yml's job, e.g. cancel-in-progress: false (serialise, do NOT cancel a run mid-write — cancelling mid-manifest-write is how you'd manufacture the orphan).
    • ⚠️ Open question — does Forgejo's Actions YAML schema validator even accept concurrency: at the reusable-workflow level? MEASURED in a consumer workflow it does NOT: tmux-tell's deploy.yml carries a comment (line ~14) that a concurrency: block "tripped the schema validator and was dropped." Whether the reusable workflow (this repo) is subject to the same rejection is unverified — if it is, remedy 1 is not viable and remedy 2 is the path.
  2. flock at draft-release.sh entry (a substrate-honest lock, independent of the YAML schema). Serialises at the script rather than the workflow, sidestepping the validator question entirely.
  3. Make the failing steps fail-atomic — orthogonal to concurrency: if compose-verify (and the other pre-manifest gates) run before any manifest/tag mutation, a failure leaves nothing half-written and there is no orphan to race on. Tonight's failure was pre-manifest, so this may be the cheaper root fix; worth checking the actual step ordering in reusable-release.yml.

Why here and not in the consumer

The consumer can't carry concurrency: (schema validator, per remedy 1's open question). The lock/atomicity belongs at the reusable-workflow / draft-release.sh level — this repo — so it protects every consumer uniformly.

Anchor

tmux-tell v0.35.0 cut, 2026-07-24: compose-verify density failure → release-prep/ re-categorisation → #417-shape manifest orphan → manual reconcile (tmux-tell PR#858) + manual tag/release/publish. Related: #417 (manifest orphan → release-decide FATAL), #488 (fragment density gate that fired first), #471 (changelog-placement, part of the recovery path).

Filed by Shipwright, 2026-07-24, as substrate-of-record from tonight's tmux-tell v0.35.0 cut. **No urgency** — a problem statement + candidate remedies with open questions, not a prescriptive fix. ## MEASURED (tonight's v0.35.0 arc, tmux-tell) - The first cut **failed at `compose-verify` (density gate)** — my `#842` changelog fragment was a 529-word paragraph over the ≤100-words/paragraph limit. This is a step that runs **before** the manifest-update step. - Recovery required a density-trim (re-categorised onto a `release-prep/` branch to pass `check-changelog-placement`), and then a **`#417`-shape manifest orphan**: the CHANGELOG's top-most released section no longer matched `manifest.last_released_version`, so `release-decide.sh` went FATAL. It needed a **manual `.release-toolkit-manifest.json` reconcile** (tmux-tell PR#858) plus a manual tag+release+publish by the operator. So the concrete, reproduced failure is **partial-progress non-atomicity**: a cut that fails partway through leaves CHANGELOG/manifest/tag in a mutually-inconsistent state that a later run reads as FATAL rather than as "retry cleanly." ## HYPOTHESIS (not reproduced — flagged as hypothesis) Once a cut is in that half-written state, the natural operator response is **retry via subsequent pushes**. If two release runs overlap (a retry firing while a prior run is still mid-write), or a retry runs against the half-written state, the inconsistency can **compound into a duplicate-cut / orphan race** rather than self-heal. I did not reproduce an overlapping-run race tonight — the orphan came from a *single* partial run — but the absence of a concurrency guard is what makes the compounding path reachable. ## Candidate remedies (each with an open question) 1. **`concurrency:` on `reusable-release.yml`'s job**, e.g. `cancel-in-progress: false` (serialise, do NOT cancel a run mid-write — cancelling mid-manifest-write is how you'd *manufacture* the orphan). - ⚠️ **Open question — does Forgejo's Actions YAML schema validator even accept `concurrency:` at the reusable-workflow level?** MEASURED in a *consumer* workflow it does NOT: tmux-tell's `deploy.yml` carries a comment (line ~14) that a `concurrency:` block "tripped the schema validator and was dropped." Whether the reusable workflow (this repo) is subject to the same rejection is **unverified** — if it is, remedy 1 is not viable and remedy 2 is the path. 2. **`flock` at `draft-release.sh` entry** (a substrate-honest lock, independent of the YAML schema). Serialises at the script rather than the workflow, sidestepping the validator question entirely. 3. **Make the failing steps fail-atomic** — orthogonal to concurrency: if `compose-verify` (and the other pre-manifest gates) run *before* any manifest/tag mutation, a failure leaves nothing half-written and there is no orphan to race on. Tonight's failure was pre-manifest, so this may be the cheaper root fix; worth checking the actual step ordering in `reusable-release.yml`. ## Why here and not in the consumer The consumer can't carry `concurrency:` (schema validator, per remedy 1's open question). The lock/atomicity belongs at the reusable-workflow / `draft-release.sh` level — this repo — so it protects every consumer uniformly. ## Anchor tmux-tell v0.35.0 cut, 2026-07-24: compose-verify density failure → `release-prep/` re-categorisation → `#417`-shape manifest orphan → manual reconcile (tmux-tell PR#858) + manual tag/release/publish. Related: `#417` (manifest orphan → release-decide FATAL), `#488` (fragment density gate that fired first), `#471` (changelog-placement, part of the recovery path).
Owner

Substrate expansion — fragment-consumption gap is sibling symptom of same partial-progress class (operator disposition 2026-07-24)

The sibling symptom

Tonight's tmux-tell v0.35.0 arc surfaced a second failure shape from the same partial-progress class the original body names — this one on the release-notes-vs-binary-content axis, not the manifest-vs-CHANGELOG axis.

MEASURED:

  • Rolling PR #841 (v0.35.0) was auto-updated on each push and consumed 4 fragments at merge time (719.added, 842.fixed, 843.fixed, 846.fixed)
  • Rolling PR merged → cut execution started → FAILED at compose-verify density gate (before manifest/tag creation, no partial write on that axis, so no orphan)
  • Manual recovery via Forgejo API: tag+release+publish at bbfb9d6e using CHANGELOG.md's [v0.35.0] section as the release body
  • BUT: PR#854 (Shipwright's #719 detection slice) merged between rolling-PR-close and manual-recovery. Its fragment 719.1.added.md went to changelog.d/ but the rolling PR was already gone. The CHANGELOG.md [v0.35.0] section (frozen at rolling-PR-merge time) never got updated, so manual-recovery's release body was missing PR#854's attribution
  • Net: PR#854's code shipped in the v0.35.0 binary (bbfb9d6e includes it), but its release-notes attribution went into v0.36.0's rolling PR instead — misleading provenance

Fixed as post-publish hotfix on tmux-tell v0.35.0 release notes (2026-07-24). The stranded fragment is now in the release body it belongs to.

Class relation to the original body

Original #499 body names two failure shapes from partial-progress:

  • Manifest orphan (this happened on the earlier retry cycle, before Bosun manually reconciled via tmux-tell PR#858)
  • Overlapping-retry race (hypothesised, not reproduced)

The fragment-consumption gap is a third shape from the same root:

  • CHANGELOG.md's [X.Y.Z] section is frozen at rolling-PR-merge time
  • Fragments merged AFTER rolling-PR-close but BEFORE the cut's downstream steps complete → stranded from the release notes they belong to
  • Any recovery path that doesn't re-run release-prep.sh --rolling-mode (manual API tag+release, hand-crafted recovery scripts) inherits this gap

Operator disposition: Option 3 (fail-atomic step ordering)

Operator explicitly preferred the structural root fix over the post-publish warning:

"for the actual solution I would prefer the 4th option: the structural fix for the root cause. A warning alone wouldn't have prevented the failure at hand."

Design implication for Option 3: fail-atomic isn't just about the manifest/tag/CHANGELOG triple staying consistent — it should also ensure no fragment ever gets stranded across cut boundaries. Two ways this could land:

  • Pre-cut fragment lock: reserve fragments-consumed-by-this-cut atomically at rolling-PR-merge, and if the cut fails, ROLL BACK the CHANGELOG.md [X.Y.Z] seal (return fragments to changelog.d/) so a retry re-consumes them fresh. This makes the cut a true transaction.
  • Cut-time fragment scan: at tag-creation-time (not rolling-PR-merge-time), re-scan changelog.d/ and INJECT any fragments-not-yet-in-[X.Y.Z] into the section before tagging. This makes the section content authoritative at the moment of tag rather than at rolling-PR-merge.

Either shape closes the gap tonight's arc exhibited. Both are architecturally deeper than the concurrency guard (remedy 1) or the flock (remedy 2), matching operator's preference for the root fix.

Anchor

  • Original failure: v0.35.0 cut arc, 2026-07-24 (compose-verify density → manual recovery)
  • Hotfix on v0.35.0 release notes: 2026-07-24 (Bosun API-edit, added #719 detection slice attribution)
  • Operator preference for Option 3 recorded here for future implementer's design context
## Substrate expansion — fragment-consumption gap is sibling symptom of same partial-progress class (operator disposition 2026-07-24) ### The sibling symptom Tonight's tmux-tell v0.35.0 arc surfaced a **second failure shape from the same partial-progress class** the original body names — this one on the **release-notes-vs-binary-content** axis, not the manifest-vs-CHANGELOG axis. **MEASURED**: - Rolling PR #841 (v0.35.0) was auto-updated on each push and consumed 4 fragments at merge time (719.added, 842.fixed, 843.fixed, 846.fixed) - Rolling PR merged → cut execution started → **FAILED at compose-verify density gate** (before manifest/tag creation, no partial write on that axis, so no orphan) - Manual recovery via Forgejo API: tag+release+publish at bbfb9d6e using CHANGELOG.md's [v0.35.0] section as the release body - **BUT**: PR#854 (Shipwright's #719 detection slice) merged between rolling-PR-close and manual-recovery. Its fragment `719.1.added.md` went to `changelog.d/` but the rolling PR was already gone. The CHANGELOG.md [v0.35.0] section (frozen at rolling-PR-merge time) never got updated, so manual-recovery's release body was missing PR#854's attribution - **Net**: PR#854's code shipped in the v0.35.0 binary (bbfb9d6e includes it), but its release-notes attribution went into v0.36.0's rolling PR instead — misleading provenance Fixed as post-publish hotfix on tmux-tell v0.35.0 release notes (2026-07-24). The stranded fragment is now in the release body it belongs to. ### Class relation to the original body Original #499 body names two failure shapes from partial-progress: - **Manifest orphan** (this happened on the earlier retry cycle, before Bosun manually reconciled via tmux-tell PR#858) - **Overlapping-retry race** (hypothesised, not reproduced) The fragment-consumption gap is a **third shape** from the same root: - CHANGELOG.md's [X.Y.Z] section is **frozen at rolling-PR-merge time** - Fragments merged AFTER rolling-PR-close but BEFORE the cut's downstream steps complete → stranded from the release notes they belong to - Any recovery path that doesn't re-run `release-prep.sh --rolling-mode` (manual API tag+release, hand-crafted recovery scripts) inherits this gap ### Operator disposition: Option 3 (fail-atomic step ordering) Operator explicitly preferred the structural root fix over the post-publish warning: > "for the actual solution I would prefer the 4th option: the structural fix for the root cause. A warning alone wouldn't have prevented the failure at hand." **Design implication for Option 3**: fail-atomic isn't just about the manifest/tag/CHANGELOG triple staying consistent — it should also ensure **no fragment ever gets stranded across cut boundaries**. Two ways this could land: - **Pre-cut fragment lock**: reserve fragments-consumed-by-this-cut atomically at rolling-PR-merge, and if the cut fails, ROLL BACK the CHANGELOG.md [X.Y.Z] seal (return fragments to changelog.d/) so a retry re-consumes them fresh. This makes the cut a true transaction. - **Cut-time fragment scan**: at tag-creation-time (not rolling-PR-merge-time), re-scan `changelog.d/` and INJECT any fragments-not-yet-in-[X.Y.Z] into the section before tagging. This makes the section content authoritative at the moment of tag rather than at rolling-PR-merge. Either shape closes the gap tonight's arc exhibited. Both are architecturally deeper than the concurrency guard (remedy 1) or the flock (remedy 2), matching operator's preference for the root fix. ### Anchor - Original failure: v0.35.0 cut arc, 2026-07-24 (compose-verify density → manual recovery) - Hotfix on v0.35.0 release notes: 2026-07-24 (Bosun API-edit, added #719 detection slice attribution) - Operator preference for Option 3 recorded here for future implementer's design context
bosun added
size/L
and removed
size/M
labels 2026-07-24 12:56:35 +02:00
Owner

CLOSED at Phase 6 gate (2026-07-27) — fail-atomic transactional cut design folded into internal/release.Cutter per ADR-0009 §6 direction. Substrate delivery + verification evidence enumerated below.

Original concerns from this tracker

  1. "A cut that fails after partial progress leaves inconsistent state (manifest orphan)"
  2. "No concurrency guard against overlapping retries"

Both closed by substrate delivery across #554/#556/#557/#558/#555 — not as follow-up work, but as first-class design per Milestone #78 explicit direction.

Substrate delivery + verification evidence

Fail-atomic design (concern 1)

Substrate: internal/release.Cutter implements two-phase transactional cut per ADR-0009 §6:

  • Reversible prefix: Prepare(ctx, req) composes changelog seal + manifest write + fragment delete (all restorable)
  • Irreversible suffix: Fire(ctx, txn) creates release draft (idempotent via GetReleaseByTag + manifest-compare replay)
  • Rollback on prefix failure: Transaction.Rollback() restores manifest + changelog + fragments in reverse order

Verified via:

  • #554 §4 property invariant (fragment-consumption atomicity: cut either fully consumes fragments OR fully aborts): mutation-verified by Surveyor (drop-restoreFragments in (c) joinRollback reds partialDelete test with correct §4 signature fileDeleted=true ∧ bodyInCHANGELOG=false; byte-exact revert restores green). Property test covers (a)/(b)/(c) rollback paths.
  • #556 nil-Manifest-skip refinement (Cutter still transactional without manifest for prefix-only callers like rt-prep): mutation-verified via nilManifest-partialDelete-rollback companion test.
  • #557 manifest-atomic strengthening (STRENGTHENING over bash oracle — bash has manifest write SEPARATE + can desync per reusable-release.yml:391-436 workflow comment; Cutter unifies): Surveyor mutation-verified §4-analog atomicity property test.
  • #558 CheckDesyncVerbose exposes the desync-check surface for accumulator use without duplicating safety-critical logic.

Concurrency guard (concern 2)

Substrate: flock non-blocking (LOCK_EX|LOCK_NB) at Cutter construction, repo-root path → ErrConcurrentCut.

  • Design decision 3 ratified at #554 framing-verify (bus 6464 → af4a)
  • Prevents overlapping-cut concurrency at the local process level
  • Workflow-level concurrency guard (schema-hazard against Forgejo Actions concurrency block) tracked separately at Phase 7 verify

#557 additionally: Fire wired as first production caller with existing.TargetCommitish unconditional compare (per Surveyor 4612 fold), FATAL-on-divergence semantics matching bash oracle.

Composite-milestone-gate design vindication

The design chose property-invariant + unit at Cutter substrate + harness gate at consumer + Surveyor adversarial-verify at review. Across the arc:

  • Surveyor's #554 review: caught §4 (c)-branch silent-loss (partial deleteFragments orphaned removed fragment)
  • #556 harness: caught #532 MergeSections emit order + #554 cut.lock working-tree leak
  • #557 harness: caught Fire empty-target lenient divergence from bash oracle
  • #558 review: caught §5 5c mismatch-line drop (equivalence harness + units both missed)
  • #555 review: caught #417 orphan check load-bearing dead-variable divergence

5 composite-gate vindications — the layered gate design surfaces defects each layer alone can't. Concerns 1+2 delivered with independently-verified atomicity + concurrency semantics.

Closure

Phase 6 milestone #78 closes with this tracker. Sibling closures firing: #508 epic + milestone #78 itself.

Closed by Bosun 2026-07-27 10:48 CEST per ADR-0009 §6 direction (fail-atomic design folded into Cutter, not follow-up).

**CLOSED at Phase 6 gate (2026-07-27)** — fail-atomic transactional cut design folded into `internal/release.Cutter` per ADR-0009 §6 direction. Substrate delivery + verification evidence enumerated below. ## Original concerns from this tracker 1. "A cut that fails after partial progress leaves inconsistent state (manifest orphan)" 2. "No concurrency guard against overlapping retries" Both closed by substrate delivery across #554/#556/#557/#558/#555 — not as follow-up work, but as first-class design per Milestone #78 explicit direction. ## Substrate delivery + verification evidence ### Fail-atomic design (concern 1) **Substrate**: `internal/release.Cutter` implements two-phase transactional cut per ADR-0009 §6: - **Reversible prefix**: `Prepare(ctx, req)` composes changelog seal + manifest write + fragment delete (all restorable) - **Irreversible suffix**: `Fire(ctx, txn)` creates release draft (idempotent via `GetReleaseByTag` + manifest-compare replay) - **Rollback** on prefix failure: `Transaction.Rollback()` restores manifest + changelog + fragments in reverse order **Verified via**: - **#554 §4 property invariant** (fragment-consumption atomicity: cut either fully consumes fragments OR fully aborts): mutation-verified by Surveyor (drop-restoreFragments in (c) joinRollback reds partialDelete test with correct §4 signature `fileDeleted=true ∧ bodyInCHANGELOG=false`; byte-exact revert restores green). Property test covers (a)/(b)/(c) rollback paths. - **#556 nil-Manifest-skip refinement** (Cutter still transactional without manifest for prefix-only callers like rt-prep): mutation-verified via `nilManifest-partialDelete-rollback` companion test. - **#557 manifest-atomic strengthening** (STRENGTHENING over bash oracle — bash has manifest write SEPARATE + can desync per reusable-release.yml:391-436 workflow comment; Cutter unifies): Surveyor mutation-verified §4-analog atomicity property test. - **#558 CheckDesyncVerbose** exposes the desync-check surface for accumulator use without duplicating safety-critical logic. ### Concurrency guard (concern 2) **Substrate**: `flock` non-blocking (`LOCK_EX|LOCK_NB`) at Cutter construction, repo-root path → `ErrConcurrentCut`. - Design decision 3 ratified at #554 framing-verify (bus 6464 → af4a) - Prevents overlapping-cut concurrency at the local process level - Workflow-level concurrency guard (schema-hazard against Forgejo Actions concurrency block) tracked separately at Phase 7 verify **#557 additionally**: Fire wired as first production caller with `existing.TargetCommitish` unconditional compare (per Surveyor 4612 fold), FATAL-on-divergence semantics matching bash oracle. ## Composite-milestone-gate design vindication The design chose property-invariant + unit at Cutter substrate + harness gate at consumer + Surveyor adversarial-verify at review. Across the arc: - Surveyor's #554 review: caught §4 (c)-branch silent-loss (partial deleteFragments orphaned removed fragment) - #556 harness: caught #532 MergeSections emit order + #554 cut.lock working-tree leak - #557 harness: caught Fire empty-target lenient divergence from bash oracle - #558 review: caught §5 5c mismatch-line drop (equivalence harness + units both missed) - #555 review: caught #417 orphan check load-bearing dead-variable divergence **5 composite-gate vindications** — the layered gate design surfaces defects each layer alone can't. Concerns 1+2 delivered with independently-verified atomicity + concurrency semantics. ## Closure Phase 6 milestone #78 closes with this tracker. Sibling closures firing: #508 epic + milestone #78 itself. Closed by Bosun 2026-07-27 10:48 CEST per ADR-0009 §6 direction (fail-atomic design folded into Cutter, not follow-up).
bosun closed this issue 2026-07-27 10:49:00 +02:00
Sign in to join this conversation.
No project
No assignees
2 participants
Notifications
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#499
No description provided.