Review caught a second-order effect of the fix: making the leaked-timer net
global invalidated the rationale of a neighbouring rule.
"Do not hand-roll a bare MantineProvider — that reintroduces the leak class"
stopped being true the moment the net moved into setup.ts, which covers every
unit test regardless of how it renders. The rule itself is still right, so keep
it and replace the justification: a hand-rolled provider skips the project theme
and the helper's options and drifts from every other test. Both copies say
explicitly that the old reason no longer holds, so nobody re-derives it.
Mirrored into .github/copilot-instructions.md in the same PR, as AGENTS.md
requires for review-relevant guidance — it carried the identical stale claim.
That makes four places this one belief had propagated to. A fix can create stale
docs by making a previously-true statement false, which is easy to miss when
only the changed lines get read.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
Signed-off-by: cliffhall <cliff@futurescale.com>
Review follow-up on #1983. Copilot was right, and about a hazard this repo
had already proven and then documented away: the claim that ink's ">=19"
peer "keeps npm from hoisting it next to a React the Inspector couldn't
also use" is false. A consumer pinning React 19.0 satisfies ">=19" while
`^19.2.4` nests underneath, so ink renders through their React and the
bundle through ours — the #1952 crash, one level up, breaking TUI startup
rather than just its forms.
Bundling ink closes it (verified: with a `createRequire` banner for the
inlined CJS, since react-reconciler calls `require("react")` and
signal-exit@3 `require("assert")`), but costs ~1.4MB — react-reconciler
and yoga-layout come along. The root `react` range closes it for free
instead: `^19.0.0` is open to the whole major, so npm can satisfy our
React and a consumer's pinned React 19 with one copy, and an external ink
resolves the same copy the bundle does.
Verified against both consumer trees — the reporter's (React 18 + ink 5)
and a pinned React 19.0.0 + ink 6.8.0, which splits under `^19.2.4`.
That makes the range load-bearing while looking like an ordinary version
bump, so the tests pin it: the root range must equal ink's own declared
peer floor. They also assert each exempt package is both external and a
root dependency — external means consumers install it, the mirror image
of the inlined ones, which must not be root-declared.
The docs in all four places now say ink is exempt on cost and never on a
peer range, and name that claim as one we got wrong.
Closes#1952
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLYmVPVLdeYpE6aMTUsCks
Round 5 was right on all three counts. Read dcoapp/app/lib/dco.js this time
rather than the README, which is where the earlier errors came from:
const authors = [commit.author.name, commit.committer.name]
const emails = [commit.author.email, commit.committer.email]
if (isMerge || ...) continue // parents.length > 1
else if (author && author.type === 'Bot') continue
So, corrected:
- Scope. Merge commits and bot-authored commits are skipped, not gated. "Every
commit" overstated it and could prompt a needless history rewrite.
- Matching. The trailer may match the author OR the committer, not the author
alone. A cherry-picker's own signoff is accepted.
- Certification. The DCO expressly covers submitting work created by others
that you have the right to submit, so signing off a cherry-pick is legitimate.
My "never sign off someone else's commit" would have told a maintainer not to
do a correct thing. The real prohibition is narrower: never fabricate a
trailer in another person's name.
This also retracts the mechanical argument in bd6c1072 for dropping the hook —
it does satisfy the check, since you are the committer. The recommendation
against it stands on honest grounds now: the trailer is a certification, a hook
makes it for every commit automatically including work merely applied on
someone else's behalf, and it provably cannot tell the difference. A judgment
about deliberateness, not a claim that it breaks.
Mirror corrected to match.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
Signed-off-by: cliffhall <cliff@futurescale.com>
Round 4 was right that the hook signs with the configured identity while the
DCO check requires a trailer matching the commit *author*, and that the two
diverge on cherry-pick and `git commit -c`.
I tried the suggested author-aware guard and it does not work: inside
prepare-commit-msg, `git var GIT_AUTHOR_IDENT` returns the config identity, not
the preserved author. Verified end to end — cherry-picking Bob's commit as
Alice yields author=Bob, committer=Alice, `Signed-off-by: Alice`. That commit
both still fails the gate and certifies work Alice did not write, which is the
one thing the last bullet of this section forbids.
Three review rounds have now found three holes in this hook, each fix opening
the next. That is a wrong mechanism rather than an unfinished one, so the hook
is out. A subtly-broken hook in the file that governs how this repo is worked
on is worse than no hook.
The bullet now recommends `git commit -s` and names BOTH false automations —
`format.signOff` and the hook — recording the cherry-pick finding so the next
person doesn't rediscover it, and pointing anyone who wants automation at a
layer that actually knows the author. Mirror updated to match.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
Signed-off-by: cliffhall <cliff@futurescale.com>
`ink-form` and `ink-scroll-view` were externalized by the TUI bundler and
both declare a loose `react` peer (">=18"). Installed under a project that
has its own React 18, npm hoists them to that project's root — where the
peer is satisfied — while the Inspector's React 19 nests underneath. The
bundle then renders through React 19 while those two call hooks on React
18, whose dispatcher is null, so opening a tool test form (or any scroll
view) dies with "Cannot read properties of null (reading 'useState')".
Inline them instead: their `import "react"` is emitted into the bundle and
so resolves from the build directory exactly like the bundle's own, with
npm's placement no longer part of the decision. This also pins their
transitive deps to what this repo resolved — notably `ink-select-input@6`
via `overrides`, which npm ignores for a package installed as a dependency,
so consumers were getting the React-18-era v5.
`ink` stays external: it can't be bundled (its CJS `signal-exit@3` fails
ESM interop with `Dynamic require of "assert"`) and doesn't need to be —
its ">=19" peer keeps npm from hoisting it next to an unusable React.
Both packages are dropped from the root `dependencies`, since the tarball
now ships their code rather than having consumers install them.
`clients/tui/__tests__/tsupConfig.test.ts` is the durable guard: the
failure is invisible in this repo and in every smoke (a dev install has
one React) and only appears once the package is installed under another
project, so the invariant is asserted against the config instead — every
dependency declaring a `react` peer must be bundled unless exempted.
Closes#1952
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLYmVPVLdeYpE6aMTUsCks
Both review points were factually right and my original text was wrong.
1. `git config format.signOff true` — which I had recommended as the *better*
option — only defaults the -s flag for `git format-patch`. `git commit` never
reads it, and there is no `commit.signoff` equivalent. Verified all three in a
throwaway repo: format.signOff → no trailer; commit.signoff → no effect; a
prepare-commit-msg hook → works. Shipping that advice would have been worse
than saying nothing, since it looks like a fix and silently changes nothing.
Now recommends `git commit -s`, gives the verified hook for automation, and
names format.signOff explicitly as a trap — the name invites exactly the
mistake I made, so it is worth warning about rather than omitting.
2. The DCO app's empty remediation-commit flow needs
`allowRemediationCommits.individual`. This repo ships no DCO config, so it
runs with that disabled and the original unsigned commits keep failing. The
escape hatch I offered does not exist here; rewriting is the only repair, so
the text now says to coordinate before rewriting shared history instead.
Mirror updated with both corrections.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
Signed-off-by: cliffhall <cliff@futurescale.com>
The DCO check fails any PR with an unsigned commit, and the rule appeared
nowhere in the repo — not AGENTS.md, CONTRIBUTING.md, README.md, or the Copilot
mirror. A hard merge gate that is invisible until it fails, whose remedy is a
history rewrite, is exactly what AGENTS.md exists to capture. It blocked #1978.
Added to the "When work is complete" list beside the other PR-hygiene gates,
weighted toward prevention over recitation: `git config format.signOff true`
once per clone beats remembering `-s`, because the failure is only discovered
after pushing. The repair path (`git rebase --signoff` + `--force-with-lease`)
carries the rebase caveat and the empty-remediation-commit alternative for when
you are not the sole author, plus a note that the signoff is a DCO assertion in
the author's own name and is never applied on someone else's behalf.
Mirrored into .github/copilot-instructions.md in the same PR, as AGENTS.md
requires for PR-hygiene rules — two lines next to `Closes #N` and the version
label, since the mirror is read on every review and is a distillation, not a
copy.
Not added to CONTRIBUTING.md: external contributors file issues rather than
PRs, so DCO never reaches them, and documenting it there would muddy that.
Closes#1979
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
Signed-off-by: cliffhall <cliff@futurescale.com>
AGENTS.md requires review-relevant changes to land in the Copilot mirror in the
same PR, and the catalog-isolation rules are review-enforceable: a new web smoke
that spawns its own server, or a teardown that removes a work dir without first
awaiting stopChild, is something a reviewer should flag.
Mirrored as one line under "Gates and PR hygiene" rather than the AGENTS.md
paragraph. The mirror is read on every review and its own rule is "a
distillation, not a copy" — the mechanism (why the 409 appeared, why only the
catalog and not HOME is redirected, how the contract is tested) belongs in
AGENTS.md, while what a reviewer needs is the invariant and the two shapes that
violate it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review round 2, four findings, all factual errors in the docs added by
the previous commit:
- `yaml` is in the root `dependencies`, not `devDependencies`, and its only
importer is `test-servers/src/load-config.ts` — not `core/` or the root
tooling, as the `vitest.shared.mts` comment claimed.
- The rule as written told a reader to put anything reached through root-owned
code — `core/` included — in `devDependencies`. That is wrong and would break
the published package silently: the client builds externalize npm packages, so
a published install resolves them from the root manifest, where
devDependencies are absent. Placement (which manifest) and classification
(which section) are now stated as separate questions, the second following
from who consumes the package at runtime.
`yaml`'s runtime classification is left alone and called out instead: moving it
changes what ships, which is not a docs change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
Per the maintenance rules, a change to the dependency layout has to land in the
docs with it. Adds the placement rule in three places, at three altitudes:
- README (Setup): the rule plus the concrete drift it prevents.
- AGENTS.md: a "Dependency placement" section — the MCP SDK packages are root
only, the v1 SDK is not ours and must not become a dependency, and anything
reached solely through root-owned code with no manifest (`test-servers/src`,
`core/`) is a root devDependency aliased to the repo root in
`vitest.shared.mts`.
- .github/copilot-instructions.md: the distilled, reviewer-citable form, so a
diff re-adding one of these to a client manifest is flagged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
All four suppressed comments from Copilot's third pass of #1962. Each was
correct.
The candidate boundary omitted `vitest.shared.mts` — root-owned, imported by
every client's vitest config, and already treated as shared non-client source
by `verify:typecheck-coverage`. It imports only Node built-ins today, which
is exactly why the omission would have gone unnoticed until a third-party
import appeared there, resolved from the root, and skewed. Shared *files* are
now a first-class list beside the shared dirs.
The allowlist was a blanket exemption by name. Each entry's rationale
establishes that a patch/minor difference is benign, which is no evidence
that a React 18-vs-19 or Hono 4-vs-5 split would be — that is a different
type surface. Being listed now tolerates skew only *within a major version*;
a cross-major skew fails like anything else. Deriving the major from the
lockfile version keeps this automatic, with no per-package range to maintain
and bump. A version that can't be parsed is treated as "cannot prove same
major" and fails rather than passes.
`verify-format-coverage.mjs`'s comment overstated the vouching graph: the
three guards do not each check the others. This one checks both siblings;
each sibling checks only this one. Described as it actually works.
The AGENTS.md rule said to bump a shared dependency in "root + all four
clients", but launcher declares no zod and `findSkew` ignores installs where
a package is absent — so the rule as written asked for spurious dependencies.
Reworded to every install that *declares* it, and mirrored in
`.github/copilot-instructions.md`.
The failure output follows: it no longer advises adding a package that is
already allowlisted, saying "MAJOR skew" instead, and it names all three
shared sources rather than the two dirs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
Closes#1896
`clients/web`'s `tsc -b` exhausted the 4GB default heap at zod 4.4.x. The
cause is not a zod regression nor a pathological inference site of ours: it
is two copies of zod at *mismatched versions* inside one tsc program.
`clients/web/tsconfig.test.json` compiles `test-servers/src`, whose files
resolve zod (and the SDK's zod) from the **root** install, alongside web's
own sources, which resolve from `clients/web/node_modules`. At 4.3.6 vs
4.4.3 TypeScript must relate two structurally-distinct declarations of every
`@modelcontextprotocol/*` schema type, which is exponential over a recursive
generic surface — `TS2589 Type instantiation is excessively deep`, then OOM.
Bumping root zod to 4.4.3 and changing nothing else returns the build to
baseline: 11.4s and 2,567,882 instantiations, against 11.2s / 2,567,503 at
4.3.6. Two copies at the same version are harmless; skew is what explodes.
So drop the `~4.3.6` hold on `clients/web` and move all four manifests and
lockfiles to zod 4.4.3 together, and add `verify:dep-lockstep` to `validate`
to keep them there. The guard derives its candidate set from what `core/`
and `test-servers/src` import, compares the committed lockfiles' top-level
entries, and fails deny-by-default on any skew not in the annotated
`TOLERATED_SKEW` allowlist. It joins the existing mutual-vouch ring so
dropping it from `validate` is caught by `verify:format-coverage`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
* chore(deps): migrate to eslint 10 to clear the brace-expansion DoS advisory (#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.
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: state useValueChange's referential-stability requirement (#1838)
Copilot review on #1900: the hook compares with `Object.is`, so an object or
array literal rebuilt in the component body compares unequal every render.
Because the callback is what updates state, that is not merely a wasted render —
it is an infinite loop.
Every current caller passes a primitive (`boolean`, `string`) or an already-
memoized value, so nothing changes behaviorally. But the hazard was implicit,
and the next caller is the one it would bite.
Documented on the hook itself, and one clause added to the AGENTS.md rule with
its mirror in `.github/copilot-instructions.md`. No runtime guard: the
requirement is the same one a `useEffect` dependency array carries, and here the
failure is loud and immediate rather than silent.
`npm run ci` passes.
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): stop asserting the --tool-args-json catch binding is an Error (#1838)
Copilot review round 2 on #1900: the `--tool-args-json` parse handler formatted
its message with `(e as Error).message`. A `catch` binding is `unknown`, so that
cast is an assertion rather than a check — a thrown non-Error would render
`--tool-args-json is not valid JSON: undefined`, which points the user at
nothing.
`JSON.parse` does only throw `SyntaxError` today, so this is not a live bug. But
the cast is the kind AGENTS.md asks us not to leave unjustified, the same
handler was already being touched here to add `{ cause: e }`, and every other
site in this PR uses the guarded form. Now it does too.
`npm run ci` passes.
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: require useValueChange's onChange to be render-pure (#1838)
Copilot review round 3 on #1900: `onChange` is called during render, so it is
bound by render-phase purity — and the web client mounts under `StrictMode`,
which deliberately double-renders in development. A callback doing anything
external would run an unpredictable number of times, and concurrent React can
abandon an in-progress render entirely.
Every current caller already passes a callback that only calls `setState`, so
nothing changes. But "runs during render" is the whole point of the hook and the
constraint that follows from it was left implicit.
Stated on the hook, with the same clause in the AGENTS.md rule and its
`.github/copilot-instructions.md` mirror. All three now 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` passes.
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: correct post-swap branch model, PR policy, and stale gate/tree entries
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
* docs: require a milestone on every new issue, defaulting to the current one
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 <title> ...`).
- 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
* docs: add .github/copilot-instructions.md and require mirroring into it
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
* docs: address Copilot review — v1 policy wording, stale V2 Go Live, typo
- 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
* docs: restore `inspector/` as the Project Structure tree root
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
* docs: settle the v1 policy wording as "security fixes only"
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
* docs: point the AGENTS.md policy link at CONTRIBUTING.md
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
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cliff Hall