diff --git a/.cursor/rules/dependency-bumps.mdc b/.cursor/rules/dependency-bumps.mdc new file mode 100644 index 00000000..e8611d1c --- /dev/null +++ b/.cursor/rules/dependency-bumps.mdc @@ -0,0 +1,52 @@ +--- +description: Dependency-bump traps — @types type renames, and what a green suite does and does not prove for a major test-runner bump. +globs: ["**/*.ts", "package.json", "package-lock.json", "tsup.config.ts"] +alwaysApply: false +--- + +# Dependency bumps + +Canonical detail lives in [docs/troubleshooting.md](../../docs/troubleshooting.md); +this rule is the short form. `AGENTS.md` carries the same two points. + +## Never import a `@types/*` type by name when you can derive it from a value + +`@types/node` renames interfaces between **minor** versions with no deprecation +window. 26.5.0 renamed the `perf_hooks` histogram `IntervalHistogram` → +`ELDHistogram`, and neither version declares both names. A bump therefore fails +`Lint` and `Build` with `TS2305: … has no exported member` while nothing is +wrong at runtime. + +Renaming the import moves the break to the other side of the bump and leaves the +same trap for the next rename. Derive the type from the value that produces it: + +```ts +// not: import { type IntervalHistogram, monitorEventLoopDelay } from "perf_hooks"; +import { monitorEventLoopDelay } from "perf_hooks"; + +type EventLoopHistogram = ReturnType; +``` + +Prefer `ReturnType`, `Parameters`, `InstanceType` +and `Awaited<…>` over importing a library's interface name, whenever that name +exists only to annotate something the library already returns. + +Before merging, typecheck against **both** the current pin and the bump, then +revert the fix under the new types and confirm the original error returns. A +type fix that passes either way proves nothing. + +Sequencing: land the derived-type fix on `main` first (it compiles against the +current pin, which is the point), then rebase the dependabot PR — the two cannot +land in one step without pushing onto dependabot's branch. + +## A green suite does not fully clear a major test-runner bump + +`vitest` 4.1.11 → 5.0.0 landed green on every check on 2026-09-12. CI runs the +suite *under* the new runner, so that is strong evidence — much stronger than for +an ordinary library bump, because the bump largely tests itself. + +It still only covers what the suite exercises. A major can change config +resolution, reporter output, worker pooling, default timeouts or globals +handling without failing a passing assertion. After a runner major, suspect the +runner first when tooling output turns odd, rather than bisecting product code. +`npx vitest --version` confirms which runner actually ran. diff --git a/AGENTS.md b/AGENTS.md index 39cfd9df..e4f4ecff 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -41,11 +41,14 @@ Vitest globals are enabled. Every new feature requires tests under `__tests__/`. ## Dependencies and build -- `vendor/scanner`, `vendor/agent-types`, and `vendor/menubar` are git submodules. Initialize submodules before `npm install`; CI checkouts remain recursive. Use HTTPS on machines without GitHub SSH keys. -- `@threadbase/scanner` and `@threadbase/agent-types` are `file:` dependencies built by `postinstall` and bundled into `dist`. -- Do not suppress install scripts unless you manually preserve `prepare` (`patch-package`) and `postinstall` (including the executable `node-pty` spawn helper). -- For the CLI bundle, only `node-pty` is external. `pg` and all other runtime dependencies must be bundled because deployed releases have no `node_modules`. -- Preserve both SQLite and Postgres migration copies in the build. Deployment details live in [docs/guides/deploy-internals.md](docs/guides/deploy-internals.md). +- `vendor/menubar` is the **only** git submodule. Build and test never need it: every `ci.yml` and `release.yml` checkout sets `submodules: false`, and only `update-menubar.yml` uses `submodules: recursive`. A fresh checkout runs `npm install` with no submodule init. +- `@threadbase-sh/scanner` and `@threadbase-sh/agent-types` are published **public npm packages** on normal semver ranges — note the `-sh` scope. They are not `file:` deps and not submodules; tsup bundles them inline into `dist/`, so the runtime does not need them installed. Bump by raising the range here and publishing from `tb-scanner` / `threadbase-agent-types`. +- Do not suppress install scripts unless you manually preserve `prepare` (`patch-package`, which also sets `core.hooksPath` to `scripts/git-hooks`) and `postinstall` (the `node-pty` spawn-helper `chmod`). +- **Externals differ per tsup entry.** The library entry (`src/index.ts`) externalizes `node-pty` and `pg`. The CLI entry (`cli/index.ts` and the docker helpers) externalizes `node-pty` and `better-sqlite3` and bundles everything else, including `pg`, because deployed releases have no `node_modules`. Both externals are native modules and must resolve at runtime on the target machine. +- The build copies **three** migration directories into `dist/`: `migrations/` (SQLite cache), `runtime-migrations/` (SQLite session registry), and `pg-migrations/` (Postgres). Both SQLite folders are required at runtime, and a missing `runtime-migrations/` disables session persistence silently while the server keeps serving. Deployment details live in [docs/guides/deploy-internals.md](docs/guides/deploy-internals.md). +- `git pull` does not refresh `node_modules`. A pull that changes `package.json`/`package-lock.json` leaves the old packages on disk, and the build bundles what is installed rather than what the lockfile pins. Check with `npm ls --depth=0` (an `invalid: "" from the root project` line means stale) and resync with `npm ci`. +- Never import a type by name from a `@types/*` package when you can derive it from a value. `@types/node` renames interfaces between minors with no deprecation window (26.5.0: `IntervalHistogram` -> `ELDHistogram`, with neither version declaring both), which fails `Lint` and `Build` on `TS2305` while runtime behaviour is unchanged. Use `ReturnType` and friends. Land the derived-type fix on `main` first, then rebase the dependabot PR; verify against both the current pin and the bump, and revert under the new types to confirm the error returns. See [docs/troubleshooting.md](docs/troubleshooting.md). +- A green suite does not fully clear a major test-runner bump. `vitest` 5.0.0 landed green on 2026-09-12; CI runs the suite under the new runner, which is strong evidence, but it does not cover config resolution, reporters, worker pooling or default timeouts. After a runner major, suspect the runner first when tooling output turns odd. See [docs/troubleshooting.md](docs/troubleshooting.md). ## Deployment and platform safeguards diff --git a/CLAUDE.md b/CLAUDE.md index a1f67ab1..06eae114 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -82,9 +82,21 @@ When `MULTI_AGENT_FLOW=true`, session start/input route through a Temporal-orche **`git pull` does not refresh `node_modules`.** A pull (or branch switch) that changes `package.json`/`package-lock.json` leaves the existing `node_modules` on disk untouched — `npm run build`/`deploy` will silently bundle whatever versions are already installed, not what the new lockfile pins. Check with `npm ls --depth=0` (an `invalid: "" from the root project` line means `node_modules` is stale) and resync with `npm ci` before building/deploying after any dependency-affecting pull. +**A `@types/*` type imported by name is a version coupling.** +`@types/node` renames interfaces between minors with no deprecation window — 26.5.0 renamed `IntervalHistogram` to `ELDHistogram`, and neither version declares both — so the bump fails `Lint` and `Build` with `TS2305: … has no exported member` while nothing is wrong at runtime. +Renaming the import moves the break to the other side of the bump; derive the type from the value instead (`ReturnType`), which tracks whatever the installed types call it. +Land the derived-type fix on `main` first, then rebase the dependabot PR — it cannot go green before the fix exists, and the two cannot land in one step without pushing onto dependabot's branch. +Verify against **both** versions, and revert under the new types to confirm the original error returns. +See [docs/troubleshooting.md](docs/troubleshooting.md) → "A `@types/node` bump fails `Lint` and `Build`". + +**A green suite does not fully clear a major test-runner bump.** +`vitest` 5.0.0 landed green on every check (2026-09-12), which is strong evidence because CI runs the suite *under* the new runner — but it covers only what the suite exercises, not config resolution, reporters, worker pooling or default timeouts. +After a runner major, suspect the runner first when tooling output turns odd, rather than bisecting product code. +See [docs/troubleshooting.md](docs/troubleshooting.md) → "A major test-runner bump passes every check". + ## Build notes -- **CLI externals**: only `node-pty` is external for the CLI tsup entry. `pg` and everything else must be bundled — the deployed CLI lives in `~/.threadbase/releases/` with no `node_modules`. +- **CLI externals**: `node-pty` and `better-sqlite3` are external for the CLI tsup entry; `pg` and everything else is bundled — the deployed CLI lives in `~/.threadbase/releases/` with no `node_modules`, so anything not bundled has to resolve natively on the target machine. The library entry (`src/index.ts`) has a *different* external list: `node-pty` and `pg`. - `npm run build` copies `src/db/migrations/` (SQLite cache), `src/db/runtime-migrations/` (SQLite session registry) and `src/db/pg-migrations/` (Postgres) into `dist/`. Deploy ships the first two unconditionally and `pg-migrations/` only when it exists (`scripts/deploy.sh`) — both SQLite folders are required at runtime, and a missing `runtime-migrations/` disables session persistence silently while the server keeps serving. Details: [docs/guides/deploy-internals.md](docs/guides/deploy-internals.md). ## Deploy & distribution diff --git a/docs/agents/tooling.md b/docs/agents/tooling.md index a476ac06..3fd8ad9f 100644 --- a/docs/agents/tooling.md +++ b/docs/agents/tooling.md @@ -14,6 +14,8 @@ Codex discovers the same four operational workflows from `.agents/skills/`. Keep ## Other agents -Cursor, Copilot, and the rest read none of the files above — the formats are Claude Code's and Codex's own. This page is the whole handoff; equivalent optional tooling in another runtime is fine. +Cursor reads `AGENTS.md` natively, and `.cursor/rules/*.mdc` carries the short form of guidance that would otherwise reach only Claude Code and Codex — currently one rule, `dependency-bumps.mdc`. Those rules are pointers, not a third copy: the canonical text stays in `docs/`, because three hand-maintained copies of the same paragraph drift. + +Copilot and the rest read none of the files above — the remaining formats are Claude Code's and Codex's own. This page is the whole handoff; equivalent optional tooling in another runtime is fine. No marketplace or plugin is registered by this repository. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 7802ceb3..c100fd4e 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -1405,3 +1405,72 @@ sqlite3 "$DB" "delete from conversation_tail where conversation_id=''; delet ``` The list endpoint queries SQLite directly, so the row disappears immediately without a restart; the only residue is a dead entry in the in-memory `fileIndex`, which matters solely if that exact path ever returns. + +## A `@types/node` bump fails `Lint` and `Build` with `TS2305: … has no exported member` + +**When:** A dependabot `@types/node` bump goes red on exactly two jobs, `Lint` and `Build`, with an identical message on both and nothing wrong at runtime: + +``` +src/services/host-pressure/hostPressure.ts(3,15): error TS2305: +Module '"perf_hooks"' has no exported member 'IntervalHistogram'. +``` + +Observed 2026-09-11 on [#853](https://github.com/RonenMars/threadbase-streamer/pull/853), `@types/node` 26.4.0 → 26.5.0. + +**Cause.** `@types/node` renames interfaces between minor versions without a deprecation window. +26.5.0 renamed the `perf_hooks` event-loop-delay histogram `IntervalHistogram` → `ELDHistogram`, and the two versions share **neither** name — 26.4.0 declares only the old one, 26.5.0 only the new one. +So any `import { type IntervalHistogram }` compiles against exactly one side of the bump. +This is a *type-only* break: nothing is wrong with the code, no runtime behaviour changes, and the only symptom is `tsc` refusing the name. + +**Fix — derive the type from the value, do not rename it.** +Swapping `IntervalHistogram` for `ELDHistogram` moves the breakage to the other side of the bump rather than removing it, and leaves the identical trap for the next rename. +Derive it from the function that produces it instead ([#874](https://github.com/RonenMars/threadbase-streamer/pull/874)): + +```ts +// not: import { type IntervalHistogram, monitorEventLoopDelay } from "perf_hooks"; +import { monitorEventLoopDelay } from "perf_hooks"; + +type EventLoopHistogram = ReturnType; +``` + +`ReturnType` resolves to whatever the installed `@types/node` calls it, so no future rename can break it. + +**The general rule.** A type imported *by name* from a `@types/*` package is a version coupling; a type *derived from a value* is not. +Prefer `ReturnType`, `Parameters`, `InstanceType` and `Awaited<…>` over importing a library's interface name, whenever the name exists only to annotate something that library already returns. +This costs one line and removes a whole class of bump failures. + +**Verify against both versions before merging.** The fix is worthless if it only compiles against the version you happen to have installed: + +```sh +npx tsc --noEmit -p tsconfig.json --pretty false # current pin +npm install --no-save @types/node@ # the bump +npx tsc --noEmit -p tsconfig.json --pretty false # must also pass +npm ci # restore the pin +``` + +Confirm it is a real fix by reverting the change under the *new* types and watching the original `TS2305` come back. +A type fix that passes both ways proves nothing. + +**Sequencing.** The code fix and the bump cannot land in one step unless you push onto dependabot's branch. +Land the derived-type fix on `main` first (it compiles against the *current* pin, which is the point), then rebase the dependabot PR — it goes green on its own. + +## A major test-runner bump passes every check and still is not fully proven + +**When:** A dependabot major bump of `vitest` (or any test runner) comes back green on all checks, including both `Test` jobs and both `Smoke` jobs, and there is a temptation to treat that as a clean bill of health. + +`vitest` 4.1.11 → **5.0.0** landed this way on 2026-09-12 ([#855](https://github.com/RonenMars/threadbase-streamer/pull/855)), green across two independent CI cycles. + +**Cause.** CI runs the suite *under* the new runner, so a green suite is genuinely strong evidence — much stronger than for an ordinary library bump, because the bump largely tests itself. +But it only covers what the suite exercises. +A major can still change config resolution, reporter output, worker pooling, default timeouts, coverage thresholds or globals handling, and none of those necessarily fails a passing assertion. +The failure mode is not a red build later; it is output that looks subtly wrong, or a suite that starts behaving differently under load, with no version change in sight to blame. + +**Fix / what to do about it.** + +- After a major runner bump, **suspect the runner first** when local test output turns odd — reporters, config resolution, worker pooling, timeouts — rather than bisecting product code. +- `npx vitest --version` confirms which runner actually ran; the banner in any `vitest run` output prints it too (`RUN v5.0.0 …`). +- Do not read a green suite as covering the runner's *own* configuration surface. `vitest.config.*` and `__tests__/setup*` are the places a major bump changes meaning silently. +- The load-sensitive suites documented above still need their isolation re-run before a failure is called real; a runner bump changes scheduling, so it can move which suites are load-sensitive. + +**The general rule.** For a dev-dependency major, "CI is green" answers *did the suite still pass*, not *does the tool still mean the same thing*. +Merge it if green, then keep the version in mind for the next unexplained tooling symptom instead of treating the bump as settled history.