Skip to content

feat(desktop): move server to utilityProcess#25962

Open
Brendonovich wants to merge 3 commits intodevfrom
feature/sidecar-server
Open

feat(desktop): move server to utilityProcess#25962
Brendonovich wants to merge 3 commits intodevfrom
feature/sidecar-server

Conversation

@Brendonovich
Copy link
Copy Markdown
Member

Summary

Move server initialization to a utility process (sidecar) for better isolation and resource management. This decouples the server lifecycle from the main Electron process, improving stability and startup control.

Changes

  • Created new sidecar.ts utility process for running the server
  • Refactored server.ts to spawn a utility process instead of importing server directly
  • Updated index.ts to handle sidecar lifecycle and SQLite migration progress
  • Added new build entry point for sidecar in electron.vite.config.ts
  • Added type definitions for Node.js utility process messaging in env.d.ts

Benefits

  • Better isolation: Server runs in separate process from main Electron process
  • Improved stability: Server crashes won't bring down the entire app
  • Resource management: Easier to monitor and control server resources
  • Cleaner lifecycle: Proper startup/shutdown handling with message-based communication

Move server initialization to a utility process (sidecar) for better
isolation and resource management. This decouples the server lifecycle
from the main Electron process, improving stability and startup control.
@Brendonovich Brendonovich requested a review from adamdotdevin as a code owner May 6, 2026 03:39
@Brendonovich Brendonovich changed the title feat(desktop): implement sidecar server process feat(desktop): move server to utilityProcess May 6, 2026
Copy link
Copy Markdown
Member

@Hona Hona left a comment

Choose a reason for hiding this comment

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

Reviewed the utilityProcess lifecycle against VS Code's implementation. The direction is right; these comments are about matching VS Code's timeout, env, crash, and shutdown semantics before merge.

child.stdout?.on("data", (chunk: Buffer) => options.onStdout?.(chunk.toString("utf8").trimEnd()))
child.stderr?.on("data", (chunk: Buffer) => options.onStderr?.(chunk.toString("utf8").trimEnd()))

await new Promise<void>((resolve, reject) => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This startup wait needs a timeout. As written, the desktop can hang forever if the sidecar stalls before posting ready/error (module import, sqlite migration, native binding load, Server.listen). The 30s health timeout in index.ts starts only after spawnLocalServer() resolves, so it does not cover this phase. VS Code wraps utility-process connection and ready/initialized handshakes with explicit 60s timeouts (localProcessExtensionHost.ts around its connect and handshake paths). Please race ready/error/exit with a startup timeout and kill the utility process on timeout.

})
parentPort.postMessage({ type: "ready" })
} catch (error) {
parentPort.postMessage({ type: "error", error: serializeError(error) })
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This startup error path posts the error but leaves the utility process alive. The parent rejects, but nothing guarantees this sidecar exits or gets killed. VS Code treats utility-process exit/crash as lifecycle state and cleans up process state immediately. Please process.exit(1) after posting the startup error, and have the parent kill on rejected startup as a backup.

return {
listener: {
stop: () => {
child.postMessage({ type: "stop" })
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shutdown is fire-and-forget here. Callers like relaunch/update/quit immediately continue, so this 2s force-kill timer may never fire if Electron main exits first. VS Code has waitForExit(maxWaitTimeMs) that waits for exit, then force kills, and WindowUtilityProcess wires that into window lifecycle grace time. Please make SidecarListener.stop() return a Promise<void> that waits for stopped or exit, then kills after a grace period; then await it in relaunch/update paths where possible.

Comment thread packages/desktop/src/main/server.ts Outdated
cors: ["oc://renderer"],
const child = utilityProcess.fork(join(dirname(fileURLToPath(import.meta.url)), "sidecar.js"), [], {
cwd: process.cwd(),
env: process.env,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This passes the live mutable process.env object directly. VS Code's utility-process wrapper clones env, applies explicit process metadata, removes dangerous env vars, handles platform-specific env adjustments, and stringifies every value before spawn (utilityProcess.ts:createEnv). We should create a fresh env object here, apply only the sidecar values we need, sanitize if possible, and ensure all values are strings rather than passing process.env by reference.

Comment thread packages/desktop/src/main/server.ts Outdated
needsMigration: options.needsMigration,
})
})
child.on("exit", (code: number) => options.onExit?.(code))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should also wire the utility-process crash/error path, not just a plain exit callback. VS Code registers stdout, stderr, message, spawn, exit, and app.child-process-gone filtered by service name so it can distinguish clean exit from crash/oom/launch-failed and clear process state. At minimum add child.on('error') and app.on('child-process-gone') filtering serviceName: 'opencode server' so sidecar failures are visible in logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants