Repository navigation
fix(sandbox): make the office node modules loadable in every guest - #278
Merged
Merged
Conversation
The office layer copies the bundle's node-module trees in verbatim, which was
right for exactly as long as they were pure JS. The codev-office bundle is
gaining a fourth tree for the pptx skill's icon path (react + react-dom +
react-icons + sharp), and sharp loads a prebuilt libvips binary from an
@img/sharp-<os>-<cpu> optional dependency. The ubuntu bundle carries the
linux-x64 one, so on an arm64 guest require("sharp") would throw and the
skill's whole Icons section would be dead there - while the amd64 image
worked, which is the shape of bug that reaches users rather than CI.
Handled the way every other arch-specific component in this file already is:
the version is read off what the bundle proved, and only the binary is
re-fetched arch-correct. Two details are load-bearing. It resolves in a
scratch dir rather than installing into the shared tree, because `npm
install` in a node_modules dir with no package.json reifies the tree and
drops everything it cannot account for - PptxGenJS and Playwright included.
And it asserts @img/sharp-linux-<arch> by name afterwards, because a
--cpu that npm ignores produces a tree that looks correct and fails only in
the guest.
The whole block is inert until a bundle actually carries sharp, so this can
land before the bundle does.
Also adds the Node-side verification the layer never had. Everything else
here is verified by use - dotnet --list-sdks, soffice converting a document,
importing the Python set - but the module trees arrive through a wildcard
glob and nothing looked at them again, so a zip that failed to unpack
surfaced only when an agent tried to build a deck. It now compiles a
one-slide deck and reads it back with markitdown (fatal, matching the setup
script), and runs the react-icons -> SVG -> PNG round trip as a warning,
since the skill's own docs call icons optional.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The office tree was reachable two ways, and both can be absent at once. A
sandbox shell reported HOME=/root while every backend sets HOME=/home/codev
(codev-init, wsl/provider.ts, vz/provider.ts), so the boot environment had
not survived to it: no NODE_PATH, and no ~/.node_modules either, because
that symlink is seeded from /etc/skel into /home/codev and /root never got
one. require("pptxgenjs") then fails in an image that contains pptxgenjs.
What the agent does next is the part worth designing against. It goes
looking - a `find /` that the shell kills at 60s, an ls of
/usr/lib/node_modules that the tarball install never used, a `which` for a
library that has no bin - and then falls back to `npm install -g pptxgenjs`,
which cannot work twice over: the guest has no route off loopback, and npm -g
installs into $PREFIX/lib/node_modules, which is not on require()'s search
path anyway.
So add the third route: $PREFIX/lib/node, the entry in Node's global fallback
list that depends on neither the environment nor $HOME. Node derives it from
its own binary, so /usr/local/lib/node for the Dockerfile's tarball install,
and require() then resolves for any user, from any directory, with NODE_PATH
unset. The list is consulted only after the full node_modules walk-up, so a
workspace project's own dependencies still win; the one behaviour it changes
is that a project MISSING a dependency the office tree carries (react,
react-dom) resolves it from here instead of failing.
The verification added earlier now runs with NODE_PATH unset from an
unrelated directory, which is both the guest's worst case and the only way to
prove the new route rather than the old one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Two ways the office layer's node-module trees fail to load in a guest, one existing and one arriving with the next bundle. Both end the same way:
require("pptxgenjs")throws in an image that contains pptxgenjs, the agent goes looking for it, and it falls back tonpm install -g pptxgenjsin a guest with no route off loopback.1. Resolution depends on things that can be absent
The tree was reachable two ways, and both can be missing at once. A sandbox shell reported
HOME=/root, while every backend setsHOME=/home/codev(image/guest/codev-init:46,wsl/provider.ts:258,vz/provider.ts:27) — so the boot environment had not survived to it. NoNODE_PATH, and no~/.node_moduleseither, since that symlink is seeded from/etc/skelinto/home/codevand/rootnever gets one.What the agent does next is the part worth designing against — this is a real transcript:
then
npm install -g pptxgenjs, which could not have worked even with a network: npm -g installs into$PREFIX/lib/node_modules, which is not onrequire()'s search path.Fix: add the third route —
$PREFIX/lib/node, the entry in Node's global fallback list that depends on neither the environment nor$HOME. Node derives it from its own binary, so/usr/local/lib/nodefor the Dockerfile's tarball install. Verified with the tree in place,NODE_PATHempty, from an unrelated cwd — including transitive deps (react-dom→react,sharp→@img/*), since the tree is flat at the top level.The fallback list is consulted only after the full
node_moduleswalk-up, so a workspace project's own dependencies always win. The one behaviour this changes: a project missing a dependency the office tree happens to carry (react,react-dom) resolves it from here instead of failing.2. sharp ships the wrong architecture on arm64
The trees are copied in verbatim — correct for exactly as long as they are pure JS. The bundle is gaining a fourth tree for the pptx skill's icon path (quickbeard/codev-scripts#37), and sharp is native: it loads a prebuilt libvips binary from an
@img/sharp-<os>-<cpu>optional dependency. The ubuntu bundle carrieslinux-x64, so on an arm64 guestrequire("sharp")throws while the amd64 image works — the asymmetry that reaches users rather than CI.Handled like every other arch-specific component here (dotnet, pandoc, LibreOffice, wheels, Chromium): version from what the bundle proved, binary refetched arch-correct.
Two details are load-bearing:
npm installinside anode_modulesdir with nopackage.jsonreifies the tree from scratch and drops everything it cannot account for — PptxGenJS and Playwright included.@img/sharp-linux-<arch>by name afterwards. A--cpunpm ignores yields a tree that looks correct and throws only in the guest.Guarded on sharp being present, so it is inert until a bundle carries it.
Plus the Node-side verification this layer never had
Everything else here is verified by use —
dotnet --list-sdks, soffice converting a real document, importing the Python set. The module trees arrived through a wildcard glob and nothing looked at them again, so a zip that failed to unpack surfaced only when an agent tried to build a deck.It now compiles a one-slide deck and reads it back with markitdown (fatal, matching the setup script), and runs the react-icons → SVG → PNG round trip as a warning, since the skill's own docs call icons optional. Both run with
NODE_PATHunset from an unrelated directory — the guest's worst case, and the only way to prove the new route rather than the old one.Verification
bash -nclean.lib/node,NODE_PATHempty, unrelated cwd → full icon round trip, 991-byte PNG. (Node derives that path fromexecPathdifferently per platform — checked, not assumed.)sharp@0.35.3 --cpu=arm64lands@img/sharp-linux-arm64+@img/sharp-libvips-linux-arm64.Fork-only file, so no
DIVERGENCES.mdrow; no user-facing documented behaviour change, so noCHANGELOG.mdentry.Companion instruction-side change: quickbeard/codev-scripts#38 makes the skill check for its dependencies instead of installing them.
🤖 Generated with Claude Code