Merge remote-tracking branch 'origin/v2/main' into v2/feat/2082-file-secret-store-lock

.github/copilot-instructions.md

@@ -105,6 +105,7 @@ Both exist and do different jobs. Theme files (`src/theme/<Component>.ts`) custo
 - **A dependency bump must land in every install that declares it.** v2 is not a workspace — the root and each `clients/*` have their own `node_modules`, and a client's test project compiles `core/` and `test-servers/src` (which resolve from the **root**) alongside the client's own sources. Bumping a shared dependency in one manifest only puts two versions of it in one `tsc` program; for a recursive-generic surface like zod that exhausts the tsc heap (#1896). `verify:dep-lockstep` fails the build on this — it derives its candidates from what each `tsc` program actually resolves (`tsc --listFilesOnly`, keeping packages that reach one program from two installs), so a package reached only through another package's `.d.ts` counts too (#1965) — so a PR bumping a package the shared sources pull in should update the root **and every client that already lists it** — not every client unconditionally, since a package absent from an install can't skew and adding it there would be a spurious dependency.
 - **A test or smoke must not touch real user state.** The web smokes run against a throwaway catalog via the shared `scripts/lib/prod-web-server.mjs` helper, never the developer's `~/.mcp-inspector/mcp.json` (#1977); the cli/tui smokes drive a temp `--catalog`. A new smoke spawning its own server, or teardown that removes a work dir without first awaiting `stopChild` (the #1801 race — `child-cleanup.mjs` exports both halves and both are required), should be flagged.
 - **Build output is never a gate target.** Lint, format, and typecheck read first-party source only; everything a build writes (`clients/*/build`, `clients/web/dist`, `storybook-static`, `coverage`, `test-servers/build`, `core/**/{build,dist}`, `*.tsbuildinfo`) stays out via each scope's `globalIgnores`, `format` globs, and tsconfig `include`. Gating generated code reports defects in vendored third-party source that nobody can fix, and a rule promotion turns that warning into a `validate` failure (#2043). Flag a PR that adds a build location without ignoring it in the same change, that widens an ignore to silence a finding in first-party code, or that adds a build directory to a tsconfig `include` to make a generated `.d.ts` resolve. Note the coverage guards don't catch this — they assert source is *covered*, not that output is *excluded*.
+- **Lint has no warning tier.** Every `lint` script runs `--max-warnings 0`, so a warning fails `validate` exactly as an error does (#2085) — a `warn`-level `react-hooks/exhaustive-deps` finding otherwise let a stale-closure bug pass the pre-push gate and reach review. Flag a PR that silences a finding to make the gate pass (widening a `globalIgnores`, dropping a rule, or an inline disable with no justification comment); the fix is the defect, not the message. A rule meant to be enforced should be set to `error` rather than left at `warn` and carried by the flag.
 - **Every PR references an issue**, first body line `Closes #<ISSUE_NUMBER>`.
 - **Every PR carries exactly one version label**, `v1` or `v2`, matching its base branch.
 - **Commits carry a `Signed-off-by:` trailer.** The DCO check is a hard merge gate and fails on a single unsigned commit; it matches the trailer against the author *or* committer, and skips only merge and bot commits. Use `git commit -s` — note `format.signOff` does *not* sign `git commit` (only `format-patch`). Repairing pushed commits means `git rebase HEAD~<n> --signoff` + `git push --force-with-lease`; remediation commits are not enabled on this repo.

AGENTS.md

@@ -816,6 +816,17 @@ Why it matters, given that these paths are all gitignored and the findings are u
 
 The two coverage guards do **not** catch this, and adding a third is not the fix. `verify:format-coverage` and `verify:typecheck-coverage` assert that first-party source is *covered*; neither asserts that generated output is *excluded* — an asymmetry that is deliberate, since a guard can't distinguish "generated" from "source" without being told, and the ignore lists are already that statement. So this class drifts silently and the check is a human one: **when a build starts writing to a new location, add it to that scope's ignore list in the same change.** The reverse of the guards' rule also holds — never widen an ignore to silence a finding in first-party code, and never add a build directory to a tsconfig `include` to make a generated `.d.ts` resolve (import the source, or fix the build's types).
 
+### Lint has no warning tier
+
+**Every `lint` script runs with `--max-warnings 0`, so a warning fails `validate` exactly as an error does (#2085).** All six scopes carry the flag — each of `clients/{web,cli,tui,launcher}`'s `eslint .`, plus the root's `lint:core` and `lint:shared`.
+
+This exists because the gate's promise — that passing `npm run ci` locally means CI's gates pass — was kept while a real bug walked through it. `react-hooks/exhaustive-deps` ships at `warn` in the recommended set, and two `useCallback`s in `App.tsx` omitted a non-stable `refresh` from their dependency arrays; ESLint printed the right message on both lines on every run, nothing consumed it, and the stale closure was caught only by a review round on #2076. It is the same argument [Build output is never a gate target](#build-output-is-never-a-gate-target) makes from the other direction: a channel nobody fails on is one people learn to skim.
+
+Two consequences worth stating:
+
+- **Do not silence a finding to satisfy the gate.** A warning is now a defect to fix. If a rule genuinely must be waived on a line, use its inline disable comment **with a one-line justification** — the same standard this document sets for `v8 ignore` and for `void` on a floating promise. Widening a `globalIgnores` or dropping a rule to make `lint` pass is not an acceptable fix.
+- **A rule left at `warn` still reads wrong in an editor.** The flag makes severity irrelevant to the *gate*, not to the developer looking at a squiggle. `react-hooks/exhaustive-deps` is therefore set to **`error`** in both React scopes (`clients/web`, `clients/tui`) rather than relying on the CLI flag alone. Prefer `error` for any rule you actually intend to enforce.
+
 ### Typescript instructions
 
 - Use TypeScript for all new code

README.md

@@ -378,6 +378,8 @@ Per-client scripts exist too (`validate:web`, `coverage:cli`, `smoke:tui`, …),
 
 **Linting is type-aware.** All five ESLint scopes (`clients/{web,cli,tui,launcher}` plus the root `core/` + shared gate) enable `@typescript-eslint/no-floating-promises` at `error`, so a promise that is neither awaited, returned, `.catch(…)`-terminated, nor explicitly discarded with `void` fails `lint` — and therefore `validate` ([#1959](https://github.com/modelcontextprotocol/inspector/issues/1959)). The rule needs type information, so each scope's config names a parser project; the root scope's is **`tsconfig.lint.json`**, a lint-only project covering `core/**`, `test-servers/src/**`, and `vitest.shared.mts`, which have no tsconfig of their own. It emits nothing and changes no typecheck — but a new first-party TS location added to the root lint scope must be added to its `include`. See **TypeScript instructions** in [`AGENTS.md`](./AGENTS.md) for when `void` is acceptable.
 
+**And lint has no warning tier.** Every `lint` script runs with `--max-warnings 0`, so a warning fails `validate` exactly as an error does ([#2085](https://github.com/modelcontextprotocol/inspector/issues/2085)) — a `warn`-level `react-hooks/exhaustive-deps` finding had otherwise let a stale-closure bug pass the mandatory pre-push gate and reach review. Fix the finding rather than silencing it; if a rule genuinely must be waived, use its inline disable comment with a one-line justification.
+
 For the full testing rules — the ≥90% per-file gate, where test files live, the unit vs. integration vs. storybook projects, and the `v8 ignore` policy — see [`AGENTS.md`](./AGENTS.md).
 
 ## Publishing

clients/cli/package.json

@@ -27,7 +27,7 @@
     "test:cli-headers": "vitest run headers.test.ts",
     "test:cli-metadata": "vitest run metadata.test.ts",
     "pretest": "npm run test-servers:build && npm run build",
-    "lint": "eslint .",
+    "lint": "eslint . --max-warnings 0",
     "format": "prettier --write src __tests__ \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"",
     "format:check": "prettier --check src __tests__ \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\""
   },

clients/launcher/package.json

@@ -14,7 +14,7 @@
   "scripts": {
     "build": "tsc",
     "postbuild": "node scripts/make-executable.js",
-    "lint": "eslint .",
+    "lint": "eslint . --max-warnings 0",
     "typecheck": "tsc --noEmit -p tsconfig.json && tsc --noEmit -p tsconfig.test.json",
     "format": "prettier --write src __tests__ scripts \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"",
     "format:check": "prettier --check src __tests__ scripts \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"",

clients/tui/eslint.config.js

@@ -27,7 +27,8 @@ export default defineConfig([
     },
     rules: {
       "react-hooks/rules-of-hooks": "error",
-      "react-hooks/exhaustive-deps": "warn",
+      // `error`, not `warn` — see the web config (#2085).
+      "react-hooks/exhaustive-deps": "error",
     },
   },
   {

clients/tui/package.json

@@ -22,7 +22,7 @@
     "validate": "npm run format:check && npm run lint && npm run typecheck && npm run build && npm run test",
     "test": "vitest run",
     "test:coverage": "vitest run --coverage",
-    "lint": "eslint .",
+    "lint": "eslint . --max-warnings 0",
     "format": "prettier --write src __tests__ \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"",
     "format:check": "prettier --check src __tests__ \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\""
   },

clients/web/eslint.config.js

@@ -27,6 +27,14 @@ export default defineConfig([
       ecmaVersion: 2020,
       globals: globals.browser,
     },
+    rules: {
+      // Promoted from the recommended set's `warn` (#2085). A stale-closure
+      // defect was reported correctly on every run and still reached review,
+      // because `lint` fails only on errors. `--max-warnings 0` now fails the
+      // gate either way; this makes the severity read correctly in an editor
+      // too, rather than depending on a CLI flag.
+      "react-hooks/exhaustive-deps": "error",
+    },
   },
   {
     // Test setup files re-export utilities and mix components with helpers — the

clients/web/package.json

@@ -25,7 +25,7 @@
     "test:integration": "npm run test-servers:build && vitest run --project=integration",
     "test:integration:watch": "npm run test-servers:build && vitest --project=integration",
     "test-servers:build": "tsc -p ../../test-servers --noCheck",
-    "lint": "eslint .",
+    "lint": "eslint . --max-warnings 0",
     "format": "prettier --write src server .storybook \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"",
     "format:check": "prettier --check src server .storybook \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"",
     "preview": "vite preview",

package.json

@@ -47,8 +47,8 @@
     "verify:format-coverage": "node scripts/verify-format-coverage.mjs",
     "verify:dep-lockstep": "node scripts/verify-dep-lockstep.mjs",
     "validate:core": "npm run format:check:core && npm run format:check:scripts && npm run format:check:shared && npm run lint:core && npm run lint:shared",
-    "lint:core": "eslint \"core/**/*.{ts,tsx}\"",
-    "lint:shared": "eslint \"test-servers/src/**/*.{ts,tsx,mts,cts}\" vitest.shared.mts eslint.config.js",
+    "lint:core": "eslint \"core/**/*.{ts,tsx}\" --max-warnings 0",
+    "lint:shared": "eslint \"test-servers/src/**/*.{ts,tsx,mts,cts}\" vitest.shared.mts eslint.config.js --max-warnings 0",
     "format:core": "prettier --write \"core/**/*.{ts,tsx}\"",
     "format:check:core": "prettier --check \"core/**/*.{ts,tsx}\"",
     "format:scripts": "prettier --write \"scripts/**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"",