From 7d3d1c6adf6adb6d7349ed6f227f5d4071bdb3eb Mon Sep 17 00:00:00 2001 From: modacker Date: Tue, 6 Oct 2026 02:39:47 +0800 Subject: [PATCH 1/2] fix(webui): three crashes the suite could not reach, and a doc that promised a release that does not happen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by reviewing the 23 merged PRs that carry no review at all (14 of them self-submitted and self-merged) and then clicking through the real UI. None of the four below is caught by a test; three of them were found by hand, and one of those three took the whole app down. ## 1 · `attachments` was the one field the message normalizer missed (#17) `normalizeWebuiClientMessage` strips and re-adds ten fields so its `typeof` / `Array.isArray` guards are authoritative. `attachments` was in neither list, so it rode through `...remainder` and `projectMessageAttachments` ran a bare `for...of` over it. Measured on `f56b26f`: attachments={"a":1} -> TypeError: attachments is not iterable attachments=42 -> TypeError: attachments is not iterable attachments=true -> TypeError: attachments is not iterable #17's title claims the malformed-field crash class is gone. It was gone for ten fields and present in the eleventh. ## 2 · `listing?.entries.length` — the `?.` stopped one level too early `WebuiWorkspaceDirectoryBrowser` read `listing?.entries.length`. The `?.` guarded `listing`; `.entries` was bare. A body with no `entries` threw `Cannot read properties of undefined (reading 'length')`. The `?.` looked correct, which is why the render read as safe. The real fix belongs at the door: `browseWorkspaceDirs`'s result is now normalised before it reaches state, so a bad body degrades to an empty listing instead of an error page. ## 3 · The sign-in panel was stored with no validation at all `getSigninPanel()`'s resolved value went straight into `setSignin({ panel })`, and `SigninProgress` does `[...days].sort(...)` unguarded. A body without `days` threw `days is not iterable`. Here the server does validate — `validateSigninPanel` throws `Invalid sign-in panel` — so this is defence at the boundary, not a known live shape. It is worth fixing anyway: the blast radius is the entire UI, and a malformed panel should cost you the seven-day strip, not the page. Both crashes above share one root: the transport resolves the server body verbatim (`resolve(frame.body as T)` in `transport.ts`), so every consumer inherits the server's shape with nothing in between. The three fixes above normalise at each consumer rather than at the transport, because only the consumer knows what a missing field should degrade to. ## 4 · The forced reconnect never reported the channel down (#26) `reconnectWhenAvailable` — tab back into view, network back — cleared `socket` and then called `previous.close()`. The close handler guards on `socket !== ws`, which is therefore always true on that path, so `markWebuiEventWatcherDown` was skipped and the banner read 已连接 for the whole reconnect window: the exact silent lie the watcher exists to remove. The path had no test at all — this file had zero `dispatchEvent` calls. ## 5 · The npm preview package has not been publishing, and the docs said it was `gh secret list` is empty — the repository has no Actions secret named `NPM_TOKEN` — so `HAS_NPM_TOKEN` in `webui-npm-preview.yml` is `false` and the publish step never runs. `docs/releasing-webui-npm.md` still promised 「Publishing is fully automatic: every push to the `webui` branch … publishes a new preview version」. The job is also not a merge gate (branch protection is 404), and #15 relaxed the step from failing to a `::warning::`, so "no credential" and "published" are the same colour. The doc now says what is true and what turning it back on involves. ## 6 · The browser fixture could not open three surfaces `fixture.mjs` answered every unknown operation with `return {}`. Three real user paths therefore crashed instead of rendering, which is why 93 specs passed while a human could not get through the app: - `getSigninPanel` — no case at all, so the sign-in row threw on every open - `browseWorkspaceDirs` — no case, so the workspace picker threw on every open - `setPermissionMode` — a no-op write, while `getPermissionMode` always answered `"default"`. `SessionComposer.changePermissionMode` writes and then reads back to verify; with this pair every mode but `default` raised 「授权模式未能保存，请重试」. That read-back is the correct pattern and is left alone — the fixture was the thing that could not satisfy it. ## Tests - `message-parts.test.ts`: a non-array `attachments` no longer throws, and a real attachments array still renders - `connection-health.test.ts`: a forced reconnect now marks the channel down, driven through the `online` listener Verified locally on `f56b26f` + this branch: `typecheck:webui` (server / client / test / root), `test:webui`, `test:capabilities`, `test:status-contract`, and the 93-spec Playwright suite. The three surfaces that used to crash were also clicked through by hand in a real browser. --- docs/releasing-webui-npm.md | 18 +++- .../src/client/components/SessionComposer.tsx | 19 +++- .../webui/src/client/components/UserMenu.tsx | 38 +++++++- .../client/projection/message-projection.ts | 10 ++ packages/webui/src/client/transport.ts | 8 ++ .../webui/test/unit/connection-health.test.ts | 34 +++++++ .../webui/test/unit/message-parts.test.ts | 22 +++++ test/webui-browser/fixture.mjs | 43 ++++++++- .../webui-browser/recovered-surfaces.spec.mjs | 95 +++++++++++++++++++ 9 files changed, 279 insertions(+), 8 deletions(-) create mode 100644 test/webui-browser/recovered-surfaces.spec.mjs diff --git a/docs/releasing-webui-npm.md b/docs/releasing-webui-npm.md index aa267d11e..7f8c9ed56 100644 --- a/docs/releasing-webui-npm.md +++ b/docs/releasing-webui-npm.md @@ -1,9 +1,21 @@ # WebUI package on the npm registry The WebUI is packaged as `@fectivnfy112358/minimax-code-web` and published to -`https://registry.npmjs.org` under the `preview` dist-tag. Publishing is fully -automatic: every push to the `webui` branch builds the standalone package and -publishes a new preview version. No tag, no version edit, no manual step. +`https://registry.npmjs.org` under the `preview` dist-tag. + +> **Preview publishing is currently OFF, and this section used to say +> otherwise.** As of 2026-10-05 the repository has **no Actions secret named +> `NPM_TOKEN`**, so `HAS_NPM_TOKEN` in `webui-npm-preview.yml` evaluates to +> `false` and **the publish step never runs**. The job still goes green — the +> steps emit `::warning::` instead of failing, so "no credential" and +> "published" are the same colour. Verified with `gh secret list` (empty) and +> `gh api .../branches/webui/protection` (404, so this job was never a merge +> gate either). +> +> To turn it back on: add the `NPM_TOKEN` secret, and nothing in the workflow +> needs to change. Be aware the first run with a real credential is a **cold +> start** — version resolution and `npm publish` have not been exercised since +> the gate was relaxed. ## How a release happens diff --git a/packages/webui/src/client/components/SessionComposer.tsx b/packages/webui/src/client/components/SessionComposer.tsx index a8167ef3e..cc00956a1 100644 --- a/packages/webui/src/client/components/SessionComposer.tsx +++ b/packages/webui/src/client/components/SessionComposer.tsx @@ -341,8 +341,23 @@ export function WebuiWorkspaceDirectoryBrowser({ setError(undefined); void browseWorkspaceDirs(dir ? { dir } : {}) .then((next) => { - setListing(next); - setManualPath(next.dir); + // The transport resolves the server body verbatim + // (`resolve(frame.body as T)`), so a body that is missing `entries` + // — or is not a listing at all — used to reach the render, where + // `listing?.entries.length` threw + // `Cannot read properties of undefined (reading 'length')` and the + // whole app tree fell into the error boundary. The `?.` stopped one + // level too early: it guarded `listing` and left `entries` bare. + // Normalise at the door instead of at the use site. + const raw = next as Partial | null; + const normalized: WebuiWorkspaceDirectoryListing = { + dir: typeof raw?.dir === "string" ? raw.dir : "", + parent: typeof raw?.parent === "string" ? raw.parent : undefined, + entries: Array.isArray(raw?.entries) ? raw.entries : [], + truncated: raw?.truncated === true, + }; + setListing(normalized); + setManualPath(normalized.dir); }) .catch((reason: unknown) => { setError( diff --git a/packages/webui/src/client/components/UserMenu.tsx b/packages/webui/src/client/components/UserMenu.tsx index 54b4807b6..a3e845cf1 100644 --- a/packages/webui/src/client/components/UserMenu.tsx +++ b/packages/webui/src/client/components/UserMenu.tsx @@ -261,6 +261,35 @@ function SigninSkeleton(): ReactElement { ); } +/** + * Coerce a sign-in panel off the wire before it reaches state. + * + * The transport resolves the server body verbatim (`resolve(frame.body as T)` + * in `transport.ts`), and `SigninProgress` below does `[...days].sort(...)` + * with no guard. A body that is not a panel — `{}`, a server error envelope, + * anything without `days` — therefore threw `days is not iterable` and took + * the entire app tree into the error boundary. + * + * The server does validate (`validateSigninPanel` throws + * `Invalid sign-in panel`), so this is defence at the boundary rather than a + * known live shape. It is here because the blast radius is the whole UI, not + * one row: a malformed panel must degrade to an empty seven-day strip, never + * to a blank page. + */ +function normalizeSigninPanel(value: unknown): WebuiSigninPanelView { + const raw = value as Partial | null; + const days = Array.isArray(raw?.days) + ? raw.days.filter((day): day is WebuiSigninPanelView["days"][number] => + Boolean(day) && typeof day === "object", + ) + : []; + return { + scene: + typeof raw?.scene === "number" ? (raw.scene as WebuiSigninPanelView["scene"]) : 0, + days, + }; +} + /** Seven-day progress row (desktop `eY`). */ function SigninProgress({ days, @@ -830,7 +859,12 @@ export function UserMenu({ void getSigninPanel() .then((panel) => { // Keep an in-flight claim flag: a silent refresh must not clear it. - setSignin((state) => ({ loading: false, claiming: state.claiming, panel, error: undefined })); + setSignin((state) => ({ + loading: false, + claiming: state.claiming, + panel: normalizeSigninPanel(panel), + error: undefined, + })); }) .catch((error: unknown) => { setSignin((state) => ({ ...state, loading: false, error: formatSigninError(error) })); @@ -845,7 +879,7 @@ export function UserMenu({ setAnimatedDay( result.claim_result === SigninClaimResult.Claimed ? result.day_no : null, ); - setSignin({ loading: false, claiming: false, panel: result.panel }); + setSignin({ loading: false, claiming: false, panel: normalizeSigninPanel(result.panel) }); }) .catch((error: unknown) => { setSignin((state) => ({ ...state, claiming: false, error: formatSigninError(error) })); diff --git a/packages/webui/src/client/projection/message-projection.ts b/packages/webui/src/client/projection/message-projection.ts index bd964460c..dce84410d 100644 --- a/packages/webui/src/client/projection/message-projection.ts +++ b/packages/webui/src/client/projection/message-projection.ts @@ -386,6 +386,13 @@ function normalizeWebuiClientMessage( kind: _incomingKind, timestamp: _incomingTimestamp, contextUsage: _incomingContextUsage, + // `attachments` rides through `...remainder` untouched without this, and + // `projectMessageAttachments` iterates it with a bare `for...of` — so + // `attachments: {}` / `42` / `true` on the wire threw + // `TypeError: attachments is not iterable` and took the whole transcript + // render with it. Same class as the fields above; it was just missing from + // the list, so the guard was not authoritative in both directions. + attachments: _incomingAttachments, ...remainder } = message; return { @@ -395,6 +402,9 @@ function normalizeWebuiClientMessage( ...(typeof thinkingDurationMs === "number" ? { thinkingDurationMs } : {}), ...(Array.isArray(toolCalls) ? { toolCalls } : {}), ...(Array.isArray(parts) ? { parts: parts as Record[] } : {}), + ...(Array.isArray(read("attachments")) + ? { attachments: read("attachments") as WebuiMessageAttachment[] } + : {}), ...(typeof read("role") === "string" ? { role: read("role") as string } : {}), ...(typeof read("source") === "string" ? { source: read("source") as string } : {}), ...(typeof read("kind") === "string" ? { kind: read("kind") as string } : {}), diff --git a/packages/webui/src/client/transport.ts b/packages/webui/src/client/transport.ts index 87aa9e9d0..094fff003 100644 --- a/packages/webui/src/client/transport.ts +++ b/packages/webui/src/client/transport.ts @@ -329,6 +329,14 @@ export function createWebuiTransport({ if (reconnectTimer !== undefined) clearTimeout(reconnectTimer); reconnectTimer = undefined; const previous = socket; + // The close handler below guards on `socket !== ws`, and this function + // has already cleared `socket`, so that guard would have skipped + // `markWebuiEventWatcherDown` entirely. A forced reconnect — tab coming + // back into view, network coming back — therefore reported the channel + // as healthy for the whole reconnect window, which is exactly the silent + // "connected" lie this watcher exists to remove. Mark it down here, on + // the path that is closing the socket without going through the guard. + if (previous) markWebuiEventWatcherDown(watcherToken); socket = undefined; previous?.close(); connect(); diff --git a/packages/webui/test/unit/connection-health.test.ts b/packages/webui/test/unit/connection-health.test.ts index 6d5c5b9af..5ff50f5e8 100644 --- a/packages/webui/test/unit/connection-health.test.ts +++ b/packages/webui/test/unit/connection-health.test.ts @@ -197,4 +197,38 @@ describe("the transport's watcher reporting", () => { // though its socket never came back before the unsubscribe. expect(isWebuiEventChannelDegraded()).toBe(false); }); + it("marks the channel down on a FORCED reconnect, not only on a guarded close", () => { + // The defect: `reconnectWhenAvailable` (tab back into view, network back) + // cleared `socket` and then called `previous.close()`. The close handler + // guards on `socket !== ws`, which is therefore always true on that path, + // so `markWebuiEventWatcherDown` was skipped and the banner read 已连接 + // for the whole reconnect window — the exact lie this watcher exists to + // remove. Zero tests reached it: this file had no `dispatchEvent` at all. + const online = new Map void>(); + const globals = globalThis as unknown as Record; + const saved = { document: globals.document, window: globals.window }; + globals.document = { + visibilityState: "visible", + addEventListener: () => undefined, + removeEventListener: () => undefined, + }; + globals.window = { + addEventListener: (type: string, fn: () => void) => online.set(type, fn), + removeEventListener: (type: string) => online.delete(type), + }; + try { + const { transport, sockets } = watchEventsHarness(); + const unsubscribe = transport.watchEvents(() => undefined); + accept(sockets[0]!); + expect(isWebuiEventChannelDegraded()).toBe(false); + + online.get("online")?.(); + + expect(isWebuiEventChannelDegraded()).toBe(true); + unsubscribe(); + } finally { + globals.document = saved.document; + globals.window = saved.window; + } + }); }); diff --git a/packages/webui/test/unit/message-parts.test.ts b/packages/webui/test/unit/message-parts.test.ts index 07df71fe3..dc9d6b4f7 100644 --- a/packages/webui/test/unit/message-parts.test.ts +++ b/packages/webui/test/unit/message-parts.test.ts @@ -399,4 +399,26 @@ describe("wire message normalization drops a non-string content field", () => { msgContent: JSON.stringify({ eventType: "todo_updated", todos: [] }), })).toEqual([]); }); + it("does not throw when `attachments` is not an array on the wire", () => { + // `attachments` was the one field the normalizer guarded everywhere + // EXCEPT: it is not in the strip list and not in the conditional spreads, + // so it rode through `...remainder` and `projectMessageAttachments` ran a + // bare `for...of` over it. Measured on f56b26f: + // attachments={"a":1} -> TypeError: attachments is not iterable + for (const attachments of [{ a: 1 }, 42, true, "str"] as unknown[]) { + expect(() => + projectWebuiMessage({ msgId: "bad-attachments", role: "assistant", msgContent: "ok", attachments } as never), + ).not.toThrow(); + } + }); + + it("keeps a real attachments array working", () => { + const items = projectWebuiMessage({ + msgId: "ok-attachments", + role: "user", + msgContent: "see this", + attachments: [{ id: "a1", type: "image", file_name: "shot.png" }], + } as never); + expect(JSON.stringify(items)).toContain("shot.png"); + }); }); diff --git a/test/webui-browser/fixture.mjs b/test/webui-browser/fixture.mjs index b9616f509..f77382837 100644 --- a/test/webui-browser/fixture.mjs +++ b/test/webui-browser/fixture.mjs @@ -83,6 +83,32 @@ export function installFixtureTransport() { // block (`{}` — the default — has no signedIn field and reads as signed // in but quota-less, not as signed out). let usageQuotaResult = {}; + // The composer's permission-mode write is verified by reading it back + // (`setPermissionMode` then `getPermissionMode`, compared in + // `SessionComposer.changePermissionMode`). With the old pair — a + // no-op write and a `getPermissionMode` that always answered "default" — + // every mode other than `default` raised 「授权模式未能保存，请重试」, so the + // read-back could never pass. Store it, so the verification can pass. + let permissionMode = "default"; + // A seven-day sign-in panel, shaped exactly like the server's + // `validateSigninPanel` output. The empty default has no `days`, and + // `SigninProgress` does `[...days]` with no guard, so a fixture that answered + // `{}` crashed the whole app the first time a human opened the sign-in row. + // Day 3 is today and claimable; days 1-2 are behind us and claimed; 4-7 are + // still ahead. `SigninDayStatus`: 1 Upcoming, 2 Claimable, 3 Claimed. + const signinPanel = { + scene: 2, + days: Array.from({ length: 7 }, (_, index) => { + const dayNo = index + 1; + return { + day_no: dayNo, + points: 10, + bonus_points: dayNo === 3 ? 400 : 0, + is_today: dayNo === 3, + status: dayNo < 3 ? 3 : dayNo === 3 ? 2 : 1, + }; + }), + }; let activeTurn; const requests = []; const sockets = new Set(); @@ -129,7 +155,22 @@ export function installFixtureTransport() { if (operation === "listModels" || operation === "loadProjects") return []; if (operation === "getUsageQuota") return clone(usageQuotaResult); if (operation === "isGoalEnabled") return false; - if (operation === "getPermissionMode") return { mode: "default" }; + if (operation === "getPermissionMode") return { mode: permissionMode }; + if (operation === "setPermissionMode") { + if (typeof body?.mode === "string") permissionMode = body.mode; + return { mode: permissionMode }; + } + // A directory listing with an EMPTY `entries` array, not the `{}` default: + // `WebuiWorkspaceDirectoryBrowser` reads `listing?.entries.length` and the + // missing array threw `Cannot read properties of undefined (reading + // 'length')` on every open of the workspace picker. + if (operation === "browseWorkspaceDirs") + return { + dir: typeof body?.dir === "string" ? body.dir : "", + entries: [], + truncated: false, + }; + if (operation === "getSigninPanel") return clone(signinPanel); if (operation === "getSessionUsage") return {}; return {}; }; diff --git a/test/webui-browser/recovered-surfaces.spec.mjs b/test/webui-browser/recovered-surfaces.spec.mjs new file mode 100644 index 000000000..3cedd65ff --- /dev/null +++ b/test/webui-browser/recovered-surfaces.spec.mjs @@ -0,0 +1,95 @@ +// Three surfaces that 93 specs could not reach, and that a human could not get +// past either. +// +// The shared root is one line of the transport: `resolve(frame.body as T)`. +// The server's body reaches the component verbatim, so every consumer +// inherits the server's shape with nothing in between. The browser fixture +// answered every unknown operation with `return {}`, and each of these three +// consumers read a nested field off that `{}` without a guard: +// +// sign-in UserMenu -> setSignin({ panel }) -> SigninProgress `[...days]` +// -> `days is not iterable` +// workspace SessionComposer -> setListing(next) -> `listing?.entries.length` +// -> `Cannot read properties of undefined (reading 'length')` +// permission setPermissionMode was a no-op while getPermissionMode always +// answered "default", so the write-then-read-back check in +// `changePermissionMode` could never pass and every mode but +// `default` raised 「授权模式未能保存，请重试」 +// +// Each one is an error BOUNDARY trip: the whole app tree is replaced, not one +// row. So the assertion that matters is the negative — the boundary must stay +// out of the page — plus the positive that the surface actually rendered. + +import { expect } from "@playwright/test"; + +import { openApp, test } from "./harness.mjs"; + +const boundary = (page) => page.locator('[data-testid="webui-error-boundary"]'); + +async function openUserMenu(page) { + await page.locator('[data-testid="sidebar-user-menu-trigger"]').click(); + await expect(page.locator(".webui-user-menu-uid")).toBeVisible(); +} + +test("the sign-in row renders seven days instead of tripping the error boundary", async ({ page }) => { + const pageErrors = []; + page.on("pageerror", (error) => pageErrors.push(error.message)); + + await openApp(page, "#session=A"); + await openUserMenu(page); + + // Hovering the row is what loads the panel (`onMouseEnter` -> loadSignin), + // so the request is what a real pointer produces. + await page.locator(".webui-user-menu-signin-anchor").hover(); + + await expect(page.locator('[data-testid="signin-card"]')).toBeVisible(); + await expect(page.locator('[data-testid="signin-day-1"]')).toBeVisible(); + await expect(page.locator('[data-testid="signin-day-7"]')).toBeVisible(); + // No error card, and no card-level error panel either: a degraded panel is + // allowed, a crashed one is not. + await expect(boundary(page)).toHaveCount(0); + await expect(page.locator('[data-testid="signin-card-error"]')).toHaveCount(0); + expect(pageErrors).toEqual([]); +}); + +test("the workspace picker opens 选择新项目 without tripping the error boundary", async ({ page }) => { + const pageErrors = []; + page.on("pageerror", (error) => pageErrors.push(error.message)); + + // The project row renders only on the home surface — the bar is inside + // `!sessionLayout` (SessionComposer.tsx). Opening `#session=A` puts the + // composer in a session, where there is no workspace bar to click at all. + await openApp(page, ""); + await page.locator(".webui-workspace-bar-trigger").click(); + await page.getByRole("option", { name: "选择新项目" }).click(); + + // The directory browser reads `listing?.entries.length`; with the fixture + // answering `{}` this used to throw before a single entry rendered. + const browser = page.locator('[data-webui-workspace-browser="true"]'); + await expect(browser).toBeVisible(); + await expect(page.locator(".webui-composer-menu-empty")).toBeVisible(); + await expect(boundary(page)).toHaveCount(0); + expect(pageErrors).toEqual([]); +}); + +test("choosing a permission mode survives the write-then-read-back check", async ({ page }) => { + const pageErrors = []; + page.on("pageerror", (error) => pageErrors.push(error.message)); + + await openApp(page, "#session=A"); + + const trigger = page.locator('[data-testid="composer-permission-mode"]'); + await trigger.click(); + // The options are plain buttons with no `role="menuitem"`, so they are + // located by their own class rather than by ARIA role. + await page.locator(".webui-composer-permission-option", { hasText: "帮我批准" }).click(); + + // The verification itself is the subject: `changePermissionMode` writes, then + // reads back and throws when the two disagree. It is the right pattern — the + // fixture was the thing that could not satisfy it — so the assertion is that + // the error message never appears, not that the check was removed. + await expect(page.getByText("授权模式未能保存")).toHaveCount(0); + await expect(trigger).toContainText("帮我批准"); + await expect(boundary(page)).toHaveCount(0); + expect(pageErrors).toEqual([]); +}); From 3f621387aa1ef82d3f8c53ac830f42e34e754692 Mon Sep 17 00:00:00 2001 From: modacker Date: Tue, 6 Oct 2026 03:00:20 +0800 Subject: [PATCH 2/2] chore(release): record recovered-surfaces.spec.mjs in the source inventory `check:source` fails on an unregistered path, which is what turned the three platform failures on #30 red: `Added: test/webui-browser/recovered-surfaces.spec.mjs`. The verification job is only the gate that requires the matrix, so it went red with them. One line, alphabetical, next to the other browser specs. --- release/public-source.json | 1 + 1 file changed, 1 insertion(+) diff --git a/release/public-source.json b/release/public-source.json index afac3397a..e00d878c8 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -3848,6 +3848,7 @@ "test/webui-browser/rail-pin.spec.mjs", "test/webui-browser/rail-search.spec.mjs", "test/webui-browser/rail-star.spec.mjs", + "test/webui-browser/recovered-surfaces.spec.mjs", "test/webui-browser/server.mjs", "test/webui-browser/settings-account-tab.spec.mjs", "test/webui-browser/transcript.spec.mjs",