fix(core,opencode): resolve audit CRITICAL/HIGH defects across crash recovery, settlement races, and hot paths - #154
Merged
Merged
Conversation
…recovery, settlement races, and hot paths - worker.ts: replace empty exception handlers with file logging, worker.fatal RPC notification, and safe exit; main process listens for worker fatal/error/close - rpc.ts: propagate RPC handler failures to caller instead of unhandledRejection + hung promise - llm.ts (V2 runner): close awaitToolFibers race where awaitEmpty could swallow the last tool fiber failure - processor.ts (V1): guard cleanup against overwriting settled tool results; claim toolcalls synchronously - prompt.ts: parallelize independent request-build I/O with concurrency unbounded - recovery.ts: narrow session status checker to NotFoundError; propagate unexpected errors instead of inventing node failures - loop.ts: move orchestrator_unresponsive dag.fail under evalLock; log wake redelivery store failures instead of swallowing - stream.transport.ts: cap buffered events at 1000, add prompt-turn idle escape threshold, bound recoverQuestion polling to 120s - convert-to-openai-responses-input.ts: batch reasoning provider-option parsing out of the per-part loop - handler.ts (zen): cumulative-weight provider pick replaces O(sum-of-weights) array expansion - plugin/index.ts: observe plugin event hook rejections via forkDetach + logError - dag.ts: structural guard in parseWorkflowConfig for nodes/id/depends_on invariants - acp/service.ts: drop failed contextLimit lookups from cache so they retry - event-reducer.ts: batch per-delta store updates into one reactive notification - instance-state.ts: document why the ScopedCache is deliberately unbounded
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景
一次全代码库审查(DAG 工作流:4 并行 reviewer + 1 仲裁者)报告了 3 个 CRITICAL + 12 个 HIGH 级缺陷,集中在三个域:崩溃恢复数据丢失、资源无界增长、热路径性能退化。本 PR 逐项定位、论证并修复。
每项都经过独立诊断验证,其中 5 项修正了审查报告的论断(详见下方"论断修正")。
CRITICAL 修复
C1 · TUI Worker 空异常处理器 —
packages/opencode/src/cli/tui/worker.ts全局
uncaughtException/unhandledRejection处理器为空函数,静默丢弃所有未捕获异常;进程在损坏状态下继续运行,掩盖所有其他缺陷的崩溃信号(表现为 TUI 黑屏/挂死且零日志)。uncaughtException:同步落盘到Global.Path.log/opencode.log(appendFileSync保证 exit 前写完)→Rpc.emit("worker.fatal")通知主进程 →process.exit(1)unhandledRejection:落盘日志但不退出 —— 后台任务的游离 rejection 不等于进程损坏,保留原"保活"意图,只消除静默util/rpc.ts:原本 worker 内任何 RPC 方法抛错都会变成 unhandledRejection,且主进程侧 Promise 永久 pending(pending Map 泄漏 + 调用方冻结)。现在listen捕获错误回传rpc.result { error },client.call相应 rejectcli/cmd/tui.ts三路兜底:worker.fatalRPC 事件、workererror、workerclose,任一触发即 terminate +UI.error+exit(1);stopped标志保证正常stop()不误触发C2 · LLM 工具结算竞争 —
packages/core/src/session/runner/llm.tsraceFirst(FiberSet.join, FiberSet.awaitEmpty)在最后一个工具 fiber 失败时,awaitEmpty的成功可能赢得竞争,吞掉工具结算失败。机制已从 Effect 源码证实:FiberSet observer 先backing.delete(使awaitEmpty就绪)再Deferred.doneUnsafe(使join就绪),两者同 tick 竞争。修复:
awaitEmpty胜出后复查Deferred.isDone(fibers.deferred),有失败则改走join。保留"任一 fiber 失败立即中止等待"的原语义,确定性关闭吞错窗口。C3 · LLM 请求构建串行阻塞 — 两处
packages/opencode/src/session/prompt.ts:Effect.all([...])未传 concurrency 选项,Effect 默认顺序执行 —— skills 文件读取、environment DB 查询、instruction glob+HTTP fetch(5s 超时)、MCP 通信、hooks 查询、消息投影这 6 个相互独立的 I/O 被串行化,估算每 turn 额外 100–600ms。同文件同类路径(llm.ts:95、V2 runner)都已正确用{ concurrency: "unbounded" },这是唯一漏掉的热路径packages/core/src/github-copilot/responses/convert-to-openai-responses-input.ts:reasoning 的parseProviderOptions从循环内串行 await 提升为循环外Promise.all批量解析,保持input顺序与跨迭代reasoningMessages状态HIGH 修复
dag/runtime/recovery.tscatchTag("NotFoundError");意外错误传播而非编造 nodeFailed;移除[] as never类型谎言dag/runtime/loop.tscatchCause+ logWarning,失败不再静默cli/cmd/run/stream.transport.tsdag/runtime/loop.tsdag.fail移入 evalLock 同一 permit,消除检查→fail 之间的 spawn 窗口console/.../zen/util/handler.tsArray(weight).fill物理展开,槽位布局逐位等价cli/cmd/run/stream.transport.tseffect/instance-state.tsplugin/index.tsvoid hook.event(...)→tryPromise + tapError(logError) + forkDetach,不阻塞总线但 rejection 全部可观测dag/dag.tsparseWorkflowConfig加结构守卫(nodes数组 + 每节点id/depends_on)acp/service.tsapp/.../event-reducer.tsbatch合并为一次响应式通知cli/cmd/run/stream.transport.ts论断修正
诊断过程中有 5 项与审查报告的结论不同,按实际根因处理:
orDie转为 defect,Effect.catch吞不掉。真实问题是宽 catch 折叠所有类型化错误item.live = true"会引入更糟的新竞态 ——promptAsync是持久准入,返回时 drain 未启动、会话仍 idle,立即置 live 会让零输出回合提前完成。改为 poll 层逃生阈值InstanceStore.disposeDirectory按目录跨服务协调;单缓存加 LRU/TTL 会独立驱逐产生撕裂的实例状态、释放活跃 fiber 持有的资源。改为把该约束写成注释text + delta在 V8/JSC 是 rope 串接,均摊 O(1),O(n²) 论断对现代引擎不成立,session-data.ts未改动;真实开销是每 token 两次 SolidJS 通知Semaphore.makeUnsafe(1).withPermit+tool.settled守卫免疫;V1processor.ts存在窄窗口(cleanup 250ms 超时后覆盖已完成结果),已加状态守卫 + 同步认领(单线程 fiber 模型下delete ctx.toolcalls[id]即互斥,比加锁更轻且无死锁风险)验证
core/opencode/app/console-app四包tsgo --noEmit全部 0 错误unbound-method×2、no-unsafe-type-assertion×3、await-thenable×1)门禁
目标分支
dev,需通过 Typecheck(含 lint 棘轮)。合并后 push 到dev将触发全量 Unit Tests + E2E 验证。