fix: correct two dead bun.lockb references (EXSC-771) - #2199
Conversation
…ng workflows (EXSC-771) Both workflows called foundry-rs/foundry-toolchain directly, whose version input defaults to "stable". They therefore ran a floating foundry release while the repo pins 1.7.1 in .foundry-version, so a foundry release could change forge build output and flap the verify-clear-signing diff gate. Regenerating config/clearSigningProposal.json with the pinned 1.7.1 locally reproduces the committed artifact byte-for-byte (88 entries, empty diff). Ref .agents/rules/500-github-actions.md [CONV:FOUNDRY-SETUP]. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The path filter referenced bun.lockb, which no longer exists — the repo tracks bun.lock. Lockfile changes therefore never triggered the smoke test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 2 reviews per rolling hour; 0 remain after this review. WalkthroughThe pull request updates workflow and Dockerfile references from ChangesWorkflow and build configuration updates
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to The PR makes localized CI configuration updates, and no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
…e (EXSC-771) The COPY referenced bun.lockb, removed in #1767. Docker errors on a COPY whose source is absent, so the image could not build at all; the file has no in-repo consumers, so no CI check surfaced it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review-gate residual findings (not fixed here — need a decision)Auto-fixed during the gate: completed the 1. gh workflow run syncLedgerClearSigning.yml --ref chore/exsc-771-ci-cache-bun-install -f dry_run=trueNot run unilaterally because the job checks out the external Ledger registry fork with a sync token. Requesting a maintainer dispatch it, or explicit sign-off to proceed without it. 2. Unsuffixed fork-RPC cache key. (minor, pre-existing pattern) 3. 4. Procedural: |
…ar-signing workflows (EXSC-771)" This reverts commit 0efb7a1.
|
Scope narrowed: the foundry-pin commits were reverted here because #2196 (approved) already covers those two workflows. The residual findings in my earlier comment moved with them — raised on #2196 instead, along with the CI verification of the pin. This PR is now only the two dead |
Review-gate residual findings — round 2 (supersedes the comment above)The earlier residual comment on this PR is stale: items 1–4 there all concerned This round re-gated the narrowed diff. The code is clean — no changes were made to it. Two claims in the description were corrected (see the edited body), and one finding is escalated: 1. The Dockerfile still cannot build after this fix. (major, pre-existing and orthogonal — not introduced here)
Because the file has no in-repo consumer ( Decision needed: merge this as the scoped lockfile cleanup and track the Not blocking, for the record: CodeRabbit reported no actionable comments. The revert is complete — |
|
The Tracked in EXSC-773: delete the unused image, or rewrite it so it actually runs. Out of scope here — this PR only corrects the dead The round-2 residual comment above is closed by that ticket. |
Which Linear task belongs to this PR?
EXSC-771
Why did I implement it this way?
bun.lockbwas removed in #1767 (2026-05-07). Two references outlived it; a repo-widegit grep bun.lockbnow returns zero.1.
script/deploy/safe/ExecutePendingTimelockDockerfile:6—COPY package.json bun.lockb ./This one is hard-failing, not merely inert:
docker builderrors on aCOPYwhose source does not exist, and the next line isRUN bun install --frozen-lockfile --production, which would have no lockfile even if theCOPYwere tolerant. The file has no in-repo consumers (grep -rn ExecutePendingTimelockacross*.yml/*.sh/*.ts/*.json→ nothing; there is nodocker build, compose file or Makefile target anywhere in the repo), so it is hand-built or driven externally and no CI check would ever surface this.This fix removes the first of at least two blockers — the image still does not build. I ran
docker buildon both versions. Pre-fix it dies at step 3/9,"/bun.lockb": not found. Post-fix it gets past theCOPYand dies one step later:usbis a native addon pulled in transitively by@ledgerhq/hw-transport-node-hid(package.json:134) — a runtime dependency, so--productionkeeps it — andoven/bun:1-alpinecarries neither Python nor a C toolchain to compile it. That blocker is orthogonal and pre-existing. Even a green install would not run: the Dockerfile copies four paths while the script imports many more local modules. Tracked in EXSC-773 (delete vs rewrite); out of scope here.2.
.github/workflows/deploy-smoke-test.yml:56— path filter onbun.lockbSilently dead — the entry matches no tracked path, so it could never fire. It was added in #1791 eight days after the lockfile was deleted, so it was never correct.
validateScripts.yml:64andts-unit-tests.yml:55already spell itbun.lock, so this aligns with both siblings rather than inventing a convention.The real gap is narrower than "lockfile changes never triggered the smoke test":
package.jsonis in the same filter list (:55), and a lockfile change usually rides along with one. What slipped through werebun.lock-only changes — transitive re-resolution,bunLockfileCheck.ymlresyncs. The entry is load-bearing rather than decorative::31exportssmoke: ${{ steps.filter.outputs.smoke }}and:70gates the heavy job on it, while branch protection reads thesmoke-test-requiredaggregator (:81+) which treatsskippedas green — so a dead filter meant a silent false-green.Scope note
This PR originally also switched
verifyClearSigning.ymlandsyncLedgerClearSigning.ymlfromfoundry-rs/foundry-toolchainto./.github/actions/setup-foundry. That work is already done and approved in #2196 (@gvladika), which additionally unfreezes the sync workflow — so those commits were reverted here to avoid a conflicting duplicate. #2196 should land; this PR is deliberately narrowed to thebun.lockbcleanup, which #2196 does not touch.Supporting evidence gathered for that change, in case it is useful to #2196's security review:
foundry-rs/foundry-toolchain'sversioninput defaults to"stable"at the pinned SHA, so both workflows were running a floating foundry against a repo pin of1.7.1— and regeneratingconfig/clearSigningProposal.jsonunder 1.7.1 reproduces the committed artifact byte-for-byte (88 entries, empty diff), verified in CI on this branch before the revert (cold cache, full 206-file recompile,verify-clear-signinggreen, log lineInstalling Foundry (version: 1.7.1)).The third instance of that same violation,
runPendingTimelockTXs.yml, is not covered by #2196 and is tracked in EXSC-772, along with a better fix for it: one runtimetypechainimport in a demo-script helper forcesbun typechain(38s) on every 10-minute tick, and removing that coupling drops foundry from the funded-key job entirely rather than pinning it.The leftover Dockerfile (still unbuildable after the lockfile rename, no in-repo consumer) is tracked in EXSC-773 and is not part of this PR.
What CI here does and does not prove
deploy-smoke-testis skipped on drafts, and this PR does not touchbun.lock, so the corrected filter entry is never the matching rule. The Dockerfile has no CI consumer at all. Both are one-token corrections of references to a file that demonstrably does not exist (git ls-files | grep bun.lock→bun.lockonly), with two in-repo precedents for the spelling.docker buildwas run on both the pre-fix and post-fix Dockerfile (results above). It confirms theCOPYfailure was real and that this change clears it — and also that the image still fails afterwards on the unrelated native-addon build. No.dockerignoreexists in the repo, so nothing excludesbun.lockfrom the build context.Checklist before requesting a review
COPYChecklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)