Skip to content

fix: gate CLI background usage fetch on confirmed-absent OAuth creds - #1

Merged
axisrow merged 3 commits into
mainfrom
fix/claude-cli-background-fallback
Aug 9, 2026
Merged

fix: gate CLI background usage fetch on confirmed-absent OAuth creds#1
axisrow merged 3 commits into
mainfrom
fix/claude-cli-background-fallback

Conversation

@axisrow

@axisrow axisrow commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add directCredentialIsMissingOverride TaskLocal to ClaudeOAuthFetchStrategy for deterministic test overrides
  • Thread an oauthCredentialsConfirmedAbsent check into ClaudeCLIBackgroundAvailability.allowsBackgroundAutoUsageFetch so the CLI background fallback only fires when direct OAuth credentials are confirmed absent
  • Update related characterization/compatibility tests to cover the new gating behavior

Test plan

  • swift test

🤖 Generated with Claude Code

axisrow and others added 3 commits August 9, 2026 18:02
Add directCredentialIsMissingOverride TaskLocal for deterministic testing
and thread an oauthCredentialsConfirmedAbsent check into
ClaudeCLIBackgroundAvailability.allowsBackgroundAutoUsageFetch so the CLI
background fallback only proceeds when direct OAuth credentials are
confirmed absent.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HqBGFKF9M8etkRpgz2bk2Z
allowsBackgroundAutoUsageFetch's deadlock-breaker only checked
isEstablished(), which drops a marker as soon as it's revoked by a
failed foreground fetch. That let the OAuth-absence probe re-permit
a background CLI usage attempt on every tick after a failure,
defeating the existing revocation/backoff guarantee. Check
isRevoked() before falling through to oauthCredentialsConfirmedAbsent
so a revoked marker stays denied until the next foreground success.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HqBGFKF9M8etkRpgz2bk2Z
allowsBackgroundAutoUsageFetch's deadlock-breaker could still fire for
a profile ClaudeAccountProfile.identifiedSessionScope can't identify
(missing/malformed/unreadable account config), since isEstablished and
the round-1 isRevoked check both silently return false when
captureMarker is nil. That let a background CLI fetch launch with no
stable account binding, and a failed attempt could never be recorded
as a revocation (revoke() needs a marker), so nothing would bound
repeated launches. Require a non-nil marker before consulting
oauthCredentialsConfirmedAbsent, matching the fail-closed contract
identifiedSessionScope already documents for background work.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HqBGFKF9M8etkRpgz2bk2Z
@axisrow

axisrow commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

🔍 Local review (cycle 3/3, final) — round a4f67011-f2ea-4bdb-a3c3-1b6e0d0f6b5a

Reviewed locally (/review + Codex companion), no bots pinged.

Verdict Reviewer Finding Location
IRRELEVANT codex Same conservative-absence-check tradeoff already accepted in rounds 1-2; not a new defect introduced by this PR. ClaudeProviderDescriptor.swift:548
SKIP claude The revocation store is a single consistent instance across all its operations, so no marker-identity mismatch exists. ClaudeProviderDescriptor.swift:1128
SKIP claude Only one production call site exists for the new gate, and the default parameter preserves prior fail-closed behavior for any future caller. ClaudeProviderDescriptor.swift:1119
SKIP claude Verified at the guard level that the credential-absence probe cannot trigger a Keychain prompt in any build configuration. ClaudeProviderDescriptor.swift:997

No FIX or UNVERIFIED findings this round. Cycle complete.

@axisrow

axisrow commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

📋 Review summary — all cycles

Cycle Reviewer Finding Verdict Resolution
1 codex Background CLI usage fetch re-permitted every tick after a failed foreground attempt because revocation state was not checked before the OAuth-absence deadlock breaker. FIX Fixed in 5e92940
1 claude Absence-probe closure default only affects the one call site that supplies it; other callers keep prior fail-closed behavior. HALLUCINATION Claim did not match the actual single-call-site code
1 claude DEBUG-only test override does not change production credential-check behavior. SKIP Intentional test scaffolding
1 claude Sequential-branch structure in the availability gate is a maintainability note, not a functional defect. SKIP Style/maintainability observation only
1 codex Conservative absence check reused from explicit OAuth mode does not independently confirm Keychain-only credentials, but this is deliberate pre-existing semantics, not a new bypass. SKIP Deliberate, pre-existing, reused semantics
2 codex Unidentified profiles (no resolvable account marker) could still reach the OAuth-absence fallback, permitting an unbounded background CLI launch with no revocation binding. FIX Fixed in e818260
2 codex Conservative absence check reused from explicit OAuth mode does not independently confirm Keychain-only credentials, but this is deliberate pre-existing semantics, not a new bypass. SKIP Restated round-1 SKIP, same resolution
3 codex Repeat of the round-1 conservative-absence-check observation; no new code path or angle raised. IRRELEVANT Third restatement of the same settled theme
3 claude Revocation store is a single consistent instance across all its read/write operations; no marker-identity mismatch. SKIP Verified correct as implemented
3 claude Exactly one production call site exists for the new gate; the closures default preserves fail-closed behavior for any future caller. SKIP Verified correct as implemented
3 claude Credential-absence probe cannot trigger an interactive Keychain prompt in any build configuration, verified at the guard level. SKIP Verified correct as implemented

Totals: 2 FIX (both resolved), 6 SKIP, 1 HALLUCINATION, 1 IRRELEVANT, 0 UNVERIFIED.

Review complete after 3 cycles — no FIX or UNVERIFIED findings remain. Per the review-only-on-merge policy, merging is a manual step for the PR author.

@axisrow
axisrow merged commit d481955 into main Aug 9, 2026
@axisrow
axisrow deleted the fix/claude-cli-background-fallback branch August 9, 2026 12:06
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