Skip to content

fix(orcad): Windows orcad stops cleanly when its Electron browser sidecar's profile is still locked - #29562

Merged
nwparker merged 2 commits into
mainfrom
nwparker/sta-10787-orcad-stop-hang
Oct 16, 2026
Merged

nwparker merged 2 commits into
mainfrom
nwparker/sta-10787-orcad-stop-hang

Conversation

@nwparker

@nwparker nwparker commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​46 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​43
Prod 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​7 $\color{#cf222e}{\Huge{\mathbf{−}}}$​2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​5

ELI5

On a Windows machine that also has the Orca desktop app installed, the headless Orca server (orcad) borrows that app as its browser. When orcad stopped, it tried to delete the borrowed browser's scratch folder right after killing it, but Windows was still holding the folder for a moment. That delete failed, and orcad treated the whole stop as failed: it exited with an error and kept its instance lock. Now a scratch folder that won't delete just gets a warning, and the stop still succeeds.

Separately, the CI test that checks orcad's stop timed out on Windows ARM runners. orcad itself stopped fine there. The test's cleanup (deleting its temp folder) was simply slower than its 60-second allowance.

What Changed

  • src/main/orcad/electron-serve-browser-process.ts: stop() now removes the sidecar's temporary profile with the shared removeTree helper (Node's Windows retry policy) and only logs a failure instead of throwing it. Before: on Windows, orcad's stop could fail with EBUSY ... orcad-browser-XXXX and exit 1 without releasing its instance lock. After: orcad exits 0 and releases the lock. At worst a throwaway temp folder is left behind.
  • src/main/orcad/orcad-stop-request-shutdown.integration.test.ts: the afterEach teardown gets 180 s instead of Vitest's default 60 s.

Why

  • The scratch folder belongs only to the killed sidecar. Nothing writes to orcad's profile through it, so leaving it behind is harmless. Failing the stop, on the other hand, keeps the profile fenced and reports a clean shutdown as a failure. I considered waiting for the whole Electron process tree to exit, but that needs per-platform tree enumeration and still can't promise Windows has let go of the handles. Retrying, then giving up with a warning, covers both cases.
  • Teardown timeout: on windows-11-arm, instrumented runs at current main and at 7005af3 alike took 14–22 s to delete the test's freshly written slot and runtime, even with no process left under the root. Under full-suite load, that went past 60 s. The 64 s failures (about 4 s of test body plus exactly 60 s of hook) match this. This isn't a regression from fix(ssh): agents on a managed SSH host can drive the desktop Orca browser again, behind the per-host switch #29384: the same timings show up at 7005af3 and with that change reverted.

Linked Issue

Linear STA-10787

Visual Proof

N/A: no UI change. These are orcad shutdown and test-teardown changes.

Testing

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

  • New unit test: electron-serve-browser-process.test.ts "still stops cleanly when the sidecar profile stays locked after the process exits". It fails before the fix (stop rejects with EBUSY) and passes after.

  • Windows x64 (awin, Orca desktop installed, test pinned to 1 CPU): before, the stop-request integration test failed 5/6 on main and 4/4 at 7005af3 with orcad exiting 1 on EBUSY orcad-browser-*. After the fix, orcad exited 0 every time.

  • With the installed app hidden from the test, main and 7005af3 both passed 8/8 at 1 CPU and 11/11 unconstrained. macOS passes too.

  • windows-11-arm dispatch runs at main, at main with the fix(ssh): agents on a managed SSH host can drive the desktop Orca browser again, behind the per-host switch #29384 lifecycle lines reverted, and at 7005af3 all passed with an instrumented, longer hook timeout. They show where the teardown time goes.

AI Disclosure

Review

Codex review (gpt-6.1-sol, high): no findings.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Cross-platform: removeTree adds retries only on Windows. On macOS and Linux the behavior is unchanged except that a failed delete no longer fails the stop. SSH: this is the execution host's own orcad stop. No wire change.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@nwparker
nwparker merged commit 172a5d2 into main Oct 16, 2026
25 of 27 checks passed
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