From 25656d873fdafa8ee2e0b257f665d5b5f0f8637f Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 02:26:37 -0700 Subject: [PATCH] fix(workspace): stop apps/loopover-ui silently resolving a stale published contract apps/loopover-ui declared "@loopover/contract": "^0.1.0". While contract sat at 0.1.0 that matched the workspace copy and npm linked it. The moment release-please bumped contract into the engine-and-dependents group at 3.17.0, the range stopped matching -- and npm did NOT fail. It resolved the dependency from the REGISTRY, installing the published 0.1.0 tarball into apps/loopover-ui/node_modules while every other consumer used 3.17.0. The website would have built against a contract ~17 minor versions stale: same import specifiers, different schemas and types, no error anywhere. It surfaced only as an `npm ci` lockfile-sync failure on the release PR (#9780), which reads like a mechanical lockfile problem rather than the dependency bug it is. Pin the private apps to "*" -- the one range that means "the workspace copy" and cannot drift -- and add workspace-dep-ranges:check. The check requires "*" for PRIVATE packages rather than merely checking that a range is satisfied, because satisfaction alone cannot see the trap: `^0.1.0` against contract@0.1.0 is valid today and silently wrong the day contract is bumped. That rule caught a second latent instance, discovery-index pinning engine to ^3.4.0. Published packages keep real ranges (npm consumers need them) and are checked for satisfaction instead. --- apps/loopover-miner-ui/package.json | 2 +- apps/loopover-ui/package.json | 4 +- package-lock.json | 8 +- package.json | 3 +- packages/discovery-index/package.json | 4 +- scripts/check-workspace-dep-ranges.ts | 140 +++++++++++++++++++ test/unit/check-workspace-dep-ranges.test.ts | 88 ++++++++++++ 7 files changed, 239 insertions(+), 10 deletions(-) create mode 100644 scripts/check-workspace-dep-ranges.ts create mode 100644 test/unit/check-workspace-dep-ranges.test.ts diff --git a/apps/loopover-miner-ui/package.json b/apps/loopover-miner-ui/package.json index c1696ac07..8421a4207 100644 --- a/apps/loopover-miner-ui/package.json +++ b/apps/loopover-miner-ui/package.json @@ -16,7 +16,7 @@ "format:check": "prettier --check ." }, "dependencies": { - "@loopover/ui-kit": ">=0.1.0 <2.0.0", + "@loopover/ui-kit": "*", "@tailwindcss/vite": "^4.3.3", "@tanstack/react-router": "^1.170.18", "react": "^19.2.8", diff --git a/apps/loopover-ui/package.json b/apps/loopover-ui/package.json index c1f732747..e6b360272 100644 --- a/apps/loopover-ui/package.json +++ b/apps/loopover-ui/package.json @@ -23,8 +23,8 @@ "format:check": "prettier --check ." }, "dependencies": { - "@loopover/contract": "^0.1.0", - "@loopover/ui-kit": ">=0.1.0 <2.0.0", + "@loopover/contract": "*", + "@loopover/ui-kit": "*", "@radix-ui/react-accordion": "^1.2.18", "@radix-ui/react-alert-dialog": "^1.1.21", "@radix-ui/react-aspect-ratio": "^1.1.13", diff --git a/package-lock.json b/package-lock.json index 2e0306df4..149a96b9c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -71,7 +71,7 @@ "name": "@loopover/ui-miner", "version": "0.0.0", "dependencies": { - "@loopover/ui-kit": ">=0.1.0 <2.0.0", + "@loopover/ui-kit": "*", "@tailwindcss/vite": "^4.3.3", "@tanstack/react-router": "^1.170.18", "react": "^19.2.8", @@ -484,8 +484,8 @@ "version": "0.0.0", "hasInstallScript": true, "dependencies": { - "@loopover/contract": "^0.1.0", - "@loopover/ui-kit": ">=0.1.0 <2.0.0", + "@loopover/contract": "*", + "@loopover/ui-kit": "*", "@radix-ui/react-accordion": "^1.2.18", "@radix-ui/react-alert-dialog": "^1.1.21", "@radix-ui/react-aspect-ratio": "^1.1.13", @@ -21531,7 +21531,7 @@ "dependencies": { "@cloudflare/containers": "^0.3.7", "@hono/node-server": "^2.0.11", - "@loopover/engine": "^3.4.0", + "@loopover/engine": "*", "@posthog/cli": "0.9.1", "hono": "^4.12.31", "posthog-node": "^5.46.1" diff --git a/package.json b/package.json index ae70787bf..950522fd8 100644 --- a/package.json +++ b/package.json @@ -58,6 +58,7 @@ "engine:coverage": "node --experimental-strip-types scripts/engine-coverage.ts", "db:migrations:check": "tsx scripts/check-migrations.ts", "db:migrations:immutable:check": "tsx scripts/check-released-migrations-immutable.ts", + "workspace-dep-ranges:check": "tsx scripts/check-workspace-dep-ranges.ts", "db:schema-drift:check": "tsx scripts/check-schema-drift.ts", "actionlint": "node --experimental-strip-types scripts/actionlint.ts", "lint:composite-actions": "node --experimental-strip-types scripts/lint-composite-actions.ts", @@ -136,7 +137,7 @@ "test:smoke:browser:install": "playwright install chromium", "test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts", "pretest:ci": "npm run check-node-version", - "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:migrations:immutable:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run control-plane:contract:check && npm run control-plane:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run server-manifest:check && npm run dead-source-files:check && npm run regate-sort-key:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run mcp:client-config:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", + "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:migrations:immutable:check && npm run workspace-dep-ranges:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run control-plane:contract:check && npm run control-plane:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run server-manifest:check && npm run dead-source-files:check && npm run regate-sort-key:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run mcp:client-config:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", "test:release": "npm run test:ci && npm run changelog:check", "test:release:mcp": "npm run test:ci", "test:watch": "vitest", diff --git a/packages/discovery-index/package.json b/packages/discovery-index/package.json index d95ff164b..594d7cbeb 100644 --- a/packages/discovery-index/package.json +++ b/packages/discovery-index/package.json @@ -3,7 +3,7 @@ "version": "0.1.0", "private": true, "type": "module", - "description": "LoopOver hosted discovery-index service (#4250) — a shared, cached GitHub issue/search fan-out and soft-claim coordinator opted-in miner instances can query instead of each independently hammering GitHub. Deploys standalone; see #7164.", + "description": "LoopOver hosted discovery-index service (#4250) \u2014 a shared, cached GitHub issue/search fan-out and soft-claim coordinator opted-in miner instances can query instead of each independently hammering GitHub. Deploys standalone; see #7164.", "engines": { "node": ">=22.0.0 <23.0.0" }, @@ -20,7 +20,7 @@ "dependencies": { "@cloudflare/containers": "^0.3.7", "@hono/node-server": "^2.0.11", - "@loopover/engine": "^3.4.0", + "@loopover/engine": "*", "@posthog/cli": "0.9.1", "hono": "^4.12.31", "posthog-node": "^5.46.1" diff --git a/scripts/check-workspace-dep-ranges.ts b/scripts/check-workspace-dep-ranges.ts new file mode 100644 index 000000000..fc1d369e1 --- /dev/null +++ b/scripts/check-workspace-dep-ranges.ts @@ -0,0 +1,140 @@ +#!/usr/bin/env node +// A workspace dependency's range must actually match the workspace version it points at. +// +// THE INCIDENT: `apps/loopover-ui` declared `"@loopover/contract": "^0.1.0"`. While the contract package +// sat at 0.1.0 that range matched the workspace copy, so npm linked it and everything was fine. The moment +// release-please bumped contract into the engine-and-dependents group at 3.17.0, `^0.1.0` stopped matching +// -- and npm did NOT fail. It quietly resolved the dependency from the REGISTRY instead, installing the +// published 0.1.0 tarball into apps/loopover-ui/node_modules while every other consumer used 3.17.0. +// +// The website would then have built against a contract nearly twenty minor versions stale: same import +// specifier, different schemas and types, no error anywhere. It surfaced only as an `npm ci` lockfile-sync +// failure on the release PR, which reads like a mechanical lockfile problem rather than the dependency bug +// it actually is. +// +// So: every dependency on a workspace package must be satisfied by that package's CURRENT version. `*` is +// the honest way for a PRIVATE app to say "always the workspace copy" and is accepted as such. A published +// package cannot use `*` (real consumers install it from npm and need a meaningful range), so those must +// keep a range that genuinely matches -- which the same check enforces. +import { readdirSync, readFileSync } from "node:fs"; +import { join } from "node:path"; +import { fileURLToPath, URL } from "node:url"; +import { satisfies, validRange } from "semver"; + +export type WorkspaceManifest = { + name?: string; + version?: string; + private?: boolean; + dependencies?: Record; + devDependencies?: Record; + path?: string; +}; + +export type RangeViolation = { consumer: string; dependency: string; range: string; workspaceVersion: string; reason: string }; + +/** + * PURE core: every workspace-internal dependency whose range the workspace version does not satisfy. + * + * `*` is exempt by design -- it is the explicit "whatever the workspace has" declaration, and the only + * range that cannot drift. Non-workspace (third-party) dependencies are ignored entirely: their versions + * come from the registry and the lockfile, which is a different problem with different guards. + */ +export function findWorkspaceRangeDrift(manifests: readonly WorkspaceManifest[]): RangeViolation[] { + const versions = new Map(); + for (const manifest of manifests) { + if (manifest.name && manifest.version) versions.set(manifest.name, manifest.version); + } + + const violations: RangeViolation[] = []; + for (const manifest of manifests) { + const consumer = manifest.name ?? manifest.path ?? ""; + for (const section of ["dependencies", "devDependencies"] as const) { + for (const [dependency, range] of Object.entries(manifest[section] ?? {})) { + const workspaceVersion = versions.get(dependency); + if (workspaceVersion === undefined) continue; // third-party: not ours to police + if (range === "*") continue; // explicitly "the workspace copy", cannot drift + // A PRIVATE package is never installed from a registry, so a pinned range buys it nothing and can + // only rot. Requiring "*" here is what makes this check catch the trap BEFORE the bump that springs + // it: `^0.1.0` against contract@0.1.0 is satisfied today and silently wrong the day contract is + // bumped, which is exactly how apps/loopover-ui ended up resolving a stale published copy. + if (manifest.private) { + violations.push({ + consumer, + dependency, + range, + workspaceVersion, + reason: `${consumer} is private, so it is never installed from a registry -- pin it to "*" instead of "${range}", which buys nothing and breaks silently the next time ${dependency} is version-bumped`, + }); + continue; + } + if (!validRange(range)) { + violations.push({ + consumer, + dependency, + range, + workspaceVersion, + reason: `"${range}" is not a valid semver range, so npm's resolution here is unpredictable`, + }); + continue; + } + if (!satisfies(workspaceVersion, range)) { + violations.push({ + consumer, + dependency, + range, + workspaceVersion, + reason: `the workspace has ${dependency}@${workspaceVersion}, which "${range}" does NOT match -- npm will silently install a published copy from the registry instead of linking the workspace one`, + }); + } + } + } + } + return violations; +} + +function readManifests(root: string): WorkspaceManifest[] { + const manifests: WorkspaceManifest[] = []; + for (const group of ["packages", "apps"]) { + let entries: string[]; + try { + entries = readdirSync(join(root, group), { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name); + } catch { + continue; // a workspace group that does not exist here is not an error + } + for (const name of entries) { + const path = join(group, name, "package.json"); + try { + manifests.push({ ...(JSON.parse(readFileSync(join(root, path), "utf8")) as WorkspaceManifest), path }); + } catch { + // A directory without a manifest is not a workspace package; skip rather than fail the check. + } + } + } + return manifests; +} + +function main(): void { + const root = join(fileURLToPath(new URL(".", import.meta.url)), ".."); + const violations = findWorkspaceRangeDrift(readManifests(root)); + + if (violations.length > 0) { + console.error("A workspace dependency's range no longer matches the package it points at:\n"); + for (const violation of violations) { + console.error(` ${violation.consumer} -> "${violation.dependency}": "${violation.range}"`); + console.error(` ${violation.reason}`); + } + console.error( + "\n npm does NOT fail on this. It resolves the dependency from the registry instead, so the consumer\n" + + " silently builds against a published copy while the rest of the repo uses the workspace one.\n\n" + + " For a PRIVATE app: use \"*\" -- it means \"the workspace copy\" and cannot drift.\n" + + " For a PUBLISHED package: widen the range to include the current workspace version, since real\n" + + " consumers install it from npm and need a range that means something.", + ); + process.exit(1); + } + console.log("workspace-dep-ranges: OK — every workspace dependency resolves to the workspace copy."); +} + +if (process.argv[1]?.endsWith("check-workspace-dep-ranges.ts")) main(); diff --git a/test/unit/check-workspace-dep-ranges.test.ts b/test/unit/check-workspace-dep-ranges.test.ts new file mode 100644 index 000000000..252c05a81 --- /dev/null +++ b/test/unit/check-workspace-dep-ranges.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, it } from "vitest"; +import { findWorkspaceRangeDrift, type WorkspaceManifest } from "../../scripts/check-workspace-dep-ranges"; + +// apps/loopover-ui declared "@loopover/contract": "^0.1.0". That matched while contract sat at 0.1.0, so npm +// linked the workspace copy. The moment release-please bumped contract to 3.17.0 the range stopped matching +// and npm — without failing — resolved it from the REGISTRY instead, installing the published 0.1.0 tarball +// while every other consumer used 3.17.0. It surfaced only as an opaque `npm ci` lockfile-sync error. + +describe("findWorkspaceRangeDrift", () => { + it("REGRESSION: reproduces the real drift — a range the bumped workspace version no longer satisfies", () => { + const manifests: WorkspaceManifest[] = [ + { name: "@loopover/contract", version: "3.17.0" }, + { name: "@loopover/mcp", version: "3.17.0", dependencies: { "@loopover/contract": "^0.1.0" } }, + ]; + const violations = findWorkspaceRangeDrift(manifests); + expect(violations).toHaveLength(1); + expect(violations[0]).toMatchObject({ consumer: "@loopover/mcp", dependency: "@loopover/contract", workspaceVersion: "3.17.0" }); + expect(violations[0]?.reason).toContain("silently install a published copy"); + }); + + it("REGRESSION: catches the trap BEFORE the bump springs it, for a private consumer", () => { + // The decisive case. At contract@0.1.0 the range is satisfied, so a satisfaction-only check sees nothing + // and the bug lands later, in a release PR, disguised as a lockfile problem. A private app gains nothing + // from a range, so requiring "*" makes the problem visible while it is still cheap to fix. + const manifests: WorkspaceManifest[] = [ + { name: "@loopover/contract", version: "0.1.0" }, + { name: "@loopover/ui", version: "0.0.0", private: true, dependencies: { "@loopover/contract": "^0.1.0" } }, + ]; + const violations = findWorkspaceRangeDrift(manifests); + expect(violations).toHaveLength(1); + expect(violations[0]?.reason).toContain("is private"); + }); + + it('"*" is accepted — it is the only range that cannot drift', () => { + const manifests: WorkspaceManifest[] = [ + { name: "@loopover/contract", version: "3.17.0" }, + { name: "@loopover/ui", version: "0.0.0", private: true, dependencies: { "@loopover/contract": "*" } }, + ]; + expect(findWorkspaceRangeDrift(manifests)).toEqual([]); + }); + + it("a PUBLISHED package keeps a real range, and a matching one passes", () => { + // Published packages must not use "*" — consumers install them from npm and need a meaningful range — + // so for them the rule is only that the range actually matches. + const manifests: WorkspaceManifest[] = [ + { name: "@loopover/engine", version: "3.17.0" }, + { name: "@loopover/mcp", version: "3.17.0", dependencies: { "@loopover/engine": "^3.17.0" } }, + ]; + expect(findWorkspaceRangeDrift(manifests)).toEqual([]); + }); + + it("third-party dependencies are ignored entirely", () => { + const manifests: WorkspaceManifest[] = [{ name: "@loopover/mcp", version: "1.0.0", dependencies: { zod: "^4.0.0", "posthog-node": "^5.0.0" } }]; + expect(findWorkspaceRangeDrift(manifests)).toEqual([]); + }); + + it("an unparseable range is reported rather than silently skipped", () => { + const manifests: WorkspaceManifest[] = [ + { name: "@loopover/engine", version: "3.17.0" }, + { name: "@loopover/mcp", version: "1.0.0", dependencies: { "@loopover/engine": "not-a-range" } }, + ]; + expect(findWorkspaceRangeDrift(manifests)[0]?.reason).toContain("not a valid semver range"); + }); + + it("devDependencies are checked too — a stale one still resolves from the registry at build time", () => { + const manifests: WorkspaceManifest[] = [ + { name: "@loopover/engine", version: "3.17.0" }, + { name: "@loopover/mcp", version: "1.0.0", devDependencies: { "@loopover/engine": "^1.0.0" } }, + ]; + expect(findWorkspaceRangeDrift(manifests)).toHaveLength(1); + }); + + it("INVARIANT: this repo's own workspace is clean", async () => { + const { readdirSync, readFileSync } = await import("node:fs"); + const manifests: WorkspaceManifest[] = []; + for (const group of ["packages", "apps"]) { + for (const entry of readdirSync(group, { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + try { + manifests.push(JSON.parse(readFileSync(`${group}/${entry.name}/package.json`, "utf8")) as WorkspaceManifest); + } catch { + // not a workspace package + } + } + } + expect(findWorkspaceRangeDrift(manifests)).toEqual([]); + }); +});