From 12ee7f696f05ab3ec8830917382a5583b57e92b9 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 29 Apr 2026 21:20:41 +0100 Subject: [PATCH] fix(ollama): avoid broad provider hooks for local runs --- CHANGELOG.md | 1 + .../model.skip-pi-discovery-hooks.test.ts | 79 +++++++++++++++++++ src/agents/pi-embedded-runner/model.ts | 29 ++++++- src/agents/simple-completion-runtime.test.ts | 6 +- src/agents/simple-completion-runtime.ts | 3 +- src/cli/capability-cli.ts | 1 + 6 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 src/agents/pi-embedded-runner/model.skip-pi-discovery-hooks.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 52c90ee26fa..c137525d66d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ Docs: https://docs.openclaw.ai - CLI/agent: isolate Gateway-timeout embedded fallback runs under explicit `gateway-fallback-*` sessions so accepted Gateway runs cannot race transcript locks or replace the routed conversation session. Fixes #62981. Thanks @HemantSudarshan. - CLI/QR/device-pair: reject malformed public setup URLs before issuing mobile pairing bootstrap tokens, while keeping valid bare host:port setup URLs supported. Thanks @Lucenx9. - Models/UI: hide unauthenticated providers from the default Web chat, `/models`, and model setup pickers while keeping explicit full-catalog browse paths through `view: "all"`, `/models all`, and `models list --all`. Fixes #74423. Thanks @guarismo and @SymbolStar. +- Ollama: keep explicit local model runs on target-provider runtime hooks when PI discovery is skipped, so one-shot Ollama calls no longer cold-load unrelated provider runtimes before streaming. Fixes #74078. Thanks @sakalaboator. - Slack/prompts: rely on Slack `interactiveReplies` guidance instead of generic `inlineButtons` config hints so enabled Slack button directives are not contradicted. Fixes #46647. Thanks @jeremykoerber. - Slack/reactions: treat duplicate `already_reacted` responses as idempotent success so repeated agent reaction adds no longer surface as tool failures. Fixes #69005. Thanks @shipitsteven and @martingarramon. - Slack/tools: expose `fileId` in the shared message tool schema so `download-file` can receive Slack attachment IDs from inbound placeholders. Fixes #45574. Thanks @chadvegas. diff --git a/src/agents/pi-embedded-runner/model.skip-pi-discovery-hooks.test.ts b/src/agents/pi-embedded-runner/model.skip-pi-discovery-hooks.test.ts new file mode 100644 index 00000000000..90c037c02e0 --- /dev/null +++ b/src/agents/pi-embedded-runner/model.skip-pi-discovery-hooks.test.ts @@ -0,0 +1,79 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + discoverAuthStorage: vi.fn(() => ({ mocked: true })), + discoverModels: vi.fn(() => ({ find: vi.fn(() => null) })), + applyProviderResolvedModelCompatWithPlugins: vi.fn(() => { + throw new Error("compat hook should not run during skipPiDiscovery"); + }), + applyProviderResolvedTransportWithPlugin: vi.fn(() => { + throw new Error("transport hook should not run during skipPiDiscovery"); + }), + buildProviderUnknownModelHintWithPlugin: vi.fn(() => undefined), + normalizeProviderResolvedModelWithPlugin: vi.fn(() => undefined), + normalizeProviderTransportWithPlugin: vi.fn(() => { + throw new Error("transport normalization hook should not run during skipPiDiscovery"); + }), + prepareProviderDynamicModel: vi.fn(async () => undefined), + runProviderDynamicModel: vi.fn( + ({ context }: { context: { provider: string; modelId: string } }) => ({ + id: context.modelId, + name: context.modelId, + provider: context.provider, + api: "ollama", + baseUrl: "http://127.0.0.1:11434", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 8192, + maxTokens: 1024, + }), + ), + shouldPreferProviderRuntimeResolvedModel: vi.fn(() => false), +})); + +vi.mock("../pi-model-discovery.js", () => ({ + discoverAuthStorage: mocks.discoverAuthStorage, + discoverModels: mocks.discoverModels, +})); + +vi.mock("../../plugins/provider-runtime.js", () => ({ + applyProviderResolvedModelCompatWithPlugins: mocks.applyProviderResolvedModelCompatWithPlugins, + applyProviderResolvedTransportWithPlugin: mocks.applyProviderResolvedTransportWithPlugin, + buildProviderUnknownModelHintWithPlugin: mocks.buildProviderUnknownModelHintWithPlugin, + normalizeProviderResolvedModelWithPlugin: mocks.normalizeProviderResolvedModelWithPlugin, + normalizeProviderTransportWithPlugin: mocks.normalizeProviderTransportWithPlugin, + prepareProviderDynamicModel: mocks.prepareProviderDynamicModel, + runProviderDynamicModel: mocks.runProviderDynamicModel, + shouldPreferProviderRuntimeResolvedModel: mocks.shouldPreferProviderRuntimeResolvedModel, +})); + +let resolveModelAsync: typeof import("./model.js").resolveModelAsync; + +beforeEach(async () => { + vi.clearAllMocks(); + ({ resolveModelAsync } = await import("./model.js")); +}); + +describe("resolveModelAsync skipPiDiscovery runtime hooks", () => { + it("uses only target-provider dynamic hooks", async () => { + const result = await resolveModelAsync("ollama", "llama3.2:latest", "/tmp/agent", undefined, { + skipPiDiscovery: true, + }); + + expect(result.error).toBeUndefined(); + expect(result.model).toMatchObject({ + provider: "ollama", + id: "llama3.2:latest", + api: "ollama", + }); + expect(mocks.discoverAuthStorage).not.toHaveBeenCalled(); + expect(mocks.discoverModels).not.toHaveBeenCalled(); + expect(mocks.prepareProviderDynamicModel).toHaveBeenCalledTimes(1); + expect(mocks.runProviderDynamicModel).toHaveBeenCalledTimes(1); + expect(mocks.normalizeProviderResolvedModelWithPlugin).toHaveBeenCalledTimes(1); + expect(mocks.applyProviderResolvedModelCompatWithPlugins).not.toHaveBeenCalled(); + expect(mocks.applyProviderResolvedTransportWithPlugin).not.toHaveBeenCalled(); + expect(mocks.normalizeProviderTransportWithPlugin).not.toHaveBeenCalled(); + }); +}); diff --git a/src/agents/pi-embedded-runner/model.ts b/src/agents/pi-embedded-runner/model.ts index 1dddf97fca5..a3a98a2c85a 100644 --- a/src/agents/pi-embedded-runner/model.ts +++ b/src/agents/pi-embedded-runner/model.ts @@ -67,14 +67,23 @@ type ProviderRuntimeHooks = { ) => unknown; }; -const DEFAULT_PROVIDER_RUNTIME_HOOKS: ProviderRuntimeHooks = { - applyProviderResolvedModelCompatWithPlugins, - applyProviderResolvedTransportWithPlugin, +const TARGET_PROVIDER_RUNTIME_HOOKS: ProviderRuntimeHooks = { buildProviderUnknownModelHintWithPlugin, prepareProviderDynamicModel, runProviderDynamicModel, shouldPreferProviderRuntimeResolvedModel, normalizeProviderResolvedModelWithPlugin, + // Target-provider resolution keeps owner hooks, but avoids broad + // cross-provider hooks that can load unrelated bundled provider runtimes. + applyProviderResolvedModelCompatWithPlugins: () => undefined, + applyProviderResolvedTransportWithPlugin: () => undefined, + normalizeProviderTransportWithPlugin: () => undefined, +}; + +const DEFAULT_PROVIDER_RUNTIME_HOOKS: ProviderRuntimeHooks = { + ...TARGET_PROVIDER_RUNTIME_HOOKS, + applyProviderResolvedModelCompatWithPlugins, + applyProviderResolvedTransportWithPlugin, normalizeProviderTransportWithPlugin, }; @@ -88,6 +97,11 @@ const STATIC_PROVIDER_RUNTIME_HOOKS: ProviderRuntimeHooks = { normalizeProviderTransportWithPlugin: () => undefined, }; +const SKIP_PI_DISCOVERY_PROVIDER_RUNTIME_HOOKS: ProviderRuntimeHooks = { + // skipPiDiscovery is the lean path used before PI discovery/models.json has run. + ...TARGET_PROVIDER_RUNTIME_HOOKS, +}; + function createEmptyPiDiscoveryStores(): { authStorage: AuthStorage; modelRegistry: ModelRegistry; @@ -106,11 +120,18 @@ function createEmptyPiDiscoveryStores(): { function resolveRuntimeHooks(params?: { runtimeHooks?: ProviderRuntimeHooks; skipProviderRuntimeHooks?: boolean; + skipPiDiscovery?: boolean; }): ProviderRuntimeHooks { if (params?.skipProviderRuntimeHooks) { return STATIC_PROVIDER_RUNTIME_HOOKS; } - return params?.runtimeHooks ?? DEFAULT_PROVIDER_RUNTIME_HOOKS; + if (params?.runtimeHooks) { + return params.runtimeHooks; + } + if (params?.skipPiDiscovery) { + return SKIP_PI_DISCOVERY_PROVIDER_RUNTIME_HOOKS; + } + return DEFAULT_PROVIDER_RUNTIME_HOOKS; } function canonicalizeLegacyResolvedModel(params: { diff --git a/src/agents/simple-completion-runtime.test.ts b/src/agents/simple-completion-runtime.test.ts index 06eebe6f682..b2c0a214687 100644 --- a/src/agents/simple-completion-runtime.test.ts +++ b/src/agents/simple-completion-runtime.test.ts @@ -487,6 +487,9 @@ describe("completeWithPreparedSimpleCompletionModel", () => { ...model, api: "openclaw-ollama-simple-test", }; + const cfg = { + models: { providers: { ollama: { baseUrl: "http://remote-ollama:11434", models: [] } } }, + }; hoisted.prepareModelForSimpleCompletionMock.mockReturnValueOnce(preparedModel); await completeWithPreparedSimpleCompletionModel({ @@ -496,12 +499,13 @@ describe("completeWithPreparedSimpleCompletionModel", () => { source: "models.json (local marker)", mode: "api-key", }, + cfg, context: { messages: [{ role: "user", content: "pong", timestamp: 1 }], }, }); - expect(hoisted.prepareModelForSimpleCompletionMock).toHaveBeenCalledWith({ model }); + expect(hoisted.prepareModelForSimpleCompletionMock).toHaveBeenCalledWith({ model, cfg }); expect(hoisted.completeMock).toHaveBeenCalledWith( preparedModel, { diff --git a/src/agents/simple-completion-runtime.ts b/src/agents/simple-completion-runtime.ts index 04ee880b5e7..9e5604176be 100644 --- a/src/agents/simple-completion-runtime.ts +++ b/src/agents/simple-completion-runtime.ts @@ -278,9 +278,10 @@ export async function completeWithPreparedSimpleCompletionModel(params: { model: Model; auth: ResolvedProviderAuth; context: Parameters[1]; + cfg?: OpenClawConfig; options?: SimpleCompletionModelOptions; }) { - const completionModel = prepareModelForSimpleCompletion({ model: params.model }); + const completionModel = prepareModelForSimpleCompletion({ model: params.model, cfg: params.cfg }); return await complete(completionModel, params.context, { ...params.options, apiKey: params.auth.apiKey, diff --git a/src/cli/capability-cli.ts b/src/cli/capability-cli.ts index 64f1d2abee7..185e0e275ed 100644 --- a/src/cli/capability-cli.ts +++ b/src/cli/capability-cli.ts @@ -656,6 +656,7 @@ async function runModelRun(params: { const result = await completeWithPreparedSimpleCompletionModel({ model: prepared.model, auth: prepared.auth, + cfg, context: { messages: [ {