Skip to content

fix(runtime-host): unify durable message lifecycle - #3633

Open
Astro-Han wants to merge 29 commits into
mainfrom
fix/steering-durable-admission
Open

fix(runtime-host): unify durable message lifecycle#3633
Astro-Han wants to merge 29 commits into
mainfrom
fix/steering-durable-admission

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Queued messages were split across transcript rows, an in-memory Host queue, Client bindings, and recovery-time inference. A Host restart could therefore lose an accepted message, replay a discarded one, or change the capabilities under which it ran.

This change gives the lifecycle four explicit authorities:

  • durable message admissions own accepted identity, current placement, and queue order
  • the Runtime Host in-memory queue owns only live delivery and leases
  • durable provider/root/Stop/retraction facts own settlement
  • the transcript owns presentation only

Admission and transcript materialization now share one SQLite transaction. Promotion and reorder are durable-first. Recovery reads unresolved admissions and starts them through the normal successor-root path; it no longer scans the transcript or waits for the original Client. Retracted identities retain only their original placement and canonical content digest, so they cannot be resurrected while message contents are removed.

A queued successor executes under the durable Session contract. It does not inherit session-, turn-, call-, or initiating-Client bindings, and an explicit durable capability marker prevents online call-affine providers from being discovered later.

Stop is settled only by the immutable terminal Stop fact. If the Host dies before that fact, Stop was not confirmed and accepted messages recover; after the fact, they do not.

The parallel embedded RuntimeKernel queue and its dead admission path/tests are removed. The CLI and Desktop continue to render from canonical transcript/Host projections rather than owning fallback delivery state.

Schema migration

  • core execution schema v6 adds durable message admissions, queue order, and minimal retraction identity proofs
  • the unmerged v5 pending-steering shape is discarded because its old two-write protocol cannot prove that a row was accepted
  • Session retirement removes both pending admissions and settlements

Verification

After rebasing onto current main:

  • Storage, Runtime, Runtime Host, and CLI builds pass
  • CLI transcript/TUI focused suites: 202 passed
  • Client Capability coordinator: 25 passed
  • Message coordinator: 48 passed
  • Root Turn coordinator: 48 passed
  • real Host restart/interrupt integration: 2 passed
  • SQLite admission, settlement, order, and v5-to-v6 migration: 5 passed
  • hosted Runtime owner tests: 3 passed
  • git diff --check

Repository-wide tests and the full Desktop Playwright suite were not run locally; CI owns full coverage.

Screenshots are not included because this revision changes lifecycle and projection ownership without changing visual styling or layout. The observable transcript/queue behavior is covered by focused CLI, Desktop contract, and Host integration tests.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Maka and Codex contributed implementation, tests, architecture analysis, and delegated read-only reviews. Material commits include Generated-by trailers.

Checklist

  • Tests cover the change and fail without it
  • Format, typecheck, affected builds, and focused suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — the simplification at the core of this is sound, and most of the deleted CLI fallback tests genuinely remove an unreachable client-owned authority. Production has a single SessionManager construction that always supplies Host authority, and these packages are private, so that removal holds. Two items below.

[P1] Steering admission has no atomic recoverable proof across the durable-append boundary

Admission durably appends the UserMessage before it mutates the in-memory queue and before the submit receipt exists. A Host crash inside that window leaves neither a queue entry nor any recoverable admission proof, while the durable append has already landed.

Retry then consults only root-source receipts and provider-consumption events, and SQLite here permits duplicate message IDs. So the same cut produces two different bad outcomes depending on timing: a message the user saw accepted can be lost outright, or retry can append the same message identity a second time before any single delivery has occurred.

The fix needs one atomic, recoverable admission fact spanning the append and the queue/receipt update, rather than an ordering that is only correct when no fault occurs. Please add a restart/fault regression that cuts exactly at the append→queue/receipt boundary; an ordering assertion in the happy path cannot show this.

[P2] The CLI never renders the durable append, so a successful steering send becomes invisible

The CLI deletes the pending Steering row but does not render the durable append live. subscription.transcript_advanced has no consumer anywhere under packages/cli — I checked, there are zero references — and transcript reload only happens on a tool result, a reconnect, or a terminal state.

During a long-running step the consequence is that the user presses Enter, the text leaves the editor, and the message then appears nowhere at all until one of those events happens. From the user's side a successful send is indistinguishable from a dropped one.

The rewritten test asserts the pending row's absence but never asserts that a conversation entry replaces it, so it passes on exactly the broken behaviour.

One observation, not a blocker

The fallback QueueEnqueueOutcome representation and RuntimeKernel's complete embedded queue authority both survive this change. That is consistent with the deletions being safe today, but it means the second authority still exists in the tree even though nothing in production reaches it.


ran: Core → Storage → Runtime → Runtime Host → CLI builds, all green; Runtime Host 50/50, CLI 235/235, Runtime steering 39/39; deletion-reachability probes for the removed fallback tests; independent confirmation that packages/cli contains no transcript_advanced consumer.
did-not-run: the hosted exact-head run 32647365821 (job 97213551805) was still in progress at the time of writing, so no terminal CI result is claimed here. Neither finding depends on it.

@Astro-Han

Copy link
Copy Markdown
Contributor Author

Follow-up on the CI caveat in my review: the hosted run at 946f4268282146af26f4ab2502fbcecba8fdfc35 has since finished and test is completed/success. Correcting the note that it was still in progress — the two findings above are unchanged, since neither depended on that result.

@Astro-Han
Astro-Han force-pushed the fix/steering-durable-admission branch from bf1b2f6 to 07a11c7 Compare August 23, 2026 16:36

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed on behalf of WAWQAQ under the standing Kabi review authorization, scoped to exact head 220bdebbae3d42c6e2c01f18f81fe80655fbadf5.

NO-GO: 2×P1. The old CLI visibility P2 is closed: subscription.transcript_advanced now reloads canonical stored messages and publishes a transcript replacement rather than a live-only echo. The large deletion is also a real simplification: it removes CLI fallback/live-steering ownership and leaves Host queue + durable transcript as the authority.

Exact-head hosted test is completed/success; the PR is OPEN, non-draft, and mergeable against current main. Local focused verification passed: message coordinator 40/40, Host message integration 4/4, Runtime steering recovery 8/8, CLI durable/transcript 23/23, Desktop contract 1/1. The two inline P1s still block approval. No merge performed.

Comment thread packages/runtime-host/src/server/message-coordinator.ts Outdated
Comment thread packages/runtime-host/src/server/message-coordinator.ts Outdated
Comment thread packages/runtime-host/src/server/message-coordinator.ts
@Astro-Han
Astro-Han force-pushed the fix/steering-durable-admission branch from 220bdeb to d526e8b Compare August 23, 2026 18:53

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Representative review action on behalf of @WAWQAQ; scope: fix-head re-review of the two previously reported Runtime Host steering-recovery findings; exact head d526e8bfd01762561bc80a45e979efbd0c5aeccf.

Result: NO-GO — 1×P1 remains.

The previous promote → Stop content-loss finding is closed: promotion now durably commits the steering admission before removing the follow-up entry, and a failed durability write leaves the entry retractable. Stop can still discard the live steering queue, but it no longer deletes the durable transcript row or fabricates provider delivery.

The automatic-recovery finding remains: the helper exists, but its only production call is behind a new-root admission path that the target crash state cannot reach. A production Host restart reproduction retains the durable user message but never delivers it to the model (durable row = 1, provider steering proof = 0, root admission chain = 1, where recovery requires a successor/root chain of 2). See the two inline comments for the wiring and mutation evidence.

Required repair boundary: do not fix this by replaying every durable steering row that lacks a provider proof. Crash-before-queue-commit and an intentional Stop-discard currently leave the same durable-row shape. Blind replay would resend a message after the user explicitly pressed Stop. The repair must durably distinguish recovery-eligible queue state from intentional discard (or provide an equivalent fence) while preserving Stop’s “no successor” contract.

Local verification on this head: focused coordinator tests 41/41, Runtime Host full suite green, and root build green. The PR is OPEN/non-draft/MERGEABLE; exact-head hosted test is completed/success. This review is COMMENT only; no approval or merge.

Comment thread packages/runtime-host/src/server/root-turn-coordinator.ts Outdated
Comment thread packages/runtime-host/src/server/message-coordinator.ts Outdated

@ARE404 ARE404 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head re-review at 064157d3f09039e7f9842be39d447db8f18a59c4. NO-GO: 3×P1, 1×P3.

The previously reported disconnected restart recovery is now wired at Host startup, and the normal promote path correctly waits for durable transcript admission before exposing steering. Successful Stop also now preserves the transcript row while suppressing delivery. The three inline P1s are new crash/recovery boundary defects in that replacement design.

[P3 — documented standard] This diff changes renderer/UI behavior and stories, but the PR description has no before/after screenshots or recording as required by CONTRIBUTING.md:85.

No other documented-standard violations. The admission-time transcript model also semantically supersedes #3316's synthetic in-flight projection; textual overlap there is not an additional correctness finding. git diff --check is clean, and the only expected hosted workflow for these paths is CI test, which is terminal SUCCESS (run 32668976841, job 97266724286).

Comment thread packages/runtime-host/src/server/message-coordinator.ts Outdated
Comment thread packages/runtime-host/src/server/message-coordinator.ts Outdated
Comment thread packages/runtime-host/src/server/message-coordinator.ts Outdated
@Astro-Han
Astro-Han force-pushed the fix/steering-durable-admission branch from ca08e9a to 7d6a5fb Compare August 24, 2026 04:44
@Astro-Han Astro-Han changed the title fix: persist steering at message admission fix(runtime-host): unify durable message lifecycle Aug 24, 2026
@Astro-Han
Astro-Han force-pushed the fix/steering-durable-admission branch from 7d6a5fb to 1343cee Compare August 24, 2026 04:49

@ARE404 ARE404 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head re-review at 1343cee25bddb40b84456d7f15f08d74d8ffc0f7. NO-GO: 3×P2, 1×P3.

The three P1 crash/recovery defects from my prior review are closed: promotion is now atomic with transcript materialization, Stop suppression waits for immutable terminal Stop proof, and queued successors execute under the explicit durable Session capability contract. The replacement design still has three independent product regressions, documented inline.

[P3 — documented standard] CONTRIBUTING.md requires before/after screenshots or a recording for UI changes. This PR changes observable CLI pending/transcript rendering and Desktop existing-session submission behavior, but the description explicitly omits media; focused tests do not satisfy that evidence rule.

git diff --check is clean. Exact-head dependency audit is SUCCESS; hosted test is still IN_PROGRESS, so I do not claim a terminal CI result.

中文摘要

本次复审锁定 1343cee2,结论为 NO-GO:3×P2、1×P3。旧 review 的三条 P1 均已关闭:promotion 与 transcript 写入已原子化,Stop 丢弃已由不可变终态事实约束,durable successor 也不再继承临时 client capability。

当前三条 P2 分别是:CLI 同一 steering 同时走 durable transcript 与 steering_message 两条渲染路径,导致重复显示;Quote Companion 在发送竞态中会把任意旧 turn 的首个事件误认成本次 turn,随后丢失真正 successor 的回复;follow-up promotion 未重新执行 queue/canonical projection 容量预检,边界大小输入可把 Host 推入 fail-stop。另有一条 P3:可观察 UI 行为已变化,但缺少 CONTRIBUTING 要求的前后截图或录屏。

当前 audit 已通过,test 仍在运行中。

Comment thread packages/cli/src/pi-tui-runner.ts
Comment thread apps/desktop/src/renderer/features/workbar/tools/side-chat/use-quote-companion.ts Outdated
Comment thread packages/runtime-host/src/server/message-coordinator.ts

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

English

Review result: NO-GO (3 × P1)

The architectural direction is sound: this change genuinely removes the RuntimeKernel/CLI fallback queue authorities instead of merely moving them. However, the new unified durable authority still has three reachable crash cuts that violate the PR's no-loss/no-resurrection/restart guarantees. I reproduced each with a focused probe on this exact head.

Validation completed: Core, Storage, MCP, Runtime, and Runtime Host builds; 107 focused Runtime Host tests; 18 SQLite core-execution tests; and 5 focused AiSdk steering tests. All existing tests pass. The three added diagnostic probes fail as described in the inline comments; they were removed afterward and the worktree is clean. Hosted audit is green; hosted test was still in progress at the final freshness check.

Simplify audit: production-like changes are +1299/-1085 and test-like changes are +1443/-1929. Across the relevant RuntimeKernel and CLI files, +179/-1009 removes pendingFallback (16 → 0 references), takePendingFollowup (5 → 0), and confirmLiveTurn (21 → 0). Please preserve that single-authority direction while closing these crash cuts; reintroducing a fallback owner would be the wrong fix.

中文

审查结论:NO-GO(3 × P1)

架构方向成立:这次确实删除了 RuntimeKernel / CLI 的 fallback queue authority,不是简单搬目录。但新的统一 durable authority 仍有三个可达崩溃切点,分别破坏本 PR 承诺的“不丢消息 / 不复活 / 可重启恢复”。我在这个 exact head 上分别做了最小探针复现。

已完成验证:Core、Storage、MCP、Runtime、Runtime Host 构建;Runtime Host 相关 107 个测试;SQLite core-execution 18 个测试;AiSdk steering 定向 5 个测试,现有测试全部通过。三个临时诊断探针均按内联评论描述失败,随后已移除,worktree clean。最终刷新时 hosted audit 已绿,hosted test 仍在运行。

简化审计:production-like +1299/-1085,test-like +1443/-1929;相关 RuntimeKernel 与 CLI 文件合计 +179/-1009,并把 pendingFallback(16 → 0)、takePendingFollowup(5 → 0)、confirmLiveTurn(21 → 0)完全删掉。修复以下崩溃切点时应保留单一 authority 方向,不要重新引入 fallback owner。

Comment thread packages/runtime-host/src/server/message-coordinator.ts Outdated
Comment thread packages/runtime-host/src/server/message-coordinator.ts
Comment thread packages/runtime-host/src/server/root-turn-coordinator.ts

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kabi exact-head review under the standing WAWQAQ review authorization, scoped to 1343cee25bddb40b84456d7f15f08d74d8ffc0f7.

NO-GO — Kabi grading: 2×P1, 2×P2, 1×P3. Four defects were independently reproduced after a sealed provisional judgment. To avoid opening duplicate threads, their current-head anchors are: provider-echo recovery loss (P1), materialized-source/no-Run startup failure (P1), Stop-recovery identity resurrection (P2 in this line), and double CLI rendering of one steering identity (P2). The non-duplicate P3 is inline.

The architectural simplification itself is real: durable Host/Storage state is now the message authority, while the old CLI fallback/live-steering ownership has been removed. The repair should preserve that direction. A post-seal promotion-capacity candidate was withdrawn after boundary probing: submit preflight reserves the worst-case revision width and rejects larger candidates before promotion. The Quote Companion race was not included in this line's independent count because the final hook-level reproduction was not completed.

Verification on this head: Runtime Host 1134/1134, CLI 423/423, UI 223/223, Desktop 1339/1339; hosted test and audit are completed/success. Fresh gate: OPEN, non-draft, MERGEABLE/BLOCKED, six unresolved P0–P2 threads, and zero current or stale APPROVED reviews. COMMENT only; no approval or merge.

Comment thread packages/runtime-host/src/server/message-coordinator.ts

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supplement to review 5005153696 on the unchanged exact head 1343cee25bddb40b84456d7f15f08d74d8ffc0f7: the Quote Companion hook-level probe completed before the stop instruction arrived and independently reproduced the existing P2 thread. The correct Kabi count is therefore NO-GO — 2×P1, 3×P2, 1×P3. The earlier sentence excluding this race from Kabi independent reproduction is superseded; all other findings and the promotion-capacity withdrawal remain unchanged.\n\nThe exact hook sequence was: settle turn-old; begin a second send and execute onBeforeSend while the Host result is pending (activeTurnId=null, in-flight true); deliver a late terminal event for turn-old, which claims the active ref; return turn-new, whose ID is then not installed because the ref is non-null; deliver turn-new text, which remains absent from the live projection; release the old settlement, after which the hook reports streaming false while the real Host turn remains active. The React hook plus fake Workbar production service seam reproduced this in 30.5 ms; temporary probe removed and worktree clean. COMMENT only; no approval or merge.

Persist a Host-owned pending steering admission before exposing it to the active Run. Recover unresolved admissions after interrupted Runs are terminalized, and settle them when provider consumption, successor root admission, or an intentional stop makes replay unnecessary.

Add a production startup regression for the crash between durable admission and the in-memory queue commit.

Generated-by: Maka
Route ordinary messages through the Runtime Host admission API, stop creating renderer-owned empty live turns, and keep transcript publication within the active subscription. Preserve visible live tails across sends, return sparse transcripts to latest before submission, and remove redundant live/durable filtering and unconfirmed-arm bookkeeping.

Generated-by: Codex
Update the real-framing execution test to cover the ordinary-message operation now used by Desktop instead of retaining the removed direct turn-start contract.

Generated-by: Codex
Keep revision and Skill inputs on the turn-start control path while ordinary messages use Host admission. Rebind Side Chat to the Host-owned Turn identity from either the first event or admission response, and assert remount continuity without requiring an unobservable React intermediate paint.

Generated-by: Codex
@Astro-Han
Astro-Han force-pushed the fix/steering-durable-admission branch from 1343cee to c6a899e Compare August 24, 2026 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants