feat: [release] harden /release — preflight script, fail-closed tags, publish gates#1032
Conversation
…pt, fail-closed tags, publish gates
Retro of the v0.8.10 / v0.9.1 / v0.9.2 release traces (.github/meta/release-retro-2026-07-22.md)
found recurring failure classes; this change closes them:
- `script/release-preflight.ts` (+34 unit tests): deterministic fail-closed gate —
clean tree, HEAD == fresh `origin/main` (stage `pre`) or ancestor (stage `tag`),
local+remote tag-collision check (never auto-delete), version sanity vs npm,
prerelease-line ancestry, `release-blocker` PRs, `PREV_TAG` dry-run, marker guard
with full output in one pass
- `release.yml`: `publish-npm` now gated on the `test` job (npm could previously
publish with tests red); tag format/SHA/CHANGELOG validated BEFORE publish;
pre-publish smoke test asserts the exact tag version (a stale 0.7.3 binary once
passed while releasing 0.9.2)
- `/release` skill: preflight as Step 2 gate; three-step verified commit/tag +
`git push --atomic origin HEAD:main v{V}`; stop-never-improvise on structural
failures; deferred findings require durable issue links; P0-candidates verified
or explicitly downgraded; exit-status integrity rule (no trailing status echoes);
smoke-test the dist binary by path; every step visible in the summary table;
personas deliver via SendMessage before idling
- RELEASING.md: targeted staging instead of `git add -A`, atomic push, honest
AUR-disabled / Docker-best-effort wording
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…_TAG upper bound Codex review of the release-hardening commit found fail-open paths; all fixed: - skill/RELEASING.md: tag verification now aborts (`|| exit 1`) instead of echo-and-continue; stage-`tag` preflight moved AFTER the release commit (it previously ran against a tree dirtied by the CHANGELOG edit and could never pass); `cd` blocks made subshells so repo-root paths stay valid; smoke test builds with `OPENCODE_VERSION` injected and asserts equality - `release-preflight.ts`: npm publish-check only treats E404 as "not published" (other errors FAIL); `gh` errors FAIL (WARN only when the binary is missing); malformed `gh` JSON FAILs instead of double-reporting; `run()` no longer throws on missing binaries; SemVer prerelease compare is numeric-aware (`beta.10` > `beta.2`); `selectPrevTag` picks the greatest stable tag strictly LOWER than the target (a merged fork-inherited `v1.18.3` must not become PREV_TAG for `v0.9.x`) — same fix applied to `release.yml`'s PREV_TAG loop - `/release-beta`: wires in the preflight (`--allow-prerelease`) and fail-closed tag collision checks before pushing the beta tag - tests: 34 -> 38 (numeric prerelease precedence, prefix ranking, fork-inherited higher-tag exclusion) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
full receipts (2 sessions)
orchestrator ·
|
| subagent | cost |
|---|---|
| agent-atrace-v091-b5d58e39c4eda8f9 · claude-sonnet-5 | ≥ $1.6261 |
| agent-apreflight-builder-9fc8c0141179718c · claude-sonnet-5 | ≥ $1.1144 |
| agent-atrace-v0810-ae7ebf35d089c1b0 · claude-sonnet-5 | ≥ $0.9787 |
| agent-atrace-v092-2886c3ff0222cb21 · claude-sonnet-5 | ≥ $0.7885 |
codex · d654234c
- - - - - - - - - - - - - - - - - - - - - - - - -
AIRECEIPTS
“Review the commit at HEAD (56fcfb6085) on bra…”
Codex · Jul 23 2026 06:51:39 UTC · 10m 34s
gpt-5.6-sol 100%
cache served 95% of input tokens
pre-edit: no named edit tool observed
(share before the first named edit tool)
exec.........................≥ $2.7152 (29 calls)
update_plan...................≥ $0.1872 (2 calls)
index_status...................≥ $0.0936 (1 call)
list_projects..................≥ $0.0936 (1 call)
run............................≥ $0.0936 (1 call)
caveat: Codex trace omits GPT-5.6 cache-write tokens — floor excludes any write premium
--------------------------------------------------
KNOWN PRICED SUBTOTAL....................≥ $3.1832
standard API-equivalent floor; not an invoice
partial pricing coverage; invoice total unknown
same tokens on gpt-5.4-mini..............≥ $0.4775
(85% lower observable floor)
(arithmetic, not a prediction)
- - - - - - - - - - - - - - - - - - - - - - - - -
npx aireceipts-cli
github.com/anandgupta42/receipts
- - - - - - - - - - - - - - - - - - - - - - - - -
handoff — flagged pattern cost ≈ 545,502 tok
FLAGGED PATTERN COST.................≈ 545,502 tok
heuristic pattern subtotal · not proven savings
≈ re-priced eligible trivial spans.......≈ $0.0589
(4 tiny turns, priced at claude-haiku-4-5)
→ route short replies to a cheaper model
covers: 2 sessions · 117 turns · 1 flagged-pattern line
Generated by aireceipts
📝 WalkthroughWalkthroughAdds a deterministic release preflight script with SemVer and ancestry checks, updates stable and beta release procedures, strengthens publication workflow validation, and adds unit-test execution to CI. ChangesRelease hardening
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Maintainer
participant ReleasePreflight
participant Git
participant Npm
participant GitHub
Maintainer->>ReleasePreflight: Run staged release preflight
ReleasePreflight->>Git: Fetch and validate worktree, ancestry, and tags
ReleasePreflight->>Npm: Check publication and dist-tags
ReleasePreflight->>GitHub: Check release-blocking pull requests
ReleasePreflight-->>Maintainer: Return checklist status
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42a6251a91
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const tag of lineTags) { | ||
| const ancestor = git(["merge-base", "--is-ancestor", tag, "origin/main"]) | ||
| if (ancestor.code !== 0) offenders.push(tag) |
There was a problem hiding this comment.
When /release-beta cuts beta.2 from the same release branch as beta.1, the earlier tag is an ancestor of HEAD but normally not of origin/main; this check therefore marks it as an offender and fails the mandatory preflight. That blocks the beta-to-beta round trip explicitly required by .claude/commands/release-beta.md, so prerelease targets should check prior beta tags against HEAD while stable promotion can continue requiring ancestry to origin/main.
Useful? React with 👍 / 👎.
| if (npmView.code !== 0) { | ||
| add({ | ||
| name: "version sanity", | ||
| status: "WARN", | ||
| detail: `could not reach npm to check current 'latest' version:\n${npmView.stderr || npmView.stdout}`, |
There was a problem hiding this comment.
When the npm registry is transiently unreachable, this required version-sanity check becomes only a WARN, and the final exit logic permits warnings. The script then skips both the monotonic comparison and the exact-version publication check, allowing the release tag to be pushed for an already-published or lower version and deferring failure until the irreversible release workflow has started; an inconclusive registry query should be a FAIL.
Useful? React with 👍 / 👎.
| export function parseSemver(version: string): Semver | null { | ||
| const m = /^(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z-.]+))?$/.exec(version) | ||
| if (!m) return null |
There was a problem hiding this comment.
For malformed inputs such as 01.2.3, 1.2.3-01, or 1.2.3-alpha..1, this regex returns a parsed version even though SemVer forbids leading zeroes in numeric identifiers and empty prerelease identifiers. The workflow's new tag regex accepts the same forms, so such a tag can pass both local preflight and CI tag validation before invalid version data reaches the publishing path; validate each core and prerelease identifier according to SemVer.
Useful? React with 👍 / 👎.
| # Shallow checkouts may not carry the tag ref locally — fetch it so the | ||
| # comparison runs against what origin actually has. | ||
| git fetch --no-tags --depth 1 origin "refs/tags/$CURRENT_TAG:refs/tags/$CURRENT_TAG" 2>/dev/null || true | ||
| TAG_SHA=$(git rev-parse "refs/tags/$CURRENT_TAG^{commit}") |
There was a problem hiding this comment.
If the release tag is force-moved between the tag-push event and this publish job, Git refuses to overwrite the checkout's existing local tag (git fetch -h describes -f as “force overwrite local reference,” and the unforced command reports would clobber existing tag). The || true suppresses that failure, after which rev-parse reads the stale event tag and still matches HEAD, so npm can publish the old commit even though the tag on origin now names another commit; fetch into a temporary ref or fail the validation when the fetch cannot establish the current remote value.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,200 @@ | |||
| import { describe, test, expect } from "bun:test" | |||
There was a problem hiding this comment.
No workflow executes this new test file: the repository root bunfig.toml redirects tests to ./do-not-run-tests-from-root, so bun test script/release-preflight.test.ts fails before discovery, while the only script-level CI test step runs with working-directory: script/upstream. Consequently changes to this release-critical gate can merge without any of these 38 assertions running; add a CI step whose working directory is script or otherwise place the suite under an existing test runner.
Useful? React with 👍 / 👎.
| } else { | ||
| const npmView = run(["npm", "view", "@altimateai/altimate-code", "version"]) | ||
| if (npmView.code !== 0) { |
There was a problem hiding this comment.
For an allowed prerelease, this only reads npm's latest version. If beta already points to 0.10.0-beta.5 and an operator supplies the unpublished 0.10.0-beta.4, the target is still greater than a stable latest such as 0.9.2, so preflight passes and npm publish --tag beta moves new beta installs backward to beta.4. Query and compare the current beta dist-tag for prerelease targets rather than validating only against latest.
Useful? React with 👍 / 👎.
| if (guard.code !== 0) { | ||
| add({ name: "marker guard", status: "FAIL", detail: fullOutput || `exited ${guard.code} with no output` }) | ||
| } else { | ||
| add({ name: "marker guard", status: "PASS", detail: fullOutput || "no unmarked upstream-shared changes" }) |
There was a problem hiding this comment.
In a fresh clone without the upstream remote, analyze.ts warns and falls back to treating only packages/opencode/src/ as upstream-shared, which can miss unmarked changes in other upstream-owned packages. Because that fallback exits zero, this wrapper reports the mandatory marker guard as PASS and the release proceeds despite an incomplete check; detect the analyzer's degraded mode or establish/fetch the required upstream ref and fail when it is unavailable.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
362-375: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPREV_TAG selection logic is now duplicated between bash (here) and
selectPrevTaginscript/release-preflight.ts.The algorithm here (iterate stable tags merged into HEAD, sorted descending by version, skip the current tag and anything not strictly lower, take the first match) is logically equivalent to the unit-tested
selectPrevTaghelper — but it's a second, independent implementation in a different language. Given the PR's own retro explicitly calls out a PREV_TAG selection bug as one of the root causes being fixed, having two implementations that must be kept in sync is a latent risk for exactly the kind of drift this PR is hardening against.Consider having this job invoke
bun script/release-preflight.ts's tag-selection logic (e.g. via a smallbun runcall after adding a bun setup step togithub-release, mirroringtest/sanity-verdaccio) instead of reimplementing the same semantics in bash.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 362 - 375, Remove the duplicated PREV_TAG iteration and version comparison from the release workflow, and invoke the existing selectPrevTag logic in script/release-preflight.ts instead. Add the minimal Bun setup or workflow invocation needed for github-release, then use the helper’s result to set PREV_TAG while preserving the current stable, merged-ancestor, strictly-lower-than-current selection semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/meta/release-retro-2026-07-22.md:
- Line 53: Update the release ancestry note in the document to use the
established “pre-release” spelling consistently, replacing “prerelease” without
changing the surrounding guidance or example.
- Around line 48-49: Correct the preflight script reference in the release retro
section from script/release/preflight.ts to script/release-preflight.ts,
matching the supplied implementation so operators can locate the gate.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 362-375: Remove the duplicated PREV_TAG iteration and version
comparison from the release workflow, and invoke the existing selectPrevTag
logic in script/release-preflight.ts instead. Add the minimal Bun setup or
workflow invocation needed for github-release, then use the helper’s result to
set PREV_TAG while preserving the current stable, merged-ancestor,
strictly-lower-than-current selection semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 37613be8-7f0b-46b3-9beb-1f208dfe3dd3
📒 Files selected for processing (7)
.claude/commands/release-beta.md.claude/commands/release.md.github/meta/release-retro-2026-07-22.md.github/workflows/release.ymldocs/RELEASING.mdscript/release-preflight.test.tsscript/release-preflight.ts
| ### 6. Move the deterministic checks into `script/release/preflight.ts` | ||
| Six `fix: [release]` commits in one cycle (Bun pin, Verdaccio .dockerignore, version derivation, npm E413 de-dupe, smoke-test target, marker-guard base ref) = every release debugs the pipeline live. The skill's new Step 2 = run the script. Checks (objective invariants only): |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the preflight script path.
The document names script/release/preflight.ts, but the supplied implementation is script/release-preflight.ts. This path should be corrected so release operators can locate the required gate.
Proposed fix
-### 6. Move the deterministic checks into `script/release/preflight.ts`
+### 6. Move the deterministic checks into `script/release-preflight.ts`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### 6. Move the deterministic checks into `script/release/preflight.ts` | |
| Six `fix: [release]` commits in one cycle (Bun pin, Verdaccio .dockerignore, version derivation, npm E413 de-dupe, smoke-test target, marker-guard base ref) = every release debugs the pipeline live. The skill's new Step 2 = run the script. Checks (objective invariants only): | |
| ### 6. Move the deterministic checks into `script/release-preflight.ts` | |
| Six `fix: [release]` commits in one cycle (Bun pin, Verdaccio .dockerignore, version derivation, npm E413 de-dupe, smoke-test target, marker-guard base ref) = every release debugs the pipeline live. The skill's new Step 2 = run the script. Checks (objective invariants only): |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/meta/release-retro-2026-07-22.md around lines 48 - 49, Correct the
preflight script reference in the release retro section from
script/release/preflight.ts to script/release-preflight.ts, matching the
supplied implementation so operators can locate the gate.
| - clean worktree; HEAD == fresh origin/main (per #5) — also covers the "user just merged a PR" staleness that forced a manual interruption in v0.9.2 | ||
| - tag collision, local + remote (per #1) — fail closed | ||
| - target version: valid SemVer, strictly greater than current npm `latest`, not already published (NOT equality with package.json — it intentionally says `1.17.9`) | ||
| - prerelease ancestry scoped to the target release line: e.g. releasing `v0.9.1` with `v0.9.0-beta.*` not an ancestor of origin/main → stop (v0.9.1's Step-2 stop turned into a 17-hour merge side-quest improvised via 3 AskUserQuestion rounds) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use one prerelease spelling consistently.
This document mixes “prerelease” with “pre-release”. Use the established pre-release spelling here for consistency.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~53-~53: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ...son — it intentionally says 1.17.9) - prerelease ancestry scoped to the target release l...
(EN_WORD_COHERENCY)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/meta/release-retro-2026-07-22.md at line 53, Update the release
ancestry note in the document to use the established “pre-release” spelling
consistently, replacing “prerelease” without changing the surrounding guidance
or example.
Source: Linters/SAST tools
…pm, authoritative tag check
Addresses all 7 chatgpt-codex-connector review comments:
- P1 prerelease ancestry now checks against HEAD, not origin/main — cutting
beta.N+1 from a release branch no longer trips on beta.N being unmerged
- P1 npm unreachable/unparseable is now FAIL, not WARN — an inconclusive
registry query must not defer failure to the irreversible release workflow
- P1 release.yml validates the tag via `git ls-remote origin` (with annotated
`^{}` peel) instead of an unforced fetch that a force-moved tag would defeat
- P1 prerelease targets are also compared against the current `beta` dist-tag
so a stale beta.N cannot move new beta installs backward
- P2 `parseSemver` + the workflow tag regex now enforce semver.org rules:
no leading zeros in numeric identifiers, no empty prerelease identifiers
- P2 ci.yml marker-guard job runs the preflight test suite (root bunfig.toml
pins `bun test` away from the repo root, so it runs from `script/`)
- P2 marker-guard check FAILs when analyze.ts degrades to pattern-only
detection because the upstream remote is missing
tests: 38 -> 41 (leading-zero rejection, empty-identifier rejection,
zero/alphanumeric prerelease acceptance)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed all 7 Codex review comments in 19dd09a:
The Kilo check failure was "Assistant request was rate limited" — infra flake, no findings. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
2 similar comments
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
dfc9e05 to
e06bcde
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@script/release-preflight.ts`:
- Around line 419-464: Make the beta dist-tag validation in the prerelease
branch fail closed when parsing distTags.stdout fails. Track parse success
separately from betaCurrent, and add a version sanity FAIL for an exit-code-zero
response with invalid JSON; only treat betaCurrent as absent and allow the
existing PASS path when the JSON parses successfully but contains no beta tag.
Preserve the existing nonzero-exit failure and beta monotonicity checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aea28076-cebe-4af0-8644-db1f0d42f23b
📒 Files selected for processing (4)
.github/workflows/ci.yml.github/workflows/release.ymlscript/release-preflight.test.tsscript/release-preflight.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/release.yml
- script/release-preflight.test.ts
| } else if (!currentParsed) { | ||
| add({ | ||
| name: "version sanity", | ||
| status: "FAIL", | ||
| detail: `could not parse current npm 'latest' version "${currentLatest}" — cannot verify monotonicity`, | ||
| }) | ||
| } else if (compareSemver(parsed, currentParsed) <= 0) { | ||
| add({ | ||
| name: "version sanity", | ||
| status: "FAIL", | ||
| detail: `${version} is not strictly greater than current npm latest (${currentLatest})`, | ||
| }) | ||
| } else if (parsed.prerelease) { | ||
| // A prerelease target must also move the `beta` dist-tag forward — | ||
| // being greater than stable `latest` is not enough. Publishing | ||
| // 0.10.0-beta.4 while `beta` points at 0.10.0-beta.5 would move new | ||
| // beta installs BACKWARD. | ||
| const distTags = run(["npm", "view", "@altimateai/altimate-code", "dist-tags", "--json"]) | ||
| let betaCurrent: string | null = null | ||
| if (distTags.code === 0) { | ||
| try { | ||
| betaCurrent = (JSON.parse(distTags.stdout) as Record<string, string>).beta ?? null | ||
| } catch { | ||
| betaCurrent = null | ||
| } | ||
| } | ||
| const betaParsed = betaCurrent ? parseSemver(normalizeVersion(betaCurrent)) : null | ||
| if (distTags.code !== 0) { | ||
| add({ | ||
| name: "version sanity", | ||
| status: "FAIL", | ||
| detail: `could not read npm dist-tags to compare against the current beta channel:\n${distTags.stderr || distTags.stdout}`, | ||
| }) | ||
| } else if (betaParsed && compareSemver(parsed, betaParsed) <= 0) { | ||
| add({ | ||
| name: "version sanity", | ||
| status: "FAIL", | ||
| detail: `${version} is not strictly greater than the current beta dist-tag (${betaCurrent}) — new beta installs would move backward`, | ||
| }) | ||
| } else { | ||
| add({ | ||
| name: "version sanity", | ||
| status: "PASS", | ||
| detail: `${version} > latest (${currentLatest})${betaCurrent ? ` and > beta (${betaCurrent})` : "; no beta dist-tag"}; not yet published; prerelease allowed`, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Beta dist-tag check fails open on a corrupted/unparseable npm response.
The distTags JSON parse failure is swallowed by the catch block, which just sets betaCurrent = null with no way to distinguish "no beta tag exists" from "couldn't parse the output." Only distTags.code !== 0 triggers the fail-closed FAIL branch (line 446); a parse exception with exit code 0 falls straight through to the PASS branch at line 458 with the message "no beta dist-tag," even though a beta tag may actually exist. This is exactly the "false-passing checks caused by trailing status output" failure mode issue #1031 calls out, reintroduced in the very check meant to prevent beta channel regressions. Contrast with the release-blocker PR check a few lines below (543-550), which correctly FAILs on unparseable gh JSON output instead of defaulting to a benign state.
🐛 Proposed fix to distinguish parse failure from a genuinely absent beta tag
let betaCurrent: string | null = null
+ let distTagsParseFailed = false
if (distTags.code === 0) {
try {
betaCurrent = (JSON.parse(distTags.stdout) as Record<string, string>).beta ?? null
} catch {
- betaCurrent = null
+ distTagsParseFailed = true
}
}
const betaParsed = betaCurrent ? parseSemver(normalizeVersion(betaCurrent)) : null
- if (distTags.code !== 0) {
+ if (distTags.code !== 0 || distTagsParseFailed) {
add({
name: "version sanity",
status: "FAIL",
- detail: `could not read npm dist-tags to compare against the current beta channel:\n${distTags.stderr || distTags.stdout}`,
+ detail: `could not read/parse npm dist-tags to compare against the current beta channel:\n${distTags.stderr || distTags.stdout}`,
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if (!currentParsed) { | |
| add({ | |
| name: "version sanity", | |
| status: "FAIL", | |
| detail: `could not parse current npm 'latest' version "${currentLatest}" — cannot verify monotonicity`, | |
| }) | |
| } else if (compareSemver(parsed, currentParsed) <= 0) { | |
| add({ | |
| name: "version sanity", | |
| status: "FAIL", | |
| detail: `${version} is not strictly greater than current npm latest (${currentLatest})`, | |
| }) | |
| } else if (parsed.prerelease) { | |
| // A prerelease target must also move the `beta` dist-tag forward — | |
| // being greater than stable `latest` is not enough. Publishing | |
| // 0.10.0-beta.4 while `beta` points at 0.10.0-beta.5 would move new | |
| // beta installs BACKWARD. | |
| const distTags = run(["npm", "view", "@altimateai/altimate-code", "dist-tags", "--json"]) | |
| let betaCurrent: string | null = null | |
| if (distTags.code === 0) { | |
| try { | |
| betaCurrent = (JSON.parse(distTags.stdout) as Record<string, string>).beta ?? null | |
| } catch { | |
| betaCurrent = null | |
| } | |
| } | |
| const betaParsed = betaCurrent ? parseSemver(normalizeVersion(betaCurrent)) : null | |
| if (distTags.code !== 0) { | |
| add({ | |
| name: "version sanity", | |
| status: "FAIL", | |
| detail: `could not read npm dist-tags to compare against the current beta channel:\n${distTags.stderr || distTags.stdout}`, | |
| }) | |
| } else if (betaParsed && compareSemver(parsed, betaParsed) <= 0) { | |
| add({ | |
| name: "version sanity", | |
| status: "FAIL", | |
| detail: `${version} is not strictly greater than the current beta dist-tag (${betaCurrent}) — new beta installs would move backward`, | |
| }) | |
| } else { | |
| add({ | |
| name: "version sanity", | |
| status: "PASS", | |
| detail: `${version} > latest (${currentLatest})${betaCurrent ? ` and > beta (${betaCurrent})` : "; no beta dist-tag"}; not yet published; prerelease allowed`, | |
| }) | |
| } | |
| const distTags = run(["npm", "view", "`@altimateai/altimate-code`", "dist-tags", "--json"]) | |
| let betaCurrent: string | null = null | |
| let distTagsParseFailed = false | |
| if (distTags.code === 0) { | |
| try { | |
| betaCurrent = (JSON.parse(distTags.stdout) as Record<string, string>).beta ?? null | |
| } catch { | |
| distTagsParseFailed = true | |
| } | |
| } | |
| const betaParsed = betaCurrent ? parseSemver(normalizeVersion(betaCurrent)) : null | |
| if (distTags.code !== 0 || distTagsParseFailed) { | |
| add({ | |
| name: "version sanity", | |
| status: "FAIL", | |
| detail: `could not read/parse npm dist-tags to compare against the current beta channel:\n${distTags.stderr || distTags.stdout}`, | |
| }) | |
| } else if (betaParsed && compareSemver(parsed, betaParsed) <= 0) { | |
| add({ | |
| name: "version sanity", | |
| status: "FAIL", | |
| detail: `${version} is not strictly greater than the current beta dist-tag (${betaCurrent}) — new beta installs would move backward`, | |
| }) | |
| } else { | |
| add({ | |
| name: "version sanity", | |
| status: "PASS", | |
| detail: `${version} > latest (${currentLatest})${betaCurrent ? ` and > beta (${betaCurrent})` : "; no beta dist-tag"}; not yet published; prerelease allowed`, | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@script/release-preflight.ts` around lines 419 - 464, Make the beta dist-tag
validation in the prerelease branch fail closed when parsing distTags.stdout
fails. Track parse success separately from betaCurrent, and add a version sanity
FAIL for an exit-code-zero response with invalid JSON; only treat betaCurrent as
absent and allow the existing PASS path when the JSON parses successfully but
contains no beta tag. Preserve the existing nonzero-exit failure and beta
monotonicity checks.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfc9e05e29
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # 5. Deterministic preflight (tag collisions, version sanity vs npm, prerelease | ||
| # ancestry, release-blocker PRs). For a beta cut from main use it as-is; for a | ||
| # branch beta the `base` check will FAIL by design — then rely on the manual | ||
| # tag verification in Step 4 instead. | ||
| bun script/release-preflight.ts --version X.Y.Z-beta.N --stage tag --allow-prerelease |
There was a problem hiding this comment.
When a beta is cut from a supported release branch that does not contain the latest origin/main, this --stage tag invocation always fails because the preflight requires origin/main to be an ancestor of HEAD. The surrounding instructions simultaneously say branch betas are supported, suggest disregarding this failure, and require stopping on any red gate, leaving the operator to either block a valid beta or bypass the supposedly mandatory preflight; add a branch-beta base mode that can pass while retaining the other gates.
Useful? React with 👍 / 👎.
| const ghMissing = gh.code !== 0 && /not found|ENOENT|no such file/i.test(gh.stderr) | ||
| if (ghMissing) { | ||
| add({ name: "release-blocker PRs", status: "WARN", detail: `gh CLI not installed — cannot check release-blocker PRs:\n${gh.stderr}` }) |
There was a problem hiding this comment.
When the release environment lacks gh, this mandatory blocker query becomes only a warning, and warnings do not affect the final exit status. A release can therefore pass preflight and push its irreversible tag while an open release-blocker PR exists; treat an unavailable CLI the same as authentication or network failures, or perform the query through another fail-closed mechanism.
Useful? React with 👍 / 👎.
| // ── 9. marker guard ────────────────────────────────────────────────────── | ||
| { | ||
| const guard = run(["bun", "run", "script/upstream/analyze.ts", "--markers", "--base", "origin/main", "--strict"]) | ||
| const fullOutput = [guard.stdout, guard.stderr].filter((s) => s.length > 0).join("\n") |
There was a problem hiding this comment.
During the required pre invocation, HEAD must equal origin/main, so passing --base origin/main makes the analyzer inspect an empty origin/main...HEAD diff; during tag, it inspects only locally created release commits. Consequently unmarked upstream-shared changes already merged since the previous release—such as changes admitted through an admin merge or a faulty CI run—are reported as clean by this release gate. Use the previous released tag (or another base covering the candidate's full release range) for the release-wide marker check.
Useful? React with 👍 / 👎.
| const currentLatest = npmView.stdout.trim() | ||
| const currentParsed = parseSemver(normalizeVersion(currentLatest)) | ||
| const alreadyPublished = run(["npm", "view", `@altimateai/altimate-code@${version}`, "version"]) | ||
| const isPublished = alreadyPublished.code === 0 && alreadyPublished.stdout.trim().length > 0 |
There was a problem hiding this comment.
If a previous interrupted or manual publish left this target version on a platform package or the unscoped altimate-code wrapper but not on @altimateai/altimate-code, this single-package query reports the version as unpublished. publish.ts then publishes the platform packages concurrently before the scoped and unscoped wrappers, so the duplicate can fail only after additional immutable package versions have been created, reproducing the partial-publish state the preflight is intended to prevent. Verify the target against every package the publish script emits, not only the scoped wrapper.
Useful? React with 👍 / 👎.
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e06bcde32e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const remoteLs = git(["ls-remote", "origin", `refs/tags/${targetTag}`]) | ||
| const remoteExists = remoteLs.code === 0 && remoteLs.stdout.trim().length > 0 |
There was a problem hiding this comment.
When the initial fetch succeeds but this follow-up git ls-remote fails due to a transient network or authentication error, remoteExists becomes false; if no local tag exists, the next branch incorrectly reports PASS even though the remote collision state is unknown. git ls-remote -h documents that --exit-code is what produces status 2 for no matching refs, so without that flag an empty successful result should mean “absent,” while any nonzero result must fail this advertised fail-closed gate.
Useful? React with 👍 / 👎.
| exit 1 | ||
| fi | ||
| VERSION="${CURRENT_TAG#v}" | ||
| if ! grep -q "\[$VERSION\]" CHANGELOG.md && [ "${CURRENT_TAG#*-}" = "$CURRENT_TAG" ]; then |
There was a problem hiding this comment.
For a stable release whose version merely appears elsewhere in CHANGELOG.md—for example in prose or a link while the new release heading was omitted—this search succeeds and allows publishing without the required entry. The unescaped dots in $VERSION are also regex wildcards, so even a malformed heading such as [0-9-3] satisfies a 0.9.3 release; match the exact ## [$VERSION] heading instead.
Useful? React with 👍 / 👎.
| exit 1 | ||
| } | ||
| # Annotated tags list a peeled ^{} line pointing at the commit; prefer it. | ||
| TAG_SHA=$(echo "$LS" | grep '\^{}' | cut -f1 | head -1) |
There was a problem hiding this comment.
For the lightweight tags created by git tag v... in both release guides, git ls-remote returns no peeled ^{} row. GitHub Actions runs this step with Bash's -e -o pipefail, so grep exits 1 and makes this assignment terminate the step before the fallback on the next line can run; consequently the new validation blocks every normally created release before npm publishing. Make the absent peeled row nonfatal or select the peeled/base row inside one conditional.
Useful? React with 👍 / 👎.
1 participant
PINEAPPLE
Summary
Hardens the
/releaseprocess based on a full-trace retro of the last three releases (v0.8.10, v0.9.1, v0.9.2). Each release hit at least one live failure that process should have caught:v0.9.1tag madegit tagsilently no-op and the push sent a tag pointing at a 2641-commit-divergent commit. Caught by vigilance, not process.0.7.3binary while releasing0.9.2, and burned 7 serial marker-guard fix rounds.publish-npmwas never gated on thetestjob — npm could publish with typecheck/tests red.Changes:
script/release-preflight.ts(+34 unit tests) — deterministic, fail-closed preflight gate: clean tree; HEAD == freshorigin/main(stagepre) or ancestor (stagetag); local+remote tag-collision check that never auto-deletes; version sanity vs npm (SemVer, monotonic, unpublished); prerelease-line ancestry (the missing check that turned v0.9.1 into a 17-hour merge side-quest); openrelease-blockerPRs;PREV_TAGdry-run (replicatesrelease.ymlselection); marker guard with full output in one pass..github/workflows/release.yml—publish-npmnowneeds: [test, build, sanity-verdaccio]; newValidate release tagstep (format, tag-SHA == checked-out SHA,CHANGELOG.mdentry for stable releases) runs BEFORE any publish; pre-publish smoke test asserts the exact tag version instead of just "binary starts"..claude/commands/release.md— preflight as the Step 2 gate; stop-never-improvise on structural failures; three-step verified commit/tag +git push --atomic origin HEAD:main v{V}; fail-closed tag rules; exit-status integrity rule (no trailing status echoes on gates); smoke-test the dist binary by path; deferred findings require durable issue links; P0-candidates verified or explicitly downgraded with rationale; every step visible (✅/⏭️/❌) in the final summary; persona reviewers deliver viaSendMessagebefore idling.docs/RELEASING.md— targeted staging instead ofgit add -A; atomic push with pre-push tag verification; honest wording: AUR publishing is disabled, Docker publish is best-effort.Test Plan
bun test script/release-preflight.test.ts— 34/34 pass (semver precedence incl. stable > prerelease, the exact v0.9.1PREV_TAGbug, malformed-tag rejection)bun script/release-preflight.ts --version 99.0.0 --stage precorrectly FAILs (branch ahead oforigin/main, exit 1);--stage tagPASSes 9/9--version 0.9.2correctly FAILs on tag collision (shows local+remote SHA) and already-published npm versionbun test test/branding/ test/install/(the CItestjob's exact scope, which assertsrelease.ymlcontents) — 442 pass / 0 fail--strictclean;release.ymlYAML validatedChecklist
Closes #1031
🤖 Generated with Claude Code
Summary by cubic
Tightens the release flow with a deterministic preflight gate, fail‑closed tag rules, strict SemVer, and stronger CI publish gates. Adds authoritative tag validation, exact‑version smoke checks, and blocks beta downgrades.
New Features
script/release-preflight.ts: clean worktree; HEAD == freshorigin/main(pre) ororigin/mainis ancestor (tag); fail‑closed local/remote tag collision check; strict SemVer (no leading zeros, no empty prerelease ids); npm reachability/unpublished enforced (fail‑closed); prerelease ancestry checked against HEAD; beta targets must be > currentbetadist‑tag;release-blockerPRs gate;PREV_TAGdry‑run; marker guard with full output; unit tests added./release-beta: runs preflight with--allow-prerelease; fail‑closed local/remote tag collision and “tag-at-HEAD” checks before pushing.Bug Fixes
publish-npmnow needstest;release.ymlvalidates tag format and SHA viagit ls-remote origin(incl.^{}peel) and requires a stable changelog entry before publish; pre‑publish smoke asserts the exact tag version;PREV_TAGpicks the highest merged stable strictly lower than the target; preflight tests run in CI.analyze.tsdegrades to pattern‑only due to a missing upstream remote. Docs (RELEASING.md,.claude/commands/release(-beta).md): preflight is the gate; verify tags before push; pushHEAD:mainatomically; targeted staging (no-A); run stage‑tagpreflight after the release commit; smoke‑test the built binary with an injected version; Docker is best‑effort and AUR is disabled.Written for commit e06bcde. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests