fix: show clear error when only one CLI tool is installed#3
Conversation
📝 WalkthroughWalkthroughAdds defensive early-return guards in interactive tool handoff flows within the CLI. When no alternative tools are available for cross-tool handoff, the code now gathers all tools, identifies missing ones, logs a warning to the user, and exits early before attempting tool selection. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/cli.ts (1)
297-302: Extract the duplicate guard into a helper; deriveallToolsfrom the type constant.The same 5-line block appears verbatim in both code paths. Beyond DRY, the hardcoded
allToolsarray (['claude', 'codex', 'copilot', 'gemini', 'opencode', 'droid']) can silently drift from theSessionSourceunion type if a new tool is ever added. There's also a subtle edge case:missingis computed asallTools.filter(t => !availableTools.includes(t)), which will includesession.sourceitself when that tool is not inavailableTools(e.g., session file outlives the uninstalled tool) — making the "Only X is installed" message factually wrong.♻️ Proposed refactor
Define a single source-of-truth constant near the
sourceColorsmap (or insrc/types/index.ts), and extract a helper:+// All supported tools — keep in sync with SessionSource type +const ALL_TOOLS: SessionSource[] = ['claude', 'codex', 'copilot', 'gemini', 'opencode', 'droid']; +function warnNoTargetTools(sessionSource: SessionSource, availableTools: SessionSource[]): void { + const missing = ALL_TOOLS + .filter(t => t !== sessionSource && !availableTools.includes(t)) + .map(t => t.charAt(0).toUpperCase() + t.slice(1)); + clack.log.warn( + `Only ${sourceColors[sessionSource](sessionSource)} is installed. ` + + `Install at least one more (${missing.join(', ')}) to enable cross-tool handoff.` + ); + clack.outro(chalk.gray('Install more tools to enable handoff')); +}Then replace both guard blocks with:
- if (targetOptions.length === 0) { - const allTools: SessionSource[] = ['claude', 'codex', 'copilot', 'gemini', 'opencode', 'droid']; - const missing = allTools.filter(t => !availableTools.includes(t)).map(t => t.charAt(0).toUpperCase() + t.slice(1)); - clack.log.warn(`Only ${sourceColors[session.source](session.source)} is installed. Install at least one more (${missing.join(', ')}) to enable cross-tool handoff.`); - return; - } + if (targetOptions.length === 0) { + warnNoTargetTools(session.source, availableTools); + return; + }The
t !== sessionSourceaddition also fixes the edge case wheresession.sourceis not inavailableTools(session from an uninstalled tool), preventing it from appearing in the install suggestions.Also applies to: 516-521
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.ts` around lines 297 - 302, Extract the duplicated guard into a reusable helper and derive the list of known sources from a single source-of-truth constant instead of hardcoding; e.g. add a constant ALL_SESSION_SOURCES (next to sourceColors or in src/types) typed as SessionSource[] and implement a helper function (e.g. ensureCrossToolHandoff(targetOptions, availableTools, sessionSource)) that returns early when targetOptions.length === 0, computes missing = ALL_SESSION_SOURCES.filter(t => t !== sessionSource && !availableTools.includes(t)).map(...) and emits the same clack.log.warn using sourceColors[sessionSource](sessionSource); then replace the two identical 5-line blocks (the one using targetOptions/availableTools/session.source) with a call to this helper so the list stays in sync with the SessionSource type and the edge case where session.source is absent is handled.
🤖 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/cli.ts`:
- Around line 297-302: The early-return guard in interactivePick/resume that
checks if targetOptions.length === 0 logs a warning with clack.log.warn(...) but
never closes the TUI; call clack.outro() immediately before the return to
properly close the session (do the same for the other similar guard that
currently returns after clack.log.warn, e.g., the occurrence around the second
availability check). Ensure you add clack.outro() right before each return in
those guard blocks so the TUI opened by clack.intro() is always closed.
---
Nitpick comments:
In `@src/cli.ts`:
- Around line 297-302: Extract the duplicated guard into a reusable helper and
derive the list of known sources from a single source-of-truth constant instead
of hardcoding; e.g. add a constant ALL_SESSION_SOURCES (next to sourceColors or
in src/types) typed as SessionSource[] and implement a helper function (e.g.
ensureCrossToolHandoff(targetOptions, availableTools, sessionSource)) that
returns early when targetOptions.length === 0, computes missing =
ALL_SESSION_SOURCES.filter(t => t !== sessionSource &&
!availableTools.includes(t)).map(...) and emits the same clack.log.warn using
sourceColors[sessionSource](sessionSource); then replace the two identical
5-line blocks (the one using targetOptions/availableTools/session.source) with a
call to this helper so the list stays in sync with the SessionSource type and
the edge case where session.source is absent is handled.
| if (targetOptions.length === 0) { | ||
| const allTools: SessionSource[] = ['claude', 'codex', 'copilot', 'gemini', 'opencode', 'droid']; | ||
| const missing = allTools.filter(t => !availableTools.includes(t)).map(t => t.charAt(0).toUpperCase() + t.slice(1)); | ||
| clack.log.warn(`Only ${sourceColors[session.source](session.source)} is installed. Install at least one more (${missing.join(', ')}) to enable cross-tool handoff.`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Add clack.outro() before each early return to close the TUI session properly.
Both guard blocks return after clack.log.warn() but neither calls clack.outro(). Since clack.intro() was already called (line 178 in interactivePick, line 502 in the resume interactive branch), the terminal will show an unclosed intro top-bar — every other exit path in these functions calls outro() or cancel().
🛠️ Proposed fix (same pattern for both occurrences)
if (targetOptions.length === 0) {
const allTools: SessionSource[] = ['claude', 'codex', 'copilot', 'gemini', 'opencode', 'droid'];
const missing = allTools.filter(t => !availableTools.includes(t)).map(t => t.charAt(0).toUpperCase() + t.slice(1));
clack.log.warn(`Only ${sourceColors[session.source](session.source)} is installed. Install at least one more (${missing.join(', ')}) to enable cross-tool handoff.`);
+ clack.outro(chalk.gray('Install more tools to enable handoff'));
return;
}Also applies to: 516-521
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli.ts` around lines 297 - 302, The early-return guard in
interactivePick/resume that checks if targetOptions.length === 0 logs a warning
with clack.log.warn(...) but never closes the TUI; call clack.outro()
immediately before the return to properly close the session (do the same for the
other similar guard that currently returns after clack.log.warn, e.g., the
occurrence around the second availability check). Ensure you add clack.outro()
right before each return in those guard blocks so the TUI opened by
clack.intro() is always closed.
|
thanks @barisgirismen, finally we're collab on sth! |
Rewrote the entire README to read like an actual developer wrote it: - Punchier intro that leads with the problem, not a feature list - Collapsed the 14×14 checkmark matrix into one sentence - Cut 430→238 lines by removing redundancy and AI-doc patterns - Added Community Contributions section referencing #1, #3, #4, #14 - Documented the 7 new agents (Amp, Kiro, Crush, Cline, Roo Code, Kilo Code, Antigravity) and bugs fixed in this round - Prose over bullet spam, natural flow over exhaustive structure Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When only one CLI tool is installed,
clack.select()receives an empty options array and crashes with:This adds a guard in both places where target tool selection happens. Instead of crashing, it now tells the user which tool was detected and lists the ones they could install:
Summary by CodeRabbit