Skip to content

fix(desktop): bind Side Conversation events to Host admission - #3716

Open
Astro-Han wants to merge 27 commits into
apache:mainfrom
Astro-Han:fix/side-conversation-event-ownership
Open

fix(desktop): bind Side Conversation events to Host admission#3716
Astro-Han wants to merge 27 commits into
apache:mainfrom
Astro-Han:fix/side-conversation-event-ownership

Conversation

@Astro-Han

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

Copy link
Copy Markdown
Contributor

Summary

  • Give every Side Conversation send or steer one admission ticket; Runtime Host remains the sole authority that accepts or retracts it and names the exact owning Turn.
  • Keep an already owned Turn and a pending admission as two simultaneous Host facts. The active Turn continues streaming while the new ticket waits; a queued-ticket retraction clears only that ticket.
  • Bind direct starts from the Host command result. For queued current-Turn and successor-Turn admission, consume only the Host-owned message_admission fact; unrelated terminal, steering, and queue events never infer ownership.
  • Buffer only events whose owner is not known yet. Events for the already owned Turn continue through the normal live projection immediately.
  • Preserve unresolved admission across recoverable subscription failure and Host replacement. The complete catalog-preflight-plus-observer-registration operation uses the existing reconnectable Desktop IPC seam.
  • Scope admission projection, retry, unknown-outcome, and Stop fencing to canonically labelled Side Conversation Sessions. Ordinary Desktop/CLI streams keep their previous contract.

No durable message schema, settlement/recovery policy, CLI projection, or Desktop primary-conversation projection is changed.

Review follow-up

  • A queued steer Stop now waits for the Host retracted event instead of treating a successful Stop RPC as global idle. The still-running Turn remains owned and interruptible, and a second Stop can target it.
  • Stop fencing is owned by the current Stop promise, so a late failure from an older queued-ticket Stop cannot release a newer active-Turn Stop.
  • A queued ticket entering the active Turn now produces the same Host-owned message_admission fact as successor admission; observation reseeding replays that fact while the in-flight queue entry remains authoritative.
  • If observation misses the entire in-flight window, reseeding recovers the same exact ticket/Turn ownership from the existing durable steering UserMessage; no renderer recovery state or new protocol field is needed.
  • sessions:observe is registered as one reconnectable read operation, so Runtime Host replacement retries catalog preflight and observer registration together.
  • The React test seam now forwards to the latest Hook callbacks; the previous captured callback could leave steer tests exercising a stale turnInFlight=false closure.

Behavior evidence

The production-shaped React hook + Workbar seam covers:

  • old Turn settled, new send waiting for Host admission, then a late old terminal event;
  • the active Turn continuing to project text while a steer waits for admission;
  • successor events replaying only after the exact ticket is admitted;
  • queued-ticket Stop preserving active-Turn streaming and allowing a later active-Turn Stop;
  • an older Stop failure not releasing a newer Stop fence;
  • explicit queue retraction using the submitted ticket;
  • recoverable subscription failure and Host replacement preserving the original ticket;
  • busy retry identity, fork/copy cleanup, disposal, and observation-seed failure.

Protocol boundary

A full Host replacement can discard an old in-memory queued ticket before it reaches either an active Turn or a durable successor admission. Current Host facts can then prove neither admitted nor retracted; the existing submit contract intentionally reports outcome_unknown. This PR preserves that ticket instead of inventing cancellation. Tickets that reached an active Turn are recovered from the durable steering message. Fully settling the never-admitted queue-only case requires a prior Host protocol fact for the durable terminal disposition of a queued admission; it is outside this PR's storage/recovery scope.

Verification

  • Desktop typecheck: preload, main, renderer, and Storybook
  • 107 focused Desktop tests across Quote Companion, Runtime Host execution/observation/replacement, disposal, and Workbar adapter
  • 68 focused Runtime Host projector/protocol tests
  • 36 Runtime event-mapper tests
  • production renderer build and third-party notice check
  • ASF source-header check
  • git diff --check

Exact pushed head: e896f4648, rebased onto current origin/main. Exact-head CI is green, including Desktop e2e, Runtime Host, Browser/Storybook smoke, alignment, and installed CLI candidate validation. Earlier pre-rebase runs reached 64/66 Desktop e2e tests before the slash-command menu DOM-stability assertion failed; the rebase removed that failure and the full five-test Slash menu spec also passes locally.

AI use

Codex contributed the implementation, regression tests, verification, and adversarial/simplification review. No commit from Draft PR #3633 was cherry-picked or copied; its old branch/review was used only as failure evidence.

@Astro-Han
Astro-Han force-pushed the fix/side-conversation-event-ownership branch 2 times, most recently from 41aa14c to f51928a Compare August 24, 2026 19:38
@Astro-Han
Astro-Han marked this pull request as ready for review August 25, 2026 01:23

@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.

I reviewed this head and found blocking issues.

[P2] Queued steer Stop retracts the queued turn but drops UI ownership of the still-running turn

use-quote-companion.ts:312-320,723-758 with runtime-host-session-execution-ipc-main.ts:816-840. When a Side Conversation owns active A and steered S is still queued, Stop retracts only S without interrupting A. The renderer restores A but abandonAdmission() clears turnInFlight, so later A deltas render while the Composer appears non-streaming.

[P2] Ordinary Desktop observation now requires a catalog preflight that may not recover

runtime-host-session-execution-ipc-main.ts:192-206 with preload.ts:1722-1775. If Host reconnect interrupts getSession before registry registration, no observation is registered and live events are lost until remount. Previously sessions:observe registered directly.

Additional standards note: UI changes should include before/after visual evidence per CONTRIBUTING.md:85.

Checks on 364dd1a96 are test: success but mergeable: dirty — needs rebase.

简体中文存在两项状态机/观测阻断,需修复后再审。

@Astro-Han
Astro-Han force-pushed the fix/side-conversation-event-ownership branch from 364dd1a to b2c9946 Compare August 25, 2026 03:39
Derive admission observation from the Host-owned Session mode and keep ordinary Session streams unchanged. Remove the duplicate renderer send intent and tighten continuity decoding.

Generated-by: Codex
Keep Session event listeners bound to the current validated Host target epoch instead of filtering replacement events through the original host identity.

Generated-by: Codex
Reconcile buffered Host admission events when Stop outcome is unknown and bind successor/retraction fixtures to the actual submitted ticket.

Generated-by: Codex
Follow replacement target epochs within the owning profile while rejecting same-named Session channels from other Hosts.

Generated-by: Codex
Assert that replaying admission and text after an unknown Stop restores the real Host Turn streaming projection.

Generated-by: Codex
@Astro-Han
Astro-Han force-pushed the fix/side-conversation-event-ownership branch from 695ddfa to e896f46 Compare August 25, 2026 04:24

@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.

I reviewed this head and found a blocking issue.

[P2] Queued Stop succeeds on the Host but the retraction event is lost before observation

use-quote-companion.ts:719-736 keeps the admission pending after a queued Stop, waiting for message_admission(retracted). The Main path confirms and awaits the exact queue retraction, then returns void (runtime-host-session-execution-ipc-main.ts:822-829). The projector recovery seed only replays current admissions, so with an empty queue no terminal fact is projected and replacement does not compare old/new queues. The UI stays pending with processing=true, blocking further sends/steers, and stopRequestRef suppresses future Stops.

Fix: have sessions:stop return a typed retracted | interrupted(turnId) outcome and consume it, or persist a replayable terminal admission fact; cover queued send/steer → Stop success → dropped live retraction → empty-queue reseed.

Checks on e896f46483 are test: success.

简体中文该头仍有排队停止后身份丢失导致的永久 pending。

@Astro-Han

Copy link
Copy Markdown
Contributor Author

Addressed in 1a5d92d.

The failure was at the existing Host command boundary: Main had already awaited the exact queued-entry retraction, but sessions:stop erased that fact into a void reply, leaving the renderer dependent on a lossy live message_admission(retracted) event.

The Stop reply now carries only the confirmed queued retraction identity ({ kind: retracted, messageId }). The Side Conversation hook releases admission only when that identity matches its current pending message. A rejected Stop still means unknown and preserves admission; live admission events remain the idempotent observation path for other observers. No durable terminal state or second authority was added.

Regression coverage now drops the live retraction entirely for queued send and queued steer, verifies the admission clears, preserves the active Host Turn streaming state, and verifies the next Stop targets that active Turn. Local verification: Desktop typecheck plus 46 focused Side Conversation, Runtime Host IPC, Workbar adapter, and WorkHub adapter tests. Exact-head CI is running.

@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

I reviewed exact head 1a5d92d87229f873bf0f540da29f3e8eaf2c37e5 and found one blocking issue: a Host-retracted busy-raced send can still permanently consume its staged quotes even though the send returns false and keeps the rest of the draft for retry.

The exact-head test check is green. I also ran 238 focused Runtime Host, runtime-mapper, Desktop execution/observation, and Side Conversation ownership tests; the existing suite passes because its retraction probes use no staged quotes. The inline comment includes a production-hook reproduction and the repair direction.

简体中文

我审查了精确提交 1a5d92d87229f873bf0f540da29f3e8eaf2c37e5,发现一项阻断问题:Host 撤回忙碌竞态中的发送后,发送会返回 false 并保留草稿的其他部分,但已暂存的引用仍会被永久消费。

当前提交的 test 检查为绿色。我还运行了 238 项 Runtime Host、runtime mapper、Desktop 执行/观测与 Side Conversation 所有权相关测试;现有测试会通过,是因为撤回探针没有携带已暂存引用。行内评论包含生产 Hook 复现与修复方向。

}
return { status: 'error', code: 'send_rejected' };
}
deps.onQuotesConsumed();

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

[P2] Keep staged quotes when Host retracts a busy-raced send

This callback runs for every ok: true reply, including { steered: true, messageId }, before the Hook reconciles that ticket with the Host admission stream. In use-quote-companion.ts:640-658, the exact ticket can then already be retracted (or its queued Stop can be confirmed), causing send() to return false. The panel therefore keeps the draft and attachments for retry, but use-workbar-controller.ts:709-712 has already removed the staged quote chips permanently.

I reproduced this through the production Hook with one staged quote: deliver message_admission(outcome='retracted') for the submitted ticket before resolving sideChat.send as ok:true, steered:true. The result is send() === false while the quote-consumption callback runs once. Existing retraction/Stop probes cannot catch this because QuoteCompanionOwnershipProbe always passes pendingQuotes: [] and a no-op consumer.

Please defer quote consumption for steered sends until the exact ticket is authoritatively admitted, as the pending/outcome-unknown branch already does. Direct starts can still consume after immediate Turn binding; retracted or confirmed-stopped tickets must retain their quote snapshot. A regression should use a non-empty staged quote and assert zero consumption when the send returns false.

简体中文

[P2] Host 撤回忙碌竞态发送时应保留已暂存引用

这里会在 Hook 与 Host admission 流核对 ticket 之前,对所有 ok: true 回复调用消费回调,包括 { steered: true, messageId }。随后 use-quote-companion.ts:640-658 可能发现该 ticket 已被撤回,或排队 Stop 已确认,于是 send() 返回 false。面板会保留草稿和附件以便重试,但 use-workbar-controller.ts:709-712 已经永久移除了引用 chips。

我通过生产 Hook 携带一个已暂存引用复现:先为该 ticket 投递 message_admission(outcome='retracted'),再让 sideChat.send 返回 ok:true, steered:true。结果是 send() === false,但引用消费回调执行了一次。现有撤回/Stop 探针无法发现它,因为 QuoteCompanionOwnershipProbe 始终传入 pendingQuotes: [] 和空消费回调。

请像 pending/outcome-unknown 分支一样,把 steered 发送的引用消费延后到精确 ticket 被 Host 明确接纳之后。直接启动仍可在立即绑定 Turn 后消费;已撤回或已确认停止的 ticket 必须保留引用快照。回归测试应携带非空引用,并断言发送返回 false 时消费次数为零。

@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.

I reviewed exact head 6dc0af38a512ca97c903da95793118e1c1566cc2 and found two P1 blockers and one P2 reconnect defect.

[P1] Re-resolve the admission against the final Stop snapshot

runtime-host-session-execution-ipc-main.ts:819-862 can acknowledge Stop without stopping the Turn that now owns the ticket. The first snapshot can bind expectedAdmissionId to Turn A, then production beforeStop() awaits Computer Use resource release. If the Host assigns the same ticket to successor Turn B during that await, the final snapshot sees B and returns undefined; no interrupt is sent. The renderer treats every fulfilled pending-admission Stop as confirmed and leaves its Stop request fence set unless it receives an explicit retraction, so B keeps running and later Stop clicks are ignored until settlement or panel recreation.

I reproduced this through the registered production IPC handler: beforeStop changed A/in-flight to B/running with the ticket in rootTurnSourceMessageIds; the call fulfilled with undefined, B remained running, and interruptTurn was called zero times. Existing tests use static snapshots and cannot hit the ownership change inside the await. After the final snapshot, resolve the same admission ticket again. If it now belongs to B, interrupt B; if disposition cannot be proven, reject as unknown rather than reporting a successful no-op.

[P1] Do not publish a revocable in-flight lease as final Turn ownership

session-projector.ts:514 treats every transition into in_flight as message_admission(ticket, A), but that lease is still revocable. The backend can fail attachment materialization, abort before durable consumption, or lose its consumer, and drainSteeringInto() then nacks it back to queued. The renderer has already consumed its sole pending ticket and bound A; the projector emits no retraction for in-flight -> queued, and when the ticket is later folded into successor B, the second admission is ignored because no pending ticket remains. B's live answer is therefore invisible, and on this head staged quotes are also consumed at the false A admission.

A production Hook probe performed A start -> ticket admission A -> A completion -> the same ticket admitted to B plus B text. The final live Turn id stayed empty and B's text never rendered. Current tests cover queued -> in-flight and queued -> successor independently, not in-flight -> nack -> queued -> successor. Publish final ownership only after durable ack/root-source admission, or keep ticket ownership rebindable until the Host's durable admission is known. Stop must likewise distinguish a revocable lease from durable Turn ownership.

[P2] Retry a dispatched queued retraction with the same identity

At runtime-host-session-execution-ipc-main.ts:825-832, queued Stop calls retractQueueEntry directly even though this PR adds idempotent reconnect retry for Side Conversation start/submit. If the Host commits the retract and the connection closes before the reply, RuntimeHostRequestInterruptedError(dispatch='dispatched') is propagated. The renderer correctly keeps the admission unknown, but the replacement seed cannot replay the retraction because the ticket is now absent; the panel remains processing, and a retry cannot find the deleted ticket. Recreating the panel is the only recovery.

A production-handler probe recorded the first retraction and then threw a dispatched interruption. The handler rejected immediately and made no reconnect/retry, while retrying with the original id would be safe. Generate retractId once, wait for reconnect, and retry the exact operation with that same identity; add the dispatched-retract plus dropped-live-event case.

The current head does close the previous staged-quote retraction issue, and the compatibility epoch/strict snapshot decoding are correct. I materialized the current-main merge ref c0f73f9cd27bddf0a1ac8bb25d0e2c8694e3f860: the full repository and Desktop renderer build passed, along with 143 focused merge-result tests. Those green checks do not exercise the three interleavings above. Exact-head hosted test is currently queued.

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.

3 participants