From 06998004fdeb15457013c597e0b907462759ba1f Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Tue, 18 Aug 2026 13:36:30 +0200 Subject: [PATCH 1/2] Revert "Merge pull request #13197 from Kilo-Org/fix-agent-manager-tool-schema" This reverts commit eb731248cbf54065dbb76a30daeb49e518aab39e, reversing changes made to 9f04990c5532f37c7d54fbf39fd40043a6c3624c. --- .../strict-agent-manager-tool-requests.md | 5 - .../src/kilocode/tool/agent-manager.ts | 90 +++++++---------- .../test/kilocode/agent-manager-tool.test.ts | 98 ++++++------------- 3 files changed, 64 insertions(+), 129 deletions(-) delete mode 100644 .changeset/strict-agent-manager-tool-requests.md diff --git a/.changeset/strict-agent-manager-tool-requests.md b/.changeset/strict-agent-manager-tool-requests.md deleted file mode 100644 index 0b7b3cbd78b..00000000000 --- a/.changeset/strict-agent-manager-tool-requests.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@kilocode/cli": patch ---- - -Make Agent Manager tool requests use a strict operation union so starting sessions and managing existing sessions cannot be confused. diff --git a/packages/opencode/src/kilocode/tool/agent-manager.ts b/packages/opencode/src/kilocode/tool/agent-manager.ts index 6e4a89f4de9..8c4104e25eb 100644 --- a/packages/opencode/src/kilocode/tool/agent-manager.ts +++ b/packages/opencode/src/kilocode/tool/agent-manager.ts @@ -14,18 +14,6 @@ import { Effect, Schema } from "effect" import { matchesQuery } from "./model-search" import DESCRIPTION from "./agent-manager.txt" -function strict(fields: Fields) { - const target = Schema.Struct(fields) - // Preserve unknown keys long enough for the branch check to reject mixed operations. - const source = Schema.StructWithRest(target, [Schema.Record(Schema.String, Schema.Unknown)]).check( - Schema.makeFilter((value) => { - const extra = Object.keys(value).find((key) => !Object.hasOwn(fields, key)) - return extra === undefined ? undefined : `Unexpected Agent Manager parameter: ${extra}` - }), - ) - return source.pipe(Schema.decodeTo(target)) -} - const Task = Schema.Struct({ prompt: Schema.optional(Schema.NullOr(Schema.String)).annotate({ description: "Initial prompt to send to the new session", @@ -58,35 +46,7 @@ const Task = Schema.Struct({ ), ) -function wireSchema() { - const schema = structuredClone(ToolJsonSchema.fromSchema(Params)) - - // llama.cpp rejects the prefix-only SessionID pattern. Keep the runtime brand - // check, but omit that provider-incompatible hint from the advertised schema. - function strip(value: unknown): void { - if (Array.isArray(value)) { - value.forEach(strip) - return - } - if (!value || typeof value !== "object") return - const item = value as Record - if (item.type === "object" && item.additionalProperties === undefined) { - item.additionalProperties = false - } - if (item.properties && typeof item.properties === "object") { - const properties = item.properties as Record - if (properties.sessionID && typeof properties.sessionID === "object") { - delete (properties.sessionID as Record).pattern - } - } - Object.values(item).forEach(strip) - } - - strip(schema) - return schema -} - -const StartParams = strict({ +const StartParams = Schema.Struct({ mode: Schema.Literals(["worktree", "local"]).annotate({ description: "Use worktree for isolated git worktrees, or local for same-directory Agent Manager sessions", }), @@ -99,14 +59,14 @@ const StartParams = strict({ .annotate({ description: "Agent Manager sessions to start" }), }) -const ListParams = strict({ +const ListParams = Schema.Struct({ action: Schema.Literal("list").annotate({ description: "Read the current Agent Manager sections, worktrees, and sessions before any assignment. This is the source of truth for section and session IDs.", }), filter: Schema.optional( Schema.NullOr( - strict({ + Schema.Struct({ sectionIDs: Schema.optional(Schema.Array(Schema.String).check(Schema.isMaxLength(100))), states: Schema.optional( Schema.Array(Schema.Literals(["idle", "busy", "retry", "offline", "waiting"])).check(Schema.isMaxLength(5)), @@ -118,24 +78,20 @@ const ListParams = strict({ }), }) -const PromptParams = strict({ +const PromptParams = Schema.Struct({ action: Schema.Literal("prompt"), - sessionID: SessionID.annotate({ - description: "Session ID returned by action=list. Do not use a worktree name, branch, or section name.", - }), + sessionID: SessionID, prompt: Schema.String.check(Schema.isMinLength(1), Schema.isMaxLength(100_000)).check( Schema.makeFilter((value) => (value.trim() ? undefined : "Prompt must not be empty")), ), }) -const StopParams = strict({ +const StopParams = Schema.Struct({ action: Schema.Literal("stop"), - sessionID: SessionID.annotate({ - description: "Session ID returned by action=list. Do not use a worktree name, branch, or section name.", - }), + sessionID: SessionID, }) -const MoveParams = strict({ +const MoveParams = Schema.Struct({ action: Schema.Literal("move").annotate({ description: "Move exactly one managed worktree by targeting one of its session IDs returned by action=list.", }), @@ -147,7 +103,25 @@ const MoveParams = strict({ }), }) -export const Params = Schema.Union([StartParams, ListParams, PromptParams, MoveParams, StopParams]) +export const Params = Schema.Union([StartParams, ListParams, PromptParams, StopParams, MoveParams]) + +const WireParams = Schema.Struct({ + mode: Schema.optional(StartParams.fields.mode), + versions: Schema.optional(StartParams.fields.versions), + tasks: Schema.optional(StartParams.fields.tasks), + action: Schema.optional( + Schema.Literals(["list", "prompt", "stop", "move"]).annotate({ + description: + "Use list first to discover IDs and assignments. Use move only after list, once per worktree. Never edit .kilo/agent-manager.json for these operations.", + }), + ), + filter: Schema.optional(ListParams.fields.filter), + sessionID: Schema.optional( + Schema.String.annotate({ description: "For move, use a session ID returned by action=list (IDs start with ses_)." }), + ), + prompt: Schema.optional(PromptParams.fields.prompt), + sectionID: Schema.optional(MoveParams.fields.sectionID), +}) type Input = Schema.Schema.Type type Selected = { task?: AgentManagerTask; error?: string } @@ -307,10 +281,18 @@ export const AgentManagerTool = Tool.define< const bus = yield* Bus.Service const host = yield* AgentManager.Service const provider = yield* Provider.Service + const wire = ToolJsonSchema.fromSchema(WireParams) + const section = wire.properties?.sectionID + if (section && typeof section === "object" && wire.properties) { + wire.properties.sectionID = { + anyOf: [{ type: "string", minLength: 1 }, { type: "null" }], + description: "Section ID returned by action=list. Use null to unassign the worktree from its current section.", + } + } return { description: DESCRIPTION, parameters: Params, - jsonSchema: wireSchema(), + jsonSchema: wire, execute: (params, ctx) => Effect.gen(function* () { if ("action" in params) { diff --git a/packages/opencode/test/kilocode/agent-manager-tool.test.ts b/packages/opencode/test/kilocode/agent-manager-tool.test.ts index d611edd48eb..2f44f3f5c7d 100644 --- a/packages/opencode/test/kilocode/agent-manager-tool.test.ts +++ b/packages/opencode/test/kilocode/agent-manager-tool.test.ts @@ -1,6 +1,6 @@ import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" import { describe, expect, test } from "bun:test" -import { Effect, Layer, ManagedRuntime, Queue, Result, Schema } from "effect" +import { Effect, Layer, ManagedRuntime, Queue, Schema } from "effect" import { MessageID, SessionID } from "../../src/session/schema" import { provideTmpdirInstance } from "../fixture/fixture" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" @@ -151,87 +151,45 @@ function publish( } describe("agent_manager tool", () => { - test("advertises each operation as a strict union branch", async () => { + test("uses an object-root input schema without combinators", async () => { const tool = await init() const schema = ToolJsonSchema.fromTool(tool) - expect(schema.type).toBeUndefined() - expect(schema.anyOf).toHaveLength(5) + expect(schema.type).toBe("object") + expect(schema.anyOf).toBeUndefined() expect(schema.oneOf).toBeUndefined() expect(schema.allOf).toBeUndefined() - const branches = schema.anyOf as Array> - const properties = (branch: Record) => branch.properties as Record - expect(branches.map((branch) => branch.required)).toEqual([ - ["mode", "tasks"], - ["action"], - ["action", "sessionID", "prompt"], - ["action", "sessionID", "sectionID"], - ["action", "sessionID"], - ]) - expect(branches.every((branch) => branch.additionalProperties === false)).toBe(true) - expect(properties(branches[2]!).sessionID).not.toHaveProperty("pattern") - expect(properties(branches[3]!).sessionID).not.toHaveProperty("pattern") - expect(properties(branches[4]!).sessionID).not.toHaveProperty("pattern") - expect(properties(branches[0]!)).toEqual( - expect.objectContaining({ mode: expect.anything(), tasks: expect.anything() }), + const action = schema.properties?.action + expect(action && typeof action === "object" ? action.enum : undefined).toEqual(["list", "prompt", "stop", "move"]) + expect(action && typeof action === "object" ? action.description : undefined).toContain("Use list first") + expect(action && typeof action === "object" ? action.description : undefined).toContain("Never edit") + expect(schema.properties?.sessionID).toEqual( + expect.objectContaining({ description: expect.stringContaining("IDs start with ses_") }), ) - expect(properties(branches[1]!)).toEqual( - expect.objectContaining({ action: expect.objectContaining({ enum: ["list"] }) }), + expect(schema.properties?.sessionID).not.toHaveProperty("pattern") + expect(schema.properties?.sectionID).toEqual( + expect.objectContaining({ description: expect.stringContaining("Use null to unassign") }), ) - expect(properties(branches[2]!)).toEqual( + expect(schema.properties?.sectionID).toEqual( expect.objectContaining({ - action: expect.objectContaining({ enum: ["prompt"] }), - sessionID: expect.objectContaining({ - description: expect.stringContaining("Session ID returned by action=list"), - }), + anyOf: expect.arrayContaining([expect.objectContaining({ type: "string" }), { type: "null" }]), }), ) - expect(properties(branches[3]!)).toEqual( - expect.objectContaining({ action: expect.objectContaining({ enum: ["move"] }) }), - ) - expect(properties(branches[4]!)).toEqual( - expect.objectContaining({ - action: expect.objectContaining({ enum: ["stop"] }), - sessionID: expect.objectContaining({ - description: expect.stringContaining("Session ID returned by action=list"), - }), - }), - ) - }) - - test("accepts each operation branch and rejects ambiguous payloads", () => { - const task = { prompt: "Fix the issue" } - const accepts = (input: unknown) => Result.isSuccess(Schema.decodeUnknownResult(Params)(input)) - expect(accepts({ mode: "local", tasks: [task] })).toBe(true) - expect(accepts({ action: "list" })).toBe(true) - expect(accepts({ action: "list", filter: null })).toBe(true) - expect(accepts({ action: "prompt", sessionID: "ses_target", prompt: "Continue" })).toBe(true) - expect(accepts({ action: "stop", sessionID: "ses_target" })).toBe(true) - expect(accepts({ action: "move", sessionID: "ses_target", sectionID: null })).toBe(true) - expect(accepts({ action: "stop", sessionID: "invalid" })).toBe(false) - - expect(accepts({ mode: "local", tasks: [task], action: "list" })).toBe(false) - expect(accepts({ action: "list", mode: "local", tasks: [task] })).toBe(false) - expect(accepts({ action: "prompt", sessionID: "ses_target", prompt: "Continue", mode: "local" })).toBe(false) - expect(accepts({ action: "stop", sessionID: "ses_target", prompt: "Continue" })).toBe(false) - expect(accepts({ action: "move", sessionID: "ses_target", sectionID: null, filter: null })).toBe(false) + expect(Object.keys(schema.properties ?? {})).toEqual([ + "mode", + "versions", + "tasks", + "action", + "filter", + "sessionID", + "prompt", + "sectionID", + ]) }) - test("rejects mixed payloads before dispatch", async () => { - const tool = await init() - const calls: unknown[] = [] - - await expect( - runtime.runPromise( - provideTmpdirInstance(() => - tool.execute( - { mode: "local", tasks: [{ prompt: "Fix issue" }], action: "list" }, - { ...ctx, ask: (input: unknown) => Effect.sync(() => calls.push(input)) }, - ), - ).pipe(Effect.scoped), - ), - ).rejects.toThrow("Unexpected Agent Manager parameter") - expect(calls).toEqual([]) + test("keeps session ID validation local", () => { + expect(Schema.is(Params)({ action: "stop", sessionID: "ses_target" })).toBe(true) + expect(Schema.is(Params)({ action: "stop", sessionID: "invalid" })).toBe(false) }) test("asks for agent_manager permission", async () => { From b563c0a2c99501076488f5a1a37cc2531e7159be Mon Sep 17 00:00:00 2001 From: Marius Date: Tue, 18 Aug 2026 13:43:57 +0200 Subject: [PATCH 2/2] Update packages/opencode/test/kilocode/agent-manager-tool.test.ts Co-authored-by: Christiaan Arnoldus --- packages/opencode/test/kilocode/agent-manager-tool.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opencode/test/kilocode/agent-manager-tool.test.ts b/packages/opencode/test/kilocode/agent-manager-tool.test.ts index 2f44f3f5c7d..eb151db552f 100644 --- a/packages/opencode/test/kilocode/agent-manager-tool.test.ts +++ b/packages/opencode/test/kilocode/agent-manager-tool.test.ts @@ -151,7 +151,7 @@ function publish( } describe("agent_manager tool", () => { - test("uses an object-root input schema without combinators", async () => { + test("uses an object-root input schema without combinators because more complex schemas break Claude models", async () => { const tool = await init() const schema = ToolJsonSchema.fromTool(tool)