fix(tests): drive runtime deadlines from injected clocks instead of wall time - #434
Merged
Merged
Conversation
…all time - rsc-runtime: createAgentRenderEventSequence accepts a time source and the render session accepts an Effect Clock so the maxElapsedMs deadline (elapsed check + pending-boundary sleep) runs on one injectable clock; the dispatcher deadline test now uses TestClock and no longer retries. - agent-bundle: McpProbeService gains a timers seam behind its budget timeout, bounded teardown wait, plugin-data cap, and removal retry; the stalled teardown tests fire those timers in event order. - lifecycle-replay route-unit tests render in-process where the pool already runs under react-server instead of forking a jiti child per replay. - playground-service replay-boundary test waits on subscriber-driven deferreds and awaited appends instead of a fixed polling budget.
🦋 Changeset detectedLatest commit: 72da724 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Owner
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four wall-clock-bound tests wobbled under load. Each is fixed at its source (an injected clock, an injected timer seam, an event-ordered wait, or a cheaper in-process path) — no retries, no padding, no longer timeouts.
rsc-runtime/tests/dispatcher.test.ts› fails a permanently pending boundary when the elapsed deadline expiresmaxElapsedMsdeadline ran onDate.now()+ the real Effect clock, starting before the Flight worker was spawned. Under load the worker's shell arrived after 150 ms and the shell emit itself threwelapsed-time-exceeded(expected a shell event). The test also carried{ retry: 2 }.createAgentRenderEventSequenceaccepts anowtime source;createAgentRenderEventSessionaccepts an EffectClockand provides it to the stream, so the elapsed check and the pending-boundaryEffect.sleeprun on one clock. The test isit.effectwithTestClock: the shell may take any real time,TestClock.adjust(150)expires the deadline whether the sleep is already armed (it fires) or armed later (no time remains). Retry removed.agent-bundle/tests/mcp-probe-service.test.ts› returns a timed-out report without awaiting stalled teardown (+ its budget-spent sibling)McpProbeServicegains atimersseam (McpProbeTimers.schedule(callback, delayMs, { unref })) behind the budget timeout, teardown wait, plugin-data cap, and removal retry; production keeps Node timers (unrefpreserved). The tests inject a manual scheduler,fire(10)/fire(mcpProbeTeardownWaitMs)in event order, assert the report settled before the next macrotask while both closes hang forever, then fire the cap andsettle()so the plugin-data directory is removed.agent-bundle/tests/route-unit/lifecycle-replay.test.ts(5 s timeout)replay()without an injected renderer forks a Node child (--conditions=react-server --import jiti-register) that jiti-transpiles the whole render graph; ~1.2–1.5 s per fork on this machine, > 5 s for the two-fixture test under load.render: renderRouteEvents: the route-unit pool already runs underreact-server, so the route renders in-process through the same renderer the child uses (433 ms and 38 ms instead of > 5 s). The child-fork path stays covered by the captured-fixture tests, which keep their existing budgets.agent-bundle/tests/playground-service.test.ts› sets an atomic subscription replay boundary …latest = nextSequence - 1, backlog filter, subscriber registration, first drain) is computed inside the session's serial queue, and the reentrantappendissued fromonEventis enqueued behind that same task, so the boundary is already atomic — no source change. The observed[1]vs[1, 2]is theeventuallyhelper's fixed budget (100 × 1 ms polls) racing an fsync-backed append under I/O load.sequence: 2), and asserts fail-closed right after the overflowing append resolves (publish runs synchronously insideappend). No polling budget remains in the test.Evidence
Runs in the worktree on a loaded machine (other lanes +
pnpm test:unitrunning concurrently); loop logs under/tmp/final-sweep/a3a-*.log.Agent render elapsed time exceeds 150ms, retry disabled for the baseline)test timed out in 5000ms, first test)Gates:
pnpm typecheck✅,pnpm lint✅,pnpm test:route-unit✅ (44/44),pnpm test:unit3002/3003 — the one failure isnative-claude-contract.test.ts › fails closed when the candidate plugin…timing out at 5 s; it fails identically 3/3 on a pristineorigin/mainworktree on this machine (~4.95 s ofrunNativeClaudeSmokework per test) and is unrelated to this branch.Test plan
pnpm exec rstest --config rstest.unit.config.ts packages/rsc-runtime/tests/dispatcher.test.ts(32/32)pnpm exec rstest --config rstest.unit.config.ts packages/agent-bundle/tests/mcp-probe-service.test.ts packages/agent-bundle/tests/playground-service.test.ts(82/82)pnpm exec rstest --config rstest.route-unit.config.ts packages/agent-bundle/tests/route-unit/lifecycle-replay.test.ts(5/5)pnpm typecheck,pnpm lint,pnpm test:route-unitReview status
chatgpt-codex-connectoranswered "You have reached your Codex usage limits for code reviews" to both the automatic trigger and the explicit@codex reviewon head72da724.071af8794(fix),72da72439(changeset PR reference).