Skip to content

fix(release): reject rebuilds of existing versions - #98

Merged
steipete merged 1 commit into
mainfrom
fix/release-retry-guard
Aug 28, 2026
Merged

fix(release): reject rebuilds of existing versions#98
steipete merged 1 commit into
mainfrom
fix/release-retry-guard

Conversation

@steipete

@steipete steipete commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Resolves a problem where dispatching an already published version rebuilt and re-signed its assets, created a redundant draft, and failed the correct final checksum-binding gate.

The red run 31838411168 did not leave v0.14.7 unpublished: run 31836986812 had already published it 14 minutes earlier. The public release is intact; the failed run left a separate complete eight-asset draft, ID 370810895. The tag and Go proxy/checksum-database version already exist.

Why This Change Was Made

A read-only preflight now rejects existing version tags and releases before entering the shared release workflow. One repository-wide concurrency group covers preflight through publication, including equivalent 0.14.8 / v0.14.8 inputs. Tags are rejected even without a published GitHub Release because they already expose Go module versions and precede draft creation in this pipeline. API errors fail closed.

The shared workflow retains signing, notarization, independent verification, and final byte binding. No checksum comparison is relaxed and no existing tag, release, draft, or asset is changed. The generic shared-workflow retry behavior for other callers is outside this PR. Historical runs still use their old workflow definition and must not be rerun to publish v0.14.7.

User Impact

New dispatches cannot rebuild an existing release version. Failed releases require reconciliation of their original payload or a new patch version. Publishing guidance now reflects the actual v0.14.7 state and identifies v0.14.8 as the next unused version, carrying #92's HTTP timeout fix, #96/#97's dependency/toolchain updates, and this guard. This PR does not publish or dispatch a release.

Evidence

Detailed reconciliation, hashes, asset IDs, provenance, and repeatable reads.

  • Public checksums match both original verifier attestations; retry checksums match both retry attestations. Every file in both payloads passes its own manifest.
  • Linux executables are identical; tar/gzip timestamps differ. Both macOS binaries differ only inside LC_CODE_SIGNATURE, with identical code directory hashes and new signing timestamps. This is a fresh rebuild/re-sign, not stale cache, changed source, or a publication race.
  • 25 regression cases pass, including the observed v0.14.7 state, optional v, existing tags/releases, and API failures. They run in CI and make check.
  • Live read-only execution rejects both 0.14.7 and v0.14.7, accepts absent 0.14.8, and never invokes a release workflow.
  • actionlint and git diff --check pass. Tidy, formatting, vet, deadcode, and govulncheck pass; the vulnerability gate reports no reachable vulnerabilities.
  • Isolated Codex autoreview (.agents/skills/autoreview/scripts/autoreview --mode local --engine codex) completed with no findings.
  • Full local make check passed (tidy/diff, formatting, vet, deadcode, govulncheck, all Go tests, race tests, and release regression tests). The separate GOWORK=off go test -count=1 ./... also passed.
  • Linux and Windows CI, CodeQL, and secret scanning are green for af84dd5996f5510627f640be18732aacf0be1510; no retries were needed.
  • Final live readback confirms both release records retain their original eight asset IDs, hashes, and update times, and the tag still points to its original annotated object.

@clawsweeper

clawsweeper Bot commented Aug 28, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 28, 2026
@clawsweeper

clawsweeper Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 28, 2026, 12:05 PM ET / 16:05 UTC.

ClawSweeper review

What this changes

The PR adds a read-only release preflight that blocks existing tags or releases before the shared publishing workflow, serializes release runs, and documents the v0.14.7 duplicate-dispatch incident.

Merge readiness

Blocked until stronger real behavior proof is added - 7 items remain

Keep open: this is a focused release-safety fix, but its validation accepts non-SemVer numeric identifiers and it changes the documented same-version recovery policy without maintainer confirmation.

Priority: P2
Reviewed head: af84dd5996f5510627f640be18732aacf0be1510
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The incident analysis is useful, but a correctness defect and absent inspectable real-workflow proof prevent merge readiness.
Proof confidence 🧂 unranked krab (1/6) Needs stronger real behavior proof before merge: The body asserts live execution but provides no trace tied to this head; add a redacted job log showing an existing version stops in preflight before release, then update the body for automatic re-review or ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The body asserts live execution but provides no trace tied to this head; add a redacted job log showing an existing version stops in preflight before release, then update the body for automatic re-review or ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 6 items Introduced validation defect: The new regex accepts leading-zero numeric identifiers such as v01.2.3 despite declaring that input must be SemVer; those values then proceed to GitHub lookups and the release job.
Release handoff: The introduced preflight is required before the shared release workflow receives the unchanged version input.
Focused test run: The added Node test file passes locally, but its invalid-input cases do not cover leading-zero SemVer components.
Findings 1 actionable finding [P2] Reject non-canonical numeric SemVer identifiers
Security None None.

How this fits together

The unified release workflow receives a version, delegates artifact creation and signing to the shared release pipeline, then publishes a Git tag and release. The new preflight sits before that handoff to prevent a duplicate version from starting another build.

flowchart LR
  A[Release dispatch] --> B[Version preflight]
  B --> C{Tag or release exists?}
  C -->|Yes| D[Stop duplicate build]
  C -->|No| E[Shared release workflow]
  E --> F[Signed artifacts and release]
Loading

Decision needed

Question Recommendation
Should crawlkit permanently require a new version after any release tag exists, including an unpublished or failed release attempt? Adopt immutable version dispatches: Approve the new-version-only policy, then require canonical SemVer validation and real workflow proof before merging.

Why: This PR intentionally replaces the repository’s documented same-version retry behavior, and the appropriate recovery path affects release policy beyond the narrow validation repair.

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The body asserts live execution but provides no trace tied to this head; add a redacted job log showing an existing version stops in preflight before release, then update the body for automatic re-review or ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Reject non-canonical numeric SemVer identifiers (P2) - The new regex permits v01.2.3 and v1.2.3-01, although this guard and the workflow input both require SemVer. Those values pass preflight and can invoke the release job; enforce SemVer’s no-leading-zero rule and add regression cases.
  • Resolve merge risk (P1) - Malformed numeric versions can pass preflight and invoke release automation rather than failing at the declared SemVer boundary.
  • Resolve merge risk (P1) - Existing operators lose the documented retry path after a tag is created; recovery for an unpublished draft is intentionally left to a separate decision.
  • Resolve merge risk (P1) - The referenced shared-workflow revision could not be fetched because repository DNS was unavailable, so exact canonicalization parity remains unverified.
  • Complete next step (P2) - A maintainer must approve the new no-retry release policy, and the contributor must provide real workflow proof; the narrow regex repair alone does not unblock merge.

Findings

  • [P2] Reject non-canonical numeric SemVer identifiers — .github/scripts/release-preflight.cjs:3
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 8 files, +294/-12 The release gate spans workflow behavior, tests, local checks, changelog, and operational documentation.

Merge-risk options

Maintainer options:

  1. Validate canonical versions and confirm policy (recommended)
    Reject leading-zero SemVer identifiers, add focused regression cases, obtain approval for the no-retry policy, and provide a redacted real preflight trace.
  2. Preserve an audited recovery route
    Design a narrowly authorized recovery path for retained unpublished payloads rather than making every tagged failure require a new version.

Technical review

Best possible solution:

Adopt the crawlkit-specific version-immutability policy only with maintainer approval, then enforce canonical SemVer and retain a documented, separately authorized recovery procedure.

Do we have a high-confidence way to reproduce the issue?

Yes—source-reproducible: the introduced regex accepts v01.2.3, while the added invalid-input tests omit leading-zero numeric identifiers.

Is this the best way to solve the issue?

No—not yet: fail-closed dispatching is plausible, but it needs canonical SemVer validation and explicit approval for replacing the existing retry policy.

Full review comments:

  • [P2] Reject non-canonical numeric SemVer identifiers — .github/scripts/release-preflight.cjs:3
    The new regex permits v01.2.3 and v1.2.3-01, although this guard and the workflow input both require SemVer. Those values pass preflight and can invoke the release job; enforce SemVer’s no-leading-zero rule and add regression cases.
    Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8620d26db83f.

Labels

Label changes:

  • add P2: The change affects a rare but consequential release workflow and has a bounded, concrete validation defect.
  • add merge-risk: 🚨 compatibility: It removes the repository’s previously documented same-version retry behavior after tag creation.
  • add merge-risk: 🚨 automation: Invalid version input can pass the new preflight and enter the release automation path.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The body asserts live execution but provides no trace tied to this head; add a redacted job log showing an existing version stops in preflight before release, then update the body for automatic re-review or ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: The change affects a rare but consequential release workflow and has a bounded, concrete validation defect.
  • merge-risk: 🚨 compatibility: It removes the repository’s previously documented same-version retry behavior after tag creation.
  • merge-risk: 🚨 automation: Invalid version input can pass the new preflight and enter the release automation path.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The body asserts live execution but provides no trace tied to this head; add a redacted job log showing an existing version stops in preflight before release, then update the body for automatic re-review or ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Introduced validation defect: The new regex accepts leading-zero numeric identifiers such as v01.2.3 despite declaring that input must be SemVer; those values then proceed to GitHub lookups and the release job. (.github/scripts/release-preflight.cjs:3, af84dd5996f5)
  • Release handoff: The introduced preflight is required before the shared release workflow receives the unchanged version input. (.github/workflows/release-unified.yml:36, af84dd5996f5)
  • Focused test run: The added Node test file passes locally, but its invalid-input cases do not cover leading-zero SemVer components. (.github/scripts/release-preflight.test.cjs:67, af84dd5996f5)
  • Policy change: The PR replaces the prior same-version retry guidance with a new-version-only rule after any tag exists, making release recovery a maintainer policy decision. (docs/publishing.md:83, af84dd5996f5)
  • History and ownership: The release guard was introduced by the current PR commit; the same contributor also authored the current main tip. (.github/scripts/release-preflight.cjs:1, af84dd5996f5)
  • Autoreview environment limitation: The required isolated autoreview helper could not start because the sandbox exposed no usable temporary directory; this is reviewer infrastructure, not contributor-proof failure.

Likely related people:

  • steipete: Authored this release-workflow change and the current main tip; prior merged repository work also includes the adjacent dependency refresh. (role: recent release and CI contributor; confidence: medium; commits: af84dd5996f5, 8620d26db83f, 5309f4cd9246; files: .github/workflows/release-unified.yml, .github/scripts/release-preflight.cjs, docs/publishing.md)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Reject leading-zero SemVer identifiers and add focused regression cases.
  • Post a redacted preflight job trace for an existing version that shows the release job was not entered.
  • Obtain maintainer confirmation of the no-retry release policy.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete marked this pull request as ready for review August 28, 2026 16:11
@steipete
steipete requested a review from a team as a code owner August 28, 2026 16:11
@steipete
steipete merged commit 52f0fb1 into main Aug 28, 2026
13 checks passed
@steipete
steipete deleted the fix/release-retry-guard branch August 28, 2026 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant