Repository navigation
fix(terminal): bypass rejected macOS login sessions - #9301
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a cached macOS 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/providers/macos-tcc-login-shell.ts (1)
1-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Node path utilities for the macOS executable paths.
Construct the
/usr/bin/loginand/usr/bin/printfpaths withpath.join(or an equivalent Node path utility) instead of hard-coding slash-separated paths.As per coding guidelines,
**/*.{js,jsx,ts,tsx}files must usepath.joinor 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
📒 Files selected for processing (2)
src/main/providers/macos-tcc-login-shell.test.tssrc/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
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: aa9dfc20-28d4-485c-9f7a-d643cebd90aa
📒 Files selected for processing (10)
src/main/daemon/daemon-entry.tssrc/main/daemon/daemon-main.tssrc/main/daemon/daemon-server.test.tssrc/main/daemon/daemon-server.tssrc/main/daemon/pty-subprocess.test.tssrc/main/daemon/types.tssrc/main/ipc/pty.test.tssrc/main/providers/local-pty-provider.tssrc/main/providers/macos-tcc-login-shell.test.tssrc/main/providers/macos-tcc-login-shell.ts
AmethystLiang
self-requested a review
July 18, 2026 16:47
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 65fcae62-6fd1-4735-be39-3dca475f1c62
📒 Files selected for processing (4)
src/main/daemon/daemon-server.test.tssrc/main/daemon/daemon-server.tssrc/main/providers/local-pty-provider.test.tssrc/main/providers/local-pty-provider.ts
brennanb2025
added a commit
that referenced
this pull request
Jul 20, 2026
…ups) (#9606) * fix(terminal): harden macOS login-preflight spawn path (#9301 follow-ups) Follow-ups to the #9301 two-pass review. Re-derived severities: none are release blockers; these are low-likelihood robustness/observability fixes. F1 macos-tcc-login-shell: only cache a conclusive PAM verdict. A probe our own SIGKILL timeout / ETIMEDOUT / maxBuffer killed proves nothing about PAM, so it no longer sticks the whole process into direct-spawn — it fails open for that spawn, re-probes next time, and self-heals once PAM answers. F2 daemon-entry: prepareMacosTccLoginShell returns the outcome; the daemon emits a structured macos-login-preflight {ok, reason} log on degrade, since detached daemons destroy stderr and lose the console.warn. F3 local-pty-provider: an atomic guard before the tracking write coalesces a concurrent same-session-id spawn onto the winner instead of orphaning an untracked, unkillable PTY. F4 daemon-server: preparations carry their clientId; a control-socket close cancels only that client's pending preps, so a mid-preflight disconnect no longer leaves a durable orphan daemon PTY. F7 warm the PAM probe at daemon startup (mirrors warmPwshAvailabilityCache); note the pipe-vs-PTY probe fidelity limit; test that kill with no pending prep still surfaces SessionNotFoundError. Deliberately skipped F5 (self-limits to next reload; a real fix needs speculative in-flight-generation tracking) and F6 (inherent to legacy v22 attach; self-heals once daemons are v23). * fix(terminal): bound preflight recovery work * fix(terminal): cancel preflight when daemon stream closes
This was referenced Jul 22, 2026
brennanb2025
added a commit
that referenced
this pull request
Jul 22, 2026
…9826) * fix(daemon): retire macOS daemons whose login session died (#7936) A daemon that survives a full macOS logout is unsalvageable: its PAM context can no longer host login(1) spawns (every new PTY becomes a 'Login incorrect' prompt zombie) and its Mach bootstrap namespace has lost the system DNS resolver, so terminals it hosts have no egress. Today it also keeps the #9301 preflight's cached 'accepted' verdict, so it keeps wrapping spawns in login(1) forever; only a manual daemon restart recovers. GUI-spawned daemons now watch for login-session death from the inside: a fresh cache-bypassing PAM probe (triggered by PTY-exit bursts, fresh client hellos, and a slow periodic timer) must conclusively reject three consecutive times AND the in-process system resolver must be degraded; then the daemon exits crash-style so session meta stays unclean and the replacement daemon cold-restores scrollback. A conclusive rejection also flips the spawn-wrapper cache off immediately. Headless serve/SSH daemons never get the watch (they must survive their spawning session ending), and a session that never conclusively accepted login(1) never arms it — a PAM anomaly alone can't kill a healthy daemon (fast user switching keeps accepting, so switched-away sessions are preserved). * test(daemon): e2e seam to drive login-session death oracles from a verdict file A dead macOS login session cannot be fabricated without root (PAM owns audit-session teardown), so live lifecycle QA drives the death watch's probe and resolver oracles from ORCA_E2E_LOGIN_SESSION_PROBE_FILE: 'alive' → accepted/healthy, 'dead' → rejected/unhealthy, anything else inconclusive — with compressed watch timing. Mirrors the existing ORCA_E2E_DAEMON_INIT_DELAY_MS seam; inert unless the env var is set. * fix(daemon): close the hang-shaped gap in login-session death detection The conclusive-PAM-verdict trigger had one blind failure shape: login(1) hanging at the prompt past the probe bound (killed → inconclusive forever → the watch never fires). Three changes close it: - The death-watch probe gets its own 4s bound (the 500ms preflight bound exists for spawn-path latency, which doesn't apply off-path), so a slow-but-answering PAM stack isn't misread as a hang. - An inconclusive pipe probe escalates to a PTY-hosted probe via script(1) — a dead session's PAM stack may only misbehave under a real tty (the pipe-vs-PTY fidelity limit the preflight documents). - A streak of timeout-killed probes (which a live session never produces) is a second retirement trigger, at a higher threshold (5) and still gated on the degraded resolver, logged with a distinct cause so field logs discriminate the two paths. Every dead-session behavior — fast reject, prompt-then-EOF, or hang — now fires retirement; all inconclusive states still fail toward preserving the daemon. * fix(daemon): keep login-session retirement conclusive * fix(daemon): stop login watch before clean shutdown * fix(daemon): make login-session PTY probe reliable * fix(daemon): close login-session watch races * fix(daemon): ignore health probes for login watch activity
6 tasks
enrick-dev
pushed a commit
to enrick-dev/orca
that referenced
this pull request
Oct 15, 2026
* fix(terminal): bypass rejected macOS login sessions * fix(terminal): bound macOS login preflight * test(terminal): isolate macOS login preflight * fix(terminal): avoid blocking macOS login preflight * fix(terminal): cancel pending PTY preparation * fix(terminal): cancel preparation on daemon shutdown
enrick-dev
pushed a commit
to enrick-dev/orca
that referenced
this pull request
Oct 15, 2026
…follow-ups) (stablyai#9606) * fix(terminal): harden macOS login-preflight spawn path (stablyai#9301 follow-ups) Follow-ups to the stablyai#9301 two-pass review. Re-derived severities: none are release blockers; these are low-likelihood robustness/observability fixes. F1 macos-tcc-login-shell: only cache a conclusive PAM verdict. A probe our own SIGKILL timeout / ETIMEDOUT / maxBuffer killed proves nothing about PAM, so it no longer sticks the whole process into direct-spawn — it fails open for that spawn, re-probes next time, and self-heals once PAM answers. F2 daemon-entry: prepareMacosTccLoginShell returns the outcome; the daemon emits a structured macos-login-preflight {ok, reason} log on degrade, since detached daemons destroy stderr and lose the console.warn. F3 local-pty-provider: an atomic guard before the tracking write coalesces a concurrent same-session-id spawn onto the winner instead of orphaning an untracked, unkillable PTY. F4 daemon-server: preparations carry their clientId; a control-socket close cancels only that client's pending preps, so a mid-preflight disconnect no longer leaves a durable orphan daemon PTY. F7 warm the PAM probe at daemon startup (mirrors warmPwshAvailabilityCache); note the pipe-vs-PTY probe fidelity limit; test that kill with no pending prep still surfaces SessionNotFoundError. Deliberately skipped F5 (self-limits to next reload; a real fix needs speculative in-flight-generation tracking) and F6 (inherent to legacy v22 attach; self-heals once daemons are v23). * fix(terminal): bound preflight recovery work * fix(terminal): cancel preflight when daemon stream closes
enrick-dev
pushed a commit
to enrick-dev/orca
that referenced
this pull request
Oct 15, 2026
…7936) (stablyai#9826) * fix(daemon): retire macOS daemons whose login session died (stablyai#7936) A daemon that survives a full macOS logout is unsalvageable: its PAM context can no longer host login(1) spawns (every new PTY becomes a 'Login incorrect' prompt zombie) and its Mach bootstrap namespace has lost the system DNS resolver, so terminals it hosts have no egress. Today it also keeps the stablyai#9301 preflight's cached 'accepted' verdict, so it keeps wrapping spawns in login(1) forever; only a manual daemon restart recovers. GUI-spawned daemons now watch for login-session death from the inside: a fresh cache-bypassing PAM probe (triggered by PTY-exit bursts, fresh client hellos, and a slow periodic timer) must conclusively reject three consecutive times AND the in-process system resolver must be degraded; then the daemon exits crash-style so session meta stays unclean and the replacement daemon cold-restores scrollback. A conclusive rejection also flips the spawn-wrapper cache off immediately. Headless serve/SSH daemons never get the watch (they must survive their spawning session ending), and a session that never conclusively accepted login(1) never arms it — a PAM anomaly alone can't kill a healthy daemon (fast user switching keeps accepting, so switched-away sessions are preserved). * test(daemon): e2e seam to drive login-session death oracles from a verdict file A dead macOS login session cannot be fabricated without root (PAM owns audit-session teardown), so live lifecycle QA drives the death watch's probe and resolver oracles from ORCA_E2E_LOGIN_SESSION_PROBE_FILE: 'alive' → accepted/healthy, 'dead' → rejected/unhealthy, anything else inconclusive — with compressed watch timing. Mirrors the existing ORCA_E2E_DAEMON_INIT_DELAY_MS seam; inert unless the env var is set. * fix(daemon): close the hang-shaped gap in login-session death detection The conclusive-PAM-verdict trigger had one blind failure shape: login(1) hanging at the prompt past the probe bound (killed → inconclusive forever → the watch never fires). Three changes close it: - The death-watch probe gets its own 4s bound (the 500ms preflight bound exists for spawn-path latency, which doesn't apply off-path), so a slow-but-answering PAM stack isn't misread as a hang. - An inconclusive pipe probe escalates to a PTY-hosted probe via script(1) — a dead session's PAM stack may only misbehave under a real tty (the pipe-vs-PTY fidelity limit the preflight documents). - A streak of timeout-killed probes (which a live session never produces) is a second retirement trigger, at a higher threshold (5) and still gated on the degraded resolver, logged with a distinct cause so field logs discriminate the two paths. Every dead-session behavior — fast reject, prompt-then-EOF, or hang — now fires retirement; all inconclusive states still fail toward preserving the daemon. * fix(daemon): keep login-session retirement conclusive * fix(daemon): stop login watch before clean shutdown * fix(daemon): make login-session PTY probe reliable * fix(daemon): close login-session watch races * fix(daemon): ignore health probes for login watch activity
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
login(1)account/session path before using it to launch a terminal shell.login(1)can return zero after an EOF-driven failed authentication prompt.SIGKILLtimeout; cache and deduplicate the result per PTY-host process.Login incorrect/login:.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
login:prompt. Existing terminals and unrelated daemon RPCs stay responsive during detection.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.ORCA_DISABLE_MACOS_LOGIN_SHELL=1remains 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
oxfmtandoxlintpass (10 files)pnpm run typecheck:nodepingremains responsive while preparation is pending and concurrent creates yield one PTY subprocesskill/cancelCreateOrAttach/ shutdown and local shutdown prevent pending subprocess creation, cancel all concurrent generations, and permit later session-ID reusepnpm lintreaches an unrelated existing exhaustiveness error insrc/renderer/src/components/skills/skill-freshness-group.tsx:101; changed-file lint passespty-subprocess.test.tshas 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 updatepnpm buildnot run; node typecheck, local Electron dev build, CLI build, and macOS computer-use sidecar build passAI Review Report
A deeper review found and fixed three material regressions in the initial implementation:
spawnSynccould 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.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.