fix(dev): #75 — completed MultiStats cohorts are the authoritative compile identity, no FIFO hook pairing - #83
Conversation
|
commit: |
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. |
Review against #75 / #39 (with stress evidence)Design verdict: the completed-cohort identity redesign is sound — identity is allocated per completed MultiStats cohort in completion order, pre-compile observation owns no identity or barrier, supersession keys only on the monotonic captured-cohort ordinal plus prepared-runtime authority (equivalent prepared revisions still never supersede), and failed/no-op cohorts never bump the ordinal, so the #38/#39 commit-window protections held in review and under test. One real regression found and fixed (605b2bc). Removing the guard's wait-on-live-attempts also removed a behavior PR #39 documented and the pinned suites rely on: while an older cohort's activation sits between its prepared phases and the commit check, a compile that starts in that window could no longer supersede it before commit. One settled edit could then activate two generations. Instrumented traces showed a spontaneous watcher recompile ~500 ms after a repair activation on both branches ( The fix keeps #75's acceptance criteria intact: pre-compile observation is now a bounded collapse hint — every settled completion (captured, no-op, or failed) resets the observed-start count after its ordinal bump, and Post-fix verification (all local, quiet 96-core host unless noted):
|
605b2bc to
fa8cf22
Compare
Replace the FIFO attempt queue between the global onBeforeDevCompile and onAfterDevCompile hooks with completion-allocated identity: every completed MultiStats cohort allocates the next monotonic ordinal at the start of its completion callback, and onBeforeDevCompile becomes a purely advisory observation that owns no identity, queue, or activation barrier. The activation guard no longer waits on live pre-compile attempts; supersession keys only on completed cohort ordinals (bumped synchronously with each completed callback) and prepared-runtime authority, so coalesced, missing, duplicated, or reordered global callbacks can neither wedge an activation nor associate Stats with a stale observation. Failed and no-op completed cohorts still never bump the ordinal, preserving the #38 commit-window protections. Refs #75 Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…h a bounded grace Completed-cohort identity dropped the one job the pre-compile hook still did for the activation guard: while an older cohort's activation sat between its prepared phases and the commit check, a newly started compile could no longer supersede it before commit, so one settled edit could activate two generations (the doomed older cohort, then the newer one). The runtime playground pins one automatic replay per recovered activation, and its HMR e2e failed roughly half of local runs on this branch while main held 11/11 - a spontaneous watcher recompile ~500ms after a repair activation double-committed. Pre-compile observation now counts as a bounded collapse hint: every settled completion (captured, no-op, or failed) resets the count after its ordinal bump, and guard.wait() waits for observed in-flight compiles only until half the activation phase budget before proceeding. A completion that never arrives can therefore delay one activation once and self-heals at the next settled completion - it can never wedge or fail an activation, preserving the #75 acceptance criteria, while the newest completed cohort again supersedes a doomed older activation before it becomes visible (#39's single visible activation per settled edit). Two deterministic real-session tests cover the seam: an observed compile completing during the guard wait collapses the older activation (one activated generation, one transient failed event), and a dangling observation with no completion commits after the grace with zero failed events. Post-fix stress: HMR e2e 10/10 (2 under taskset -c 0,1), capture 3/3, dev-provider 41/41.
fa8cf22 to
0e0c095
Compare
2 participants
Fixes #75. Follow-up to the Rsbuild 2.2.1 conformance audit (#72); this is the identity rearchitecture that #76 deferred.
Problem
The runtime compile observer allocated an attempt ID in the global
onBeforeDevCompilehook and consumed IDs FIFO inonAfterDevCompile. Rsbuild documents lifecycle order but not a stable cycle ID, callback cardinality under coalesced invalidations, or one-to-one pairing when MultiCompiler children invalidate at different times. On top of that, the session's activation guard waited on every attempt barrier newer than the activating snapshot — so aonBeforeDevCompilewhose completion never arrived (a coalesced or superseded invalidation) held an activation hostage until the bounded budget failed it loudly.What changed
examples/rsc-agent-runtime/rsbuild.config.ts— the FIFOpendingAttemptIdsqueue, the duplicate-identity throw, and the "completed without a matching attempt" throw are gone.onBeforeDevCompilenow only calls the advisoryobserveCompileStart(); it owns no identity, queue, or barrier. EveryonAfterDevCompilecallback allocates the next monotonic identity viabeginCompletedCohort()at its start, in global completion order. Nothing pairs the two hooks, so there is no queue to mispair.examples/rsc-agent-runtime/src/dev/rsbuild-runtime-session.ts—AttemptBarrier, the#attemptsmap,#latestAttemptSequence, and#sequenceForare removed.#beginCompletedCohort()allocatescohort-Nordinals at completion time. The activation guard'swait()no longer owns an attempt barrier: the completed-cohort ordinal (#latestRscCohortRevision) bumps synchronously with each completed changed cohort's callback, and supersession is fully decided bycheck()against completed ordinals and prepared-runtime authority. Pre-compile observation remains a bounded collapse hint: an in-flight activation waits for observed compiles only up to half the activation phase budget (then proceeds, never fails), so one settled edit keeps activating exactly one visible generation, while a completion that never arrives can only delay one activation once and self-heals at the next settled completion. In-flight compiles have no identity at all; failed and no-op completed cohorts still never bump the ordinal, so 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 protections (a live/failed/no-op later compile must not discard the newest successful one) are intact and covered by the existing commit-window tests.#candidatesByAttempt), compiler-asset checkpoint lifecycle, bounded activation phases from fix(dev): harden runtime relay against Rsbuild internals #76, and thewriteToDiskcopy path are all unchanged.How this differs from #76
#76 kept FIFO identity and made its violations loud: a non-singleton pending queue or duplicate identity failed all live attempts and threw rather than silently misassociating, and a wedged activation failed within a bounded budget. This PR removes the FIFO identity entirely, so those failure modes cannot arise: identity is allocated per completed MultiStats cohort in completion order, pre-compile observation is advisory, and no activation ever waits on a pre-compile observation. The bounded activation budgets from #76 remain as a backstop for genuinely wedged store/registry phases.
New deterministic coverage
Plugin-level (driven directly against the observer hooks):
onBeforeDevCompilecallbacks — identity derives only from completions, no failures;rsc/widgethash pair is its own cohort in completion order.Real-session (via the intercepted compile observer on a live
RsbuildRuntimeSession):runtime.generation.failedevents.The two #76-era commit-window regression tests are retained (retitled/re-commented for the new model), as are all failed-attempt, no-op-attempt, and source-build-diagnostic tests.
No changeset — why
Every change is inside
examples/rsc-agent-runtime(the private, unpublished@agent-bundle/rsc-agent-runtime-demoexample runtime). No published package (agent-bundle,@agent-bundle/rsc-runtime,create-agent-bundle) changes behavior, so per the changeset convention there is nothing to version. (#76 needed a changeset because it also changed the published dev-server relay inagent-bundle.)Out of scope (stayed in lane)
No changes to #73's reload channel, #74's per-environment output staging or
onAfterEnvironmentCompilecheckpoints, how livewriteToDiskroots are copied,packages/agent-bundle/src/build/rslib.ts,build.test.ts,hooks.test.ts, orRuntimeClientSurfaceProxyframe parsing.Test plan
pnpm buildpnpm typecheckpnpm --filter @agent-bundle/rsc-agent-runtime-demo typecheckrslint examples/rsc-agent-runtime(0 errors)tests/dev-provider.integration.test.ts— 39/39 passed (includes the 4 new tests)tests/generation-materializer.test.ts— all passed@agent-bundle/rsc-agent-runtime-demosuite — 181 tests: 175 passed, 6 skipped (env-gated native/eval tests, pre-existing), 0 failed