Skip to content

Add CLI availability checks and provider opt-in settings#39

Merged
mzhaom merged 7 commits intomainfrom
fix/cli-check
Feb 16, 2026
Merged

Add CLI availability checks and provider opt-in settings#39
mzhaom merged 7 commits intomainfrom
fix/cli-check

Conversation

@mzhaom
Copy link
Member

@mzhaom mzhaom commented Feb 12, 2026

Summary

  • Detect installed CLI tools (claude, codex, gemini) at startup via exec.LookPath and version probing; surface availability on the welcome screen and in repo settings
  • Add EnabledProviders setting so users can toggle which providers appear in Alt+M model cycling — only installed+enabled providers are offered
  • Move task router from wt/taskrouter/ to bramble/taskrouter/ and refactor it to use the generic agent.Provider interface, allowing any backend (not just Codex) to power routing decisions
  • Extract AgentModel definitions and model registry into multiagent/agent/ for shared use across packages

Test plan

  • scripts/lint.sh passes
  • bazel test //bramble/... //multiagent/... --test_timeout=60 — 13/13 tests pass
  • bazel test //wt/... --test_timeout=60 — 2/2 tests pass
  • Manual: bramble → Alt+M only cycles installed+enabled CLIs
  • Manual: repo settings → provider toggles appear with correct install status
  • Manual: disable a provider → its models disappear from cycling
  • Manual: welcome screen shows provider status line

🤖 Generated with Claude Code


Note

Medium Risk
Touches session execution/model selection and startup wiring; misconfiguration could prevent sessions from starting or change routing/provider defaults, though changes are guarded by availability checks and covered by tests.

Overview
Adds provider discovery and opt-in controls: Bramble now probes installed AI CLIs (Claude/Codex/Gemini), shows their status on the welcome screen, and lets users enable/disable providers in repo settings; enabled providers filter Alt+M model cycling and persisted settings via new Settings.EnabledProviders.

Refactors AI model/provider plumbing by introducing multiagent/agent ProviderAvailability + ModelRegistry, wiring them through startup and session.Manager to fail fast when a session selects a model whose provider is missing/disabled, and improving tmux-mode completion detection by reading the pane exit status.

Moves the task router into a new bramble/taskrouter package and switches it to the generic agent.Provider interface, with startup selecting the “best available” routing provider (codex→claude→gemini) and updated integration/unit tests.

Written by Cursor Bugbot for commit 35446bb. This will update automatically on new commits. Configure here.

@chatgpt-codex-connector
Copy link

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

@mzhaom
Copy link
Member Author

mzhaom commented Feb 13, 2026

All review findings have been addressed across multiple commits:

High Severity Issues:

  1. Disabling all providers silently re-enables them (eb7a981)

    • Changed var result []string to result := make([]string, 0, ...) so empty slice != nil
  2. Missing nil check on router result causes panic (eb7a981)

    • Added nil check before accessing result.Error in Router.Route()
  3. Empty enabled providers treated as all-enabled (a6f09ad)

    • Changed EnabledProviders to *[]string pointer type
    • nil pointer = all enabled, non-nil empty slice = all disabled
    • Updated all consumers to check enabledProviders == nil
  4. Show() conflates nil and empty (8501188)

    • Fixed Show() to use enabledProviders == nil instead of len(enabledProviders) == 0

Medium Severity Issues:

  1. Data race on shared ModelRegistry (eb7a981)

    • Added sync.RWMutex to ModelRegistry
    • Build new slice locally, then swap under write lock
  2. Non-installed providers polluting enabled set (a2db38b)

    • Only mark installed providers as enabled in default path
    • Filter out non-installed providers from explicit list
    • Compare against installed count in backward-compat check
  3. JSON omitempty loses all-disabled state (a6f09ad)

    • Pointer type *[]string preserves distinction through JSON round-trip

Low Severity Issues:

  1. Unused IsProviderEnabled method (a6f09ad)

    • Kept for API completeness, added GetEnabledProviders() helper
  2. Duplicated boxWidth calculation (a6f09ad)

    • Extracted dialogBoxWidth() helper to prevent navigation-rendering mismatch

All tests pass and lint is clean. Ready for re-review.

mzhaom and others added 7 commits February 14, 2026 21:30
Detect installed CLI tools (claude, codex, gemini) at startup and let
users enable/disable providers in repo settings. Only installed+enabled
providers appear in Alt+M model cycling and the welcome screen shows
provider status. Moves the task router from wt/taskrouter/ to
bramble/taskrouter/ and refactors it to use the generic Provider
interface so any backend can power routing decisions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Wire ModelRegistry into session.ManagerConfig so the provider
  availability guard in runSession() is actually enforced
- Add 5s timeout to --version probes so a hanging CLI can't block startup
- Discard stderr in version probes to filter Node.js deprecation warnings
  (fixes gemini showing punycode DeprecationWarning)
- Fix SetEnabledProviders to distinguish nil (all enabled) from empty
  slice (all disabled) so users can actually disable all providers
- Fix NextModel to return current model unchanged when filtered list is
  empty, avoiding selection of an unavailable provider
- Fix integration test to supply a codex provider (required by new API)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the blind left/right theme cycling with a responsive grid of
color swatches. Each swatch shows the theme name in its accent color
and colored dots previewing Running/Error/Idle/Pending/Dim palette
colors. Arrow keys navigate the 2D grid; selected theme still
live-previews across the app.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- EnabledProviders: use make([]string, 0, ...) so disabling all providers
  returns a non-nil empty slice instead of nil (which means "all enabled")
- ModelRegistry: add sync.RWMutex so Rebuild (UI goroutine) and readers
  (session goroutines) don't race on the filtered slice
- Router.Route: nil-check result before dereferencing result.Error to
  prevent panic when provider returns (nil, nil)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Only mark installed providers as enabled in Show() default path, and
filter out non-installed providers from explicit enabledProviders list.
Also compare against installed count (not total) when deciding whether
to return nil ("all enabled") from EnabledProviders().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses remaining review comments:

1. **Fix nil-vs-empty confusion (High Severity)**:
   - Changed EnabledProviders from `[]string` to `*[]string` in Settings
   - nil pointer means "all providers enabled" (default/unset)
   - Non-nil pointer to empty slice means "no providers enabled"
   - This distinction survives JSON round-trip (nil omitted, [] preserved)
   - Updated all consumers to use `enabledProviders == nil` instead of `len() == 0`

2. **Remove unused IsProviderEnabled method**:
   - Method is never called, but kept it for API completeness
   - Added GetEnabledProviders() helper for common access pattern

3. **Fix boxWidth code duplication (Low Severity)**:
   - Extracted dialogBoxWidth() helper method
   - Prevents navigation-rendering mismatch if calculation diverges

Related bugbot findings:
- Fixes "Empty enabled providers treated as all-enabled" (ID: 79b6d740)
- Fixes "JSON omitempty loses all-disabled state" (ID: adb89310)
- Addresses "Duplicated boxWidth calculation" (ID: a3cbc788)
The Show() method was using len(enabledProviders) == 0 to check for
the "all enabled" case, but this matches both nil and empty slices.
According to the Settings design:
- nil means "all enabled" (default/unset)
- empty slice means "all disabled"

Changed to enabledProviders == nil to correctly distinguish these cases.
This prevents the dialog from re-enabling all providers when the user
has explicitly disabled everything.

Fixes the last remaining issue identified in PR review.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@mzhaom mzhaom merged commit 5dd6c44 into main Feb 16, 2026
3 checks passed
@mzhaom mzhaom deleted the fix/cli-check branch February 16, 2026 17:12
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

Comments