diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ffa605359..d66bc875f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,6 +41,7 @@ jobs: - run: bun run check:ui-vocabulary - run: bun run check:react-ui-drift - run: bun run check:react-ui-pin + - run: bun run check:tool-package-pins walking-skeleton: runs-on: ubuntu-latest diff --git a/package.json b/package.json index 8031bda72..45447ddab 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "check:ui-vocabulary": "bun run scripts/checks/ui-vocabulary.ts", "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", "build:sidecar-image": "docker build -f apps/sidecar/Dockerfile -t corbits-sidecar:dev .", "eval": "bun run scripts/evals-run.ts" }, diff --git a/packages/agent-directory-tools/package.json b/packages/agent-directory-tools/package.json index 111127e46..c7640058c 100644 --- a/packages/agent-directory-tools/package.json +++ b/packages/agent-directory-tools/package.json @@ -2,7 +2,7 @@ "name": "@corbits/agent-directory-tools", "private": true, "description": "Myra's manager tools: list_agents and create_agent as an @intx/agent tool bundle — creates a new specialist agent definition in the caller's own tenant and, by default, invites it into the caller's own channel. Creation is free; the reactor never parks the create/invite pair", - "version": "0.0.3", + "version": "0.0.4", "license": "LGPL-2.1-or-later", "type": "module", "exports": { diff --git a/scripts/checks/test/tool-package-pins.test.ts b/scripts/checks/test/tool-package-pins.test.ts new file mode 100644 index 000000000..cee871ade --- /dev/null +++ b/scripts/checks/test/tool-package-pins.test.ts @@ -0,0 +1,105 @@ +import { expect, test } from "bun:test"; +import { auditToolPackagePins, extractPins } from "../tool-package-pins"; + +test("extractPins reads a single-line array-entry pin literal", () => { + const contents = [ + "export const PINS = [", + ' { name: "@corbits/memory-tools", version: "0.0.4" },', + ' { name: "@corbits/mcp-tools", version: "0.0.8" },', + "];", + ].join("\n"); + const pins = extractPins("workflows/assistant/src/index.ts", contents); + expect(pins).toEqual([ + { + relPath: "workflows/assistant/src/index.ts", + line: 2, + name: "@corbits/memory-tools", + version: "0.0.4", + }, + { + relPath: "workflows/assistant/src/index.ts", + line: 3, + name: "@corbits/mcp-tools", + version: "0.0.8", + }, + ]); +}); + +test("extractPins reads a multi-line, trailing-comma pin literal", () => { + const contents = [ + "export const SKILLS_TOOL_PACKAGE_PIN = {", + ' name: "@corbits/tools-skills",', + ' version: "0.0.1",', + "} as const;", + ].join("\n"); + const pins = extractPins( + "packages/agent-directory/src/agent-workflow.ts", + contents, + ); + expect(pins).toEqual([ + { + relPath: "packages/agent-directory/src/agent-workflow.ts", + line: 1, + name: "@corbits/tools-skills", + version: "0.0.1", + }, + ]); +}); + +test("extractPins ignores non-@corbits and non-pin-shaped object literals", () => { + const contents = [ + '{ assetName: "granola-call", version: "0.0.1" }', + '{ name: `${recording.server}-fake`, version: "0.0.1" }', + ].join("\n"); + expect(extractPins("irrelevant.ts", contents)).toEqual([]); +}); + +test("a pin matching its package's manifest version is not a violation", () => { + const report = auditToolPackagePins( + [ + { + relPath: "workflows/assistant/src/index.ts", + line: 47, + name: "@corbits/agent-directory-tools", + version: "0.0.4", + }, + ], + new Map([["@corbits/agent-directory-tools", "0.0.4"]]), + ); + expect(report.violations).toEqual([]); +}); + +test("a pin behind its package's manifest version is a violation naming the file, line, and fix", () => { + const report = auditToolPackagePins( + [ + { + relPath: "workflows/assistant/src/index.ts", + line: 49, + name: "@corbits/connections-tools", + version: "0.0.4", + }, + ], + new Map([["@corbits/connections-tools", "0.0.5"]]), + ); + expect(report.violations).toHaveLength(1); + expect(report.violations[0]).toContain("workflows/assistant/src/index.ts:49"); + expect(report.violations[0]).toContain("0.0.4"); + expect(report.violations[0]).toContain("0.0.5"); +}); + +test("a pin naming a package with no workspace manifest is a violation", () => { + const report = auditToolPackagePins( + [ + { + relPath: "workflows/assistant/src/index.ts", + line: 60, + name: "@corbits/does-not-exist", + version: "0.0.1", + }, + ], + new Map([["@corbits/agent-directory-tools", "0.0.4"]]), + ); + expect(report.violations).toHaveLength(1); + expect(report.violations[0]).toContain("@corbits/does-not-exist"); + expect(report.violations[0]).toContain("no workspace package"); +}); diff --git a/scripts/checks/tool-package-pins.ts b/scripts/checks/tool-package-pins.ts new file mode 100644 index 000000000..f52c4be4c --- /dev/null +++ b/scripts/checks/tool-package-pins.ts @@ -0,0 +1,167 @@ +// check:tool-package-pins — a workflow's `{ name, version }` tool-package +// pin literal must match that package's own package.json version. +// +// Tool resolution keys on `name@version`, and every pin is hand-maintained +// (CL-6437): nothing ripples a version bump to the workflows that pin it. +// PR #165 bumped `@corbits/connections-tools` to 0.0.5 and left +// `workflows/assistant/src/index.ts` pinning 0.0.4, breaking that +// workflow's deploy; the same drift hit `@corbits/mcp-tools` during +// CL-6456. Nothing caught either at merge time. This check is the static, +// cheap half of that class (CL-6497): every `{ name: "@corbits/x", +// version: "y" }` literal anywhere in the tree must name the version its +// package.json actually carries, or the pin resolves to a version the +// registry never publishes and every deploy that pins it fails. +// +// It does not (and, in this idiom, practically cannot) catch the other +// half of the class — a package whose `src/` changed without a version +// bump — since that needs the PR's git history (a merge-base diff), not +// a snapshot of the working tree; see CL-6497's PR description for why +// that half is out of scope here. +import { Glob } from "bun"; +import path from "node:path"; +import { + emptyReport, + reportAndExit, + rootFromArgs, + type CheckReport, +} from "./lib/repo"; + +const SCAN_DIRS = ["apps", "packages", "tools", "workflows"]; + +const EXCLUDED_SEGMENTS = ["node_modules", "dist", ".worktrees", "vendor"]; + +// Matches a pin object literal in the shape every pin site in this repo +// uses today: `name` first, then `version`, both string literals, an +// optional trailing comma before the closing brace. `\s` already spans +// newlines, so the multi-line form (`SKILLS_TOOL_PACKAGE_PIN`) matches +// the same as the single-line array-entry form. +const PIN_PATTERN = + /\{\s*name:\s*"(@corbits\/[a-z0-9-]+)"\s*,\s*version:\s*"([^"]*)"\s*,?\s*\}/g; + +export interface PinReference { + readonly relPath: string; + readonly line: number; + readonly name: string; + readonly version: string; +} + +export interface ScannedFile { + readonly relPath: string; + readonly contents: string; +} + +function lineNumberAt(contents: string, index: number): number { + return contents.slice(0, index).split("\n").length; +} + +/** Every `{ name: "@corbits/x", version: "y" }` pin literal in a file. */ +export function extractPins(relPath: string, contents: string): PinReference[] { + const pins: PinReference[] = []; + for (const match of contents.matchAll(PIN_PATTERN)) { + if (match.index === undefined) continue; + const [, name, version] = match; + if (name === undefined || version === undefined) continue; + pins.push({ + relPath, + line: lineNumberAt(contents, match.index), + name, + version, + }); + } + return pins; +} + +/** + * Every pin must name a version equal to its package's own manifest + * version. A pin naming a package with no workspace manifest at all is + * also a violation — that pin can never resolve, whatever version it + * names. + */ +export function auditToolPackagePins( + pins: readonly PinReference[], + manifestVersions: ReadonlyMap, +): CheckReport { + const report = emptyReport(); + for (const pin of [...pins].sort( + (a, b) => a.relPath.localeCompare(b.relPath) || a.line - b.line, + )) { + const manifestVersion = manifestVersions.get(pin.name); + if (manifestVersion === undefined) { + report.violations.push( + `${pin.relPath}:${pin.line}: pins "${pin.name}" but no workspace ` + + `package publishes that name — this pin can never resolve.`, + ); + continue; + } + if (manifestVersion === pin.version) continue; + report.violations.push( + `${pin.relPath}:${pin.line}: pins ${pin.name}@${pin.version} but ` + + `its package.json is at ${manifestVersion} — update the pin to ` + + `"${manifestVersion}" (or, if the manifest is the one that's ` + + `behind, bump the package in the same commit).`, + ); + } + return report; +} + +async function manifestVersions(root: string): Promise> { + const versions = new Map(); + const glob = new Glob("{apps,packages,tools,workflows}/*/package.json"); + for await (const relPath of glob.scan(root)) { + if (relPath.includes("node_modules/")) continue; + const manifest = (await Bun.file(path.join(root, relPath)).json()) as { + name?: string; + version?: string; + }; + if (manifest.name === undefined || manifest.version === undefined) { + continue; + } + versions.set(manifest.name, manifest.version); + } + return versions; +} + +function isExcludedPath(relPath: string): boolean { + return EXCLUDED_SEGMENTS.some( + (segment) => + relPath === segment || + relPath.startsWith(`${segment}/`) || + relPath.includes(`/${segment}/`), + ); +} + +async function scanFiles( + root: string, + dirs: readonly string[], +): Promise { + const files: ScannedFile[] = []; + for (const dir of dirs) { + const glob = new Glob("**/*.ts"); + for await (const file of glob.scan({ cwd: path.join(root, dir) })) { + if (file.endsWith(".test.ts")) continue; + const relPath = path.join(dir, file); + if (isExcludedPath(relPath)) continue; + files.push({ + relPath, + contents: await Bun.file(path.join(root, relPath)).text(), + }); + } + } + return files.sort((a, b) => a.relPath.localeCompare(b.relPath)); +} + +async function main(): Promise { + const root = rootFromArgs(Bun.argv.slice(2)); + const files = await scanFiles(root, SCAN_DIRS); + const pins = files.flatMap((file) => + extractPins(file.relPath, file.contents), + ); + const versions = await manifestVersions(root); + const report = auditToolPackagePins(pins, versions); + report.notes.push( + `${pins.length} pin(s) found across ${files.length} file(s) under ${SCAN_DIRS.join(", ")}`, + ); + reportAndExit("check:tool-package-pins", report); +} + +if (import.meta.main) await main(); diff --git a/workflows/assistant/src/index.ts b/workflows/assistant/src/index.ts index 29fa6885d..7d1846bf8 100644 --- a/workflows/assistant/src/index.ts +++ b/workflows/assistant/src/index.ts @@ -44,7 +44,7 @@ export const ASSISTANT_TOOL_PACKAGE_PINS: readonly ToolPackagePin[] = [ { name: "@corbits/memory-tools", version: "0.0.4" }, { name: "@corbits/capability-tools", version: "0.0.3" }, { name: "@corbits/routines-tools", version: "0.0.5" }, - { name: "@corbits/agent-directory-tools", version: "0.0.3" }, + { name: "@corbits/agent-directory-tools", version: "0.0.4" }, { name: "@corbits/task-dispatch-tools", version: "0.0.3" }, { name: "@corbits/connections-tools", version: "0.0.5" }, { name: "@corbits/catalog-tools", version: "0.0.1" },