Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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: 0 additions & 5 deletions .changeset/strict-agent-manager-tool-requests.md

This file was deleted.

90 changes: 36 additions & 54 deletions packages/opencode/src/kilocode/tool/agent-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,6 @@ import { Effect, Schema } from "effect"
import { matchesQuery } from "./model-search"
import DESCRIPTION from "./agent-manager.txt"

function strict<const Fields extends Schema.Struct.Fields>(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",
Expand Down Expand Up @@ -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<string, unknown>
if (item.type === "object" && item.additionalProperties === undefined) {
item.additionalProperties = false
}
if (item.properties && typeof item.properties === "object") {
const properties = item.properties as Record<string, unknown>
if (properties.sessionID && typeof properties.sessionID === "object") {
delete (properties.sessionID as Record<string, unknown>).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",
}),
Expand All @@ -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)),
Expand All @@ -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.",
}),
Expand All @@ -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<typeof Task>
type Selected = { task?: AgentManagerTask; error?: string }
Expand Down Expand Up @@ -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) {
Expand Down
98 changes: 28 additions & 70 deletions packages/opencode/test/kilocode/agent-manager-tool.test.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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 because more complex schemas break Claude models", 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<Record<string, unknown>>
const properties = (branch: Record<string, unknown>) => branch.properties as Record<string, unknown>
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 () => {
Expand Down
Loading