Skip to content

Pause blocked background Claude CLI auth probes#2163

Open
axhoff wants to merge 1 commit into
steipete:mainfrom
axhoff:fix/claude-cli-auth-preflight-cooldown
Open

Pause blocked background Claude CLI auth probes#2163
axhoff wants to merge 1 commit into
steipete:mainfrom
axhoff:fix/claude-cli-auth-preflight-cooldown

Conversation

@axhoff

@axhoff axhoff commented Jul 14, 2026

Copy link
Copy Markdown

Summary

  • classify and coalesce claude auth status --json probes instead of collapsing every result to false;
  • persist a 15-minute cooldown after ambiguous background Auto timeouts or generic probe failures, preventing CodexBar's refresh loop from repeatedly launching a Claude CLI process blocked on a macOS Keychain dialog;
  • preserve recovery paths: clean loggedIn: false results are not cooled down, user-initiated/explicit CLI fetches and CLI runtime bypass the app-background gate, and successful auth/fetch paths clear the cooldown;
  • cover persisted cooldown reload and expiry cleanup in addition to timeout, failure, bypass, repair, coalescing, and cancellation behavior.

Root cause and scope

In #2115, packaged-app unified logs identify /usr/bin/security as the process requesting Claude Code-credentials. That is the delegated Claude CLI path: CodexBar launches claude auth status --json, Claude invokes security, the modal prompt prevents the preflight from completing, CodexBar times it out after five seconds, and the next background refresh starts the cycle again.

This complements #2137, which guards CodexBar's direct in-process Security.framework read for MCP-only credentials. It does not cover the child-process /usr/bin/security requester seen here. #2102 repairs a different missing-credentials-file state, while #1742 introduced the auth-status preflight involved in this loop.

The patch does not read, modify, or delete Keychain credentials and does not alter Claude authentication. The only persisted value is the cooldown expiry timestamp in UserDefaults.

Tests

  • swift test --filter ClaudeCLIAuth
  • swift test --filter ClaudeBaselineCharacterizationTests
  • swift test --filter ClaudeCLIAuthPreflightGateTests — 4/4 passed, including persistence reload/expiry
  • make check — passed (SwiftFormat and SwiftLint clean)
  • make test was also run locally. It progressed through the first nine groups, then stopped in the unrelated ClaudeOAuthCredentialsStoreCLIStorageOwnershipTests, which detected the host's real Claude credential/keychain state; the PR-focused and characterization suites above are green, and CI remains the clean-environment signal.

Addresses #2115.

Diagnostic evidence: #2115 (comment)

Related: #2137, #2102, #1742

@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. P2 Normal priority bug or improvement with limited blast radius. 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. labels Jul 14, 2026
@clawsweeper

clawsweeper Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 15, 2026, 11:49 PM ET / July 16, 2026, 03:49 UTC.

Summary
The branch classifies and coalesces Claude CLI auth-status probes, persists a 15-minute background cooldown after ambiguous failures, and adds coverage for bypass, recovery, cancellation, persistence, and expiry behavior.

Reproducibility: no. current broken-state reproduction is available, but pre-fix packaged-app logs attribute the prompt to the delegated Claude CLI security subprocess and current source clearly repeats that preflight after timeout. The original machine was repaired before the patched bundle could be exercised against the same state.

Review metrics: 3 noteworthy metrics.

  • Changed surface: 6 files, +544/-67. The patch is substantial for a narrowly scoped provider-auth mitigation and includes new production state plus broad characterization updates.
  • Persisted cooldowns: 2 outcomes, 15 minutes each. Both ambiguous timeout and generic failure now affect subsequent background provider availability across launches.
  • New production components: 1 gate added, 1 probe coordinator added. Maintainers should evaluate both the persisted routing policy and the concurrency/coalescing behavior before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #2115
Summary: The recurring Keychain-prompt issue is canonical; this PR mitigates the delegated auth-status subprocess path while other PRs address an in-process discovery path and a broader policy-first subprocess gate.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Provide redacted packaged-app logs or a recording showing several background refresh cycles without repeated Keychain prompts and demonstrating user-initiated recovery or cooldown clearing.
  • Document the intended relationship with Fix Claude background Keychain prompts #2191 after a maintainer chooses the permanent prompt-policy contract.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Focused tests cover the new state machine, but the contributor explicitly has no after-fix packaged-app observation in the formerly broken credential state; add redacted logs, terminal output, or a recording, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P2] A timeout or generic subprocess failure writes one global 15-minute cooldown, so a transient failure can temporarily suppress otherwise healthy background Claude CLI usage until expiry or an explicit successful fetch clears it.
  • [P1] There is no after-fix packaged-app evidence showing that repeated background refreshes stop reopening the macOS Keychain prompt and that explicit recovery still clears or bypasses the cooldown.
  • [P1] This PR and Fix Claude background Keychain prompts #2191 implement overlapping background-auth controls at different policy layers; landing both without choosing their intended relationship could leave redundant or confusing routing behavior.

Maintainer options:

  1. Prove and unify the contract (recommended)
    Require redacted packaged-app proof and reconcile this cooldown with the onlyOnUserAction policy and the overlapping comprehensive candidate before merge.
  2. Accept bounded suppression
    Merge the 15-minute cooldown as a mitigation while explicitly accepting delayed recovery after transient failures and possible recurring prompts after expiry.
  3. Pause for the broader candidate
    Hold or close this PR if maintainers choose the policy-first implementation in Fix Claude background Keychain prompts #2191 instead.

Next step before merge

  • [P1] A maintainer should choose how this mitigation composes with the policy-first candidate after the contributor supplies after-fix packaged-app proof.

Maintainer decision needed

  • Question: Should Claude background Auto refresh use this failure-triggered cooldown, enforce the existing onlyOnUserAction prompt policy before launching opaque Claude CLI subprocesses, or combine both layers?
  • Rationale: Both approaches affect provider availability and user prompt-policy semantics, while the two open PRs overlap without a confirmed permanent contract.
  • Likely owner: steipete — Recent Claude auth-policy and provider-routing history makes this the strongest available owner for choosing the permanent contract.
  • Options:
    • Combine policy and backoff (recommended): Honor onlyOnUserAction before opaque background subprocesses and retain coalescing plus bounded cooldowns for configurations that explicitly permit background probing.
    • Land cooldown mitigation: Merge this PR after packaged-app proof, accepting that the first prompt and a later prompt after cooldown expiry can still occur.
    • Prefer policy gate: Pause this PR and evaluate Fix Claude background Keychain prompts #2191 as the single comprehensive fix for both identified subprocess paths.

Security
Cleared: The patch does not expand Keychain access, credential handling, process permissions, dependency trust, or release automation; it persists only a cooldown timestamp.

Review details

Best possible solution:

Adopt one coherent Claude background-auth policy that honors onlyOnUserAction before opaque CLI subprocesses start, retains coalescing and a short bounded cooldown only as defense-in-depth where background probes remain allowed, and validates prompt suppression plus explicit recovery in a packaged app.

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

No current broken-state reproduction is available, but pre-fix packaged-app logs attribute the prompt to the delegated Claude CLI security subprocess and current source clearly repeats that preflight after timeout. The original machine was repaired before the patched bundle could be exercised against the same state.

Is this the best way to solve the issue?

Unclear: the cooldown is a bounded mitigation and preserves recovery paths, but enforcing the existing onlyOnUserAction policy before opaque subprocesses start is a more direct prevention path; maintainers should define how the two approaches compose.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: The PR addresses recurring Claude background Keychain prompts with limited provider-specific blast radius rather than a core-runtime emergency.
  • merge-risk: 🚨 compatibility: Persisted cooldowns can delay background Claude CLI recovery for existing users after transient timeout or generic failure outcomes.
  • merge-risk: 🚨 auth-provider: The patch changes when Claude CLI authentication is probed and when the CLI provider is considered available during background Auto refresh.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Focused tests cover the new state machine, but the contributor explicitly has no after-fix packaged-app observation in the formerly broken credential state; add redacted logs, terminal output, or a recording, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

What I checked:

Likely related people:

  • afarwind: Authored the merged background Claude CLI auth-status preflight that this PR modifies and supplied extensive exact-head packaged-app proof for that feature. (role: introduced behavior; confidence: high; commits: e486c6c5f0ef, 05475115182f; files: Sources/CodexBarCore/Providers/Claude/ClaudeCLIAuthStatusProbe.swift, Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift, Tests/CodexBarTests/ClaudeCLIAuthStatusProbeTests.swift)
  • steipete: Recent current-main commits and merged history touch Claude credential synchronization, prompt behavior, and the provider-fetch path implicated by this PR. (role: recent Claude authentication contributor and merger; confidence: high; commits: 18e3469627a1, 0c8fbffe2583, 05475115182f; files: Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift, Sources/CodexBarCore/Providers/Claude/ClaudeOAuthCredentials.swift)
  • enieuwy: Introduced the merged Claude Keychain prompt-mode surface whose onlyOnUserAction semantics are central to deciding whether background opaque CLI probes should run. (role: prompt-policy feature contributor; confidence: medium; commits: 409a77e05c82; files: Sources/CodexBarCore/Providers/Claude/ClaudeOAuthCredentials.swift, Sources/CodexBar/Settings/ProvidersPane.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.
Review history (5 earlier review cycles)
  • reviewed 2026-07-14T13:41:20.223Z sha b154b41 :: needs real behavior proof before merge. :: [P1] Avoid six-hour cooldowns for every timeout | [P3] Remove the release-owned changelog edit
  • reviewed 2026-07-14T14:04:55.892Z sha b154b41 :: needs real behavior proof before merge. :: [P1] Avoid six-hour cooldowns for every timeout | [P3] Remove the release-owned changelog edit
  • reviewed 2026-07-14T14:28:10.160Z sha b154b41 :: needs real behavior proof before merge. :: [P1] Do not apply a six-hour cooldown to every timeout | [P3] Remove the release-owned changelog edit
  • reviewed 2026-07-14T14:55:22.884Z sha b154b41 :: needs real behavior proof before merge. :: [P1] Avoid six-hour suppression for every timeout | [P3] Remove the release-owned changelog edit
  • reviewed 2026-07-14T15:30:00.678Z sha b154b41 :: needs real behavior proof before merge. :: [P1] Reserve the six-hour cooldown for blocked probes | [P3] Remove the release-owned changelog edit

@axhoff
axhoff force-pushed the fix/claude-cli-auth-preflight-cooldown branch from b154b41 to dbe477e Compare July 15, 2026 16:59
@axhoff

axhoff commented Jul 15, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 15, 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: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@axhoff

axhoff commented Jul 15, 2026

Copy link
Copy Markdown
Author

Proof/status clarification for the current head dbe477e2:

  • The durable ClawSweeper verdict above reviewed the old head b154b419. Since then, the generic timeout cooldown was narrowed from six hours to 15 minutes, the release-owned changelog edit was removed, the branch was rebased and is currently reported mergeable, and persisted cooldown reload/expiry coverage was added.
  • Real packaged-app pre-fix evidence and executable attribution are recorded in Constantly reoccuring keychain prompt for "Claude Code-credentials" #2115 (comment): the recurring prompt was attributed to the delegated Claude CLI's /usr/bin/security process, disabling the Claude provider stopped the loop, and a clean Claude login restored prompt-free operation.
  • Post-fix behavior is covered by the focused suites listed in the PR body: timeout/failure backoff, persisted reload/expiry, background gating, user/CLI bypass, successful-repair clearing, subprocess coalescing, and cancellation.
  • The original machine no longer has the broken credential state after the documented clean-login workaround, so that comment is honest pre-fix runtime proof plus recovery evidence—not a claim that the patched bundle was observed in the formerly broken state.
  • GitHub's native CI run for this fork currently shows action_required with no jobs and needs maintainer approval to start.

A fresh ClawSweeper review has been requested for the current head.

@steipete

Copy link
Copy Markdown
Owner

Thank you @axhoff for the decisive /usr/bin/security unified-log diagnosis on #2115. That evidence separated the delegated Claude CLI boundary from CodexBar’s direct Security.framework reads and made the broader fix possible.

With #2191 and #2195 now merged, the known background CLI preflight, delegated refresh, MCP-only discovery, and startup-bootstrap prompt routes are policy-blocked before they can launch under the default user-action-only policy. That makes the persisted 15-minute cooldown gate in this PR likely unnecessary.

I’m keeping this PR open for now. If any background Claude auth probing still survives in the next release, a useful rescope would be just the probe-outcome classification/coalescing piece, without the persisted cooldown policy. Fresh release-build evidence from the surviving path would make that narrower follow-up easy to evaluate.

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. P2 Normal priority bug or improvement with limited blast radius. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants