Both sides added a showcase config row and section after the pagination
entry; kept both, structured output first.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
Two gaps from the Copilot review on #1975:
- The `Large` story asserted nothing, so dropping the section's `mah` (or
applying it to the wrong element) would still pass CI. Its play function
now measures the real ScrollArea viewport: the visible height stays at
the cap and the overflow is scrollable rather than clipped.
- Nothing exercised the `list_items` fixture, so a preset-registration
typo would leave the documented showcase broken with a green suite. A
new integration test loads `structured-output-http.json` through
loadConfig/resolveConfig — covering the file, the registry entry, and
the fixture — connects, calls the tool, and asserts both the summary
block and the nested structuredContent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SW1p8E2uiyyLx4RKwrwSrt
Review round 2. Three findings, all real.
1. Precision. Mantine's `NumberInput` returns the raw *string* rather than
a number once the value reaches `Number.MAX_SAFE_INTEGER` (see its
`isValidNumber` guard), precisely because JS cannot hold it. Parsing
that turned `90071992547409910` into `90071992547409904` and would
have sent the server a number the user never typed. An inspector must
not misreport what it transmits, so such input now reports no value —
which is also what this field did before #1888, so nothing regresses.
Only the integer part overflows, so long decimals stay parsed.
2. `AppDetailPanel` has the same reuse pattern as `ToolDetailPanel`:
`AppsScreen.handleSelect` swaps `selectedAppName` + `formValues` in
place without remounting, so switching between apps with a same-named
number field could retain an equal-valued draft. It now passes
`resetKey` too.
3. An exponent was never reachable: `NumberInput` masks input through
`NumericFormat`, which rejects `e`. Dropped that claim from the doc
comment rather than implying `"1e"` is a case being handled.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0119LaPQ3v4NyBK3ZTdM9j84
- A named volume created by an image predating the ownership fix keeps its
root ownership if it is non-empty, since Docker only applies the image
directory's ownership to an *empty* volume. Verified both halves: an empty
pre-existing volume repairs itself on the first run of the fixed image,
while one holding a file stays root-owned and still EACCESes. Document the
distinction and the one-shot `chown` repair (verified to restore writes).
- "pair it with a known MCP_INSPECTOR_API_TOKEN" was bad advice for a wider
publication: the same paragraph notes `GET /` discloses the token, so a
custom one is harvested exactly as a generated one is. Point at a real
access-control boundary instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SFXDQBEjvjCEBhmYkmw79a
Review follow-up. The draft/value re-sync compares parsed numbers, so it
cannot see a reset to an *equal* value. `ToolDetailPanel` is not keyed by
tool, so its fields are reused across selections: type `-` (draft `"-"`,
value `undefined`), switch to a tool with a same-named number field and
no default, and the value is `undefined` on both sides — no divergence is
detected and the stale `-` stays in the box for the new tool.
Add a `resetKey` prop carrying the identity of whatever the form edits,
and vary the number field's React key with it so no in-progress text can
outlive the entity it was typed into. `ToolDetailPanel` passes the tool
name; nested object forms inherit it. The schema object is no substitute
— callers rebuild it every render, so its identity is unstable.
The regression test drives the switch with `fireEvent` rather than
`user.click`: a real click also blurs the input, and Mantine sanitizes an
incomplete value on blur, which masked the defect entirely. A paired test
asserts a *stable* `resetKey` does not remount, since remounting on every
render would wipe the draft and reinstate #1888.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0119LaPQ3v4NyBK3ZTdM9j84
Closes#1964.
- README's Docker recipes dropped v1's `127.0.0.1:` prefix, so
`-p 6274:6274` published the Inspector on every host interface. The
container's `HOST=0.0.0.0` governs the container's interfaces, not the
host's, so `DANGEROUSLY_BIND_ALL_INTERFACES` never covered this. Verified
against a real container: from a LAN peer, `GET /` served the injected
`MCP_INSPECTOR_API_TOKEN`, and that token then unlocked `/api/*` with no
`Origin` header (server.ts:241 allows origin-less requests). Restore the
prefix on both recipes, on the port-remap examples, and in the web README.
- Adding a server failed inside the container the moment you mounted a
volume to keep it. Docker seeds a named volume's ownership from the
image's directory at the mount point, and creates it `root:root` when
that directory is absent — so the non-root `node` user got
`EACCES ... open '/home/node/.mcp-inspector/mcp.json.tmp-*'`. Create the
dir in the image, owned by `node`, and document the volume recipe (plus
the bind-mount `--user` caveat) — without a volume the catalog lives in
the writable layer and `--rm` throws it away.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SFXDQBEjvjCEBhmYkmw79a
A tool declaring an `outputSchema` returns its real payload in
`structuredContent`; the `content[]` blocks usually only summarize it. v1
rendered that payload as its own inspectable JSON section — v2's
ToolResultPanel only ever walked `result.content`, so the payload was
dropped with no label, section, or hint that it existed.
Adds a `StructuredOutputPanel` group: a collapsible "Structured Output"
box (bordered like the existing "Resource Links" box) rendering the
payload as pretty-printed, syntax-highlighted, copyable JSON, bounded so
a large payload scrolls within itself. ToolResultPanel renders it below
the content blocks — and also when `content` is empty (where "No results
yet" was simply wrong) and below the alert on an error result, so it is
never silently dropped.
Adds a `list_items` test-server preset (nested payload, the issue's repro
shape) and a `structured-output-http.json` showcase config, documented in
the README.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SW1p8E2uiyyLx4RKwrwSrt
`SchemaForm` drove Mantine's `NumberInput` directly off the parent's
numeric value. `NumberInput` reports the raw string while the text does
not parse, so pressing `.` after `1` produced `"1."`, which the handler
collapsed to `undefined` — and because the control is fully controlled,
the `value` prop immediately rewrote the box back to `1`. The decimal
point was discarded on every keystroke, making `1.5` unreachable. A
trailing zero, a lone leading `-`, and an exponent failed the same way.
Introduce `SchemaNumberInput`, which holds the in-progress text as the
source of truth for what is displayed while the parent still only ever
sees a `number | undefined`. Draft and value re-sync only when they
genuinely diverge, via `useValueChange` (adjust-state-during-render)
rather than an effect, so an external reset still rewrites the box but
an in-progress `"1."` is left alone.
Also pass `allowDecimal` from the schema, so an `integer` field rejects
the decimal point instead of accepting a value its schema forbids —
matching what the TUI already does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0119LaPQ3v4NyBK3ZTdM9j84
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
The ToolControls regressions build their `Tool[]` by hand, so they would still
pass if the new server option broke — config loading, resolveConfig, the
ordering, the title marker, or its composition with pagination. Copilot flagged
exactly that gap.
Adds an integration test that connects a real InspectorClient to a real
streamable-HTTP test server and asserts the wire shape: repeats appended (not
adjacent — the ordering the defect depends on), only the appended copy marked
"(duplicate)", an unregistered name ignored, an empty list a no-op, and
duplication running before the page slice so a pair straddles a boundary. The
last case loads `duplicate-tool-names-http.json` through loadConfig +
resolveConfig, so the JSON plumbing the manual repro uses is covered too.
Verified non-vacuous: mutating withDuplicates to place repeats adjacent fails
four of the five.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SW1p8E2uiyyLx4RKwrwSrt
Nothing in the repo could serve the shape this fix is about: every preset
registers a unique name, and the SDK's `registerTool` rejects a repeat. So the
only reproduction was a unit test — there was no way to smoke the real app
against a server that returns duplicates.
Adds `ServerConfig.duplicateToolNames` (a list of registered tool names) to the
composable test server, plumbed through the JSON config, and a showcase config
`duplicate-tool-names-http.json`. The named tools are emitted a second time in
`tools/list` with the same `name` and a "(duplicate)" title.
The repeats are appended after the whole list rather than placed beside their
twin. That is both the realistic shape (two tool sources concatenated) and what
makes the defect observable: React matches a leading run of same-key children
first, so a head-adjacent duplicate happens to line up and the bug hides. Only
a separated pair collides in the keyed map and orphans a row — verified both
ways against the prod bundle.
The `tools/list` override is shared with the existing pagination handler (which
already hand-builds the list); with no `maxPageSize` it returns a single
unbounded page. Duplication happens before pagination, so a duplicated pair can
straddle a page boundary the way a real server's would.
Also adds a ToolControls test for this appended ordering, distinct from the
reporter's interleaved fixture, and documents the config in the README showcase
table.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SW1p8E2uiyyLx4RKwrwSrt
`ToolControls` keyed both the main list and the SEP-2243 excluded list by
`tool.name`. A server may legitimately return the same tool name more than
once, and colliding React keys let a filtered-out row survive reconciliation:
searching narrows the array, React matches the surviving rows against the
stale keys, and an unrelated row stays mounted (plus a "two children with the
same key" console warning).
Stamp each row's position in the unfiltered list before filtering and use
`<index>:<name>` as the key, on both lists. The position is captured before
the filter, so the key stays stable as the search narrows. No deduplication,
no change to what `tools/list` returns, and the name/title search behavior is
unchanged.
Adds regression tests for both lists using the reporter's fixture.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SW1p8E2uiyyLx4RKwrwSrt
* fix: load @napi-rs/keyring lazily so an unsupported platform degrades
`@napi-rs/keyring` ships one prebuilt binary per platform triple and
throws on import where it has none. On Android / Termux there is no
`@napi-rs/keyring-android-arm64`, so the static top-level import in
`core/auth/node/secret-store.ts` threw during module evaluation and the
Inspector exited at startup with "Cannot find native binding" — before
any of the keychain-unavailable handling in that same file could run.
Replaces the static import with a cached dynamic one. The outcome is
cached, not just the module, so a box without a binary doesn't
re-attempt (and re-throw) resolution on every secret operation —
`expectedSecretFields` means that would otherwise be once per server per
`GET /api/servers` — and so `set` can name the underlying cause in its
`KeychainUnavailableError`.
An unloadable package now folds into the same degradation contract as an
unreachable keychain: `get` returns null, `delete` and
`deleteAllForServer` no-op, and `set` throws `KeychainUnavailableError`
(not double-wrapped — the load failure is already typed by the time the
catch sees it). The class doc comment now enumerates all three ways
"unavailable" can arrive, since each escaped the contract at some point.
The shared `vi.mock` stub can't express this — it models a keyring that
loads — so the new tests build on `vi.resetModules()` + `vi.doMock` with
a throwing factory and re-import the module, covering the load failure,
each method under it, and the load-once caching.
Verified the dynamic import survives bundling in all three clients
(cli, tui, and the web runner) rather than being inlined back to a
static one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAR2Nr9kXbrywFNUVoTe9F
* fix: steer a missing native binding to a reinstall hint
`KeychainUnavailableError` gave the Linux libsecret / gnome-keyring
advice for every cause, including the one where the keychain is fine and
the @napi-rs/keyring platform package is what's missing — an unsupported
triple (#1905) or npm's optional-deps bug dropping it on a supported one
(npm/cli#4828, the Windows report on #1852). Detect the loader's "Cannot
find native binding" phrasing and point those users at a reinstall /
npx-cache clear instead.
Ported from the superseded #1943, which fixed this alongside the same
lazy-load change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
* fix: name a bad keyring namespace instead of "is not a constructor"
The lazy import accepted whatever the resolution produced. The named
exports arrive through CJS interop, so a resolution that stopped
yielding them — a default-only export upstream, a bundler changing
interop, a platform where named-export detection fails — would hand us
`undefined`, and `new mod.AsyncEntry(...)` would throw a TypeError
inside the very try that implements graceful degradation.
Shape-check the resolved module and treat a bad shape as unavailable.
Scope, because it is narrower than it first appears: this does NOT
prevent a bad shape from emptying the secret list. `get` returns null
either way — that is its read-tolerance contract, and the TypeError
lands in the same catch a dead keychain does. What changes is the
diagnosis. Without the check the only signal is `set` reporting
"keyring.mod.AsyncEntry is not a constructor", which reads like an
Inspector bug; with it, `set` names the real problem once, at the load
boundary, in the same actionable 503 as every other unavailability.
Catching the silent-empty-list case itself would need a round-trip
against the unmocked package, which nothing in the suite does today.
The member access sits inside the try because reading a missing export
is not always a harmless undefined — a Proxy-backed namespace can throw,
as vitest's module mocks do — and letting that escape would reject the
cached promise, which is otherwise guaranteed to always resolve.
Of the four new tests only the cause-message one fails without the
guard; the rest pin the surrounding contract.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
* docs: correct the test header's count of unavailability entry points
The shape check added a fourth way "unavailable" can arrive, and the
enumeration in secret-store.ts was updated to match, but the test file's
header still said three — so it read as an exhaustive list that silently
omitted the case a whole describe below it covers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
* fix: give the wrong-shape cause its own remediation hint
The shape check added a cause that fell through to the libsecret
advice — so a packaging mismatch told the user to install a keyring
daemon, which cannot help, and on Windows or macOS points somewhere that
does not exist. That is the same wrong-advice bug the native-binding
branch was added to fix, reintroduced by the guard itself.
Hint selection moves into `hintFor`, where libsecret is the explicit
fallback rather than the default, and the shape failure becomes a
distinct `KeyringModuleShapeError` matched by type — it is our own error,
so there is no reason to re-parse text we wrote (the native-binding
branch matches a string only because that message comes from napi-rs).
Also retitles the wrong-shape test: it claimed the guard avoids
"returning null secrets", but `get` returns null either way by the
read-tolerance contract. The title now says what actually differs —
`set` hard-fails — matching the correction already made in the comments.
Both raised by Copilot review.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
* fix: give both shape-check failure paths the packaging hint
The shape check can fail two ways — the members are absent, or reading
them throws — and only the first carried KeyringModuleShapeError. The
catch path returned the raw error, so a Proxy-backed namespace throwing
on access fell through to the libsecret advice: the same wrong-remedy
bug as the previous two commits, one branch further along.
Both paths now carry the type. KeyringModuleShapeError takes a detail
string plus a standard `cause`, so the throwing path reads "its exports
could not be read: <original>" and keeps the underlying failure legible
while still earning the packaging hint.
Three instances of one mistake in a row (a new cause added without
deciding which remediation it inherits) says the shape was wrong, not
just the branch: making libsecret an explicit fallback in `hintFor` did
not stop me adding a second exit that skipped it. The tests now pin the
invariant directly — one asserts both construction paths produce the
same hint — so the next cause that forgets is a failing test rather than
a reviewer's catch.
Raised by Copilot review.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: construct AsyncEntry inside the try so keychain degradation engages
`KeyringSecretStore` built its `AsyncEntry` outside the `try` in `get`,
`set`, and `delete`. `AsyncEntry::new` performs the platform-store setup
(on Linux, the Secret Service connect with a keyutils fallback) and
throws when no backend is reachable, so the documented degradation
contract never engaged for a construction-time failure: the raw keyring
error escaped, 500ing every `GET /api/servers` on a box without a Secret
Service (the published container, which has no D-Bus session).
`expectedSecretFields` always includes the OAuth slot, so `rehydrateConfig`
constructs an entry for every server — the default seeded catalog was
enough to 500 the first list load, before any server was added. And
because the escaping error wasn't a `KeychainUnavailableError`, it
bypassed both the routes' 503 translation and the `migratePlaintextSecrets`
skip branch, yielding a generic 500 instead of the actionable message.
Moving construction inside the existing `try` restores the contract for
that failure mode: `get` returns null, `delete` no-ops, and `set` is the
only operation that throws — as `KeychainUnavailableError`, which is what
the 503 translation and the migration skip both match on.
The test stub's constructor could not fail, which is why the coverage
gate never saw the gap. Adds a `constructorThrows` hook alongside the
existing method-level `failures` flags and covers all three methods plus
`deleteAllForServer` under it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAR2Nr9kXbrywFNUVoTe9F
* test: pin why the OAuth secret field is unconditional
`expectedSecretFields` always includes the OAuth slot, and #1848's report
frames that as why the 500 fired so broadly — which invites a follow-up
that skips the keychain read for servers with "no OAuth config". That
change would lose data.
`extractSecretsFromStored` deletes the `oauth` block outright when
`clientSecret` was its only property, so such a server carries NO marker
on disk that a secret exists — the keychain is the sole record, and the
unconditional slot is what finds it again. Gating the read on a
disk-visible `oauth` block would silently stop rehydrating exactly that
shape.
The existing tests do fail if the slot is made conditional (verified by
applying the change: 4 red), but only under names that explain nothing
about the consequence — `always lists the OAuth slot first` reads like a
tautology worth updating rather than a trap. Add a round-trip case that
states the consequence, so the next reader sees the field is load-bearing
rather than defensive.
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>
Two fixes from Copilot's review of #1953.
useManagedListError re-synced React state from the `state` prop inside a
useEffect, which renders one frame carrying the PREVIOUS store's error
after `state` changes (switching servers) before the effect corrects it —
the pattern AGENTS.md forbids. useSyncExternalStore reads the snapshot
during render, so a store swap lands in the same frame, and it also
closes the window where an error recorded between render and subscribe
was missed. The snapshot returns the stored Error instance (or null), so
it is referentially stable as the API requires.
The `markResponseRejected` correlation maps were never cleared. Ids of
requests that never got a response — a timeout, a dropped connection —
are the only entries trackResponse can't remove, so they accumulated
across reconnects. Cleared on the start-clean connect path, per the
convention documented on resetSessionState (one route out, `onerror`
with no `onclose`, tears down nothing).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SW1p8E2uiyyLx4RKwrwSrt
markResponseRejected was called for ANY refresh failure. The id it
recovers is "the last response received for this method", which is the
failing exchange only when a response actually arrived and was refused
while decoding. A transport drop, a timeout, or an abort produces no
response frame at all — the last-answered id then still points at an
EARLIER successful call, so marking it stamped "Rejected by the
Inspector" onto an exchange that worked. That is the same class of lie
this issue exists to remove.
A server-sent JSON-RPC error is excluded for a different reason: the id
would be right, but the failure is the server's and its entry already
renders as an error from the error frame.
Gated on SdkErrorCode.InvalidResult / UnsupportedResultType — decisions
the client made about a frame in hand. The failure is still recorded as
list state either way; only the Protocol attribution is gated.
Caught in Copilot's review of #1953.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SW1p8E2uiyyLx4RKwrwSrt
extractStatus returned none for any non-request entry, so a clientError
annotation that landed on a standalone response frame — the fallback
messageLogState uses when no request entry was there to fold into, e.g. a
trimmed log or a reconnect boundary — rendered with no status badge at
all. The clientError check now runs before the request-only lifecycle.
Caught in Copilot's review of #1953.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SW1p8E2uiyyLx4RKwrwSrt
A list load that failed — a transport error, or a result the SDK's era
codec rejects as invalid — reported nothing. The connect-time refresh is
fired with no caller to await it (`void this.refresh()`), so its rejection
became an unhandled rejection: the header said "Connected", the panel
rendered an empty list indistinguishable from a server that has none, and
the failed exchange showed in the Protocol panel as a clean success.
Two surfaces now tell the truth:
- ManagedListState records the failure as observable state (`getError` +
an `errorChange` event) and still re-throws, so App's auth-recovery
wrapper keeps working. The four `useManaged*` hooks expose it via a
shared `useManagedListError`, and the Tools/Prompts/Resources sidebars
render a `ListLoadError` alert with the reason and a Retry.
- A response the client refused is attributed back to its Protocol entry
(`markResponseRejected` → `responseRejected` → `MessageEntry.clientError`),
which now renders an Error status and a "Rejected by the Inspector"
alert rather than the green resultType badge alone. The SDK gives no
request id with a decode failure, so the id is recovered by correlation;
see the comment on `markResponseRejected` for why that is exact.
Also stops `listAllTools` from dropping a failing excluded-tools walk on
the floor — it stays non-fatal but is logged.
Closes#1953
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SW1p8E2uiyyLx4RKwrwSrt
Copilot's third pass (suppressed comment) caught the remaining hole in the
child-process handling: `spawn()` reports a failure to start via an `error`
event, not `exit`. With no `error` listener Node throws it uncaught, replacing
the smoke's diagnostic with a raw stack — and because `exit` never fires in that
case, the readiness poll would otherwise spin for the full 30s before reporting
a timeout that misattributes the cause. `close` is now listened to alongside
`exit` for the same reason: it fires in cases `exit` does not.
prod-web-server.mjs already documents this exact hazard for the launcher child;
this brings the test-server child in line.
Both new branches verified by hand:
- unspawnable executable → "could not spawn the MCP test server (…): spawn
/nonexistent/node-binary ENOENT", immediately, instead of an uncaught throw
- missing module → "MCP test server exited early" with the loader stack, fast,
with no 30s hang
Full `npm run ci` green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAR2Nr9kXbrywFNUVoTe9F
Copilot's second pass (suppressed comments) caught a real gap: the smoke
listened only for `pageerror`, which is the *synchronous* half of the
uncaught-crash class. Its async twin — an unhandled rejection, or a failed
dynamic import (this app lazy-loads chunks) — is not a `pageerror`; Chromium
reports it on the console channel. So an async crash during the app-open path
could leave the smoke green.
smoke-web-browser.mjs already solved this; this now mirrors it exactly — same
FATAL_CONSOLE pattern, same split between hard failures and diagnostics, so
benign noise (a font-CDN miss, a React warning) still can't flake CI. Fatal
console errors are now included both in the catch-block diagnostics and in the
final assertion, which is where they were missing.
Confirmed working on a passing run: a benign 409 subresource error is reported
as a non-fatal diagnostic rather than failing the smoke.
Full `npm run ci` green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAR2Nr9kXbrywFNUVoTe9F
Copilot review on #1934, both comments actionable:
- smoke:web:app defaulted to 6299 — the same port smoke:web uses — while its
own comment claimed the port was "distinct … so back-to-back runs can't
collide". The three web smokes run sequentially in `npm run smoke`, so it
passed, but a slow teardown, a TIME_WAIT socket, or a parallel run would
EADDRINUSE it. Moved to 6297 (smoke:web 6299, smoke:web:browser 6298) and
rewrote the comment to name the actual values rather than assert distinctness.
- The showcase table tells readers to connect with Protocol Era = Modern unless
noted, and mcp-app-http.json is legacy-only. That was stated in the prose
below but not in the row, so a reader scanning the table would try Modern
first. Marked the row.
Verified: smoke:web:app passes standalone on 6297, and the full `npm run smoke`
chain passes with all three web smokes on distinct ports.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAR2Nr9kXbrywFNUVoTe9F
`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
The `mcp_app_demo` tool and `mcp_app_demo_widget` UI resource presets already
existed in the preset registry, but no config wired them into a server — so
there was no runnable way to reach a rendered MCP App, and therefore no way to
exercise the sandbox proxy path that #1859 broke.
`mcp-app-http.json` composes the two over plain streamable-HTTP. With it, the
Apps tab renders a real widget and the packaging bug is reproducible on demand:
without clients/web/static in the tarball the widget area shows "Sandbox not
loaded: ENOENT … /clients/web/static/sandbox_proxy.html" instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAR2Nr9kXbrywFNUVoTe9F
`clients/web/static/sandbox_proxy.html` is a committed source file, not a
build artifact, and it was never listed in the root package.json "files"
allowlist. `sandbox-controller.ts` reads it at runtime as
`<runner dir>/../static/sandbox_proxy.html`, so it resolved fine in-repo but
was absent from every published tarball — the read threw and the controller
served its "Sandbox not loaded" fallback page, breaking the Apps tab for
anyone running `npx @modelcontextprotocol/inspector`.
v1 shipped the equivalent as `server/static` in both the root and the server
workspace "files" lists; that entry has no counterpart in v2.
Add `clients/web/static` to the allowlist, and assert it in pack:verify twice
over — once in the tarball packlist and once on disk after install, since what
the runtime needs is the path *relative to* clients/web/build rather than mere
presence in the tarball.
No .npmignore change is needed: clients/web/.gitignore does not list `static`,
so the nested-gitignore packlist hazard that hid `build/` does not apply here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAR2Nr9kXbrywFNUVoTe9F
The version label says which line work belongs to; nothing said what KIND of
work it is. In practice that meant reaching for bug or enhancement and
pressing everything else into one of them — a migration guide and a
tsup->tsdown swap both landing as "enhancement", which degrades the label to
"not a bug" and makes filtering by it meaningless.
Every issue created or triaged now carries exactly one of bug, enhancement,
documentation, chore, or question, independent of its v1/v2 label. Adds a
table of what each is and is not for, and states the anti-pattern explicitly
so the two familiar labels don't absorb the other three. A PR needs no type
label — it is classified through the issue it closes, the same way it is
tracked through that issue's board card.
Wires the requirement into the three places that would otherwise drift: the
"not created until" callout, triage pass 1 (an outside reporter can set
neither label, so both are applied in the same step), and the board audit,
which gains an "open, no type label" check and a matching row in the
invariant table.
Note `chore` did not exist as a repo label and was created for this; the
existing `dependencies` label is Dependabot's and scoped to PRs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
A board card in Done asserts that the work shipped, and Done is read as the
record of what a milestone actually delivered. Nothing said so, so an issue
closed as a duplicate or not-planned was being moved to Done like any other
close — which makes that record wrong in a way no later reader can detect:
counting Done cards can no longer tell a shipped fix from a report closed as
a duplicate of one.
New rule: exactly two things earn a card a place in Done — its PR merged, or
it is a parent whose last sub-issue closed. Every other close (duplicate,
won't fix, not planned, obsolete, superseded) means nothing shipped, so the
card is deleted instead. Deleting a card touches only the board; the issue
keeps its labels and comments and stays searchable, so nothing is lost.
Also notes that the close reason is the machine-readable form of the same
distinction, and that `gh issue close --reason` cannot express `duplicate` —
it accepts only completed/not planned, so duplicate must be set through the
API or the web UI's "Mark as duplicate" (which also records a duplicate-of
link).
Adds the invariant to the board audit as a tenth check. That required the
audit to read closed issues too, so its fetch moves to --state all with
stateReason, `open(n)` becomes a state test rather than "present in the map",
and --limit must now clear the repo's total issue count, not just the open
ones. The two wrong-board label checks are scoped to open issues in the same
pass — over all history they flag four long-closed cards whose labels nobody
intends to rewrite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
Follow-up on the same issue: the triage rules were clear on the happy path
but three gaps meant a bare "triage new issues" would diverge from what a
maintainer actually does.
Define triage by STATE, not authorship. An issue needs triage when it
arrives with no board card and no milestone — whoever filed it. Framing it
as "externally-filed" was wrong: a maintainer opening an issue by hand in
the GitHub UI lands in exactly the same state, because write access makes
the board reachable, not automatic. What puts an issue in Todo is somebody
performing the approval, not who owns the account. Renamed the section and
reworded every dependent passage accordingly.
Fix the discovery query, which diffed open issues against board #28 only.
Verified against the live boards: it reports #1929 as unboarded when that
issue is correctly carded on #11, so following it literally double-boards
the issue — recreating a defect a past sweep introduced. Now diffs against
the union of both boards, filtered by .content.repository (an org project
can hold other repos' issues; #11 carries one from modelcontextprotocol/
servers), and prints each hit's destination, since an unboarded issue that
already has a milestone is approved and belongs in Todo, not Incoming.
Add "The board audit" — the other drift classes a single-issue rule can't
catch (double-boarded, non-Issue items, statusless cards, the Incoming ⇔
milestone invariant in both directions, wrong board for label, missing
version label, missing Priority), as a table of invariant + fix plus one
runnable jq check that should print 0 across the board. Both snippets were
executed verbatim against the live boards.
Add "Recording the score". The rubric claimed its reasoning "survives in a
form someone can argue with later", but the board stores only the result
and is private, so nothing kept that promise. Triage now posts the axes,
the bonuses claimed, and the total as a comment.
Also scope pass 2 explicitly to a human — deciding what ships in which
release is not inferable from a rubric — and correct the stale note about
three statusless cards on #11.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
AGENTS.md had no rule linking a milestone to a board Status, and its two
existing rules contradicted each other: every new issue was to be milestoned
at create time, yet new items were to start in Incoming, with Todo reserved
for work a maintainer had approved. If every issue is milestoned at birth,
"has a milestone" carries no approval information and nothing records the
moment of sign-off.
Make assigning a milestone the approval act, giving one invariant:
Incoming <=> no milestone; everything past it <=> milestoned. External
reports and maintainer-filed issues then differ only in whether approval has
already happened -- an outside reporter cannot set a label, milestone, or
board field, so their issue arriving with none of them is normal rather than
a defect to fix on arrival.
- scope the "not created until five things" callout to maintainer-filed issues
- split the Status bullets into the two cases
- scope the milestone mandate to issues you create, stating the external
exception explicitly
- add a "Triaging externally-filed issues" section: the two passes, plus a
gh/jq snippet that diffs open issues against the board to find unboarded ones
- reword both prose descriptions of the Incoming/Todo line
- reword the rubric's "Linked to a milestone" bonus as "already approved" and
note it is the re-scoring case only; under the old rules it was a constant
offset on every issue, shifting every band boundary down by one
- update both gh recipes (#28 and #11) to default to Todo, with an Incoming
variant for triage
No mirror to .github/copilot-instructions.md: board and milestone mechanics
are on that file's deliberately-absent list.
Closes#1930
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
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>
* 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>
GHSA-p63j-vcc4-9vmv (**critical**) — `@vitest/browser` Browser Mode provider
commands accept a file path from the browser and act on it without checking the
`allowWrite` permission gate. Patched in 4.1.10; `clients/web` was pinned at
4.1.0.
Dev-scope only — `@vitest/*` is absent from the published `files` allowlist, and
Browser Mode here runs this project's own Storybook play functions, never a
third-party page. That is why #1837 deferred it rather than attempting a
toolchain upgrade immediately before an irreversible publish.
## The knot, and what actually unties it
`^4.1.0` already admits 4.1.10, so the declared ranges were never the problem.
The family is welded by *exact* peer pins — `vitest` pins
`@vitest/browser-playwright`, which pins `@vitest/browser` — so nothing moves
alone.
The issue predicted that an explicit coordinated `npm i -D` would fix it. It
does not. With `clients/web/package-lock.json` in place, npm ERESOLVEs even when
every member of the family plus the Storybook packages are named in one command
and `node_modules` is deleted first:
Found: @storybook/addon-a11y@10.2.19
Could not resolve dependency: dev @storybook/addon-a11y@"^10.5.5"
Conflicting peer dependency: storybook@10.5.5
npm resolves against the existing lock tree and will not move the whole
constellation at once. **Regenerating `clients/web/package-lock.json` is the
only thing that unties it** — a clean resolve places all four at 4.1.10 with no
peer complaints at all.
## Versions
vitest 4.1.0 -> 4.1.10
@vitest/browser 4.1.0 -> 4.1.10 (transitive)
@vitest/browser-playwright 4.1.0 -> 4.1.10
@vitest/coverage-v8 4.1.0 -> 4.1.10
storybook + @storybook/* 10.2.19 -> 10.5.5
eslint-plugin-storybook 10.2.19 -> 10.5.5
@chromatic-com/storybook 5.0.1 -> 5.2.1
`clients/{cli,tui,launcher}` were already resolving 4.1.9 (no browser pins, so
they floated freely) and were never vulnerable. Their declared `^4.1.0` still
*permitted* 4.1.0 on a fresh resolve, so the floor is raised to `^4.1.10` in all
four manifests — same reasoning as the vite floor in #1841. Pinning only the
lockfile would have left a regression path open.
## The vite 8 peer override
Resolved, as a consequence rather than a workaround.
`@joshwooding/vite-plugin-react-docgen-typescript@0.6.4` declared
`peer vite@"^3 || ^4 || ^5 || ^6 || ^7"` against our vite 8. It is not a direct
dependency — it arrives through `@storybook/react-vite`, which at 10.5.5 depends
on `^0.7.0`, and 0.7.0 adds `^8.0.0` to that range. So taking Storybook 10.5.5
for the peer set drops the override too; nothing is forced.
## Two holds the regeneration forced, each with a tracked issue
Regenerating the lock floats every other in-range dependency to current. Most of
that is welcome — it is what takes `clients/web` from 19 advisories to 1 — but
two floats broke the gate and are unrelated to this security fix:
- **zod 4.3.6 -> 4.4.3** makes `clients/web`'s `tsc -b` die with
`FATAL ERROR: Ineffective mark-compacts near heap limit` at the ~4GB default.
Reverting zod alone makes it exit 0, so the attribution is unambiguous.
Held at `~4.3.6`. See #1896.
- **eslint-plugin-react-hooks 7.0.1 -> 7.1.1** enables `set-state-in-effect`,
which fails `lint` on 8 pre-existing violations across 7 components. Fixing
them is a real refactor with interaction-behavior consequences, not a
suppression. Held at `~7.0.1`. See #1897.
Both `~` pins are deliberate and temporary, and both issues say so — they are
constraints to remove, not preferences.
Storybook 10.5.5 also reports that `setProjectAnnotations` is applied
automatically since 10.3, making `.storybook/vitest.setup.ts` redundant. Left in
place: `./preview` carries the Mantine decorator and the a11y annotations drive
the play-function assertions, and if automatic provisioning missed either, the
462 stories would render unthemed with a11y checks inert and very likely still
pass. That silent-failure risk does not belong in a security bump. See #1898.
## Audit delta
`clients/web`, where the advisory lived:
before 19 (4 critical, 6 high, 6 moderate, 3 low)
after 1 (1 low — esbuild-in-tsup, Windows dev server only)
All four critical entries — `vitest`, `@vitest/browser`,
`@vitest/browser-playwright`, `@vitest/coverage-v8` — are gone. The other 14 are
transitive dev-tree advisories (hono, ws, js-yaml, flatted, brace-expansion, ...)
cleared as a side effect of the clean resolve. Elsewhere: cli 6 -> 5,
launcher 3 -> 2, tui 4 -> 4, root unchanged at 9 (its lock is untouched). Those
remainders are pre-existing and unrelated.
## Verification
`npm run ci` green end to end on the upgraded stack — the point of caring here,
since this *is* the test infrastructure:
- coverage gate (>=90 per file, all four dimensions) passes on all four clients;
no file regressed, so no gate was lowered and no `v8 ignore` added
- 4802 web tests, 304 cli, 282 tui, 5 launcher
- 462 Storybook play functions across 109 files, on vitest 4.1.10 Browser Mode
- verify:build-gate, and all five smokes including `smoke:tui` (real TTY here)
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* 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>
* docs: add Incoming board status, adopt Priority field, document triage rubric (#1891)
Board #28 had no state for "filed but not yet triaged" — Todo means
maintainer-approved, so parking unreviewed issues there falsely asserted
sign-off. Adds an Incoming status ahead of Todo.
The board's Priority single-select existed but was unused (0 of 264 cards)
and undocumented, with P0/P1/P2 options matching no vocabulary in the repo.
Re-optioned to Urgent/High/Medium/Low and documented alongside Status.
Completes the priority rubric: fills in the score bands (12+/9-11/6-8/<=5),
expands both axes into scored tables, and fixes the truncated signal-indicator
line. Severity alone tops out at High by design.
Also drops the "V2 Go Live" status reference — that option is not in the
field's option list.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
* docs: correct the Done option id and harden the deleted-column guidance (#1891)
The Done Status option was deleted from board #28, blanking the Status of
all 247 cards that held it. Restored from a pre-change item-list snapshot;
verified all 247 are Done again and no other card was disturbed.
A recreated option never regains its old id, so Done is now 259d6aab rather
than 248a3910 — updated in the option table and the merge step.
The hazard callout recommended the web UI as the safe path, which is what
made this look safe. It is not: deleting an option blanks its cards in the
UI exactly as it does via the API, with no undo. Corrects that, adds a
snapshot-first rule, and documents the recovery recipe used here — including
the grouping check that proves the orphaned set is exactly the deleted
option's cards before re-applying.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
* docs: add Incoming to the v1 board and document its recipes (#1891)
Board #11 had the same gap as #28 — no state for "filed but not yet
triaged" — so a v1 security-fix issue had nowhere to sit that didn't
assert maintainer approval. Adds Incoming there too.
AGENTS.md carried no v1 board recipes at all, only a link, so anyone
boarding a v1 issue had to discover the project and field ids by hand
and was liable to reach for #28's (which are rejected, but only after
the attempt). Adds a V1 board section with its ids, states that Incoming
is the default status for new items on both boards, and notes that
Priority is v2-only — #11 has no such field.
The Incoming option was added with all four existing option ids echoed
back, and a before/after item-list diff confirms all 74 cards kept their
Status. Three cards on #11 carry no Status; that predates this change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
* docs: distinguish the board Priority from the org-level issue field (#1891)
An issue page shows two fields named "Priority" and nothing syncs them.
Ours is the project field on board #28. The other is a GitHub issue field
(IFSS_kgDOAdAWeg) defined at the modelcontextprotocol org and shared by
every repo in it, alongside Effort/Start date/Target date. Same name, same
four option names, unrelated storage — #1891 sat at Urgent in Fields and
High on the board simultaneously.
Adds a table naming both, states that no pass-through exists in either
direction, and says to leave the org-level one alone: it is not repo-scoped,
so deleting it would strip Priority from every other org repo.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
* docs: mark the org Priority field untrusted and cap it at a +1 signal (#1891)
Both boards are private (public: false, verified), so maintainer-assigned
Status and Priority are invisible to reporters — board priority is a
working queue, not a published commitment.
The org-level Fields -> Priority is the opposite: public on the issue page
and not part of triage. A value there is a reporter's preference, not an
assessment, so it is untrusted input. It now feeds the rubric as a sixth
signal bonus — flat +1 for Urgent or High, identical for both — and never
maps to a band. Urgent needs 12, so nothing a reporter types reaches it
alone; the issue must already sit at 11 on maintainer-assessed axes.
Supersedes the earlier "it is noise" framing: the signal is worth keeping,
it just cannot be allowed to decide the outcome. Also corrects the band
arithmetic (2-15 -> 2-16) now that a sixth bonus exists, and notes that
scores recorded on 2026-08-01 cite the old /15 denominator.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
* docs: drop the /15 denominator caveat now the recorded scores are restated (#1891)
The 51 triage comments were rewritten from /15 to /16, so the note saying
they cite the old denominator no longer describes anything. No total moved:
the reporter-set signal only scores for Urgent or High, and the one scored
issue carrying an org-field value (#1826) had Low, which earns nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
* docs: describe the board-option hazard per field, not Status-only (#1891)
Copilot review on #1892 flagged three places that still read as
Status-only now that the section covers Priority too:
- the version-label bullet said a v2 issue needs "a board card with a
Status"; it needs a Status and a Priority (v1 needs only a Status,
since board #11 has no Priority field)
- the updateProjectV2Field hazard said a bad edit "orphans the Status
of every card"; it orphans whichever field was edited
- the recovery recipe is genuinely Status-specific (reads .status,
writes the Status field id), so say so and give the two
substitutions for a Priority deletion
Verified `gh project item-list --format json` exposes both .status and
.priority before documenting the .priority swap.
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>
* fix(scripts): await the pack:verify web child's exit before removing its work dir
`pack-and-verify.mjs` had the same kill-then-remove shape that made
`smoke:tui` flake with ENOTEMPTY in #1801: `verifyWeb()`'s
`finally { stop() }` only *delivered* SIGTERM, then the main flow
`rmSync`'d the work dir immediately, and `fail()` did the same back to
back.
Applies the #1814 treatment, factored into a shared
`scripts/lib/child-cleanup.mjs` so the two scripts can't drift:
- `stopChild()` — SIGTERM, await the child's `exit`/`close`, escalate to
SIGKILL after a grace period, and ultimately proceed with a warning so
a wedged child can never hang the script. `verifyWeb()`'s `stop()` now
goes through it and is awaited, so the child is gone before the caller
removes `work`.
- `removeSafe()` — `rmSync` that warns instead of throwing. Used on both
success-path removals (a leftover temp dir must never fail a run that
passed) and in `fail()`, where an ENOTEMPTY could otherwise bury the
real diagnostic under an rmSync stack and skip the "tarball retained
at ..." hint. `fail()` stays synchronous — it is called from sync
contexts and ends in `process.exit()` — so it best-effort signals the
child; `removeSafe()` is what makes that safe.
`smoke-tui.mjs` adopts `removeSafe()` for its `cleanup()`. Its `done()`
keeps its bespoke wait (documented inline): that one must keep waiting
for `close` *after* `exit` on a shorter re-armed deadline so the quoted
output is complete, which `stopChild()`'s first-of-exit/close contract
deliberately does not do.
Adds `scripts/lib/child-cleanup.test.mjs` (10 cases, `node --test` via
`npm run test:scripts`) covering the SIGTERM/SIGKILL escalation, the
`close`-only spawn-failure path, the never-hang give-up, late events
after giving up, and that `removeSafe` warns rather than throws.
`npm run ci` passes, and `npm run pack:verify` (not in the gate — needs
network) was run end to end and passes with no cleanup warnings.
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scripts): normalize stopChild's graceMs and correct removeSafe's JSDoc
Copilot review round 1 on #1895.
- `stopChild()` trusted `graceMs` as given, but call sites build it as
`Number(process.env.X ?? 5000)` — a typo yields `NaN`, which
`setTimeout` treats as `0`. That would fire SIGTERM and SIGKILL in the
same tick and print "did not exit within NaNms", collapsing the whole
escalation silently. New exported `normalizeGraceMs()` falls back to
`DEFAULT_EXIT_GRACE_MS` for anything non-finite or non-positive, with
two tests: the table of bad values, and an end-to-end one asserting a
`NaN` grace period still leaves only SIGTERM sent a turn later.
- `removeSafe()`'s `@returns` said "true if the path was removed", which
is inaccurate for a missing path (`force: true` makes it a no-op
success). Reworded to say what it actually reports.
`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>
The 2.0.0-rc series shipped these dependency bumps to `main` only, so
`v2/main` was developing against older versions and would have met them
for the first time at the milestone merge.
- `@hono/node-server` ^1.19.14 -> ^2.0.12 (root, clients/web) — a major.
v2.0.0's only breaking changes are dropping Node 18 (the repo already
requires >=22.19.0) and removing the `@hono/node-server/vercel` adapter
(unused here). The used surface — `serve()`, `serveStatic`, `ServerType`
— is unchanged, so no source edits were needed.
- `vite` ^8.0.0 -> ^8.1.5 (root, clients/web) and ^8.0.16 -> ^8.1.5
(clients/tui) — the security bumps from #1841.
`npm install` at the root (postinstall cascades into every client)
regenerated the three touched lockfiles. `npm run ci` passes.
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore: restore repo files dropped by the v2 tree swap
A sweep of the pre-swap tree (ac3c1a12) against v2/main turned up 14 files the
tree swap dropped and that were never restored to either branch. Most are
correctly gone (v1 CI workflows, v1 sample-config, husky hook, two 0-byte
files, v1-specific prettier config). These four are worth having:
- .github/dependabot.yml — rewritten, not restored. v1's covered only
github-actions at /; this covers github-actions plus the six npm manifests
v2 actually has (root + four clients), monthly rather than weekly, grouped
to one PR per ecosystem/directory, targeting v2/main, and labeled `v2` so
the version-label rule holds for bot PRs too.
- .github/ISSUE_TEMPLATE/bug_report.md — rewritten. v1's asked for an
"Inspector Version e.g. 0.16.5" and had no client or version field. Under
the issues-only policy this template is the primary intake, so it now asks
the routing questions first (v1 vs v2, which client) and points at the
prompt-not-a-diff path.
- .gitattributes — restored byte-for-byte (package-lock.json
linguist-generated=true). More useful in v2 than v1: there are now six
lockfiles, and they dominate dependency-bump diffs.
- CODE_OF_CONDUCT.md — restored byte-for-byte, so GitHub surfaces it as a
community standard. Left unformatted deliberately: it is verbatim upstream
Contributor Covenant text, v1 listed it in .prettierignore for that reason,
and no v2 format glob reaches root markdown.
CONTRIBUTING.md is deliberately not restored here — it is handled by renaming
CONTRIBUTORS.md to that name, which also lights up GitHub's contributing
banner without leaving two files to keep in sync.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
* docs(dependabot): state the documented target-branch/security-update rule
Copilot review: the comment read as if `target-branch` had no bearing on
security updates. Per GitHub's Dependabot options reference, an entry whose
`target-branch` names a non-default branch is not applied to security
updates at all — so the schedule/labels/groups here shape version updates
only, and security PRs continue to be raised against the default branch.
Comment-only; the configured settings are unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
* docs: correct the branch description and the labeling ask
Two suppressed Copilot comments, both correct:
- dependabot.yml called `main` "release-only". It isn't — README.md and
AGENTS.md both describe it as the legacy v1 implementation taking security
and bug fixes only. Reworded to match, keeping the point that matters
(Dependabot reads config from the default branch).
- bug_report.md asked reporters to label the issue `v2`/`v1`, which most
external reporters have no permission to do. It now asks only for the
version checkbox and says a maintainer labels it at triage.
Comment/prose only; no configured Dependabot setting changed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
* docs(dependabot): describe main as release-only, not "legacy v1"
Reverts the branch description introduced in c7fbf350. That change was made to
match README.md:17 and AGENTS.md — but those are precisely the stale sentences
PR #1866 exists to correct. `main` has not been the legacy v1 implementation
since the go-live tree swap; it is the release branch holding the latest
released v2, and the deprecated v1 line lives on `v1/main`.
So the comment now describes the real model: v2/main is the develop branch,
main is release-only and receives milestone merges, v1/main is maintenance and
takes security fixes only.
The rest of c7fbf350 stands — the bug_report.md change (asking reporters for
the version checkbox rather than to apply a label they lack permission to set)
was a genuine catch.
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>
* ci: restore the Claude Code workflow, lost in the v2 tree swap
Closes#1850.
Restored byte-for-byte from ac3c1a12 (sha256
1c44f2d9d51c936adb40c9acefd4b44628fe7810360e2f8196034347f9d2602d,
3592 bytes). No content changes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
* ci: restore .mcp.json, which claude.yml's --mcp-config points at
The restored workflow runs with `--mcp-config .mcp.json` and
`--allowedTools "Bash,mcp__mcp-docs"`, so without this file the mcp-docs
server never registers and those tools do not resolve.
Lost in the same tree swap: .mcp.json survives only on v1/main, and is
absent from both main and v2/main. Restored byte-for-byte from 47f92841
(sha256 b0f0b8ea0d6c6e4bd9d19270db8c4ed13a951c6a7d8fa9a7b76d0d12ddd2491f,
123 bytes). No content changes.
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>
Closes#1864.
SECURITY.md was restored to main in #1843 but never added to v2/main.
Because v2/main is the develop branch merged into main at milestones, and
the two share no common git ancestor, the next milestone merge would
silently remove the security policy from the default branch a second time.
Copied byte-for-byte from main (sha256
bbc8aaa5a33d481f902c1b7ac260a4c9c0d41dac5bb2916607222dd205dda924,
2633 bytes). No content changes.
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* 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>
* docs: rename CONTRIBUTORS.md to CONTRIBUTING.md (#1883)
GitHub's contributing-guidelines banner — shown when someone clicks
"New issue" or opens a pull request — keys on the exact filename
CONTRIBUTING.md. Naming the file CONTRIBUTORS.md left that prompt dark
precisely where the "open an issue, not a PR" policy needs to land.
Renamed with `git mv` so history follows the file; contents are
unchanged (its heading already reads "Contributing to MCP Inspector").
Updated the one in-repo reference, in AGENTS.md. The two links in
.github/pull_request_template.md are deliberately untouched — PR #1879
makes them relative so they follow the rename.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
* docs: drop the AGENTS.md link edit; #1866 owns that line
This PR edited the `See [CONTRIBUTORS.md](./CONTRIBUTORS.md)` link in
AGENTS.md, which is the exact line #1866 rewrites — guaranteeing a one-line
conflict when this merges after it.
Handing that line to its owner instead: #1866 now writes the link as
`CONTRIBUTING.md` directly, so this PR is a pure `git mv` and the two touch no
common line. No conflict to resolve at merge time.
The tradeoff is a short window between the two merges where AGENTS.md links to
a filename that doesn't exist yet. That's deliberate and bounded: #1866 is
first in the merge order and this 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>
* docs: converge README Test servers section with main's restructure
Transplants main's restructured "Test servers" section (#1836) onto
v2/main — the "Serving the modern protocol era" and "Showcase configs"
headings, the config table, and the per-config subsections — replacing
the eight long paragraphs v2/main still carried.
Folds in v2/main's #1846 correction: main's copy still claims
Mcp-Param-* mirroring is skipped in the browser, which stopped being
true when the Inspector took over building the mirrored headers itself.
The blockquote callout keeps main's shape with the corrected wording.
Scoped to that section only — the whole file is deliberately NOT copied
from main, so #1866's repo-status callout and release-section edits (and
every other v2/main-only line) are untouched.
Shrinks the first v2/main -> main milestone merge from a whole-section
conflict to a single 3-line hunk.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
* docs: point the #1846 reference at the issue, not a nonexistent PR URL
#1846 is the issue; the fix shipped as PR #1847. The callout's link used
/pull/1846, which 404s. Per Copilot review.
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>
* 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
Cliff Hall