Handle missing mux binaries gracefully - #12
Merged
Conversation
|
WalkthroughAdds session-mux (tmux/zellij) detection and diagnostics across doctor and global install flows, introduces installation guidance, and adds defensive guards and tests so project/session listing tolerates missing mux executables. Changes
Sequence Diagram(s)sequenceDiagram
participant Global as GlobalInstall
participant Warn as warnIfSessionsMuxUnavailable
participant Resolver as resolveSessionsMuxMode
participant Shell as findExecutableInPath
participant Flow as GlobalFlow
Global->>Warn: invoke before Docker checks
Warn->>Resolver: resolve configured mux mode
Resolver->>Shell: check `tmux` in PATH
Shell-->>Resolver: tmux found / not found
Resolver->>Shell: check `zellij` in PATH (if needed)
Shell-->>Resolver: zellij found / not found
Resolver-->>Warn: mode + availability
Warn->>Global: warn/ok and provide install command if needed
Global-->>Flow: continue install flow
sequenceDiagram
participant Doctor as DoctorCommand
participant Checker as checkSessionsMuxConfig
participant Resolver as resolveSessionsMuxMode
participant Shell as findExecutableInPath
participant Tools as ToolChecks
Doctor->>Checker: run sessions-mux check during analysis
Checker->>Resolver: determine mux mode (none/tmux/zellij/auto)
Resolver->>Shell: probe PATH for `tmux` and `zellij`
Shell-->>Resolver: availability results
Resolver-->>Checker: mode + availability
Checker->>Doctor: return CheckResult with message/install guidance
Doctor->>Tools: include zellij/tmux in tool discovery results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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
♻️ Duplicate comments (1)
src/commands/doctor.ts (1)
389-396:muxInstallCommandis an exact copy of the one insrc/commands/global.ts.See the refactor suggestion on
global.tslines 375–382 for the extraction approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/doctor.ts` around lines 389 - 396, The function muxInstallCommand in doctor.ts duplicates the implementation already in global.ts; extract this helper into a shared module (e.g., a new util or the existing global helper location) and have both doctor.ts and global.ts import and call muxInstallCommand instead of redefining it; ensure the extracted function signature (muxInstallCommand(opts: { provider: "tmux" | "zellij"; })) and dependency on isMac() are preserved and update imports in both files accordingly so there's a single authoritative implementation.
🧹 Nitpick comments (2)
src/commands/doctor.ts (1)
398-465:findProjectContextis called twice perdoctorrun for the samestartDir.
checkProject(line 310) andcheckSessionsMuxConfig(line 316) each independently callfindProjectContext(startDir), duplicating the async FS traversal. Consider threading the resolvedProjectContext | nullintocheckSessionsMuxConfigto eliminate the redundant walk.♻️ Suggested change (sketch)
- results.push( - await runCheck(s, "sessions mux", () => - checkSessionsMuxConfig({ startDir }) - ) - ); + const projectCtxValue = projectCtx.status === "ok" + ? await findProjectContext(startDir) + : null; + results.push( + await runCheck(s, "sessions mux", () => + checkSessionsMuxConfig({ project: projectCtxValue }) + ) + );Then update
checkSessionsMuxConfigto accept{ project: ProjectContext | null }directly instead of re-resolving fromstartDir.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/doctor.ts` around lines 398 - 465, checkSessionsMuxConfig currently calls findProjectContext(startDir) again causing duplicate async FS work; change its signature to accept the already-resolved ProjectContext | null (e.g. async function checkSessionsMuxConfig(opts: { project: ProjectContext | null }) ) and update callers (notably checkProject which already resolves project via findProjectContext) to pass the project through instead of startDir, leaving resolveSessionsMuxMode, findExecutableInPath, and muxInstallCommand usage unchanged; ensure any remaining callers that still pass startDir are updated or adapted to resolve project first and pass it in.src/commands/global.ts (1)
375-382:muxInstallCommandis duplicated verbatim insrc/commands/doctor.ts.Both files independently define the same function body. Extract it to a shared helper (e.g.,
src/mux/mux-install.tsalongsidemux-config.ts) and import it in both callsites.♻️ Suggested extraction
New file
src/mux/mux-install.ts:import { isMac } from "../lib/os.ts"; /** * Returns a platform-appropriate install hint for a session mux provider. * `@param` opts.provider - The mux binary to install. * `@returns` A shell command string suitable for display in user-facing messages. */ export function muxInstallCommand(opts: { readonly provider: "tmux" | "zellij"; }): string { if (isMac()) { return `brew install ${opts.provider}`; } return `install ${opts.provider} with your package manager`; }Then in both
global.tsanddoctor.ts:-function muxInstallCommand(opts: { - readonly provider: "tmux" | "zellij"; -}): string { - if (isMac()) { - return `brew install ${opts.provider}`; - } - return `install ${opts.provider} with your package manager`; -} +import { muxInstallCommand } from "../mux/mux-install.ts";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/global.ts` around lines 375 - 382, The function muxInstallCommand is duplicated; extract it into a shared helper module (e.g., a new mux-install module) and export the function so both callers import it instead of redefining it. Move the existing implementation of muxInstallCommand (which uses isMac()) into that new module, export function muxInstallCommand(opts: { provider: "tmux" | "zellij" }), then update both files that currently define muxInstallCommand to import and call the shared muxInstallCommand; keep the same signature and behavior so callers in global.ts and doctor.ts work unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/project-views.test.ts`:
- Around line 421-438: The test "buildProjectViews tolerates missing optional
mux binaries" captures previousPath = process.env.PATH and restores it
unconditionally, which turns undefined into the string "undefined"; change the
cleanup to restore PATH safely by checking previousPath: if it is undefined
delete process.env.PATH, otherwise set process.env.PATH = previousPath. Update
the finally block in this test (around previousPath and process.env.PATH
manipulation) to perform that conditional restore so PATH is not corrupted when
previousPath was undefined.
---
Duplicate comments:
In `@src/commands/doctor.ts`:
- Around line 389-396: The function muxInstallCommand in doctor.ts duplicates
the implementation already in global.ts; extract this helper into a shared
module (e.g., a new util or the existing global helper location) and have both
doctor.ts and global.ts import and call muxInstallCommand instead of redefining
it; ensure the extracted function signature (muxInstallCommand(opts: { provider:
"tmux" | "zellij"; })) and dependency on isMac() are preserved and update
imports in both files accordingly so there's a single authoritative
implementation.
---
Nitpick comments:
In `@src/commands/doctor.ts`:
- Around line 398-465: checkSessionsMuxConfig currently calls
findProjectContext(startDir) again causing duplicate async FS work; change its
signature to accept the already-resolved ProjectContext | null (e.g. async
function checkSessionsMuxConfig(opts: { project: ProjectContext | null }) ) and
update callers (notably checkProject which already resolves project via
findProjectContext) to pass the project through instead of startDir, leaving
resolveSessionsMuxMode, findExecutableInPath, and muxInstallCommand usage
unchanged; ensure any remaining callers that still pass startDir are updated or
adapted to resolve project first and pass it in.
In `@src/commands/global.ts`:
- Around line 375-382: The function muxInstallCommand is duplicated; extract it
into a shared helper module (e.g., a new mux-install module) and export the
function so both callers import it instead of redefining it. Move the existing
implementation of muxInstallCommand (which uses isMac()) into that new module,
export function muxInstallCommand(opts: { provider: "tmux" | "zellij" }), then
update both files that currently define muxInstallCommand to import and call the
shared muxInstallCommand; keep the same signature and behavior so callers in
global.ts and doctor.ts work unchanged.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/commands/doctor.tssrc/commands/global.tssrc/lib/project-views.tssrc/lib/runtime-projects.tstests/project-views.test.ts
Comment on lines
+421
to
+438
| test("buildProjectViews tolerates missing optional mux binaries", async () => { | ||
| const emptyPathDir = await mkdtemp(join(tmpdir(), "hack-empty-path-")); | ||
| const previousPath = process.env.PATH; | ||
| process.env.PATH = emptyPathDir; | ||
|
|
||
| try { | ||
| const views = await buildProjectViews({ | ||
| registryProjects: [], | ||
| runtime: [], | ||
| runtimeOk: true, | ||
| filter: null, | ||
| includeUnregistered: false, | ||
| }); | ||
| expect(views).toEqual([]); | ||
| } finally { | ||
| process.env.PATH = previousPath; | ||
| await rm(emptyPathDir, { recursive: true, force: true }); | ||
| } |
There was a problem hiding this comment.
Guard against corrupting PATH when it was originally undefined.
process.env.PATH is typed string | undefined. If previousPath is undefined, assigning it back with process.env.PATH = previousPath silently coerces to the string "undefined" in Bun/Node, leaving PATH set to a nonsense value for the rest of the test run.
🛡️ Proposed fix
- const previousPath = process.env.PATH;
- process.env.PATH = emptyPathDir;
+ const previousPath = process.env.PATH;
+ process.env.PATH = emptyPathDir;
try {
...
} finally {
- process.env.PATH = previousPath;
+ if (previousPath === undefined) {
+ delete process.env.PATH;
+ } else {
+ process.env.PATH = previousPath;
+ }
await rm(emptyPathDir, { recursive: true, force: true });
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/project-views.test.ts` around lines 421 - 438, The test
"buildProjectViews tolerates missing optional mux binaries" captures
previousPath = process.env.PATH and restores it unconditionally, which turns
undefined into the string "undefined"; change the cleanup to restore PATH safely
by checking previousPath: if it is undefined delete process.env.PATH, otherwise
set process.env.PATH = previousPath. Update the finally block in this test
(around previousPath and process.env.PATH manipulation) to perform that
conditional restore so PATH is not corrupted when previousPath was undefined.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/commands/setup.ts`:
- Around line 164-168: Replace the object-shaped type alias SetupMultiLogResult
with an equivalent interface declaration named SetupMultiLogResult (keeping the
same members: status: string; optional path?: string; optional message?: string
and readonly modifiers if desired) so it follows the project guideline to prefer
interface for object shapes; locate the declaration of SetupMultiLogResult and
change the `type` to `interface` and leave all usages untouched.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
src/commands/setup.ts
Comment on lines
+164
to
+168
| type SetupMultiLogResult = { | ||
| readonly status: string; | ||
| readonly path?: string; | ||
| readonly message?: string; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use interface for object shapes.
The new SetupMultiLogResult should follow the TypeScript guideline to prefer interface over type for object shapes.
♻️ Suggested change
-type SetupMultiLogResult = {
- readonly status: string;
- readonly path?: string;
- readonly message?: string;
-};
+interface SetupMultiLogResult {
+ readonly status: string
+ readonly path?: string
+ readonly message?: string
+}As per coding guidelines: “**/*.ts?(x): Prefer interface for defining object shapes in TypeScript”.
📝 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
| type SetupMultiLogResult = { | |
| readonly status: string; | |
| readonly path?: string; | |
| readonly message?: string; | |
| }; | |
| interface SetupMultiLogResult { | |
| readonly status: string | |
| readonly path?: string | |
| readonly message?: string | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/setup.ts` around lines 164 - 168, Replace the object-shaped type
alias SetupMultiLogResult with an equivalent interface declaration named
SetupMultiLogResult (keeping the same members: status: string; optional path?:
string; optional message?: string and readonly modifiers if desired) so it
follows the project guideline to prefer interface for object shapes; locate the
declaration of SetupMultiLogResult and change the `type` to `interface` and
leave all usages untouched.
roodboi
pushed a commit
that referenced
this pull request
Feb 24, 2026
## <small>1.12.1 (2026-02-24)</small> * fix: trigger patch release ([d0b2d42](d0b2d42)) * Debug hack projects mux detection ([67720dc](67720dc)) * Investigate hack projects mux issue ([6a862b0](6a862b0)) * Investigate hack projects mux lookup ([b579038](b579038)) * Merge branch 'main' into dependabot/npm_and_yarn/examples/next-app/next-16.1.5 ([9fe8b94](9fe8b94)) * Merge branch 'main' into hack-projects-mux-issue ([c4cd388](c4cd388)) * Merge pull request #12 from hack-dance/hack-projects-mux-issue ([b34e802](b34e802)), closes [#12](#12) * Merge pull request #6 from hack-dance/dependabot/npm_and_yarn/examples/next-app/next-16.1.5 ([b671a64](b671a64)), closes [#6](#6) * Merge pull request #7 from hack-dance/dependabot/npm_and_yarn/modelcontextprotocol/sdk-1.26.0 ([ab4d64c](ab4d64c)), closes [#7](#7) * chore(release): trigger release ([e3bb438](e3bb438)) * build(deps): bump @modelcontextprotocol/sdk from 1.25.3 to 1.26.0 ([ef1278c](ef1278c)) * build(deps): bump next from 16.1.1 to 16.1.5 in /examples/next-app ([ea1460d](ea1460d))
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.
Summary
Testing
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Refactor