Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe Claude Code login command now uses asynchronous macOS Terminal launching with timeout and failure handling. New Tauri permission wiring, automated tests, and a macOS release smoke check cover the flow. ChangesClaude Code login flow
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant MainWindowSettings
participant claude_code_login_launch
participant osascript
MainWindowSettings->>claude_code_login_launch: invoke login command
claude_code_login_launch->>osascript: launch Terminal.app asynchronously
osascript-->>claude_code_login_launch: return exit status or timeout
claude_code_login_launch-->>MainWindowSettings: return success or launch error
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The sign-in launch flow is scoped to the main window and handles successful, failed, missing, and timed-out macOS launcher outcomes without an identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
I hop through commands with a bright little cheer Comment |
How this change flows1 changed behaviour across 4 relationships. 3 surrounding behaviours are shown (60 graph nodes walked). 22 further behaviours left out to keep the diagram readable. flowchart LR
n0["claude_code_login_launch<br/>changed"]:::changed
n1["format"]:::impacted
n2["map_err"]:::impacted
n3["apply_app_update"]:::impacted
n0 -->|calls| n1
n0 -->|calls| n2
n3 -->|calls| n1
n3 -->|calls| n2
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
|
pls merge this, wanna use Claude |
# Conflicts: # app/src-tauri/src/claude_code.rs
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
@tinyhumansai/maintainers @M3gA-Mind can you guys check on this? |
senamakel
left a comment
There was a problem hiding this comment.
@tinyhumansai/maintainers @M3gA-Mind can you guys check on this?
test this live and lmk if this looks good.
Summary
osascriptspawn as a successful launch.Problem
On macOS, clicking Claude Code Sign in can show “Could not open the login terminal” before Rust runs:
claude_code_login_launchis registered but absent from the app permissions. After permission is granted, the existing launcher reports success even when AppleScript exits unsuccessfully, for example after an Automation denial.This complements #5790. That PR fixes the authentication command and translated hints; it does not add the Tauri permission or wait for the launcher result.
Solution
A dedicated desktop capability allows only
claude_code_login_launch, only in the main window. The macOS branch awaits/usr/bin/osascriptusing Tokio, checks the exit status, and applies a 30-second timeout withkill_on_drop. Tests exercise the ACL wiring, successful and unsuccessful exit statuses, missing executables, and timeout. The release smoke checklist covers the real macOS Automation prompt.Submission Checklist
claude auth login, not the obsoleteclaude login#5790 and references Claude Code login launcher uses the wrong authentication command #5710 without claiming to close its command bug.Impact
Desktop permission applies to Linux, macOS, and Windows. The launcher wait changes macOS only. No migration or configuration change. Other windows receive no new permission. The existing Windows/Linux launch strategy and authentication command remain unchanged; #5790 owns the command correction.
Related
claude auth login, not the obsoleteclaude login#5790 (authentication command); Resolve the claude CLI from well-known install paths, not only PATH #5736 / fix(claude-code): resolve the CLI off PATH and surface setup failures #5996 / fix(claude-code): harden CLI resolution and setup-error classification #6002 (CLI discovery, intentionally excluded).AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
cargo test --manifest-path app/src-tauri/Cargo.toml --lib --no-default-features --features gateways claude_code::tests— 4 passed, 0 failed.cargo fmt --manifest-path app/src-tauri/Cargo.toml --checkandcargo clippy --manifest-path app/src-tauri/Cargo.toml --lib --no-default-features --features gateways -- -D warningspassed. The core dependency emitted a pre-existing unused-import warning; the shell check completed successfully.Validation Blocked
command:cargo llvm-coverror:cargo-llvm-cov is not installed locally.impact:numerical diff coverage must be confirmed in CI. Real Automation UI remains a documented release smoke check, not a claimed automated test.Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
claude auth login, not the obsoleteclaude login#5790 is related, not duplicated.claude auth login, not the obsoleteclaude login#5790 for the authentication command.claude auth login, not the obsoleteclaude login#5790 is not writable by this contributor, so this is a complementary PR against main.Summary by CodeRabbit
New Features
Bug Fixes
Documentation