Compare commits

...

6 Commits

Author SHA1 Message Date
Dax Raad
1da02fd4c6 docs(bash): clarify output capture guidance 2026-03-03 19:49:50 -05:00
Dax Raad
81f17b1f96 refactor: align shutdown naming with agent rules 2026-03-03 19:43:35 -05:00
Dax Raad
c3d840ff14 docs(agents): enforce strict naming rule for agents 2026-03-03 19:38:41 -05:00
Dax Raad
c8546dae4d refactor(mcp): remove any casts in close path 2026-03-03 19:35:36 -05:00
Dax Raad
159164ae8e fix(mcp): force-close local client subprocesses 2026-03-03 19:31:14 -05:00
Dax Raad
570038ac3c fix(tui): ensure thread worker cleanup on exit 2026-03-03 19:24:00 -05:00
4 changed files with 165 additions and 119 deletions

View File

@@ -20,6 +20,17 @@
Prefer single word names for variables and functions. Only use multiple words if necessary. Prefer single word names for variables and functions. Only use multiple words if necessary.
### Naming Enforcement (Read This)
THIS RULE IS MANDATORY FOR AGENT WRITTEN CODE.
- Use single word names by default for new locals, params, and helper functions.
- Multi-word names are allowed only when a single word would be unclear or ambiguous.
- Do not introduce new camelCase compounds when a short single-word alternative is clear.
- Before finishing edits, review touched lines and shorten newly introduced identifiers where possible.
- Good short names to prefer: `pid`, `cfg`, `err`, `opts`, `dir`, `root`, `child`, `state`, `timeout`.
- Examples to avoid unless truly required: `inputPID`, `existingClient`, `connectTimeout`, `workerPath`.
```ts ```ts
// Good // Good
const foo = 1 const foo = 1

View File

@@ -5,8 +5,8 @@ import { type rpc } from "./worker"
import path from "path" import path from "path"
import { fileURLToPath } from "url" import { fileURLToPath } from "url"
import { UI } from "@/cli/ui" import { UI } from "@/cli/ui"
import { iife } from "@/util/iife"
import { Log } from "@/util/log" import { Log } from "@/util/log"
import { withTimeout } from "@/util/timeout"
import { withNetworkOptions, resolveNetworkOptions } from "@/cli/network" import { withNetworkOptions, resolveNetworkOptions } from "@/cli/network"
import { Filesystem } from "@/util/filesystem" import { Filesystem } from "@/util/filesystem"
import type { Event } from "@opencode-ai/sdk/v2" import type { Event } from "@opencode-ai/sdk/v2"
@@ -45,6 +45,20 @@ function createEventSource(client: RpcClient): EventSource {
} }
} }
async function target() {
if (typeof OPENCODE_WORKER_PATH !== "undefined") return OPENCODE_WORKER_PATH
const dist = new URL("./cli/cmd/tui/worker.js", import.meta.url)
if (await Filesystem.exists(fileURLToPath(dist))) return dist
return new URL("./worker.ts", import.meta.url)
}
async function input(value?: string) {
const piped = process.stdin.isTTY ? undefined : await Bun.stdin.text()
if (!value) return piped
if (!piped) return value
return piped + "\n" + value
}
export const TuiThreadCommand = cmd({ export const TuiThreadCommand = cmd({
command: "$0 [project]", command: "$0 [project]",
describe: "start opencode tui", describe: "start opencode tui",
@@ -97,23 +111,17 @@ export const TuiThreadCommand = cmd({
} }
// Resolve relative paths against PWD to preserve behavior when using --cwd flag // Resolve relative paths against PWD to preserve behavior when using --cwd flag
const baseCwd = process.env.PWD ?? process.cwd() const root = process.env.PWD ?? process.cwd()
const cwd = args.project ? path.resolve(baseCwd, args.project) : process.cwd() const cwd = args.project ? path.resolve(root, args.project) : process.cwd()
const localWorker = new URL("./worker.ts", import.meta.url) const file = await target()
const distWorker = new URL("./cli/cmd/tui/worker.js", import.meta.url)
const workerPath = await iife(async () => {
if (typeof OPENCODE_WORKER_PATH !== "undefined") return OPENCODE_WORKER_PATH
if (await Filesystem.exists(fileURLToPath(distWorker))) return distWorker
return localWorker
})
try { try {
process.chdir(cwd) process.chdir(cwd)
} catch (e) { } catch {
UI.error("Failed to change directory to " + cwd) UI.error("Failed to change directory to " + cwd)
return return
} }
const worker = new Worker(workerPath, { const worker = new Worker(file, {
env: Object.fromEntries( env: Object.fromEntries(
Object.entries(process.env).filter((entry): entry is [string, string] => entry[1] !== undefined), Object.entries(process.env).filter((entry): entry is [string, string] => entry[1] !== undefined),
), ),
@@ -121,76 +129,88 @@ export const TuiThreadCommand = cmd({
worker.onerror = (e) => { worker.onerror = (e) => {
Log.Default.error(e) Log.Default.error(e)
} }
const client = Rpc.client<typeof rpc>(worker)
process.on("uncaughtException", (e) => {
Log.Default.error(e)
})
process.on("unhandledRejection", (e) => {
Log.Default.error(e)
})
process.on("SIGUSR2", async () => {
await client.call("reload", undefined)
})
const prompt = await iife(async () => { const client = Rpc.client<typeof rpc>(worker)
const piped = !process.stdin.isTTY ? await Bun.stdin.text() : undefined const error = (e: unknown) => {
if (!args.prompt) return piped Log.Default.error(e)
return piped ? piped + "\n" + args.prompt : args.prompt }
}) const reload = () => {
client.call("reload", undefined).catch((err) => {
Log.Default.warn("worker reload failed", {
error: err instanceof Error ? err.message : String(err),
})
})
}
process.on("uncaughtException", error)
process.on("unhandledRejection", error)
process.on("SIGUSR2", reload)
let stopped = false
const stop = async () => {
if (stopped) return
stopped = true
process.off("uncaughtException", error)
process.off("unhandledRejection", error)
process.off("SIGUSR2", reload)
await withTimeout(client.call("shutdown", undefined), 6000).catch((error) => {
Log.Default.warn("worker shutdown failed", {
error: error instanceof Error ? error.message : String(error),
})
})
worker.terminate()
}
const prompt = await input(args.prompt)
const config = await Instance.provide({ const config = await Instance.provide({
directory: cwd, directory: cwd,
fn: () => TuiConfig.get(), fn: () => TuiConfig.get(),
}) })
// Check if server should be started (port or hostname explicitly set in CLI or config) const network = await resolveNetworkOptions(args)
const networkOpts = await resolveNetworkOptions(args) const external =
const shouldStartServer =
process.argv.includes("--port") || process.argv.includes("--port") ||
process.argv.includes("--hostname") || process.argv.includes("--hostname") ||
process.argv.includes("--mdns") || process.argv.includes("--mdns") ||
networkOpts.mdns || network.mdns ||
networkOpts.port !== 0 || network.port !== 0 ||
networkOpts.hostname !== "127.0.0.1" network.hostname !== "127.0.0.1"
let url: string const transport = external
let customFetch: typeof fetch | undefined ? {
let events: EventSource | undefined url: (await client.call("server", network)).url,
fetch: undefined,
if (shouldStartServer) { events: undefined,
// Start HTTP server for external access }
const server = await client.call("server", networkOpts) : {
url = server.url url: "http://opencode.internal",
} else { fetch: createWorkerFetch(client),
// Use direct RPC communication (no HTTP) events: createEventSource(client),
url = "http://opencode.internal" }
customFetch = createWorkerFetch(client)
events = createEventSource(client)
}
const tuiPromise = tui({
url,
config,
directory: cwd,
fetch: customFetch,
events,
args: {
continue: args.continue,
sessionID: args.session,
agent: args.agent,
model: args.model,
prompt,
fork: args.fork,
},
onExit: async () => {
await client.call("shutdown", undefined)
},
})
setTimeout(() => { setTimeout(() => {
client.call("checkUpgrade", { directory: cwd }).catch(() => {}) client.call("checkUpgrade", { directory: cwd }).catch(() => {})
}, 1000) }, 1000).unref?.()
await tuiPromise try {
await tui({
url: transport.url,
config,
directory: cwd,
fetch: transport.fetch,
events: transport.events,
args: {
continue: args.continue,
sessionID: args.session,
agent: args.agent,
model: args.model,
prompt,
fork: args.fork,
},
onExit: stop,
})
} finally {
await stop()
}
} finally { } finally {
unguard?.() unguard?.()
} }

View File

@@ -27,6 +27,7 @@ import open from "open"
export namespace MCP { export namespace MCP {
const log = Log.create({ service: "mcp" }) const log = Log.create({ service: "mcp" })
const DEFAULT_TIMEOUT = 30_000 const DEFAULT_TIMEOUT = 30_000
const CLOSE = 5_000
export const Resource = z export const Resource = z
.object({ .object({
@@ -182,6 +183,52 @@ export namespace MCP {
return pids return pids
} }
function proc(client: MCPClient) {
const pid = (client.transport as { pid?: number } | undefined)?.pid
if (typeof pid === "number") return pid
return undefined
}
function signal(pid: number, kind: NodeJS.Signals) {
try {
process.kill(pid, kind)
return true
} catch {
return false
}
}
async function tree(pid: number, kind: NodeJS.Signals) {
const pids = await descendants(pid)
pids.forEach((child) => {
signal(child, kind)
})
}
async function close(client: MCPClient, key: string, raw?: number) {
const pid = raw ?? proc(client)
if (pid !== undefined) {
await tree(pid, "SIGTERM")
}
const closed = await withTimeout(client.close(), CLOSE)
.then(() => true)
.catch((error) => {
log.error("Failed to close MCP client", { key, error })
return false
})
if (closed || pid === undefined) return
signal(pid, "SIGTERM")
await tree(pid, "SIGTERM")
if (process.platform === "win32") return
await Bun.sleep(200)
signal(pid, "SIGKILL")
await tree(pid, "SIGKILL")
}
const state = Instance.state( const state = Instance.state(
async () => { async () => {
const cfg = await Config.get() const cfg = await Config.get()
@@ -218,30 +265,7 @@ export namespace MCP {
} }
}, },
async (state) => { async (state) => {
// The MCP SDK only signals the direct child process on close. await Promise.all(Object.entries(state.clients).map(([key, client]) => close(client, key)))
// Servers like chrome-devtools-mcp spawn grandchild processes
// (e.g. Chrome) that the SDK never reaches, leaving them orphaned.
// Kill the full descendant tree first so the server exits promptly
// and no processes are left behind.
for (const client of Object.values(state.clients)) {
const pid = (client.transport as any)?.pid
if (typeof pid !== "number") continue
for (const dpid of await descendants(pid)) {
try {
process.kill(dpid, "SIGTERM")
} catch {}
}
}
await Promise.all(
Object.values(state.clients).map((client) =>
client.close().catch((error) => {
log.error("Failed to close MCP client", {
error,
})
}),
),
)
pendingOAuthTransports.clear() pendingOAuthTransports.clear()
}, },
) )
@@ -311,11 +335,9 @@ export namespace MCP {
} }
} }
// Close existing client if present to prevent memory leaks // Close existing client if present to prevent memory leaks
const existingClient = s.clients[name] const prev = s.clients[name]
if (existingClient) { if (prev) {
await existingClient.close().catch((error) => { await close(prev, name)
log.error("Failed to close existing MCP client", { name, error })
})
} }
s.clients[name] = result.mcpClient s.clients[name] = result.mcpClient
s.status[name] = result.status s.status[name] = result.status
@@ -380,14 +402,14 @@ export namespace MCP {
] ]
let lastError: Error | undefined let lastError: Error | undefined
const connectTimeout = mcp.timeout ?? DEFAULT_TIMEOUT const timeout = mcp.timeout ?? DEFAULT_TIMEOUT
for (const { name, transport } of transports) { for (const { name, transport } of transports) {
try { try {
const client = new Client({ const client = new Client({
name: "opencode", name: "opencode",
version: Installation.VERSION, version: Installation.VERSION,
}) })
await withTimeout(client.connect(transport), connectTimeout) await withTimeout(client.connect(transport), timeout)
registerNotificationHandlers(client, key) registerNotificationHandlers(client, key)
mcpClient = client mcpClient = client
log.info("connected", { key, transport: name }) log.info("connected", { key, transport: name })
@@ -460,19 +482,20 @@ export namespace MCP {
log.info(`mcp stderr: ${chunk.toString()}`, { key }) log.info(`mcp stderr: ${chunk.toString()}`, { key })
}) })
const connectTimeout = mcp.timeout ?? DEFAULT_TIMEOUT const timeout = mcp.timeout ?? DEFAULT_TIMEOUT
const client = new Client({
name: "opencode",
version: Installation.VERSION,
})
try { try {
const client = new Client({ await withTimeout(client.connect(transport), timeout)
name: "opencode",
version: Installation.VERSION,
})
await withTimeout(client.connect(transport), connectTimeout)
registerNotificationHandlers(client, key) registerNotificationHandlers(client, key)
mcpClient = client mcpClient = client
status = { status = {
status: "connected", status: "connected",
} }
} catch (error) { } catch (error) {
await close(client, key, (transport as { pid?: number } | undefined)?.pid)
log.error("local mcp startup failed", { log.error("local mcp startup failed", {
key, key,
command: mcp.command, command: mcp.command,
@@ -505,11 +528,7 @@ export namespace MCP {
return undefined return undefined
}) })
if (!result) { if (!result) {
await mcpClient.close().catch((error) => { await close(mcpClient, key)
log.error("Failed to close MCP client", {
error,
})
})
status = { status = {
status: "failed", status: "failed",
error: "Failed to get tools", error: "Failed to get tools",
@@ -578,11 +597,9 @@ export namespace MCP {
s.status[name] = result.status s.status[name] = result.status
if (result.mcpClient) { if (result.mcpClient) {
// Close existing client if present to prevent memory leaks // Close existing client if present to prevent memory leaks
const existingClient = s.clients[name] const prev = s.clients[name]
if (existingClient) { if (prev) {
await existingClient.close().catch((error) => { await close(prev, name)
log.error("Failed to close existing MCP client", { name, error })
})
} }
s.clients[name] = result.mcpClient s.clients[name] = result.mcpClient
} }
@@ -592,9 +609,7 @@ export namespace MCP {
const s = await state() const s = await state()
const client = s.clients[name] const client = s.clients[name]
if (client) { if (client) {
await client.close().catch((error) => { await close(client, name)
log.error("Failed to close MCP client", { name, error })
})
delete s.clients[name] delete s.clients[name]
} }
s.status[name] = { status: "disabled" } s.status[name] = { status: "disabled" }

View File

@@ -24,7 +24,7 @@ Usage notes:
- The command argument is required. - The command argument is required.
- You can specify an optional timeout in milliseconds. If not specified, commands will time out after 120000ms (2 minutes). - You can specify an optional timeout in milliseconds. If not specified, commands will time out after 120000ms (2 minutes).
- It is very helpful if you write a clear, concise description of what this command does in 5-10 words. - It is very helpful if you write a clear, concise description of what this command does in 5-10 words.
- If the output exceeds ${maxLines} lines or ${maxBytes} bytes, it will be truncated and the full output will be written to a file. You can use Read with offset/limit to read specific sections or Grep to search the full content. Because of this, you do NOT need to use `head`, `tail`, or other truncation commands to limit output - just run the command directly. - If the output exceeds ${maxLines} lines or ${maxBytes} bytes, it will be truncated and the full output will be written to a file. You can use Read with offset/limit to read specific sections or Grep to search the full content. Do NOT use `head`, `tail`, or other truncation commands to limit output; the full output will already be captured to a file for more precise searching.
- Avoid using Bash with the `find`, `grep`, `cat`, `head`, `tail`, `sed`, `awk`, or `echo` commands, unless explicitly instructed or when these commands are truly necessary for the task. Instead, always prefer using the dedicated tools for these commands: - Avoid using Bash with the `find`, `grep`, `cat`, `head`, `tail`, `sed`, `awk`, or `echo` commands, unless explicitly instructed or when these commands are truly necessary for the task. Instead, always prefer using the dedicated tools for these commands:
- File search: Use Glob (NOT find or ls) - File search: Use Glob (NOT find or ls)