Skip to content

fix(terminal): recover from transient macOS login preflight failures#9404

Open
brennanb2025 wants to merge 1 commit into
mainfrom
brennanb2025/macos-login-preflight-followup
Open

fix(terminal): recover from transient macOS login preflight failures#9404
brennanb2025 wants to merge 1 commit into
mainfrom
brennanb2025/macos-login-preflight-followup

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #9301. This keeps the macOS TCC attribution fix enabled after temporary PAM probe failures and closes the small spawn-lifecycle races identified in post-merge review.

  • Cache only deterministic outcomes: successful support or explicit PAM rejection.
  • Treat timeout, output-limit, unexpected-output, and process-launch failures as transient.
  • Fail open to a direct shell, then retry with exponential backoff from 5 seconds to a 5 minute cap.
  • Emit a structured macos-login-preflight daemon event containing only enabled, reason, retryable, and optional retryAfterMs.
  • Deduplicate concurrent local spawns with the same stable session id.
  • Cancel pending local spawns during renderer orphan cleanup and pending daemon spawns when their control client disconnects.

ELI5

The first version asks macOS whether Orca can launch shells in the privacy-friendly way. If macOS was merely slow once, Orca remembered that as a permanent no until restart. This follow-up remembers only real yes/no answers. Temporary failures use the normal shell so terminals still open, then Orca tries again later.

It also makes sure two requests cannot accidentally create two shells for the same terminal, and that a request abandoned by a reload or disconnect cannot finish in the background.

Safety and performance

  • Healthy and explicitly rejected PAM results remain process-cached, so normal terminal startup still probes once.
  • Concurrent callers share one probe.
  • Persistent transient failures can add at most the existing 500 ms probe timeout when a later terminal request crosses the backoff boundary; backoff grows to 5 minutes.
  • Probe diagnostics never include username, home directory, cwd, stdout, stderr, or other PAM/account data.
  • The PAM path remains Darwin-gated. Linux, Windows, WSL, and SSH shell wrapping behavior is unchanged.
  • Legacy v22 hibernated-session wake behavior remains intentionally unchanged because it is transitional and self-healing.

Validation

  • Real macOS /usr/bin/login preflight: success in 56 ms with the production 500 ms timeout and 1 KiB output cap.
  • Focused post-rebase suite: 137 tests passed.
  • Broader PTY/daemon/IPC suite: 453 tests passed.
  • pnpm run typecheck:node
  • pnpm exec oxlint on all changed files
  • pnpm exec oxfmt --check on all changed files
  • pnpm run check:max-lines-ratchet
  • pnpm run check:reliability-gates
  • git diff --check

Screenshots

No visual change.

AI Review Report

  • CodeRabbit initial review: no actionable comments.
  • Manual review additionally found and closed the stale-generation window while a replacement waits for the previous local PTY to exit.
  • Final-head CI is green; CodeRabbit posted no actionable comments.

Security Audit

  • The probe still uses fixed absolute binaries and literal argv elements with no shell interpolation.
  • PAM output, account identity, environment, and paths are not logged.
  • Failure remains fail-open to the pre-fix(terminal): bypass rejected macOS login sessions #9301 direct-shell behavior.
  • Disconnect and reload cancellation reduce untracked shell lifetime rather than expanding process authority.

Notes

  • No protocol change is required.
  • Legacy v22 hibernated-session wake behavior is out of scope because it is transitional and self-healing.
  • The automated docstring-coverage suggestion was not applied because these are short private lifecycle helpers and boilerplate comments would not improve maintainability.

@brennanb2025
brennanb2025 force-pushed the brennanb2025/macos-login-preflight-followup branch from e34f0ff to d010bf3 Compare July 19, 2026 02:00
@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The changes add typed macOS TCC preflight results with failure classification, retry backoff, concurrent request sharing, and daemon log reporting. Local PTY spawning now deduplicates stable-session requests and cancels stale-generation preparations. Daemon PTY preparation tracking is scoped to control clients, allowing disconnect and reconnect cleanup without canceling unrelated preparations. Tests cover retries, reporting, spawn deduplication, generation cancellation, and client disconnect handling.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% 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 PR’s main change: making macOS login preflight failures transient and recoverable.
Description check ✅ Passed The description covers all required sections and includes substantial testing, review, security, and notes content.
✨ 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

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

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