mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
24a13632c7
* feat(daemon): deliver web-shell mid-turn messages into the running turn
Let the web-shell hand a message typed while a turn is running to that turn instead of holding it until the next turn. The daemon now answers the ACP child's `craft/drainMidTurnQueue` ext-method from a per-session queue the browser feeds; previously BridgeClient had no `extMethod`, so the child got -32601 and latched the drain off for the session.
Server: `SessionEntry` gains a mid-turn queue; `bridge.enqueueMidTurnMessage` accepts only while a turn is active and the queue is emptied at the idle boundary; `BridgeClient.extMethod` drains it and publishes a `mid_turn_message_injected` SSE frame. A new `POST /session/:id/mid-turn-message` endpoint plus DaemonClient/DaemonSessionClient methods feed the queue.
Browser: `enqueuePrompt` also pushes text-only messages to the daemon; a sidechannel hook drops the matching entries from the local queue when the injection frame arrives. A message is therefore delivered exactly once — mid-turn when a turn is live, or via the existing next-turn queue otherwise — and never both. Exactly-once rests on the injection frame arriving in order ahead of the turn-complete frame, plus the dedupe effect running before the next-turn drain.
Out of scope: live "sent" rendering of an injected message (it shows on reload); ACP-transport (non-REST) ingestion parity.
* fix(daemon): address mid-turn drain review — exactly-once + hardening
Review follow-ups on the web-shell mid-turn drain.
[Critical] The injected-message sidechannel was single-slot (latest-wins), so two drain frames landing back-to-back — a multi-batch turn, or a backgrounded tab flushing buffered SSE — coalesced: the first batch's messages were never removed from the browser queue and got resent next turn = double delivery, the exact failure this feature prevents. The sidechannel now ACCUMULATES batches; the consumer reconciles every batch and then clears. The queue-dedup is extracted into a pure `removeInjectedFromQueue` helper and unit-tested (the App.tsx path had no test, so this regressed silently).
Hardening and tests:
- Cap mid-turn message length (server, 16 KB) and per-session queue depth (bridge, 20), matching the bounds on the sibling /btw and /prompt; over-cap returns `{accepted:false}` and the browser keeps the message for its next-turn queue.
- Drop the dead try/catch around `EventBus.publish` (never-throws contract — "don't wrap publish()"); check the return value and emit one diagnostic line per non-empty drain.
- Assert the settle-clear: a new test seam exposes the agent-side connection so a test can drive `extMethod('craft/drainMidTurnQueue')` after settle and assert the leftover was cleared (not re-drained next turn), plus the back-to-back FIFO survival case.
* fix(daemon): address Copilot review — trim consistency + doc accuracy
- POST /session/:id/mid-turn-message now length-checks and enqueues the TRIMMED message (it was checking the raw `message.length` while the bridge stores the trimmed value), so whitespace-padded input whose real content fits is no longer rejected.
- Correct the `mid_turn_message_injected` docs (events.ts payload + bridgeClient.ts): it is a transient dedupe signal, not a transcript render — the message reaches the model mid-turn and the persisted transcript shows it on reload.
* fix(daemon): address review — client-ownership gate + per-originator mid-turn dedupe
Addresses the /review main finding and doudouOUC's inline comments on the
web-shell mid-turn drain.
- Authorize the mid-turn endpoint per session (review main finding): the
route now forwards the client id via `parseClientIdHeader` and
`enqueueMidTurnMessage` runs `resolveTrustedClientId` before queuing —
mirrors `/prompt` and `/btw`, so a token-holding client bound to another
session can no longer push into this turn (throws `InvalidClientIdError`).
- Route the drain's SSE echo per originator (doudouOUC #3417739340): the
trusted client id is recorded on each queue entry and the drain publishes
one `mid_turn_message_injected` frame per originator carrying
`originatorClientId`, so a peer on the same session can't dedupe a
coincidentally-equal entry it never queued.
- Wire the web-shell consumer to its own client id: the daemon now stamps
every drained frame, and the web-shell always sends a client id, so
`removeInjectedFromQueue` must filter on it. Plumbed `clientId` onto
`DaemonConnectionState` (set from the bound session) and passed
`connection.clientId` into the dedupe — without this the new filter would
skip every batch, leaving our own messages to be resent next turn (the
exact double-delivery this feature prevents).
- Tests (doudouOUC #3417739347 + coverage for the above): per-originator
publishing, the `published === false` (bus-closed) degradation, the
endpoint ownership gate, end-to-end originator stamping, and the
web-shell originator-filtering matrix (match / peer-skip / anonymous /
mixed / missing-id regression guard).
- Comment-only: point `MAX_MID_TURN_QUEUE_DEPTH` at
`maxPendingPromptsPerSession` as the promotion model (doudouOUC #3417739352).
* fix(daemon): address review round 2 — mid-turn races, route tests, observability
Addresses the qwen3.7-max /review pass on the web-shell mid-turn drain (3
criticals + 6 suggestions).
Criticals
- consume() race (sidechannel): the buffer is read during render but reconciled
in an async effect, so a frame appended in that window was wiped by an
unconditional clear → resent next turn (double delivery). `consume` now does a
compare-and-swap — it only clears if the buffer still holds the exact snapshot
it reconciled; a newly-arrived batch survives to the next reconcile.
- Late-arriving enqueue (web-shell): the fire-and-forget mid-turn POST is now
scoped to a per-turn AbortController, aborted when the turn settles, so a slow
push can't land during a SUBSEQUENT turn and be injected twice. An aborted
push resolves `{ accepted: false }`, so the message just follows its normal
next-turn path.
- HTTP route had zero tests: add a `POST /session/:id/mid-turn-message` suite
(accept, reject, missing/empty/oversized body, unknown session, malformed
client id) plus the fakeBridge wiring it needed.
Suggestions
- Telemetry: register `mid-turn-message` in the daemon route regex so the
endpoint gets a route label / spans / latency like its siblings.
- Single source of truth for `craft/drainMidTurnQueue`: export
`MID_TURN_QUEUE_DRAIN_METHOD` from acp-bridge and import it in both the
answerer (BridgeClient) and the caller (Session.ts), so a rename can't desync
them into a silent -32601 latch.
- Observability: `enqueueMidTurnMessage` now logs idle/empty/full rejects and
the drop-at-settle path (the drain already logged); rejects are low-volume
(the browser only pushes when it believes a turn is live).
- Buffer safety cap (sidechannel): bound the accumulating buffer and evict
oldest so an orphaned consumer can't grow it without limit.
- Tests: depth-cap overflow + trimming (bridge), compare-and-swap + cap
(sidechannel).
* fix(daemon): scope mid-turn consume to reconciled batches; correct originator doc
Addresses the follow-up /qreview pass.
- Cross-session wipe (web-shell): the dedupe reconcile is session-scoped, but
`consume()` cleared the whole accumulating buffer. The buffer is a
cross-session singleton, so a late `mid_turn_message_injected` frame for the
PREVIOUS session (e.g. after an in-place `/resume` switch) was wiped
un-reconciled and lost on switch-back → resent next turn = double delivery.
Replace the blanket clear with identity-removal: `consumeSidechannelMidTurnInjected(handled)`
drops only the batches actually reconciled (the active session's). Batches for
other sessions — and frames that arrived after the render snapshot (the
render→effect race the prior compare-and-swap covered) — are not in `handled`
and stay buffered for their own reconcile. So this subsumes the race fix and
adds multi-session correctness. `consume` is now stable (no per-render churn).
- originatorClientId doc (SDK): the field is declared on
`DaemonMidTurnMessageInjectedData` (the `data` shape) but, unlike the sibling
permission events, this event is not reduced and the daemon never merges the
id into `data` — it rides the SSE envelope (`event.originatorClientId`) and is
lifted into `data` only by the web-shell's own parser. Document that so an SDK
consumer doesn't read an always-undefined `data.originatorClientId` and treat
every batch as anonymous (dedupe-for-all foot-gun).
- Tests: identity-removal across the race, the cross-session leave-behind, and
the already-evicted no-op.
* fix(daemon): mid-turn robustness — fetch timeout, observability, docs, tests
Addresses the latest /review pass (DeepSeek + qwen3.7-max). Stale duplicates of
already-shipped fixes (shared drain-method constant, depth-cap test, consume
cross-session/race) are answered inline; the substantive new items:
- DaemonClient.enqueueMidTurnMessage now routes through `fetchWithTimeout` like
every other method, so a hung daemon can't wedge the void-ed caller in
actions.ts forever. The helper composes the caller's signal with its timeout,
so the turn-settle abort still propagates. (+ propagation and timeout tests.)
- Browser-side observability (mirrors the server-side writeStderrLine added
earlier): the actions catch logs non-abort failures at debug (an abort is the
designed settle cancel, kept silent); the settle-abort and sidechannel buffer
eviction each get a `console.debug`; and a debug warns when stamped batches
arrive but `connection.clientId` is undefined (dedupe would skip them).
- Docs: spell out the `originatorClientId` CONTRACT — a consumer that dedupes
MUST compare it against its own client id (the daemon broadcasts, it does not
route), or it drops another client's coincidentally-equal message.
- Tests: `POST /session/:id/mid-turn-message` InvalidClientIdError → 400; the
SSE event-pump routing of `mid_turn_message_injected` to the sidechannel (not
the transcript); DaemonClient signal propagation + hung-daemon timeout.
68 lines
2.3 KiB
TypeScript
68 lines
2.3 KiB
TypeScript
/**
|
|
* @license
|
|
* Copyright 2025 Qwen Team
|
|
* SPDX-License-Identifier: Apache-2.0
|
|
*/
|
|
|
|
export interface MidTurnQueueItem {
|
|
text: string;
|
|
images?: unknown[];
|
|
}
|
|
|
|
export interface MidTurnInjectedBatch {
|
|
sessionId: string;
|
|
messages: readonly string[];
|
|
/** Trusted client id that queued the messages (from the SSE envelope). */
|
|
originatorClientId?: string;
|
|
}
|
|
|
|
/**
|
|
* Reconcile injected mid-turn messages against the local pending queue: remove
|
|
* the first text-only entry matching each injected message for `sessionId`,
|
|
* across ALL `batches` (a multi-batch turn drains once per tool batch, so the
|
|
* consumer must process every accumulated batch, not just the latest).
|
|
*
|
|
* Matching is count-based — one removal per injected message — so a queue that
|
|
* holds the same text twice loses one entry per matching injection. Entries
|
|
* carrying images are never matched: image messages aren't pushed mid-turn (the
|
|
* drain channel carries plain strings), so they stay queued for the next turn.
|
|
*
|
|
* Skips a batch whose `originatorClientId` is some OTHER client: the daemon
|
|
* broadcasts the injection frame to every client on the session, but only the
|
|
* client that queued the message should drop it — a peer with a coincidentally
|
|
* equal text must keep its own entry. Batches with no originator (anonymous
|
|
* push) are reconciled regardless.
|
|
*
|
|
* Returns a NEW array when something was removed, or `null` when nothing matched
|
|
* (so the caller can skip a redundant state update).
|
|
*/
|
|
export function removeInjectedFromQueue<T extends MidTurnQueueItem>(
|
|
prompts: readonly T[],
|
|
batches: readonly MidTurnInjectedBatch[],
|
|
sessionId: string,
|
|
clientId?: string,
|
|
): T[] | null {
|
|
const remaining = [...prompts];
|
|
let changed = false;
|
|
for (const batch of batches) {
|
|
if (batch.sessionId !== sessionId) continue;
|
|
if (
|
|
batch.originatorClientId !== undefined &&
|
|
batch.originatorClientId !== clientId
|
|
) {
|
|
continue;
|
|
}
|
|
for (const message of batch.messages) {
|
|
const index = remaining.findIndex(
|
|
(prompt) =>
|
|
prompt.text === message &&
|
|
(!prompt.images || prompt.images.length === 0),
|
|
);
|
|
if (index >= 0) {
|
|
remaining.splice(index, 1);
|
|
changed = true;
|
|
}
|
|
}
|
|
}
|
|
return changed ? remaining : null;
|
|
}
|