[CI] test: remove repeated idle polling from PTY cleanup

[nwparker]

ELI5

The PTY cleanup test repeatedly waits for a mocked kill signal using a 50ms polling interval. Check that same positive result every 1ms so successful cleanup does not spend most of its time waiting for the next check.

What Changed

Add { interval: 1 } to the existing vi.waitFor assertion in pty-subprocess-io-failure-cleanup.test.ts. That assertion executes 192 times across three simulated platforms, two I/O operations and 32 cleanup iterations. The same signal, exit, destruction and cleanup assertions, fixture work and real clock remain. The default 1,000ms deadline is preserved.

Why

Use the existing polling API to remove repeated idle time while preserving actual asynchronous cleanup. Shortening deadlines or reducing iterations would weaken the contract. More frequent retries can cost additional CPU when the expected signal does not arrive.

Linked Issue

Related measured test-suite improvements: #26327.

Visual Proof

N/A — test polling only; no visual or interaction change.

Testing

Correctness run 37694502389 passed all eight original/candidate commands under Bun 1.4.2 and actual Node 24.21.0, shuffled with seeds 104729 and 130363. Every command retained 36 exact original case identities and statuses: 32 passes and the same four platform skips. No errors, process leaks or source/routing changes were found. Root and independent raw reviews match the successful CI collector.

Timing run 37700932895 retained every whole pnpm test command in before/after, after/before, before/after order:

Pair	Before	After	Less time
1	13.454828271s	3.791638305s	71.82%
2	13.446895677s	3.815485544s	71.63%
3	13.309136070s	3.762464583s	71.73%
Mean	13.403620006s	3.789862811s	71.73%

This is 3.54× faster for this one fixture. All three pairs improve, including the first commands. There was no warmup, retry or discarded sample. Each command kept the same 36 cases, four workers, isolation, six setup files and disabled Vitest filesystem module cache. The held Linux ARM host used Vitest 5.0.3 with the pinned runtimes above; shared OS and Bun transpiler caches can still warm between counted commands.

The measurements use frozen source 31a5b42b0d272526c405458154eea932cd0ded8e. The product fixture before/after bytes match it exactly on current base 7abd74e8187d39cbdc8ff127969c5cd384a2a20b. Root independently replayed the collector, raw case/status maps, settings, process exit and arithmetic. These single-file results do not establish the overall 2× full-suite goal or faster real PTY termination.

• Existing cases, assertions, repetition counts and deadlines retained; no mirrored test added.
• Local oxfmt --check, changed-code checks and Node typecheck passed; no native or Electron tests launched locally.
• Full shipping-head CI37702546436 passed all ten unit shards, static analysis/typecheck and relay integration. All41 current-head checks completed:21 successes and20 explicit scope skips. Native smoke37702546026 passed on Linux, Windows and macOS; Windows packaged serve37702546366 passed.

Review

Independent source and all-eight correctness/all-six timing reviews passed. Root verifies the current candidate's exact qualified bytes. CodeRabbit reviewed current head16830 and reported no actionable findings. No unresolved review threads or change requests remain. Root verified the fixture before/after bytes match qualified source exactly, including the unchanged fixture on observed main e9b3efa. Candidate CI covers head16830; it does not cover additional upstream commits merged afterward. Merged squash5a1009f82fa1e1ae5e33e5ff965885fadc00415e has actual parent e9b3efa, exactly the final observed main. Root verified its one changed path and both fixture byte hashes match the qualified candidate exactly. The actual merge tree also contains seven upstream commits after candidate parent7abd (relay pacing, OpenCode ACP, remote history, Radix leaves, cache reset, compact tool runs and rail previews); candidate CI is not claimed to cover those additional commits.

Agent skill upstream boundary

• Not applicable; no upstream skill source copied.

Notes

No production process termination, SSH execution, remote protocol, workspace ownership or platform routing changes. Native contracts remain covered by CI; no native or Electron test was launched on the user's desktop.

Checklist

• Focused change; before/after measurements, mechanism and alternatives explained.
• N/A visual proof explained; original tests retained.
• Self-reviewed and independently checked; cross-platform and SSH considered.
• Full current-head CI and actionable review disposition complete.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 443fdb2e-d123-4041-b6c6-342b63658491

📥 Commits

Reviewing files that changed from the base of the PR and between a551aa5 and 16830a4.

📒 Files selected for processing (1)

• src/main/daemon/pty-subprocess-io-failure-cleanup.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

The failed-I/O cleanup test now uses a 1 ms polling interval while waiting for the SIGKILL call. The expected signal and process ID are unchanged.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 16830

No actionable merge-blocking risk remains from this polling change; it is reasonable to merge with normal checks.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the main change: removing repeated idle polling from the PTY cleanup test.
Description check	✅ Passed	The description covers the change, reason, testing, performance results, scope, review status, and checklist. The Linked Issue section gives issue #26327 but does not provide an actual issue link as r…

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}