diff --git a/.claude/skills/local-dev/SKILL.md b/.claude/skills/local-dev/SKILL.md index 36f6d43f..2a1462ad 100644 --- a/.claude/skills/local-dev/SKILL.md +++ b/.claude/skills/local-dev/SKILL.md @@ -118,11 +118,16 @@ which is the point. A lint or format tool that differs per client makes the gate verdict a function of *where you ran it*, and the exact `prettier` pin (#1790) only means something when there is one of it. -⚠️ The line is **who uses it**, not what kind of thing it is. `tsup`, `tsx`, -`vite`, `vite-node`, `playwright`, `storybook`, `happy-dom`, -`ink-testing-library` and each client's own `@types/*` are toolchain too and stay -where they are, because only one client runs them. Hoisting those would make -every client install the union of all four. +⚠️ The line is **used by every client**, not "used by one" and not "is it +toolchain". `tsx`, `playwright`, `storybook`, `happy-dom`, `ink-testing-library`, +`vite-node` and each client's own `@types/*` are toolchain too and stay where +they are — hoisting them would make every client install the union of all four. +So do the ones **more than one** client declares without all of them doing so: +`tsup` sits in web, cli and tui, and `vite` in web and tui on top of the root +*runtime* `dependency` that `--web --dev` needs. Neither is in scope here; +whether to consolidate them is a separate call with a separate rationale (`vite` +especially, since its root declaration is a `dependency`, not a +`devDependency`). #### What the walk-up does *not* buy you @@ -145,8 +150,21 @@ rather than assuming — the assumption is what made the first cut of #2196 clai more than it delivered (Copilot). So the consolidation buys **one declaration and one place to bump**, not one copy -on disk. What has to hold is that the copies *agree*, and -`npm run verify:dep-lockstep` is the guard for exactly that (see below). +on disk. Each surviving copy is pinned by its holder's peer range rather than by +a second declaration of ours, which is why they agree today. + +⚠️ **Nothing gates that automatically, and `verify:dep-lockstep` is not it.** +That guard derives its candidate set from what each `tsc` **program** resolves +(see below), so it sees only packages a program loads from two installs. A tool +*binary* — `eslint`, `prettier`, `vitest` — never enters a program, so it is +outside the candidate set no matter how far it drifts, and cli's `@types/node` +`24.13.1` against the root's `24.13.3` is a live skew that passes for the same +reason (no one program sees both). When you change what a client declares, check +by hand from that client: + +```sh +cd clients/web && npm exec -- which eslint prettier tsc vitest +``` #### Why the vitest trio is pinned exactly diff --git a/AGENTS.md b/AGENTS.md index 7152cc1b..7e93cad2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -87,8 +87,8 @@ The reasoning behind each of these, and what breaks when it is ignored, is the - **The MCP SDK packages** — `@modelcontextprotocol/client`, `core`, `server`, `server-legacy`, `ext-apps` — **are declared in the repo-root `package.json` and nowhere else.** So is anything reached only through root-owned code with no manifest of its own (`test-servers/src`, `core/`), aliased to the repo root in `vitest.shared.mts`. The v1 SDK (`@modelcontextprotocol/sdk`) is **not** a dependency of this repo and must not become one. - **`dependencies` vs `devDependencies` follows from who consumes it at runtime**, not from where it is declared. Anything `core/` imports at runtime must be a root **`dependency`** — the client builds externalize npm packages and a published install resolves them from the root manifest, where devDependencies are absent. -- **The shared toolchain is declared once, at the repo root, and in no client manifest.** `eslint`, `@eslint/js`, `typescript-eslint`, `globals`, `prettier`, `typescript`, `vitest`, `@vitest/coverage-v8` and `@types/node` are used by every client's own scripts, and a client that declares none of them still resolves the root copy by walk-up — `npm run` puts each ancestor `node_modules/.bin` on `PATH`, and Node and TypeScript walk parent `node_modules` / `node_modules/@types` the same way. `clients/launcher` declares no `devDependencies` at all and its `validate` is unchanged. A client-side declaration buys nothing and installs a second copy free to drift, as `globals` (`^17.7.0` root / `^17.4.0` clients) and `typescript-eslint` (`^8.65.0` / `^8.56.1`) had before #2196. These stay **`devDependencies`** — none is consumed at runtime and the tarball ships only each client's `build/`. A tool **one** client runs stays in that client (`tsup`, `tsx`, `vite`, `vite-node`, `playwright`, `storybook`, `happy-dom`, `ink-testing-library`, and the `@types/*` for its own libraries). - - ⚠️ **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; what must hold is that the copies **agree**, and `npm run verify:dep-lockstep` is the guard that says so. +- **The shared toolchain is declared once, at the repo root, and in no client manifest.** `eslint`, `@eslint/js`, `typescript-eslint`, `globals`, `prettier`, `typescript`, `vitest`, `@vitest/coverage-v8` and `@types/node` are used by every client's own scripts, and a client that declares none of them still resolves the root copy by walk-up — `npm run` puts each ancestor `node_modules/.bin` on `PATH`, and Node and TypeScript walk parent `node_modules` / `node_modules/@types` the same way. `clients/launcher` declares no `devDependencies` at all and its `validate` is unchanged. A client-side declaration buys nothing and installs a second copy free to drift, as `globals` (`^17.7.0` root / `^17.4.0` clients) and `typescript-eslint` (`^8.65.0` / `^8.56.1`) had before #2196. These stay **`devDependencies`** — none is consumed at runtime and the tarball ships only each client's `build/`. The boundary is **used by every client**, not "used by one": anything narrower stays where it is, whether one client declares it (`tsx`, `playwright`, `storybook`, `happy-dom`, `ink-testing-library`, `vite-node`, each client's own `@types/*`) or several do — `tsup` is declared in web, cli and tui, and `vite` in web and tui on top of the root **runtime** `dependency` that `--web --dev` needs. Those are out of scope here; consolidating them is a different call with a different rationale. + - ⚠️ **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. What keeps the surviving copies aligned is that each is pinned by its holder's peer range rather than by a second declaration of ours — and **nothing gates it automatically**. ⚠️ `verify:dep-lockstep` is *not* that gate: it compares only packages that one `tsc` **program** loads from two installs, so a stray `eslint`, `prettier` or `vitest` binary is outside its candidate set entirely, and the `@types/node` patch difference between cli (`24.13.1`) and the root (`24.13.3`) is real and unreported for exactly that reason. Check a tool copy by hand — `npm exec -- which eslint` from the client — when you change what a client declares. - ⚠️ **`vitest`, `@vitest/coverage-v8` and web's `@vitest/browser-playwright` are pinned exactly, and move together.** `@vitest/browser-playwright` declares an **exact** peer on `vitest`, so it — not the root range — decides which `vitest` web installs. Left to float, the root resolves a newer patch and web's tests then run on one `vitest` while loading a coverage provider built against another. Bumping means editing all three in one change, the same discipline the exact `prettier` pin (#1790) exists for. - **A root-declared package that `core/` imports at runtime must also be named in all three bundler `external` lists** (`clients/{cli,tui}/tsup.config.ts`, `clients/web/tsup.runner.config.ts`), since which client reaches it is a function of what `core/` imports rather than of what the client's own code names. `npm run verify:bundle-externals` enforces this against the **built output**. - **A dependency that renders React components must be bundled** into the client that uses it (`noExternal`) and declared only there — an externalized one resolves its own `react` and splits the tree. `ink` is the single exemption, on cost, and it is only safe while the root `react` range stays open to the whole major (`^19.0.0`).