Skip to content

feat: permit disabling claude cli fallback#162

Merged
hanrw merged 2 commits intotddworks:mainfrom
farmdawgnation:msf/disable-claude-fallback-6i1
Mar 23, 2026
Merged

feat: permit disabling claude cli fallback#162
hanrw merged 2 commits intotddworks:mainfrom
farmdawgnation:msf/disable-claude-fallback-6i1

Conversation

@farmdawgnation
Copy link
Copy Markdown
Contributor

@farmdawgnation farmdawgnation commented Mar 23, 2026

The claude cli callback sometimes creates issues along the lines of ssh key prompts and the like. Sometimes, folks would just prefer to see the failure message. This enables folks to turn off the cli fallback if users would prefer it to not happen as a usability issue.

closes #139

Summary by CodeRabbit

  • New Features
    • Added a "CLI fallback" toggle in Claude settings (visible in API probe mode) to enable/disable falling back to the command-line client when API calls fail.
  • Behavior Change
    • Claude will no longer attempt CLI fallback when the setting is disabled; API failures are reported without invoking CLI probes.
  • Tests
    • Added acceptance and unit tests covering the new setting and its persistence.

The claude cli callback sometimes creates issues along the lines of ssh
key prompts and the like. Sometimes, folks would just prefer to see the
failure message. This enables folks to turn off the cli fallback.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5df301ce-f141-4561-b9a3-3b8d1b8174fd

📥 Commits

Reviewing files that changed from the base of the PR and between e360ca6 and 4552a47.

📒 Files selected for processing (3)
  • Tests/DomainTests/Provider/Claude/ClaudeProviderTests.swift
  • Tests/InfrastructureTests/Settings/JSONSettingsRepositoryProviderTests.swift
  • Tests/InfrastructureTests/UserDefaultsProviderSettingsRepositoryTests.swift

📝 Walkthrough

Walkthrough

Adds a user-facing toggle and backing settings to disable Claude's CLI fallback when probeMode == .api, updates provider logic to respect the setting during availability checks and fallback selection, and adds persistence and tests for the new preference.

Changes

Cohort / File(s) Summary
UI & Settings Exposure
Sources/App/Views/Settings/ClaudeConfigCard.swift
Adds @State claudeCliFallbackEnabled, initializes it from settings on appear, and shows a "CLI fallback" toggle (visible in API probe mode) that persists changes via settings repository.
Domain Logic & Protocol
Sources/Domain/Provider/Claude/ClaudeProvider.swift, Sources/Domain/Provider/ProviderSettingsRepository.swift
Adds claudeCliFallbackEnabled() / setClaudeCliFallbackEnabled(_:) to ClaudeSettingsRepository. ClaudeProvider now consults the setting: in .api mode it will not treat CLI as a fallback when the flag is disabled (adjusts isAvailable() and fallbackProbe()).
Storage Implementation
Sources/Infrastructure/Storage/JSONSettingsRepository.swift, Sources/Infrastructure/Storage/UserDefaultsProviderSettingsRepository.swift
Implements getter/setter for claude.cliFallbackEnabled with default true and key providerConfig.claudeCliFallbackEnabled in JSON and UserDefaults repositories.
Acceptance & Unit Tests
Tests/AcceptanceTests/ClaudeConfigSpec.swift, Tests/DomainTests/Provider/Claude/ClaudeProviderTests.swift, Tests/InfrastructureTests/...
Adds acceptance test ensuring no CLI fallback when disabled; unit tests for isAvailable() with CLI fallback on/off; repository tests for default and persisted values.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant UI as ClaudeConfigCard
    participant Repo as SettingsRepository
    participant Provider as ClaudeProvider
    participant API as API Probe
    participant CLI as CLI Probe

    User->>UI: Toggle "CLI fallback" off
    UI->>Repo: setClaudeCliFallbackEnabled(false)
    Repo->>Repo: Persist setting

    Note over User,CLI: Later: Quota refresh attempt

    Provider->>Repo: claudeCliFallbackEnabled()
    Repo-->>Provider: false
    Provider->>API: check availability
    API-->>Provider: unavailable (auth error)
    Provider->>Provider: fallback disabled -> do not consult CLI
    Provider-->>UI: set lastError, snapshot stays nil

    rect rgba(255,100,100,0.5)
    Note over CLI: CLI probe NOT consulted
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A tiny toggle, quietly bright,

When API falls, no CLI in sight.
No ssh knocks, no restless hum—
Just calm alerts until you're done.
Hop, toggle, hop—control is won! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: permit disabling claude cli fallback' accurately and concisely describes the main change: adding an option to disable Claude CLI fallback behavior.
Linked Issues check ✅ Passed All coding requirements from issue #139 are met: UI toggle in ClaudeConfigCard, setting persistence via JSONSettingsRepository and UserDefaultsProviderSettingsRepository, CLI fallback gating in ClaudeProvider, and comprehensive test coverage.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing the CLI fallback disable feature: UI toggle, setting storage, provider logic, and related tests. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.29%. Comparing base (cc42fb5) to head (4552a47).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #162      +/-   ##
==========================================
+ Coverage   80.06%   80.29%   +0.23%     
==========================================
  Files         102      104       +2     
  Lines        7741     7847     +106     
==========================================
+ Hits         6198     6301     +103     
- Misses       1543     1546       +3     
Files with missing lines Coverage Δ
...ources/Domain/Provider/Claude/ClaudeProvider.swift 89.14% <100.00%> (+0.43%) ⬆️
...s/Domain/Provider/ProviderSettingsRepository.swift 100.00% <ø> (ø)
...nfrastructure/Storage/JSONSettingsRepository.swift 57.74% <100.00%> (+0.80%) ⬆️
...orage/UserDefaultsProviderSettingsRepository.swift 81.18% <100.00%> (+0.41%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hanrw hanrw requested a review from Copilot March 23, 2026 04:41
@hanrw hanrw merged commit 8b32d36 into tddworks:main Mar 23, 2026
6 checks passed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a user-facing setting to disable Claude’s CLI fallback while in API probe mode, preventing claude /usage from being invoked when API probing fails (addressing the SSH key prompt/usability issues described in #139).

Changes:

  • Introduces a persisted claudeCliFallbackEnabled setting (defaulting to true) across settings repositories.
  • Updates ClaudeProvider to respect the setting by skipping CLI fallback in API mode when disabled.
  • Adds UI toggle (shown in API probe mode) plus unit/integration/acceptance tests for default + persistence + behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Tests/InfrastructureTests/UserDefaultsProviderSettingsRepositoryTests.swift Verifies default value and persistence for the new UserDefaults-backed setting.
Tests/InfrastructureTests/Settings/JSONSettingsRepositoryProviderTests.swift Verifies default value and persistence for the new JSON-backed setting.
Tests/DomainTests/Provider/Claude/ClaudeProviderTests.swift Adds behavior tests for API mode availability with fallback enabled/disabled.
Tests/AcceptanceTests/ClaudeConfigSpec.swift Ensures refresh in API mode does not fall back to CLI when the toggle is disabled.
Sources/Infrastructure/Storage/UserDefaultsProviderSettingsRepository.swift Implements claudeCliFallbackEnabled storage + key.
Sources/Infrastructure/Storage/JSONSettingsRepository.swift Implements claudeCliFallbackEnabled storage in JSON settings.
Sources/Domain/Provider/ProviderSettingsRepository.swift Extends ClaudeSettingsRepository with CLI fallback enablement APIs.
Sources/Domain/Provider/Claude/ClaudeProvider.swift Gates CLI fallback behavior in API mode based on the new setting.
Sources/App/Views/Settings/ClaudeConfigCard.swift Adds the “CLI fallback” toggle UI under API probe mode.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.toggleStyle(.switch)
.tint(theme.accentPrimary)
.onChange(of: claudeCliFallbackEnabled) { _, newValue in
settings.claude.setClaudeCliFallbackEnabled(newValue)
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The “CLI fallback” toggle persists the setting but doesn’t trigger a refresh, so the UI may continue showing a snapshot fetched via the previous behavior until the next scheduled/manual refresh. Consider mirroring the probe-mode picker by refreshing monitor (or clearing the snapshot) when this toggle changes so the effect is immediately visible.

Suggested change
settings.claude.setClaudeCliFallbackEnabled(newValue)
settings.claude.setClaudeCliFallbackEnabled(newValue)
Task {
await monitor.refresh(providerId: "claude")
}

Copilot uses AI. Check for mistakes.
Comment on lines +174 to +178
/// Whether to fall back to the CLI probe when the OAuth API probe is unavailable.
/// Defaults to true. Disable to prevent `claude /usage` from running in API mode.
func claudeCliFallbackEnabled() -> Bool

/// Sets whether CLI fallback is enabled in API mode
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new doc comment says CLI fallback is used when the OAuth API probe is “unavailable”, but in ClaudeProvider.refresh() fallback can occur for any API probe error (e.g., authenticationRequired, network errors) as long as the CLI is available. Consider rewording to reflect that this setting controls fallback when the API probe fails or is unavailable, so the documentation matches runtime behavior.

Suggested change
/// Whether to fall back to the CLI probe when the OAuth API probe is unavailable.
/// Defaults to true. Disable to prevent `claude /usage` from running in API mode.
func claudeCliFallbackEnabled() -> Bool
/// Sets whether CLI fallback is enabled in API mode
/// Whether to fall back to the CLI probe when the OAuth API probe fails or is unavailable.
/// Defaults to true. Disable to prevent `claude /usage` from running as a fallback in API mode.
func claudeCliFallbackEnabled() -> Bool
/// Sets whether CLI fallback is enabled when the OAuth API probe fails or is unavailable.

Copilot uses AI. Check for mistakes.
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.

Ability to disable fallback for Claude

3 participants