Skip to content

Commit cb70bb0

Browse files
committed
fix: resolve critical bugs across CLI codebase
Critical fixes: - provider/models.ts: wrap JSON.parse in try/catch with timeout on fetch, add .catch() on setInterval refresh and initial startup - cli/cmd/tui/context/sync.tsx: guard each non-blocking Promise with individual .catch(), replace non-null assertions (x.data!) with fallback defaults (x.data ?? {}), replace console.log with Log - agent/critic.ts: change fail-safe from permissive (score 0.5, isSafe true) to blocking (score 0, isSafe false) on validation error - cli/cmd/debug/agent.ts: remove new Function() code injection, replace with safe JSON relaxed-key parsing - mcp/oauth-callback.ts: add 2s timeout and cleanup on isPortInUse() socket to prevent resource leak and hanging promises High/Medium fixes: - cli/cmd/web.ts: inform user to open browser manually on open() failure - cli/upgrade.ts: log upgrade check and auto-upgrade failures instead of silently swallowing with .catch(() => {}) - global/index.ts: document empty catch block intention - tool/registry.ts: replace `as any` with `as Record<string, unknown>` - session/summary.ts: document intentional fire-and-forget pattern https://claude.ai/code/session_01A8LkdCir3TS3uCbx9L8CAb
1 parent 28a73fd commit cb70bb0

10 files changed

Lines changed: 79 additions & 37 deletions

File tree

packages/opencode/src/agent/critic.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,12 @@ export namespace Critic {
4040

4141
return result
4242
} catch (e) {
43-
log.error("Critic validation failed, defaulting to cautious score", { error: e })
44-
const fallback = {
45-
score: 0.5,
46-
justification: "Audit failed due to technical error.",
47-
isSafe: true,
48-
} as ValidationResult
43+
log.error("Critic validation failed, blocking content for safety", { error: e })
44+
const fallback: ValidationResult = {
45+
score: 0,
46+
justification: "Validation error — content blocked for safety.",
47+
isSafe: false,
48+
}
4949
GlobalBus.emit("event", { payload: { type: "critic.validated", ...fallback } })
5050
return fallback
5151
}

packages/opencode/src/cli/cmd/debug/agent.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,14 @@ function parseToolParams(input?: string) {
9595
try {
9696
return JSON.parse(trimmed)
9797
} catch (jsonError) {
98+
// Try adding quotes around unquoted keys for relaxed JSON-like input:
99+
// { foo: "bar" } → { "foo": "bar" }
98100
try {
99-
return new Function(`return (${trimmed})`)()
100-
} catch (evalError) {
101+
const relaxed = trimmed.replace(/([{,]\s*)(\w+)\s*:/g, '$1"$2":')
102+
return JSON.parse(relaxed)
103+
} catch {
101104
throw new Error(
102-
`Failed to parse --params. Use JSON or a JS object literal. JSON error: ${jsonError}. Eval error: ${evalError}.`,
105+
`Failed to parse --params. Provide valid JSON (e.g. '{"key": "value"}'). Parse error: ${jsonError}`,
103106
)
104107
}
105108
}

packages/opencode/src/cli/cmd/tui/context/sync.tsx

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ export const { use: useSync, provider: SyncProvider } = createSimpleContext({
356356
const args = useArgs()
357357

358358
async function bootstrap() {
359-
console.log("bootstrapping")
359+
Log.Default.info("bootstrapping")
360360
const start = Date.now() - 30 * 24 * 60 * 60 * 1000
361361
const sessionListPromise = sdk.client.session
362362
.list({ start: start })
@@ -408,24 +408,25 @@ export const { use: useSync, provider: SyncProvider } = createSimpleContext({
408408
})
409409
.then(() => {
410410
if (store.status !== "complete") setStore("status", "partial")
411-
// non-blocking
411+
// non-blocking — each call is individually guarded so one failure
412+
// doesn't prevent the rest from completing
412413
Promise.all([
413-
...(args.continue ? [] : [sessionListPromise.then((sessions) => setStore("session", reconcile(sessions)))]),
414-
sdk.client.command.list().then((x) => setStore("command", reconcile(x.data ?? []))),
415-
sdk.client.lsp.status().then((x) => setStore("lsp", reconcile(x.data!))),
416-
sdk.client.mcp.status().then((x) => setStore("mcp", reconcile(x.data!))),
417-
sdk.client.experimental.resource.list().then((x) => setStore("mcp_resource", reconcile(x.data ?? {}))),
418-
sdk.client.formatter.status().then((x) => setStore("formatter", reconcile(x.data!))),
414+
...(args.continue ? [] : [sessionListPromise.then((sessions) => setStore("session", reconcile(sessions))).catch(() => {})]),
415+
sdk.client.command.list().then((x) => setStore("command", reconcile(x.data ?? []))).catch(() => {}),
416+
sdk.client.lsp.status().then((x) => setStore("lsp", reconcile(x.data ?? {}))).catch(() => {}),
417+
sdk.client.mcp.status().then((x) => setStore("mcp", reconcile(x.data ?? {}))).catch(() => {}),
418+
sdk.client.experimental.resource.list().then((x) => setStore("mcp_resource", reconcile(x.data ?? {}))).catch(() => {}),
419+
sdk.client.formatter.status().then((x) => setStore("formatter", reconcile(x.data ?? {}))).catch(() => {}),
419420
sdk.client.session.status().then((x) => {
420-
setStore("session_status", reconcile(x.data!))
421-
}),
422-
sdk.client.provider.auth().then((x) => setStore("provider_auth", reconcile(x.data ?? {}))),
423-
sdk.client.vcs.get().then((x) => setStore("vcs", reconcile(x.data))),
424-
sdk.client.path.get().then((x) => setStore("path", reconcile(x.data!))),
425-
syncWorkspaces(),
421+
if (x.data) setStore("session_status", reconcile(x.data))
422+
}).catch(() => {}),
423+
sdk.client.provider.auth().then((x) => setStore("provider_auth", reconcile(x.data ?? {}))).catch(() => {}),
424+
sdk.client.vcs.get().then((x) => setStore("vcs", reconcile(x.data))).catch(() => {}),
425+
sdk.client.path.get().then((x) => { if (x.data) setStore("path", reconcile(x.data)) }).catch(() => {}),
426+
syncWorkspaces().catch(() => {}),
426427
]).then(() => {
427428
setStore("status", "complete")
428-
})
429+
}).catch(() => {})
429430
})
430431
.catch(async (e) => {
431432
Log.Default.error("tui bootstrap failed", {

packages/opencode/src/cli/cmd/web.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,15 @@ export const WebCommand = cmd({
6868
}
6969

7070
// Open localhost in browser
71-
open(localhostUrl.toString()).catch(() => {})
71+
open(localhostUrl.toString()).catch(() => {
72+
UI.println(UI.Style.TEXT_NORMAL, ` Open manually: ${localhostUrl}`)
73+
})
7274
} else {
7375
const displayUrl = server.url.toString()
7476
UI.println(UI.Style.TEXT_INFO_BOLD + " Web interface: ", UI.Style.TEXT_NORMAL, displayUrl)
75-
open(displayUrl).catch(() => {})
77+
open(displayUrl).catch(() => {
78+
UI.println(UI.Style.TEXT_NORMAL, ` Open manually: ${displayUrl}`)
79+
})
7680
}
7781

7882
await new Promise(() => {})

packages/opencode/src/cli/upgrade.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,16 @@ import { Bus } from "@/bus"
22
import { Config } from "@/config/config"
33
import { Flag } from "@/flag/flag"
44
import { Installation } from "@/installation"
5+
import { Log } from "@/util/log"
6+
7+
const log = Log.create({ service: "upgrade" })
58

69
export async function upgrade() {
710
const config = await Config.getGlobal()
811
const method = await Installation.method()
9-
const latest = await Installation.latest(method).catch(() => {})
12+
const latest = await Installation.latest(method).catch((e) => {
13+
log.info("failed to check for updates", { method, error: String(e) })
14+
})
1015
if (!latest) return
1116

1217
if (Flag.OPENCODE_ALWAYS_NOTIFY_UPDATE) {
@@ -27,5 +32,7 @@ export async function upgrade() {
2732
if (method === "unknown") return
2833
await Installation.upgrade(method, latest)
2934
.then(() => Bus.publish(Installation.Event.Updated, { version: latest }))
30-
.catch(() => {})
35+
.catch((e) => {
36+
log.info("auto-upgrade failed", { version: latest, error: String(e) })
37+
})
3138
}

packages/opencode/src/global/index.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ export namespace Global {
5858
}),
5959
),
6060
)
61-
} catch (e) {}
61+
} catch {
62+
// Cache directory may not exist yet or items may be locked — safe to ignore
63+
}
6264
await Filesystem.write(path.join(Global.Path.cache, "version"), CACHE_VERSION)
6365
}
6466
}

packages/opencode/src/mcp/oauth-callback.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,22 @@ export namespace McpOAuthCallback {
184184
export async function isPortInUse(): Promise<boolean> {
185185
return new Promise((resolve) => {
186186
const socket = createConnection(OAUTH_CALLBACK_PORT, "127.0.0.1")
187-
socket.on("connect", () => {
187+
const cleanup = () => {
188+
socket.removeAllListeners()
188189
socket.destroy()
190+
}
191+
const timer = setTimeout(() => {
192+
cleanup()
193+
resolve(false)
194+
}, 2000)
195+
socket.on("connect", () => {
196+
clearTimeout(timer)
197+
cleanup()
189198
resolve(true)
190199
})
191200
socket.on("error", () => {
201+
clearTimeout(timer)
202+
cleanup()
192203
resolve(false)
193204
})
194205
})

packages/opencode/src/provider/models.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,15 @@ export namespace ModelsDev {
9494
.catch(() => undefined)
9595
if (snapshot) return snapshot
9696
if (Flag.OPENCODE_DISABLE_MODELS_FETCH) return {}
97-
const json = await fetch(`${url()}/api.json`).then((x) => x.text())
98-
return JSON.parse(json)
97+
try {
98+
const json = await fetch(`${url()}/api.json`, {
99+
signal: AbortSignal.timeout(10_000),
100+
}).then((x) => x.text())
101+
return JSON.parse(json)
102+
} catch (e) {
103+
log.error("failed to fetch or parse models.dev", { error: String(e) })
104+
return {}
105+
}
99106
})
100107

101108
export async function get() {
@@ -122,10 +129,14 @@ export namespace ModelsDev {
122129
}
123130

124131
if (!Flag.OPENCODE_DISABLE_MODELS_FETCH && !process.argv.includes("--get-yargs-completions")) {
125-
ModelsDev.refresh()
132+
ModelsDev.refresh().catch((e) => {
133+
log.error("initial models.dev refresh failed", { error: String(e) })
134+
})
126135
setInterval(
127-
async () => {
128-
await ModelsDev.refresh()
136+
() => {
137+
ModelsDev.refresh().catch((e) => {
138+
log.error("periodic models.dev refresh failed", { error: String(e) })
139+
})
129140
},
130141
60 * 1000 * 60,
131142
).unref()

packages/opencode/src/session/summary.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,9 @@ export namespace SessionSummary {
164164
const { runPromise } = makeRuntime(Service, defaultLayer)
165165

166166
export const summarize = (input: { sessionID: SessionID; messageID: MessageID }) =>
167-
void runPromise((svc) => svc.summarize(input)).catch(() => {})
167+
void runPromise((svc) => svc.summarize(input)).catch(() => {
168+
// Intentional fire-and-forget — summarization failure is non-critical
169+
})
168170

169171
export const DiffInput = z.object({
170172
sessionID: SessionID.zod,

packages/opencode/src/tool/registry.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ export namespace ToolRegistry {
7878
directory: ctx.directory,
7979
worktree: ctx.worktree,
8080
} as unknown as PluginToolContext
81-
const result = await def.execute(args as any, pluginCtx)
81+
// args is already validated by the AI SDK against the zod schema above
82+
const result = await def.execute(args as Record<string, unknown>, pluginCtx)
8283
const out = await Truncate.output(result, {}, initCtx?.agent)
8384
return {
8485
title: "",

0 commit comments

Comments
 (0)