Repository navigation
Conversation
Kill held Pi processes after an idle timeout without sealing the session, publish session.runtime.stopped, and let the next prompt ensureRuntime again. UI clears live turn state but stays ready (not a terminal error).
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
Use TestClock under @effect/vitest so the idle fiber actually fires, split toContain calls, and allow the stopped verb in the event naming invariant.
commit: |
|
reviewer: CI not passed at current head |
…timeout # Conflicts: # packages/server/src/harness/session-fold.ts
|
reviewer: CI not passed at current head |
|
reviewer: CI not passed at current head
Per |
Review-and-merge gate record — not merged
Required CI observed for this head
Blocking gate and conclusion
Stopped before code review and independent verification; not merged. These are gate observations, not an implementation-quality verdict. No branch update, conflict resolution, CI rerun, protection bypass or fix-and-merge was performed. Revisit only after a relevant change resolves the recorded blocker: an author-provided head containing current |
CI 失败定位与修复方向
本次 Check 失败明确来自 这是对既有阻塞新增的日志定位，不是代码审查通过。未本地重现或验证修复方案，未修改分支、重跑 CI 或合并。修复/环境恢复并得到满足当前 base 的全绿候选后，重新从 CI → 审查 → 独立验收开始。 |
修复提交与独立审查交接
已修复唯一已定位的格式错误： 这是实现者的修复自检记录，不是独立验收或批准。已普通推送并触发新 CI；不复用旧 head 的全绿状态。下一步：新 required CI 全部成功后，由独立审查者在干净 worktree 审查完整 PR 并验收。UI 产品路径仍需 Web/Desktop 截图与视频（适用时）。本轮没有合并。 |
独立审查发现新的 P1：空闲释放与新 prompt 存在竞态
独立审查定位 审查者用隔离的内存生命周期诊断暂停旧 runtime.close 并交错新 prompt，实际断言确认 结论：独立审查不通过，不合并。需要协调旧 runtime 关闭、新 runtime 获取及 stop 事件发布，使旧资源结束不能覆盖新 turn。后续修复须把该交错变成回归测试，再由独立审查者重新检查。此前格式修复有效，但不构成整个功能通过。 证据边界：独立审查环境无法访问 GitHub，CI 由协调端查验；真实子进程 suspend/resume、Web/Desktop 证据尚未完成。主工作区及审查源码均未改动。 |
P1 修复已提交，等待新版本独立复核新 head： 沿用现有 Deferred 所有权机制，增加 suspension ticket：旧 runtime 关闭及 stopped 事件发布完成前，新 acquisition 和 release 等待。不改公共 API、事件格式或存储结构。 实现者证据：确定性回归 这是修复自检，不是独立批准。新 CI 与独立复核必须针对此 head；真实子进程和 UI 验收仍未完成，没有合并。 |
Independent re-review of the suspension repair — still blockedReviewed head The previous replacement-turn reset finding is resolved by source inspection: acquisition waits through shutdown and stopped-event publication, and release waits on the same suspension ticket. The reviewer did not independently execute the added tests: locked installation encountered sandbox network restrictions, so Vitest never ran. Earlier implementer tests remain separate evidence. Remaining P1: idle shutdown can interrupt prompt admission
This is a source-traced finding, independently reported and cross-checked by the coordinator, not a claimed live reproduction. The ticket protects later acquisitions but does not reserve work that already obtained the runtime. Required repair: coordinate prompt admission/runtime use with idle shutdown ownership and atomically validate eligibility before shutdown. A phase-only check does not cover admission. Add deterministic deadline/admission and turn-start-publication interleaving regressions. Then obtain new CI, independent re-review and real-process/UI acceptance; no self-approval or merge is granted by a repair. No additional concrete security finding was established. Real child cleanup, same-session resume/history, interruption/release, disabled timeout, running/requires-action protection and applicable Web/Desktop proof remain acceptance obligations. |
|
auto-merge: no head out of
Din review-first: no squash-merge. |
Deadline/admission repair pushed — not yet independently acceptedFollow-up to the independent P1 finding.
Implementation and regression evidencePrompt lookup/acquisition/admission now holds a scoped runtime-use reservation. Idle shutdown validates reservation count, idle phase and timer generation before claiming ownership under the existing event-fold/publication lock. Shutdown still runs outside that lock, and the prior suspension ticket remains held through stopped-event publication. Scope finalization releases reservations on success, failure and interruption. The implementation worker demonstrated both deterministic regressions failing before the fix:
After correction, the coordinator reran the focused checks, then reran them again after main integration:
6 files / 91 tests passed; no type errors. Coverage includes both new races, concurrent reservation ownership/release, interrupted admission waiters, prior suspension/reacquisition/release cases, running/requires-action protection and disabled timeout. Server typecheck, explicit scoped oxlint, scoped oxfmt check and diff checks passed. Worktree is clean. This is implementation evidence, not independent acceptance. New required CI must finish successfully before a new independent full-PR review. Previous findings cannot be considered independently resolved solely by this repair record. Real-process cleanup/resume/history and applicable UI acceptance remain outstanding; no merge or deferred auto-merge has been armed. |
|
auto-merge: no head out of
fail closed; no merge / no update-branch. |
Current-base progress — waiting at required CIAfter independently verified #412 merged, this branch was updated without resolving conflicts to base/trusted rules Both preview checks and react-doctor succeeded; required Check is still queued at this observation. No old-head review/acceptance is carried forward. Current-head full review and independent E2E have not started because the CI-first gate has not passed. No merge, protection bypass or deferred auto-merge armed. Resume at successful required CI, then independent review and affected real-runtime acceptance; no routine human approval is being requested. |
|
auto-merge: no head out of
rules from |
Independent current-head acceptance — P1 queued-message loss; not mergedHead Previous findingsThe suspension ticket and scoped prompt-admission reservation address the two previously reported ownership/admission races. Independently executed 91 server tests across 6 files (including those regressions) and 45 chat tests; all passed. Full React Doctor scan: 292 files, 100/100. This resolves those specific findings, not the entire PR. New P1: idle suspension silently destroys queued user input after Stop
The trusted session-chat recipe explicitly requires Stop to leave the queue unchanged. Phase-idle is not equivalent to having no outstanding user work. Actual Electron reproduction, not a mock:
Attached screenshots show the same stopped session with the queued text and then without it. The 76.934-second, 60fps VP8 recording captures sending, queuing, stopping and the loss. The visible aborted-operation notice is caused by the intentional Stop; it is not the new finding. Required author follow-up: preserve pending user input across idle handling, or keep runtimes with pending work ineligible for idle disposal. Cover retained follow-up/steering after cancellation and subsequent user actions with deterministic regressions and real runtime proof. Any new persistence design still needs its normal design authorization. After an author update, restart new-head CI → full review → acceptance. Passed checks and evidence boundaries
Conclusion: blocked on reproducible user-input loss. No merge or repair attempted.
recording-001.webm
recording-001.webm |
Queue update after verified #423 merge — waiting at CIHead Required CI for this new head is queued/in progress at this observation, so no old-head review or acceptance is carried forward. The independently reproduced queued-message-loss blocker remains unresolved: #199 (comment) No merge or deferred auto-merge. Continue only after the applicable blocker is resolved and the new version passes required CI → review → independent acceptance. |
|
auto-merge: no head Out of the allowed groups; hits exclusions:
|
2 participants
Summary
idleforPIE_SESSION_RUNTIME_IDLE_MS(default 5 minutes;0disables), the session suspends it: kill the process without sealing the session.session.runtime.stopped(reason: idle). Session stays queryable; the next prompt re-ensureRuntimes as today.session.crashed). EventBus subscription keeps flowing — no forced client re-attach; process comes back on the next prompt.Why not client ChatManager LRU
Client unsubscribe does not free Pi child processes. This puts the control on the server where the cost is.
Test plan
packages/serversession tests:suspendRuntime+ short idle timeoutapps/appchat test:session.runtime.stopped→ ready, no error