Skip to content

Allow Claude Auto CLI on cold boot when Keychain is disabled - #2494

Open
gmkbenjamin wants to merge 6 commits into
steipete:mainfrom
gmkbenjamin:fix/claude-startup-auto-cli
Open

Allow Claude Auto CLI on cold boot when Keychain is disabled#2494
gmkbenjamin wants to merge 6 commits into
steipete:mainfrom
gmkbenjamin:fix/claude-startup-auto-cli

Conversation

@gmkbenjamin

@gmkbenjamin gmkbenjamin commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • With Advanced → Disable Keychain access, Claude Auto can use CLI on cold boot (and later background ticks) without a prior Manual Refresh, so usage loads when OAuth/web paths are empty.
  • With Keychain enabled, keep Peter's opaque-child gate: background Auto (startup and timers) still requires an established CLI binary in-process plus prompt mode Always.
  • When restrictive background Auto still has no strategy, preserve last-good Claude usage and stay quiet instead of sticky-erroring.

Fixes #2493

Related: #2425, #2426, #2408, #2241

Why

#2426 fixed cold boot with Disable Keychain, then absorbed Peter's background CLI establishment gate while merging main, which reintroduced “Not fetched yet until Refresh” for that path. An earlier draft of this PR also treated startup like Manual Refresh when Keychain was enabled; review correctly flagged that as undoing the no-prompt safety boundary. This revision keeps the narrow Disable Keychain compatibility path only.

Real behavior proof

Fresh packaged CodexBar.app at head 949812e1.

Settings: debugDisableKeychainAccess=true, claudeOAuthKeychainPromptMode=never.

Cold start (pkill + open -n), no Manual Refresh:

  • App stayed running
  • Widget snapshot published Claude usage at 2026-07-31T08:19:54Z — Session 100% left, Weekly 75% left
  • CLI usage --provider claude matched (Session 100% / Weekly 75%)
  • No Keychain / authentication UI observed

Artifact: docs/screenshots/claude-cold-boot-disable-keychain-2026-07-31.txt

Test plan

  • Quit and relaunch with Claude Auto + Disable Keychain — usage loads without Refresh
  • Keychain-enabled + Only on user action startup stays gated (unit coverage)
  • Later background ticks still honor established-binary + Always when Keychain enabled
  • Manual Refresh still works
  • make check
  • swift test --filter 'ClaudeCLIBackgroundAvailabilityTests|ClaudeBackgroundAutoRefreshFailureTests'

Commands run

  • make check (pass)
  • focused Claude availability / quiet-failure tests (pass)
  • packaged cold-start proof above

Made with Cursor

user and others added 4 commits July 29, 2026 19:39
Restore CLI availability for Disable Keychain and the initial startup
refresh so Claude usage loads without a manual Refresh, keep last-good
snapshots quiet on gated background Auto ticks, and document the gate.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jul 29, 2026
@clawsweeper

clawsweeper Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed July 31, 2026, 4:32 AM ET / 08:32 UTC.

ClawSweeper review

What this changes

This PR allows Claude Auto to use the installed Claude CLI on startup and later background refreshes when Disable Keychain access is enabled, preserves the stricter Keychain-enabled CLI gate, and keeps last-good Claude usage during intentionally deferred background refreshes.

Merge readiness

⚠️ Needs maintainer review before merge - 7 items remain

Keep this PR open for maintainer review. The focused Disable Keychain cold-boot path and real packaged-app transcript are credible, but merging intentionally permits an opaque Claude CLI child during automatic refresh without the regular Keychain-enabled establishment gate; a maintainer should explicitly confirm that this opt-out is the intended security boundary. A separate failure-handling condition also suppresses genuine no-strategy errors in Disable Keychain mode, which can recreate a silent “Not fetched yet” state when no CLI route exists.

Priority: P1
Reviewed head: 949812e1cb2d4291e033775723cb82961d145d55
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The intended cold-start behavior has credible real-runtime evidence and focused coverage, but a P1 silent-failure regression and a maintainer-owned CLI authorization boundary prevent a merge-ready rating.
Proof confidence 🐚 platinum hermit (4/6) ✨ media proof bonus Sufficient (linked_artifact): The PR includes a redacted packaged-app cold-start transcript that directly shows the intended Disable Keychain behavior: a published widget snapshot and matching Claude CLI usage without observed Keychain or authentication UI; its recorded code SHA predates the proof-only head commit but corresponds to the behavior-changing commit.
Patch quality 🦐 gold shrimp (3/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Verified Sufficient (linked_artifact): The PR includes a redacted packaged-app cold-start transcript that directly shows the intended Disable Keychain behavior: a published widget snapshot and matching Claude CLI usage without observed Keychain or authentication UI; its recorded code SHA predates the proof-only head commit but corresponds to the behavior-changing commit.
Evidence reviewed 5 items Targeted CLI availability change: The branch makes a resolved Claude CLI available for every background Auto refresh when Disable Keychain access is explicitly enabled; with Keychain enabled it retains the established-binary plus Always-prompt-mode gate.
Silent-failure condition: The new quiet-failure helper treats both Disable Keychain access and restrictive Keychain prompt settings as a reason to clear a Claude no-available-strategy error, even though the Disable Keychain path is no longer an intentionally gated CLI route.
Focused regression coverage: The PR adds tests for Disable Keychain background availability and keeps Keychain-enabled startup background Auto unavailable under Only on user action.
Findings 1 actionable finding [P1] Do not silence unavailable routes with Keychain disabled
Security Needs attention Confirm the opaque CLI opt-out boundary: The Disable Keychain branch returns CLI availability without the established-binary/authentication gate. The linked proof shows an authenticated environment with no prompt, but it does not establish that an unauthenticated external CLI cannot open an authentication flow during automatic refresh.

How this fits together

CodexBar refreshes Claude usage through OAuth, browser, or local CLI strategies and then publishes the selected result to the menu and widget. The changed logic determines whether background Auto refresh may launch the Claude CLI and how a missing strategy changes the visible cached state.

flowchart LR
    Settings[Claude and Keychain settings] --> Refresh[Background Auto refresh]
    Refresh --> Strategies[OAuth web and CLI strategies]
    Strategies --> Gate[Background CLI safety gate]
    Gate --> CLI[Claude CLI usage fetch]
    Strategies --> Snapshot[Usage snapshot]
    CLI --> Snapshot
    Snapshot --> Display[Menu and widget]
Loading

Decision needed

Question Recommendation
Does enabling Advanced → Disable Keychain access intentionally authorize CodexBar to launch the opaque Claude CLI during automatic startup and timer refreshes without the normal established-binary/authentication safety gate? Approve the explicit opt-out boundary: Treat Disable Keychain access as the documented opt-in for background CLI execution, then fix the separate silent no-strategy condition before merge.

Why: This is a product and security-boundary choice: the implementation cannot determine whether a user who opted out of CodexBar Keychain access also opted into any authentication behavior the external Claude CLI may initiate.

Before merge

  • Do not silence unavailable routes with Keychain disabled (P1) - debugDisableKeychainAccess makes this helper clear a real .noAvailableStrategy(.claude) error even though the new CLI rule no longer intentionally defers the Disable Keychain path. If the CLI is missing or no other strategy exists on a fresh start, the app retains no snapshot and no error, recreating the “Not fetched yet” state this PR is meant to repair. Restrict quiet preservation to the Keychain-enabled restrictive gate, where a safe route is deliberately deferred.
  • Resolve security concern: Confirm the opaque CLI opt-out boundary - The Disable Keychain branch returns CLI availability without the established-binary/authentication gate. The linked proof shows an authenticated environment with no prompt, but it does not establish that an unauthenticated external CLI cannot open an authentication flow during automatic refresh.
  • Resolve merge risk (P1) - Disable Keychain access currently becomes authorization to launch an opaque Claude CLI child during automatic startup and timer refreshes without confirming its login state; an unauthenticated CLI could still surface its own authentication flow outside CodexBar’s Keychain controls.
  • Resolve merge risk (P1) - When no CLI, OAuth, or web strategy is available with Disable Keychain access enabled, the new quiet-failure condition clears the error even without a prior snapshot, leaving the user at “Not fetched yet” rather than explaining that no route is available.
  • Resolve merge risk (P1) - The supplied real-behavior transcript demonstrates the intended authenticated cold-start case, but it does not demonstrate the unauthenticated/no-CLI failure mode or an upgrade from existing settings.
  • Complete next step (P2) - Maintainers need to confirm the intended Disable Keychain security boundary before merge; after that, the P1 failure-state fix is a narrow mechanical follow-up.

Findings

  • [P1] Do not silence unavailable routes with Keychain disabled — Sources/CodexBar/UsageStore+Refresh.swift:1062-1063
  • [medium] Confirm the opaque CLI opt-out boundary — Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift:695
Agent review details

Security

Needs attention: The diff intentionally broadens when CodexBar may start an opaque external Claude CLI process in the background, so a maintainer must confirm that Disable Keychain access is the intended authorization boundary.

Review metrics

Metric Value Why it matters
Patch scope 8 files affected; 371 additions, 25 deletions The PR changes production strategy selection and failure state, adds focused tests, documents the Keychain behavior, and includes a runtime-proof artifact.
Focused test coverage 2 test files changed, 1 test file added The patch covers both the cold-boot availability rule and the preserved-snapshot failure path.

Merge-risk options

Maintainer options:

  1. Narrow quiet failure handling (recommended)
    Change the new no-strategy suppression so it applies only to Keychain-enabled restrictive prompt modes, then add coverage showing Disable Keychain with no available route surfaces a diagnostic error.
  2. Accept silent unavailable state
    Maintain the current quiet behavior for Disable Keychain users, explicitly accepting that fresh installs without a usable CLI can remain at “Not fetched yet.”
  3. Pause for boundary decision
    Pause this PR if maintainers do not want Disable Keychain access to authorize opaque background CLI authentication behavior.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Restrict Claude no-available-strategy quiet preservation to the Keychain-enabled restrictive background gate; add focused coverage for Disable Keychain with no resolved route and retain current cold-boot behavior.

Technical review

Best possible solution:

Keep the explicit Disable Keychain cold-boot compatibility path only if maintainers affirm that it intentionally authorizes opaque background Claude CLI execution; otherwise retain a noninteractive safety check, and in either case limit quiet last-good preservation to genuinely deferred Keychain-enabled routes so unavailable Disable Keychain configurations remain diagnosable.

Do we have a high-confidence way to reproduce the issue?

Yes for the reported cold-start path: the supplied packaged-app transcript shows a fresh Disable Keychain launch publishing Claude usage without a manual refresh, and the source change clearly explains the prior background CLI rejection. The no-route silent-state concern is source-reproducible from the new failure predicate, though it was not run locally in this read-only review.

Is this the best way to solve the issue?

No: restoring the Disable Keychain cold-boot route is narrowly targeted, but the quiet-failure predicate is broader than the intentional Keychain-enabled deferral case and can suppress a real unavailable-route diagnostic. The external CLI authorization boundary also needs explicit maintainer approval.

Full review comments:

  • [P1] Do not silence unavailable routes with Keychain disabled — Sources/CodexBar/UsageStore+Refresh.swift:1062-1063
    debugDisableKeychainAccess makes this helper clear a real .noAvailableStrategy(.claude) error even though the new CLI rule no longer intentionally defers the Disable Keychain path. If the CLI is missing or no other strategy exists on a fresh start, the app retains no snapshot and no error, recreating the “Not fetched yet” state this PR is meant to repair. Restrict quiet preservation to the Keychain-enabled restrictive gate, where a safe route is deliberately deferred.
    Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8ef86077e70a.

Labels

Label changes:

  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (linked_artifact): The PR includes a redacted packaged-app cold-start transcript that directly shows the intended Disable Keychain behavior: a published widget snapshot and matching Claude CLI usage without observed Keychain or authentication UI; its recorded code SHA predates the proof-only head commit but corresponds to the behavior-changing commit.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: ⏳ waiting on author.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P1: A Claude Auto startup regression can leave real users without usage data until a manual refresh, and the proposed failure handling can still hide an unavailable route.
  • merge-risk: 🚨 compatibility: Existing users with Disable Keychain access receive a different automatic-refresh behavior and a different error-versus-empty-state outcome.
  • merge-risk: 🚨 auth-provider: The patch changes when Claude CLI, OAuth, and web routes are considered available during background refresh.
  • merge-risk: 🚨 security-boundary: The explicit opt-out bypasses the normal background authentication-establishment guard for an opaque external CLI process.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (linked_artifact): The PR includes a redacted packaged-app cold-start transcript that directly shows the intended Disable Keychain behavior: a published widget snapshot and matching Claude CLI usage without observed Keychain or authentication UI; its recorded code SHA predates the proof-only head commit but corresponds to the behavior-changing commit.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR includes a redacted packaged-app cold-start transcript that directly shows the intended Disable Keychain behavior: a published widget snapshot and matching Claude CLI usage without observed Keychain or authentication UI; its recorded code SHA predates the proof-only head commit but corresponds to the behavior-changing commit.

Evidence

Security concerns:

  • [medium] Confirm the opaque CLI opt-out boundary — Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift:695
    The Disable Keychain branch returns CLI availability without the established-binary/authentication gate. The linked proof shows an authenticated environment with no prompt, but it does not establish that an unauthenticated external CLI cannot open an authentication flow during automatic refresh.
    Confidence: 0.8

What I checked:

Likely related people:

  • gmkbenjamin: Authored merged pull request fix: Disable Keychain — Cursor/Claude refresh, Claude boot, overview Weekly #2426, which previously implemented the Disable Keychain Claude cold-boot path that this PR narrows and restores. (role: introduced adjacent implementation; confidence: medium; commits: 5ce21f6f4058; files: Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift, docs/KEYCHAIN_FIX.md)
  • steipete: The PR discussion identifies the retained opaque-child background CLI gate as Peter’s existing safety boundary; repository ownership also makes this the strongest available routing candidate for the opt-out decision. (role: likely security-boundary owner; confidence: medium; files: Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Restrict quiet no-strategy handling to the Keychain-enabled restrictive background gate and add the missing unavailable-route test.
  • Obtain explicit maintainer confirmation that Disable Keychain access permits opaque background Claude CLI execution.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)
  • reviewed 2026-07-29T13:05:17.409Z sha 8d0b63d :: needs real behavior proof before merge. :: [P1] Preserve the no-prompt gate for startup CLI
  • reviewed 2026-07-31T08:17:29.267Z sha 4e5fc9f :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-31T08:24:45.275Z sha 949812e :: needs maintainer review before merge. :: none

Keep Keychain-enabled Only on user action behind the established-binary gate; preserve last-good quiet failure handling.

Co-authored-by: Cursor <cursoragent@cursor.com>
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jul 31, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
@gmkbenjamin gmkbenjamin changed the title Allow Claude Auto CLI on cold-boot startup Allow Claude Auto CLI on cold boot when Keychain is disabled Jul 31, 2026
@gmkbenjamin

Copy link
Copy Markdown
Contributor Author

Addressed ClawSweeper blockers on 949812e1:

  1. P1 no-prompt gate: removed the Keychain-enabled startup CLI bypass. Startup/timer background Auto with Keychain enabled still requires established binary + prompt mode Always. Disable Keychain remains the prompt-free cold-boot path.
  2. Rebased/merged current main (mergeable).
  3. Cold-start proof from packaged app with Disable Keychain — widget snapshot + CLI usage, no Keychain UI. See PR body + docs/screenshots/claude-cold-boot-disable-keychain-2026-07-31.txt.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 31, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Claude Auto shows Not fetched yet on startup until manual Refresh

1 participant