Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion apps/desktop/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ fixPath();

import { spawn } from "node:child_process";
import path from "node:path";
import { BrowserWindow, app, dialog, ipcMain, session } from "electron";
import { BrowserWindow, app, dialog, ipcMain, session, shell } from "electron";

import {
EDITORS,
IPC_CHANNELS,
type TerminalCommandInput,
type TerminalCommandResult,
Expand Down Expand Up @@ -125,6 +126,29 @@ function registerIpcHandlers(): void {
}),
);

// Shell handlers
ipcMain.handle(
IPC_CHANNELS.shellOpenInEditor,
async (_event, cwd: string, editor: string) => {
if (!cwd) throw new Error("cwd is required");
const editorDef = EDITORS.find((e) => e.id === editor);
if (!editorDef) throw new Error(`Unknown editor: ${editor}`);
if (!editorDef.command) {
const error = await shell.openPath(cwd);
if (error) throw new Error(error);
return;
}
const child = spawn(editorDef.command, [cwd], {
detached: true,
stdio: "ignore",
});
child.on("error", () => {
/* ignore spawn failures for detached editors */
});
child.unref();
},
Comment on lines +129 to +149
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n apps/desktop/src/main.ts | sed -n '135,153p'

Repository: pingdotgg/codething-mvp

Length of output: 848


🌐 Web query:

Electron shell.openPath() return value error handling

💡 Result:

shell.openPath(path) returns a Promise<string> that resolves (not rejects) to:

  • "" (empty string) on success
  • a non-empty error message string on failure (electronjs.org)

So error handling is typically:

import { shell } from "electron";

async function openFile(p: string) {
  try {
    const err = await shell.openPath(p);
    if (err) {
      // open failed; Electron returns an error message string
      console.error("openPath failed:", err);
      // show UI / log / fallback here
    }
  } catch (e) {
    // for unexpected exceptions (e.g., bad argument types, other runtime issues)
    console.error("openPath threw:", e);
  }
}

Key point: check the resolved string—don’t rely only on catch. (electronjs.org)

Citations:


🌐 Web query:

Node.js child_process spawn error event handling

💡 Result:

In Node.js, child_process.spawn() can fail asynchronously, so you should always attach an 'error' handler to the returned ChildProcess:

  • 'error' event is emitted when the process could not be spawned (e.g., ENOENT, EACCES), could not be killed, or IPC send failed. The 'exit' event may or may not fire after an 'error', so don’t rely on 'exit' alone. [1]
  • 'close' event fires after the process ends and all stdio streams are closed; it will emit after 'exit', or after 'error' if the child failed to spawn. [2]

A common safe pattern is to funnel both 'error' and 'close' into a single “done once” handler:

import { spawn } from 'node:child_process';

function spawnChecked(cmd, args, opts) {
  const child = spawn(cmd, args, opts);

  let settled = false;
  const done = (err, result) => {
    if (settled) return;
    settled = true;
    if (err) throw err;              // or reject a Promise / call cb(err)
    return result;
  };

  child.once('error', (err) => done(err));                  // spawn/kill/IPC failure
  child.once('close', (code, signal) => {                   // stdio fully closed
    if (code === 0) return done(null, { code, signal });
    done(Object.assign(new Error(`exit ${code ?? 'null'} (${signal ?? 'no-signal'})`), { code, signal }));
  });

  return child;
}

Sources: Node.js ChildProcess event docs for 'error' and 'close'. [1] [2]

References
[1] Node.js docs — ChildProcess event: 'error' (nodejs.org)
[2] Node.js v22 docs — ChildProcess event: 'close' (nodejs.org)

Citations:


Add error handling for shell.openPath() and spawn() to prevent crashes and silent failures.
shell.openPath() returns a Promise that resolves to an error string (not thrown), and spawn() emits an 'error' event asynchronously when the process cannot be spawned (e.g., missing CLI). Without handling these, the IPC handler silently fails or crashes. Also guard against empty cwd.

🛠️ Suggested fix
-    async (_event, cwd: string, editor: string) => {
-      if (editor === "file-manager") {
-        await shell.openPath(cwd);
-        return;
-      }
+    async (_event, cwd: string, editor: string) => {
+      if (!cwd) {
+        throw new Error("cwd is required");
+      }
+      if (editor === "file-manager") {
+        const error = await shell.openPath(cwd);
+        if (error) {
+          throw new Error(error);
+        }
+        return;
+      }
       const EDITOR_COMMANDS: Record<string, { command: string; args: (cwd: string) => string[] }> = {
         cursor: { command: "cursor", args: (p) => [p] },
       };
       const entry = EDITOR_COMMANDS[editor];
       if (!entry) throw new Error(`Unknown editor: ${editor}`);
-      const child = spawn(entry.command, entry.args(cwd), {
-        detached: true,
-        stdio: "ignore",
-      });
-      child.unref();
+      await new Promise<void>((resolve, reject) => {
+        const child = spawn(entry.command, entry.args(cwd), {
+          detached: true,
+          stdio: "ignore",
+        });
+        child.once("error", reject);
+        child.once("spawn", () => {
+          child.unref();
+          resolve();
+        });
+      });
     },
   );
📝 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
// Shell handlers
ipcMain.handle(
IPC_CHANNELS.shellOpenInEditor,
async (_event, cwd: string, editor: string) => {
if (editor === "file-manager") {
await shell.openPath(cwd);
return;
}
const EDITOR_COMMANDS: Record<string, { command: string; args: (cwd: string) => string[] }> = {
cursor: { command: "cursor", args: (p) => [p] },
};
const entry = EDITOR_COMMANDS[editor];
if (!entry) throw new Error(`Unknown editor: ${editor}`);
const child = spawn(entry.command, entry.args(cwd), {
detached: true,
stdio: "ignore",
});
child.unref();
},
// Shell handlers
ipcMain.handle(
IPC_CHANNELS.shellOpenInEditor,
async (_event, cwd: string, editor: string) => {
if (!cwd) {
throw new Error("cwd is required");
}
if (editor === "file-manager") {
const error = await shell.openPath(cwd);
if (error) {
throw new Error(error);
}
return;
}
const EDITOR_COMMANDS: Record<string, { command: string; args: (cwd: string) => string[] }> = {
cursor: { command: "cursor", args: (p) => [p] },
};
const entry = EDITOR_COMMANDS[editor];
if (!entry) throw new Error(`Unknown editor: ${editor}`);
await new Promise<void>((resolve, reject) => {
const child = spawn(entry.command, entry.args(cwd), {
detached: true,
stdio: "ignore",
});
child.once("error", reject);
child.once("spawn", () => {
child.unref();
resolve();
});
});
},
);
🤖 Prompt for AI Agents
In `@apps/desktop/src/main.ts` around lines 135 - 153, The IPC handler for
ipcMain.handle should validate cwd is non-empty, await shell.openPath(cwd) and
check its returned string for an error (log/throw when non-empty) instead of
assuming success, and protect the spawn path by wrapping the spawn call in
try/catch and attaching an 'error' listener to the returned child (e.g.,
child.on('error', ...) to log/throw when the CLI cannot be spawned); reference
EDITOR_COMMANDS and entry (from editor) to locate the spawn logic, and ensure
you handle and surface errors for both shell.openPath and spawn so the handler
doesn't crash or fail silently.

);

// Agent handlers
ipcMain.handle(
IPC_CHANNELS.agentSpawn,
Expand Down
4 changes: 4 additions & 0 deletions apps/desktop/src/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ const nativeApi: NativeApi = {
ipcRenderer.removeListener(IPC_CHANNELS.providerEvent, listener);
},
},
shell: {
openInEditor: (cwd: string, editor: string) =>
ipcRenderer.invoke(IPC_CHANNELS.shellOpenInEditor, cwd, editor),
},
};

contextBridge.exposeInMainWorld("nativeApi", nativeApi);
95 changes: 95 additions & 0 deletions apps/renderer/src/components/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
useState,
} from "react";

import { EDITORS, type EditorId } from "@acme/contracts";
import {
DEFAULT_MODEL,
DEFAULT_REASONING,
Expand All @@ -32,6 +33,18 @@ function formatMessageMeta(createdAt: string, duration: string | null): string {
return `${formatTimestamp(createdAt)} • ${duration}`;
}

const FILE_MANAGER_LABEL = navigator.platform.includes("Mac")
? "Finder"
: navigator.platform.includes("Win")
? "Explorer"
: "Files";

function editorLabel(editor: (typeof EDITORS)[number]): string {
return editor.command ? editor.label : FILE_MANAGER_LABEL;
}

const LAST_EDITOR_KEY = "codething:last-editor";

function workToneClass(tone: "thinking" | "tool" | "info" | "error"): string {
if (tone === "error") return "text-rose-300/50";
if (tone === "tool") return "text-[#8a8a8a]";
Expand All @@ -46,12 +59,20 @@ export default function ChatView() {
const [isSending, setIsSending] = useState(false);
const [isConnecting, setIsConnecting] = useState(false);
const [isModelMenuOpen, setIsModelMenuOpen] = useState(false);
const [isEditorMenuOpen, setIsEditorMenuOpen] = useState(false);
const [lastEditor, setLastEditor] = useState<EditorId>(() => {
const stored = localStorage.getItem(LAST_EDITOR_KEY);
return EDITORS.some((e) => e.id === stored)
? (stored as EditorId)
: EDITORS[0].id;
});
const [selectedEffort, setSelectedEffort] =
useState<string>(DEFAULT_REASONING);
const [nowTick, setNowTick] = useState(() => Date.now());
const messagesEndRef = useRef<HTMLDivElement>(null);
const textareaRef = useRef<HTMLTextAreaElement>(null);
const modelMenuRef = useRef<HTMLDivElement>(null);
const editorMenuRef = useRef<HTMLDivElement>(null);

const activeThread = state.threads.find((t) => t.id === state.activeThreadId);
const activeProject = state.projects.find(
Expand Down Expand Up @@ -179,6 +200,47 @@ export default function ChatView() {
};
}, [isModelMenuOpen]);

useEffect(() => {
if (!isEditorMenuOpen) return;

const handleClickOutside = (event: MouseEvent) => {
if (!editorMenuRef.current) return;
if (
event.target instanceof Node &&
!editorMenuRef.current.contains(event.target)
) {
setIsEditorMenuOpen(false);
}
};

window.addEventListener("mousedown", handleClickOutside);
return () => {
window.removeEventListener("mousedown", handleClickOutside);
};
}, [isEditorMenuOpen]);

// Cmd+O / Ctrl+O to open in last-used editor
useEffect(() => {
const handler = (e: globalThis.KeyboardEvent) => {
if (e.key === "o" && (e.metaKey || e.ctrlKey) && !e.shiftKey) {
if (api && activeProject) {
e.preventDefault();
void api.shell.openInEditor(activeProject.cwd, lastEditor);
}
}
};
window.addEventListener("keydown", handler);
return () => window.removeEventListener("keydown", handler);
}, [api, activeProject, lastEditor]);

const openInEditor = (editorId: EditorId) => {
if (!api || !activeProject) return;
void api.shell.openInEditor(activeProject.cwd, editorId);
setLastEditor(editorId);
localStorage.setItem(LAST_EDITOR_KEY, editorId);
setIsEditorMenuOpen(false);
};

const ensureSession = async (): Promise<string | null> => {
if (!api || !activeThread || !activeProject) return null;
if (activeThread.session && activeThread.session.status !== "closed") {
Expand Down Expand Up @@ -313,6 +375,39 @@ export default function ChatView() {
</h2>
</div>
<div className="flex items-center gap-3">
{/* Open in editor */}
{activeProject && (
<div className="relative" ref={editorMenuRef}>
<button
type="button"
className="rounded-md px-2 py-1 text-[10px] text-[#a0a0a0]/40 transition-colors duration-150 hover:text-[#a0a0a0]/60"
onClick={() => setIsEditorMenuOpen((v) => !v)}
>
Open in&hellip;
</button>
{isEditorMenuOpen && (
<div className="absolute right-0 top-full z-50 mt-1 min-w-[120px] rounded-md border border-white/[0.08] bg-[#1b1b1d] py-1 shadow-xl">
{EDITORS.map((editor) => (
<button
key={editor.id}
type="button"
className="flex w-full items-center gap-2 px-3 py-1.5 text-left text-[11px] text-[#e0e0e0] hover:bg-white/[0.06]"
onClick={() => openInEditor(editor.id)}
>
{editorLabel(editor)}
{editor.id === lastEditor && (
<kbd className="ml-auto text-[9px] text-[#a0a0a0]/40">
{navigator.platform.includes("Mac")
? "\u2318O"
: "Ctrl+O"}
</kbd>
)}
</button>
))}
</div>
)}
</div>
)}
{/* Diff toggle */}
<button
type="button"
Expand Down
11 changes: 11 additions & 0 deletions packages/contracts/src/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ import type {
import type { TerminalCommandInput, TerminalCommandResult } from "./terminal";
import type { NewTodoInput, Todo } from "./todo";

export const EDITORS = [
{ id: "cursor", label: "Cursor", command: "cursor" },
{ id: "file-manager", label: "File Manager", command: null },
] as const;

export type EditorId = (typeof EDITORS)[number]["id"];

export const IPC_CHANNELS = {
todosList: "todos:list",
todosAdd: "todos:add",
Expand All @@ -29,6 +36,7 @@ export const IPC_CHANNELS = {
providerSessionStop: "provider:session:stop",
providerSessionList: "provider:session:list",
providerEvent: "provider:event",
shellOpenInEditor: "shell:open-in-editor",
} as const;

export interface NativeApi {
Expand Down Expand Up @@ -63,4 +71,7 @@ export interface NativeApi {
listSessions: () => Promise<ProviderSession[]>;
onEvent: (callback: (event: ProviderEvent) => void) => () => void;
};
shell: {
openInEditor: (cwd: string, editor: EditorId) => Promise<void>;
};
}