Catch tool-package source changing without a version bump

.github/workflows/ci.yml

@@ -73,6 +73,9 @@ jobs:
       - run: bun run check:react-ui-drift
       - run: bun run check:react-ui-pin
       - run: bun run check:tool-package-pins
+      - run: bun run check:tool-package-freshness
+        env:
+          CHECK_BASE_REF: ${{ github.event.pull_request.base.sha }}
 
   walking-skeleton:
     runs-on: ubuntu-latest

package.json

@@ -24,7 +24,7 @@
     "setup:memory": "bun run scripts/setup-memory.ts",
     "seed": "bun packages/cli/src/index.ts seed",
     "reset": "bun packages/cli/src/index.ts reset",
-    "check:structural": "bun test scripts/checks/test && bun run check:deletion && bun run check:killdates && bun run check:licenses && bun run check:no-product-tenancy && bun run check:browser-safe-subpaths && bun run check:web-utilities && bun run check:tailwind-source && bun run check:ui-vocabulary && bun run check:react-ui-drift && bun run check:react-ui-pin && bun run check:tool-package-pins",
+    "check:structural": "bun test scripts/checks/test && bun run check:deletion && bun run check:killdates && bun run check:licenses && bun run check:no-product-tenancy && bun run check:browser-safe-subpaths && bun run check:web-utilities && bun run check:tailwind-source && bun run check:ui-vocabulary && bun run check:react-ui-drift && bun run check:react-ui-pin && bun run check:tool-package-pins && bun run check:tool-package-freshness",
     "check:deletion": "bun run scripts/checks/deletion.ts",
     "check:killdates": "bun run scripts/checks/killdates.ts",
     "check:packages": "bun run scripts/checks/packages.ts",
@@ -37,6 +37,7 @@
     "check:react-ui-drift": "bun run scripts/checks/react-ui-drift.ts",
     "check:react-ui-pin": "bun run scripts/checks/react-ui-pin.ts",
     "check:tool-package-pins": "bun run scripts/checks/tool-package-pins.ts",
+    "check:tool-package-freshness": "bun run scripts/checks/tool-package-freshness.ts",
     "build:sidecar-image": "docker build -f apps/sidecar/Dockerfile -t corbits-sidecar:dev .",
     "eval": "bun run scripts/evals-run.ts"
   },

packages/github-tools/package.json

@@ -2,7 +2,7 @@
   "name": "@corbits/github-tools",
   "private": true,
   "description": "GitHub search integration: a minimal REST client and an @intx/agent tool bundle exposing github_activity",
-  "version": "0.0.5",
+  "version": "0.0.6",
   "license": "LGPL-2.1-or-later",
   "type": "module",
   "exports": {

scripts/checks/browser-safe-subpaths.ts

@@ -89,6 +89,19 @@ interface ImportSpecifier {
   readonly typeOnly: boolean;
 }
 
+/**
+ * Blanks out line and block comments, preserving offsets and newlines so
+ * the patterns below cannot match prose. Without this, a comment
+ * mentioning the word `import` before a real import swallows the lines
+ * between them — `[^;]*?` spans newlines — and reports the file's genuine
+ * type-only import as a value import.
+ */
+export function stripComments(contents: string): string {
+  return contents
+    .replace(/\/\*[\s\S]*?\*\//g, (match) => match.replace(/[^\n]/g, " "))
+    .replace(/\/\/[^\n]*/g, (match) => " ".repeat(match.length));
+}
+
 /**
  * Extracts every static `from "..."` import/export specifier from a
  * source file, plus dynamic `import("...")` and bare side-effect
@@ -97,7 +110,8 @@ interface ImportSpecifier {
  * `import { type X, y } from "z"` is conservatively treated as a value
  * import, since `y` really is one.
  */
-export function parseImportSpecifiers(contents: string): ImportSpecifier[] {
+export function parseImportSpecifiers(source: string): ImportSpecifier[] {
+  const contents = stripComments(source);
   const results: ImportSpecifier[] = [];
 
   const fromPattern =

scripts/checks/killdates.ts

@@ -80,12 +80,28 @@ export function listVendoredPaths(root: string): string[] {
     const bucketPath = path.join(vendorRoot, bucket.name);
     for (const entry of readdirSync(bucketPath, { withFileTypes: true })) {
       if (!entry.isDirectory()) continue;
+      // `vendor/intx/*` is a workspace glob, so `bun install` materializes a
+      // directory holding nothing but `node_modules` for every linked
+      // package — an install artifact, not a vendored tree. Only a
+      // directory carrying its own source is something the ledger owes a
+      // kill date.
+      if (!hasVendoredSource(path.join(bucketPath, entry.name))) continue;
       vendored.push(path.join("vendor", bucket.name, entry.name));
     }
   }
   return vendored.sort();
 }
 
+/**
+ * A vendored tree carries hand-copied source. A directory holding only
+ * `node_modules` is what a workspace glob leaves behind after an install.
+ */
+export function hasVendoredSource(directory: string): boolean {
+  return readdirSync(directory, { withFileTypes: true }).some(
+    (entry) => entry.name !== "node_modules",
+  );
+}
+
 /** Every vendored directory must carry a kill-date registry row. */
 export function auditVendorCoverage(
   vendoredPaths: readonly string[],

scripts/checks/test/browser-safe-subpaths.test.ts

@@ -251,3 +251,26 @@ test("parseImportSpecifiers handles multi-line export-from lists", () => {
   );
   expect(specs).toEqual([{ specifier: "./group", typeOnly: false }]);
 });
+
+test("a comment mentioning import does not swallow a later type-only import", () => {
+  const parsed = parseImportSpecifiers(
+    [
+      "// The tool modules import `defineTool` from `@intx/agent`, whose",
+      "// module graph reaches `node:path`, so browser-reachable callers",
+      "// import from here instead.",
+      'import type { ToolPackagePin } from "@intx/types/tool-packages";',
+    ].join("\n"),
+  );
+  expect(parsed).toEqual([
+    { specifier: "@intx/types/tool-packages", typeOnly: true },
+  ]);
+});
+
+test("a block comment cannot hide a real value import", () => {
+  const parsed = parseImportSpecifiers(
+    ['/* import x from "commented-out"; */', 'import y from "real";'].join(
+      "\n",
+    ),
+  );
+  expect(parsed).toEqual([{ specifier: "real", typeOnly: false }]);
+});

scripts/checks/test/killdates.test.ts

@@ -1,11 +1,12 @@
 import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import path from "node:path";
-import { expect, test } from "bun:test";
+import { describe, expect, test } from "bun:test";
 import {
   auditKillDates,
   auditVendorCoverage,
   auditVendorDrift,
+  hasVendoredSource,
   listVendoredPaths,
   parseKillDates,
 } from "../killdates";
@@ -80,6 +81,13 @@ test("listVendoredPaths finds directories two levels under vendor/, not files",
   try {
     mkdirSync(path.join(root, "vendor", "intx", "log"), { recursive: true });
     mkdirSync(path.join(root, "vendor", "intx", "agent"), { recursive: true });
+    writeFileSync(path.join(root, "vendor", "intx", "log", "index.ts"), "");
+    writeFileSync(path.join(root, "vendor", "intx", "agent", "index.ts"), "");
+    // What `bun install` leaves behind for a workspace glob: a directory
+    // holding nothing but linked dependencies, and no source of its own.
+    mkdirSync(path.join(root, "vendor", "intx", "installed", "node_modules"), {
+      recursive: true,
+    });
     writeFileSync(path.join(root, "vendor", "intx", "LICENSE"), "LGPL");
     writeFileSync(path.join(root, "vendor", "stray-file"), "");
     expect(listVendoredPaths(root)).toEqual([
@@ -191,3 +199,18 @@ test("a vendored row without a valid hash column is a drift violation", () => {
     rmSync(root, { recursive: true, force: true });
   }
 });
+
+describe("hasVendoredSource", () => {
+  test("a directory holding only node_modules is an install artifact", () => {
+    const dir = mkdtempSync(path.join(tmpdir(), "killdates-"));
+    mkdirSync(path.join(dir, "node_modules"));
+    expect(hasVendoredSource(dir)).toBe(false);
+  });
+
+  test("a directory carrying its own source is a vendored tree", () => {
+    const dir = mkdtempSync(path.join(tmpdir(), "killdates-"));
+    mkdirSync(path.join(dir, "node_modules"));
+    mkdirSync(path.join(dir, "src"));
+    expect(hasVendoredSource(dir)).toBe(true);
+  });
+});

scripts/checks/test/tool-package-freshness.test.ts

@@ -0,0 +1,99 @@
+import { describe, expect, test } from "bun:test";
+
+import {
+  auditFreshness,
+  packagesWithChangedSource,
+  readToolPackageNames,
+} from "../tool-package-freshness";
+
+const TOOL_PACKAGES = ["github-tools", "memory-tools"];
+
+describe("packagesWithChangedSource", () => {
+  test("names a package whose src/ moved", () => {
+    expect(
+      packagesWithChangedSource(
+        ["packages/github-tools/src/client.ts"],
+        TOOL_PACKAGES,
+      ),
+    ).toEqual(["github-tools"]);
+  });
+
+  test("ignores tests — they ship no source an agent resolves", () => {
+    expect(
+      packagesWithChangedSource(
+        [
+          "packages/github-tools/src/client.test.ts",
+          "packages/chat-ui/src/timeline.test.tsx",
+        ],
+        TOOL_PACKAGES,
+      ),
+    ).toEqual([]);
+  });
+
+  test("ignores everything outside a package's src/", () => {
+    expect(
+      packagesWithChangedSource(
+        [
+          "packages/github-tools/README.md",
+          "apps/hub/src/index.ts",
+          "workflows/code-review/src/index.ts",
+        ],
+        TOOL_PACKAGES,
+      ),
+    ).toEqual([]);
+  });
+});
+
+describe("auditFreshness", () => {
+  test("the recurring incident: src moved, version did not", () => {
+    const report = auditFreshness([
+      { name: "github-tools", baseVersion: "0.0.5", headVersion: "0.0.5" },
+    ]);
+    expect(report.violations).toHaveLength(1);
+    expect(report.violations[0]).toContain("packages/github-tools");
+    expect(report.violations[0]).toContain("stayed at 0.0.5");
+  });
+
+  test("a bumped package passes", () => {
+    const report = auditFreshness([
+      { name: "github-tools", baseVersion: "0.0.5", headVersion: "0.0.6" },
+    ]);
+    expect(report.violations).toEqual([]);
+  });
+
+  test("a package that did not exist at the base ref is new, not stale", () => {
+    const report = auditFreshness([
+      { name: "scout-agent", baseVersion: undefined, headVersion: "0.0.1" },
+    ]);
+    expect(report.violations).toEqual([]);
+  });
+
+  test("names every stale package, not just the first", () => {
+    const report = auditFreshness([
+      { name: "github-tools", baseVersion: "0.0.5", headVersion: "0.0.5" },
+      { name: "memory-tools", baseVersion: "0.0.4", headVersion: "0.0.4" },
+    ]);
+    expect(report.violations).toHaveLength(2);
+  });
+});
+
+describe("scope", () => {
+  test("ignores a workspace package the registry does not publish", () => {
+    expect(
+      packagesWithChangedSource(
+        ["packages/workflow-catalog/src/templates.ts"],
+        TOOL_PACKAGES,
+      ),
+    ).toEqual([]);
+  });
+
+  test("reads the publisher's own list so the two cannot disagree", () => {
+    const names = readToolPackageNames(`
+      export const CORBITS_TOOL_PACKAGE_DIRS: readonly string[] = [
+        new URL("../../memory-tools", import.meta.url).pathname,
+        new URL("../../github-tools", import.meta.url).pathname,
+      ];
+    `);
+    expect(names).toEqual(["github-tools", "memory-tools"]);
+  });
+});