feat: auto-discover installed AI CLIs in command palette#89
Open
n00ki wants to merge 2 commits intoerictli:mainfrom
Open
feat: auto-discover installed AI CLIs in command palette#89n00ki wants to merge 2 commits intoerictli:mainfrom
n00ki wants to merge 2 commits intoerictli:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds dynamic AI provider discovery and integration: a new ai service exports provider order and a helper to probe availability; CommandPalette now accepts an Changes
Sequence DiagramsequenceDiagram
participant User
participant CommandPalette
participant aiService
participant ProviderCheckers
User->>CommandPalette: Open/trigger palette
Note over CommandPalette: useEffect (currentNote present)
CommandPalette->>aiService: getAvailableAiProviders()
aiService->>ProviderCheckers: Concurrently probe (claude, codex, ollama)
ProviderCheckers-->>aiService: Availability results
aiService-->>CommandPalette: availableAiProviders[]
CommandPalette->>CommandPalette: Build aiCommands from providers
CommandPalette-->>User: Display commands including AI variants
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/services/ai.ts (1)
49-57: Add a timeout fallback so one hung checker doesn’t block all provider discovery.If a single checker never resolves, discovery can stall indefinitely and hide all AI commands.
Proposed diff
+const PROVIDER_CHECK_TIMEOUT_MS = 1500; + +async function checkProviderAvailability( + provider: AiProvider, +): Promise<AiProvider | null> { + try { + const installed = await Promise.race<boolean>([ + providerCheckers[provider](), + new Promise<boolean>((resolve) => + setTimeout(() => resolve(false), PROVIDER_CHECK_TIMEOUT_MS), + ), + ]); + return installed ? provider : null; + } catch { + return null; + } +} + export async function getAvailableAiProviders(): Promise<AiProvider[]> { - const checks = await Promise.all( - AI_PROVIDER_ORDER.map(async (provider) => { - try { - const installed = await providerCheckers[provider](); - return installed ? provider : null; - } catch { - return null; - } - }), - ); + const checks = await Promise.all( + AI_PROVIDER_ORDER.map((provider) => checkProviderAvailability(provider)), + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/ai.ts` around lines 49 - 57, The provider discovery loop using AI_PROVIDER_ORDER and providerCheckers can hang if a checker never resolves; wrap each call to providerCheckers[provider]() in a timeout fallback (e.g., Promise.race between the checker promise and a timeout that rejects or resolves false after a short interval like 2–5s) inside the async mapping function so a stuck checker returns null/false instead of blocking Promise.all; ensure errors from the checker or timeout are caught and mapped to null so the existing return installed ? provider : null logic remains valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/command-palette/CommandPalette.tsx`:
- Around line 129-133: The catch block for provider discovery currently swallows
errors; change the .catch to accept the error (e.g. .catch((err) => { ... }))
and surface a friendly, non-blocking message while still clearing providers:
call setAvailableAiProviders([]) when active, log the error (console.error(err))
and trigger a user-facing notification/state update (for example call an
existing showToast or introduce setAiProviderDiscoveryError state and set a
message like "Could not discover AI providers — some commands may be
unavailable.") so the UI informs the user of the discovery failure.
---
Nitpick comments:
In `@src/services/ai.ts`:
- Around line 49-57: The provider discovery loop using AI_PROVIDER_ORDER and
providerCheckers can hang if a checker never resolves; wrap each call to
providerCheckers[provider]() in a timeout fallback (e.g., Promise.race between
the checker promise and a timeout that rejects or resolves false after a short
interval like 2–5s) inside the async mapping function so a stuck checker returns
null/false instead of blocking Promise.all; ensure errors from the checker or
timeout are caught and mapped to null so the existing return installed ?
provider : null logic remains valid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6bad62a2-9df9-4103-95c9-c3dd4eadc2b3
📒 Files selected for processing (2)
src/components/command-palette/CommandPalette.tsxsrc/services/ai.ts
Comment on lines
+129
to
+133
| .catch(() => { | ||
| if (active) { | ||
| setAvailableAiProviders([]); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Don’t fail provider discovery silently.
When discovery throws, users currently just see no AI commands, which is indistinguishable from “nothing installed.” Add a friendly error message for this non-blocking async failure path.
Proposed diff
- .catch(() => {
+ .catch((error) => {
if (active) {
setAvailableAiProviders([]);
+ console.error("Failed to discover AI providers:", error);
+ toast.error("Unable to load AI commands right now");
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .catch(() => { | |
| if (active) { | |
| setAvailableAiProviders([]); | |
| } | |
| }); | |
| .catch((error) => { | |
| if (active) { | |
| setAvailableAiProviders([]); | |
| console.error("Failed to discover AI providers:", error); | |
| toast.error("Unable to load AI commands right now"); | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/command-palette/CommandPalette.tsx` around lines 129 - 133,
The catch block for provider discovery currently swallows errors; change the
.catch to accept the error (e.g. .catch((err) => { ... })) and surface a
friendly, non-blocking message while still clearing providers: call
setAvailableAiProviders([]) when active, log the error (console.error(err)) and
trigger a user-facing notification/state update (for example call an existing
showToast or introduce setAiProviderDiscoveryError state and set a message like
"Could not discover AI providers — some commands may be unavailable.") so the UI
informs the user of the discovery failure.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/command-palette/CommandPalette.tsx (1)
122-133:⚠️ Potential issue | 🟡 MinorDiscovery failures are still silent to users in this flow.
getAvailableAiProviders()currently resolves with[]on checker errors, so this.catch(...)path won’t reliably surface failures; users still can’t distinguish “none installed” vs “discovery failed.” Please surface a user-friendly non-blocking message for discovery failures (either by propagating failure info from the service or by returning diagnostics and handling them here).As per coding guidelines: "Implement proper error handling with user-friendly messages for all non-blocking async operations".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/command-palette/CommandPalette.tsx` around lines 122 - 133, The discovery flow currently swallows errors so users can't tell if there are truly no providers or a discovery failure; update the CommandPalette component to surface a non-blocking user message on discovery failures by adding a small UI state (e.g., discoveryError via useState) and setting it in the error path of aiService.getAvailableAiProviders(), instead of only console.error; update rendering to show a subtle message when discoveryError is set while still calling setAvailableAiProviders([]) to preserve behavior, and optionally change aiService.getAvailableAiProviders() to return diagnostics or throw so this component can display the diagnostic text.
🧹 Nitpick comments (1)
src/components/command-palette/CommandPalette.tsx (1)
167-190: Make provider-to-command mapping exhaustive-safe.The
if/elsefallback currently defaults unknown providers to Claude metadata. Prefer an exhaustive mapping (e.g.,Record<AiProvider, ...>orswitch+never) so new providers fail at compile time instead of silently mislabeling commands.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/command-palette/CommandPalette.tsx` around lines 167 - 190, Replace the ad-hoc if/return chain that maps provider to command with an exhaustive mapping so new AiProvider values fail at compile time: either (A) use a typed Record<AiProvider, CommandMeta> keyed by the exact AiProvider union and return record[provider], or (B) use a switch(provider) with explicit cases for "codex", "ollama", and "claude" that return the same objects (id "ai-edit-codex"/"ai-edit-ollama"/"ai-edit-claude", label strings, icons CodexIcon/OllamaIcon/ClaudeIcon and action), and add a default branch that asserts exhaustiveness (const _exhaustive: never = provider; throw new Error(`Unhandled provider ${provider}`)); ensure the AiProvider type is used for the provider variable so the compiler will complain when a new provider is added.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/command-palette/CommandPalette.tsx`:
- Around line 122-133: The discovery flow currently swallows errors so users
can't tell if there are truly no providers or a discovery failure; update the
CommandPalette component to surface a non-blocking user message on discovery
failures by adding a small UI state (e.g., discoveryError via useState) and
setting it in the error path of aiService.getAvailableAiProviders(), instead of
only console.error; update rendering to show a subtle message when
discoveryError is set while still calling setAvailableAiProviders([]) to
preserve behavior, and optionally change aiService.getAvailableAiProviders() to
return diagnostics or throw so this component can display the diagnostic text.
---
Nitpick comments:
In `@src/components/command-palette/CommandPalette.tsx`:
- Around line 167-190: Replace the ad-hoc if/return chain that maps provider to
command with an exhaustive mapping so new AiProvider values fail at compile
time: either (A) use a typed Record<AiProvider, CommandMeta> keyed by the exact
AiProvider union and return record[provider], or (B) use a switch(provider) with
explicit cases for "codex", "ollama", and "claude" that return the same objects
(id "ai-edit-codex"/"ai-edit-ollama"/"ai-edit-claude", label strings, icons
CodexIcon/OllamaIcon/ClaudeIcon and action), and add a default branch that
asserts exhaustiveness (const _exhaustive: never = provider; throw new
Error(`Unhandled provider ${provider}`)); ensure the AiProvider type is used for
the provider variable so the compiler will complain when a new provider is
added.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c1ec745e-ff47-4bc4-bd3b-f067ee3d5f6d
📒 Files selected for processing (1)
src/components/command-palette/CommandPalette.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
src/services/ai.ts:AI_PROVIDER_ORDERgetAvailableAiProviders()(parallel checks, graceful fallback on errors)src/components/command-palette/CommandPalette.tsx:themereference in command palette dependenciesWhy
Manual checks:
Summary by CodeRabbit