fix(workhub): admit coordination actions through Runtime turns - #4993
fix(workhub): admit coordination actions through Runtime turns#4993ARE404 wants to merge 9 commits into
Conversation
f1557a6 to
b8d69b4
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for moving WorkHub actions onto real Runtime-owned Turns and removing the synthetic assistant/terminal path. I checked this against #3492's admission follow-up, the ADR, and the production callers at b8d69b4593cba75964596e705f934a7bb4134e15. The direction fits the existing authority boundaries; I did not find a second execution authority.
[P1] Complete the production transcript cutover. On the normal clarification/resume path, the new receipt is persisted only in RuntimeEvents (runtime-kernel.ts:1021–1042), but the production reader wired by execution-composition.ts:621–632 still routes the Coordination Session exclusively to the legacy SessionStore reader (session-transcript-reader.ts:93–101, 571–603). Once the Turn is terminal, its active overlay is also empty (:119–120). Consequently, successful clarification/resume acknowledgements disappear from durable pages on completion/reopen; existing delegation/Stop rows remain visible because they still use the old store. The new test's direct readLedgerMessages assertion bypasses this reader, while the existing reader test explicitly expects zero ledger reads.
Could we finish this as one bounded production transcript projection that consumes Runtime receipts while preserving existing atomic delegation/link facts and released history? Simply switching to ledger-only reads would lose linkage rows; writing receipts back to session_messages would recreate the duplicate path this repair removes. The regression should go through the real action handler and session.transcript.page after completion and restart, checking clarification/resume, retained links, and pagination. Please include same-action retries so their physical Turns do not become duplicate conversation entries.
[P2] Recover acknowledgement from the target admission when the receipt is missing. This is a remaining acceptance gap in the new retry contract, rather than a newly introduced duplicate-resume bug. A Host crash after turn.resume.start commits but before runtime-kernel.ts:1042 leaves a durable target resume admission without a Coordination receipt. The retry loop then reruns the callback. resumeDelegation follows the latest target lineage and replans, but reuses workHubResumedTurnId(actionId); after that continuation is interrupted, startTurnResume rejects the same ID against the changed source boundary (root-turn-coordinator.ts:2103–2112). The original successful admission cannot be acknowledged by retrying the same action.
Please consult the existing deterministic target admission before planning another resume and reconstruct its original acknowledgement. No separate resume journal is needed. A production fault-cut test after target admission and before receipt persistence should reopen the Host, retry the same action, and assert the original target Turn is acknowledged without another continuation.
I left the local retired-grant migration issue inline. Current CI passes build/typecheck and the affected workspace/Host suites, but fails the published-baseline credential qualification; Desktop E2E and subsequent browser checks were skipped. Review used five independent reviewer passes plus source-level reconciliation. An exact-head reader probe with stub stores reproduced zero durable records, zero completed overlay messages, and zero ledger reads despite an available receipt; it did not launch a full Host/SQLite/Desktop. No local full-suite or rendered UI validation is claimed.
简体中文
感谢把 WorkHub 动作接入真正由 Runtime 管理的 Turn,并删除假的 assistant/终态写入。对照 #3492、ADR 和当前生产调用链,方向正确,没有发现第二个执行权威。
P1:生产 transcript 的切换尚未完成。 新澄清、恢复回执只写入 RuntimeEvent,但真正接入 Host 的 reader 仍对 Coordination Session 只读旧 SessionStore;Turn 完成后 active overlay 也为空。因此动作虽然成功,完成后或重新打开时却读不到这些持久回执。旧委派/Stop 行因仍在旧存储而可见。新增测试直接读 ledger,绕过了生产 reader;旧 reader 测试甚至仍要求 ledger 读取次数为零。
建议在共享、有界的生产投影处完成收敛,消费 Runtime 回执并保留原子委派/link 事实和已发布历史。直接改成只读 ledger 会丢 link;把回执再写回旧表则会重建被删除的重复路径。回归应经过真实 action handler 和 session.transcript.page,验证完成后、重启后、分页及同 action 重试的结果,避免物理重试 Turn 变成重复会话条目。
P2:回执缺失时,恢复确认仍未从 target admission 收敛。 target resume 已提交、Coordination 回执尚未写入时崩溃,同 action 重试会沿最新 lineage 重新规划,却复用原 target Turn ID;后续 continuation 中断后,会因 source boundary 不同而冲突。它不会重复恢复,但无法补回已经成功的原始确认。这是本次重试承诺仍未覆盖的边界,不是说 PR 新增了重复 resume。
应先读取该 action 对应的确定性 target admission,重建原确认,再决定是否需要规划。无需新增 resume 日志。测试应覆盖 target admission 后、receipt 前的故障切点,重启后同 action 重试,并确认返回原 target Turn、没有新增 continuation。
另一个局部的旧 grant 迁移问题已放到行内。当前 CI 构建、类型检查及受影响 workspace/Host 测试通过,但发布基线凭据检查失败,后续 Desktop E2E/浏览器检查被跳过。本轮使用 5 位普通 reviewer 和主审源码交叉核对,未声称完成本地全套测试或界面验收。
b8d69b4 to
0737751
Compare
6854cbd to
0970c58
Compare
There was a problem hiding this comment.
Thanks for completing the previous repair. Re-reviewed 0454c2784369aecbb6face7437e9bbd4cada0871 with five focused reviewers and primary-agent source/probe reconciliation. The previous receipt-reader, missing-resume-acknowledgement and retired-grant findings are addressed. Runtime owns admission, Run completion and recovery; the target admission supplies the original resume acknowledgement. Receipt bodies stay in RuntimeEvents, while the existing atomic delegation/Stop/link facts retain their own responsibilities. I found no second execution authority or duplicate receipt store.
[P2] Keep index catch-up out of an unbounded foreground page read. This is a normal first-open/upgrade path, or a read after unindexed history accumulates. readDurablePage calls readHighWater, which synchronously runs mergeTranscriptSources.refresh() until both sources are exhausted (session-transcript-reader.ts:698–730). The 64-reference batches bound each transaction, not the complete request. Moreover, the sources return projected message bodies: the legacy store decodes each returned record before this code retains only its reference.
I reproduced this through the real reader and SQLite Session store with legacy history and an empty Runtime source. For the same newest-page request (maxBytes: 128, maxMessages: 1), 257 historical rows caused 7 source-page calls, 321 decoded rows and 5 index writes; 10,000 rows caused 159 calls, 10,064 decoded rows and 157 index writes. The extra 64 rows are the final page-source batch. The responses were only 83 and 88 bytes. This is measured work amplification, not a claim of measured production outage. It restores full-history work on the opening path that #4699 removed, even though subsequent reads can reuse the index.
Could we keep foreground reads bounded at the existing source/index seam and make any necessary catch-up an explicit resumable operation, or use source metadata/reference reads that satisfy the requested page without projecting the entire backlog? A hard truncation presented as complete history would not fix the contract. Please preserve stable snapshots and the existing Runtime/link authorities, and add one growth regression through the production reader that checks work done before a small page returns. The inline P2 covers a separate failed-action visibility gap.
Invariants the repair should preserve
- Bound the whole foreground read, before materialization. A page's byte/message budget must not hide an unbounded source walk or index catch-up. Required catch-up needs explicit progress/completeness semantics; stopping early must not report unvisited history as absent. Existing snapshot/cursor stability must survive the repair.
- Deduplicate only against an available visible replacement. Every admitted, persisted user input must remain reconstructible in the conversation after failure/reopen. A receipt or atomic link may replace its display; the expectation that one will eventually exist cannot justify hiding it. A failed action must retain its input and terminal status without duplicating successful action cards.
- Recover from the existing owners. RuntimeEvents retain the input, receipt and Run terminal facts; the target Session admission retains the accepted effect. A missing Coordination receipt must not cause another target continuation. Keep the index derived and body-free; do not repair either gap by adding a second summary store, resume journal, or independent execution state.
For acceptance, please extend the existing small-page growth test to account for total source reads/decodes/index work, and the existing missing-receipt fault-cut test to assert visibility after reopen and before retry, then the original target acknowledgement and one logical action row after retry. These assertions protect the contracts directly; an extra per-batch LIMIT or a special case for the example action would not be sufficient.
Validation: 29 focused tests passed through an exact-head source loader (13 production WorkHub/Coordination composition cases, 7 reader cases and 9 grant-migration cases), plus the real-SQLite growth probe and the existing missing-receipt fault test instrumented before retry. This is not a fresh workspace build or independent Electron acceptance. Current-head CI is green; the PR currently conflicts with main. No request to expand the model-routing scope or broadly rewrite the tests. AI-assisted review.
简体中文
感谢补齐上一轮修复。五个 reviewer 分工并经主审源码与探针核对,旧的生产回执读取、缺失 resume 确认、退役 grant 三项问题已解决。Runtime 仍持有准入、Run 终态和恢复;原始恢复确认来自 target admission。没有发现第二执行权威或重复回执存储。
P2:索引追赶不能让前台小页请求遍历完整历史。 正常升级后首次打开,或存在未索引积压时,readHighWater 会等待 refresh 将两个来源耗尽。每批 64 条只限制单次事务,不限制整个请求;来源还会先解码完整消息,之后才留下引用。真实 reader + SQLite 探针请求最多 128 字节、1 条消息:257 条历史产生 7 次 source page、321 条解码、5 批索引写入;10,000 条历史产生 159 次读取、10,064 条解码、157 批写入。额外 64 条来自最终页的 source batch,实际响应只有 83/88 字节。这里证明的是工作量放大,不声称已经测得生产故障。它把 #4699 删除的首次打开全历史工作重新带回前台。
建议在现有 source/index 接缝保证前台读取预算,将必要追赶明确做成可恢复维护,或只读取足够满足页面的元数据/引用;不能截断后冒充完整历史。保留稳定快照和现有 Runtime/link 权威,加一项经过生产 reader 的规模增长回归。行内另有失败动作显示缺口。
修复时需要保持的不变量
- 整个前台读取有界,预算在物化前生效。 页面的字节/消息预算不能隐藏无界 source 遍历或索引追赶;必要追赶必须有明确的进度与完整性语义,不能截断后把未访问历史当成不存在。修复仍须保持已有快照和游标稳定。
- 仅在可见替代记录已存在时去重。 每个已准入、已持久化的用户输入,故障后重开仍应能在对话中重建。回执或原子 link 可以替代它的显示,但“预计以后会出现”不能成为提前隐藏的依据。失败动作保留输入及终态,成功动作仍只有一张逻辑卡片。
- 恢复使用现有事实权威。 输入、回执及 Run 终态归 RuntimeEvent,已接受的效果归 target Session admission;缺少 Coordination 回执不能触发另一条 target continuation。索引保持派生且不存正文,不增加第二个 summary store、resume 日志或独立执行状态。
验收请扩展现有小页规模测试,统计整个请求的 source 读取、解码及索引工作量;扩展现有 missing-receipt 故障测试,检查重开后、重试前的失败输入可见性,再检查重试后仍确认原 target Turn、仅显示一个逻辑 action。这些断言直接保护契约;再加一层 batch LIMIT 或只特判示例 action 都不足以解决根因。
主审精确源码加载测试 29 项通过:生产组合 13、reader 7、grant 9;另有真实 SQLite 增长探针和 missing-receipt 故障测试在重试前的观测。不代表全新构建或独立 Electron 验收。当前 CI 绿色,但与 main 冲突。不要求扩展模型路由范围或大改测试。AI 辅助评审。
Generated-by: Codex
…t inputs Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Preserve the typed stale-candidate refusal through Host and IPC, and retry only the same action and selected Session with fresh opaque references. Bound retries and retain fail-closed behavior for changed targets and other failures. Stabilize E2E terminal and title setup, and align the preload fixture with removed synthetic recording. Generated-by: Codex
0454c27 to
a637d1a
Compare
There was a problem hiding this comment.
Thanks for addressing the previous read-amplification and failed-input visibility findings. I rechecked a637d1a5 with seven fresh reviewer passes and independently reconciled the candidates against the production paths. The direction still fits #3492 and the WorkHub ADR: Runtime owns admission, execution and terminal facts; the transcript index stores source references, and missing-receipt recovery consults the existing target admission. I did not find a second execution authority or a justified need for a broader rewrite.
Two localized P2 regressions remain, detailed inline. The repair invariants are:
- Action identity remains bound to the incoming payload, including concurrent replay. An existing admission can supply the result after validation; it must not replace the incoming execution descriptor before that validation.
- Preparation and later reads have separate budgets. The bounded open deadline limits index preparation; a successfully opened subscription must retain the intended per-request timeout for later pages and overlay release.
Validation: 89 focused tests passed through an exact-head source loader (57 reader/client/pager, 13 production composition, 19 Desktop projection). I additionally reproduced both findings using the existing real SQLite/Host composition and IPC peer fixtures; removing the execution override and restoring the page request budget respectively made the failing probes pass. No fresh workspace build or independent Electron UI acceptance is claimed. Current-head CI is green. AI-assisted review.
Follow-up: reduce the remaining maintenance surface in this PR (P3, separate from the two correctness findings). A further simplification pass found a retired effect and duplicate test coverage, rather than evidence for a second execution authority. Please make the following deletion pass as part of the same revision:
- Remove the obsolete clarification effect and its mock/spy scaffolding together, as detailed inline. Preserve the Runtime receipt and action-claim behavior.
- In
execution-composition.test.ts, remove the oldreadLedgerMessagesreceipt assertion around lines 899–908. Extend the existingreadProductionTranscriptresult assertion to check both the create and clarification action receipts. Also remove the repeated bidirectional single-message paging loop around lines 961–976: the real-SQLite reader case already covers that algorithm, including reader recreation, late append and regressing clocks. Keep the composition's physical-Turn landmark assertion and its actual subscription/page path. - Fix the execution-identity P2 by deleting the redundant stored-descriptor override and its now-unneeded second read, using the existing admission validation. Avoid adding another comparison layer around the override.
- Please confirm whether a deployed version that we need to support actually wrote
operation: 'action'admissions withoutactionId. The current writer always supplies it, while the orphaned-admission fixture still omits it. If there is no such compatibility obligation, update that fixture and tighten the action descriptor/decoder so the missing-actionId retry compatibility branch can disappear. If there is an obligation, identify the version and retain only the narrow normalization needed; this is not a request to remove compatibility without checking.
The execution-override, clarification-effect and duplicate-test ablations each passed the 13 focused composition cases through the exact-head source loader. These are scoped deletion checks, not a fresh typecheck/build. Please retain the distinct restart/fault-cut cases, the body-free source index, and the legacy atomic linkage facts. The aim is to remove replaced mechanisms and duplicate protection, not to hit an arbitrary line-count target or add a new generic framework.
简体中文
感谢修复上一轮的读放大和失败输入可见性问题。本轮对 a637d1a5 派了七位 fresh-eye reviewer,并由主审对照生产路径独立核实候选。方向仍符合 #3492 和 WorkHub ADR:准入、执行及终态归 Runtime;索引只保存来源引用;回执缺失时读取现有 target admission。没有发现第二个执行权威,也没有证据要求大范围重写。
仍有两个局部 P2,已写行内评论。修复需要保持两个不变量:
- 并发重放也必须保持 action 身份与当前请求内容的绑定。 校验通过后可以读取原 admission 的结果,但不能先用原 execution 覆盖当前请求,再进行校验。
- 准备阶段与后续读取分别计预算。 打开订阅的 deadline 用于限制索引准备;打开成功后,分页和 overlay release 仍应使用各自应有的请求超时。
精确源码加载验证 89 项通过:reader/client/pager 57、生产组合 13、Desktop 投影 19。主审另外使用现有真实 SQLite/Host 组合和 IPC peer 夹具复现两项问题;分别移除 execution 覆盖、恢复分页请求预算后,失败探针均通过。不代表全新 workspace 构建或独立 Electron 界面验收。当前 head CI 全绿。AI 辅助评审。
补充:在本 PR 内收敛剩余维护面(P3,与上面两个正确性问题区分)。 专门的精简审查找到了旧 effect 残留和重复测试,没有证据表明新增了第二执行权威。建议同一轮修改完成以下删除任务:
- 按行内意见,一起移除旧 clarification effect 及 mock/spy;保留 Runtime receipt 和 action claim。
- 删除
execution-composition.test.ts约 899–908 行旧的readLedgerMessages回执断言,改为在已有readProductionTranscript结果中同时检查 create 和 clarification 的回执。删除约 961–976 行重复的双向单消息分页循环;真实 SQLite reader 用例已经覆盖该算法、reader 重建、晚到数据和时钟回退。保留生产组合的物理 Turn landmark 断言及真实 subscription/page 路径。 - execution 身份 P2 优先通过删除 stored-descriptor 覆盖及因此不再需要的第二次读取来修复,继续使用已有准入校验,不在错误覆盖外围再叠一层校验。
- 请确认是否实际部署过需要支持的版本,写入了
operation: 'action'却没有actionId的 admission。当前 writer 始终填写该字段,但 orphaned-admission 夹具还省略它。若没有这项兼容义务,更新夹具、收紧 action descriptor/decoder,并删除缺失 actionId 的 retry 兼容分支;若有,请指出版本,只保留必要的窄范围归一化。这不是要求未经核实就删除兼容。
execution 覆盖、空澄清 effect、重复测试这三类消融分别通过了精确源码加载下的 13 项生产组合测试,不代表重新 typecheck/build。不同重启/故障切点的用例、只存来源引用的索引及旧原子关联事实仍应保留。目标是删除被替代的机制和重复保护,不是凑回某个行数,也不需要增加通用框架。
| } | ||
| }, | ||
| // Clarification is recorded by the admitted Run as a host receipt. | ||
| clarify: async () => undefined, |
There was a problem hiding this comment.
[P3] Remove the retired clarification effect rather than retaining a no-op port. The normal clarification path now persists its receipt through the admitted Runtime operation, so this is the only production implementation of WorkHubActionGateEffects.clarify, and it does nothing. The Gate still calls it, while the Gate fixture records a clarifications array and asserts that this obsolete call occurred. Could we remove the interface member, Gate call, this no-op, and the corresponding fixture array/method/type/count assertion together? Keep the action claim, returned clarification identity, and the production receipt/reopen assertions. This removes a retired write-path abstraction and tests that preserve it; it does not remove clarification behavior. A source-loader ablation removing the call and production no-op passed all 13 focused WorkHub/Coordination composition cases.
简体中文
P3:删除已退役的 clarification effect,不保留空接口。 正常澄清现在由已准入的 Runtime operation 持久化 receipt;这里是 WorkHubActionGateEffects.clarify 唯一的生产实现,已经没有任何行为,但 Gate 仍调用它,测试夹具还记录 clarifications 数组并断言旧调用发生。建议一起删除接口成员、Gate 调用、空实现,以及对应夹具数组/方法/类型/计数断言。保留 action claim、返回的澄清身份和真实 receipt/reopen 验证。删除的是旧写路径抽象和维持它的测试,不是澄清功能。主审通过源码加载消融移除调用和空实现后,13 项 WorkHub/Coordination 生产组合测试全部通过。
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for the latest fixes. Fresh-eye review of 69c83c7264daffd5a31e26d0e7384d53c2910fd7 used six focused reviewers, followed by primary-agent source checks and probes.
Two P2s remain, detailed inline: released clarification facts no longer replay after upgrade, and the new clarification receipt identity interacts with the old visibility filter so a lost response can hide an already committed answer. Both concern convergence on existing durable facts, not another execution engine. The fix should preserve same-content replay and let committed receipts outrank local transport uncertainty.
The previous execution-descriptor overwrite and preparation-timeout findings are fixed. I reran both original probes: they fail on a637d1a58 and pass here. All 89 focused Host/client/reader/pager tests passed. The new upgrade probe uses a real base-written SQLite database; the visibility comparison uses real Host/reader output with the surface's failed-local state and source-level projection/filter calls. These are not full rendered UI or Electron acceptance runs.
No new execution authority was found: Runtime Host retains Session/Turn/Run ownership, RuntimeEvents own receipts, and the derived index stores references rather than copied receipt bodies. I did not retain the proposed live-index P1 because its required production growth path was not demonstrated.
The earlier clarification no-op and composition-test cleanup suggestions still apply. One additional P3 inline suggests folding the new timeout regression into the existing fragmented-page scenario. Missing-actionId compatibility is conditional on whether an intermediate PR version was actually deployed; repository history alone does not justify deleting it.
Current-head CI is green; the latest mergeability query returned unknown. This is a COMMENT review; no approval or merge was performed. AI-assisted review; manual UI acceptance is not claimed.
简体中文
感谢本轮修复。基于 69c83c72,六位 reviewer 独立分工,再由主审核对源码和探针。
仍有两项 P2,详见行内: 已发布版本的澄清事实升级后无法按原身份重放;新澄清 receipt 身份与旧可见性过滤不兼容,响应丢失时会隐藏已提交回答。两者都是未向既有 durable 事实收敛的问题,不是又建了执行引擎。修复应保持同内容幂等重放,并让已提交 receipt 优先于本地传输不确定性。
上轮 execution 覆盖和准备超时问题已修复:两项原探针在 a637d1a58 失败、当前 head 通过。89 项 Host/client/reader/pager 定向测试全部通过。新的升级探针读取真实 base 写出的 SQLite;界面对照使用真实 Host/reader 输出,加上 surface 的本地失败状态,调用源码中的 projection/filter。这不等于完整渲染界面或 Electron 验收。
没有发现新的执行权威:Session/Turn/Run 仍归 Runtime Host,receipt 归 RuntimeEvents,派生索引只存引用,不复制正文。候选 live-index P1 缺少已证实的生产增长路径,未保留为评审问题。
此前空 clarification effect 和 composition 重复测试的精简建议仍适用。本次补一项 P3:将超时回归合入现有分片分页场景。缺 actionId 的兼容能否删除,取决于是否真实部署过 PR 中间版本,不能只凭仓库历史直接删。
当前 CI 全绿,最近一次查询的合并状态为 unknown。本次仅 COMMENT,未批准或合并。AI 辅助评审,未宣称完成人工界面验收。
| }, | ||
| archivedMessage: 'WorkHub Coordination Session is unavailable', | ||
| prepareFreshContent: async () => | ||
| (await this.#readSummaryMessages(coordinationTurnId)).length > 0 |
There was a problem hiding this comment.
[P2] Preserve replay of an already committed legacy clarification. On the supported upgrade/recovery path, WorkHubSendLease can retain the original action ID after the old Host committed clarification but the response was lost. The old implementation wrote its summary under this same wha_… Turn ID. This unconditional existence check now rejects an identical replay before the Gate can validate the original action claim. I wrote the clarification through the real base Host (42fa4d07), closed it, and reopened the same SQLite state: base accepts the identical replay and rejects changed content; this head rejects both. Desktop then reports “This action changed” and abandons the retained ID, requiring another send. History remains intact, so this is P2, not data loss.
Please preserve the invariant same action + same committed content returns the original acknowledgement; changed content conflicts, including released legacy clarification facts. Validate against the existing summary and action claim at the Host boundary and provide a narrow replay/migration path; do not restore the retired writer or add another receipt store. A base-write → current-reopen → same/changed replay regression should cover this.
简体中文
P2:保留已提交旧澄清的幂等重放。 在升级/恢复路径上,旧 Host 已提交澄清但响应丢失时,WorkHubSendLease 会保留原 action ID。旧实现把 summary 写在同一个 wha_… Turn 下;这里仅因旧记录存在就拒绝相同重放,尚未进入 Gate 对原 action claim 的校验。主审用真实 base Host(42fa4d07)写入并关闭,再打开同一 SQLite:base 对相同内容成功、不同内容冲突;当前 head 对两者都冲突。Desktop 随后误报“动作已变化”、清除原 ID,要求重新发送。历史未丢失,定为 P2。
应保持不变量:同一动作、相同已提交内容返回原确认;内容不同才冲突,包括已发布版本的澄清事实。建议在 Host 边界利用现有 summary 和 action claim 校验,提供范围明确的重放/迁移路径;不要恢复旧 writer 或另建 receipt store。用 base 写入→当前版本重开→相同/不同内容重放验证。
| const earlier = receiptIndexByActionId.get(receipt.actionId) ?? latestUserIndexByTurnId.get(message.turnId); | ||
| const row = { | ||
| messageId: message.id, | ||
| turnId: receipt.actionId, |
There was a problem hiding this comment.
[P2] Reconcile the new receipt identity with the final visibility filter. A clarification can commit successfully while its RPC/IPC response is lost. The surface then retains a local failed row keyed by the action ID. Mapping the durable receipt to that same ID is appropriate, but visibleWorkHubConversation (workhub-surface.tsx:133–150) treats that local failure as grounds to hide the completed durable row. The user sees only a delivery failure, although the clarification answer is available.
This is a new clarification-path regression: the base summary projects under its physical wha_… ID and remains visible in this situation; the new receipt projects under actionId and is filtered out. A source-level probe using each version's real Host writer, production reader, projection and unchanged visibility function confirms base keeps the answer while this head returns zero visible durable rows for the same failed-local state.
Keep the stable action identity and fix the consumer: a committed receipt must outrank uncertainty about its transport acknowledgement. Replace the corresponding failed/routing placeholder when a durable result is available, while preserving necessary clarification controls and every physical failed input on pages without a replacement receipt. Do not collapse all retry inputs by action ID.
简体中文
P2:新 receipt 身份需要接通最终可见性过滤。 澄清可能已提交成功,但 RPC/IPC 响应丢失,surface 因而保留以 action ID 为键的本地 failed 行。把 durable receipt 映射到同一 ID 的方向正确,但 visibleWorkHubConversation(workhub-surface.tsx:133–150)会因本地失败而隐藏已完成的权威结果,用户只看到发送失败,看不到已经存在的澄清回答。
这是新增澄清路径的回归:base summary 使用物理 wha_… ID,此时回答仍可见;新 receipt 使用 actionId,恰好被过滤。主审分别使用两版真实 Host writer、生产 reader、projection 和原样 visibility 函数验证:相同的本地失败状态下,base 保留回答,当前 head 的可见 durable 行为零。
保留稳定 action identity,修正消费者:已提交 receipt 的事实优先于传输确认的不确定性。已有 durable 结果时替换对应的 failed/routing 占位,同时保留必要的澄清控件,以及没有替代 receipt 的页面上的各次物理失败输入;不要按 action ID 一刀切合并所有重试输入。
| ); | ||
| }); | ||
|
|
||
| test('keeps page timeout independent of index preparation time', async () => { |
There was a problem hiding this comment.
[P3] Fold this regression into the existing fragmented-page scenario. The preceding test already constructs the same two fragments and verifies backward-page hydration; this adds almost another copy of that protocol exchange. The preparation delay and independent page budget are valuable and must stay, but they can be added to that existing scenario (or its existing fixture) while retaining its reconstructed-message assertion. This would protect both obligations without maintaining two near-identical exchanges; no general test framework is needed.
简体中文
P3:把这项回归合入已有分片分页场景。 前一项测试已经构造相同的两段分片并验证向前补齐,这里几乎又复制了一遍协议交互。准备耗时和独立分页预算的保护有价值,必须保留;可直接加入原场景或现有夹具,保留完整消息重组断言,以一套交互保护两项义务,不必再造通用测试框架。
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for the continued fixes. I completed another operation-chain review of 69c83c7264daffd5a31e26d0e7384d53c2910fd7 with eight additional focused reviewers and primary-agent adjudication.
[P2] Recover an already admitted Resume before applying fresh target-name checks. This is a supported failure/recovery path: the target continuation admission commits, the Coordination receipt write fails, the Host reopens, and the target is renamed before retrying the original action. The existing production composition test promises the original resume_started acknowledgement, but the Gate checks the current candidate/name first (workhub-coordination-action-gate.ts:497–524), so execution never reaches the existing admission lookup (execution-composition.ts:1433–1447). The unchanged request fails with operation_conflict: WorkHub resume target is not affirmed in trusted user text.
I reproduced this by adding a real manager.renameSession() before retry in the existing “WorkHub Resume and Stop across restart” test: receipt-present replay passes; missing-receipt replay fails. Both current head and a637d1a58 behave this way. This is a newly found gap in this PR's recovery contract, not a regression introduced by the latest fix. The ADR explicitly requires consulting the deterministic target admission first (lines 104–106).
Please recover the original acknowledgement from that existing admission after validating the immutable action descriptor and original target/delegation identity. Keep fresh candidate/name/ownership checks for an action that has not committed its target admission. No new recovery store or parallel claim authority is needed. Extend the existing fault-cut test with the rename and assert the original target Turn, one receipt, and no new continuation.
The previously reported visibility P2 also affects Resume receipts after response loss: the same action-ID projection and local-failed filter hide the committed result. This is another affected consumer of the same finding, not an additional issue; please cover it in the same convergence fix while preserving local clarification controls where they are still needed.
Another 140 targeted Desktop tests passed, including controller, surface flow, port, IPC and real preload fragmentation. These are source-based exact-head tests, not rendered Electron acceptance. I excluded candidates that depended on an unsupported cancellation contract or already failed on base. No second execution authority was found. CI remains green; this is a COMMENT, with no approval or merge.
简体中文
感谢继续修复。本轮在同一 69c83c72 上增加了八位 reviewer,沿完整操作链审查,再由主审复现和裁决。
[P2] 已准入的 Resume 应先恢复原确认,再考虑首次执行的目标名称校验。 可达链路是:目标 continuation 已准入 → Coordination receipt 写入失败 → Host 重启 → 目标正常改名 → 原 action 重试。Gate 先检查当前名称,导致既有 target admission 的恢复查询无法到达,原请求返回“目标未在用户文本中明确指定”的冲突。
我在已有生产 composition 测试的 retry 前调用真实 manager.renameSession():receipt 已写入时通过,receipt 丢失时失败。当前 head 和 a637d1a58 都一样。这是本 PR 新增恢复承诺中此前漏查的分支,不是最新修复引入的回归;ADR 第 104–106 行明确要求先查询确定性的目标准入事实。
修复应验证不可变 action descriptor 和原目标/委派身份,再从既有 admission 返回原确认。尚未提交目标 admission 的首次执行仍保留完整的候选、名称和归属校验。不需要新增恢复存储或并行 claim 权威;直接扩展已有故障切点测试,断言原 target Turn、单条 receipt、没有新 continuation 即可。
此前的可见性 P2 也影响响应丢失后的 Resume receipt,根因仍是同一 action-ID 投影与 local-failed 过滤,不另算一项。请在同一收敛修复中覆盖,同时保留仍有必要的本地澄清交互控件。
另有 140 项 Desktop 定向测试通过,覆盖 controller、surface flow、port、IPC 和真实 preload 分片路径;这是源码级固定 SHA 验证,不等于 Electron 界面验收。未采纳缺乏取消契约的候选或 base 已有的旧问题。没有发现第二个执行权威。CI 全绿,本次仅 COMMENT,未批准或合并。
|
Thanks for working through these rounds. I think the next pass should consolidate the cumulative change around its invariants, rather than treat each review comment as a separate patch. A correction from our side: the latest findings also reproduce on The recurring technical pattern is that individual layers work in isolation, but their decisions do not compose: replay goes through checks intended for a fresh action, and a committed Host result can still lose to a local transport-failure placeholder. This is not evidence of another execution engine. It is a gap in convergence on the existing authorities. The prompt below is intended to close those gaps without growing another recovery or projection mechanism. Copyable prompt for the implementation agent
简体中文感谢持续修复。下一轮建议围绕不变量收敛累计改动,不再把每条 review 当作独立补丁任务。 我们也需要纠正归因:最新发现的问题在 反复暴露的技术模式是:单层各自成立,但连起来不闭合——重放被首次执行的校验挡住,已提交的 Host 结果又被本地传输失败占位状态覆盖。这不是发现了第二个执行引擎,而是尚未向既有权威完整收敛。 可直接交给实现 agent 的提示词(中文对应版) 修复 #4993,只完成原 Coordination Turn/receipt 契约所需的最小累计改动。先刷新 live head;本轮审查基于 改代码前,比较整个 PR 与当前 base,提交一份简短归并计划:每条未解决意见的真实性、不变量、现有负责人、最小修法,以及可以删除什么。区分本 PR 的义务/回归和 base 旧债务。已确认的 P2 只有以下三个根因组:
权威保持不变:Runtime Host 管准入、执行、终态;RuntimeEvents 管新 receipt;target admission 管已提交 continuation;历史事实保留原权威;索引只存可重建引用;本地状态只管待确认意图和控件,不裁定已提交结果。 扩展最近的现有边界,同一改动删除被替代分支。不要新增 resume journal、并行 claim/receipt 存储、通用恢复框架、新 routing pass、公开 API 或模型调用。若认为必须新增,先说明哪个具体契约无法由现有权威满足,再决定,不直接加码。不要顺手修 base-only Resume ID 旧债务、推测性的取消/并发行为或无关清理。保留已验证的 descriptor 校验、独立分页超时、有界读取和稳定分页。 同步做有证据的减法:删除没有职责的空 clarification effect 及只断言 mock 调用的测试;将重复的直接 ledger receipt 断言合入生产 reader 断言,保留独立 ledger 测试;去掉 reader 测试已覆盖的重复 composition 分页循环;复用既有分片夹具验证两个独立超时预算,不为此新建通用测试框架。缺 actionId 的兼容是否可删,需要部署和持久化事实,不能猜。不要为了行数强行删必要保护。 最小验收链:
最后再看累计 diff,删除修复后冗余的分支、helper 和测试,保留独立行为保护。跑受影响测试和仓库要求的检查,不扩成全路径测试矩阵。交付 SHA、不变量闭环、负对照、删掉的概念/路径,以及确需净增长的具体理由。三组问题和已有回归通过后停止;不确定性明确说明,不再猜出一层兼容或恢复机制。 |
Summary
WorkHub actions previously bypassed root Turn admission, and clarification/resume could write synthetic assistant summaries. Admit Host actions into real Coordination Turns/Runs and persist typed Runtime receipts, without invoking an extra model answer. Target Sessions retain assignment, Stop and resume authority.
The production transcript reader now consumes Runtime receipts alongside atomic delegation/Stop/link facts and released history. A rebuildable SQLite index stores only source references, preserving stable pagination when clocks regress or either source receives new records. Receipt bodies remain exclusively in RuntimeEvents. The WorkHub view groups physical receipt retries by action identity. Failed admitted inputs remain readable until a visible receipt or atomic link replaces them; physical retry identity is retained separately. Delegation and Stop cards also use the stable action identity to replace local placeholders after a retry creates a new physical Turn.
Completed requests replay their original acknowledgement. If target resume admission committed before a missing Coordination receipt, the same action acknowledges that deterministic target Turn without starting another continuation. Interrupted attempts close through Runtime recovery; subsequent attempts have their own admitted Turn. Intent, Resolver and clarification model calls remain future work within this boundary.
Replacement could also fail when normal Session progress expired its candidate snapshot between discovery and execution. Preserve
candidate_set_staleacross Host/IPC, then refresh at most twice for the same chosen Session and action. A missing/renamed target, another refusal, or continued churn still fails closed; routing is not rerun.Refs #3492. Independent follow-up to #4924.
Index catch-up commits at most 64 references per foreground request. Until caught up,
subscription.openreturnstranscript_preparingwith committed progress instead of an incomplete snapshot. The client yields between resumable requests while retaining its loading state and respecting the open deadline. The derived index remains body-free; reader recreation resumes the same source positions.Admitted action identity now accompanies the projected user input. Missing receipts do not hide failed inputs, and incomplete confirmation is distinguished from failure of the target effect. A successful visible receipt/link replaces all inputs for that action; pages without that replacement retain their inputs. Original target admission still supplies the resume acknowledgement.
Concurrent replays retain the incoming execution descriptor through admission validation. Legacy compatibility omits only a missing action-identity field; it cannot replace the incoming input digest with stored content. Subscription preparation has a bounded open deadline, while subsequent pages and overlay release retain the caller's independent request timeout.
Verification
Latest localized fixes: 130 production Host, Root Turn and IPC subscription tests passed. Deterministic SQLite interleavings cover same/changed payloads for both current and legacy admissions. The IPC regression spends 700 ms of a 1,000 ms preparation budget, then successfully hydrates a 500 ms page using its own 1,000 ms budget. Both reported failures were reproduced before the fixes.
Runtime Host build/typecheck, repository lint/format and diff checks pass. No renderer change or fresh full Electron run for these localized fixes; current-head remote CI is required.
Rebased onto
42fa4d070(latest main at rebase); preserved localization changes and adapted the new fallback-title test input. Compatibility epoch remains 134 against main's 133.Previous-head (
a637d1a58) Desktop Electron E2E: 37/37 passed; focused WorkHub startup, navigation and exact linked replacement: 3/3 passed.Host/reader/client/pager/storage regressions: 89 passed, then 51 Host/pager cases passed after adding the shared-input identity regression. Desktop projection/surface regressions: 43 passed.
Real SQLite growth tests with 257 and 10,000 historical rows check every resumable small-page request: at most 3 source reads, 192 decoded legacy rows and 1 index write. Each batch recreates the reader; catch-up completes with the latest message visible. Existing snapshot, bidirectional cursor, late-append and regressing-clock tests pass.
The production missing-receipt fault test now checks the failed input and terminal status after reopen, before retry; retry acknowledges the original target Turn without another continuation. Projection tests cover several failed physical attempts, a successful receipt, and older pages where that replacement is absent.
Build, typecheck, lint, formatting, renderer architecture against
42fa4d070, and epoch guard pass. Current-head remote CI remains required.Storybook screenshots below are component fixtures; Runtime delivery is validated by production tests.
Dark appearance
Ablation and review
Shared one strict Action result validator and removed unused clarification input. Retained Runtime admission, durable receipts and target-owned retry boundaries because the fault-cut tests exercise those behaviors. Earlier independent Standards/Spec review identified provider coupling, recovery and deferred invalidation problems; those were corrected before this rebase.
This update addresses Astro-Han's review at
b8d69b459: production transcript visibility, acknowledgement recovery from target admission, and explicit release of the retired persisted grant. It adds no resume journal or duplicate receipt store. Human review and current-head CI are still required.Compatibility and review focus
Compatibility epoch moves from 133 to 134. The synthetic
workhub.coordination.recordoperation is removed; persisted grants for it are explicitly released, never widened toact. Typed receipts and admitted action identity extend transcript decoding; typed errors distinguish expired candidates and resumable transcript preparation. Session metadata schema 39 adds a derived source-reference index; released history and transactional linkage facts retain their original authorities.AI use
Tool(s) and scope: Codex implemented the repair, tests, documentation and component evidence.
Checklist
Does this PR entail a change in behavior?