Repository navigation
feat(chat): expose run diagnostics without child-driven parent keepalive - #696
Conversation
ObsisMc
left a comment
There was a problem hiding this comment.
Reviewed head f8ab7e06 (base 3fa5fc03), against ora-space/specs#145 head 09d5774a.
The runtime half looks right: projected child text/plan/tool/tool-delta and child permission round trips no longer reset or pause the parent window, a projected tool id stays excluded across metadata-free deltas, and the parent delegate's own in_progress path is untouched. The production-route test in prompt_policy_tests.rs exercises exactly the 45 s scenario. ADR status in the description is accurate (proposed → not mergeable yet).
One issue I'd like fixed before this goes further:
1. "Latest live activity" counts child projections, so the panel contradicts the runtime's own timeout.
observeDiagnostics (packages/chat/src/diagnostics.ts:26-41) updates lastActivity for any message/thought/tool/plan update, including ones carrying _meta["opencode/child-session"] (only session_info_update checks for it). Since this PR makes the runtime ignore exactly those events for liveness, an OpenCode v2 run where only a background child is streaming will show "No new activity for 2 seconds" under "Parent turn: Processing" while the parent prompt is about to be cancelled and re-sent at 45 s. That is the "background output read as parent progress" confusion #283's acceptance criteria and ADR D3/D4 want to avoid. Suggested fix: don't let projected updates advance the parent lastActivity (or record them as a separately labelled child activity), and add a store test feeding a projected chunk. "Reported active tools" (run-diagnostics.tsx:26-34) mixes child and parent tools the same way; at minimum mark the child ones.
Smaller notes (non-blocking):
store.ts:435-436: the doc comment "Flushes one buffered text batch…" now attaches tolet pendingDiagnostics, notflushPendingTextChunk.- Tests cover reload clearing and session isolation; ADR acceptance 5 also lists provider switch, failure, cancel and the workflow chat consumer. The clears are there in code (
store.tsagent-switch / error / abort / finally) but untested. - "Pending requests" can never show anything today because the actor auto-allows every permission before the UI sees it. You already say so in the run issue; maybe say it in the panel too (or in the doc section) so "No known pending requests" isn't read as "nothing is waiting on you".
- The child-session handling is validated only with synthetic frames. Worth one real OpenCode v2 capture in the run issue confirming the parent
tasktool's in-progress updates don't carryopencode/child-session(if they did, the delegate pause would be lost).
|
Addressed in
Chat: 48 passed. Diagnostic/workflow UI: 18 passed. Type/lint checks pass. The linked run also contains SHA-256-backed real v2 captures verifying 7 parent delegates / 14 in-progress updates carry no child projection marker. Runtime code is unchanged in this revision. |
ObsisMc
left a comment
There was a problem hiding this comment.
Re-reviewed head a4e0c848 (delta from f8ab7e06).
All points from the previous review are addressed:
- Parent
lastActivityno longer takes projected child updates; child activity has its own clock and label, and the silence counter only measures the parent stream. - Tool sources follow the runtime rule: a known child tool stays child across metadata-free deltas, the cache is cleared on retry, and unobserved tools show as "unconfirmed" instead of being counted as parent.
- The flush comment is back in place, and the panel and docs now say that auto-handled permissions and unsupported forms are not listed.
- New store tests cover provider switch, failure, cancel and new-prompt clearing. There is also a workflow consumer test.
- The real OpenCode v2.0.32 captures (7 parent delegates, 14
in_progressupdates, none withopencode/child-session) settle my concern about the delegate pause.
Locally on a4e0c848: packages/chat deno tests 48 passed; app-shell run-diagnostics + run-node-session-chat 18 passed. I did not re-run the Rust suite (runtime unchanged in this revision).
Non-blocking nit: after an Ora inactivity retry, the diagnostics are cleared, so the panel shows "Unknown; replay is not new activity" during the new attempt. Nothing is being replayed at that point, so a neutral "no activity observed in this attempt yet" would be more accurate.
Merge is still gated on ora-space/specs#145 reaching approved, as the description says.
a4e0c84 to
08295ae
Compare
ObsisMc
left a comment
There was a problem hiding this comment.
审查 head 08295ae。Rebase 后前 6 个提交与此前批准的 a4e0c84 一致(range-diff 无差异),D3/D4 的主体结论不变。新提交 34d0621 有一处需要修改:
阻塞:run-diagnostics.tsx:71-76 只用 isResponding 区分"新尝试"和"回放"。但 loadSession 回放未结束的回合时,preview() 会设置 isResponding: hasOpenTurn(store.ts:1031),且不写入 diagnostics。工作流自动首个 prompt、重开运行中的会话都走这条路径(store.ts:352、run-node-session-chat.tsx:81)。结果是回合内容在逐段出现,面板却显示"本次尝试尚未观察到活动",把"未知"说成了"没有",与 ADR 要求的加载流标明未知不符,也与本提交保留历史重放文案的意图相反。
建议:面板同时考虑 isLoading(isResponding && !isLoading);或由 store 在 sendMessage 和重试时把 diagnostics 设为 {},面板用 diagnostics === undefined 表示未知/回放。请补一个 load 流中有未结束回合时仍显示回放文案的测试。
非阻塞:
- 只有子代理活动时,"最近父回合活动:本次尝试尚未观察到活动"容易被读成整个尝试都没有活动,建议改为"本次尝试尚未观察到父回合活动"。
- 子权限往返不续期目前只有 PromptLiveness 单元测试,actor 路由测试没有覆盖权限;建议补一个用例,或调整 PR 描述里的说法。
After an inactivity retry clears the diagnostics, the panel said the replay was not new activity even though nothing was replayed. A responding turn with no observation now says no activity was observed yet; a loaded history keeps the replay wording. Prettier also wraps one pre-existing long string in the same file.
`loadSession` publishes an unfinished replayed turn as responding without recording live observations, so the panel turned that unknown into "no activity observed in this attempt yet" - including workflow-owned first prompts and reopened running sessions. An empty diagnostics snapshot now means a live attempt is being observed, and `sendMessage` plus an inactivity retry start one; replay keeps none. The fresh-attempt wording names the parent stream, and an attempt that ended without observations drops "yet".
…policy The child-permission exemption was covered only by PromptLiveness unit tests. Let the fake provider request permission at the real ACP boundary and assert through the routed actor that a projected child round trip leaves the parent deadline alone while a parent round trip rearms it.
08295ae to
e988bee
Compare
ObsisMc
left a comment
There was a problem hiding this comment.
Approve @ e988bee。
增量复审:range-diff 显示原 8 个提交与 08295ae 一致,只新增 9e1d0d4 和 e988bee;已追平 main be72c25,crates/history 与 main 无差异。CI 8 项全绿。
- 阻塞项已修复:sendMessage 与不活动重试把 diagnostics 置为 {},loadSession 回放保持 undefined。面板先看 diagnostics 是否存在,回放(包括未结束回合、工作流自动首个 prompt)显示“未知;历史重放不代表刚发生的活动”,实时尝试才显示“尚未观察到父回合活动”。RunDiagnostics 字段全可选,{} 合法;observeDiagnostics 与 retry/compaction 分支对 {} 都按已有快照合并,没有行为回退。store 和面板都补了加载流用例。
- 文案:“本次尝试尚未观察到父回合活动”/“本次尝试未观察到父回合活动”区分进行中与已结束,避免与“未在处理”矛盾。
- 权限往返:e988bee9 让 fake provider 走真实 ACP session/request_permission,actor 路由用例证明子工具权限往返不续期、父工具权限往返重新计时,PR 描述的说法现在有直接证据。
Summary
在普通聊天和工作流对话中展示只读运行诊断:独立的父/子活动接收时间、按来源分组的当前工具、已知待处理请求和明确上报的 provider 重试/压缩状态。历史重放不会伪造最近活动,静默不被称作卡死。
Refs ora-space/circus#301
ADR 状态
实现
specs/decisions/node/agent-runtime/20261020-child-projection-history-and-diagnostics.md(ora-space/specs#145,head23de5ef,merge commite473b2581ec174953db8573d884d5996d0abad49);该 ADR 已以approved合并到 specs main。本 PR 实现 D3 的运行观察与 D4 的父子存活隔离:
in_progress、Timeout/Wait、取消与错误收尾不变。不在本 PR 范围:history 来源保真由 #695 承接;只读子代理详情树与「委托已返回结果 / 后续未知(投影已停止)」两种标注由 #697 承接。三份 PR 合并并验收后,由落地运行在 specs 开 PR 把该 ADR 改为
implemented。Evidence
-D warnings通过。sendMessage与重试开始一个空的实时快照,因此「本次尝试尚未观察到父回合活动」只用于确有实时观察的尝试;加载流重放未结束回合时保持「未知;历史重放不代表刚发生的活动」,已结束且未观察到活动时去掉「尚未」。加载流本身不记录实时活动。be72c252(含 fix(history): preserve projected child attribution across reload #695),crates/history与 main 无差异;当前 heade988bee9。Merge Danger
Door: two-way
Blast Radius: runtime
不更改 Timeout/Wait 配置、窗口、重试次数或 JSONL。无法证明被等待的投影子活动不能独立保活;真正等待的父委托应继续上报
in_progress。诊断只读,不增加子代理控制或健康判定。