feat(hooks): Get the model identifier string right.

This commit is contained in:
cline-test
2026-02-03 10:11:24 -08:00
parent 696e580d21
commit 3e977874bd
3 changed files with 312 additions and 26 deletions
+218 -4
View File
@@ -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:<openAiModelId>, NOT openai-compat:<stale actModeApiModelId>
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", () => {
+88 -12
View File
@@ -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<Name extends HookName> {
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 = <T>(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<string>("planModeApiModelId", "actModeApiModelId")
switch (apiProvider) {
// Providers using openRouterModelId
case "cline":
return `cline:${getSetting<string>("planModeOpenRouterModelId", "actModeOpenRouterModelId") || ""}`
case "openrouter":
return `openrouter:${getSetting<string>("planModeOpenRouterModelId", "actModeOpenRouterModelId") || ""}`
// OpenAI compatible - uses openAiModelId
case "openai":
return `openai-compat:${getSetting<string>("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<string>("planModeRequestyModelId", "actModeRequestyModelId") || ""}`
case "ollama":
return `ollama:${getSetting<string>("planModeOllamaModelId", "actModeOllamaModelId") || ""}`
case "lmstudio":
return `lmstudio:${getSetting<string>("planModeLmStudioModelId", "actModeLmStudioModelId") || ""}`
case "litellm":
return `litellm:${getSetting<string>("planModeLiteLlmModelId", "actModeLiteLlmModelId") || ""}`
case "groq":
return `groq:${getSetting<string>("planModeGroqModelId", "actModeGroqModelId") || ""}`
case "baseten":
return `baseten:${getSetting<string>("planModeBasetenModelId", "actModeBasetenModelId") || ""}`
case "huggingface":
return `huggingface:${getSetting<string>("planModeHuggingFaceModelId", "actModeHuggingFaceModelId") || ""}`
case "huawei-cloud-maas":
return `huawei-cloud-maas:${getSetting<string>("planModeHuaweiCloudMaasModelId", "actModeHuaweiCloudMaasModelId") || ""}`
case "vercel-ai-gateway":
return `vercel-ai-gateway:${getSetting<string>("planModeVercelAiGatewayModelId", "actModeVercelAiGatewayModelId") || ""}`
case "fireworks":
return `fireworks:${getSetting<string>("planModeFireworksModelId", "actModeFireworksModelId") || ""}`
case "oca":
return `oca:${getSetting<string>("planModeOcaModelId", "actModeOcaModelId") || ""}`
case "aihubmix":
return `aihubmix:${getSetting<string>("planModeAihubmixModelId", "actModeAihubmixModelId") || ""}`
case "nousResearch":
return `nousResearch:${getSetting<string>("planModeNousResearchModelId", "actModeNousResearchModelId") || ""}`
case "hicap":
return `hicap:${getSetting<string>("planModeHicapModelId", "actModeHicapModelId") || ""}`
case "together":
return `together:${getSetting<string>("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<Name extends HookName> extends HookRunner<Name> {
}
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
@@ -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()
})