chore: widen verify:dep-lockstep to declared cross-install copies

[cliffhall]

Closes #2226

clients/cli resolved @types/node 24.13.1 against the root's 24.13.3. Neither client declares it — #2196 consolidated it to the root — so the cli copy arrived transitively via @types/express, constrained by no range of ours. verify:dep-lockstep reported OK and was right to: it compares only packages that reach one tsc program from two installs, and no one program sees both copies.

The issue offers three options and notes that (2) fixes the instance while (3) stops the next one, and that they are not exclusive. This does both, and declines (1) — re-declaring @types/node in clients/cli would re-create the second declaration #2196 removed.

The class — a second guard tier

verify:dep-lockstep grows a second tier alongside the program-derived one, asking the weaker but broader question AGENTS.md's one version per install-crossing dependency rule actually states: does a package this repo declares anywhere resolve to two versions across our installs at all?

The candidate set is every name in any install's dependencies/devDependencies that more than one install holds a top-level copy of — 17 packages today. Nested copies are excluded on purpose: one exists because some dependency asked for a different version, so it is that dependency's range to govern, not ours.

Neither tier subsumes the other.

	program tier (#1965)	declared tier (this PR)
A copy no manifest names, arriving through another package's .d.ts	✅ @modelcontextprotocol/sdk	✗
A transitive copy no program loads	✗	✅ this issue
Two clients disagreeing, no root copy involved	✗ (no single program sees both)	✅ @types/react, web vs tui
Peer shadows AGENTS.md calls out as ungated (eslint, typescript, vitest)	✗	✅

Deny-by-default, with its own (empty) TOLERATED_DECLARED_SKEW and the same within-a-major rule — now stated once in a shared toleratesSkew so the two tiers cannot drift.

Two limits are stated rather than papered over: the tier reads lockfiles, so a tool binary installed by hand and never committed is still invisible; and it compares only names some manifest declares, so a purely transitive package no manifest names stays the program tier's business.

The instance, and three others the new tier surfaced

All four were patch-level:

package	was	mechanism
@types/node	cli 24.13.1 vs root 24.13.3	overrides entry in clients/cli — the repo's sanctioned lever for a transitive pin, at the root's own ^24.12.4 rather than a frozen number
@types/react	web 19.2.18 vs tui 19.2.17	npm update in tui
react	web 19.2.8 vs root/tui 19.2.7	npm update in root + tui
@vitejs/plugin-react	web 6.0.5 vs root 6.0.2	npm update in both

npm update rather than npm install <pkg>@<version>: it moves the lockfile within the declared range without widening a range, which matters for the root react ^19.0.0 that ink's externalized-bundle exemption depends on. No declaration or range changed — the only manifest edit in the diff is cli's overrides line.

Verified against the reported state: reverting cli's lockfile entry to 24.13.1 makes the guard exit 1 and name all three holders.

Done when

• clients/cli and the root resolve the same @types/node
• Something fails when they next diverge — the widened tier, plus the overrides pin
• npm run local:gate passes

Tests

• 4 end-to-end cases on main(). The fixture gains manifest dependencies and a client-side solo install, so the skew is real and no program holds both copies — the chore: clients/cli drifts to @types/node 24.13.1 via @types/express, and verify:dep-lockstep cannot see it #2226 shape exactly. Covers: skew with no program seeing it (fails), agreement (passes, counted), an installed package no manifest declares (not a candidate), a declared package only one install holds (cannot skew).
• 13 helper cases: declaredPackages (dev deps count, peer/optional don't), topLevelVersions (nested excluded), findDeclaredSkew (root-vs-client, client-vs-client, one holder, undeclared, sorting), partitionDeclaredSkew and toleratesSkew (deny-by-default, within-a-major only, unparseable fails), countDeclaredHeld.

Gate

npm run local:gate — every stage green. Two stages needed a re-run under machine contention from concurrent gate runs in other worktrees (load average 158): the web coverage timeouts were the documented 5s-timeout flake, with a different failing set each run and all of them passing in isolation, and one Storybook assertion (JsonObjectInput > Annotates The Offending Line) which passes 123/123 on a clean re-run. The diff touches no client source — git diff --name-only is AGENTS.md, three scripts/verify-dep-lockstep* files, cli's package.json, and four lockfiles.

Docs

AGENTS.md said this drift existed and that nothing caught it. Both sentences are updated to say what does now, and the "One version per install-crossing dependency" rule names both tiers.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J86a6vMcA9csPtdsg7ZcCZ

[Copilot]

🟡 Changes recommended

Two moderate coverage gaps and two documentation nits remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a second dependency lockstep tier to detect version skew across installations.

Changes:

• Detects declared-package skew across top-level installs.
• Aligns four dependency versions and pins CLI’s transitive @types/node.
• Adds helper/end-to-end tests and updates policy documentation.

Required changes:

• Moderate (2 votes): Include optionalDependencies in candidate detection and update tests.
• Moderate (2 votes): Adjust the solo fixture to test an undeclared transitive client copy.
• Nit (1 vote each): Update stale guidance in .claude/skills/local-dev/SKILL.md and docs/quality-gate.md.

File summaries

File	Description
scripts/verify-dep-lockstep.test.mjs	Tests declared-package skew helpers.
scripts/verify-dep-lockstep.mjs	Implements the second guard tier; optional dependencies are currently omitted.
scripts/verify-dep-lockstep.main.test.mjs	Adds end-to-end cases; the key transitive-copy fixture needs correction.
package-lock.json	Aligns React and Vite plugin versions.
clients/web/package-lock.json	Aligns the Vite React plugin.
clients/tui/package-lock.json	Aligns React and its types.
clients/cli/package.json	Overrides transitive @types/node.
clients/cli/package-lock.json	Aligns @types/node.
AGENTS.md	Documents the two-tier guard; related guidance remains stale.

Review details

Files not reviewed (3)

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

Suppressed comments (2)

AGENTS.md:95

• The new statement leaves the repository's dependency-troubleshooting skill contradictory: .claude/skills/local-dev/SKILL.md:217-223 still says that nothing gates peer/transitive copies and that verify:dep-lockstep cannot catch them. Update that section with the new second-tier behavior and remaining limits so loading the skill does not give stale guidance.

  - ⚠️ **Deleting the declaration does not always delete the copy, and the local copy still wins.** npm auto-installs an unmet **peer** into the install that needs it, and it has no visibility into the root's tree — so a client-only ESLint plugin drags a client-local `eslint` in (`eslint-plugin-react-refresh`/`-storybook` in web, `eslint-plugin-react-hooks` in tui), and web's Storybook/Vitest stack drags in a local `typescript` and `vitest`. A hoisted transitive does the same: `@types/express` puts an `@types/node` in web and cli. Those copies sit *nearer* than the root's and take precedence. The consolidation is therefore about **one declaration and one place to bump**, not about a single copy on disk. ⚠️ **Nothing keeps the surviving copies aligned, and nothing gates them.** A **peer** copy is at least constrained by its holder's peer range — tightly for `vitest` (an exact peer, hence the pin below), loosely for `eslint` (`^9 || ^10`), where the copies agree only because npm resolves the same latest in both installs. A **transitive** copy is constrained by nothing of ours at all, and cli's `@types/node` (`24.13.1` against the root's `24.13.3`) diverged on exactly that. **Since #2226 `verify:dep-lockstep` does catch this class** — its second tier compares every package any install *declares* against every top-level copy across all five installs, independent of what a `tsc` program loads, so a transitive drift and a peer shadow (`eslint`, `typescript`, `vitest`) are both in scope now. Two limits remain: the tier reads lockfiles, so a tool binary you installed by hand and never committed is still invisible; and it only compares names some manifest declares, so a purely transitive package no manifest names is out of scope in both tiers unless a `tsc` program loads both copies. Aligning a stale install is `npm update <pkg>` there; a transitive copy that will not move takes an `overrides` entry in that install (`clients/cli` pins `@types/node` this way).

scripts/verify-dep-lockstep.mjs:633

• The command's canonical documentation is now stale: docs/quality-gate.md:32 and docs/quality-gate.md:42 still describe verify:dep-lockstep as checking only dependencies that meet in one tsc program. Update that entry to cover this declared-package tier and its lockfile-based scope so developers can understand and remediate its new failures.

  // ---- Second tier: every DECLARED package held by more than one install.
  // Independent of what any `tsc` program loads, so it sees the transitive and
  // peer-shadow copies the program tier structurally cannot (#2226).

• Files reviewed: 5/9 changed files
• Comments generated: 2
• Review effort level: Balanced

[cliffhall]

Copilot review round 1 — all four addressed

Pushed as 8e2c4d4. Mirroring the inline replies here, since they go outdated once the fix lands, and covering the two nits, which arrived in the review body with no inline comment to reply to.

Moderate — include optionalDependencies in candidate detection. Accepted; the rationale I'd written was wrong rather than merely incomplete. I had grouped optionalDependencies with peerDependencies, but a peer range constrains the consumer's host while an optional range is a direct declaration npm attempts to install. declaredPackages now unions dependencies / devDependencies / optionalDependencies; peerDependencies stays out, with the reason — and why it is safe — in the doc comment. Tests updated on both sides.

Moderate — the solo fixture didn't exercise the defining case. Accepted, and this was the better of the two findings: declaring solo in both manifests meant a per-install implementation (intersect each install's own manifest with its own tree) would have passed the test while failing the actual #2226 shape. Only the root declares solo now; clients/web holds an undeclared top-level copy. Added the mirror case as well — declared only by a client, held by the root — which pins the @types/react shape a root-manifest-only candidate set would miss. The union is now pinned in both directions.

Nit — stale guidance in .claude/skills/local-dev/SKILL.md. Accepted. It carried "⚠️ Nothing gates either of those, and verify:dep-lockstep is not it." and described the cli @types/node drift in the present tense. Now states what the second tier covers (tool binaries no program loads, transitive copies no program meets, two clients disagreeing), the two limits that remain (it reads lockfiles, so an uncommitted hand-installed copy is invisible; it compares only declared names), and that the by-hand npm exec -- which check is still worth running because of those limits. The "why a skew is worth aligning" section now names both tiers and the npm update vs npm install <pkg>@<version> distinction.

Nit — stale guidance in docs/quality-gate.md. Accepted. Both the validate row's one-line summary and the verify:dep-lockstep row described the program tier only. The row now documents the second tier, its candidate set, why neither tier subsumes the other, and its two limits.

Frontmatter descriptions were not touched, so no skills:eval re-run is implied.

npm run local:gate passes end to end on this commit (exit 0, no re-runs).

[Copilot]

🔵 Needs a closer look

The guard’s documentation must accurately include optionalDependencies; minor documentation issues also remain.

Review details

Files not reviewed (3)

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

Suppressed comments (4)

AGENTS.md:95

• This paragraph still says “Nothing keeps the surviving copies aligned, and nothing gates them,” but later in the same paragraph says the new second tier gates exactly those copies. Update the earlier sentence to distinguish automatic alignment from detection (for example, nothing keeps them aligned automatically, but the guard now rejects drift) so the rule is not self-contradictory.

  - ⚠️ **Deleting the declaration does not always delete the copy, and the local copy still wins.** npm auto-installs an unmet **peer** into the install that needs it, and it has no visibility into the root's tree — so a client-only ESLint plugin drags a client-local `eslint` in (`eslint-plugin-react-refresh`/`-storybook` in web, `eslint-plugin-react-hooks` in tui), and web's Storybook/Vitest stack drags in a local `typescript` and `vitest`. A hoisted transitive does the same: `@types/express` puts an `@types/node` in web and cli. Those copies sit *nearer* than the root's and take precedence. The consolidation is therefore about **one declaration and one place to bump**, not about a single copy on disk. ⚠️ **Nothing keeps the surviving copies aligned, and nothing gates them.** A **peer** copy is at least constrained by its holder's peer range — tightly for `vitest` (an exact peer, hence the pin below), loosely for `eslint` (`^9 || ^10`), where the copies agree only because npm resolves the same latest in both installs. A **transitive** copy is constrained by nothing of ours at all, and cli's `@types/node` (`24.13.1` against the root's `24.13.3`) diverged on exactly that. **Since #2226 `verify:dep-lockstep` does catch this class** — its second tier compares every package any install *declares* against every top-level copy across all five installs, independent of what a `tsc` program loads, so a transitive drift and a peer shadow (`eslint`, `typescript`, `vitest`) are both in scope now. Two limits remain: the tier reads lockfiles, so a tool binary you installed by hand and never committed is still invisible; and it only compares names some manifest declares, so a purely transitive package no manifest names is out of scope in both tiers unless a `tsc` program loads both copies. Aligning a stale install is `npm update <pkg>` there; a transitive copy that will not move takes an `overrides` entry in that install (`clients/cli` pins `@types/node` this way).

docs/quality-gate.md:44

• This paragraph interrupts the Markdown table before the local:gate and pack:verify rows. Those rows will render as pipe-delimited text rather than as part of the table. Keep the verify:dep-lockstep summary within its table cell, or move this detailed explanation after the table so all script rows remain contiguous.

A **second tier** (#2226) runs alongside it, asking the weaker but broader question the `AGENTS.md` rule actually states: does a package this repo *declares* anywhere resolve to two versions across our installs at all? Its candidate set is every name in any install's `dependencies`/`devDependencies`/`optionalDependencies` — unioned across the root and all four clients, so a copy declared by only one of them still counts — that **more than one install holds a top-level copy of** (17 packages today). Nested copies are excluded: one exists because some dependency asked for a different version, so it is that dependency's range to govern, not ours. Neither tier subsumes the other — the program tier sees a copy no manifest names (`@modelcontextprotocol/sdk`, arriving through another package's `.d.ts`), while the declared tier sees a **transitive** copy no program loads (cli's `@types/node`, hoisted via `@types/express` — the case that motivated it), two **clients** disagreeing with no root copy involved (`@types/react`, web against tui), and the peer shadows `eslint`/`typescript`/`vitest` that never enter a program. Same deny-by-default and same within-a-major rule, against its own `TOLERATED_DECLARED_SKEW` — also empty. Two limits: it reads **lockfiles**, so an uncommitted hand-installed copy is invisible, and it compares only declared names, so a purely transitive package no manifest names stays the first tier's business.

scripts/verify-dep-lockstep.mjs:279

• The summary still names only two dependency fields even though the implementation and the detailed bullets now include optionalDependencies. Keep the doc comment aligned with the candidate set.

 * Every package name any install declares, across `dependencies` and
 * `devDependencies`.

scripts/verify-dep-lockstep.mjs:62

• The module-level description also omits optionalDependencies, so it documents a narrower candidate set than the implementation now enforces. Include the third field here as well.

// different versions across our installs at all? The candidate set is every name
// in any install's `dependencies`/`devDependencies` (root or client) that more
// than one install holds a top-level copy of — 17 packages today, against the

• Files reviewed: 7/11 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[cliffhall]

Copilot review round 2 — all four addressed

Pushed as 5daa3f5. All four arrived as suppressed comments with no inline thread, so they are answered here.

docs/quality-gate.md:44 — the paragraph broke the table. Correct, and this was the one real defect of the four: I had added the second-tier explanation as a paragraph between table rows, so everything below it — the local:gate and pack:verify rows — would have rendered as raw pipe-delimited text rather than as table rows. Folded back inside the verify:dep-lockstep cell; the table is contiguous again and the row ends exactly as it did before (no trailing pipe, matching its siblings).

scripts/verify-dep-lockstep.mjs:279 — declaredPackages summary named two fields. Fixed; it names all three. The detailed bullets below it were already right, which is exactly the drift worth catching — a summary line is what most readers stop at.

scripts/verify-dep-lockstep.mjs:62 — module header named two fields. Fixed the same way, and it now also states that peerDependencies is the one excluded field and points at declaredPackages for why, so the header describes the candidate set completely rather than approximately.

AGENTS.md:95 — self-contradictory paragraph. Agreed, and your framing is the right fix: the paragraph was conflating alignment with detection. It now reads "Nothing keeps the surviving copies aligned automatically — but since #2226 the guard rejects the drift", and the later sentence leads with "That is detection, not alignment … you still do the bump by hand." Took the opportunity to name the three fields there too, since that paragraph is where someone reading the dependency rules will look.

npm run local:gate passes end to end on this commit (exit 0, no re-runs).

[Copilot]

🔵 Needs a closer look

The repository-wide validation change warrants final human review, and two documentation inaccuracies remain.

Review details

Files not reviewed (3)

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

Suppressed comments (2)

.claude/skills/local-dev/SKILL.md:130

• This documentation update leaves .claude/skills/local-dev/SKILL.md:150 stale: it still says web's React 19.2.8 is "against the root's 19.2.7", while this PR aligns the root to 19.2.8. Remove the volatile version comparison so the skill no longer describes a skew that this PR fixes.

the opportunity.

scripts/verify-dep-lockstep.mjs:288

• The claim that all four surfaced skews were devDependencies is inaccurate: react is declared only as a root runtime dependency, while its client copies are peer-installed. The field should still be included, but describe this as three of the four skews instead.

 *   • `devDependencies` — the `@types/*` and toolchain case #2226 is about; every
 *     one of the four skews this tier first surfaced was a devDependency
 *     somewhere.

• Files reviewed: 7/11 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[cliffhall]

Copilot review round 3 — both addressed

Pushed as 95633a2. Both arrived suppressed with no inline thread.

.claude/skills/local-dev/SKILL.md:150 — stale version pair. Correct, and a nice catch given this PR is the thing that made it stale: the react/react-dom exception cited web's 19.2.8 "against the root's 19.2.7", a skew this PR removes. Took your suggested fix and dropped the comparison outright rather than updating the numbers — the paragraph's actual point is which install resolves the pair together, and that stands without a version that goes stale on the next npm update. This is the same failure mode the round-2 AGENTS.md finding was about: prose asserting a live version fact.

scripts/verify-dep-lockstep.mjs:288 — "all four skews were devDependencies" is wrong. You're right. react is declared only as a root runtime dependency, and its web/tui copies are peer-installed rather than declared at all. So it is three of the four (@types/node, @types/react, @vitejs/plugin-react).

Worth more than a number fix, so I rewrote the bullet rather than just changing "four" to "three": react being the exception is the clearest demonstration that the union has to span fields as well as installs — a devDependencies-only or dependencies-only candidate set would have missed it — so the bullet now names the three and then names react as the case that motivates the wider union.

npm run local:gate: every stage green. The Storybook stage needed one re-run under contention from a concurrent gate in another worktree (load average 97, a different story failing each time); it passes 123/123 on the quiet machine, and this commit changes two prose lines in a comment and a Markdown file.

[Copilot]

🟢 Approval recommended

The reviewed changes have no unresolved approval-blocking issues.

Review details

Files not reviewed (3)

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

• Files reviewed: 7/11 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

The reviewed changes have comprehensive coverage and no unresolved issues.

Review details

Files not reviewed (3)

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

• Files reviewed: 7/11 changed files
• Comments generated: 0 new
• Review effort level: Balanced