From 143ba32b11ca1cba4f8e43535508fe31eadc1423 Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 20 May 2026 22:28:04 +0000 Subject: [PATCH] fix(coding-agent): forced ollama tool choice Route /force through a named Ollama tool choice and scope the Ollama request tools to that selected name so local models cannot pick a different tool.\n\nFixes #1236 --- packages/ai/CHANGELOG.md | 1 + packages/ai/src/providers/ollama.ts | 27 ++++++++- packages/ai/test/ollama-provider.test.ts | 59 +++++++++++++++++++ packages/coding-agent/CHANGELOG.md | 4 ++ .../coding-agent/src/utils/tool-choice.ts | 14 ++--- .../test/slash-commands/force.test.ts | 19 ++++++ 6 files changed, 116 insertions(+), 8 deletions(-) diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 2aa8bf2aa..25563756c 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixed +- Fixed Ollama named tool forcing to send only the requested tool when the caller passes a named `toolChoice`, preserving `tool_choice: "required"` while preventing local models from selecting a different tool. ([#1236](https://github.com/can1357/oh-my-pi/issues/1236)) - Fixed `/btw` (and IRC background replies) returning a `BedrockException` 400 (`The toolConfig field must be defined when using toolUse and toolResult content blocks.`) on LiteLLM → Bedrock once the session has tool-call history. Two source fixes in `buildParams`: (1) `if (context.tools)` → `if (context.tools?.length)` so an explicit `context.tools = []` (the /btw opt-out) never routes through `convertTools` and never emits an empty `"tools"` array; (2) `else if (hasToolHistory(...))` → `else if (context.tools === undefined && hasToolHistory(...))` so the Anthropic-proxy sentinel that injects `tools: []` for tool-history turns is suppressed when the caller explicitly opted out, preventing it from re-introducing the empty array. As defence-in-depth, `tool_choice: "none"` is also dropped when the resolved tools list is missing or empty. ([#1227](https://github.com/can1357/oh-my-pi/issues/1227)) ## [15.1.8] - 2026-05-20 diff --git a/packages/ai/src/providers/ollama.ts b/packages/ai/src/providers/ollama.ts index d0da9295d..820b10fa1 100644 --- a/packages/ai/src/providers/ollama.ts +++ b/packages/ai/src/providers/ollama.ts @@ -116,6 +116,29 @@ function mapToolChoice(toolChoice: ToolChoice | undefined): "auto" | "none" | "r return undefined; } +function getNamedToolChoiceName(toolChoice: ToolChoice | undefined): string | undefined { + if (!toolChoice || typeof toolChoice === "string") { + return undefined; + } + if ("function" in toolChoice) { + return toolChoice.function.name; + } + return toolChoice.name; +} + +function selectToolsForToolChoice(tools: Tool[] | undefined, toolChoice: ToolChoice | undefined): Tool[] | undefined { + const toolName = getNamedToolChoiceName(toolChoice); + if (!toolName || !tools) { + return tools; + } + for (const tool of tools) { + if (tool.name === toolName) { + return [tool]; + } + } + return []; +} + function toPlainContent(content: string | Array<{ type: "text" | "image"; text?: string; data?: string }>): { content: string; images?: string[]; @@ -231,10 +254,12 @@ function convertTools(tools: Tool[] | undefined): OllamaFunctionTool[] | undefin function createChatBody(model: Model<"ollama-chat">, context: Context, options: OllamaChatOptions | undefined) { const think = mapReasoning(options?.reasoning); const toolChoice = mapToolChoice(options?.toolChoice); + const selectedTools = selectToolsForToolChoice(context.tools, options?.toolChoice); + const tools = convertTools(selectedTools); return { model: model.id, messages: convertMessages(model, context), - ...(convertTools(context.tools) ? { tools: convertTools(context.tools) } : {}), + ...(tools ? { tools } : {}), ...(think !== undefined ? { think } : {}), ...(toolChoice !== undefined ? { tool_choice: toolChoice } : {}), ...(options?.maxTokens !== undefined ? { options: { num_predict: options.maxTokens } } : {}), diff --git a/packages/ai/test/ollama-provider.test.ts b/packages/ai/test/ollama-provider.test.ts index db5cd0181..164d17812 100644 --- a/packages/ai/test/ollama-provider.test.ts +++ b/packages/ai/test/ollama-provider.test.ts @@ -1,6 +1,8 @@ import { afterEach, describe, expect, test, vi } from "bun:test"; import { Effort } from "../src/model-thinking"; import { ollamaModelManagerOptions } from "../src/provider-models/openai-compat"; +import { streamOllama } from "../src/providers/ollama"; +import type { Context, Model, Tool } from "../src/types"; const originalFetch = global.fetch; @@ -9,6 +11,11 @@ afterEach(() => { vi.restoreAllMocks(); }); +interface OllamaRequestBody { + tools?: Array<{ function: { name: string } }>; + tool_choice?: string; +} + describe("ollama local provider discovery", () => { test("applies /api/show context and thinking capabilities to OpenAI-compatible local models", async () => { global.fetch = vi.fn(async (input, init) => { @@ -47,3 +54,55 @@ describe("ollama local provider discovery", () => { expect(model?.input).toEqual(["text", "image"]); }); }); + +describe("ollama tool forcing", () => { + test("limits named forced tool requests to the selected tool", async () => { + let requestBody: OllamaRequestBody | undefined; + global.fetch = vi.fn(async (_input, init) => { + requestBody = JSON.parse(String(init?.body ?? "{}")) as OllamaRequestBody; + return new Response(`${JSON.stringify({ done: true })}\n`, { + status: 200, + headers: { "Content-Type": "application/x-ndjson" }, + }); + }) as unknown as typeof fetch; + + const model = { + id: "ggml-org/gemma-3-1b-it/GGUF", + name: "Gemma 3 1B", + api: "ollama-chat", + provider: "ollama", + baseUrl: "http://127.0.0.1:11434", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 32_768, + maxTokens: 8_192, + } satisfies Model<"ollama-chat">; + const readTool = { + name: "read", + description: "Read a file", + parameters: { type: "object", properties: {}, additionalProperties: false }, + } satisfies Tool; + const writeTool = { + name: "write", + description: "Write a file", + parameters: { type: "object", properties: {}, additionalProperties: false }, + } satisfies Tool; + const context = { + messages: [{ role: "user", content: "Create README.md", timestamp: Date.now() }], + tools: [readTool, writeTool], + } satisfies Context; + + const eventTypes: string[] = []; + for await (const event of streamOllama(model, context, { + apiKey: "test-key", + toolChoice: { type: "function", name: "write" }, + })) { + eventTypes.push(event.type); + } + + expect(eventTypes).toContain("done"); + expect(requestBody?.tool_choice).toBe("required"); + expect(requestBody?.tools?.map(tool => tool.function.name)).toEqual(["write"]); + }); +}); diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6526c6481..1f3cfe376 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `/force ` rejecting Ollama/local models before the requested tool could run; Ollama now receives a named forced choice that the provider transport narrows to the selected tool. ([#1236](https://github.com/can1357/oh-my-pi/issues/1236)) + ## [15.1.8] - 2026-05-20 ### Fixed diff --git a/packages/coding-agent/src/utils/tool-choice.ts b/packages/coding-agent/src/utils/tool-choice.ts index 323ed716e..49956a2ac 100644 --- a/packages/coding-agent/src/utils/tool-choice.ts +++ b/packages/coding-agent/src/utils/tool-choice.ts @@ -2,7 +2,8 @@ import type { Api, Model, ToolChoice } from "@oh-my-pi/pi-ai"; /** * Build a provider-aware tool choice that targets one specific tool when supported. - * Some providers only support "any tool" forcing, not a named tool. + * Providers that only expose required/any forcing may still honor named choices by + * narrowing their request tool list before transport. */ export function buildNamedToolChoice(toolName: string, model?: Model): ToolChoice | undefined { if (!model) return undefined; @@ -20,12 +21,11 @@ export function buildNamedToolChoice(toolName: string, model?: Model): Tool return { type: "function", name: toolName }; } - if ( - model.api === "google-generative-ai" || - model.api === "google-gemini-cli" || - model.api === "google-vertex" || - model.api === "ollama-chat" - ) { + if (model.api === "ollama-chat") { + return { type: "function", name: toolName }; + } + + if (model.api === "google-generative-ai" || model.api === "google-gemini-cli" || model.api === "google-vertex") { return "required"; } diff --git a/packages/coding-agent/test/slash-commands/force.test.ts b/packages/coding-agent/test/slash-commands/force.test.ts index 88f27751c..28ce73f31 100644 --- a/packages/coding-agent/test/slash-commands/force.test.ts +++ b/packages/coding-agent/test/slash-commands/force.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it, vi } from "bun:test"; +import type { Model } from "@oh-my-pi/pi-ai"; import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; import { executeBuiltinSlashCommand } from "@oh-my-pi/pi-coding-agent/slash-commands/builtin-registry"; +import { buildNamedToolChoice } from "@oh-my-pi/pi-coding-agent/utils/tool-choice"; function createRuntimeHarness(overrides?: { setForcedToolChoice?: (toolName: string) => void }) { const setForcedToolChoice = vi.fn(overrides?.setForcedToolChoice ?? ((_toolName: string) => {})); @@ -95,4 +97,21 @@ describe("/force slash command", () => { expect(harness.showStatus).not.toHaveBeenCalled(); expect(harness.setText).toHaveBeenCalledWith(""); }); + + it("builds a named Ollama choice for local forced tools", () => { + const model = { + id: "ggml-org/gemma-3-1b-it/GGUF", + name: "Gemma 3 1B", + api: "ollama-chat", + provider: "ollama", + baseUrl: "http://127.0.0.1:11434", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 32_768, + maxTokens: 8_192, + } satisfies Model<"ollama-chat">; + + expect(buildNamedToolChoice("write", model)).toEqual({ type: "function", name: "write" }); + }); });