Skip to content

test: fix the #576 test-level P1/P2 items — injectable proxy timeout, stubbed watcher, json-report scaffold assertions, relay-state readiness, polled request log, wire-derived outage fences - #584

Merged
ScriptedAlchemy merged 20 commits into
mainfrom
test/576-p1-test-fixes
Sep 5, 2026
Merged

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Part of #576 (Rstest audit) — the test-level P1/P2 items. Out of scope here, as agreed on the issue: the config-level items (worker-root teardown, setup-file loading, timeouts, stale-dist guard, adapter libId, pool membership — follow-up to #570), the contract-matrix split and mcp-lineage import (after #569), and CI shape (ci.yml, separate PR).

Six lanes, one file set each, integrated on one branch. Every mechanism below was re-derived from the current code, not copied from the issue; two of the issue's diagnoses are corrected (items 3 and 6).

1. P1 — runtime-client-surface-proxy.test.ts waited the real 15 s twice

Mechanism. upstreamRequestTimeout = 15_000 was a module constant (src/dev/runtime-client-surface-proxy.ts:26) armed before every upstream dispatch — the bootstrap entry fetch and every proxied asset. bounds an upstream HTTP request before headers arrive and keeps a completed 502 response intact when a response body stalls after headers both drive an upstream that never completes, so each case sat on that deadline: 15.0 s + 15.0 s, 30.6 s of a 33 s file, and the unit pool's wall was bound by this one file regardless of worker count (issue: 46.2 / 43.1 / 45.5 s at 16 / 32 / 95 workers).

Fix. RuntimeClientSurfaceProxy.open(...) takes a trailing options: RuntimeClientSurfaceProxyOptions = {} with upstreamRequestTimeoutMs?: number; the default is exported as defaultRuntimeClientSurfaceUpstreamRequestTimeoutMs = 15_000 and validated (positive safe integer ≤ the 2^31−1 setTimeout ceiling — Node collapses larger delays to 1 ms, which would silently drop the bound; out-of-range values throw TypeError before the proxy opens). Production callers (workbench-server.ts) pass four arguments and keep 15 s. The two tests pass { upstreamRequestTimeoutMs: 500 }, keep their exact assertions (status: 502; { body: 'Not Found', status: 502 } + socket closed within 250 ms), and tighten their outer bound from within(pending, 16_000) to 2_000, so a proxy that ignored the option now fails instead of passing at 15 s. A new fast test asserts the default once and rejects 0, -1, 1.5, NaN, Infinity, 2^31.

Proof (shared 96-core host, 1-min load noted; same command shape before/after, unmodified vs. patched scratch worktree):

reporter total wall load
unit pool before ×3 42.7 / 42.5 / 43.2 s 44.9 / 44.5 / 45.5 s 23–58
unit pool after ×2 39.9 / 37.1 s 42.1 / 39.1 s 37–44
unit pool after, on the merged branch (gate) see gate table

The two cases: 15.0 s → 0.21 s each; file host span 30.6 s → 1.1 s. The pool's wall is now bound by the next file (eval-native-mount.test.ts, 25.8 s), as the issue predicted. 10× file loop: 10/10, 3.1–3.8 s per run.

2. P1 — dev-host-install.test.ts re-sync test raced a real ProjectWatcher

Mechanism. The test built its DevCoordinator without createWatcher, so the coordinator installed the real ProjectWatcher (coordinator.ts:244, #startEffect :331-338, onInvalidation → this.rebuild): chokidar over the fixture root, 100 ms debounce (watcher.ts:90-93). The test's own writeFiles therefore scheduled an unrequested rebuild C, queued behind the explicit rebuild B (coordinator.ts:288-295) and started from B's onExit (:421-424). Every successful build mints a fresh epoch id and publishes artifact.available; the install manager turns each into a sync() that rewrites DEV_INSTALL_MARKER, and settled() (host-install-manager.ts:391-393) only awaits the syncs pending at call time. Normally C fails or is rejected as sameInputs because the test's broken write lands while C compiles; when host sync of B is slow relative to C (a loaded runner), C completes first, its sync moves the marker, and :370 sees epochId: CmarkerBeforeFailure — the exact CI failure. Reproduced deterministically with an instrumented harness outside the repo: with a 2.5 s delayed epoch acquisition the marker moves (FAIL (marker b306cf8d -> 75fa7946)); with a stubbed watcher it does not.

Fix. Four lines: createWatcher: () => ({ close: async () => undefined }) — the DevelopmentWatcher shape dev-coordinator.test.ts already uses (:917, 955, 1052, 1094). The coordinator now rebuilds only on the test's explicit rebuild() calls; assertions unchanged. Not "drain": there is no idle signal on the coordinator, so a drain would be a wall-clock wait around inotify + debounce.

Proof. Single test ×20 before: 0 failures (does not reproduce natively on this host — fast disk); ×20 after: 20/20, 11.8–16.8 s per run (fixture build dominates), load 27–58. Whole file under the integration config: 7/7 (Claude and Codex variants ran), 106 s; under the root config the CI host-install-proofs job uses: 7/7, 105 s. Integrator's 10× loop on the merged branch: see gate table.

3. P1 — scaffold-packed-matrix.e2e asserted "failedTests": 0, a key the default reporter never prints

Mechanism (corrects the issue). The key does exist — in Rstest's md reporter, which initCli selects automatically when it detects an AI-agent environment (CURSOR_AGENT, CLAUDECODE, CODEX_*, …) and no reporter is configured. The scaffolded project inherits the developer's environment, so under an agent the template's npm run test:routes printed the md summary block and the assertion passed for its author; in CI (GITHUB_ACTIONS, no agent variables) the reporters are ['default', 'github-actions'] and the key never appears — red every nightly, and pnpm test:evidence (the next step) has never run in CI. Second latent failure the issue did not list: the default reporter expands per-test lines only for single-file runs, so the cli-tool projection pool (two files) fails toContain('greets through the routed CLI shell …') at line 96 before it reaches line 98. Real default-reporter output of a scaffolded pool, for the record:

 ✓ tests/projection/cli-dispatch.test.ts (5)
 ✓ tests/projection/script-dispatch.test.ts (3)

 Test Files 2 passed
      Tests 8 passed
   Duration 1.36s (build 75ms, tests 1.29s)

Fix. Test-only; the templates are untouched. tests/support/scaffold-fixture.ts gains expectPassedPool(projectRoot, script, testNames): runs npm run <script> -- --reporter=json (npm appends the flag to the template's rstest --config … script; an explicit CLI reporter overrides both the agent md swap and the CI default), locates the report on stdout (the only thing there that opens a line with {), reads a non-zero exit rather than throwing (the report is already written and names the failing test), and asserts in order: failing tests [], failing files [], tests[].name ⊇ testNames (a dropped or empty pool reports status: 'fail', tests: 0 — and failedTests: 0, so the names are what catch it), then { status: 'pass', summary: { failedTests: 0 } }. Three call sites replace the six toContains; the two hunks stay clear of #569's edits in the same file. Takeover addendum (91a2485b9, from the self-review): the helper also keeps npm's exit beside the report and, after the diagnostic-first assertions, fails a pool whose report says pass while the script exited non-zero (`npm run <script>` exited <code> although its report says pass + stderr — a lifecycle script or a crash after the report was written is not a pass); the "wrote no Rstest JSON report" error quotes the exit and stderr so a config-load failure is no longer hidden. New unit test packages/create-agent-bundle/tests/scaffold-fixture.test.ts pins the fence against a stub project whose npm scripts print a Rstest-shaped report and exit as instructed: pass + exit 0 resolves; pass + exit 1 rejects with the new message; a missing expected name rejects; a failing entry is reported before the exit code; no report rejects quoting exit 2 and stderr (5/5, 1.5 s).

Proof. CI-faithful (env -u CURSOR_AGENT CI=true GITHUB_ACTIONS=true) pnpm test:packed:release packages/create-agent-bundle/tests/scaffold-packed-matrix.e2e.test.ts: before → exit 1 (expected ' Rstest v0.11.10\n\n ✓ tests/route-u…' to contain '"failedTests": 0' at line 50; line 96 on the cli-tool test); after → exit 0, Test Files 1 passed, Tests 2 passed, 1m15s wall (load 41→49); again under the agent environment: exit 0, 1m11s. Negative proofs against sabotaged scaffolded copies: an appended failing it → the failing-tests assertion prints the entry with its errors[]; a deleted tests/projection/expected [] to deeply equal ArrayContaining [...]. Note for the next person: pnpm test:packed:release -- <file> forwards -- to pnpm exec rstest, where cac treats it as end-of-options and drops the filter — pass the file without --. Nightly run 33941185524 is this exact shape — see Integration (takeover) below. On the merged tip (91a2485b9, Rstest 0.11.12 in the root and in all three templates), CI-faithful env -u CURSOR_AGENT -u CLAUDECODE CI=true GITHUB_ACTIONS=true run of the file against pre-packed tarballs: 2/2 passed, 40.9 s reporter / 43.8 s wall.

What the evidence artifact looks like now. pnpm test:evidence (the step that has never run in CI) passes: 1 file / 1 test, 51 s wall including pnpm build (34 s of test). runtime-playground-capture.test.ts runs packages/workbench/scripts/capture-runtime-playground.mjs into mkdtemp(join(tmpdir(), 'agent-bundle-runtime-capture-')) under the worker's isolated tmp root and removes it in finally — nothing survives the run and nothing is uploaded. Reproduced by invoking the script directly: five 1440×900 PNGs (desktop.png — populated Runtime playground with "All outputs are from the current runtime generation (generation-3). No stale views."; hmr-before.png; hmr-after.png; compile-error.png — Diagnostics tab showing AB8206 RSC runtime source build failed beside the last-good generation-2 banner; recovered.png) and evidence.json (1.4 kB): appMarkerVisible: true, appRefreshPreservedDocument: true, appVisibleBefore/After/Recovered: true, compactRunGeneration: "generation-2", compactRunId (== compileErrorRunId), compileErrorDiagnosticsVisible: true, compileErrorGeneration: "generation-2", compileErrorHistoryUnchanged: true, compileErrorLastGoodVisible: true, compileErrorLayout: { diagnostics: { top, bottom, viewportHeight: 900 }, lastGood: { … } }, documentTimeOriginBefore === documentTimeOriginAfter (HMR without reload), desktopControlColumns: 4, generationBefore/After/Recovered: "generation-1"/"generation-2"/"generation-3", lastGoodGenerationDuringError: "generation-2", lastGoodPreserved: true, providerSessionId, runBefore/runAfter (distinct UUIDs), recovered: true, sandboxOpaqueOrigin: true, viewports: { desktop: { width: 1440, height: 900 } }. ci.yml still needs test:evidence as its own step with if: ${{ !cancelled() }} — that is the CI-shape PR.

4. P1 — mcp-app-real.e2e inferred readiness from a notification count, then waited 120 s for a /close that could never come

Mechanism. The gate is McpAppFrameRelay.#resourceProvided (packages/workbench/src/mcp/mcp-app-frame.tsx): it flips only when the sandbox proxy posts ui/notifications/sandbox-proxy-ready, and close() branches on it — unset → #forceClose()DELETE /api/mcp/apps/<id>; set → POST …/close. The test counted ui/notifications/initialized /messages requests across both previews (toBe(2)), a different channel from the relay flag and not tied to the reopened binding, then waitForRequest('/close', { timeout: 30_000 * timeScale }) — 120 s in CI. When the relay it closed had not flipped, only a DELETE was ever sent and the wait consumed the whole budget (2m16s failures on ~15 % of Node 22.19 main pushes).

Fix. Option (2) from the issue, the one that observes the actual gating state, plus (3) and (4). The relay publishes its lifecycle on the outer iframe as data-mcp-app-relay-stateloading | ready | closing | closed (#publishFrameState(), called on exactly the events that change #state/#resourceProvided: start(), the proxy-ready flip, close(), #completeClose(); no React state, timers, or polling — the same imperative-attribute pattern applyMcpAppFramePolicy already uses on this iframe). McpAppFrameIframe.setAttribute is optional so the unit-test fakes publish nothing. The test: captures the reopened /apps POST response to derive the binding path (per-binding instead of a count); await expect(outerFrame).toHaveAttribute('data-mcp-app-relay-state', 'ready', { timeout: browserTimeout }) before clicking; then waits for that binding's own initialized; after the click, polls the recorded requests for the first close-path request for that binding and throws immediately with Expected the reopened App preview to close gracefully (POST …/close); the relay sent DELETE /api/mcp/apps/<id> instead. if it was the force path; asserts the teardown acknowledgement; every wait is browserTimeout (32 s in CI) instead of 120 s. AppRouteRequest records method (moved up from RuntimeAppRouteRequest, which already had it).

Proof. Workbench rebuilt (data-mcp-app-relay-state present in packages/workbench/dist/static/js/index.js); e2e file 10×: 10/10 (40/40 tests), 34.6–47.8 s per run, close-step test 4.0–5.5 s, load 34–50; a passing toHaveAttribute(..., 'ready') in every run is direct evidence the relay publishes on the real production path. mcp-app-frame.test.ts (read-only, includes the two force-delete cases and the markup test): 19/19. Integrator's 10× loop on the merged branch: see gate table.

Second mechanism, found by the integrator's loops (504351f8c). With the relay wait in place the file still failed 1/10 on the merged branch, then 3/199 when three copies ran concurrently on the loaded host, always the same way: Closing the session sent no close request for the reopened App preview. — no POST …/close and no DELETE, i.e. the relay never received a close at all. Instrumented runs (in-page capture-phase pointerdown/mousedown/click listeners on the document, a MutationObserver on the button's disabled, a wire log, and a Playwright trace retained on failure) showed why: the session controls sit ~2000 px above the reopened App's cross-origin iframe. click() scrolled the page from scrollY=1964 to 33 and dispatched in the same action; Playwright's own log says scrolling into view if needed … done scrolling … performing click action … click action done, yet the document saw no pointer event, the button stayed enabled, and document.activeElement became the IFRAME. Chromium routes pointer input in the browser process from compositor hit-test regions that update asynchronously after a scroll, so under load the click went to the out-of-process frame that had occupied that point a frame earlier. Fix, in the same test: scrollIntoViewIfNeeded()toBeInViewport() (an in-page IntersectionObserver round-trip, so the layout has settled before the dispatch) → click()expect(button).toBeDisabled()run('close') sets pendingActions synchronously and mcpPageSessionControls keeps close false through the terminal phase, so a swallowed click now fails in one browserTimeout with The Close MCP session click did not start the close action. instead of a vague /close timeout. The temporary instrumentation is not in the PR. Proof: 3 concurrent 15-run loops of the whole file (the shape that produced the 3/199): 45/45, 35.0–44.2 s per run, 1-min load 29–104 (heavier than the 14–66 the failures reproduced under).

5. P1 — mcp-app-preview-browser.test.ts read a Node-side request log synchronously after an in-page signal

Mechanism. bootstrapRequests is appended by the bootstrap server's request handler (:21-25) — i.e. after Chrome has opened the connection and Node's parser has emitted 'request'. Every in-page signal the test waited on fires before that: create:* is pushed inside the mocked fetch before the create response is even parsed, two React commits and a network round-trip ahead of the GET; factory is pushed in the passive effect of the commit that arms the iframe src, so the GET is at best concurrently in flight. waitForFunction (raf polling) sees the event on the next frame, replies over CDP, the test reads the array — and on a 2-worker 4-core runner the CDP replies occasionally win: expected [ '/runtime-bootstrap' ] to deeply equal [ '/runtime-bootstrap', …(1) ].

Status. Carried by #589, not by this PR. d0367b7c9 on this branch added a local expectBootstrapRequests(requests, expected) (expect.poll on the log length, then the exact toEqual) at the four sites that observe a freshly issued bootstrap GET; #589 (59e6f465e) landed the same fix on main through a shared requestRecorder().arrived(n) helper (packages/workbench/tests/support/http.ts:43) at the same four sites (mcp-app-preview-browser.test.ts:235, 320, 434, 464, each expect(await within(fixture.bootstrapRequests.arrived(n), 5_000 * timeScale)).toEqual([...])), keeping the four "still N" sites synchronous as the lane did. The 38cc2b17f merge conflicted on this file and was resolved to main's version, so git diff origin/main -- packages/workbench/tests/mcp-app-preview-browser.test.ts is empty. The mechanism above stands — it is the reason #589's helper awaits arrival instead of reading the array.

Proof (historical). The lane's 40-run loop (39/40 — the one failure a mid-edit of mcp-app-frame.tsx landing in the fixture bundle; 36/36 after) and the 2/300 vs 1/300 scratch reproduction were against d0367b7c9's helper, which is no longer in the diff.

6. P2 — packed-outage-ledger Date.now() fences

Mechanism (corrects the issue's second shape). Shape 1 (unexpected pre-outage failures: GET /api/logs/replay?after=0 … net::ERR_ABORTED): the Logs page issues the replay from its mount effect and aborts it from the effect cleanup; the e2e left Logs on the heading alone, so on a loaded runner the navigation aborted the replay 22 ms in, before any headers, and the allowlist accepted a replay abort only with a 2xx status (ledger:149-156). Shape 2 (unknown post-recovery failure: […after=0…, …after=5…, /api/logs/stream?after=36]): not the after=0→5 pair — the code shows that message can only fire after the fresh-B filter and its per-failure assertion passed, and the two-reader shape (transport stream?after=0 right after the POST; session controller stream?after=N after the trace refresh; both aborted before the DELETE) was already accepted. The unclaimed entry was the Logs navigation stream: its request and response events arrived in one Playwright batch, the awaited continuation ran synchronously into the next iteration's openedAt = Date.now() — which is the previous record's leftAt — so leftAt === at and the exclusive window request.at < navigation.leftAt excluded it; the message then dumped every post-recovery failure, making the recognised ones look guilty. Both shapes reproduce deterministically by replaying the exact CI ledger entries through the old validator.

Fix (as merged — dd80b8426, 7d40b2279, cbdf3738c, bd25a7c2f, d13d8a2bf, b9a34080d). Newly accepted, each documented in the ledger: (a) a pre-outage GET /api/logs/replay net::ERR_ABORTED with no response (pre-header navigation abort; a non-2xx answer is still rejected); (b) navigation ownership by delivery order — postRecovery.navigation[].openedIndex/leftIndex are the ledger length at the arrival and departure stamps, and a request belongs to the visit when its index sits in [openedIndex, leftIndex) and its abort completes no earlier than leftAt (a batch of events and the stamp that follows it share one millisecond; the array can order them, a clock cannot). The fresh-B close window opens on a Date.now() stamp taken immediately before the Close click (packed-release.e2e.test.ts:998 — the click is issued from the test, so nothing it causes can be delivered earlier; this is also the fix for the P2 review thread) and closes on the completion of the session's DELETE wire entry, awaited via expect.poll; the e2e waitForResponses the Logs replay (and requires .ok()) before navigating away; the project/session retry cadence is asserted as an accumulated delivery-lateness credit (projectSessionRetryDelayMs = 250, ceiling maximumDeliveryDelayMs = 125: every gap adds or spends its difference from 250 ms, capped at the ceiling, never negative — a burst overspends at once, an under-paced client cannot borrow from gaps it has not produced yet); recognised failures are a Set and the assertion reports only unclaimed entries. Committed guard rows in packed-outage-ledger.test.ts, each asserting its specific message: logsReplayCancellationWithoutTerminal, logsReplayCancellationAfterFailure, preCloseFreshStreamCancellation, postCloseFreshStreamCancellation (takeover, b9a34080d — an abort completing after the DELETE completed), navigationCancellationBeforeDeparture, sameMillisecondNextPageRequest, navigationNonGetCancellation, burstRetry, underpacedRetries, borrowedRetryLateness, plus the resetWith* / duplicateOldStreamReset / postRecoveryReset family; accepted controls: knownPreOutageLogsReplayPreHeaderCancellation, sameMillisecondDepartedRequest, navigationLiveStreamCancellation, navigationRespondedCatalogCancellation, lateDeliveredRetry, lateDeliveredFirstRetry.

Proof. Old validator vs. CI entries: shape 1 throws unexpected pre-outage failures, shape 2 throws the same three-entry dump as CI; controls (200 replay; leftAt + 1 ms; the after=0/after=5 pair alone) pass. New validator: both shapes pass, all 13 guards still throw. packed-release.e2e 10× against pre-packed tarballs: 10/10, 39.4–45.1 s per run, load 49→18. packed-outage-ledger.test.ts untouched and green. Integrator's 10× loop on the merged branch: see gate table.

7. Takeover — ERR_SOCKET_NOT_CONNECTED on the first outage probe (879411485)

Mechanism. Release-gates runs 33933481002 and 33936651225 (attempt 1; attempt 2 passed unchanged) rejected the foreground outage ledger with project/session retry had a non-connection failure: the first GET /api/project/session probe of the outage — issued while closeChild (packed-release-harness.ts) was SIGTERM-ing the server — went out over a keep-alive connection the server had already closed and failed 9 ms / 18 ms in with net::ERR_SOCKET_NOT_CONNECTED and no response headers; every later probe was ERR_CONNECTION_REFUSED at the 250 ms cadence and recovery answered 200. downServerProbeCodes accepted only REFUSED and RESET.

Fix. net::ERR_SOCKET_NOT_CONNECTED joins downServerProbeCodes (packages/workbench/tests/support/packed-outage-ledger.ts), so it is accepted exactly where RESET already is: a same-origin GET /api/project/session probe inside the outage window with one terminal state, paced by the credit model, paired 1:1 with a console error carrying the same code and URL. Still rejected with their existing messages: the old-session DELETE (REFUSED-only), a non-GET on the probe path, a probe carrying response headers, a probe outside the window, and either side of the console pairing missing. Eight rows in packed-outage-ledger.test.ts: socketNotConnectedSessionProbe and socketNotConnectedFirstRetry (the CI shape) accepted; socketNotConnectedDelete, socketNotConnectedSessionPost, socketNotConnectedRespondedProbe, socketNotConnectedPreOutageProbe, socketNotConnectedConsoleWithoutFailure, socketNotConnectedProbeWithoutConsole rejected.

Proof. Negative control: with the source edit reverted, the CI-shape row fails with the verbatim CI message. Residual, unverifiable from the CI logs: the validator failed before console pairing, so Chromium's console line for this code is inferred from the Failed to load resource: net::ERR_… convention the already-accepted old-stream SOCKET_NOT_CONNECTED case relies on; if it were ever absent, the next failure would read outage request failures lack a unique paired console error.

Integration (takeover)

origin/main was merged into this branch twice after the lane commits; the diff against origin/main is 13 files.

Nightly run 33941185524

Run 33941185524, job Packed release matrix (Node 22.19), step pnpm check:release, on main dd3f550b4 (before this PR): scaffold-packed-matrix.e2e.test.ts failed at line 50 (expected '\n> status-plugin@0.1.0 test:routes\n…' to contain '"failedTests": 0') and line 96 (… to contain 'greets through the routed CLI shell a…') while both inner pools passed (3/3 and 8/8) — item 3's shape exactly. Root cause re-confirmed on the merged toolchain from @rstest/core 0.11.12's source: initCli sets reporters = ['md'] when determineAgent().isAgent and no reporter is configured (AI_AGENT, CLAUDECODE/CLAUDE_CODE, CURSOR_AGENT, CODEX_SANDBOX/CODEX_THREAD_ID, GEMINI_CLI, … — kill switch RSTEST_NO_AGENT=1); getDefaultReporters returns ['default', 'github-actions'] iff GITHUB_ACTIONS === 'true'; a CLI --reporter is merged in mergeWithCLIOptions before the md check and overrides both; the json reporter writes JSON.stringify(report, null, 2) to stdout (there is no --outputFile flag in 0.11.12), so the report is the only unindented { line. All three scaffolder templates pin @rstest/core 0.11.12 (the failing nightly had 0.11.10). Fixed by item 3 (60c64635c, hardened in 91a2485b9); no further change was needed — verified CI-faithfully on this tip (2/2) and under the agent environment (2/2).

Gate (takeover — 91a2485b9, after both merges, pnpm install --frozen-lockfile && pnpm build on the new lockfile: Rslib 1.0.0, Rsbuild 2.2.x, @rspack/core 2.2.2, Rstest 0.11.12)

Shared 96-core host; 1-min load noted. b9a34080d91a2485b9 changed only scaffold-fixture.ts and added the unit test, so the integration row on b9a34080d still describes the tip.

gate sha result wall 1-min load
pnpm build 38cc2b17f pass (Publint ×4) 14 s 14
pnpm typecheck 91a2485b9 pass 7.1 s 8
pnpm lint 91a2485b9 pass — 1301 files, 88 rules 2.1 s 8
rstest-pool-lists.test.ts (unit config) 91a2485b9 34/34 1.9 s 8
pnpm test:unit (95 forks) 91a2485b9 253 files / 3916 tests — 3910 passed, 6 skipped; reporter total 36.1 s 41.3 s 19
pnpm test:integration:run (full pool, 4 workers) b9a34080d 95 files / 1123 tests — 1119 passed, 4 skipped 5m11s 26→39
pnpm check:release (pack dry-run, attw ×4 "No problems found", test:packed:release) b9a34080d 13 files / 36 tests — 35 passed, 1 skipped (opt-in native smoke) 2m03s 39→44
pnpm check:release 91a2485b9 pass — attw ×4 "No problems found"; packed release pool 13 files / 36 tests — 35 passed, 1 skipped 2m03s 38→34
scaffold-fixture.test.ts (new, unit config) 91a2485b9 5/5 1.5 s 8
CI-faithful scaffold-packed-matrix.e2e.test.ts (env -u CURSOR_AGENT -u CLAUDECODE CI=true GITHUB_ACTIONS=true, pre-packed tarballs) 91a2485b9 2/2 43.8 s 8
runtime-client-surface-proxy.test.ts (unit config) b9a34080d 5/5 (28 tests/run) 5.3–5.6 s per run 35–38
packed-outage-ledger.test.ts (unit config) b9a34080d 5/5 1.9–2.4 s per run 32–34
mcp-app-frame.test.ts (integration config) b9a34080d 5/5 (21 tests/run) 6.2–6.5 s per run 29–32
dev-host-install.test.ts -t "re-syncs the isolated Cursor install" (integration config) b9a34080d 5/5 12.3–13.0 s per run 28–41
mcp-app-real.e2e.test.ts (integration config) b9a34080d 5/5 (4 tests/run) 35.0–36.6 s per run 25–38
packed-release.e2e.test.ts (packed config, pre-packed tarballs) b9a34080d 5/5 37.8–39.2 s per run 18–24
scaffold-packed-matrix.e2e.test.ts (packed config, pre-packed tarballs, AGENT_BUNDLE_PACKED_RELEASE=1) b9a34080d 5/5 (2 tests/run) 41.7–50.6 s per run 16–34

35/35 loop runs green; the loops ran one file at a time after the full pools had finished. After gh pr update-branch merged d30d9acb6 (#588, no overlapping files, no lockfile change) as ffe5f5a98, pnpm build && pnpm typecheck && pnpm lint && pnpm test:unit were re-run locally on that tip: all green (build 13 s; unit 253 files / 3916 tests — 3910 passed, 6 skipped, 36.8 s). mcp-app-preview-browser.test.ts is no longer in this diff (item 5), so it has no loop row here; it ran green inside the integration pool.

Historical gate on 32a7c4d43 (lane integration, before the takeover merges)
gate result wall 1-min load
pnpm typecheck pass 6.5 s 13
pnpm lint pass 2.9 s 13
pnpm test:unit (95 forks) 243 files / 3644 tests — 3638 passed, 6 skipped; reporter total 33.7 s (issue baseline 43.1 s) 35.8 s 13→23
pnpm test:integration:run (full pool, 4 workers) 90 files / 1100 tests — 1067 passed, 33 skipped 6m00s 23→57
pnpm test:packed:release (full release pool, 11 files) 30 tests — 29 passed, 1 skipped (opt-in native smoke); reporter total 72.1 s 1m33s 30→46
pnpm test:evidence 1 file / 1 test passed 45 s 30→57
10× runtime-client-surface-proxy.test.ts (unit config) 10/10 4.7–5.4 s per run 32→60
10× mcp-app-preview-browser.test.ts (integration config) 10/10 6.0–9.3 s per run 32→63
10× dev-host-install.test.ts -t "re-syncs the isolated Cursor install" 10/10 11.6–16.4 s per run 32→63
10× mcp-app-real.e2e.test.ts (before 504351f8c) 9/10 — run 4: the swallowed-click failure described under item 4; then 196/199 across the diagnostic loops 34.9–55.1 s per run 29→57
3 × 15× mcp-app-real.e2e.test.ts, concurrent (after 504351f8c) 45/45 35.0–44.2 s per run 29→104
10× scaffold-packed-matrix.e2e.test.ts (packed config, pre-packed tarballs, AGENT_BUNDLE_PACKED_RELEASE=1) 10/10 45.8–52.2 s per run 42→26
10× packed-release.e2e.test.ts (packed config, pre-packed tarballs) 10/10 40.3–45.0 s per run 38→27

504351f8c is test-only (one file), so the typecheck/lint/unit/pool rows above still describe the branch; the file's own lint and the workbench tsc project were re-run on it (both clean).

Changeset

patch for agent-bundle — production source changed in item 1 (RuntimeClientSurfaceProxy.open options); .changeset/576-proxy-upstream-timeout.md is the only changeset this branch adds (summary trimmed to the user-facing sentence in b9a34080d). Items 3, 6 and 7 are tests/**. RuntimeClientSurfaceProxy is exported only from the internal src/dev/index.ts barrel, not from src/index.ts or any exports entry, so no docsite page describes it; no docs change. packages/workbench (item 4) is private.

Self-review

Reviewer: a generalPurpose subagent on gpt-5.6-sol-medium (the change-risk-reviewer type needs the TraceDecay daemon, which is intentionally stopped), given the repo path, the PR, AGENTS.md, and asked for concrete merge risks only against git diff origin/main...HEAD. A separate read-only hygiene lane audited the changeset, dead-module rules and the merge itself.

Pass 1 (on b9a34080d)

  1. should-fixscaffold-fixture.ts poolReport swallowed every non-zero npm run exit and trusted the parsed report, so a report saying pass with npm exiting non-zero (a lifecycle script, a crash after the report was written) would pass the release fence. Fixed in 91a2485b9: exit and stderr are kept beside the report; after the diagnostic-first assertions the helper throws `npm run <script>` exited <code> although its report says pass; scaffold-fixture.test.ts pins pass+0, pass+1, missing name, failing entry first, and no-report (with exit and stderr) — see item 3.
  2. should-fix — the retry-lateness credit starts at 125 ms, so one genuine 150 ms first retry followed by a correct cadence passes (lateDeliveredFirstRetry). Dismissed: at is stamped on Playwright's Node-side delivery of the request event and the ledger has no browser-side issue timestamp; the first probe's delivery is exactly as subject to lateness as every later one, so treating it as exact would reject a correctly paced client whose first event was delivered late. The credit is monotone and capped at the ceiling, so a consistently under-paced client — the plausible regression of project-client.ts's constant 250 ms delay loop — is rejected on the following gap (underpacedRetries, borrowedRetryLateness, burstRetry); the only escaping shape is one isolated gap ≥125 ms with a correct cadence after it, which is indistinguishable from delivery lateness with Node-side timestamps and is not a shape a constant-delay loop produces. A browser-side time base (request.timing().startTime, whose availability for connection-refused requests is unverified) is a follow-up candidate on Rstest audit: pools, adapter, browser mode, flake risk, CI shape #576.

Hygiene lane (read-only): changeset exactly one, agent-bundle: patch, private workbench not named — pass; every new identifier has a non-definition user; RuntimeClientSurfaceProxy is reachable from neither src/index.ts nor any exports entry, so no docsite change; production caller workbench-server.ts passes four arguments. Non-blocking findings, all taken: (a) no committed row for a fresh-B abort completing after the DELETEpostCloseFreshStreamCancellation added in b9a34080d; (b) item 6's Fix paragraph described the pre-7d40b2279 window → rewritten above; (c) the changeset summary ended in a test note → trimmed in b9a34080d.

Pass 2 (on 91a2485b9)

  1. 91a2485b9 (the expectPassedPool exit fence and its unit test) reviewed as correct: "its focused tests cover the original false-pass, diagnostic ordering, missing-name, and missing-report cases". No other new findings from the full-diff rescan.
  2. The dismissal of finding 2 was contested with a concrete shape: a short outage whose retryAttempts hold one failed probe and one success 150 ms later passes on the seeded 125 ms credit, and no later gap exists to expose a constant-delay regression. Dismissed again, with the shape checked against this e2e: the outage here is closeChild (SIGTERM, awaited exit) → two Playwright visibility assertions on the disconnected state → startInstalledServer (a fresh Node process loading the installed workbench server) → awaitReady, so the browser always issues several failed probes before one succeeds — the two CI ledgers in item 7 hold 12 and 15 failed probes — and two gaps already reject a 150 ms cadence (125 → 25 → −75; underpacedRetries); within the outage lengths this e2e produces the credit model rejects any constant delay ≤ ~240 ms. Encoding the floor as a minimum-gap assertion would mean re-timing the base fixture (one failed probe at 1_010, success at 1_300) and every post-recovery fixture built on its recoveredAt, for a shape the e2e cannot produce. Recorded as a follow-up on Rstest audit: pools, adapter, browser mode, flake risk, CI shape #576: a browser-side issue timestamp (CDP Network.requestWillBeSent, since Playwright's request.timing() is not populated for connection-refused requests) would make the credit unnecessary.

@changeset-bot

changeset-bot Bot commented Sep 5, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ffe5f5a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
agent-bundle Patch

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-05T02:28:11.770973Z 32a7c4d PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@pkg-pr-new

pkg-pr-new Bot commented Sep 5, 2026

Copy link
Copy Markdown
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle@584
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/create-agent-bundle@584
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/rsc-markdown-stream@584
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/@agent-bundle/runtime@584

commit: ffe5f5a

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 32a7c4d434

ℹ️ 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".

Comment thread packages/agent-bundle/src/dev/runtime-client-surface-proxy.ts
Comment thread packages/workbench/tests/packed-release.e2e.test.ts Outdated
ScriptedAlchemy and others added 12 commits September 5, 2026 03:51
…ean over the sequence, not per delivered gap (#576)
…im the Skills suites abort, cover the relay-state attribute (#576)
…illisecond; explain short retry gaps only by accumulated delivery lateness (#576)
…er probe failure in the outage ledger (#576)

CI (Release gates, runs 33933481002 and 33936651225) rejected the foreground
outage ledger with "project/session retry had a non-connection failure": the
first GET /api/project/session probe of the outage, issued while closeChild
was SIGTERM-ing the server, went out over a keep-alive connection the server
had already closed and failed 9 ms / 18 ms in with
net::ERR_SOCKET_NOT_CONNECTED (no response headers); every later probe was
REFUSED and recovery answered 200.

Add the code to downServerProbeCodes beside ERR_CONNECTION_RESET, so it is
accepted exactly where RESET is: a same-origin GET /api/project/session probe
inside the outage window with one terminal state, paced by the client's
250 ms cadence, and paired 1:1 with a console error carrying the same code
and URL. The old-session DELETE, non-GET probes, probes carrying response
headers, probes outside the window, and unpaired console errors stay
rejected with their existing messages; the unit test gains a row for each
plus the CI-shaped first-probe ledger.
…ter the DELETE; trim the changeset summary to the user-facing sentence (#584)
…report; pin expectPassedPool's fence in a unit test (#584)
@ScriptedAlchemy
ScriptedAlchemy enabled auto-merge (squash) September 5, 2026 06:00
@ScriptedAlchemy
ScriptedAlchemy merged commit 1fb100f into main Sep 5, 2026
16 checks passed
@ScriptedAlchemy
ScriptedAlchemy deleted the test/576-p1-test-fixes branch September 5, 2026 06:09
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