From 3e977874bd04e55f788b597f0b7442284c0245b6 Mon Sep 17 00:00:00 2001 From: cline-test <132302818+candieduniverse@users.noreply.github.com> Date: Tue, 3 Feb 2026 10:09:52 -0800 Subject: [PATCH] feat(hooks): Get the model identifier string right. --- src/core/hooks/__tests__/hook-factory.test.ts | 222 +++++++++++++++++- src/core/hooks/hook-factory.ts | 100 +++++++- .../terminal/VscodeTerminalProcess.test.ts | 16 +- 3 files changed, 312 insertions(+), 26 deletions(-) diff --git a/src/core/hooks/__tests__/hook-factory.test.ts b/src/core/hooks/__tests__/hook-factory.test.ts index 943acb6402..382aa24e15 100644 --- a/src/core/hooks/__tests__/hook-factory.test.ts +++ b/src/core/hooks/__tests__/hook-factory.test.ts @@ -81,7 +81,7 @@ describe("Hook System", () => { try { await fs.rm(tempDir, { recursive: true, force: true }) - } catch (error) { + } catch (_error) { // Ignore cleanup errors } }) @@ -442,8 +442,13 @@ console.log(JSON.stringify({ mode?: "plan" | "act" actModeApiProvider?: string actModeApiModelId?: string + actModeOpenRouterModelId?: string + actModeOpenAiModelId?: string + actModeOllamaModelId?: string planModeApiProvider?: string planModeApiModelId?: string + planModeOpenRouterModelId?: string + planModeOpenAiModelId?: string }): void => { // Replace only this stub; avoid sandbox.restore() which can unintentionally // remove unrelated stubs/mocks within the test. @@ -462,8 +467,13 @@ console.log(JSON.stringify({ if (key === "mode") return overrides.mode ?? "act" if (key === "actModeApiProvider") return overrides.actModeApiProvider ?? "anthropic" if (key === "actModeApiModelId") return overrides.actModeApiModelId ?? "claude-sonnet-4-20250514" + if (key === "actModeOpenRouterModelId") return overrides.actModeOpenRouterModelId + if (key === "actModeOpenAiModelId") return overrides.actModeOpenAiModelId + if (key === "actModeOllamaModelId") return overrides.actModeOllamaModelId if (key === "planModeApiProvider") return overrides.planModeApiProvider ?? "anthropic" if (key === "planModeApiModelId") return overrides.planModeApiModelId ?? "claude-sonnet-4-20250514" + if (key === "planModeOpenRouterModelId") return overrides.planModeOpenRouterModelId + if (key === "planModeOpenAiModelId") return overrides.planModeOpenAiModelId return undefined }, } as any) @@ -500,9 +510,11 @@ console.log(JSON.stringify({ restubStateManagerGet({ mode: "act", actModeApiProvider: "cline", - actModeApiModelId: "anthropic/claude-sonnet-4.5", + actModeApiModelId: "claude-sonnet-4-20250514", // should not be used for cline modelId display + actModeOpenRouterModelId: "anthropic/claude-sonnet-4.5", planModeApiProvider: "cline", - planModeApiModelId: "anthropic/claude-sonnet-4.5", + planModeApiModelId: "claude-sonnet-4-20250514", + planModeOpenRouterModelId: "anthropic/claude-sonnet-4.5", }) const hookPath = path.join(tempDir, ".clinerules", "hooks", "PreToolUse") @@ -536,7 +548,8 @@ console.log(JSON.stringify({ actModeApiProvider: "anthropic", actModeApiModelId: "claude-sonnet-4-20250514", planModeApiProvider: "cline", - planModeApiModelId: "openai/gpt-5.2", + planModeApiModelId: "claude-opus-4-5-20251101", // should not be used for cline modelId display + planModeOpenRouterModelId: "openai/gpt-5.2", }) const hookPath = path.join(tempDir, ".clinerules", "hooks", "PreToolUse") @@ -564,6 +577,207 @@ console.log(JSON.stringify({ // Should use plan mode provider/model since mode is "plan" result.contextModification!.should.equal("Model ID: cline:openai/gpt-5.2") }) + + it("should fall back to empty model for cline if openRouterModelId is missing", async () => { + restubStateManagerGet({ + mode: "act", + actModeApiProvider: "cline", + actModeApiModelId: "claude-opus-4-5-20251101", + // intentionally missing actModeOpenRouterModelId + }) + + const hookPath = path.join(tempDir, ".clinerules", "hooks", "PreToolUse") + const hookScript = `#!/usr/bin/env node +const input = JSON.parse(require('fs').readFileSync(0, 'utf-8')); +console.log(JSON.stringify({ cancel: false, contextModification: "Model ID: " + input.modelId }))` + + await writeHookScript(hookPath, hookScript) + + const factory = new HookFactory() + const runner = await factory.create("PreToolUse") + + const result = await runner.run({ + taskId: "test-task", + preToolUse: { + toolName: "test_tool", + parameters: {}, + }, + }) + + result.cancel.should.be.false() + result.contextModification!.should.equal("Model ID: cline:") + }) + + it("should use openai-compat prefix when provider is openai", async () => { + restubStateManagerGet({ + mode: "act", + actModeApiProvider: "openai", + actModeApiModelId: "claude-opus-4-5-20251101", // should not be used when OpenAI model id is present + actModeOpenAiModelId: "gpt-4.1", + planModeApiProvider: "openai", + planModeApiModelId: "claude-opus-4-5-20251101", + planModeOpenAiModelId: "gpt-4.1", + }) + + const hookPath = path.join(tempDir, ".clinerules", "hooks", "PreToolUse") + const hookScript = `#!/usr/bin/env node +const input = JSON.parse(require('fs').readFileSync(0, 'utf-8')); +console.log(JSON.stringify({ + cancel: false, + contextModification: "Model ID: " + input.modelId +}))` + + await writeHookScript(hookPath, hookScript) + + const factory = new HookFactory() + const runner = await factory.create("PreToolUse") + + const result = await runner.run({ + taskId: "test-task", + preToolUse: { + toolName: "test_tool", + parameters: {}, + }, + }) + + result.cancel.should.be.false() + result.contextModification!.should.equal("Model ID: openai-compat:gpt-4.1") + }) + + it("should not use stale actModeApiModelId when switching to openai (uses actModeOpenAiModelId)", async () => { + // This reproduces a real-world scenario: + // 1) user previously used Anthropic provider (actModeApiModelId set to a claude model) + // 2) user switches to OpenAI-compatible provider and selects a GPT model + // The hook modelId should be openai-compat:, NOT openai-compat: + restubStateManagerGet({ + mode: "act", + actModeApiProvider: "openai", + actModeApiModelId: "claude-opus-4-5-20251101", // stale value from prior provider + actModeOpenAiModelId: "gpt-4.1", + }) + + const hookPath = path.join(tempDir, ".clinerules", "hooks", "PreToolUse") + const hookScript = `#!/usr/bin/env node +const input = JSON.parse(require('fs').readFileSync(0, 'utf-8')); +console.log(JSON.stringify({ cancel: false, contextModification: "Model ID: " + input.modelId }))` + await writeHookScript(hookPath, hookScript) + + const factory = new HookFactory() + const runner = await factory.create("PreToolUse") + const result = await runner.run({ + taskId: "test-task", + preToolUse: { toolName: "test_tool", parameters: {} }, + }) + + result.cancel.should.be.false() + result.contextModification!.should.equal("Model ID: openai-compat:gpt-4.1") + }) + + it("should use default model for openai-native when apiModelId is from different provider", async () => { + // Simulates switching from Anthropic to openai-native: + // - actModeApiModelId still has claude model (not valid for openai-native) + // - Should fall back to openai-native default model + restubStateManagerGet({ + mode: "act", + actModeApiProvider: "openai-native", + actModeApiModelId: "claude-opus-4-5-20251101", // stale value - not in openAiNativeModels + }) + + const hookPath = path.join(tempDir, ".clinerules", "hooks", "PreToolUse") + const hookScript = `#!/usr/bin/env node +const input = JSON.parse(require('fs').readFileSync(0, 'utf-8')); +console.log(JSON.stringify({ cancel: false, contextModification: "Model ID: " + input.modelId }))` + await writeHookScript(hookPath, hookScript) + + const factory = new HookFactory() + const runner = await factory.create("PreToolUse") + const result = await runner.run({ + taskId: "test-task", + preToolUse: { toolName: "test_tool", parameters: {} }, + }) + + result.cancel.should.be.false() + // Should use default openai-native model, not the stale claude model + result.contextModification!.should.equal("Model ID: openai-native:gpt-5.2") + }) + + it("should use valid openai-native model when apiModelId is correct", async () => { + restubStateManagerGet({ + mode: "act", + actModeApiProvider: "openai-native", + actModeApiModelId: "gpt-5.2", // valid openai-native model + }) + + const hookPath = path.join(tempDir, ".clinerules", "hooks", "PreToolUse") + const hookScript = `#!/usr/bin/env node +const input = JSON.parse(require('fs').readFileSync(0, 'utf-8')); +console.log(JSON.stringify({ cancel: false, contextModification: "Model ID: " + input.modelId }))` + await writeHookScript(hookPath, hookScript) + + const factory = new HookFactory() + const runner = await factory.create("PreToolUse") + const result = await runner.run({ + taskId: "test-task", + preToolUse: { toolName: "test_tool", parameters: {} }, + }) + + result.cancel.should.be.false() + result.contextModification!.should.equal("Model ID: openai-native:gpt-5.2") + }) + + it("should use openrouter-specific model ID field (not stale apiModelId)", async () => { + // Simulates switching to openrouter provider + // openrouter uses *ModeOpenRouterModelId, not *ModeApiModelId + restubStateManagerGet({ + mode: "act", + actModeApiProvider: "openrouter", + actModeApiModelId: "gpt-5.2-codex", // stale value from prior provider + actModeOpenRouterModelId: "openai/gpt-5.2", // correct openrouter model + }) + + const hookPath = path.join(tempDir, ".clinerules", "hooks", "PreToolUse") + const hookScript = `#!/usr/bin/env node +const input = JSON.parse(require('fs').readFileSync(0, 'utf-8')); +console.log(JSON.stringify({ cancel: false, contextModification: "Model ID: " + input.modelId }))` + await writeHookScript(hookPath, hookScript) + + const factory = new HookFactory() + const runner = await factory.create("PreToolUse") + const result = await runner.run({ + taskId: "test-task", + preToolUse: { toolName: "test_tool", parameters: {} }, + }) + + result.cancel.should.be.false() + // Should use openRouterModelId, not the stale apiModelId + result.contextModification!.should.equal("Model ID: openrouter:openai/gpt-5.2") + }) + + it("should use provider-specific model fields for other providers", async () => { + // Test ollama which uses *ModeOllamaModelId + restubStateManagerGet({ + mode: "act", + actModeApiProvider: "ollama", + actModeApiModelId: "claude-opus-4-5-20251101", // stale + actModeOllamaModelId: "llama3:latest", // correct ollama model + }) + + const hookPath = path.join(tempDir, ".clinerules", "hooks", "PreToolUse") + const hookScript = `#!/usr/bin/env node +const input = JSON.parse(require('fs').readFileSync(0, 'utf-8')); +console.log(JSON.stringify({ cancel: false, contextModification: "Model ID: " + input.modelId }))` + await writeHookScript(hookPath, hookScript) + + const factory = new HookFactory() + const runner = await factory.create("PreToolUse") + const result = await runner.run({ + taskId: "test-task", + preToolUse: { toolName: "test_tool", parameters: {} }, + }) + + result.cancel.should.be.false() + result.contextModification!.should.equal("Model ID: ollama:llama3:latest") + }) }) describe("Global Hooks", () => { diff --git a/src/core/hooks/hook-factory.ts b/src/core/hooks/hook-factory.ts index c722057574..38873e0125 100644 --- a/src/core/hooks/hook-factory.ts +++ b/src/core/hooks/hook-factory.ts @@ -1,5 +1,6 @@ import fs from "fs/promises" import path from "path" +import { openAiCodexDefaultModelId, openAiCodexModels, openAiNativeDefaultModelId, openAiNativeModels } from "@/shared/api" import { Logger } from "@/shared/services/Logger" import { version as clineVersion } from "../../../package.json" import { getDistinctId } from "../../services/logging/distinctId" @@ -189,20 +190,95 @@ export abstract class HookRunner { const stateManager = StateManager.get() const workspaceRoots = stateManager.getGlobalStateKey("workspaceRoots")?.map((root) => root.path) || [] - // Build modelId in "provider:modelId" format (e.g., "cline:openai/gpt-5.2") - // The current mode determines which provider/model settings to use. - // Treat any unrecognized value as "act" to avoid surprising behavior. - const rawMode = stateManager.getGlobalSettingsKey("mode") - const mode = rawMode === "plan" || rawMode === "act" ? rawMode : "act" - const provider = + // Build modelId using the same format shown under the prompt input field in the VSCode extension + // (see webview-ui/src/components/chat/ChatTextArea.tsx) + const mode = stateManager.getGlobalSettingsKey("mode") || "act" + const apiProvider = mode === "plan" ? stateManager.getGlobalSettingsKey("planModeApiProvider") : stateManager.getGlobalSettingsKey("actModeApiProvider") - const modelIdValue = - mode === "plan" - ? stateManager.getGlobalSettingsKey("planModeApiModelId") - : stateManager.getGlobalSettingsKey("actModeApiModelId") - const formattedModelId = provider && modelIdValue ? `${provider}:${modelIdValue}` : provider || modelIdValue || "" + + // Helper to get a mode-specific settings key + const getSetting = (planKey: string, actKey: string): T | undefined => + (mode === "plan" + ? stateManager.getGlobalSettingsKey(planKey as any) + : stateManager.getGlobalSettingsKey(actKey as any)) as T | undefined + + // Get the correct model ID based on provider + // This mirrors webview-ui/src/components/settings/utils/providerUtils.ts normalizeApiConfiguration() + // Each provider may have its own dedicated model ID field + const formattedModelId = (() => { + const apiModelId = getSetting("planModeApiModelId", "actModeApiModelId") + + switch (apiProvider) { + // Providers using openRouterModelId + case "cline": + return `cline:${getSetting("planModeOpenRouterModelId", "actModeOpenRouterModelId") || ""}` + case "openrouter": + return `openrouter:${getSetting("planModeOpenRouterModelId", "actModeOpenRouterModelId") || ""}` + + // OpenAI compatible - uses openAiModelId + case "openai": + return `openai-compat:${getSetting("planModeOpenAiModelId", "actModeOpenAiModelId") || ""}` + + // OpenAI native - validate against known models + case "openai-native": + return `openai-native:${apiModelId && apiModelId in openAiNativeModels ? apiModelId : openAiNativeDefaultModelId}` + + // OpenAI Codex - validate against known models + case "openai-codex": + return `openai-codex:${apiModelId && apiModelId in openAiCodexModels ? apiModelId : openAiCodexDefaultModelId}` + + // Providers with their own dedicated model ID fields + case "requesty": + return `requesty:${getSetting("planModeRequestyModelId", "actModeRequestyModelId") || ""}` + case "ollama": + return `ollama:${getSetting("planModeOllamaModelId", "actModeOllamaModelId") || ""}` + case "lmstudio": + return `lmstudio:${getSetting("planModeLmStudioModelId", "actModeLmStudioModelId") || ""}` + case "litellm": + return `litellm:${getSetting("planModeLiteLlmModelId", "actModeLiteLlmModelId") || ""}` + case "groq": + return `groq:${getSetting("planModeGroqModelId", "actModeGroqModelId") || ""}` + case "baseten": + return `baseten:${getSetting("planModeBasetenModelId", "actModeBasetenModelId") || ""}` + case "huggingface": + return `huggingface:${getSetting("planModeHuggingFaceModelId", "actModeHuggingFaceModelId") || ""}` + case "huawei-cloud-maas": + return `huawei-cloud-maas:${getSetting("planModeHuaweiCloudMaasModelId", "actModeHuaweiCloudMaasModelId") || ""}` + case "vercel-ai-gateway": + return `vercel-ai-gateway:${getSetting("planModeVercelAiGatewayModelId", "actModeVercelAiGatewayModelId") || ""}` + case "fireworks": + return `fireworks:${getSetting("planModeFireworksModelId", "actModeFireworksModelId") || ""}` + case "oca": + return `oca:${getSetting("planModeOcaModelId", "actModeOcaModelId") || ""}` + case "aihubmix": + return `aihubmix:${getSetting("planModeAihubmixModelId", "actModeAihubmixModelId") || ""}` + case "nousResearch": + return `nousResearch:${getSetting("planModeNousResearchModelId", "actModeNousResearchModelId") || ""}` + case "hicap": + return `hicap:${getSetting("planModeHicapModelId", "actModeHicapModelId") || ""}` + case "together": + return `together:${getSetting("planModeTogetherModelId", "actModeTogetherModelId") || ""}` + + // VSCode LM has special format: vendor/family + case "vscode-lm": { + const selector = getSetting<{ vendor?: string; family?: string }>( + "planModeVsCodeLmModelSelector", + "actModeVsCodeLmModelSelector", + ) + return `vscode-lm:${selector ? `${selector.vendor || ""}/${selector.family || ""}` : ""}` + } + + // Dify has hardcoded model + case "dify": + return "dify:dify-workflow" + + // All other providers use apiModelId (anthropic, bedrock, vertex, gemini, deepseek, etc.) + default: + return `${apiProvider || ""}:${apiModelId || ""}` + } + })() return { clineVersion, @@ -369,7 +445,7 @@ class StdioHookRunner extends HookRunner { } return output - } catch (parseError) { + } catch (_parseError) { // Try to extract JSON from stdout (it might have debug output before/after) // Scan from the end to find the last complete JSON object // This handles cases where hooks output debug info before the actual JSON response diff --git a/src/hosts/vscode/terminal/VscodeTerminalProcess.test.ts b/src/hosts/vscode/terminal/VscodeTerminalProcess.test.ts index 457904e254..1d79ae802c 100644 --- a/src/hosts/vscode/terminal/VscodeTerminalProcess.test.ts +++ b/src/hosts/vscode/terminal/VscodeTerminalProcess.test.ts @@ -47,7 +47,9 @@ describe("TerminalProcess (Integration Tests)", () => { // Remove any event listeners left on the TerminalProcess process.removeAllListeners() // Dispose all terminals created during the test - createdTerminals.forEach((t) => t.dispose()) + createdTerminals.forEach((t) => { + t.dispose() + }) createdTerminals = [] }) @@ -218,9 +220,7 @@ describe("TerminalProcess (Integration Tests)", () => { // Check that the correct methods were called and events emitted sendTextStub.calledWith("test-command", true).should.be.true() ;(emitSpy as sinon.SinonSpy).calledWith("completed").should.be.true() - ;(emitSpy as sinon.SinonSpy) - .calledWith("continue") - .should.be.true() + ;(emitSpy as sinon.SinonSpy).calledWith("continue").should.be.true() // This event should be emitted for terminals without shell integration ;(emitSpy as sinon.SinonSpy).calledWith("no_shell_integration").should.be.true() @@ -254,9 +254,7 @@ describe("TerminalProcess (Integration Tests)", () => { await process.run(terminal, "echo test") // Verify the executeCommand was called with the right command - mockExecuteCommand - .calledWith("echo test") - .should.be.true() + mockExecuteCommand.calledWith("echo test").should.be.true() // Check that the events were emitted ;(emitSpy as sinon.SinonSpy).calledWith("completed").should.be.true() @@ -375,9 +373,7 @@ describe("TerminalProcess (Integration Tests)", () => { // Check that "test-command" was filtered out but "test command" was not ;(emitSpy as sinon.SinonSpy).calledWith("line", "test command").should.be.true() - ;(emitSpy as sinon.SinonSpy) - .calledWith("line", "other output") - .should.be.true() + ;(emitSpy as sinon.SinonSpy).calledWith("line", "other output").should.be.true() // This should never be called because it should be filtered ;(emitSpy as sinon.SinonSpy).calledWith("line", "test-command").should.be.false() })