Runtime tool-package pins resolve to a published version, never * (CL-7389)

packages/agent-directory/README.md

@@ -7,6 +7,45 @@ and the hub routes that create a definition and manage its attached skills
 (`routes.ts`) — server-side, backed by `@intx/agent`, `@intx/workflow`,
 `@intx/hub-sessions`, and Postgres via `@intx/db`.
 
+## Runtime tool-package pins are always versioned
+
+Every pin this package writes onto a definition (`create_agent`'s
+`toolPackagePins`, guided capability-add's `POST /:definitionId/capabilities`)
+resolves to a concrete, published version — never the npm "any version"
+range `*`, and never a range or tag like `latest`, `^1`, `~1.2`, `>=1.0.0`,
+or `1.x`; `NonWildcardToolPackagePin` (`src/agent-workflow.ts`) requires
+`semver.valid`, an exact version. `resolvePinnedVersion` (and
+`createPinnedVersionResolver`, `src/tool-package-version.ts`) resolves a bare
+package name to `{ name, version }` by reading the tenant's (possibly
+inherited) `corbits-tools` `package-registry` asset and picking the highest
+version among its tarballs — preferring a stable version over a
+higher-sorting prerelease; a prerelease only wins when the registry carries
+no stable version for that package at all. Resolution throws the same
+`CapabilityOutOfInventoryError` guided capability-add's inventory check
+throws when the registry or the package is absent, so every caller's
+existing 4xx mapping covers it with no new wiring. `create_agent`'s own
+multi-pin create shares one `createPinnedVersionResolver` across every named
+pin, so a five-pin create still costs one registry lookup and one tarball
+listing, not five.
+
+This matters because a `*` (or any non-exact) pin would let a later tarball
+landing in the registry silently change what an already-deployed specialist
+runs, with no record of the change (CL-7389). Resolving a version only
+happens when a pin is newly added or a definition is newly created with
+named pins:
+
+- A plain redeploy of an existing definition (`PUT /:definitionId`) never
+  touches tool-package pins at all, let alone re-resolves one — it keeps
+  whatever version its existing pins already carry, even after a newer
+  tarball lands in the registry.
+- Guided capability-add re-adding a package **already pinned** on the
+  definition also keeps its existing stored version rather than
+  re-resolving — re-adding is a no-op on the version, never a silent bump
+  to whatever the registry's newest tarball happens to be today. Only a
+  name with no existing pin resolves fresh against the registry. Bumping an
+  existing pin to a newer published version is a distinct, explicit action
+  this package does not yet expose.
+
 ## `/client` subpath contract
 
 `@corbits/agent-directory/client` (`src/client.ts`) is the browser-safe

packages/agent-directory/package.json

@@ -18,6 +18,7 @@
     "@corbits/error-sink": "workspace:*",
     "@corbits/folded-run-one-shot": "workspace:*",
     "@corbits/skills": "workspace:*",
+    "@corbits/tool-registry-publish": "workspace:*",
     "@workbench/templates": "workspace:*",
     "@corbits/workflows": "workspace:*",
     "@intx/agent": "workspace:*",
@@ -31,12 +32,13 @@
     "drizzle-orm": "catalog:",
     "hono": "catalog:",
     "isomorphic-git": "catalog:",
-    "postgres": "catalog:"
+    "postgres": "catalog:",
+    "semver": "catalog:"
   },
   "devDependencies": {
-    "@corbits/tool-registry-publish": "workspace:*",
     "@intx/crypto": "0.3.0",
     "@types/bun": "catalog:",
+    "@types/semver": "catalog:",
     "typescript": "catalog:"
   }
 }

packages/agent-directory/src/agent-workflow.test.ts

@@ -4,8 +4,17 @@
 // a definition pinning skills carried a pin the corbits-tools registry
 // could never resolve at launch.
 import { describe, expect, test } from "bun:test";
+import type { DB } from "@intx/db";
+import type { AssetService } from "@intx/hub-sessions";
 import { describeCorbitsToolPackages } from "@corbits/tool-registry-publish";
-import { SKILLS_TOOL_PACKAGE_PIN } from "./agent-workflow";
+import {
+  buildAgentDefinitionWorkflow,
+  createAgentDefinitionCore,
+  serializeAgentDefinitionWorkflow,
+  SKILLS_TOOL_PACKAGE_PIN,
+  withAgentToolPackagePin,
+} from "./agent-workflow";
+import { createInMemoryDefinitionSkillsStore } from "./skills-store";
 
 describe("SKILLS_TOOL_PACKAGE_PIN", () => {
   test("resolves through the corbits-tools registry", async () => {
@@ -17,3 +26,165 @@ describe("SKILLS_TOOL_PACKAGE_PIN", () => {
     expect(match?.version).toBe(SKILLS_TOOL_PACKAGE_PIN.version);
   });
 });
+
+// CL-7389: a runtime tool-package pin must resolve to a concrete,
+// published version — never the npm "any version" range `*` — so a
+// later tarball landing in the registry never silently changes what an
+// already-deployed specialist runs.
+describe("withAgentToolPackagePin", () => {
+  function freshWorkflowJson(): string {
+    return serializeAgentDefinitionWorkflow(
+      buildAgentDefinitionWorkflow({
+        handle: "pin-test",
+        tenantDomain: "example.test",
+        description: "",
+        systemPrompt: "You are a test agent.",
+      }),
+    );
+  }
+
+  test("rejects a wildcard version", () => {
+    expect(() =>
+      withAgentToolPackagePin(freshWorkflowJson(), {
+        name: "@corbits/memory-tools",
+        version: "*",
+      }),
+    ).toThrow(/never "\*"/);
+  });
+
+  test.each(["latest", "^1", "~1.2", "", ">=1.0.0", "1.x"])(
+    "rejects a non-exact version %p",
+    (version) => {
+      expect(() =>
+        withAgentToolPackagePin(freshWorkflowJson(), {
+          name: "@corbits/memory-tools",
+          version,
+        }),
+      ).toThrow();
+    },
+  );
+
+  test("accepts a concrete version", () => {
+    const nextWorkflowJson = withAgentToolPackagePin(freshWorkflowJson(), {
+      name: "@corbits/memory-tools",
+      version: "1.2.3",
+    });
+    const definition = JSON.parse(nextWorkflowJson) as {
+      steps: Record<
+        string,
+        { agent: { toolPackagePins?: { name: string; version: string }[] } }
+      >;
+    };
+    const [step] = Object.values(definition.steps);
+    expect(step?.agent.toolPackagePins).toContainEqual({
+      name: "@corbits/memory-tools",
+      version: "1.2.3",
+    });
+  });
+});
+
+// CL-7389: a `create_agent`/`POST /agent-definitions` call pinning several
+// tool packages by name shares one registry resolver across all of them
+// (`createPinnedVersionResolver`), so it costs one ancestor walk and one
+// tarball listing, never one per pin.
+describe("createAgentDefinitionCore: shared registry resolution across pins", () => {
+  test("a five-pin create does one tenant lookup, one asset lookup, and one tarball listing", async () => {
+    const counters = { tenant: 0, asset: 0, list: 0 };
+    const db = {
+      query: {
+        tenant: {
+          findFirst: async () => {
+            counters.tenant++;
+            return { parentId: null };
+          },
+        },
+        asset: {
+          findFirst: async () => {
+            counters.asset++;
+            return {
+              id: "ast_corbits_tools",
+              tenantId: "tnt_1",
+              kind: "package-registry" as const,
+              name: "corbits-tools",
+            };
+          },
+        },
+        workflowDefinition: {
+          findFirst: async () => ({
+            id: "def_1",
+            tenantId: "tnt_1",
+            assetId: "ast_1",
+            name: "scout",
+            description: null,
+            currentVersion: "1",
+            status: "deployed",
+            createdAt: new Date(),
+            updatedAt: new Date(),
+          }),
+        },
+      },
+    } as unknown as DB["db"];
+    const assetService: AssetService = {
+      createAsset: () =>
+        Promise.resolve({
+          id: "ast_1",
+          tenantId: "tnt_1",
+          kind: "workflow" as const,
+          name: "scout",
+          displayName: "Scout",
+          creatorPrincipalId: "prn_1",
+          createdAt: new Date(),
+          updatedAt: new Date(),
+        }),
+      populateAsset: () => Promise.resolve({ commitSha: "deadbeef" }),
+      readAssetBlob: () => {
+        throw new Error("not used");
+      },
+      listAssetBlobs: () => {
+        counters.list++;
+        return Promise.resolve([
+          "corbits-a-tools-1.0.0.tgz",
+          "corbits-b-tools-1.0.0.tgz",
+          "corbits-c-tools-1.0.0.tgz",
+          "corbits-d-tools-1.0.0.tgz",
+          "corbits-e-tools-1.0.0.tgz",
+        ]);
+      },
+    };
+    await createAgentDefinitionCore(
+      {
+        db,
+        assetService,
+        skillIndex: { resolve: () => Promise.resolve([]) },
+        skillsStore: createInMemoryDefinitionSkillsStore(),
+        deployer: {
+          deploy: () =>
+            Promise.resolve({
+              deploymentId: "dep_1",
+              definitionAssetId: "ast_1",
+              status: "deployed" as const,
+            }),
+        },
+      },
+      {
+        tenantId: "tnt_1",
+        principalId: "prn_1",
+        tenantDomain: "acme.example",
+        handle: "scout",
+        name: "Scout",
+        systemPrompt: "You are Scout.",
+        skills: [],
+        toolPackagePins: [
+          "@corbits/a-tools",
+          "@corbits/b-tools",
+          "@corbits/c-tools",
+          "@corbits/d-tools",
+          "@corbits/e-tools",
+        ],
+      },
+    );
+    expect(counters.list).toBe(1);
+    expect(counters.asset).toBe(1);
+    expect(counters.tenant).toBe(1);
+  });
+});

packages/agent-directory/src/agent-workflow.ts

@@ -26,12 +26,14 @@ import {
   type PinnedSkillIndexEntry,
 } from "@corbits/skills";
 import { type } from "arktype";
+import semver from "semver";
 
 import {
   writeAndDeployAgentDefinition,
   type AgentDefinitionDeployer,
 } from "./definition-asset";
 import type { DefinitionSkillsStore } from "./skills-store";
+import { createPinnedVersionResolver } from "./tool-package-version";
 
 export const AGENT_DEFINITION_STEP_ID = "agent";
 
@@ -199,15 +201,47 @@ export function readAgentCapabilities(
     : { toolPackagePins: step.agent.toolPackagePins ?? [] };
 }
 
+/**
+ * A tool-package pin resolved to a concrete, published version — never
+ * the npm "any version" range `*`. `ToolPackagePin` itself (the wire
+ * type `@intx/types/tool-packages` declares) still accepts `*`, because
+ * an operator hand-authoring a workflow source file is free to write
+ * one; but every RUNTIME site this package writes a pin from
+ * (`create_agent`'s tool-package pins, guided capability-add) must name
+ * a version the resolver actually offers today (CL-7389) — a `*` pin
+ * means a later tarball landing in the registry silently changes what
+ * an already-deployed specialist runs, with no record of the change.
+ * `./tool-package-version.ts`'s `resolvePinnedVersion` is how a caller
+ * that only has a package name gets one of these.
+ */
+export const NonWildcardToolPackagePin = type({
+  name: "string",
+  version: "string",
+}).narrow((pin, ctx) =>
+  semver.valid(pin.version) !== null
+    ? true
+    : ctx.mustBe(
+        'a concrete published version, never "*", "latest", or a range/tag like "^1", "~1.2", ">=1.0.0", "1.x" — anything but an exact version would let a later tarball silently change what this pin resolves to (CL-7389)',
+      ),
+);
+export type NonWildcardToolPackagePin = typeof NonWildcardToolPackagePin.infer;
+
 /** Adds or replaces one tool-package pin by name, leaving every other
  * pin — including the skills bundle `reindexPinnedSkills` manages —
  * untouched. Mirrors `withSkillsToolPin`'s replace-by-name shape,
  * generalized to a caller-supplied pin rather than the fixed skills
- * bundle. */
+ * bundle. Rejects a `"*"` version outright (see `NonWildcardToolPackagePin`) —
+ * every runtime caller must supply a concrete, resolved version. */
 export function withAgentToolPackagePin(
   workflowJson: string,
-  pin: ToolPackagePin,
+  pin: NonWildcardToolPackagePin,
 ): string {
+  const parsedPin = NonWildcardToolPackagePin(pin);
+  if (parsedPin instanceof type.errors) {
+    throw new Error(
+      `withAgentToolPackagePin: pin must be ${parsedPin.summary}`,
+    );
+  }
   const raw: unknown = JSON.parse(workflowJson);
   const definition = DefinitionWithAgentSteps(raw);
   if (definition instanceof type.errors) {
@@ -217,9 +251,9 @@ export function withAgentToolPackagePin(
   }
   for (const step of Object.values(definition.steps)) {
     const others = (step.agent.toolPackagePins ?? []).filter(
-      (existing) => existing.name !== pin.name,
+      (existing) => existing.name !== parsedPin.name,
     );
-    step.agent.toolPackagePins = [...others, { ...pin }];
+    step.agent.toolPackagePins = [...others, { ...parsedPin }];
   }
   return JSON.stringify(definition);
 }
@@ -503,11 +537,17 @@ export async function createAgentDefinitionCore(
       input.skills,
     ),
   );
+  // One resolver shared across every named pin: it loads the tenant's
+  // registry asset and tarball listing at most once, so a five-pin
+  // create still costs one ancestor walk and one listing, not five
+  // (CL-7389).
+  const resolvePin = createPinnedVersionResolver(
+    { db: deps.db, assetService: deps.assetService },
+    input.tenantId,
+  );
   for (const name of input.toolPackagePins ?? []) {
-    workflowJson = withAgentToolPackagePin(workflowJson, {
-      name,
-      version: "*",
-    });
+    const resolvedPin = await resolvePin(name);
+    workflowJson = withAgentToolPackagePin(workflowJson, resolvedPin);
   }
 
   let assetId: string;