refactor(runtime): derive Session transcripts from RuntimeEvents - #4879
refactor(runtime): derive Session transcripts from RuntimeEvents#4879Astro-Han wants to merge 12 commits into
Conversation
jackwener
left a comment
jackwener
left a comment
There was a problem hiding this comment.
Reviewed at exact head a6fca96a. One [P1] and three [P2]s. The P1 is the failure mode this PR exists to remove, reintroduced on the recovery path. The direction is right and most of the cut checks out; details below.
[P1] One executed prompt can become two transcript users after a restart
The write path and the recovery path derive the user event's id by different rules:
- A Root folded from several queued Messages has no single Message identity, so
admission.userMessageIdisnullandbegin()writes its user event under a freshnewId(). - Recovery looks for the derived
${runId}-admitted-prompt.
The window named in the description — invocation open, no user event, process dies — is genuinely repaired: no terminal, the admission is still in core_root_turn_admissions, and recovery writes the admitted-prompt id.
The adjacent window is not. If the user event has already landed under newId() and the Host dies before the terminal, recovery filters on the admitted-prompt id, does not see the live event, and appends a second one. One prompt the model executed once, two user messages in the transcript. That is a Session disagreeing with itself, which is precisely what making the ledger the single authority was meant to end.
Single-source Roots are unaffected: there the admitted id and the begin() id are the same Message id.
On reachability, since that is what should decide the grade: folding happens when someone queues a few messages before a turn runs, and losing the Host mid-turn is an ordinary crash. Neither leg is exotic, and the result survives the restart rather than being repaired by it.
The fix that fits this PR's own design is to make the derivation one rule on both sides — have begin() use ${runId}-admitted-prompt when the admission carries no userMessageId, so the write and the recovery agree and the store's exact-duplicate dedupe absorbs the second attempt. Filtering recovery by run rather than by derived id would also work; the first is smaller and matches how the rest of the cutover derives ids.
[P2] An in-process start failure seals the hole that crash recovery would fill
runAgentTurn's catch runs finalizeFailedRunStart → failStart → finalize. If opening already committed, the second openInvocation is a no-op and a terminal is written with no user event. The sealed-run skip then refuses to backfill it, permanently.
The asymmetry is the point: a process crash on the same state recovers, and this exception path does not. The user saw the turn fail and can resend, so the cost is a permanent empty sealed Run rather than lost work — but it is the more common of the two failures and it is the one that cannot be repaired.
[P2] Turns with no remaining user after the steering filter are dropped silently
materializeTranscriptLedger converts a turn only if some message is still type === 'user' after user rows carrying steeringEventId are removed. A turn that is only notes, only tools, or only steering users never becomes a run, with no diagnostic. Production context_compacted carries the live turnId and does convert, and I did not find a tagged-release writer that produces a user-less conversation turn — so this is a live silent filter rather than a demonstrated loss.
[P2] The bounded unread tail can fail to clear
session.read_marker.set reads the newest 64 messages / 256 KiB instead of scanning every visible row, and clears hasUnread only when that window's newest user|assistant id matches. It never clears falsely — a newer visible message would be nearer the tail and inside the window — but it can fail to clear when the newest records are all hidden tool or system rows. Badge only; the request itself is still there. lastReadMessageId has no in-tree client reader, so hasUnread is the live bit.
What holds
The dual-write window is genuinely gone from the live path. The base wrote session_messages via appendUserMessageOnce and then the user RuntimeEvent, markMessagesHandedOff inserted user rows, and finalize appended session_resume. At this head agent-run.ts has no appendUserMessageOnce and no appendMessage, and handoff only deletes admissions. There is no longer a pair of authorities for a crash to split — which is why the P1 above is worth fixing rather than accepting: the design achieves its goal everywhere except that one id mismatch.
The importer is total over the legacy row types, enumerated from the tagged writers (v0.1.0–v0.1.11, then v0.2.0-dev.9+) rather than from what remains in the tree. Each type converts, or is deliberately skipped with the fact owned elsewhere. An unknown type throws StoredSessionMessageIncompatibleError and fails the whole readMessages, so conversion never starts — a fail-closed Session instead of a quietly truncated history, which is the right way for this to break.
Import staging is reentrant and cannot strand a Session. transcriptLedgerVersion === 0 is hidden from the catalog and blocked from every execution kind, conversion runs in admitTurn before begin, and externalSessions.recover() retries on every Host start. Re-running an interrupted import writes the same derived ids and the production SQLite store dedupes them — verified on the real store, not assumed.
The five deletions carry their proofs elsewhere. Handoff's content lives in the admission's sourceMessages plus the user RuntimeEvent; the catalog projection's user path is fail-closed and its assistant path fail-open, and the lock and preview share one SQLite transaction so "lock taken, projection failed" cannot happen; readMessagesForRecovery was byte-identical to readMessages; buildTurnStateMessage's lineage is rebuilt from invocation.opening.lineage.
Scope note
The WorkHub linkage lane — the second item the description flags as needing a human pass, and the one that matters because #4699's target linkage enumerated a lifecycle table this PR stops writing — is still running. I will post it separately rather than amend this.
One disclosure: the importer, second-write, and crash/race lanes were all carried out by the same reviewer, so they are not independent cross-checks of each other.
简体中文
在 exact head a6fca96a 上评审。一条 [P1],三条 [P2]。而这条 P1 恰恰是本 PR 立意要消灭的那种失败,在恢复路径上又出现了。 方向是对的,大部分切除也经得起核。
[P1] 一次已执行的 prompt,重启后可能变成两条 transcript user
写入路径与恢复路径用两套规则派生 user 事件的 id:由多条排队 Message 折叠而成的 Root 没有单一 Message 身份,admission.userMessageId 为 null,于是 begin() 用新的 newId() 写下 user 事件;而恢复侧寻找的是派生的 ${runId}-admitted-prompt。
描述中点名的那个窗口(invocation 已开、无 user 事件、进程死亡)确实被修好了。但紧邻的那个没有:若 user 事件已以 newId() 落盘、而 Host 在写 terminal 之前死亡,恢复会按派生 id 过滤、看不见那条已存在的事件,于是再追加一条。模型只执行过一次的 prompt,在 transcript 里成了两条 user 消息 —— 这正是「让账本成为唯一权威」本要终结的「会话自相矛盾」。
单源 Root 不受影响:那时 admitted id 与 begin() 的 id 是同一个 Message id。
关于可及性(既然定级应由它决定):折叠发生在有人在一轮执行前排入几条消息时,而 Turn 执行中失去 Host 是普通崩溃。两条腿都不罕见,而且结果会熬过重启,而不是被重启修好。
与本 PR 自身设计相符的修法,是让派生在两侧成为同一条规则 —— 当 admission 不带 userMessageId 时,让 begin() 也用 ${runId}-admitted-prompt,使写入与恢复一致,并由 store 的精确去重吸收第二次写入。让恢复按 run 而非派生 id 过滤同样可行;前者更小,且与这次切换其余部分派生 id 的方式一致。
[P2] 同进程内的启动失败,会把崩溃恢复本可填上的洞封死
runAgentTurn 的 catch 走 finalizeFailedRunStart → failStart → finalize。若 opening 已提交,第二次 openInvocation 是 no-op,随后写下一个没有 user 事件的 terminal;已封存跳过规则此后永久拒绝补写。
不对称才是要点:同样的状态下,进程崩溃能被恢复,而这条异常路径不能。 用户看到那一轮失败、可以重发,所以代价是一个永久的空封存 Run 而非丢失工作 —— 但它是两者中更常见的那一个,也是唯一无法修复的那一个。
[P2] steering 过滤后不再剩 user 的 turn 被静默丢弃
materializeTranscriptLedger 仅在去掉带 steeringEventId 的 user 行之后仍有 type === 'user' 时才转换该 turn。只有 note、只有工具、或只有 steering user 的 turn 永远不会成为 run,且无任何诊断。生产的 context_compacted 带着实时 turnId,会被转换;我也没有找到会产出「无 user 的会话 turn」的已发布写入方 —— 所以这是一个仍然存活的静默过滤,而不是已被证实的丢失。
[P2] 有界的未读尾部可能清不掉
session.read_marker.set 改为读最新 64 条 / 256 KiB 而不再扫描全部可见行,仅当该窗口内最新的 user|assistant id 匹配时才清除 hasUnread。它永远不会误清 —— 更新的可见消息必然更靠近尾部、落在窗口内 —— 但当最新记录全是隐藏的工具/系统行时,它可能清不掉。 只影响角标,请求本身仍在。lastReadMessageId 在本仓库没有任何客户端读取方,真正起作用的是 hasUnread。
成立的部分
双写窗口在活路径上确实消失了。 基线上 appendUserMessageOnce 先写 session_messages、再写 user RuntimeEvent,markMessagesHandedOff 还会插入 user 行,finalize 追加 session_resume。在此 head 上,agent-run.ts 既无 appendUserMessageOnce 也无 appendMessage,handoff 只删除 admission。同一事实不再有两个权威可供崩溃劈开 —— 这也正是上面那条 P1 值得修而不是被接受的原因:这个设计在除那一处 id 不一致之外的每一处都达成了目标。
导入器对 legacy 行类型是全的,而且是从已发布 tag 的真实写入方(v0.1.0–v0.1.11,以及 v0.2.0-dev.9+)枚举,而不是从树里剩下的类型倒推。每一类要么被转换,要么被有意跳过且该事实由别处拥有。遇到未知类型会抛 StoredSessionMessageIncompatibleError 并让整次 readMessages 失败,于是转换根本不会开始 —— fail-closed 的会话,而不是被悄悄截断的历史,这是它该有的坏掉方式。
导入的暂存态可重入,且不会让会话搁浅。 transcriptLedgerVersion === 0 对目录隐藏、并被拦截在所有执行种类之外,转换在 admitTurn 中于 begin 之前运行,而 externalSessions.recover() 在每次 Host 启动时重试。重跑一次被中断的导入会写出同样的派生 id,生产 SQLite store 会去重 —— 这是在真实 store 上验证的,不是假定的。
五处删除的证据确实在别处。 handoff 的内容存在于 admission 的 sourceMessages 与随后的 user RuntimeEvent;目录投影的用户路径 fail-closed、助手路径 fail-open,而锁与预览共享同一个 SQLite 事务,所以「锁已取走、投影失败」不可能发生;readMessagesForRecovery 与 readMessages 逐字节相同;buildTurnStateMessage 的 lineage 由 invocation.opening.lineage 在读模型中重建。
范围说明
WorkHub 联动那条车道仍在进行 —— 那是描述中点名需要人工过一遍的第二项,也是要紧的一项,因为 #4699 的目标联动此前从一张生命周期表枚举身份,而那张表正是本 PR 停止写入的。 结果我会另发一条,而不是修改本条。
一项披露:导入器、第二写点、崩溃/竞态三条车道由同一位评审者完成,因此它们彼此之间不是独立的交叉验证。
Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
…un without it Review of #4879 found the failure the PR exists to remove, reintroduced on the recovery path. `begin()` derived the prompt event's id as `userMessageId ?? newId()`; recovery derived it as `userMessageId ?? ${runId}-admitted-prompt`. The two rules agree only for a single-source Root. A Root folded from several queued Messages has no Message identity, so a Host that died after the prompt landed and before the terminal came back to a ledger whose prompt it could not see, and recorded the same executed prompt a second time. `admittedPromptEventId` is now the one derivation, and recovery asks whether the Turn has a prompt rather than whether it has one under that exact id: a Run written by an older build derived the id differently, and matching on the id would read its prompt as missing. Steering leaves that index — it is typed as a user message but is something said into an already-admitted Turn, so it is never the Turn's own prompt. The same review found the in-process mirror of that crash: `begin()` failing between opening the invocation and recording the prompt runs `failStart` -> `finalize`, whose terminal event seals the run against every later append, recovery's repair included. Before this cutover recovery could still append to `session_messages`, which has no seal; a sealed ledger cannot be repaired, so `finalize` records the prompt itself before sealing, next to the openInvocation call that already keeps the sibling rule "a run cannot end without having begun". The read marker's tail scan now pages past hidden records. It read one bounded page and gave up, so a Turn ending on tool traffic could leave a Session showing unread after it had been read. It never cleared falsely, so this is a badge, not a lost message. Two review points are not taken. The reviewer's fix for the id mismatch was to inline the derived id in `begin()`; that leaves the same string template in two packages, which is still two rules that happen to agree. The reviewer also read the importer's "convert only turns that still have a user row" filter as a silent drop with no producer. It has one: a turn whose only user row was steering belongs to a Turn some durable Root already owns, and converting it stands a second synthetic run beside that one. Ablating the filter fails `does not import Host-handed-off transcript messages as synthetic runs`, so it stays, with its reason written down. storage 1092 pass, runtime-host 1711 pass, runtime 3130 pass, cli 805 pass, 0 failures. Generated-by: Claude Code
|
Follow-up: the WorkHub linkage lane, posted separately as promised rather than amending the earlier review. Exact head This was the second item the description flags as needing a human pass, and it is a cross-PR question: #4699's target linkage enumerates a delegated Message's identity from three lifecycle tables, and one of those arms read the transcript row this PR stops writing. The concern was the upgrade case, and it is closed. A Session that completed a delegation before this lands would have the old transcript row and, if the proofs table were new, no replacement — so its linkage would quietly disappear. That is not the situation: The identities line up because the proof is the same id. For each It is also a better record than the one it replaces. The three arms now map cleanly onto the three states a delegated Message can be in — pending in Two limits worth stating rather than leaving implied:
With this, all four lanes on this PR are reported. The [P1] on the recovery path stands as the one thing to fix. 简体中文补充:WorkHub 联动这条车道,按先前承诺另发一条,而不是修改已发出的评审。 exact head 这是描述中点名需要人工过一遍的第二项,而且是个跨 PR 的问题:#4699 的目标联动从三张生命周期表枚举被委派 Message 的身份,其中一条臂读的正是本 PR 停止写入的 transcript 行。 我担心的是升级情形,而它已经排除。 一个在本 PR 落地之前完成过委派的 Session,会有旧的 transcript 行;若 proofs 表是新建的、没有替代记录,它的联动就会静默消失。事实并非如此: 身份能对上,是因为 proof 存的就是同一个 id。 对 而且它比被取代的那条记录更合适。 三条臂现在干净地对应一条被委派 Message 可能处于的三种状态 —— 在 有两处限制,与其留作暗示不如明说:
至此本 PR 的四条车道全部报完。恢复路径上的那条 [P1] 仍是唯一需要修的东西。
|
|
Head [P1] Fixed. Confirmed, and a regression this PR introduced — on the base that branch threw and Not fixed the way you proposed: inlining Unifying it is also not sufficient. Test: [P2 seal] Fixed at Narrower than a generic append failure: a latching store error also refuses the terminal, leaving the run open for crash recovery. The sealed-without-prompt shape needs a non-latching refusal, which is what the test injects. [P2 importer] No finding. I removed the filter as suggested; [P2 unread] Fixed, P3. Confirmed, never clears falsely. Raising the constant only moves the boundary, so the scan pages back until it finds a visible record or exhausts the ledger. Stale badge on a read Session, self-healing at the next Turn. Your single-reviewer disclosure: taken — it is why I treated the importer item as a gap to close rather than a defect to patch, and closing it is what produced the counterexample. storage 1092, runtime-host 1711, runtime 3130, cli 805 — 0 failures. WorkHub linkage lane: waiting on your post. |
|
Re-reviewed at exact head [P1] Closed, and it also covers rows already on disk
The part worth calling out is not in the summary: recovery also stopped filtering the turn's user messages down to the derived id. The mirror risk is avoided. Where [P2] Closed at the seam that made it worse than a crash
Residuals, both narrower than the hole they came from:
[P2] The unread tail is genuinely paged now, and the bound it replaces is not lost
I checked the termination, since replacing a bound with a loop is where this kind of fix usually overcorrects. It stops on the first visible message or when [P2] The steering-only turn is now explained rather than changedThe filter still drops a turn whose only user row was steering, and three lines of comment now say why: that steering was said into a Turn a durable Root already owns, so converting it would stand a second synthetic run beside the real one. The reasoning holds and I am not asking for a behaviour change. Stating it plainly, though: the filter is still silent — what changed is that a reader of the code can now find out why, not that a workspace where it fires reports anything. StandingThe [P1] is gone, so the objection that made this NO-GO is resolved. The PR is Everything else from the earlier passes stands: the importer is total over the legacy row types enumerated from tagged writers, an unknown type fails closed rather than truncating history, import staging is reentrant with dedupe verified on the real store, the five deleted second-writes carry their proofs elsewhere, and the WorkHub linkage swap is safe including for Sessions that delegated before this PR. 简体中文在 exact head [P1] 已关闭,而且覆盖了已经落盘的行
值得点出的一处不在摘要里:恢复侧同时不再把该 turn 的 user 消息过滤到派生 id。 镜像风险被避开了。 [P2] 关在了「它比崩溃更糟」的那个接缝上
两处残留,都比它们所出自的洞更窄:
[P2] 未读尾部确实改成了翻页,而它取代的那个上限并没有丢
我核了终止条件 —— 因为「用循环取代上限」正是这类修复容易矫枉过正的地方。 它在遇到第一条可见消息、或 [P2] 只有 steering 的 turn 现在是被解释了,而不是被改变了 该过滤仍会丢弃「唯一 user 行是 steering」的 turn,现在有三行注释说明原因:那段 steering 是说进某个已有持久 Root 所拥有的 Turn 里的,转换它就会在真实 run 旁边立起第二个合成 run。 这个理由成立,我不要求改变行为。 但把话说清楚:该过滤仍然是静默的 —— 改变的是读代码的人现在能查到原因,而不是「它触发的那个工作区会报告些什么」。 当前立场 [P1] 已消除,所以让本单成为 NO-GO 的那条反对意见已解决。该 PR 相对 先前各轮的其余结论均成立:导入器对从已发布 tag 枚举的 legacy 行类型是全的;未知类型 fail-closed 而不是截断历史;导入暂存可重入且去重已在真实 store 上验证;被删的五处第二写点其证据确在别处;WorkHub 联动的替换是安全的,包括对本 PR 之前就发生过委派的 Session。
|
7c2c74e to
82325ad
Compare
…un without it Review of #4879 found the failure the PR exists to remove, reintroduced on the recovery path. `begin()` derived the prompt event's id as `userMessageId ?? newId()`; recovery derived it as `userMessageId ?? ${runId}-admitted-prompt`. The two rules agree only for a single-source Root. A Root folded from several queued Messages has no Message identity, so a Host that died after the prompt landed and before the terminal came back to a ledger whose prompt it could not see, and recorded the same executed prompt a second time. `admittedPromptEventId` is now the one derivation, and recovery asks whether the Turn has a prompt rather than whether it has one under that exact id: a Run written by an older build derived the id differently, and matching on the id would read its prompt as missing. Steering leaves that index — it is typed as a user message but is something said into an already-admitted Turn, so it is never the Turn's own prompt. The same review found the in-process mirror of that crash: `begin()` failing between opening the invocation and recording the prompt runs `failStart` -> `finalize`, whose terminal event seals the run against every later append, recovery's repair included. Before this cutover recovery could still append to `session_messages`, which has no seal; a sealed ledger cannot be repaired, so `finalize` records the prompt itself before sealing, next to the openInvocation call that already keeps the sibling rule "a run cannot end without having begun". The read marker's tail scan now pages past hidden records. It read one bounded page and gave up, so a Turn ending on tool traffic could leave a Session showing unread after it had been read. It never cleared falsely, so this is a badge, not a lost message. Two review points are not taken. The reviewer's fix for the id mismatch was to inline the derived id in `begin()`; that leaves the same string template in two packages, which is still two rules that happen to agree. The reviewer also read the importer's "convert only turns that still have a user row" filter as a silent drop with no producer. It has one: a turn whose only user row was steering belongs to a Turn some durable Root already owns, and converting it stands a second synthetic run beside that one. Ablating the filter fails `does not import Host-handed-off transcript messages as synthetic runs`, so it stays, with its reason written down. storage 1092 pass, runtime-host 1711 pass, runtime 3130 pass, cli 805 pass, 0 failures. Generated-by: Claude Code
jackwener
left a comment
jackwener
left a comment
There was a problem hiding this comment.
Approving at exact head 82325adc. The findings from the earlier rounds are resolved, and I re-bound them to this head by comparing bytes rather than assuming they carried.
The rebase moved 205 files, so the conclusions were not taken on trust. Of the six files the findings and fixes live in, five are byte-identical to 7c2c74e5:
message-authority.ts,agent-run.ts,hosted-execution-recovery.ts— the P1 fix, unchanged;runtime-ledger-repair.ts— the steering-only turn rationale, unchanged;sqlite-session-metadata-store.ts— the WorkHub linkage arm, unchanged.
session-catalog-coordinator.ts does differ, so I read it rather than counting it: the delta is the import-model candidate work (NoUsableImportModelError, ImportModelCandidate) that arrived from main, and #newestVisibleMessage — the paging fix for the unread marker — is character-for-character what was verified before, including the termination on either the first visible message or nextPosition === null.
So the state is: the [P1] is closed by a shared derivation used on both sides, the sealing [P2] is closed by writing the prompt before the terminal, the unread tail pages instead of giving up, and the steering-only filter is documented. The two residuals I recorded stay recorded and neither is asked for here: the prompt retry is .catch(() => {}), and a throw inside openInvocation never sets the pending flag.
Two things this approval does not claim:
- Required
testis still pending on this head. Branch protection enforces it independently, so this approval is a statement about the code, not about the gate. - An independent blind review is in flight — a reviewer of a different lineage, working from the live head without reading these comments or any of the earlier conclusions. That seat exists because the first four lanes on this PR were carried out by the same reviewer, which I disclosed at the time. Its result will be posted separately whatever it says, and it may well find something these passes did not.
简体中文
在 exact head 82325adc 上批准。先前各轮的 finding 均已解决,而且我是通过比对字节把结论重新绑定到本 head 的,不是假定它们自动转移。
这次 rebase 动了 205 个文件,所以结论没有被采信。在 finding 与修复所在的六个文件中,五个与 7c2c74e5 逐字节相同:message-authority.ts、agent-run.ts、hosted-execution-recovery.ts(P1 修复)、runtime-ledger-repair.ts(steering-only turn 的理由)、sqlite-session-metadata-store.ts(WorkHub 联动那条臂)。
session-catalog-coordinator.ts 确实不同,所以我是读了它而不是数它:差异是从 main 带进来的导入模型候选改动(NoUsableImportModelError、ImportModelCandidate),而 #newestVisibleMessage —— 未读标记的翻页修复 —— 与此前验证过的逐字相同,包括「遇到第一条可见消息或 nextPosition === null 才停」这一终止条件。
所以当前状态是:[P1] 由两侧共用的同一条派生关闭;封存类 [P2] 由「先写 prompt 再写 terminal」关闭;未读尾部改为翻页而不是放弃;只有 steering 的 turn 得到了书面理由。 我记录的两处残留仍然记录在案,且此处都不要求改动:补写 prompt 用的是 .catch(() => {});以及 openInvocation 内部抛出时待写标志从未置位。
这条批准不主张两件事:
- 本 head 上必需的
test仍处于 pending。 分支保护会独立强制它,所以这条批准是对代码的陈述,不是对门禁的陈述。 - 一次独立盲审正在进行中 —— 由不同谱系的评审者从 live head 开始,不读这些评论、也不读此前任何结论。设这一席的原因是:本 PR 最初的四条车道由同一位评审完成,这一点我当时已经披露。 无论它得出什么结论,都会另行发布;它完全可能发现这几轮没有发现的东西。
Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
The pending flag was set after `openInvocation()`, so the one failure it did not cover was a throw from the opening itself: `finalize` reopens what it can, and a run it manages to open then sealed with a terminal and no prompt — the same hole the previous commit closed, entered from one step earlier. Moving the flag ahead of the opening costs nothing when the invocation never opens: `finalize`'s reopen fails too, and the backfill is a no-op on a run that does not exist. Reported as a residual on #4879 and not asked for; it is one line and it closes the last entrance to a shape that cannot be repaired after the fact. runtime 3131 pass, runtime-host 1723 pass, cli 805 pass, 0 failures. Generated-by: Claude Code
M4n5ter
left a comment
M4n5ter
left a comment
There was a problem hiding this comment.
English
Reviewed at 69e6d066ca1e6d71ca1eb1ab7be73830074b053f.
[P1] Interrupted migration cannot resume when the clock advances.
The converter now derives deterministic event IDs but still passes the live clock into backfill. The generated payload contains generatedAt: now(), and SQLite's duplicate handling requires the entire event to match.
Reproduced with the production SQLite stores: seed an unmarked legacy Session containing a user, an assistant answer and a completed turn; interrupt conversion immediately after the user event commits; retry with a later clock. The retry throws RuntimeEvent identity conflict for transcript-…-e1. Further conversion attempts encounter the same conflict, blocking reads through ensureTranscriptLedgerForRead.
Make the complete regenerated payload deterministic, including its recovery timestamps. The existing resumability test uses a constant clock and runs two successful, complete conversions; it never interrupts the first one.
[P1] Startup recovery seals an unfinished migration and then marks truncated history as migrated.
For an unmarked legacy Session, interrupt conversion at the same point, then run recoverInterruptedSessionsStrict() before reading it. Generic recovery treats the synthetic transcript-* invocation as an interrupted execution and appends a failed terminal event. The converter's terminal guard then skips that turn, and ensureTranscriptLedger writes transcriptLedgerVersion: 1.
Reproduced with a constant clock to isolate this from the first finding: the original user + assistant + completed becomes user + failed. All three legacy rows remain in SQLite, but normal reads no longer return the assistant answer and subsequent reads skip conversion altogether.
Keep incomplete conversion runs out of generic execution recovery, or finish their conversion before that recovery can seal them. Fixing timestamp determinism alone does not fix this path.
[P2] The new pager bounds its response, but not its storage reads.
endedInvocations and projectRun enumerate every invocation in the Session and load/project the entire selected Run before applying page limits. The SQLite invocation listing also performs a terminal lookup for each invocation. Even a tiny page therefore incurs work proportional to Session length and the full selected Turn, including payloads outside the requested page. Long Sessions and large Turns lose the storage/memory bounds required by #4791.
Apply the bounds to invocation/event retrieval before materializing transcript payloads. This finding follows from the read path; I did not reproduce an OOM.
Validation: 181 existing targeted tests passed. Both migration failures above were reproduced by separate fault-injection tests using the production SQLite stores. Full Host integration validation was blocked by local dependency version mismatches.
简体中文
审查版本：69e6d066ca1e6d71ca1eb1ab7be73830074b053f。
[P1] 迁移中断后，时间一变，重试就会报事件身份冲突。
迁移器固定了事件 ID，但事件内容里仍有 generatedAt: now()。SQLite 去重要求同 ID 的整个事件完全一致。
已用生产 SQLite store 复现：准备一个没有迁移标记的旧会话，包含 user、assistant 回答和 completed 状态；迁移写入 user 事件后中断，再用稍晚的时间重试，直接报 RuntimeEvent identity conflict for transcript-…-e1。后续重试仍会撞上同一条记录，经过迁移入口的会话读取也会失败。
需要保证重试生成的整个事件内容一致，包括恢复时间戳。现有测试固定了时间，而且只是把完整迁移执行两遍，没有真正测中断。
[P1] 重启恢复会抢先封存未迁完的 Run，随后把缺失回答的会话标成迁移完成。
同样在旧会话迁移写入 user 后中断，如果接着先执行 recoverInterruptedSessionsStrict()，通用恢复逻辑会把 transcript-* Run 当成崩溃的执行任务，写入 failed 终态。之后迁移器跳过已封存的 Turn，ensureTranscriptLedger却照常把版本写成 1。
这次复现全程固定时间，排除了上一项的影响。实测原来的 user + assistant + completed 只剩 user + failed。旧表三条记录都还在，但正常读取不再返回原回答，后续也不会再尝试迁移。
需要让未完成的迁移避开通用执行恢复，或先迁完再允许恢复逻辑封存。只修时间戳，挡不住这个问题。
[P2] 分页限制了返回量，却没有限制底层读取量。
新读取器每次先枚举整个 Session 的 invocations，再完整读取并投影目标 Run，最后才裁剪页面；SQLite 枚举时还会逐个查询终态。即使只要很小的一页，开销也会随会话历史和单个 Turn 的大小增长，页外的大块内容同样先读进内存，不满足 #4791 对有界读取的要求。
需要在获取 invocation 和事件时就限定范围，再生成页面。这项依据是静态调用链，没有做 OOM 复现。
验证：181 项现有定向测试通过；两个迁移问题分别做了故障注入，均在生产 SQLite store 上复现。完整 Host 集成验证受本地依赖版本不匹配阻断。
M4n5ter
left a comment
M4n5ter
left a comment
There was a problem hiding this comment.
English
Fixed all three findings in two follow-up commits:
- fd073a8 makes the entire imported event payload deterministic, not just its ID. Startup recovery now leaves importer-owned invocations alone so an interrupted conversion resumes instead of sealing a partial transcript.
- b7d27cf seeks Session event ordinals before loading message payloads, fetches only the selected message's projection context, and uses indexed queries for Turn summaries, landmarks, and live-to-durable message lookup. RuntimeEvents remain the only transcript authority; schema v17 adds indexes, not another transcript copy. Host epoch 119 fences the changed cursor semantics.
Regression coverage includes interruption after a committed user event, startup recovery followed by full history retrieval, and a single Turn exceeding 5 MiB. Small-page/index/lookup reads decode under 512 KiB in that fixture; complete pagination matches the full projection, including thinking, permissions, terminal state, and multibyte byte-fragment reassembly.
Local verification: 6,899 passed, 29 skipped, 0 failed across core, storage, runtime, and runtime-host. TypeScript builds and staged checks passed. No manual Desktop smoke test was performed.
中文
三个问题都已修好，分成两笔提交：
- fd073a8：导入事件的内容也改成确定性的，重试不再撞上同 ID、不同 payload 的冲突。启动恢复不再提前结束导到一半的迁移 Run，重启后仍能接着导完，避免漏掉旧对话。
- b7d27cf：分页先按 Session 事件位置定位，再取当前消息和它需要的上下文；Turn 摘要、跳转位置和实时消息转入历史时的 ID 查询也改走索引。没有再存一份 transcript，schema v17 只加索引；Host epoch 升到 119，防止新旧游标混用。
回归覆盖了“用户消息已落盘后中断 → 启动恢复 → 读取完整旧对话”，也覆盖了单个 Turn 超过 5 MiB 的情况。后者的小页、索引和 ID 查询合计解码不到 512 KiB；逐页拼回的内容与完整投影一致，thinking、权限、终态和中文跨字节分片都做了对照。
本地 core、storage、runtime、runtime-host 合计 6899 项通过、29 项跳过、0 失败，TypeScript 构建和提交前检查也已通过。尚未做桌面端人工验收。
A note the runtime writes during a turn -- context compacted, step cap reached, the turn aborted -- is a fact of that invocation, but the only place it could be written was the Session transcript. That left the ledger unable to state part of what a run did, and left the importer dropping those rows on the floor. Notes that happen between turns belong to no invocation, so they stay Session transcript rows. The split is by owner, not by how they render. Generated-by: Claude Code
…g some The converter used to answer "this row cannot be recovered losslessly" by writing nothing, which loses conversation the user can still read today. Losslessness is a model-replay property, not a transcript one, so the rows it could not replay now convert hidden: the card stays in the transcript and no provider request is ever built from it. A permission decision names its own tool, so it no longer needs a matching call in the same turn; it carries the prompt's hint when nothing else records it. A turn whose transcript never said how it ended now ends as the failure it was, because an invocation left open is not a legal ledger state and would strand the turn in recovery forever. Generated-by: Claude Code
Every event id is now derived from the run it belongs to and its position in that run, so converting the same transcript twice writes the same events and the store keeps one copy. That is what lets an interrupted conversion resume: a turn is skipped once its invocation has ended, and re-derived until then. Before, a turn was skipped as soon as its opening existed, which froze a half-converted turn in that state forever. Maka's own history now converts whole. Only a foreign transcript stays conversation-text: another runtime's tool calls belong to its protocol, not to the provider this Session talks to next. Generated-by: Claude Code
A running turn's rows came from the Session transcript store while every finished turn's came from the RuntimeEvent ledger. That is the double write: the same execution facts written twice so a reader could find them in whichever place it looked. Now one place answers. An open invocation is read the way the Host's active overlay already read it -- arriving text presented as settled, a step that has only thought given the empty assistant row that thinking hangs on -- and that reading moves next to the projection so both readers share it instead of keeping a copy each. "Still running" is the absence of the terminal event, so it is stated on the turn record where it belongs rather than as a transcript row. Generated-by: Claude Code
…rest A system note stated one of two things. The ones that describe what happened inside an invocation — compaction, context pressure, the step cap — are facts of that invocation, and now live where its facts live: the RuntimeEvent ledger, through `AgentRun.recordSystemNote` and a `recordSystemNote` hook the backend reaches like its other recorders. They stay `modelVisibility: 'hidden'`, so the reader sees them and the provider never replays them. The others said something that already had an owner. The Session header carries the mode, the model and the copy lineage; the invocation's opening fact carries its own configuration; the terminal event carries the abort and its source. `session_start`, `session_resume`, `mode_change`, `model_change`, `error` and `abort` only wrote those facts a second time, into rows nothing rendered. Their write sites are gone; the kinds stay decodable so legacy transcripts still read. Two readers depended on `session_start` as a position marker for "this revision copy admitted a turn of its own". The admission ledger answers that directly — a copy clones history but never admissions — and the Host revision coordinator, which already reads it, settles every `preparing` copy at recovery before SessionManager's duplicate check ever ran. Generated-by: Claude Code
…uthority
Every execution fact was written twice: once as a RuntimeEvent and once as a
`session_messages` row. Two authorities for the same fact means every writer has
to keep them in step, every reader has to pick one, and a crash between the two
writes leaves a Session that disagrees with itself.
This cuts the second write. The ledger is the durable record; `session_messages`
survives only as input to the one-way importer that converts a pre-ledger
transcript on first read, and as the WorkHub Coordination Session's own store,
which is out of scope here.
What moved:
- `markMessagesHandedOff` no longer projects admitted Messages into transcript
rows. It validates the admission and retires it; the durable proof the rows
used to carry already lives in the agent-run admission's `sourceMessages` and
in the RuntimeEvent steering proof.
- Catalog projection (`lastMessagePreview`, `lastMessageAt`, `connectionLocked`)
is committed by `AgentRun` through `commitMessageCatalogProjection` instead of
falling out of a transcript insert. It is fail-closed for a user message,
because that write also takes the Session's one-way connection lock, and
fail-open for the assistant preview, which costs a stale sidebar line at worst.
- The read marker no longer needs an ordered index of visible transcript rows.
`lastReadMessageId` has no consumer, so `hasUnread` is the only decision left:
it clears when the client has caught up with the ledger's newest visible
message, read off a bounded tail of the last run.
- Startup recovery writes a crashed Turn's admitted prompt into the invocation
that had already opened for it. A Root folded from several queued Messages has
no single admitted Message identity, so the prompt is durable under a derived
`${runId}-admitted-prompt`, which makes recovering the same crash twice a
no-op append. A sealed Run takes nothing: it is immutable, and a Run that
reached its terminal fact has a prompt the crash did not eat.
- WorkHub target linkage (#4699) enumerated a delegated Message's identity from
three lifecycle tables, one of which was the transcript row this change stops
writing. Its handed-off arm now reads `core_root_source_message_proofs` — the
Root admission that consumed the Message, in the same database and as durable
as the Session.
Deleted with their last caller: `markSessionReadThroughMessage`,
`SessionReadMarkerMessageNotFoundError`, `readMessagesForRecovery` (identical to
`readMessages`), `listForRecovery`'s separate query, the transcript-ordering
privates in the SQLite store, and `buildTurnStateMessage` with its lineage types.
Ablations kept out: an `existing.some(role !== 'system')` guard in recovery on
top of the terminal-event check (the terminal check alone is exact), and
removing the singular `appendMessage` (pure test churn for no production gain).
Verified on this base: storage 1092 pass, runtime-host 1709 pass, runtime 3129
pass, core 821 pass, cli 805 pass; 0 failures.
Closes #4791
Generated-by: Claude Code
…un without it Review of #4879 found the failure the PR exists to remove, reintroduced on the recovery path. `begin()` derived the prompt event's id as `userMessageId ?? newId()`; recovery derived it as `userMessageId ?? ${runId}-admitted-prompt`. The two rules agree only for a single-source Root. A Root folded from several queued Messages has no Message identity, so a Host that died after the prompt landed and before the terminal came back to a ledger whose prompt it could not see, and recorded the same executed prompt a second time. `admittedPromptEventId` is now the one derivation, and recovery asks whether the Turn has a prompt rather than whether it has one under that exact id: a Run written by an older build derived the id differently, and matching on the id would read its prompt as missing. Steering leaves that index — it is typed as a user message but is something said into an already-admitted Turn, so it is never the Turn's own prompt. The same review found the in-process mirror of that crash: `begin()` failing between opening the invocation and recording the prompt runs `failStart` -> `finalize`, whose terminal event seals the run against every later append, recovery's repair included. Before this cutover recovery could still append to `session_messages`, which has no seal; a sealed ledger cannot be repaired, so `finalize` records the prompt itself before sealing, next to the openInvocation call that already keeps the sibling rule "a run cannot end without having begun". The read marker's tail scan now pages past hidden records. It read one bounded page and gave up, so a Turn ending on tool traffic could leave a Session showing unread after it had been read. It never cleared falsely, so this is a badge, not a lost message. Two review points are not taken. The reviewer's fix for the id mismatch was to inline the derived id in `begin()`; that leaves the same string template in two packages, which is still two rules that happen to agree. The reviewer also read the importer's "convert only turns that still have a user row" filter as a silent drop with no producer. It has one: a turn whose only user row was steering belongs to a Turn some durable Root already owns, and converting it stands a second synthetic run beside that one. Ablating the filter fails `does not import Host-handed-off transcript messages as synthetic runs`, so it stays, with its reason written down. storage 1092 pass, runtime-host 1711 pass, runtime 3130 pass, cli 805 pass, 0 failures. Generated-by: Claude Code
The pending flag was set after `openInvocation()`, so the one failure it did not cover was a throw from the opening itself: `finalize` reopens what it can, and a run it manages to open then sealed with a terminal and no prompt — the same hole the previous commit closed, entered from one step earlier. Moving the flag ahead of the opening costs nothing when the invocation never opens: `finalize`'s reopen fails too, and the backfill is a no-op on a run that does not exist. Reported as a residual on #4879 and not asked for; it is one line and it closes the last entrance to a shape that cannot be repaired after the fact. runtime 3131 pass, runtime-host 1723 pass, cli 805 pass, 0 failures. Generated-by: Claude Code
b7d27cf to
816fa17
Compare
|
New finding at exact head What the code does
The reader gates on that. Every durable page goes through Why it mattersOpen a long legacy Session after upgrading — a long transcript, or one carrying large tool results — and peak memory and latency scale with the entire history rather than with a page or batch budget. If that exceeds what the process can do, or if it is interrupted, the next read starts the same full load again, because nothing durable records partial progress. The Session stays unavailable for as long as that keeps failing. Nothing is lost: the legacy rows are intact, the deterministic event ids make a re-run idempotent, and a successful run recovers the Session. That is why this is an availability and upgrade-cost regression rather than a correctness one, and why it is [P2] rather than [P1]. It is worth raising against this PR's own stated requirement. The design records that upgrade time needs a measured upper bound and crash-resumable execution. The deterministic ids deliver the idempotency half — an interrupted import re-derives the same events and the store dedupes them — but not the bounded half: there is no bounded read, no batch, and no recorded progress, so an interruption costs the whole conversion again rather than resuming. DirectionPage the legacy rows or turns rather than reading them all; persist a per-Session conversion watermark with each committed batch; and let the reader expose either a completed bounded prefix or an explicit "preparing" state instead of re-reading everything before every page. That keeps the idempotency this already has and adds the bound the design asked for. Test gap alongside itThe existing tests cover conversion semantics and restart idempotency, and they cover them well. What I could not find is a test that drives the production reader through an oversized legacy transcript, or one that asserts bounded peak work and forward progress across a restart. Without that, the property above has no guard: a later change could make the conversion heavier and nothing would notice. Scope of this commentThis supersedes my approval at 简体中文在 exact head 代码做了什么
而读取路径以此为闸:每一个持久页在被返回之前都要过 为什么要紧 升级之后打开一个长的旧 Session —— 长 transcript,或带有大块工具结果的 —— 峰值内存与延迟按整份历史增长,而不是按页或批的预算。 如果它超出进程能承受的范围,或者中途被打断,下一次读取会重新开始同样的全量加载,因为没有任何持久记录保存部分进度。只要这件事持续失败,该 Session 就一直不可用。 没有东西丢失:旧行完好,确定性事件 id 让重跑幂等,一次成功的运行即可恢复该 Session。这也是它属于可用性与升级成本的回退、而不是正确性问题的原因,以及它是 [P2] 而不是 [P1] 的原因。 值得对照本 PR 自己写下的要求来看。 设计里记着:升级耗时需要一个可测量的上界与可从崩溃续跑的执行。确定性 id 交付了幂等那一半 —— 被中断的导入会重新派生出同样的事件、由 store 去重 —— 但没有交付「有界」那一半:没有有界读取、没有分批、也没有记录进度,所以一次中断的代价是整次转换重来,而不是续跑。 方向 对旧行或旧 turn 分页,而不是一次读完;每提交一批就持久化一个按 Session 的转换水位;并让读取方要么暴露一个已完成的有界前缀、要么暴露一个明确的「准备中」状态,而不是在每一页之前把所有行重读一遍。这样既保住它已经具备的幂等,又补上设计所要求的那个上界。 与之相伴的测试缺口 既有测试覆盖了转换语义与重启幂等,而且覆盖得不错。我没有找到的是:驱动生产读取路径穿过一份超大旧 transcript 的测试,或断言「峰值工作量有界」与「跨重启有前进」的测试。 没有它,上面那条性质就没有守卫 —— 日后某次改动让转换变重,不会有任何东西发现。 本条评论的范围 它取代我在
|
jackwener
left a comment
jackwener
left a comment
There was a problem hiding this comment.
Inline finding at exact head 816fa172.
jackwener
left a comment
jackwener
left a comment
There was a problem hiding this comment.
Two further findings at exact head 816fa172, one per line.
| state.permissionRequestById.set(request.requestId, { | ||
| requestId: request.requestId, | ||
| toolUseId: request.toolUseId, | ||
| toolName: request.toolName, |
There was a problem hiding this comment.
[P2] The cut removes a second authority but leaves a second reader, and nothing cross-checks them.
After this change there are still two places that decide what a transcript row is: this JS projection (RuntimeReadModel — getMessages, history, copy) and the SQL RuntimeTranscriptQuery used for UI paging, unread, and session.turns.query, which grew by ~436 lines on this head. The SQL side re-encodes the same rule rather than deriving it from this one, and no production path compares the two.
That is the same hazard this PR set out to remove, moved one level: the PR deletes the dual write authority, and what remains is a dual read definition. A row dropped on one side and not the other is silent — there is no error, just a transcript that reads differently depending on which path asked. compareRuntimeReadModelMessages exists, but only in tests.
I am not claiming a concrete divergence — I did not produce one, and this is graded on structure rather than on a reproduced loss. The reason it is still worth raising is that the final seek commit on this head shows this layer is load-bearing: it is being actively shaped, which is exactly when a second definition drifts.
Direction: have one side derive from the other, or add a production-path comparison rather than a test-only one, so a divergence surfaces as a failure instead of as a differently-rendered transcript.
简体中文
[P2] 这次切除移走了第二个权威,却留下了第二个读者,而没有东西在对照它们。
改动之后,「什么算一条 transcript 行」仍由两处决定:这里的 JS 投影(RuntimeReadModel —— getMessages、history、copy),以及用于 UI 分页、未读与 session.turns.query 的 SQL RuntimeTranscriptQuery(在本 head 上增加了约 436 行)。SQL 那侧是把同一条规则重新编码了一遍,而不是从这里派生,并且没有任何生产路径去比较两者。
这正是本 PR 想消除的那类危险,只是被挪高了一层:PR 删掉的是双写权威,留下的是双读定义。 一侧丢了一行而另一侧没丢,是静默的 —— 不会报错,只会让同一个 transcript 因为「谁来问」而读起来不同。compareRuntimeReadModelMessages 是存在的,但只存在于测试里。
我并不主张已经存在一处具体的分歧 —— 我没有打出来,这一条是按结构定级而不是按已复现的丢失。它仍值得提出,是因为本 head 上最后那笔 seek 提交说明这一层是承重的:它正在被积极改动,而那恰恰是第二份定义开始漂移的时刻。
方向: 让一侧从另一侧派生,或加一条生产路径上的比对(而不是只存在于测试里的),使分歧以失败的形式暴露,而不是以「渲染得不一样的 transcript」的形式存在。
Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
There was a problem hiding this comment.
Verified as a structure, deferred rather than fixed here — and I want to be explicit that this is a decision, not an oversight.
RuntimeTranscriptQuery arrived in b7d27cfe6 to give the reader the bounded seek #4791 asks for. Removing it would put back the unbounded read. So the two end states available are "derive one side from the other" or "compare them on a production path", and both are a larger design change than this PR should absorb on top of the migration fixes it is already carrying.
One correction to the framing: the SQL side seeks, it does not re-decide what a transcript row is. session-transcript-reader.ts still runs the selected events through projectRuntimeEventsToStoredMessages, so the JS projection remains the one place an event becomes a message. What is genuinely duplicated is the selection rule, which is narrower than a second definition of the row — but it is duplicated, and I am not claiming otherwise.
Leaving this open rather than resolving it, since nothing changed in response.
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
结论：当前 head 816fa17 尚不具备合入条件。两项功能问题已分别附行内评论：P1，正常升级后，已发布 runtime 写出的用户可见 warning 从 transcript 中消失；P2，旧转换器留下的随机 ID opening 使中断后的升级读取持续报唯一键错误。两项均通过真实 SQLite 和公开 reader/SessionManager 入口复现，旧数据库行本身仍保留。
此 PR 将 RuntimeEvents 作为 Session transcript 的持久权威，移除重复写入并增加查询及历史转换逻辑。本次覆盖转换、分页/active overlay、读模型、恢复和终态持久化；确认功能失败后，没有继续推进复杂度或风格评价。
构建及 283 项定点测试通过。全量测试只有一项 Bash sandbox 集成失败，已在 exact base 3697e63 重现同一失败，不归因于本 PR。当前 head 的 hosted checks 成功（Eval 跳过），但 GitHub 同时报告与 main 冲突。未执行 merge、APPROVE 或 REQUEST_CHANGES。
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
| }, | ||
| readDurablePage: async (sessionId, request) => { | ||
| await prepared(sessionId); | ||
| return durable.readPage(sessionId, request); |
There was a problem hiding this comment.
[P1] Migrate retained runtime warnings before switching existing Sessions to ledger-only reads
Released v0.2.0-dev.22.20260905 writes context_compaction_failed_open directly to the Session transcript in runtime-kernel.ts:1096–1104, without a corresponding RuntimeEvent. This is not one of the retired session-level kinds: the current core/session.ts:1146 explicitly retains it as a user-visible runtime note. A normal send already sets transcriptLedgerVersion: 1, so SessionManager.ensureTranscriptLedger skips conversion for these existing Sessions. This new ledger-only page consequently omits their stored warning.
Reproduced by running the released SessionManager/RuntimeKernel against real SQLite stores: send a normal turn, then compact with the backend returning failed/write_failed. The production kernel constructs and persists the warning; the compaction invocation is terminal and the header is version 1. The old public durable reader returns that warning. After opening the same databases with this head and calling the new reader through ensureTranscriptLedgerForRead, the identical warning-preservation assertion fails, although the original Session row still exists. The released producer files are identical to the PR base. No interrupted write or malformed fixture is needed.
Please add an explicit upgrade path for still-supported runtime notes before treating the old version-1 marker as evidence that every visible historical fact is in RuntimeEvents. Merely filtering the intentionally retired note kinds does not address this case.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
There was a problem hiding this comment.
Confirmed, and it is broader than the one kind you named — but I am accepting the loss rather than fixing it, so here is the whole reasoning.
Verified your chain in code: released sendMessage calls ensureTranscriptLedger and sets version 1; released runtime-kernel.ts:1104 and ai-sdk-turn.ts:2504 write the note with store.appendMessage / backend.appendMessage and no RuntimeEvent; this head skips conversion at version 1. Scanning the released ai-sdk-turn.ts, seven of the eight retained kinds are written transcript-only: context_provider_dropping, context_window_overrun, context_reported_window_exceeded, context_window_suggestion, context_overflow_after_compaction, context_compaction_failed_open, context_compacted. The retired kinds (abort, session_resume) are unaffected — their facts are owned by the terminal.
Why not repaired: these notes belong to turns a real run already sealed, and assertRunNotSealed is an interface obligation of every store, not a detail of one. Re-running the converter cannot reach them; putting them back would need a migration path that bypasses the seal, i.e. an exception on the guard that makes a run's ending single and final. That is a bigger change to the transcript authority than the thing it repairs.
Deriving them at read time instead only works for two of the seven. context_compacted / context_compaction_failed_open are recoverable in principle — contextCompactionOutcome is in the compaction terminal's stateDelta, and contextBudget.compactionDecisions is in the token-usage event, both released identically. But context-budget.ts:218 records that a fold reaches the user as one note per send, and mergeCompactionDecisionDiagnostics accumulates, so every later token-usage event in the turn carries the same decision. The writer gets "once" from a per-turn flag; a reader would have to get it from bounded pages, and two token-usage events of one turn can land on different pages. Writing the fact as its own event is the page-stable shape. The other five depend on the then-declared window, the provider-reported window, and a cross-session crossing anchor — mutable config, not ledger data.
So: hidden from the model, no effect on context or execution, bounded to Sessions a released build already touched, legacy rows intact. Recorded as the accepted cost of the cutover, with a comment at the version gate in session-manager.ts saying plainly that version 1 means a conversion ran, not that nothing is left in session_messages — so the next person does not build on it as proof.
Leaving this thread open, since I did not change the behaviour you reported.
A released build derived the transcript run id exactly as this one does but every event id with `newId()`. An interrupted conversion of its therefore leaves an opening this build cannot name, and appending a second one is refused by `runtime_events_one_opening_per_invocation` — so the Session's durable reads threw `UNIQUE constraint failed` on every later attempt, with its legacy rows intact and unreachable. The opening is adopted rather than rewritten: a run needs exactly one, the index refuses a second, and which id it landed under changes nothing a reader sees. Its converted messages are not adoptable the same way — rederiving them would stand a second, deterministic copy of each beside the one already there, and a Session that disagrees with itself is the failure this ledger exists to remove. Deleting the prefix instead would need a delete path into the transcript authority, which costs more than the shape it repairs. So a run holding such messages is sealed as the unfinished conversion it is: the legacy rows stay, and nothing reads that turn as converted whole. Reported by M4n5ter and hqhq1025, both against real SQLite. Ablation: with the adoption removed, `resumes a conversion a released build opened under a random event id` reproduces the reported UNIQUE constraint failure. Generated-by: Claude Code
Every durable page goes through `ensureTranscriptLedgerForRead`, and the converter read the Session's whole `session_messages` array before it wrote anything. So the first page of a legacy Session required loading all of it — the bound #4791 asks for ("a page must not require loading the full Session") and the one the base kept, since its reader paged over the same table. The converter now walks the rows forward a page at a time. A turn is only whole once a row of another turn follows it, so the last turn of a page is carried into the next rather than converted from a prefix of itself: peak memory is one page plus one turn, not one history. Durable progress needs no new record — a turn is skipped once its invocation has a terminal, which is what already made an interrupted import resumable. `openedAt` can no longer come from a count of every turn, because a paged conversion never holds one. It is derived from where the turn starts in the transcript instead, which keeps both properties the count gave it: every imported opening still sorts ahead of the Session's own runs, and turns keep the transcript's order. The high-water sequence rides along with each page so that derivation costs no read of its own. Reported by jackwener. Generated-by: Claude Code
|
Head Fixed
Deferred, stated rather than silent
Pushed back
runtime 3135, runtime-host 1726, storage 1110 — 0 failures. Thanks to @M4n5ter for fixing the three findings from the blind review directly, and to @hqhq1025 — the released-build compatibility case was reachable and neither of the earlier lanes had it. |
4 participants
Closes #4791.
The problem
Every execution fact was written twice: once as a
RuntimeEvent, once as asession_messagesrow. Two authorities for one fact means every writer has to keep them in step, every reader has to choose one, and a crash between the two writes leaves a Session that disagrees with itself.What this does
Makes the
RuntimeEventledger the only durable transcript authority and deletes the second write.session_messagessurvives in exactly two roles: input to the one-way importer that converts a pre-ledger transcript on first read, and the WorkHub Coordination Session's own store, which is out of scope for this issue.The six commits are the staging of one cutover — notes onto the ledger, a whole and resumable conversion, the read model, then the cut — and are meant to land together. Losing a legacy tool card in conversion is acceptable; losing a Session or a conversation is not, and the importer is a total function over every legacy row type.
The cut, site by site
markMessagesHandedOffno longer projects admitted Messages into transcript rows. It validates the admission and retires it. The proof those rows carried already lives in the agent-run admission'ssourceMessagesand in the RuntimeEvent steering proof.lastMessagePreview,lastMessageAt,connectionLocked) used to fall out of a transcript insert.AgentRunnow commits it explicitly throughcommitMessageCatalogProjection: fail-closed for a user message, because that write also takes the Session's one-way connection lock, and fail-open for the assistant preview, which costs a stale sidebar line at worst.lastReadMessageIdhas no client consumer, sohasUnreadis the only decision left, and it clears when the client has caught up with the ledger's newest visible message — read off a bounded tail of the last run rather than a whole-Session scan.${runId}-admitted-prompt; recovering the same crash twice writes the same event and the store dedupes it. A sealed Run takes nothing — it is immutable, and a Run that reached its terminal fact has a prompt the crash did not eat.core_root_source_message_proofs— the Root admission that consumed the Message, in the same database and as durable as the Session.Deleted with their last caller:
markSessionReadThroughMessage,SessionReadMarkerMessageNotFoundError,readMessagesForRecovery(byte-identical toreadMessages),listForRecovery's separate query, the transcript-ordering privates in the SQLite store, andbuildTurnStateMessagewith its lineage types.Migration
transcriptLedgerVersiondistinguishes the three states: absent means pre-ledger and converted on read,0means an imported transcript staged for conversion,1means ledger-authoritative. Event ids are derived from the run and the position within it, so an interrupted import is resumable — re-running it writes the same events and the store dedupes them.Ablations kept out
existing.some(role !== 'system')guard in recovery layered on top of the terminal-event check: the terminal check alone is exact, so the extra read was removed.appendMessage: pure test churn with no production gain, so it stayed.Verification
Net -2545 lines. On this base, all five workspace suites pass with zero failures: storage 1092, runtime-host 1709, runtime 3129, core 821, cli 805.
Not covered by automated tests and worth a human pass before merge: opening a Session created by an older build and confirming its whole history renders, and the desktop WorkHub view after a delegated Message has been consumed by a Turn.