inspector/clients/web/src/hooks/useImportClientConfig.test.tsx
Cliff Hall 4defa32ac6
chore: close a batch of quality-gate & tidy gaps (#1785 #1787 #1789 #1790 #1767) (#1792)
* chore: close a batch of quality-gate & tidy gaps (#1785 #1787 #1789 #1790 #1767)

Bundles five small, related tidy/gate items into one PR:

- #1785: delete the dead src/types/navigation.ts InspectorTab (0 importers;
  duplicated/drifted from utils/inspectorTabs InspectorTabId). src/types/
  now holds only ambient .d.ts stubs.
- #1787: add src/hooks/** to the web coverage `include` (it was silently
  outside the ≥90 gate) and add tests for the two untested hooks,
  useServerJsonImport and useImportClientConfig, bringing every hook to
  ≥90 on lines/statements/functions/branches.
- #1789: widen clients/{cli,tui,launcher} format/format:check to their
  top-level configs; add root format:shared / format:check:shared covering
  test-servers/src/**, vitest.shared.mts, and the repo-root eslint.config.js.
- #1790: pin prettier to an exact version (3.8.4, dropping the ^3.8.1
  caret) in all five package.json files so the format gate's verdict can't
  shift with an in-range patch bump; lockfiles synced.
- #1767: its root-file format+lint gap is closed here (format:shared +
  a new lint:shared via a second files block in the root eslint.config.js;
  web/server/ was already gated in #1788). Its larger cli/tui __tests__
  typecheck piece (76 type errors) is split out to #1791.

Prettier-normalized the newly-covered files (pure formatting). No runtime
behavior change. `npm run ci` green.

Closes #1785
Closes #1787
Closes #1789
Closes #1790
Closes #1767

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore: address PR #1792 review findings

- #1789 gap: widen clients/web format to include .storybook and clients/
  launcher format to include scripts/; reformat the 3 .storybook files
  (were non-conforming under the pinned prettier 3.8.4). Audited all 885
  tracked source files — now fully prettier-clean.
- Export VALIDATE_DEBOUNCE_MS / COLLAPSE_DELAY_MS / HIGHLIGHT_DURATION_MS
  from useServerJsonImport and use them in the test instead of hardcoded
  300/1000/250 (so a tuned constant can't silently desync the test).
- Assert the symmetric renameErrors["alpha"] alongside ["beta"] so a
  one-directional collision-check regression fails.
- Comment the blank-rename test as exercising a UI-unreachable defensive
  branch (canImport gates submit).
- Un-stale the two specification/v2_ux_interfaces_plan.md references to the
  now-removed navigation.ts InspectorTab.

npm run ci green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* docs: sync AGENTS.md format-surface description with the widened globs (#1792 review r2)

1ce3b64 added .storybook to clients/web's format glob and scripts/ to
clients/launcher's, but AGENTS.md:222 still described the pre-fix scope.
Bring it in line and note the non-recursive top-level glob is why nested
dirs are named explicitly. Docs only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore: handle PR #1792 round-3 follow-ups in-place (format-coverage guard, uniform globs, theme gate)

Rather than defer these to tickets, fold them into this PR:

- Obs 2: widen every client + root format glob to a uniform
  *.{ts,tsx,mts,cts,js,jsx,mjs,cjs} so a future .mts/.cjs/.tsx top-level
  file can't silently slip the gate.
- Obs 1: add scripts/verify-format-coverage.mjs (wired into `validate`) —
  it parses the format:check globs out of every package.json, enumerates
  all tracked source files, and fails listing any not covered by a glob.
  The durable, standing version of #1789's one-shot audit; negative-tested.
- Obs 3: bring src/theme/** under the coverage gate (21/23 files were
  already ≥90 transitively; add Paper.test.tsx for the one laggard,
  Paper.ts). src/App.tsx (~4.5k lines, ~42% branches) is converted from a
  silent omission into an explicit, documented exclusion (comment on the
  coverage `include` + AGENTS.md) — gating it is a dedicated effort, not a
  whitelist tweak.

npm run ci green (the new guard reports 885/885 tracked source files gated).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore: address PR #1792 round-4 findings (guard hardening, docs, exclusion list)

- Harden verify-format-coverage: only harvest format:check globs from
  scripts transitively reachable from `validate` (via npm run refs), so an
  unwired/renamed format:check no longer counts as coverage. Negative-tested
  — renaming clients/web format:check to an unreferenced name now fails the
  guard (627 files flagged) instead of passing.
- Name src/main.tsx and src/index.ts (browser + bin bootstraps) alongside
  src/App.tsx in the coverage-include comment and AGENTS.md, so the
  documented exclusion list is complete rather than claiming App.tsx is the
  only one.
- Doc sync: document verify:format-coverage (AGENTS.md + README script table
  + scripts/ layout line), fix the `validate` ordering (verify:format-
  coverage now runs before validate:core) in AGENTS.md and README, and
  update the quoted globs to the uniform *.{ts,tsx,mts,cts,js,jsx,mjs,cjs} /
  test-servers *.{ts,tsx,mts,cts}.

npm run ci green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore: address PR #1792 round-5 findings (airtight guard + doc/config sync)

- verify-format-coverage: assert the root `validate` chain invokes each
  client's `validate` (cd <dir> && npm run validate), closing the last
  reachability edge — a client dropped from the root chain now fails the
  guard even though its own globs still exist. Negative-tested.
- eslint.config.js: widen the shared `files` block to
  test-servers/src/**/*.{ts,tsx,mts,cts} to match the widened lint:shared
  glob, so a future .mts/.cts there is lint-gated, not just prettier-gated.
- AGENTS.md: scope the "uniform extension set" claim to client globs
  (core stays {ts,tsx}, shared {ts,tsx,mts,cts}), with verify:format-coverage
  as the backstop — removing the self-contradiction with the line below.

npm run ci green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-26 10:43:11 -04:00

391 lines
13 KiB
TypeScript

import { describe, it, expect, vi } from "vitest";
import { renderHook, act } from "@testing-library/react";
import {
useImportClientConfig,
type UseImportClientConfigOptions,
} from "./useImportClientConfig";
import type { ImportSourceResult } from "@inspector/core/mcp/import/index.js";
import type { MCPConfig } from "@inspector/core/mcp/types.js";
function config(...ids: string[]): MCPConfig {
const mcpServers: MCPConfig["mcpServers"] = {};
for (const id of ids) mcpServers[id] = { command: "node", args: [id] };
return { mcpServers };
}
function foundSource(cfg: MCPConfig): ImportSourceResult {
return { type: "claude", found: true, config: cfg, searched: ["/path"] };
}
function setup(over: Partial<UseImportClientConfigOptions> = {}) {
const onFetchSource =
over.onFetchSource ?? vi.fn(async () => foundSource(config("alpha")));
const onAddServer = over.onAddServer ?? vi.fn(async () => {});
const onUpdateServer = over.onUpdateServer ?? vi.fn(async () => {});
const opts: UseImportClientConfigOptions = {
opened: true,
existingIds: [],
onFetchSource,
onAddServer,
onUpdateServer,
...over,
};
const view = renderHook(
(p: UseImportClientConfigOptions) => useImportClientConfig(p),
{ initialProps: opts },
);
return { ...view, onFetchSource, onAddServer, onUpdateServer };
}
describe("useImportClientConfig", () => {
it("starts in the select phase with nothing to import", () => {
const { result } = setup();
expect(result.current.phase).toBe("select");
expect(result.current.plan).toBeNull();
expect(result.current.canImport).toBe(false);
expect(result.current.importCount).toBe(0);
});
it("setSelectedType records the choice", () => {
const { result } = setup();
act(() => result.current.setSelectedType("claude"));
expect(result.current.selectedType).toBe("claude");
});
it("pickSource loads a config and moves to review", async () => {
const { result } = setup({
onFetchSource: vi.fn(async () => foundSource(config("alpha", "beta"))),
});
await act(async () => {
await result.current.pickSource("claude");
});
expect(result.current.phase).toBe("review");
expect(result.current.plan?.additions).toHaveLength(2);
expect(result.current.importCount).toBe(2);
expect(result.current.canImport).toBe(true);
});
it("pickSource surfaces a fetch-reported error", async () => {
const { result } = setup({
onFetchSource: vi.fn(async () => ({
type: "claude",
found: false,
searched: [],
error: "backend blew up",
})),
});
await act(async () => {
await result.current.pickSource("claude");
});
expect(result.current.phase).toBe("select");
expect(result.current.error).toBe("backend blew up");
});
it("pickSource shows a not-found notice", async () => {
const { result } = setup({
onFetchSource: vi.fn(async () => ({
type: "claude",
found: false,
searched: ["/a", "/b"],
})),
});
await act(async () => {
await result.current.pickSource("claude");
});
expect(result.current.phase).toBe("select");
expect(result.current.notice).toContain("No config found");
});
it("pickSource handles an empty config as no servers", async () => {
const { result } = setup({
onFetchSource: vi.fn(async () => foundSource(config())),
});
await act(async () => {
await result.current.pickSource("claude");
});
expect(result.current.phase).toBe("select");
expect(result.current.error).toContain("No servers found");
});
it("pickSource surfaces a thrown fetch error", async () => {
const { result } = setup({
onFetchSource: vi.fn(async () => {
throw new Error("network down");
}),
});
await act(async () => {
await result.current.pickSource("claude");
});
expect(result.current.phase).toBe("select");
expect(result.current.error).toBe("network down");
});
it("pickFile parses an uploaded config", async () => {
const { result } = setup();
const raw = JSON.stringify({ mcpServers: { gamma: { command: "go" } } });
const file = new File([raw], "claude.json");
await act(async () => {
await result.current.pickFile(file);
});
expect(result.current.phase).toBe("review");
expect(result.current.plan?.additions[0].id).toBe("gamma");
});
it("pickFile ignores a null file", async () => {
const { result } = setup();
await act(async () => {
await result.current.pickFile(null);
});
expect(result.current.phase).toBe("select");
});
it("pickFile surfaces a parse error", async () => {
const { result } = setup();
const file = new File(["{ not json"], "claude.json");
await act(async () => {
await result.current.pickFile(file);
});
expect(result.current.phase).toBe("select");
expect(result.current.error).toBeTruthy();
});
it("runImport adds new servers and reports outcomes", async () => {
const onAddServer = vi.fn(async () => {});
const { result } = setup({
onFetchSource: vi.fn(async () => foundSource(config("alpha", "beta"))),
onAddServer,
});
await act(async () => {
await result.current.pickSource("claude");
});
await act(async () => {
await result.current.runImport();
});
expect(result.current.phase).toBe("summary");
expect(onAddServer).toHaveBeenCalledTimes(2);
expect(result.current.outcomes.every((o) => o.status === "added")).toBe(
true,
);
});
it("runImport skips additions opted out", async () => {
const onAddServer = vi.fn(async () => {});
const { result } = setup({
onFetchSource: vi.fn(async () => foundSource(config("alpha", "beta"))),
onAddServer,
});
await act(async () => {
await result.current.pickSource("claude");
});
act(() => result.current.setAdditionAction("beta", "skip"));
expect(result.current.importCount).toBe(1);
await act(async () => {
await result.current.runImport();
});
expect(onAddServer).toHaveBeenCalledTimes(1);
expect(result.current.outcomes.find((o) => o.id === "beta")?.status).toBe(
"skipped",
);
});
it("runImport reports an addition failure", async () => {
const onAddServer = vi.fn(async () => {
throw new Error("add failed");
});
const { result } = setup({
onFetchSource: vi.fn(async () => foundSource(config("alpha"))),
onAddServer,
});
await act(async () => {
await result.current.pickSource("claude");
});
await act(async () => {
await result.current.runImport();
});
const outcome = result.current.outcomes[0];
expect(outcome.status).toBe("failed");
expect(outcome.detail).toBe("add failed");
});
it("resolves conflicts by overwrite, skip, and rename", async () => {
const onAddServer = vi.fn(async () => {});
const onUpdateServer = vi.fn(async () => {});
const { result } = setup({
existingIds: ["alpha", "beta", "gamma"],
onFetchSource: vi.fn(async () =>
foundSource(config("alpha", "beta", "gamma")),
),
onAddServer,
onUpdateServer,
});
await act(async () => {
await result.current.pickSource("claude");
});
expect(result.current.plan?.conflicts).toHaveLength(3);
// Default is skip for all three.
act(() => result.current.setResolution("alpha", "overwrite"));
act(() => result.current.setResolution("gamma", "rename"));
act(() => result.current.setRenameTo("gamma", "gamma-new"));
await act(async () => {
await result.current.runImport();
});
expect(onUpdateServer).toHaveBeenCalledWith(
"alpha",
"alpha",
expect.any(Object),
);
expect(onAddServer).toHaveBeenCalledWith("gamma-new", expect.any(Object));
const statuses = Object.fromEntries(
result.current.outcomes.map((o) => [o.id, o.status]),
);
expect(statuses["alpha"]).toBe("overwritten");
expect(statuses["beta"]).toBe("skipped");
expect(statuses["gamma-new"]).toBe("renamed");
});
it("flags invalid and colliding rename targets", async () => {
const { result } = setup({
existingIds: ["alpha", "beta"],
onFetchSource: vi.fn(async () => foundSource(config("alpha", "beta"))),
});
await act(async () => {
await result.current.pickSource("claude");
});
// Empty rename target.
act(() => result.current.setResolution("alpha", "rename"));
act(() => result.current.setRenameTo("alpha", " "));
expect(result.current.renameErrors["alpha"]).toContain("required");
// Syntactically invalid.
act(() => result.current.setRenameTo("alpha", "bad id!"));
expect(result.current.renameErrors["alpha"]).toContain("letters");
// Collides with the other existing id.
act(() => result.current.setRenameTo("alpha", "beta"));
expect(result.current.renameErrors["alpha"]).toContain("already in use");
expect(result.current.canImport).toBe(false);
});
it("flags a rename target that collides with another rename", async () => {
const { result } = setup({
existingIds: ["alpha", "beta"],
onFetchSource: vi.fn(async () => foundSource(config("alpha", "beta"))),
});
await act(async () => {
await result.current.pickSource("claude");
});
act(() => result.current.setResolution("alpha", "rename"));
act(() => result.current.setResolution("beta", "rename"));
act(() => result.current.setRenameTo("alpha", "shared"));
act(() => result.current.setRenameTo("beta", "shared"));
// The collision check is symmetric — each rename sees the other's target
// already claimed, so both must be flagged (asserting only one would pass
// even if the check regressed to one-directional).
expect(result.current.renameErrors["alpha"]).toContain("already in use");
expect(result.current.renameErrors["beta"]).toContain("already in use");
expect(result.current.canImport).toBe(false);
});
it("renames to the original id when the rename target is blank", async () => {
const onAddServer = vi.fn(async () => {});
const { result } = setup({
existingIds: ["alpha"],
onFetchSource: vi.fn(async () => foundSource(config("alpha"))),
onAddServer,
});
await act(async () => {
await result.current.pickSource("claude");
});
act(() => result.current.setResolution("alpha", "rename"));
act(() => result.current.setRenameTo("alpha", " "));
// A blank rename also sets renameErrors, so canImport is false and the real
// UI blocks submit here — this exercises the defensive fallback at
// useImportClientConfig.ts (`res.renameTo.trim() || conflict.id`)
// programmatically, which the UI can't reach.
await act(async () => {
await result.current.runImport();
});
// A blank rename target falls back to the original id.
expect(onAddServer).toHaveBeenCalledWith("alpha", expect.any(Object));
expect(result.current.outcomes[0].status).toBe("renamed");
});
it("back returns to the select phase", async () => {
const { result } = setup();
await act(async () => {
await result.current.pickSource("claude");
});
expect(result.current.phase).toBe("review");
act(() => result.current.back());
expect(result.current.phase).toBe("select");
});
it("resets when the modal is re-opened", async () => {
const { result, rerender } = setup({ opened: false });
const props = (opened: boolean): UseImportClientConfigOptions => ({
opened,
existingIds: [],
onFetchSource: vi.fn(async () => foundSource(config("alpha"))),
onAddServer: vi.fn(async () => {}),
onUpdateServer: vi.fn(async () => {}),
});
rerender(props(true));
await act(async () => {
await result.current.pickSource("claude");
});
expect(result.current.phase).toBe("review");
rerender(props(false));
rerender(props(true));
expect(result.current.phase).toBe("select");
expect(result.current.plan).toBeNull();
});
it("stringifies a non-Error thrown by pickSource", async () => {
const { result } = setup({
onFetchSource: vi.fn(async () => {
throw "plain fetch failure";
}),
});
await act(async () => {
await result.current.pickSource("claude");
});
expect(result.current.error).toBe("plain fetch failure");
});
it("stringifies a non-Error thrown by pickFile", async () => {
const { result } = setup();
const file = new File(["x"], "claude.json");
vi.spyOn(file, "text").mockRejectedValue("plain read failure");
await act(async () => {
await result.current.pickFile(file);
});
expect(result.current.error).toBe("plain read failure");
});
it("stringifies a non-Error thrown while applying an import", async () => {
const onAddServer = vi.fn(async () => {
throw "plain add failure";
});
const { result } = setup({
onFetchSource: vi.fn(async () => foundSource(config("alpha"))),
onAddServer,
});
await act(async () => {
await result.current.pickSource("claude");
});
await act(async () => {
await result.current.runImport();
});
expect(result.current.outcomes[0]).toMatchObject({
status: "failed",
detail: "plain add failure",
});
});
it("runImport with no plan is a no-op", async () => {
const { result } = setup();
await act(async () => {
await result.current.runImport();
});
expect(result.current.phase).toBe("select");
expect(result.current.outcomes).toEqual([]);
});
});