From 26d6c1255cdf6b776864692c5fcba4c3ad403035 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 04:11:08 -0700 Subject: [PATCH 1/3] Add tests for create_agent model catalog validation and currentVersion string parsing Covers CL-6477 (an invented/unavailable modelPreference must fall back to the tenant's default rather than create a dead agent) and CL-6480 (currentVersion is a string on the wire, and a successful create must still report success and complete the invite). --- .../agent-directory-tools/src/client.test.ts | 32 ++++++++- .../agent-directory-tools/src/tool.test.ts | 72 +++++++++++++++++-- .../test/workflow-create-routes.test.ts | 70 +++++++++++++++++- 3 files changed, 166 insertions(+), 8 deletions(-) diff --git a/packages/agent-directory-tools/src/client.test.ts b/packages/agent-directory-tools/src/client.test.ts index 8d698f0c2..279785f50 100644 --- a/packages/agent-directory-tools/src/client.test.ts +++ b/packages/agent-directory-tools/src/client.test.ts @@ -32,9 +32,13 @@ test("createAgentDefinition posts to the workflow-agent-directory definitions en id: "def_1", name: "Research Buddy", description: null, - currentVersion: 1, + // The real create route serializes a `text` DB column, always + // a string on the wire — never the JS number literal CL-6480 + // let this schema wrongly accept. + currentVersion: "1", status: "deployed", skills: [], + modelNote: null, }), { status: 201 }, ); @@ -57,6 +61,32 @@ test("createAgentDefinition posts to the workflow-agent-directory definitions en systemPrompt: "You are a careful research assistant.", }); expect(result.id).toBe("def_1"); + expect(result.currentVersion).toBe("1"); + expect(result.modelNote).toBeNull(); +}); + +test("createAgentDefinition rejects a response whose currentVersion is a number, not a string", async () => { + const fetchImpl = (async () => + new Response( + JSON.stringify({ + id: "def_1", + name: "Research Buddy", + description: null, + currentVersion: 1, + status: "deployed", + skills: [], + modelNote: null, + }), + { status: 201 }, + )) as unknown as typeof fetch; + + await expect( + createAgentDefinition(testConfig(fetchImpl), { + name: "x", + handle: "x", + systemPrompt: "x", + }), + ).rejects.toThrow(/did not match the expected shape/); }); test("createAgentDefinition throws CreateAgentDefinitionError on a 400", async () => { diff --git a/packages/agent-directory-tools/src/tool.test.ts b/packages/agent-directory-tools/src/tool.test.ts index 7f2f0ee96..d4bd7f45c 100644 --- a/packages/agent-directory-tools/src/tool.test.ts +++ b/packages/agent-directory-tools/src/tool.test.ts @@ -56,6 +56,19 @@ test("create_agent's description does not say a human must approve before anythi ); }); +test("create_agent's modelPreference field tells the model to omit it rather than guess a name", () => { + const bundle = agentDirectoryTools(testEnv()); + const definition = bundle.definitions[1] as unknown as { + inputSchema: { + properties: { modelPreference: { description: string } }; + }; + }; + const description = + definition.inputSchema.properties.modelPreference.description; + expect(description).toMatch(/omit/i); + expect(description).toMatch(/do not (guess|invent)/i); +}); + test("create_agent's input schema requires name and systemPrompt only", () => { const bundle = agentDirectoryTools(testEnv()); const definition = bundle.definitions[1] as unknown as { @@ -142,9 +155,10 @@ test("create_agent creates then invites by default, in one call sequence", async id: "def_1", name: "Research Buddy", description: null, - currentVersion: 1, + currentVersion: "1", status: "deployed", skills: [], + modelNote: null, }), { status: 201 }, ); @@ -189,9 +203,10 @@ test("create_agent with invite: false creates but never calls the invite route", id: "def_1", name: "Research Buddy", description: null, - currentVersion: 1, + currentVersion: "1", status: "deployed", skills: [], + modelNote: null, }), { status: 201 }, ); @@ -225,9 +240,10 @@ test("create_agent reports a create-succeeded/invite-failed half-failure honestl id: "def_1", name: "Research Buddy", description: null, - currentVersion: 1, + currentVersion: "1", status: "deployed", skills: [], + modelNote: null, }), { status: 201 }, ); @@ -265,9 +281,10 @@ test("create_agent maps modelPreference to the create route's model field", asyn id: "def_1", name: "Research Buddy", description: null, - currentVersion: 1, + currentVersion: "1", status: "deployed", skills: [], + modelNote: null, }), { status: 201 }, ); @@ -299,6 +316,53 @@ test("create_agent maps modelPreference to the create route's model field", asyn } }); +test("create_agent surfaces a model fallback note in its content, and still completes the invite, rather than producing a silently dead agent", async () => { + const originalFetch = globalThis.fetch; + globalThis.fetch = (async (url: string | URL) => { + if (String(url).endsWith("/definitions")) { + return new Response( + JSON.stringify({ + id: "def_1", + name: "Research Buddy", + description: null, + currentVersion: "1", + status: "deployed", + skills: [], + modelNote: + 'Requested model "gpt-4o" is not in this workbench\'s catalog; used the workspace default "ollama/llama3" instead.', + }), + { status: 201 }, + ); + } + return new Response( + JSON.stringify({ + address: "ins_1@acme.example", + definitionId: "def_1", + handle: "research-buddy", + }), + { status: 201 }, + ); + }) as unknown as typeof fetch; + try { + const bundle = agentDirectoryTools(testEnv()); + const result = await bundle.run( + callFor(CREATE_AGENT_TOOL, { + name: "Research Buddy", + systemPrompt: "You are a careful research assistant.", + modelPreference: "gpt-4o", + }), + new AbortController().signal, + ); + expect(result.isError).toBeFalsy(); + expect(result.content).toMatch(/Created "Research Buddy"/); + expect(result.content).toMatch(/invited/); + expect(result.content).toMatch(/gpt-4o/); + expect(result.content).toMatch(/ollama\/llama3/); + } finally { + globalThis.fetch = originalFetch; + } +}); + test("create_agent surfaces the create route's own rejection honestly on failure", async () => { const originalFetch = globalThis.fetch; globalThis.fetch = (async () => diff --git a/packages/agent-directory/test/workflow-create-routes.test.ts b/packages/agent-directory/test/workflow-create-routes.test.ts index af56a773d..7ee5a5051 100644 --- a/packages/agent-directory/test/workflow-create-routes.test.ts +++ b/packages/agent-directory/test/workflow-create-routes.test.ts @@ -339,7 +339,7 @@ test("a create with no model bakes the tenant's catalog default in, so the defin expect(written).toContain("anthropic/claude-sonnet"); }); -test("a create with an explicit model never consults the tenant default", async () => { +test("a create with an explicit model the tenant's catalog offers never consults the tenant default", async () => { let writtenFiles: Record | undefined; const app = buildApp({ assetService: fakeAssetService({ @@ -355,16 +355,80 @@ test("a create with an explicit model never consults the tenant default", async const response = await app.request("/definitions", { method: "POST", headers: { "content-type": "application/json", ...AUTH_HEADERS }, + // fakeCapabilityInventory (see above) offers exactly this model. body: JSON.stringify({ name: "Research Buddy", handle: "research-buddy", systemPrompt: "You are a careful research assistant.", - model: "openrouter/some-model", + model: "anthropic/claude-sonnet", }), }); expect(response.status).toBe(201); const written = definitionFrom(writtenFiles); - expect(written).toContain("openrouter/some-model"); + expect(written).toContain("anthropic/claude-sonnet"); + const body = (await response.json()) as { modelNote: string | null }; + expect(body.modelNote).toBeNull(); +}); + +test("a create naming a model outside the tenant's catalog falls back to the tenant default and says so, rather than creating a dead agent", async () => { + let writtenFiles: Record | undefined; + const app = buildApp({ + assetService: fakeAssetService({ + populateAsset: (params) => { + writtenFiles = params.tree.files; + return Promise.resolve({ commitSha: "deadbeef" }); + }, + }), + tenantDefaultModel: (tenantId) => + Promise.resolve( + tenantId === TENANT_ID ? "anthropic/claude-sonnet" : undefined, + ), + }); + const response = await app.request("/definitions", { + method: "POST", + headers: { "content-type": "application/json", ...AUTH_HEADERS }, + body: JSON.stringify({ + name: "Research Buddy", + handle: "research-buddy", + systemPrompt: "You are a careful research assistant.", + model: "gpt-4o", + }), + }); + expect(response.status).toBe(201); + const written = definitionFrom(writtenFiles); + expect(written).not.toContain("gpt-4o"); + expect(written).toContain("anthropic/claude-sonnet"); + const body = (await response.json()) as { modelNote: string | null }; + expect(body.modelNote).toMatch(/gpt-4o/); + expect(body.modelNote).toMatch(/anthropic\/claude-sonnet/); +}); + +test("a create naming a model outside the catalog with no tenant default still creates a working, unpinned agent rather than a dead one", async () => { + let writtenFiles: Record | undefined; + const app = buildApp({ + assetService: fakeAssetService({ + populateAsset: (params) => { + writtenFiles = params.tree.files; + return Promise.resolve({ commitSha: "deadbeef" }); + }, + }), + tenantDefaultModel: () => Promise.resolve(undefined), + }); + const response = await app.request("/definitions", { + method: "POST", + headers: { "content-type": "application/json", ...AUTH_HEADERS }, + body: JSON.stringify({ + name: "Research Buddy", + handle: "research-buddy", + systemPrompt: "You are a careful research assistant.", + model: "gpt-4o", + }), + }); + expect(response.status).toBe(201); + const written = definitionFrom(writtenFiles); + expect(written).not.toContain("gpt-4o"); + const body = (await response.json()) as { modelNote: string | null }; + expect(body.modelNote).toMatch(/gpt-4o/); }); test("an invalid body is a 400", async () => { From 1bbb5c2e7477ef5c392e40de0f50c04705cac1cb Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 04:11:17 -0700 Subject: [PATCH 2/3] CL-6477: validate create_agent's model against the tenant's catalog, fix currentVersion parsing modelPreference is free text a language model supplies at a tool-call boundary. The create route now checks it against the tenant's capability inventory (already resolved for toolPackagePins) and, when it isn't offered, falls back to the tenant's catalog default instead of baking in a name that can never resolve, surfacing the substitution as a modelNote the tool relays honestly. The tool's own description now tells the model to look up a real catalog name or omit the field, rather than inviting a guess. Also fixes CL-6480: the client parsed currentVersion as a number, but the create route serializes a text DB column, so every genuine create failed this shape check and reported as an error, silently dropping the follow-on invite. Parses it as a string, matching the API. --- packages/agent-directory-tools/src/client.ts | 15 ++++++- packages/agent-directory-tools/src/tool.ts | 24 +++++++--- .../src/workflow-create-routes.ts | 44 +++++++++++++++---- 3 files changed, 67 insertions(+), 16 deletions(-) diff --git a/packages/agent-directory-tools/src/client.ts b/packages/agent-directory-tools/src/client.ts index a80017345..0d3b29c59 100644 --- a/packages/agent-directory-tools/src/client.ts +++ b/packages/agent-directory-tools/src/client.ts @@ -40,9 +40,19 @@ export interface CreatedAgentDefinition { readonly id: string; readonly name: string; readonly description: string | null; - readonly currentVersion: number; + /** The create route serializes `workflow_definition.current_version`, + * a `text` DB column, verbatim — always a string on the wire, never + * a number (CL-6480: parsing this as `"number"` made every genuine + * success fail this schema and read as a create failure). */ + readonly currentVersion: string; readonly status: string; readonly skills: readonly string[]; + /** Set when `model` was requested but the tenant's catalog didn't + * offer it, so the route substituted its default (or left the + * definition modelless) instead of baking in a name that can never + * resolve (CL-6477). `null` when the requested model — or its + * absence — needed no substitution. */ + readonly modelNote: string | null; } export interface ListedAgentDefinition { @@ -93,9 +103,10 @@ const CreatedAgentDefinitionResponse = type({ id: "string", name: "string", description: "string | null", - currentVersion: "number", + currentVersion: "string", status: "string", skills: "string[]", + modelNote: "string | null", }); /** Thrown when the create-agent route rejects the request — a bad diff --git a/packages/agent-directory-tools/src/tool.ts b/packages/agent-directory-tools/src/tool.ts index cd2f23289..32ff86459 100644 --- a/packages/agent-directory-tools/src/tool.ts +++ b/packages/agent-directory-tools/src/tool.ts @@ -161,6 +161,14 @@ async function runCreateAgent( return errorResult(call.id, err); } + // Set when the requested `modelPreference` fell outside the tenant's + // catalog and the route substituted its default (or left the + // definition modelless) instead of baking in a name that can never + // resolve (CL-6477) — surfaced on every branch below so the model + // relays the substitution to the user rather than claiming the + // model it originally asked for. + const modelSuffix = created.modelNote !== null ? ` ${created.modelNote}` : ""; + // `invite` defaults to `true` — never require the model to pass it, // only to opt out explicitly. const shouldInvite = call.arguments["invite"] !== false; @@ -168,7 +176,7 @@ async function runCreateAgent( return { callId: call.id, isError: false, - content: `Created "${created.name}" (use this id for routines/dispatch: ${created.id}). It is not in this channel — invite it explicitly if you want it here.`, + content: `Created "${created.name}" (use this id for routines/dispatch: ${created.id}). It is not in this channel — invite it explicitly if you want it here.${modelSuffix}`, }; } @@ -177,7 +185,7 @@ async function runCreateAgent( return { callId: call.id, isError: false, - content: `Created "${created.name}" (use this id for routines/dispatch: ${created.id}) and invited it into this channel.`, + content: `Created "${created.name}" (use this id for routines/dispatch: ${created.id}) and invited it into this channel.${modelSuffix}`, }; } catch (err) { // The agent was genuinely created — that half-success must never @@ -194,7 +202,7 @@ async function runCreateAgent( return { callId: call.id, isError: false, - content: `Created "${created.name}" (use this id for routines/dispatch: ${created.id}), but could not invite it into this channel: ${reason}.`, + content: `Created "${created.name}" (use this id for routines/dispatch: ${created.id}), but could not invite it into this channel: ${reason}.${modelSuffix}`, }; } } @@ -254,8 +262,14 @@ export const agentDirectoryTools = defineTool({ modelPreference: { type: "string", description: - "A canonical model name for the new agent, or omitted " + - "to use the workspace's catalog default.", + "A canonical model name from this workspace's own " + + "connected catalog — check list_agents or a models " + + "listing tool for real names first. Do not guess or " + + "invent a name (e.g. a well-known provider model like " + + '"gpt-4o") on the assumption it is available: a name ' + + "outside this workspace's catalog is never used and " + + "falls back to the workspace default instead. Omit " + + "this field entirely to use that default.", }, invite: { type: "boolean", diff --git a/packages/agent-directory/src/workflow-create-routes.ts b/packages/agent-directory/src/workflow-create-routes.ts index b81c3f724..bec54ebcc 100644 --- a/packages/agent-directory/src/workflow-create-routes.ts +++ b/packages/agent-directory/src/workflow-create-routes.ts @@ -162,11 +162,16 @@ export function createWorkflowAgentCreateRoutes( ); } + // Resolved once, unconditionally: every branch below already needed + // it (the pin check when pins are named, the baseline lookup when + // they aren't), and it is now also the source of truth for `model` + // validation just below. + const inventory = await deps.capabilityInventory.resolve({ + tenantId: scope.tenantId, + principalId: scope.principalId, + }); + if (body.toolPackagePins !== undefined && body.toolPackagePins.length > 0) { - const inventory = await deps.capabilityInventory.resolve({ - tenantId: scope.tenantId, - principalId: scope.principalId, - }); // Throws `CapabilityOutOfInventoryError`, caught by `app.onError` // above — fail closed against exactly the inventory this call // just fetched, never a stale or wider one, for every named pin. @@ -199,17 +204,37 @@ export function createWorkflowAgentCreateRoutes( systemPrompt: body.systemPrompt, skills, }; - if (body.model !== undefined) coreInput.model = body.model; + + // `body.model` is free text a language-model tool call supplied — + // untrusted input at a trust boundary (AGENTS.md). A name the + // tenant's own catalog doesn't offer can never resolve at launch, + // so it is never baked in verbatim: fall back to the tenant's + // catalog default and say so, rather than creating a dead agent + // (CL-6477). A name the catalog does offer is used exactly as + // asked, no fallback consulted. + let modelNote: string | null = null; + if (body.model !== undefined) { + const knownModel = inventory.models.some( + (entry) => entry.canonicalName === body.model, + ); + if (knownModel) { + coreInput.model = body.model; + } else { + const fallback = await deps.tenantDefaultModel?.(scope.tenantId); + modelNote = + fallback !== undefined + ? `Requested model "${body.model}" is not in this workbench's catalog; used the workspace default "${fallback}" instead.` + : `Requested model "${body.model}" is not in this workbench's catalog, and the workspace has no default model to fall back to.`; + if (fallback !== undefined) coreInput.model = fallback; + } + } + if (body.toolPackagePins !== undefined && body.toolPackagePins.length > 0) { coreInput.toolPackagePins = body.toolPackagePins; } else { // No pins named: the specialist still gets the baseline set this // tenant can resolve, so a created "research agent" can actually // search, remember, and ask (CL-6206). - const inventory = await deps.capabilityInventory.resolve({ - tenantId: scope.tenantId, - principalId: scope.principalId, - }); const baseline = baselineAgentToolPins(inventory); if (baseline.length > 0) coreInput.toolPackagePins = baseline; } @@ -236,6 +261,7 @@ export function createWorkflowAgentCreateRoutes( currentVersion: row.currentVersion, status: row.status, skills, + modelNote, }, 201, ); From 575fca414babef6ace2b5ff4f5c07b3b9de14c97 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 04:25:37 -0700 Subject: [PATCH 3/3] Fix manager-tools-scenario fixture for currentVersion/modelNote response shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixture hand-mocked the create-agent-definition response with currentVersion as a number and no modelNote field — CL-6480's exact bug, reproduced in a fixture. With the client's schema now requiring currentVersion as a string and modelNote as string|null, the mocked response failed to parse and create_agent reported isError: true. Updated the fixture to match what the real route actually returns. --- workflows/assistant/test/manager-tools-scenario.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/workflows/assistant/test/manager-tools-scenario.test.ts b/workflows/assistant/test/manager-tools-scenario.test.ts index da0ab297f..b8a5e80cd 100644 --- a/workflows/assistant/test/manager-tools-scenario.test.ts +++ b/workflows/assistant/test/manager-tools-scenario.test.ts @@ -138,9 +138,10 @@ function createFakeHub() { id, name: body["name"], description: null, - currentVersion: 1, + currentVersion: "1", status: "deployed", skills: [], + modelNote: null, }, { status: 201 }, );