From ff0f5d28cd1224023423419e03ac229a6a61610d Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 7 Sep 2026 17:24:49 -0400 Subject: [PATCH 1/2] fix: wait for the reopened SkillsScreen layout to settle before comparing geometry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tail of the `Long Skill Document` story collapsed and reopened the Skill Resource accordion and then compared all four section heights against the pre-collapse sample with an exact `toEqual`. `userEvent.click` resolves once the event is dispatched, ahead of React committing the `openSections` update, remounting the panel's content, and the browser running the layout pass that redistributes height across the flex sections — so the read sampled the panes mid-redistribution, two heights off by 23px in opposite directions and summing to the same total. There is no panel animation involved: the accordion sets `transitionDuration={0}` because a height animation fights its flex sizing, and App.css restores a transition only for the chevron transform. Read the geometry under `waitFor` so it retries until the panes settle. The reopened layout converges on the pre-collapse geometry rather than passing through it, so retrying cannot mask a real regression; a tolerance would have hidden the race instead of removing it, and would still fail on an early enough sample. Closes #2278 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XtdD6rzc1ddT28ZNE9wUjV Signed-off-by: cliffhall --- .../SkillsScreen/SkillsScreen.stories.tsx | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/clients/web/src/components/screens/SkillsScreen/SkillsScreen.stories.tsx b/clients/web/src/components/screens/SkillsScreen/SkillsScreen.stories.tsx index bfe69899..da17cf50 100644 --- a/clients/web/src/components/screens/SkillsScreen/SkillsScreen.stories.tsx +++ b/clients/web/src/components/screens/SkillsScreen/SkillsScreen.stories.tsx @@ -1,7 +1,7 @@ import { useState } from "react"; import type { ComponentProps } from "react"; import type { Meta, StoryObj } from "@storybook/react-vite"; -import { expect, fn, userEvent, within } from "storybook/test"; +import { expect, fn, userEvent, waitFor, within } from "storybook/test"; import type { SkillEntry } from "@inspector/core/mcp/skillsSchemas"; import { SkillsScreen } from "./SkillsScreen"; import type { SkillsUiState } from "./SkillsScreen"; @@ -428,9 +428,22 @@ export const LongSkillDocument: Story = { detailCard.clientHeight + 1, ); - // Collapse then reopen restores the same geometry. + // Collapse then reopen restores the same geometry. There is no panel + // animation to wait out — the accordion sets `transitionDuration={0}` + // because a height animation fights its flex sizing — but the reopen is + // still not settled when the click resolves: `userEvent.click` returns once + // the event is dispatched, ahead of React committing the `openSections` + // update, remounting the panel's content, and the browser running the + // layout pass that redistributes height across the flex sections. Sampling + // once lands mid-redistribution under load and fails intermittently + // (#2278), so read the geometry under `waitFor` and let it retry until the + // panes stop moving. The reopened layout only ever converges ON `before` + // rather than passing through it, so retrying cannot mask a real + // regression. await userEvent.click(viewerControl); await userEvent.click(viewerControl); - await expect(geometry()).toEqual(before); + await waitFor(async () => { + await expect(geometry()).toEqual(before); + }); }, }; From 16a743f89578cadd29b1ca7fe6244b8354dd9463 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 7 Sep 2026 18:08:09 -0400 Subject: [PATCH 2/2] fix: give the Ace-worker story a budget that can outlast a loaded machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `JsonObjectInput > Annotates The Offending Line` waits for Ace's JSON worker to produce gutter annotations. The worker starts out of process and debounces its result, so the story already used an explicit `waitFor` — but it asked for 5000ms while the storybook vitest project set no `testTimeout` at all, leaving it on vitest's 5000ms default. The wait could therefore never win: the test was killed at exactly the moment its budget expired, and on a loaded machine that is what happened, reporting `expected 0 to be greater than 0`. Raise the project's `testTimeout` to 15s and the story's own wait to 10s, so the wait has room to report what it actually saw. Neither loosens an assertion — a story that blows 15s has genuinely failed, and a worker that never starts still fails, just later. Closes #2292 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XtdD6rzc1ddT28ZNE9wUjV Signed-off-by: cliffhall --- .../JsonObjectInput/JsonObjectInput.stories.tsx | 9 ++++++--- clients/web/vite.config.ts | 10 ++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.stories.tsx b/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.stories.tsx index 82a6bcc3..d5565f8e 100644 --- a/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.stories.tsx +++ b/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.stories.tsx @@ -209,9 +209,12 @@ export const AnnotatesTheOffendingLine: Story = { await expect(annotations.length).toBeGreaterThan(0); await expect(annotations.some((a) => a.type === "error")).toBe(true); }, - // The worker is asynchronous and debounced, so this needs a real wait - // rather than a tick. - { timeout: 5000 }, + // The worker is asynchronous, starts out of process and debounces its + // result, so this needs a real wait rather than a tick — and a budget + // that survives a loaded machine, where 5000ms did not (#2292). It also + // has to stay under the storybook project's `testTimeout`, or the test is + // killed before the wait can report what it saw. + { timeout: 10000 }, ); }, }; diff --git a/clients/web/vite.config.ts b/clients/web/vite.config.ts index 65311ba6..97034569 100644 --- a/clients/web/vite.config.ts +++ b/clients/web/vite.config.ts @@ -375,6 +375,16 @@ export default defineConfig(({ command }) => { ], test: { name: "storybook", + // Vitest's default is 5000ms, which is the whole budget a play + // function gets — including work that is genuinely slow rather than + // racy. `JsonObjectInput`'s "Annotates The Offending Line" waits on + // Ace's JSON worker, which starts out of process and debounces its + // result; its own `waitFor` asked for 5000ms and so could never win + // against the per-test ceiling, and it lost on a loaded machine + // (#2292). A larger ceiling does not hide a defect here: a story + // that blows 15s has genuinely failed, and every assertion stays as + // strict as it was. + testTimeout: 15000, browser: { enabled: true, headless: true,