`lint` failed only on errors, so a warn-level rule was a channel nothing
consumed. `react-hooks/exhaustive-deps` reported two stale-closure
`useCallback` deps in App.tsx on every run, `npm run ci` passed anyway,
and the defect was caught only by a review round on #2076.
Add `--max-warnings 0` to all six lint scopes — each of
clients/{web,cli,tui,launcher}'s `eslint .`, plus the root's `lint:core`
and `lint:shared` — so a warning fails `lint`, and therefore `validate`
and `ci`. All six were already warning-free, so no suppressions, rule
downgrades, or ignore-list widening were needed.
Also promote `react-hooks/exhaustive-deps` to `error` in both React
scopes (web, tui). The flag makes severity irrelevant to the gate; it
does not make it read correctly in an editor.
Document the invariant in AGENTS.md as a new "Lint has no warning tier"
section, note it in the README beside the type-aware-linting paragraph,
and mirror it into .github/copilot-instructions.md.
Signed-off-by: cliffhall <cliff@futurescale.com>
A floated promise is invisible at review time — the call reads like an
awaited one minus four characters — and the unhandled rejection it
produces surfaces in a different test, in a different file, as a stack
pointing at SDK internals. In #1947 two un-held `callTool` calls made
`npm run ci` unpassable and took a full investigation to attribute; the
fix was two lines. `@typescript-eslint/no-floating-promises` catches the
class outright, so enable it at `error` everywhere.
The rule is type-aware and `tseslint.configs.recommended` is not, so each
scope's config gains a parser project. The four clients name their own
tsconfig projects (both of them — `src` and `__tests__`/`server` live in
different ones). The root scope has no tsconfig at all, so add
`tsconfig.lint.json`: a lint-only project covering `core/**`,
`test-servers/src/**`, and `vitest.shared.mts`, emitting nothing and
gating nothing. It sets `moduleResolution: bundler` deliberately — under
NodeNext `core/`'s extensionless imports fail to resolve, every import
degrades to `any`, and the rule silently stops seeing promises at all.
Measured surface, and the fixes:
- core (2) — `stream.close()` in two Hono `onAbort` listeners. Hono
invokes subscribers with a bare `subscriber()`, so a promise returned
from the listener is floated one layer up; `closeAbortedStream` owns
the rejection where it can actually be owned.
- tui (5) — `handleDisconnect` had no catch of its own and its only
caller is a key handler that cannot await, so a failing disconnect was
a live unhandled rejection; it now surfaces the message the way
`handleConnect` does. The rest are `void` at callers that genuinely
cannot await, over callees that already own their failures.
- web (92) — 83 are Storybook `expect(...)`, which is instrumented and
returns a promise, now awaited (`scrollAreaStoryAssertions` is async
for the same reason). Two tests were asserting against a provider call
they never awaited. `open(url)` in both web servers now warns instead
of escaping a synchronous callback.
- cli, launcher — already clean.
Type-aware linting costs time: `clients/web`'s `eslint .` goes from ~8s
to ~19s, inside `validate`. Narrowing the loaded projects is the lever if
that needs reducing later; dropping the rule is not.
Closes#1959
Signed-off-by: cliffhall <cliff@futurescale.com>
* 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#1785Closes#1787Closes#1789Closes#1790Closes#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>
* 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>
cliffhall