diff --git a/.claude/skills/testing/SKILL.md b/.claude/skills/testing/SKILL.md index 840ee9ac..85c8caa5 100644 --- a/.claude/skills/testing/SKILL.md +++ b/.claude/skills/testing/SKILL.md @@ -171,10 +171,20 @@ Scope notes: only exclusion. `commander` uses `.exitOverride()` so a parse error throws instead of tearing down the test worker. - **TUI** covers **all of `src/**`, React surface included**. Components mount - through `ink-testing-library` with the passthrough doubles in - `__tests__/helpers/`; keypresses are driven through stdin. The only exclusion - is `src/tui-servers.ts` (a pure re-export, excluded so it doesn't surface as a + through `__tests__/helpers/renderTui.tsx` — `ink-testing-library`'s `render` + with every frame ANSI-stripped — alongside the passthrough doubles in the same + directory; keypresses are driven through stdin. The only exclusion is + `src/tui-servers.ts` (a pure re-export, excluded so it doesn't surface as a misleading 0/0 row). + ⚠️ **Import `render` from that helper, not from `ink-testing-library`.** Ink + writes styling *inside* the styled run, so `Info` + reaches the frame buffer with escapes between `I` and `nfo` and a plain + `toContain("Info")` fails against a component that is rendering correctly. It + only shows up where chalk emits color — a developer whose shell exports + `FORCE_COLOR` — so CI, which has no TTY, stays green on a suite that is red + for them (#2207). If a frame assertion fails on a string you can plainly see + in the printed diff, that is the tell. Reach `stdout.lastFrame()` on the + returned instance for the raw bytes. ### When a `v8 ignore` is justified diff --git a/AGENTS.md b/AGENTS.md index a416da3f..a42c9669 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -386,6 +386,7 @@ diagnose a failing gate — is the `testing` skill. These are the rules. - **`clients/web`**: `.test.tsx` **next to the source** — components, hooks, `lib/`, `utils/`. A web-owned test living under `src/test/` instead is a bug. `src/test/` is for the three things that cannot be co-located: tests of the repo-root **`core/`** package (`src/test/core/…`, mirroring the `core/` layout — it lives outside `clients/web/` and has no harness of its own); the **`integration`** project (`src/test/integration/…` — _placement is the manifest_, picked up by a folder glob, with no enumeration to keep in sync); and **shared test infrastructure** (`renderWithMantine.tsx`, `setup.ts`, `fixtures/`). - **`clients/cli`, `clients/tui`, `clients/launcher`**: **all** tests in a top-level **`__tests__/`**, not beside their source. Their `tsconfig.json` excludes `**/*.test.*`, so a co-located test lands in **no** tsconfig project and fails `npm run verify:typecheck-coverage`. - **Root tooling**: a `scripts/*.mjs` helper with pure logic gets a sibling `*.test.mjs`. Keep that exact filename — `node --test` silently _skips_ a file its glob misses and still exits 0. +- **Render Ink components through the TUI's own `render`** (`clients/tui/__tests__/helpers/renderTui.tsx`), never `ink-testing-library`'s directly. It is the same function with every frame ANSI-stripped, which is what keeps an assertion on styled text from depending on the ambient environment: Ink writes styling *inside* the styled run, so `Info` reaches the frame buffer with escapes between `I` and `nfo` and `toContain("Info")` fails. It only bites where chalk emits color — a developer whose shell exports `FORCE_COLOR` — so CI is green on a suite that is broken for them (#2207). A test that genuinely needs the raw bytes reads `stdout.lastFrame()` off the returned instance. - **Render React components through `renderWithMantine`** (`src/test/renderWithMantine.tsx`); do not hand-roll a bare `MantineProvider`, which skips the project theme and the helper's options and drifts from every other test. Pass the `colorScheme` option to exercise a forced scheme rather than hand-rolling `defaultColorScheme`. Use `renderWithMantineTransitions` **only** when a test must assert mid-flight transition state, and read the long comment on the helper before changing anything about it. - **The web coverage `include` is a whitelist.** It names `components`/`hooks`/`theme`/`lib`/`utils`/`server` plus the browser-consumed `core/*` runtime, so a module placed **outside** those directories falls out of the gate entirely, silently. Place new modules inside a gated directory. The documented exceptions — `src/App.tsx` and the `src/main.tsx` / `src/index.ts` bootstraps — are called out in a comment on the `include` array itself. diff --git a/clients/tui/README.md b/clients/tui/README.md index bab524ef..ee7da505 100644 --- a/clients/tui/README.md +++ b/clients/tui/README.md @@ -103,8 +103,10 @@ stricter react-hooks@7 rules are not enforced on the interim component surface (#1501). Tests live in `__tests__/`. The coverage gate covers **all of `src/**`**, React -surface included — the Ink components mount through `ink-testing-library` (with -the `ink-scroll-view` / `ink-form` passthrough doubles in `__tests__/helpers/`), +surface included — the Ink components mount through +`__tests__/helpers/renderTui.tsx`, `ink-testing-library`'s `render` wrapped so +every frame it hands back is ANSI-stripped (with the `ink-scroll-view` / +`ink-form` passthrough doubles in the same directory), `App.tsx` mounts against a mock of the `@inspector/core` surface, and keypresses are driven through stdin. The former interim exclusion of the components and `App.tsx` was lifted in #1501; the only exclusion left in `vitest.config.ts` is @@ -113,6 +115,16 @@ statements of its own (its logic is measured in `core/` via the web suite, and `tui-servers.test.ts` still exercises it behaviorally — it is excluded only so it doesn't surface as a misleading 0/0 row). +**Import `render` from `__tests__/helpers/renderTui.tsx`, never from +`ink-testing-library` directly.** Ink writes styling as escape sequences *inside* +the styled run, so an accelerator underline splits the word it decorates — +`Info` reaches the frame buffer with escapes between `I` +and `nfo`, and `expect(frame).toContain("Info")` fails against a component that +is rendering correctly. Because chalk only emits color when it detects a TTY, +this is invisible in CI and hits exactly the developer whose shell exports +`FORCE_COLOR` (#2207). The wrapper strips styling from `lastFrame()` and +`frames`; the untouched bytes stay available as `stdout.lastFrame()`. + ### Bundling: React-rendering dependencies must be inlined (#1952) `tsup.config.ts` splits the TUI's dependencies into bundled (`noExternal`) and diff --git a/clients/tui/__tests__/App.test.tsx b/clients/tui/__tests__/App.test.tsx index 3139174f..1d8faf7c 100644 --- a/clients/tui/__tests__/App.test.tsx +++ b/clients/tui/__tests__/App.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; type RenderResult = ReturnType; diff --git a/clients/tui/__tests__/AuthTab.test.tsx b/clients/tui/__tests__/AuthTab.test.tsx index 213416c5..4f462e48 100644 --- a/clients/tui/__tests__/AuthTab.test.tsx +++ b/clients/tui/__tests__/AuthTab.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; import type { OAuthConnectionState } from "@inspector/core/auth/types.js"; import type { InspectorClient } from "@inspector/core/mcp/index.js"; diff --git a/clients/tui/__tests__/DetailsModal.test.tsx b/clients/tui/__tests__/DetailsModal.test.tsx index 41689568..655041a4 100644 --- a/clients/tui/__tests__/DetailsModal.test.tsx +++ b/clients/tui/__tests__/DetailsModal.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; import { Text } from "ink"; // ScrollView: passthrough so `content` mounts and the imperative ref API diff --git a/clients/tui/__tests__/HistoryTab.test.tsx b/clients/tui/__tests__/HistoryTab.test.tsx index 4710402d..6d872522 100644 --- a/clients/tui/__tests__/HistoryTab.test.tsx +++ b/clients/tui/__tests__/HistoryTab.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; import type { MessageEntry } from "@inspector/core/mcp/index.js"; // MUST mock ink-scroll-view: the real ScrollView renders a placeholder minimap diff --git a/clients/tui/__tests__/InfoTab.test.tsx b/clients/tui/__tests__/InfoTab.test.tsx index 275cb586..099eee47 100644 --- a/clients/tui/__tests__/InfoTab.test.tsx +++ b/clients/tui/__tests__/InfoTab.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; // MUST mock ink-scroll-view: the real ScrollView renders a placeholder minimap // in the non-TTY test env and never mounts its children. This passthrough diff --git a/clients/tui/__tests__/NotificationsTab.test.tsx b/clients/tui/__tests__/NotificationsTab.test.tsx index cd79c458..205a4cbf 100644 --- a/clients/tui/__tests__/NotificationsTab.test.tsx +++ b/clients/tui/__tests__/NotificationsTab.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; // MUST mock ink-scroll-view: the real ScrollView renders a placeholder minimap // in the non-TTY test env and never mounts its children. This passthrough diff --git a/clients/tui/__tests__/PromptTestModal.test.tsx b/clients/tui/__tests__/PromptTestModal.test.tsx index d79e77be..6caba3e4 100644 --- a/clients/tui/__tests__/PromptTestModal.test.tsx +++ b/clients/tui/__tests__/PromptTestModal.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi, afterEach } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; import type { InspectorClient } from "@inspector/core/mcp/index.js"; import { AuthRecoveryRequiredError } from "@inspector/core/auth/challenge.js"; import type { Prompt } from "@modelcontextprotocol/client"; diff --git a/clients/tui/__tests__/PromptsTab.test.tsx b/clients/tui/__tests__/PromptsTab.test.tsx index c9adee62..3d7b4ba0 100644 --- a/clients/tui/__tests__/PromptsTab.test.tsx +++ b/clients/tui/__tests__/PromptsTab.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; import type { InspectorClient } from "@inspector/core/mcp/index.js"; import type { Prompt } from "@modelcontextprotocol/client"; diff --git a/clients/tui/__tests__/RequestsTab.test.tsx b/clients/tui/__tests__/RequestsTab.test.tsx index 35f59e88..63fcb12d 100644 --- a/clients/tui/__tests__/RequestsTab.test.tsx +++ b/clients/tui/__tests__/RequestsTab.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; import type { FetchRequestEntry } from "@inspector/core/mcp/index.js"; // MUST mock ink-scroll-view: the real ScrollView renders a placeholder minimap diff --git a/clients/tui/__tests__/ResourceTestModal.test.tsx b/clients/tui/__tests__/ResourceTestModal.test.tsx index 525051a0..81614f95 100644 --- a/clients/tui/__tests__/ResourceTestModal.test.tsx +++ b/clients/tui/__tests__/ResourceTestModal.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi, afterEach } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; import type { InspectorClient } from "@inspector/core/mcp/index.js"; import { AuthRecoveryRequiredError } from "@inspector/core/auth/challenge.js"; diff --git a/clients/tui/__tests__/ResourcesTab.test.tsx b/clients/tui/__tests__/ResourcesTab.test.tsx index 952b21c8..ad1fa78d 100644 --- a/clients/tui/__tests__/ResourcesTab.test.tsx +++ b/clients/tui/__tests__/ResourcesTab.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; import type { InspectorClient } from "@inspector/core/mcp/index.js"; import type { Resource } from "@modelcontextprotocol/client"; diff --git a/clients/tui/__tests__/SelectableItem.test.tsx b/clients/tui/__tests__/SelectableItem.test.tsx index 395d6a98..b9222d90 100644 --- a/clients/tui/__tests__/SelectableItem.test.tsx +++ b/clients/tui/__tests__/SelectableItem.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; import { SelectableItem } from "../src/components/SelectableItem.js"; describe("SelectableItem", () => { diff --git a/clients/tui/__tests__/Tabs.test.tsx b/clients/tui/__tests__/Tabs.test.tsx index cb3f27a6..e15df183 100644 --- a/clients/tui/__tests__/Tabs.test.tsx +++ b/clients/tui/__tests__/Tabs.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; import { Tabs } from "../src/components/Tabs.js"; const noop = () => {}; diff --git a/clients/tui/__tests__/ToolTestModal.test.tsx b/clients/tui/__tests__/ToolTestModal.test.tsx index 63c2100c..bf9af688 100644 --- a/clients/tui/__tests__/ToolTestModal.test.tsx +++ b/clients/tui/__tests__/ToolTestModal.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi, afterEach } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; import type { InspectorClient } from "@inspector/core/mcp/index.js"; import type { Tool } from "@modelcontextprotocol/client"; diff --git a/clients/tui/__tests__/ToolsTab.test.tsx b/clients/tui/__tests__/ToolsTab.test.tsx index 0c1913d6..2871ab63 100644 --- a/clients/tui/__tests__/ToolsTab.test.tsx +++ b/clients/tui/__tests__/ToolsTab.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect, vi } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; import type { Tool } from "@modelcontextprotocol/client"; // MUST mock ink-scroll-view: the real ScrollView renders a placeholder minimap diff --git a/clients/tui/__tests__/helpers/renderTui.tsx b/clients/tui/__tests__/helpers/renderTui.tsx new file mode 100644 index 00000000..c356ea22 --- /dev/null +++ b/clients/tui/__tests__/helpers/renderTui.tsx @@ -0,0 +1,38 @@ +// A drop-in replacement for ink-testing-library's `render` that strips ANSI +// styling from every frame the tests read back. +// +// Ink writes styling as escape sequences *inside* the styled run, so an +// accelerator underline splits the word it decorates: `Info` +// reaches the frame buffer as `ESC[4mI ESC[24m nfo` (without the spaces). A plain +// `expect(frame).toContain("Info")` then fails against a component that is +// rendering perfectly — and only for a developer whose shell exports +// FORCE_COLOR, since CI has no TTY and chalk emits nothing there (#2207). +// +// Making every assertion read a stripped frame fixes the whole class rather +// than the six assertions that happened to trip it, and keeps the suite +// covering the styled path instead of pinning FORCE_COLOR=0 to hide it. No test +// asserts on escape sequences; one that needs the raw bytes can reach +// `stdout.lastFrame()` on the returned instance, which is left untouched. +import { render as inkRender } from "ink-testing-library"; +import stripAnsi from "strip-ansi"; +import type { ReactElement } from "react"; + +// ink-testing-library does not export its `Instance` type. +type Instance = ReturnType; + +/** Strip ANSI styling from a frame, passing `undefined` through unchanged. */ +export const stripFrameStyling = (frame: string | undefined) => + frame === undefined ? undefined : stripAnsi(frame); + +export const render = (tree: ReactElement): Instance => { + const instance = inkRender(tree); + return { + ...instance, + lastFrame: () => stripFrameStyling(instance.lastFrame()), + // Read through to the live array on each access — ink appends to it as the + // component re-renders, so a copy taken here would go stale. + get frames() { + return instance.frames.map(stripAnsi); + }, + }; +}; diff --git a/clients/tui/__tests__/renderTui.test.tsx b/clients/tui/__tests__/renderTui.test.tsx new file mode 100644 index 00000000..24d0b2f7 --- /dev/null +++ b/clients/tui/__tests__/renderTui.test.tsx @@ -0,0 +1,57 @@ +import React from "react"; +import { Text } from "ink"; +import { describe, expect, it } from "vitest"; +import { render, stripFrameStyling } from "./helpers/renderTui"; + +// The escape sequences ink emits for ``, spelled out here so +// the assertions hold regardless of whether the ambient environment has +// FORCE_COLOR set — chalk emits nothing without a TTY, which is exactly why +// #2207 never reproduced in CI. +const ESC = "\u001B"; +const UNDERLINE_OPEN = `${ESC}[4m`; +const UNDERLINE_CLOSE = `${ESC}[24m`; +const SPLIT_WORD = `${UNDERLINE_OPEN}I${UNDERLINE_CLOSE}nfo`; + +describe("stripFrameStyling", () => { + it("removes styling that splits a word", () => { + expect(stripFrameStyling(SPLIT_WORD)).toBe("Info"); + }); + + it("leaves an unstyled frame untouched", () => { + expect(stripFrameStyling("Info")).toBe("Info"); + }); + + it("passes undefined through", () => { + expect(stripFrameStyling(undefined)).toBeUndefined(); + }); +}); + +describe("render", () => { + it("strips styling from lastFrame", () => { + const { stdout, lastFrame } = render(placeholder); + stdout.write(SPLIT_WORD); + expect(lastFrame()).toBe("Info"); + }); + + it("strips styling from frames, and keeps reading the live array", () => { + const instance = render(placeholder); + const before = instance.frames.length; + instance.stdout.write(SPLIT_WORD); + expect(instance.frames).toHaveLength(before + 1); + expect(instance.frames.at(-1)).toBe("Info"); + }); + + it("leaves the raw bytes reachable through stdout", () => { + const instance = render(placeholder); + instance.stdout.write(SPLIT_WORD); + expect(instance.stdout.lastFrame()).toBe(SPLIT_WORD); + }); + + it("still exposes the rest of the ink-testing-library instance", () => { + const { rerender, lastFrame, unmount } = render(first); + expect(lastFrame()).toContain("first"); + rerender(second); + expect(lastFrame()).toContain("second"); + unmount(); + }); +}); diff --git a/clients/tui/__tests__/useSelectableList.test.tsx b/clients/tui/__tests__/useSelectableList.test.tsx index 132ed324..9ac86a9a 100644 --- a/clients/tui/__tests__/useSelectableList.test.tsx +++ b/clients/tui/__tests__/useSelectableList.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { describe, it, expect } from "vitest"; -import { render } from "ink-testing-library"; +import { render } from "./helpers/renderTui"; import { Text, useInput } from "ink"; import { useSelectableList, diff --git a/clients/tui/package-lock.json b/clients/tui/package-lock.json index 5c105f2f..d4907b51 100644 --- a/clients/tui/package-lock.json +++ b/clients/tui/package-lock.json @@ -19,6 +19,7 @@ "@types/react": "^19.2.14", "eslint-plugin-react-hooks": "^7.1.1", "ink-testing-library": "^4.0.0", + "strip-ansi": "^7.1.2", "tsup": "^8.5.0", "tsx": "^4.21.0", "vite": "^8.1.5", diff --git a/clients/tui/package.json b/clients/tui/package.json index 94f74558..60fa4a95 100644 --- a/clients/tui/package.json +++ b/clients/tui/package.json @@ -39,6 +39,7 @@ "@types/react": "^19.2.14", "eslint-plugin-react-hooks": "^7.1.1", "ink-testing-library": "^4.0.0", + "strip-ansi": "^7.1.2", "tsup": "^8.5.0", "tsx": "^4.21.0", "vite": "^8.1.5",