Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/invalid-tool-repair-surfaces-real-name.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@kilocode/cli": patch
---

Surface the actual tool name when the model calls an unavailable tool, instead of a confusing "unavailable tool 'invalid'" error. Malformed tool calls that cannot be repaired now report the real tool name and available tools, so the model can self-correct.
21 changes: 0 additions & 21 deletions packages/opencode/src/cli/cmd/run/tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import type { ShellTool as BashTool } from "@/tool/shell"
import type { EditTool } from "@/tool/edit"
import type { GlobTool } from "@/tool/glob"
import type { GrepTool } from "@/tool/grep"
import type { InvalidTool } from "@/tool/invalid"
import type { LspTool } from "@/tool/lsp"
import type { PlanExitTool } from "@/tool/plan"
import type { InteractiveTerminalTool } from "@/kilocode/tool/interactive-terminal" // kilocode_change
Expand Down Expand Up @@ -93,7 +92,6 @@ type ToolPermissionCtx = {
}

type ToolDefs = {
invalid: typeof InvalidTool
bash: typeof BashTool
write: typeof WriteTool
edit: typeof EditTool
Expand Down Expand Up @@ -452,15 +450,6 @@ function scrollInteractiveTerminalFinal(p: ToolProps<typeof InteractiveTerminalT
}
// kilocode_change end

function runInvalid(p: ToolProps<typeof InvalidTool>): ToolInline {
return {
icon: "✗",
title: text(p.frame.state.title) || "Invalid Tool",
mode: "block",
body: p.frame.status === "completed" ? text(p.frame.state.output) : undefined,
}
}

function runBatch(p: ToolProps): ToolInline {
const calls = list(dict(p.input).tool_calls).length
return {
Expand Down Expand Up @@ -1049,16 +1038,6 @@ function permLsp(p: ToolPermissionProps<typeof LspTool>): ToolPermissionInfo {
}

const TOOL_RULES = {
invalid: {
view: {
output: true,
final: false,
},
run: runInvalid,
scroll: {
start: () => "",
},
},
bash: {
view: {
output: true,
Expand Down
13 changes: 5 additions & 8 deletions packages/opencode/src/session/llm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,14 +380,11 @@ const live: Layer.Layer<
l.info("repairing tool call", { tool: failed.toolCall.toolName, repaired: lower }) // kilocode_change
return { ...failed.toolCall, toolName: lower }
}
return {
...failed.toolCall,
input: JSON.stringify({
tool: failed.toolCall.toolName,
error: failed.error.message,
}),
toolName: "invalid",
}
// kilocode_change start - surface the original tool-name error instead of a
// repaired call to the hidden "invalid" tool, which activeTools excludes and
// therefore fails with a confusing "unavailable tool 'invalid'" error
return null
// kilocode_change end
},
temperature: prepared.params.temperature,
topP: prepared.params.topP,
Expand Down
21 changes: 0 additions & 21 deletions packages/opencode/src/tool/invalid.ts

This file was deleted.

4 changes: 0 additions & 4 deletions packages/opencode/src/tool/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { Database } from "@opencode-ai/core/database/database"
import { TodoWriteTool } from "./todo"
import { WebFetchTool } from "./webfetch"
import { WriteTool } from "./write"
import { InvalidTool } from "./invalid"
import { SkillTool } from "./skill"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep the upstream invalid-tool cleanup out of this fix

Returning null from the repair callback is sufficient to correct the reported behavior, but this hunk also begins an unnecessary cleanup that deletes the upstream invalid tool and modifies its registry, CLI renderer, and schema tests across five shared files. Retaining that now-unused definition avoids expanding Kilo's upstream delta and the resulting merge-conflict surface; the repository explicitly requires shared-file changes to be minimal and prohibits unnecessary upstream refactoring.

AGENTS.md reference: AGENTS.md:L185-L190

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed: the invalid-tool cleanup was reverted in 4c2b02e (Revert "refactor(tool): remove orphaned invalid tool"), restoring packages/opencode/src/tool/invalid.ts, the registry registration, the TUI renderer, and the schema test/snapshot to their upstream state. The branch now touches only the repair callback in session/llm.ts plus its regression test and changeset — the upstream diff is minimal again.

import * as Tool from "./tool"
import { Config } from "@/config/config"
Expand Down Expand Up @@ -124,7 +123,6 @@ const layer = Layer.effect(
const mcp = yield* MCP.Service
const sessions = yield* Session.Service

const invalid = yield* InvalidTool
const task = yield* TaskTool
const read = yield* ReadTool
const question = yield* QuestionTool
Expand Down Expand Up @@ -254,7 +252,6 @@ const layer = Layer.effect(
const questionEnabled = ["app", "cli", "desktop", "vscode"].includes(flags.client) || flags.enableQuestionTool // kilocode_change: add vscode client

const tool = yield* Effect.all({
invalid: Tool.init(invalid),
shell: Tool.init(shell),
read: Tool.init(read),
glob: Tool.init(globtool),
Expand Down Expand Up @@ -289,7 +286,6 @@ const layer = Layer.effect(
// kilocode_change start
builtin: KiloToolRegistry.describe(
[
tool.invalid,
...(questionEnabled ? [tool.question] : []),
tool.shell,
tool.read,
Expand Down
99 changes: 99 additions & 0 deletions packages/opencode/test/session/llm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ const it = testEffect(AppNodeBuilder.build(LayerNode.group([LLM.node, Provider.n
// LLM.stream returns a Stream, not an Effect, so we can't use the serviceUse proxy.
const drain = (input: LLM.StreamInput) => LLM.Service.use((svc) => svc.stream(input).pipe(Stream.runDrain))

// collect runs the stream and returns every emitted LLMEvent for assertions. // kilocode_change
const collect = (input: LLM.StreamInput) => // kilocode_change
LLM.Service.use((svc) => svc.stream(input).pipe(Stream.runCollect)) // kilocode_change

// drainWith builds an isolated runtime so custom replacements fully own LLM and
// its transitive deps.
const drainWith = (layer: Layer.Layer<LLM.Service>, input: LLM.StreamInput) =>
Expand Down Expand Up @@ -2250,5 +2254,100 @@ describe("session.llm.stream", () => {
}),
},
)
it.instance(
"surfaces the real tool name when a tool call cannot be repaired",
() =>
Effect.gen(function* () {
const fixture = loadFixture(alibabaQwenFixture.providerID, alibabaQwenFixture.modelID)

waitRequest(
"/chat/completions",
createEventResponse(
[
{
id: "chatcmpl-1",
object: "chat.completion.chunk",
choices: [
{
index: 0,
delta: {
role: "assistant",
content: null,
tool_calls: [{ index: 0, id: "call-1", type: "function", function: { name: "Write", arguments: "" } }],
},
},
],
},
{
id: "chatcmpl-1",
object: "chat.completion.chunk",
choices: [{ index: 0, delta: { tool_calls: [{ index: 0, function: { arguments: "{}" } }] } }],
},
{
id: "chatcmpl-1",
object: "chat.completion.chunk",
choices: [{ index: 0, delta: {}, finish_reason: "tool_calls" }],
},
],
true,
),
)

const resolved = yield* Provider.use.getModel(
ProviderV2.ID.make(alibabaQwenFixture.providerID),
ModelV2.ID.make(fixture.model.id),
)
const sessionID = SessionID.make("session-test-unavailable-tool")
const agent = {
name: "test",
mode: "primary",
options: {},
permission: [{ permission: "*", pattern: "*", action: "allow" }],
} satisfies Agent.Info

const user = {
id: MessageID.make("msg_user-unavailable-tool"),
sessionID,
role: "user",
time: { created: Date.now() },
agent: agent.name,
model: { providerID: ProviderV2.ID.make(alibabaQwenFixture.providerID), modelID: resolved.id },
} satisfies MessageV2.User

const events = Array.from(
yield* collect({
user,
sessionID,
model: resolved,
agent,
system: ["You are a helpful assistant."],
messages: [{ role: "user", content: "Write a file" }],
tools: {
read: tool({
description: "Read a file",
inputSchema: z.object({ filePath: z.string() }),
execute: async () => ({ output: "stub" }),
}),
},
}),
)

const errors = events.filter((event) => event.type === "tool-error")
expect(errors).toHaveLength(1)
expect(errors[0].name).toBe("Write")
expect(errors[0].message).toContain("unavailable tool 'Write'")
expect(errors[0].message).not.toContain("invalid")
}),
{
config: () => ({
enabled_providers: [alibabaQwenFixture.providerID],
provider: {
[alibabaQwenFixture.providerID]: {
options: { apiKey: "test-key", baseURL: `${state.server!.url.origin}/v1` },
},
},
}),
},
)
// kilocode_change end
})
19 changes: 0 additions & 19 deletions packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -155,25 +155,6 @@ exports[`tool parameters JSON Schema (wire shape) grep 1`] = `
}
`;

exports[`tool parameters JSON Schema (wire shape) invalid 1`] = `
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"properties": {
"error": {
"type": "string",
},
"tool": {
"type": "string",
},
},
"required": [
"tool",
"error",
],
"type": "object",
}
`;

exports[`tool parameters JSON Schema (wire shape) lsp 1`] = `
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
Expand Down
12 changes: 0 additions & 12 deletions packages/opencode/test/tool/parameters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { Parameters as ApplyPatch } from "../../src/tool/apply_patch"
import { Parameters as Edit } from "../../src/tool/edit"
import { Parameters as Glob } from "../../src/tool/glob"
import { Parameters as Grep } from "../../src/tool/grep"
import { Parameters as Invalid } from "../../src/tool/invalid"
import { Parameters as Lsp } from "../../src/tool/lsp"
import { Parameters as Plan } from "../../src/tool/plan"
import { Parameters as Question } from "../../src/tool/question"
Expand Down Expand Up @@ -41,7 +40,6 @@ describe("tool parameters", () => {
test("edit", () => expect(toJsonSchema(Edit)).toMatchSnapshot())
test("glob", () => expect(toJsonSchema(Glob)).toMatchSnapshot())
test("grep", () => expect(toJsonSchema(Grep)).toMatchSnapshot())
test("invalid", () => expect(toJsonSchema(Invalid)).toMatchSnapshot())
test("lsp", () => expect(toJsonSchema(Lsp)).toMatchSnapshot())
test("plan", () => expect(toJsonSchema(Plan)).toMatchSnapshot())
test("question", () => expect(toJsonSchema(Question)).toMatchSnapshot())
Expand Down Expand Up @@ -183,16 +181,6 @@ describe("tool parameters", () => {
})
})

describe("invalid", () => {
test("accepts tool + error", () => {
expect(parse(Invalid, { tool: "foo", error: "bar" })).toEqual({ tool: "foo", error: "bar" })
})
test("rejects missing fields", () => {
expect(accepts(Invalid, { tool: "foo" })).toBe(false)
expect(accepts(Invalid, { error: "bar" })).toBe(false)
})
})

describe("lsp", () => {
test("accepts all fields", () => {
const parsed = parse(Lsp, { operation: "hover", filePath: "/a.ts", line: 1, character: 1 })
Expand Down
Loading