chore(repo): migrate from bun to pnpm - #129
Merged
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88ae719a35
ℹ️ 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".
haydenshively
added a commit
that referenced
this pull request
Aug 5, 2026
Addresses review on #129. - All three images dropped to the unprivileged `node` user. The oven/bun base ended with `USER bun`; node-slim defines `node` (uid 1000) but does not switch to it, so the base swap would have run a process holding a funded EOA key as root. `corepack enable` stays root (its shim lands in /usr/local/bin), then /repo is chowned and every later layer, install, and CMD runs as `node`. - CI installs pnpm with `pnpm/setup` v2 instead of `pnpm/action-setup`, which upstream now scopes to pnpm v10 and older. v2 also fetches pnpm's self-contained release binary against GitHub's published SHA-256 digest rather than bootstrapping through an `npm ci` of `@pnpm/exe` — one fewer npm artifact on the path this migration exists to shorten. Verified: all three images build; `docker run` reports uid=1000(node); the blue-liquidation container runs `prestart` (writing the gitignored @repo/contracts dist as non-root) and reaches its fail-loud config check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
haydenshively
added a commit
that referenced
this pull request
Aug 5, 2026
No file changes. After #129 and this branch were pushed in quick succession, GitHub cached mergeable_state=dirty for this PR even though its base (chore/migrate-to-pnpm@176a6a6) is a strict ancestor of this head — a fast-forward that cannot conflict. The stale state also suppressed the Checks workflow, which only fires on pull_request opened/synchronize. This empty commit forces both to recompute. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
haydenshively
force-pushed
the
chore/migrate-to-pnpm
branch
from
August 5, 2026 18:20
176a6a6 to
c39786f
Compare
cashd
approved these changes
Aug 7, 2026
pnpm 11.1.1 takes over installs, workspace resolution, the version catalog, and script running; bun 1.3.12 remains the runtime and test runner. pnpm's default-deny on dependency lifecycle scripts is the point of the migration, per the team supply-chain decision in #curators. Three settings the original migration branch omitted are set explicitly: strictDepBuilds makes an un-opted-in build script a hard install failure rather than a warning (proven: an esbuild probe exits 1 with ERR_PNPM_IGNORED_BUILDS); enablePrePostScripts keeps prestart alive, which builds @repo/contracts' gitignored dist/; and allowBuilds starts empty because nothing in the tree declares an install script. Catalog entries that a fresh resolution would have bumped are pinned to what bun.lock had resolved, so the migration rides no dependency upgrades. solc is pinned exactly because it fixes the compiled bytecode of the soltag lens templates and the Executor. The bot images move to a Node base carrying a copied bun binary: pnpm is activated through corepack, which oven/bun does not ship, and both runtimes must stay on PATH because start fires prestart. Test suite is untouched and byte-identical to main: 1412 pass, 3 skip, 3 fail, 1 error across 136 files on both (the failures are a pre-existing missing RPC_URL_8453 for the fork suites). CRTR-2822 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses review on #129. - All three images dropped to the unprivileged `node` user. The oven/bun base ended with `USER bun`; node-slim defines `node` (uid 1000) but does not switch to it, so the base swap would have run a process holding a funded EOA key as root. `corepack enable` stays root (its shim lands in /usr/local/bin), then /repo is chowned and every later layer, install, and CMD runs as `node`. - CI installs pnpm with `pnpm/setup` v2 instead of `pnpm/action-setup`, which upstream now scopes to pnpm v10 and older. v2 also fetches pnpm's self-contained release binary against GitHub's published SHA-256 digest rather than bootstrapping through an `npm ci` of `@pnpm/exe` — one fewer npm artifact on the path this migration exists to shorten. Verified: all three images build; `docker run` reports uid=1000(node); the blue-liquidation container runs `prestart` (writing the gitignored @repo/contracts dist as non-root) and reaches its fail-loud config check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…etup The switch to pnpm/setup failed every job at the Setup step: morpho-org's GitHub Actions allowlist permits `pnpm/action-setup@*` and not `pnpm/setup@*`. action-setup installs pnpm 11.1.1 correctly, so this reverts to it and records the constraint plus the follow-up (an org admin allowlisting the successor) in the action and the TIB. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
haydenshively
force-pushed
the
chore/migrate-to-pnpm
branch
from
August 7, 2026 15:55
c39786f to
1b7deea
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 1 of 2 for CRTR-2822. pnpm takes over installs, workspace resolution, the version catalog, and
script running. bun stays as the runtime and test runner, so this PR touches no test file and
bun testis unchanged. Part 2 removes bun (vitest, esbuild bundles, Node runtime).Splitting it this way lands the entire security motivation now, without waiting on a 136-file vitest
port.
Why
A npm supply-chain attack prompted a team-wide decision (Slack
#curators, 2026-07-17) tostandardize on pnpm. The deciding property is pnpm's default-deny on dependency lifecycle scripts: a
compromised transitive dependency cannot run arbitrary code at install time. bun has no equivalent.
The decision was already written up in
docs/decisions/TIB-2026-07-20-migrate-to-pnpm.md, whichlived only on the unmerged
claude/migrate-repo-pnpm-e5f4c1(3 commits, @cashd, 2026-07-20, no PR).That branch is 190 commits behind main and predates four workspaces, so this re-derives from it
rather than rebasing. The TIB is carried over here with Addendum A recording what changed.
Three settings the original branch omitted
strictDepBuilds: trueenablePrePostScripts: trueprestartbuilds@repo/contracts' gitignoreddist/; without this the bots cannot resolve their own workspace dependency.allowBuilds: {}{esbuild: true}— nothing in the current tree declarespreinstall/install/postinstall, and esbuild is absent entirely. It becomes the first entry in Part 2.No dependency upgrades ride along
Regenerating the lockfile re-resolves every caret range, which would contradict the TIB's non-goal.
Catalog entries that drifted are pinned to what
bun.lockhad resolved:solc0.8.35 → pinned. This one is not just parity: solc fixes the compiled bytecode of thesoltag
sol\``` lens templates and the Executor, so a silent bump would invalidate the recordedexplorer-verification settings (0.8.35, optimizer runs=200).
@internationalized/date3.12.1,date-fns4.1.0,@types/bun1.3.13 — pinned.one is
valibot1.4.1 → 1.4.2 (patch, via@morpho-org/viem-dlc).Docker
Keeping
FROM oven/bunwas not viable: pnpm is activated through corepack, which those images do notprovide, and
bun install --frozen-lockfileis no fallback oncebun.lockis deleted. All threeimages move to
node:24.14.1-slimwith the bun binary copied in. Both runtimes must stay on PATHin the final image for this PR, because
CMDfiresprestart, which shells out to pnpm. Part 2drops the bun binary and switches
CMDtonode dist/src/index.js.Evidence
esbuildprobe on a cleannode_modulesexits 1 withERR_PNPM_IGNORED_BUILDS; probe reverted, lockfile byte-identicalafter.
prestartstill runs — wipedpackages/contracts/dist/, ranpnpm --filter @morpho-org/blue-liquidation run start, watched pnpm firepnpm -r --parallel --if-present run buildand rebuild it before reaching the bot's own config check.1412 pass, 3 skip, 3 fail, 1 error, 7732 expect() calls, 1418 tests across 136 fileson both main (in a baseline worktree) and this branch. Same failingtest names. The 3 failures are a pre-existing missing
RPC_URL_8453for the fork suites; CIsupplies it as a secret.
pnpm install --frozen-lockfileon a clean checkout: clean.pnpm -r run typecheck12/12 Done ·pnpm lint0 warnings 0 errors ·pnpm knipclean ·pnpm formatclean.pnpm --filter @repo/contracts run generatere-run; the only diff in the tracked generated sourcesis the command string in the header.
--forwarding verified against the market-making CLI (--jsonis honored), before rewriting~22 README examples.
Containers run unprivileged
oven/bunends withUSER bun; the officialnodeimages define anodeuser (uid 1000) but donot switch to it, so the base swap would have promoted a process holding a funded liquidator key to
root. Each Dockerfile now runs
corepack enableas root (its shim lands in/usr/local/bin),chowns/repo, then drops toUSER node— every layer after that, the install, andCMDareunprivileged.
Docker is verified
All three images build,
docker run … idreportsuid=1000(node) gid=1000(node), and theblue-liquidation container runs
prestart— writing@repo/contracts' gitignoreddist/asnon-root — before reaching its fail-loud
Missing required env var: CHAIN_ID. Both pnpm 11.1.1 andbun 1.3.12 are on PATH in the final image, as this intermediate state requires.
Not verified
Nothing outstanding.
Follow-ups
bun:testfiles andBun.*calls andwill need a mechanical fix on rebase. feat(market-making): add docker image, compose, and docker hub publish #123 is the sharpest — it adds a bun-based
bots/market-making/Dockerfileplus a Docker Hub publish workflow needing a node-slim port.oxlinttogether withoxlint-tsgolint >= 0.24(pin comment left in the catalog).pnpm/setup@*, then switch the setup action to it. Upstream scopespnpm/action-setupto pnpm v10 and older and points v11+ atpnpm/setup, butmorpho-org'sActions allowlist permits only the former — switching fails every job at the Setup step. Beyond the
deprecation,
pnpm/setupv2 verifies pnpm's release binary against GitHub's published SHA-256digest instead of bootstrapping through an
npm ciof@pnpm/exe, which is the same threat modelthis PR is about.
action-setupinstalls pnpm 11.1.1 correctly meanwhile.🤖 Generated with Claude Code