mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 16:02:32 +00:00
Brennan Benson
2653794c82
* fix(terminal): verify Windows PTY root identity before taskkill /T /F killWithDescendantSweep guarded its Windows tree kill with ownsRoot() alone, which is JS state only. node-pty's ConPTY exit watcher closes the last shell handle before it queues the JS exit callback, so Windows can recycle the PID while the session map still looks live — force-killing an unrelated process and its whole descendant tree. Walk the recycled PID's ancestry back to this process before taskkill: skip the sweep when the root is gone or resolves to a stranger, and keep the sweep when identity is unknown so #10004 orphan cleanup still runs. Also gate the local provider's ownsRoot on observed physical exit. * fix(terminal): dedupe the Windows root-identity scan, drop dead exit gate Review fixes on the PID-identity guard. The probe read the process table through a new uncached export, bypassing the reader that worktree teardown depends on: worktree-teardown.ts fans out 32-wide inside a 10s deadline, so a delete forked 32 powershell cold-starts (the churn windows-foreground-process-rows.ts:25-32 warns about, #6288/#6667). getFreshSnapshot() already guarantees a scan that starts after the request -- the exact property the bypass existed for -- and coalesces concurrent callers, so use it. Measured on the new test: 32 scans -> 1. The PhysicalExitTracker.hasExited gate could never fire. markExited() is only reached at local-pty-provider.ts:985/:1431, and both are followed synchronously by clearPtyState(), which deletes the ptyProcesses entry -- so ownsRoot's map check is already false whenever hasExited is true. Reverting it broke no test. Drop it and the shared getter it added; the identity probe already covers every ownsRoot caller from inside killWithDescendantSweep. Also point the Windows terminal-restart E2E job at the files that own this behavior, so a change to the new Windows-only module runs the one job that executes on a real Windows host. * docs(terminal): state what the Windows root probe actually proves The probe checks subtree membership, not root identity: a recycle that lands on another Orca descendant (another pane's shell, an agent CLI, a git.exe we spawned) still reads `own`, and that is not remote during teardown when Orca is itself allocating pids. It bounds the blast radius rather than closing the class. Say so at the type and at classifyWindowsTreeKillTarget, and name what a real close would need (a CreationDate baseline -- the analogue of the POSIX lstart check already used here -- or an inherited handle / Job Object). Also note why our own pid must classify `foreign`. * ci(windows): trigger the terminal-restart E2E on the shared snapshot reader The Windows root-identity probe now reads through getFreshSnapshot, so an edit to that module changes Windows teardown behavior without touching any path the job already watches. * test(terminal): guard the teardown probe against a reintroduced scan bypass The existing volume guard covers queryWindowsProcessRowsFresh directly, but the identity-probe cases all inject readRows, so nothing exercised the DEFAULT reader wiring -- a bypass reintroduced inside windows-pty-root-identity would have gone unnoticed. Drive verifyWindowsTreeKillTarget 32-wide through the real reader and assert one scan. Verified it fails at 32 when the bypass is put back.