Windows · Changes requested · PR 9520
Native discovery works, but failed fallbacks erase known servers
144 focused TypeScript tests and 18 Rust tests pass on Windows x64. Native IPv4 and IPv6 discovery identifies the correct process, and live terminal ownership follows a real HTTP server. Two added regression tests fail because nonzero and truncated PowerShell results clear a known server and skip cooldown. Fix that failure classification before approval.
The review decision follows the changed behavior
Source: focused TypeScript and Rust results, extra review tests, native request probe. Latency is a local sample, not a CPU benchmark.
Request changes. Preserve known listeners and apply cooldown when PowerShell returns a failed or partial result.
Reviewed PR #9520 at c27fa4e1b131311051d609a344ca2c6f5e7262a2, against common base 761d4bac1c238ea7af4dd36b56719ad5e30771c3. This draft stacks on #9476; its incremental diff was reviewed against the #9476 head.
Numbered findings distinguish regressions from follow-ups
1. Reject unsuccessful PowerShell results before replacing the snapshot
P2 · verified PortScanner.ts:547 parses stdout without checking code or stdoutTruncated. ProcessRunner.run returns nonzero exit codes as ordinary results, and this call explicitly requests truncated output.
Start with one valid listener, then return exit code 1 with empty stdout. The next scan returns zero servers. An immediate third scan launches PowerShell again because the success branch reset both the failure counter and retry timestamp. Returning a truncated table produces the same result. Both review tests expect the prior listener and two total launches, but receive zero listeners and three launches.
Validate exit status and completeness before parsing. Route invalid results through the existing failure branch, retaining the last successful snapshot and advancing cooldown. Add both cases to the focused suite.
Source: two failing review tests against c27fa4e1; production source lines 543–569; ProcessRunner result handling.
2. Track the existing Windows PTY close crash separately
Follow-up · reproduced Rapid restart followed by close crashes the server with Signals not supported on windows. The unchanged adapter forwards SIGTERM to node-pty@1.1.0. Before its socket is ready, node-pty defers that call and throws later, outside the manager's synchronous error handling.
A standalone reproduction using the same dependency exits with code 1. The adapter and dependency lockfile are unchanged from the common base. The manager's explicit signal path also predates these changes. Use the Windows PTY termination API without POSIX signals and cover close-before-ready in a separate fix. The operation acknowledgements passed before cleanup, so they must not be described as a clean shutdown test.
Source: baseline NodePtyAdapter.kill and Manager kill escalation; standalone reproduction and live-server crash on both PRs.
3. Tighten request lifecycle tests and fallback consistency
Nonblocking The new request methods repeat the existing deferred-map, timeout, and finalizer sequence. A small shared request helper would reduce drift if more sidecar commands are planned. Add direct request/response, timeout, interruption, and sidecar-restart coverage for the new commands. The existing NativeTelemetryClient suite passes, but this PR does not add tests to that file.
Also consider matching #9476's treatment of native failure. Here a successful PowerShell fallback resets the counter, so a missing sidecar restores a PowerShell query on every scan. That is separate from the confirmed nonzero-exit bug and may be an intentional compatibility tradeoff.
Source: NativeTelemetryClient.ts and the PR file list.
The design keeps process discovery inside the sidecar
Listener request path
Source: reviewed manager/scanner, NativeTelemetryClient and native/resource-monitor at the pinned head.
Protocol v4 adds windowsListeners. The sidecar reads both TCP address families, filters local listeners and joins process names. The client maps request IDs to deferred responses; the scanner deduplicates ports, attaches current terminal owners and checks HTTP readiness. The required correction belongs at the fallback-result boundary, before replacing lastSnapshot.
Keep native binary and contract versions aligned in release packaging. A mismatched or unavailable binary must take the fallback path without corrupting the last authoritative state. Scope remains server-side; this review did not change product code.
Windows execution supports the verdict
| Check | Result | Evidence boundary |
|---|---|---|
| bun fmt | Pass | No tracked formatting changes |
| bun lint | Pass | Existing warnings remain; exit 0 |
| bun typecheck | Pass | 15 workspace tasks; exit 0 |
| Focused TypeScript suite | 144 passed | 7 files, including terminal, preview, telemetry and diagnostics |
| Rust tests and release build | 18 passed | Native Windows x64; cargo fmt --check also passes |
| Native protocol probe | Pass | IPv4 and IPv6 listener ownership; removal after close |
| Live server / real PTY | Activity and discovery pass | Node label, terminal ownership, Ctrl+C idle, port removal, restart acknowledgement |
| Rapid restart / close | Existing crash reproduced | Separate PTY signal path; not a clean shutdown pass |
| Additional failure cases | 2 failed | Nonzero exit and truncated output lose the prior snapshot |
Source: commands and raw outputs in the downloadable evidence bundle.
Confidence is high for the tested Windows x64 native path and the reproduced failure cases. No macOS, Linux or Windows ARM64 run was performed. Installer packaging, a sustained CPU/fan comparison, and a long soak were not measured. The separate #9520 frontend was not walked through; its native and live RPC behavior was exercised directly.
The browser extension could not attach through its Firefox compatibility layer, and its emulated tab failed in a matchMedia listener. An ordinary tab operated through Windows computer use loaded successfully. These tooling errors are excluded from PR findings.
Raw evidence makes the checks reproducible
The bundle includes the native probe, live RPC test, focused test outputs and standalone PTY reproduction. Credentials and dev-server logs are excluded. The live RPC script needs a freshly issued local test credential; no token is distributed.
native-9520.json · full captured output
{
"version": 4,
"platform": "win32",
"architecture": "x64",
"node": "v24.13.1",
"requests": [
{
"type": "processTable",
"ms": 104.712,
"count": 418
},
{
"type": "processTable",
"ms": 23.444,
"count": 418
},
{
"type": "processTable",
"ms": 27.4,
"count": 418
},
{
"type": "processTable",
"ms": 23.355,
"count": 418
},
{
"type": "processTable",
"ms": 23.596,
"count": 418
},
{
"type": "processTable",
"ms": 21.843,
"count": 418
},
{
"type": "processTable",
"ms": 24.156,
"count": 419
},
{
"type": "processTable",
"ms": 21.814,
"count": 419
},
{
"type": "processTable",
"ms": 22.734,
"count": 419
},
{
"type": "processTable",
"ms": 21.912,
"count": 419
},
{
"type": "processTable",
"ms": 23.649,
"count": 419
},
{
"type": "processTable",
"ms": 22.526,
"count": 419
},
{
"type": "windowsListeners",
"ms": 24.43,
"count": 32
},
{
"type": "windowsListeners",
"ms": 23.722,
"count": 32
},
{
"type": "windowsListeners",
"ms": 23.994,
"count": 32
},
{
"type": "windowsListeners",
"ms": 22.2,
"count": 32
},
{
"type": "windowsListeners",
"ms": 26.838,
"count": 32
},
{
"type": "windowsListeners",
"ms": 21.839,
"count": 32
}
],
"checks": [
"12 full process tables include the sidecar with its correct parent",
"IPv4 and IPv6 loopback listeners found with correct PID and process name in 6 snapshots",
"Closed listeners disappear on the next native snapshot"
],
"pass": true,
"stderr": "",
"unsolicitedEvents": [
"hello"
]
}live-9520.json · full captured output
{
"number": "9520",
"checks": [
{
"name": "real Windows PTY opened",
"status": "running"
},
{
"name": "running child updates terminal activity",
"label": "node"
},
{
"name": "HTTP fixture discovered with terminal ownership",
"port": 19520,
"processName": "node.exe"
},
{
"name": "stopped child returns terminal to idle"
},
{
"name": "closed listener disappears from discovery"
},
{
"name": "terminal restarts",
"status": "running",
"newPid": true
}
],
"pass": true,
"cleanup": "Server crashed after rapid restart then close with node-pty Signals not supported on windows. Reproduced independently in the unchanged baseline adapter/dependency path.",
"passScope": "Listed operation acknowledgements and observed activity/discovery transitions; not clean end-to-end shutdown."
}
tests-9520.log · full captured output
$ vp test run src/terminal/Manager.test.ts src/preview/PortScanner.test.ts src/resourceTelemetry/NativeTelemetryClient.test.ts src/resourceTelemetry/ResourceTelemetry.test.ts src/resourceTelemetry/ResourceTelemetryHistory.test.ts src/resourceTelemetry/Model.test.ts src/diagnostics/ProcessDiagnostics.test.ts
RUN v4.1.11 E:/Projects/t3code-review-9520/apps/server
(node:31244) ExperimentalWarning: SQLite is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
Test Files 7 passed (7)
Tests 144 passed (144)
Start at 14:14:10
Duration 21.82s (transform 6.08s, setup 944ms, import 15.69s, tests 2.72s, environment 1ms)
regressions-9520.log · full captured output
$ vp test run src/preview/PortScanner.review.test.ts -t review:
RUN v4.1.11 E:/Projects/t3code-review-9520/apps/server
stdout | src/preview/PortScanner.review.test.ts > review: nonzero PowerShell exits retain the last snapshot and back off
[14:14:22.724] ERROR (#10): Error [AssertionError]: expected { afterFailure: +0, …(2) } to deeply equal { afterFailure: 1, …(2) }
at Array.<anonymous> (E:/Projects/t3code-review-9520/apps/server/src/preview/PortScanner.review.test.ts:907:96) {
actual: { afterFailure: 0, duringCooldown: 0, runs: 3 },
expected: { afterFailure: 1, duringCooldown: 1, runs: 2 },
showDiff: true,
operator: 'deepStrictEqual',
[cause]: undefined
}
stdout | src/preview/PortScanner.review.test.ts > review: truncated PowerShell tables retain the last snapshot and back off
[14:14:22.739] ERROR (#33): Error [AssertionError]: expected { afterFailure: +0, runs: 3 } to deeply equal { afterFailure: 1, runs: 2 }
at Array.<anonymous> (E:/Projects/t3code-review-9520/apps/server/src/preview/PortScanner.review.test.ts:927:57) {
actual: { afterFailure: 0, runs: 3 },
expected: { afterFailure: 1, runs: 2 },
showDiff: true,
operator: 'deepStrictEqual',
[cause]: undefined
}
❯ src/preview/PortScanner.review.test.ts (26 tests | 2 failed | 24 skipped) 37ms
× review: nonzero PowerShell exits retain the last snapshot and back off 25ms
× review: truncated PowerShell tables retain the last snapshot and back off 11ms
⎯⎯⎯⎯⎯⎯⎯ Failed Tests 2 ⎯⎯⎯⎯⎯⎯⎯
FAIL src/preview/PortScanner.review.test.ts > review: nonzero PowerShell exits retain the last snapshot and back off
AssertionError: expected { afterFailure: +0, …(2) } to deeply equal { afterFailure: 1, …(2) }
- Expected
+ Received
{
- "afterFailure": 1,
- "duringCooldown": 1,
- "runs": 2,
+ "afterFailure": 0,
+ "duringCooldown": 0,
+ "runs": 3,
}
❯ Array.<anonymous> src/preview/PortScanner.review.test.ts:907:96
905| const afterFailure = yield* scanner.scan();
906| const duringCooldown = yield* scanner.scan();
907| expect({ afterFailure: afterFailure.length, duringCooldown: during…
| ^
908| }).pipe(Effect.provide(layer));
909| });
❯ Object.~effect/Effect/successCont ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/internal/effect.ts:1355:25
❯ Object.~effect/Effect/evaluate ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/internal/core.ts:514:30
❯ FiberImpl.runLoop ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/internal/effect.ts:654:39
❯ FiberImpl.evaluate ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/internal/effect.ts:606:22
❯ Array.<anonymous> ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/internal/effect.ts:978:12
❯ MixedSchedulerDispatcher.runTasks ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/Scheduler.ts:230:16
❯ Immediate.afterScheduled [as _onImmediate] ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/Scheduler.ts:219:9
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/2]⎯
FAIL src/preview/PortScanner.review.test.ts > review: truncated PowerShell tables retain the last snapshot and back off
AssertionError: expected { afterFailure: +0, runs: 3 } to deeply equal { afterFailure: 1, runs: 2 }
- Expected
+ Received
{
- "afterFailure": 1,
- "runs": 2,
+ "afterFailure": 0,
+ "runs": 3,
}
❯ Array.<anonymous> src/preview/PortScanner.review.test.ts:927:57
925| const afterFailure = yield* scanner.scan();
926| yield* scanner.scan();
927| expect({ afterFailure: afterFailure.length, runs }).toEqual({ afte…
| ^
928| }).pipe(Effect.provide(layer));
929| });
❯ Object.~effect/Effect/successCont ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/internal/effect.ts:1355:25
❯ Object.~effect/Effect/evaluate ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/internal/core.ts:514:30
❯ FiberImpl.runLoop ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/internal/effect.ts:654:39
❯ FiberImpl.evaluate ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/internal/effect.ts:606:22
❯ Array.<anonymous> ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/internal/effect.ts:978:12
❯ MixedSchedulerDispatcher.runTasks ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/Scheduler.ts:230:16
❯ Immediate.afterScheduled [as _onImmediate] ../../node_modules/.pnpm/effect@4.0.0-beta.103_patch_6038b7730ee3fca18d01d9095da33905/node_modules/effect/src/Scheduler.ts:219:9
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/2]⎯
Test Files 1 failed (1)
Tests 2 failed | 24 skipped (26)
Start at 14:14:20
Duration 2.22s (transform 610ms, setup 171ms, import 1.69s, tests 37ms, environment 0ms)
error: script "test" exited with code 1
pty-close-baseline.log · full captured output
PTY spawned, forwarding the SIGTERM used by unchanged NodePtyAdapter.kill
E:\Projects\t3code-review-9476\node_modules\.pnpm\node-pty@1.1.0\node_modules\node-pty\lib\windowsTerminal.js:151
throw new Error('Signals not supported on windows.');
^
Error: Signals not supported on windows.
at WindowsTerminal.<anonymous> (E:\Projects\t3code-review-9476\node_modules\.pnpm\node-pty@1.1.0\node_modules\node-pty\lib\windowsTerminal.js:151:23)
at Object.run (E:\Projects\t3code-review-9476\node_modules\.pnpm\node-pty@1.1.0\node_modules\node-pty\lib\windowsTerminal.js:166:50)
at E:\Projects\t3code-review-9476\node_modules\.pnpm\node-pty@1.1.0\node_modules\node-pty\lib\windowsTerminal.js:73:28
at Array.forEach (<anonymous>)
at Socket.<anonymous> (E:\Projects\t3code-review-9476\node_modules\.pnpm\node-pty@1.1.0\node_modules\node-pty\lib\windowsTerminal.js:68:38)
at Object.onceWrapper (node:events:623:26)
at Socket.emit (node:events:520:35)
at addChunk (node:internal/streams/readable:559:12)
at readableAddChunkPushByteMode (node:internal/streams/readable:510:3)
at Readable.push (node:internal/streams/readable:390:5)
Node.js v24.13.1
Generated 2026-09-05 · Codex review for Dara · Windows x64, Node v24.13.1 · commit c27fa4e1b131311051d609a344ca2c6f5e7262a2 · revision v1