Author	SHA1	Message	Date
Cliff Hall	de9d8a5024	fix: wait for the TUI to exit before removing its temp dir (#1801) (#1814) ... * fix: wait for the TUI to exit before removing its temp dir (#1801) `done()` sent SIGTERM and then immediately `rmSync`'d the work dir, which doubles as the TUI's HOME. Nothing waited for the child, so the Ink process could still be writing into the dir while rmSync walked it — a file landing after a directory was read but before it was removed surfaces as ENOTEMPTY on macOS, failing `smoke:tui` after a successful render. Removal now happens only once the child's `exit` event fires: SIGTERM, then SIGKILL after SMOKE_TUI_EXIT_GRACE_MS (default 5s), then clean up regardless after twice that. The crash-before-render path short-circuits via a `childExited` flag set by the pre-registered exit handler. cleanup() also warns instead of throwing, so a leftover temp dir can never fail a passing smoke. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MvKF2mqi4hoQSUqQpZXNTq * fix: wait on close, not exit, so spawn failure resolves instantly (review #1814) A failed spawn emits `error` + `close` and never `exit`, so the previous `child.once("exit")` wait never fired on that path: done() sent SIGTERM to a process that never existed and resolved only via the give-up timer — 10s at the default grace, with a warning blaming a SIGTERM the child never received. Waiting on `close` covers both normal termination and spawn failure, and additionally guarantees the stdio pipes are drained. The short-circuit gate moves with it, `childExited` → `childClosed`: keying it on `exit` would have skipped the drain on the crash-before-render path, which is exactly where the quoted output matters. Messages that quote that output are now thunks rendered in finish(), after the drain, instead of being sliced at call time. SIGTERM is now sent only to a child that is still running, since on both the crash and spawn-failure paths there is nothing left to signal. finish() also gets a `finished` guard so its single-entry invariant is local rather than inferred from process.exit() being synchronous. Measured, spawn failure (grace shortened to 500ms): 1.04s + spurious warning before, 0.02s and silent after. Crash-before-render now prints the child's stderr detail line that the pre-drain slice could truncate. Happy path 3x, timeout path, and full `npm run smoke` + `npm run validate` all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MvKF2mqi4hoQSUqQpZXNTq * fix: bound the drain, and make the give-up warning true (review #1814 r2) Three follow-ons from the second review pass: The give-up warning still said the TUI "did not exit within Xms of SIGTERM", but after switching the wait to `close` neither clause is reliably true — the timer's most plausible trigger is now a child that has already exited, and on the crash and spawn-failure paths no SIGTERM is sent at all. That is the same defect this PR fixed one round ago (a warning naming a signal never sent), so the deadline is now re-armed with a message describing what is actually being awaited. `close` is bounded by whoever holds the stdio pipes, which can outlive the direct child: a descendant that inherited them keeps it pending. That is the one real cost of `exit` → `close`, and it is moot only because the TUI spawns nothing at boot — a reason that lives entirely outside this file. Once `exit` has fired the deadline drops to DRAIN_MS, keeping the drain guarantee without inheriting an unrelated process's lifetime. Measured against a grandchild holding the pipes for 3s: 0.54s and an accurate warning, instead of 3s. The quoted output is sliced from the tail rather than the head. The thunk added last round exists to capture bytes that arrive late, which a head slice then discards as soon as output exceeds 800 chars — and on the timeout path the last frame is what diagnoses a stuck render, not the first. Verified: a 2KB stderr crash now reports its final line. The sibling scripts keep `slice(0, 800)`; changing them is a deliberate sweep, not a drive-by here. Also corrected the cleanup() comment — the wait lives in done(), and it is for `close`, not `exit`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MvKF2mqi4hoQSUqQpZXNTq * docs: correct the cleanup() and DRAIN_MS comments; tail-slice cleanly (review #1814 r3) The cleanup() comment claimed its warning "should not fire now that removal waits for close" — true only of the normal path. Both give-up branches call finish() without close, and those are precisely the states where something may still hold the dir, so the warning is expected there. Same defect class as the two warnings already fixed in this PR (a message asserting what the code no longer guarantees), relocated into a comment. The DRAIN_MS comment explained why the cap exists but not what it costs: on that path the work dir is removed while a descendant holding HOME=work may still be writing to it — #1801's race, re-entered deliberately. That is only acceptable because cleanup() warns instead of throwing, so record the two as load-bearing for each other where the constant is defined. Tail-slicing an Ink stream can begin mid-CSI-sequence, leading a diagnostic with an escape-code fragment that mangles the line after it. outputTail() drops through the first newline when the output was actually truncated, and returns short output untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MvKF2mqi4hoQSUqQpZXNTq --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-27 20:25:27 -04:00
Cliff Hall	d9e4f444a2	test(cli,tui): typecheck the __tests__ dirs (#1791) (#1799) ... * test(cli,tui): typecheck the __tests__ dirs (#1791) The cli/tui `typecheck` scripts only checked `src` (the src-only tsconfig excludes `**/*.test.*`), leaving the `__tests__` dirs — which do the most `as` casting — with no tsc pass. Add a `tsconfig.test.json` per client that includes `__tests__` with the test-exclusions dropped and the test-only path aliases (`@modelcontextprotocol/inspector-test-server`, the `@inspector/core/*` deep paths, express/vitest) resolving what vitest resolves, then fold `tsc -p tsconfig.test.json` into each `typecheck`. Fixes the 76 errors it surfaced: - cli: a wrong `@modelcontextprotocol/inspector-core/...` module specifier (should be `@inspector/core/...`; only a type import so vitest never caught it), `r.headers` optional-access guards, and partial OAuth mocks replaced with typed `makeFakeCliOAuthClient()` / `makeFakeServerSettings()` factories instead of `as` casts. Adds `@types/express` (devDep) so the transitively aliased test-server source typechecks, mirroring clients/web. - tui: rest-param signatures on the App OAuth spies (fixes the spread-arg and narrow-return errors), a widened callback param (`iss?`), a `makeFakeServer- Settings()` factory for the tuiOAuth tests, required OAuth-metadata/client fields in AuthTab, and a loosened `entry()` message param for the deliberately-malformed wire messages HistoryTab's defensive branches render. Gate coverage only; no runtime behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test(cli,tui): address review — real spy signatures, config typecheck, nits Responds to the @claude review of #1799: - App.test.tsx: type the OAuth spies against the real InspectorClient method signatures (`vi.fn<InspectorClient["…"]>()`) and forward `Parameters<…>` tuples from the FakeClient wrappers, so `toHaveBeenCalledWith(...)` on the six OAuth-lifecycle spies stays argument-checked under the new gate rather than accepting anything via `unknown[]`. `getOAuthState` regains its wide `OAuthConnectionState | undefined` return. - Typecheck the client config files too: add `vitest.config.ts` / `tsup.config.ts` (and tui `dev.ts`) to each src `tsconfig.json` include — they were the one first-party surface still getting no tsc pass. - Trim the redundant `src/**/*` from each `tsconfig.test.json` include (tsc pulls in the src the tests import; the src-only config already checks it). - Rename the tui settings helper to `oauth-test-fakes.ts` to match cli's, and soften both factories' docstrings to stop naming a loader they don't mirror field-for-field. - Fix a stale `@modelcontextprotocol/inspector-core/...` specifier in a createRemoteTransport JSDoc example (the same wrong package name the gate caught in the cli tests, surviving in a comment). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test: address round-2 review — correct claims, tighten types, typecheck launcher Responds to the round-2 @claude review of #1799: - Correct the toHaveBeenCalledWith claim: vitest doesn't type-check `toHaveBeenCalledWith(...)` arguments against the mock's signature, so the App.test.tsx comment no longer claims it does — the typed spies buy checked implementations/return payloads (the stale-literal fix), not arg-checking. - authenticate spy now uses the real `InspectorClient["authenticate"]` signature (returns a `URL`), so it no longer diverges from production; the one mockResolvedValue is updated to a URL. - makeFakeCliOAuthClient members are typed against the real CliOAuthClient signatures (not bare vi.fn()), so a wrong-signature override is rejected. - HistoryTab: revert the `entry()` `message: unknown` widening (it un-checked the ~15 well-formed fixtures) and instead scope a named `MALFORMED` cast to the two intentionally-malformed messages. - Typecheck the launcher `__tests__` too (the same gate hole in the one client the PR hadn't touched): new clients/launcher/tsconfig.test.json + a typecheck script folded into validate. Completes the "every client's __tests__ is typechecked" invariant. - Doc the new test-typecheck + config-file typecheck in AGENTS.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test: address round-3 nits — launcher config typecheck, callTool typed, docs Responds to the round-3 @claude review of #1799 (all non-blocking): - launcher: add `vitest.config.ts` to tsconfig.test.json's include (it was in neither project — the build config's rootDir: ./src rejects it). Now typechecked. - App.test.tsx: type `callTool` against `InspectorClient["callTool"]` (+ the Parameters<…> wrapper) so its `mockResolvedValue` payloads are checked, and give `clearOAuthTokens` an `async () => {}` impl so the mock actually returns the `Promise<void>` its type promises. - AGENTS.md: correct the per-client alias-set description (cli's is widest, tui's is core+react only, launcher's has no paths) and note launcher's config file lives in its test project, not the src project. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test: address round-4 — durable typecheck-coverage guard + launcher/doc nits Responds to the round-4 @claude review of #1799 (all non-blocking): - Add `verify:typecheck-coverage` (scripts/verify-typecheck-coverage.mjs), the typecheck-coverage analog of verify:format-coverage and the durable guard for the invariant this PR establishes: for each Node client it runs the tsconfig projects its `typecheck` names with `tsc --listFiles`, unions them, and fails on any tracked .ts/.tsx that lands in no project. This closes the class the review kept surfacing by hand (launcher/__tests__ in round 3, launcher/vitest.config.ts in round 4) — a new top-level file can no longer go untypechecked silently. Wired into `npm run ci` and the GitHub workflow. - launcher: reorder `validate` to run `typecheck` before `build` (matches tui and the documented order), and make `typecheck` self-contained — it now runs both `tsconfig.json` and `tsconfig.test.json`, so running it standalone means the same thing it does in cli/tui. - makeFakeCliOAuthClient: note on the comment that the typing covers the defaults, not `overrides` (a bare vi.fn() override is Mock<(...args)=>any>). - Docs: README table + AGENTS.md updated for the new guard and the corrected launcher typecheck/validate ordering. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test: address round-5 — wire-check the guard, --listFilesOnly, into validate Responds to the round-5 @claude review of #1799: - Finding 1 (the pre-merge one): the guard now ports verify:format-coverage's reachability checks — it asserts each client's `typecheck` is reachable from its own `validate` and that the root chain runs each client's `validate`, so it can't stay green while measuring a typecheck nothing invokes. Verified: stripping `npm run typecheck` from launcher's validate now fails the guard. - Nit 3: switch `tsc --listFiles` → `tsc --listFilesOnly` (identical file list, ~4× faster: whole guard 23s → ~4s). Now cheap enough for the inner loop, so it moves into `validate` (right after verify:format-coverage) instead of a standalone ci step — matching its sibling. Removed the separate ci-chain entry and GitHub workflow step (validate covers it). - Nit 4: `npx --no-install tsc` so a missing local tsc fails fast instead of reaching for the registry (matches verify-build-gate.mjs). - Nit 5: corrected projectFiles' JSDoc to describe the filter that's actually there (drops repo-external + node_modules paths; core/ + test-servers/ stay but are harmless). - Finding 2: fixed the 2-space misindent on the new root package.json line (prettier --write). Docs (README/AGENTS) updated for the validate placement. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): address round-6 guard nits — dedupe wiring, robust diagnostics Responds to the round-6 @claude review of #1799 (all guard-ergonomics nits): - Nit 2: extract the shared reachability logic (`reachableScripts`, `rootRunsClientValidate`) into scripts/lib/npm-scripts.mjs and consume it from BOTH verify-format-coverage.mjs and verify-typecheck-coverage.mjs, so the "is this gate actually run?" wiring can't drift between them (same rationale as scripts/lib/prod-web-server.mjs). - Nit 1: typecheckProjects now harvests `-p`/`--project` from every script reachable from `typecheck` (not the single string), so a delegating `typecheck` (`npm run typecheck:src && …`) is handled instead of failing with a false "names no -p project" diagnosis. Also accepts `--project`. - Nit 3: on a tsc config error, echo the `error TS…` diagnostic (scanning both streams — tsc prints config errors to stdout) so a broken tsconfig is self-explaining rather than surfacing only as a misleading "file in no project" report. - Nit 4: refreshed the CI workflow's stale "Validate" step name/comment (it now runs the coverage guards + validate:core + per-client typecheck). - Nit 5: reworded the `.d.ts` carve-out comment to read as pre-emptive (there are none under cli/tui/launcher today). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-7 — guard .mts/.cts too; drop dead export Responds to the round-7 @claude review of #1799: - Finding 1: verify-typecheck-coverage now requires a tsc pass for tracked `.mts`/`.cts` (not just `.ts`/`.tsx`), matching verify-format-coverage's extension set — `.mts` is already the shared-config idiom here, so a client `vitest.config.ts` → `.mts` rename can no longer drop out of both the tsconfig include and the guard's required set. No-op on today's tree (no client `.mts`/`.cts`); confirmed a tracked `.mts` with a type error is now caught. Ambient `.d.{ts,mts,cts}` stay excluded. - Nit 2: drop the unused `export` on `rootReachedCommands` (internal to the lib; nothing imports it, and scripts/ isn't eslint-gated to catch dead exports). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test: round-8 — single casts in HistoryTab, uniform wrappers, doc scope Responds to the round-8 @claude review of #1799: - Finding 1: reduce the `MALFORMED` helper and `entry()`'s return cast from `as unknown as` to a single `as` (both compile clean — the malformed shapes are comparable to the JSONRPCMessage union), and delete the false "only option" sentence. Fixes the AGENTS.md avoid-double-casts violation. - Nit 3: write all seven FakeClient OAuth wrappers as `(...a: Parameters<InspectorClient["…"]>)` (the three zero-arg ones were hand-written `()` forwarders), so the block comment's "forward the same Parameters<…> tuple" is literally true and a future added parameter can't be silently dropped. - Nit 2: update the five places that still described the guard as `.ts`/`.tsx` only to `.ts`/`.tsx`/`.mts`/`.cts` (script header + trackedSourceFiles JSDoc, AGENTS.md ×2, README.md). Nits 4 (js config files out of the typecheck guard by construction) and the footnoted callTool/ancestor-tsc items left as agreed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test: round-9 — drop entry()'s harmful cast; guards vouch for each other Responds to the round-9 @claude review of #1799: - Finding 1: remove `entry()`'s `as MessageEntry` return cast in HistoryTab.test.tsx. It was unnecessary (the spread of Partial<MessageEntry> over complete defaults is directly assignable) and harmful — it silenced type errors in the helper's OWN default fields (a `direction`/`method` typo now surfaces TS2820/TS2353). MALFORMED's single cast stays (load-bearing). This leaves MALFORMED as the file's only cast, matching what the comments describe. - Nit 2: add `rootReachesScript` to scripts/lib/npm-scripts.mjs and have each coverage guard assert its SIBLING is still wired into the root `validate`. A guard can't detect being unrun itself, but the two now vouch for each other, so dropping either from `validate` is caught by the other (only deleting both slips through). Verified: removing verify:typecheck-coverage from validate now fails verify:format-coverage, and vice-versa. Nits 3 (git-index visibility) and 4 (pre-existing as-never/as-unknown casts in tests — out of scope, none added by this PR) left as agreed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test: round-10 — reject non-checking typecheck passes; pre/post hooks; docs Responds to the round-10 @claude review of #1799: - Finding 1: verify-typecheck-coverage now rejects a `-p <project>` whose own command carries `--noCheck` or `--listFilesOnly` — such a pass lists files without type-checking them, which previously satisfied both the wiring and file-coverage halves while checking nothing. (Splits each script on `&&`/`||`/`;` so a flag on one command can't taint another.) Verified a `typecheck` using `tsc … --noCheck` now fails with a clear message; `--noCheck` is a live idiom here (cli's test-servers:build), so this is a real copy-paste hazard. - Nit 2: reachableScripts (shared lib) now also follows npm's implicit `pre<name>`/`post<name>` lifecycle hooks, so moving a gate into e.g. `prevalidate` is recognized instead of producing a false "not reachable" failure (cli already uses a `pretest` hook). Verified. - Nit 3: AGENTS.md "Test placement" now states the Node clients (cli/tui/ launcher) keep all tests in `__tests__/`, enforced by verify:typecheck-coverage (a co-located src test lands in no tsconfig project). - Nit 4: noted on the factory comments that `Partial<…>` overrides admit an explicit `undefined` for a required field (exactOptionalPropertyTypes off). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * fix: round-11 — launcher build excludes tests; guard catches all noCheck vectors Responds to the round-11 @claude review of #1799: - Finding 1 (shipping bug): clients/launcher/tsconfig.json now excludes `**/*.test.{ts,tsx}` (matching cli/tui). launcher's `build` is an EMITTING tsc over src/**, and clients/launcher/build is in the root `files` allowlist, so a co-located `src/**/*.test.ts` would have been compiled into the published tarball. Excluding it also makes such a test land in no tsconfig project, so verify:typecheck-coverage catches it — making the AGENTS.md placement rule literally true for all three clients. - Finding 2: verify-typecheck-coverage now catches both remaining ways a typecheck could check nothing: (a) case-insensitive `--noCheck`/`--nocheck`/ `--listFilesOnly` on the command, and (b) `noCheck` set in the tsconfig itself (detected via `tsc --showConfig`, which surfaces it from the config or its extends chain). Verified both fail the guard. - Nits 3 & 4: restructured into an explicit gate-integrity phase (wiring + neutered + config-noCheck + no-projects) that reports under its own accurate header and exits BEFORE the file-coverage pass — so an inert gate no longer prints file-remediation advice, nor buries the real cause under an "in no tsconfig project" line for every file that gate covered. The one CIMD OAuth E2E integration test that flaked in the coverage run passes 32/32 in isolation and on re-run; unrelated to these script/config changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * fix: round-12 — exclude .mts/.cts tests from build; drop false integrity line Responds to the round-12 @claude review of #1799: - Finding 1 (tarball, again): the round-11 test exclude only covered `.ts`/`.tsx`, so a co-located `src/*.test.mts` still compiled into launcher's shipped `build/`. All three clients now exclude `**/*.test.*` (one glob, covering .ts/.tsx/.mts/.cts) — which is also exactly what AGENTS.md already claims. Verified a `src/probe.test.mts` is no longer emitted and now fails the guard. - Finding 2: the gate-integrity phase no longer prints a false "names no `-p <project>`" line when config-`noCheck` disables projects — the condition now keys off `projects.length` (harvested), not `checking.length` (post- filter), so a client whose projects were all config-disabled shows only the accurate per-project lines. - Nit 3: recorded the boundary on the phase comment — it does not detect shell-level failure suppression (`… || true`, `; exit 0`). - Nit 5: the file-coverage remediation now leads with "for a co-located test, move it to `__tests__/`" (adding it to the src `include` would reintroduce the build emit) before the general add-to-include advice. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-13 — implicit-tsconfig + derived clients; diagnostics Responds to the round-13 @claude review of #1799: - Finding 1: verify-typecheck-coverage now understands a `tsc` command with no `-p`/`-b` project flag — it resolves the implicit `./tsconfig.json` (tsc's own default), so the idiomatic `tsc --noEmit` form counts as coverage instead of producing false "in no tsconfig project" failures. Also harvests `-b`/`--build` project paths. Verified `tsc --noEmit && tsc --noEmit -p tsconfig.test.json` now reports OK. - Finding 2: CLIENTS is no longer hardcoded — it's derived from the `cd clients/<x> && npm run validate` entries in the root `validate` chain (minus an explicit web exclusion), so a new Node client is auto-required, matching the repo-wide reach of its sibling verify-format-coverage. Added a guard-would-check-nothing bail if the derivation yields an empty set. Verified a new `clients/newclient` is picked up and its uncovered file flagged. - Nit 3: the phase-2 remediation now leads with the general add-to-`include` advice and only appends the "move it to __tests__/" note when a failing path is actually a test file (was misdirecting plain files). - Nit 4: AGENTS.md no longer says the test config includes "only" __tests__ (launcher's also includes its vitest.config.ts). (Two different web integration tests each flaked once under parallel load during the coverage runs — both pass in isolation and on a clean re-run; timing- sensitive and unrelated to these script/doc-only changes.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-14 — enumerate clients from disk (fail-closed) Responds to the round-14 @claude review of #1799: - Finding 1: replace the root-chain `CLIENTS` derivation (round 13) with a disk enumeration — every `clients/<name>` whose package.json has a `typecheck` script. The chain derivation was fail-open: a root-chain edit that kept running a client but didn't match the strict regex silently dropped it from the required set (112→53 files, still green). Disk is fail-closed — a client can't be silently dropped, a new client with a typecheck is auto-required, and web (no typecheck script; builds via `tsc -b`) is excluded on its own, so the explicit exclusion is gone. The existing `rootRunsClientValidate` wiring check still verifies the root chain runs each — now LOUD when it doesn't. Verified: `cd ./clients/tui && …` now hard-fails the integrity phase instead of silently shrinking coverage; `cd clients/tui && npm run build && npm run validate` stays gated. Removed the now-unused `clientsRunByRootValidate` from the shared lib. - Nits 2 & 3: documented the two unreachable-today limitations on typecheckProjects — a solution-style `tsc -b` (empty `files` + `references`) would under-report, and the implicit-tsconfig fallback assumes no file operands. - Nit 4: refreshed the file header to mention `-b`/`--build` + the implicit `./tsconfig.json` (only the JSDoc had been updated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-15 — enroll-or-exempt enumeration; normalize `cd ./` Responds to the round-15 @claude review of #1799: - Finding 1: the disk enumeration keyed on the presence of a script named `typecheck`, so renaming it silently dropped the client (112→53, still green) — contradicting the docstring. Now every `clients/*` with a readable package.json must EITHER declare a `typecheck` (→ enrolled) OR be in an explicit EXEMPT map (`clients/web`, reason: `tsc -b`); anything else is a hard gate-integrity failure. This is fail-closed on the rename axis too, and web's skip is a stated decision rather than an accident of which scripts it declares — so web gaining a `typecheck` no longer auto-enrolls it into the solution-`-b` false failures. Verified: renaming tui's `typecheck` now hard-fails; a `tsc -b` on web stays exempt. - Nit 3: `rootRunsClientValidate` normalizes a leading `./`, so `cd ./clients/tui && npm run validate` (which genuinely runs it) is no longer a false "no longer runs" alarm. - Nit 2: README/AGENTS/script-header prose no longer names a fixed cli/tui/launcher scope — they describe the disk-discovered "every clients/* with a typecheck script (web exempt)" rule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-16 — anchor wiring match; close enumeration axes Responds to the round-16 @claude review of #1799: - Finding 1 (only false-PASS): rootRunsClientValidate matched the client dir as a bare substring, so a prefix-sibling (`clients/tui` vs `clients/tui-next`) satisfied the wiring check for the shorter name — dropping a client from the root chain went green if a sibling was present. Now an anchored regex (escaped dir + optional leading `./` + a trailing boundary). Verified the sibling case now hard-fails. - Nit 2: a `clients/*` dir holding tracked TS but no readable package.json is now a gate-integrity failure (was treated as "not a client"), so the "fail-closed on every axis" claim holds. Verified. - Nit 3: readdirSync is wrapped, so a missing `clients/` dir yields the intended "found no clients/* dir" message rather than a raw ENOENT stack. - Nit 4: EXEMPT's reason is now surfaced in the success line ("clients/web exempt: typechecks via tsc -b …"), so the exemption is visible at runtime, not only in source. (Also moved `isRequiredSource` above nodeClients — nit 2's new tracked-TS check calls trackedSourceFiles at module-init, which referenced it before init.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-17 — share quote-aware tokenizer between guards Responds to the round-17 @claude review of #1799: - Finding 1: the two coverage guards parsed shell strings differently — verify-format-coverage had a quote-aware tokenize(); verify-typecheck-coverage used a raw regex. So a quoted `-p "tsconfig.test.json"` (a form tsc accepts) made the typecheck guard report every file that project covers as uncovered, and a quoted `cd "clients/tui"` in the root chain tripped a false wiring failure. Moved tokenize() (now single+double quote) into the shared lib, imported it into both guards, rewrote typecheckProjects to walk tokens (`-p`/`--project`/`-b`/`--build` → next token), and strip quotes before the `cd` match in rootRunsClientValidate. Verified both quoted forms now pass. - Nit 2: a stale EXEMPT key (a dir that no longer exists) was asserted in the success line unchecked. nodeClients now fails if an EXEMPT key isn't a real `clients/*` directory. Verified. - Nit 3 (build/dist filter belt-and-braces) left as a recorded boundary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-18 — match tsc by basename; strip quotes on both halves Responds to the round-18 @claude review of #1799: - Finding 1 (regression from round 17): `tokens.includes("tsc")` didn't match a path-invoked binary (`node_modules/.bin/tsc`), which the previous `\btsc\b` regex did — so such a `typecheck` was falsely reported as naming no project. Now matches on the token basename (`isTsc`), keeping round-17's quoting fix while restoring the tolerance. Verified a `node_modules/.bin/tsc` typecheck reports OK. - Nit 2: `rootRunsClientValidate` stripped quotes for the `cd` match but tested `npm run validate` against the unstripped string, so `npm run "validate"` false-failed. Both halves now test the stripped string. - Nits 3 & 4: documented on typecheckProjects — the contrived `--noCheck false` (checking on) is still treated as disabling, and the `&&`/`||`/`;` split runs before tokenizing (a quoted operator inside an arg would split mid-token, unreachable for project paths). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test: round-19 — typecheck test-servers' unimported bin; --prefix wiring form Responds to the round-19 @claude review of #1799: - Finding 1: `test-servers/src/server-composable.ts` (a `#!/usr/bin/env node` bin entry nothing imports) got no tsc pass — the barrel alias covers the other 13 test-servers/src files transitively, but not this one, and its own build runs `tsc --noCheck`, so its errors were emitted into `test-servers/build` unchecked. Added it explicitly to `clients/cli/tsconfig.test.json`'s include (zero source changes; verified a bad line in it now fails cli's typecheck). Softened the AGENTS.md clause that implied the whole test-server source is transitively covered. - Nit 2: `rootRunsClientValidate` now also recognizes the `npm --prefix <dir> run validate` invocation form (was `cd`-only). - Nit 3 (stderr spy drops the completion-callback overload): left as-is — latent, not live (no exercised path awaits a stderr drain), and adding typed overload params would reintroduce the write-overload complexity the round-1 vi.spyOn refactor removed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-20 — anchor validate tail; guard shared-source class Responds to the round-20 @claude review of #1799: - Finding 1 (false PASS): the wiring matcher's tail `/\brun validate\b/` also matched `run validate:fast` (`:` is a word boundary) — a DIFFERENT script that may skip typecheck. So the root chain could run a client's `validate:fast` while the guard kept measuring its `validate` (unrun), green. Anchored the tail to a boundary (`/\brun validate(?=$|[\s&;])/`). Verified `validate:fast` now hard-fails. - Finding 2: durably close the test-servers/src class instead of point-fixing. Added a SHARED_ROOTS check — every tracked TS under `test-servers/src` + the root `vitest.shared.mts` (the `format:check:shared`/`lint:shared` surface) must land in the GLOBAL union of client projects. So a NEW unimported bin entry is caught (not just server-composable.ts, named in round 19). Verified a fresh `server-orphan-bin.ts` now fails. Zero source changes — the union already covers 15/15. - Nit 4: the integrity message and rootRunsClientValidate docstring now mention the `npm --prefix` form too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-21 — deny-by-default non-client coverage Responds to the round-21 @claude review of #1799 (findings 1 & 2, one fix): - Replaced the `SHARED_ROOTS` allowlist with a deny-by-default non-client set: all tracked TS minus `clients/*` (per-client pass) minus a reasoned `core/` exemption (covered by web's `tsc -b`). This closes BOTH round-21 findings at once: - Finding 1 (stale allowlist fail-open): a rename of `test-servers/src` no longer silently drops it — its files reappear in the required "other" set and hard-fail (verified: `git mv test-servers/src test-servers/source` now fails instead of going 127→113 green). The subtractive `core/` exemption is fail-closed on staleness for the same reason. - Finding 2 (allowlist vs deny-by-default): a new top-level TS location is now auto-required (verified `tools/probe.ts` is flagged), matching the repo-wide reach of the sibling verify-format-coverage. - Nit 3: added the shared-source clause to the README table row (AGENTS got it last round). Header comment updated to describe the non-client coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-22 — narrow core/ exemption to what web's tsc -b covers Responds to the round-22 @claude review of #1799: - Finding 1: the `core/` exemption was whole-tree, but web's `tsc -b` only covers `core/**/*.ts` MINUS co-located `*.test.ts(x)` (its app project's include). So a `core/*.tsx`/`*.mts`/`*.cts` or a co-located `core` test got no tsc pass from anything. Narrowed the exemption to exactly that shape (`isExemptCoreFile`): a core `.ts` that isn't a co-located test. So a core `.tsx`/`.mts`/co-located test is now required in some project. Zero-fix today (0 such files); verified `core/mcp/probe-widget.tsx` and a co-located core `.test.ts` are now flagged. - Nit 2: `EXEMPT_ROOTS` is now a Map (dir → reason) and its reason is surfaced in the success line alongside the client exemption. - Nit 3: the non-client miss message is now the general "in no client's tsconfig project"; the `test-servers` remediation clause is conditional on an actual `test-servers/` path (tracked via `nonClientMisses`, not string-sniffing the composed message). - Nit 4: success line says "(N clients + non-client)". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-23 — core exemption mirrors both web projects Responds to the round-23 @claude review of #1799: - Finding 1: round 22's `isExemptCoreFile` mirrored only web's app project, but web's `tsc -b` also builds `tsconfig.test.json`, which includes `core/**/__tests__/**/*.{ts,tsx}`. So a `core/**/__tests__/*.test.ts` (a live layout — `core/mcp/__tests__/fakeInspectorClient.ts` already sits there) was flagged though web checks it. Now mirrors the UNION: under `__tests__/` → exempt `.ts`/`.tsx`; otherwise `.ts` minus co-located `*.test.ts(x)`. Verified a core `__tests__` test now passes while a core `.tsx` outside `__tests__` stays required. - Finding 2: `EXEMPT_ROOTS` was a Map whose keys nothing read (round 22 replaced its filter with hardcoded `isExemptCoreFile`) — a phantom registry. Collapsed to a single `CORE_EXEMPT_REASON` const used in the OK line. - Nit 3: README/AGENTS scope descriptions updated (core is no longer wholesale excluded; the non-client set is deny-by-default). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-24 — enroll clients/web via tsconfig references Responds to the round-24 @claude review of #1799: - Finding 1: `clients/web` was whole-tree EXEMPT, but web's `tsc -b` only reaches what its four referenced projects include — a file at web's package root (e.g. `playwright.config.ts`) got no tsc pass and passed EVERY gate, fully silently. This was the last whole-tree exemption wider than the gate it defers to. Now web is ENROLLED: a client with no `typecheck` script but a `tsconfig.json` `references` array is measured through those reference projects (`tsc -p <ref> --listFilesOnly`), and is wired iff its `validate` runs `tsc -b` (`validateRunsTscBuild`). EXEMPT is now the empty escape hatch. Verified: a web-root `playwright.config.ts` with a type error is now flagged, and dropping `tsc -b` from web's build hard-fails the integrity phase. Zero-fix on the tree — the union of web's 4 projects covers all 625 tracked web files. - Nit 2: assert `core/` exists (the OK line unconditionally states its exemption) — parity with the EXEMPT stale-key check. - Nit 3: a `core/` miss now gets core-specific remediation (widen web's `tsconfig.app.json` include, not a cli/tui project). - Nit 4: README/AGENTS scope descriptions corrected — web is enrolled (not exempt), and the exact core-exempt shape (app `*.ts` minus co-located tests + test `__tests__/**/*.{ts,tsx}`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-25 — drop redundant core exemption; guard tsc -b --noCheck Responds to the round-25 @claude review of #1799: - Finding 1: enrolling web (round 24) made the hand-maintained `core/` exemption both redundant and fail-open — web's measured `tsc -b` projects already cover core, and the mirror could go stale (narrowing web's include let an unimported core error pass). Deleted `isExemptCoreFile`, `CORE_EXEMPT_REASON`, the core presence assertion, and the exemptNote core clause. `core/` is now required like everything else and covered by measurement (127→894 files; a `core/*.tsx` web doesn't reach is now caught by measurement, not a hand-model). Kept the core-specific remediation clause. - Finding 2: the reference (`tsc -b`) path skipped the non-inertness check — a `tsc -b --noCheck` build gated nothing while the guard stayed green. Turned `validateRunsTscBuild` into `tscBuildStatus` (ok/neutered/none); a neutered `tsc -b` now hard-fails, matching the typecheck-script path. Factored the disabling-flag predicate (`isDisablingFlag`) so both paths share it. - Nit 3: `clientTsconfigReferences` now strips block comments too (every other tsconfig in the repo uses `/* */`). - Nit 5: README/AGENTS wiring clause covers web's `tsc -b` (not just the `typecheck` script), and core is described as measured, not exempt. Nit 4 resolves itself: EXEMPT is empty and the exemptNote/ternary now collapse cleanly. Nit 6 (tsc -b bound to a specific tsconfig) left as recorded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-26 — expand solution configs wherever harvested Responds to the round-26 @claude review of #1799: - Finding 1: the reference (`tsc -b`) path was selected only by "no typecheck script", so adding `typecheck: "tsc -b"` to web (the natural uniformity move) flipped it to the typecheck-script path, where the harvested solution config lists nothing → 665 false failures. Fixed at the measurement layer: projectFiles now expands a solution config (`{"files":[], "references":[…]}` — lists nothing under --listFilesOnly) to its references and measures those recursively, so a `tsc -b` project is measured no matter which path harvests it. Generalized the references reader (tsconfigReferences(path)). Verified web with `typecheck: "tsc -b"` now passes. Dropped the stale "web … is out of scope" JSDoc clause. - Nit 2: a `core/**/__tests__/**` miss now points at `tsconfig.test.json` (its actual owner), not `tsconfig.app.json`. - Nit 4: AGENTS.md's one-line summary now mentions the non-client (`core/`, `test-servers/src`) coverage, matching the detailed bullet. Nit 3 (EXEMPT empty, dormant stale-key check) left as the intended escape hatch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-27 — non-inertness follows references on both paths Responds to the round-27 @claude review of #1799: - Finding 1: round 26 made COVERAGE follow a solution config's references, but the non-inertness (`noCheck`) check on the typecheck-script path still only inspected the harvested solution (empty compilerOptions) — so a `noCheck` in a REFERENCED project was invisible there (caught only on the reference path). Factored a shared `resolveLeafProjects` (a solution → its real leaf projects, cached rawProjectFiles) used by BOTH coverage (`projectFiles`) and a shared `checkingLeaves` disable-check, so a `noCheck` in a referenced project is now caught on either path. Verified: `typecheck: "tsc -b"` + `noCheck` in tsconfig.app.json now hard-fails (was green). Caching also cut the guard from ~20s to ~11s. - Nit 2: directory-form references (`{ "path": "./packages/a" }`) now resolve to `<dir>/tsconfig.json`, so a nested-solution dir reference is expanded. - Nit 3: reworded the empty-listFiles comment (it's "solution config OR errored config", both of which the reference expansion handles). - Nit 4: script header notes a `tsc -b` solution is reduced to its references on either enrollment path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-28 — dedup shared leaf in checkingLeaves Responds to the round-28 @claude review of #1799 (which found no new gate hole): - Finding 1: `checkingLeaves` dedup'd the `checking` array but ran projectDisablesChecking + integrity.push on every visit, so a leaf reachable from two harvested projects produced a duplicate `noCheck` line (and a redundant `tsc --showConfig`). Now tracks visited leaves in a Set and skips an already-seen leaf entirely. Verified a duplicate harvested project yields one line, not two. - Nit 3 (record-completeness): noted in AGENTS that the one tier the guard structurally can't see — a per-file `// @ts-nocheck` — is owned by `@typescript-eslint/ban-ts-comment` across lint:core/lint:shared/each client. Nit 2 (the ~11s perf claim isn't reproducible on a CI-class runner) and nit 4 (hybrid-solution / composite boundaries, both tsc-rejected → unreachable) left as recorded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test(scripts): unit-test the typecheck-coverage guard's parsers Responds to the round-29 @claude review of #1799 (no new gate hole; the finding is that the guard itself has no tests): - Made the guard's pure helpers importable: exported `typecheckProjects`, `tscBuildStatus`, `tsconfigReferences`, `clientTsconfigReferences`, `resolveLeafProjects`, `isTsc`, `isDisablingFlag`, `isRequiredSource`, and moved the module-init + phase execution behind an exported `main()` run only when the file is executed directly (importing it for tests no longer runs the guard). Behavior when run directly is unchanged (894 files). - Added table-driven tests (node's built-in `node --test`; the root has no vitest harness by design) for the shared lib (`tokenize`, `reachableScripts` incl. pre/post hooks, `rootRunsClientValidate` across cd/--prefix/quoted/ prefix-sibling/`validate:fast`, `rootReachesScript`) and the guard's parsers (`isRequiredSource`, `isTsc`, `isDisablingFlag`, `typecheckProjects`, `tscBuildStatus`) — one case per rule a review round found (r7/r10/r13/r15/ r16/r17/r18/r19/r20/r25). 12 tests. Wired `test:scripts` into `validate`. - Nit 2: a `scripts/*.ts` miss now gets a scripts-specific remediation (keep it `.mjs`, or give `scripts/` its own tsconfig project). - Documented the guard's test suite in AGENTS.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test(scripts): make test:scripts self-guarding; fill parser test gaps Responds to the round-30 @claude review of #1799 (mutation testing killed 5/5; findings are about the new test gate + suite completeness): - Finding 1: `test:scripts` lacked the two properties the guard enforces for every tsc pass. verify-typecheck-coverage now asserts, in phase 1, that `test:scripts` is reachable from the root `validate` (via the already-imported rootReachesScript) AND that its tracked `scripts/**/*.{test,spec}.*` file set is non-empty and every file is matched by the command's glob (a new exported `globToRegExp`). So dropping it from validate, or renaming a test to `*.spec.mjs`/`*.test.mts` (which node --test silently skips), now hard-fails. Verified both. - Nit 2: extracted a pure `parseTsconfigReferences(raw)` from tsconfigReferences and table-tested the JSONC tolerance (block/line comments, trailing comma, no references, malformed, no path) — the r17/r26 rules had no cover. - Nit 3: added `-b`/`--build`/bare-`tsc -b` and explicit `--project` cases to typecheckProjects's test so its title matches its table. - Nit 4: rewrote the rootRunsClientValidate test to put `cmd` in a script reached from validate, and added the reachability cases (an orphan cd-validate must NOT count; validate:tui indirection must) — the r5 property. - Nit 5: added test:scripts to AGENTS's validate ordering + the README table. 15 guard tests now. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore(scripts): round-31 — glob harvest follows delegation; brace/? globs Round-31 review findings, both false failures in the new `test:scripts` self-guard: 1. The glob half read `rootPkg.scripts["test:scripts"]` literally while the wiring half two lines above used `reachableScripts`, so a delegating `test:scripts` (`npm run test:scripts:lib && npm run test:scripts:guard`) — which genuinely runs all 15 tests — reported every tracked test file as "not matched by the glob", advising a rename the files already satisfy. Now harvested across every script reachable FROM `test:scripts`, the same fix round 6 applied to the `typecheck` harvest; `pretest:scripts` counts for free. 2. `globToRegExp` escaped `{`/`}` as literals and left `?` as a regex quantifier, both divergent from `node --test`'s glob. `*.{test,spec}.mjs` is the natural widening here (the guard's own file scan already treats a `.spec.` file as a test) and false-failed; `?` silently meant "optional previous char" and threw outright when segment-leading. Adds `?` → `[^/]` and a brace-alternation pass, with an unbalanced brace falling back to a literal. Nit 3: `resolveLeafProjects`'s two path-resolution rules (the r26 directory-form reference, and refs resolving against the referring config's dir) extracted as `projectConfigFile`/`refToProject` so they are one-row tables like the other parsers, rather than untestable behind a `tsc` spawn. Nit 4: AGENTS.md + README document that the guard now enforces `test:scripts` on three axes (wired, non-empty, glob-covered). Verified: probes C and D exit 0; probe B (rename to `*.spec.mjs`) still caught; `test:scripts` 18/18; `verify:typecheck-coverage` OK 894 files; `verify:format-coverage` OK 919; `npm run ci` green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw * chore(scripts): round-32 — measure globs with node's own matcher Round-32 review; both findings point at deleting code. 1. `globToRegExp` hand-modeled `node --test`'s glob and was still one form short — character classes (`*[.]test.mjs`, `*.test.[mc]js`) hit the escape set and became literals, so the guard false-failed on files node genuinely runs. Replaced the whole translator (and its brace balance pre-scan) with node's `path.matchesGlob` — 22.5.0+, and the repo floor is >=22.19.0. Same *measure, don't model* move round 25 made when it deleted `isExemptCoreFile` in favor of measuring web's projects; being exact for every form node accepts is the point, since node's matcher is the thing being modeled. 2. The round-31 glob harvest was inline in `main()`, so mutation-reverting it to the literal single-script read left the suite green — the commit's headline fix was its one untested change. Extracted as `testScriptGlobs`, with rows for the direct form, the delegating form (both child globs, not `npm`/`run`/`<name>`), a `pretest:scripts` hook, and an unreachable script contributing nothing. Nit 3: `resolveLeafProjects`' JSDoc moved back onto it (the extraction left it stranded above `projectConfigFile`, which had two stacked doc blocks). Nit 4a: a `test:scripts` naming no path/glob harvested nothing, and `.some()` over an empty list blamed every test file with unfollowable advice. Now one explicit message naming the real problem. Verified: probes for the direct, delegating, brace, `[.]` and `[mc]` forms all exit 0 (the last two were false failures before); probe B (rename to `*.spec.mjs`) still caught; bare `node --test` reports the new single message; `test:scripts` 18/18; `verify:typecheck-coverage` OK 894; `npm run ci` green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw * chore(scripts): round-33 — scope the glob harvest to `node --test` segments 1. False pass. `testScriptGlobs` kept every non-flag token from every reachable script, so a glob belonging to a NEIGHBOURING command was attributed to the test runner — and since `.some()` needs only one glob to match, a broader one suppressed a real miss rather than adding an inert pattern. A `pretest:scripts` of `prettier --check "scripts/**/*.mjs"` (reachable via the pre-hook round 31 added) was enough: the probe-B rename to `*.spec.mjs` then passed while `node --test` silently ran 12 of 18 tests. Now scoped per command segment to the ones invoking `--test`, exactly as `typecheckProjects` gates on `tokens.some(isTsc)`. The JSDoc paragraph claiming a surplus token can't suppress a miss is corrected — it was false. 2. The gate-integrity block is extracted as `testScriptProblems(scripts, testFiles)`, pure and outside `main()`. The nit-4a branch added last round was the third consecutive commit whose newest branch sat outside the test seam and mutation-survived; extracting the block ends the pattern rather than pinning one more condition from the outside. Verified. The finding-1 probe (prettier pre-hook + `*.spec.mjs` rename) now exits 1 where it exited 0; the hook alone still exits 0. Mutation testing over the whole helper — empty-harvest branch to `if (false)`, segment gate removed, wiring axis removed, non-empty axis removed, segment split removed, `reachableScripts` to `Object.keys`, `matchesTestGlob` to equality — kills 7/7, including the branch that survived the last three rounds. `test:scripts` 21/21 · `verify:typecheck-coverage` OK 894 · `verify:format-coverage` OK 919 · `npm run ci` green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw * chore(scripts): round-34 — harvest only positional args; normalize `./` Both findings are the axis round 33 fixed, one level in. 1. False pass. The segment gate stopped a neighbouring COMMAND's glob, but within a kept segment the filter was still "any non-flag token", so a glob-valued flag's VALUE was harvested as a positional arg. `--test-coverage-include "scripts/**/*.mjs"` is broader than the test glob and matches a renamed `*.spec.mjs`, so it vouched for the very file the runner skips — and it isn't hypothetical: `scripts/` sits outside the ≥90 coverage gate, so that flag is the obvious next thing added here. Tokens preceded by a value-taking `--test-*` flag are now dropped. 2. False failure. `./scripts/**/*.test.mjs` — which `node --test` accepts, and the common npm-script idiom — matched nothing, because `git ls-files` emits no `./`. Both files were then blamed for a rename that never happened, the unfollowable-remediation shape fixed for the bare-runner case last round. A leading `./` is normalized off, as `rootRunsClientValidate` already does. Nit 4: the gate-integrity footer's typecheck-wiring advice is now conditioned on at least one issue being about the typecheck wiring — a `test:scripts` failure carries its own per-line remediation, and appending `--noCheck`/`tsc -b` advice under a renamed-test-file message sends the reader after the wrong thing. Verified. The coverage-include probe with the rename now exits 1 where it exited 0, and exits 0 without the rename; the `./` probe exits 0 where it exited 1; a `test:scripts`-only failure no longer prints the typecheck footer. Mutation sweep 9/9 killed, including both new rules. `test:scripts` 22/22 · `verify:typecheck-coverage` OK 894 · `verify:format-coverage` OK 919 · `npm run ci` green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw * chore(scripts): round-35 — fourth axis: flags that narrow the run 1. False pass on a new axis. The three axes ask whether the tests are run, whether there are any, and whether the runner is told about each — none asks what it then DOES with them. `--test-shard 1/2` keeps every glob intact and executes 6 of 22 tests, exiting 0, with the guard green; `--test-only` runs 2, as does a non-matching `--test-name-pattern`. This is `--noCheck` for the test runner, one domain over. Adds `NARROWING_TEST_FLAGS` and a fourth axis in `testScriptProblems`. Three of those flags were already in `VALUE_TAKING_TEST_FLAGS` — the guard knew them well enough to drop their values but not that they shrink the run. Both `--flag value` and `--flag=value` report as the bare flag name. 2. The footer conditioning added last round was itself inside `main()` — the pattern the same commit's comment documented, recurring in the commit that documented it. Extracted as `integrityAdvice(problems, nonTypecheck)`. Doing that fixes nit 3 in the same move: the old condition keyed off `testScriptIssues` specifically, so it covered one of three non-typecheck categories. A sibling-vouch failure ("the root validate no longer runs verify:format-coverage") and client-enrollment problems still got `--noCheck`/`tsc -b` advice about a typecheck pass they have nothing to do with. All three sets are now excluded. Verified. `--test-shard 1/2`, `--test-only` and `--test-name-pattern zzz` each now exit 1 naming the flag, while node runs 6/2/2 of 22. A sibling-vouch failure no longer prints the typecheck footer. Mutation sweep 12/12 killed, including all three new rules and the branch this commit replaces. `test:scripts` 24/24 · `verify:typecheck-coverage` OK 894 · `verify:format-coverage` OK 919 · `npm run ci` green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw * chore(scripts): round-36 — discover test files by content, not name 1. False pass on the axis underneath the other four. The required test-file set was built with the same name pattern the `test:scripts` glob uses, so a rename escaping BOTH was invisible: the file left `testFiles` entirely, so nothing asked whether the runner was told about it, and the survivor satisfied the non-empty axis. A dot→hyphen slip (`npm-scripts.test.mjs` → `npm-scripts-test.mjs`) dropped 6 of 24 tests with every gate green — and the file that stops running is the one pinning the r16 prefix-sibling and r20 tail anchors, the series' two other false passes. Round 30 fixed "renamed to a form the glob misses" by broadening the name pattern; a fifth name pattern only moves the boundary again. `node --test` decides what a test is by whether it registers tests, so `isScriptsTestFile` reads that instead — the same measure-don't-model move as r25 and r32. It also covers what no name pattern can: a NEW test file added under a name the glob misses, never run and never noticed. Nit 2: `enrollmentProblems` no longer suppresses the footer. "declares no `typecheck` script …" IS about the typecheck pass and the footer's first clause is its canonical fix; excluding it lost the advice for the r15 rename scenario. The criterion is "is this about the typecheck pass at all", not "does it carry per-line advice" — several typecheck problems carry their own and still want the footer. JSDoc corrected accordingly. Nit 3: the complementary-shard boundary (`1/2` + `2/2` across two reachable scripts genuinely runs everything, and is flagged anyway) recorded on the helper, alongside the guard's other documented boundaries. Nit 4: the three `--test-coverage-{lines,branches,functions}` thresholds added to VALUE_TAKING_TEST_FLAGS. Their values are numeric and so inert rather than suppressing, but model completeness is cheaper than re-deriving that later. Verified. The hyphen rename now exits 1 naming the file; the `.spec.mjs` rename is still caught; an enrollment failure prints the footer again; the guard is unchanged at 894 files OK (exactly the 2 tracked test files reference `node:test`, none of the other 12 `scripts/` files do). Mutation sweep 14/14. `test:scripts` 25/25 · `verify:format-coverage` OK 919 · `npm run ci` green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw * chore(scripts): round-37 — register-not-mention; guard the discovery read Both findings were introduced by round 36's content predicate. 1. False failure. The predicate matched a bare `node:test` mention, so a shared helper importing the runner's `mock` API — registering no tests — was enrolled as a required test file and told to rename itself to `*.test.mjs`, which would only make node run an empty file. `scripts/lib/` is already the repo's shared-helper location, so this is the plausible next file there. Tightened to node's actual criterion: an import/require OF `node:test` AND a test registration. 2. Regression. `readFileSync` was passed as an argument, so it ran for every tracked `scripts/` file before the predicate's extension gate could reject it — a file in the index but not the worktree (`rm` without `git rm`) took the guard down with a raw ENOENT stack, aborting phase 1 before any real finding printed. Discovery did no reads at all before round 36. Extension is now gated first and the read wrapped: an unreadable tracked file warns and is skipped, the round-6 precedent where a failed `tsc` echoes its diagnostic instead of dying. Verified: the mock-helper probe exits 0; the tracked-but-deleted probe exits 0 with a one-line skip warning instead of a stack; the dot→hyphen rename is still caught. Mutation sweep over the predicate 3/3 (import form, registration, extension). `test:scripts` 25/25 · `verify:typecheck-coverage` OK 894 · `verify:format-coverage` OK 919 · `npm run ci` green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw * chore(scripts): round-37 nit 3 — one definition of the JS-extension check Finding 2's fix hoisted the extension gate into `main()` (it must run before the read), leaving a second copy of the regex inside the predicate that reads as protection but can never fire there. Extracted as `isJsFile` and used in both positions, so the two can't drift. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-27 12:05:35 -04:00
Cliff Hall	cea22d86b8	feat(web): enforce localhost-only binding; expand loopback origin allow-list (#1796) ... Closes #1795. Enforce localhost-only binding in code (the "never 0.0.0.0" rule was only a spec note): refuse an all-interfaces HOST — across every spelling the OS binds as the wildcard (inet_aton decimal/octal/hex, IPv6, IPv4-mapped, zone-scoped, IDNA/Unicode) — unless DANGEROUSLY_BIND_ALL_INTERFACES=true (Docker's sanctioned opt-in). Fixes the reported stdio-connect 403 by expanding the default loopback origin allow-list to all three interchangeable forms. Hardening surfaced across an extended review: closes an IDNA/fullwidth wildcard bypass, a blank-ALLOWED_ORIGINS fail-open, and the CLI/TUI OAuth-callback wildcard bind (loopback-only now, RFC 8252); canonicalizes the origin allow-list the way a browser does; derives frame-ancestors from the real embedder list (IPv6-literal exclusion verified in headless Chromium); strictPort on all three bind points; and preserves the CLI exit-code map + JSON envelope through the mcp-inspector --cli launcher path. Shared host normalization lives in core/node/hostUrl.ts. Docs + tests + smokes throughout. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-07-26 22:15:10 -04:00
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
Cliff Hall	166ed38e7f	chore(web): format-gate the ungated first-party TS/JS (server/, vite.config.ts, scripts/) (#1788) ... * chore(web): format-gate the ungated first-party TS/JS (#1778) Prettier only covered core/ and each client's src/, leaving the web Node backend (clients/web/server/**), web top-level configs (vite.config.ts, tsup.runner.config.ts, eslint.config.js, ...), and the root build/verify tooling (scripts/**/*.mjs) ungated — so their formatting was never checked by validate/CI. eslint already reached these via `eslint .`, so this is a prettier-only gap. - clients/web format/format:check now cover src, server, and top-level *.{ts,js}, not just src. - New root format:scripts / format:check:scripts for scripts/**/*.mjs, folded into the root format and into validate:core. - Normalized the newly-covered files with prettier --write (pure formatting; no runtime change). Closes #1778 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore: future-proof the scripts format glob (#1778 review) Widen format:scripts / format:check:scripts from "scripts/**/*.mjs" to "scripts/**/*.{mjs,js,cjs,ts}" so a future non-.mjs file under scripts/ is auto-gated, matching the future-proof glob style clients/web now uses. A brace glob is a single pattern, so prettier does not error on the unmatched branches. Addresses observation #2 from the PR #1788 review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * docs: sync AGENTS.md scripts glob after the widening (#1788 review r2) d86d8f7 widened the scripts glob to {mjs,js,cjs,ts} and updated the validate:core bullet, but the format bullet at AGENTS.md:217 still said scripts/**/*.mjs. Bring it in line so the two mentions agree. 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-25 23:15:45 -04:00
Cliff Hall	07d6316fba	build(web): fail vite build on a browser-externalized Node built-in (#1769) (#1777) ... * build(web): fail vite build on a browser-externalized Node built-in (#1769) Vite 8 (rolldown) externalizes a node:* / Node built-in that reaches the browser graph with a warning and ships a `module.exports = {}` stub — the build still succeeds, shipping a broken bundle. The runtime browser smoke (smoke:web:browser, #1615) only catches the subset where that stub is *called* at module init; a stub imported but never called ships silently and nothing gates it. Add a Vite build plugin that turns that specific browser-externalization warning into a hard `vite build` error, moving detection upstream into `npm run build` / `validate` and additionally catching the never-called case the runtime smoke can't see. - clients/web/server/browser-externalized-builtin-gate.ts: Vite-agnostic detection + error + per-build state machine (unit-tested to the ≥90 gate). - vite.config.ts: thin Vite wiring. A throw inside rolldown's onLog is swallowed (the one hook where it doesn't abort — verified against vite@8.0.0), so the gate records the warning in onLog and re-throws in buildEnd. Scoped to `vite build` (apply: 'build'); the Node runner build (tsup) is unaffected. - scripts/verify-build-gate.mjs + verify:build-gate (in npm run ci and the GitHub workflow): runs a real build with a node:fs probe and asserts it fails via the gate — the only check that catches the warning phrasing drifting in a future Vite bump and silently disabling the message-keyed gate. There is no stable log `code` on the rolldown warning (only message + plugin), so the gate keys off the documented message phrasing; the drift guard above is what keeps that safe across Vite versions. smoke:web:browser stays as the runtime backstop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * build(web): harden the browser-externalized-builtin gate per review (#1769) Review follow-ups on PR #1777: - Per-build state: add gate.reset() called from the plugin's buildStart, so a `vite build --watch` rebuild doesn't inherit a prior build's recorded warning (the plugin instance is reused across rebuilds). Makes the docblock true. - buildEnd(error) now only asserts on the success path — rolldown also calls buildEnd(error) when the build already failed for another reason, and throwing then would mask that real error with the #1769 message. - verify-build-gate.mjs: drop the shell `rm -rf` cache-clear entirely (verified the gate fires reliably without it — the earlier "caching" symptom was the onLog-swallow behavior, not a stale cache); add a result.error spawn-failure check so a missing `npx` isn't misdiagnosed as "not via the gate"; add SIGINT/SIGTERM handlers so an interrupt during the multi-minute build still restores the mutated entry; add a progress log before the spawn. - Match the double-quote style of the sibling server/ files. - Docs: list browser-externalized-builtin-gate.ts in the AGENTS.md server/ block and the clients/web/README server/ list (with why a build helper lives under server/), and add verify-build-gate to the root README scripts/ line. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * build(web): round-2 review hardening for the #1769 build gate - Scope the gate to the browser (`client`) environment via applyToEnvironment, so a future SSR/node environment built from this config isn't failed for a legitimate node:* import — browser-only intent is now structural. - Report ALL externalized built-ins in one build, deduped, instead of only the first offender (N leaks no longer means N fix-and-rebuild cycles). browserExternalizedBuiltinError now takes string[]; gate uses a Set. - verify-build-gate.mjs: pin Vite with `npx --no-install` so the drift check can never silently run a registry-fetched Vite instead of the repo-pinned one (the whole point is version fidelity); append the probe instead of prepending (ES imports hoist, and appending won't demote a future leading directive); clarify the signal-handler comment (handlers are a backstop — a blocked spawnSync tears down via the process group first). - AGENTS.md: update both `ci` pipeline enumerations to include verify:build-gate and drop the now-false "Storybook is the only CI step left out" claim. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * build(web): round-3 review polish for the #1769 build gate - Error message: name the transitive-dependency remedy (a resolve.alias to a browser shim) — "remove the import / gate behind the backend" doesn't apply when the node:* import lives in node_modules. - isBrowserExternalizedBuiltinLog is now a `message is string` type guard, so recordLog drops its redundant `!== undefined` clause while still typing the Set.add() — a real one-liner, not a coverage-only removal. - verify-build-gate.mjs: docblock now states WHY it builds the real config + entry (catches config-level regressions — plugin removed, onwarn suppression added — that a temp config would miss), so the minute-long build isn't later "optimized" into a temp entry and silently lose that coverage. Guard the entry read so a renamed main.tsx fails through fail() with an actionable message. On a botched restore, save the captured original to a sidecar .bak and point there instead of recommending `git checkout --`, which would discard uncommitted edits. - AGENTS.md: fix the muddled CI-steps sentence (GitHub CI runs the whole chain as separate steps, Storybook last) and note the applyToEnvironment client-only scoping in the gate prose. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * build(web): round-4 review polish — sharper verify-build-gate diagnostics (#1769) All in scripts/verify-build-gate.mjs's failure paths: - Split the "build SUCCEEDED" failure into three diagnoses keyed off the captured output, so it names the right file: warning present but build passed → plugin not applying (removed from `plugins`, `applyToEnvironment` env-name mismatch, or an `onwarn` suppression); node:fs present but phrase absent → phrasing drift, update the constant; neither → probe never reached the graph. The old message hard-coded "phrasing drifted", which misdirects for the config-regression class this script exists to catch. Added applyToEnvironment to the docblock's config-regression list too. - Bound the build with a 10-min spawnSync timeout (+ SIGKILL); a timeout sets result.error (ETIMEDOUT) and reports via the existing branch, instead of burning to the GitHub job's 360-min default with no output. - Guard the probe write so a read-only/EACCES checkout fails through fail() with an actionable message instead of escaping as a raw stack. - SIGTERM now exits 143 (was 130, which is SIGINT's code). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * build(web): round-5 review polish — self-consistent verify-build-gate (#1769) All in scripts/verify-build-gate.mjs: - Drift guard: assert the gate source file still contains the mirrored KNOWN_PHRASE literal, failing fast if the two drift. Without it, a stale KNOWN_PHRASE after a phrasing update would make the three-way diagnosis misreport a plugin-not-applying regression as a phrasing drift — the exact misdirection the diagnosis exists to avoid. - Make the .bak safety net reachable on the path that needs it most: restoreEntry now catches its own write failure and routes to a shared saveBackupAndFail helper, instead of letting the write escape the finally as a raw stack (which skipped the .bak fallback and left the probe injected in the entry). - Add the fourth "plugin isn't applying" cause to the diagnosis: a future Vite emitting the warning before the client env's buildStart reset clears it (same symptom as the other three). - Flag once, on the failing-check path, that clients/web/dist holds a probe build so a local debugger doesn't serve it unaware. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * build(web): round-6 review polish — verify-build-gate literal contracts (#1769) All in scripts/verify-build-gate.mjs; one theme — the literals the script asserts on can each go stale independently of what they mirror: - Success key: assert on the gate's distinctive thrown-error prefix "Build failed (#1769)", not a bare "#1769". The issue number also appears in vite.config.ts comments, so a code frame from an unrelated build error there could otherwise make the check report OK with the gate dead — the one direction it must never get wrong (a false pass hiding a dead gate). - Drift guard now anchors on the BROWSER_EXTERNALIZED_BUILTIN_PHRASE assignment (regex) instead of a whole-file substring match, so the old wording lingering in a comment can't mask a changed constant; and its readFileSync of the gate module is guarded so a moved file fails through fail() actionably. - Extract PROBE_MODULE ("node:fs") used to build both the PROBE and the diagnosis check + messages, so changing the probe module can't silently misroute a phrasing-drift failure into "probe never reached the graph". - Comment the load-bearing diagnosis order (phrase before module-name). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * build(web): round-7 review polish — drift-guard both mirrored literals (#1769) - Extract ERROR_PREFIX ("Build failed (#1769)") and drift-guard it against the gate module too, and use it as the success key. Previously only KNOWN_PHRASE was guarded; the prefix is mirrored across the same TS/.mjs boundary for the same reason, so rewording the gate's thrown error would make a correctly-firing gate report "the build broke for another reason" (benign direction, but the same misdirection the other guards close). - Write the restore-failure .bak under the OS temp dir, not next to the entry: a sidecar inside src/ is untracked and a `git add -A` (likely while recovering) would sweep it into a commit. - Branch-(b) remediation now says update BOTH the gate constant AND KNOWN_PHRASE, so a phrasing-drift fix lands in one pass instead of tripping the drift guard on the next run. - Note next to BROWSER_EXTERNALIZED_BUILTIN_PHRASE that it must stay a single string literal (the script's drift guard anchors a regex on the assignment). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * build(web): round-8 review polish — safe recovery path + structural onLog order (#1769) - saveBackupAndFail no longer claims a backup it may not have written: it tracks whether the write succeeded and only points the user at the .bak in that case (otherwise it says the probe is still injected and to restore from VCS). Uses mkdtempSync for a unique dir instead of a fixed tmp filename, so a pre-existing/foreign-owned /tmp file can't make the recovery message point at someone else's content. - Guard the restore-verification readFileSync too, routing a failure through saveBackupAndFail so the whole restore path is uniformly safe (a raw throw there would skip the backup net). - vite.config.ts: enforce: 'pre' on the gate plugin so it runs ahead of the normal-plugin group in the onLog chain — a plugin whose onLog returns false filters the log for later plugins, so trailing the array would let a future log-filtering plugin silently blind the gate. Makes "the gate sees every log" structural rather than array-position-dependent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * build(web): round-9 review polish — prove the gate fired on the probe (#1769) The success key confirmed the #1769 gate fired, but not that OUR probe tripped it. If the repo already leaked a Node built-in, the gate would fire on that and the script would report OK even if the probe was tree-shaken or entryPath went stale — branch (c)'s failure silently inverted into a pass. Add an assertion that PROBE_MODULE appears among the gate's embedded offender warnings. Unreachable inside `npm run ci` (validate's build:web fails first on a pre-existing leak), but this script is a documented standalone command — run exactly when debugging a leak, the case where this mattered. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * build(web): round-10 review polish — roll back a partial probe write (#1769) The probe writeFileSync uses the default 'w' flag (truncates at open), so a mid-write ENOSPC/EIO could leave clients/web/src/main.tsx empty or half-written — and its catch called fail() directly, the one path on this script that could destroy the entry rather than merely dirty it (every other read/write is already routed into the recovery net). Call restoreEntry() in that catch to roll the partial write back (falling through to the .bak net if the restore itself can't write), and correct the comment that claimed a write failure happens "before any mutation". Also widen the enforce:'pre' comment to note it orders ahead of Vite's core plugins too (harmless — the gate defines no resolve/load/transform). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * build(web): round-11 review polish — don't let the rollback swallow the write error (#1769) Round 10's restoreEntry() in the probe-write catch swallowed the primary error: restoreEntry() on its own write failure calls saveBackupAndFail → fail → exit, so the "could not write the probe" diagnosis never ran when the rollback also failed — which is exactly the read-only-checkout / ENOSPC case (both writes fail at open). On a read-only checkout nothing was ever written, yet the developer was told the restore failed and pointed at a /tmp backup of an untouched file. Roll back inline instead: attempt the restore without exiting, track whether it succeeded, then always report the PRIMARY write error, appending "restore from version control" only when the rollback didn't take. Also pin the "Build failed (#1769)" error prefix as contiguous in a single string fragment (a comment symmetric with the phrase pin), since verify-build-gate.mjs drift-guards on it as ERROR_PREFIX. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * build(web): round-12 review polish — don't let the pin comment defeat its own guard (#1769) The round-11 pin comment quoted "Build failed (#1769)" verbatim, so the prefix appeared twice in the gate module and verify-build-gate.mjs's whole-file includes(ERROR_PREFIX) drift guard passed on the comment alone — rewording the thrown error would have kept the guard green while a correctly-firing gate reported "the build broke for another reason." Reworded the comment to not repeat the full lead (matching how the phrase pin avoids repeating its phrase); the literal now occurs exactly once, restoring the guard's meaning. Also make the probe-write recovery message factual instead of inferred: read the entry back and only warn "restore from version control" when it actually differs from the original. On a read-only checkout both writes fail at open, so the file is untouched and the old hedge needlessly implied damage. 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-25 19:45:11 -04:00
Cliff Hall	742e82515a	ci: add headless-browser boot smoke for the web client (#1615) (#1768) ... * ci: add headless-browser boot smoke for the web client (#1615) smoke:web only asserts GET / serves the SPA HTML with the injected token; it never runs the React app, so a Node built-in reaching the browser bundle (which crashes the app to a blank page at runtime, e.g. #1612) slips through. Add smoke:web:browser, which boots the same prod --web server and runs the bundle in headless Chromium (Playwright, already a clients/web devDependency), asserting the app renders its first frame (the "Add Servers" control) with no uncaught page errors — in particular no `Module "node:*" has been externalized`. This catches "Node code reached the browser bundle" as a class, not one import at a time. Wire it into the root smoke chain (and thus npm run ci / GitHub CI), and move the Playwright install/cache steps ahead of the smoke step so the new smoke reuses the cache. Update AGENTS.md and README. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * ci: address review on the headless-browser boot smoke - Don't fail on benign console.error (CDN font loads, React warnings) — the #1612 class arrives as an uncaught pageerror. console errors are now diagnostics, hard-failing only when they carry the externalization signature. - Broaden the externalization regex to non-prefixed built-ins ("fs" as well as "node:fs") and label the signature on both the pageerror and console channels. - Rename the port override to SMOKE_WEB_BROWSER_PORT so it doesn't collide with smoke:web's SMOKE_WEB_PORT. - Add a settle window (networkidle + 500ms) so a late-evaluating lazy chunk that throws is caught, and race the render against launcher death so a mid-load crash reports the real cause instead of a render timeout. - Print an actionable hint when Playwright/chromium can't launch (missing --with-deps system libs). - Extract the shared spawn/readiness boilerplate into scripts/lib/prod-web-server.mjs so smoke-web and smoke-web-browser can't drift. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * ci: fix playwright resolution in the browser smoke (CI failure) The bare `import("playwright")` resolved relative to the script's own directory (scripts/), not the cwd — so `cd clients/web` in the npm script never made it resolvable. It only passed locally because an ancestor node_modules happened to carry playwright; CI has none, so it failed with "Cannot find package 'playwright'". Resolve it explicitly from clients/web via createRequire, independent of cwd and ancestor dirs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * ci: address second review on the prod-web-server helper - waitForReady now records the last non-ok status and includes it in the timeout message, so a server that boots but persistently answers 500 (broken dist / injection failure) reports "server did not start … (last response: HTTP 500)" instead of a bare timeout that points at the wrong thing. Keeps polling on non-ok (a warming server may answer 503). - Add a child 'error' handler so a spawn-level failure surfaces as the smoke's FAILED line instead of an uncaught raw stack; readiness and the child-exit race both report it. - Drop the unused isExited/exitCode/child exports from the helper API. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * ci: reframe browser smoke around uncaught pageerror; fix stop() edge Third-review findings. 1. The `Module "…" has been externalized` string can't fire against a Vite 8 *prod* bundle — verified end-to-end against the installed vite@8.0.0: a node: import reaching the browser graph is a BUILD-time warning, and the shipped stub is a silent `module.exports = {}`. The real #1612 manifestation is the first *call* into that empty stub throwing a TypeError at module init (confirmed: injecting a used `node:fs` import fails the smoke with "readFileSync is not a function"; an unused one ships {} and is harmless). So the signature machinery was dead against the bundle it runs on — dropped it. The smoke now asserts exactly what it verifies: the prod bundle boots and paints with no uncaught page error. Reworded the script header, the AGENTS.md bullet, and the README row to match. Console errors stay diagnostic-only. 2. The helper's child 'error' handler set `exited = true`, but Node emits 'error' for a failed kill/send too (child still alive) — so `stop()` (guarded on `exited`) would skip the SIGTERM and orphan the launcher on the port. Track the error separately (`childError`); `bootFailed()` = exited || childError, while `stop()` still SIGTERMs a live child. Docs: fixed the stale AGENTS.md claim that `cd clients/web` makes `import("playwright")` resolve (createRequire does, cwd-independently; the cd is only for `npx playwright install`), and noted the shared scripts/lib/prod-web-server.mjs helper on both smoke bullets. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * ci: also hard-fail the async half of the crash class (fourth review) page.on("pageerror") fires for uncaught SYNCHRONOUS exceptions only. The async twin of the #1612 shape — the same empty-stub TypeError reached through an await/.then(), or a failed dynamic import (this app lazy-loads chunks) — lands on the console channel as `Uncaught (in promise) …` / `Failed to fetch dynamically imported module`, which were diagnostic-only, so the smoke would have passed. Hard-fail console errors matching /^Uncaught\b|Failed to fetch dynamically imported module/; every other console error stays a printed non-fatal note. The prefixes are unambiguous (a font/CDN miss reads "Failed to load resource: net::ERR_…"; React warnings never start with "Uncaught"), so this can't reintroduce the flake the earlier round fixed. Verified: an unhandled rejection at module init now fails the smoke where it previously wouldn't. Also prod-scope the AGENTS.md note that the "externalized" string isn't a runtime message (under `npm run dev` Vite's stub is a Proxy that console.warns it), and document both the sync and async failure signals. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * docs: reword smoke success line + README to two-channel wording (fifth review) After the async hard-failure signature, the assertion spans both the pageerror (sync) and console (unhandled rejection / failed dynamic import) channels. The OK console line and the README smoke row still said "page error"; match the two-channel wording already in the AGENTS.md bullet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * ci: correct the playwright-package-resolution hint (sixth review) The catch around requireFromWeb("playwright") advised `playwright install --with-deps chromium`, but that installs browser binaries, not the npm package — and the npm script already ran `playwright install` a step earlier. A failure there means the package isn't resolvable from clients/web, so the actionable fix is `npm install` at the repo root (the postinstall cascade installs clients/web's devDependencies). The chromium.launch() catch keeps its --with-deps advice (correct for the missing-system-libraries case). 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-24 19:11:46 -04:00
cliffhall	9324f9f21c	fix(pack-verify): kill the --web child on failure too (same exit-skips-finally) ... Re-review found the same process.exit()-skips-finally pattern in verifyWeb: a failure during the --web phase goes through fail() → process.exit(1), skipping its `finally { stop() }`, so the spawned web server was orphaned — left on the port, where a later run could fetch a stale false-200 and mask a regression. Track the live child in a module `webChild` and have fail() SIGTERM it before exiting, alongside the workDir cleanup; stop() clears it on the success path. Verified: a forced web-phase failure exits 1, retains the tarball, and leaves no listener on the port (child killed, not orphaned). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw	2026-07-10 18:35:46 -04:00
cliffhall	3dead1f83d	fix(pack-verify): actually clean up on failure (fail() bypasses finally) ... Re-review caught that the previous fix was inert: fail() calls process.exit(1), which skips finally blocks — so `cleanupTarball = false` never took effect and, worse, the temp work dir (a full node_modules from the real install) leaked on every failed run. Move cleanup into fail() itself — the single failure-exit point, called from everywhere including deep inside the run. On failure it removes the heavy work dir and leaves the packed tarball in place for post-mortem inspection; the success path removes both inline before the OK log. Drop the dead `cleanupTarball` flag entirely. Verified both paths: success removes work dir + tarball (exit 0); a forced failure removes the work dir, retains the tarball, and exits 1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw	2026-07-10 18:32:03 -04:00
cliffhall	c1b168e522	chore(pack-verify): retain tarball on failure (fix dead cleanup flag) ... Review follow-up: `cleanupTarball` was always true (set true at init and again in the catch), so the finally guard was dead logic. Make it meaningful — retain the packed tarball on failure so a packaging problem can be inspected, and log where it is; the happy path still removes it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw	2026-07-10 18:25:10 -04:00
cliffhall	7815548300	refactor(version): single-source the version from the root package.json ... The Inspector ships as one package with one version, but four client package.jsons each carried their own `version` and were read at runtime (cli.ts, tui App.tsx), and core hardcoded a third, stale value (0.20.0) that the TUI and web actually reported to servers. Consolidate onto the root package.json as the single source of truth. - Add core/node/version.ts: shared, Node-only readInspectorVersion() that walks up to the root package.json (always shipped in the tarball). 100% covered by a new integration test. - CLI and TUI read the version via the shared reader instead of their own manifest; TUI now also passes a real clientIdentity (was falling through to the stale core default). - Drop the `version` field from all four client package.jsons; the root keeps the only version. TUI header name/description become local display constants. - core default client identity: replace the stale 0.20.0 hardcode with a neutral 0.0.0 placeholder (real clients pass clientIdentity; web will source its version from GET /api/config, see #1639). - Stop shipping clients/cli/package.json and clients/tui/package.json (no longer read at runtime) — supersedes the earlier "ship them" fix; update pack:verify and the root README packaging invariants accordingly. - Add core/node/** to the web coverage include so the new module is gated. - Add the shared-code architecture diagram to the root README. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw	2026-07-10 17:31:21 -04:00
cliffhall	3075d0aaf3	build(publish): fix v2 single-package bundle and add pack:verify ... Prepare the single `@modelcontextprotocol/inspector` tarball for publishing and add a script that verifies the real published package end to end. - Add `scripts/pack-and-verify.mjs` (`npm run pack:verify`): build, `npm pack`, install the tarball into a clean consumer, and drive the installed `mcp-inspector` bin (web/cli/tui) — catching "works in --dev, breaks under npx" packaging/path failures the in-repo smokes miss. Local/release tool (install needs network), not wired into the fast validate/ci loop. - Disable source maps in the cli/tui/web-runner tsup configs (~halves the unpacked payload; runtime doesn't need them). - Add `clients/web/.npmignore` so `clients/web/build` (prod web-server runner) ships — npm's packlist was honoring the nested `.gitignore` `build/` entry over the root "files" allowlist, dropping the runner while `dist/` slipped through. - Ship `clients/cli/package.json` and `clients/tui/package.json`: both bundles read their own package.json at runtime (CLI client identity, TUI header), so the installed CLI crashed on connect and the installed TUI crashed on launch without them. Document the packaging invariants and pack:verify in the launcher README. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw	2026-07-10 12:36:26 -04:00
Cliff Hall	5efe9ddc7b	refactor: self-validating clients, fast validate + separate coverage/smoke (#1504) (#1505) ... * refactor(launcher): self-contained validate; split smokes into top-level step (#1504) Previously `validate:launcher` built cli/tui/web and ran the cross-client smokes, so the launcher's "validation" depended on every other client being built — and the launcher had no lint/format/coverage of its own. - clients/launcher: add a web-style `validate` (format:check && lint && build && test:coverage) with a Node-only eslint flat config, prettier, and a per-file coverage gate (parse-launcher-argv.ts covered; src/index.ts excluded as binary bootstrap). 100% on the gated file. - Root `validate:launcher` now just delegates: `cd clients/launcher && npm run validate` — no dependency on the other clients. - New top-level `smoke` step (smoke:launcher + smoke:cli + smoke:tui + smoke:web) with NO build commands; `validate` runs it last. Each client's own validation builds its bundle first, so nothing builds twice (cli/tui via test:coverage, launcher via its validate; smoke:web builds dist on demand). - clients/tui test:coverage now builds the TUI bundle (mirroring cli) so the build-free smoke has it available. - CI: replace the standalone cli/tui/launcher build step with a "Validate launcher" step. Docs updated (AGENTS.md, launcher README). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli,tui,root): every client self-validates; smoke separate from validate Extends the launcher self-validate work to the CLI and TUI, and makes the top-level `validate` nothing but the per-client validations — smokes are now a separate top-level target. - clients/cli, clients/tui: add a web-style `validate` with a Node flat eslint config + prettier. CLI lint/format pass as-is. TUI registers eslint-plugin-react-hooks for the classic rules only (rules-of-hooks + exhaustive-deps, which the source already references); the stricter react-hooks@7 rules are not enforced on the interim component surface (#1501). - Avoid double builds: - CLI's e2e spawns the built binary, so its `test:coverage` builds first; `validate` folds that in (`format:check && lint && test:coverage`, no separate `build`). - TUI tests don't spawn a bundle, so `validate` keeps an explicit `build` (for the smoke) and `test:coverage` is pure. - Root: `validate` = validate:web && validate:cli && validate:tui && validate:launcher (each delegates to the client's own validate). `smoke` is no longer part of `validate` — it's a separate top-level target run after a build/validate, with no build commands of its own. - CI: per-client "Validate X" steps + a single "Run cross-client smokes" (npm run smoke). Docs updated (AGENTS.md, cli/tui READMEs). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(root): drop redundant test aggregator scripts Now that every client self-validates and the top-level `validate` chains the per-client validations, the root `test` / `test:web` / `test:cli` / `test:tui` / `test:coverage` aggregators are redundant — run `npm run validate` (all) or `cd clients/<name> && npm run validate` (one). `test:storybook` stays (it's the one check not part of validate). CI was already using each client's own scripts, so it's unaffected. Docs updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(web): silence happy-dom infra noise in unit runs Passing unit runs printed dozens of alarming-but-expected lines that made green runs look broken — all from happy-dom performing real I/O with no server behind it: - `<iframe src="/sandbox.html">` (MCP Apps sandbox) made happy-dom fetch and later unload that URL, emitting ~40 `DOMException [NetworkError/ AbortError]` + `AsyncTaskManager destroyed` lines. Disable child-frame navigation in the unit project (the tests only assert on the iframe element/attributes, not its document), removing the noise with no coverage loss. - App-level renders read `GET /api/config` via useSandboxUrl / useServerListWritable, 404-ing ~76x. Add a benign default `fetch` (empty 200) in the unit setup; tests that exercise fetch still override it. Negative tests that assert on errors already suppress their own console output (e.g. jsonUtils/schemaUtils); CLI, TUI, and the web integration suite were already clean. Unit suite is now noise-free; 2424 web tests pass and the coverage gate is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(scripts): make validate fast (test), split coverage into its own target Each client's `validate` now runs `test` instead of `test:coverage`, so the everyday pre-push check skips v8 instrumentation (and, for web, the slower integration project). The per-file coverage gate moves to a dedicated top-level `coverage` script: coverage = coverage:web && coverage:cli && coverage:tui && coverage:launcher coverage:<name> = cd clients/<name> && npm run test:coverage `validate` still builds each client's bundle (cli via its test `pretest`, the others via an explicit `build`), so `npm run validate && npm run smoke` works unchanged. CI now runs `validate` (fast checks + builds), then `coverage` (the gate), then `smoke` — collapsing the old per-client steps. Docs updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: drop the coverage gate from CI; keep web integration as its own step CI no longer runs `npm run coverage` — the per-file coverage gate is now a local/manual check (`npm run coverage` before pushing). To avoid silently dropping the web integration suite (it only ran via test:coverage, and `validate` runs the unit project only), CI gains a standalone `clients/web` `test:integration` step that exercises the real stdio/HTTP/OAuth/storage paths without coverage instrumentation. CI is now: validate (fast) → web integration → smoke → storybook. Docs updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(smoke): give smoke:launcher its own script like the others smoke:launcher was the odd one out — an inline chain of three `node clients/launcher/build/index.js ... --help` commands while smoke:cli / smoke:tui / smoke:web each run a `scripts/smoke-*.mjs`. Extract it to `scripts/smoke-launcher.mjs`, which runs the same `--help` / `--cli --help` / `--tui --help` dispatch checks but now also asserts each prints that mode's usage banner (proving the launcher resolved the right client build) and verifies the required builds exist up front with a clear error. Prints a `smoke:launcher OK — ...` summary in the same style as the rest. Docs updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: reorder package.json scripts for readability Move frequently-run scripts toward the top and rarely-run-by-hand ones (pretest hooks, lint/format internals, etc.) toward the end, consistently across the root and client manifests. Pure reordering — no script names or values changed, and no other manifest content is touched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: drop references to removed root inspector / test:storybook scripts The reordering dropped the root `inspector` and `test:storybook` aggregators (intentional — `inspector` is redundant with `web`, and storybook is run from clients/web in CI). Update AGENTS.md so it no longer points at `npm run inspector`, and run `test:storybook` from `clients/web/`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: address PR #1505 review (format glob gap, fetch-baseline comment) - TUI format/format:check now also cover the client-root source files (index.ts, tui.tsx, dev.ts) that live outside src/ — `eslint .` already lints them, so they were linted but not format-checked. (cli/launcher have no source outside src/; their root .ts are single-quoted configs intentionally left out of prettier, matching web.) - Reword the setup.ts default-fetch comment: it overstated auto-restore (the unit project sets no restoreMocks/unstubGlobals), and now also flags that the permissive 200 default means a test asserting on a fetch FAILURE must install its own rejecting stub. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: remove redundant root install:clients script `install:clients` ran the same `node scripts/install-clients.mjs` as `postinstall`, which fires automatically on `npm install` — so the manual alias was redundant. Drop it and the doc references (AGENTS.md, spec). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): skip smoke:tui in CI (Ink needs a real TTY) This PR made CI run the full `npm run smoke`, which surfaced that smoke:tui fails in headless CI: the Ink TUI throws "Raw mode is not supported" on mount (no TTY) and exits before rendering its first frame. The old CI never ran smoke:tui, so this was latent. smoke-tui.mjs now self-skips when `process.env.CI` is set — the boot/ render check is inherently local-only (a TUI needs a terminal). The TUI is still built and unit-tested in CI; `npm run smoke` stays the single command everywhere (full locally, tui-skipped in CI). Verified `CI=true npm run smoke` is green (launcher/cli OK, tui SKIPPED, web OK). Docs + CI comment updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-21 11:41:33 -04:00
Cliff Hall	e72d4307f3	feat(cli): lift config-file headers/timeouts/OAuth into serverSettings (#1482) (#1500) ... * feat(cli): lift config-file headers/timeouts/OAuth into serverSettings (#1482) The CLI resolved file-sourced servers via resolveServerConfigs(), which returns bare MCPServerConfig and dropped disk-level headers, timeouts, and OAuth — CLI connections from a --catalog/--config file silently ignored them. The TUI already lifted these via mcpConfigToServerEntries(). Extract the TUI's catalog/config -> per-server-settings resolution into a shared core module (core/mcp/node/servers.ts: loadServerEntries + selectServerEntry) and route both CLI and TUI through it, so the two share one file->settings path. --header still overrides the file's headers for the run while preserving the file's timeouts/OAuth. - core: add loadServerEntries/selectServerEntry/headersToServerSettings - tui: tui-servers.ts re-exports the shared core helpers (no logic dup) - cli: resolve via loadServerEntries + selectServerEntry; drop the CLI-only conflict check and bare-config path - tests: core servers.test.ts (settings lifting, header merge/override, stdio overrides, seed/error/conflict, selection); CLI integration tests asserting a config header is sent over HTTP and --header overrides it - docs: CLI README note on file settings being applied Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(core): address #1500 review — dedup overrides, drop dead wrapper, clearer error, spec refresh - Remove the now-unused resolveLaunchServerConfigs() wrapper (both CLI and TUI route through loadServerEntries now); migrate its default-catalog launch tests to compose withDefaultCatalogPath + resolveServerConfigs. - Deduplicate applyStdioOverrides against config.ts applyOverrides: export the latter (now trims cwd) and reuse it in servers.ts. - selectServerEntry's unknown-name error is source-agnostic ("not found" not "not found in config file") since entries may be a single ad-hoc default, where "config file" would mislead. - Refresh specification/ docs (v2_cli_tui_launcher.md, v2_catalog_launch_config.md): CLI/TUI resolution now via loadServerEntries/selectServerEntry; mark gap G1 resolved (#1482); withDefaultConfigPath → withDefaultCatalogPath. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cli): extend smoke:cli to cover #1482 lift→serverSettings Adds end-to-end smoke coverage for the config-file headers/timeouts/OAuth lift introduced in #1482: - unknown --server now errors source-agnostically ("not found") - multi-server catalog selection via --server (and the omit-it error) - --header merge does not break the stdio connect path - HTTP transport: a config-file `headers` object is lifted onto the wire, and a CLI --header overrides it (verified against an in-process test server's recorded requests via a non-blocking spawn) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-20 11:33:21 -04:00
Cliff Hall	73b651ff21	feat(cli,tui): normalize --catalog / --config with web; update --help (#1347) (#1499) ... * feat(cli,tui): normalize --catalog / --config with web; update --help (#1347) Bring the CLI and TUI in line with the web catalog vs read-only session model introduced in #1481/#1483, and close the outstanding --help item from #1347. Core (core/mcp/node/config.ts): - Add `catalogPath` to ServerConfigOptions (writable catalog slot). - `resolveServerSource()` picks the active source: `--catalog` (writable, seed-if-missing) wins over `--config` (read-only, error-if-missing). - Writable sources seed an empty `{"mcpServers":{}}` on first run (matches the web backend) instead of erroring; read-only `--config` still errors. - `serverSourceConflict()` shared conflict matrix (mirrors run-web): --catalog/--config mutually exclusive; neither combines with an ad-hoc target. - `readServerListFile()` public loader for runners (TUI) needing the raw map. - Rename `withDefaultConfigPath` → `withDefaultCatalogPath`; the no-flag default now fills the writable catalog slot, not --config. CLI / TUI: - Add `--catalog <path>` (+ MCP_CATALOG_PATH) and re-document `--config` as a read-only session in --help. - Enforce the shared conflict matrix before resolution. Full writable persistence/CRUD threading remains deferred to #1482 / #1432 (CLI/TUI are read consumers today; the split surfaces as seed- vs error-on-missing). Tests: core resolution matrix (source/conflict/seed/read-only), CLI catalog + conflict integration tests, TUI loadTuiServers unit tests. READMEs updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(launcher): add smoke:cli + smoke:tui e2e launcher smokes (#1347) The launcher smokes only checked `--help` for --cli/--tui; nothing exercised the launcher → CLI/TUI → core path on the built artifact (only smoke:web did, for web). Add two end-to-end smokes wired into validate:launcher between smoke:launcher and smoke:web: - smoke:cli (scripts/smoke-cli.mjs): runs `--cli` against the bundled stdio test server via a temp --catalog and asserts tools/list returns the tools, plus the #1347 resolution paths — default-catalog seed-on-missing, read-only --config error-without-seed, and the --catalog/--config conflict. - smoke:tui (scripts/smoke-tui.mjs): launches `--tui --catalog <temp>` and asserts the Ink app renders its first frame ("MCP Servers") within a timeout, then SIGTERMs it (shallow boot/render check). Both build test-servers/build on demand if missing. Docs updated (AGENTS.md validate description, launcher README). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: address review on #1499 (comment accuracy, CLI/TUI flag parity) Review feedback from @claude on PR #1499 (all non-blocking): - Align CLI catalogPath resolution with the TUI: `options.catalog?.trim() || process.env.MCP_CATALOG_PATH` so an explicit empty `--catalog ""` falls back to MCP_CATALOG_PATH identically in both front-ends (CLI used `??`, which only guards undefined/null). - Clarify the `serverSourceConflict` comment: it mirrors only the source-selection portion of web's matrix and deliberately omits web's `--header` rejection, since CLI/TUI merge --header into per-server settings. - Note the deliberate --header broadcast semantics in loadTuiServers. - Share an EMPTY_CATALOG_CONTENT constant between seedEmptyCatalog and readServerListContent instead of two separate literals. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli): trim --config to match --catalog and the TUI (#1499 review) Cosmetic uniformity follow-up from the re-review: configPath now uses `options.config?.trim() || undefined`, matching the catalog slot above it and the TUI. Behavior is unchanged (resolveServerSource/serverSourceConflict already trim downstream). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-19 17:30:26 -04:00
Cliff Hall	36b48e61a2	feat(web): build dist on demand for prod --web + end-to-end CI smoke (#1486) (#1497) ... * feat(web): build dist on demand for prod --web + end-to-end CI smoke (#1486) Prod `mcp-inspector --web` serves static assets from clients/web/dist, which only exists after a build. In a fresh checkout it was absent and the prod path served a broken page (ENOENT on index.html); the launcher smoke test only covered --help, so this path was unverified in CI. - ensure-web-build.ts: build dist on demand (vite build via build:client) when missing, or exit with an actionable error if the build can't run. - run-web.ts: invoke ensureWebBuild before startHonoServer in the prod path (--dev runs Vite directly and skips it). - scripts/smoke-web.mjs + smoke:web: start --web against built dist and assert GET / returns 200 with the injected __INSPECTOR_API_TOKEN__. Wired into validate:launcher and a new CI step. - Docs: launcher README + AGENTS.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): surface spawn error in build-on-demand; clarify smoke comment Address PR #1497 review: - runViteBuild now logs result.error.message (e.g. ENOENT when npm isn't on PATH) before returning a null status, so the spawn-failure cause surfaces instead of only the generic actionable error. The build seam gains a `log` param; added a test covering the ENOENT path. - Reword scripts/smoke-web.mjs header so it no longer implies the readiness wait covers a cold build-on-demand vite build (it's sized for a pre-built dist, which validate/CI guarantee). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-16 18:34:52 -04:00
Bob Dickinson	2488ca5e38	feat: port CLI, TUI, and launcher from v1.5 into v2 (#1452) ... Ports the CLI, TUI, and launcher from v1.5 into v2 behind a single `mcp-inspector` entrypoint (launcher → web/cli/tui). Includes the `mcp-inspector --web --dev` launcher fix (Vite alias resolution + prod-server double-start), the root `postinstall` install cascade with simplified CI, and spec/AGENTS docs (no-workspaces rationale, Development setup). Closes #1246. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-14 20:27:49 -04:00
cliffhall	442bb68532	Remove all from v2/main	2025-11-28 13:49:31 -05:00
Jerome	a30a3fbe8d	Lint	2025-06-14 19:56:58 +01:00
Jerome	b70cf563a8	feat: add version management scripts and CI check ... - Added update-version.js script to automate version bumps across all packages - Added check-version-consistency.js script to verify version consistency - Integrated version check into CI workflow - Scripts check package.json versions AND package-lock.json sync - Added npm scripts: `npm run update-version` and `npm run check-version` - Added documentation in scripts/README.md This ensures version consistency across the monorepo and prevents accidental version mismatches or outdated lock files. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>	2025-06-14 19:52:36 +01:00