| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
cliffhall
|
4fd7f39acf |
docs: state that build output is never a lint/format/typecheck target
Adds an AGENTS.md rule under the pre-push gate naming the gated surface
(first-party source) and everything excluded from it (each client's build/,
web's dist/ and storybook-static/, coverage/, test-servers/build,
core/**/{build,dist}, *.tsbuildinfo), why gating generated output is harmful
even when it only warns, and the fact that the three coverage guards assert
the converse and so cannot catch this drift. Mirrored into
.github/copilot-instructions.md per the maintenance rules.
Signed-off-by: cliffhall <cliff@futurescale.com>
|
||
|
cliffhall
|
519ecf49ee |
chore(lint): enable no-floating-promises in all five ESLint scopes
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> |
||
|
cliffhall
|
7f9be24bf3 |
chore(scripts): derive dep-lockstep candidates from what enters each tsc program
`verify:dep-lockstep` derived its candidate set from the packages the shared first-party sources named *directly*, so a package whose declarations reach a program only through another package's `.d.ts` was invisible to it — it could skew across installs and recreate the two-copy blow-up #1896 was about while the guard stayed green. `@modelcontextprotocol/sdk` was the live example: never written in first-party code, yet 16 of its `.d.ts` files land in clients/web's test program. Replace the derivation with the measured one: list every client tsconfig project with `tsc --listFilesOnly`, map each resolved `node_modules` file to its owning install and package, and keep the packages that reach ONE program from TWO installs. That is exactly the set that can put two structurally distinct copies of a type in front of one checker. The rest of the guard is unchanged — top-level lockfile comparison, deny-by-default, `TOLERATED_SKEW` as a name allowlist scoped to within-a-major. The listing machinery moves to `scripts/lib/tsc-program.mjs`, shared with `verify:typecheck-coverage`, so the two guards can't disagree about what a program contains. It is deliberately not cached to disk between the two processes: a stale fingerprint would make a guard measure a program that no longer exists and pass on a real miss. The second pass costs ~14s. `TOLERATED_SKEW` is now empty. `jose` and `@modelcontextprotocol/ext-apps` are root-only since #1970 and cannot skew; `react` (whose types are `@types/react`, resolved from one install) and `hono` never reach one program from two installs. Each would be a rationale for a skew that cannot occur. Closes #1965 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
Cliff Hall
|
372403d9cc |
Merge branch 'v2/main' into dependabot/github_actions/v2/main/github-actions-d1cfab875f | ||
|
Cliff Hall
|
532ad82791 |
Merge pull request #1982 from modelcontextprotocol/v2/docs/1979-dco-signoff
docs: document the DCO signoff requirement |
||
|
Cliff Hall
|
f9d893f3d7 |
Merge pull request #1986 from modelcontextprotocol/v2/fix/1984-mantine-transition-timer-leak
fix(test): clear leaked timers so one cannot fail an unrelated CI run |
||
|
cliffhall
|
b451949c9e |
docs: the renderWithMantine rule no longer rests on timer safety
Review caught a second-order effect of the fix: making the leaked-timer net global invalidated the rationale of a neighbouring rule. "Do not hand-roll a bare MantineProvider — that reintroduces the leak class" stopped being true the moment the net moved into setup.ts, which covers every unit test regardless of how it renders. The rule itself is still right, so keep it and replace the justification: a hand-rolled provider skips the project theme and the helper's options and drifts from every other test. Both copies say explicitly that the old reason no longer holds, so nobody re-derives it. Mirrored into .github/copilot-instructions.md in the same PR, as AGENTS.md requires for review-relevant guidance — it carried the identical stale claim. That makes four places this one belief had propagated to. A fix can create stale docs by making a previously-true statement false, which is easy to miss when only the changed lines get read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
94a227b9c1 |
fix(tui): widen the root react range so an external ink can't split React
Review follow-up on #1983. Copilot was right, and about a hazard this repo had already proven and then documented away: the claim that ink's ">=19" peer "keeps npm from hoisting it next to a React the Inspector couldn't also use" is false. A consumer pinning React 19.0 satisfies ">=19" while `^19.2.4` nests underneath, so ink renders through their React and the bundle through ours — the #1952 crash, one level up, breaking TUI startup rather than just its forms. Bundling ink closes it (verified: with a `createRequire` banner for the inlined CJS, since react-reconciler calls `require("react")` and signal-exit@3 `require("assert")`), but costs ~1.4MB — react-reconciler and yoga-layout come along. The root `react` range closes it for free instead: `^19.0.0` is open to the whole major, so npm can satisfy our React and a consumer's pinned React 19 with one copy, and an external ink resolves the same copy the bundle does. Verified against both consumer trees — the reporter's (React 18 + ink 5) and a pinned React 19.0.0 + ink 6.8.0, which splits under `^19.2.4`. That makes the range load-bearing while looking like an ordinary version bump, so the tests pin it: the root range must equal ink's own declared peer floor. They also assert each exempt package is both external and a root dependency — external means consumers install it, the mirror image of the inlined ones, which must not be root-declared. The docs in all four places now say ink is exempt on cost and never on a peer range, and name that claim as one we got wrong. Closes #1952 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLYmVPVLdeYpE6aMTUsCks |
||
|
cliffhall
|
c158ad809f |
fix(docs): correct the DCO rules against the validator source
Round 5 was right on all three counts. Read dcoapp/app/lib/dco.js this time
rather than the README, which is where the earlier errors came from:
const authors = [commit.author.name, commit.committer.name]
const emails = [commit.author.email, commit.committer.email]
if (isMerge || ...) continue // parents.length > 1
else if (author && author.type === 'Bot') continue
So, corrected:
- Scope. Merge commits and bot-authored commits are skipped, not gated. "Every
commit" overstated it and could prompt a needless history rewrite.
- Matching. The trailer may match the author OR the committer, not the author
alone. A cherry-picker's own signoff is accepted.
- Certification. The DCO expressly covers submitting work created by others
that you have the right to submit, so signing off a cherry-pick is legitimate.
My "never sign off someone else's commit" would have told a maintainer not to
do a correct thing. The real prohibition is narrower: never fabricate a
trailer in another person's name.
This also retracts the mechanical argument in
|
||
|
cliffhall
|
bd6c1072bf |
fix(docs): drop the signoff hook — it cannot know the commit author
Round 4 was right that the hook signs with the configured identity while the DCO check requires a trailer matching the commit *author*, and that the two diverge on cherry-pick and `git commit -c`. I tried the suggested author-aware guard and it does not work: inside prepare-commit-msg, `git var GIT_AUTHOR_IDENT` returns the config identity, not the preserved author. Verified end to end — cherry-picking Bob's commit as Alice yields author=Bob, committer=Alice, `Signed-off-by: Alice`. That commit both still fails the gate and certifies work Alice did not write, which is the one thing the last bullet of this section forbids. Three review rounds have now found three holes in this hook, each fix opening the next. That is a wrong mechanism rather than an unfinished one, so the hook is out. A subtly-broken hook in the file that governs how this repo is worked on is worse than no hook. The bullet now recommends `git commit -s` and names BOTH false automations — `format.signOff` and the hook — recording the cherry-pick finding so the next person doesn't rediscover it, and pointing anyone who wants automation at a layer that actually knows the author. Mirror updated to match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
eb5bba62ed |
fix(tui): bundle React-rendering deps so a consumer install can't split React
`ink-form` and `ink-scroll-view` were externalized by the TUI bundler and
both declare a loose `react` peer (">=18"). Installed under a project that
has its own React 18, npm hoists them to that project's root — where the
peer is satisfied — while the Inspector's React 19 nests underneath. The
bundle then renders through React 19 while those two call hooks on React
18, whose dispatcher is null, so opening a tool test form (or any scroll
view) dies with "Cannot read properties of null (reading 'useState')".
Inline them instead: their `import "react"` is emitted into the bundle and
so resolves from the build directory exactly like the bundle's own, with
npm's placement no longer part of the decision. This also pins their
transitive deps to what this repo resolved — notably `ink-select-input@6`
via `overrides`, which npm ignores for a package installed as a dependency,
so consumers were getting the React-18-era v5.
`ink` stays external: it can't be bundled (its CJS `signal-exit@3` fails
ESM interop with `Dynamic require of "assert"`) and doesn't need to be —
its ">=19" peer keeps npm from hoisting it next to an unusable React.
Both packages are dropped from the root `dependencies`, since the tarball
now ships their code rather than having consumers install them.
`clients/tui/__tests__/tsupConfig.test.ts` is the durable guard: the
failure is invisible in this repo and in every smoke (a dev install has
one React) and only appears once the package is installed under another
project, so the invariant is asserted against the config instead — every
dependency declaring a `react` peer must be bundled unless exempted.
Closes #1952
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLYmVPVLdeYpE6aMTUsCks
|
||
|
cliffhall
|
b3242bc04c |
fix(docs): format.signOff does not sign commits, and remediation is unavailable
Both review points were factually right and my original text was wrong. 1. `git config format.signOff true` — which I had recommended as the *better* option — only defaults the -s flag for `git format-patch`. `git commit` never reads it, and there is no `commit.signoff` equivalent. Verified all three in a throwaway repo: format.signOff → no trailer; commit.signoff → no effect; a prepare-commit-msg hook → works. Shipping that advice would have been worse than saying nothing, since it looks like a fix and silently changes nothing. Now recommends `git commit -s`, gives the verified hook for automation, and names format.signOff explicitly as a trap — the name invites exactly the mistake I made, so it is worth warning about rather than omitting. 2. The DCO app's empty remediation-commit flow needs `allowRemediationCommits.individual`. This repo ships no DCO config, so it runs with that disabled and the original unsigned commits keep failing. The escape hatch I offered does not exist here; rewriting is the only repair, so the text now says to coordinate before rewriting shared history instead. Mirror updated with both corrections. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
f4529df219 |
docs: document the DCO signoff requirement
The DCO check fails any PR with an unsigned commit, and the rule appeared nowhere in the repo — not AGENTS.md, CONTRIBUTING.md, README.md, or the Copilot mirror. A hard merge gate that is invisible until it fails, whose remedy is a history rewrite, is exactly what AGENTS.md exists to capture. It blocked #1978. Added to the "When work is complete" list beside the other PR-hygiene gates, weighted toward prevention over recitation: `git config format.signOff true` once per clone beats remembering `-s`, because the failure is only discovered after pushing. The repair path (`git rebase --signoff` + `--force-with-lease`) carries the rebase caveat and the empty-remediation-commit alternative for when you are not the sole author, plus a note that the signoff is a DCO assertion in the author's own name and is never applied on someone else's behalf. Mirrored into .github/copilot-instructions.md in the same PR, as AGENTS.md requires for PR-hygiene rules — two lines next to `Closes #N` and the version label, since the mirror is read on every review and is a distillation, not a copy. Not added to CONTRIBUTING.md: external contributors file issues rather than PRs, so DCO never reaches them, and documenting it there would muddy that. Closes #1979 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
f060c97e8f |
docs: mirror the smoke-isolation invariant into copilot-instructions
AGENTS.md requires review-relevant changes to land in the Copilot mirror in the same PR, and the catalog-isolation rules are review-enforceable: a new web smoke that spawns its own server, or a teardown that removes a work dir without first awaiting stopChild, is something a reviewer should flag. Mirrored as one line under "Gates and PR hygiene" rather than the AGENTS.md paragraph. The mirror is read on every review and its own rule is "a distillation, not a copy" — the mechanism (why the 409 appeared, why only the catalog and not HOME is redirected, how the contract is tested) belongs in AGENTS.md, while what a reviewer needs is the invariant and the two shapes that violate it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
Cliff Hall
|
59f5b6d25c |
Merge pull request #1971 from modelcontextprotocol/v2/chore/root-only-mcp-sdk-deps
chore: declare the MCP SDK packages only at the repo root (#1970) |
||
dependabot[bot]
|
df399272f2 |
build(deps): bump the github-actions group across 1 directory with 5 updates
Bumps the github-actions group with 5 updates in the / directory: | Package | From | To | | --- | --- | --- | | [actions/checkout](https://github.com/actions/checkout) | `6` | `7` | | [actions/setup-node](https://github.com/actions/setup-node) | `6` | `7` | | [actions/cache](https://github.com/actions/cache) | `4` | `6` | | [docker/setup-qemu-action](https://github.com/docker/setup-qemu-action) | `3` | `4` | | [docker/build-push-action](https://github.com/docker/build-push-action) | `6` | `7` | Updates `actions/checkout` from 6 to 7 - [Release notes](https://github.com/actions/checkout/releases) - [Commits](https://github.com/actions/checkout/compare/v6...v7) Updates `actions/setup-node` from 6 to 7 - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](https://github.com/actions/setup-node/compare/v6...v7) Updates `actions/cache` from 4 to 6 - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](https://github.com/actions/cache/compare/v4...v6) Updates `docker/setup-qemu-action` from 3 to 4 - [Release notes](https://github.com/docker/setup-qemu-action/releases) - [Commits](https://github.com/docker/setup-qemu-action/compare/v3...v4) Updates `docker/build-push-action` from 6 to 7 - [Release notes](https://github.com/docker/build-push-action/releases) - [Commits](https://github.com/docker/build-push-action/compare/v6...v7) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major dependency-group: github-actions - dependency-name: actions/setup-node dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major dependency-group: github-actions - dependency-name: actions/cache dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major dependency-group: github-actions - dependency-name: docker/setup-qemu-action dependency-version: '4' dependency-type: direct:production update-type: version-update:semver-major dependency-group: github-actions - dependency-name: docker/build-push-action dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major dependency-group: github-actions ... Signed-off-by: dependabot[bot] <support@github.com> |
||
|
Cliff Hall
|
16b9eff8f4 |
Merge pull request #1962 from modelcontextprotocol/v2/chore/zod-version-lockstep
chore(deps): align zod across installs and guard against version skew |
||
|
cliffhall
|
5a3a3436ea |
docs: correct the dependency-classification claims
Copilot review round 2, four findings, all factual errors in the docs added by the previous commit: - `yaml` is in the root `dependencies`, not `devDependencies`, and its only importer is `test-servers/src/load-config.ts` — not `core/` or the root tooling, as the `vitest.shared.mts` comment claimed. - The rule as written told a reader to put anything reached through root-owned code — `core/` included — in `devDependencies`. That is wrong and would break the published package silently: the client builds externalize npm packages, so a published install resolves them from the root manifest, where devDependencies are absent. Placement (which manifest) and classification (which section) are now stated as separate questions, the second following from who consumes the package at runtime. `yaml`'s runtime classification is left alone and called out instead: moving it changes what ships, which is not a docs change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 |
||
|
cliffhall
|
e132d551e6 |
docs: record where a dependency is declared, and why
Per the maintenance rules, a change to the dependency layout has to land in the docs with it. Adds the placement rule in three places, at three altitudes: - README (Setup): the rule plus the concrete drift it prevents. - AGENTS.md: a "Dependency placement" section — the MCP SDK packages are root only, the v1 SDK is not ours and must not become a dependency, and anything reached solely through root-owned code with no manifest (`test-servers/src`, `core/`) is a root devDependency aliased to the repo root in `vitest.shared.mts`. - .github/copilot-instructions.md: the distilled, reviewer-citable form, so a diff re-adding one of these to a client manifest is flagged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 |
||
|
cliffhall
|
719da6fbdd |
ci: gate claude.yml on same-repo PRs and pin actions to commit SHAs (#1882)
Two pieces of hardening on the Claude Code workflow, plus a comment for a finding that needed no change. Same-repo gating. `Get PR details` now emits an `is_fork` output, and the PR-head checkout and the `Run Claude Code` step are both gated on it. A fork PR is declined outright — no checkout, no agent run, with the reason written to the step summary — rather than falling through to a "metadata-only" review of the base tree, which would trade an untrusted-code problem for a wrong-tree one. A deleted fork (`head.repo` null) counts as a fork. The head checkout also drops its `repository:` input: only a same-repo head reaches that step now, and checkout's default `github.repository` is a value no PR can influence. SHA pins. actions/checkout, actions/github-script, and claude-code-action were all on mutable major tags in a job that holds ANTHROPIC_API_KEY and grants the agent Bash. Each is now pinned to a full commit SHA with the trailing `# vX.Y.Z` comment Dependabot reads, so upgrade automation is unaffected. Pinned to the v7 / v9 SHAs so this does not regress the bumps in the open Dependabot PR #1922. The failure fallback Copilot flagged turns out not to exist: `Checkout repository` carries no status-check function, so GitHub applies an implicit `success()` and skips it after a failed lookup regardless of the `outcome` comparison. Its condition is now the explicit `== 'skipped'` and says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW1p8E2uiyyLx4RKwrwSrt |
||
|
cliffhall
|
35317fd9ed |
chore(deps): scope the allowlist to a major, cover vitest.shared.mts, fix the vouch comment
All four suppressed comments from Copilot's third pass of #1962. Each was correct. The candidate boundary omitted `vitest.shared.mts` — root-owned, imported by every client's vitest config, and already treated as shared non-client source by `verify:typecheck-coverage`. It imports only Node built-ins today, which is exactly why the omission would have gone unnoticed until a third-party import appeared there, resolved from the root, and skewed. Shared *files* are now a first-class list beside the shared dirs. The allowlist was a blanket exemption by name. Each entry's rationale establishes that a patch/minor difference is benign, which is no evidence that a React 18-vs-19 or Hono 4-vs-5 split would be — that is a different type surface. Being listed now tolerates skew only *within a major version*; a cross-major skew fails like anything else. Deriving the major from the lockfile version keeps this automatic, with no per-package range to maintain and bump. A version that can't be parsed is treated as "cannot prove same major" and fails rather than passes. `verify-format-coverage.mjs`'s comment overstated the vouching graph: the three guards do not each check the others. This one checks both siblings; each sibling checks only this one. Described as it actually works. The AGENTS.md rule said to bump a shared dependency in "root + all four clients", but launcher declares no zod and `findSkew` ignores installs where a package is absent — so the rule as written asked for spurious dependencies. Reworded to every install that *declares* it, and mirrored in `.github/copilot-instructions.md`. The failure output follows: it no longer advises adding a package that is already allowlisted, saying "MAJOR skew" instead, and it names all three shared sources rather than the two dirs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 |
||
|
cliffhall
|
6c33da38c9 |
chore(deps): align zod across installs and guard against version skew
Closes #1896 `clients/web`'s `tsc -b` exhausted the 4GB default heap at zod 4.4.x. The cause is not a zod regression nor a pathological inference site of ours: it is two copies of zod at *mismatched versions* inside one tsc program. `clients/web/tsconfig.test.json` compiles `test-servers/src`, whose files resolve zod (and the SDK's zod) from the **root** install, alongside web's own sources, which resolve from `clients/web/node_modules`. At 4.3.6 vs 4.4.3 TypeScript must relate two structurally-distinct declarations of every `@modelcontextprotocol/*` schema type, which is exponential over a recursive generic surface — `TS2589 Type instantiation is excessively deep`, then OOM. Bumping root zod to 4.4.3 and changing nothing else returns the build to baseline: 11.4s and 2,567,882 instantiations, against 11.2s / 2,567,503 at 4.3.6. Two copies at the same version are harmless; skew is what explodes. So drop the `~4.3.6` hold on `clients/web` and move all four manifests and lockfiles to zod 4.4.3 together, and add `verify:dep-lockstep` to `validate` to keep them there. The guard derives its candidate set from what `core/` and `test-servers/src` import, compares the committed lockfiles' top-level entries, and fails deny-by-default on any skew not in the annotated `TOLERATED_SKEW` allowlist. It joins the existing mutual-vouch ring so dropping it from `validate` is caught by `verify:format-coverage`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 |
||
|
cliffhall
|
cf0ed4ac78 |
test: add smoke:web:app — drive a real MCP App end to end in headless Chromium
`smoke:web:browser` stops at first paint and never connects to a server, so everything downstream of the connect — the Apps tab, the sandbox controller, the UI-protocol bridge — was unexercised by any smoke. That is the code #1859 broke, and nothing would have caught a regression in it. smoke:web:app boots the same prod --web server, spawns the mcp-app-http.json composable server, and drives connect → open app → widget ready through one deep-link navigate. The assertion is the documented data-app-status="ready" contract, which the renderer only reports once the widget has loaded inside the sandbox iframe AND completed its bridge handshake — so a single attribute covers the proxy being served, the UI resource loading, and the handshake. Two mechanics found by running it, both silent-failure shaped: - server-composable.ts announces readiness on stderr, not stdout, so watching stdout alone times out with an empty diagnostic. Both streams are scanned. - the bound port is not the configured one — createTestServerHttp resolves via findAvailablePort(), which walks upward when the port is taken. The smoke parses the announced URL instead of assuming 3130. Scope: this runs against the repo build tree like every other smoke, so it would NOT have caught #1859 itself (a packaging failure; the file is always present in-repo). pack:verify owns that dimension — the two are complements, and the header comment says so. A cheap structural pre-check does assert the proxy page sits where sandbox-controller.ts resolves it, so a move/rename fails fast. Both failure modes verified by hand: with clients/web/static removed, the pre-check fires; with the pre-check bypassed, the data-app-status assertion catches it independently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAR2Nr9kXbrywFNUVoTe9F |
||
|
Cliff Hall
|
07a2b4bdfd |
ci: sync the release workflow on v2/main with main's verbatim (#1902) (#1904)
main's copy of .github/workflows/main.yml carried three release fixes v2/main never took -- #1831 (least-privilege default GITHUB_TOKEN scope), #1834 (derive the npm dist-tag from the version) and #1836 (publish via npm OIDC trusted publishing). Diffing the file both directions shows v2/main has NOTHING main lacks: its only unique content is the superseded publish step, run: npm publish --access public --provenance env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} which is exactly what #1836 replaced. Every other line already matched. So this is a verbatim copy of main's file rather than a hunk-by-hunk forward-port -- fewer ways to get it wrong, and it leaves the two branches byte-identical (`git diff origin/main -- .github/workflows` is now empty). Not a back-merge: only this one file is taken, so none of main's pre-swap v1 lineage enters v2/main's ancestry (see #1868). Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
834fd2d77d |
chore(deps): migrate to eslint 10 to clear the brace-expansion DoS advisory (#1900)
* chore(deps): migrate to eslint 10 to clear the brace-expansion DoS advisory (#1838) eslint 9 pulled `minimatch@3` → `brace-expansion@1.1.16`, which carries GHSA-mh99-v99m-4gvg (high, DoS via unbounded expansion length). It appeared in all five lockfiles — the root plus each of the four clients, none of which is an npm workspace, so all five had to move together. Deferred from the pre-2.0.0 sweep because it is dev-scope, not attacker- controlled, and a major bump across five packages was the wrong thing to attempt immediately before an irreversible publish. This is that work. ## What moved - `eslint` `^9.39.4/5` → `^10.8.0` and `@eslint/js` → `^10.0.1`, in all five manifests. - `eslint-plugin-react-hooks` `^7.0.1` → `^7.1.1` in web and tui. Required, not cosmetic: 7.0.1's peer range stops at `^9.0.0`, so installing it alongside eslint 10 fails `ERESOLVE`. - Everything else already declared eslint 10 support and stayed put: `typescript-eslint@^8.56.1` (`^8.57 || ^9 || ^10`), `eslint-plugin-react-refresh@^0.5.2` (`^9 || ^10`), `eslint-plugin-storybook@^10.2.19` (`>=8`). ## Config migration None was needed. All five configs were already flat (`eslint.config.js` with `defineConfig`/`globalIgnores` from `eslint/config`), which is the format v10 keeps and the only one it still supports. The root config's `core/` and shared- surface blocks — including the `_`-prefix `argsIgnorePattern` / `varsIgnorePattern` / `caughtErrorsIgnorePattern` — are unchanged and still gate `lint:core` and `lint:shared`. No rule was disabled, downgraded, or scoped away to make the new eslint pass. v10 surfaced 21 real violations across three rules; all 21 are fixed in the code. ### `preserve-caught-error` (new in `eslint:recommended`) — 15 sites A `throw new Error(msg)` inside a `catch` silently drops the original error. Each now passes `{ cause }`, so the underlying failure survives into the chain. Messages are unchanged, so nothing that asserts on them moves. 12 in `core/`, 2 in `clients/cli`, 1 in `test-servers/src`. ### `no-useless-assignment` (new in `eslint:recommended`) — 2 sites `clients/cli/src/cli.ts`'s `optionArgs` and `test-servers/src/test-server-oauth` 's `valid` were initialized and then unconditionally overwritten on every path. Both are now declared without the dead initializer; TypeScript's definite- assignment analysis covers the remaining paths. ### `react-hooks/set-state-in-effect` — 8 sites (web) These were false negatives the plugin fixed in 7.1.0, not a rule we newly opted into: `set-state-in-effect` has been `error` in `flat/recommended` since 7.0.1, and the web config has always extended it. Every one was the same anti-pattern — resetting or re-syncing local state from a prop inside a `useEffect`, which paints the stale value for one frame and then renders again to correct it. All eight now use a new `useValueChange(value, onChange)` hook (`clients/web/src/hooks/useValueChange.ts`), which is React's documented "adjusting state during render": compare against the previous render's value with `Object.is` and call back *during* render, so React discards the in-progress output before it reaches the DOM. - `MrtrConversation`, `ProtocolEntry`, `TaskCard` — mirror `isListExpanded`. - `NetworkEntry` — same mirror, plus the "Reveal in Network" force-open. The reveal's rAF scroll stays an effect (real external-system work); only the `setIsExpanded(true)` moved. `isExpanded` is now seeded from `isListExpanded || revealed`, since a render-time sync fires on change and so cannot cover an entry that mounts already revealed. - `PromptArgumentsForm`, `ResourceTemplatePanel` — reset on template/prompt switch. - `ServerConfigModal` — reset on open. Keyed on `opened ? initial : undefined`, which collapses "opened flipped" and "initial changed while open" into one value; `initial` is already memoized, so it cannot thrash. `useValueChange` has its own test file, and `ServerConfigModal` gains an open→edit→close→reopen test (the close leg was previously untested, which is what left the new guard uncovered). ## Advisory delta `npm audit`, per package, before → after: | Package | before | after | | ------------------- | -------------------------- | -------------------------- | | root | 9 (1 low, 6 mod, 2 high) | 8 (1 low, 6 mod, 1 high) | | `clients/web` | 19 (3 low, 6 mod, 6 high, 4 crit) | 17 (3 low, 6 mod, 4 high, 4 crit) | | `clients/cli` | 6 (2 low, 4 high) | 4 (2 low, 2 high) | | `clients/tui` | 4 (1 low, 3 high) | 2 (1 low, 1 high) | | `clients/launcher` | 3 (3 high) | 1 (1 high) | `brace-expansion` is gone from all five — `eslint@10` resolves `minimatch@10` → `brace-expansion@5.0.9`. Dropping `@eslint/eslintrc` (removed in v10) also took `js-yaml` with it, clearing GHSA-52cp-r559-cp3m (high) from all four clients as a bonus. The remainder are unrelated and tracked separately. `npm run ci` passes end to end — validate, the per-file ≥90 coverage gate, the build gate, all five smokes (including `smoke:tui`, run locally on a real TTY), and 462 Storybook tests. Also documents the effect rule in AGENTS.md and mirrors it into `.github/copilot-instructions.md`, since it is now something a reviewer cites. Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: state useValueChange's referential-stability requirement (#1838) Copilot review on #1900: the hook compares with `Object.is`, so an object or array literal rebuilt in the component body compares unequal every render. Because the callback is what updates state, that is not merely a wasted render — it is an infinite loop. Every current caller passes a primitive (`boolean`, `string`) or an already- memoized value, so nothing changes behaviorally. But the hazard was implicit, and the next caller is the one it would bite. Documented on the hook itself, and one clause added to the AGENTS.md rule with its mirror in `.github/copilot-instructions.md`. No runtime guard: the requirement is the same one a `useEffect` dependency array carries, and here the failure is loud and immediate rather than silent. `npm run ci` passes. Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(cli): stop asserting the --tool-args-json catch binding is an Error (#1838) Copilot review round 2 on #1900: the `--tool-args-json` parse handler formatted its message with `(e as Error).message`. A `catch` binding is `unknown`, so that cast is an assertion rather than a check — a thrown non-Error would render `--tool-args-json is not valid JSON: undefined`, which points the user at nothing. `JSON.parse` does only throw `SyntaxError` today, so this is not a live bug. But the cast is the kind AGENTS.md asks us not to leave unjustified, the same handler was already being touched here to add `{ cause: e }`, and every other site in this PR uses the guarded form. Now it does too. `npm run ci` passes. Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: require useValueChange's onChange to be render-pure (#1838) Copilot review round 3 on #1900: `onChange` is called during render, so it is bound by render-phase purity — and the web client mounts under `StrictMode`, which deliberately double-renders in development. A callback doing anything external would run an unpredictable number of times, and concurrent React can abandon an in-progress render entirely. Every current caller already passes a callback that only calls `setState`, so nothing changes. But "runs during render" is the whole point of the hook and the constraint that follows from it was left implicit. Stated on the hook, with the same clause in the AGENTS.md rule and its `.github/copilot-instructions.md` mirror. All three now point at `NetworkEntry` as the worked example of the split: the reveal's force-open is a state update and lives in `useValueChange`, while its `requestAnimationFrame` scroll stays a `useEffect`. `npm run ci` passes. Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
0742f78357 |
docs: replace the markdown bug template with GitHub issue forms (#1894)
* docs: replace the markdown bug template with issue forms (#1844) Issues are the only intake channel now that external pull requests are off (#1820), so the chooser is where the contribution policy has to be stated — GitHub gives a would-be contributor no explanation when the "New pull request" button isn't there. The one existing template was a legacy **markdown** template, whose fields are prose headings a reporter can delete or ignore. Most reports still arrive missing the client and the transport, which are the two facts that decide where to look. Issue forms enforce them: this replaces `bug_report.md` with `1-bug_report.yml`, where client (web/cli/tui/core), version line (v1/v2), Inspector version, Node version, transport, MCP server, repro, expected, and actual are all `validations.required`. It auto-labels `bug`, and its intro says maintainers implement the fix, so a prompt plus screenshots beats a diff — with a field for exactly that. Adds `2-feature_request.yml` (auto-labels `enhancement`), which puts the problem statement first and marks the solution optional, since the problem is what survives when a specific solution turns out not to fit. Neither form applies a `v1`/`v2` label: GitHub can't map a form answer to a label, so the version line is a required dropdown and a maintainer still labels at triage. v1 takes security fixes only, so routing that answer early is the point. `config.yml` disables blank issues and carries the contact links. **There is deliberately no security *template*** — a template still opens a public issue, which is precisely what a vulnerability report must not do. The redirect is a contact link straight to the private advisory form (`/security/advisories/new`, verified enabled on this repo) plus one to `SECURITY.md`, which covers the v1 line too. The remaining links deflect the recurring misfiled classes: the specification repo and the TypeScript SDK for reports that aren't about this tool, the docs site, the contribution policy, and `#inspector-dev`. Also fixes three links the #1884 rename left dangling — `SECURITY.md` and the PR template still pointed at `CONTRIBUTORS.md`, which no longer exists on this branch. A policy nobody can open is the same as no policy. Validated against GitHub's issue-forms schema (unique ids, allowed `type`s, `markdown` blocks carrying no `id`/`validations`, `checkboxes` marking `required` per option rather than under `validations`). Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: address Copilot review on the issue forms (#1844) - PR template said `v2` targets `main`; the v2 base branch is `v2/main`. Correct on its face, and the one line in that template a reader acts on. - Scoped the Node floor on the bug form to v2, since the form serves both lines and v1's floor is older — reading `>= 22.19.0` as universal would make a legitimate v1 report look out of support. - Both the CONTRIBUTING.md paragraph and the AGENTS.md section now say the chooser is served from the **default branch**, so neither claims a form added on `v2/main` is live before the milestone merge. * docs: re-wrap the CONTRIBUTING chooser paragraph (#1844) Round-2 Copilot nit: the previous wrap left a dangling "The" at end of line, splitting a phrase mid-sentence in the raw markdown. Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: clarify the protocol-era prompt on the bug form (#1844) Round-3 Copilot nit. "the protocol era (legacy / modern) if you selected one" reads as referring to a field on this form, which has none — the era is selected on the *connection*, in the Inspector's server settings. Reworded to "the protocol era you connected with (auto / legacy / modern)", which also picks up `auto`, the default the previous list omitted. Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: stop describing the feature form as sharing the bug form's fields (#1844) Round-4 Copilot nits, and it was right three times over — I had described "client, version line, transport" as the required set for *both* forms in CONTRIBUTING.md, AGENTS.md, and the config.yml header. The feature form has neither a version-line dropdown nor a transport field, and cannot have the first: v1 takes security fixes only, so a feature request is v2 by construction. Each of the three now distinguishes the two forms rather than generalizing from the bug form. Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: declare v2 statically on the feature form (#1844) Round-5 Copilot nit caught a real contradiction: the AGENTS.md paragraph said no form can apply a version label, then asserted the feature form "is always labeled v2". Both halves can't be true. Resolved in the direction that makes the claim true rather than softer. A form's `labels:` is static, which is only an obstacle when the value depends on a reporter's answer — and for the feature form it doesn't: v1 takes security fixes only, so a feature request is v2 by construction. So it now declares `["enhancement", "v2"]` outright, which also satisfies AGENTS.md's label-at-create-time rule without a triage step. The bug form still can't, and shouldn't: its version line is genuinely the reporter's to answer. Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
13e981e9dd |
chore: restore repo files dropped by the v2 tree swap (#1871)
* chore: restore repo files dropped by the v2 tree swap
A sweep of the pre-swap tree (
|
||
|
Cliff Hall
|
ee707db52d |
ci: restore the Claude Code workflow and .mcp.json, lost in the v2 tree swap (#1869)
* ci: restore the Claude Code workflow, lost in the v2 tree swap Closes #1850. Restored byte-for-byte from |
||
|
Cliff Hall
|
7cc3c1276d |
docs: unpin the branch-pinned CONTRIBUTORS.md links in the PR template (#1879)
* docs: say "security fixes only" for v1 and unpin the PR-template links The v1 maintenance policy was stated two different ways across the docs — "security fixes only" in CONTRIBUTORS.md, "bug fixes" in README.md. The #1819 PR triage told 120 contributors that v1 is security-fixes-only, so that is the wording we keep. Also make the PR template's two CONTRIBUTORS.md links relative instead of pinned to blob/main/. `main` is release-only, so those resolved against the last released tree rather than the branch the reader is on. The remaining AGENTS.md occurrences are left alone deliberately: PR #1866 rewrites those exact lines and lands first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU * docs: drop the README change; #1866 owns that line The "Repo status" callout edit here patched `(bug fixes only)` → `(security fixes only)` on the callout as it exists on v2/main. But #1866 replaces that entire callout, because the surrounding sentence is false today: it still says "The `main` branch is the legacy v1 implementation", when `main` now holds the latest released v2. So this edit both collides with #1866 on the same line and, taken on its own, would preserve the false statement while correcting only the parenthetical. #1866 already sets the v1 policy wording to "security fixes only" across AGENTS.md and README (ee020ff2), so nothing is lost by dropping it here. This PR is now exactly its intended half: the two branch-pinned `blob/main/CONTRIBUTORS.md` links in .github/pull_request_template.md, made relative. That file is untouched by #1866, so the two PRs no longer overlap on any line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
3ac34e4092 |
docs: post-swap branch model, PR policy, milestone rule, and Copilot review instructions (#1866)
* docs: correct post-swap branch model, PR policy, and stale gate/tree entries Part of the go-live phase-8 docs hygiene: everything that silently inverted when `main` became v2. Branch/release model — documented canonically in AGENTS.md and corrected everywhere it was restated: - AGENTS.md: replace the one-line "Base Branches" bullet with a table giving each branch its role (v2/main = develop, main = release, v1/main = maintenance), whether PRs target it, and which npm dist-tag it publishes. - README.md: the "Repo status" callout had it backwards, claiming `main` was the legacy v1 implementation. Also note that a v2 release is cut from `main` after the milestone merge, not from v2/main. - CONTRIBUTORS.md: the version table listed v2's base branch as `main`. PR policy: - Every PR must reference an issue, from anyone, no exceptions. - PRs are opened by repo maintainers only — org write access is not authorization. Everyone else files a detailed issue. - Branch names start with the target version segment (v2/…, v1/…). - Issues and PRs carry exactly one of `v1`/`v2` at creation; default `v2`. - Screenshots proving UI/TUI changes go in the gitignored pr-screenshots/. Corrections to stale content found while in there: - The TUI coverage bullet still described the Ink/App.tsx/hooks exclusion as interim pending #1501, which closed 2026-06-29; the gate now covers all of src/** with only tui-servers.ts excluded. - The completion checklist said `npm run validate` was the gate and covered "e2e tests", contradicting the mandatory-pre-push-gate section (npm run ci). - v1 PR base was given as `main`; it is `v1/main`. - Project tree omitted core/client, docs/, and scripts/ (README: core/client, docs/). core/client is under the coverage gate and imported by both App.tsx trees. Also adds the launcher and `npm run test:scripts` to the "run the tests" list and a tiers overview, since neither was mentioned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU * docs: require a milestone on every new issue, defaulting to the current one Adds the rule alongside the version-label rule, since they fail the same way: an unmilestoned issue drops out of release planning as silently as an unlabeled one drops out of version filtering. - Set it at create time (`gh issue create --milestone <title> ...`). - If the user didn't specify one, default to the current milestone — the open milestone with the nearest due date — rather than leaving it blank pending a decision. Includes the `gh api .../milestones` one-liner that identifies it. - Milestones are release buckets, so pick by when the work ships, not by size; sub-issues normally inherit their parent's milestone. Also widens the section's opening callout: an issue is not "created" until it is labeled, milestoned, AND boarded with a Status — four distinct steps, not three. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU * docs: add .github/copilot-instructions.md and require mirroring into it GitHub Copilot reads .github/copilot-instructions.md when reviewing a PR. The repo had no such file, so Copilot reviewed without any of the conventions that actually govern this codebase — the ban on `any` and on unjustified double casts, the Mantine-first styling rules, the `.withProps()` extraction rule, the lib/utils split, test placement, and the per-file >=90% four-dimension gate. The new file is a review-focused distillation of AGENTS.md: the rules a reviewer would cite against a diff, plus a short "what to prioritize" list that puts correctness and security first (this backend spawns processes and proxies outbound requests). Deliberately omitted: board recipes and IDs, milestone and branch mechanics, release procedure, and the project tree — no reviewer cites those, and copying them would double the maintenance surface. AGENTS.md's "Keep documentation files up to date" section now requires mirroring review-relevant changes into it in the same PR, states that AGENTS.md stays the source of truth, and spells out what counts as review-relevant. There is no generation step and nothing detects drift, so the rule is the only thing keeping the two in sync. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU * docs: address Copilot review — v1 policy wording, stale V2 Go Live, typo - Align the v1 maintenance policy on "security and bug fixes only" in AGENTS.md and CONTRIBUTORS.md; both said "security fixes only" while the README callout, the branch table, and the board description said "security and bug fixes". - Drop the stale "or V2 Go Live" instruction from the work-begins step — that column no longer exists on board #28 (verified via `gh project field-list 28`), so the option id it implied would be rejected. Restore the removed-column id list the same edit dropped, now including V2 Go Live, and re-date the table to today's verification. - Fix "contains the the new version" typo. - Strip a stray trailing blank line at the end of AGENTS.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU * docs: restore `inspector/` as the Project Structure tree root The tree is a filesystem layout — every child is a real directory (`clients/`, `core/`, `test-servers/`). Labeling its root `v2/main/` named a branch instead, which reads as a path that does not exist in a checkout and diverged from the identical tree in README.md, which still says `inspector/`. The branch model is documented in its own table under Repository & Project Boards; the tree does not need to restate it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU * docs: settle the v1 policy wording as "security fixes only" Reverses the direction the Copilot round took in 33cc0366. That commit normalized the six v1-policy sites onto "security and bug fixes" because that was the majority wording — but majority is the wrong tiebreak here. The policy is set by what we told contributors: the #1819 backlog close note, posted to 120 closed PRs, says "v1 will receive security fixes only", and the five PRs retained for 1.0.5 are all security fixes. So the two sites that already said "security fixes only" were the correct ones, and the four that said otherwise are now aligned to them: - AGENTS.md: the `v1` label description, the branch-role table, and the v1 board line. - README.md: the "Repo status" callout. CONTRIBUTORS.md and AGENTS.md's Project Status bullet already read correctly and are unchanged. This is the substance of #1813, which is why that issue's PR is now reduced to its PR-template-link half — the wording work landed here to avoid two PRs editing the same lines in opposite directions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU * docs: point the AGENTS.md policy link at CONTRIBUTING.md This PR rewrites the paragraph holding the only in-repo link to the contributing policy, so it owns that line. #1884 renames the file CONTRIBUTORS.md -> CONTRIBUTING.md; having #1884 also edit this line would guarantee a one-line conflict between the two. Writing the new filename here instead makes #1884 a pure `git mv` with no overlapping line, so neither PR conflicts with the other. Between this merging and #1884 merging, the link points at a filename that doesn't exist yet. That window is deliberate and bounded — this PR is first in the merge order (#1821) and #1884 is third. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
b3be65b0b4 |
docs: add CONTRIBUTORS.md issues-only policy (closes #1517) (#1604)
* docs: add CONTRIBUTORS.md issues-only policy (#1517) Document the contribution policy: we accept issues, not pull requests. Maintainers handle design and implementation through a prompt-driven workflow. Contributors who have already built a change locally should share the prompt they used rather than a diff. Cross-reference the policy from AGENTS.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: add PR template redirecting external authors to issues-only policy (#1517) GitHub auto-populates pull_request_template.md into every new PR body, so it steers would-be external PR authors to CONTRIBUTORS.md before they submit a diff. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: drop version-board details from CONTRIBUTORS.md (#1517) The policy takes effect after v2/main merges into main, leaving a single line of development. Tell contributors to open a well-formed issue rather than routing them to per-version project boards. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: add version-board/label table to CONTRIBUTORS.md (closes #1595) Completes requirement #2 of #1517: a table mapping each active version to its base branch, project board, and version label, plus a "Label by version" note mirroring AGENTS.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX * docs: drop v1.5 from CONTRIBUTORS.md, invite deeper contribution (#1517) Review feedback on PR #1604: - Remove the v1.5 row/bullet from the version table — v1.5 was an internal intermediate branch that external contributors never saw, so surfacing it in an outward-facing doc is noise. The table is now v1 / v2 only. - Add a "Want to work on the Inspector with us?" section pointing at the MCP Contributor Discord (#inspector-dev) and the community calls, framing the issues-only rule as being about unsolicited patches rather than a closed door, with supervised work as the path toward maintainership. - Link that section from "Why this policy exists", and add the same Discord pointer to the PR template. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Txmv2qqv3yeKgRzoqXytzD * docs: drop project-board links from CONTRIBUTORS.md (#1517) The org project boards aren't visible to anyone outside the organization, so linking them in an outward-facing doc points contributors at a 404. The version table is now base branch + label only, and the PR template names the version labels instead of "the appropriate version board". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Txmv2qqv3yeKgRzoqXytzD * docs: v1 takes security fixes only (#1517) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Txmv2qqv3yeKgRzoqXytzD * docs: use post-runbook branch names (v1/main, main) (#1517) This doc will be read after the branch runbook completes, when v1 lives on v1/main and v2 is the default main. Update the version table, the label-by- version line, and the PR template's label hint + CONTRIBUTORS.md links accordingly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Txmv2qqv3yeKgRzoqXytzD --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
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> |
||
|
Cliff Hall
|
742e82515a |
ci: add headless-browser boot smoke for the web client (#1615) (#1768)
Some checks failed
Run install, format, lint, build, and test on every push / build (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish-github-container-registry (push) Has been cancelled
* 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> |
||
|
cliffhall
|
f5e095e8f0 |
review: harden the Docker image + align GHCR action versions
Address PR #1648 review: - Bump the GHCR action majors to match current v1 (docker/login-action v4, docker/metadata-action v6, docker/setup-buildx-action v4, actions/attest-build-provenance v4) to avoid Dependabot churn. - Make the image tags explicit (`type=semver` + `flavor: latest=true`) instead of relying on `flavor.latest=auto`, so a release lands both the version tags and `:latest` — the README's bare `ghcr.io/…/inspector` URL won't 404. - Run the container as the non-root `node` user with /home/node as workdir (its writable home is where the inspector stores the catalog / OAuth state). - Add a HEALTHCHECK that probes `/` via Node's global fetch (no curl in slim). - Trim specification/ and docs/ from the build context. (test-servers/ stays — `tsc -b` typechecks the integration tests that import its alias, so excluding it breaks the build.) Re-verified locally: `docker build` succeeds, the container runs as uid 1000 (node), serves `/` (200), and the HEALTHCHECK reports healthy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw |
||
|
cliffhall
|
941bf13587 |
ci(docker): add v2 Dockerfile + GHCR publish job (#1646)
v1 published a container image but v2 arrived with no Dockerfile (the client/server/cli layout is gone). Add a v2 image for the single-package / launcher architecture and wire the release-gated GHCR publish job. - `Dockerfile`: two-stage build — stage 1 installs + `npm pack`s the publishable tarball; stage 2 `npm install -g`s that tarball, so the image ships the exact same artifact as npm with a clean `mcp-inspector` bin. Defaults to `--web` on `0.0.0.0:6274`, browser auto-open off. `.dockerignore` keeps the context lean and forces a clean in-container install/build. - `publish-github-container-registry` job in main.yml (ported from v1): release-gated, multi-arch (amd64/arm64) build+push to GHCR with a provenance attestation. Independent of the npm `publish` job. - README: document the image + how to run it; the release section now covers both publish jobs. Verified locally: `docker build` succeeds, the container serves `/` (HTTP 200 with the injected token) and dispatches `--cli` / `--tui`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw |
||
|
cliffhall
|
85d5bd447e |
ci(publish): pass release tag via env to avoid script injection
Re-review nit: interpolating `${{ github.event.release.tag_name }}` directly
into the `run:` block is a script-injection surface (a crafted tag with shell
metacharacters would splice into the script). Pass it through `env: TAG` so it
arrives as a plain shell variable instead. Purely defensive; no behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
|
||
|
cliffhall
|
ccbcd3d719 |
ci(publish): harden the release job per review
Address PR #1647 review: - Assert the release tag matches the root package.json version before publishing (fail fast, before install/pack:verify) — publishing is irreversible, so a release drafted without `npm version` or cut from the wrong commit shouldn't silently ship a mismatched version. Tolerates a leading `v`. - Add a `concurrency: { group: publish-npm, cancel-in-progress: false }` guard so two releases in quick succession can't run overlapping publishes. - Note in-comment that the triple build (build job → pack:verify → prepack) is intentional, so a future reader doesn't "optimize" prepack away and break the tarball. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw |
||
|
cliffhall
|
3b7ab5f548 |
ci(publish): add release-gated npm publish job for the single v2 package (#1640)
Wire the v2 publish pipeline into main.yml. On a published GitHub release, a new `publish` job runs `npm run pack:verify` (the pre-publish gate — builds, packs the real tarball, installs it into a clean consumer, and drives the bin) then `npm publish --access public --provenance`. This is a single `npm publish` (v2 is not an npm workspace, so no v1-style publish-all/--workspaces), with a signed provenance attestation via GitHub OIDC (id-token: write, environment: release, NPM_TOKEN). Add the `release: types: [published]` trigger; the existing `build` job is unchanged and the publish job is gated on the release event. Document the single-version release/tag flow in the root README (one `npm version` at the root; no per-client sync / check-version since only the root package.json carries a version). Docker/GHCR is deferred: v2 has no Dockerfile (the v1 client/server/cli layout is gone), and a correct image needs a net-new, docker-build-tested Dockerfile. Split into #1646. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw |
||
|
cliffhall
|
859911d5ad | revert(rollup): remove CONTRIBUTORS docs (#1537/#1517) from Wave-1 rollup — tracked separately, not Wave 1 | ||
|
cliffhall
|
61ca22003f |
ci: enforce the ≥90% per-file coverage gate in CI (closes #1550)
Add a CI step that runs `npm run coverage` so a PR that drops any file below 90% on lines/statements/functions/branches fails the job. Remove the now-redundant standalone `Run web integration tests` step, since web's `test:coverage` already runs `--project=unit --project=integration --coverage`, and update AGENTS.md to reflect that CI now enforces the gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX |
||
|
cliffhall
|
b2ff34a901 | merge: chore/1517-contributors-md (#1537) | ||
|
cliffhall
|
b152f74ccd |
fix(ci): run CI on Node 22.x and correct undici 8 Node floor (#1563)
undici 8.5.0 requires Node >= 22.19.0 (its declared engines), so CI's Node 20.x could not load undici's CacheStorage (missing webidl.util.markAsUncloneable) and the proxy integration tests failed. Bump the CI setup-node to 22.x to match the repo's Node floor. Reconcile the declared floor with undici's real requirement: bump root engines.node to >=22.19.0 and correct the CLI README note (undici 8.5.0 needs >=22.19.0, not >=22.7.5). Also document proxy support in the web README and clarify that a caller-supplied eventSourceInit.fetch bypasses the proxy wrapper by design. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX |
||
|
cliffhall
|
cc1e707ff0 |
docs: add PR template redirecting external authors to issues-only policy (#1517)
GitHub auto-populates pull_request_template.md into every new PR body, so it steers would-be external PR authors to CONTRIBUTORS.md before they submit a diff. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
Cliff Hall
|
388ea5a7bd |
test(cli,tui): add in-process source coverage gates (#1484) (#1502)
* test(cli,tui): add in-process source coverage gates (#1484) CLI and TUI source had no coverage gate: CLI tests spawned the built binary out-of-process (coverage never saw clients/cli/src) and the TUI had only two smoke tests. Regressions there were invisible to CI. CLI: - Rewrite __tests__/helpers/cli-runner.ts to invoke runCli() in-process (capture process.std*.write, map thrown errors to an exit code), preserving the CliResult shape so all existing tests run unchanged and clients/cli/src is now measured under v8 instrumentation. - cli.ts: program.exitOverride() so a commander usage error throws (catchable in-process) instead of tearing down the worker; --help / --version still exit cleanly. Drop the dead, never-imported main(). - Add a thin out-of-process e2e.test.ts that spawns the built binary for the shebang / process.exit paths the in-process runner can't reach. - Add @vitest/coverage-v8, a per-file gate (90/85/80/50) excluding only src/index.ts (binary bootstrap), and pool: 'forks' for capture isolation. cli.ts lands at ~98% lines / 100% functions. TUI: - Add @vitest/coverage-v8 and an interim per-file gate over the non-React logic (logger.ts, components/tabsConfig.ts, utils/*); the Ink components, App.tsx, and hooks/ are excluded with a documented follow-up (#1501). Add tests covering the included modules (100%). Wiring + docs: - CI runs each client's test:coverage; root test:cli / test:tui (used by validate) and root test:coverage now enforce the gates. - gitignore clients/**/coverage; update AGENTS.md and client READMEs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(cli): clarify in-process runner footguns from PR #1502 review Review notes on the in-process CLI test runner (no behavior change): - Warn in runCli's docblock that --help/--version must NOT go through the in-process runner: exitOverride only re-throws non-zero exits, so exitCode 0 falls through to process.exit(0) and would kill the vitest worker. Those belong in the out-of-process e2e.test.ts layer. - Document that a timeout orphans the uncancellable invokeCli promise (its stdio subprocess stays alive; later writes hit the restored stdout) — a known, acceptable trade-off on the hang-only failure path. - Fix a stale reference to the removed main() (errors now route through index.ts). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(cli): fix remaining stale main() reference in vitest exclusion comment PR #1502 re-review caught a second stale `main()` mention (the first was fixed in 52c23e41). src/index.ts inlines the bootstrap as runCli(process.argv).then(...).catch(handleError) — there is no main(). Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: compose root test:coverage from per-client scripts Replace the brittle inline cd-chain with a composition of the existing test:cli / test:tui scripts plus a new matching test:web. Behavior is unchanged (web → cli → tui coverage gates) but there's a single source of truth for each client's coverage invocation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
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> |
||
|
Bob Dickinson
|
62d442b073 |
Revert "First pass at pulling the v1.5 CLI, TUI, and Launcher clients into v2."
This reverts commit
|
||
|
Bob Dickinson
|
467b01edd6 | First pass at pulling the v1.5 CLI, TUI, and Launcher clients into v2. |
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
4fd7f39acf | ||
|
|
519ecf49ee | ||
|
|
7f9be24bf3 | ||
|
|
372403d9cc |
||
|
|
532ad82791 |
||
|
|
f9d893f3d7 |
||
|
|
b451949c9e | ||
|
|
94a227b9c1 | ||
|
|
c158ad809f | ||
|
|
bd6c1072bf | ||
|
|
eb5bba62ed | ||
|
|
b3242bc04c | ||
|
|
f4529df219 | ||
|
|
f060c97e8f | ||
|
|
59f5b6d25c |
||
|
|
df399272f2 |
||
|
|
16b9eff8f4 |
||
|
|
5a3a3436ea | ||
|
|
e132d551e6 | ||
|
|
719da6fbdd | ||
|
|
35317fd9ed | ||
|
|
6c33da38c9 | ||
|
|
cf0ed4ac78 | ||
|
|
07a2b4bdfd |
||
|
|
834fd2d77d |
||
|
|
0742f78357 |
||
|
|
13e981e9dd |
||
|
|
ee707db52d |
||
|
|
7cc3c1276d |
||
|
|
3ac34e4092 |
||
|
|
b3be65b0b4 |
||
|
|
d9e4f444a2 |
||
|
|
07d6316fba |
||
|
|
742e82515a |
||
|
|
f5e095e8f0 | ||
|
|
941bf13587 | ||
|
|
85d5bd447e | ||
|
|
ccbcd3d719 | ||
|
|
3b7ab5f548 | ||
|
|
859911d5ad | ||
|
|
61ca22003f | ||
|
|
b2ff34a901 | ||
|
|
b152f74ccd | ||
|
|
cc1e707ff0 | ||
|
|
5efe9ddc7b |
||
|
|
388ea5a7bd |
||
|
|
36b48e61a2 |
||
|
|
2488ca5e38 |
||
|
|
62d442b073 | ||
|
|
467b01edd6 |