From 946edfb67c4e0e9d506b4fd196ef1a92c574ebbe Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Jul 2026 20:48:52 +0000 Subject: [PATCH] refactor(state): dedup lockfile/bundle discovery gates into shared absorb helpers Follow-up polish to #56, from its review findings. The State constructor carried the artifact-loading gates twice: once in the discovery loop (a committed rootDir) and once, copy-pasted, in the post-loop fallbacks for explicit (rootDir-independent) paths -- the lockfile pair was even flagged in-code ("a future refactor could merge the two by extracting a helper"), and #56 added a second such pair for the bundle side. A future change to mode gating edited only in the obvious in-loop branch would silently leave the explicit-path fallbacks -- exactly the paths #56 exists to protect -- on stale semantics. Extract both into shared helpers: - #absorbLockfile(lock, lockPath): the lock='none' guard + parse/populate + frozen warnings; returns lockfileLoaded. - #loadBundleArtifacts(sources, sourcesPath, lockfileLoaded): the bundle='none' and missing-lockfile guards, #absorbCodeBundle, and the split-resources branch. Two deliberate, commented semantic notes: - The in-loop missing-lockfile guard now tests `lockfileLoaded` (post-absorb) where it previously tested raw lockfile presence (pre-absorb). Equivalent whenever the guard can fire (useLockfile && !replaceLockfile), since #absorbLockfile absorbs the lockfile iff it exists; the only observable difference is error PRECEDENCE in doubly-broken setups (e.g. a corrupt lockfile now reports its parse error before a bundle='none' violation). - The post-loop fallback now also fires for an explicit resourcesBundleFile WITHOUT a bundleFile, absorbing the resources half -- the resources-only split shape #absorbResourcesBundle documents as legal. Previously the block was gated on explicitBundlePath alone and silently skipped it. Consistency fix: no CLI-reachable byte-serving change found (a resources bundle only carries workspace bytes in full scope, where a load needs the code bundle anyway -- which is itself a root signal). Also documents the post-loop root caveat surfaced in review: the outermost root depends on the upward-walk boundary (PROJECT_CWD, .git, pnpm-workspace.yaml), which capture and load must agree on. Test polish (same review): the monorepo regression test now pins the capture side (bundle exists + carries the hoisted dep under its monorepo-root-relative key) instead of only the near-tautological save stdout; its header comment covers both Node-version-dependent pre-fix symptoms (crash vs silent tampered-disk read); and cli.test.js's cleanEnv strips the remaining stasis env vars Config reads (RESOURCES_BUNDLE_FILE, RESOURCES, FS) so ambient values can't leak into spawned runs. No behavior change intended outside the two notes above; the full suite passes on Node 24.14 and 26.3. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UenCGinFJrGDyt8E7XEftP --- stasis-core/src/state.js | 232 ++++++++++++++++++--------------------- tests/cli.test.js | 16 ++- 2 files changed, 123 insertions(+), 125 deletions(-) diff --git a/stasis-core/src/state.js b/stasis-core/src/state.js index bf54a1d..686ce7e 100644 --- a/stasis-core/src/state.js +++ b/stasis-core/src/state.js @@ -444,71 +444,8 @@ export class State { : lockProbe const lockPath = explicitLockPath || join(rootDir, FILE_LOCK) - if (lock && !this.config.useLockfile && !this.config.ignoreLockfile) { - throw new Error(`Unexpected ${lockPath} with config.lock = 'none'`) - } - if (sources && !this.config.writeBundle && !this.config.loadBundle && !this.config.ignoreBundle && !this.config.frozenBundle) { - throw new Error(`Unexpected ${sourcesPath} with config.bundle = 'none'`) - } - - // A frozen bundle is self-attesting -- it verifies disk against the bundle's - // own bytes, so (unlike load/add) it needs no sibling lockfile and is exempt - // from this "a lockfile is required before a bundle's sources can be trusted" - // guard. This is what lets bundle=frozen compose with lock=add (bootstrapping a - // fresh lockfile from the verified run) without a pre-existing lockfile. - if (sources && !lock && this.config.useLockfile && !this.config.replaceLockfile && !this.config.frozenBundle) { - throw new Error('stasis.lock.json missing, can not use sources') - } - - if (lock && this.config.useLockfile && !this.config.replaceLockfile) { - const lockfile = Lockfile.parse(lock) - if (this.config.frozen) assert.equal(lockfile.config.scope, this.config.scope) - - const includeSources = lockfile.config.scope === 'full' && this.config.full - for (const [dir, info] of lockfile.modules) { - if (!dir.includes('node_modules') && !includeSources) continue - this.modules.set(dir, info) - } - if (includeSources) this.entries = lockfile.entries - - for (const [dir, { files }] of this.modules) { - for (const [name, hash] of Object.entries(files)) { - noupsert(this.hashes, join(dir, name), hash) - } - } - this.#lockImports = lockfile.imports - this.#lockFormats = lockfile.formats - // Frozen promises verification, but a lockfile predating resolution - // or format attestation can only vouch for bytes -- those metadata - // (from a bundle OR observed on disk) go unchecked until it's - // regenerated. - if (this.config.frozen && this.#lockImports === null) { - console.warn('[stasis] Warning: lockfile does not attest resolutions; they are trusted as-is. Regenerate the lockfile to enable this check.') - } - if (this.config.frozen && this.#lockFormats === null) { - console.warn('[stasis] Warning: lockfile does not attest formats; they are trusted as-is. Regenerate the lockfile to enable this check.') - } - lockfileLoaded = true - this.#lockfileLoaded = true - } - - if (sources && (this.config.writeBundle || this.config.loadBundle || this.config.frozenBundle) && !this.config.replaceBundle) { - this.#absorbCodeBundle(sources, sourcesPath, lockfileLoaded) - } - - // Split-bundle layout: when `resourcesBundleFile` is configured, resources - // live in a separate file. Load it as a standalone Bundle (entries empty, - // imports empty, formats restricted to resource tags) and union into the - // state already absorbed from `bundleFile`. The strict-shape asserts catch - // a writer that accidentally leaked code into the resources file or a - // tampered file claiming entries/imports it shouldn't have. - if (this.config.resourcesBundleFile) { - const resourcesPath = this.config.resourcesBundleFile - const resourcesData = readFileSyncMaybe(dirname(resourcesPath), basename(resourcesPath)) - if (resourcesData && (this.config.writeBundle || this.config.loadBundle || this.config.frozenBundle) && !this.config.replaceBundle) { - this.#absorbResourcesBundle(resourcesData, { lockfileLoaded, resourcesPath }) - } - } + lockfileLoaded = this.#absorbLockfile(lock, lockPath) + this.#loadBundleArtifacts(sources, sourcesPath, lockfileLoaded) // The innermost matching rootDir is authoritative (per-dir opt-in, see above), so stop // scanning once committed. An explicit/config `bundleFile` is rootDir-independent: without @@ -522,67 +459,33 @@ export class State { // Explicit lockfile path with no other discovery indicator at any rootDir: the // discovery loop never set `loaded`, so the lockfile branch above never ran. Process // the explicit lockfile here against the already-resolved `this.root` (outermost - // package.json) so the run still has its attestation. Inlines the same gates as the - // in-loop branch -- a future refactor could merge the two by extracting a helper. + // package.json) so the run still has its attestation. if (!loaded && explicitLockPath) { const lock = readFileSyncMaybe(dirname(explicitLockPath), basename(explicitLockPath), 'utf-8') - if (lock && !this.config.useLockfile && !this.config.ignoreLockfile) { - throw new Error(`Unexpected ${explicitLockPath} with config.lock = 'none'`) - } - if (lock && this.config.useLockfile && !this.config.replaceLockfile) { - const lockfile = Lockfile.parse(lock) - if (this.config.frozen) assert.equal(lockfile.config.scope, this.config.scope) - const includeSources = lockfile.config.scope === 'full' && this.config.full - for (const [dir, info] of lockfile.modules) { - if (!dir.includes('node_modules') && !includeSources) continue - this.modules.set(dir, info) - } - if (includeSources) this.entries = lockfile.entries - for (const [dir, { files }] of this.modules) { - for (const [name, hash] of Object.entries(files)) { - noupsert(this.hashes, join(dir, name), hash) - } - } - this.#lockImports = lockfile.imports - this.#lockFormats = lockfile.formats - if (this.config.frozen && this.#lockImports === null) { - console.warn('[stasis] Warning: lockfile does not attest resolutions; they are trusted as-is. Regenerate the lockfile to enable this check.') - } - if (this.config.frozen && this.#lockFormats === null) { - console.warn('[stasis] Warning: lockfile does not attest formats; they are trusted as-is. Regenerate the lockfile to enable this check.') - } - lockfileLoaded = true - this.#lockfileLoaded = true - } - } - - // Explicit (flag/env) bundleFile with no rootDir-dependent indicator at any rootDir: like the - // explicit lockfile above, it was suppressed as a root-detection signal so it couldn't pull the - // root down to the innermost package, so the loop never loaded it. Load it here against - // `this.root` (the outermost package.json) -- the same root capture commits to when no signal - // exists -- so its capture-root-relative keys line up with how they're looked up. Inlines the - // same gates as the in-loop bundle branch. - if (!loaded && explicitBundlePath) { - const sourcesPath = this.config.bundleFile - const sources = readFileSyncMaybe(dirname(sourcesPath), basename(sourcesPath)) - if (sources && !this.config.writeBundle && !this.config.loadBundle && !this.config.ignoreBundle && !this.config.frozenBundle) { - throw new Error(`Unexpected ${sourcesPath} with config.bundle = 'none'`) - } - if (sources && !lockfileLoaded && this.config.useLockfile && !this.config.replaceLockfile && !this.config.frozenBundle) { - throw new Error('stasis.lock.json missing, can not use sources') - } - if (sources && (this.config.writeBundle || this.config.loadBundle || this.config.frozenBundle) && !this.config.replaceBundle) { - this.#absorbCodeBundle(sources, sourcesPath, lockfileLoaded) - } - // Split-bundle resources: same post-loop treatment (resourcesBundleFile is likewise an - // explicit, rootDir-independent path). Mirrors the in-loop resources branch. - if (this.config.resourcesBundleFile) { - const resourcesPath = this.config.resourcesBundleFile - const resourcesData = readFileSyncMaybe(dirname(resourcesPath), basename(resourcesPath)) - if (resourcesData && (this.config.writeBundle || this.config.loadBundle || this.config.frozenBundle) && !this.config.replaceBundle) { - this.#absorbResourcesBundle(resourcesData, { lockfileLoaded, resourcesPath }) - } - } + lockfileLoaded = this.#absorbLockfile(lock, explicitLockPath) + } + + // Explicit (flag/env) bundleFile and/or resourcesBundleFile with no rootDir-dependent + // indicator at any rootDir: like the explicit lockfile above, an explicit bundleFile was + // suppressed as a root-detection signal so it couldn't pull the root down to the innermost + // package, so the loop never loaded it. Load it here against `this.root` (the outermost + // package.json) -- the same root capture commits to when no signal exists -- so its + // capture-root-relative keys line up with how they're looked up. `resourcesBundleFile` is in + // the gate for the resources-only split shape (#absorbResourcesBundle documents it as legal): + // with no code bundle on disk there is no other reason to reach the loader here, yet the + // explicit resources half must still be absorbed. Post-loop, both paths are necessarily + // flag/env: a stasis.config.json could only have set them by also committing the loop. + // + // NB: the outermost root depends on where the upward walk stopped (PROJECT_CWD, .git, + // pnpm-workspace.yaml -- see potentialRoots above). Capture and load must agree on that + // boundary for the keys to line up: e.g. a capture under yarn (PROJECT_CWD set to an inner + // workspace dir) and a bare load of the same bundle (walks up to the repo's .git) commit + // different roots and fail with "outside the project root" / "not attested". + if (!loaded && (explicitBundlePath || this.config.resourcesBundleFile)) { + const sources = explicitBundlePath + ? readFileSyncMaybe(dirname(explicitBundlePath), basename(explicitBundlePath)) + : null + this.#loadBundleArtifacts(sources, explicitBundlePath, lockfileLoaded) } // Frozen modes must have actually loaded their attestation. Asserting here -- @@ -624,6 +527,87 @@ export class State { liveStates().add(this) } + // Absorb a lockfile's attestation into this State: the run-scope module/hash maps, the + // imports/formats attestation, and the frozen-mode warnings for lockfiles predating those + // attestations. Shared by the discovery loop (a committed rootDir) and the post-loop fallback + // for an explicit `config.lockFile` -- a rootDir-independent path, so it is suppressed as a + // root-detection signal and loaded at the outermost root instead. Returns whether the lockfile + // was actually absorbed (the caller's `lockfileLoaded`). + #absorbLockfile(lock, lockPath) { + if (lock && !this.config.useLockfile && !this.config.ignoreLockfile) { + throw new Error(`Unexpected ${lockPath} with config.lock = 'none'`) + } + if (!lock || !this.config.useLockfile || this.config.replaceLockfile) return false + const lockfile = Lockfile.parse(lock) + if (this.config.frozen) assert.equal(lockfile.config.scope, this.config.scope) + + const includeSources = lockfile.config.scope === 'full' && this.config.full + for (const [dir, info] of lockfile.modules) { + if (!dir.includes('node_modules') && !includeSources) continue + this.modules.set(dir, info) + } + if (includeSources) this.entries = lockfile.entries + + for (const [dir, { files }] of this.modules) { + for (const [name, hash] of Object.entries(files)) { + noupsert(this.hashes, join(dir, name), hash) + } + } + this.#lockImports = lockfile.imports + this.#lockFormats = lockfile.formats + // Frozen promises verification, but a lockfile predating resolution + // or format attestation can only vouch for bytes -- those metadata + // (from a bundle OR observed on disk) go unchecked until it's + // regenerated. + if (this.config.frozen && this.#lockImports === null) { + console.warn('[stasis] Warning: lockfile does not attest resolutions; they are trusted as-is. Regenerate the lockfile to enable this check.') + } + if (this.config.frozen && this.#lockFormats === null) { + console.warn('[stasis] Warning: lockfile does not attest formats; they are trusted as-is. Regenerate the lockfile to enable this check.') + } + this.#lockfileLoaded = true + return true + } + + // Gate and load the bundle artifacts for an already-committed root: the unified + // code[+resource] bundle read from `sourcesPath`, plus the split `resourcesBundleFile` when + // one is configured. Shared by the discovery loop and the post-loop explicit-path fallback so + // the mode guards cannot drift between the two. `sources` may be null -- no bundle on disk, or + // a resources-only deployment -- in which case only the resources half applies. + // + // The missing-lockfile guard tests `lockfileLoaded` where the pre-extraction in-loop code + // tested raw lockfile presence; equivalent at this call point, since whenever the guard can + // fire (useLockfile && !replaceLockfile) #absorbLockfile absorbed the lockfile iff it existed. + #loadBundleArtifacts(sources, sourcesPath, lockfileLoaded) { + if (sources && !this.config.writeBundle && !this.config.loadBundle && !this.config.ignoreBundle && !this.config.frozenBundle) { + throw new Error(`Unexpected ${sourcesPath} with config.bundle = 'none'`) + } + // A frozen bundle is self-attesting -- it verifies disk against the bundle's + // own bytes, so (unlike load/add) it needs no sibling lockfile and is exempt + // from this "a lockfile is required before a bundle's sources can be trusted" + // guard. This is what lets bundle=frozen compose with lock=add (bootstrapping a + // fresh lockfile from the verified run) without a pre-existing lockfile. + if (sources && !lockfileLoaded && this.config.useLockfile && !this.config.replaceLockfile && !this.config.frozenBundle) { + throw new Error('stasis.lock.json missing, can not use sources') + } + if (sources && (this.config.writeBundle || this.config.loadBundle || this.config.frozenBundle) && !this.config.replaceBundle) { + this.#absorbCodeBundle(sources, sourcesPath, lockfileLoaded) + } + // Split-bundle layout: when `resourcesBundleFile` is configured, resources + // live in a separate file. Load it as a standalone Bundle (entries empty, + // imports empty, formats restricted to resource tags) and union into the + // state already absorbed from `bundleFile`. The strict-shape asserts catch + // a writer that accidentally leaked code into the resources file or a + // tampered file claiming entries/imports it shouldn't have. + if (this.config.resourcesBundleFile) { + const resourcesPath = this.config.resourcesBundleFile + const resourcesData = readFileSyncMaybe(dirname(resourcesPath), basename(resourcesPath)) + if (resourcesData && (this.config.writeBundle || this.config.loadBundle || this.config.frozenBundle) && !this.config.replaceBundle) { + this.#absorbResourcesBundle(resourcesData, { lockfileLoaded, resourcesPath }) + } + } + } + // Absorb a unified code+resource bundle's metadata and payloads into this State. Shared by the // discovery loop (a committed rootDir carrying a stasis signal) and the post-loop fallback that // loads an explicit (rootDir-independent) bundleFile against the outermost root. diff --git a/tests/cli.test.js b/tests/cli.test.js index 0698910..e7a4c73 100644 --- a/tests/cli.test.js +++ b/tests/cli.test.js @@ -22,6 +22,9 @@ const { EXODUS_STASIS_SCOPE: _s, EXODUS_STASIS_BUNDLE: _b, EXODUS_STASIS_BUNDLE_FILE: _bf, + EXODUS_STASIS_RESOURCES_BUNDLE_FILE: _rbf, + EXODUS_STASIS_RESOURCES: _r, + EXODUS_STASIS_FS: _fs, EXODUS_STASIS_DEBUG: _d, EXODUS_STASIS_PID: _pid, EXODUS_STASIS_CHILD_PROCESS: _cp, @@ -276,7 +279,10 @@ test('run --bundle=load with an explicit --bundle-file resolves the root consist // and Node's ESM->CJS translator re-resolved it through Module._load(absPath, /* no parent */), // which fell through hooks.js's `typeof parent?.filename === 'string'` guard to native disk // resolution -- `Cannot find module '/node_modules/dep/index.js'` once node_modules was - // pruned/not shipped. Choosing the root without the explicit bundleFile biasing it keeps capture + // pruned/not shipped, or (on Node versions whose translator takes the served-source path) a + // SILENT read of the on-disk copy instead of a crash. The load assertions below cover both + // symptoms: `status === 0` catches the crash, `doesNotMatch(/TAMPERED/)` the silent disk read. + // Choosing the root without the explicit bundleFile biasing it keeps capture // and load agreed, so the hoisted dep stays in-root and is served from the bundle. // // Layout yields potentialRoots = [packages/app, ] (the .git at stops the upward walk). @@ -298,6 +304,14 @@ test('run --bundle=load with an explicit --bundle-file resolves the root consist const save = run(['run', '--lock=none', '--dependencies', '--bundle=add', `--bundle-file=${bundlePath}`, 'index.mjs'], { cwd: app }) t.assert.equal(save.status, 0, `save stderr: ${save.stderr}`) t.assert.match(save.stdout, /DEP-FROM-BUNDLE/) + // Pin the capture side too: stdout alone would pass even if the dep never landed in the + // bundle (disk still holds the original bytes here), and the load step would then fail with + // an opaque "not attested" instead of pointing at the capture. The key must be relative to + // the OUTER (monorepo) root -- that is the very thing the root fix pins down. + t.assert.ok(existsSync(bundlePath), 'bundle written at the explicit path') + const bundled = JSON.parse(brotliDecompressSync(readFileSync(bundlePath))) + t.assert.equal(bundled.modules['node_modules/dep']?.files['index.js'], "module.exports = 'DEP-FROM-BUNDLE'\n", + 'hoisted dep captured under its monorepo-root-relative key') // Tamper the dep's on-disk bytes: the node_modules layout stays on disk (node_modules scope // resolves the bare specifier through it), but the CONTENT must come from the bundle. Pre-fix,