Skip to content

fix(tui): resolve streaming freeze from unhandled finish event, missing timeouts, and unguarded effects #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
coleleavitt wants to merge 1 commit into from
Closed

fix(tui): resolve streaming freeze from unhandled finish event, missing timeouts, and unguarded effects #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/opencode/src/cli/cmd/tui/context/sdk.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ export const { use: useSDK, provider: SDKProvider } = createSimpleContext({
const elapsed = Date.now() - last

if (timer) return
// If we just flushed recently (within 16ms), batch this with future events
// If we just flushed recently (within 50ms), batch this with future events
// Otherwise, process immediately to avoid latency
if (elapsed < 16) {
timer = setTimeout(flush, 16)
if (elapsed < 50) {
timer = setTimeout(flush, 50)
Comment on lines +57 to +58
Copy link

@gemini-code-assist gemini-code-assist bot Feb 27, 2026

Choose a reason for hiding this comment

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

medium

The batch window time 50 is used as a magic number in both the condition and the setTimeout call. To improve readability and maintainability, consider defining it as a constant, for example const BATCH_WINDOW_MS = 50;, at a higher scope (e.g., at the top of the init function) and using it here.

return
}
flush()
Expand Down
4 changes: 3 additions & 1 deletion packages/opencode/src/mcp/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,9 @@ export namespace MCP {

const toolsResults = await Promise.all(
connectedClients.map(async ([clientName, client]) => {
const toolsResult = await client.listTools().catch((e) => {
const mcpEntry = config[clientName]
const timeout = (isMcpConfigured(mcpEntry) ? mcpEntry.timeout : undefined) ?? defaultTimeout ?? DEFAULT_TIMEOUT
const toolsResult = await withTimeout(client.listTools(), timeout).catch((e) => {
log.error("failed to get tools", { clientName, error: e.message })
const failedStatus = {
status: "failed" as const,
Comment on lines +580 to 585
Copy link

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects bot Feb 27, 2026

Choose a reason for hiding this comment

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

Action required

2. Mcp timeout leaks clients 🐞 Bug ⛯ Reliability

On listTools() timeout/failure in MCP.tools(), the client is deleted from state without calling
client.close(), potentially leaking transports/sockets. This is especially risky now that
withTimeout() will trigger failures that previously would hang indefinitely.
Agent Prompt 
### Issue description
When `listTools()` times out/fails in `MCP.tools()`, the client is removed from state but never closed, potentially leaking resources.

### Issue Context
Other code paths (e.g., `disconnect()` and `create()` when initial `listTools()` fails) close clients, suggesting this is expected cleanup behavior.

### Fix Focus Areas
- packages/opencode/src/mcp/index.ts[578-590]

### Suggested changes
1) Before `delete s.clients[clientName]`, do:
- `await client.close().catch((error) => log.error("Failed to close MCP client", { clientName, error }))`

2) Make the error logging safe:
- Replace `error: e.message` with `error: e instanceof Error ? e.message : String(e)` (to avoid issues if `e` is `null`/non-object).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Expand Down
5 changes: 4 additions & 1 deletion packages/opencode/src/session/processor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export namespace SessionProcessor {
try {
let currentText: MessageV2.TextPart | undefined
let reasoningMap: Record<string, MessageV2.ReasoningPart> = {}
let finished = false
const stream = await LLM.stream(streamInput)

for await (const value of stream.fullStream) {
Expand Down Expand Up @@ -337,6 +338,8 @@ export namespace SessionProcessor {
break

case "finish":
log.info("stream finish event received")
finished = true
break

default:
Expand All @@ -345,7 +348,7 @@ export namespace SessionProcessor {
})
continue
}
if (needsCompaction) break
if (needsCompaction || finished) break
}
} catch (e: any) {
log.error("process", {
Expand Down
4 changes: 2 additions & 2 deletions packages/opencode/src/storage/db.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export namespace Database {
if (err instanceof Context.NotFound) {
const effects: (() => void | Promise<void>)[] = []
const result = ctx.provide({ effects, tx: Client() }, () => callback(Client()))
for (const effect of effects) effect()
for (const effect of effects) Promise.resolve(effect()).catch((e) => log.error("effect failed", { error: e }))
Copy link

@cubic-dev-ai cubic-dev-ai bot Feb 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P1: Promise.resolve(effect()) does not catch synchronous throws from effect(). If the effect function throws synchronously, the exception escapes before Promise.resolve wraps it, crashing the caller and skipping remaining effects. Use Promise.resolve().then(() => effect()) to defer execution into the microtask queue, which catches both sync throws and async rejections.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/storage/db.ts, line 125:

<comment>`Promise.resolve(effect())` does not catch synchronous throws from `effect()`. If the effect function throws synchronously, the exception escapes before `Promise.resolve` wraps it, crashing the caller and skipping remaining effects. Use `Promise.resolve().then(() => effect())` to defer execution into the microtask queue, which catches both sync throws and async rejections.</comment>

<file context>
@@ -122,7 +122,7 @@ export namespace Database {
         const effects: (() => void | Promise<void>)[] = []
         const result = ctx.provide({ effects, tx: Client() }, () => callback(Client()))
-        for (const effect of effects) effect()
+        for (const effect of effects) Promise.resolve(effect()).catch((e) => log.error("effect failed", { error: e }))
         return result
       }
</file context>
Suggested change
for (const effect of effects) Promise.resolve(effect()).catch((e) => log.error("effect failed", { error: e }))
for (const effect of effects) Promise.resolve().then(() => effect()).catch((e) => log.error("effect failed", { error: e }))
Fix with Cubic

Copy link

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects bot Feb 27, 2026

Choose a reason for hiding this comment

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

Action required

1. catch uses implicit any 📘 Rule violation ✓ Correctness

The new Promise rejection handlers introduce e as an implicit any (from Promise.catch), which
weakens type safety. This can hide mistakes when logging/inspecting errors and violates the no-any
rule.
Agent Prompt 
## Issue description
New `.catch((e) => ...)` handlers introduce an implicit `any` rejection reason, violating the requirement to avoid `any`.

## Issue Context
In TypeScript, `Promise.catch` typically types the rejection reason as `any`; explicitly annotating as `unknown` and narrowing preserves type safety.

## Fix Focus Areas
- packages/opencode/src/storage/db.ts[125-125]
- packages/opencode/src/storage/db.ts[149-149]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

return result
}
throw err
Expand All @@ -146,7 +146,7 @@ export namespace Database {
const result = Client().transaction((tx) => {
return ctx.provide({ tx, effects }, () => callback(tx))
})
for (const effect of effects) effect()
for (const effect of effects) Promise.resolve(effect()).catch((e) => log.error("effect failed", { error: e }))
return result
}
throw err
Expand Down
Loading