fix(terminal): bypass rejected macOS login sessions

[brennanb2025]

Summary

• Probe the macOS login(1) account/session path before using it to launch a terminal shell.
• Run the one-time probe asynchronously so PAM cannot block Electron's main loop or the daemon's existing terminal/RPC traffic.
• Require both a child-emitted marker and a clean exit because login(1) can return zero after an EOF-driven failed authentication prompt.
• Send EOF immediately, cap captured output at 1 KiB, and enforce a 500 ms SIGKILL timeout; cache and deduplicate the result per PTY-host process.
• Fall back to direct shell spawning when PAM rejects or the probe cannot run, preventing terminals from getting stranded at Login incorrect / login:.
• Run the probe from the account home so a detached daemon whose launch worktree was deleted still works.
• Bump the daemon protocol to v23 so old v22 sessions can remain attached while new terminal sessions route through a daemon with the fixed spawn behavior.
• Preserve the TCC-attributed login(1) launch path on compatible macOS accounts.

ELI5

On macOS, Orca starts terminal shells through a system login helper so command-line tools can keep the right privacy permissions. On some Macs, account policy rejects that helper even though the user is already signed in. The terminal then gets stuck showing “Login incorrect” instead of opening a shell.

This change gives the helper one quick test before using it. If the test succeeds, Orca keeps the permission-friendly behavior. If it fails or hangs, Orca safely opens the shell the normal way. The test runs only once, does not freeze the app, and has a half-second hard limit. The daemon version is also bumped so an old background process cannot keep using the broken behavior.

Electron + real Codex account proof

Tested in the Electron build from this PR worktree with the machine's real ChatGPT-backed Codex login. Orca spawned Codex CLI 0.144.5 through a fresh visible terminal, and the authenticated model returned the deterministic response ORCA_E2E_AUTH_OK.

The screenshot contains no token, email address, account ID, or other credential.

Reliability contract

• Invariant: every fresh terminal reaches the configured shell; a rejected or hung PAM session must never leave the PTY at a login: prompt. Existing terminals and unrelated daemon RPCs stay responsive during detection.
• Failure sources: PAM rejection/hang, misleading zero exit after EOF, excessive child output, deleted daemon cwd, concurrent restores, and a still-running pre-fix daemon.
• Success oracle: only the exact fixed marker from the requested child program plus a clean process exit enables the wrapper.
• Reliability gate: stdin EOF is immediate, lifetime is capped at 500 ms with SIGKILL, output is capped at 1 KiB, concurrent callers share one child, pending creations are cancelable before subprocess registration, and failure is cached as direct-spawn fallback.
• Coverage: both the in-process local provider and detached daemon await preparation at their async fresh-spawn boundary. Non-macOS paths are unchanged; SSH/remote macOS evaluates capability in the PTY-host process that actually launches the shell.
• Performance budget: no synchronous child process remains. Healthy live probes completed in roughly 55-70 ms on the test Mac, once per PTY-host process; the hard worst-case delay is 500 ms and concurrent restores do not multiply probes.
• Diagnostics / accepted gap: one warning records fallback without logging PAM output. ORCA_DISABLE_MACOS_LOGIN_SHELL=1 remains the escape hatch. A mid-process account-policy change takes effect after PTY-host restart; until then the cached result deliberately favors a usable direct shell.

Testing

• Changed-file oxfmt and oxlint pass (10 files)
• pnpm run typecheck:node
• 445 focused tests across the wrapper, local fallback, daemon server/main/entry, and IPC suites
• Daemon protocol gate test
• Daemon concurrency test proves ping remains responsive while preparation is pending and concurrent creates yield one PTY subprocess
• Cancellation tests prove daemon kill / cancelCreateOrAttach / shutdown and local shutdown prevent pending subprocess creation, cancel all concurrent generations, and permit later session-ID reuse
• Reliability-gate manifest and max-lines ratchet checks
• Live healthy macOS preflight (roughly 55-70 ms)
• Live rejected-user probe exits without enabling the wrapper
• Real Electron terminal + authenticated Codex CLI spawn, shown above
• Full pnpm lint reaches an unrelated existing exhaustiveness error in src/renderer/src/components/skills/skill-freshness-group.tsx:101; changed-file lint passes
• Full pty-subprocess.test.ts has two existing deleted-cwd cases that exceed Vitest's 5 s timeout on this Mac; the changed protocol assertion passes and the macOS case already took about 9 s before this update
• Fresh PR CI: full repository test suite, unpacked Electron build, packaged CLI smoke, Windows terminal restart E2E, and Linux/Windows native smoke
• Full local pnpm build not run; node typecheck, local Electron dev build, CLI build, and macOS computer-use sidecar build pass

AI Review Report

A deeper review found and fixed three material regressions in the initial implementation:

1. spawnSync could stall Electron or the daemon event loop for 40-500 ms, freezing UI and live terminal traffic. The probe is now async, bounded, and deduplicated.
2. Keeping daemon protocol v22 allowed an already-running buggy v22 daemon to accept the updated client and continue creating broken terminals. Protocol v23 forces new sessions onto the fixed daemon while retaining legacy-session discovery.
3. Moving the probe to an async boundary created a close-during-preparation race. Pending creations are now generation-scoped and canceled before any subprocess can be registered.

The final review covered local and daemon spawn boundaries, concurrency, process cleanup, caching, deleted-cwd behavior, daemon migration, shell fallback, cross-platform gates, SSH execution, and failure diagnostics. No additional change met the bar for a meaningful senior-level fix.

Security audit

The probe uses fixed absolute system binaries, passes the resolved username as a literal argv element without shell interpolation, bounds time and captured output, closes stdin, and never logs captured output. No secrets, credentials, dependencies, IPC schemas, or persisted settings are added. A rejected probe only reduces behavior to the established direct-shell path.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a cached macOS login(1) PAM preflight using execFile before shell wrapping for TCC attribution. Successful results enable /usr/bin/login wrapping; failures fall back to direct spawning and warn once. Preparation is integrated into PTY spawning with cancellation handling for pending requests. The daemon protocol advances to version 23 while retaining version 22 compatibility. Tests cover preflight execution, caching, environment handling, fallback behavior, disabled execution, cancellation, and PTY integration.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	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 matches the main change: handling rejected macOS login sessions.
Description check	✅ Passed	The description covers summary, testing, review, and security details; only the explicit Screenshots/Notes headings are missing.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

src/main/providers/macos-tcc-login-shell.ts (1)

1-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use Node path utilities for the macOS executable paths.

Construct the /usr/bin/login and /usr/bin/printf paths with path.join (or an equivalent Node path utility) instead of hard-coding slash-separated paths.

As per coding guidelines, **/*.{js,jsx,ts,tsx} files must use path.join or Electron/Node path utilities for file paths and must not assume / or \.

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0b8dcf46-7b8f-4465-b662-814916dbc7ab

📥 Commits

Reviewing files that changed from the base of the PR and between 0f1b9ef and b261cde.

📒 Files selected for processing (2)

• src/main/providers/macos-tcc-login-shell.test.ts
• src/main/providers/macos-tcc-login-shell.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/main/providers/macos-tcc-login-shell.test.ts

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: aa9dfc20-28d4-485c-9f7a-d643cebd90aa

📥 Commits

Reviewing files that changed from the base of the PR and between 2af2eb3 and 2d505dd.

📒 Files selected for processing (10)

• src/main/daemon/daemon-entry.ts
• src/main/daemon/daemon-main.ts
• src/main/daemon/daemon-server.test.ts
• src/main/daemon/daemon-server.ts
• src/main/daemon/pty-subprocess.test.ts
• src/main/daemon/types.ts
• src/main/ipc/pty.test.ts
• src/main/providers/local-pty-provider.ts
• src/main/providers/macos-tcc-login-shell.test.ts
• src/main/providers/macos-tcc-login-shell.ts

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 65fcae62-6fd1-4735-be39-3dca475f1c62

📥 Commits

Reviewing files that changed from the base of the PR and between 2d505dd and 128241e.

📒 Files selected for processing (4)

• src/main/daemon/daemon-server.test.ts
• src/main/daemon/daemon-server.ts
• src/main/providers/local-pty-provider.test.ts
• src/main/providers/local-pty-provider.ts