Skip to content

feat(sdk/daemon-ui): unified completeness follow-up to #4328 (PR-A through PR-E)#4353

Open
chiga0 wants to merge 14 commits into
QwenLM:daemon_mode_b_mainfrom
chiga0:feat/daemon-ui-completeness-followup
Open

feat(sdk/daemon-ui): unified completeness follow-up to #4328 (PR-A through PR-E)#4353
chiga0 wants to merge 14 commits into
QwenLM:daemon_mode_b_mainfrom
chiga0:feat/daemon-ui-completeness-followup

Conversation

@chiga0
Copy link
Copy Markdown
Collaborator

@chiga0 chiga0 commented May 20, 2026

Summary

Unified follow-up to PR #4328 — closes every SDK-only gap from the unified-renderer-layer review. PR #4328 alone shipped ~55%; this PR brings the daemon UI surface to ~95% (remainder requires daemon/Core coordination — see TODO §B/§C/§D).

9 commits, each independently reviewable, +5080 LOC (additive only):

# Commit Scope LOC New tests
1 5128ff03f PR-A — event coverage 13→28+ types, errorKind, tool provenance +1297 +29
2 bdffe3a34 PR-B — server timestamps + eventId-based ordering +394 +5
3 2c3bca107 PR-E — state machine: currentTool / approvalMode / cancellation +400 +5
4 e6f5758db PR-C — tool preview taxonomy (4 new kinds) + multimodal content +494 +16
5 86cd62fed PR-D — render contract: markdown / HTML / plainText helpers +595 +10
6 0856a2fd5 PR-F — 5 additional preview kinds (code_block / search / tabular / image_generation / subagent_delegation) +604 +14
7 768cae43d PR-G — adapter conformance test framework + fixture corpus (10 fixtures) +641 +6
8 1785f5ff9 PR-H — WebUI transcriptAdapter migration to SDK render contract +75 (typecheck)
9 768eb4e1d PR-I — developer guide + migration cookbook (docs/developers/daemon-ui/) +593 (docs)

Tests: 97/97 pass (vitest run test/unit/daemonUi.test.ts), full SDK + WebUI typecheck green.

🟢 Daemon-side status update (2026-05-22, post-rebase onto daemon_mode_b_main @ d0563ecf5+)

After rebasing onto current daemon_mode_b_main, source-verified against the new base:

TODO item Status now
§C1 _meta.serverTimestamp envelope stamping DONE in server.ts:2670 (cited "chiga0 issue #19 P0"); my extractServerTimestamp 3-location probe lights up automatically
§C2 tool_call provenance + serverId stamping DONE in ToolCallEmitter.emitStart via resolveToolProvenance; my MCP heuristic now redundant on stamped paths
§C3 errorKind on stream_error DONE in server.ts:2046
§C3 errorKind on session_died ⚠ Equivalent: daemon stamps closed-enum reason (channel_closed / killed / daemon_shutdown) instead — semantically covers the same UI affordance need
§B1 subagent nesting (parentToolCallId) DONE daemon-side — SubAgentTracker.getSubagentMeta() returns {parentToolCallId, subagentType}, flowed via ToolCallEmitter._meta; SDK consumption is the only gap (see follow-up below)
§B2 tool.progress event ❌ Still not emitted by daemon
§D Multimodal MessageEmitter.emitUserContent + HistoryReplayer inlineData/fileData branches ❌ Still not implemented in Core

Net: 5 of 7 declared dependencies are now satisfied on daemon_mode_b_main. Once #4353 merges, serverTimestamp / provenance / serverId / errorKind / subagent stamping all light up automatically through the SDK's forward-compat code paths shipped in PR-A / PR-B.

Remaining:

  • Subagent SDK consumption (PR-E deferred subagent nesting; daemon now stamps, SDK can consume — ~150 LOC follow-up)
  • tool.progress (~50 LOC daemon)
  • Multimodal echo (~80 LOC Core + ~80 LOC SDK)

Validation

# SDK
cd packages/sdk-typescript
npx vitest run test/unit/daemonUi.test.ts    # 97/97 pass
npx tsc --noEmit                              # no errors

# WebUI
cd packages/webui
npx tsc --noEmit                              # no errors

Reference adapter conformance:

runAdapterConformanceSuite({
  reduce: (events) => reduceDaemonTranscriptEvents(createDaemonTranscriptState(), events),
  renderToText: (s) => s.blocks.map(daemonBlockToMarkdown).join('\n\n'),
});
// → { passed: 10, failed: [], total: 10 }

CHANGELOG (per commit)

PR-A — Event coverage (5128ff03f)

DaemonUiEventType 13 → 28; daemon's full Wave 3-4 event surface reaches UI as typed events instead of debug fallbacks. New events split across 3 categories: session-meta (3), workspace (8), auth device-flow (5). Closed-enum propagation for errorKind and tool provenance + serverId (with mcp__<server>__<tool> heuristic fallback).

PR-B — Time schema (bdffe3a34)

DaemonUiEventBase.serverTimestamp? + DaemonTranscriptBlockBase.serverTimestamp? + clientReceivedAt. Forward-compat 3-location extraction; selectTranscriptBlocksOrderedByEventId ordering helper; formatBlockTimestamp(block, opts) Intl-based formatter.

PR-E — State machine (2c3bca107)

currentToolCallId + approvalMode + toolProgress sidechannel state. propagateCancellationToInFlightTools on assistant.done.reason === 'cancelled'. Forward-compat: unknown statuses leave pointer untouched. Selectors: selectCurrentTool / selectApprovalMode / selectToolProgress.

PR-C — Tool preview + content extraction (e6f5758db)

DaemonToolPreview 4 → 8 kinds: file_diff / file_read / web_fetch / mcp_invocation. extractContentPart() helper + DaemonUiContentPart discriminated union for multimodal (text / image / audio / resource). Helper shipped but NOT yet wired into reducer — see §D below.

PR-D — Render contract (86cd62fed)

daemonBlockToMarkdown / daemonBlockToHtml / daemonBlockToPlainText / daemonToolPreviewToMarkdown. Conservative HTML sanitizer (ANSI strip → HTML escape; role="alert" for errors). sanitizeUrls strips token-like query params. maxFieldLength truncation (default 8192).

PR-F — 5 additional preview kinds (0856a2fd5)

DaemonToolPreview 8 → 13 kinds: code_block / search / tabular / image_generation / subagent_delegation. Each with markdown + plain-text projections. Detector priority extended (most-specific-wins).

PR-G — Adapter conformance framework (768cae43d)

runAdapterConformanceSuite(adapter, opts?) + embedded DAEMON_UI_CONFORMANCE_FIXTURES corpus (10 fixtures). Format-agnostic semantic assertions via expectedContains + expectedAbsent. SDK reference adapter (markdown + plainText) passes all 10 fixtures.

PR-H — WebUI migration (1785f5ff9)

daemonTranscriptToUnifiedMessages(blocks, options?) gains opt-in useMarkdown + enrichToolDetailsWithPreview flags. Defaults preserve legacy behavior (additive). WebUI's transcriptAdapter now bridges to SDK's render contract — file_diff / mcp_invocation / tabular previews surface as markdown when opted in. SDK daemon root index.ts updated to re-export PR-B/D/E/F/G helpers (15+ symbols).

PR-I — Documentation (768eb4e1d)

docs/developers/daemon-ui/README.md (full API reference with cookbook) + docs/developers/daemon-ui/MIGRATION.md (9-step adapter author migration guide with before/after code).

TODO — Categorized by who needs to do the work

✅ A. Pure SDK work — COMPLETE in this PR

  • ✅ 5 additional preview kinds — PR-F
  • ✅ Adapter conformance framework — PR-G
  • ✅ WebUI transcriptAdapter.ts migration — PR-H (opt-in additive)
  • ✅ Migration guide + render cookbook — PR-I

B. Daemon must emit new event types (2 items)

  • Subagent nesting — requires daemon to stamp parentToolCallId / delegationId on tool_call events, OR introduce subagent.delegation.started events. PR-F already ships the subagent_delegation preview kind via heuristic; full nesting requires daemon-side context.
  • tool.progress event — ACP doesn't have this notification today. SDK state shape (toolProgress: Record<...>) ready in PR-E.

C. SDK already forward-compat ready — daemon "just" needs to stamp the field (3 items)

  • _meta.serverTimestamp stamping — PR-B extracts from 3 envelope locations.
  • provenance + serverId stamping on tool_call events — PR-A has mcp__<server>__<tool> heuristic; explicit stamping removes the guess for builtin / subagent.
  • errorKind stamping on session_died / stream_error — PR-A already forwards if present.

D. Multimodal parts integration — Core dependency (3 items)

Source-verified in packages/cli/src/acp-integration/session/emitters/MessageEmitter.ts and HistoryReplayer.ts:

// MessageEmitter.ts — all 3 methods hardcode 'text'
async emitUserMessage(text: string, ...) {
  await this.sendUpdate({
    sessionUpdate: 'user_message_chunk',
    content: { type: 'text', text },     // ← hardcoded 'text'
  });
}

// HistoryReplayer.ts:134 — only walks part.text and part.functionCall
for (const part of content.parts ?? []) {
  if ('text' in part && part.text) { ... }
  if ('functionCall' in part && part.functionCall) { ... }
  // No 'inlineData' (image/audio) branch
  // No 'fileData' (resource) branch
}

#resolvePrompt handles multimodal INPUT (line 2421-2455) but echo direction drops everything except text. User image attachments silently dropped on replay.

To unlock multimodal in UI:

  • Daemon-side: new MessageEmitter.emitUserContent(parts: ContentBlock[]) (~50 LOC)
  • Daemon-side: HistoryReplayer.replayContent add inlineData + fileData branches (~30 LOC)
  • SDK-side: wire extractContentPart (already shipped in PR-C) into normalizer + reducer; text blocks accumulate parts: DaemonUiContentPart[] (~80 LOC)

Net: ~160 LOC of coordinated Core + SDK work. Marked for follow-up PR (PR-J: "Core+SDK multimodal echo").

Dependencies

Base commit: current tip of feat/daemon-ui-core (PR #4328). Diff includes PR #4328's content as base layer; will be clean once #4328 lands.

Scope / Risk

  • Scale: 9 commits = ~5080 LOC of net additive code. All changes are additive to the public API; no existing API broken or removed. createdAt field preserved as @deprecated alias.
  • Backward-compat: every existing v1 consumer continues to work. New behavior is opt-in via additional parameters.
  • Forward-compat: SDK is ready to consume daemon-side fields (serverTimestamp / provenance / errorKind) the moment daemon emits them; graceful degradation when absent.
  • Not covered: multimodal Core integration (TODO §D); subagent nesting (TODO §B); tool.progress event (TODO §B).

Linked

cc @wenshao @doudouOUC


Generated with assistance from Claude Opus 4.7 (claude-opus-4-7). 9 commits authored interactively; full diff reviewed for typecheck + 97 unit tests pass before each push. TODO §D classification corrected after source inspection of MessageEmitter.ts and HistoryReplayer.ts.

@github-actions
Copy link
Copy Markdown
Contributor

📋 Review Summary

This PR delivers a comprehensive follow-up to #4328, implementing a unified daemon UI layer across 5 coordinated commits (PR-A through PR-E). The changes introduce typed event schemas, server-side timestamps, state machine tracking, tool preview taxonomy, and render contract helpers. Test coverage is strong (77/77 passing), and the implementation demonstrates solid architectural thinking around forward-compatibility and cross-client consistency.

🔍 General Feedback

Positive aspects:

  • Excellent commit organization—each of the 5 commits is independently reviewable and addresses a specific gap
  • Strong forward-compatibility patterns throughout (3-location timestamp extraction, unknown status handling)
  • Comprehensive test coverage with defensive edge cases (ANSI stripping, C1 controls, bidi characters)
  • Clear separation between sidechannel state and transcript blocks
  • Well-documented roadmap and gap-closing rationale in commit messages

Architectural decisions:

  • Monotonic eventId as primary ordering key with serverTimestamp fallback is sound
  • Deliberate deferral of subagent nesting design shows good judgment
  • Tool provenance heuristic (mcp__<server>__<tool>) is pragmatic

Potential concerns:

  • Large diff (+6956/-1991 across 34 files) makes holistic review challenging
  • Some files deleted (DaemonTuiAdapter) while new SDK files added—ensure no functionality regression
  • Heavy reliance on AI co-authorship—verify all type safety and edge cases manually

🎯 Specific Feedback

🟡 High

  • packages/sdk-typescript/src/daemon/ui/store.ts — The reducer handles many event types but lacks explicit handling for all 28+ DaemonUiEventType variants. Verify that session-meta, workspace, and auth events properly update sidechannel state without unintended no-ops.

  • packages/webui/src/daemon/transcriptAdapter.ts:144-156normalizeToolStatus defaults unknown statuses to 'in_progress'. While the comment mentions forward-compat, this could cause future statuses like 'paused' to incorrectly display as active. Consider returning a distinct 'unknown' status or leaving the pointer untouched as PR-E does for currentToolCallId.

  • packages/sdk-typescript/src/daemon/ui/transcript.ts — The propagateCancellationToInFlightTools function walks all blocks to mark in-flight tools as cancelled. For long sessions with many tools, this could be O(n) on every cancel. Consider maintaining an index of in-flight tool IDs for O(1) lookup.

🟢 Medium

  • packages/sdk-typescript/src/daemon/ui/types.ts:58-72 — The DaemonTranscriptBlockBase has both serverTimestamp? and clientReceivedAt with a deprecated createdAt alias. While well-documented, this creates three timestamp fields that could confuse consumers. Consider consolidating documentation or providing a single getTimestamp() helper that returns the most authoritative available value.

  • packages/sdk-typescript/src/daemon/ui/utils.ts — The extractContentPart function handles multimodal content but silently returns undefined for unknown types. This is defensive but could hide daemon evolution. Consider logging unknown content kinds to debug output for observability.

  • packages/webui/src/daemon/DaemonSessionProvider.tsx:155-175 — The getReconnectDelayMs function implements exponential backoff but doesn't expose jitter. In a thundering herd scenario (many clients reconnecting simultaneously), synchronized retries could overload the daemon. Add optional randomization (e.g., ±20% jitter).

  • packages/sdk-typescript/src/daemon/ui/terminal.ts — The daemonUiEventToTerminalText function handles many event types but the switch statement is lengthy. Consider extracting per-kind handlers into separate functions for better testability and readability.

🔵 Low

  • packages/sdk-typescript/src/daemon/ui/types.ts:104 — The DaemonUiToolProvenance type includes 'unknown' as a catch-all. Consider adding JSDoc examples of when each provenance is assigned, especially the heuristic fallback logic for mcp__ prefix detection.

  • packages/webui/src/types/toolCall.ts:10-16 — The ToolCallStatus union now includes 'cancelled', but existing tool call components (GenericToolCall, ShellToolCall, etc.) may need updates to handle the new status visually. Verify all consumers render cancelled state appropriately.

  • packages/sdk-typescript/src/daemon/ui/render.ts — The daemonBlockToMarkdown and daemonBlockToHtml functions accept opts? but default parameter handling could be clearer. Consider using explicit default options object pattern for better discoverability.

  • packages/webui/vite.config.ts:23-30 — The alias configuration duplicates the tsconfig.json paths. While necessary for Vite, consider documenting this duplication or extracting to a shared config to avoid drift.

  • docs/developers/daemon-client-adapters/tui.md — This file is deleted. Ensure the new web-ui.md documentation covers equivalent guidance for TUI consumers, or migrate relevant content rather than removing.

✅ Highlights

  • Event coverage expansion (13→28 types) — Comprehensive typing for session-meta, workspace, and auth events closes significant gaps in daemon observability

  • Server timestamp extraction — The 3-location forward-compat pattern (event.serverTimestamp, event._meta.serverTimestamp, event.data._meta.serverTimestamp) is elegantly designed for gradual daemon adoption

  • Security sanitizationsanitizeDaemonTerminalText handles ANSI escapes, C1 controls, OSC/DCS sequences, and bidi characters comprehensively. HTML output escapes XSS vectors while preserving content integrity.

  • Tool preview taxonomy — The 8-kind DaemonToolPreview union (file_diff, file_read, web_fetch, mcp_invocation, etc.) provides rich structured display without over-engineering

  • Cancellation propagation — The propagateCancellationToInFlightTools logic prevents infinite spinner scenarios when daemon doesn't guarantee terminal events for all tools on cancel

  • Test quality — Tests cover edge cases like secret field redaction, malformed payloads, protocol version mismatches, and ANSI control sequence stripping

Copy link
Copy Markdown
Collaborator

@wenshao wenshao left a comment

Choose a reason for hiding this comment

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

A deterministic typecheck also reports TS4111 in packages/webui/src/components/toolcalls/ShellToolCall.tsx for existing Record<string, unknown> dot-property accesses (description / command). Those lines are not part of the PR diff, so I am not posting them as inline comments, but the changed-file typecheck will still need to be clean before merge.

— gpt-5.5 via Qwen Code /review

Comment thread packages/webui/src/daemon/transcriptAdapter.test.ts
Comment thread packages/webui/src/index.ts
Comment thread packages/webui/src/daemon/DaemonSessionProvider.tsx
Comment thread packages/webui/src/daemon/DaemonSessionProvider.tsx
@chiga0 chiga0 requested review from doudouOUC and yiliang114 May 20, 2026 07:56
Comment thread packages/sdk-typescript/src/daemon/index.ts
Comment thread packages/sdk-typescript/src/daemon/ui/transcript.ts Outdated
Comment thread packages/sdk-typescript/src/daemon/ui/normalizer.ts Outdated
Comment thread packages/sdk-typescript/src/daemon/ui/store.ts Outdated
Comment thread packages/webui/src/daemon/transcriptAdapter.ts
Comment thread packages/webui/src/daemon/DaemonSessionProvider.tsx
Comment thread packages/sdk-typescript/src/daemon/ui/render.ts
Comment thread packages/sdk-typescript/src/daemon/ui/types.ts Outdated
Comment thread packages/sdk-typescript/src/daemon/ui/transcript.ts
Comment thread packages/sdk-typescript/src/daemon/ui/transcript.ts Outdated
chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request May 20, 2026
…ete (PR-F)

Closes the "5 additional preview kinds" item in PR QwenLM#4353's TODO §A
(SDK-only work).

## New preview kinds (8 → 13)

- `code_block` — `{ language?, code, origin? }` — REPL / formatter /
  generator output, fenced as `\`\`\`<language>` in markdown
- `search` — `{ query, resultCount?, top? }` — grep / ripgrep / find /
  glob results with up to 5 top hits
- `tabular` — `{ columns, rows, totalRows? }` — structured table output
  (50-row cap with `totalRows` truncation indicator); supports both
  `columns: string[] + rows: unknown[][]` explicit shape and legacy
  `data: Array<Record<>>` shape (auto-infers columns from first row)
- `image_generation` — `{ prompt, thumbnailUrl?, model? }` — dall-e /
  diffusion / imagen / flux / sora style tools
- `subagent_delegation` — `{ agentName, task, parentDelegationId? }` —
  Anthropic-style Task tool and similar sub-agent dispatchers

## Detector priority

Order matters — most specific wins. New detectors slot in between
`mcp_invocation` and `file_diff`:

```
mcp_invocation > subagent_delegation > search > image_generation
  > file_diff > file_read > web_fetch > code_block > tabular
  > command > key_value > generic
```

Rationale: subagent / search / image generation are most discriminable
(distinct toolName patterns); file ops next; code_block / tabular last
because their shapes (`code:`, `columns:`) can appear in other tools.

## Render projections

Both `daemonToolPreviewToMarkdown` and the plain-text rendering paths
extended with cases for all 5 new kinds:

- code_block: fenced markdown code block with language tag
- search: bold header + GFM bullet list of top results
- tabular: GFM pipe table with header / separator / body / truncation hint
- image_generation: bold header + blockquoted prompt + embedded markdown
  image (URL sanitization respected via `sanitizeUrls` opt)
- subagent_delegation: bold delegate-arrow header + blockquoted task +
  optional parent delegation reference

## Test coverage (91/91 pass, +14 new)

- Each detector with positive case
- Detector priority verified: subagent_delegation wins over file_diff
  when toolName='Task' has both subagent + file-edit fields
- Tabular row cap (50) + totalRows stamping for truncated data
- Legacy data: Array<Record<>> auto-column inference
- Each render projection with structural assertions (markdown table
  format, image embed, bullet lists)

## Roadmap

PR-F of the unified follow-up to PR QwenLM#4328. Brings the preview taxonomy
to 13 kinds covering: file ops (3), web (1), code/data (2), media (1),
agent control (2 — ask_user_question + subagent_delegation), MCP (1),
search (1), generic fallbacks (2).

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request May 20, 2026
…PR-G)

Closes the "Adapter conformance test framework" item in PR QwenLM#4353's TODO §A.
Lets any daemon-ui adapter (TUI / web / IDE / channel / mobile) validate
that it projects a fixed corpus of daemon SSE event streams to the same
semantic shape — catches projection drift before it reaches users.

## API surface

```ts
interface DaemonUiAdapterUnderTest {
  reduce(events: readonly DaemonUiEvent[]): unknown;
  renderToText(state: unknown): string;
}

interface DaemonUiConformanceFixture {
  name: string;
  description: string;
  envelopes: DaemonEvent[];           // raw daemon envelopes
  expectedContains: string[];          // phrases the rendered text MUST contain
  expectedAbsent?: string[];           // phrases that MUST NOT appear
  normalizeOptions?: { ... };          // forward-compat normalize opts
}

runAdapterConformanceSuite(adapter, opts?): ConformanceSuiteResult
DAEMON_UI_CONFORMANCE_FIXTURES: ReadonlyArray<DaemonUiConformanceFixture>
```

## Design

**Format-agnostic assertion**: adapters can render to ANSI / HTML /
markdown / JSX — the framework only inspects plain text via
`renderToText`. Catches semantic divergence (missing user message,
wrong tool status, leaked secret) without forcing identical formatting.

**Embedded fixture corpus** (no fs reads — works in browser bundle):
- `simple-chat` — user/assistant streaming flow
- `tool-call-lifecycle` — running → completed transition
- `file-edit-diff` — file_diff preview surfacing
- `mcp-invocation` — MCP serverId/toolName extraction via heuristic
- `permission-lifecycle` — request + resolved with outcome
- `mcp-budget-warning` — Wave 3 event (adapter must observe but rendering
  is its choice)
- `cancellation-propagates` — tool block status flows
- `malformed-payload-redaction` — uses `includeRawEvent: true` to verify
  even a debug-mode adapter doesn't leak `token: secret-do-not-leak`
- `auth-device-flow-success` — Wave 4 OAuth events
- `available-commands-typed-event` — PR-A upgrade from status text

Per-fixture `expectedContains` and `expectedAbsent` describe the
content contract independently of format.

## Suite result

```ts
{
  passed: number,
  failed: ConformanceFailure[],   // each carries missing + leaked + excerpt
  total: number,
}
```

**Does not throw** — caller asserts on `result.failed` so adapter test
suites can produce per-fixture diagnostics rather than a single opaque
exception.

## Filter options

`only` / `skip` allow targeted runs during adapter development:

```ts
runAdapterConformanceSuite(myAdapter, { only: ['simple-chat'] });
runAdapterConformanceSuite(myAdapter, { skip: ['cancellation-propagates'] });
```

## Test coverage (97/97 pass, +6 new)

- SDK reference adapter (reducer + markdown render) passes all fixtures
- SDK reference adapter (reducer + plainText render) also passes
- Buggy adapter (empty string output) fails every fixture with non-empty
  `expectedContains`
- Buggy adapter (raw event dump via JSON.stringify) caught by redaction
  fixture's `expectedAbsent`
- `only` filter narrows to a single fixture
- `skip` filter excludes named fixtures from the corpus

## Usage from adapter authors

```ts
// In your adapter's test file
import { runAdapterConformanceSuite } from '@qwen-code/sdk/daemon';
import { reduceForTui, renderTuiState } from './my-tui-adapter';

it('TUI adapter conforms to daemon UI corpus', () => {
  const result = runAdapterConformanceSuite({
    reduce: reduceForTui,
    renderToText: renderTuiState,
  });
  expect(result.failed).toEqual([]);
});
```

## Roadmap

PR-G of the unified follow-up to PR QwenLM#4328. The corpus is intentionally
small (10 fixtures) but extensible — adapter authors can submit new
fixtures via additions to `DAEMON_UI_CONFORMANCE_FIXTURES` to lock in
regression coverage for edge cases their adapter encountered.

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request May 20, 2026
…act (PR-H)

Closes the "WebUI transcriptAdapter migration" item in PR QwenLM#4353's TODO §A.
Validates the PR-D render contract end-to-end on the real WebUI consumer.

## Migration approach — additive opt-in

`daemonTranscriptToUnifiedMessages(blocks, options?)` gains a new options
parameter:

```ts
interface DaemonTranscriptAdapterOptions {
  useMarkdown?: boolean;                  // default: false
  enrichToolDetailsWithPreview?: boolean; // default: false
}
```

Defaults preserve legacy behavior — existing callers see no change.

## What `useMarkdown: true` does

For `user` / `assistant` / `thought` blocks, content is projected via
SDK's `daemonBlockToMarkdown` instead of raw sanitized text. The WebUI's
markdown renderer (markdown-it) then gets:

- `**You**\n\n<content>` for user blocks (bold "You" label)
- Raw text for assistant blocks (markdown formatting in agent output
  passes through cleanly)
- `> *thought:* <text>` blockquote for thought blocks

## What `enrichToolDetailsWithPreview: true` does

For `tool` blocks, `rawOutput` is replaced with `daemonToolPreviewToMarkdown(block.preview)`.
This lets WebUI surfaces without per-preview-kind React components still
display:

- `file_diff` as a fenced unified diff
- `mcp_invocation` as `server::tool` with args summary
- `tabular` as GFM pipe table
- `search` as bullet list with match count
- `image_generation` as embedded markdown image
- `subagent_delegation` as delegate arrow + task quote

Renderers with per-kind components should leave this opt-out.

## SDK daemon root index.ts re-exports

`packages/sdk-typescript/src/daemon/index.ts` was missing exports for
PR-D / PR-F / PR-G / PR-B / PR-E surface — WebUI's `@qwen-code/sdk/daemon`
import path uses the daemon root, not the ui/ sub-index. Added 15+
re-exports so consumers don't need to use the longer
`@qwen-code/sdk/daemon/ui/index.js` path.

Now exported from `@qwen-code/sdk/daemon` root:
- `daemonBlockToMarkdown` / `daemonBlockToHtml` / `daemonBlockToPlainText`
- `daemonToolPreviewToMarkdown`
- `extractContentPart` + `DaemonUiContentPart` type
- `formatBlockTimestamp` + `selectTranscriptBlocksOrderedByEventId`
- `selectCurrentTool` / `selectApprovalMode` / `selectToolProgress`
- `runAdapterConformanceSuite` + `DAEMON_UI_CONFORMANCE_FIXTURES`
- All associated types

## Test fixture migration

`webui/src/daemon/transcriptAdapter.test.ts` mock blocks updated to include
`clientReceivedAt` (required field added in PR-B). Mechanical change —
every `createdAt: N` test fixture gets a matching `clientReceivedAt: N`.

## Validation

- WebUI `npm run typecheck` — clean
- SDK `npm run typecheck` — clean
- SDK `vitest run test/unit/daemonUi.test.ts` — 97/97 pass
- WebUI transcriptAdapter test fixtures typecheck against updated
  DaemonTranscriptBlockBase schema

## Roadmap

PR-H of the unified follow-up to PR QwenLM#4328. Closes the WebUI migration
gap in TODO §A.

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request May 20, 2026
Closes the final "Documentation" item in PR QwenLM#4353's TODO §A. Brings the
unified daemon UI surface to ~95% SDK-side completion.

## Files added

- `docs/developers/daemon-ui/README.md` — full API reference
  - Three-layer model (normalizer → reducer → render helpers)
  - Quick start with idiomatic event-loop pattern
  - Event taxonomy (28+ types categorized: chat-stream / session-meta /
    workspace / auth device-flow)
  - Render contract cookbook (markdown / HTML / plainText)
  - Tool preview taxonomy (13 kinds with use cases)
  - State selectors (currentTool / approvalMode / toolProgress / ordering)
  - Cancellation propagation explanation
  - Time semantics (eventId > serverTimestamp > clientReceivedAt
    precedence)
  - Adapter conformance usage
  - ErrorKind dispatch pattern
  - Tool provenance dispatch pattern
  - Forward-compat principles

- `docs/developers/daemon-ui/MIGRATION.md` — adapter author migration
  cookbook
  - Step-by-step recommended adoption order (9 steps, value-ranked)
  - Before/after code examples for each step
  - Backward-compat checklist (everything is additive — no breaking
    changes)
  - Cross-references to PR-A through PR-H commits

## Roadmap

PR-I of the unified follow-up to PR QwenLM#4328. Documentation-only — no
code changes; no tests affected.

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@chiga0
Copy link
Copy Markdown
Collaborator Author

chiga0 commented May 20, 2026

Self-review — PR #4353 多轮审计 (9 commits / +5080 LOC)

Multi-round self-review covering correctness / coverage / side effects / bundle size + cold start impact + React dependency check.

Round 1 — 依赖与 bundle 影响

React dependency

Check Result
packages/sdk-typescript/package.json deps @modelcontextprotocol/sdk + zod (unchanged)
packages/sdk-typescript/package.json peer typescript only
grep -r "from 'react'" packages/sdk-typescript/src/ empty
grep -r "@types/react" packages/sdk-typescript/src/ empty
webui's React import inherited from PR #4328, this PR adds none

Conclusion: SDK has zero React dependency. This PR adds no new external deps.

Bundle size — actual measurement

@qwen-code/sdk/daemon subpath (full):
  pre-PR  #4353 (= PR #4328 head):    54 KB min / 13.7 KB gzip
  after PR #4353:                      80 KB min / 20.8 KB gzip
  Δ:                                  +26 KB min / +7 KB gzip

Critical caveat — tree-shaking measurement. Simulated webui's actual import surface (daemonBlockToMarkdown + daemonToolPreviewToMarkdown + sanitizeDaemonTerminalText):

tree-shaken (webui actual subset):    4.9 KB min / 2.2 KB gzip

Implication:

  • Full bundle grows ~26 KB only if a consumer imports the entire API surface
  • typical webui consumer adds ~2.2 KB gzip — for React + markdown-it (~500 KB total) webui, <0.5% cold-start impact
  • The conformance framework (641 LOC + 10 fixtures), 16 unused selectors, and 9 unused normalizers all tree-shake out

Cold-start analysis

  • daemon subpath has no top-level side effects — pure functions + type defs + static fixture arrays
  • No top-level IIFE / no fs reads / no fetch / no Date.now() at module-load
  • Parse + compile cost on modern V8: 1-3ms for 80 KB minified
  • With tree-shaking, webui parses only ~5 KB → cold start impact negligible

Round 2 — Test regression

SDK pre-PR:      77/77 pass
SDK after PR:    97/97 pass (+20 new tests)
SDK FULL suite (14 files): 521/521 pass
  - DaemonAuthFlow.test.ts: untouched, passes
  - DaemonClient.test.ts: untouched, passes
  - Query.test.ts: untouched, passes
  - All others untouched
typecheck:       SDK + WebUI both clean

0 regressions across the entire SDK test suite.


Round 3 — Per-commit correctness audit

PR-A (event coverage)

  • ✅ 16 new event normalizers; each with malformed-payload fallback to debug
  • ✅ Reducer no-op on new events; lastEventId still advances monotonically
  • errorKind validated via Set.has() (closed enum)
  • ✅ Provenance MCP heuristic (mcp__<server>__<tool>) correctly parsed

PR-B (time schema)

  • extractServerTimestamp checks 3 candidates + Number.isFinite guard
  • compareBlocksByEventOrder 3-level fallback (eventId → serverTimestamp → clientReceivedAt)
  • formatBlockTimestamp Intl.DateTimeFormat with safe-value guard
  • Minor: clientReceivedAt made required — every mock block in external code needs the field. Already fixed in this PR's webui transcriptAdapter.test.ts (11 occurrences).

PR-E (state machine)

  • IN_FLIGHT_TOOL_STATUSES + TERMINAL_TOOL_STATUSES closed sets
  • ✅ Unknown status leaves pointer untouched — forward-compat
  • propagateCancellationToInFlightTools uses getWritableBlockById COW path
  • assistant.done.reason !== 'cancelled' does NOT propagate — test-verified

PR-C (preview taxonomy + content extraction)

  • ✅ 4 new detector priority order correct (MCP > file_diff > file_read > web_fetch)
  • extractContentPart handles 4 kinds + undefined fallback
  • ✅ Legacy getTextContent preserved for backward compat

PR-D (render contract)

  • ✅ HTML sanitizer strips ANSI BEFORE HTML escape — defends against agent-emitted escape sequences in HTML output
  • sanitizeUrls only strips token-like query params (token=, key=, auth=, signature=, x-amz-*, x-goog-*)
  • maxFieldLength default 8192 with truncation indicator
  • role="alert" for error blocks (a11y)

PR-F (5 additional preview kinds)

  • ✅ Detector priority order: specific-first (MCP > subagent > search > image_gen > file_diff > ...)
  • ✅ MCP heuristic wins over subagent — mcp__editor__delegate_task correctly classified as mcp_invocation
  • ✅ Tabular row cap MAX_TABULAR_ROWS = 50 + totalRows truncation indicator
  • ✅ Search top results cap MAX_SEARCH_TOP_RESULTS = 5
  • ✅ Code block requires explicit language OR REPL-style toolName — no false positives on arbitrary code: fields

PR-G (conformance framework)

  • ✅ Reference adapters (markdown + plainText projections) both pass all 10 fixtures
  • ✅ Buggy adapter (empty string) surfaces missing phrases per-fixture
  • ✅ Buggy adapter (raw JSON dump) caught by redaction fixture's expectedAbsent
  • only / skip filter options work
  • 6 of 10 fixtures have expectedContains: [] — intentional (observation-only fixtures for events where adapter chooses rendering strategy, e.g., auth modal vs banner). Stronger assertions could be added in follow-up.

PR-H (WebUI migration)

  • useMarkdown + enrichToolDetailsWithPreview are opt-in flags (default false) — additive, zero breaking risk
  • Tradeoff noted: WebUI default behavior unchanged — downstream needs to explicitly opt in to get richer markdown rendering. A follow-up PR could default useMarkdown: true in DaemonSessionProvider to make the benefit automatic.
  • ✅ SDK daemon root index.ts re-exports for PR-B/D/E/F/G surfaces (15+ symbols)

PR-I (docs)

  • ✅ README + MIGRATION cover all 9 commits with code examples
  • ⚠ No separate cookbook file, but README has a cookbook section — can split later based on user feedback

Round 4 — Side effect scan

Public API breaking changes

Concern Status
Deleted exports 0
Renamed exports 0 (createdAt preserved as @deprecated alias for clientReceivedAt)
Changed signatures daemonTranscriptToUnifiedMessages gains optional options parameter — additive
Behavior change on existing functions 0 (available_commands_update becomes typed event but this lives only on feat/daemon-ui-core, not yet merged to main)

Type-level changes (additive widening only)

  1. DaemonTranscriptBlockBase.clientReceivedAt: number required — external mock blocks need the field. Risk localized: any private fork / IDE extension manually constructing blocks. Mitigation: trivial sed (every createdAt: N gets paired clientReceivedAt: N).

  2. DaemonUiEventType union expanded — exhaustive switches on the union require new cases. Already handled in SDK internals. This is intentional/healthy — forces downstream adapters to explicitly observe new events.


Round 5 — Coverage completeness audit (against original review)

Cross-check each gap in the PR #4328 unified renderer review:

Original review item Status in this PR
§1 — 12+ daemon events fall through to debug ✅ All normalized (PR-A)
§2 — Free-string schema (tool/error/status) ⚠ Partial: errorKind + provenance closed enums (PR-A); status / outcome still strings
§3 — Time not standardized ✅ eventId ordering + serverTimestamp + Intl formatter (PR-B)
§4 — Provider differences leak 3 ways ⚠ Partial: provenance closed enum + content discrimination helper; reasoning signature still daemon-layer concern
§5 — Reducer state machine gaps (in-flight / nesting / progress / cancel) ⚠ Most: currentTool + approvalMode + cancellation propagation (PR-E); subagent nesting deferred to daemon
§6 — Render contract terminal-only ✅ markdown / HTML / plainText + conformance + webui wired + docs (PR-D/G/H/I)
§7 — Tool preview only 4 kinds ✅ Extended to 13 (PR-C + PR-F)

Completion ~95%, matching the PR description. Remaining 5% all declared in the daemon dependency declaration — waiting on daemon/Core landing.


Round 6 — Known minor issues

⚠ Minor 1 — Empty user.text produces trailing newlines in markdown

daemonBlockToMarkdown for user block: **You**\n\n${cap(block.text)}. If block.text === '', output is **You**\n\n (trailing empty paragraph after rendering).
Impact: cosmetic; user blocks rarely empty in practice.
Fix path: early-return '' when text empty — follow-up PR.

⚠ Minor 2 — MCP heuristic parses mcp__a__b__c__d greedily

mcp__a__b__c__d parses as serverId=a, toolName=b__c__d.
Whether triggered: depends on daemon naming convention. If server names contain __, heuristic misclassifies. Daemon side provenance stamping (declared in deps) eliminates the heuristic entirely — no fix needed in SDK.

⚠ Minor 3 — 6 conformance fixtures with expectedContains: []

Intentional: those fixtures verify "adapter observes the event without throwing", not "adapter renders specific text". Adapter rendering strategy for auth/mcp/system events varies (modal vs banner vs hidden).
Not a bug, but stronger fixtures could be added (e.g., expectedKindObserved: 'auth_event') in a follow-up.

❌ No critical issues


Aggregate verdict

Dimension Assessment
React dependency ❌ None added — SDK pure
Full bundle size +26 KB minified (acceptable for the feature surface)
Webui actual increase +2.2 KB gzip (tree-shaking removes 75%)
Cold start <0.5% impact, negligible
Side effects 0 critical; 3 documented minor issues
Test regression 0 (521/521 SDK tests pass)
Type-level breaking clientReceivedAt required + union widening — internally self-consistent; downstream exhaustive switches need new cases (healthy)
Coverage completeness ~95% of original review (rest explicitly declared as daemon/Core deps)
Mergeable ✅ — no rollback or rewrite required

The three minor issues are not blockers and can be follow-up.


Generated with assistance from Claude Opus 4.7 (claude-opus-4-7) — bundle sizes measured via esbuild --minify --bundle against the actual PR branch worktree; tree-shaking simulated with the webui's verbatim import set; full SDK test suite (vitest run) executed against post-PR HEAD.

chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request May 20, 2026
…ete (PR-F)

Closes the "5 additional preview kinds" item in PR QwenLM#4353's TODO §A
(SDK-only work).

## New preview kinds (8 → 13)

- `code_block` — `{ language?, code, origin? }` — REPL / formatter /
  generator output, fenced as `\`\`\`<language>` in markdown
- `search` — `{ query, resultCount?, top? }` — grep / ripgrep / find /
  glob results with up to 5 top hits
- `tabular` — `{ columns, rows, totalRows? }` — structured table output
  (50-row cap with `totalRows` truncation indicator); supports both
  `columns: string[] + rows: unknown[][]` explicit shape and legacy
  `data: Array<Record<>>` shape (auto-infers columns from first row)
- `image_generation` — `{ prompt, thumbnailUrl?, model? }` — dall-e /
  diffusion / imagen / flux / sora style tools
- `subagent_delegation` — `{ agentName, task, parentDelegationId? }` —
  Anthropic-style Task tool and similar sub-agent dispatchers

## Detector priority

Order matters — most specific wins. New detectors slot in between
`mcp_invocation` and `file_diff`:

```
mcp_invocation > subagent_delegation > search > image_generation
  > file_diff > file_read > web_fetch > code_block > tabular
  > command > key_value > generic
```

Rationale: subagent / search / image generation are most discriminable
(distinct toolName patterns); file ops next; code_block / tabular last
because their shapes (`code:`, `columns:`) can appear in other tools.

## Render projections

Both `daemonToolPreviewToMarkdown` and the plain-text rendering paths
extended with cases for all 5 new kinds:

- code_block: fenced markdown code block with language tag
- search: bold header + GFM bullet list of top results
- tabular: GFM pipe table with header / separator / body / truncation hint
- image_generation: bold header + blockquoted prompt + embedded markdown
  image (URL sanitization respected via `sanitizeUrls` opt)
- subagent_delegation: bold delegate-arrow header + blockquoted task +
  optional parent delegation reference

## Test coverage (91/91 pass, +14 new)

- Each detector with positive case
- Detector priority verified: subagent_delegation wins over file_diff
  when toolName='Task' has both subagent + file-edit fields
- Tabular row cap (50) + totalRows stamping for truncated data
- Legacy data: Array<Record<>> auto-column inference
- Each render projection with structural assertions (markdown table
  format, image embed, bullet lists)

## Roadmap

PR-F of the unified follow-up to PR QwenLM#4328. Brings the preview taxonomy
to 13 kinds covering: file ops (3), web (1), code/data (2), media (1),
agent control (2 — ask_user_question + subagent_delegation), MCP (1),
search (1), generic fallbacks (2).

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request May 20, 2026
…PR-G)

Closes the "Adapter conformance test framework" item in PR QwenLM#4353's TODO §A.
Lets any daemon-ui adapter (TUI / web / IDE / channel / mobile) validate
that it projects a fixed corpus of daemon SSE event streams to the same
semantic shape — catches projection drift before it reaches users.

## API surface

```ts
interface DaemonUiAdapterUnderTest {
  reduce(events: readonly DaemonUiEvent[]): unknown;
  renderToText(state: unknown): string;
}

interface DaemonUiConformanceFixture {
  name: string;
  description: string;
  envelopes: DaemonEvent[];           // raw daemon envelopes
  expectedContains: string[];          // phrases the rendered text MUST contain
  expectedAbsent?: string[];           // phrases that MUST NOT appear
  normalizeOptions?: { ... };          // forward-compat normalize opts
}

runAdapterConformanceSuite(adapter, opts?): ConformanceSuiteResult
DAEMON_UI_CONFORMANCE_FIXTURES: ReadonlyArray<DaemonUiConformanceFixture>
```

## Design

**Format-agnostic assertion**: adapters can render to ANSI / HTML /
markdown / JSX — the framework only inspects plain text via
`renderToText`. Catches semantic divergence (missing user message,
wrong tool status, leaked secret) without forcing identical formatting.

**Embedded fixture corpus** (no fs reads — works in browser bundle):
- `simple-chat` — user/assistant streaming flow
- `tool-call-lifecycle` — running → completed transition
- `file-edit-diff` — file_diff preview surfacing
- `mcp-invocation` — MCP serverId/toolName extraction via heuristic
- `permission-lifecycle` — request + resolved with outcome
- `mcp-budget-warning` — Wave 3 event (adapter must observe but rendering
  is its choice)
- `cancellation-propagates` — tool block status flows
- `malformed-payload-redaction` — uses `includeRawEvent: true` to verify
  even a debug-mode adapter doesn't leak `token: secret-do-not-leak`
- `auth-device-flow-success` — Wave 4 OAuth events
- `available-commands-typed-event` — PR-A upgrade from status text

Per-fixture `expectedContains` and `expectedAbsent` describe the
content contract independently of format.

## Suite result

```ts
{
  passed: number,
  failed: ConformanceFailure[],   // each carries missing + leaked + excerpt
  total: number,
}
```

**Does not throw** — caller asserts on `result.failed` so adapter test
suites can produce per-fixture diagnostics rather than a single opaque
exception.

## Filter options

`only` / `skip` allow targeted runs during adapter development:

```ts
runAdapterConformanceSuite(myAdapter, { only: ['simple-chat'] });
runAdapterConformanceSuite(myAdapter, { skip: ['cancellation-propagates'] });
```

## Test coverage (97/97 pass, +6 new)

- SDK reference adapter (reducer + markdown render) passes all fixtures
- SDK reference adapter (reducer + plainText render) also passes
- Buggy adapter (empty string output) fails every fixture with non-empty
  `expectedContains`
- Buggy adapter (raw event dump via JSON.stringify) caught by redaction
  fixture's `expectedAbsent`
- `only` filter narrows to a single fixture
- `skip` filter excludes named fixtures from the corpus

## Usage from adapter authors

```ts
// In your adapter's test file
import { runAdapterConformanceSuite } from '@qwen-code/sdk/daemon';
import { reduceForTui, renderTuiState } from './my-tui-adapter';

it('TUI adapter conforms to daemon UI corpus', () => {
  const result = runAdapterConformanceSuite({
    reduce: reduceForTui,
    renderToText: renderTuiState,
  });
  expect(result.failed).toEqual([]);
});
```

## Roadmap

PR-G of the unified follow-up to PR QwenLM#4328. The corpus is intentionally
small (10 fixtures) but extensible — adapter authors can submit new
fixtures via additions to `DAEMON_UI_CONFORMANCE_FIXTURES` to lock in
regression coverage for edge cases their adapter encountered.

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request May 20, 2026
…act (PR-H)

Closes the "WebUI transcriptAdapter migration" item in PR QwenLM#4353's TODO §A.
Validates the PR-D render contract end-to-end on the real WebUI consumer.

`daemonTranscriptToUnifiedMessages(blocks, options?)` gains a new options
parameter:

```ts
interface DaemonTranscriptAdapterOptions {
  useMarkdown?: boolean;                  // default: false
  enrichToolDetailsWithPreview?: boolean; // default: false
}
```

Defaults preserve legacy behavior — existing callers see no change.

For `user` / `assistant` / `thought` blocks, content is projected via
SDK's `daemonBlockToMarkdown` instead of raw sanitized text. The WebUI's
markdown renderer (markdown-it) then gets:

- `**You**\n\n<content>` for user blocks (bold "You" label)
- Raw text for assistant blocks (markdown formatting in agent output
  passes through cleanly)
- `> *thought:* <text>` blockquote for thought blocks

For `tool` blocks, `rawOutput` is replaced with `daemonToolPreviewToMarkdown(block.preview)`.
This lets WebUI surfaces without per-preview-kind React components still
display:

- `file_diff` as a fenced unified diff
- `mcp_invocation` as `server::tool` with args summary
- `tabular` as GFM pipe table
- `search` as bullet list with match count
- `image_generation` as embedded markdown image
- `subagent_delegation` as delegate arrow + task quote

Renderers with per-kind components should leave this opt-out.

`packages/sdk-typescript/src/daemon/index.ts` was missing exports for
PR-D / PR-F / PR-G / PR-B / PR-E surface — WebUI's `@qwen-code/sdk/daemon`
import path uses the daemon root, not the ui/ sub-index. Added 15+
re-exports so consumers don't need to use the longer
`@qwen-code/sdk/daemon/ui/index.js` path.

Now exported from `@qwen-code/sdk/daemon` root:
- `daemonBlockToMarkdown` / `daemonBlockToHtml` / `daemonBlockToPlainText`
- `daemonToolPreviewToMarkdown`
- `extractContentPart` + `DaemonUiContentPart` type
- `formatBlockTimestamp` + `selectTranscriptBlocksOrderedByEventId`
- `selectCurrentTool` / `selectApprovalMode` / `selectToolProgress`
- `runAdapterConformanceSuite` + `DAEMON_UI_CONFORMANCE_FIXTURES`
- All associated types

`webui/src/daemon/transcriptAdapter.test.ts` mock blocks updated to include
`clientReceivedAt` (required field added in PR-B). Mechanical change —
every `createdAt: N` test fixture gets a matching `clientReceivedAt: N`.

- WebUI `npm run typecheck` — clean
- SDK `npm run typecheck` — clean
- SDK `vitest run test/unit/daemonUi.test.ts` — 97/97 pass
- WebUI transcriptAdapter test fixtures typecheck against updated
  DaemonTranscriptBlockBase schema

PR-H of the unified follow-up to PR QwenLM#4328. Closes the WebUI migration
gap in TODO §A.

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@chiga0 chiga0 force-pushed the feat/daemon-ui-completeness-followup branch from 768eb4e to ae72935 Compare May 20, 2026 09:39
chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request May 20, 2026
Closes the final "Documentation" item in PR QwenLM#4353's TODO §A. Brings the
unified daemon UI surface to ~95% SDK-side completion.

## Files added

- `docs/developers/daemon-ui/README.md` — full API reference
  - Three-layer model (normalizer → reducer → render helpers)
  - Quick start with idiomatic event-loop pattern
  - Event taxonomy (28+ types categorized: chat-stream / session-meta /
    workspace / auth device-flow)
  - Render contract cookbook (markdown / HTML / plainText)
  - Tool preview taxonomy (13 kinds with use cases)
  - State selectors (currentTool / approvalMode / toolProgress / ordering)
  - Cancellation propagation explanation
  - Time semantics (eventId > serverTimestamp > clientReceivedAt
    precedence)
  - Adapter conformance usage
  - ErrorKind dispatch pattern
  - Tool provenance dispatch pattern
  - Forward-compat principles

- `docs/developers/daemon-ui/MIGRATION.md` — adapter author migration
  cookbook
  - Step-by-step recommended adoption order (9 steps, value-ranked)
  - Before/after code examples for each step
  - Backward-compat checklist (everything is additive — no breaking
    changes)
  - Cross-references to PR-A through PR-H commits

## Roadmap

PR-I of the unified follow-up to PR QwenLM#4328. Documentation-only — no
code changes; no tests affected.

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@chiga0
Copy link
Copy Markdown
Collaborator Author

chiga0 commented May 20, 2026

Updated this PR in ae729357e.

Handled the latest review threads as follows:

  • Rebased feat/daemon-ui-completeness-followup onto the latest feat(daemon): add shared UI transcript layer #4328 head (6dbfffaaf) so the WebUI reconnect / assistant.done / prompt cancel / text trimming fixes are now included in this stacked PR.
  • Fixed the valid SDK/UI feedback: public daemon barrel re-exports, local-block ordering, qwen-oauth device-flow provider, per-listener store notification isolation, permission trim tombstone, selected permission status false positives, markdown code-fence escaping, createdAt JSDoc, selectToolProgress alpha marker, and cached Intl.DateTimeFormat usage.
  • Updated daemon UI docs to clarify the current web chat / web terminal adoption target. Native TUI, channel, and IDE remain on their existing default paths.
  • Rechecked the earlier WebUI typecheck concerns. packages/webui typecheck now passes locally after the update. The @qwen-code/sdk/daemon import resolves through the package tsconfig path mapping, and CreateSessionRequest exposes explicit fields rather than only an index signature.

Verification:

  • cd packages/sdk-typescript && npm run typecheck
  • cd packages/sdk-typescript && npx vitest run test/unit/daemonUi.test.ts --reporter dot -> 105 passed
  • cd packages/webui && npm run typecheck

Note: packages/webui Vitest startup is still blocked in my local worktree by missing vite-plugin-dts, before running assertions. I did not count that as a PR assertion failure.

Generated by GPT-5 model.

@chiga0 chiga0 requested a review from wenshao May 20, 2026 09:42
Comment thread packages/sdk-typescript/src/daemon/ui/render.ts Outdated
Comment thread packages/webui/src/daemon/transcriptAdapter.ts Outdated
Comment thread packages/webui/src/daemon/DaemonSessionProvider.tsx
Comment thread packages/webui/src/daemon/DaemonSessionProvider.tsx
Comment thread packages/sdk-typescript/src/daemon/ui/transcript.ts Outdated
Comment thread packages/sdk-typescript/src/daemon/ui/transcript.ts
Comment thread packages/sdk-typescript/src/daemon/ui/transcript.ts
Comment thread packages/webui/src/daemon/DaemonSessionProvider.tsx
Comment thread packages/sdk-typescript/src/daemon/ui/render.ts
Comment thread packages/webui/src/daemon/DaemonSessionProvider.tsx
Comment thread packages/sdk-typescript/test/unit/daemonUi.test.ts
Comment thread packages/webui/package.json Outdated
Comment thread packages/sdk-typescript/src/daemon/ui/normalizer.ts Outdated
Comment thread packages/sdk-typescript/src/daemon/ui/transcript.ts
Comment thread packages/webui/src/daemon/transcriptAdapter.ts
Comment thread packages/sdk-typescript/src/daemon/ui/conformance.ts Outdated
doudouOUC added a commit that referenced this pull request May 20, 2026
…stion)

Adopts all 7 review threads from the first wenshao + Copilot review
round on PR #4360. All technical fixes (no judgment calls).

**[Critical] BridgeTimeoutError constructor blocks tsc** (wenshao
PRRT_kwDOPB-92c6DfcRI)

`server.test.ts:4670` called `new BridgeTimeoutError('initialize
timed out')` but the constructor signature is `(label: string,
timeoutMs: number)` — TS2554 blocked `tsc --noEmit` and `npm run
build`. Fixed to `new BridgeTimeoutError('initialize', 5000)` per
suggested fix; resulting message `"HttpAcpBridge initialize timed
out after 5000ms"` still satisfies the existing
`.toContain('timed out')` assertion.

**[Suggestion] Copilot JSDoc package name** (Copilot
PRRT_kwDOPB-92c6De-Sm, ToolCallEmitter.ts:210)

JSDoc referenced `@qwen-code/core/mcp-tool` but the actual package
is `@qwen-code/qwen-code-core` with the file at
`packages/core/src/tools/mcp-tool.ts`. Updated the reference.

**[Suggestion] Copilot errorKind type widening** (Copilot
PRRT_kwDOPB-92c6De-Ro, events.ts:244)

`DaemonStreamErrorData.errorKind` was typed as `string` and the
JSDoc said "7-value" closed enum — but `DAEMON_ERROR_KINDS` actually
has 8 values, and `SERVE_ERROR_KINDS` (daemon-side) has 9 (adds
`stat_failed`). Typed as `DaemonErrorKind | (string & {})` for
forward-compat: SDK consumers get IDE autocomplete on the known 8
kinds while still accepting future daemon-side additions (like
`stat_failed`) without a type error. Updated JSDoc to accurately
list 8 current values + call out the forward-compat widening.

Side observation (NOT in scope of this PR): `DAEMON_ERROR_KINDS`
(SDK) lacks `stat_failed` that exists in `SERVE_ERROR_KINDS`
(daemon). That's a separate drift fix.

**[Suggestion] TERMINAL wording misleading** (wenshao
PRRT_kwDOPB-92c6Dj-JL, eventBus.ts:369)

Comment called `state_resync_required` a "TERMINAL synthetic frame"
but it's emitted FIRST (before replay) and the stream stays OPEN.
Genuine terminals like `client_evicted` close the stream after the
frame. Rewrote the comment per suggestion: "id-less synthetic
frame... Unlike `client_evicted`, the stream stays OPEN" — so an
oncall reading the source at 3am gets the right mental model.

**[Suggestion] `_meta` merge dead code + stale reference** (wenshao
PRRT_kwDOPB-92c6Dj-JF, server.ts:2569)

The `existingMeta` merge reads `event._meta` at BridgeEvent top
level, but ToolCallEmitter's `_meta` lives nested inside
`event.data._meta` (publish path goes through `events.publish({type:
'session_update', data: params})`). In production `existingMeta` is
always undefined — the merge is a forward-compat escape hatch, not
an active merge. Also the comment referenced
`extractServerTimestamp` (sdk-typescript) which grep confirms
doesn't exist yet (it's planned in chiga0 PR #4353).

Rewrote the comment block to (1) acknowledge no current producer
sets `_meta` at the top level — it's a forward-compat hook for
future envelope-level metadata; (2) drop the stale
`extractServerTimestamp` reference and instead note that chiga0
PR #4353 plans the 3-location probe. Code shape unchanged
(forward-compat spread stays).

**[Suggestion] session_closed + client_evicted passthrough tests**
(wenshao PRRT_kwDOPB-92c6Dj-JW, daemonEvents.test.ts:2284)

`RESYNC_PASSTHROUGH_TYPES` has 5 members but only `session_died`
and `stream_error` had passthrough tests. Added two missing tests:
`session_closed` and `client_evicted` while awaitingResync.
Critical because if a future refactor accidentally drops either
from the set, a consumer in resync limbo would silently swallow
the terminal signal and the UI would hang on "loading resync
state…".

**[Suggestion] readTextFile non-FsError passthrough test** (wenshao
PRRT_kwDOPB-92c6Dj-JX, bridgeClient.test.ts:251)

The non-FsError pass-through test only covered `writeTextFile`.
Added a symmetric `readTextFile` test — the two `try/catch` blocks
in `bridgeClient.ts` are independent, so test parity guards against
divergent refactors (e.g. someone adding wrapping on one side but
not the other).

Verification
- `packages/acp-bridge`: 6 files, 114/114 pass (+1 new readTextFile
  non-FsError test).
- `packages/sdk-typescript`: 75/75 pass on daemonEvents.test.ts
  (+2 new session_closed / client_evicted passthrough tests).
- `packages/cli/src/serve/server.test.ts`: 248 tests pass on
  touched cases (5 SSE / serverTimestamp / stream_error tests).
  Pre-existing F3 (#4335 merge) test failures unrelated to this
  PR's changes — verified by stash-test-restore on clean tree.
- TypeScript clean on touched regions; `BridgeTimeoutError`
  2-arg fix unblocks `tsc --noEmit` for the test file.
doudouOUC added a commit that referenced this pull request May 21, 2026
…amp / provenance / errorKind / state_resync_required) (#4360)

* feat(serve): stamp serverTimestamp / tool provenance / errorKind on daemon events (#4175 F4 prereq)

Adopts chiga0's three P0 SDK-side blockers from #4175 comment #19 —
the SDK side already consumes these fields (PR #4353), but daemon
hadn't stamped them yet, leaving the corresponding UI affordances
inert. All three stampings are purely additive on the wire and don't
require any SDK type changes (SDK already has forward-compat field
slots).

**#19.1 — `_meta.serverTimestamp` on every SSE frame** (`server.ts`
`formatSseFrame()`)

Stamped at the SSE write boundary (NOT EventBus.publish) so the
in-memory `BridgeEvent` type stays unchanged and internal consumers
don't see `_meta`. Pre-existing `_meta` keys (e.g. tool_call's
`_meta.toolName`) are preserved via spread merge. SDK reads via the
3-location probe in `extractServerTimestamp` (chiga0's PR #4353);
we pick `_meta.serverTimestamp` (Anthropic convention) so top-level
event type stays unpolluted.

Why this matters: pre-fix, multi-client UIs showing "X minutes ago"
or sorting transcript blocks by emit time used each client's local
clock — drifts of tens of seconds to minutes across browsers/tabs/
mobile produced visibly inconsistent timestamps.

**#19.2 — `tool_call` `provenance` + `serverId` on every emitter
event** (`ToolCallEmitter.ts`)

New static `ToolCallEmitter.resolveToolProvenance(toolName,
subagentMeta)` returns `{ provenance: 'builtin' | 'mcp' | 'subagent';
serverId? }`. Resolution rules (per user-confirmed design decision
from issue comment): subagent takes precedence (set when
subagentMeta is present); `mcp__<server>__<tool>` naming heuristic
classifies MCP tools with serverId; everything else is builtin.

Stamped on `emitStart` AND `emitResult` AND `emitError` (all three
emit paths) so a reconnecting client receiving a `tool_call_update`
frame from the replay ring (without the original `tool_call` start
event) can still derive the provenance. Provenance is stable per
tool, so stamping on every event is redundant — but the marginal
serialization cost is tiny and reconnect correctness wins.

Chose the naming heuristic (not ToolRegistry lookup) per user
confirmation: matches the SDK's own fallback (chiga0 PR #4353), no
new ctx-dep on emit hot path, no signature changes.

**#19.3 — `errorKind` on `stream_error`** (`server.ts` line ~1955)

Stamped via `mapDomainErrorToErrorKind(err)` — the 7-value classifier
already lives in `@qwen-code/acp-bridge/status.ts` since #4319. When
the classifier returns `undefined` (generic Error etc.) the field is
omitted — strictly additive. SDK consumers handle "errorKind absent"
as before (fall back to rendering `error` text).

NOT stamped on `session_died` because the 3 emit sites in `acp-bridge/
bridge.ts` don't have a classifiable `err` in scope:
- `channel_closed` carries only exitCode/signalCode (no error)
- `killed` is user-initiated (no domain error)
- `daemon_shutdown` is operator-initiated (no domain error)

A follow-up could thread channel-spawn errors through to the
session_died emit site to enable `errorKind: 'init_timeout'` /
`missing_binary` classification — left for a separate PR to avoid
mixing protocol stamping with lifecycle plumbing.

Verification
- `npx vitest run packages/cli/src/serve/server.test.ts -t "serverTimestamp|stream_error|errorKind"` — 5 pass
- `npx vitest run packages/cli/src/acp-integration/session/emitters/ToolCallEmitter.test.ts` — 46 pass (+ 11 new tests for resolveToolProvenance + provenance stamping on all 3 emit paths)
- `npx vitest run packages/cli/src/acp-integration/session/HistoryReplayer.test.ts` — 17 pass
- TypeScript clean on touched regions; pre-existing F3 (#4335 merge) errors elsewhere are unrelated.

Existing test updates
- 15 `_meta: { toolName: 'X' }` assertions in ToolCallEmitter.test.ts updated to include `provenance: 'builtin'` (defensive — catches accidental drift if a future refactor stops stamping). 2 strict-equality assertions in HistoryReplayer.test.ts similarly updated. The first SSE-frame test in server.test.ts switched from `toEqual` to `toMatchObject` since `_meta.serverTimestamp` makes exact equality brittle; a dedicated test pins the new field's shape.

* feat(serve+sdk): detect SSE ring eviction on resume, expose state_resync_required (#4175 F4 prereq)

Closes the multi-client SSE reducer divergence bug Ilya0527 raised in
#4175 comment #15. Pre-fix scenario:

1. Consumer's SSE stream drops; client buffers `Last-Event-ID: N`.
2. Network reconnects long enough later that events `[N+1, ringHead-1]`
   were evicted from the daemon's per-session ring.
3. Daemon's `subscribe({lastEventId: N})` silently replays only the
   surviving suffix.
4. Consumer's SDK reducer keeps applying deltas as if the stream was
   contiguous. Its state has now drifted from the daemon's truth —
   no terminal signal, no warning. The `SessionState` reducer's
   "same event stream in → same state out" purity guarantee is
   broken.

The bug's blast radius is exactly when multi-client matters: F4
brings up the TUI / IDE / web client adapters that share session
state, so divergence becomes visibly inconsistent across clients.

**Daemon side** (`packages/acp-bridge/src/eventBus.ts`)

In `subscribe()`'s replay path, detect ring eviction by comparing
the ring's earliest id against `lastEventId + 1`. When a gap exists,
force-push a synthetic terminal `state_resync_required` frame BEFORE
the surviving replay events:

```
{ v: 1, type: 'state_resync_required',
  data: { reason: 'ring_evicted',
          lastDeliveredId: N,
          earliestAvailableId: M } }
```

Per user-confirmed design (issue comment discussion): the frame has
NO `id` (mirrors the `client_evicted` synthetic terminal pattern so
it doesn't burn a slot in the per-session monotonic sequence). Replay
continues after the resync frame — the SDK reducer auto-skips
subsequent deltas (see below) but the frames stay on the wire so
adapters have the option to compute a "what you missed" diff later.

**SDK side** (`packages/sdk-typescript/src/daemon/events.ts`)

Adds:
- `'state_resync_required'` to `DAEMON_EVENT_TYPES` union
- `DaemonStateResyncRequiredData` + `DaemonStateResyncRequiredEvent`
- `isStateResyncRequiredData` predicate
- `DaemonStreamLifecycleEvent` union widened
- Reducer state fields: `awaitingResync: boolean`,
  `resyncRequiredCount: number`, `lastResyncRequired?`
- Reducer case for `state_resync_required` — sets the flag, increments
  count, records data
- **Top-of-reducer gate**: when `awaitingResync === true`, all non-
  terminal events are auto-skipped (still advance `lastEventId`).
  Terminal lifecycle events (`session_died` / `session_closed` /
  `client_evicted` / `stream_error`) STILL apply — critical end-of-
  stream signals don't depend on prior state being current.
- Re-exported `DaemonStateResyncRequiredData` / Event from
  `daemon/index.ts` and `src/index.ts` (matches surface posture of
  sibling lifecycle types).

Consumer recovery contract: when `state.awaitingResync === true`,
call `loadSession` (out of band) to fetch the daemon's canonical
session snapshot, then reconstruct view state via
`createDaemonSessionViewState({...seed from loaded state})`. The
fresh state defaults `awaitingResync: false` so the seed implicitly
clears the flag.

**Side fix** (`stream_error` errorKind)

`DaemonStreamErrorData.errorKind?: string` typed for the optional
classification field that Commit 1 (`14637cd79`) added daemon-side.
Strictly additive — old daemons omit the field, SDK falls back to
rendering `error` text.

Verification
- `packages/acp-bridge`: 6 files, 108/108 pass (+5 new resync-detection
  tests; 1 existing "default ring size 8000" test updated to acknowledge
  the synthetic resync frame at the head of its replay batch).
- `packages/sdk-typescript`: 13 files, 451/451 pass (+8 new reducer
  resync tests covering set/skip/terminal-passthrough/recovery/
  repeated-resync/malformed-payload).
- TypeScript clean across both packages on touched regions.

* fix(acp-bridge): preserve FsError structure over ACP wire (#4360 Codex round 2 fold-in)

Adopts Codex review round 2 P2 finding on PR #4360 — fold-in to the
F4 prereq scope per user's "a" decision.

**Problem**: When the `BridgeFileSystem` adapter (introduced in
#4334 fs adapter wiring) throws a structured `FsError` (e.g.
`kind: 'untrusted_workspace'` / `kind: 'symlink_escape'` / `kind:
'file_too_large'`), the `@agentclientprotocol/sdk` default RPC
error serialization only sends `error.message` as JSON-RPC -32603
"Internal error". The structured `kind` / `status` / `hint` fields
on FsError are stripped on the way to the agent.

Downstream impact: SDK consumers receiving the ACP error payload
lose the typed discriminator and have to regex-match the human-
readable message to dispatch UI (auth retry vs file picker vs
proxy hint). This silently regresses what the FsError-typed
contract was supposed to provide.

**Fix**: At the bridge boundary (`BridgeClient.writeTextFile` and
`BridgeClient.readTextFile`), catch errors from `this.fileSystem.
writeText/readText` calls. Duck-type check for FsError shape
(`err.name === 'FsError'` + `typeof err.kind === 'string'`); when
matched, rethrow as ACP `RequestError(-32603, message, {errorKind,
hint, status})`. The agent's RPC client now receives `data.
errorKind` and can branch on the closed-enum kind.

Cross-package note: FsError lives in `cli/src/serve/fs/errors.ts`
and acp-bridge can't `import { FsError }` from cli (dependency
inversion). Same duck-typing pattern that `mapDomainErrorToErrorKind`
(status.ts) already applies to `TrustGateError` / `SkillError` for
the same cross-package bundling reason — `instanceof` would fail
across package boundaries when bundlers don't dedupe.

**Code shape**

```typescript
function isFsErrorShape(err: unknown): err is FsErrorShape {
  return (
    err instanceof Error &&
    err.name === 'FsError' &&
    typeof (err as { kind?: unknown }).kind === 'string'
  );
}

function preserveFsErrorOverAcp(err: unknown): never {
  if (isFsErrorShape(err)) {
    throw new RequestError(-32603, err.message, {
      errorKind: err.kind,
      ...(err.hint !== undefined ? { hint: err.hint } : {}),
      ...(err.status !== undefined ? { status: err.status } : {}),
    });
  }
  throw err;
}
```

Applied at both `if (this.fileSystem) { ... }` blocks (writeTextFile
+ readTextFile) — wrapped the adapter call in try/catch +
`preserveFsErrorOverAcp(err)`. Non-FsError errors are rethrown
unchanged (default ACP serialization is fine for unstructured
errors; only the structured shape needs preservation).

JSON-RPC code stays at -32603 (internal error) rather than mapping
FsError.kind → JSON-RPC code. Rationale: the JSON-RPC standard
defines only a handful of code values (-32700/-32600/-32601/-32602/
-32603 + a reserved range for application errors), and mapping
~10 FsError kinds to that narrow space is lossy. Instead the
structured `data.errorKind` carries the semantic information SDK
consumers need; JSON-RPC code remains the generic "an error happened"
signal.

**Tests** (+5 in `bridgeClient.test.ts`)

- writeTextFile FsError → ACP RequestError with errorKind in data
- readTextFile FsError preserving symlink_escape kind (no hint
  field present → not stamped, spread guard works)
- non-FsError pass-through (plain Error stays plain Error, no
  RequestError wrap)
- hint field preservation when present
- defensive: error with `kind` field but wrong `name` does NOT get
  wrapped (e.g. PermissionForbiddenError happens to have a kind
  field internally — must NOT be confused for FsError)

Verification: 113/113 acp-bridge tests pass (+5 new FsError-
preservation tests). Full serve suite shows pre-existing F3-related
failures unrelated to this change (verified in isolation).

* fix: 7 wenshao/copilot review fold-ins on #4360 (1 Critical + 6 Suggestion)

Adopts all 7 review threads from the first wenshao + Copilot review
round on PR #4360. All technical fixes (no judgment calls).

**[Critical] BridgeTimeoutError constructor blocks tsc** (wenshao
PRRT_kwDOPB-92c6DfcRI)

`server.test.ts:4670` called `new BridgeTimeoutError('initialize
timed out')` but the constructor signature is `(label: string,
timeoutMs: number)` — TS2554 blocked `tsc --noEmit` and `npm run
build`. Fixed to `new BridgeTimeoutError('initialize', 5000)` per
suggested fix; resulting message `"HttpAcpBridge initialize timed
out after 5000ms"` still satisfies the existing
`.toContain('timed out')` assertion.

**[Suggestion] Copilot JSDoc package name** (Copilot
PRRT_kwDOPB-92c6De-Sm, ToolCallEmitter.ts:210)

JSDoc referenced `@qwen-code/core/mcp-tool` but the actual package
is `@qwen-code/qwen-code-core` with the file at
`packages/core/src/tools/mcp-tool.ts`. Updated the reference.

**[Suggestion] Copilot errorKind type widening** (Copilot
PRRT_kwDOPB-92c6De-Ro, events.ts:244)

`DaemonStreamErrorData.errorKind` was typed as `string` and the
JSDoc said "7-value" closed enum — but `DAEMON_ERROR_KINDS` actually
has 8 values, and `SERVE_ERROR_KINDS` (daemon-side) has 9 (adds
`stat_failed`). Typed as `DaemonErrorKind | (string & {})` for
forward-compat: SDK consumers get IDE autocomplete on the known 8
kinds while still accepting future daemon-side additions (like
`stat_failed`) without a type error. Updated JSDoc to accurately
list 8 current values + call out the forward-compat widening.

Side observation (NOT in scope of this PR): `DAEMON_ERROR_KINDS`
(SDK) lacks `stat_failed` that exists in `SERVE_ERROR_KINDS`
(daemon). That's a separate drift fix.

**[Suggestion] TERMINAL wording misleading** (wenshao
PRRT_kwDOPB-92c6Dj-JL, eventBus.ts:369)

Comment called `state_resync_required` a "TERMINAL synthetic frame"
but it's emitted FIRST (before replay) and the stream stays OPEN.
Genuine terminals like `client_evicted` close the stream after the
frame. Rewrote the comment per suggestion: "id-less synthetic
frame... Unlike `client_evicted`, the stream stays OPEN" — so an
oncall reading the source at 3am gets the right mental model.

**[Suggestion] `_meta` merge dead code + stale reference** (wenshao
PRRT_kwDOPB-92c6Dj-JF, server.ts:2569)

The `existingMeta` merge reads `event._meta` at BridgeEvent top
level, but ToolCallEmitter's `_meta` lives nested inside
`event.data._meta` (publish path goes through `events.publish({type:
'session_update', data: params})`). In production `existingMeta` is
always undefined — the merge is a forward-compat escape hatch, not
an active merge. Also the comment referenced
`extractServerTimestamp` (sdk-typescript) which grep confirms
doesn't exist yet (it's planned in chiga0 PR #4353).

Rewrote the comment block to (1) acknowledge no current producer
sets `_meta` at the top level — it's a forward-compat hook for
future envelope-level metadata; (2) drop the stale
`extractServerTimestamp` reference and instead note that chiga0
PR #4353 plans the 3-location probe. Code shape unchanged
(forward-compat spread stays).

**[Suggestion] session_closed + client_evicted passthrough tests**
(wenshao PRRT_kwDOPB-92c6Dj-JW, daemonEvents.test.ts:2284)

`RESYNC_PASSTHROUGH_TYPES` has 5 members but only `session_died`
and `stream_error` had passthrough tests. Added two missing tests:
`session_closed` and `client_evicted` while awaitingResync.
Critical because if a future refactor accidentally drops either
from the set, a consumer in resync limbo would silently swallow
the terminal signal and the UI would hang on "loading resync
state…".

**[Suggestion] readTextFile non-FsError passthrough test** (wenshao
PRRT_kwDOPB-92c6Dj-JX, bridgeClient.test.ts:251)

The non-FsError pass-through test only covered `writeTextFile`.
Added a symmetric `readTextFile` test — the two `try/catch` blocks
in `bridgeClient.ts` are independent, so test parity guards against
divergent refactors (e.g. someone adding wrapping on one side but
not the other).

Verification
- `packages/acp-bridge`: 6 files, 114/114 pass (+1 new readTextFile
  non-FsError test).
- `packages/sdk-typescript`: 75/75 pass on daemonEvents.test.ts
  (+2 new session_closed / client_evicted passthrough tests).
- `packages/cli/src/serve/server.test.ts`: 248 tests pass on
  touched cases (5 SSE / serverTimestamp / stream_error tests).
  Pre-existing F3 (#4335 merge) test failures unrelated to this
  PR's changes — verified by stash-test-restore on clean tree.
- TypeScript clean on touched regions; `BridgeTimeoutError`
  2-arg fix unblocks `tsc --noEmit` for the test file.

* fix: 3 wenshao observability fold-ins on #4360 (all Suggestion)

Adopts all 3 threads from wenshao's second review round on PR #4360.
All Suggestion-level — daemon-side observability + 1 missing SDK
reducer test.

**[Suggestion] SSE ring eviction silently emits state_resync_required**
(PRRT_kwDOPB-92c6Dp_Uk, eventBus.ts:394)

Pre-fix: when a consumer reconnects past the ring boundary, the
daemon emits `state_resync_required` with zero stderr breadcrumb.
A 3am oncall chasing "my UI is frozen with stale state" couldn't
grep daemon logs to distinguish (a) ring undersized, (b) client
reconnecting too slowly, (c) network partition causing repeated
reconnects.

Fix: detect `next.value.type === 'state_resync_required'` in the
SSE handler's iter loop in `server.ts` and emit a `writeStderrLine`
with the gap details (`lastEventId`, `earliestInRing`, computed
`gap` count, `reason`). Logged at the route boundary rather than
inside `EventBus.subscribe` to keep the bus implementation pure +
concentrate daemon-side observability in the route handler that
already logs socket errors + heartbeats.

**[Suggestion] Bridge iterator throw forwarded to client but not
logged daemon-side** (PRRT_kwDOPB-92c6Dp_Uo, server.ts:1956)

Pre-fix inconsistency: the adjacent `res.on('error', ...)` handler
at line ~1925 logs SSE socket errors with `writeStderrLine`, but
the bridge-iterator-catch block at line ~1940-1965 sends a
`stream_error` SSE frame to the client AND swallows the error
daemon-side. When the bridge iterator throws (subprocess crash,
channel protocol error, unhandled rejection), distinguishing
"subprocess OOM-killed" from "protocol bug" required attaching a
debugger.

Fix: mirror the adjacent handler's pattern — add `writeStderrLine`
before the `stream_error` SSE frame send, including the classified
`errorKind` (when available) in brackets so operators can grep for
`[init_timeout]` / `[missing_binary]` etc.

**[Suggestion] No SDK reducer test verifying stream_error.errorKind
flowthrough** (PRRT_kwDOPB-92c6Dp_Uq, daemonEvents.test.ts:2331)

The daemon-side wire format is tested in `server.test.ts`
(`parsed.data.errorKind === 'init_timeout'`) and
`DaemonStreamErrorData` now declares `errorKind?`, but the SDK
reducer test suite never fed a `stream_error` event with
`errorKind` and asserted `state.streamError?.errorKind`. A future
refactor stripping `errorKind` from the reducer's data assignment
(e.g. spreading only `{error}`) would silently regress without
test signal.

Fix: added `captures errorKind on stream_error in view state` test
exercising the full pipeline — reducer receives stream_error with
errorKind, view state's `streamError.errorKind` matches.

Verification
- `packages/sdk-typescript`: 76/76 daemonEvents tests pass (+1
  new flowthrough test).
- `packages/cli/src/serve/server.test.ts`: 6 targeted serverTimestamp
  / stream_error / errorKind tests pass — server.ts changes are
  observability-only (no behavior change to wire format).
- Pre-existing F3 (#4335 merge) test failures elsewhere are
  unrelated to this PR's changes.

* test(serve): 2 wenshao observability fold-ins on #4360 (stderr log coverage)

Adopts both threads from wenshao's third review round on PR #4360.
Both Suggestion-level — pin the daemon-side stderr log artifacts that
commit `dce2fed0f` introduced. Pre-fix: the EventBus-level
state_resync_required emission was tested in eventBus.test.ts, and
the SSE wire shape was tested in server.test.ts, but the actual
operator-facing artifacts (the stderr log lines themselves) had no
test coverage. A regression swapping operands in the `gap`
arithmetic, dropping the sessionId from the log, or breaking the
`[errorKind]` suffix would ship silently and only surface when an
operator went grepping during an incident.

**[Suggestion] SSE ring eviction stderr log untested**
(PRRT_kwDOPB-92c6Dqtlb, server.ts:1948)

Added 2 tests:
- `writes a daemon-side stderr log on SSE ring eviction` — yields
  a `state_resync_required` frame from a fake bridge, spies on
  `process.stderr.write`, asserts the captured log contains
  `session sess-A` + `lastEventId=5` + `earliestInRing=12` +
  `gap=6 events` (pins the arithmetic) + `reason=ring_evicted` +
  `loadSession` (the recovery hint).
- `falls back to "?" placeholders when state_resync_required data is
  partial` — yields a frame with empty `data: {}`, asserts every
  `?? '?'` branch fires (lastEventId=? / earliestInRing=? /
  gap=? events / reason=?). Defensive against future daemon schema
  changes that drop one of these fields.

**[Suggestion] Bridge iterator error stderr log untested**
(PRRT_kwDOPB-92c6Dqtlh, server.ts:1993)

Added 2 tests:
- `writes a daemon-side stderr log on bridge iterator error` — fake
  bridge throws plain `Error('agent died')` mid-stream, captures
  stderr, asserts the log contains `session sess-A` + `agent died`,
  and **no** `[…]` suffix (plain Error → `mapDomainErrorToErrorKind`
  returns undefined → no suffix).
- `includes [errorKind] suffix in bridge iterator error log when
  classified` — fake bridge throws `BridgeTimeoutError('initialize',
  5000)`, asserts the log contains `[init_timeout]`. Pins the
  classified-vs-unclassified branch of the conditional suffix
  template.

All 4 tests use `vi.spyOn(process.stderr, 'write').mockReturnValue(
true)` + filter `mock.calls` for the relevant log prefix — same
pattern as the existing `mcp-client-manager.test.ts` stderr-spy
tests in core, plus `startupProfiler.test.ts` in cli.

Verification: 7/7 targeted observability tests pass. Pre-existing
F3 (#4335 merge) test failures elsewhere are unrelated to this
PR's changes.
@doudouOUC
Copy link
Copy Markdown
Collaborator

Daemon-side dependencies landed — F4 prereq #4360 merged

@chiga0 — PR #4360 (F4 prereq — daemon protocol completion) merged into daemon_mode_b_main at 2026-05-21 03:11Z (commit a60c1c52a). This addresses the 3 P0 stamping items you listed in #4175 comment #19.

What's now emitted on the wire:

Field Location SDK consumer
_meta.serverTimestamp every SSE frame via formatSseFrame boundary stamp your extractServerTimestamp 3-location probe (PR-B bdffe3a34)
tool_call _meta.provenance ('builtin' | 'mcp' | 'subagent') + optional _meta.serverId ToolCallEmitter.emitStart/emitResult/emitError via ToolCallEmitter.resolveToolProvenance(toolName, subagentMeta) heuristic on mcp__<server>__<tool> your DaemonUiToolUpdateEvent.provenance/serverId (PR-A 5128ff03f)
stream_error.errorKind (typed as DaemonErrorKind | (string & {}) for forward-compat) server.ts stream-error frame via mapDomainErrorToErrorKind(err) your DaemonUiErrorEvent.errorKind + asDaemonErrorKind validator (PR-A)

Plus addressed the state-divergence hazard Ilya0527 raised in #15 — SDK reducer now has awaitingResync + lastResyncRequired view state that flips on a new state_resync_required synthetic frame the daemon emits when consumers reconnect past the SSE ring eviction point.

SDK PR #4353 unblocking: the forward-compat field slots you preserved are no longer dead code on daemon_mode_b_main HEAD. The 5% gap you flagged ("daemon-side stamps SDK already-ready to consume") is now 0%. The 3-location probe, provenance dispatch, errorKind classification, and state_resync_required reducer case all have real wire data flowing from daemon to SDK.

Caveat: my PR #4360 implements daemon→SDK protocol fields, but I left the SDK-side type definitions in your domain — the extractServerTimestamp helper and the _meta-typed envelope still need to ship via #4353 for SDK consumers to read them without as any. Filed as a Codex-round review note on #4360 cross-referencing #4353 so the dependency is visible.

Anything else from your comment #19 P1 (subagent nesting + tool.progress) or P2 (multimodal echo) you want me to look at next? Those touch Core (MessageEmitter + HistoryReplayer + ACP child-side emitter plumbing) so they're separate scope from #4360, but I can pick them up if the timing's right.

🤖 Generated with Qwen Code

Comment thread packages/webui/src/daemon/DaemonSessionProvider.tsx
Comment thread packages/webui/src/daemon/DaemonSessionProvider.tsx
Comment thread packages/sdk-typescript/src/daemon/ui/render.ts Outdated
Comment thread packages/sdk-typescript/src/daemon/ui/render.ts Outdated
chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request May 21, 2026
…ete (PR-F)

Closes the "5 additional preview kinds" item in PR QwenLM#4353's TODO §A
(SDK-only work).

## New preview kinds (8 → 13)

- `code_block` — `{ language?, code, origin? }` — REPL / formatter /
  generator output, fenced as `\`\`\`<language>` in markdown
- `search` — `{ query, resultCount?, top? }` — grep / ripgrep / find /
  glob results with up to 5 top hits
- `tabular` — `{ columns, rows, totalRows? }` — structured table output
  (50-row cap with `totalRows` truncation indicator); supports both
  `columns: string[] + rows: unknown[][]` explicit shape and legacy
  `data: Array<Record<>>` shape (auto-infers columns from first row)
- `image_generation` — `{ prompt, thumbnailUrl?, model? }` — dall-e /
  diffusion / imagen / flux / sora style tools
- `subagent_delegation` — `{ agentName, task, parentDelegationId? }` —
  Anthropic-style Task tool and similar sub-agent dispatchers

## Detector priority

Order matters — most specific wins. New detectors slot in between
`mcp_invocation` and `file_diff`:

```
mcp_invocation > subagent_delegation > search > image_generation
  > file_diff > file_read > web_fetch > code_block > tabular
  > command > key_value > generic
```

Rationale: subagent / search / image generation are most discriminable
(distinct toolName patterns); file ops next; code_block / tabular last
because their shapes (`code:`, `columns:`) can appear in other tools.

## Render projections

Both `daemonToolPreviewToMarkdown` and the plain-text rendering paths
extended with cases for all 5 new kinds:

- code_block: fenced markdown code block with language tag
- search: bold header + GFM bullet list of top results
- tabular: GFM pipe table with header / separator / body / truncation hint
- image_generation: bold header + blockquoted prompt + embedded markdown
  image (URL sanitization respected via `sanitizeUrls` opt)
- subagent_delegation: bold delegate-arrow header + blockquoted task +
  optional parent delegation reference

## Test coverage (91/91 pass, +14 new)

- Each detector with positive case
- Detector priority verified: subagent_delegation wins over file_diff
  when toolName='Task' has both subagent + file-edit fields
- Tabular row cap (50) + totalRows stamping for truncated data
- Legacy data: Array<Record<>> auto-column inference
- Each render projection with structural assertions (markdown table
  format, image embed, bullet lists)

## Roadmap

PR-F of the unified follow-up to PR QwenLM#4328. Brings the preview taxonomy
to 13 kinds covering: file ops (3), web (1), code/data (2), media (1),
agent control (2 — ask_user_question + subagent_delegation), MCP (1),
search (1), generic fallbacks (2).

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request May 21, 2026
…PR-G)

Closes the "Adapter conformance test framework" item in PR QwenLM#4353's TODO §A.
Lets any daemon-ui adapter (TUI / web / IDE / channel / mobile) validate
that it projects a fixed corpus of daemon SSE event streams to the same
semantic shape — catches projection drift before it reaches users.

## API surface

```ts
interface DaemonUiAdapterUnderTest {
  reduce(events: readonly DaemonUiEvent[]): unknown;
  renderToText(state: unknown): string;
}

interface DaemonUiConformanceFixture {
  name: string;
  description: string;
  envelopes: DaemonEvent[];           // raw daemon envelopes
  expectedContains: string[];          // phrases the rendered text MUST contain
  expectedAbsent?: string[];           // phrases that MUST NOT appear
  normalizeOptions?: { ... };          // forward-compat normalize opts
}

runAdapterConformanceSuite(adapter, opts?): ConformanceSuiteResult
DAEMON_UI_CONFORMANCE_FIXTURES: ReadonlyArray<DaemonUiConformanceFixture>
```

## Design

**Format-agnostic assertion**: adapters can render to ANSI / HTML /
markdown / JSX — the framework only inspects plain text via
`renderToText`. Catches semantic divergence (missing user message,
wrong tool status, leaked secret) without forcing identical formatting.

**Embedded fixture corpus** (no fs reads — works in browser bundle):
- `simple-chat` — user/assistant streaming flow
- `tool-call-lifecycle` — running → completed transition
- `file-edit-diff` — file_diff preview surfacing
- `mcp-invocation` — MCP serverId/toolName extraction via heuristic
- `permission-lifecycle` — request + resolved with outcome
- `mcp-budget-warning` — Wave 3 event (adapter must observe but rendering
  is its choice)
- `cancellation-propagates` — tool block status flows
- `malformed-payload-redaction` — uses `includeRawEvent: true` to verify
  even a debug-mode adapter doesn't leak `token: secret-do-not-leak`
- `auth-device-flow-success` — Wave 4 OAuth events
- `available-commands-typed-event` — PR-A upgrade from status text

Per-fixture `expectedContains` and `expectedAbsent` describe the
content contract independently of format.

## Suite result

```ts
{
  passed: number,
  failed: ConformanceFailure[],   // each carries missing + leaked + excerpt
  total: number,
}
```

**Does not throw** — caller asserts on `result.failed` so adapter test
suites can produce per-fixture diagnostics rather than a single opaque
exception.

## Filter options

`only` / `skip` allow targeted runs during adapter development:

```ts
runAdapterConformanceSuite(myAdapter, { only: ['simple-chat'] });
runAdapterConformanceSuite(myAdapter, { skip: ['cancellation-propagates'] });
```

## Test coverage (97/97 pass, +6 new)

- SDK reference adapter (reducer + markdown render) passes all fixtures
- SDK reference adapter (reducer + plainText render) also passes
- Buggy adapter (empty string output) fails every fixture with non-empty
  `expectedContains`
- Buggy adapter (raw event dump via JSON.stringify) caught by redaction
  fixture's `expectedAbsent`
- `only` filter narrows to a single fixture
- `skip` filter excludes named fixtures from the corpus

## Usage from adapter authors

```ts
// In your adapter's test file
import { runAdapterConformanceSuite } from '@qwen-code/sdk/daemon';
import { reduceForTui, renderTuiState } from './my-tui-adapter';

it('TUI adapter conforms to daemon UI corpus', () => {
  const result = runAdapterConformanceSuite({
    reduce: reduceForTui,
    renderToText: renderTuiState,
  });
  expect(result.failed).toEqual([]);
});
```

## Roadmap

PR-G of the unified follow-up to PR QwenLM#4328. The corpus is intentionally
small (10 fixtures) but extensible — adapter authors can submit new
fixtures via additions to `DAEMON_UI_CONFORMANCE_FIXTURES` to lock in
regression coverage for edge cases their adapter encountered.

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request May 21, 2026
…act (PR-H)

Closes the "WebUI transcriptAdapter migration" item in PR QwenLM#4353's TODO §A.
Validates the PR-D render contract end-to-end on the real WebUI consumer.

`daemonTranscriptToUnifiedMessages(blocks, options?)` gains a new options
parameter:

```ts
interface DaemonTranscriptAdapterOptions {
  useMarkdown?: boolean;                  // default: false
  enrichToolDetailsWithPreview?: boolean; // default: false
}
```

Defaults preserve legacy behavior — existing callers see no change.

For `user` / `assistant` / `thought` blocks, content is projected via
SDK's `daemonBlockToMarkdown` instead of raw sanitized text. The WebUI's
markdown renderer (markdown-it) then gets:

- `**You**\n\n<content>` for user blocks (bold "You" label)
- Raw text for assistant blocks (markdown formatting in agent output
  passes through cleanly)
- `> *thought:* <text>` blockquote for thought blocks

For `tool` blocks, `rawOutput` is replaced with `daemonToolPreviewToMarkdown(block.preview)`.
This lets WebUI surfaces without per-preview-kind React components still
display:

- `file_diff` as a fenced unified diff
- `mcp_invocation` as `server::tool` with args summary
- `tabular` as GFM pipe table
- `search` as bullet list with match count
- `image_generation` as embedded markdown image
- `subagent_delegation` as delegate arrow + task quote

Renderers with per-kind components should leave this opt-out.

`packages/sdk-typescript/src/daemon/index.ts` was missing exports for
PR-D / PR-F / PR-G / PR-B / PR-E surface — WebUI's `@qwen-code/sdk/daemon`
import path uses the daemon root, not the ui/ sub-index. Added 15+
re-exports so consumers don't need to use the longer
`@qwen-code/sdk/daemon/ui/index.js` path.

Now exported from `@qwen-code/sdk/daemon` root:
- `daemonBlockToMarkdown` / `daemonBlockToHtml` / `daemonBlockToPlainText`
- `daemonToolPreviewToMarkdown`
- `extractContentPart` + `DaemonUiContentPart` type
- `formatBlockTimestamp` + `selectTranscriptBlocksOrderedByEventId`
- `selectCurrentTool` / `selectApprovalMode` / `selectToolProgress`
- `runAdapterConformanceSuite` + `DAEMON_UI_CONFORMANCE_FIXTURES`
- All associated types

`webui/src/daemon/transcriptAdapter.test.ts` mock blocks updated to include
`clientReceivedAt` (required field added in PR-B). Mechanical change —
every `createdAt: N` test fixture gets a matching `clientReceivedAt: N`.

- WebUI `npm run typecheck` — clean
- SDK `npm run typecheck` — clean
- SDK `vitest run test/unit/daemonUi.test.ts` — 97/97 pass
- WebUI transcriptAdapter test fixtures typecheck against updated
  DaemonTranscriptBlockBase schema

PR-H of the unified follow-up to PR QwenLM#4328. Closes the WebUI migration
gap in TODO §A.

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request May 21, 2026
Closes the final "Documentation" item in PR QwenLM#4353's TODO §A. Brings the
unified daemon UI surface to ~95% SDK-side completion.

## Files added

- `docs/developers/daemon-ui/README.md` — full API reference
  - Three-layer model (normalizer → reducer → render helpers)
  - Quick start with idiomatic event-loop pattern
  - Event taxonomy (28+ types categorized: chat-stream / session-meta /
    workspace / auth device-flow)
  - Render contract cookbook (markdown / HTML / plainText)
  - Tool preview taxonomy (13 kinds with use cases)
  - State selectors (currentTool / approvalMode / toolProgress / ordering)
  - Cancellation propagation explanation
  - Time semantics (eventId > serverTimestamp > clientReceivedAt
    precedence)
  - Adapter conformance usage
  - ErrorKind dispatch pattern
  - Tool provenance dispatch pattern
  - Forward-compat principles

- `docs/developers/daemon-ui/MIGRATION.md` — adapter author migration
  cookbook
  - Step-by-step recommended adoption order (9 steps, value-ranked)
  - Before/after code examples for each step
  - Backward-compat checklist (everything is additive — no breaking
    changes)
  - Cross-references to PR-A through PR-H commits

## Roadmap

PR-I of the unified follow-up to PR QwenLM#4328. Documentation-only — no
code changes; no tests affected.

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@chiga0 chiga0 force-pushed the feat/daemon-ui-completeness-followup branch from ae72935 to 20ce5f3 Compare May 21, 2026 05:20
Comment thread packages/sdk-typescript/src/daemon/ui/normalizer.ts
秦奇 and others added 12 commits May 22, 2026 15:26
…(PR-A)

Closes the "12+ daemon events fall through to debug" gap surfaced in the PR
the daemon currently emits (Stage 1 + Wave 3-4), so renderers stop having
to peek at `rawEvent.data` for known event categories.

Session-meta:
- session.metadata.changed (from session_metadata_updated)
- session.approval_mode.changed (from approval_mode_changed)
- session.available_commands (from available_commands_update; upgraded
  from a status-text fallback to a typed event carrying the command list)

Workspace state (Wave 3-4):
- workspace.memory.changed
- workspace.agent.changed
- workspace.tool.toggled
- workspace.initialized
- workspace.mcp.budget_warning
- workspace.mcp.child_refused
- workspace.mcp.server_restarted
- workspace.mcp.server_restart_refused

Auth device-flow (Wave 4 OAuth, RFC 8628):
- auth.device_flow.started
- auth.device_flow.throttled
- auth.device_flow.authorized
- auth.device_flow.failed (carries DaemonAuthDeviceFlowSdkErrorKind)
- auth.device_flow.cancelled

- `DaemonUiErrorEvent.errorKind?: DaemonErrorKind` — closed-enum error
  category propagated from daemon's typed-error taxonomy. Renderers can
  branch on errorKind for "retry auth" vs "check file path" affordances
  instead of regex-matching `text`.
- `DaemonUiToolUpdateEvent.provenance?: DaemonUiToolProvenance` +
  `.serverId?` — closed enum ('builtin' | 'mcp' | 'subagent' | 'unknown').
  Falls back to the `mcp__<server>__<tool>` naming heuristic when the
  daemon doesn't stamp provenance explicitly. Unblocks UI namespace
  dispatch without string-matching toolName.

Session-meta / workspace / auth events do NOT push transcript blocks.
They are intentional sidechannel observations: `lastEventId` advances
(monotonic invariant preserved), but the chat-stream transcript stays
focused on user/assistant/tool/shell/permission content. Renderers
consume them via selectors (introduced in follow-up PRs).

All new event types produce short structured lines in
`daemonUiEventToTerminalText` for tail-style debug consumers. Web/IDE
renderers should consume the typed events directly via subscription.

40/40 tests pass. New tests verify:
- All 16 new event types normalize correctly
- Malformed payloads fall back to debug without leaking raw data
  (`secret` field never appears in fallback text)
- MCP tool provenance heuristic (`mcp__github__create_issue` →
  provenance='mcp', serverId='github')
- errorKind propagation on session_died / stream_error
- Reducer is no-op on new event types; lastEventId still advances

This is PR-A of the unified-renderer-layer follow-up series:
- PR-A (this commit) — event coverage + closed-enum schema
- PR-B — server-side timestamps + ordering refactor
- PR-C — multimodal content + tool preview taxonomy
- PR-D — render contract (toMarkdown / toHtml / toPlainText) + adapter
  conformance test framework
- PR-E — reducer state machine (subagent / progress / current tool /
  cancellation propagation)

See QwenLM#4328 (comment)
for the full proposal.

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Closes the "时间定义不标准" gap surfaced in the PR QwenLM#4328 review:
- Client-side `Date.now()` drifts across clients
- No daemon-authoritative timestamp propagated to UI
- Out-of-order replay events get fresher `state.now` than originals,
  breaking `createdAt` ordering

- `DaemonUiEventBase.serverTimestamp?: number` — daemon-authoritative
  wall-clock timestamp extracted from envelope.
- `DaemonTranscriptBlockBase.serverTimestamp?: number` + `clientReceivedAt: number`.
- `createdAt` preserved as `@deprecated` alias for `clientReceivedAt`
  (backward compat for code written before this PR).

`extractServerTimestamp` looks at three candidate envelope locations:

1. `event.serverTimestamp` (preferred when daemon adds it)
2. `event._meta.serverTimestamp` (Anthropic-style metadata convention)
3. `event.data._meta.serverTimestamp` (sessionUpdate nested location)

The SDK is ready to consume serverTimestamp WHEN daemon emits it, without
requiring a coordinated SDK release. Undefined when daemon doesn't emit
(current state) — graceful degradation to client-clock ordering.

`selectTranscriptBlocksOrderedByEventId(state)` — returns blocks sorted by:

1. `eventId` (daemon-monotonic SSE cursor) — primary key
2. `serverTimestamp` (daemon wall clock) — fallback for synthetic frames
3. `clientReceivedAt` (local clock) — last resort

Use this when displaying long sessions where event id 5 may arrive AFTER
event id 7 (typical in SSE replay-after-reconnect).

`formatBlockTimestamp(block, opts)` — formats the most authoritative
timestamp on a block using `Intl.DateTimeFormat`. Prefers
`serverTimestamp` over `clientReceivedAt` for cross-client consistency.
Accepts locale / timeZone / dateStyle / timeStyle.

Daemon needs to stamp `_meta.serverTimestamp` on every SSE envelope. This
SDK PR is ready to consume it the moment the daemon ships the field; no
coordination needed.

- serverTimestamp extraction from all three envelope locations
- Defaults undefined when envelope has none
- `selectTranscriptBlocksOrderedByEventId` sorts mixed-arrival events by
  eventId (replay scenario)
- `formatBlockTimestamp` prefers serverTimestamp; returns localized string

PR-B of the unified follow-up to PR QwenLM#4328 (PR-A + PR-B + PR-C + PR-D +
PR-E in one branch).

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…de / cancellation propagation (PR-E)

Closes the "reducer state machine 设计缺漏" gap surfaced in the PR QwenLM#4328 review:
- No `currentTool` — UI scans `blocks[]` to find the running tool
- No mirrored approval mode — UI walks events to badge "plan"/"yolo"
- Cancellation does not propagate — in-flight tool blocks stuck at
  'in_progress' forever when the parent prompt is cancelled

## State additions (sidechannel, no transcript blocks)

`DaemonTranscriptSidechannelState`:
- `currentToolCallId?: string` — toolCallId of the in-flight tool
- `approvalMode?: string` — mirrored from session.approval_mode.changed
- `toolProgress: Record<string, { ratio?, step? }>` — per-tool progress
  shape (daemon-side emission of `tool.progress` events pending)

## Reducer behavior

### `tool.update` events

`IN_FLIGHT_TOOL_STATUSES` = { pending, confirming, running, in_progress }
`TERMINAL_TOOL_STATUSES` = { completed, success, failed, error, canceled, cancelled }

- Tool enters in-flight: set `currentToolCallId = event.toolCallId`
- Tool enters terminal: clear `currentToolCallId` if it matches
- Unknown status (forward-compat): leave pointer untouched

This avoids the failure mode where a future daemon-emitted status like
`'paused'` would silently mark unknown states as either in-flight or
terminal incorrectly.

### `session.approval_mode.changed`

Mirror `event.next` onto `state.approvalMode`. Renderers can render a
mode badge ("plan" / "default" / "auto-edit" / "yolo") with a single
selector call, no event-stream walking.

### `assistant.done` with `reason === 'cancelled'`

`propagateCancellationToInFlightTools` walks every tool block whose
status is still in-flight and force-sets it to 'cancelled'. The daemon
does not guarantee terminal `tool_call_update` for every in-flight tool
when the parent prompt is cancelled, so this propagation prevents UI
spinners from spinning forever.

`currentToolCallId` is also cleared in the same call.

Non-cancellation `assistant.done` (e.g., `reason: 'end_turn'`) does NOT
propagate — in-flight tools remain in-flight until the daemon emits
their terminal update naturally.

## Selectors

- `selectCurrentTool(state)` — returns the running tool block, or undefined
- `selectApprovalMode(state)` — returns the mirrored approval mode
- `selectToolProgress(state, toolCallId)` — per-tool progress query

All exported from `@qwen-code/sdk/daemon`.

## Scope deliberately deferred

Subagent nesting (`parentBlockId` / `delegationId` / `DaemonSubagentTranscriptBlock`)
is NOT in this PR. The shape needs design discussion (how to project nested
events; whether to bake delegation tracking into transcript or sidechannel).
PR-D / PR-F follow-up.

## Test coverage (51/51 pass)

- currentToolCallId set on enter, cleared on terminal
- approvalMode mirrors changes
- Cancellation marks in-flight tools 'cancelled', leaves completed alone
- Unknown status does NOT clear currentToolCallId (forward-compat)
- Non-cancellation `assistant.done` does NOT propagate

## Roadmap

PR-E of the unified follow-up to PR QwenLM#4328 (PR-A + PR-B + PR-E in this
branch; PR-C / PR-D pending).

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…ction (PR-C)

Closes two related gaps surfaced in the PR QwenLM#4328 review:
- `DaemonToolPreview` had only 4 kinds — UI fell back to `key_value` /
  `generic` for tools that deserved structured display
- `getTextContent` silently dropped non-text content (image / audio /
  resource), so multimodal conversations vanished from the UI

`DaemonToolPreview` extends from 4 to 8 variants:

- `file_diff` — `{ path, oldText?, newText?, patch? }` — file edit tools
  (Anthropic-style `oldText/newText`, aider-style `patch`, write-style
  `newText` alone)
- `file_read` — `{ path, range?: [start, end] }` — file read tools, with
  range extracted from `lineRange` tuple OR `offset/limit` pair
- `web_fetch` — `{ url, method? }` — HTTP fetch tools (requires URL
  with scheme to avoid false positives on relative paths)
- `mcp_invocation` — `{ serverId, toolName, argsSummary? }` — MCP server
  tool calls, identified via `mcp__<server>__<tool>` naming convention
  (same heuristic as PR-A `DaemonUiToolUpdateEvent.provenance`)

Detector order matters — MCP wins first (most specific), then file_diff,
file_read, web_fetch, then the existing command / key_value fallbacks.

New helper `extractContentPart(value): DaemonUiContentPart | undefined`
returns a discriminated union:

```ts
type DaemonUiContentPart =
  | { kind: 'text'; text: string }
  | { kind: 'image'; mediaType: string; source: { url?, data? } }
  | { kind: 'audio'; mediaType: string; source: { url?, data? } }
  | { kind: 'resource'; uri: string; mediaType?, description? };
```

The existing `getTextContent` is preserved for backward compat. Renderers
that need to surface non-text content (web UI thumbnails, IDE attachment
chips) now have a typed shape to consume.

- Wiring `extractContentPart` into the normalizer / reducer so text
  blocks accumulate `parts: DaemonUiContentPart[]` alongside `text`
  (additive shape change requires render contract coordination — PR-D).
- 5 additional tool preview kinds (image_generation / code_block /
  tabular / subagent_delegation / search) — useful but not urgent;
  current 8 kinds cover the typical agent flows.

- file_diff detection from Anthropic / aider / write shapes
- file_read with lineRange tuple AND offset+limit pair
- web_fetch with method, REJECTS relative paths (no scheme)
- mcp_invocation with serverId + toolName extraction
- Detector priority: MCP wins over file_diff on conflicting shapes
- extractContentPart for text / image (url) / audio (data) / resource
- Unknown content type returns undefined (skip rather than synthesize)
- Image without source returns undefined (defensive)

PR-C of the unified follow-up to PR QwenLM#4328 (PR-A + PR-B + PR-E + PR-C in
this branch; PR-D render contract pending).

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…elpers (PR-D)

Closes the "render 契约只覆盖 terminal" gap surfaced in the PR QwenLM#4328 review:

> PR ships `daemonUiEventToTerminalText` for terminal. Web/IDE/channel
> adapters each roll their own projection. No shared contract → adapter
> divergence is inevitable.

## New helpers

```ts
daemonBlockToMarkdown(block, opts?): string  // GFM-compatible
daemonBlockToHtml(block, opts?): string      // conservatively escaped HTML
daemonBlockToPlainText(block, opts?): string // for copy-paste / logs
daemonToolPreviewToMarkdown(preview, opts?): string
```

All three respect the same `kind` discrimination so adapters can switch
between them without touching call sites.

## Per-kind projection

For each `DaemonTranscriptBlock['kind']`:

- `user` / `assistant` / `thought` — plain text with role labels
- `tool` — header with toolName + structured preview + status badge
- `shell` — fenced code block, stream-discriminated (stdout vs stderr)
- `permission` — title + options list + resolved/pending indicator
- `status` / `debug` / `error` — semantic class / role (error → role=alert)

For each `DaemonToolPreview['kind']`:

- `ask_user_question` — question + options as bullet list
- `command` — fenced bash with optional cwd comment
- `file_diff` — unified diff in fenced code block (oldText/newText OR patch)
- `file_read` — `path (lines N-M)` line
- `web_fetch` — `METHOD url` line
- `mcp_invocation` — `serverId::toolName` with args summary
- `key_value` — bullet list
- `generic` — emphasized summary

## Security

- Default HTML sanitizer escapes `<`, `>`, `&`, `"`, `'` and FIRST strips
  ANSI/control sequences via `sanitizeTerminalText` (defense against
  agent-emitted escape codes in HTML output).
- Custom sanitizer hook for consumers wanting markdown→HTML pipelines
  (markdown-it + DOMPurify, etc.).
- `sanitizeUrls` option strips token-like query params (`token=`, `key=`,
  `x-amz-`, etc.) from URLs in `web_fetch` previews.
- `maxFieldLength` truncation defaults 8192, prevents pathological
  rendering on huge content.

## Adapter conformance (out of scope for this commit)

The conformance test framework (fixture corpus + `runAdapterConformanceSuite`)
mentioned in PR-D scope is deferred to a follow-up. The render helpers
here are the precondition — once stable, the conformance framework can
use them as the reference projection.

## Test coverage (77/77 pass)

- All 9 block kinds render in markdown (verified for user/assistant/tool/
  shell/permission/error specifically)
- file_diff renders as unified diff with old/new lines
- mcp_invocation renders as `server::tool` format
- HTML escapes XSS (`<script>` → `&lt;script&gt;`)
- HTML strips terminal escape sequences before escaping
- Error blocks emit `role="alert"` for screen readers
- plain text drops markdown delimiters
- maxFieldLength truncates with ellipsis
- sanitizeUrls strips token query params
- Custom sanitizer hook works

## Roadmap

PR-D of the unified follow-up to PR QwenLM#4328 — completes the 5-PR series
(A: event coverage, B: time schema, E: state machine, C: tool preview +
content extraction, D: render contract).

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…ete (PR-F)

Closes the "5 additional preview kinds" item in PR QwenLM#4353's TODO §A
(SDK-only work).

## New preview kinds (8 → 13)

- `code_block` — `{ language?, code, origin? }` — REPL / formatter /
  generator output, fenced as `\`\`\`<language>` in markdown
- `search` — `{ query, resultCount?, top? }` — grep / ripgrep / find /
  glob results with up to 5 top hits
- `tabular` — `{ columns, rows, totalRows? }` — structured table output
  (50-row cap with `totalRows` truncation indicator); supports both
  `columns: string[] + rows: unknown[][]` explicit shape and legacy
  `data: Array<Record<>>` shape (auto-infers columns from first row)
- `image_generation` — `{ prompt, thumbnailUrl?, model? }` — dall-e /
  diffusion / imagen / flux / sora style tools
- `subagent_delegation` — `{ agentName, task, parentDelegationId? }` —
  Anthropic-style Task tool and similar sub-agent dispatchers

## Detector priority

Order matters — most specific wins. New detectors slot in between
`mcp_invocation` and `file_diff`:

```
mcp_invocation > subagent_delegation > search > image_generation
  > file_diff > file_read > web_fetch > code_block > tabular
  > command > key_value > generic
```

Rationale: subagent / search / image generation are most discriminable
(distinct toolName patterns); file ops next; code_block / tabular last
because their shapes (`code:`, `columns:`) can appear in other tools.

## Render projections

Both `daemonToolPreviewToMarkdown` and the plain-text rendering paths
extended with cases for all 5 new kinds:

- code_block: fenced markdown code block with language tag
- search: bold header + GFM bullet list of top results
- tabular: GFM pipe table with header / separator / body / truncation hint
- image_generation: bold header + blockquoted prompt + embedded markdown
  image (URL sanitization respected via `sanitizeUrls` opt)
- subagent_delegation: bold delegate-arrow header + blockquoted task +
  optional parent delegation reference

## Test coverage (91/91 pass, +14 new)

- Each detector with positive case
- Detector priority verified: subagent_delegation wins over file_diff
  when toolName='Task' has both subagent + file-edit fields
- Tabular row cap (50) + totalRows stamping for truncated data
- Legacy data: Array<Record<>> auto-column inference
- Each render projection with structural assertions (markdown table
  format, image embed, bullet lists)

## Roadmap

PR-F of the unified follow-up to PR QwenLM#4328. Brings the preview taxonomy
to 13 kinds covering: file ops (3), web (1), code/data (2), media (1),
agent control (2 — ask_user_question + subagent_delegation), MCP (1),
search (1), generic fallbacks (2).

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…PR-G)

Closes the "Adapter conformance test framework" item in PR QwenLM#4353's TODO §A.
Lets any daemon-ui adapter (TUI / web / IDE / channel / mobile) validate
that it projects a fixed corpus of daemon SSE event streams to the same
semantic shape — catches projection drift before it reaches users.

## API surface

```ts
interface DaemonUiAdapterUnderTest {
  reduce(events: readonly DaemonUiEvent[]): unknown;
  renderToText(state: unknown): string;
}

interface DaemonUiConformanceFixture {
  name: string;
  description: string;
  envelopes: DaemonEvent[];           // raw daemon envelopes
  expectedContains: string[];          // phrases the rendered text MUST contain
  expectedAbsent?: string[];           // phrases that MUST NOT appear
  normalizeOptions?: { ... };          // forward-compat normalize opts
}

runAdapterConformanceSuite(adapter, opts?): ConformanceSuiteResult
DAEMON_UI_CONFORMANCE_FIXTURES: ReadonlyArray<DaemonUiConformanceFixture>
```

## Design

**Format-agnostic assertion**: adapters can render to ANSI / HTML /
markdown / JSX — the framework only inspects plain text via
`renderToText`. Catches semantic divergence (missing user message,
wrong tool status, leaked secret) without forcing identical formatting.

**Embedded fixture corpus** (no fs reads — works in browser bundle):
- `simple-chat` — user/assistant streaming flow
- `tool-call-lifecycle` — running → completed transition
- `file-edit-diff` — file_diff preview surfacing
- `mcp-invocation` — MCP serverId/toolName extraction via heuristic
- `permission-lifecycle` — request + resolved with outcome
- `mcp-budget-warning` — Wave 3 event (adapter must observe but rendering
  is its choice)
- `cancellation-propagates` — tool block status flows
- `malformed-payload-redaction` — uses `includeRawEvent: true` to verify
  even a debug-mode adapter doesn't leak `token: secret-do-not-leak`
- `auth-device-flow-success` — Wave 4 OAuth events
- `available-commands-typed-event` — PR-A upgrade from status text

Per-fixture `expectedContains` and `expectedAbsent` describe the
content contract independently of format.

## Suite result

```ts
{
  passed: number,
  failed: ConformanceFailure[],   // each carries missing + leaked + excerpt
  total: number,
}
```

**Does not throw** — caller asserts on `result.failed` so adapter test
suites can produce per-fixture diagnostics rather than a single opaque
exception.

## Filter options

`only` / `skip` allow targeted runs during adapter development:

```ts
runAdapterConformanceSuite(myAdapter, { only: ['simple-chat'] });
runAdapterConformanceSuite(myAdapter, { skip: ['cancellation-propagates'] });
```

## Test coverage (97/97 pass, +6 new)

- SDK reference adapter (reducer + markdown render) passes all fixtures
- SDK reference adapter (reducer + plainText render) also passes
- Buggy adapter (empty string output) fails every fixture with non-empty
  `expectedContains`
- Buggy adapter (raw event dump via JSON.stringify) caught by redaction
  fixture's `expectedAbsent`
- `only` filter narrows to a single fixture
- `skip` filter excludes named fixtures from the corpus

## Usage from adapter authors

```ts
// In your adapter's test file
import { runAdapterConformanceSuite } from '@qwen-code/sdk/daemon';
import { reduceForTui, renderTuiState } from './my-tui-adapter';

it('TUI adapter conforms to daemon UI corpus', () => {
  const result = runAdapterConformanceSuite({
    reduce: reduceForTui,
    renderToText: renderTuiState,
  });
  expect(result.failed).toEqual([]);
});
```

## Roadmap

PR-G of the unified follow-up to PR QwenLM#4328. The corpus is intentionally
small (10 fixtures) but extensible — adapter authors can submit new
fixtures via additions to `DAEMON_UI_CONFORMANCE_FIXTURES` to lock in
regression coverage for edge cases their adapter encountered.

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…act (PR-H)

Closes the "WebUI transcriptAdapter migration" item in PR QwenLM#4353's TODO §A.
Validates the PR-D render contract end-to-end on the real WebUI consumer.

`daemonTranscriptToUnifiedMessages(blocks, options?)` gains a new options
parameter:

```ts
interface DaemonTranscriptAdapterOptions {
  useMarkdown?: boolean;                  // default: false
  enrichToolDetailsWithPreview?: boolean; // default: false
}
```

Defaults preserve legacy behavior — existing callers see no change.

For `user` / `assistant` / `thought` blocks, content is projected via
SDK's `daemonBlockToMarkdown` instead of raw sanitized text. The WebUI's
markdown renderer (markdown-it) then gets:

- `**You**\n\n<content>` for user blocks (bold "You" label)
- Raw text for assistant blocks (markdown formatting in agent output
  passes through cleanly)
- `> *thought:* <text>` blockquote for thought blocks

For `tool` blocks, `rawOutput` is replaced with `daemonToolPreviewToMarkdown(block.preview)`.
This lets WebUI surfaces without per-preview-kind React components still
display:

- `file_diff` as a fenced unified diff
- `mcp_invocation` as `server::tool` with args summary
- `tabular` as GFM pipe table
- `search` as bullet list with match count
- `image_generation` as embedded markdown image
- `subagent_delegation` as delegate arrow + task quote

Renderers with per-kind components should leave this opt-out.

`packages/sdk-typescript/src/daemon/index.ts` was missing exports for
PR-D / PR-F / PR-G / PR-B / PR-E surface — WebUI's `@qwen-code/sdk/daemon`
import path uses the daemon root, not the ui/ sub-index. Added 15+
re-exports so consumers don't need to use the longer
`@qwen-code/sdk/daemon/ui/index.js` path.

Now exported from `@qwen-code/sdk/daemon` root:
- `daemonBlockToMarkdown` / `daemonBlockToHtml` / `daemonBlockToPlainText`
- `daemonToolPreviewToMarkdown`
- `extractContentPart` + `DaemonUiContentPart` type
- `formatBlockTimestamp` + `selectTranscriptBlocksOrderedByEventId`
- `selectCurrentTool` / `selectApprovalMode` / `selectToolProgress`
- `runAdapterConformanceSuite` + `DAEMON_UI_CONFORMANCE_FIXTURES`
- All associated types

`webui/src/daemon/transcriptAdapter.test.ts` mock blocks updated to include
`clientReceivedAt` (required field added in PR-B). Mechanical change —
every `createdAt: N` test fixture gets a matching `clientReceivedAt: N`.

- WebUI `npm run typecheck` — clean
- SDK `npm run typecheck` — clean
- SDK `vitest run test/unit/daemonUi.test.ts` — 97/97 pass
- WebUI transcriptAdapter test fixtures typecheck against updated
  DaemonTranscriptBlockBase schema

PR-H of the unified follow-up to PR QwenLM#4328. Closes the WebUI migration
gap in TODO §A.

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Closes the final "Documentation" item in PR QwenLM#4353's TODO §A. Brings the
unified daemon UI surface to ~95% SDK-side completion.

## Files added

- `docs/developers/daemon-ui/README.md` — full API reference
  - Three-layer model (normalizer → reducer → render helpers)
  - Quick start with idiomatic event-loop pattern
  - Event taxonomy (28+ types categorized: chat-stream / session-meta /
    workspace / auth device-flow)
  - Render contract cookbook (markdown / HTML / plainText)
  - Tool preview taxonomy (13 kinds with use cases)
  - State selectors (currentTool / approvalMode / toolProgress / ordering)
  - Cancellation propagation explanation
  - Time semantics (eventId > serverTimestamp > clientReceivedAt
    precedence)
  - Adapter conformance usage
  - ErrorKind dispatch pattern
  - Tool provenance dispatch pattern
  - Forward-compat principles

- `docs/developers/daemon-ui/MIGRATION.md` — adapter author migration
  cookbook
  - Step-by-step recommended adoption order (9 steps, value-ranked)
  - Before/after code examples for each step
  - Backward-compat checklist (everything is additive — no breaking
    changes)
  - Cross-references to PR-A through PR-H commits

## Roadmap

PR-I of the unified follow-up to PR QwenLM#4328. Documentation-only — no
code changes; no tests affected.

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@chiga0 chiga0 force-pushed the feat/daemon-ui-completeness-followup branch from 90b05f1 to 21152fb Compare May 22, 2026 07:37
Closes the SDK-side gap for §B1 in PR QwenLM#4353's TODO list. PR-E originally
deferred subagent nesting because daemon-side parent-context wasn't yet
stamped on tool_call events. After the rebase onto current
daemon_mode_b_main, source verification confirms the daemon now emits
`tool_call._meta.parentToolCallId` + `tool_call._meta.subagentType` via
`SubAgentTracker.getSubagentMeta()` (core), so the SDK side is unblocked.

## Schema additions (additive, forward-compat-safe)

`DaemonUiToolUpdateEvent`:
  - parentToolCallId?: string  — toolCallId of the parent Task / delegation
  - subagentType?: string      — sub-agent type label (e.g. 'code-reviewer')

`DaemonToolTranscriptBlock`:
  - parentToolCallId?: string  — mirror of event field
  - subagentType?: string      — mirror of event field
  - parentBlockId?: string     — pre-resolved by reducer when parent already
                                 in state, so renderers don't re-correlate

## Normalizer wiring

`normalizeToolUpdate` checks both top-level and `_meta` for parentToolCallId
+ subagentType (fallback chain mirrors how provenance/serverId are read).
Top-level tool calls without sub-agent context omit the fields cleanly.

## Reducer behavior

- New tool block: resolves `parentBlockId` from `toolBlockByCallId` at
  create time. Out-of-order arrival (child before parent) leaves
  `parentBlockId` undefined — selectors fall back to `parentToolCallId`
  lookup.
- Existing tool block update: adopts parent context if not yet
  correlated, never overwrites established correlation (handles the
  flow where SubAgentTracker activates after the initial tool_call).

## New public selectors

- selectSubagentChildBlocks(state, parentToolCallId): returns the
  array of tool blocks invoked inside a given parent delegation
- isSubagentChildBlock(block): type guard for "this tool block came
  from a sub-agent"

Both exported from @qwen-code/sdk/daemon root + ui/index.

## Forward-compat properties

- Top-level tool calls (no sub-agent) work identically as before
- Trimmed parent blocks: child fallback to undefined parentBlockId
- Daemon emits both fields together; SDK reads independently to tolerate
  partial future stamping

## Test coverage (129/129 pass, +5 new tests)

- Extract parentToolCallId + subagentType from `_meta`
- Top-level tool calls have undefined parent fields (forward-compat)
- Reducer correlates parentBlockId at create time
- Reducer adopts parent context on later update (out-of-order arrival)
- isSubagentChildBlock discriminator

## Roadmap

PR-K of the unified follow-up to PR QwenLM#4353. Closes §B1 (subagent nesting)
in the TODO declaration; daemon-side already shipped on
`daemon_mode_b_main` via SubAgentTracker (core).

Remaining TODO §B / §D items still depend on further daemon/Core work:
- §B2 `tool.progress` event type (daemon emit pending)
- §D MessageEmitter multimodal echo + HistoryReplayer inlineData/fileData
  (core change pending)

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@chiga0 chiga0 requested a review from wenshao May 22, 2026 09:04
…ref / docs

Multi-round self-review of PR-K (d8375fe) surfaced two real bugs, a
few defensive gaps, and missing docs/fixture coverage. All addressed
in one commit.

## Bugs fixed

### Bug 1 — `parentBlockId` never back-filled for out-of-order arrival

Original PR-K resolved `parentBlockId` only at child create time, which
broke this flow:

  1. Child arrives WITH parent stamp → block created with
     `parentToolCallId` set, `parentBlockId` undefined (parent not in
     state yet)
  2. Parent arrives later → block created, `toolBlockByCallId` indexed
  3. Subsequent child updates: existing-block branch only ran the
     back-fill inside `!existing.parentToolCallId`, which is false (we
     already adopted the stamp in step 1). `parentBlockId` stayed
     undefined forever.

Fix: separate the two correlations.
  - existing-block update: independently back-fill `parentBlockId`
    whenever `parentToolCallId` is set and `parentBlockId` is missing
  - new-block create: scan existing children whose `parentToolCallId`
    matches the new block's `toolCallId` and back-fill their
    `parentBlockId`. Cheap O(n) over current blocks.

### Bug 2 — dangling `parentBlockId` after trim

`trimTranscriptState` reset `toolBlockByCallId[id]` to the trimmed
sentinel for evicted blocks but did NOT walk surviving children to
null their `parentBlockId` references. Renderers walking
`blockIndexById.get(parentBlockId)` would get undefined, with no
"why" signal.

Fix: post-trim, walk remaining tool blocks; if `parentBlockId`
references an id not in `keptIds`, null it. `parentToolCallId` stays
(survives trimming so selector-keyed queries still work).

## Defensive hardening

- **Self-reference guard** (normalizer): drop
  `parentToolCallId === toolCallId` before it reaches the reducer.
  Daemon should never emit this, but defending costs nothing.
- **Selector docstring**: clarify `selectSubagentChildBlocks` returns
  **direct** children only; document cycle / depth-cap responsibility
  for renderers walking up the chain.
- **Cosmetic**: remove redundant `as DaemonToolTranscriptBlock` cast
  in `isSubagentChildBlock` (TypeScript already narrows after
  `block.kind === 'tool'` on the discriminated union).
- **Alphabetical**: move `isSubagentChildBlock` re-export to correct
  position in both `daemon/index.ts` and `daemon/ui/index.ts`.

## Docs + conformance gaps closed

- `README.md` — new "Sub-agent nesting (PR-K)" section with full
  reducer behavior, out-of-order handling note, recursive walk example,
  cycle-defense note.
- `MIGRATION.md` — new step 8a with before/after for nested rendering.
- `conformance.ts` — new `subagent-nesting` fixture covering parent +
  nested child via `tool_call._meta`. Markdown-safe phrases chosen
  (markdown escapes `-` so titles cannot be substring-matched as-is).

## Test coverage (+5 tests, 134/134 pass)

- Self-reference dropped in normalizer
- Back-fill on out-of-order parent arrival (child first, parent after)
- Back-fill on later child update when parent now exists
- Dangling `parentBlockId` nulled after parent trimmed
- New `subagent-nesting` conformance fixture passes SDK reference adapter

## Side-effect verification

Verified no regressions:
- Cancellation propagation still cancels parent + children together
  (iterates `toolBlockByCallId`, which includes both)
- Render contract unchanged (`daemonBlockToMarkdown` etc. project per
  block, no nested awareness required)
- No serializer to update
- `selectTranscriptBlocksOrderedByEventId` unaffected (parent-agnostic)

Generated with AI

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@wenshao wenshao left a comment

Choose a reason for hiding this comment

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

[Critical] resolvePermissionBlock (transcript.ts:581) does not guard against TRIMMED_PERMISSION_BLOCK_ID. The sibling upsertPermissionBlock (line 544) correctly returns early when existingId === TRIMMED_PERMISSION_BLOCK_ID, but resolvePermissionBlock lacks this check. When maxBlocks trimming evicts a pending permission request, a subsequent permission.resolved event creates a dangling orphan block — wasting a block slot, accelerating further trimming, and silently breaking the trimmed-block contract.

function resolvePermissionBlock(
  state: DaemonTranscriptState,
  event: DaemonUiPermissionResolvedEvent,
): void {
  const existingId = state.permissionBlockByRequestId[event.requestId];
  if (existingId === TRIMMED_PERMISSION_BLOCK_ID) return;

[Suggestion] permissionBlockByRequestId (transcript.ts:693-709) grows unboundedly in long sessions. Unlike toolBlockByCallId which has pruneTrimmedToolIndexes, trimmed permission entries (TRIMMED_PERMISSION_BLOCK_ID) are never pruned. Add a pruneTrimmedPermissionIndexes function analogous to the tool index pruning.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

state: DaemonTranscriptState,
event: Extract<DaemonUiEvent, { type: 'session.state_resync_required' }>,
): void {
state.awaitingResync = true;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Critical] awaitingResync permanently freezes the transcript on same-session reconnect.

When state_resync_required sets awaitingResync = true (line 244), all non-terminal events are blocked. If the SSE connection then drops and reconnects to the same session, store.reset() is NOT called (only on session-id change in DaemonSessionProvider.tsx:155-162). awaitingResync stays true forever — all replayed events from the new SSE stream are silently dropped.

The connection shows "connected" while the transcript is dead. No console error, no thrown exception, no automatic recovery.

Suggested fix: In DaemonSessionProvider.tsx, when reconnecting the same session and awaitingResync is true, call store.reset():

} else if (previousSessionId !== undefined) {
  if (store.getSnapshot().awaitingResync) {
    promptAbortRef.current?.abort();
    promptAbortRef.current = undefined;
    promptBusyRef.current = false;
    store.reset();
  }
  store.dispatch({ type: 'assistant.done', reason: 'reconnected' });
}

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

if (!path) return undefined;
const toolName = opts.toolName ?? getFirstString(input, ['toolName', 'name']);
const looksLikeRead =
toolName !== undefined && /read|view|cat/i.test(toolName);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Critical] detectFileRead regex matches substrings — "concat", "locate", "catalog", "preview" all match.

The regex /read|view|cat/i is not anchored. A tool named concat_files or locate_binary would be misidentified as a file-read tool, producing an incorrect file_read preview with a path instead of falling through to the correct command preview.

Suggested change
toolName !== undefined && /read|view|cat/i.test(toolName);
toolName !== undefined && /\b(?:read|view|cat)\b/i.test(toolName);

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

export function selectSubagentChildBlocks(
state: DaemonTranscriptState,
parentToolCallId: string,
): ReadonlyArray<DaemonToolTranscriptBlock> {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Critical] ESLint @typescript-eslint/array-type: ReadonlyArray<DaemonToolTranscriptBlock> is forbidden for simple types. Use readonly DaemonToolTranscriptBlock[] instead.

Suggested change
): ReadonlyArray<DaemonToolTranscriptBlock> {
readonly DaemonToolTranscriptBlock[],

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

);
const safeTitle = sanitizer(block.title);
const safeStatus = sanitizer(block.status);
return `<div class="daemon-block daemon-tool" data-status="${safeStatus}"><div class="title">${safeTitle}</div><pre>${previewHtml}</pre></div>`;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] daemonBlockToHtml uses the custom sanitizer for HTML attribute context (data-status="${safeStatus}"). DOMPurify and similar HTML-focused sanitizers do not escape " in text-node mode, so a tool status string like ok" onclick="alert(1) from a malicious MCP server would break out of the data-status attribute and inject arbitrary HTML attributes.

The default defaultEscapeHtml escapes " to &quot;, so this only triggers with a custom sanitizer.

Fix: Always use defaultEscapeHtml for attribute-context values regardless of the custom sanitizer:

const attrEscape = defaultEscapeHtml;
// In tool block:
const safeStatus = attrEscape(block.status);
// In shell block:
const safeStream = attrEscape(block.stream ?? 'stdout');

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

}

function escapeTableCell(raw: string, opts: DaemonRenderOptions = {}): string {
return escapeMarkdownText(raw, opts).replace(/\|/g, '\\|');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] escapeTableCell doesn't strip newlines — multi-line cell values break GFM table structure.

GFM table rows are delimited by newlines. A cell value containing \n (e.g., a multi-line error message in tabular tool output) splits the row across lines, producing malformed markdown that renders as a garbled table.

Suggested change
return escapeMarkdownText(raw, opts).replace(/\|/g, '\\|');
return escapeMarkdownText(raw, opts).replace(/\|/g, '\\|').replace(/\n/g, ' ');

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

}
}

function renderToolHeader(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] renderToolHeader doesn't accept DaemonRenderOptions, so maxFieldLength truncation is not applied to tool header fields.

All other 16+ calls to escapeMarkdownText and 11+ calls to inlineCode correctly pass opts, but this function calls them without opts. A tool with a very long title or name renders without truncation, violating the render contract's consistency.

Fix: Add opts parameter and pass it through:

function renderToolHeader(
  block: Extract<DaemonTranscriptBlock, { kind: 'tool' }>,
  opts: DaemonRenderOptions = {},
): string {
  const parts: string[] = [`### ${escapeMarkdownText(block.title, opts)}`];
  if (block.toolName) parts.push(inlineCode(block.toolName, opts));
  if (block.toolKind) parts.push(`(${escapeMarkdownText(block.toolKind, opts)})`);
  return parts.join(' ');
}

And update the call site: const header = renderToolHeader(block, opts);

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

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