Skip to content

Commit 5cc0ba8

Browse files
ammar-agentammar-agent
authored
🤖 refactor: unify editor APIs and fix plan file SSH support (#1043)
## Summary Consolidates the editor-opening APIs into a single unified interface and fixes plan file handling for SSH workspaces. ## Changes ### Editor API Consolidation - Merged `openWorkspaceInEditor` + `openFileInEditor` → single `openInEditor(workspaceId, targetPath, editorConfig)` - Removed old non-runtime-aware `openInEditor` and `canOpenInEditor` APIs (~140 lines) - Hook now takes explicit `targetPath` parameter (no implicit workspace root fallback) ### Plan File SSH Support - Plan files now use workspace runtime instead of always using local filesystem - Removed `localRuntime` special-casing from file tools (~50 lines across 3 files) - Simplified `planStorage.ts` to just return `~/.mux/plans/{workspaceId}.md` ### Files Changed - **EditorService**: Single `openInEditor()` method - **API Schema**: Single `openInEditor` endpoint with required `targetPath` - **Router**: Removed old editor discovery/fallback code - **useOpenInEditor**: Simplified to `(workspaceId, targetPath, runtimeConfig?)` - **File tools**: Removed localRuntime parameter and special-casing - **Callers**: WorkspaceHeader, ProposePlanToolCall, chatCommands updated ## Testing - All 1607 unit tests pass - TypeCheck passes - Static check passes --- _Generated with `mux`_
1 parent a033b83 commit 5cc0ba8

24 files changed

+358
-562
lines changed

‎Makefile‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,13 +133,13 @@ dev: node_modules/.installed build-main ## Start development server (Vite + node
133133
# https://github.com/oven-sh/bun/issues/18275
134134
@NODE_OPTIONS="--max-old-space-size=4096" npm x concurrently -k --raw \
135135
"bun x nodemon --watch src --watch tsconfig.main.json --watch tsconfig.json --ext ts,tsx,json --ignore dist --ignore node_modules --exec node scripts/build-main-watch.js" \
136-
"npx esbuild src/cli/api.ts --bundle --format=esm --platform=node --outfile=dist/cli/api.mjs --external:zod --external:commander --external:@trpc/server --watch" \
136+
"npx esbuild src/cli/api.ts --bundle --format=esm --platform=node --target=node20 --outfile=dist/cli/api.mjs --external:zod --external:commander --external:@trpc/server --watch" \
137137
"vite"
138138
else
139139
dev: node_modules/.installed build-main build-preload ## Start development server (Vite + tsgo watcher for 10x faster type checking)
140140
@bun x concurrently -k \
141141
"bun x concurrently \"$(TSGO) -w -p tsconfig.main.json\" \"bun x tsc-alias -w -p tsconfig.main.json\"" \
142-
"bun x esbuild src/cli/api.ts --bundle --format=esm --platform=node --outfile=dist/cli/api.mjs --external:zod --external:commander --external:@trpc/server --watch" \
142+
"bun x esbuild src/cli/api.ts --bundle --format=esm --platform=node --target=node20 --outfile=dist/cli/api.mjs --external:zod --external:commander --external:@trpc/server --watch" \
143143
"vite"
144144
endif
145145

@@ -153,7 +153,7 @@ dev-server: node_modules/.installed build-main ## Start server mode with hot rel
153153
@# On Windows, use npm run because bunx doesn't correctly pass arguments
154154
@npmx concurrently -k \
155155
"npmx nodemon --watch src --watch tsconfig.main.json --watch tsconfig.json --ext ts,tsx,json --ignore dist --ignore node_modules --exec node scripts/build-main-watch.js" \
156-
"npx esbuild src/cli/api.ts --bundle --format=esm --platform=node --outfile=dist/cli/api.mjs --external:zod --external:commander --external:@trpc/server --watch" \
156+
"npx esbuild src/cli/api.ts --bundle --format=esm --platform=node --target=node20 --outfile=dist/cli/api.mjs --external:zod --external:commander --external:@trpc/server --watch" \
157157
"npmx nodemon --watch dist/cli/index.js --watch dist/cli/server.js --delay 500ms --exec \"node dist/cli/index.js server --host $(or $(BACKEND_HOST),localhost) --port $(or $(BACKEND_PORT),3000)\"" \
158158
"$(SHELL) -lc \"MUX_VITE_HOST=$(or $(VITE_HOST),127.0.0.1) MUX_VITE_PORT=$(or $(VITE_PORT),5173) VITE_BACKEND_URL=http://$(or $(BACKEND_HOST),localhost):$(or $(BACKEND_PORT),3000) vite\""
159159
else
@@ -165,7 +165,7 @@ dev-server: node_modules/.installed build-main ## Start server mode with hot rel
165165
@echo "For remote access: make dev-server VITE_HOST=0.0.0.0 BACKEND_HOST=0.0.0.0"
166166
@bun x concurrently -k \
167167
"bun x concurrently \"$(TSGO) -w -p tsconfig.main.json\" \"bun x tsc-alias -w -p tsconfig.main.json\"" \
168-
"bun x esbuild src/cli/api.ts --bundle --format=esm --platform=node --outfile=dist/cli/api.mjs --external:zod --external:commander --external:@trpc/server --watch" \
168+
"bun x esbuild src/cli/api.ts --bundle --format=esm --platform=node --target=node20 --outfile=dist/cli/api.mjs --external:zod --external:commander --external:@trpc/server --watch" \
169169
"bun x nodemon --watch dist/cli/index.js --watch dist/cli/server.js --delay 500ms --exec 'NODE_ENV=development node dist/cli/index.js server --host $(or $(BACKEND_HOST),localhost) --port $(or $(BACKEND_PORT),3000)'" \
170170
"MUX_VITE_HOST=$(or $(VITE_HOST),127.0.0.1) MUX_VITE_PORT=$(or $(VITE_PORT),5173) VITE_BACKEND_URL=http://$(or $(BACKEND_HOST),localhost):$(or $(BACKEND_PORT),3000) vite"
171171
endif
@@ -192,6 +192,7 @@ dist/cli/api.mjs: src/cli/api.ts src/cli/proxifyOrpc.ts $(TS_SOURCES)
192192
--bundle \
193193
--format=esm \
194194
--platform=node \
195+
--target=node20 \
195196
--outfile=dist/cli/api.mjs \
196197
--external:zod \
197198
--external:commander \

‎src/browser/components/Settings/sections/GeneralSection.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export function GeneralSection() {
6565
<div className="flex items-center justify-between">
6666
<div>
6767
<div className="text-foreground text-sm">Editor</div>
68-
<div className="text-muted text-xs">Editor to open workspaces in</div>
68+
<div className="text-muted text-xs">Editor to open files in</div>
6969
</div>
7070
<Select value={editorConfig.editor} onValueChange={handleEditorChange}>
7171
<SelectTrigger className="border-border-medium bg-background-secondary hover:bg-hover h-9 w-auto cursor-pointer rounded-md border px-3 text-sm transition-colors">

‎src/browser/components/WorkspaceHeader.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,13 @@ export const WorkspaceHeader: React.FC<WorkspaceHeaderProps> = ({
4040

4141
const handleOpenInEditor = useCallback(async () => {
4242
setEditorError(null);
43-
const result = await openInEditor(workspaceId, runtimeConfig);
43+
const result = await openInEditor(workspaceId, namedWorkspacePath, runtimeConfig);
4444
if (!result.success && result.error) {
4545
setEditorError(result.error);
4646
// Clear error after 3 seconds
4747
setTimeout(() => setEditorError(null), 3000);
4848
}
49-
}, [workspaceId, openInEditor, runtimeConfig]);
49+
}, [workspaceId, namedWorkspacePath, openInEditor, runtimeConfig]);
5050

5151
// Start workspace tutorial on first entry (only if settings tutorial is done)
5252
useEffect(() => {

‎src/browser/components/tools/ProposePlanToolCall.tsx‎

Lines changed: 47 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,15 @@ import { useCopyToClipboard } from "@/browser/hooks/useCopyToClipboard";
2121
import { Tooltip, TooltipTrigger, TooltipContent } from "../ui/tooltip";
2222
import { cn } from "@/common/lib/utils";
2323
import { useAPI } from "@/browser/contexts/API";
24+
import { useOpenInEditor } from "@/browser/hooks/useOpenInEditor";
25+
import { useWorkspaceContext } from "@/browser/contexts/WorkspaceContext";
26+
import { usePopoverError } from "@/browser/hooks/usePopoverError";
27+
import { PopoverError } from "../PopoverError";
2428

2529
/**
26-
* Check if the result is from the new file-based propose_plan tool
30+
* Check if the result is a successful file-based propose_plan result
2731
*/
28-
function isNewProposePlanResult(result: unknown): result is ProposePlanToolResult {
32+
function isProposePlanResult(result: unknown): result is ProposePlanToolResult {
2933
return (
3034
result !== null &&
3135
typeof result === "object" &&
@@ -50,7 +54,7 @@ function isProposePlanError(result: unknown): result is ProposePlanToolError {
5054
}
5155

5256
/**
53-
* Check if the result is from the legacy propose_plan tool
57+
* Check if the result is from the legacy propose_plan tool (title + plan params)
5458
*/
5559
function isLegacyProposePlanResult(result: unknown): result is LegacyProposePlanToolResult {
5660
return (
@@ -105,22 +109,18 @@ export const ProposePlanToolCall: React.FC<ProposePlanToolCallProps> = (props) =
105109
const { expanded, toggleExpanded } = useToolExpansion(true); // Expand by default
106110
const [showRaw, setShowRaw] = useState(false);
107111
const { api } = useAPI();
112+
const openInEditor = useOpenInEditor();
113+
const { workspaceMetadata } = useWorkspaceContext();
114+
const editorError = usePopoverError();
115+
116+
// Get runtimeConfig for the workspace (needed for SSH-aware editor opening)
117+
const runtimeConfig = workspaceId ? workspaceMetadata.get(workspaceId)?.runtimeConfig : undefined;
108118

109119
// Fresh content from disk for the latest plan (external edit detection)
110120
// Skip for ephemeral previews which already have fresh content
111121
const [freshContent, setFreshContent] = useState<string | null>(null);
112122
const [freshPath, setFreshPath] = useState<string | null>(null);
113123

114-
// Check if an editor is available (hides Edit button if not)
115-
const [canEdit, setCanEdit] = useState(false);
116-
117-
useEffect(() => {
118-
if (!api) return;
119-
void api.general.canOpenInEditor().then((result) => {
120-
setCanEdit(result.method !== "none");
121-
});
122-
}, [api]);
123-
124124
// Fetch fresh plan content for the latest plan
125125
// Re-fetches on mount and when window regains focus (after user edits in external editor)
126126
useEffect(() => {
@@ -152,7 +152,7 @@ export const ProposePlanToolCall: React.FC<ProposePlanToolCallProps> = (props) =
152152
};
153153
}, [api, workspaceId, isLatest, isEphemeralPreview]);
154154

155-
// Determine plan content and title based on result type (prefer result over args)
155+
// Determine plan content and title based on result type
156156
// For ephemeral previews, use direct content/path props
157157
// For the latest plan, prefer fresh content from disk (external edit support)
158158
let planContent: string;
@@ -172,13 +172,14 @@ export const ProposePlanToolCall: React.FC<ProposePlanToolCallProps> = (props) =
172172
// Extract title from first markdown heading or use filename
173173
const titleMatch = /^#\s+(.+)$/m.exec(freshContent);
174174
planTitle = titleMatch ? titleMatch[1] : (planPath?.split("/").pop() ?? "Plan");
175-
} else if (isNewProposePlanResult(result)) {
175+
} else if (isProposePlanResult(result)) {
176176
planContent = result.planContent;
177177
planPath = result.planPath;
178178
// Extract title from first markdown heading or use filename
179179
const titleMatch = /^#\s+(.+)$/m.exec(result.planContent);
180180
planTitle = titleMatch ? titleMatch[1] : (planPath.split("/").pop() ?? "Plan");
181181
} else if (isLegacyProposePlanResult(result)) {
182+
// Legacy format: title + plan passed directly (no file)
182183
planContent = result.plan;
183184
planTitle = result.title;
184185
} else if (isProposePlanError(result)) {
@@ -187,11 +188,11 @@ export const ProposePlanToolCall: React.FC<ProposePlanToolCallProps> = (props) =
187188
planTitle = "Plan Error";
188189
errorMessage = result.error;
189190
} else if (isLegacyProposePlanArgs(args)) {
190-
// Fallback to args for backwards compatibility
191+
// Fallback to args for legacy format (streaming state before result)
191192
planContent = args.plan;
192193
planTitle = args.title;
193194
} else {
194-
// No valid plan data available
195+
// No valid plan data available (e.g., pending state)
195196
planContent = "";
196197
planTitle = "Plan";
197198
}
@@ -212,37 +213,17 @@ export const ProposePlanToolCall: React.FC<ProposePlanToolCallProps> = (props) =
212213
// Copy to clipboard with feedback
213214
const { copied, copyToClipboard } = useCopyToClipboard();
214215

215-
const handleOpenInEditor = async () => {
216-
if (!planPath || !api) {
216+
const handleOpenInEditor = async (event: React.MouseEvent) => {
217+
if (!planPath || !workspaceId) {
217218
return;
218219
}
219-
try {
220-
const result = await api.general.openInEditor({
221-
filePath: planPath,
222-
workspaceId,
220+
const result = await openInEditor(workspaceId, planPath, runtimeConfig);
221+
if (!result.success && result.error) {
222+
const rect = (event.target as HTMLElement).getBoundingClientRect();
223+
editorError.showError("plan-editor", result.error, {
224+
top: rect.bottom + 8,
225+
left: rect.left,
223226
});
224-
if (!result.success) {
225-
console.error("Failed to open plan in editor:", result.error);
226-
return;
227-
}
228-
// If opened in embedded terminal (server mode), open terminal window
229-
if (
230-
result.data.openedInEmbeddedTerminal &&
231-
result.data.workspaceId &&
232-
result.data.sessionId
233-
) {
234-
const isBrowser = !window.api;
235-
if (isBrowser) {
236-
const url = `/terminal.html?workspaceId=${encodeURIComponent(result.data.workspaceId)}&sessionId=${encodeURIComponent(result.data.sessionId)}`;
237-
window.open(
238-
url,
239-
`terminal-editor-${result.data.sessionId}`,
240-
"width=1000,height=600,popup=yes"
241-
);
242-
}
243-
}
244-
} catch (err) {
245-
console.error("openInEditor threw error:", err);
246227
}
247228
};
248229

@@ -262,12 +243,12 @@ export const ProposePlanToolCall: React.FC<ProposePlanToolCallProps> = (props) =
262243
)}
263244
</div>
264245
<div className="flex items-center gap-1.5">
265-
{/* Edit button: show for ephemeral preview OR latest tool call, only if editor available */}
266-
{(isEphemeralPreview ?? isLatest) && planPath && api && canEdit && (
246+
{/* Edit button: show for ephemeral preview OR latest tool call */}
247+
{(isEphemeralPreview ?? isLatest) && planPath && workspaceId && (
267248
<Tooltip>
268249
<TooltipTrigger asChild>
269250
<button
270-
onClick={() => void handleOpenInEditor()}
251+
onClick={(e) => void handleOpenInEditor(e)}
271252
className={cn(
272253
controlButtonClasses,
273254
"plan-chip-ghost hover:plan-chip-ghost-hover"
@@ -357,21 +338,29 @@ export const ProposePlanToolCall: React.FC<ProposePlanToolCallProps> = (props) =
357338

358339
// Ephemeral preview mode: simple wrapper without tool container
359340
if (isEphemeralPreview) {
360-
return <div className={cn("px-4 py-2", className)}>{planUI}</div>;
341+
return (
342+
<>
343+
<div className={cn("px-4 py-2", className)}>{planUI}</div>
344+
<PopoverError error={editorError.error} prefix="Failed to open editor" />
345+
</>
346+
);
361347
}
362348

363349
// Tool call mode: full tool container with header
364350
return (
365-
<ToolContainer expanded={expanded}>
366-
<ToolHeader onClick={toggleExpanded}>
367-
<ExpandIcon expanded={expanded}>▶</ExpandIcon>
368-
<ToolName>propose_plan</ToolName>
369-
<StatusIndicator status={status}>{statusDisplay}</StatusIndicator>
370-
</ToolHeader>
351+
<>
352+
<ToolContainer expanded={expanded}>
353+
<ToolHeader onClick={toggleExpanded}>
354+
<ExpandIcon expanded={expanded}>▶</ExpandIcon>
355+
<ToolName>propose_plan</ToolName>
356+
<StatusIndicator status={status}>{statusDisplay}</StatusIndicator>
357+
</ToolHeader>
371358

372-
{expanded && <ToolDetails>{planUI}</ToolDetails>}
359+
{expanded && <ToolDetails>{planUI}</ToolDetails>}
373360

374-
{modal}
375-
</ToolContainer>
361+
{modal}
362+
</ToolContainer>
363+
<PopoverError error={editorError.error} prefix="Failed to open editor" />
364+
</>
376365
);
377366
};

‎src/browser/hooks/useOpenInEditor.ts‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,26 @@ export interface OpenInEditorResult {
1616
}
1717

1818
/**
19-
* Hook to open a workspace in the user's configured code editor.
19+
* Hook to open a path in the user's configured code editor.
2020
*
2121
* If no editor is configured, opens Settings to the General section.
2222
* For SSH workspaces with unsupported editors (Zed, custom), returns an error.
2323
*
24-
* @returns A function that takes workspaceId and optional runtimeConfig,
25-
* returns a result object with success/error status.
24+
* @returns A function that opens a path in the editor:
25+
* - workspaceId: required workspace identifier
26+
* - targetPath: the path to open (workspace directory or specific file)
27+
* - runtimeConfig: optional, used to detect SSH workspaces for validation
2628
*/
2729
export function useOpenInEditor() {
2830
const { api } = useAPI();
2931
const { open: openSettings } = useSettings();
3032

3133
return useCallback(
32-
async (workspaceId: string, runtimeConfig?: RuntimeConfig): Promise<OpenInEditorResult> => {
34+
async (
35+
workspaceId: string,
36+
targetPath: string,
37+
runtimeConfig?: RuntimeConfig
38+
): Promise<OpenInEditorResult> => {
3339
// Read editor config from localStorage
3440
const editorConfig = readPersistedState<EditorConfig>(
3541
EDITOR_CONFIG_KEY,
@@ -44,10 +50,13 @@ export function useOpenInEditor() {
4450
return { success: false, error: "Please configure a custom editor command in Settings" };
4551
}
4652

47-
// For SSH workspaces, validate the editor supports Remote-SSH
53+
// For SSH workspaces, validate the editor supports Remote-SSH (only VS Code/Cursor)
4854
if (isSSH) {
4955
if (editorConfig.editor === "zed") {
50-
return { success: false, error: "Zed does not support Remote-SSH for SSH workspaces" };
56+
return {
57+
success: false,
58+
error: "Zed does not support Remote-SSH for SSH workspaces",
59+
};
5160
}
5261
if (editorConfig.editor === "custom") {
5362
return {
@@ -58,8 +67,9 @@ export function useOpenInEditor() {
5867
}
5968

6069
// Call the backend API
61-
const result = await api?.general.openWorkspaceInEditor({
70+
const result = await api?.general.openInEditor({
6271
workspaceId,
72+
targetPath,
6373
editorConfig,
6474
});
6575

‎src/browser/utils/chatCommands.test.ts‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -254,9 +254,7 @@ describe("handlePlanOpenCommand", () => {
254254
getPlanContentResult:
255255
| { success: true; data: { content: string; path: string } }
256256
| { success: false; error: string },
257-
openInEditorResult?:
258-
| { success: true; data: { openedInEmbeddedTerminal: boolean } }
259-
| { success: false; error: string }
257+
openInEditorResult?: { success: true; data: undefined } | { success: false; error: string }
260258
): CommandHandlerContext => {
261259
const setInput = mock(() => undefined);
262260
const setToast = mock(() => undefined);
@@ -271,9 +269,7 @@ describe("handlePlanOpenCommand", () => {
271269
},
272270
general: {
273271
openInEditor: mock(() =>
274-
Promise.resolve(
275-
openInEditorResult ?? { success: true, data: { openedInEmbeddedTerminal: false } }
276-
)
272+
Promise.resolve(openInEditorResult ?? { success: true, data: undefined })
277273
),
278274
},
279275
} as unknown as CommandHandlerContext["api"],
@@ -309,7 +305,7 @@ describe("handlePlanOpenCommand", () => {
309305
test("opens plan in editor when plan exists", async () => {
310306
const context = createMockContext(
311307
{ success: true, data: { content: "# My Plan", path: "/path/to/plan.md" } },
312-
{ success: true, data: { openedInEmbeddedTerminal: false } }
308+
{ success: true, data: undefined }
313309
);
314310

315311
const result = await handlePlanOpenCommand(context);
@@ -320,8 +316,10 @@ describe("handlePlanOpenCommand", () => {
320316
workspaceId: "test-workspace-id",
321317
});
322318
expect(context.api.general.openInEditor).toHaveBeenCalledWith({
323-
filePath: "/path/to/plan.md",
324319
workspaceId: "test-workspace-id",
320+
targetPath: "/path/to/plan.md",
321+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
322+
editorConfig: expect.any(Object),
325323
});
326324
});
327325

0 commit comments

Comments
 (0)