diff --git a/CHANGELOG.md b/CHANGELOG.md index 872cbb5f6..fa789eea5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,15 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### Tool selection reads a skill choice the model wrapped in a code fence + +Before a run, the deployment's model picks which of a Bot's skills the message needs, so a Bot holding +many tools is offered only the relevant ones. The request asks for bare JSON, but an endpoint that +ignores `response_format`, as Anthropic's OpenAI-compatible one does, lets the model fence the object +or lead with a sentence. Every such answer read as no answer, so the Bot was offered every tool it +holds and the audit row said `unavailable`. The object is now read out of the answer, the way the +router already reads its own. A bare JSON answer is read as before. + ### A long message reaches the coworker it is for, and is recorded A message over 10,000 characters, such as a pasted email thread or log, was refused by the router diff --git a/server/src/plugins/selection.ts b/server/src/plugins/selection.ts index 9cbd766b8..1cd9314c1 100644 --- a/server/src/plugins/selection.ts +++ b/server/src/plugins/selection.ts @@ -128,9 +128,20 @@ export function readChosenSkills( answer: string, skills: readonly SelectableSkill[], ): string[] | null { + /* + * The object in the answer, not the answer as a whole. + * + * `response_format` asks for bare JSON and does not guarantee it: Anthropic's OpenAI-compatible + * endpoint ignores the field, and a model left to itself often fences its object or leads with a + * sentence. Parsed whole, every such answer read as a selector that could not say, and a Bot on + * that model was offered its entire catalogue on every run. The router reads its answer from the + * same completer this way already (`classify.ts`); an answer with no object in it is still null. + */ + const object = answer.match(/\{[\s\S]*\}/); + if (!object) return null; let parsed: unknown; try { - parsed = JSON.parse(answer); + parsed = JSON.parse(object[0]); } catch { return null; } diff --git a/server/tests/plugin-selection.test.ts b/server/tests/plugin-selection.test.ts index 9ae8f77e3..776d0a061 100644 --- a/server/tests/plugin-selection.test.ts +++ b/server/tests/plugin-selection.test.ts @@ -114,6 +114,23 @@ describe("what gets offered", () => { expect(offered).toContain("drive/tool_0"); expect(offered).toContain("slack/tool_0"); }); + + test("a choice wrapped in a code fence still narrows, rather than reading as no answer", async () => { + const selection = await selectTools({ + tools: manyTools, + skills, + text: "read my drive", + choose: async () => + ["```json", JSON.stringify({ skills: ["drive-audit"] }), "```"].join( + "\n", + ), + }); + expect(selection.reason).toBe("selected"); + expect(selection.skills).toEqual(["drive-audit"]); + expect(selection.offered.map((entry) => entry.ref)).not.toContain( + "slack/tool_0", + ); + }); }); describe("a declaration is not a grant", () => { @@ -274,6 +291,20 @@ describe("reading pass one's answer", () => { expect(readChosenSkills("null", skills)).toBeNull(); expect(readChosenSkills("{}", skills)).toBeNull(); expect(readChosenSkills("not json", skills)).toBeNull(); + expect(readChosenSkills("{skills: drive-audit}", skills)).toBeNull(); + }); + + test("a fenced or padded answer is read, the way the router reads its own", () => { + // `response_format` is a request, not a guarantee: an endpoint that ignores it, as Anthropic's + // OpenAI-compatible one does, lets the model wrap the object in a fence or a sentence. + const fenced = [ + "```json", + JSON.stringify({ skills: ["drive-audit"] }), + "```", + ].join("\n"); + expect(readChosenSkills(fenced, skills)).toEqual(["drive-audit"]); + const padded = `Here is the selection:\n${JSON.stringify({ skills: ["slack-digest"] })}`; + expect(readChosenSkills(padded, skills)).toEqual(["slack-digest"]); }); test("an answer naming only unknown slugs is an empty choice, not a failure", () => {