bring back electron (+ websocket/package changes)#10
Conversation
- Create apps/server with package.json, tsconfig, tsup config - Copy codexAppServerManager, providerManager, fixPath from desktop (no Electron deps) - Add WebSocket message protocol schemas (WsRequest, WsResponse, WsPush) to @acme/contracts - Define WS_METHODS and WS_CHANNELS constants for type-safe routing Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
- wsServer.ts: WebSocket server routes NativeApi methods, serves static files - Provider events pushed to all connected clients - SPA fallback for client-side routing - Server welcome message with cwd/projectName on connect - index.ts: CLI entry point - Auto-detects available port (default 3773) - Resolves renderer dist for static serving - Opens browser on startup via 'open' package - Graceful shutdown on SIGINT/SIGTERM Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
- wsTransport.ts: WebSocket client with request/response, push subscriptions, auto-reconnect with exponential backoff - wsNativeApi.ts: Full NativeApi implementation over WebSocket transport - All provider methods routed through WS_METHODS - Provider events streamed via WS_CHANNELS.providerEvent - Todos/terminal/agent stubbed (not needed for web mode) - dialogs.pickFolder returns null (text input fallback in UI) - session-logic.ts: readNativeApi() now auto-detects environment - Prefers Electron preload bridge if available (window.nativeApi) - Falls back to WebSocket transport for browser mode - onServerWelcome() hook for renderer to receive cwd on connect Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
- App.tsx: Add AutoProjectBootstrap that listens for server welcome and auto-creates project + thread from server's cwd - Sidebar: Conditionalize drag region (Electron only), hide Browse button in browser - ChatView: Conditionalize drag region, adjust padding for browser - DiffPanel: Same drag region treatment - index.html: Update title to 'CodeThing' - env.ts: isElectron detection utility Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
- Root package.json: Add dev, dev:server, dev:web, start scripts - dev runs both server and renderer in parallel via turbo - VITE_WS_URL injected so renderer connects to server in dev mode - turbo.json: Add WS env vars to globalEnv - vite.config.ts: Inject VITE_WS_URL at build time for WS server discovery - wsTransport.ts: Use VITE_WS_URL env var in dev, fall back to page URL in prod - Server dev script: Uses tsx for direct TypeScript execution with VITE_DEV_SERVER_URL Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
- Delete apps/desktop/ entirely (replaced by apps/server) - Remove IPC_CHANNELS constant from contracts (no longer needed) - Keep NativeApi interface (still the renderer's API contract) - Keep EDITORS constant (used by both server and renderer) - Update README.md with new architecture diagram and scripts - Update AGENTS.md with new package roles and architecture Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
- Tests welcome message on connect with cwd/projectName - Tests server.getConfig method - Tests providers.listSessions returns empty array - Tests error response for unknown methods - Tests graceful handling of invalid JSON - All 73 tests pass across 12 test files Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
- wsTransport.ts: Use addEventListener instead of on-function assignments - wsServer.ts: Replace filter(Boolean).pop() with direct array access - wsServer.test.ts: Use addEventListener for WebSocket events Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
- wsNativeApi: Cache last welcome payload and replay to late subscribers. Fixes race where WS connect + welcome arrives before React effect fires. - App.tsx: Add bootstrappedRef to prevent duplicate project creation, skip bootstrap in Electron mode - wsTransport: Use standard import.meta.env for VITE_WS_URL access - CLAUDE.md: Fix header (was incorrectly '# AGENTS.md') Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
Use 'tsx src/index.ts' instead of 'node --import tsx' for simpler dev execution Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
- Remove smoke-test script (was desktop-only) - Remove dist-electron from turbo outputs and .gitignore - Rename ELECTRON_RENDERER_PORT → PORT in vite.config.ts - Clean up turbo globalEnv (remove ELECTRON_RENDERER_PORT, WS_SERVER_PORT) - Update CI workflow to use bun 1.3.9 (matches package.json engines) Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
- Remove private:true so package can be published to npm - Add files field to limit published files to dist/ - Move @acme/contracts to devDependencies (bundled by tsup) - Add @acme/renderer devDependency for turbo build ordering - Add bundle-client.mjs: copies renderer build into dist/client/ - Update resolveStaticDir to check dist/client/ first (npx scenario), then fall back to monorepo layout (../../renderer/dist) - Full build pipeline: contracts → renderer → server + bundle client Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
WalkthroughRefactors desktop IPC to a WebSocket-backed server architecture: adds a new apps/server service with WS RPC and project persistence, changes the desktop to spawn/manage a backend process and expose a minimal desktopBridge, and updates the renderer to connect via a WsTransport and sync projects over WebSocket. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Electron as Electron Main
participant Backend as Backend Process
participant Renderer as Renderer UI
User->>Electron: Launch app
Electron->>Electron: Reserve loopback port & gen token
Electron->>Backend: Spawn backend with port+token
Backend->>Backend: Start server on port
Backend-->>Electron: Ready
Electron->>Renderer: Create BrowserWindow + inject WS URL
Renderer->>Backend: Open WebSocket -> Authenticate
Backend-->>Renderer: Push serverWelcome
Renderer->>Renderer: Sync projects (SYNC_PROJECTS)
sequenceDiagram
participant Renderer as UI
participant Backend as WS Server
participant Registry as ProjectRegistry
Renderer->>Backend: request projects.add(cwd)
Backend->>Registry: validate, dedupe, persist
Registry-->>Backend: { project, created }
Backend-->>Renderer: response ProjectAddResult
alt created
Renderer->>Renderer: create thread for project
else exists
Renderer->>Renderer: focus most recent thread
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 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)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
t3dotgg
changed the title
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| app.whenReady().then(() => { | ||
| void bootstrap(); | ||
|
|
||
| app.on("activate", () => { |
There was a problem hiding this comment.
🟢 Low
src/main.ts:204 The activate handler can fire while bootstrap is still awaiting, creating a window before backendWsUrl is set and then bootstrap creates another. Consider moving the activate listener inside bootstrap after initialization completes, or adding a guard flag.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/src/main.ts around line 204:
The `activate` handler can fire while `bootstrap` is still awaiting, creating a window before `backendWsUrl` is set and then `bootstrap` creates another. Consider moving the `activate` listener inside `bootstrap` after initialization completes, or adding a guard flag.
| ); | ||
| if (!child.killed) { | ||
| child.kill("SIGTERM"); | ||
| setTimeout(() => { |
There was a problem hiding this comment.
🟢 Low
src/main.ts:120 The SIGKILL fallback timer won't fire because the main process exits before 2 seconds elapse. Consider waiting for the backend to actually terminate before allowing quit (e.g., using event.preventDefault() in before-quit and calling app.quit() after confirmation), or synchronously killing with SIGKILL if graceful shutdown is not critical.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/src/main.ts around line 120:
The `SIGKILL` fallback timer won't fire because the main process exits before 2 seconds elapse. Consider waiting for the backend to actually terminate before allowing quit (e.g., using `event.preventDefault()` in `before-quit` and calling `app.quit()` after confirmation), or synchronously killing with `SIGKILL` if graceful shutdown is not critical.
| }; | ||
| const PICK_FOLDER_CHANNEL = "desktop:pick-folder"; | ||
| const ROOT_DIR = path.resolve(__dirname, "../../.."); | ||
| const BACKEND_ENTRY = path.join(ROOT_DIR, "apps/server/dist/index.js"); |
There was a problem hiding this comment.
🟡 Medium
src/main.ts:16 Spawning process.execPath without ELECTRON_RUN_AS_NODE=1 will launch another Electron instance in production (can recurse). Suggest adding ELECTRON_RUN_AS_NODE: "1" in the backend env (e.g., via backendEnv()) when starting the backend.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/src/main.ts around line 16:
Spawning `process.execPath` without `ELECTRON_RUN_AS_NODE=1` will launch another Electron instance in production (can recurse). Suggest adding `ELECTRON_RUN_AS_NODE: "1"` in the backend env (e.g., via `backendEnv()`) when starting the backend.
| if (isQuitting || restartTimer) return; | ||
|
|
||
| setupEventForwarding(window); | ||
| const delayMs = Math.min(500 * 2 ** restartAttempt, 10_000); |
There was a problem hiding this comment.
🟡 Medium
src/main.ts:62 Resetting restartAttempt on spawn defeats backoff for quick crash loops. Suggest resetting only after a minimum healthy uptime (e.g., 10–30s).
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/src/main.ts around line 62:
Resetting `restartAttempt` on `spawn` defeats backoff for quick crash loops. Suggest resetting only after a minimum healthy uptime (e.g., 10–30s).
| }); | ||
|
|
||
| app.whenReady().then(() => { | ||
| void bootstrap(); |
There was a problem hiding this comment.
🟠 High
src/main.ts:202 If bootstrap() rejects, the app remains running with no window. Consider adding .catch() to handle the error and quit gracefully.
| void bootstrap(); | |
| void bootstrap().catch((err) => { | |
| console.error("[desktop] bootstrap failed:", err); | |
| app.quit(); | |
| }); |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/src/main.ts around line 202:
If `bootstrap()` rejects, the app remains running with no window. Consider adding `.catch()` to handle the error and quit gracefully.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@AGENTS.md`:
- Line 1: The document header "# CLAUDE.md" does not match the filename
AGENTS.md; either rename the file to CLAUDE.md or change the top-level Markdown
header to "# AGENTS.md" to keep the current filename; locate the header string
"# CLAUDE.md" and replace it with "# AGENTS.md" if you choose to keep the
filename, or rename the file to "CLAUDE.md" to match the existing header.
In `@apps/desktop/dist-electron/preload.js`:
- Around line 1-11: The build output folder dist-electron/ is committed; update
repository ignores and CI: add "dist-electron/" to apps/desktop/.gitignore,
remove any committed files from that directory (git rm --cached) so only source
(src/preload.ts) remains tracked, and ensure the tsup build step runs in
CI/packaging so artifacts (which generate preload.js exposing desktopBridge,
getWsUrl, PICK_FOLDER_CHANNEL and wsUrl) are produced during CI and not checked
in; verify that the preload build is produced by the existing tsup task in
package.json and included in the release artifact rather than source control.
In `@apps/desktop/src/main.ts`:
- Around line 185-209: The bootstrap() call is unhandled and may reject; wrap
the bootstrap invocation so any errors from reserveLoopbackPort(),
startBackend(), createWindow(), etc. are caught, show the error to the user
(e.g., via Electron's dialog.showErrorBox or dialog.showMessageBox with an error
message including error.message/stack), ensure you stop/cleanup the backend by
calling stopBackend() if it started, and then quit the app cleanly (app.quit or
app.exit) to avoid an unhandled rejection crash; update the
app.whenReady().then(...) invocation to call bootstrap().catch(...) and perform
the dialog + cleanup + quit sequence.
In `@apps/server/package.json`:
- Line 19: The package.json exposes a "test" script that runs "vitest run" but
vitest is not declared in this package's devDependencies; add "vitest" (with an
appropriate version) to the apps/server package.json devDependencies so the
script works when this package is used in isolation, updating the
devDependencies section in package.json to include vitest and then run
npm/yarn/pnpm install to refresh node_modules.
In `@apps/server/src/index.ts`:
- Around line 150-158: The shutdown sequence currently calls server.stop()
synchronously and immediately exits, which aborts async server close operations;
update the Server.stop implementation to return a Promise that resolves after
httpServer.close() and wss.close() complete (wrap their callback-style closes in
Promises and await both), then make the shutdown handler async (function
shutdown() { ... }) and await server.stop() before calling process.exit(0)
(i.e., await server.stop(); process.exit(0)); update any call sites accordingly
so graceful shutdown waits for HTTP and WebSocket cleanup to finish.
In `@README.md`:
- Around line 12-28: The fenced diagram block starting with ``` and containing
"Browser (React + Vite)" should include a language identifier (e.g., text or
plaintext) after the opening backticks to satisfy markdown linters and ensure
consistent rendering; update the opening fence from ``` to ```text (or
```plaintext) while leaving the ASCII diagram content (the box drawing and
labels like "Browser (React + Vite)", "apps/server (Node.js)", "codex
app-server") unchanged.
🧹 Nitpick comments (13)
apps/desktop/scripts/dev-electron.mjs (1)
8-14: Consider using an absolute path for robustness.The relative path
file:../server/dist/index.jscurrently works when invoked viabun run dev:electronfromapps/desktop/, but it's implicitly tied to the execution context. Using an absolute path viaimport.meta.urlwould eliminate this fragility and ensure the file path resolves regardless of how the script is invoked.Suggested improvement (optional)
import { spawn } from "node:child_process"; +import { resolve, dirname } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; import waitOn from "wait-on"; +const __dirname = dirname(fileURLToPath(import.meta.url)); +const serverDistIndex = resolve(__dirname, "..", "..", "server", "dist", "index.js"); + const port = Number(process.env.ELECTRON_RENDERER_PORT ?? 5173); const devServerUrl = `http://localhost:${port}`; await waitOn({ resources: [ `tcp:${port}`, "file:dist-electron/main.js", "file:dist-electron/preload.js", - "file:../server/dist/index.js", + pathToFileURL(serverDistIndex).href, ], });apps/server/src/fixPath.ts (1)
1-18: Consider extracting to a shared package to avoid duplication.This implementation is nearly identical to
apps/desktop/src/fixPath.ts. Both files contain the same macOS PATH-fix logic usingexecFileSyncwith the same shell invocation pattern.Consider extracting this utility to a shared package (e.g.,
packages/utils) to maintain a single source of truth and reduce maintenance burden.apps/server/scripts/bundle-client.mjs (1)
19-21: Consider cleaning the target directory before copying.If
dist/clientalready exists from a previous build,fs.cpSyncwill overwrite matching files but may leave orphaned files from prior builds. Consider removing the target directory first for a clean slate.♻️ Proposed fix
if (!fs.existsSync(rendererDist)) { console.log( "⚠ Renderer dist not found — skipping client bundle. Run `bun run --cwd apps/renderer build` first.", ); process.exit(0); } +if (fs.existsSync(target)) { + fs.rmSync(target, { recursive: true }); +} fs.cpSync(rendererDist, target, { recursive: true }); console.log("✓ Bundled renderer into dist/client");apps/renderer/src/wsNativeApi.ts (1)
38-48: Consider validating the welcome payload before caching.The
datareceived from the WebSocket is cast directly toWsWelcomePayloadwithout validation. If the server sends malformed data, this could propagate invalid state to all listeners.♻️ Suggested validation
+import { wsWelcomePayloadSchema } from "@acme/contracts"; + transport.subscribe(WS_CHANNELS.serverWelcome, (data) => { - const payload = data as WsWelcomePayload; + const parsed = wsWelcomePayloadSchema.safeParse(data); + if (!parsed.success) { + console.warn("[wsNativeApi] Invalid welcome payload:", parsed.error); + return; + } + const payload = parsed.data; lastWelcome = payload;apps/server/src/wsServer.test.ts (2)
57-69: Potential test hang if expected response never arrives.The
while (true)loop insendRequestwill hang indefinitely if the server never sends a response with the matchingid. Consider adding a timeout to fail the test gracefully.♻️ Add timeout to prevent test hangs
async function sendRequest(ws: WebSocket, method: string, params?: unknown): Promise<WsResponse> { const id = crypto.randomUUID(); const message = JSON.stringify({ id, method, ...(params !== undefined ? { params } : {}) }); ws.send(message); + const deadline = Date.now() + 5000; // 5 second timeout // Wait for response with matching id while (true) { + if (Date.now() > deadline) { + throw new Error(`Timeout waiting for response to ${method}`); + } const parsed = (await waitForMessage(ws)) as Record<string, unknown>; if (parsed.id === id) { return parsed as WsResponse; } } }
26-34: Missing error handling for malformed WebSocket messages.
JSON.parse(String(raw))will throw if the message is not valid JSON, which could cause unexpected test failures. While this may be acceptable in tests, consider wrapping in try-catch for robustness.♻️ Add error handling
ws.on("message", (raw) => { - const parsed = JSON.parse(String(raw)); + let parsed: unknown; + try { + parsed = JSON.parse(String(raw)); + } catch { + console.warn("Test received non-JSON message:", String(raw)); + return; + } const waiter = pending.waiters.shift();apps/renderer/src/App.tsx (1)
31-91: Stale closure may cause missed project detection.The
onServerWelcomecallback on line 40-87 capturesstate.projects,state.threads, andstate.activeThreadIdat subscription time. If projects are added through other means before the welcome message arrives, this callback won't see them.However, the
bootstrappedRefguard and the fact that welcome typically arrives before any user interaction likely make this a non-issue in practice.♻️ Alternative: use refs for latest state
function AutoProjectBootstrap() { const { state, dispatch } = useStore(); const bootstrappedRef = useRef(false); + const stateRef = useRef(state); + stateRef.current = state; useEffect(() => { if (isElectron) return; return onServerWelcome((payload) => { if (bootstrappedRef.current) return; + const currentState = stateRef.current; // Don't create duplicate projects for the same cwd - const existing = state.projects.find((p) => p.cwd === payload.cwd); + const existing = currentState.projects.find((p) => p.cwd === payload.cwd); // ... rest of the callback using currentState }); - }, [state.projects, state.threads, state.activeThreadId, dispatch]); + }, [dispatch]);apps/server/src/index.ts (1)
52-72: TOCTOU race condition in port availability check.Between
server.close()returning and the actual server binding to the port later, another process could claim it. This is a known limitation of port probing, but worth noting.For the fallback case (lines 60-69), consider binding the actual server to port 0 directly rather than probing then releasing.
apps/desktop/dist-electron/main.js (1)
1-226: dist-electron is a generated artifact that is intentionally tracked by this project.The file is bundled by tsup (configured in
tsup.config.ts) and outputs todist-electron/main.js. While the root.gitignorepattern ignoresapps/*/dist, it deliberately excludesdist-electron, suggesting the project has decided to commit this artifact for CI/deployment purposes.If the goal is to keep the repository lighter and generate this during CI instead, consider:
- Adding
apps/desktop/dist-electron/to.gitignore- Ensuring the build step runs in CI before deployment
However, note that this appears to be an intentional project pattern—review with the team whether committing electron bundles aligns with your deployment strategy.
apps/renderer/src/components/Sidebar.tsx (1)
101-112: Simplify duplicate detection logic.The
existingByIdcheck on line 101 is never used in the subsequent logic. Sinceapi.projects.addalready handles deduplication on the server side (returningcreated: falsefor existing projects), you only need to sync the local state. Consider simplifying:♻️ Proposed simplification
- const existingById = state.projects.find((p) => p.id === project.id); const existingByCwd = state.projects.find((p) => p.cwd === project.cwd); - if (!existingById && !existingByCwd) { + if (!existingByCwd) { dispatch({ type: "ADD_PROJECT", project }); } const resolvedProjectId = existingByCwd?.id ?? project.id;apps/server/src/wsServer.ts (2)
174-189: Consider timing-safe token comparison.The token comparison on line 185 uses
!==which is not constant-time. While this is likely acceptable for a local/desktop application context, for defense-in-depth you could usecrypto.timingSafeEqual:🔒 Optional: Use timing-safe comparison
+import { timingSafeEqual } from "node:crypto"; + +function safeTokenCompare(a: string, b: string): boolean { + if (a.length !== b.length) return false; + return timingSafeEqual(Buffer.from(a), Buffer.from(b)); +} + // In the upgrade handler: - if (providedToken !== authToken) { + if (!providedToken || !safeTokenCompare(providedToken, authToken)) {
284-305: ValidateshellOpenInEditorparams with schema.The params are type-asserted without validation. While
EDITORSlookup provides some safety, consider validating the input structure to avoid runtime errors if malformed params are sent:🛡️ Proposed validation
case WS_METHODS.shellOpenInEditor: { - const params = request.params as { - cwd: string; - editor: string; - }; - if (!params?.cwd) throw new Error("cwd is required"); + const params = request.params as Record<string, unknown> | undefined; + if (!params || typeof params.cwd !== "string" || typeof params.editor !== "string") { + throw new Error("cwd and editor are required strings"); + } + const { cwd: targetCwd, editor } = params; const editorDef = EDITORS.find((e) => e.id === params.editor); if (!editorDef) throw new Error(`Unknown editor: ${params.editor}`);apps/renderer/src/wsTransport.ts (1)
159-182: Polling-based send queue could be more efficient.The current implementation creates a new polling interval for each message sent while disconnected. Consider using a proper message queue that drains on connection:
♻️ Optional: Queue-based approach
private readonly sendQueue: WsRequest[] = []; private send(message: WsRequest) { if (this.ws?.readyState === WebSocket.OPEN) { this.ws.send(JSON.stringify(message)); return; } this.sendQueue.push(message); } // In the "open" event handler: ws.addEventListener("open", () => { this.ws = ws; this.reconnectAttempt = 0; // Drain queued messages while (this.sendQueue.length > 0 && this.ws?.readyState === WebSocket.OPEN) { const msg = this.sendQueue.shift()!; this.ws.send(JSON.stringify(msg)); } });
| @@ -1,7 +1,7 @@ | |||
| # AGENTS.md | |||
| # CLAUDE.md | |||
There was a problem hiding this comment.
Fix header/filename mismatch.
The file header says # CLAUDE.md but the filename is AGENTS.md. This inconsistency should be resolved—either rename the file to CLAUDE.md or update the header to # AGENTS.md.
Proposed fix (if keeping filename as AGENTS.md)
-# CLAUDE.md
+# AGENTS.md📝 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.
| # CLAUDE.md | |
| # AGENTS.md |
🤖 Prompt for AI Agents
In `@AGENTS.md` at line 1, The document header "# CLAUDE.md" does not match the
filename AGENTS.md; either rename the file to CLAUDE.md or change the top-level
Markdown header to "# AGENTS.md" to keep the current filename; locate the header
string "# CLAUDE.md" and replace it with "# AGENTS.md" if you choose to keep the
filename, or rename the file to "CLAUDE.md" to match the existing header.
| "use strict"; | ||
|
|
||
| // src/preload.ts | ||
| var import_electron = require("electron"); | ||
| var PICK_FOLDER_CHANNEL = "desktop:pick-folder"; | ||
| var wsUrl = process.env.CODETHING_DESKTOP_WS_URL ?? null; | ||
| import_electron.contextBridge.exposeInMainWorld("desktopBridge", { | ||
| getWsUrl: () => wsUrl, | ||
| pickFolder: () => import_electron.ipcRenderer.invoke(PICK_FOLDER_CHANNEL) | ||
| }); | ||
| //# sourceMappingURL=preload.js.map No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's a corresponding source file and if dist-electron is in gitignore
echo "=== Checking for source file ==="
fd -t f "preload.ts" apps/desktop
echo "=== Checking .gitignore for dist-electron ==="
if [ -f ".gitignore" ]; then
rg -n "dist-electron" .gitignore || echo "No gitignore entries found for dist-electron in root .gitignore"
else
echo "No root .gitignore"
fi
if [ -f "apps/desktop/.gitignore" ]; then
rg -n "dist-electron" apps/desktop/.gitignore || echo "No gitignore entries found for dist-electron in apps/desktop/.gitignore"
else
echo "No apps/desktop/.gitignore"
fi
echo "=== Checking package.json build scripts ==="
if [ -f "apps/desktop/package.json" ]; then
cat apps/desktop/package.json | head -100
else
echo "No package.json found in apps/desktop"
fiRepository: pingdotgg/codething-mvp
Length of output: 1157
dist-electron is a build output directory that should be gitignored.
The source file apps/desktop/src/preload.ts exists and is compiled via tsup into dist-electron/. However, this directory is not in .gitignore, meaning built artifacts are being committed to version control. This leads to:
- Git merge conflicts when builds change
- Stale builds if source changes without rebuilding
- Harder code review of transpiled vs. source code
Add dist-electron/ to apps/desktop/.gitignore and ensure the build step (tsup) runs as part of CI/packaging.
🤖 Prompt for AI Agents
In `@apps/desktop/dist-electron/preload.js` around lines 1 - 11, The build output
folder dist-electron/ is committed; update repository ignores and CI: add
"dist-electron/" to apps/desktop/.gitignore, remove any committed files from
that directory (git rm --cached) so only source (src/preload.ts) remains
tracked, and ensure the tsup build step runs in CI/packaging so artifacts (which
generate preload.js exposing desktopBridge, getWsUrl, PICK_FOLDER_CHANNEL and
wsUrl) are produced during CI and not checked in; verify that the preload build
is produced by the existing tsup task in package.json and included in the
release artifact rather than source control.
| async function bootstrap(): Promise<void> { | ||
| setupCSP(); | ||
|
|
||
| todoStore = new TodoStore(path.join(app.getPath("userData"), "todos.json")); | ||
| await todoStore.init(); | ||
| backendPort = await reserveLoopbackPort(); | ||
| backendAuthToken = randomBytes(24).toString("hex"); | ||
| backendWsUrl = `ws://127.0.0.1:${backendPort}/?token=${encodeURIComponent(backendAuthToken)}`; | ||
| process.env.CODETHING_DESKTOP_WS_URL = backendWsUrl; | ||
|
|
||
| registerIpcHandlers(); | ||
| createWindow(); | ||
|
|
||
| app.on("activate", () => { | ||
| if (BrowserWindow.getAllWindows().length === 0) { | ||
| createWindow(); | ||
| } | ||
| }); | ||
| startBackend(); | ||
| mainWindow = createWindow(); | ||
| } | ||
|
|
||
| app.on("before-quit", () => { | ||
| processManager.killAll(); | ||
| providerManager.stopAll(); | ||
| providerManager.dispose(); | ||
| isQuitting = true; | ||
| stopBackend(); | ||
| }); | ||
|
|
||
| app.whenReady().then(() => { | ||
| void bootstrap(); | ||
|
|
||
| app.on("activate", () => { | ||
| if (BrowserWindow.getAllWindows().length === 0) { | ||
| mainWindow = createWindow(); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the file exists and examine the relevant section
cd apps/desktop && head -n 220 src/main.ts | tail -n 45Repository: pingdotgg/codething-mvp
Length of output: 1041
🏁 Script executed:
# Let's also check the bootstrap function implementation to see what can fail
cd apps/desktop && rg -n "async function bootstrap" -A 30 src/main.tsRepository: pingdotgg/codething-mvp
Length of output: 920
🏁 Script executed:
# Check for other error handling patterns with .catch or try-catch in the codebase
cd apps/desktop && rg -n "\.catch\(" src/main.ts | head -20Repository: pingdotgg/codething-mvp
Length of output: 49
🏁 Script executed:
# Check if there's a pattern of error handling with dialog.showErrorBox in the codebase
cd apps/desktop && rg -n "dialog\.showErrorBox" src/main.tsRepository: pingdotgg/codething-mvp
Length of output: 49
🏁 Script executed:
# Check imports at the top of the file to see if dialog is available
cd apps/desktop && head -n 50 src/main.tsRepository: pingdotgg/codething-mvp
Length of output: 1656
🏁 Script executed:
# Check if there are other app lifecycle error handlers
cd apps/desktop && rg -n "app\.on\(" src/main.tsRepository: pingdotgg/codething-mvp
Length of output: 173
🏁 Script executed:
# Look for error handling in the startBackend or reserveLoopbackPort functions
cd apps/desktop && rg -n "function startBackend|function reserveLoopbackPort|const startBackend|const reserveLoopbackPort" -A 10 src/main.tsRepository: pingdotgg/codething-mvp
Length of output: 849
Add error handling to bootstrap() to prevent app crashes from unhandled promise rejections.
Line 202 calls bootstrap() without a catch handler. Since bootstrap() is async and can fail (e.g., at port reservation in line 186 or window creation in line 193), unhandled rejections will crash the app. Catch errors, display them to the user via error dialog, and quit cleanly.
🛠️ Suggested fix
app.whenReady().then(() => {
- void bootstrap();
+ void bootstrap().catch((error) => {
+ console.error("[desktop] bootstrap failed", error);
+ dialog.showErrorBox(
+ "CodeThing failed to start",
+ error instanceof Error ? error.message : String(error),
+ );
+ app.quit();
+ });
app.on("activate", () => {
if (BrowserWindow.getAllWindows().length === 0) {
mainWindow = createWindow();
}
});
});🤖 Prompt for AI Agents
In `@apps/desktop/src/main.ts` around lines 185 - 209, The bootstrap() call is
unhandled and may reject; wrap the bootstrap invocation so any errors from
reserveLoopbackPort(), startBackend(), createWindow(), etc. are caught, show the
error to the user (e.g., via Electron's dialog.showErrorBox or
dialog.showMessageBox with an error message including error.message/stack),
ensure you stop/cleanup the backend by calling stopBackend() if it started, and
then quit the app cleanly (app.quit or app.exit) to avoid an unhandled rejection
crash; update the app.whenReady().then(...) invocation to call
bootstrap().catch(...) and perform the dialog + cleanup + quit sequence.
| "start": "node dist/index.js", | ||
| "start:desktop": "CODETHING_MODE=desktop CODETHING_NO_BROWSER=1 node dist/index.js", | ||
| "typecheck": "tsc --noEmit", | ||
| "test": "vitest run" |
There was a problem hiding this comment.
Missing vitest devDependency for the test script.
The test script uses vitest run, but vitest is not listed in devDependencies. While it exists at the root workspace level, explicitly declaring it here ensures the dependency is clear and avoids potential issues if the package is run in isolation.
Proposed fix
"devDependencies": {
"@acme/contracts": "workspace:*",
"@acme/renderer": "workspace:*",
"@types/node": "^22.10.2",
"@types/ws": "^8.5.13",
"tsup": "^8.3.5",
"tsx": "^4.19.0",
- "typescript": "^5.7.3"
+ "typescript": "^5.7.3",
+ "vitest": "^3.0.0"
}🤖 Prompt for AI Agents
In `@apps/server/package.json` at line 19, The package.json exposes a "test"
script that runs "vitest run" but vitest is not declared in this package's
devDependencies; add "vitest" (with an appropriate version) to the apps/server
package.json devDependencies so the script works when this package is used in
isolation, updating the devDependencies section in package.json to include
vitest and then run npm/yarn/pnpm install to refresh node_modules.
| // Graceful shutdown | ||
| const shutdown = () => { | ||
| logger.info("shutting down"); | ||
| server.stop(); | ||
| process.exit(0); | ||
| }; | ||
|
|
||
| process.on("SIGINT", shutdown); | ||
| process.on("SIGTERM", shutdown); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and find the server definition
head -160 apps/server/src/index.ts | tail -30Repository: pingdotgg/codething-mvp
Length of output: 692
🏁 Script executed:
# Search for where 'server' is defined/initialized
rg "const server|let server|var server" apps/server/src/index.ts -A 3Repository: pingdotgg/codething-mvp
Length of output: 306
🏁 Script executed:
# Find the stop() method implementation in the codebase
ast-grep --pattern 'stop() {
$$$
}'Repository: pingdotgg/codething-mvp
Length of output: 49
🏁 Script executed:
# Also search for server initialization to understand what type of server it is
rg "createServer|new.*Server|server\s*=" apps/server/src/index.ts -B 2 -A 2Repository: pingdotgg/codething-mvp
Length of output: 763
🏁 Script executed:
# Find the wsServer file
fd "wsServer" apps/server/src/Repository: pingdotgg/codething-mvp
Length of output: 127
🏁 Script executed:
# Check the createServer function and stop() method in wsServer
cat apps/server/src/wsServer.tsRepository: pingdotgg/codething-mvp
Length of output: 10130
Shutdown will interrupt async cleanup operations.
The server.stop() method calls httpServer.close() and wss.close() (WebSocketServer from the 'ws' library), both of which are asynchronous operations. The current code calls them synchronously without waiting for their callbacks, then immediately exits with process.exit(0), terminating before the HTTP and WebSocket servers finish closing.
The suggested fix assumes server.stop() returns a Promise, but it currently returns void. Instead, modify stop() to return a Promise that waits for all async cleanup to complete:
♻️ Properly await async cleanup
- function stop() {
+ function stop(): Promise<void> {
+ return new Promise<void>((resolve) => {
providerManager.stopAll();
providerManager.dispose();
for (const client of clients) {
client.close();
}
clients.clear();
wss.close(() => {
httpServer.close(() => {
resolve();
});
});
- }
+ });
+ }Then in index.ts:
- const shutdown = () => {
+ const shutdown = async () => {
logger.info("shutting down");
- server.stop();
+ await server.stop();
process.exit(0);
};
- process.on("SIGINT", shutdown);
- process.on("SIGTERM", shutdown);
+ process.on("SIGINT", () => void shutdown());
+ process.on("SIGTERM", () => void shutdown());🤖 Prompt for AI Agents
In `@apps/server/src/index.ts` around lines 150 - 158, The shutdown sequence
currently calls server.stop() synchronously and immediately exits, which aborts
async server close operations; update the Server.stop implementation to return a
Promise that resolves after httpServer.close() and wss.close() complete (wrap
their callback-style closes in Promises and await both), then make the shutdown
handler async (function shutdown() { ... }) and await server.stop() before
calling process.exit(0) (i.e., await server.stop(); process.exit(0)); update any
call sites accordingly so graceful shutdown waits for HTTP and WebSocket cleanup
to finish.
| ``` | ||
| ┌─────────────────────────────────┐ | ||
| │ Browser (React + Vite) │ | ||
| │ Connected via WebSocket │ | ||
| └──────────┬──────────────────────┘ | ||
| │ ws://localhost:3773 | ||
| ┌──────────▼──────────────────────┐ | ||
| │ apps/server (Node.js) │ | ||
| │ WebSocket + HTTP static server │ | ||
| │ ProviderManager │ | ||
| │ CodexAppServerManager │ | ||
| └──────────┬──────────────────────┘ | ||
| │ JSON-RPC over stdio | ||
| ┌──────────▼──────────────────────┐ | ||
| │ codex app-server │ | ||
| └─────────────────────────────────┘ | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced code block.
The architecture diagram lacks a language specifier. Adding text or plaintext satisfies markdown linters and improves rendering consistency.
Proposed fix
-```
+```text
┌─────────────────────────────────┐
│ Browser (React + Vite) │📝 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.
| ``` | |
| ┌─────────────────────────────────┐ | |
| │ Browser (React + Vite) │ | |
| │ Connected via WebSocket │ | |
| └──────────┬──────────────────────┘ | |
| │ ws://localhost:3773 | |
| ┌──────────▼──────────────────────┐ | |
| │ apps/server (Node.js) │ | |
| │ WebSocket + HTTP static server │ | |
| │ ProviderManager │ | |
| │ CodexAppServerManager │ | |
| └──────────┬──────────────────────┘ | |
| │ JSON-RPC over stdio | |
| ┌──────────▼──────────────────────┐ | |
| │ codex app-server │ | |
| └─────────────────────────────────┘ | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 12-12: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@README.md` around lines 12 - 28, The fenced diagram block starting with ```
and containing "Browser (React + Vite)" should include a language identifier
(e.g., text or plaintext) after the opening backticks to satisfy markdown
linters and ensure consistent rendering; update the opening fence from ``` to
```text (or ```plaintext) while leaving the ASCII diagram content (the box
drawing and labels like "Browser (React + Vite)", "apps/server (Node.js)",
"codex app-server") unchanged.
Reintroduce Electron by launching a separate Node server with WebSocket RPC and project registry, and route the desktop renderer through this backendAdd a new server package with HTTP+WebSocket ( 📍Where to StartStart manual pipeline review at the server entry in index.ts, then follow server creation in wsServer.ts and the desktop bootstrap in main.ts. Macroscope summarized 67636da. |
| if (typeof data !== "string") { | ||
| throw new Error("agent:write data must be a string"); | ||
| } | ||
| fixPath(); |
There was a problem hiding this comment.
🟢 Low
src/main.ts:12 Using -i (interactive) flag may cause shell config output (banners, motd) to be captured alongside $PATH, corrupting it. Consider using -l (login) without -i, or filtering the output.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/src/main.ts around line 12:
Using `-i` (interactive) flag may cause shell config output (banners, motd) to be captured alongside `$PATH`, corrupting it. Consider using `-l` (login) without `-i`, or filtering the output.
| ? bridgeUrl | ||
| : envUrl && envUrl.length > 0 | ||
| ? envUrl | ||
| : `ws://${window.location.hostname}:${window.location.port}`); |
There was a problem hiding this comment.
🟡 Medium
src/wsTransport.ts:35 Default URL building isn’t robust: it always uses ws:// and appends an empty :port on default ports, causing mixed content or invalid URLs. Suggest deriving protocol (ws/wss) from window.location.protocol and omitting the port when window.location.port is empty.
| : `ws://${window.location.hostname}:${window.location.port}`); | |
| : `${window.location.protocol === 'https:' ? 'wss' : 'ws'}://${window.location.hostname}:${window.location.port}`); |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/renderer/src/wsTransport.ts around line 35:
Default URL building isn’t robust: it always uses `ws://` and appends an empty `:port` on default ports, causing mixed content or invalid URLs. Suggest deriving protocol (`ws`/`wss`) from `window.location.protocol` and omitting the port when `window.location.port` is empty.
| fixPath(); | ||
|
|
||
| const DEFAULT_PORT = 3773; | ||
| const cwd = process.cwd(); |
There was a problem hiding this comment.
🟢 Low
src/index.ts:15 Consider moving process.cwd() inside main() so exceptions (e.g., ENOENT if the directory was deleted) are caught by the existing error handler.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/server/src/index.ts around line 15:
Consider moving `process.cwd()` inside `main()` so exceptions (e.g., `ENOENT` if the directory was deleted) are caught by the existing error handler.
| import { ProjectRegistry } from "./projectRegistry"; | ||
| import { createServer } from "./wsServer"; | ||
|
|
||
| fixPath(); |
There was a problem hiding this comment.
🟡 Medium
src/index.ts:12 The -i flag causes interactive shells to source profile scripts that may print to stdout (banners, messages), corrupting PATH. Consider using -l alone instead of -il to get a login shell without interactive mode.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/server/src/index.ts around line 12:
The `-i` flag causes interactive shells to source profile scripts that may print to stdout (banners, messages), corrupting `PATH`. Consider using `-l` alone instead of `-il` to get a login shell without interactive mode.
| return path.resolve(expandHomePath(raw.trim())); | ||
| } | ||
|
|
||
| async function findAvailablePort(preferred: number): Promise<number> { |
There was a problem hiding this comment.
🟡 Medium
src/index.ts:52 This port-finding approach has a TOCTOU race: between closing the test server and starting the real one, another process could claim the port. Consider passing the server socket directly to createServer instead of closing it, or documenting this as an acceptable tradeoff for simplicity.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/server/src/index.ts around line 52:
This port-finding approach has a TOCTOU race: between closing the test server and starting the real one, another process could claim the port. Consider passing the server socket directly to `createServer` instead of closing it, or documenting this as an acceptable tradeoff for simplicity.
|
@greptileai review |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| try { | ||
| const result = await routeRequest(request); | ||
| const response: WsResponse = { id: request.id, result }; | ||
| ws.send(JSON.stringify(response)); | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : "Unknown server error"; | ||
| const response: WsResponse = { | ||
| id: request.id, | ||
| error: { message }, | ||
| }; | ||
| ws.send(JSON.stringify(response)); | ||
| } |
There was a problem hiding this comment.
🔴 Unhandled exception in handleMessage catch block when WebSocket closes during async request
The ws.send() call inside the catch block of handleMessage has no error protection. If the WebSocket connection closes while routeRequest() is being awaited (e.g. during a slow provider session start), the first ws.send() at line 241 throws (caught by catch), then the catch-block's ws.send() at line 248 also throws — and this second throw propagates as an unhandled promise rejection because the caller uses void handleMessage(ws, raw) at line 212.
Root Cause and Impact
The ws library's send() method throws synchronously when called on a non-OPEN WebSocket without a callback. The code path:
- Client sends a request, then disconnects.
routeRequest(request)completes (or throws).ws.send(JSON.stringify(response))throwsError: WebSocket is not open→ caught bycatch.ws.send(JSON.stringify(response))in the catch block at line 248 also throws → unhandled.
Since handleMessage is invoked with void (apps/server/src/wsServer.ts:212), the returned promise rejection is unhandled. In Node.js 15+, unhandled rejections terminate the process by default, which would crash the entire server and disconnect all clients.
| try { | |
| const result = await routeRequest(request); | |
| const response: WsResponse = { id: request.id, result }; | |
| ws.send(JSON.stringify(response)); | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : "Unknown server error"; | |
| const response: WsResponse = { | |
| id: request.id, | |
| error: { message }, | |
| }; | |
| ws.send(JSON.stringify(response)); | |
| } | |
| try { | |
| const result = await routeRequest(request); | |
| const response: WsResponse = { id: request.id, result }; | |
| if (ws.readyState === ws.OPEN) ws.send(JSON.stringify(response)); | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : "Unknown server error"; | |
| const response: WsResponse = { | |
| id: request.id, | |
| error: { message }, | |
| }; | |
| if (ws.readyState === ws.OPEN) ws.send(JSON.stringify(response)); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| ? bridgeUrl | ||
| : envUrl && envUrl.length > 0 | ||
| ? envUrl | ||
| : `ws://${window.location.hostname}:${window.location.port}`); |
There was a problem hiding this comment.
🟡 WebSocket URL fallback produces invalid URL when page is served on default HTTP port
When neither desktopBridge nor VITE_WS_URL provides a URL, the fallback constructs ws://${window.location.hostname}:${window.location.port}. If the page is served on the default port (80 for HTTP, 443 for HTTPS), window.location.port is the empty string "", producing an invalid URL like ws://example.com: which causes new WebSocket() to throw a SyntaxError.
Detailed Explanation
The fallback at apps/renderer/src/wsTransport.ts:35 is:
`ws://${window.location.hostname}:${window.location.port}`
Per the spec, window.location.port returns "" for default ports (80/443). This produces ws://hostname: which fails URL parsing and throws, preventing any WebSocket connection.
This triggers in production when the CodeThing server runs behind a reverse proxy on port 80/443. Using window.location.host (which already includes the port only when non-default) fixes the issue.
Impact: The renderer cannot connect to the WebSocket server at all when deployed behind a standard reverse proxy.
| : `ws://${window.location.hostname}:${window.location.port}`); | |
| : `ws://${window.location.host}`); |
Was this helpful? React with 👍 or 👎 to provide feedback.
Bug (related to pingdotgg#234): - codex availability checks can fail at startup because the app/server process gets an incorrect or incomplete PATH, causing startup spawn failures and false “codex not installed/on PATH” errors. - Server health probes could run before PATH repair during layer initialization. - crash loop observed: ``` @t3tools/desktop:start: [10:59:35.373] INFO (pingdotgg#10): Migrations ran successfully @t3tools/desktop:start: [10:59:35.375] ERROR (pingdotgg#10): RangeError: The value of "err" is out of range. It must be a negative integer. Received 1 @t3tools/desktop:start: at Object.getSystemErrorName (node:util:331:11) @t3tools/desktop:start: at new ErrnoException (node:internal/errors:733:23) @t3tools/desktop:start: at ChildProcess.spawn (node:internal/child_process:421:11) @t3tools/desktop:start: at Module.spawn (node:child_process:810:9) @t3tools/desktop:start: at MixedScheduler.<anonymous> (file://personal/t3code/node_modules/.bun/@effect+platform-node-shared@https+++pkg.pr.new+Effect-TS+effect-smol+@effect+platform-node-shared@8881a9b606d84a6f5eb6615279138322984f5368+eecd35a1726d7096/node_modules/@effect/platform-node-shared/dist/NodeChildProcessSpawner.js:228:37) @t3tools/desktop:start: [desktop] backend exited unexpectedly (code=1 signal=null); restarting in 500ms ``` - Error `RangeError ... getSystemErrorName ... Received 1` came from a failed `spawn()` during startup health checks, and in my case it was triggered by the broken PATH state (caused by shell/PATH normalization behavior). Cause: - PATH normalization assumed shell output shape and did not robustly sanitize interactive shell artifacts. - Non-Windows handling was inconsistent across desktop/server startup paths. - Provider health check (`codex --version`) executed before PATH was reliably normalized. Fix: - Add shared shell PATH resolver (`@t3tools/shared/shellPath`) with: - fish-aware probing - ANSI/control-sequence stripping - delimiter normalization - Use shared resolver in desktop and server `fixPath` logic. - Expand PATH repair to non-Windows startup paths (macOS + Linux). - Ensure server `fixPath` executes before provider layer startup/health probing. - Add focused tests for shell PATH normalization behavior.
Bug (related to pingdotgg#234): - codex availability checks can fail at startup because the app/server process gets an incorrect or incomplete PATH, causing startup spawn failures and false “codex not installed/on PATH” errors. - Server health probes could run before PATH repair during layer initialization. - crash loop observed: ``` @t3tools/desktop:start: [10:59:35.373] INFO (pingdotgg#10): Migrations ran successfully @t3tools/desktop:start: [10:59:35.375] ERROR (pingdotgg#10): RangeError: The value of "err" is out of range. It must be a negative integer. Received 1 @t3tools/desktop:start: at Object.getSystemErrorName (node:util:331:11) @t3tools/desktop:start: at new ErrnoException (node:internal/errors:733:23) @t3tools/desktop:start: at ChildProcess.spawn (node:internal/child_process:421:11) @t3tools/desktop:start: at Module.spawn (node:child_process:810:9) @t3tools/desktop:start: at MixedScheduler.<anonymous> (file://personal/t3code/node_modules/.bun/@effect+platform-node-shared@https+++pkg.pr.new+Effect-TS+effect-smol+@effect+platform-node-shared@8881a9b606d84a6f5eb6615279138322984f5368+eecd35a1726d7096/node_modules/@effect/platform-node-shared/dist/NodeChildProcessSpawner.js:228:37) @t3tools/desktop:start: [desktop] backend exited unexpectedly (code=1 signal=null); restarting in 500ms ``` - Error `RangeError ... getSystemErrorName ... Received 1` came from a failed `spawn()` during startup health checks, and in my case it was triggered by the broken PATH state (caused by shell/PATH normalization behavior). Cause: - PATH normalization assumed shell output shape and did not robustly sanitize interactive shell artifacts. - Non-Windows handling was inconsistent across desktop/server startup paths. - Provider health check (`codex --version`) executed before PATH was reliably normalized. Fix: - Add shared shell PATH resolver (`@t3tools/shared/shellPath`) with: - fish-aware probing - ANSI/control-sequence stripping - delimiter normalization - Use shared resolver in desktop and server `fixPath` logic. - Expand PATH repair to non-Windows startup paths (macOS + Linux). - Ensure server `fixPath` executes before provider layer startup/health probing. - Add focused tests for shell PATH normalization behavior.
* feat: scaffold apps/server package and WebSocket protocol types - Create apps/server with package.json, tsconfig, tsup config - Copy codexAppServerManager, providerManager, fixPath from desktop (no Electron deps) - Add WebSocket message protocol schemas (WsRequest, WsResponse, WsPush) to @acme/contracts - Define WS_METHODS and WS_CHANNELS constants for type-safe routing Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * feat: implement WebSocket + HTTP server and CLI entry point - wsServer.ts: WebSocket server routes NativeApi methods, serves static files - Provider events pushed to all connected clients - SPA fallback for client-side routing - Server welcome message with cwd/projectName on connect - index.ts: CLI entry point - Auto-detects available port (default 3773) - Resolves renderer dist for static serving - Opens browser on startup via 'open' package - Graceful shutdown on SIGINT/SIGTERM Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * feat: WebSocket transport and NativeApi implementation for browser - wsTransport.ts: WebSocket client with request/response, push subscriptions, auto-reconnect with exponential backoff - wsNativeApi.ts: Full NativeApi implementation over WebSocket transport - All provider methods routed through WS_METHODS - Provider events streamed via WS_CHANNELS.providerEvent - Todos/terminal/agent stubbed (not needed for web mode) - dialogs.pickFolder returns null (text input fallback in UI) - session-logic.ts: readNativeApi() now auto-detects environment - Prefers Electron preload bridge if available (window.nativeApi) - Falls back to WebSocket transport for browser mode - onServerWelcome() hook for renderer to receive cwd on connect Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * feat: adapt renderer for browser mode - App.tsx: Add AutoProjectBootstrap that listens for server welcome and auto-creates project + thread from server's cwd - Sidebar: Conditionalize drag region (Electron only), hide Browse button in browser - ChatView: Conditionalize drag region, adjust padding for browser - DiffPanel: Same drag region treatment - index.html: Update title to 'CodeThing' - env.ts: isElectron detection utility Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * feat: monorepo & CLI integration - Root package.json: Add dev, dev:server, dev:web, start scripts - dev runs both server and renderer in parallel via turbo - VITE_WS_URL injected so renderer connects to server in dev mode - turbo.json: Add WS env vars to globalEnv - vite.config.ts: Inject VITE_WS_URL at build time for WS server discovery - wsTransport.ts: Use VITE_WS_URL env var in dev, fall back to page URL in prod - Server dev script: Uses tsx for direct TypeScript execution with VITE_DEV_SERVER_URL Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * feat: remove Electron desktop app, clean up contracts and docs - Delete apps/desktop/ entirely (replaced by apps/server) - Remove IPC_CHANNELS constant from contracts (no longer needed) - Keep NativeApi interface (still the renderer's API contract) - Keep EDITORS constant (used by both server and renderer) - Update README.md with new architecture diagram and scripts - Update AGENTS.md with new package roles and architecture Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * test: add WebSocket server integration tests - Tests welcome message on connect with cwd/projectName - Tests server.getConfig method - Tests providers.listSessions returns empty array - Tests error response for unknown methods - Tests graceful handling of invalid JSON - All 73 tests pass across 12 test files Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * fix: address lint warnings in new files - wsTransport.ts: Use addEventListener instead of on-function assignments - wsServer.ts: Replace filter(Boolean).pop() with direct array access - wsServer.test.ts: Use addEventListener for WebSocket events Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * fix: race condition in auto-project bootstrap + cleanup - wsNativeApi: Cache last welcome payload and replay to late subscribers. Fixes race where WS connect + welcome arrives before React effect fires. - App.tsx: Add bootstrappedRef to prevent duplicate project creation, skip bootstrap in Electron mode - wsTransport: Use standard import.meta.env for VITE_WS_URL access - CLAUDE.md: Fix header (was incorrectly '# AGENTS.md') Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * fix: use tsx command directly in server dev script Use 'tsx src/index.ts' instead of 'node --import tsx' for simpler dev execution Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * chore: clean up stale Electron references and config - Remove smoke-test script (was desktop-only) - Remove dist-electron from turbo outputs and .gitignore - Rename ELECTRON_RENDERER_PORT → PORT in vite.config.ts - Clean up turbo globalEnv (remove ELECTRON_RENDERER_PORT, WS_SERVER_PORT) - Update CI workflow to use bun 1.3.9 (matches package.json engines) Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * feat: make t3 package publishable for npx - Remove private:true so package can be published to npm - Add files field to limit published files to dist/ - Move @acme/contracts to devDependencies (bundled by tsup) - Add @acme/renderer devDependency for turbo build ordering - Add bundle-client.mjs: copies renderer build into dist/client/ - Update resolveStaticDir to check dist/client/ first (npx scenario), then fall back to monorepo layout (../../renderer/dist) - Full build pipeline: contracts → renderer → server + bundle client Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * fix tests * turborepo and better logging * bring back electron * fix * Promise catching fixes --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
* feat: scaffold apps/server package and WebSocket protocol types - Create apps/server with package.json, tsconfig, tsup config - Copy codexAppServerManager, providerManager, fixPath from desktop (no Electron deps) - Add WebSocket message protocol schemas (WsRequest, WsResponse, WsPush) to @acme/contracts - Define WS_METHODS and WS_CHANNELS constants for type-safe routing Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * feat: implement WebSocket + HTTP server and CLI entry point - wsServer.ts: WebSocket server routes NativeApi methods, serves static files - Provider events pushed to all connected clients - SPA fallback for client-side routing - Server welcome message with cwd/projectName on connect - index.ts: CLI entry point - Auto-detects available port (default 3773) - Resolves renderer dist for static serving - Opens browser on startup via 'open' package - Graceful shutdown on SIGINT/SIGTERM Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * feat: WebSocket transport and NativeApi implementation for browser - wsTransport.ts: WebSocket client with request/response, push subscriptions, auto-reconnect with exponential backoff - wsNativeApi.ts: Full NativeApi implementation over WebSocket transport - All provider methods routed through WS_METHODS - Provider events streamed via WS_CHANNELS.providerEvent - Todos/terminal/agent stubbed (not needed for web mode) - dialogs.pickFolder returns null (text input fallback in UI) - session-logic.ts: readNativeApi() now auto-detects environment - Prefers Electron preload bridge if available (window.nativeApi) - Falls back to WebSocket transport for browser mode - onServerWelcome() hook for renderer to receive cwd on connect Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * feat: adapt renderer for browser mode - App.tsx: Add AutoProjectBootstrap that listens for server welcome and auto-creates project + thread from server's cwd - Sidebar: Conditionalize drag region (Electron only), hide Browse button in browser - ChatView: Conditionalize drag region, adjust padding for browser - DiffPanel: Same drag region treatment - index.html: Update title to 'CodeThing' - env.ts: isElectron detection utility Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * feat: monorepo & CLI integration - Root package.json: Add dev, dev:server, dev:web, start scripts - dev runs both server and renderer in parallel via turbo - VITE_WS_URL injected so renderer connects to server in dev mode - turbo.json: Add WS env vars to globalEnv - vite.config.ts: Inject VITE_WS_URL at build time for WS server discovery - wsTransport.ts: Use VITE_WS_URL env var in dev, fall back to page URL in prod - Server dev script: Uses tsx for direct TypeScript execution with VITE_DEV_SERVER_URL Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * feat: remove Electron desktop app, clean up contracts and docs - Delete apps/desktop/ entirely (replaced by apps/server) - Remove IPC_CHANNELS constant from contracts (no longer needed) - Keep NativeApi interface (still the renderer's API contract) - Keep EDITORS constant (used by both server and renderer) - Update README.md with new architecture diagram and scripts - Update AGENTS.md with new package roles and architecture Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * test: add WebSocket server integration tests - Tests welcome message on connect with cwd/projectName - Tests server.getConfig method - Tests providers.listSessions returns empty array - Tests error response for unknown methods - Tests graceful handling of invalid JSON - All 73 tests pass across 12 test files Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * fix: address lint warnings in new files - wsTransport.ts: Use addEventListener instead of on-function assignments - wsServer.ts: Replace filter(Boolean).pop() with direct array access - wsServer.test.ts: Use addEventListener for WebSocket events Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * fix: race condition in auto-project bootstrap + cleanup - wsNativeApi: Cache last welcome payload and replay to late subscribers. Fixes race where WS connect + welcome arrives before React effect fires. - App.tsx: Add bootstrappedRef to prevent duplicate project creation, skip bootstrap in Electron mode - wsTransport: Use standard import.meta.env for VITE_WS_URL access - CLAUDE.md: Fix header (was incorrectly '# AGENTS.md') Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * fix: use tsx command directly in server dev script Use 'tsx src/index.ts' instead of 'node --import tsx' for simpler dev execution Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * chore: clean up stale Electron references and config - Remove smoke-test script (was desktop-only) - Remove dist-electron from turbo outputs and .gitignore - Rename ELECTRON_RENDERER_PORT → PORT in vite.config.ts - Clean up turbo globalEnv (remove ELECTRON_RENDERER_PORT, WS_SERVER_PORT) - Update CI workflow to use bun 1.3.9 (matches package.json engines) Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * feat: make t3 package publishable for npx - Remove private:true so package can be published to npm - Add files field to limit published files to dist/ - Move @acme/contracts to devDependencies (bundled by tsup) - Add @acme/renderer devDependency for turbo build ordering - Add bundle-client.mjs: copies renderer build into dist/client/ - Update resolveStaticDir to check dist/client/ first (npx scenario), then fall back to monorepo layout (../../renderer/dist) - Full build pipeline: contracts → renderer → server + bundle client Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com> * fix tests * turborepo and better logging * bring back electron * fix * Promise catching fixes --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Theo Browne <t3dotgg@users.noreply.github.com>
2 participants
Summary by CodeRabbit
New Features
Improvements
Rebranding