Skip to content

Commit 971a69d

Browse files
秦奇claude
andcommitted
fix(daemon-ui): wenshao R6 — recovery flow chicken-and-egg + pending pointer
Three Criticals from R6 review (4351217188) all pointing at real bugs introduced by R4/R5 work — not false positives. Fixes plus regression tests. ## Critical 1 — same-session reconnect never clears the latch When the daemon emitted `state_resync_required`, the reducer set `awaitingResync = true`. The webui provider dispatched `assistant.done { reason: 'reconnected' }` after re-attaching SSE but never called `store.clearAwaitingResync()`. Result: events flowed in on the fresh stream but every one got dropped by the `applyDaemonTranscriptEvent` passthrough guard. Transcript appeared permanently frozen with no diagnostic clue (the `console.warn` fired on each drop, but the user wouldn't necessarily check DevTools). Fix: in `DaemonSessionProvider.tsx`, after dispatching the synthetic `reconnected` `assistant.done`, check `awaitingResync` and clear it BEFORE the new SSE event loop starts. ## Critical 2 — updateCurrentToolPointer breaks on undefined status In `upsertToolBlock`, a new tool block is created with `status: event.status ?? 'pending'`. But `updateCurrentToolPointer` was called with raw `event.status` — when undefined, the function's own `if (status === undefined) return;` guard short-circuited without ever pointing at the new (visually-pending) block. Result: `selectCurrentTool` returned `undefined` for daemon events that omitted the explicit `status` field, while the block sat at "pending" in the UI — invisible to the current-tool selector. Fix: pass the EFFECTIVE status (`event.status ?? 'pending'`) so the pointer logic mirrors the actual stored status. ## Critical 3 — clearAwaitingResync flow chicken-and-egg The earlier (R4) JSDoc documented the recovery flow as: "re-subscribe with `Last-Event-ID: 0`, then call clearAwaitingResync after replay drains." But while the latch is true, EVERY non-passthrough event is dropped at `applyDaemonTranscriptEvent`. So during the replay drain, zero events made it into state, and clearing the latch afterward did nothing — transcript permanently empty. Correct flow: clear FIRST, then stream events. Updated JSDoc on both `types.ts` interface and `store.ts` impl to document this clearly. Added a regression test (`clearAwaitingResync AFTER dispatching events: events ARE dropped`) that pins the correct flow in code. ## Regression tests (+3) - `undefined status` creates pending block AND sets currentToolCallId - clear-then-dispatch ✓ events flow - dispatch-then-clear ✗ events dropped (correct flow documentation) ## Validation | | | |---|---| | SDK tests | **175/175** (was 172, +3) | | WebUI tests | **9/9** | | SDK typecheck | clean | | WebUI typecheck | clean | ## Note on doudouOUC heads-up QwenLM#4469 (main → daemon_mode_b_main sync, 45 commits since 2026-05-19) will land soon. doudouOUC's note says rebase should be smooth (no daemon-ui surface conflicts). Will rebase on the cron's next pass after QwenLM#4469 merges. Generated with AI Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent e394f49 commit 971a69d

5 files changed

Lines changed: 160 additions & 16 deletions

File tree

packages/sdk-typescript/src/daemon/ui/store.ts

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,16 +68,26 @@ export function createDaemonTranscriptStore(
6868
});
6969
scheduleNotify();
7070
},
71-
// wenshao R4 (qwen3.7-max): explicit recovery from the
72-
// `awaitingResync` one-way latch. After the client receives a
73-
// `session.state_resync_required` event, it should:
74-
// 1. Drop local state if a full replay isn't feasible, OR
75-
// 2. Re-subscribe with `Last-Event-ID: 0` to receive a full
76-
// replay, then call `clearAwaitingResync()` once the replay
77-
// stream has drained.
78-
// Without this API the latch could only be cleared by `reset()`,
79-
// which forces session-id reset semantics — wrong shape for the
80-
// same-session-with-replay recovery flow.
71+
// wenshao R4-R6 (qwen3.7-max): explicit recovery from the
72+
// `awaitingResync` one-way latch.
73+
//
74+
// RECOVERY FLOW (correct order — wenshao R6 caught a flow bug):
75+
// 1. Daemon emits `session.state_resync_required`; reducer sets
76+
// `state.awaitingResync = true` and starts dropping events.
77+
// 2. Consumer decides on recovery strategy and calls EITHER:
78+
// a. `reset()` — clean slate, discard local blocks
79+
// b. `clearAwaitingResync()` — keep local blocks, accept
80+
// new events. Call BEFORE the new SSE stream starts
81+
// delivering events (or BEFORE a `Last-Event-ID: 0`
82+
// replay starts), otherwise the replay events get
83+
// dropped by the latch guard.
84+
// 3. Re-subscribe to SSE; events flow normally.
85+
//
86+
// (The earlier JSDoc said "after replay drains" — that was wrong.
87+
// While the latch is set, every replay event is dropped, so the
88+
// window between latch-clear and stream-start is what receives
89+
// events. Clear early; if dispatch order misses something the
90+
// daemon will eventually emit a new `state_resync_required`.)
8191
clearAwaitingResync() {
8292
if (!state.awaitingResync) return;
8393
state = {

packages/sdk-typescript/src/daemon/ui/transcript.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,17 @@ function upsertToolBlock(
498498
}
499499
}
500500
}
501-
updateCurrentToolPointer(state, event.toolCallId, event.status);
501+
// wenshao R6 (qwen3.7-max): pass the EFFECTIVE status — the block
502+
// was just created with `event.status ?? 'pending'`. If we pass
503+
// raw `event.status === undefined`, `updateCurrentToolPointer` early-
504+
// returns and the block sits as visually-pending but currentToolCallId
505+
// never points at it. Effective-status keeps the pointer in sync
506+
// with what was actually written to the block.
507+
updateCurrentToolPointer(
508+
state,
509+
event.toolCallId,
510+
event.status ?? 'pending',
511+
);
502512
clearActiveText(state);
503513
}
504514

packages/sdk-typescript/src/daemon/ui/types.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -695,11 +695,16 @@ export interface DaemonTranscriptStore {
695695
reset(seed?: Partial<DaemonTranscriptState>): void;
696696
/**
697697
* Clear the `awaitingResync` latch that gets set when the daemon emits
698-
* `session.state_resync_required`. Call this after re-subscribing to
699-
* SSE with `Last-Event-ID: 0` and the replay stream has fully drained
700-
* (or after dropping state via your own flow). Without this API, the
701-
* latch could only be cleared by `reset()`, which forces session-id
702-
* change semantics that don't fit same-session reconnect.
698+
* `session.state_resync_required`.
699+
*
700+
* **Recovery flow (call BEFORE the new SSE stream starts):**
701+
* 1. Receive `session.state_resync_required` event → latch sets
702+
* 2. Call `clearAwaitingResync()` (keep blocks) OR `reset()` (clean slate)
703+
* 3. Re-subscribe to SSE (optionally with `Last-Event-ID: 0` for replay)
704+
*
705+
* (R6 review caught a flow bug — the earlier JSDoc said "after replay
706+
* drains" but while the latch is set every replay event is dropped.
707+
* Clear FIRST, then stream events.)
703708
*/
704709
clearAwaitingResync(): void;
705710
}

packages/sdk-typescript/test/unit/daemonUi.test.ts

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4783,3 +4783,111 @@ describe('R5 review batch — coverage additions', () => {
47834783
expect(store.getSnapshot().awaitingResync).toBe(false);
47844784
});
47854785
});
4786+
4787+
describe('R6 review batch — recovery flow + pending pointer', () => {
4788+
it('newly-created tool block with undefined status sets currentToolCallId to its default `pending`', () => {
4789+
let state = createDaemonTranscriptState({ now: 1 });
4790+
state = reduceDaemonTranscriptEvents(
4791+
state,
4792+
normalizeDaemonEvent({
4793+
id: 1,
4794+
v: 1,
4795+
type: 'session_update',
4796+
data: {
4797+
update: {
4798+
sessionUpdate: 'tool_call',
4799+
toolCallId: 'unspecified',
4800+
title: 'starting',
4801+
// no status — daemon emit without explicit status field
4802+
},
4803+
},
4804+
} as never),
4805+
{ now: 2 },
4806+
);
4807+
// Block has effective status 'pending' AND currentToolCallId points to it.
4808+
const block = state.blocks.find(
4809+
(b): b is Extract<typeof b, { kind: 'tool' }> =>
4810+
b.kind === 'tool' && b.toolCallId === 'unspecified',
4811+
)!;
4812+
expect(block.status).toBe('pending');
4813+
expect(state.currentToolCallId).toBe('unspecified');
4814+
});
4815+
4816+
it('clearAwaitingResync FIRST then dispatch new events: events flow', async () => {
4817+
const { createDaemonTranscriptStore } = await import(
4818+
'../../src/daemon/ui/index.js'
4819+
);
4820+
const store = createDaemonTranscriptStore();
4821+
// Set the latch.
4822+
store.dispatch({
4823+
type: 'session.state_resync_required',
4824+
reason: 'sse_eviction',
4825+
lastDeliveredId: 5,
4826+
earliestAvailableId: 12,
4827+
} as never);
4828+
expect(store.getSnapshot().awaitingResync).toBe(true);
4829+
// Clear BEFORE the new event stream.
4830+
store.clearAwaitingResync();
4831+
expect(store.getSnapshot().awaitingResync).toBe(false);
4832+
// Now dispatch a normal event — should land in transcript.
4833+
store.dispatch(
4834+
normalizeDaemonEvent({
4835+
id: 100,
4836+
v: 1,
4837+
type: 'session_update',
4838+
data: {
4839+
update: {
4840+
sessionUpdate: 'agent_message_chunk',
4841+
content: { type: 'text', text: 'replay-event-1' },
4842+
},
4843+
},
4844+
} as never),
4845+
);
4846+
const text = store
4847+
.getSnapshot()
4848+
.blocks.map((b) =>
4849+
b.kind === 'assistant' ? (b as { text: string }).text : '',
4850+
)
4851+
.join('');
4852+
expect(text).toContain('replay-event-1');
4853+
});
4854+
4855+
it('clearAwaitingResync AFTER dispatching events: events ARE dropped (documents the flow)', async () => {
4856+
// This test pins the correct flow as documented: latch drops everything
4857+
// until cleared. If a consumer dispatches events FIRST then clears, the
4858+
// events are lost.
4859+
const { createDaemonTranscriptStore } = await import(
4860+
'../../src/daemon/ui/index.js'
4861+
);
4862+
const store = createDaemonTranscriptStore();
4863+
store.dispatch({
4864+
type: 'session.state_resync_required',
4865+
reason: 'sse_eviction',
4866+
lastDeliveredId: 5,
4867+
earliestAvailableId: 12,
4868+
} as never);
4869+
// WRONG order — dispatch BEFORE clear (replay window).
4870+
store.dispatch(
4871+
normalizeDaemonEvent({
4872+
id: 101,
4873+
v: 1,
4874+
type: 'session_update',
4875+
data: {
4876+
update: {
4877+
sessionUpdate: 'agent_message_chunk',
4878+
content: { type: 'text', text: 'replay-event-2' },
4879+
},
4880+
},
4881+
} as never),
4882+
);
4883+
store.clearAwaitingResync();
4884+
// Event was dropped by the latch.
4885+
const text = store
4886+
.getSnapshot()
4887+
.blocks.map((b) =>
4888+
b.kind === 'assistant' ? (b as { text: string }).text : '',
4889+
)
4890+
.join('');
4891+
expect(text).not.toContain('replay-event-2');
4892+
});
4893+
});

packages/webui/src/daemon/DaemonSessionProvider.tsx

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,17 @@ export function DaemonSessionProvider({
154154
store.reset();
155155
} else if (previousSessionId !== undefined) {
156156
store.dispatch({ type: 'assistant.done', reason: 'reconnected' });
157+
// wenshao R6 (qwen3.7-max): clear the awaitingResync latch
158+
// BEFORE the new SSE event loop starts. Otherwise, if the
159+
// prior connection ended after `state_resync_required` set
160+
// the latch, every event from the fresh stream gets dropped
161+
// by `applyDaemonTranscriptEvent`'s passthrough guard —
162+
// transcript stays permanently frozen even though the
163+
// connection is healthy. Same-session reconnect IS the
164+
// recovery path; signal it to the reducer now.
165+
if (store.getSnapshot().awaitingResync) {
166+
store.clearAwaitingResync();
167+
}
157168
}
158169
session = nextSession;
159170
lastSessionIdRef.current = session.sessionId;

0 commit comments

Comments
 (0)