fix: hook command grandchildren hold stdio pipes and hang triggers forever - #505
Merged
Merged
Conversation
This was referenced Sep 2, 2026
LeXwDeX
marked this pull request as ready for review
September 2, 2026 05:35
LeXwDeX
force-pushed
the
fix/500-hook-command-grandchildren
branch
from
September 2, 2026 05:42
6cc0c04 to
1e8ee35
Compare
…ldren cannot hang triggers A timed-out command hook only SIGTERM'd the shell wrapper; grandchildren keeping the stdio pipes open meant the close event never fired and the hook trigger hung forever. Exit and stream-drain are now awaited separately with a bounded grace, then the whole group (detached, negative pid on POSIX; taskkill /T /F on Windows) is SIGKILL'd and reaped.
watch() discarded every Unsubscribe, so each instance dispose/remount cycle leaked five permanent EventV2 listeners holding the instance context. The finalizer now unsubscribes before closing the scope.
Three event.on handlers discarded their unsubscribe functions and accumulated in the app-level SDK handler set on every route transition, retaining the opentui editor and renderer trees.
…nt shutdown stop() on POSIX returned right after SIGTERM, so servers ignoring it stayed alive as orphans while instance finalizers reported success. It now waits a bounded grace, escalates to SIGKILL, and awaits exit; the SDK copy adapts the same escalation synchronously with an unref'd timer. MCP client shutdown reaps the whole process tree through a shared shutdownClient used by the state finalizer, closeClient, and the create rollback path.
Every memory commit copied the full topic set into a new generations directory that was never removed; keep the latest 3 plus orphan-staging sweep, best-effort so a cleanup failure never fails a commit. Heap snapshots (hundreds of MB each) now rotate to the latest 2 in the log directory.
Redundant toString on stream text, .resolves on a synchronous prune helper, and one intentional wire-shape JSON assertion (disabled inline with reason, matching the schema-contract test precedent).
heap-<pid>-<ts> sorted lexicographically orders snapshots by pid across runs (digit-count changes, wraparound), so pruning could delete the newest snapshot and keep stale ones. Sort by the timestamp segment instead, surface non-ENOENT readdir failures as warnings, fix the always-true missing-dir assertion (unawaited async), and add a cross-pid regression test.
The finally-block process.exit() cut off pending async cleanup (instance finalizers, MCP/process stop escalation) — the #503 clause the first pass skipped. An unref'd 5s force-exit timer (matching the stop escalation budget) keeps the hanging-subprocess protection while a clean drain exits immediately with all cleanup completed.
…in window" This reverts commit f40bf17.
LeXwDeX
force-pushed
the
fix/500-hook-command-grandchildren
branch
from
September 2, 2026 07:35
479469a to
7c44188
Compare
The escalates-to-SIGKILL child wrote "ready" before registering its trap, so a parent preempted between the two statements delivered SIGTERM to the default handler and the child exited SIGTERM instead of escalating — observed as a load-dependent failure on the linux runner (262ms, signalCode SIGTERM). Ordering the trap first makes the ready signal prove the handler is installed.
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
1 participant
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.
Closes #500
Closes #501
Closes #502
Closes #503
Closes #504
Why
2026-09-02 资源泄露审计确认的 7 项风险中的 5 项确认级修复：hook 子进程挂死（用户环境可直接踩）、服务端/TUI 内存泄露 ×2、子进程孤儿化向量、磁盘无限增长 ×2。附带：cherry-pick 了 PR #509 的 CI
ready_for_review触发修复（本 PR 的 SpecGit 锚点同样依赖它）。What changed
hook/settings.ts：command hook 超时后对整个进程组收割（POSIX detached + 负 PID；Windowstaskkill /T /F）；exit 与流排水分离等待 + 有界宽限，孙进程持有 stdio 管道不再永久挂起 triggershare/share-next.ts：5 个 EventV2watch()的 Unsubscribe 收集进实例 finalizer（先退订再关 scope），dispose/重挂载不再线性累积 listenerroutes/session/index.tsx×2、component/prompt/index.tsx×1 的event.on退订包进onCleanup，路由切换不再向 app 级 handler Set 累积 opentui editor/renderer 闭包util/process.ts：stop()SIGTERM → 有界等待（3s）→ SIGKILL 升级 → await exit；新增stopTree()（signal-0 探活）；SDK 同步副本以 unref'd timer 适配；mcp/index.ts提取shutdownClient覆盖 state finalizer、closeClient、create 回滚三条路径（descendants 快照先于 close）。index.ts的 finallyprocess.exit()取舍保留（issue 内授权的 Plan B：无可观测清理信号，fix: unify child-process stop semantics with await-exit and SIGKILL escalation #503 的有界收割已把孤儿窗口从"无限"缩到 ≤5s）memory/store.ts：generations 保留最近 3 代 + 孤儿 staging 清扫（best-effort，清理失败绝不 fail commit）；cli/heap.ts：快照轮转保留最近 2 个Evidence
stop()对 SIGTERM-忽略真实进程 6s 内终止、MCP 进程树全杀（真实 sh 派生 sleep 后代）、memory 5 次 commit 后仅 3 代 + chmod 0555 注入 commit 仍成功、heap 轮转 3 用例bun typecheck（根，29/29 包）；packages/opencode目标测试 56 pass；hook 全目录 148 pass；memory 全目录 106 pass；packages/tui全量 250 pass / 1 skip；oxlint 4840/4850 棘轮内Checklist
specgit finishexits 0.