fix(web-shell): close the four deferred #9812 review follow-ups (#11107)

Each of the four suggestions deferred out of #9812 was left with nothing
pinning it, so the behaviour each one protects could regress silently.

Pin the historical sessionStorage key. `clientLifecycle.test.ts` only ever
round-tripped through `SESSION_CLIENT_ID_STORAGE_PREFIX`, so renaming that
constant moved the read and the write together and left every test green,
while a tab that persisted its id under the WebUI-era key lost it across the
migration. Three assertions now spell the key literally, including its
percent-encoded session suffix.

Make the ChatRecord export script testable and cover both paths. It had no
exports and ran `main()` at import, which is why the legacy-JSONL rejection —
a behaviour flip #9812 introduced — had no test. The input gate is now
`selectChatRecords`, the render is `renderHtmlFromObjects(objects, api)` with
the export API passed in, and `main()` runs only as the process entry point.
The CLI behaves as before. The new suite asserts the rejection message
verbatim and that the renderer is never reached, so removing the throw fails
rather than falling through to the neighbouring error.

Restore the substring-trap fixture the retirement deleted, under a live path.
`Shellfish.tsx` guards the keyword being the head of a stem, which is a
different loosening from the `web-shell` compound case already covered.

Replace the deleted `InputForm` in the public hook's docblock with
`ChatEditor`, which declares the three props, naming `ChatPane` and `App` as
the in-tree hosts.

Verification brief for the two vitest-backed items is committed alongside;
item 3 was run here, mutation included, and the mutant survives without the
new fixture.

Refs #11076, #9812.

Co-authored-by: yiliang114 <[email protected]>

.github/scripts/ci/classify-platform-sensitivity.test.mjs

@@ -111,11 +111,19 @@ test('platform-coupled subsystems match on segments, not substrings', () => {
 
   // The substring trap: these contain "shell", "pty", "os" or "platform"
   // inside a longer word and must NOT drag both lanes in.
+  //
+  // `Shellfish.tsx` is the one that pins the segment boundary itself: the
+  // keyword is the HEAD of the stem, not buried mid-word, so a rule that
+  // accepts a keyword prefix followed by anything would classify every
+  // `Shell*.tsx` component as sensitive and summon both expensive lanes on
+  // each change. The case above covers the sibling trap — `web-shell` as a
+  // compound — which a different loosening breaks.
   for (const file of [
     'packages/core/src/utils/cryptic.ts',
     'packages/cli/src/ui/emptyState.ts',
     'packages/core/src/telemetry/uploader.ts',
     'packages/cli/src/services/plateauDetector.ts',
+    'packages/web-shell/client/components/Shellfish.tsx',
   ]) {
     assert.equal(classifyChangedFiles([file]), PLATFORM_INSENSITIVE, file);
   }

docs/verification/11076-webui-retirement-followups/README.md

@@ -0,0 +1,117 @@
+# Verification: #11076 WebUI-retirement follow-ups
+
+Every claim below is **unverified on the authoring machine** except where this
+file says otherwise. That machine cannot run vitest (small RAM; running suites
+there has OOM'd it before), so the two vitest-backed items ship on static
+reasoning plus a direct module probe, and this brief is the handoff.
+
+What was actually run here, and what it proves, is stated per item. Nothing
+else was executed.
+
+## What to run
+
+| #   | Command                                                                                                                        | Covers                                                      |
+| --- | ------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------- |
+| 1   | `npx vitest run client/daemon/session/clientLifecycle.test.ts` (from `packages/web-shell`)                                     | Historical sessionStorage key                               |
+| 2   | `npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/export-html-from-chatrecord-jsonl.test.js` (repo root) | Legacy-JSONL rejection + ChatRecord happy path              |
+| 3   | `node --test --test-concurrency=1 .github/scripts/ci/classify-platform-sensitivity.test.mjs`                                   | Classifier word boundary — **already run here, 12/12 pass** |
+| 4   | —                                                                                                                              | Comment-only; nothing to run                                |
+
+Item 2's suite is part of `npm run test:ci` (via `npm run test:scripts`), and
+item 3's file is in `HELPER_TESTS` in `ci.yml`, so both lanes cover these on
+their own. Item 1 rides the `packages/web-shell` suite.
+
+## Item 1 — historical sessionStorage key
+
+Three assertions added to `clientLifecycle.test.ts`. The point of all three is
+that the key is **spelled as a literal**, never imported: every pre-existing
+test round-trips through `SESSION_CLIENT_ID_STORAGE_PREFIX`, so renaming that
+constant moves the read and the write together and the suite stays green.
+
+Expected: all pass.
+
+**Mutation to prove they bite** — change `clientLifecycle.ts:8` to
+`'qwen-code-web-shell-client-id:session:'` and re-run. Expected: the two
+literal-key tests go red (`writes under the historical WebUI key`, `reads an
+id a WebUI-era tab left under the historical key`); the third
+(`percent-encodes the session id in the key`) goes red too, since it also
+spells the prefix. Every other test in the file stays green — that contrast is
+the whole point, and the review that asked for this measured the same thing
+(15/15 green under the mutant before, 15 pass / 1 fail after).
+
+_Not run here._ The assertion shape is taken verbatim from the review thread's
+own suggested fix, and `encodeURIComponent('work/space:1') === 'work%2Fspace%3A1'`
+was checked by hand, not by running the suite.
+
+## Item 2 — export-html-from-chatrecord-jsonl.js
+
+The script had no exports and ran `main()` at import, so it could not be
+imported by a test at all. Two changes make it testable without changing what
+the CLI does:
+
+- the input gate is now `selectChatRecords(objects)` and the render is
+  `renderHtmlFromObjects(objects, api)`, with `api` passed in — the real one
+  still comes from `loadExportApi()`, which needs built CLI output;
+- `main()` runs only when the file is the process entry point.
+
+**Run here, and it passed:** a standalone `node` script that imports the real
+module and asserts every behaviour the vitest file asserts — legacy rejection
+message verbatim, first-line-only rejection, non-record filtering, the two
+distinct error messages for empty vs unrecognized input, both predicates,
+`readJsonlObjects` blank-line skipping and its invalid-line error, the happy
+path's earliest-timestamp `startTime`, and that the renderer is never reached
+on the legacy path. All passed. **What that does not cover** is the vitest
+file itself — its imports, `vi.fn()` usage, and whether the suite picks the
+file up. That is what run 2 above is for.
+
+Also confirmed here: importing the module executes nothing, and invoking the
+script directly still runs `main()` (it exits 1 on the missing export API,
+which is pre-existing — `loadExportApi()` is awaited before argument parsing,
+so even `--help` needs a build. Left alone; out of scope).
+
+**Mutation to prove the test bites** — delete the `throw` in
+`selectChatRecords`'s `looksLikeExportJsonl` branch. Expected: `rejects legacy
+exported JSONL with the exact message` and `never reaches the renderer for
+legacy exported JSONL` go red. Note the failure is _not_ an absence of an
+error: with the throw gone, a legacy-only input falls through to the filter
+and raises `Unrecognized JSONL format` instead, so the test must be asserting
+the exact message — it is — or the mutant survives.
+
+## Item 3 — classifier word boundary
+
+**Run here in full, including the mutation.** Fixture added:
+`packages/web-shell/client/components/Shellfish.tsx`, expected
+`PLATFORM_INSENSITIVE`.
+
+```
+intact tree + new fixture                    → 12 pass, 0 fail
+mutant (SUBSYSTEM_STEM_HEAD `[-_]` → `[^/]*`) + new fixture   → 11 pass, 1 fail
+mutant + original test file (fixture absent) → 12 pass, 0 fail   ← mutant survived before
+```
+
+The middle and bottom rows are the pair that matters: the fixture is what kills
+the mutant. Nothing else in the file did.
+
+Note this is a _different_ trap from the `packages/web-shell/client/App.tsx`
+case already in the suite. That one guards the compound (`web-shell` as a
+dashed segment); this one guards the keyword being the **head of a stem**
+(`Shellfish`), which a different loosening breaks.
+
+## Item 4 — hook documentation
+
+`InputForm` was deleted with `packages/webui`; the public hook's docblock still
+told integrators to render it. Replaced with `ChatEditor`, which declares the
+three props (`ChatEditor.tsx:271-273`, typed from
+`UseDaemonFollowupSuggestionReturn`), and a note that `ChatPane` and `App` are
+the two in-tree hosts that call the hook and thread them down.
+
+`grep -c InputForm packages/web-shell/client/daemon/useDaemonFollowupSuggestion.ts`
+returns 0. Comment-only.
+
+## What to report back
+
+1. Whether runs 1 and 2 are green, with the failing output if not.
+2. Whether the two mutations above go red as described. If a mutation does
+   **not** go red, that is the important result — say so, because it means the
+   test does not pin what this PR claims it pins.
+3. Anything in the PR description contradicted by what you saw.

integration-tests/concurrent-runner/export-html-from-chatrecord-jsonl.js

@@ -9,6 +9,7 @@ import fs from 'node:fs';
 import fsp from 'node:fs/promises';
 import path from 'node:path';
 import readline from 'node:readline';
+import { fileURLToPath } from 'node:url';
 
 async function loadExportApi() {
   try {
@@ -53,7 +54,7 @@ function parseArgs(argv) {
   return { input: args[0] ?? null, output };
 }
 
-async function readJsonlObjects(inputPath) {
+export async function readJsonlObjects(inputPath) {
   const input =
     inputPath === '-'
       ? process.stdin
@@ -75,7 +76,7 @@ async function readJsonlObjects(inputPath) {
   return objects;
 }
 
-function looksLikeChatRecord(value) {
+export function looksLikeChatRecord(value) {
   return (
     value !== null &&
     typeof value === 'object' &&
@@ -89,7 +90,7 @@ function looksLikeChatRecord(value) {
   );
 }
 
-function looksLikeExportJsonl(objects) {
+export function looksLikeExportJsonl(objects) {
   const first = objects[0];
   return (
     first !== null &&
@@ -132,12 +133,14 @@ function defaultOutPath(inputPath) {
   return path.resolve(directory, `${basename}.html`);
 }
 
-async function main() {
-  const { collectSessionMetadata, toHtml } = await loadExportApi();
-  const { input, output } = parseArgs(process.argv);
-  if (!input) printUsage(1);
-
-  const objects = await readJsonlObjects(input);
+/**
+ * The input gate. Legacy exported JSONL is rejected rather than rendered: it
+ * has already been through a renderer once, so feeding it back in would put
+ * previously-rendered markup on the page without passing the export API's
+ * document allowlist. Source ChatRecords are the only shape that goes through
+ * that allowlist, so they are the only shape accepted.
+ */
+export function selectChatRecords(objects) {
   if (objects.length === 0) throw new Error('Input JSONL is empty.');
 
   if (looksLikeExportJsonl(objects)) {
@@ -151,19 +154,45 @@ async function main() {
       'Unrecognized JSONL format (expected ChatRecord-per-line).',
     );
   }
+  return records;
+}
+
+/**
+ * Render accepted records to HTML. `api` is the `@qwen-code/qwen-code/export`
+ * module, taken as an argument so a caller can supply it — the real one comes
+ * from `loadExportApi()`, which needs built CLI output.
+ */
+export async function renderHtmlFromObjects(objects, api) {
+  const records = selectChatRecords(objects);
   const sessionData = await buildProductSessionData(
     records,
-    collectSessionMetadata,
+    api.collectSessionMetadata,
   );
+  return api.toHtml(sessionData, records);
+}
 
-  const html = toHtml(sessionData, records);
+async function main() {
+  const api = await loadExportApi();
+  const { input, output } = parseArgs(process.argv);
+  if (!input) printUsage(1);
+
+  const objects = await readJsonlObjects(input);
+  const html = await renderHtmlFromObjects(objects, api);
   const outputPath = output ? path.resolve(output) : defaultOutPath(input);
   await fsp.mkdir(path.dirname(outputPath), { recursive: true });
   await fsp.writeFile(outputPath, html, 'utf8');
   console.log(`Wrote HTML export to: ${outputPath}`);
 }
 
-main().catch((error) => {
-  console.error(error instanceof Error ? error.message : String(error));
-  process.exitCode = 1;
-});
+// Only run when invoked as the CLI. Importing this module (the test does)
+// must not execute a render or touch process state.
+const invokedDirectly =
+  typeof process.argv[1] === 'string' &&
+  path.resolve(process.argv[1]) === fileURLToPath(import.meta.url);
+
+if (invokedDirectly) {
+  main().catch((error) => {
+    console.error(error instanceof Error ? error.message : String(error));
+    process.exitCode = 1;
+  });
+}

packages/web-shell/client/daemon/session/clientLifecycle.test.ts

@@ -82,6 +82,45 @@ describe('persistStableClientId', () => {
       window.sessionStorage.getItem('qwen-code-webui-client-id'),
     ).toBeNull();
   });
+
+  // The key below is spelled out instead of imported on purpose. Every other
+  // test here round-trips through `SESSION_CLIENT_ID_STORAGE_PREFIX`, so
+  // renaming that constant moves the read and the write together and leaves
+  // the suite green — while a tab that persisted its id under the historical
+  // WebUI key loses it across the migration, and the daemon then sees a fresh
+  // `X-Qwen-Client-Id` for the same controller. These two assertions are the
+  // only thing that goes red on a rename.
+  it('writes under the historical WebUI key', () => {
+    persistStableClientId('client-a', 'session-a');
+
+    expect(
+      window.sessionStorage.getItem(
+        'qwen-code-webui-client-id:session:session-a',
+      ),
+    ).toBe('client-a');
+  });
+
+  it('reads an id a WebUI-era tab left under the historical key', () => {
+    window.sessionStorage.setItem(
+      'qwen-code-webui-client-id:session:session-a',
+      'legacy-client',
+    );
+
+    expect(getStableClientId(undefined, 'session-a')).toBe('legacy-client');
+    expect(getPersistedClientId('session-a')).toBe('legacy-client');
+  });
+
+  it('percent-encodes the session id in the key', () => {
+    // The suffix is part of the persisted shape too: a session id carrying `/`
+    // or `:` would otherwise collide with the prefix's own separator.
+    persistStableClientId('client-slash', 'work/space:1');
+
+    expect(
+      window.sessionStorage.getItem(
+        'qwen-code-webui-client-id:session:work%2Fspace%3A1',
+      ),
+    ).toBe('client-slash');
+  });
 });
 
 describe('getPersistedClientId', () => {

packages/web-shell/client/daemon/useDaemonFollowupSuggestion.ts

@@ -192,12 +192,13 @@ function createDaemonFollowupController(options: {
 export interface UseDaemonFollowupSuggestionReturn {
   /**
    * Current follow-up suggestion display state — pass directly to
-   * `<InputForm followupState={...} />`. Reflects the controller's
+   * `<ChatEditor followupState={...} />`. Reflects the controller's
    * post-debounce visible state, not the raw daemon push.
    */
   followupState: FollowupState;
   /**
-   * Accept the visible suggestion. Wire to `<InputForm onAcceptFollowup={...} />`.
+   * Accept the visible suggestion. Wire to
+   * `<ChatEditor onAcceptFollowup={...} />`.
    * Calls the underlying controller's accept (which invokes the
    * consumer-provided `onAccept` from options) AND clears the daemon
    * store's `lastFollowupSuggestion` so the same suggestion does not
@@ -208,7 +209,8 @@ export interface UseDaemonFollowupSuggestionReturn {
     options?: { skipOnAccept?: boolean },
   ) => void;
   /**
-   * Dismiss the visible suggestion. Wire to `<InputForm onDismissFollowup={...} />`.
+   * Dismiss the visible suggestion. Wire to
+   * `<ChatEditor onDismissFollowup={...} />`.
    * Same store-clear semantics as `onAcceptFollowup`.
    */
   onDismissFollowup: () => void;
@@ -226,8 +228,11 @@ export interface UseDaemonFollowupSuggestionReturn {
  * Wire the daemon's server-pushed `followup_suggestion` event into the
  * Web Shell's composer. Consumers:
  *
- *   1. Render `<InputForm followupState={...} onAcceptFollowup={...}
+ *   1. Render `<ChatEditor followupState={...} onAcceptFollowup={...}
  *      onDismissFollowup={...} />` with the three values returned here.
+ *      `ChatEditor` declares exactly these three props, typed from this
+ *      interface; `ChatPane` and `App` are the two in-tree hosts that call
+ *      this hook and thread them down.
  *   2. Call `clear()` from the hook just before `actions.sendPrompt(...)`
  *      so the prior turn's ghost-text disappears immediately.
  *

scripts/tests/export-html-from-chatrecord-jsonl.test.js

@@ -0,0 +1,191 @@
+/**
+ * @license
+ * Copyright 2026 Qwen Team
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+
+import {
+  looksLikeChatRecord,
+  looksLikeExportJsonl,
+  readJsonlObjects,
+  renderHtmlFromObjects,
+  selectChatRecords,
+} from '../../integration-tests/concurrent-runner/export-html-from-chatrecord-jsonl.js';
+
+const LEGACY_REJECTION =
+  'Legacy exported JSONL cannot be rendered safely; provide source ChatRecord JSONL.';
+
+function chatRecord(overrides = {}) {
+  return {
+    uuid: 'rec-1',
+    parentUuid: null,
+    sessionId: 'sess-1',
+    timestamp: '2026-01-02T03:04:05.000Z',
+    type: 'user',
+    cwd: '/work',
+    version: '0.1.0',
+    ...overrides,
+  };
+}
+
+function legacyMetadata(overrides = {}) {
+  return {
+    type: 'session_metadata',
+    sessionId: 'sess-1',
+    startTime: '2026-01-02T03:04:05.000Z',
+    ...overrides,
+  };
+}
+
+/** Stand-in for `@qwen-code/qwen-code/export`, which needs built CLI output. */
+function stubExportApi() {
+  return {
+    collectSessionMetadata: vi.fn(async (conversation) => ({
+      sessionId: conversation.sessionId,
+      messageCount: conversation.messages.length,
+    })),
+    toHtml: vi.fn(() => '<html>rendered</html>'),
+  };
+}
+
+const tempDirs = [];
+function writeJsonl(lines) {
+  const dir = mkdtempSync(join(tmpdir(), 'chatrecord-jsonl-'));
+  tempDirs.push(dir);
+  const file = join(dir, 'input.jsonl');
+  writeFileSync(file, lines.join('\n'), 'utf8');
+  return file;
+}
+
+afterEach(() => {
+  while (tempDirs.length > 0) {
+    rmSync(tempDirs.pop(), { recursive: true, force: true });
+  }
+});
+
+describe('selectChatRecords', () => {
+  it('rejects legacy exported JSONL with the exact message', () => {
+    // The behaviour this pins is a deliberate flip: legacy exports used to be
+    // rendered through a separate path. Dropping the throw, or letting
+    // looksLikeExportJsonl stop matching the legacy shape, must not pass
+    // silently — that would put already-rendered markup back on the page
+    // without the export API's document allowlist ever seeing it.
+    expect(() => selectChatRecords([legacyMetadata()])).toThrow(
+      LEGACY_REJECTION,
+    );
+  });
+
+  it('rejects on the first line alone, even with real records behind it', () => {
+    expect(() => selectChatRecords([legacyMetadata(), chatRecord()])).toThrow(
+      LEGACY_REJECTION,
+    );
+  });
+
+  it('accepts ChatRecords and drops entries that are not one', () => {
+    const keep = chatRecord({ uuid: 'rec-keep' });
+    const records = selectChatRecords([keep, { note: 'not a record' }]);
+
+    expect(records).toEqual([keep]);
+  });
+
+  it('reports an empty input distinctly from an unrecognized one', () => {
+    expect(() => selectChatRecords([])).toThrow('Input JSONL is empty.');
+    expect(() => selectChatRecords([{ note: 'nope' }])).toThrow(
+      'Unrecognized JSONL format (expected ChatRecord-per-line).',
+    );
+  });
+});
+
+describe('renderHtmlFromObjects', () => {
+  it('renders the ChatRecord happy path through the export API', async () => {
+    const api = stubExportApi();
+    const records = [
+      chatRecord({ uuid: 'rec-1', timestamp: '2026-01-02T03:04:05.000Z' }),
+      chatRecord({ uuid: 'rec-2', timestamp: '2026-01-02T02:00:00.000Z' }),
+    ];
+
+    const html = await renderHtmlFromObjects(records, api);
+
+    expect(html).toBe('<html>rendered</html>');
+    // startTime is the earliest record, not the first one in file order.
+    expect(api.collectSessionMetadata).toHaveBeenCalledWith(
+      expect.objectContaining({
+        sessionId: 'sess-1',
+        startTime: '2026-01-02T02:00:00.000Z',
+        messages: records,
+      }),
+      expect.anything(),
+    );
+    const [sessionData, passedRecords] = api.toHtml.mock.calls[0];
+    expect(passedRecords).toEqual(records);
+    expect(sessionData).toEqual(
+      expect.objectContaining({
+        sessionId: 'sess-1',
+        messages: [],
+        metadata: { sessionId: 'sess-1', messageCount: 2 },
+      }),
+    );
+  });
+
+  it('never reaches the renderer for legacy exported JSONL', async () => {
+    const api = stubExportApi();
+
+    await expect(
+      renderHtmlFromObjects([legacyMetadata()], api),
+    ).rejects.toThrow(LEGACY_REJECTION);
+    expect(api.collectSessionMetadata).not.toHaveBeenCalled();
+    expect(api.toHtml).not.toHaveBeenCalled();
+  });
+});
+
+describe('looksLikeExportJsonl', () => {
+  it('matches only the legacy metadata header shape', () => {
+    expect(looksLikeExportJsonl([legacyMetadata()])).toBe(true);
+    expect(looksLikeExportJsonl([chatRecord()])).toBe(false);
+    expect(looksLikeExportJsonl([legacyMetadata({ type: 'user' })])).toBe(
+      false,
+    );
+    expect(
+      looksLikeExportJsonl([legacyMetadata({ startTime: undefined })]),
+    ).toBe(false);
+  });
+});
+
+describe('looksLikeChatRecord', () => {
+  it('requires every field the renderer reads', () => {
+    expect(looksLikeChatRecord(chatRecord())).toBe(true);
+    expect(looksLikeChatRecord(null)).toBe(false);
+    // parentUuid may be null, but the key has to be present.
+    const { parentUuid: _dropped, ...withoutParent } = chatRecord();
+    expect(looksLikeChatRecord(withoutParent)).toBe(false);
+    expect(looksLikeChatRecord(chatRecord({ sessionId: 7 }))).toBe(false);
+  });
+});
+
+describe('readJsonlObjects', () => {
+  it('parses one object per line and skips blank lines', async () => {
+    const file = writeJsonl([
+      JSON.stringify(chatRecord({ uuid: 'rec-1' })),
+      '',
+      JSON.stringify(chatRecord({ uuid: 'rec-2' })),
+      '',
+    ]);
+
+    const objects = await readJsonlObjects(file);
+
+    expect(objects.map((o) => o.uuid)).toEqual(['rec-1', 'rec-2']);
+  });
+
+  it('names the offending line when JSON is invalid', async () => {
+    const file = writeJsonl([JSON.stringify(chatRecord()), '{ not json']);
+
+    await expect(readJsonlObjects(file)).rejects.toThrow(
+      /Invalid JSONL line[\s\S]*\{ not json/,
+    );
+  });
+});