Skip to content

fix(computer-use): clean up permission helpers left by earlier runs - #26114

Open
silee9019 wants to merge 1 commit into
stablyai:mainfrom
silee9019:fix-cu-helper-leak
Open

silee9019 wants to merge 1 commit into
stablyai:mainfrom
silee9019:fix-cu-helper-leak

Conversation

@silee9019

@silee9019 silee9019 commented Oct 7, 2026 •

Copy link
Copy Markdown

ELI5

Orca now cleans up macOS permission-check helpers left running by earlier runs when it starts. These helpers can accumulate and contribute to the system-wide app-launch hangs reported in #22436.

What Changed

Before, a helper that outlived its permission check stayed running after Orca deleted the check's temporary directory. After this change, startup lists only the current user's processes and stops matching helpers whose status directory has disappeared.

A matching helper must end its command with orca-computer-use-macos --permission-status-file .../orca-computer-use-permissions-*/status.json. An existing directory preserves the helper, including a check in another Orca instance. Setup helpers and unrelated commands are excluded. Process listing is limited to five seconds and 4 MiB; listing errors and timeouts skip cleanup. A helper that already exited is ignored.

The startup call does not wait for cleanup. This runs only on macOS and adds no logging, settings, dependencies or remote protocol changes.

Why

#22451 stops a helper when its own check ends. Startup cleanup covers helpers left by earlier runs and builds, so the changes complement each other. Checking the per-check directory preserves ongoing checks across profiles; a broad process-name kill would not.

Linked Issue

Refs #22436. Complements #22451; this change does not close the issue on its own.

Visual Proof

N/A: background process cleanup; no UI or interaction changes.

Testing

Verified on macOS after rebasing onto upstream main. Commands used Node 24 and pnpm 12.8.1 with ORCA_BACKGROUND_LAUNCH=1.

  • pnpm test src/main/computer: exit 0; 185 passed, 4 skipped; 28 files passed, 1 skipped. The new sweep file contains six tests.
  • pnpm tc:node: exit 0.
  • pnpm run check:code-quality:changed 4077fb4a8de80059bbb218b58d14ed1efdef113f: exit 0; zero new findings across all three changed files.
  • pnpm exec oxlint src/main/computer/macos-computer-use-permission-helper-sweep.ts src/main/computer/macos-computer-use-permission-helper-sweep.test.ts src/main/startup/main-process-ready-runtime.ts: exit 0.

Earlier development-mode evidence used two fake helpers: one with an existing status directory and one with a missing directory. Startup stopped only the latter. The sweep implementation and tests are unchanged by rebase (67f91d8227 to 7323f46326, equal patches). A real helper stuck during system initialization and a packaged app were not tested; the development-mode process test is the available runtime evidence. No app or daemon was restarted for this submission.

  • I manually tested these changes locally (development-mode evidence above)
  • Automated tests added/updated, or explained why not below

Review

No additional review was run for this submission.

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

The cleanup touches only macOS processes on the execution host, independent of workspace type. Windows and Linux return without listing processes. SSH workspace execution, mobile clients and wire compatibility are unchanged. A directory left on disk after a crash is conservatively kept. Cleanup is repeated at startup and tolerates already-exited helpers.

Before and After Evidence

The same sweep tests distinguish source before and after the change. Earlier RED evidence used the added test against the previous source: the sweep module was absent, so the test failed to load. GREEN at 7323f46326 runs the six sweep tests within the passing computer suite. The startup integration retains the same added call after rebase onto 4077fb4a8d.

Before

Before (9fdd90ffa7): the new test failed because the sweep module did not exist; no startup sweep reclaimed helpers whose status directories had disappeared.

After

After (7323f46326): six sweep tests pass; the reused development-mode check stopped the fake helper with a missing directory while preserving the helper with an existing directory. The original runtime observation was made before rebase; patch equality confirms the same sweep implementation.

Issue Evidence

  • Issue evidence not applicable: authorization covers this PR only; posting comments on the issue or related PR is outside scope.

Review Findings and Disposition

  • No findings.

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)

… behind

A permission check starts its helper with `open -n`, so a helper that wedges
outlives the timeout and keeps its LaunchServices registration. At startup,
SIGKILL this user's helpers whose status directory no longer exists. A live
directory means a check is still in flight, possibly in another Orca instance,
so those helpers are left alone.
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: c92ea267-c07b-4c1e-b3cb-48685e43d927
📥 Commits

Reviewing files that changed from the base of the PR and between 4077fb4 and 7323f46.

📒 Files selected for processing (3)
  • src/main/computer/macos-computer-use-permission-helper-sweep.test.ts
  • src/main/computer/macos-computer-use-permission-helper-sweep.ts
  • src/main/startup/main-process-ready-runtime.ts

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


📝 Walkthrough

Walkthrough

The change adds functions to find and terminate stale macOS permission-status helper processes. The sweep lists processes for the current user, identifies helpers whose status directories no longer exist, and returns the PIDs it successfully terminates. Startup invokes the sweep after sweeping orphaned agent-browser sessions.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 7323f

This change cleans up stale macOS permission helpers at startup without blocking launch. No merge-blocking risk is evident. A real wedged helper and a packaged app were not tested.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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: cleaning up permission helpers left by earlier runs.
Description check ✅ Passed The description covers the required sections and explains the change, rationale, issue reference, lack of UI changes, testing, and platform considerations. It reports test and type-check results, alth…
  • Fix all pre-merge checks with AI
  • 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

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

This branch has not been deployed

No deployments
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