Skip to content

Dedup lockfile/bundle discovery gates into shared absorb helpers (post-#56 polish)#57

Open
exo-nikita wants to merge 1 commit into
mainfrom
claude/stasis-module-resolution-fix-89tewu
Open

Dedup lockfile/bundle discovery gates into shared absorb helpers (post-#56 polish)#57
exo-nikita wants to merge 1 commit into
mainfrom
claude/stasis-module-resolution-fix-89tewu

Conversation

@exo-nikita

Copy link
Copy Markdown
Collaborator

Summary

Follow-up polish to #56, applying its code-review findings. No functional change intended outside the two commented notes below; net −2 lines with the duplication gone.

Changes

Refactor: shared absorb helpers (stasis-core/src/state.js)

The State constructor carried the artifact-loading gates twice — once in the discovery loop and once, copy-pasted, in the post-loop fallbacks for explicit (rootDir-independent) paths. The lockfile pair was already 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 edit to mode gating made only in the in-loop branch would silently leave the explicit-path fallbacks — exactly the paths #56 protects — on stale semantics. Both are now extracted:

  • #absorbLockfile(lock, lockPath) — the lock='none' guard, parse/populate, frozen-mode 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:

  1. The in-loop missing-lockfile guard now tests lockfileLoaded (post-absorb) instead of 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 (a corrupt lockfile now reports its parse error before a bundle='none' violation).
  2. 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 was 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 (tests/cli.test.js)

  • The Fix explicit bundleFile root selection in monorepo layouts #56 monorepo regression test now pins the capture side — bundle exists and carries the hoisted dep under its monorepo-root-relative key — instead of only the near-tautological save-stdout match, so a capture-side regression fails with a pointed message rather than an opaque "not attested" at load.
  • Its header comment now covers both Node-version-dependent pre-fix symptoms (crash vs silent tampered-disk read) and names which assertion catches which.
  • cleanEnv additionally strips EXODUS_STASIS_RESOURCES_BUNDLE_FILE, EXODUS_STASIS_RESOURCES, and EXODUS_STASIS_FS (all read by Config), so ambient values can't leak into spawned runs.

Verification

  • Full test suite (all 51 files) passes on Node 24.14.0; key suites re-run green on Node 26.3.0.
  • oxlint clean.
  • The newly reachable post-loop branch (explicit resources bundle, no bundleFile, no root signal) was exercised manually end-to-end: construction absorbs the resources half and the run completes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UenCGinFJrGDyt8E7XEftP


Generated by Claude Code

…sorb 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UenCGinFJrGDyt8E7XEftP
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants