Windows · Re-reviewed · Approved
The remaining discovery failures are fixed and the review cases pass
153 focused TypeScript tests, 18 Rust tests, and all five carried-forward review cases pass on Windows x64. The scanner retains native listener state during an outage and starts failed-probe cooldowns at completion. The new client integration test exercises exit handling and replacement response dispatch with controlled child-process handles. No blocking finding remains in this scoped review.
Approve the reviewed commit
Source: focused Vitest suite, Rust tests, and five carried-forward reviewer cases.
Approve. Both previously blocking scanner failures are fixed, and the request-lifecycle coverage concern is addressed.
PR #9520, head 4c5178e31ac95496fdc89a355b76a9237db29ad8, base 6c583620ff7ad3235b135af7107c0543467eecfa. The previous reviewed head was b58b4cbc43edf9a73f95a388be9ea6dc2bcc8c85. This report supersedes the unresolved findings in the September 8 review.
The numbered findings are resolved
1. Native discovery now updates retained state
A successful native result now replaces the retained snapshot, including an empty result. When native and PowerShell discovery later fail, the scanner can preserve a known non-common port and reapply current terminal ownership. The earlier port-43123 reproduction passes; the added repository test verifies refreshed ownership too.
Source: native snapshot storage, scanner tests and reviewer cases.
2. Failure cooldown starts after the probe finishes
The failure path reads the completion time before computing its next attempt. A 15-second failed probe no longer consumes its own initial cooldown. The earlier slow-failure reproduction passes, as do the nonzero, truncated-output, and successful-pacing cases.
Source: completion-based retry deadline and five reviewer cases.
3. The restart test exercises the actual client service
The new test provides controlled child-process handles to the production NativeTelemetryClient. It exits the first handle during an in-flight process-table request, observes NativeTelemetryExited, waits for replacement configuration, and verifies the replacement response. Fixtures pass through the production protocol schemas. This is stronger than the earlier helper-only test, while remaining a simulated-spawner test rather than a native OS-process crash test.
Source: client restart integration test.
4. Track the inherited Node watch-mode PTY crash separately
Terminal restart in the dev runner crashes in node-pty 1.1.0 at windowsPtyAgent.js:141 because the console process list is undefined. A standalone PTY program reproduces the same parent crash under Node --watch, outside the discovery code. The dependency, adapter and watch launch are unchanged by this PR. Without watch mode, the reviewed server passes restart, immediate close and a subsequent RPC. This follow-up does not reopen the fixed discovery findings.
Source: live diagnostic output, standalone watch-kill-repro.cjs and log, base comparison.
One retained snapshot supports degraded discovery
Implemented discovery flow
Source: reviewed PortScanner implementation at the pinned head.
Native protocol, Rust listener collection, and the production client request helper are unchanged since September 8. The production fixes are confined to retained state and failure deadlines. The new base separately fixes Windows PTY signal forwarding; that fix is not attributed to this PR.
Windows execution supports approval
| Check | Result | Evidence |
|---|---|---|
| Retain native snapshot during outage | Resolved | Previous reproduction passes; current ownership is reapplied |
| Slow failed fallback cooldown | Resolved | Retry deadline starts at completion |
| Nonzero / truncated fallback results | Still resolved | Both original regression cases pass |
| Successful fallback pacing | Pass | 5-second query waits 20 seconds from completion |
| Client restart coverage | Resolved | Production service, simulated child handles, exit and replacement dispatch |
| Focused TypeScript tests | 153 passed | 7 files including terminal, scanner, telemetry and diagnostics |
| Rust tests / release build / format | 18 passed | Native Windows x64 |
| Carried-forward review cases | 5 passed | All previous reproductions are now green |
| bun fmt / bun lint / bun typecheck | Pass | Clean worktree; existing warnings remain |
Source: downloadable evidence bundle.
Native probes pass for process ancestry, IPv4/IPv6 ownership and listener removal. The direct server passes live activity, ownership, Ctrl+C, removal, restart and immediate close followed by a server RPC. Windows computer use loads the isolated app and opens its terminal drawer. The initial watch-mode run crashed in the unchanged node-pty dependency; a standalone watch-mode reproduction is included.
Source: Windows computer-use screenshot of the isolated review app. Browser chrome is cropped; live command assertions ran through public RPC.
Confidence is high for the reproduced failure paths and tested Windows x64 behavior. No sustained CPU/fan comparison, installer acceptance, ARM64, POSIX, or long soak was performed. The UI smoke and RPC checks cover specific flows, not every frontend entry point.
Evidence can be rerun on the pinned head
The bundle contains commands, regression fixtures and outputs. Credentials, private application state and dev-server logs are excluded. Run the extra reviewer file beside PortScanner.test.ts with bun run test src/preview/PortScanner.rereview.test.ts -t 'review:', then remove it before clean-head checks. Live RPC probes require a new local test credential.
tests.log, full 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:55676) 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 153 passed (153)
Start at 15:36:38
Duration 26.12s (transform 7.71s, setup 1.08s, import 19.35s, tests 2.76s, environment 1ms)
regressions.log, full output
$ vp test run src/preview/PortScanner.rereview.test.ts -t review:
RUN v4.1.11 E:/Projects/t3code-review-9520/apps/server
Test Files 1 passed (1)
Tests 5 passed | 28 skipped (33)
Start at 15:36:55
Duration 2.38s (transform 647ms, setup 177ms, import 1.87s, tests 49ms, environment 0ms)
rust-tests.log, full output
Finished `test` profile [unoptimized + debuginfo] target(s) in 3.61s
Running unittests src\main.rs (native\resource-monitor\target\debug\deps\t3_resource_monitor-fed045c766d21202.exe)
running 18 tests
test tests::accepts_clock_shifts_without_accepting_reused_process_starts ... ok
test tests::bounds_history_by_estimated_process_bytes ... ok
test tests::counts_external_processes_toward_history_limits ... ok
test tests::ignores_missing_roots ... ok
test tests::counts_selected_processes_that_could_not_be_materialized ... ok
test tests::clamps_sample_interval ... ok
test tests::refreshes_commands_without_enumerating_linux_tasks ... ok
test tests::excludes_and_trims_future_history_after_the_clock_moves_backward ... ok
test tests::rejects_descendants_older_than_a_reused_parent_pid ... ok
test tests::decodes_protocol_commands ... ok
test tests::truncates_process_strings_at_utf8_boundaries ... ok
test tests::sample_now_does_not_postpone_an_existing_periodic_deadline ... ok
test tests::validates_external_process_start_identity ... ok
test tests::selects_roots_and_all_descendants ... ok
test tests::waits_for_a_cpu_measurement_window_after_priming ... ok
test tests::reads_windows_listener_table ... ok
test tests::retains_bounded_history_without_request_ids ... ok
test tests::loads_details_when_an_existing_process_becomes_selected ... ok
test result: ok. 18 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.51s
native.json, full output
{
"version": 4,
"platform": "win32",
"architecture": "x64",
"node": "v24.13.1",
"requests": [
{
"type": "processTable",
"ms": 87.115,
"count": 515
},
{
"type": "processTable",
"ms": 57.339,
"count": 515
},
{
"type": "processTable",
"ms": 47.937,
"count": 515
},
{
"type": "processTable",
"ms": 37.434,
"count": 515
},
{
"type": "processTable",
"ms": 33.582,
"count": 515
},
{
"type": "processTable",
"ms": 37.793,
"count": 512
},
{
"type": "processTable",
"ms": 32.903,
"count": 514
},
{
"type": "processTable",
"ms": 40.004,
"count": 514
},
{
"type": "processTable",
"ms": 29.874,
"count": 513
},
{
"type": "processTable",
"ms": 32.943,
"count": 514
},
{
"type": "processTable",
"ms": 32.691,
"count": 513
},
{
"type": "processTable",
"ms": 31.101,
"count": 513
},
{
"type": "windowsListeners",
"ms": 39.948,
"count": 54
},
{
"type": "windowsListeners",
"ms": 50.7,
"count": 54
},
{
"type": "windowsListeners",
"ms": 34.588,
"count": 54
},
{
"type": "windowsListeners",
"ms": 33.923,
"count": 54
},
{
"type": "windowsListeners",
"ms": 29.641,
"count": 54
},
{
"type": "windowsListeners",
"ms": 30.756,
"count": 54
}
],
"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 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
},
{
"name": "server responds after immediate restart-close"
}
],
"pass": true
}watch-kill-repro.log, full output
PTY emitted output; calling kill without a POSIX signal
E:\Projects\t3code-review-9520\node_modules\.pnpm\node-pty@1.1.0\node_modules\node-pty\lib\windowsPtyAgent.js:141
consoleProcessList.forEach(function (pid) {
^
TypeError: Cannot read properties of undefined (reading 'forEach')
at E:\Projects\t3code-review-9520\node_modules\.pnpm\node-pty@1.1.0\node_modules\node-pty\lib\windowsPtyAgent.js:141:40
at process.processTicksAndRejections (node:internal/process/task_queues:103:5)
Node.js v24.13.1
{"watchMode":true,"parentCrashReproduced":true}
Failed running 'E:\\Projects\\t3code-review-evidence\\2026-09-12/pty-kill-repro.cjs'. Waiting for file changes before restarting...
Generated 2026-09-12 · Codex review for Dara · Windows x64, Node v24.13.1 · 4c5178e31ac95496fdc89a355b76a9237db29ad8 · revision v3