Skip to content

Fix lockfiles lost across processes, review findings - #8

Merged
toabctl merged 1 commit into
mainfrom
wt-review-fixes
Oct 8, 2026
Merged

toabctl merged 1 commit into
mainfrom
wt-review-fixes

Conversation

@toabctl

@toabctl toabctl commented Oct 8, 2026

Copy link
Copy Markdown
Owner

Fixes from a correctness review. Each fix has a regression test that fails on main.

Fixes

  • webpack, two processes writing one directory lost packages (reproduced). The lockfile asset was rendered after processAssets and written by webpack without the lock, over packages another process had written since. The rewrite was then skipped because the file looked unshared. On the real disk the lockfile is no longer an asset: it is written in afterEmit, under the lock, like Rollup's. In-memory output (webpack-dev-server) keeps the asset. As a result, webpack's stats no longer list the lockfile, and plugins that compress or upload assets don't get it.
  • Circular require() warning in every webpack-cli build. The Rollup adapter read exp.rollupInternal before checking the request.
  • Packages outside the project listed twice across processes. New "outside" field in the bundle-lockfile record.
  • readMeta threw on a lockfile of another shape, which blocked every later write.
  • Atomic replacement of lockfiles in the output directory; a warning when BUNDLE_LOCKFILE_INLINE=0 has no export directory; stale lock takeover can no longer delete a lock another process just took; bounded hash cache for nested bundles.
  • Node shim: runs asdf/nodenv script shims; doesn't add a second --require when it is already there through a symlinked install path; can't loop; no globbing of PATH.

Documented instead of changed

  • After a config change, a build into an uncleaned directory keeps the previous writer's packages while one of its files exists. Dropping writers by recorded hashes would drop shipped packages after post-build edits (e.g. envsubst).
  • Rolldown's build()/watch() don't list packages from style-sheet @imports (only rolldown() exposes watch files). Vite 8 is not affected.

Tests

  • node --test test/unit.cjs: 58/58 pass (10 new; all 10 fail against main).
  • Full matrix locally in Wolfi (6 shards + Node 22 Vite job): all pass. test/lib/watch.cjs now checks the lockfile on disk (deleted before each build) instead of a compilation asset.

🤖 Generated with Claude Code

- webpack: on the real disk the lockfile is no longer a webpack asset. It
  was rendered after processAssets and written by webpack without the lock,
  over packages another process had put there since, and the rewrite was
  skipped because the file then looked unshared. It is now written in
  afterEmit, under the lock, like Rollup's. In-memory output keeps the asset.
- rollup adapter: check the request before reading exports, so circular
  require()s no longer make Node warn (seen in every webpack-cli build).
- lockfile: record which keys are packages outside the project ("outside"),
  so another process does not list them a second time; readMeta ignores
  lockfiles of other shapes instead of throwing and blocking every write.
- outputs: replace lockfiles in the output atomically; warn once when
  BUNDLE_LOCKFILE_INLINE=0 has no export dir; a stale lock is taken over
  only if it is still the stale one.
- nested: bounded hash cache (core/hashes.cjs).
- node shim: run version managers' script shims (asdf, nodenv), skipping
  itself, copies of it and scripts it already passed through; a --require
  through a symlinked install path counts as there; no globbing of PATH.
- tests: regression tests for each, the cross-process race deterministically
  in child processes; the watch-mode case checks the file on disk.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@toabctl
toabctl merged commit 66b9f58 into main Oct 8, 2026
8 checks passed
toabctl added a commit that referenced this pull request Oct 8, 2026
…ix for Next.js and hand-configured plugins (#10)

* webpack: graceful-fs in any copy is the real disk (Next.js, plugin in the config)

onDisk() knew webpack 5's graceful-fs only as the copy resolved next to the
Compiler module. Next.js bundles its own copy, and a BundleLockfilePlugin
configured by hand does not know where webpack is: their lockfiles were
treated as in-memory output - emitted as a webpack asset, without the lock,
not shared with other processes (the race fixed in #8, still there for
them). graceful-fs is now known by its gracefulify(); Node's fs counts too;
memfs (webpack-dev-middleware) still does not.

Found with a real Next.js 16 build and a hand-configured webpack 5 config:
both outputFileSystems are graceful-fs objects the old check did not match.

Tests: the on-disk unit test also with a graceful-fs-shaped file system and
Node's fs (fails on main); a matrix case with the plugin in the config of two
webpack processes writing one directory in parallel.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* README: every statement checked against code, tests and sources; architecture diagrams

Each statement was checked against the code, a test that exercises it, or a
primary source for other tools (package tarballs, upstream source and issues).

Corrected:
- only Vite/Rollup/Rolldown lockfiles record output hashes: a webpack-built
  bundle bundled by another build is not attributed
- keys are relative to the working directory for Vite/Rollup/Rolldown, not
  webpack's context
- vite build --watch on Vite 8 (Rolldown's watch()) lists no CSS @import
  packages
- webpack versions: shutdown hook since 5.17 (not 5.20), "#" cut since 5.104
  (not 5.105), also in code comments and test names; inlineExports inlines
  constants of up to 6 bytes
- syft, Yarn 2 and nodejs/node#60380 details; tests: cases without an oracle
  and packages oracles cannot see

Added: the Vite/Rollup plugin for a config (matrix cases on Vite 7 and 8),
the exact limits (recorded files and hashes, scanned entries, file size, lock
timeouts), warnings, the module.register deprecation, and "How it works":
mermaid diagrams of how bundle-lockfile gets into each build system, a webpack
and a Rollup/Vite build's hook timeline, files to packages, and several
writers of one lockfile.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* README: fixes from an independent audit against the code

- install from the repository, not v0.0.2 (which lacks the graceful-fs fix
  the README describes)
- the [fullhash] case tests separate directories, not a shared one
- Vite installers as tested: Vite 8 with npm, npx, pnpm 10, yarn 1, yarn 4
  and bun, Vite 7 with npm and yarn 4 Plug'n'Play
- only the ESM hook is limited to bundler processes on Node < 24.12; Rollup's
  CommonJS build is hooked everywhere; rolldown-vite counts too
- Sass partials and style @imports, copies by another process: Vite, Rollup
  and Rolldown only
- the d3 re-export claim scoped to what was observed (GitLab, LibreChat)
- the 2 s timestamp slack, the 20 MiB limit's exception, the in-memory write
  conditions, debug lines, warnings, writer ids (kind, entry template if a
  string), the example lockfile, BUNDLE_LOCKFILE_FILE in nested lookups,
  esmPackages for new adapters, diagram details, conditions of test claims

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@toabctl
toabctl deleted the wt-review-fixes branch October 9, 2026 10:39
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.

1 participant

Sponsor
SponsoredKunjungi sekarang
Promo