chore: extract AGENTS.md recipes into agent skills
Closes #2163 `AGENTS.md` was one 1,223-line contract loaded in full on every turn, whether the task was a one-line CSS fix or a release cut. This splits it: rules stay resident, procedures move to `.claude/skills/`. - Ten skills under `.claude/skills/`, seven `disable-model-invocation: true` (invoked by name only, so they cost no listing context and have no trigger question) and three model-invoked with committed eval cases. - `AGENTS.md` 1,223 → 380 lines: rules, a skills index, and a project tree reduced to one line per entry. The removed recipes are not duplicated. - `README.md` 811 → 105 lines, split into `docs/{architecture,quality-gate, test-servers,publishing,docker}.md`, and dropped from `CLAUDE.md`'s auto-load. - `.github/copilot-instructions.md` and its mirroring rule removed — Copilot reads `AGENTS.md`. - `npm run verify:skills` (in `validate`, so in `local:gate` and CI) parses each SKILL.md the way Claude Code does, requires an explicit invocation mode and eval cases for model-invoked skills, and budgets the listing. - `npm run skills:eval` runs the committed trigger cases headless. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall
parent
8c91ae8820
commit
73447c72d1
13 changed files with 3291 additions and 1927 deletions
265
.claude/skills/board-ops/SKILL.md
Normal file
265
.claude/skills/board-ops/SKILL.md
Normal file
|
|
@ -0,0 +1,265 @@
|
|||
---
|
||||
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: true
|
||||
---
|
||||
|
||||
# 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.
|
||||
|
||||
```sh
|
||||
ITEM_ID=$(gh project item-list 28 --owner modelcontextprotocol --format json --limit 500 \
|
||||
--jq '.items[] | select(.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.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.
|
||||
|
||||
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:
|
||||
|
||||
```sh
|
||||
gh project item-list 28 --owner modelcontextprotocol --format json --limit 600 > board-snapshot.json
|
||||
```
|
||||
|
||||
### 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
|
||||
# 1. Which cards lost their value, and what did they hold?
|
||||
gh project item-list 28 --owner modelcontextprotocol --format json --limit 600 > board-broken.json
|
||||
jq -r '[.items[]|select(.status==null)|.id]' board-broken.json > lost-ids.json
|
||||
jq -r --slurpfile L 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-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 '.[]' 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`).
|
||||
111
.claude/skills/issue-create/SKILL.md
Normal file
111
.claude/skills/issue-create/SKILL.md
Normal file
|
|
@ -0,0 +1,111 @@
|
|||
---
|
||||
name: issue-create
|
||||
description: Create a tracked Inspector issue end to end — version label, type label, milestone, board card, Status and Priority. Use when filing a new issue, or when work discovered mid-task needs an issue of its own.
|
||||
disable-model-invocation: true
|
||||
---
|
||||
|
||||
# Creating an issue
|
||||
|
||||
An issue **you** create is not "created" until all five of these are true. 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.
|
||||
|
||||
1. **Version label** — exactly one of `v1` / `v2`
|
||||
2. **Type label** — exactly one of `bug` / `enhancement` / `documentation` / `chore` / `question`
|
||||
3. **Milestone**
|
||||
4. **A card on the board for that version** — `v2` → #28, `v1` → #11
|
||||
5. **Status _and_ Priority set on that card** (Priority is v2-only)
|
||||
|
||||
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
|
||||
|
||||
```sh
|
||||
gh issue list --repo modelcontextprotocol/inspector --state open --limit 1000 \
|
||||
--search "<keywords>" --json number,title,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**.
|
||||
|
||||
```sh
|
||||
ITEM_ID=$(gh project item-add 28 --owner modelcontextprotocol --url <issue-url> --format json --jq '.id')
|
||||
# Status → Todo
|
||||
gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" \
|
||||
--field-id PVTSSF_lADOCt2Azc4BJVxtzg5iI8c --single-select-option-id fbdaf21e
|
||||
# Priority → Medium (score it with the rubric in /issue-triage; don't eyeball it)
|
||||
gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" \
|
||||
--field-id PVTSSF_lADOCt2Azc4BJVxtzg5iJE4 --single-select-option-id da944a9c
|
||||
```
|
||||
|
||||
Full ID tables, the v1 board's recipe, and the option-deletion hazard are in
|
||||
`/board-ops`. The priority rubric is in `/issue-triage`.
|
||||
|
||||
## Note on issues that arrive from elsewhere
|
||||
|
||||
An issue opened by hand in the GitHub UI — by an outside reporter *or* by a
|
||||
maintainer — arrives with no label, no milestone, and no card. That is normal on
|
||||
arrival, not a defect to fix the moment it lands: it comes into the system
|
||||
through `/issue-triage` instead, and starts in **Incoming** with no milestone,
|
||||
because nobody has approved it.
|
||||
266
.claude/skills/issue-triage/SKILL.md
Normal file
266
.claude/skills/issue-triage/SKILL.md
Normal file
|
|
@ -0,0 +1,266 @@
|
|||
---
|
||||
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: true
|
||||
---
|
||||
|
||||
# Triaging issues
|
||||
|
||||
**An issue needs triage when it arrives with no board card and no milestone —
|
||||
regardless of who filed it.** That is the whole test, and it is deliberately
|
||||
about *state*, not authorship. 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 | Incoming ⇔ no milestone | Approval was never recorded: move to **Todo**, or clear the milestone |
|
||||
| Past Incoming **without** a milestone | 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 |
|
||||
| No version label | Every issue carries exactly one of `v1`/`v2` | Apply it |
|
||||
| No type label | Every issue carries one of the five types | Classify it |
|
||||
| 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 |
|
||||
|
||||
```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[] | select(.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[], $B11[]) | select(.s=="Incoming" and ms(.n)!=null) | .n],
|
||||
"past Incoming, no ms": [($B28[], $B11[]) | 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, no version label":[$o[0][] | select(.state=="OPEN")
|
||||
| select(([.labels[].name]|index("v1") or index("v2"))|not) | .number],
|
||||
"open, no type label": [$o[0][] | select(.state=="OPEN")
|
||||
| select(([.labels[].name] | index("bug") or index("enhancement")
|
||||
or index("documentation") or index("chore")
|
||||
or index("question"))|not) | .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]
|
||||
} | 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.
|
||||
- **Only open issues have a `$I` entry.** A closed issue still has a card
|
||||
(correctly, in `Done`), so a check that treats "no milestone found" as a
|
||||
violation must gate on `isopen(.n)` or it flags every closed card.
|
||||
|
||||
**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.
|
||||
176
.claude/skills/local-dev/SKILL.md
Normal file
176
.claude/skills/local-dev/SKILL.md
Normal file
|
|
@ -0,0 +1,176 @@
|
|||
---
|
||||
name: local-dev
|
||||
description: Getting the Inspector running locally and adding dependencies to it. Which install to run and which script starts each client (web, cli, tui, launcher); why v2 is not an npm workspace; how the @inspector/core alias works; and where a new dependency must be declared — root vs client, dependencies vs devDependencies, and the bundler `external` lists a root package must also appear in.
|
||||
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 the published tarball ships only each client's `build/`. Set
|
||||
`INSPECTOR_SKIP_CLIENT_INSTALL=1` to skip it.
|
||||
|
||||
⚠️ **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** — the same alias in `clients/web/vite.config.ts`, for both the browser
|
||||
app and the Node backend runner.
|
||||
|
||||
## Where a dependency goes
|
||||
|
||||
Three independent questions. Get them in this order.
|
||||
|
||||
### 1. Which manifest declares it?
|
||||
|
||||
**The MCP SDK packages — `@modelcontextprotocol/client`, `core`, `server`,
|
||||
`server-legacy`, `ext-apps` — are declared in the repo-root `package.json` and
|
||||
nowhere else.** Node resolution walks up, so the root install already serves
|
||||
every client; a per-client declaration installs a *second copy* that drifts. That
|
||||
is not theoretical — it put two versions of `ext-apps` and of the transitive v1
|
||||
`@modelcontextprotocol/sdk` in the tree at once (#1970), and a second copy of
|
||||
`client`/`core` is exactly the failure the `dedupe` + `server.deps.inline`
|
||||
workaround in `vitest.shared.mts` exists for.
|
||||
|
||||
The same rule covers anything reached only through **root-owned code with no
|
||||
manifest of its own** (`test-servers/src`, `core/`): declare it at the root, and
|
||||
alias it to the **repo root** in `vitest.shared.mts`. `express` and `yaml` are
|
||||
the two today.
|
||||
|
||||
⚠️ The v1 SDK (`@modelcontextprotocol/sdk`) is **not** a dependency of this repo
|
||||
and must not become one. It appears in the lock files only as a `"peer": true`
|
||||
entry pulled in by `ext-apps`.
|
||||
|
||||
### 2. `dependencies` or `devDependencies`?
|
||||
|
||||
Follows from who consumes it **at runtime**, not from where it is declared:
|
||||
|
||||
- A package `core/` imports at runtime → root **`dependencies`**. The client
|
||||
builds externalize npm packages, so a published install resolves them from the
|
||||
root manifest, and devDependencies are not installed for consumers — a runtime
|
||||
import parked there breaks the published package while passing every local
|
||||
check.
|
||||
- A package only the tests, the test servers, or the build tooling need →
|
||||
**`devDependencies`** (`express`).
|
||||
- `vite` and `@vitejs/plugin-react` are root **`dependencies`** on purpose:
|
||||
`clients/web/server/start-vite-dev-server.ts` imports them at runtime for
|
||||
`--web --dev`, and `tsup.runner.config.ts` externalizes both.
|
||||
|
||||
### 3. Does it also need adding to the bundler `external` lists?
|
||||
|
||||
**Yes, if `core/` imports it at runtime.** tsup and Vite externalize what the
|
||||
*client's* `package.json` declares, and a root-only dependency is in none of
|
||||
them — so it gets **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.
|
||||
|
||||
The three lists are `clients/cli/tsup.config.ts`,
|
||||
`clients/tui/tsup.config.ts`, and `clients/web/tsup.runner.config.ts` — add it
|
||||
to **all three**, since which client reaches it is a function of what `core/`
|
||||
imports, not of what the client's own code names.
|
||||
|
||||
**`npm run verify:bundle-externals` enforces this.** It reads the **built
|
||||
output**, not the config (the two disagreed for four releases), over the union of
|
||||
each client's `external` array and the root manifest's `dependencies`. Note the
|
||||
ordinary test tiers cannot catch this class: unit and integration tests run
|
||||
against source, and the smokes never take the lazy-`import()` path.
|
||||
|
||||
⚠️ Probing it by hand with `node -e` **falsely succeeds** — `node -e` exposes a
|
||||
global `require` that satisfies esbuild's guard. Reproduce from a real `.mjs`
|
||||
file.
|
||||
|
||||
### The exception: a dependency that renders React must be bundled
|
||||
|
||||
An externalized package resolves its own `react` from wherever npm placed *it*,
|
||||
beside a React satisfying **that package's** peer range — looser than ours in
|
||||
every case here, which is all it takes to split React. `ink-form` and
|
||||
`ink-scroll-view` declare `">=18"`; the bundle renders through one React, those
|
||||
packages call hooks on another, and the TUI crashes on the first hook (#1952).
|
||||
Both are inlined by `clients/tui/tsup.config.ts` (`noExternal`) and declared
|
||||
only in `clients/tui/package.json`.
|
||||
|
||||
**`ink` is the single exemption, justified by cost** (~1.4MB), not by safety.
|
||||
Never justify an exemption by a peer range. What makes it safe is that the root
|
||||
`react` range stays open to the whole major (`^19.0.0`), so npm can dedupe our
|
||||
React with a consumer's. `clients/tui/__tests__/tsupConfig.test.ts` enforces all
|
||||
of it.
|
||||
|
||||
### One version per install-crossing dependency
|
||||
|
||||
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 4GB tsc heap
|
||||
(#1896).
|
||||
|
||||
**Raising the heap hides this class; align the versions instead** — bump it in
|
||||
every install that *declares* it (not all four unconditionally; a package absent
|
||||
from an install can't skew). `npm run verify:dep-lockstep` is the guard.
|
||||
|
||||
### Pinning a transitive dependency
|
||||
|
||||
Reach for an `overrides` entry before `npm audit fix`.
|
||||
`clients/{web,cli,tui}/package.json` each override `esbuild` to `^0.28.2`
|
||||
because `tsup@8.5.1` declares `^0.27.0` and the advisory has no upward escape
|
||||
inside that range — `npm audit fix` "resolves" it by silently *downgrading*.
|
||||
Two costs: it puts `tsup` on an esbuild major it does not declare, so `npm run
|
||||
build` 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.
|
||||
26
.claude/skills/local-dev/evals/evals.json
Normal file
26
.claude/skills/local-dev/evals/evals.json
Normal file
|
|
@ -0,0 +1,26 @@
|
|||
[
|
||||
{
|
||||
"prompt": "How do I get the web client running locally?",
|
||||
"expect": "local-dev"
|
||||
},
|
||||
{
|
||||
"prompt": "I added a dependency and the TUI bundle broke at import time. Where should it have gone?",
|
||||
"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": "Write a haiku about autumn.",
|
||||
"expect": null
|
||||
},
|
||||
{
|
||||
"prompt": "Explain the difference between let and const in JavaScript.",
|
||||
"expect": null
|
||||
}
|
||||
]
|
||||
180
.claude/skills/pr-flow/SKILL.md
Normal file
180
.claude/skills/pr-flow/SKILL.md
Normal file
|
|
@ -0,0 +1,180 @@
|
|||
---
|
||||
name: pr-flow
|
||||
description: Take an issue through to a merged PR — branch naming, the DCO signoff, screenshots, opening the PR against the right base, requesting a Copilot review, responding to review rounds, and closing out (manual close plus Done).
|
||||
disable-model-invocation: true
|
||||
---
|
||||
|
||||
# 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 `v2/main`**, never from a milestone-merge branch — the
|
||||
latter 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; 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"):
|
||||
|
||||
```sh
|
||||
curl -sX POST -H "Authorization: token $(gh auth token)" \
|
||||
--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"
|
||||
```
|
||||
|
||||
## 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** (`259d6aab`), since auto-close won't fire on `v2/main`.
|
||||
|
||||
`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`.
|
||||
134
.claude/skills/pre-push-gate/SKILL.md
Normal file
134
.claude/skills/pre-push-gate/SKILL.md
Normal file
|
|
@ -0,0 +1,134 @@
|
|||
---
|
||||
name: pre-push-gate
|
||||
description: Run the mandatory pre-push gate and diagnose a failing stage — npm run format then npm run local:gate, what each stage checks, and the known traps (stale tsc cache, symlinked worktree node_modules, concurrent runs, orphaned smoke ports).
|
||||
disable-model-invocation: true
|
||||
---
|
||||
|
||||
# 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
|
||||
```
|
||||
|
||||
`local:gate` is `validate` → `coverage` → `verify:build-gate` →
|
||||
`verify:bundle-externals` → `smoke` → `smoke:web:firefox` → `local:storybook`.
|
||||
|
||||
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: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).
|
||||
106
.claude/skills/project-structure/SKILL.md
Normal file
106
.claude/skills/project-structure/SKILL.md
Normal 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 |
|
||||
| `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`.
|
||||
22
.claude/skills/project-structure/evals/evals.json
Normal file
22
.claude/skills/project-structure/evals/evals.json
Normal file
|
|
@ -0,0 +1,22 @@
|
|||
[
|
||||
{
|
||||
"prompt": "What lives in clients/web versus @inspector/core?",
|
||||
"expect": "project-structure"
|
||||
},
|
||||
{
|
||||
"prompt": "I'm adding a helper both the CLI and the web client need. Which directory does it belong in?",
|
||||
"expect": "project-structure"
|
||||
},
|
||||
{
|
||||
"prompt": "Which package owns the OAuth secret storage backends?",
|
||||
"expect": "project-structure"
|
||||
},
|
||||
{
|
||||
"prompt": "Convert this array of strings to uppercase.",
|
||||
"expect": null
|
||||
},
|
||||
{
|
||||
"prompt": "What is the capital of France?",
|
||||
"expect": null
|
||||
}
|
||||
]
|
||||
102
.claude/skills/release/SKILL.md
Normal file
102
.claude/skills/release/SKILL.md
Normal file
|
|
@ -0,0 +1,102 @@
|
|||
---
|
||||
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
|
||||
git checkout -b v2/chore/<ISSUE>-bump-2-3-0 v2/main
|
||||
npm version minor --no-git-tag-version # or major / patch; bump only, no tag
|
||||
# PR → v2/main
|
||||
```
|
||||
|
||||
⚠️ **`--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
|
||||
|
||||
```sh
|
||||
git fetch origin main
|
||||
git tag 2.3.0 origin/main && git push origin 2.3.0
|
||||
# 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` — `2.2.0`,
|
||||
`2.1.0`, `2.0.0`. 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`.
|
||||
85
.claude/skills/test-servers/SKILL.md
Normal file
85
.claude/skills/test-servers/SKILL.md
Normal file
|
|
@ -0,0 +1,85 @@
|
|||
---
|
||||
name: test-servers
|
||||
description: Run a composable MCP test server by hand — pick the showcase config that reproduces a given feature or bug, build test-servers, and connect with the right protocol era. Covers the stale-build hazard.
|
||||
disable-model-invocation: true
|
||||
---
|
||||
|
||||
# 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
|
||||
|
||||
```sh
|
||||
# From the repo root, with a built launcher:
|
||||
node clients/launcher/build/index.js --web
|
||||
# then in the Inspector, add the server the config announces
|
||||
```
|
||||
|
||||
The server prints its URL on **stderr**, and the bound port is **not
|
||||
necessarily** the config's — `findAvailablePort()` walks upward when the
|
||||
configured port is taken. Parse 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".
|
||||
157
.claude/skills/testing/SKILL.md
Normal file
157
.claude/skills/testing/SKILL.md
Normal file
|
|
@ -0,0 +1,157 @@
|
|||
---
|
||||
name: testing
|
||||
description: Testing procedures for this repo — which npm command runs which suite (web unit, web integration, Storybook, cli, tui, launcher, scripts), the test tiers from unit through smoke and pack:verify, where a new test file goes (side-by-side vs src/test/ vs __tests__/), diagnosing the per-file 90% coverage gate, and the renderWithMantine / renderWithMantineTransitions rules for React and Mantine tests. Use for any question about running, placing, or fixing a test here.
|
||||
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.
|
||||
|
||||
## 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.
|
||||
22
.claude/skills/testing/evals/evals.json
Normal file
22
.claude/skills/testing/evals/evals.json
Normal file
|
|
@ -0,0 +1,22 @@
|
|||
[
|
||||
{
|
||||
"prompt": "I just added clients/tui/__tests__/serverList.test.ts. What command runs only the TUI suite?",
|
||||
"expect": "testing"
|
||||
},
|
||||
{
|
||||
"prompt": "In clients/web/src/components/Foo.test.tsx I need to assert on a Mantine transition mid-flight. How?",
|
||||
"expect": "testing"
|
||||
},
|
||||
{
|
||||
"prompt": "Which test tier in this repo spawns the built binary rather than importing it?",
|
||||
"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
|
||||
}
|
||||
]
|
||||
Loading…
Reference in a new issue