Repository navigation
chore(bestax-mcp): restamp index after release - #991
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe catalog’s ChangesCatalog version update
Priority: ➖ Normal Estimated code review effort: 1 (Trivial) | ~2 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to This metadata update matches the package’s current version. No merge-blocking risk is established. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Preview DeploymentPreview URL: https://5a54741e.bestax.pages.dev |
AI triage — issues this PR may resolve
If this PR resolves one of these, add the line below to the PR description so the issue closes on merge: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Deep review — 0 blocking · 1 advisory
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔵 Advisory | Robustness | Restamp push has no fetch/rebase retry and no concurrency: guard; the race that caused this is untracked |
bestax-mcp/data/catalog.json:5 |
Overall: Sound, and verified rather than inferred. The committed file is bit-for-bit what gen:mcp:check produces on main — the failing run on 77e5c6d5 emitted the identical blob pair (e63cf4e2..565d1710) and the identical one-line hunk, so nothing else under bestax-mcp/data/ is stale alongside the version stamp. A chore scoped to bestax-mcp releases nothing ({ type: "chore", release: false } in bestax-mcp/release.config.js), so the claim in the PR body holds. The human should merge this promptly rather than scrutinise it: while it sits open, every branch cut from main fails a check on a file it never touched.
Residual risk:
- Recurrence of the push rejection — open. The
Commit and push the MCP index restampstep inci.ymlpushes once with nofetch/rebaseand no retry,ci.ymldeclares noconcurrency:, andpublishchecks out the triggering SHA. Any merge landing during a release repeats this exactly. Posted as finding 1; it is a.github/**fix, so maintainer-only. - The generator test cannot catch committed staleness — refuted as already-covered.
scripts/gen-mcp-index.test.mjsassertscatalog.generatedFrom.version === pkg.versionagainst the in-memorybuild()result, so it is always true and never reads the committed file. Checked: the CI diff gate is the only guard, and release commits carry[skip ci]somainnever evaluates it. That is exactly the gap the restamp comment inci.ymlalready documents (#518/#520), not a new one. - Other generated artefacts left stale by the same release — refuted. Grepped the tree:
5.26.6survives only inbulma-ui/CHANGELOG.md(history, correct).eslint-plugin/src/generated/metadata.tsand the skill component catalog embed no library version, andgen:catalog:checkpassed in the same failing run, immediately beforegen:mcp:checkaborted it. - Whether the stale stamp does user-visible harm — confirmed real, and fixed here.
bestax-mcp/src/version.tsfeedscatalog.generatedFrom.versionintocompare(indexed, installed), andserver.tsputs it in the server instructions, so5.26.6against a5.27.0project yields a spurious minor-drift warning. The already-published package still carries the old stamp — that half is tracked by #932, which this PR does not and need not address. - Staleness of this PR itself — refuted for now.
bulma-ui/package.jsononmainis5.27.0; anotherbulma-uirelease landing before merge would re-stamp it, but none is pending. - Not verified by me:
Build and Teston9414bb9cwas stillin_progresswhen this review was written. The log evidence above makes itsgen:mcp:checkoutcome determined rather than assumed.
🏄 One line, one version bump, and it is the exact line CI already told us it wanted — same blob hash and everything, dude. Pure paddle-out-and-catch-it: merge it and
maingoes green again. Only bummer is that the wave will break the same way next release unless someone fixes that push inci.yml.
|
deep-review: verify Every open thread has a reply: fixed, with the commit, or refuted with a reason. This settles them on 9414bb9. |
There was a problem hiding this comment.
Deep review (verify) — 1 resolved · 0 open
| # | Thread | Outcome | Location |
|---|---|---|---|
| 1 | 🔵 Advisory · Robustness — the restamp push race is unautomated and untracked | Accepted and resolved — reasoned refutation from an admin, fix tracked in open #993 | bestax-mcp/data/catalog.json:5 |
Overall: This pass settled threads and reviewed no commits. The one open thread was my own Advisory note that this data-only restamp cannot close the ci.yml push race that made it necessary. The PR author (role admin, verified live) replied that the scope is deliberate — this PR is only the commit the release job failed to push, so main can go green — and that making the restamp survive merges during a release is a human-authored .github/ change tracked in #993. I read #993: it is open and describes exactly that race, including the fetch/rebase/retry and concurrency: options. The reason holds on both counts the rules allow (a deliberate choice, and a fix tracked where the reply names it), so the thread is accepted and resolved rather than re-raised. Nothing is left for the human here beyond merging.
Residual risk: none newly identified in this pass — a verify pass raises no findings. For the record, re-checked while settling the thread: bestax-mcp/data/catalog.json now stamps generatedFrom.version: 5.27.0, matching the 5.27.0 in bulma-ui/package.json, and the bestax-mcp suite is green (7 suites, 313 tests), so the restamp itself is consistent. The underlying push race stays open, and #993 is where it is tracked.
🏄 One-line restamp, clean set, nothing gnarly in the water. That thread about the release-job wipeout got paddled over to #993 where it belongs — this one is good to go.
github-actions
Bot
added
the
review-converged
allxsmith
added
the
needs-human-review
github-actions
Bot
removed
the
review-converged
📸 Story screenshots at handoff —
|
github-actions
Bot
added
the
review-converged
|
🎉 This PR is included in version 5.27.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 4.2.14 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.25.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.14.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
The publish job regenerated the MCP index after every release, so a run that released bulma-ui and bestax-mcp together shipped bestax-mcp with an index stamped one bulma-ui release behind (#932). "Regenerate the MCP index" now runs after Semantic Release (bulma-ui) and before Semantic Release (bestax-mcp). bestax-mcp's publish packs data/ from the working tree, so the tarball carries this run's version. The release commit leaves data/ alone (its git assets name nothing there), and the last step still commits the restamp to main. That last step pushed once and gave up, so a merge landing during the release left main's index stale and every branch red (#993, #991). It now retries a bounded number of times with a short, growing wait, and replays the restamp commit onto main's fetched tip before each retry. It fails loudly when the replay conflicts, when bulma-ui's version on main is no longer the one it stamped, and after the last attempt. Invariants checked (.github/CLAUDE.md, middle row): - Token split: the release App token is still in no step that runs repository code. The regenerate step moved with its env unchanged, and the push step runs only git, jq, seq and sleep. It replays rather than regenerates for exactly this reason. core.hooksPath=/dev/null now reaches every git command in the step through GIT_CONFIG_COUNT, so the push, fetch, reset and cherry-pick run no hook either; the old push ran husky's pre-push stub. - Permissions: unchanged. The job keeps `contents: read` and `id-token: write`, and the token still reaches the step through env. - Triggers: unchanged, as is the publish job's `if:`. - Rule 1: no action added or changed; both pin greps come back clean. - Pushes stay fast-forward only, the skip-CI marker, signed (-S on the commit and the cherry-pick) and by the same identity. - Rule 9: the loop stays in the workflow, since a script would be repo code beside the token. scripts/mcp-restamp.test.mjs reads the step out of ci.yml and runs it against scratch remotes instead. - I1/I2: not in play; this job holds no model token and posts nothing.
2 participants
The 5.27.0 release regenerated the MCP index, but its push of the restamp to main was rejected as non-fast-forward, because more PRs merged while the release job ran. Main's index still says it was built from 5.26.6, so
gen:mcp:checkfails on main and on every PR merged with it.This is the commit the release job would have pushed:
pnpm gen:mcpon current main, which changes onlygeneratedFrom.versioninbestax-mcp/data/catalog.jsonfrom 5.26.6 to 5.27.0. It is achore, so it releases nothing.pnpm allpasses locally.Summary by CodeRabbit