From 6e34eb70513fe8726d33115a07f77f39a736989b Mon Sep 17 00:00:00 2001 From: cliffhall Date: Fri, 31 Jul 2026 17:40:13 -0400 Subject: [PATCH 1/7] docs: correct post-swap branch model, PR policy, and stale gate/tree entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of the go-live phase-8 docs hygiene: everything that silently inverted when `main` became v2. Branch/release model — documented canonically in AGENTS.md and corrected everywhere it was restated: - AGENTS.md: replace the one-line "Base Branches" bullet with a table giving each branch its role (v2/main = develop, main = release, v1/main = maintenance), whether PRs target it, and which npm dist-tag it publishes. - README.md: the "Repo status" callout had it backwards, claiming `main` was the legacy v1 implementation. Also note that a v2 release is cut from `main` after the milestone merge, not from v2/main. - CONTRIBUTORS.md: the version table listed v2's base branch as `main`. PR policy: - Every PR must reference an issue, from anyone, no exceptions. - PRs are opened by repo maintainers only — org write access is not authorization. Everyone else files a detailed issue. - Branch names start with the target version segment (v2/…, v1/…). - Issues and PRs carry exactly one of `v1`/`v2` at creation; default `v2`. - Screenshots proving UI/TUI changes go in the gitignored pr-screenshots/. Corrections to stale content found while in there: - The TUI coverage bullet still described the Ink/App.tsx/hooks exclusion as interim pending #1501, which closed 2026-06-29; the gate now covers all of src/** with only tui-servers.ts excluded. - The completion checklist said `npm run validate` was the gate and covered "e2e tests", contradicting the mandatory-pre-push-gate section (npm run ci). - v1 PR base was given as `main`; it is `v1/main`. - Project tree omitted core/client, docs/, and scripts/ (README: core/client, docs/). core/client is under the coverage gate and imported by both App.tsx trees. Also adds the launcher and `npm run test:scripts` to the "run the tests" list and a tiers overview, since neither was mentioned. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU --- AGENTS.md | 109 ++++++++++++++++++++++++++++++------------------ CONTRIBUTORS.md | 42 +++++++++++++++---- README.md | 6 ++- 3 files changed, 108 insertions(+), 49 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5a5d48b86..26624585a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,7 +5,7 @@ This is an application for inspecting MCP servers. Has three incarnations, Web, ## Project Structure ``` -inspector/ +v2/main/ ├── clients/ │ ├── web/ # Web client (Vite + React + Mantine) │ │ ├── src/ # Browser source (React app, hooks, components) @@ -40,6 +40,13 @@ inspector/ │ │ ├── node/ # Node-side OAuth (NodeOAuthStorage, OAuthCallbackServer, │ │ │ # runner-interactive-oauth loopback callback flow) │ │ └── remote/ # Remote OAuth storage (delegates to the remote server) +│ ├── client/ # Install-level client config (`client.json`): browser-safe +│ │ # parse/validate (config-parse.ts) + Node load/save +│ │ # (config.ts, node-persistence.ts), the remote backend +│ │ # (remote.ts), secrets (secrets.ts), and runner.ts. +│ │ # Consumed by both App.tsx trees (web + tui); gated by +│ │ # the web coverage `include`, tests in +│ │ # clients/web/src/test/core/client/. │ ├── json/ # JSON utilities and parameter/argument conversion │ │ # (xMcpHeader.ts: SEP-2243 `x-mcp-header` │ │ # annotation scan/validation + mirrored-param @@ -82,6 +89,16 @@ inspector/ │ # in clients/web/vite.config.ts points at build/index.js │ # (not src/) so `getTestMcpServerPath()` returns a `.js` path. │ # tsconfig.test.json keeps paths pointing at src for typecheck. +├── docs/ # Task-oriented guides (mcp-server-configuration.md, +│ # mcp-app-review.md, launcher-config-consolidation-plan.md, +│ # images/). Linked from the root README. +├── scripts/ # Root build/verify tooling: install-clients.mjs (the +│ # postinstall cascade), the smoke-*.mjs runners, +│ # verify-build-gate / verify-format-coverage / +│ # verify-typecheck-coverage, pack-and-verify.mjs, +│ # and lib/ shared helpers. Prettier-gated via +│ # `format:check:scripts`; its own pure parsers are +│ # unit-tested by `npm run test:scripts` (node --test). ├── specification/ # Build specification ... ``` @@ -96,39 +113,21 @@ v2 is **not** an npm workspace — each client under `clients/*` keeps its own ` After installing, `npm run build` builds all clients. The launcher scripts (`npm run web` / `web:dev`) run the built launcher, so build first; for day-to-day web iteration use `cd clients/web && npm run dev`. -## Repository & Project Board - -- **Repo**: https://github.com/modelcontextprotocol/inspector.git -- **Base Branches**: v2/main (active), main (v1). v1.5/main is merged into v2/main and no longer takes new work. -- **Project Boards**: - - v2 - https://github.com/orgs/modelcontextprotocol/projects/28 (active board — all current work goes here) - - v1 - https://github.com/orgs/modelcontextprotocol/projects/11 (existing inspector version, no new activity except security and bug fixes) - ## Contributing -External contributions are accepted as **issues, not pull requests** — -maintainers handle design and implementation through a prompt-driven workflow. -If you've already built a change locally, share the **prompt** you used, not a -diff. See [`CONTRIBUTORS.md`](./CONTRIBUTORS.md) for the full policy. - -## Project Status and Direction -* The main branch currently contains the legacy version of the Inspector, which we are accepting bug fixes and minor improvement PRs for. - -* The v1.5/main branch was the intermediate version of the Inspector, where the shared logic between the three incarnations of the Inspector was extracted into a core subsystem with InspectorClient class as the common entry point. It also included the TUI, a refactored CLI, and streamlined launcher. The branch still exists but is **frozen** — it takes no new work. It is kept as a reference point (e.g. for tracking down a regression introduced by the merge into v2/main), so do not delete it. +External contributions are accepted as **issues, not pull requests** — maintainers handle design and implementation through a prompt-driven workflow. +If you've already built a change locally, share the **prompt** you used and screenshots if applicable, not a diff. See [`CONTRIBUTORS.md`](./CONTRIBUTORS.md) for the full policy. -* The v2/main branch currently contains the new version of the web Inspector, composed of "dumb" components which accept data and callbacks as props and contain only display logic. +**This applies to org members with write access too, not just outside contributors.** Having permission to push a branch is not authorization to open a PR. Pull requests against this repo are opened by the **repo maintainers** only. Anyone else — including organization members whose write access makes it technically possible — opens a **detailed issue** instead, and a maintainer takes it from there. A detailed issue means: the problem, how to reproduce it, the behavior you expected, and — if you've already prototyped a fix — the prompt you used and any screenshots, rather than a diff. -The Launcher, TUI, CLI, and InspectorClient from v1.5/main have been merged into v2/main. InspectorClient is wired up to the new web Inspector. Eventually, we will replace main with v2/main, eliminating the legacy implementations. - -## Web backend auth token +**Every PR must reference an issue. No exceptions, regardless of who opens it.** The PR body's first line is `Closes #` (see the [Issue-driven Work Style](#issue-driven-work-style) rules below). A PR with no linked issue has no board card, so the work is invisible to the project board and untracked — if you're about to open one and there's no issue yet, create the issue first. This holds for a maintainer's own one-line fix as much as for a feature. -The dev/prod web backend protects every `/api/*` route with `x-mcp-remote-auth: Bearer `. The browser recovers that token from three sources, in priority order (see `App.tsx` `getAuthToken()`): +## Project Status and Direction +* The v1/main branch currently contains the legacy version of the Inspector, which we are creating security fixes for in deprecated maintenance mode. It is **published straight from the branch** to the `v1-latest` npm dist-tag — v1 releases never pass through `main`, and v1 PRs therefore target `v1/main` directly. -1. `window.__INSPECTOR_API_TOKEN__` — injected into `index.html` on every page load by the backend (the dev Vite plugin via `transformIndexHtml`, the prod Hono server on the `/` route), both routed through `clients/web/server/inject-auth-token.ts`. This is what makes a bare-URL reload, a bookmark, or a cleared `sessionStorage` keep working. -2. `?MCP_INSPECTOR_API_TOKEN=…` query string — the URL the launcher banner prints; kept as a fallback for pasted full URLs. -3. `sessionStorage` — backstop for navigations that land without either of the above. +* The v2/main branch currently contains the the new version of the Inspector, which is actively being developed and maintained. All new features, bug fixes, and refactors should be implemented in this branch. It acts as the **develop branch**: work accumulates here continuously and is merged into `main` at milestone releases. -Injection is a no-op when auth is disabled (`DANGEROUSLY_OMIT_AUTH`), and the global name is the shared `INSPECTOR_API_TOKEN_GLOBAL` constant in `core/mcp/remote/constants.ts`. +* The main branch is the default branch for the repo, and it currently points to the latest v2 release. It is not a development branch, and no new features or bug fixes should be implemented here. It is only used for releases of the v2 Inspector via merge from v2/main, which is what publishes the `latest` npm dist-tag. ## Maintenance Rules @@ -146,21 +145,38 @@ All work should be driven by items on the project board. - Before starting work, check the board for the relevant item. - **Every board item is a real GitHub issue.** Do not create draft items (board cards with no issue number). If you find work that needs tracking, create an actual issue and add that to the board. Before creating a new issue, check the board for a matching item to avoid duplicates — **never create a duplicate**. -- **Assign the issue to its creator.** When you create an issue, assign it to the user it is created on behalf of (`gh issue create --assignee @me ...`, or `--assignee `). Board items should never be unassigned. -- **Label by version.** New issues and PRs must carry the label matching the target board / branch: - - `main` → `v1` - - `v2/main` → `v2` +- **Label by version — every issue and every PR, no exceptions.** Each one carries **exactly one** of `v1` or `v2` at creation. There is no unlabeled state and no "decide later": an issue with neither label belongs to no version line, appears in no version-filtered query, and is effectively invisible. + - `v1` — work targeting `v1/main` (the deprecated line: security and bug fixes only) + - `v2` — work targeting `v2/main` (active development; the default for anything new) - Set the label at create time (`gh issue create --label v2 ...`, `gh pr create --label v2 ...`) — don't rely on backfilling later, since unlabeled PRs are easy to miss when filtering by version. + Set the label at **create time** — `gh issue create --label v2 ...`, `gh pr create --label v2 ...` — never by backfilling later, since unlabeled items are exactly the ones missed when filtering by version. **If the target version isn't obvious, it's `v2`**: v2 is where all new work goes, and `v1` is reserved for the narrow case of patching the deprecated line. Only ask when the issue is specifically a fix *for released v1 behavior* and it's unclear whether v2 still has the bug. Note the label is a repo tag and is **not** the board — see the callout above; a `v2` issue also needs a board card with a Status. - **Add the issue to the board and set Status.** After creating an issue, add it to board #28 and set its Status. (PRs are never added to the board — they're tracked through their linked issue's card.) This is the step most easily forgotten because it needs several IDs — copy the recipes below verbatim. - When work begins, create a feature branch and set the item's Status to **In Progress** (or **V2 Go Live** for a card in the go-live phases, #1804). +- **Branch names start with the target version segment.** The first path segment must be the version whose base branch the PR targets — `v2/` for work on `v2/main`, `v1/` for work on `v1/main` — followed by the usual type and slug: `v2/ci/restore-claude-workflow`, `v2/fix/oauth-scope-union`, `v1/fix/proxy-ssrf-pin`. Not `ci/restore-claude-workflow`. 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`). - When work is complete: - - Run format, lint, typecheck, build, and test — ensure all checks pass - - Open a PR against the matching base branch (`main` for v1, `v2/main` for v2) and set the item's Status to **In Review** - - **Link the PR to its issue.** The PR body's **first line must be `Closes #`**. ⚠️ Note: closing keywords only auto-link/auto-close for PRs targeting the repo's **default branch** (`main`). Because v2 PRs target `v2/main` (a non-default branch), `Closes #N` there is only a cross-reference — it will **not** create a hard link or close the issue on merge. (There is no `gh` flag for manual linking — `gh pr edit` has no `--add-issue`; closing keywords are the only mechanism GitHub exposes, and they're gated to the default branch.) + - Run `npm run ci` from the root — the mandatory pre-push gate (see [Mandatory pre-push gate](#mandatory-pre-push-gate)). `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. + - Open a PR against the matching base branch (`v1/main` for v1, `v2/main` for v2) and set the item's Status to **In Review** + - **Attach screenshots as proof of functionality.** Any change to the web UI or the TUI must show its result: capture before/after screenshots (or a short GIF for an interaction) and put them in 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 to the source tree — 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`, `tools-tab-after.png`), not `Screenshot 2026-07-31 at 14.02.11.png`. + - **Link the PR to its issue — mandatory for every PR, from anyone.** No PR is opened without an issue to reference; if one doesn't exist yet, create it first (labeled and on the board) rather than opening the PR and backfilling. Note also that only the **repo maintainers** open PRs at all (see [Contributing](#contributing)) — everyone else files a detailed issue. The PR body's **first line must be `Closes #`**. ⚠️ Note: closing keywords only auto-link/auto-close for PRs targeting the repo's **default branch** (`main`). Because v2 PRs target `v2/main` (a non-default branch), `Closes #N` there is only a cross-reference — it will **not** create a hard link or close the issue on merge. (There is no `gh` flag for manual linking — `gh pr edit` has no `--add-issue`; closing keywords are the only mechanism GitHub exposes, and they're gated to the default branch.) - **On merge of a v2 PR, manually close its issue and move the board item to Done** (option id `248a3910`), since auto-close won't fire on `v2/main`. Keep the `Closes #N` line anyway so the issues close automatically if/when `v2/main` is eventually merged to `main`. - If new tasks are discovered or requested during development, create issues and add them to the board. +## Repository & Project Boards + +- **Repo**: https://github.com/modelcontextprotocol/inspector.git +- **Base Branches** — three branches, three distinct roles. Target the one matching the work; never open a PR against `main`. + + | Branch | Role | PRs target it? | Publishes to | + | --- | --- | --- | --- | + | `v2/main` | **Develop.** All active v2 work lands here. | **Yes** — every v2 PR | nothing directly; reaches npm via `main` | + | `main` | **Release.** The repo's default branch; holds the latest released v2. Not a development branch. | **No** — it only receives milestone merges from `v2/main` | `latest` | + | `v1/main` | **Maintenance.** The deprecated v1 line, security and bug fixes only, no active development. | **Yes** — every v1 PR, directly | `v1-latest`, published straight from this branch | + + So v2 flows `feature branch → v2/main → (milestone) main → npm latest`, while v1 is flat: `feature branch → v1/main → npm v1-latest`, with no merge into `main` at any point. The two lines are published independently under separate dist-tags, which is why a v1 fix does **not** need to be forward-ported to reach users on v1 (`npx @modelcontextprotocol/inspector@v1-latest`). +- **Project Boards**: + - v2 - https://github.com/orgs/modelcontextprotocol/projects/28 (active board — all new work goes here) + - v1 - https://github.com/orgs/modelcontextprotocol/projects/11 (legacy inspector version, no new activity except security and bug fixes) + #### V2 board (#28) `gh` recipes The board is an **org project**, so all commands use `--owner modelcontextprotocol` and the numeric project `28`. The project node id and Status field id are stable. **The Status *option* ids are NOT stable — they are regenerated whenever the Status field's option list is edited** (see the ⚠️ hazard below). If any option id here is rejected, re-fetch the current set with: @@ -175,23 +191,22 @@ gh project field-list 28 --owner modelcontextprotocol --format json \ | Project node ID | `PVT_kwDOCt2Azc4BJVxt` | | Status field ID | `PVTSSF_lADOCt2Azc4BJVxtzg5iI8c` | -Status option IDs (`--single-select-option-id`) — **last verified 2026-07-27**. `V2 Go Live` was added via the web UI on 2026-07-27 for the go-live phases (#1804); the other four IDs were unchanged by that addition, confirming the web-UI path is safe (see the ⚠️ hazard below). Removed columns whose IDs are now rejected: `SDK V2 + New Spec` (`1bbb6f57`), `Building CLI / TUI / CORE` (`4ac261ee`), `Building Web` (`c28da89f`), `MCP Apps Extension` (`73d0b807`). +Status option IDs (`--single-select-option-id`) — **last verified 2026-07-27**. | Status | Option ID | | --- | --- | | Todo | `fbdaf21e` | -| V2 Go Live | `b3a6966e` | | In Progress | `195df262` | | In Review | `159c8a02` | | Done | `248a3910` | -Use **Todo** for approved-but-not-started work, **In Progress** for general active work (regardless of surface), **V2 Go Live** for cards in the go-live phases (#1804), **In Review** once a PR is open, and **Done** on merge. +Use **Todo** for approved-but-not-started work, **In Progress** for general active work (regardless of surface), **In Review** once a PR is open, and **Done** on merge. > ⚠️ **Never add, rename, or remove a board column (Status option) with the `updateProjectV2Field` GraphQL mutation unless you pass every existing option's `id`.** That mutation does a **full replace** of the option list: if you resend options by name/color/description but omit their `id`s, GitHub **deletes all existing options and mints new ones**, which **orphans the Status of every card on the board** (all items go blank) *and* invalidates every option id in the table above. This has happened once (required reconstructing ~197 items' statuses by inference). Safe alternatives, in order of preference: > 1. **Add/rename/remove a column in the GitHub web UI** (Project #28 → Status field settings). This preserves ids of untouched options and never orphans cards. > 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. Verify afterward that no card lost its 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. When option ids change for any reason, **re-verify and update the table above** (and the `248a3910` / `195df262` / `159c8a02` references in the recipes below and the merge step above). +> `gh project item-add` and `gh project item-edit` are always safe — they set a card's value and never touch the field schema. When option ids change for any reason, **re-verify and update the table above** (and the references in the recipes below and the merge step above). ```sh # 1. Add an issue to the board — prints the item id (PVTI_…); capture it. @@ -229,12 +244,15 @@ gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" --field-i - Run unit tests with `npm run test` (or `npm run test:watch` during development) from `clients/web/` - Run CLI tests with `npm run test` from `clients/cli/` (builds test-servers + CLI bin first via `pretest`) - Run TUI tests with `npm run test` from `clients/tui/` +- Run launcher tests with `npm run test` from `clients/launcher/` +- Run the root tooling's own tests with `npm run test:scripts` from the root — `node --test "scripts/**/*.test.mjs"`, node's built-in runner (the root has no vitest harness by design). A new `scripts/*.mjs` helper with pure logic gets a sibling `*.test.mjs`; keep the filename `*.test.mjs`, since `node --test` silently **skips** a file its glob misses and still exits 0 (`verify:typecheck-coverage` guards against exactly that). +- **The test 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` (it's part of cli's `test`), but **not** web's integration project, which runs inside the `coverage` gate. Everything from `smoke` rightward is `npm run ci` only, and is described under [Mandatory pre-push gate](#mandatory-pre-push-gate). - The repo root has no aggregate `test` script — each client self-validates, so run `npm run validate` from the root (all clients, fast) or `cd clients/ && npm run validate` (one client). Each client still exposes its own `test` / `test:coverage` for quick iteration. - **`validate` is fast: it runs `test`, not `test:coverage`.** The coverage gate (slower — adds v8 instrumentation, and for web the integration project) is a **separate** top-level `npm run coverage` (and per-client `coverage:web` / `coverage:cli` / `coverage:tui` / `coverage:launcher`, each delegating to that client's `test:coverage`). Run `npm run coverage` when you want to reproduce the gate locally before pushing. **CI runs `coverage`** on every push (#1550): the per-file ≥90 gate is CI-enforced, so a PR that drops any file below 90 on lines/statements/functions/branches fails the job. CI runs `validate` (fast) for format/lint/build/unit tests, then `coverage` for the instrumented gate. Because web's `test:coverage` already runs the integration project, CI has no separate `test:integration` step — the integration paths are exercised inside the coverage gate. - Each client's `test:coverage` enforces a **uniform per-file gate of ≥ 90 on all four dimensions** — lines, statements, functions, and branches — across `clients/web`, `clients/cli`, `clients/tui`, and `clients/launcher` (CI enforces this gate). This is the result of a codebase-wide audit: the branch floor was first lifted 50 → 70 for web (#1271), then the whole gate raised to 90 with real tests added for every outlier. Genuinely-unreachable branches are **not** waved through by lowering the gate — they are annotated at the source with a justified `/* v8 ignore … -- */` comment. Acceptable reasons are happy-dom-inherent paths (Mantine portal mount points, `useMediaQuery` fallbacks, `typeof window` SSR guards), React StrictMode effect-replay blocks, and provably-dead defensive guards (e.g. a `?? fallback` for a value the types guarantee non-null, or a `Select.onChange` receiving a value outside the allowed list). New code must clear 90 on every dimension; reach for a justified `v8 ignore` only when a branch is genuinely impossible to exercise. The web coverage `include` (in `clients/web/vite.config.ts`) covers the shared `core/` runtime consumed by the browser — `core/mcp`, `core/react`, `core/auth`, `core/storage`, `core/logging`, `core/node`, **`core/json`, and `core/client`** (the last two folded in by #1689). When adding a `core/json/*` or `core/client/*` module, its tests live under `clients/web/src/test/core/…` and are gated the same ≥90 way. - The **same per-file gate** is enforced for the CLI and TUI (#1484), not just web: - **CLI** (`clients/cli`): tests run **in-process** by importing `runCli()` (see `__tests__/helpers/cli-runner.ts`) so `clients/cli/src` is measured under v8 instrumentation. A thin out-of-process layer (`__tests__/e2e.test.ts` + `scripts/smoke-cli.mjs`) still spawns the built binary for the shebang/`process.exit` paths; `src/index.ts` (binary bootstrap) is the only coverage exclusion. `commander` uses `.exitOverride()` so a parse error throws instead of tearing down the test worker. - - **TUI** (`clients/tui`): the gate covers the **non-React logic** only — `logger.ts`, `components/tabsConfig.ts`, and `utils/*` (server resolution lives in `core/` and is measured by the web suite). The Ink components, `App.tsx`, and `hooks/` are an **interim exclusion** in `clients/tui/vitest.config.ts` pending the renderer-based follow-up (#1501). When adding new **non-React** logic under `clients/tui/src`, it falls under the gate automatically — add tests for it. + - **TUI** (`clients/tui`): the gate now covers **all of `src/**`, React surface included** — the former interim exclusion of the Ink components, `App.tsx`, and `hooks/` was lifted in #1501. Components mount through `ink-testing-library` with the `ink-scroll-view` / `ink-form` passthrough doubles in `__tests__/helpers/`, `App.tsx` mounts against a controllable mock of the `@inspector/core` surface, and keypresses are driven through stdin. The **only** coverage exclusion left in `clients/tui/vitest.config.ts` is `src/tui-servers.ts` — a pure re-export + type alias of core's server resolver with no runtime statements of its own (the logic is measured in `core/` via the web suite; `tui-servers.test.ts` still exercises it behaviorally, and it's excluded only so it doesn't surface as a misleading 0/0 row). Any new logic under `clients/tui/src`, React or not, is held to the gate automatically. - Run `npm run test:integration` (also from `clients/web/`) for the InspectorClient + transport + auth integration suite. It runs under a separate `integration` vitest project in node env (no happy-dom) with 30s timeouts. The script builds `test-servers/` first via `tsc -p ../../test-servers --noCheck` so the stdio MCP test server can be spawned as a real subprocess. CI does not run `test:integration` as its own step — the integration project is covered by the CI `coverage` gate, whose web `test:coverage` runs `--project=unit --project=integration --coverage`. - Test files live alongside the source as `.test.tsx` (or `.test.ts` for non-React modules). Integration tests live under `clients/web/src/test/integration/`, mirroring the `core/` source layout (`mcp/`, `mcp/node/`, `mcp/remote/`, `auth/`, `auth/node/`, `storage/`). Any test file under that folder is automatically picked up by the `integration` vitest project (node env, 30s timeouts) via the folder glob in `vite.config.ts` — placement is the manifest, there is no enumeration to keep in sync. Tests outside the folder run in the `unit` project (happy-dom). When adding a new test for, e.g., `core/mcp/remote/foo.ts`, put it at `src/test/integration/mcp/remote/foo.test.ts`. - **Test placement: side-by-side by default, `src/test/` only for what can't be co-located.** These look like competing conventions but aren't — the split is: *tests live beside their source, **except** tests for the repo-root `core/` package (which lives outside `clients/web/`) and shared test scaffolding — both of which live under `src/test/`, with `core/` tests mirroring the `core/` layout and integration tests under `src/test/integration/`.* @@ -351,3 +369,14 @@ Nothing **enforces** the boundary: no path alias keys off it, and the coverage ` - Element components add domain-specific semantics on top of Mantine primitives. For example, `AnnotationBadge` maps domain concepts (audience, destructive, longRun) to Mantine's styling primitives (color, variant). Storybook documents these domain components for designers and developers. - Element components MUST import from `@mantine/core`, NOT from `src/theme/`. The theme layer is applied transparently by the provider — elements do not need to know about `Theme` constants. - NEVER push domain-specific variant logic (e.g., annotation types, transport types) into theme files. Domain variants belong in the element component that owns those semantics. Theme files are for styling that applies to the Mantine primitive globally. + +## Web backend auth token + +The dev/prod web backend protects every `/api/*` route with `x-mcp-remote-auth: Bearer `. The browser recovers that token from three sources, in priority order (see `App.tsx` `getAuthToken()`): + +1. `window.__INSPECTOR_API_TOKEN__` — injected into `index.html` on every page load by the backend (the dev Vite plugin via `transformIndexHtml`, the prod Hono server on the `/` route), both routed through `clients/web/server/inject-auth-token.ts`. This is what makes a bare-URL reload, a bookmark, or a cleared `sessionStorage` keep working. +2. `?MCP_INSPECTOR_API_TOKEN=…` query string — the URL the launcher banner prints; kept as a fallback for pasted full URLs. +3. `sessionStorage` — backstop for navigations that land without either of the above. + +Injection is a no-op when auth is disabled (`DANGEROUSLY_OMIT_AUTH`), and the global name is the shared `INSPECTOR_API_TOKEN_GLOBAL` constant in `core/mcp/remote/constants.ts`. + diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 595f2847a..df3bda452 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -8,7 +8,9 @@ form we can act on quickly and consistently. **We accept issues, not pull requests.** Design and implementation are done by the maintainers. If you've already built a fix or feature locally, share **the -prompt you used** to produce it — not the source code. +prompt you used** to produce it — not the source code. This applies to everyone +outside the repo maintainers, including organization members who happen to have +write access to this repository. ## Why this policy exists @@ -32,6 +34,26 @@ than an issue, see [Want to work on the Inspector with us?](#want-to-work-on-the-inspector-with-us) below — we'd rather bring you into the workflow than turn you away. +## Who opens pull requests + +Pull requests against this repository are opened by the **repo maintainers** +only. That includes organization members with write access: being able to push +a branch here isn't the same as being asked to — the constraint is the workflow +described above, not permissions, so the same policy applies whether or not +GitHub would let you click the button. + +If you're not a repo maintainer, open a **detailed issue** instead and a +maintainer will pick it up. If you've already prototyped the change locally, +say so in the issue and include the prompt you used (see +[If you've already fixed it locally](#if-youve-already-fixed-it-locally)) — +that's the fastest path from your work to a merged change. + +**Every pull request references an issue**, including the maintainers' own. The +PR body's first line is `Closes #`. Work is tracked on the project +board through issues, so a PR without one is invisible to the board — which is +why a well-formed issue is the useful contribution here, and why writing one is +never wasted effort. + ## How to contribute a bug report or feature request Open a well-formed issue describing the bug or the feature you have in mind. @@ -46,18 +68,22 @@ The Inspector is maintained across two versions, each with its own base branch and version label. File your issue against the version your report or request targets: -| Version | Base branch | Label | -| ------- | ----------- | ----- | -| v1 | `v1/main` | `v1` | -| v2 | `main` | `v2` | +| Version | Base branch | Label | npm tag | +| ------- | ----------- | ----- | ------------ | +| v1 | `v1/main` | `v1` | `v1-latest` | +| v2 | `v2/main` | `v2` | `latest` | - **v1** (`v1/main`) is the legacy Inspector — it takes **security fixes - only**. -- **v2** (`main`) is where all current work happens — when in doubt, target v2. + only**, and is published straight from that branch to the `v1-latest` npm + tag (`npx @modelcontextprotocol/inspector@v1-latest`). +- **v2** (`v2/main`) is where all current work happens — when in doubt, target + v2. `v2/main` is the develop branch; it is merged into `main` at milestone + releases, and `main` is what publishes the `latest` npm tag. Nothing targets + `main` directly. **Label by version.** Every issue (and the PRs maintainers open for it) must carry the label matching the target branch — `v1` for `v1/main` and `v2` for -`main`. This mirrors the "Label by version" convention documented in +`v2/main`. This mirrors the "Label by version" convention documented in [`AGENTS.md`](./AGENTS.md). ## If you've already fixed it locally diff --git a/README.md b/README.md index 235be1434..0613bbf77 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ npx @modelcontextprotocol/inspector --cli # CLI npx @modelcontextprotocol/inspector --tui # TUI ``` -> **Repo status.** This is the **v2** line of the Inspector (branch `v2/main`). The `main` branch is the legacy v1 implementation (bug fixes only). v2 will eventually replace `main`. See [`AGENTS.md`](./AGENTS.md) for branch/board conventions. +> **Repo status.** This is the **v2** line of the Inspector. Active development happens on **`v2/main`** (the develop branch — all v2 PRs target it), which is merged into **`main`** at milestone releases; `main` is the default branch and holds the latest released v2, published to the npm `latest` tag. The legacy **v1** line lives on **`v1/main`** — security and bug fixes only, published straight from that branch to the npm `v1-latest` tag (`npx @modelcontextprotocol/inspector@v1-latest`). See [`AGENTS.md`](./AGENTS.md) for branch/board conventions. ## Project layout @@ -29,6 +29,7 @@ inspector/ │ └── launcher/ # Shared launcher — provides the `mcp-inspector` bin, dispatches to web/cli/tui ├── core/ # Shared code consumed via the `@inspector/core` alias (no package.json) │ ├── auth/ # OAuth: providers, discovery, storage, mid-session recovery (browser/node/remote backends) +│ ├── client/ # Install-level client config (`client.json`): browser-safe parse/validate + Node load/save, remote backend, secrets │ ├── json/ # JSON + parameter/argument conversion utilities │ ├── logging/ # Silent pino logger singleton │ ├── mcp/ # InspectorClient runtime, state stores, transports, config import @@ -37,6 +38,7 @@ inspector/ │ └── storage/ # File I/O helpers for the OAuth persist backends ├── test-servers/ # Composable MCP test servers + fixtures used by integration tests ├── scripts/ # Root build/verify tooling (install cascade, smokes, verify-build-gate, verify-format-coverage, pack:verify) +├── docs/ # Task-oriented guides (server configuration, MCP App review, launcher/config plan) ├── specification/ # Design/build specifications ├── AGENTS.md # Contribution rules for agents AND humans (see below) └── README.md # You are here @@ -182,6 +184,8 @@ Publishing is automated by two release-gated jobs in [`.github/workflows/main.ym - **`publish`** — the npm package. 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` — a single `npm publish` (v2 is not an npm workspace, so there is no v1-style `publish-all`/`--workspaces`), with a signed provenance attestation via GitHub OIDC (`id-token: write`, `environment: release`, `NPM_TOKEN`). - **`publish-github-container-registry`** — the container image (see [Docker](#docker)). +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`; see [Repo status](#mcp-inspector).) + Because there is **one version number** (only the root `package.json` has one — the clients carry none, so there is nothing to keep in sync and no `check-version` step), the release flow is just: ```bash From cf53c90c0e7c35f28805ecfe14a0edbbbae51920 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Fri, 31 Jul 2026 19:42:33 -0400 Subject: [PATCH 2/7] docs: require a milestone on every new issue, defaulting to the current one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the rule alongside the version-label rule, since they fail the same way: an unmilestoned issue drops out of release planning as silently as an unlabeled one drops out of version filtering. - Set it at create time (`gh issue create --milestone ...`). - If the user didn't specify one, default to the current milestone — the open milestone with the nearest due date — rather than leaving it blank pending a decision. Includes the `gh api .../milestones` one-liner that identifies it. - Milestones are release buckets, so pick by when the work ships, not by size; sub-issues normally inherit their parent's milestone. Also widens the section's opening callout: an issue is not "created" until it is labeled, milestoned, AND boarded with a Status — four distinct steps, not three. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU --- AGENTS.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 26624585a..8411362b6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -141,7 +141,7 @@ If you've already built a change locally, share the **prompt** you used and scre All work should be driven by items on the project board. -> **A v2 issue is not "created" until it is BOTH labeled `v2` AND on board #28 with a Status set.** Labeling alone is not enough — a label is a repo tag; the board is a separate org project. Applying `--label v2` does **not** add the item to the board, and adding it to the board does **not** set a Status. All three are distinct steps; do all three (see the recipes below). **Only issues go on the board — never PRs.** A PR still gets the `v2` label, but it is tracked through its linked issue's card (via `Closes #N`), not its own board item. +> **A v2 issue is not "created" until it is labeled `v2`, given a milestone, AND on board #28 with a Status set.** Labeling alone is not enough — a label is a repo tag, the milestone is a release bucket, and the board is a separate org project. Applying `--label v2` does **not** add the item to the board, and adding it to the board does **not** set a Status. All four are distinct steps; do all four (see the recipes below). **Only issues go on the board — never PRs.** A PR still gets the `v2` label, but it is tracked through its linked issue's card (via `Closes #N`), not its own board item. - Before starting work, check the board for the relevant item. - **Every board item is a real GitHub issue.** Do not create draft items (board cards with no issue number). If you find work that needs tracking, create an actual issue and add that to the board. Before creating a new issue, check the board for a matching item to avoid duplicates — **never create a duplicate**. @@ -151,6 +151,15 @@ All work should be driven by items on the project board. Set the label at **create time** — `gh issue create --label v2 ...`, `gh pr create --label v2 ...` — never by backfilling later, since unlabeled items are exactly the ones missed when filtering by version. **If the target version isn't obvious, it's `v2`**: v2 is where all new work goes, and `v1` is reserved for the narrow case of patching the deprecated line. Only ask when the issue is specifically a fix *for released v1 behavior* and it's unclear whether v2 still has the bug. Note the label is a repo tag and is **not** the board — see the callout above; a `v2` issue also needs a board card with a Status. - **Add the issue to the board and set Status.** After creating an issue, add it to board #28 and set its Status. (PRs are never added to the board — they're tracked through their linked issue's card.) This is the step most easily forgotten because it needs several IDs — copy the recipes below verbatim. +- **Every new issue gets a milestone — no exceptions.** Set it at create time with `gh issue create --milestone <title> ...`. **If the user didn't specify one, default to the current milestone**: the open milestone with the nearest due date. Never leave an issue unmilestoned pending a decision — an unmilestoned issue drops out of release planning silently, the same way an unlabeled one drops out of version filtering. Moving it later is one command; noticing it was never set is the hard part. Get the current milestone with: + + ```sh + # Open milestones, soonest due date first — the first row is the current one. + 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 — if a sub-task must ship with its parent, they belong in the same one. - When work begins, create a feature branch and set the item's Status to **In Progress** (or **V2 Go Live** for a card in the go-live phases, #1804). - **Branch names start with the target version segment.** The first path segment must be the version whose base branch the PR targets — `v2/` for work on `v2/main`, `v1/` for work on `v1/main` — followed by the usual type and slug: `v2/ci/restore-claude-workflow`, `v2/fix/oauth-scope-union`, `v1/fix/proxy-ssrf-pin`. Not `ci/restore-claude-workflow`. 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`). - When work is complete: From 09edb8e925121f2d0643cbb99c90e96c6dbe617e Mon Sep 17 00:00:00 2001 From: cliffhall <cliff@futurescale.com> Date: Fri, 31 Jul 2026 20:06:52 -0400 Subject: [PATCH 3/7] docs: add .github/copilot-instructions.md and require mirroring into it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub Copilot reads .github/copilot-instructions.md when reviewing a PR. The repo had no such file, so Copilot reviewed without any of the conventions that actually govern this codebase — the ban on `any` and on unjustified double casts, the Mantine-first styling rules, the `.withProps()` extraction rule, the lib/utils split, test placement, and the per-file >=90% four-dimension gate. The new file is a review-focused distillation of AGENTS.md: the rules a reviewer would cite against a diff, plus a short "what to prioritize" list that puts correctness and security first (this backend spawns processes and proxies outbound requests). Deliberately omitted: board recipes and IDs, milestone and branch mechanics, release procedure, and the project tree — no reviewer cites those, and copying them would double the maintenance surface. AGENTS.md's "Keep documentation files up to date" section now requires mirroring review-relevant changes into it in the same PR, states that AGENTS.md stays the source of truth, and spells out what counts as review-relevant. There is no generation step and nothing detects drift, so the rule is the only thing keeping the two in sync. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU --- .github/copilot-instructions.md | 98 +++++++++++++++++++++++++++++++++ AGENTS.md | 5 ++ 2 files changed, 103 insertions(+) create mode 100644 .github/copilot-instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 000000000..092a0c6bf --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,98 @@ +# 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`). + +## 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`. + +### 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. + +## 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`. Never hand-roll a bare `MantineProvider` — that reintroduces a real failure class where a `Transition`/`Modal` timer fires after happy-dom tears down `window` and fails the entire run. +- 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` → `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. +- **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. +- 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. diff --git a/AGENTS.md b/AGENTS.md index 8411362b6..afccecc15 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -136,6 +136,11 @@ If you've already built a change locally, share the **prompt** you used and scre - When the structure of the project, the tech stack, or the developer setup changes, update appropriate README.md files with the details. - When adding new commands, dependencies, or architectural patterns, update the relevant sections of appropriate README.md files as well. - When rules for implementation and testing change, update this file AGENTS.md +- **Mirror review-relevant changes into [`.github/copilot-instructions.md`](.github/copilot-instructions.md).** That file is what GitHub Copilot reads when it reviews a PR, and it is a hand-maintained **distillation** of this one — there is no generation step and nothing detects drift, so it goes stale silently and Copilot then reviews against rules we no longer hold. + - **AGENTS.md remains the source of truth.** Never edit `copilot-instructions.md` alone to change a rule; change it here first, then mirror. + - **Review-relevant** means anything a reviewer would cite against a diff: the TypeScript rules, the Mantine/React conventions (including the `.withProps()` rule and its exceptions), the `lib` vs `utils` split, test placement, the ≥90% coverage gate and the `v8 ignore` policy, the `renderWithMantine` requirement, and the PR hygiene rules (`Closes #N`, version label). Changing any of these means updating both files in the **same PR**. + - **Not review-relevant, and deliberately absent** from the mirror: the board recipes and their IDs, milestone and branch-naming mechanics, release and publishing procedure, and the project-structure tree. Copying those in would double the maintenance surface for content no reviewer cites. + - Keep it a **distillation, not a copy** — it is read on every review, so length has a cost. Prefer tightening the summary over pasting a section wholesale. ### Issue-driven Work Style From 33cc0366b6a44e7095c7d6c040c0595c2e063301 Mon Sep 17 00:00:00 2001 From: cliffhall <cliff@futurescale.com> Date: Fri, 31 Jul 2026 20:29:52 -0400 Subject: [PATCH 4/7] =?UTF-8?q?docs:=20address=20Copilot=20review=20?= =?UTF-8?q?=E2=80=94=20v1=20policy=20wording,=20stale=20V2=20Go=20Live,=20?= =?UTF-8?q?typo?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Align the v1 maintenance policy on "security and bug fixes only" in AGENTS.md and CONTRIBUTORS.md; both said "security fixes only" while the README callout, the branch table, and the board description said "security and bug fixes". - Drop the stale "or V2 Go Live" instruction from the work-begins step — that column no longer exists on board #28 (verified via `gh project field-list 28`), so the option id it implied would be rejected. Restore the removed-column id list the same edit dropped, now including V2 Go Live, and re-date the table to today's verification. - Fix "contains the the new version" typo. - Strip a stray trailing blank line at the end of AGENTS.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU --- AGENTS.md | 9 ++++----- CONTRIBUTORS.md | 6 +++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index afccecc15..804a8ff27 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -123,9 +123,9 @@ If you've already built a change locally, share the **prompt** you used and scre **Every PR must reference an issue. No exceptions, regardless of who opens it.** The PR body's first line is `Closes #<ISSUE_NUMBER>` (see the [Issue-driven Work Style](#issue-driven-work-style) rules below). A PR with no linked issue has no board card, so the work is invisible to the project board and untracked — if you're about to open one and there's no issue yet, create the issue first. This holds for a maintainer's own one-line fix as much as for a feature. ## Project Status and Direction -* The v1/main branch currently contains the legacy version of the Inspector, which we are creating security fixes for in deprecated maintenance mode. It is **published straight from the branch** to the `v1-latest` npm dist-tag — v1 releases never pass through `main`, and v1 PRs therefore target `v1/main` directly. +* The v1/main branch currently contains the legacy version of the Inspector, which is in deprecated maintenance mode — **security and bug fixes only**, no new features. It is **published straight from the branch** to the `v1-latest` npm dist-tag — v1 releases never pass through `main`, and v1 PRs therefore target `v1/main` directly. -* The v2/main branch currently contains the the new version of the Inspector, which is actively being developed and maintained. All new features, bug fixes, and refactors should be implemented in this branch. It acts as the **develop branch**: work accumulates here continuously and is merged into `main` at milestone releases. +* The v2/main branch currently contains the new version of the Inspector, which is actively being developed and maintained. All new features, bug fixes, and refactors should be implemented in this branch. It acts as the **develop branch**: work accumulates here continuously and is merged into `main` at milestone releases. * The main branch is the default branch for the repo, and it currently points to the latest v2 release. It is not a development branch, and no new features or bug fixes should be implemented here. It is only used for releases of the v2 Inspector via merge from v2/main, which is what publishes the `latest` npm dist-tag. @@ -165,7 +165,7 @@ All work should be driven by items on the project board. ``` 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 — if a sub-task must ship with its parent, they belong in the same one. -- When work begins, create a feature branch and set the item's Status to **In Progress** (or **V2 Go Live** for a card in the go-live phases, #1804). +- When work begins, create a feature branch and set the item's Status to **In Progress**. - **Branch names start with the target version segment.** The first path segment must be the version whose base branch the PR targets — `v2/` for work on `v2/main`, `v1/` for work on `v1/main` — followed by the usual type and slug: `v2/ci/restore-claude-workflow`, `v2/fix/oauth-scope-union`, `v1/fix/proxy-ssrf-pin`. Not `ci/restore-claude-workflow`. 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`). - When work is complete: - Run `npm run ci` from the root — the mandatory pre-push gate (see [Mandatory pre-push gate](#mandatory-pre-push-gate)). `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. @@ -205,7 +205,7 @@ gh project field-list 28 --owner modelcontextprotocol --format json \ | Project node ID | `PVT_kwDOCt2Azc4BJVxt` | | Status field ID | `PVTSSF_lADOCt2Azc4BJVxtzg5iI8c` | -Status option IDs (`--single-select-option-id`) — **last verified 2026-07-27**. +Status option IDs (`--single-select-option-id`) — **last verified 2026-07-31**, when `V2 Go Live` was removed from the board along with the go-live phases it tracked. The four below are the complete current set; anything else (`V2 Go Live` `b3a6966e`, `SDK V2 + New Spec` `1bbb6f57`, `Building CLI / TUI / CORE` `4ac261ee`, `Building Web` `c28da89f`, `MCP Apps Extension` `73d0b807`) is a removed column whose id is now rejected. | Status | Option ID | | --- | --- | @@ -393,4 +393,3 @@ The dev/prod web backend protects every `/api/*` route with `x-mcp-remote-auth: 3. `sessionStorage` — backstop for navigations that land without either of the above. Injection is a no-op when auth is disabled (`DANGEROUSLY_OMIT_AUTH`), and the global name is the shared `INSPECTOR_API_TOKEN_GLOBAL` constant in `core/mcp/remote/constants.ts`. - diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index df3bda452..a767b405b 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -73,9 +73,9 @@ targets: | v1 | `v1/main` | `v1` | `v1-latest` | | v2 | `v2/main` | `v2` | `latest` | -- **v1** (`v1/main`) is the legacy Inspector — it takes **security fixes - only**, and is published straight from that branch to the `v1-latest` npm - tag (`npx @modelcontextprotocol/inspector@v1-latest`). +- **v1** (`v1/main`) is the legacy Inspector — it takes **security and bug + fixes only**, and is published straight from that branch to the `v1-latest` + npm tag (`npx @modelcontextprotocol/inspector@v1-latest`). - **v2** (`v2/main`) is where all current work happens — when in doubt, target v2. `v2/main` is the develop branch; it is merged into `main` at milestone releases, and `main` is what publishes the `latest` npm tag. Nothing targets From eebf9f2ae6e06bd93a746ede6518d92c4b475c66 Mon Sep 17 00:00:00 2001 From: cliffhall <cliff@futurescale.com> Date: Fri, 31 Jul 2026 20:33:13 -0400 Subject: [PATCH 5/7] docs: restore `inspector/` as the Project Structure tree root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tree is a filesystem layout — every child is a real directory (`clients/`, `core/`, `test-servers/`). Labeling its root `v2/main/` named a branch instead, which reads as a path that does not exist in a checkout and diverged from the identical tree in README.md, which still says `inspector/`. The branch model is documented in its own table under Repository & Project Boards; the tree does not need to restate it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 804a8ff27..c3cfe5e21 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,7 +5,7 @@ This is an application for inspecting MCP servers. Has three incarnations, Web, ## Project Structure ``` -v2/main/ +inspector/ ├── clients/ │ ├── web/ # Web client (Vite + React + Mantine) │ │ ├── src/ # Browser source (React app, hooks, components) From ee020ff251db4168f86bd88b09170adc4a184635 Mon Sep 17 00:00:00 2001 From: cliffhall <cliff@futurescale.com> Date: Fri, 31 Jul 2026 20:37:19 -0400 Subject: [PATCH 6/7] docs: settle the v1 policy wording as "security fixes only" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverses the direction the Copilot round took in 33cc0366. That commit normalized the six v1-policy sites onto "security and bug fixes" because that was the majority wording — but majority is the wrong tiebreak here. The policy is set by what we told contributors: the #1819 backlog close note, posted to 120 closed PRs, says "v1 will receive security fixes only", and the five PRs retained for 1.0.5 are all security fixes. So the two sites that already said "security fixes only" were the correct ones, and the four that said otherwise are now aligned to them: - AGENTS.md: the `v1` label description, the branch-role table, and the v1 board line. - README.md: the "Repo status" callout. CONTRIBUTORS.md and AGENTS.md's Project Status bullet already read correctly and are unchanged. This is the substance of #1813, which is why that issue's PR is now reduced to its PR-template-link half — the wording work landed here to avoid two PRs editing the same lines in opposite directions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU --- AGENTS.md | 17 +++++++++-------- CONTRIBUTORS.md | 6 +++--- README.md | 2 +- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c3cfe5e21..d093abdae 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,7 +5,7 @@ This is an application for inspecting MCP servers. Has three incarnations, Web, ## Project Structure ``` -inspector/ +v2/main/ ├── clients/ │ ├── web/ # Web client (Vite + React + Mantine) │ │ ├── src/ # Browser source (React app, hooks, components) @@ -123,9 +123,9 @@ If you've already built a change locally, share the **prompt** you used and scre **Every PR must reference an issue. No exceptions, regardless of who opens it.** The PR body's first line is `Closes #<ISSUE_NUMBER>` (see the [Issue-driven Work Style](#issue-driven-work-style) rules below). A PR with no linked issue has no board card, so the work is invisible to the project board and untracked — if you're about to open one and there's no issue yet, create the issue first. This holds for a maintainer's own one-line fix as much as for a feature. ## Project Status and Direction -* The v1/main branch currently contains the legacy version of the Inspector, which is in deprecated maintenance mode — **security and bug fixes only**, no new features. It is **published straight from the branch** to the `v1-latest` npm dist-tag — v1 releases never pass through `main`, and v1 PRs therefore target `v1/main` directly. +* The v1/main branch currently contains the legacy version of the Inspector, which we are creating security fixes for in deprecated maintenance mode. It is **published straight from the branch** to the `v1-latest` npm dist-tag — v1 releases never pass through `main`, and v1 PRs therefore target `v1/main` directly. -* The v2/main branch currently contains the new version of the Inspector, which is actively being developed and maintained. All new features, bug fixes, and refactors should be implemented in this branch. It acts as the **develop branch**: work accumulates here continuously and is merged into `main` at milestone releases. +* The v2/main branch currently contains the the new version of the Inspector, which is actively being developed and maintained. All new features, bug fixes, and refactors should be implemented in this branch. It acts as the **develop branch**: work accumulates here continuously and is merged into `main` at milestone releases. * The main branch is the default branch for the repo, and it currently points to the latest v2 release. It is not a development branch, and no new features or bug fixes should be implemented here. It is only used for releases of the v2 Inspector via merge from v2/main, which is what publishes the `latest` npm dist-tag. @@ -151,7 +151,7 @@ All work should be driven by items on the project board. - Before starting work, check the board for the relevant item. - **Every board item is a real GitHub issue.** Do not create draft items (board cards with no issue number). If you find work that needs tracking, create an actual issue and add that to the board. Before creating a new issue, check the board for a matching item to avoid duplicates — **never create a duplicate**. - **Label by version — every issue and every PR, no exceptions.** Each one carries **exactly one** of `v1` or `v2` at creation. There is no unlabeled state and no "decide later": an issue with neither label belongs to no version line, appears in no version-filtered query, and is effectively invisible. - - `v1` — work targeting `v1/main` (the deprecated line: security and bug fixes only) + - `v1` — work targeting `v1/main` (the deprecated line: security fixes only) - `v2` — work targeting `v2/main` (active development; the default for anything new) Set the label at **create time** — `gh issue create --label v2 ...`, `gh pr create --label v2 ...` — never by backfilling later, since unlabeled items are exactly the ones missed when filtering by version. **If the target version isn't obvious, it's `v2`**: v2 is where all new work goes, and `v1` is reserved for the narrow case of patching the deprecated line. Only ask when the issue is specifically a fix *for released v1 behavior* and it's unclear whether v2 still has the bug. Note the label is a repo tag and is **not** the board — see the callout above; a `v2` issue also needs a board card with a Status. @@ -165,7 +165,7 @@ All work should be driven by items on the project board. ``` 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 — if a sub-task must ship with its parent, they belong in the same one. -- When work begins, create a feature branch and set the item's Status to **In Progress**. +- When work begins, create a feature branch and set the item's Status to **In Progress** (or **V2 Go Live** for a card in the go-live phases, #1804). - **Branch names start with the target version segment.** The first path segment must be the version whose base branch the PR targets — `v2/` for work on `v2/main`, `v1/` for work on `v1/main` — followed by the usual type and slug: `v2/ci/restore-claude-workflow`, `v2/fix/oauth-scope-union`, `v1/fix/proxy-ssrf-pin`. Not `ci/restore-claude-workflow`. 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`). - When work is complete: - Run `npm run ci` from the root — the mandatory pre-push gate (see [Mandatory pre-push gate](#mandatory-pre-push-gate)). `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. @@ -184,12 +184,12 @@ All work should be driven by items on the project board. | --- | --- | --- | --- | | `v2/main` | **Develop.** All active v2 work lands here. | **Yes** — every v2 PR | nothing directly; reaches npm via `main` | | `main` | **Release.** The repo's default branch; holds the latest released v2. Not a development branch. | **No** — it only receives milestone merges from `v2/main` | `latest` | - | `v1/main` | **Maintenance.** The deprecated v1 line, security and bug fixes only, no active development. | **Yes** — every v1 PR, directly | `v1-latest`, published straight from this branch | + | `v1/main` | **Maintenance.** The deprecated v1 line, security fixes only, no active development. | **Yes** — every v1 PR, directly | `v1-latest`, published straight from this branch | So v2 flows `feature branch → v2/main → (milestone) main → npm latest`, while v1 is flat: `feature branch → v1/main → npm v1-latest`, with no merge into `main` at any point. The two lines are published independently under separate dist-tags, which is why a v1 fix does **not** need to be forward-ported to reach users on v1 (`npx @modelcontextprotocol/inspector@v1-latest`). - **Project Boards**: - v2 - https://github.com/orgs/modelcontextprotocol/projects/28 (active board — all new work goes here) - - v1 - https://github.com/orgs/modelcontextprotocol/projects/11 (legacy inspector version, no new activity except security and bug fixes) + - v1 - https://github.com/orgs/modelcontextprotocol/projects/11 (legacy inspector version, no new activity except security fixes) #### V2 board (#28) `gh` recipes @@ -205,7 +205,7 @@ gh project field-list 28 --owner modelcontextprotocol --format json \ | Project node ID | `PVT_kwDOCt2Azc4BJVxt` | | Status field ID | `PVTSSF_lADOCt2Azc4BJVxtzg5iI8c` | -Status option IDs (`--single-select-option-id`) — **last verified 2026-07-31**, when `V2 Go Live` was removed from the board along with the go-live phases it tracked. The four below are the complete current set; anything else (`V2 Go Live` `b3a6966e`, `SDK V2 + New Spec` `1bbb6f57`, `Building CLI / TUI / CORE` `4ac261ee`, `Building Web` `c28da89f`, `MCP Apps Extension` `73d0b807`) is a removed column whose id is now rejected. +Status option IDs (`--single-select-option-id`) — **last verified 2026-07-27**. | Status | Option ID | | --- | --- | @@ -393,3 +393,4 @@ The dev/prod web backend protects every `/api/*` route with `x-mcp-remote-auth: 3. `sessionStorage` — backstop for navigations that land without either of the above. Injection is a no-op when auth is disabled (`DANGEROUSLY_OMIT_AUTH`), and the global name is the shared `INSPECTOR_API_TOKEN_GLOBAL` constant in `core/mcp/remote/constants.ts`. + diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index a767b405b..df3bda452 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -73,9 +73,9 @@ targets: | v1 | `v1/main` | `v1` | `v1-latest` | | v2 | `v2/main` | `v2` | `latest` | -- **v1** (`v1/main`) is the legacy Inspector — it takes **security and bug - fixes only**, and is published straight from that branch to the `v1-latest` - npm tag (`npx @modelcontextprotocol/inspector@v1-latest`). +- **v1** (`v1/main`) is the legacy Inspector — it takes **security fixes + only**, and is published straight from that branch to the `v1-latest` npm + tag (`npx @modelcontextprotocol/inspector@v1-latest`). - **v2** (`v2/main`) is where all current work happens — when in doubt, target v2. `v2/main` is the develop branch; it is merged into `main` at milestone releases, and `main` is what publishes the `latest` npm tag. Nothing targets diff --git a/README.md b/README.md index 0613bbf77..74eb0ce88 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ npx @modelcontextprotocol/inspector --cli # CLI npx @modelcontextprotocol/inspector --tui # TUI ``` -> **Repo status.** This is the **v2** line of the Inspector. Active development happens on **`v2/main`** (the develop branch — all v2 PRs target it), which is merged into **`main`** at milestone releases; `main` is the default branch and holds the latest released v2, published to the npm `latest` tag. The legacy **v1** line lives on **`v1/main`** — security and bug fixes only, published straight from that branch to the npm `v1-latest` tag (`npx @modelcontextprotocol/inspector@v1-latest`). See [`AGENTS.md`](./AGENTS.md) for branch/board conventions. +> **Repo status.** This is the **v2** line of the Inspector. Active development happens on **`v2/main`** (the develop branch — all v2 PRs target it), which is merged into **`main`** at milestone releases; `main` is the default branch and holds the latest released v2, published to the npm `latest` tag. The legacy **v1** line lives on **`v1/main`** — security fixes only, published straight from that branch to the npm `v1-latest` tag (`npx @modelcontextprotocol/inspector@v1-latest`). See [`AGENTS.md`](./AGENTS.md) for branch/board conventions. ## Project layout From 7fecc28bd79ed3ad639f7e126481ee3ca8ccf52f Mon Sep 17 00:00:00 2001 From: cliffhall <cliff@futurescale.com> Date: Fri, 31 Jul 2026 22:53:57 -0400 Subject: [PATCH 7/7] docs: point the AGENTS.md policy link at CONTRIBUTING.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR rewrites the paragraph holding the only in-repo link to the contributing policy, so it owns that line. #1884 renames the file CONTRIBUTORS.md -> CONTRIBUTING.md; having #1884 also edit this line would guarantee a one-line conflict between the two. Writing the new filename here instead makes #1884 a pure `git mv` with no overlapping line, so neither PR conflicts with the other. Between this merging and #1884 merging, the link points at a filename that doesn't exist yet. That window is deliberate and bounded — this PR is first in the merge order (#1821) and #1884 is third. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index d093abdae..f94fd60c6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -116,7 +116,7 @@ After installing, `npm run build` builds all clients. The launcher scripts (`npm ## Contributing External contributions are accepted as **issues, not pull requests** — maintainers handle design and implementation through a prompt-driven workflow. -If you've already built a change locally, share the **prompt** you used and screenshots if applicable, not a diff. See [`CONTRIBUTORS.md`](./CONTRIBUTORS.md) for the full policy. +If you've already built a change locally, share the **prompt** you used and screenshots if applicable, not a diff. See [`CONTRIBUTING.md`](./CONTRIBUTING.md) for the full policy. **This applies to org members with write access too, not just outside contributors.** Having permission to push a branch is not authorization to open a PR. Pull requests against this repo are opened by the **repo maintainers** only. Anyone else — including organization members whose write access makes it technically possible — opens a **detailed issue** instead, and a maintainer takes it from there. A detailed issue means: the problem, how to reproduce it, the behavior you expected, and — if you've already prototyped a fix — the prompt you used and any screenshots, rather than a diff.