chore(deps): migrate to eslint 10 to clear the brace-expansion DoS advisory

[cliffhall]

Closes #1838

eslint 9 pulled minimatch@3 → brace-expansion@1.1.16, which carries
GHSA-mh99-v99m-4gvg (high, DoS via unbounded expansion length). It appeared in
all five lockfiles — the root plus each of the four clients, none of which is an
npm workspace, so all five had to move together.

Deferred from the pre-2.0.0 sweep because it is dev-scope, not attacker-
controlled, and a major bump across five packages was the wrong thing to attempt
immediately before an irreversible publish. This is that work.

What moved

• eslint ^9.39.4/5 → ^10.8.0 and @eslint/js → ^10.0.1, in all five
  manifests.
• eslint-plugin-react-hooks ^7.0.1 → ^7.1.1 in web and tui. Required, not
  cosmetic: 7.0.1's peer range stops at ^9.0.0, so installing it alongside
  eslint 10 fails ERESOLVE.
• Everything else already declared eslint 10 support and stayed put:
  typescript-eslint@^8.56.1 (^8.57 || ^9 || ^10),
  eslint-plugin-react-refresh@^0.5.2 (^9 || ^10),
  eslint-plugin-storybook@^10.2.19 (>=8).

Config migration

None was needed. All five configs were already flat (eslint.config.js with
defineConfig/globalIgnores from eslint/config), which is the format v10
keeps and the only one it still supports. The root config's core/ and shared-
surface blocks — including the _-prefix argsIgnorePattern /
varsIgnorePattern / caughtErrorsIgnorePattern — are unchanged and still gate
lint:core and lint:shared.

No rule was disabled, downgraded, or scoped away to make the new eslint pass.
v10 surfaced 21 real violations across three rules; all 21 are fixed in the code.

preserve-caught-error (new in eslint:recommended) — 15 sites

A throw new Error(msg) inside a catch silently drops the original error.
Each now passes { cause }, so the underlying failure survives into the chain.
Messages are unchanged, so nothing that asserts on them moves. 12 in core/,
2 in clients/cli, 1 in test-servers/src.

no-useless-assignment (new in eslint:recommended) — 2 sites

clients/cli/src/cli.ts's optionArgs and test-servers/src/test-server-oauth
's valid were initialized and then unconditionally overwritten on every path.
Both are now declared without the dead initializer; TypeScript's definite-
assignment analysis covers the remaining paths.

react-hooks/set-state-in-effect — 8 sites (web)

These were false negatives the plugin fixed in 7.1.0, not a rule we newly opted
into: set-state-in-effect has been error in flat/recommended since 7.0.1,
and the web config has always extended it. Every one was the same anti-pattern —
resetting or re-syncing local state from a prop inside a useEffect, which
paints the stale value for one frame and then renders again to correct it.

All eight now use a new useValueChange(value, onChange) hook
(clients/web/src/hooks/useValueChange.ts), which is React's documented
"adjusting state during render": compare against the previous render's value
with Object.is and call back during render, so React discards the
in-progress output before it reaches the DOM.

• MrtrConversation, ProtocolEntry, TaskCard — mirror isListExpanded.
• NetworkEntry — same mirror, plus the "Reveal in Network" force-open. The
  reveal's rAF scroll stays an effect (real external-system work); only the
  setIsExpanded(true) moved. isExpanded is now seeded from
  isListExpanded || revealed, since a render-time sync fires on change and so
  cannot cover an entry that mounts already revealed.
• PromptArgumentsForm, ResourceTemplatePanel — reset on template/prompt
  switch.
• ServerConfigModal — reset on open. Keyed on opened ? initial : undefined,
  which collapses "opened flipped" and "initial changed while open" into one
  value; initial is already memoized, so it cannot thrash.

useValueChange has its own test file, and ServerConfigModal gains an
open→edit→close→reopen test (the close leg was previously untested, which is
what left the new guard uncovered).

Advisory delta

npm audit, per package, before → after:

Package	before	after
root	9 (1 low, 6 mod, 2 high)	8 (1 low, 6 mod, 1 high)
clients/web	19 (3 low, 6 mod, 6 high, 4 crit)	17 (3 low, 6 mod, 4 high, 4 crit)
clients/cli	6 (2 low, 4 high)	4 (2 low, 2 high)
clients/tui	4 (1 low, 3 high)	2 (1 low, 1 high)
clients/launcher	3 (3 high)	1 (1 high)

brace-expansion is gone from all five — eslint@10 resolves minimatch@10 →
brace-expansion@5.0.9. Dropping @eslint/eslintrc (removed in v10) also took
js-yaml with it, clearing GHSA-52cp-r559-cp3m (high) from all four clients as
a bonus. The remainder are unrelated and tracked separately.

npm run ci passes end to end — validate, the per-file ≥90 coverage gate, the
build gate, all five smokes (including smoke:tui, run locally on a real TTY),
and 462 Storybook tests.

Also documents the effect rule in AGENTS.md and mirrors it into
.github/copilot-instructions.md, since it is now something a reviewer cites.

Test plan

npm run ci from the repo root, green end to end:

• validate — format-coverage + typecheck-coverage guards, test:scripts, the
  core//shared lint gate, then all four clients' format:check + lint +
  typecheck + build + unit tests.
• coverage — the per-file ≥90 gate on all four dimensions, web (unit +
  integration), cli, tui, launcher.
• verify:build-gate — the browser-externalized-builtin build gate.
• smoke — launcher dispatch, cli over stdio, tui (real TTY, local), prod web,
  and the headless-Chromium boot smoke.
• Storybook play functions — 109 files, 462 tests.

Lint was also run per surface to confirm each one is clean under eslint 10 and
not merely unreachable: lint:core, lint:shared, and eslint . in each of
web, cli, tui, and launcher.

No UI behavior changes, so no screenshots — the eight useValueChange
conversions are render-timing refactors of existing resets, covered by the
existing component tests plus the two added above.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU

[Copilot]

Pull request overview

This PR migrates the repo’s dev tooling from ESLint 9 → ESLint 10 across the root and all four client packages to eliminate the brace-expansion@1.x dependency advisory, while also fixing newly-surfaced lint violations (notably preserving caught errors and removing render-stale state syncing in React).

Changes:

• Bumped eslint/@eslint/js to v10 across the root and all client manifests + lockfiles (non-workspace layout).
• Preserved error context by attaching { cause } when rethrowing from catches in core/cli/test-server parsing and runtime paths.
• Replaced useEffect(() => setState(prop), [prop]) patterns in the web client with a new useValueChange hook, adding targeted tests and documenting the new rule in AGENTS.md + .github/copilot-instructions.md.

Reviewed changes

Copilot reviewed 28 out of 33 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test-servers/src/test-server-oauth.ts	Removes useless initializer by relying on definite assignment for valid.
test-servers/src/load-config.ts	Preserves parse failure context by rethrowing with { cause }.
package.json	Bumps root ESLint dependencies to v10.
package-lock.json	Updates root lockfile to reflect ESLint v10 dependency graph (including brace-expansion@5).
core/mcp/node/config.ts	Preserves configuration-load error context via { cause } in thrown Errors.
core/mcp/inspectorClient.ts	Preserves underlying failures for several runtime errors via { cause }.
core/mcp/import/serverJson.ts	Preserves JSON parse error context via { cause }.
core/mcp/import/clientConfig.ts	Preserves JSON parse error context via { cause }.
core/auth/utils.ts	Preserves URL parse error context via { cause }.
core/auth/node/runner-oauth-callback.ts	Preserves callback URL parse error context via { cause }.
core/auth/discovery.ts	Preserves MCP server URL parse error context via { cause }.
clients/web/src/hooks/useValueChange.ts	Adds render-time value-change hook for safe prop→state syncing without effects.
clients/web/src/hooks/useValueChange.test.tsx	Adds unit tests covering first render, changes, overrides, and Object.is semantics.
clients/web/src/components/groups/TaskCard/TaskCard.tsx	Replaces effect-based list expand syncing with useValueChange.
clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.tsx	Replaces effect-based form reset with useValueChange keyed by open state + memoized initial value.
clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.test.tsx	Adds coverage for open→edit→close→reopen reset behavior.
clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx	Replaces template-switch reset effect with useValueChange(uriTemplate, …).
clients/web/src/components/groups/ProtocolEntry/ProtocolEntry.tsx	Replaces effect-based expand syncing with useValueChange.
clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx	Replaces effect-based completion reset with useValueChange(name, …).
clients/web/src/components/groups/NetworkEntry/NetworkEntry.tsx	Replaces effect-based expand syncing with useValueChange, preserving reveal ordering and keeping scroll as an effect.
clients/web/src/components/groups/MrtrConversation/MrtrConversation.tsx	Replaces effect-based expand syncing with useValueChange.
clients/web/package.json	Bumps web ESLint deps to v10 and eslint-plugin-react-hooks to a v10-compatible peer range.
clients/web/package-lock.json	Updates web lockfile for ESLint v10 and related dependency graph changes.
clients/tui/package.json	Bumps tui ESLint deps to v10 and eslint-plugin-react-hooks for peer compatibility.
clients/tui/package-lock.json	Updates tui lockfile for ESLint v10 dependency graph.
clients/launcher/package.json	Bumps launcher ESLint deps to v10.
clients/launcher/package-lock.json	Updates launcher lockfile for ESLint v10 dependency graph.
clients/cli/src/handlers/run-method.ts	Preserves JSON parse failure context via { cause }.
clients/cli/src/cli.ts	Removes useless initializer for optionArgs and preserves JSON parse failure context via { cause }.
clients/cli/package.json	Bumps cli ESLint deps to v10.
clients/cli/package-lock.json	Updates cli lockfile for ESLint v10 dependency graph.
AGENTS.md	Documents the “no set-state-in-effect” rule and the useValueChange approach.
.github/copilot-instructions.md	Mirrors the state/effects guidance into the Copilot review distillation.

Files not reviewed (4)

• clients/cli/package-lock.json: Generated file
• clients/launcher/package-lock.json: Generated file
• clients/tui/package-lock.json: Generated file
• clients/web/package-lock.json: Generated file

[cliffhall]

Good catch — done in 8a39b0e.

You are right that this is worse than a normal stale-dependency mistake: because the callback is what updates state, an unstable value is an infinite render loop, not just a wasted render. The hook now says so explicitly, and the AGENTS.md rule (plus its copilot-instructions.md mirror) carries the same clause, so the guidance and the review checklist agree.

The recommendation is the one you suggested — pass a primitive key derived from the data (an id, a name, a URI) where one exists, otherwise something already memoized. Every caller in this PR already does: isListExpanded/revealed are booleans, name and uriTemplate are strings, and ServerConfigModal's opened ? initial : undefined keys off an initial that is already useMemod over [initialId, initialConfig, mode].

Deliberately not adding a runtime guard (e.g. a dev-mode "changed N renders in a row" warning): the stability requirement is the same one a useEffect dependency array carries and we do not guard those either, and the failure here is immediate and loud rather than silent, so a doc contract is proportionate.

[Copilot]

Pull request overview

Copilot reviewed 28 out of 33 changed files in this pull request and generated no new comments.

Files not reviewed (4)

• clients/cli/package-lock.json: Generated file
• clients/launcher/package-lock.json: Generated file
• clients/tui/package-lock.json: Generated file
• clients/web/package-lock.json: Generated file

Suppressed comments (1)

clients/cli/src/cli.ts:982

• In this JSON.parse error path, the message uses (e as Error).message. If JSON.parse throws a non-Error value, this can produce an unhelpful ...: undefined message. Since this block was touched (adding { cause: e }), it’s a good spot to switch to the safer e instanceof Error ? e.message : String(e) pattern (as used elsewhere in the PR).

    try {
      parsed = JSON.parse(options.toolArgsJson);
    } catch (e) {
      throw new Error(
        `--tool-args-json is not valid JSON: ${(e as Error).message}`,
        { cause: e },
      );

[cliffhall]

Round 2 responses.

Suppressed comment — clients/cli/src/cli.ts:982, (e as Error).message. Implemented in 530f8ef. Fair call and worth doing even though it is not a live bug: JSON.parse only throws SyntaxError today, but a catch binding is unknown, so the cast is an assertion rather than a check, and a thrown non-Error would have rendered --tool-args-json is not valid JSON: undefined — pointing the user at nothing. It is also exactly the unjustified as that AGENTS.md asks us not to leave lying around, the handler was already being touched here to add { cause: e }, and every other site in this PR uses the guarded e instanceof Error ? e.message : String(e) form. Now it does too.

One sibling was deliberately left alone: test-servers/src/modern-tasks.ts:414 has the same pattern, but this PR does not otherwise touch that file and the change is unrelated to the eslint migration.

Round 1 — useValueChange referential stability. Addressed in 8a39b0e, replied inline.

npm run ci re-run green after each change (validate, the per-file ≥90 coverage gate, build gate, all five smokes including smoke:tui on a real TTY, 462 Storybook tests).

[Copilot]

Pull request overview

Copilot reviewed 28 out of 33 changed files in this pull request and generated no new comments.

Files not reviewed (4)

• clients/cli/package-lock.json: Generated file
• clients/launcher/package-lock.json: Generated file
• clients/tui/package-lock.json: Generated file
• clients/web/package-lock.json: Generated file

Suppressed comments (1)

clients/web/src/hooks/useValueChange.ts:17

• useValueChange invokes onChange during render. With StrictMode enabled (clients/web/src/main.tsx:21-32), renders can be replayed/aborted in development, so a non-idempotent callback could fire more than once. The hook’s docstring should explicitly require that onChange be free of external side effects (and ideally only schedule React state updates).

 * This is React's documented "adjusting state during render" pattern
 * (https://react.dev/reference/react/useState#storing-information-from-previous-renders),
 * and it is the supported way to reset or re-sync local state from a prop.
 *

[cliffhall]

Round 3 response.

Suppressed comment — useValueChange and StrictMode replay. Implemented in ec390af. Correct and worth stating: onChange is called during render, so it inherits render-phase purity, and this app mounts under StrictMode (src/main.tsx), which double-renders in development on purpose — concurrent React can also abandon an in-progress render outright. A callback doing anything external would therefore run an unpredictable number of times.

Every current caller already passes a setState-only callback, so nothing changes behaviorally. The point is that "runs during render" is the whole mechanism of the hook and the constraint that follows from it was left implicit for the next caller.

The hook now spells out the prohibition (no fetches, DOM writes, logging, ref mutation, or parent callbacks), and the AGENTS.md rule and its copilot-instructions.md mirror carry the same clause. All three point at NetworkEntry as the worked example of the split: the reveal's force-open is a state update and lives in useValueChange, while its requestAnimationFrame scroll stays a useEffect.

npm run ci green again.

[Copilot]

Pull request overview

Copilot reviewed 28 out of 33 changed files in this pull request and generated no new comments.

Files not reviewed (4)

• clients/cli/package-lock.json: Generated file
• clients/launcher/package-lock.json: Generated file
• clients/tui/package-lock.json: Generated file
• clients/web/package-lock.json: Generated file

[cliffhall]

Merged v2/main in (e52a16b) to clear the conflicts #1899 introduced when it regenerated all four client lockfiles. No rebase — the repo squash-merges, so a merge commit here is harmless and the resolution was one pass.

How the package.json conflicts were resolved — a union of both PRs' intents:

• From chore(deps): untangle the vitest/Storybook exact-pin knot to clear GHSA-p63j-vcc4-9vmv #1899, preserved: vitest / @vitest/* 4.1.10, Storybook 10.5.5 (+ @chromatic-com/storybook 5.2.1), and the deliberate zod: "~4.3.6" hold on clients/web (4.4.3 OOMs web's tsc -b — chore(deps): zod 4.4 blows the tsc heap in clients/web — held at ~4.3.6 #1896). cli/tui/root keep their pre-existing ^4.3.6, resolving to 4.4.3 exactly as on v2/main.
• From this branch, preserved: eslint ^10.8.0 and @eslint/js ^10.0.1 across all four clients and the root.
• eslint-plugin-react-hooks → ^7.1.1, dropping chore(deps): untangle the vitest/Storybook exact-pin knot to clear GHSA-p63j-vcc4-9vmv #1899's ~7.0.1 hold.

That last one is the only judgement call, so calling it out explicitly: #1899 held the plugin at ~7.0.1 under #1897 because 7.1.1 enables react-hooks/set-state-in-effect and 8 pre-existing violations failed lint. This PR supersedes that hold on both counts — 7.0.1's peer range stops at eslint ^9.0.0 so it ERESOLVEs against eslint 10, and this PR already fixed all 8 violations via the new useValueChange hook. Restoring the hold would break the install outright. So #1897 is closed out here rather than carried forward.

Lockfiles were not hand-merged: they were reset to v2/main's regenerated versions and re-resolved with a root npm install (postinstall cascade). Verified resolution with npm ls per client.

Full npm run ci from the root passes end to end — validate, the per-file ≥90 coverage gate, verify:build-gate, every smoke (smoke:tui included, run locally with a real TTY), and the Storybook play functions (109 files / 462 tests).