modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

chore: merge v2/main for the v2.5.0 milestone release

Browse source 
The fifth v2/main -> main milestone merge, cutting 2.5.0: 33 merged PRs
and 213 non-merge commits, including the 2.4.0 -> 2.5.0 version bump
(#2213), which landed on v2/main first per the #2010 procedure.

Zero conflicted paths, and the merged tree is byte-identical to
origin/v2/main.

References #2214

Signed-off-by: Cliff Hall <cliff@futurescale.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017bussXiM47PBttWqFUkPN1
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-09-01 19:19:39 -04:00
commit d92a30ff84
333 changed files with 48357 additions and 17800 deletions
Download patch file Download diff file Expand all files Collapse all files

287
.claude/skills/board-ops/SKILL.md Normal file
View file

@ -0,0 +1,287 @@
---
name: board-ops
description: "gh recipes for the Inspector project boards — add a card, move its Status, set or re-score its Priority, delete a card, and recover from a deleted single-select option. Covers board #28 (v2) and board #11 (v1), their node/field/option IDs, and the option-deletion hazard."
disable-model-invocation: false
---
# Board operations
The rules about *what* a card's Status and Priority should be live in
[`AGENTS.md`](../../../AGENTS.md) under **Issue-driven Work Style**. This skill is
the *mechanics*: the exact `gh` calls, the IDs, and the ways the board can be
damaged.
Related: `/issue-create` (the five-step create flow), `/issue-triage` (sweeping
unboarded issues in, the priority rubric, the board audit).
## Which board
| Version label | Board | Owner | Has a Priority field? |
| --- | --- | --- | --- |
| `v2` | [#28](https://github.com/orgs/modelcontextprotocol/projects/28) | `modelcontextprotocol` | Yes |
| `v1` | [#11](https://github.com/orgs/modelcontextprotocol/projects/11) | `modelcontextprotocol` | **No** |
Both are **org** projects, so every command takes `--owner modelcontextprotocol`.
The two projects have their own field and option IDs and none of them are
interchangeable — a #28 id passed to #11 is rejected with "option Id does not
belong to the field", so the mistake is at least loud.
**Only issues go on a board — never PRs, never draft cards.** A PR is tracked
through the card of the issue it closes.
## V2 board (#28) IDs
The project node id and the field ids are stable. The **option** ids are **not** —
they are regenerated whenever a single-select field's option list is edited (see
the hazard below). If any option id here is rejected, re-fetch:
```sh
# Swap "Status" for "Priority" to fetch the other field's options.
gh project field-list 28 --owner modelcontextprotocol --format json \
| jq '.fields[] | select(.name=="Status") | .options'
```
| Thing | ID |
| --- | --- |
| Project node ID | `PVT_kwDOCt2Azc4BJVxt` |
| Status field ID | `PVTSSF_lADOCt2Azc4BJVxtzg5iI8c` |
| Priority field ID | `PVTSSF_lADOCt2Azc4BJVxtzg5iJE4` |
Status option IDs (`--single-select-option-id`) — **last verified 2026-08-01**.
| Status | Option ID | Means |
| --- | --- | --- |
| Incoming | `721a3d4c` | Arrived unboarded, awaiting review — **no milestone** |
| Todo | `fbdaf21e` | Approved (a milestone was assigned) |
| In Progress | `195df262` | Active work, whatever the surface |
| In Review | `159c8a02` | A PR is open |
| Done | `259d6aab` | **Shipped** — a merged PR, or a parent whose last sub-issue closed |
Priority option IDs — **last verified 2026-08-01**. Derive the level with the
rubric in `/issue-triage`; don't eyeball it.
| Priority | Option ID | Rubric total |
| --- | --- | --- |
| Urgent | `79628723` | 12+ |
| High | `0a877460` | 9–11 |
| Medium | `da944a9c` | 6–8 |
| Low | `d67ac7ce` | ≤5 |
## V1 board (#11) IDs
The v1 line takes security fixes only, so this board sees little traffic — but a
v1 issue still gets a card, and the same Incoming/Todo split applies.
| Thing | ID |
| --- | --- |
| Project node ID | `PVT_kwDOCt2Azc4BA5sz` |
| Status field ID | `PVTSSF_lADOCt2Azc4BA5szzgzkS-g` |
Status option IDs — **last verified 2026-08-01**.
| Status | Option ID |
| --- | --- |
| Incoming | `831820cf` |
| Todo | `f75ad846` |
| In Progress | `47fc9ee4` |
| In Review | `0439b2bf` |
| Done | `98236657` |
There is **no Priority field on this board** — the priority rubric is v2-only.
Don't try to set one here; the field id doesn't exist.
## Recipes
### Add a card and set its fields
```sh
# Prints the item id (PVTI_…); capture it.
ITEM_ID=$(gh project item-add 28 --owner modelcontextprotocol --url <issue-url> --format json --jq '.id')
# Status → Todo (an issue you filed through the create flow is approved by definition)
gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" \
--field-id PVTSSF_lADOCt2Azc4BJVxtzg5iI8c --single-select-option-id fbdaf21e
# Priority → Medium
gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" \
--field-id PVTSSF_lADOCt2Azc4BJVxtzg5iJE4 --single-select-option-id da944a9c
```
Each `item-edit` sets **one** field, so setting both takes two calls — there is
no combined form.
For an issue swept in at triage, the only difference is Status → **Incoming**
(`721a3d4c`) and that you do **not** set a milestone.
For **v1**, the same shape against board #11:
```sh
ITEM_ID=$(gh project item-add 11 --owner modelcontextprotocol --url <issue-url> --format json --jq '.id')
gh project item-edit --project-id PVT_kwDOCt2Azc4BA5sz --id "$ITEM_ID" \
--field-id PVTSSF_lADOCt2Azc4BA5szzgzkS-g --single-select-option-id f75ad846
```
### Move an existing card
Look the item id up by issue number rather than re-adding it. Keep `--limit`
above the board's item count (~265 as of 2026-08-01) — past it `item-list`
truncates **silently**, `select` matches nothing, and `item-edit --id ""` fails
with an opaque node-resolution error rather than saying the limit was too low.
⚠️ **Filter by repository, not by number alone.** These are **org** projects and
issue numbers are **repo-local**, so an unfiltered `select` can match another
repo's issue that happens to share the number — board #11 really does carry a
`modelcontextprotocol/servers` card — and then moves or deletes the wrong card,
or passes two ids at once (Copilot).
```sh
ITEM_ID=$(gh project item-list 28 --owner modelcontextprotocol --format json --limit 500 \
--jq '.items[] | select(.content.repository=="modelcontextprotocol/inspector"
and .content.number==<ISSUE_NUMBER>) | .id')
# e.g. Status → In Review, when its PR opens
gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" \
--field-id PVTSSF_lADOCt2Azc4BJVxtzg5iI8c --single-select-option-id 159c8a02
```
### Delete a card
**`Done` means the work shipped.** An issue closed as duplicate / won't fix /
not planned / obsolete / superseded shipped nothing, so its card is **deleted**,
not parked in Done:
```sh
ITEM_ID=$(gh project item-list 28 --owner modelcontextprotocol --format json --limit 500 \
--jq '.items[] | select(.content.repository=="modelcontextprotocol/inspector"
and .content.number==<ISSUE_NUMBER>) | .id')
gh project item-delete 28 --owner modelcontextprotocol --id "$ITEM_ID"
```
Deleting the card removes it from the board only — **the issue itself is
untouched**, keeps its labels and comments, and stays searchable and linkable
forever. Nothing is lost; the board simply stops claiming the work was
delivered. Done is read as the record of what a milestone actually delivered, so
a duplicate sitting there makes that record wrong in a way nobody can detect
later.
The close **reason** is the machine-readable form of the same distinction.
`gh issue close --reason` accepts only `completed` and `not planned`, so
**`duplicate` must be set through the API**:
```sh
gh api repos/modelcontextprotocol/inspector/issues/<N> -X PATCH \
-f state=closed -f state_reason=duplicate
```
(or "Mark as duplicate" in the web UI, which additionally records a
duplicate-of link).
## ⚠️ The option-deletion hazard
**Never add, rename, or remove an option on a single-select board field (Status
or Priority) with the `updateProjectV2Field` GraphQL mutation unless you pass
every existing option's `id`.** That mutation does a **full replace** of the
option list: resending options by name/color/description without their `id`s
makes GitHub **delete all existing options and mint new ones**, which **orphans
that field's value on every card on the board** *and* invalidates every option
id in the tables above. This has happened once, on Status (~197 items
reconstructed by inference).
Safe alternatives, in order of preference:
1. **Add or rename an option in the GitHub web UI** (Project → the field's
settings). This preserves the ids of untouched options.
⚠️ **Deleting is different, in the UI as much as in the API**: removing an
option blanks that field's value on every card that held it, with no undo and
no warning that says so.
2. If you must script it, first `gh api graphql` the current options **with their
`id`s**, then call `updateProjectV2Field` echoing back every existing option
**including its `id`**, appending only the new one.
`ProjectV2SingleSelectFieldOptionInput.id` is an optional `String`, so a mixed
list works. Verify afterward that no card lost its value — snapshot
`gh project item-list … --format json` before and after and diff; don't just
spot-check. Send those dumps to `$BOARD_TMP` too, for the reason above.
Both the `Incoming` Status option and the Urgent/High/Medium/Low Priority
options were added this way (#1891), with the before/after diff confirming all
264 cards kept their Status.
`gh project item-add` and `gh project item-edit` are always safe — they set a
card's value and never touch the field schema.
### Always snapshot before touching a field's options
One command, and it is the difference between a five-minute restore and
reconstructing statuses by inference:
⚠️ **Write it outside the repo.** [The boards are private](../issue-triage/SKILL.md),
so a snapshot is a full dump of item IDs and every card's Status and Priority.
Left in the working tree it is one `git add -A` away from being published in a
PR (Copilot).
```sh
BOARD_TMP=$(mktemp -d)
gh project item-list 28 --owner modelcontextprotocol --format json --limit 600 \
> "$BOARD_TMP/board-snapshot.json"
echo "snapshot: $BOARD_TMP/board-snapshot.json" # note the path; you need it to recover
```
### Recovering from a deleted option
This has happened twice — once via the API (~197 items, reconstructed by
inference) and once via the UI (the `Done` column, 247 items, restored from a
snapshot in minutes). With a snapshot the recovery is mechanical.
The recipe below is written for a deleted **Status** option. For a deleted
**Priority** option it is the same three steps with two substitutions: read
`.priority` instead of `.status` (`gh project item-list --format json` exposes
each single-select field under its lowercased name, so both keys are present),
and pass the Priority field id `PVTSSF_lADOCt2Azc4BJVxtzg5iJE4`.
```sh
# 0. Same temp dir the snapshot went to — keep every dump out of the worktree.
BOARD_TMP=${BOARD_TMP:-$(mktemp -d)}
# 1. Which cards lost their value, and what did they hold?
gh project item-list 28 --owner modelcontextprotocol --format json --limit 600 \
> "$BOARD_TMP/board-broken.json"
jq -r '[.items[]|select(.status==null)|.id]' "$BOARD_TMP/board-broken.json" \
> "$BOARD_TMP/lost-ids.json"
jq -r --slurpfile L "$BOARD_TMP/lost-ids.json" '($L[0]) as $lost
| [.items[] | select(.id as $i | $lost|index($i)) | .status // "(none)"]
| group_by(.) | map({s:.[0],c:length}) | .[] | "was \(.s): \(.c)"' \
"$BOARD_TMP/board-snapshot.json"
# 2. Recreate the option, echoing every surviving option's id (see above).
# NOTE: the recreated option gets a NEW id — the deleted one never comes back.
# 3. Re-apply it to the orphaned cards.
for id in $(jq -r '.[]' "$BOARD_TMP/lost-ids.json"); do
gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$id" \
--field-id PVTSSF_lADOCt2Azc4BJVxtzg5iI8c --single-select-option-id <NEW_OPTION_ID>
sleep 0.4
done
```
Step 1's grouping is the safety check: confirm the orphaned set is exactly the
cards that held the deleted option, so you don't overwrite a card someone
legitimately moved in the meantime.
Because the recreated option carries a **new id**, the tables above and every
reference to it must be updated in the same change — `grep` the old id across
the repo. The `Done` id has been `248a3910` and is now `259d6aab` for exactly
this reason.
## ⚠️ Two different "Priority" fields
An issue page shows two fields named Priority, and they are unrelated. **Ours is
the one under _Projects → Inspector V2_.**
| Where it appears | What it is | Ours? |
| --- | --- | --- |
| **Projects → Inspector V2 → Priority** | The **project board** field on #28 (`PVTSSF_lADOCt2Azc4BJVxtzg5iJE4`) | ✅ Yes |
| **Fields → Priority** (above _Projects_) | A GitHub **issue field**, `IFSS_kgDOAdAWeg`, defined at the **org** level and shared by every repo in it | ❌ No |
Nothing syncs them, in either direction, and they will happily disagree.
**Never delete the org-level field** — it belongs to the whole org. Don't set it
either; a value there is a *reporter's* opinion and feeds the rubric as a capped
+1 signal bonus and nothing more (see `/issue-triage`).

30
.claude/skills/board-ops/evals/evals.json Normal file
View file

@ -0,0 +1,30 @@
[
{
"prompt": "Move the board card for issue 2189 to In Review.",
"expect": "board-ops"
},
{
"prompt": "How do I set the Priority field on a card in the v2 project board?",
"expect": "board-ops"
},
{
"prompt": "The project board rejected my Status option ID. How do I get the current ones?",
"expect": "board-ops"
},
{
"prompt": "I'm about to edit the options on one of the board's single-select fields. Should I do anything first?",
"expect": "board-ops"
},
{
"prompt": "Several cards lost their Priority value after the field was edited. How do I put it back?",
"expect": "board-ops"
},
{
"prompt": "Explain the difference between TCP and UDP.",
"expect": null
},
{
"prompt": "What year was the first Moon landing?",
"expect": null
}
]

134
.claude/skills/issue-create/SKILL.md Normal file
View file

@ -0,0 +1,134 @@
---
name: issue-create
description: Create a tracked issue in this repo end to end — version label, type label, milestone, board card, Status and Priority. Use when asked to file, create, open or raise an issue; when picking its labels or its milestone; when checking whether one already exists; or when work discovered mid-task needs an issue of its own.
disable-model-invocation: false
---
# Creating an issue
An issue **you** create is not "created" until every step below that applies to
its version is done. A label is a repo tag, the milestone is a release bucket,
and the board is a separate org project — `--label v2` does **not** add a board
card, and adding a card does **not** set a Status or a Priority.
| # | Step | v2 | v1 |
| --- | --- | --- | --- |
| 1 | **Version label** — exactly one of `v1` / `v2` | ✅ | ✅ |
| 2 | **Type label** — exactly one of `bug` / `enhancement` / `documentation` / `chore` / `question` | ✅ | ✅ |
| 3 | **Milestone** | ✅ | ❌ — every milestone is a v2 release bucket, so there is none to take |
| 4 | **A card on the board for that version** — `v2` → #28, `v1` → #11 | ✅ | ✅ |
| 5 | **Status** on that card | ✅ | ✅ |
| 6 | **Priority** on that card | ✅ | ❌ — board #11 has no such field |
The two ❌ rows are not oversights to fix later: the field does not exist for
that line, and `AGENTS.md` states both exceptions (Copilot).
Set the labels and milestone at **create time**, never by backfilling: an
unlabeled issue belongs to no version line and appears in no version-filtered
query, and an unmilestoned one drops out of release planning silently.
**Never create a duplicate.** Check the board for a matching item first.
**Never create a draft card** (a board card with no issue number) — every board
item is a real GitHub issue.
## 0. Check the board first
Search **all** states, not just open. A closed issue is still a duplicate — it
may have been completed, rejected, or filed and superseded — and refiling it
loses that history. When one matches, the call is whether to reopen it or file
genuinely new work; either way you need to have seen it (Copilot).
```sh
gh issue list --repo modelcontextprotocol/inspector --state all --limit 1000 \
--search "<keywords>" --json number,title,state,stateReason,labels,milestone
```
## 1. Pick the labels
**Version.** `v2` is the default for anything new. `v1` is reserved for the
narrow case of patching the deprecated line (security fixes only). If the target
version isn't obvious, it's `v2` — only ask when the issue is specifically a fix
*for released v1 behavior* and it's unclear whether v2 still has the bug.
**Type.** Independent of the version label; every issue needs both.
| Type | Use for | Not for |
| --- | --- | --- |
| `bug` | Something is broken, wrong, or regressed against its intended behavior | A missing capability that was never built |
| `enhancement` | A new capability, or extending an existing one — features, spec support, tracking issues | A cleanup with no behavior change |
| `documentation` | Prose deliverables — READMEs, guides, `specification/` docs, `AGENTS.md` rules | Code that happens to need a doc update |
| `chore` | Maintenance with no user-facing behavior change — deps, build/CI tooling, refactors | Anything a user would notice |
| `question` | An open question or discussion with no agreed deliverable yet | Work someone has already decided to do |
**Don't force the binary.** `bug` and `enhancement` are the two most reached
for, and pressing a docs task or a dependency pin into `enhancement` degrades it
to "not a bug", at which point filtering by it stops telling you anything.
## 2. Pick the milestone
If the user didn't specify one, default to the **current** milestone: the open
one with the nearest due date.
```sh
gh api repos/modelcontextprotocol/inspector/milestones --jq \
'map(select(.state=="open")) | sort_by(.due_on) | .[] | "\(.title)\tdue \(.due_on[0:10])\topen=\(.open_issues)"'
```
Milestones are **release** buckets (`v2.1.0`, `v2.2.0`, …), so pick by *when the
work ships*, not by size. If a new issue plainly can't make the current
milestone, say so and put it in the next one rather than leaving it blank.
Sub-issues normally inherit their parent's milestone.
⚠️ **Every milestone is a v2 release bucket.** There is no v1 bucket, so a `v1`
issue cannot satisfy the milestone rule — don't drop it in a v2.x one. Leave it
unmilestoned and say so.
## 3. Create it
```sh
gh issue create --repo modelcontextprotocol/inspector \
--title "<title>" \
--label v2 --label bug \
--milestone "v2.5.0" \
--body "<body>"
```
## 4. Board it, in Todo
Filing an issue for work you intend to happen **is** approving it, so it starts
in **Todo** with its milestone already set — not in Incoming, which is the queue
for issues nobody has evaluated yet. Work you are starting immediately goes
straight to **In Progress**.
Run the **add-card recipe in `/board-ops`** with **Status → Todo** and the
Priority you scored. The project, field and option IDs live there and only
there: an option ID is regenerated whenever its field's option list is edited,
so a second copy here would go stale silently and break issue creation even
after `board-ops` was fixed.
For a **v1** issue, the same recipe against board #11 — no Priority, that board
has no such field.
The priority rubric is in `/issue-triage`.
## Note on issues that arrive from elsewhere
An issue opened through the GitHub UI — by an outside reporter *or* by a
maintainer — arrives with **no milestone and no card**, and that is normal on
arrival rather than a defect to fix the moment it lands: it comes into the
system through `/issue-triage` instead, and starts in **Incoming**, because
nobody has approved it.
Its **labels are partly set already**, which the triage pass has to account for:
blank issues are disabled, so it came through a form in
[`.github/ISSUE_TEMPLATE/`](../../../.github/ISSUE_TEMPLATE) (Copilot).
| Form | Applies | Still missing |
| --- | --- | --- |
| Bug report | `bug` | the version label — the form carries a version-line **dropdown** a maintainer reads at triage, because a `labels:` list is static and GitHub cannot map an answer to a label |
| Feature request | `enhancement`, `v2` | nothing — v1 takes security fixes only and cannot receive a feature, so `v2` is correct by construction |
So a bug report needs its version label applied from the dropdown answer, and a
feature request usually needs no label work at all. ⚠️ GitHub serves the chooser
from the **default branch**, so a form edited on `v2/main` does not take effect
until the next milestone merge.

30
.claude/skills/issue-create/evals/evals.json Normal file
View file

@ -0,0 +1,30 @@
[
{
"prompt": "Create an issue titled 'TUI crashes on an empty catalog' and set it up the way this repo wants.",
"expect": "issue-create"
},
{
"prompt": "Which milestone should a new issue go in, and how do I find the current one?",
"expect": "issue-create"
},
{
"prompt": "Before I file this, how do I check nobody has reported it already?",
"expect": "issue-create"
},
{
"prompt": "I found a bug while working on something else. How do I get it tracked in this repo?",
"expect": "issue-create"
},
{
"prompt": "What do I need to do beyond `gh issue create` for a new issue to be properly tracked here?",
"expect": "issue-create"
},
{
"prompt": "What is the chemical symbol for potassium?",
"expect": null
},
{
"prompt": "Reverse this string: stressed",
"expect": null
}
]

299
.claude/skills/issue-triage/SKILL.md Normal file
View file

@ -0,0 +1,299 @@
---
name: issue-triage
description: Triage unboarded Inspector issues — the two-pass sweep, the priority rubric and its score comment, and the board audit that checks the whole board against its invariants. Use for "triage new issues" or when scoring an issue's Priority.
disable-model-invocation: false
---
# Triaging issues
**An issue needs triage when it has no board card — regardless of who filed
it.** That is the whole test, and it is deliberately about *state*, not
authorship. The milestone is not part of the test; it decides *where the card
lands* (milestoned → **Todo**, unmilestoned → **Incoming**), so an
approved-but-unboarded issue is still swept in rather than skipped (Copilot). An outside reporter has no board access, so their
issue necessarily lands this way — but so does a maintainer's, whenever they
open one by hand instead of through the `/issue-create` flow. Write access makes
the board *reachable*, not automatic. What puts an issue in Todo is somebody
performing the approval, and an issue nobody did that for has not been approved,
whoever's name is on it.
**"Triage new issues" means running pass 1 over every unboarded open issue, then
the board audit.** Pass 2 is a human judgment call and is never done unprompted.
## Pass 1 — sweep them onto the board (no approval implied)
For each open issue with no card:
1. Apply the **version label** (`v2` unless it's a fix for released v1 behavior)
**and the type label** (`bug` / `enhancement` / `documentation` / `chore` /
`question` — see `/issue-create` for the table). An outside reporter cannot
set either, so both are applied here.
2. Add it to the board for that version — `v2` → #28, `v1` → #11.
3. Set Status to **`Incoming`**.
4. Set Priority with the rubric below (v2 only), and **record the score in a
comment**. This is an *assessment*, not an approval — it's how the queue gets
ordered for the maintainer who reviews it next.
5. **Leave the milestone unset.** Nobody has committed to shipping it yet, and an
empty milestone is precisely what marks it as awaiting review.
**The one exception is an unboarded issue that already carries a milestone.**
Someone recorded the approval and only the card is missing, so board it straight
into **Todo**, keeping the milestone. Otherwise triage would silently un-approve
work a maintainer had already scheduled.
### Finding the unboarded ones
Diff the open issues against **both boards**. Diffing against #28 alone is
wrong: a `v1` issue correctly carded on #11 is reported as unboarded and gets
double-boarded (a real defect a past sweep introduced — #1929 reproduced it).
```sh
D=$(mktemp -d)
gh issue list --repo modelcontextprotocol/inspector --state open --limit 1000 \
--json number,milestone > "$D/open.json"
# Union of BOTH boards, filtered to this repo — org boards can hold other repos' issues.
for P in 28 11; do
gh project item-list $P --owner modelcontextprotocol --format json --limit 700 \
| jq '[.items[] | select(.content.type=="Issue"
and .content.repository=="modelcontextprotocol/inspector")
| .content.number]'
done | jq -s 'add' > "$D/boarded.json"
# Prints the destination too: milestoned already → Todo, otherwise → Incoming.
jq -r --slurpfile b "$D/boarded.json" \
'.[] | select(.number as $n | ($b[0]|index($n))|not)
| "#\(.number)\t→ \(if .milestone then "Todo (has milestone \(.milestone.title))" else "Incoming" end)"' \
"$D/open.json"
```
## Pass 2 — approve what should ship
A maintainer reads the Incoming column and, for each issue worth doing,
**assigns a milestone** and moves the card to **Todo** (or **In Progress** if
picking it up now). That is the whole approval gesture.
This pass is **a judgment call reserved for a human** — deciding what ships in
which release is not something to infer from a rubric. Never promote a card out
of Incoming as part of a routine sweep; "triage new issues" stops at the end of
pass 1.
Issues that shouldn't ship stay in Incoming (or get closed). **Incoming is the
review queue, and "milestoned" is the line between reviewed and not** — which is
why the milestone stays off in pass 1, and why the rubric can treat a milestone
as a real signal rather than a formality every issue carries.
## The priority rubric
Score it rather than assert it: rate two axes 1–5, add the signal bonuses, and
read the total off the band table. The point is that two people triaging the
same issue land in the same place, and that the reasoning survives in a form
someone can argue with later.
**Axis 1 — Severity / impact (1–5).** How bad is it when it happens?
| Score | Means |
| --- | --- |
| 1 | Cosmetic — a typo, a misaligned control, a wording nit. |
| 2 | Minor friction with an easy workaround. |
| 3 | A real feature is broken or missing; the workaround is annoying or partial. |
| 4 | A core workflow is unusable, or the Inspector reports something false about the server under test. |
| 5 | Data loss, a security vulnerability, or a release that is broken on arrival for everyone. |
**Axis 2 — Urgency / staleness (1–5).** How time-sensitive or neglected is it?
| Score | Means |
| --- | --- |
| 1 | No time pressure; nothing waits on it. |
| 2 | Wanted eventually. |
| 3 | Wanted this milestone, or has sat >90 days with no activity. |
| 4 | Blocking other work, or tied to a dated external dependency (an SDK release, a spec deadline). |
| 5 | Blocking a release, or actively hurting users on a published version right now. |
**Signal indicators (+1 each — not an axis of their own).** Corroborating
evidence that the two axes may have undercounted:
- Carries a `bug` or security-related label
- Linked to a milestone — i.e. **already approved**. This is the *re-scoring*
case: an issue being scored in a pass-1 triage has no milestone yet by rule, so
it never earns this one. If you find yourself applying it to every issue in a
batch, the milestone is being used as a formality rather than as approval, and
the bonus has become a constant that discriminates nothing.
- High engagement (many comments or reactions)
- Assigned to someone
- A sub-issue of a larger epic
- The reporter set `Fields → Priority` to **Urgent or High** — **+1, flat,
whichever of the two they picked.** It does not map to a band, and `Urgent`
earns exactly what `High` earns.
**Bands.** Axes give 2–10 and there are six bonuses, so the total runs 2–16.
| Total | Priority | Meaning |
| --- | --- | --- |
| 12+ | **Urgent** | Drop what you're doing. |
| 9–11 | **High** | Next up after current work. |
| 6–8 | **Medium** | Scheduled normally. |
| ≤5 | **Low** | Nice to have; may sit. |
Severity alone doesn't reach Urgent: a 5/5 with no corroborating signals totals
10 and lands **High**. That's deliberate — Urgent is reserved for a severe
problem that something *else* also confirms is burning, and a band that
everything qualifies for stops carrying information. Override the band when it's
plainly wrong, but say why in the issue; a rubric nobody may overrule is a rubric
people route around.
### Trust boundary: who can set what
**The boards are private** (`public: false`, both #28 and #11 — verified
2026-08-01). The Status and Priority a maintainer assigns are visible only to
people with project access: a reporter cannot see them, cannot set them, and
will never learn how their issue was scored. Board priority is a maintainers'
working queue, not a published commitment.
The org-level `Fields → Priority` is the opposite. It renders on the public
issue page and is **not** part of maintainer triage, so any value there is
**untrusted** — we didn't put it there, and it carries a preference rather than
an assessment. That asymmetry is why it earns a flat +1 and nothing more:
- **It counts for something.** Someone flagging their own issue is real
information about how much it hurts them.
- **It cannot decide an outcome.** The bonus is capped, identical for `Urgent`
and `High`, and can lift an issue at most one band. Nothing a reporter can type
reaches Urgent by itself: Urgent needs 12, so the issue must already sit at 11
on maintainer-assessed axes — at which point the reporter is not the reason.
- **Never map the value across.** A reporter selecting `Urgent` does not make the
card Urgent. Doing that would hand queue position to anyone with a GitHub
account, and the queue would sort by assertiveness instead of impact.
Don't lean on GitHub's permission gate to enforce this. Whether an outside
reporter can set that field today is an implementation detail that can change
without notice; the rule holds either way, because it rests on *who assessed the
issue* rather than on who was technically able to click.
### Recording the score
The board stores only the *result* — a card reading `High` with no trace of how
it got there — and since the boards are private, that result is also invisible
to the reporter. So **post the arithmetic as a comment on the issue.** Without
it, a later re-scoring has no way to tell a considered judgment from a guess.
```sh
gh issue comment <N> --repo modelcontextprotocol/inspector --body \
'**Triage:** Priority **Medium** (total 7)
- Severity 3 — a real feature is broken; workaround is partial
- Urgency 2 — wanted eventually, nothing blocked on it
- Bonuses: +1 `bug` label, +1 reporter set Fields → Priority to High
Board: #28, Status `Incoming` (awaiting maintainer review — no milestone yet).'
```
Name the bonuses you claimed rather than just summing them — the milestone bonus
in particular should be conspicuously absent on a pass-1 triage, and a comment
that lists it is a visible sign the approval semantics were misapplied.
## The board audit
Sweeping in the unboarded issues is only the most visible defect class. A board
drifts in several other ways that no single-issue rule catches, so **finish a
triage run with the audit below** — every check should print `0`. A non-zero
count means the board contradicts a rule, not that the rule needs revisiting.
| Check | Invariant | Fix |
| --- | --- | --- |
| Double-boarded | An issue has a card on **one** board, the one matching its version label | Delete the wrong-board card |
| Non-Issue items | **Only issues go on a board** — never PRs, never drafts | Delete the item |
| No Status | Every card carries a Status | Set one — `Incoming` if unmilestoned, else by where it actually is |
| `Incoming` **with** a milestone (#28) | Incoming ⇔ no milestone | Approval was never recorded: move to **Todo**, or clear the milestone |
| Past Incoming **without** a milestone (#28) | Everything past Incoming ⇔ milestoned | Claims an approval nobody made: milestone it, or move back to Incoming |
| Wrong board for label | `v1` → #11, `v2` → #28 | Move the card to the right board |
| Not exactly one version label | Every issue carries **exactly one** of `v1`/`v2` | Apply the missing one, or remove the extra |
| Not exactly one type label | Every issue carries **exactly one** of the five types | Classify it, or remove the extra |
| No Priority (#28) | Every board item is prioritized | Score it with the rubric |
| Closed, not shipped, still carded | **Done means the work shipped** | Delete the card |
| Open, but carded `Done` | A card in Done ⇒ its issue is closed | Close the issue, or move the card back |
```sh
D=$(mktemp -d); R=modelcontextprotocol/inspector
# --limit must exceed the repo's TOTAL issue count (884 as of 2026-08-05), not just the open ones —
# the last check below reads closed issues' state reasons.
gh issue list --repo $R --state all --limit 2000 \
--json number,state,stateReason,labels,milestone > "$D/i.json"
for P in 28 11; do gh project item-list $P --owner modelcontextprotocol \
--format json --limit 700 > "$D/b$P.json"; done
jq -nr --slurpfile o "$D/i.json" --slurpfile a "$D/b28.json" --slurpfile b "$D/b11.json" --arg R "$R" '
($o[0] | map({key:(.number|tostring), value:{st:.state, sr:(.stateReason // ""),
lab:[.labels[].name], ms:(.milestone.title // null)}}) | from_entries) as $M
| def own($s): [$s[].items[]
# A DRAFT card has no `.content.repository`, so filtering on equality
# alone drops the very items the "non-Issue" check exists to find.
| select((.content.repository // null) == null or .content.repository==$R)];
def I($n): ($M[($n|tostring)] // null);
def ms($n): (I($n).ms // null);
def lab($n): (I($n).lab // []);
def isopen($n): (I($n).st == "OPEN");
def shipped($n): (I($n).sr == "COMPLETED");
[own($a)[] | select(.content.type=="Issue") | {n:.content.number, s:.status, p:.priority}] as $B28
| [own($b)[] | select(.content.type=="Issue") | {n:.content.number, s:.status}] as $B11
| {
"double-boarded": [$B28[].n | select(. as $n | [$B11[].n]|index($n))],
"non-Issue on a board": [(own($a)[], own($b)[]) | select(.content.type!="Issue") | .content.number],
"no Status": [($B28[], $B11[]) | select(.s==null) | .n],
"Incoming w/ milestone": [$B28[] | select(.s=="Incoming" and ms(.n)!=null) | .n],
"past Incoming, no ms": [$B28[] | select(.s!=null and .s!="Incoming" and .s!="Done"
and isopen(.n) and ms(.n)==null) | .n],
"v1 label on #28": [$B28[] | select(isopen(.n) and (lab(.n)|index("v1"))) | .n],
"v2 label on #11": [$B11[] | select(isopen(.n) and (lab(.n)|index("v2"))) | .n],
"open, not exactly 1 version label":
[$o[0][] | select(.state=="OPEN")
| select(([.labels[].name] | map(select(IN("v1","v2"))) | length) != 1)
| .number],
"open, not exactly 1 type label":
[$o[0][] | select(.state=="OPEN")
| select(([.labels[].name]
| map(select(IN("bug","enhancement","documentation","chore","question")))
| length) != 1)
| .number],
"#28 open, no Priority": [$B28[] | select(.p==null and isopen(.n)) | .n],
"closed unshipped, still carded":
[($B28[], $B11[]) | select(I(.n)!=null and (isopen(.n)|not)
and (shipped(.n)|not)) | .n],
"open, but carded Done": [($B28[], $B11[]) | select(.s=="Done" and isopen(.n)) | .n]
} | to_entries[] | "\(.value|length)\t\(.key)\t\(.value[0:10])"'
```
Two things the queries must account for, both learned the hard way:
- **Filter by repository.** These are **org** projects and can hold cards from
any repo in the org — board #11 currently carries one
`modelcontextprotocol/servers` issue. Without the `.content.repository` filter
it reads as a statusless-card defect, and "fixing" it would mean editing
another repo's tracking.
- **`Done` is checked from both sides.** One direction catches a card parked in
Done for work that never shipped; the other catches an issue still **open**
under a Done card. The second is not hypothetical here — closing keywords do
not fire on `v2/main`, so every v2 issue is closed **by hand** after its PR
merges, and forgetting that step is the normal way this breaks (Copilot). The
other checks cannot see it: they either gate on `isopen` or skip `Done`.
- **The two milestone checks are #28-only.** Every milestone in this repo is a
v2 release bucket, so a `v1` issue has none it could take — running the
Incoming⇔milestone invariant over board #11 would flag every card on it for a
state it cannot reach.
- **Count the labels; don't test for presence.** The invariant is *exactly
one*, so a predicate that only asks "is any version label present" passes an
issue carrying **both** `v1` and `v2` — which belongs to two lines at once
and shows up in both filtered queries. Same for the five type labels (Copilot).
- **Keep drafts inside `own`.** A draft card carries no `.content.repository`,
so a plain equality filter removes it before the "non-Issue" check can see it —
and that check then reports `0` while the invariant it states (no drafts) is
being violated (Copilot). The filter admits an item with no repository and
excludes only cards that name a *different* one.
- **`$M` holds closed issues too** — the lookup is built from
`gh issue list --state all`, which it has to be, because the last check reads
closed issues' state reasons. So `isopen` is not there to cope with a missing
entry; it is there because the *invariants* are about open work. A closed
issue legitimately sits in `Done` with whatever milestone it had, and a check
that did not gate on `isopen` would report every one of them (Copilot).
**Do not "fix" a `Done` card that is missing a milestone.** Cards predating a
rule are not defects to backfill in bulk — the audit exists to stop *new* drift,
and rewriting settled history destroys the record of when a rule started being
enforced.

30
.claude/skills/issue-triage/evals/evals.json Normal file
View file

@ -0,0 +1,30 @@
[
{
"prompt": "Triage the new issues.",
"expect": "issue-triage"
},
{
"prompt": "How do I decide what Priority an incoming issue deserves?",
"expect": "issue-triage"
},
{
"prompt": "Audit the project board for anything that has drifted.",
"expect": "issue-triage"
},
{
"prompt": "How do I find the issues that aren't on either board yet?",
"expect": "issue-triage"
},
{
"prompt": "After scoring an issue against the rubric, what do I do with the score?",
"expect": "issue-triage"
},
{
"prompt": "Who wrote the novel Moby-Dick?",
"expect": null
},
{
"prompt": "Convert 45 degrees Celsius to Fahrenheit.",
"expect": null
}
]

326
.claude/skills/local-dev/SKILL.md Normal file
View file

@ -0,0 +1,326 @@
---
name: local-dev
description: Install, build and run the Inspector locally, and place a dependency correctly. Use when starting the web, cli, tui or launcher client; when asking whether a build or rebuild is needed before a run script works; when an install, an @inspector/core import or a client bundle fails at run time; when adding or bumping a dependency and choosing which package.json declares it; or when a bundler external, a version skew or a git worktree is the suspect.
disable-model-invocation: false
---
# Local development
The **rules** a reviewer cites (TypeScript, Mantine/React, testing, the gate)
stay in [`AGENTS.md`](../../../AGENTS.md). This skill is how to get the thing
running and where a package goes.
## Install
Requires Node `>=22.19.0`.
```sh
npm install # at the REPO ROOT
```
v2 is **not** an npm workspace — each client under `clients/*` keeps its own
`package.json` and `node_modules`. A single root `npm install` is still all you
need: the root `postinstall` (`scripts/install-clients.mjs`) cascades
`npm install` into `clients/web`, `clients/cli`, `clients/tui`, and
`clients/launcher`.
- **Fresh clone:** `npm install` at the root.
- **After a pull that changes a client's dependencies:** re-run `npm install` at
the root to re-sync every client.
The cascade is dev-only — it exits early when the package is installed under
`node_modules`, and a consumer never needs it because the tarball ships
**prebuilt** artifacts (each client's `build/`, plus `clients/web/dist` and
`clients/web/static`) rather than sources to compile. Set
`INSPECTOR_SKIP_CLIENT_INSTALL=1` to skip it. The `files` allowlist itself is
[`docs/publishing.md`](../../../docs/publishing.md).
⚠️ **In a git worktree, do a real `npm install`.** A symlinked `node_modules`
passes lint, tests and coverage, then fails every Storybook story file on Vite's
`fs.allow`.
## Running
For day-to-day **web** iteration, run Vite directly — fast HMR, no launcher
build needed:
```sh
cd clients/web && npm run dev
```
The launcher-driven scripts run the **built** launcher, so `npm run build`
first:
```sh
npm run build # web → cli → tui → launcher
npm run web # prod web launcher against clients/web/dist
npm run web:dev # web launcher in --dev mode (Vite)
```
Individual builds: `build:web`, `build:cli`, `build:tui`, `build:launcher`. The
web build produces both the browser SPA (`clients/web/dist`, Vite) and the Node
prod-server runner (`clients/web/build`, tsup).
To run the CLI or TUI: `node clients/launcher/build/index.js --cli …` /
`--tui …`.
## The `@inspector/core` alias
`core/` holds the logic shared by all three clients and intentionally has **no
`package.json`** — it is not published on its own. Each client bundles it via a
build-time alias:
- **CLI / TUI** — `esbuildOptions.alias` in `tsup.config.ts` maps
`@inspector/core` → the repo `core/` directory, with
`noExternal: [/^@inspector\/core/]` inlining it.
- **Web** — **two** configs, because the client is two builds:
`clients/web/vite.config.ts` for the browser app, and
`clients/web/tsup.runner.config.ts` for the Node backend runner, which
defines the alias itself. Dependency or debugging work on the runner belongs
in the tsup config, not the Vite one (Copilot).
## Where a dependency goes
**The rules are in [`AGENTS.md`](../../../AGENTS.md) → Dependency placement, and
they are not restated here.** Read them there and come back for the *why* — what
each rule is defending against, what it looked like when it was violated, and how
to tell you have hit one.
Work the rules in order: which manifest declares it, then `dependencies` vs
`devDependencies`, then whether the bundlers must also externalise it.
### Why `core/`'s runtime dependencies are root-only
A per-client declaration installs a **second copy** that drifts from the root's.
Not theoretical: it put two versions of `ext-apps` (1.7.4 / 1.7.5) and of the
transitive v1 `@modelcontextprotocol/sdk` (1.29.0 / 1.30.0) in the tree at once
(#1970). A second copy of `client`/`core` is exactly the failure the `dedupe` +
`server.deps.inline` workaround in `vitest.shared.mts` exists for.
The same reasoning extends to anything reached only through root-owned code that
has no manifest of its own (`test-servers/src`, `core/`). #2195 made that the
general case: `ajv`, `atomically`, `chokidar`, `hono`, `@napi-rs/keyring`,
`pino`, `react` and `zod` moved to the root, joining `proper-lockfile` and
`undici`, which were already there.
Keep two distinctions straight, because AGENTS.md's rules split on them:
- **Root-declared is not the same as `core/`-imported.** `commander`, `open` and
`@hono/node-server` are root `dependencies` too, but they are reached only
from client code. Only the `core/` set has to appear in *all three* bundler
`external` lists.
- **Root-declared is not the same as aliased.** The `vitest.shared.mts` pins and
the `clients/web/tsconfig.*.json` `paths` cover the packages whose resolution
is genuinely ambiguous, which is two different situations: the importer is
`core/`, which has no manifest to resolve from, **or** a copy still sits in
the client install and would win. The second case is the one that is easy to
get wrong — `chokidar` (under `vite`), `zod` (under
`eslint-plugin-react-hooks`), `open` (under Storybook) and `react` (a peer of
`react-dom` and `ink`) are all present in a client's `node_modules` without
being declared there, so "we deleted the declaration" is not the same as "it
resolves from the root now". **Check the install, don't reason from the
manifest.** `ajv`, `commander` and `undici` carry no pin because no client
install has a copy of them at all.
The point of deleting the client-side copies rather than merely keeping them in
step is that **a package installs only into an install root that declares it**.
Aligned duplicate declarations still drift the next time someone bumps one of
them; no declaration at all cannot. `npm run verify:dep-lockstep` is the detector
for the skew, and consolidation is what removes the opportunity.
Two consequences that read as bugs and are not:
- **`clients/cli` and `clients/launcher` declare no runtime dependencies.** Their
manifests carry `devDependencies` only. Everything they import at runtime is
root-declared and resolves by walking up from the client directory.
- **A client's `node_modules` still contains some of these names.** They arrive
transitively (`chokidar` under `vite`, `react` as a peer of `react-dom` and
`ink`). That is why the `vitest.shared.mts` pins matter: an unpinned specifier
would resolve the nearest copy, which is the transitive one, not the root's.
When a package moves to the root, its `vitest.shared.mts` pin has to move with
it — a `path.resolve(dirname, …)` pin left behind points at a directory that no
longer exists, or (for the transitive cases above) at the duplicate the pin was
supposed to rule out.
**`react` and `react-dom` are the exception, and stay pinned per client.** They
are a matched pair — `react-dom` reaches into React internals — and `react-dom`
is still web-declared, so npm resolves it and its React peer together inside
`clients/web/node_modules` (19.2.8 today, against the root's 19.2.7). Pointing
`react` at the root while `react-dom` resolves from the client would pair a
renderer with a React it was not installed against, which is the same split the
pin exists to prevent, arrived at from the other side. `dedupe` still collapses
each install to one copy, which is what actually has to hold.
### Why the shared toolchain is root-only
Every client's `validate` shells out to `prettier`, `eslint`, `tsc` and `vitest`,
which makes them look like they belong beside the scripts that call them. They
do not need to be: `npm run` prepends **every ancestor** `node_modules/.bin` to
`PATH`, and Node and TypeScript walk parent `node_modules` /
`node_modules/@types` the same way — so a client that declares no toolchain at
all still resolves the root's copy. `clients/launcher` declares no
`devDependencies` whatsoever and its `validate` is unchanged.
What a per-client declaration *does* buy is a second copy free to drift, and it
had (#2196): `globals` sat at `^17.7.0` at the root against `^17.4.0` in all four
clients, and `typescript-eslint` at `^8.65.0` against `^8.56.1`. Nothing failed —
which is the point. A lint or format tool that differs per client makes the gate's
verdict a function of *where you ran it*, and the exact `prettier` pin (#1790)
only means something when there is one of it.
⚠️ The line is **used by every client**, not "used by one" and not "is it
toolchain". `tsx`, `playwright`, `storybook`, `happy-dom`, `ink-testing-library`,
`vite-node` and each client's own `@types/*` are toolchain too and stay where
they are — hoisting them would make every client install the union of all four.
So do the ones **more than one** client declares without all of them doing so:
`tsup` sits in web, cli and tui, and `vite` in web and tui on top of the root
*runtime* `dependency` that `--web --dev` needs. Neither is in scope here;
whether to consolidate them is a separate call with a separate rationale (`vite`
especially, since its root declaration is a `dependency`, not a
`devDependency`).
#### What the walk-up does *not* buy you
⚠️ **Deleting a client's declaration does not always delete the copy** — and
where a copy survives, it is the one that wins. Two mechanisms put one back,
neither of which the manifest mentions:
- **An unmet peer.** npm auto-installs a peer into the install that needs it, and
a client install has no visibility into the root's tree, so the root's copy
cannot satisfy it. Web's `eslint-plugin-react-refresh` / `eslint-plugin-storybook`
and the TUI's `eslint-plugin-react-hooks` each pull a client-local `eslint`;
web's Storybook/Vitest stack pulls a local `typescript` and `vitest`.
- **A hoisted transitive.** `@types/express` brings `@types/node` into web and
cli's trees on its own.
Those copies sit *nearer* than the root's, so `clients/web/node_modules/.bin`
precedes the root bin directory on `PATH` and TypeScript resolves the nearest
`node_modules/@types`. Verify with `npm exec -- which eslint` from the client
rather than assuming — the assumption is what made the first cut of #2196 claim
more than it delivered (Copilot).
So the consolidation buys **one declaration and one place to bump**, not one copy
on disk. The two mechanisms are **not** equally safe, and neither is a guarantee:
- A **peer** copy is at least constrained by its holder's peer range. That is a
real pin only when the range is exact — `@vitest/browser-playwright` pins
`vitest` to a single version, which is why the trio below is pinned too. For a
wide range (`eslint-plugin-react-refresh` accepts `^9 || ^10`) the copies agree
only because npm happens to resolve the same latest in both installs, which is
a coincidence that holds until it doesn't.
- A **transitive** copy is constrained by nothing of ours whatsoever, and one has
already diverged: cli's `@types/node` is `24.13.1` against the root's
`24.13.3`, and was `24.13.1` on `v2/main` too — a declared `^24.12.4` loses to
a nearer transitive.
⚠️ **Nothing gates either of those, and `verify:dep-lockstep` is not it.**
That guard derives its candidate set from what each `tsc` **program** resolves
(see below), so it sees only packages a program loads from two installs. A tool
*binary* — `eslint`, `prettier`, `vitest` — never enters a program, so it is
outside the candidate set no matter how far it drifts, and the cli `@types/node`
skew above passes for a second reason on top of that: no one program sees both
copies. When you change what a client declares, check by hand from that client:
```sh
cd clients/web && npm exec -- which eslint prettier tsc vitest
```
#### Why the vitest trio is pinned exactly
`@vitest/browser-playwright` declares an **exact** peer on `vitest` (`"vitest":
"4.1.10"`, not a range), so that package — not the root's range — decides which
`vitest` lands in `clients/web`. Left floating, the root resolves the newest
patch while web's peer stays pinned to the older one, and web's tests then run on
one `vitest` while loading a `@vitest/coverage-v8` provider built against
another. Both would still pass, which is the bad part.
So `vitest` and `@vitest/coverage-v8` at the root and `@vitest/browser-playwright`
in `clients/web` are all pinned **exactly**, and a bump edits all three in one
change — the same discipline as the exact `prettier` pin (#1790).
### Why runtime consumption decides `dependencies`
The client builds externalise npm packages, so a published install resolves them
from the **root** manifest — and devDependencies are not installed for
consumers. A runtime import parked there therefore **breaks the published
package while passing every local check**, which is the worst shape a mistake
can take here.
Two live examples worth knowing:
- `express` is test-only, so it is a devDependency.
- `vite` and `@vitejs/plugin-react` look like build tooling but are root
`dependencies`, because `clients/web/server/start-vite-dev-server.ts` imports
them at runtime for `--web --dev`. Moving them would break that for consumers
while every local check stayed green.
### Why a root dependency must also be externalised
tsup and Vite externalise what the **client's** `package.json` declares, and a
root-only dependency is in none of them — so it is **bundled**, silently. For a
CJS package inlined into an ESM bundle that is fatal: esbuild leaves a
`Dynamic require of "path" is not supported` shim that throws at *import* time,
so the binary dies before it parses a flag (`proper-lockfile`, #2082).
`undici` (#2067) is the worse variant, because it is `import()`ed lazily: the
binary boots fine and only the proxied path dies — with esbuild's rewritten
specifier (`import("./undici-HXPKCIY3.js")`) meaning **no user-side install can
ever satisfy it**. It was declared in `clients/cli/package.json`, so the CLI
looked correct while web and TUI silently inlined 1.05 MB of it.
**How to tell.** The ordinary tiers cannot: unit and integration tests run
against source, where the real package is on the resolution path, and the smokes
never take the lazy path. `npm run verify:bundle-externals` reads the **built
output** for exactly this reason — the config and the output disagreed for four
releases.
⚠️ **Probing by hand with `node -e` falsely succeeds.** `node -e` exposes a
global `require` that satisfies esbuild's `typeof require !== "undefined"`
guard, so the chunk looks loadable when it is not. Reproduce from a real `.mjs`
file.
### Why React-rendering packages are the exception
An externalised package resolves its own `react` from wherever npm placed
**it** — beside a React satisfying *that package's* peer range, which is looser
than ours in every case here. `ink-form` and `ink-scroll-view` declare `">=18"`,
so a consumer's React 18 satisfies them and hoists them while our React 19 nests
underneath: the bundle renders through one React, those packages call hooks on
another, and the TUI crashes on the first hook (#1952).
`ink` is exempt on **cost** (~1.4 MB of `react-reconciler` + `yoga-layout`, plus
a `createRequire` banner). ⚠️ **Never justify that exemption by a peer range** —
it briefly read "its `">=19"` peer keeps npm honest", which is false: a consumer
pinning React 19.0 satisfies `">=19"` while a narrower range of ours nests
underneath. What makes it safe is the *root `react` range staying open to the
whole major*, so npm can dedupe. `clients/tui/__tests__/tsupConfig.test.ts`
enforces the whole split, the exemption included.
### Why a version skew is worth aligning rather than working around
Because v2 is not a workspace, the same package can appear **twice in one `tsc`
program** — a client's own install plus the root's, reached through `core/` or
`test-servers/src`. At the same version that is harmless; on a skew TypeScript
must relate two structurally-distinct declarations, which for a
recursive-generic surface is exponential. A zod `4.3.6` / `4.4.3` skew exhausted
the 4 GB tsc heap outright with `TS2589` (#1896).
⚠️ **Raising the heap hides the class rather than fixing it.** Align the
versions; `npm run verify:dep-lockstep` is the guard, and it derives its
candidate set from what actually enters each `tsc` program, so a package whose
declarations arrive only through another package's `.d.ts` is still seen.
### Why `overrides` beats `npm audit fix`
`tsup@8.5.1` declares `esbuild: ^0.27.0`, and the advisory covers
`0.27.3 - 0.28.0` with `0.27.7` the last 0.27.x — so there is no *upward* escape
inside that range, and `npm audit fix` "resolves" it by silently **downgrading**
to `0.27.2` across three installs (~700 lines of lockfile churn for a low-severity
dev-only advisory; tried and reverted in #2058). The override forces one deduped
copy above the range instead.
Two costs: it puts `tsup` on an esbuild **major it does not declare**, so
`npm run build` — not `npm audit` — is the real gate on such a pin; and it is
invisible to the audit once clean, so **drop it when `tsup` widens its range**
rather than carrying it forever.

30
.claude/skills/local-dev/evals/evals.json Normal file
View file

@ -0,0 +1,30 @@
[
{
"prompt": "What is the difference between the launcher-driven run scripts and starting a client directly, and when should I use each?",
"expect": "local-dev"
},
{
"prompt": "I added a package and now one of the client bundles fails at run time. What do I check?",
"expect": "local-dev"
},
{
"prompt": "Do I need to rebuild anything before I can run `npm run web` in this repo?",
"expect": "local-dev"
},
{
"prompt": "How do I run the CLI from the built launcher in this repo instead of via npx?",
"expect": "local-dev"
},
{
"prompt": "Do I need to do anything special to set this repo up inside a git worktree?",
"expect": "local-dev"
},
{
"prompt": "Write a haiku about autumn.",
"expect": null
},
{
"prompt": "Explain the difference between let and const in JavaScript.",
"expect": null
}
]

195
.claude/skills/pr-flow/SKILL.md Normal file
View file

@ -0,0 +1,195 @@
---
name: pr-flow
description: Take an issue through to a merged PR in this repo, and what to do at each step. Use when asked to open, create or submit a PR; when a DCO or signoff check fails; when requesting a Copilot review or responding to review comments; when naming a branch; when attaching screenshots to a PR; or when closing out after a merge.
disable-model-invocation: false
---
# PR flow
Rules this procedure enforces live in [`AGENTS.md`](../../../AGENTS.md)
(Issue-driven Work Style, Contributing). Board mechanics are in `/board-ops`;
the gate itself is `/pre-push-gate`.
**Pull requests against this repo are opened by the repo maintainers only.**
Having write access is not authorization to open one — anyone else files a
detailed issue and a maintainer takes it from there.
## 1. Start from an issue
**Every PR references an issue. No exceptions, regardless of who opens it.** A
PR with no linked issue has no board card, so the work is invisible to the
project board and untracked. If there's no issue yet, create one first with
`/issue-create` — don't open the PR and backfill.
Move the issue's card to **In Progress** (`/board-ops`).
## 2. Branch
**Branch names start with the target version segment** — the first path segment
is the version whose base branch the PR targets, then the type, then a slug:
```
v2/fix/2071-oauth-resource-metadata
v2/chore/2146-rename-local-gate
v1/fix/proxy-ssrf-pin
```
Not `fix/oauth-resource-metadata`. This keeps the two lines legible in
`git branch -a` and in the PR list once v1 and v2 branches coexist on the same
remote, and it matches the base branches themselves (`v2/main`, `v1/main`).
**Cut the branch from the base it will target** — `v2/main` for v2 work,
`v1/main` for v1. The two lines have unrelated histories, so a `v1/fix/…`
branch cut from `v2/main` arrives at `v1/main` carrying the whole v2 tree
(Copilot). And never cut from a milestone-merge branch — it carries release-only
commits that will show up in your PR's diff.
Working in a git worktree is fine and often preferable. ⚠️ **A worktree needs a
real `npm install`, not a symlinked `node_modules`** — a symlink passes
lint/test/coverage and then fails all Storybook story files on Vite's
`fs.allow`.
For order-dependent PRs on one issue, **stack** them: each branch is based on the
previous one, not all cut from `v2/main`.
## 3. Sign off every commit
**The DCO check is a hard merge gate.** The [probot DCO
app](https://probot.github.io/apps/dco/) fails the PR unless each commit carries
a `Signed-off-by: Name <email>` trailer whose name **and** email match either the
commit's author or its committer. Its only exemptions are merge commits and
bot-authored commits; there is no partial credit — one unsigned commit out of six
fails the whole check.
**Prevent it with `git commit -s`.** Two things that look like automation and are
not:
- ⚠️ **`git config format.signOff true` does nothing here.** Despite the name it
only defaults the `-s` flag for `git format-patch`; `git commit` never reads it,
and there is no `commit.signoff` equivalent.
- ⚠️ **A `prepare-commit-msg` hook works, but think before installing one.** The
trailer is a certification, and a hook makes it on your behalf for *every*
commit, including work you merely cherry-picked. Inside that hook,
`git var GIT_AUTHOR_IDENT` returns your config identity rather than the
preserved author, so it cannot even tell it is signing for someone else.
**Repairing already-pushed commits** means rewriting them:
```sh
git rebase HEAD~<n> --signoff
git push --force-with-lease
```
Use `--force-with-lease` rather than `--force`, and only rewrite when you are the
sole author and nobody else has based work on the branch. The two apparent
alternatives are not alternatives: the app's empty "remediation commit" flow
requires `allowRemediationCommits.individual` and this repo ships no
`.github/dco.yml`, so it runs disabled; and the override button anyone with write
access sees only silences the check without anyone certifying anything.
The signoff is a [Developer Certificate of
Origin](https://developercertificate.org/) assertion made in **your own name**. It
does not claim you wrote the code, so signing off a cherry-pick is legitimate.
What is never acceptable is fabricating *someone else's* certification.
## 4. Run the gate
`npm run format`, then `npm run local:gate`. See `/pre-push-gate` — `npm run
validate` is **not** a substitute.
## 5. Screenshots, for any UI change
Any change to the web UI or the TUI must show its result: capture before/after
screenshots (or a short GIF for an interaction) into a **`pr-screenshots/`
folder off the repo root**, creating it if it doesn't exist. That folder is
**gitignored** — the images are working artifacts staged for upload, never
committed — so attach them to the PR body from there rather than referencing an
in-repo path. Name them for what they show (`tools-tab-before.png`), not
`Screenshot 2026-07-31 at 14.02.11.png`.
To host them, upload to GitHub's attachment endpoint with your `gh` token. Two
mechanics, both of which bite:
- The parameters go in the **query string**, with the raw bytes as the body. A
JSON body fails with a misleading "Invalid name for request".
- ⚠️ **Do not put the token in argv.** `-H "Authorization: token $(gh auth
token)"` puts your credential in curl's command line, where any local user or
process can read it off the process table while the upload runs (Copilot).
Feed it through `--config -` instead: curl reads its options from stdin, so
the token never becomes an argument.
```sh
printf 'header = "Authorization: token %s"\n' "$(gh auth token)" | curl -sS --config - \
-X POST --data-binary @pr-screenshots/tools-tab-after.png \
"https://uploads.github.com/user-attachments/assets?repository_id=<REPO_ID>&name=tools-tab-after.png&content_type=image/png"
```
(The token is still in the shell's environment and in `printf`'s *stdin*, which
is not world-readable the way `/proc/<pid>/cmdline` is.)
## 6. Open the PR
Target the base branch matching the work: **`v2/main`** for v2, **`v1/main`**
for v1. Never `main`.
The body's **first line is `Closes #<ISSUE_NUMBER>`**.
```sh
gh pr create --repo modelcontextprotocol/inspector \
--base v2/main --label v2 \
--title "<title>" --body "Closes #<N>
<what changed and why>"
```
⚠️ Closing keywords only auto-link and auto-close for PRs targeting the repo's
**default branch** (`main`). Because v2 PRs target `v2/main`, `Closes #N` there
is only a cross-reference — it will **not** create a hard link or close the issue
on merge. Keep it anyway, so the issues close if/when `v2/main` reaches `main`.
There is no `gh` flag for manual linking; closing keywords are the only
mechanism GitHub exposes.
Move the card to **In Review**.
## 7. Request a Copilot review
Only the GraphQL `requestReviews` mutation with the Copilot **bot id** works —
REST, `gh pr edit --add-reviewer`, `userIds`, and `copilot-swe-agent` all fail or
silently drop.
```sh
PR_ID=$(gh pr view <N> --repo modelcontextprotocol/inspector --json id --jq .id)
gh api graphql -f query='
mutation($pr:ID!,$bot:[ID!]!) {
requestReviews(input:{pullRequestId:$pr, botIds:$bot, union:true}) {
pullRequest { id }
}
}' -f pr="$PR_ID" -f bot='BOT_kgDOCnlnWA'
```
Poll for the review with a `startswith` match — the review login carries a
`[bot]` suffix.
## 8. Respond to the review
- It is **not** necessary to implement every suggestion. Implementing one a
different way, or declining it with a reason, is fine.
- After making the changes, **respond to each comment** with what was done, or
why it was ignored.
- ⚠️ **Inline replies go hidden once the fix is pushed** (the threads become
outdated), so **mirror each round at PR level** as a summary comment, and always
read the "Suppressed comments" block.
- ⚠️ **Copilot's inline comments lag its review body.** The body's "generated N
comments" count lands first; fetch by recency and reconcile. Repeated
re-review silence means the session ended.
## 9. Merge and close out
**On merge of a v2 PR, manually close its issue and move the board item to
Done**, since auto-close won't fire on `v2/main`. Use the move-a-card recipe in
`/board-ops` — the option IDs are unstable, so this file names the column and
nothing else.
`Done` is only for work that **shipped** — a merged PR, or a parent whose last
sub-issue closed. Anything else (duplicate, won't fix, not planned, obsolete)
means nothing shipped, so **delete the card** instead; see `/board-ops`.

30
.claude/skills/pr-flow/evals/evals.json Normal file
View file

@ -0,0 +1,30 @@
[
{
"prompt": "I've finished the fix for issue 2071. Take it through to a pull request.",
"expect": "pr-flow"
},
{
"prompt": "How do I get Copilot to review a PR in this repo?",
"expect": "pr-flow"
},
{
"prompt": "The DCO check is red on my branch. How do I fix the commits I already pushed?",
"expect": "pr-flow"
},
{
"prompt": "Copilot left a pile of review comments on my PR. How do I answer them so nothing gets lost?",
"expect": "pr-flow"
},
{
"prompt": "My PR is open and green. Walk me through getting it merged and closed out here.",
"expect": "pr-flow"
},
{
"prompt": "What does this regex match? /^[a-z]+$/",
"expect": null
},
{
"prompt": "Translate 'good morning' into Spanish.",
"expect": null
}
]

145
.claude/skills/pre-push-gate/SKILL.md Normal file
View file

@ -0,0 +1,145 @@
---
name: pre-push-gate
description: Diagnose a failing stage of this repo's pre-push gate, and run it correctly. Use when npm run local:gate or npm run validate fails; when lint, coverage, a smoke, or Storybook goes red; when a stage behaves differently inside a git worktree; when the whole run times out; or when deciding what to run before pushing.
disable-model-invocation: false
---
# The pre-push gate
**The rule** (stated in [`AGENTS.md`](../../../AGENTS.md)): run `npm run format`
before committing and **`npm run local:gate`** before pushing. `npm run
validate` is the fast inner-loop check and is **not** a substitute — it runs no
coverage gate, no smokes, and no Storybook tests.
The reference for what each stage covers, and the CI-vs-local split, is
[`docs/quality-gate.md`](../../../docs/quality-gate.md). This skill is how to
run it and what to do when it goes red.
## The two commands
```sh
cd <repo root>
npm run format # auto-fix: core/, scripts/, the shared surface, every client
npm run local:gate # several minutes
```
The stages it runs, in order, are listed in
[`docs/quality-gate.md`](../../../docs/quality-gate.md) — deliberately in one
place only. That list drifted apart across three copies while this PR was in
review, which is the argument for not making a fourth. `npm run local:gate`
prints each stage as it starts, so the running command is the other reliable
answer.
It is a **strict superset** of GitHub CI (which additionally runs `npm install`,
and runs `coverage` as a parallel job). So the direction that matters holds:
**passing `local:gate` locally means CI's gates will pass.** The reverse does
not.
⚠️ **There is no `npm run ci`.** The gate was renamed to `local:gate` (#2146)
precisely because `npm ci` is a built-in that clean-installs from the lockfile
and does *not* run this script. `npm run ci` now fails with npm's missing-script
error.
## Verify by exit code, not by grepping output
Prettier failures are `[warn]` lines that match no obvious failure pattern, so a
grep-based check reports success on a red run. Capture the status:
```sh
npm run local:gate; echo "EXIT=$?"
```
⚠️ If you run it as a background task, the harness's "exit code 0" notification
describes the *wrapper*, not the gate — read the `EXIT=` line.
## Diagnosing a failing stage
### `verify:format-coverage` / `format:check`
Something isn't formatted, or a tracked source file is covered by no
`format:check` glob. Run the **root** `npm run format` (it covers `core/`,
`scripts/`, the shared surface, and every client) — not a single client's.
### `verify:typecheck-coverage`
A tracked `.ts`/`.tsx`/`.mts`/`.cts` lands in no tsconfig project. Usually a new
top-level file in a client whose build config roots at `./src`; add it to that
client's `tsconfig.test.json` (or the src config's `include`).
### `verify:skills` / `verify:skills:cli`
A `.claude/skills` manifest does not parse, declares no invocation mode, or a
model-invoked skill is missing its eval cases. `verify:skills:cli` is the
authoritative validator and fetches a pinned CLI over the network if you have
none installed — so it is also the one stage that will fail offline.
### `verify:dep-lockstep`
A dependency reaching one `tsc` program from two installs resolves to two
versions. **Align the versions** — bump it in every install that declares it.
Do not raise the heap with `--max-old-space-size`; that hides the class rather
than fixing it.
### `lint`
**There is no warning tier** — every `lint` script runs `--max-warnings 0`, so a
warning fails exactly as an error does. Fix the finding; don't widen a
`globalIgnores` or drop the rule. If a rule genuinely must be waived on a line,
use its inline disable comment **with a one-line justification**.
### `typecheck` passes locally but CI's `tsc -b` fails
⚠️ The **incremental cache** hides type errors. Re-run with `tsc -b --force`, or
in a fresh worktree.
### `coverage`
The per-file gate is ≥90 on **all four** dimensions (lines, statements,
functions, branches). A genuinely unreachable branch is annotated at the source
with a justified `/* v8 ignore … -- <reason> */`, never waved through by
lowering the gate. See `/testing` for the acceptable reasons.
Also check the file is inside a gated directory — the web coverage `include` is
a whitelist, so a module placed outside it falls out of the gate silently.
### `smoke:web*`
⚠️ **An orphaned prod web server from a previous run fakes a rejection.** It
answers the readiness probe with *its* token, and the deep link comes back
`data-deeplink="rejected"` with no error. Assert the port is free before
blaming the change.
### `local:storybook`
⚠️ In a worktree with a **symlinked `node_modules`** every story file fails on
Vite's `fs.allow`. Do a real `npm install` in the worktree.
### Everything times out at once
⚠️ Two concurrent `npm run local:gate` runs starve each other — ~326 tests time
out at 5s. Run one at a time. (A `pgrep -f "npm run local:gate"` wait loop
matches *itself* and never exits.)
## Local-only steps
Two stages have no GitHub CI counterpart, each deliberately:
- **`smoke:web:firefox`** — the three browser-driven web smokes again under
Firefox. Trialled as a CI job and removed (#2086): across a dozen runs it never
disagreed with Chromium, and `playwright install --with-deps` carries a real
flake surface. Kept in front of a human about to push instead.
- **`smoke:tui`** — needs a real TTY. It *is* invoked in CI via `npm run smoke`
and self-skips there on `process.env.CI`, so it needs no guarding.
A guard (`scripts/lib/workflow-gate.mjs`, run by `npm run test:scripts`) fails
the suite if a workflow invokes a `local:*` script, a non-Chromium engine pass,
or sets `SMOKE_BROWSER` to anything but a literal `chromium`. Don't work around
it — the split is the design.
## Publish-only check
`npm run pack:verify` builds, packs the real tarball, installs it into a clean
throwaway consumer, and drives the installed `mcp-inspector` bin end to end. It
needs **network access**, so it is a local/release check and is in neither
`local:gate` nor CI. Run it when touching packaging (the `files` allowlist,
a bundler `external` list, anything read from disk at runtime).

30
.claude/skills/pre-push-gate/evals/evals.json Normal file
View file

@ -0,0 +1,30 @@
[
{
"prompt": "npm run local:gate just failed at the coverage stage. What do I do?",
"expect": "pre-push-gate"
},
{
"prompt": "The gate's Storybook stage is failing for me. How do I diagnose it?",
"expect": "pre-push-gate"
},
{
"prompt": "Everything in the gate started timing out at once. What causes that?",
"expect": "pre-push-gate"
},
{
"prompt": "How do I check whether a stage of the gate really passed?",
"expect": "pre-push-gate"
},
{
"prompt": "The gate failed at the lint stage. How do I work out what is wrong?",
"expect": "pre-push-gate"
},
{
"prompt": "What is 17 multiplied by 23?",
"expect": null
},
{
"prompt": "Write a limerick about a cat.",
"expect": null
}
]

106
.claude/skills/project-structure/SKILL.md Normal file
View file

@ -0,0 +1,106 @@
---
name: project-structure
description: Where things live in the Inspector repo — which client owns which surface, what is in core/ versus clients/web, and what each top-level directory is for. Use when locating code, deciding where a new module belongs, or answering which package owns or contains a given area of the codebase.
user-invocable: false
disable-model-invocation: false
---
# Project structure
The Inspector is one npm package, `@modelcontextprotocol/inspector`, with three
clients (Web, TUI, CLI) over a shared `core/`. It is **not** an npm workspace —
each client under `clients/*` keeps its own `package.json` and `node_modules`
(see `/local-dev`).
```
inspector/
├── clients/
│ ├── web/ Vite + React + Mantine SPA with a Node backend
│ │ ├── src/ Browser app: components, hooks, lib/, utils/, theme/
│ │ ├── server/ Node-only dev/prod backend wiring (see below)
│ │ └── static/ sandbox_proxy.html — served for the MCP Apps tab
│ ├── cli/ Scriptable CLI (tsup bundle, @inspector/core alias)
│ ├── tui/ Ink + React terminal UI (tsup bundle)
│ └── launcher/ The `mcp-inspector` bin; dispatches to web/cli/tui in-process
├── core/ Shared code, consumed via the `@inspector/core` alias (no package.json)
├── test-servers/ Composable MCP test servers + JSON configs used by tests and by hand
├── scripts/ Root build/verify tooling: install cascade, smokes, the verify:* guards
├── docs/ Task-oriented guides (see docs/README-style index in the root README)
├── specification/ Design/build specifications
└── AGENTS.md The rules contract — read this before changing anything
```
## `core/` — the shared runtime
Its entry point is the **`InspectorClient`** class, which owns the connection to
an MCP server, the request/response lifecycle, and a set of state stores.
| Directory | Owns |
| --- | --- |
| `core/mcp/` | `InspectorClient`, transports, state stores, config import, URI templates, task/subscription/App-elicitation protocol helpers |
| `core/mcp/node/` | Node stdio transport factory; `proxyFetch.ts` (the shared HTTPS_PROXY/NO_PROXY fetch) |
| `core/mcp/remote/` | Browser HTTP/SSE transport + remote logger/fetch, and (under `node/`) the Hono backend it talks to |
| `core/mcp/state/` | The stores `core/react/` hooks read |
| `core/auth/` | OAuth end to end — providers, discovery, storage, endpoint overrides, scopes, revocation, mid-session recovery — split into isomorphic logic plus `browser/`, `node/` and `remote/` backends |
| `core/auth/node/` | Node OAuth storage + loopback callback server, **and** the `SecretStore` backends (keychain / file / memory) and their selection policy |
| `core/client/` | Install-level client config (`client.json`): browser-safe parse plus Node load/save, remote backend, secrets, runner |
| `core/json/` | JSON + parameter/argument conversion; the schema normalizations all three form builders share (nullable unions, root composition) and the tool-schema portability lint |
| `core/react/` | React hooks over the state stores — consumed by both the web and TUI React trees. Every subscription reads its snapshot **during render** via `useSyncExternalStore` (#1955); `useStoreSnapshot.ts` caches the fresh-value-per-read getters |
| `core/node/` | Node-only helpers: version reader, host normalization/detection |
| `core/storage/` | File I/O helpers used by the OAuth persist backends |
| `core/logging/` | Silent pino logger singleton |
`core/` is isomorphic (browser + Node) and has **no `package.json`** — it is not
published on its own. Its tests live in `clients/web/src/test/core/`, and its
browser-consumed runtime is inside the web coverage gate.
## `clients/web/server/` — the Node backend
| File | Role |
| --- | --- |
| `vite-hono-plugin.ts` | Hono middleware on the Vite dev server |
| `server.ts` | Standalone Hono prod server |
| `start-vite-dev-server.ts` | In-process Vite starter for the launcher |
| `web-server-config.ts` | Env parsing, initial-config payload, startup banner |
| `sandbox-controller.ts` | The MCP Apps sandbox HTTP server |
| `app-origin-controller.ts` | The dedicated app origin for `_meta.ui.domain` |
| `inject-auth-token.ts` | Embeds the API token into served `index.html` |
| `resolve-bind-host.ts` | Bind-host policy (defaults to `127.0.0.1`; refuses a wildcard bind without the opt-in) |
| `browser-externalized-builtin-gate.ts` | Fails `vite build` on a browser-externalized Node built-in |
| `ensure-web-build.ts` | Builds `clients/web/dist` on demand for prod `--web` |
Each of these files carries a header comment explaining the *why*; read the
source rather than looking for a second copy of it here.
## Web source layout: `src/lib` vs `src/utils`
**`utils` = functions that compute; `lib` = things that instantiate, adapt, or
touch the environment.** If it does I/O or wraps a subsystem it's `lib`; if it's
a pure transform it's `utils`. Cross-directory imports point **one way,
`lib → utils`**.
The full rule, its carve-outs (domain types, diagnostic logging, type-only core
imports), and the coverage-whitelist hazard are in
[`AGENTS.md`](../../../AGENTS.md).
## Web components
Presentational ("dumb") components: they take data and callbacks as props and
contain only display logic. State comes from the `@inspector/core` hooks, wired
in near the top of the tree. Element components live in
`clients/web/src/components/elements/`, theme variants in
`clients/web/src/theme/`, each Mantine component's variants in its own file.
## Where to put a new file
| It is… | It goes in |
| --- | --- |
| Logic two or more clients need | `core/<area>/` |
| Browser-only React or DOM code | `clients/web/src/` |
| A pure transform used by web | `clients/web/src/utils/` |
| A stateful adapter / subsystem wrapper used by web | `clients/web/src/lib/` |
| Node-only web backend wiring | `clients/web/server/` |
| A build/verify script | `scripts/` (with a sibling `*.test.mjs` if it has pure logic) |
| A test fixture MCP server | `test-servers/src/` + a config in `test-servers/configs/` |
Test placement is a separate question with its own rules — see `/testing`.

30
.claude/skills/project-structure/evals/evals.json Normal file
View file

@ -0,0 +1,30 @@
[
{
"prompt": "Something in the web client's Node backend needs changing but I do not know which piece handles what. How do I work that out?",
"expect": "project-structure"
},
{
"prompt": "I am adding a module that both the browser side and the Node side need to run. How do I decide where it goes in this repo?",
"expect": "project-structure"
},
{
"prompt": "Does browser-only transport code belong in core or in the web client?",
"expect": "project-structure"
},
{
"prompt": "I have a new file and I'm not sure where it goes. How do I decide in this repo?",
"expect": "project-structure"
},
{
"prompt": "I need to find where a feature is implemented but I do not know which client owns it. How do I work that out in this repo?",
"expect": "project-structure"
},
{
"prompt": "Convert this array of strings to uppercase.",
"expect": null
},
{
"prompt": "What is the capital of France?",
"expect": null
}
]

121
.claude/skills/release/SKILL.md Normal file
View file

@ -0,0 +1,121 @@
---
name: release
description: Cut an Inspector v2 release — bump the version on v2/main first, merge the milestone into main, tag origin/main with a bare x.y.z, and publish via the GitHub Release. Also covers the v1 line and what the publish jobs gate on.
disable-model-invocation: true
---
# Cutting a release
Background on what ships and why (the `files` allowlist, the bundling rules, the
container image) is in [`docs/publishing.md`](../../../docs/publishing.md). This
skill is the procedure.
A v2 release is cut from **`main`**, after the milestone's work has been merged
there from `v2/main` — not from `v2/main` itself. The v1 line releases
independently from `v1/main` to the `v1-latest` tag and never touches `main`.
Publishing is automated by two release-gated jobs in
`.github/workflows/main.yml` (`github.event_name == 'release'`), both
`needs: [build, coverage]` — so a release cannot publish with either the build
job or the coverage gate red:
- **`publish`** — runs `npm run pack:verify` as the pre-publish gate, asserts the
release tag matches the root `package.json` version, then `npm publish
--access public --provenance`.
- **`publish-github-container-registry`** — the GHCR image.
There is **one version number** (only the root `package.json` has one — the
clients carry none), so the flow is three steps.
## 1. Bump on `v2/main`, before the milestone merge
The bump is part of the milestone's work, so it belongs on the develop branch
and flows into `main` with everything else.
```sh
# Branch from the REMOTE ref, and read the version only once you are on it.
# A default clone has just `main` checked out, so a local `v2/main` may not
# exist and `package.json` here is `main`'s — the released version, not the one
# you are bumping from (Copilot).
git fetch origin v2/main
git checkout -b v2/chore/<ISSUE>-bump-<X-Y-Z> origin/v2/main
node -p "require('./package.json').version" # what is on v2/main now
npm version minor --no-git-tag-version # or major / patch; bump only, no tag
node -p "require('./package.json').version" # confirm, then PR → v2/main
```
The branch name carries the version you are bumping **to**, so it is named after
that second reading. If you want it before branching:
`git show origin/v2/main:package.json | node -p "JSON.parse(require('fs').readFileSync(0)).version"`.
⚠️ **Never copy a version out of this file.** It would be a version that has
already shipped by the time you read it, and following it would cut a release
branch named for the wrong release (Copilot).
⚠️ **`--no-git-tag-version` is load-bearing.** A bare `npm version` also tags,
and the tag would land on a `v2/main` commit — but the release must be cut from
`main`, so the tag has to point at the merge commit there (step 3). Tagging here
creates a tag on a commit that is never released.
## 2. Merge `v2/main` → `main`
Through the usual milestone-merge branch. It now carries the bump, so the
release lands on `main` with the version already correct.
Between steps 1 and 2 the two branches **do** differ, and that is expected, not
drift: `v2/main` reads the version being built while `main` still reads the one
currently released. What this ordering removes is *post-release* drift — once the
milestone merge lands they agree again, and `v2/main` is never left **behind**
`main`. If you see `v2/main` ahead of `main`, a release is in flight; if you see
it behind, something went wrong.
## 3. Tag `origin/main` and draft the Release
Derive the tag from the version that just landed, rather than typing one — a
hard-coded tag is either already taken (so `git tag` aborts) or, worse, wrong:
```sh
git fetch origin main
VERSION=$(git show origin/main:package.json | node -p "JSON.parse(require('fs').readFileSync(0)).version")
echo "$VERSION" # sanity-check before tagging
git tag "$VERSION" origin/main && git push origin "$VERSION"
# then draft & publish a GitHub Release for that tag → triggers `publish`
```
⚠️ **Tag `origin/main`, not your local `HEAD`.** `git checkout main && git pull`
resolves through whatever merge-or-rebase strategy you have configured, so a
divergent local `main` can quietly produce or replay local commits. Tagging
`HEAD` there tags a commit that is not on `origin/main`, and `git push origin
<tag>` pushes only the tag — leaving a release whose commit was never published.
⚠️ **No `v` prefix.** This repo's release tags are bare `x.y.z` — which is why
the command above tags `$VERSION` and not `v$VERSION`. npm's own `tag-version-prefix` defaults to `v` and the repo
sets no `.npmrc`, so a bare `npm version` would have produced a mismatched tag;
tagging by hand is what keeps it right. (The workflow's assert step strips a
leading `v` before comparing, so a `v`-prefixed tag would still publish — it
would just be inconsistent with every previous release.)
The release's target commit selects which workflow runs, so this only publishes
when a release is cut from a commit carrying the v2 workflow.
## Why the bump goes on `v2/main` first (#2010)
It used to happen on the milestone-merge branch, which is cut from `main` — so
the bump existed only *downstream* of `v2/main` and nothing carried it back.
`v2/main` sat at `2.0.0` through both the 2.1.0 and 2.2.0 releases. That is not
cosmetic: a branch cut from a milestone-merge branch silently carries the bump
into an unrelated PR (this happened on #2009, where a container bugfix arrived
with a `2.0.0 → 2.2.0` diff), and anything reading the version in development
reported a version two releases old.
⚠️ **Never close a drift by merging `main` into `v2/main`.** `main` carries the
entire pre-v2 v1 history (~230 commits `v2/main` does not have), so a back-merge
grafts all of it into the develop branch's log permanently in order to deliver a
two-file change. Bumping first means there is nothing to back-merge.
## The v1 line
Flat: `feature branch → v1/main → npm v1-latest`, with no merge into `main` at
any point. The two lines publish independently under separate dist-tags, so a v1
fix does **not** need forward-porting to reach users on
`npx @modelcontextprotocol/inspector@v1-latest`.

98
.claude/skills/test-servers/SKILL.md Normal file
View file

@ -0,0 +1,98 @@
---
name: test-servers
description: Run a composable MCP test server by hand — pick the showcase config for a feature or bug, build it, and connect with the right protocol era. Use when a change, a PR or a smoke test needs a real server to exercise it; when reproducing a reported bug by hand; when choosing which fixture or protocol era to run; when a fixture keeps serving stale code after an edit; or when the config or preset you need does not exist yet.
disable-model-invocation: false
---
# Running a test server
`test-servers/` provides **composable MCP servers** so tests and manual checks
exercise a real server over a real transport instead of mocks. A server is
assembled from **presets** (fixture factories in
`test-servers/src/preset-registry.ts`) and configured declaratively with a JSON
file under `test-servers/configs/`.
The full catalogue of showcase configs — one per feature, each with what to click
and what the broken build did — is
[`docs/test-servers.md`](../../../docs/test-servers.md). This skill is how to
run one.
## Build first
The servers are spawned as real subprocesses, so the build output must exist:
```sh
cd clients/web && npm run test-servers:build # tsc -p test-servers → test-servers/build/
```
Scripts reach this through `scripts/ensure-test-servers.mjs`, which builds
**unconditionally** (once per process per repo root).
⚠️ **Unconditional emit is not a clean.** A **deleted** source file leaves its
stale `.js` behind, existence checks pass against it, and anything still
importing that module silently runs the old code — reported not as staleness but
as a product failure in whatever was being tested. So after deleting or renaming
a source file:
```sh
rm -rf test-servers/build
```
The `.tsbuildinfo` is pinned inside `build/` so that clean actually invalidates
the cache.
## Run one
Two processes: the test server, then the Inspector.
```sh
# 1. The server, from the repo root, with the config you picked:
node test-servers/build/server-composable.js --config test-servers/configs/<name>.json
```
```sh
# 2. The Inspector, in another terminal (needs a built launcher — `npm run build`):
node clients/launcher/build/index.js --web
```
Then add the server in the Inspector using the URL the first process announced.
Two mechanics that bite:
- **The server announces its URL on _stderr_**, not stdout (`console.error` in
`server-composable.ts`). Watching stdout alone looks like a server that never
started.
- **The bound port is not necessarily the config's.** `createTestServerHttp`
resolves through `findAvailablePort()`, which walks upward when the configured
port is taken — so read the announced URL rather than assuming.
## Pick the right protocol era
Each config in [`docs/test-servers.md`](../../../docs/test-servers.md) says
which era to connect with. The default is **legacy**; configs setting
`transport.modern` need **Protocol Era = Modern**. Connecting with the wrong era
usually looks like a missing capability rather than an error.
## Common starting points
| Want to see | Config |
| --- | --- |
| An MCP App in the Apps tab | `mcp-app-http.json` (legacy) |
| An App-rendered elicitation | `app-elicitation-http.json` (legacy) |
| `Mcp-*` headers + the modern error taxonomy | `modern-network-http.json` |
| A tool result's `structuredContent` section | `structured-output-http.json` (legacy) |
| RFC 6570 resource-template expansion | `rfc6570-templates-http.json` |
| OAuth token revocation on clear | `oauth-revocation-http.json` (legacy) |
| Cancelling a call mid-flight | `cancellation-modern-http.json` (modern) |
## Adding a config or preset
- Presets live in `test-servers/src/preset-registry.ts`; configs in
`test-servers/configs/*.json`.
- A new showcase config gets a row in `docs/test-servers.md` saying what to do
and what the broken build did — the "what it looked like broken" half is what
makes the fixture reproducible later.
- ⚠️ **An `outputSchema` override must ride a tool that returns structured
content.** A conforming client validates the result against the advertised
schema, so an override on a preset returning none makes every call fail with
"declares an output schema but returned no structured content".

30
.claude/skills/test-servers/evals/evals.json Normal file
View file

@ -0,0 +1,30 @@
[
{
"prompt": "I want to reproduce the OAuth token revocation behaviour by hand. Which test server do I run?",
"expect": "test-servers"
},
{
"prompt": "How do I start one of the composable MCP test servers with a config file?",
"expect": "test-servers"
},
{
"prompt": "I need to run a real MCP server locally to exercise this change by hand. Which one do I run, and how do I start it?",
"expect": "test-servers"
},
{
"prompt": "Which protocol era should I connect with when I run one of these fixtures by hand?",
"expect": "test-servers"
},
{
"prompt": "I need a fixture combination that doesn't exist yet. How do I add one?",
"expect": "test-servers"
},
{
"prompt": "Sort this list alphabetically: banana, apple, cherry.",
"expect": null
},
{
"prompt": "What is the boiling point of water in Fahrenheit?",
"expect": null
}
]

163
.claude/skills/testing/SKILL.md Normal file
View file

@ -0,0 +1,163 @@
---
name: testing
description: Run, place and fix tests in this repo. Use when choosing which npm command runs a given suite (web unit, web integration, Storybook, cli, tui, launcher, scripts); when deciding where a new test file belongs — beside its source, under src/test/, or in a client's __tests__/; when a per-file coverage check fails or a v8 ignore is in question; when asking which test tier spawns the built binary rather than importing it; or when rendering, mounting or asserting on Mantine components and their transitions in a test.
disable-model-invocation: false
---
# Testing
**Every change needs tests, and every file must clear ≥90% on lines,
statements, functions, and branches.** That rule and the React/Mantine
conventions live in [`AGENTS.md`](../../../AGENTS.md); this skill is where a
test goes, how to run it, and how to clear the gate.
⚠️ **Anything that needs a real server to run against — an integration test, a
smoke, reproducing a bug by hand — is `/test-servers`, and you have to load it.**
Integration and smoke tests here drive a real server over a real transport
rather than a mock, so picking, building and connecting to a fixture is a
procedure of its own that this skill does not carry.
## Where the test file goes
**Side-by-side by default; `src/test/` only for what can't be co-located; and
the Node clients are different.**
### `clients/web` — side-by-side
`<Name>.test.tsx` (or `.test.ts` for non-React modules) **next to the source**.
Components, hooks, `lib/`, `utils/`. This is the overwhelming majority; a
web-owned test living under `src/test/` instead is a bug.
`clients/web/src/test/` is for the three things that *cannot* be co-located:
1. **Tests of the repo-root `core/` package** → `src/test/core/…`, mirroring the
`core/` folder layout. `core/` physically lives outside `clients/web/`, is
consumed via the `@inspector/core` alias, and has no test harness of its own.
This includes `core/json/*` and `core/client/*`.
2. **The `integration` project** → `src/test/integration/…`, mirroring the
`core/` source layout (`mcp/`, `mcp/node/`, `mcp/remote/`, `auth/`,
`auth/node/`, `storage/`). **Placement is the manifest** — any file under that
folder is picked up by the integration project (node env, 30s timeouts) via a
folder glob; there is no enumeration to keep in sync.
3. **Shared test infrastructure** — `renderWithMantine.tsx`, `setup.ts`,
`fixtures/`, `scrollAreaStoryAssertions.ts`.
### `clients/cli`, `clients/tui`, `clients/launcher` — a top-level `__tests__/`
**All** their tests, not beside their source. Their `tsconfig.json` excludes
`**/*.test.*` and their `tsconfig.test.json` includes `__tests__/**/*`, so a
co-located `src/**/*.test.*` lands in **no** tsconfig project and fails
`npm run verify:typecheck-coverage` (#1791).
### Root tooling — `scripts/*.test.mjs`
A new `scripts/*.mjs` helper with pure logic gets a sibling `*.test.mjs`, run by
`npm run test:scripts` (node's built-in runner; the root has no vitest harness by
design). ⚠️ Keep the filename `*.test.mjs` — `node --test` silently **skips** a
file its glob misses and still exits 0.
## Running them
| Scope | From | Command |
| --- | --- | --- |
| Web unit | `clients/web` | `npm run test` (`test:watch` while iterating) |
| Web integration | `clients/web` | `npm run test:integration` |
| Web Storybook play fns | `clients/web` | `npm run test:storybook` |
| CLI | `clients/cli` | `npm run test` (`pretest` builds test-servers + the bin) |
| TUI | `clients/tui` | `npm run test` |
| Launcher | `clients/launcher` | `npm run test` |
| Root tooling | repo root | `npm run test:scripts` |
| Everything, fast | repo root | `npm run validate` |
| The coverage gate | repo root | `npm run coverage` |
There is **no aggregate root `test` script** — each client self-validates.
In unit tests that expect error output, **suppress it from the console**.
## The tiers, shallowest first
unit (`test`, per client) → web integration (`test:integration`, real
transports/servers) → out-of-process (`clients/cli/__tests__/e2e.test.ts`,
spawns the built binary) → smokes through the built launcher (`npm run smoke`) →
Storybook play functions (`test:storybook`) → the published-tarball check
(`npm run pack:verify`, local/release only — needs network).
`validate` runs the per-client `test` scripts — so web **unit** plus cli's
out-of-process `e2e.test.ts`, but **not** web's integration project, which runs
inside the `coverage` gate. CI therefore has no separate `test:integration` step.
## The coverage gate
**Per-file ≥90 on all four dimensions**, CI-enforced, across web, cli, tui and
launcher. New code must clear 90 on every dimension.
Scope notes:
- The **web** coverage `include` (in `clients/web/vite.config.ts`) also covers
the shared `core/` runtime the browser consumes — `core/mcp`, `core/react`,
`core/auth`, `core/storage`, `core/logging`, `core/node`, `core/json`,
`core/client`.
- ⚠️ That `include` is a **whitelist** naming `components`/`hooks`/`theme`/
`lib`/`utils`/`server`. A module placed **outside** those directories falls out
of the gate entirely, silently. The documented exceptions are `src/App.tsx`
(a composition root at ~42% branch coverage — gating it is a dedicated
decomposition effort) and the `src/main.tsx` / `src/index.ts` bootstraps.
- **CLI** tests run **in-process** by importing `runCli()`
(`__tests__/helpers/cli-runner.ts`) so `src` is measured; `src/index.ts` is the
only exclusion. `commander` uses `.exitOverride()` so a parse error throws
instead of tearing down the test worker.
- **TUI** covers **all of `src/**`, React surface included**. Components mount
through `ink-testing-library` with the passthrough doubles in
`__tests__/helpers/`; keypresses are driven through stdin. The only exclusion
is `src/tui-servers.ts` (a pure re-export, excluded so it doesn't surface as a
misleading 0/0 row).
### When a `v8 ignore` is justified
A genuinely-unreachable branch is annotated at the source rather than waved
through by lowering the gate. The acceptable reasons are enumerated in
[`AGENTS.md`](../../../AGENTS.md) — do not reach for one that is not on that
list.
## React tests: `renderWithMantine`
**Always render through `renderWithMantine`** (`src/test/renderWithMantine.tsx`)
— it wraps in `MantineProvider` with the project theme. Do not hand-roll a bare
`MantineProvider`; it skips the project theme and the helper's options and drifts
from every other test.
Note the justification has changed and the old one is wrong: it sets
`env="test"`, which makes Mantine skip the animated **render** — but it does
**not** stop the timers. `env` is read only at `Transition.mjs`'s render branch,
while `useTransition()` runs before that check and still schedules real
`window.setTimeout`s (opening a `<Modal>` schedules three 200ms timers). What
prevents a timer outliving its file — and throwing an uncaught
`ReferenceError: window is not defined` that fails the **whole run**, attributed
to an innocent file (#1760) — is the **leaked-timer safety net in
`src/test/setup.ts`**, which is global and covers every unit test however it
renders. The rule stands on consistency, not on timer safety.
- **Forced color scheme:** pass the option —
`renderWithMantine(ui, { colorScheme: "dark" })` — rather than hand-rolling
`defaultColorScheme="dark"`.
- **Mid-flight transition state** (e.g. asserting a `data-anim="out"` cell during
an exit crossfade) is the only reason to use `renderWithMantineTransitions`.
Pass `settleMs` derived from the component's real animation duration
(`HEADER_ANIM_MS + 200`), do **not** also use `vi.useFakeTimers()` in that test
(the auto-settle no-ops under fake timers), and if the test unmounts the tree
itself use the `unmount()` the helper returns. The mechanism is documented at
length on the helper — read there before changing it.
## Storybook play functions
Every screen and element component has a `*.stories.tsx`; play functions double
as interaction tests, run headless in CI and in the local gate.
⚠️ **`expect(...)` from `storybook/test` returns a promise.** Storybook
instruments it, so every `expect` in a play function is awaited — as is any
shared helper that wraps one.
## Test servers, not mocks
Integration and smoke tests drive a real server over a real transport. See
`/test-servers` for picking and building one.

30
.claude/skills/testing/evals/evals.json Normal file
View file

@ -0,0 +1,30 @@
[
{
"prompt": "Which test tier in this repo spawns the built binary rather than importing it?",
"expect": "testing"
},
{
"prompt": "How do I run just the TUI suite, and is there one command that runs everything?",
"expect": "testing"
},
{
"prompt": "How do I write a test for a TUI component in this repo?",
"expect": "testing"
},
{
"prompt": "How am I supposed to invoke the CLI from a new test in this repo?",
"expect": "testing"
},
{
"prompt": "How do I run the web integration tests, and do they run as part of the normal test command?",
"expect": "testing"
},
{
"prompt": "What does this regex do? /^\\d{3}-\\d{4}$/",
"expect": null
},
{
"prompt": "Rename the variable `foo` to `bar` in this snippet.",
"expect": null
}
]

121
.github/copilot-instructions.md vendored
View file

@ -1,121 +0,0 @@
# Copilot review instructions — MCP Inspector
> **`AGENTS.md` is the source of truth.** This file is a review-focused distillation of it, kept in sync by hand — see "Keep documentation files up to date" in `AGENTS.md`. Where the two disagree, `AGENTS.md` wins, and the drift is a bug worth flagging in review.
The Inspector ships as one package with three clients (**Web**, **CLI**, **TUI**) over a shared `core/`, consumed via the `@inspector/core` build-time alias. v2 is **not** an npm workspace: the root and each `clients/*` keep their own `package.json` and `node_modules`.
## TypeScript
- **Never use `any`.** Not in types, not in casts, not in generics.
- **Never suppress errors to satisfy the linter or compiler** — no disabling `no-unused-vars` / `no-explicit-any` in config, and no `// @ts-nocheck` or `// @ts-ignore` (`@typescript-eslint/ban-ts-comment` rejects these across every surface).
- **Avoid double casts (`as unknown as T`).** They erase all type safety and usually mean the real type is being worked around. Prefer a type guard, a narrower single cast, or fixing the underlying type. If genuinely unavoidable (a documented gap in a third-party type, or bridging structurally-identical shapes TS can't relate), it **must** carry an inline comment justifying why it's safe and why nothing better exists. An unjustified `as unknown as` is not acceptable in review.
- Prefer inference, type guards, and precise annotations over assertions.
- An `_` prefix is the intentionally-unused marker (`argsIgnorePattern` / `varsIgnorePattern` / `caughtErrorsIgnorePattern`).
- **Never float a promise.** `@typescript-eslint/no-floating-promises` is `error` in all five ESLint scopes, so every promise must be awaited, `.catch(…)`-terminated, returned, or explicitly discarded with `void`. Prefer **holding and settling** it — a floated rejection surfaces in an unrelated file as an SDK-internal stack, which is how two un-held `callTool` calls failed the whole `npm run ci` chain (#1947). Reach for `void` only when the callee already owns its failures (it ends in a `catch`) or the caller genuinely cannot await (a sync effect body, an Ink key handler), and say which in a comment. In Storybook play functions, `expect(...)` from `storybook/test` is instrumented and returns a promise — it must be awaited.
## React and UI (web client)
The web client is built from **presentational ("dumb") components** — they take data and callbacks as props and hold display logic only. No data fetching or client state inside them; state comes from the `@inspector/core` hooks wired near the top of the tree. A component that reaches for a store or fetches directly is a review finding.
Styling is **Mantine-first**, in this strict order of preference: component props → theme variants → CSS classes (last resort).
- **Never use inline styles.**
- **Never use raw color literals** — no hex (`#ddd`), no `rgba()`. Use the `--inspector-*` CSS custom properties from `App.css :root` (e.g. `c: 'var(--inspector-text-primary)'`). If no token fits, add one to `:root` first.
- **Avoid `div` and bare HTML for layout.** Use Mantine `Box`, `Group`, `Stack`, `Flex`, `Paper`.
- **Never add a CSS class when the styles can be component props or a theme variant.** Flat CSS properties (margin, padding, background, border, color, font-size) belong in the theme (`src/theme/<Component>.ts`, via `Component.extend()`). `App.css` may contain **only** what the theme cannot express: `@keyframes`, pseudo-selectors (`:hover`, `:focus`), cross-component hover relationships, nested child selectors for third-party HTML output, and styles for native elements (`img`, `iframe`).
- When a theme variant needs a class for nested/pseudo selectors, assign it via `classNames` in the theme extension — never a manual `className` in JSX for theme-styled components.
### The `.withProps()` rule
**Declare a named subcomponent constant via `.withProps()` whenever an inline Mantine element carries two or more _static_ props.** This applies to single-use elements too — "it's only used once" is not an exemption.
- **Static** = a literal configuring **styling, layout, or behavior**: `size="sm"`, `c="dimmed"`, `fw={500}`, `gap="xs"`, `justify="space-between"`, `variant="light"`, `withBorder`, `readOnly`, `striped`.
- **Not counted:** dynamic props (`value`, `on*`, `children`, `key`, `ref`, anything whose value is a variable) — pass these at the call site; and per-instance **content/accessibility** literals (`label`, `description`, `placeholder`, `title`, `aria-label`, `role`) — these never by themselves trigger extraction.
```tsx
const CardContent = Group.withProps({
flex: 1,
align: "flex-start",
justify: "space-between",
wrap: "nowrap",
});
```
**Legitimate exceptions** (each stays inline, with a one-line comment saying why):
- **`Box`** — does not support `.withProps()`. Use `Group`/`Stack`/`Flex`/`Text`/`Paper`/`UnstyledButton`/`Image` instead, chosen by purpose. A `Box` that genuinely needs a non-flex primitive (`component="iframe"`, `display="grid"`) stays inline.
- **`Accordion`** — a compound, `multiple`-discriminated generic; `.withProps()` loses its JSX call signature and fails to type.
- **Headless, non-`factory()` components** such as `Transition` — no Styles API, so no `.withProps` static at all.
- **`data-*` attributes** — not part of a component's typed props, so excess-property-checked out of a `withProps` literal. Pass at the call site.
- **Anything that isn't a Mantine factory component** — a `react-icons` glyph, another library's component, or a first-party plain `export function`.
### State and effects
- **Never reset or re-sync local state from a prop inside a `useEffect`.** `useEffect(() => setX(prop), [prop])` paints the stale value first and renders twice; it is an error under `react-hooks/set-state-in-effect`.
- Use **`useValueChange(value, onChange)`** (`src/hooks/useValueChange.ts`) — React's documented "adjusting state during render" pattern. It does not fire on the first render; seed the state with `useState`. The comparison is `Object.is`, so pass a **referentially stable** value — a primitive key (id/name/URI) or a memoized one, never a fresh object literal.
- The `onChange` runs **during render**, so it must be pure — `setState` and nothing else. No fetches, DOM writes, logging, ref mutation, or parent callbacks; a render can be replayed or abandoned.
- Effects remain correct for real external-system synchronization (DOM measurement, rAF, subscriptions, timers).
### Theme files vs. element components
Both exist and do different jobs. Theme files (`src/theme/<Component>.ts`) customize a Mantine primitive **app-wide**. Element components (`src/components/elements/`) add **domain semantics** on top of primitives.
- Element components import from `@mantine/core`, **not** from `src/theme/` — the theme layer is applied transparently by the provider.
- **Never push domain-specific variant logic into theme files** (annotation types, transport types, …). Domain variants belong to the element component that owns those semantics.
## Where code goes (web client)
**`utils` = functions that compute; `lib` = things that instantiate, adapt, or touch the environment.** If it does I/O or wraps a subsystem it's `lib`; if it's a pure transform it's `utils`.
- `src/utils/` — pure, side-effect-free. Also: pure shared domain types and their constructors; diagnostic `console.warn`/`error` does **not** count as a side effect; type-only imports from `@inspector/core`, and re-exporting pure functions/constants from core, are both fine.
- `src/lib/` — infrastructure, integration, stateful adapters: composes subsystems, wraps the core **runtime**, touches DOM / `window` / `sessionStorage`, or produces side effects.
- Cross-directory imports go **one way: `lib → utils`**, never the reverse.
- `src/types/` is only for ambient `.d.ts` module stubs — not a home for new domain types.
- ⚠️ The coverage `include` is a **whitelist** naming `components` / `hooks` / `theme` / `lib` / `utils` / `server` (plus the `core/*` runtime). A module placed outside those directories silently falls out of the ≥90% gate. Flag new top-level files or new grab-bag directories.
## Dependency placement
- **The MCP SDK packages (`@modelcontextprotocol/client`, `core`, `server`, `server-legacy`, `ext-apps`) belong in the root `package.json` only.** Adding one to a `clients/*/package.json` is a review finding: Node resolution walks up, so the root install already serves every client, and a per-client entry installs a second copy that drifts (it produced two versions of `ext-apps`, and of the transitive v1 SDK, at once — #1970) and reintroduces the duplicate-copy failure `vitest.shared.mts` has a `dedupe` workaround for.
- **The v1 `@modelcontextprotocol/sdk` is not a dependency of this repo** and must not become one. It is a peer of `ext-apps`, present in lock files only.
- Dependencies reached only through **root-owned code with no manifest** (`test-servers/src`, `core/`) are declared at the root and aliased to the **repo root** in `vitest.shared.mts` — as `express` and `yaml` are — not to `<client>/node_modules` like the other pins there.
- **A dependency that renders React components must be bundled into the client that uses it, and is then not a root dependency.** An externalized package resolves its own `react` from wherever npm placed it in the _consumer's_ tree, beside a React satisfying _its_ peer range — looser than ours, which is all it takes to split React. `ink-form`/`ink-scroll-view` declare `">=18"`, so a consumer's React 18 satisfies them, the TUI ends up with two React instances, and it crashes on the first hook (#1952). Both are inlined via `noExternal` in `clients/tui/tsup.config.ts`. **`ink` is the one exemption, justified by cost (~1.4MB) — never by a peer range**: flag any claim that `">=19"` keeps npm from misplacing it, which is false and was in this repo once. What keeps it safe is the **root `react` range staying open to the whole major (`^19.0.0`)** so npm can dedupe with a consumer's pinned React 19; treat narrowing that range as reopening the bug. `clients/tui/__tests__/tsupConfig.test.ts` enforces the split, the root-declaration of exempt packages, and that range.
- **Which section is a separate question from which manifest.** A package `core/` imports at runtime must be in root **`dependencies`**: client builds externalize npm packages, so a published install resolves them from the root manifest and devDependencies are absent there. Only test/build-only packages (`express`) belong in `devDependencies`. Flag a runtime `core/` import added to `devDependencies` — it passes every local check and breaks the published package.
- **A root-declared package that `core/` imports at runtime must also be named in each client's bundler `external` list** — `clients/{cli,tui}/tsup.config.ts` and `clients/web/tsup.runner.config.ts`, all three. Bundlers externalize what the _client's_ manifest declares, and these packages are root-only by rule, so omitting them means they get bundled. For a CJS package inlined into an ESM bundle that is fatal: esbuild's `Dynamic require of "path" is not supported` shim throws at import time and the binary dies before parsing a flag (#2082, `proper-lockfile`). Flag a new root runtime dependency that is not added to all three. This has broken twice — `undici` slipped past #2082 because it _was_ declared in `clients/cli/package.json`, so the CLI looked fine while the web and TUI bundles inlined 1.05MB of it, unloadably (#2067). `npm run verify:bundle-externals` now guards it from the **built output**; flag any change that would weaken or skip that guard.
- **Pin a transitive dependency past its parent's declared range with an `overrides` entry — never with `npm audit fix`.** When the advisory range has no upward escape inside the parent's range, `npm audit fix` resolves it by silently *downgrading* (esbuild 0.27.7 → 0.27.2 across three installs, #2058/#2062). Flag that in a diff. An override puts a bundler on a major it does not declare, so the gate for one is `npm run build`, not `npm audit`; a lockfile diff accompanying it should touch that package's entries only. Flag an override added without a note saying when it can be dropped.
## Tests and the coverage gate
- **All new or modified code needs tests.** The per-file gate is **≥ 90% on all four dimensions** — lines, statements, functions, **and branches** — enforced in CI for `clients/{web,cli,tui,launcher}` and the gated `core/` runtime.
- **Never lower the gate** to accommodate an unreachable branch. Annotate at the source with a justified `/* v8 ignore … -- <reason> */`. Acceptable reasons: happy-dom-inherent paths (Mantine portal mounts, `useMediaQuery` fallbacks, `typeof window` SSR guards), React StrictMode effect-replay blocks, and provably-dead defensive guards. Anything else is a missing test.
- **Suppress expected error output** from the console in tests that exercise error paths.
### Test placement
- **Web:** side-by-side by default — `<Name>.test.tsx` next to the source. A web-owned test under `src/test/` instead of beside its source is a bug. `src/test/` is only for what can't be co-located: tests of the repo-root `core/` package (`src/test/core/…`, mirroring core's layout), the `integration` project (`src/test/integration/…` — placement _is_ the manifest), and shared test infrastructure.
- **CLI / TUI / launcher:** **all** tests live in a top-level `__tests__/` directory. A co-located `src/**/*.test.*` lands in no tsconfig project and fails `verify:typecheck-coverage`.
### Rendering components in tests
- **Always render through `renderWithMantine`** from `src/test/renderWithMantine.tsx`. A hand-rolled bare `MantineProvider` skips the project theme and the helper's options, and drifts from every other test. (It does _not_ reintroduce the timer-leak class — an older version of this rule said so; the leaked-timer net in `setup.ts` is global and covers every unit test regardless of how it renders.)
- For a forced color scheme, pass the option — `renderWithMantine(ui, { colorScheme: "dark" })` — rather than a hand-rolled `defaultColorScheme` provider.
- Only when asserting _mid-flight_ transition state, use `renderWithMantineTransitions`, passing `settleMs` derived from the component's real animation duration. Do **not** combine it with `vi.useFakeTimers()`, and use the `unmount()` it returns if the test unmounts the tree itself.
## Gates and PR hygiene
- `npm run format` before committing; **`npm run ci` before pushing** (`validate` → `coverage` → `verify:build-gate` → `verify:bundle-externals` → `smoke` → Storybook). `npm run validate` is the fast inner-loop check and is **not** a substitute — it runs `test`, not `test:coverage`, so it does zero coverage gating.
- **A dependency bump must land in every install that declares it.** v2 is not a workspace — the root and each `clients/*` have their own `node_modules`, and a client's test project compiles `core/` and `test-servers/src` (which resolve from the **root**) alongside the client's own sources. Bumping a shared dependency in one manifest only puts two versions of it in one `tsc` program; for a recursive-generic surface like zod that exhausts the tsc heap (#1896). `verify:dep-lockstep` fails the build on this — it derives its candidates from what each `tsc` program actually resolves (`tsc --listFilesOnly`, keeping packages that reach one program from two installs), so a package reached only through another package's `.d.ts` counts too (#1965) — so a PR bumping a package the shared sources pull in should update the root **and every client that already lists it** — not every client unconditionally, since a package absent from an install can't skew and adding it there would be a spurious dependency.
- **A test or smoke must not touch real user state.** The web smokes run against a throwaway catalog via the shared `scripts/lib/prod-web-server.mjs` helper, never the developer's `~/.mcp-inspector/mcp.json` (#1977); the cli/tui smokes drive a temp `--catalog`; `pack:verify`'s `--web` child sets its own `MCP_CATALOG_PATH` for the same reason (#2003 — its App deep link persists a server row). Anything that boots the web backend and then *navigates* it needs that isolation, not just the scripts named `smoke:*`. A new smoke spawning its own server, or teardown that removes a work dir without first awaiting `stopChild` (the #1801 race — `child-cleanup.mjs` exports both halves and both are required), should be flagged.
- **Build output is never a gate target.** Lint, format, and typecheck read first-party source only; everything a build writes (`clients/*/build`, `clients/web/dist`, `storybook-static`, `coverage`, `test-servers/build`, `core/**/{build,dist}`, `*.tsbuildinfo`) stays out via each scope's `globalIgnores`, `format` globs, and tsconfig `include`. Gating generated code reports defects in vendored third-party source that nobody can fix, and a rule promotion turns that warning into a `validate` failure (#2043). Flag a PR that adds a build location without ignoring it in the same change, that widens an ignore to silence a finding in first-party code, or that adds a build directory to a tsconfig `include` to make a generated `.d.ts` resolve. Note the coverage guards don't catch this — they assert source is _covered_, not that output is _excluded_.
- **Lint has no warning tier.** Every `lint` script runs `--max-warnings 0`, so a warning fails `validate` exactly as an error does (#2085) — a `warn`-level `react-hooks/exhaustive-deps` finding otherwise let a stale-closure bug pass the pre-push gate and reach review. Flag a PR that silences a finding to make the gate pass (widening a `globalIgnores`, dropping a rule, or an inline disable with no justification comment); the fix is the defect, not the message. A rule meant to be enforced should be set to `error` rather than left at `warn` and carried by the flag.
- **Every PR references an issue**, first body line `Closes #<ISSUE_NUMBER>`.
- **Every PR carries exactly one version label**, `v1` or `v2`, matching its base branch.
- **Commits carry a `Signed-off-by:` trailer.** The DCO check is a hard merge gate and fails on a single unsigned commit; it matches the trailer against the author _or_ committer, and skips only merge and bot commits. Use `git commit -s` — note `format.signOff` does _not_ sign `git commit` (only `format-patch`). Repairing pushed commits means `git rebase HEAD~<n> --signoff` + `git push --force-with-lease`; remediation commits are not enabled on this repo.
- Update the relevant `README.md` / `AGENTS.md` when a change adds, removes, renames, or repurposes a file or folder, changes the structure or tech stack, or introduces a command, dependency, or architectural pattern.
## What to prioritize in review
1. Correctness and security — this backend spawns local processes and proxies outbound requests, so anything touching auth, origin validation, host binding, or the proxy's SSRF controls deserves close reading.
2. Type-safety violations (`any`, suppressions, unjustified double casts).
3. Missing or thin tests against the ≥90% four-dimension gate, and modules placed outside the gated directories.
4. Mantine convention violations — inline styles, raw colors, unnecessary CSS classes, missing `.withProps()` extraction.
5. Docs that contradict the change.

Some files were not shown because too many files have changed in this diff Show more