From 3e7827d56ddf8d6bd0e0481b72edc19124cfb49d Mon Sep 17 00:00:00 2001 From: tly Date: Sat, 11 Jul 2026 19:32:35 +0800 Subject: [PATCH] fix(core): restore permission-aware subagent guidance --- packages/core/src/agent/guidance.ts | 94 +++++++++++++++++++++++ packages/core/src/session/runner/llm.ts | 4 + packages/core/test/agent-guidance.test.ts | 94 +++++++++++++++++++++++ packages/core/test/tool-subagent.test.ts | 56 +++++++++++++- 4 files changed, 245 insertions(+), 3 deletions(-) create mode 100644 packages/core/src/agent/guidance.ts create mode 100644 packages/core/test/agent-guidance.test.ts diff --git a/packages/core/src/agent/guidance.ts b/packages/core/src/agent/guidance.ts new file mode 100644 index 000000000000..f64e6653e379 --- /dev/null +++ b/packages/core/src/agent/guidance.ts @@ -0,0 +1,94 @@ +export * as SubagentGuidance from "./guidance" + +import { Context, Effect, Layer, Schema } from "effect" +import { makeLocationNode } from "../effect/app-node" +import { Instructions } from "../instructions/index" +import { PermissionV2 } from "../permission" +import { AgentV2 } from "../agent" + +const Summary = Schema.Struct({ + id: AgentV2.ID, + description: Schema.String.pipe(Schema.optional), +}) +type Summary = typeof Summary.Type + +const entries = (agents: ReadonlyArray) => + agents.flatMap((agent) => [ + " ", + ` ${agent.id}`, + ...(agent.description === undefined ? [] : [` ${agent.description}`]), + " ", + ]) + +const render = (agents: ReadonlyArray) => + [ + "Use the subagent tool to delegate work only to the agents listed below.", + "", + ...entries(agents), + "", + ].join("\n") + +const update = (previous: ReadonlyArray, current: ReadonlyArray) => { + const diff = Instructions.diffByKey( + previous, + current, + (agent) => agent.id, + (before, after) => before.description !== after.description, + ) + if (diff.changed.length > 0 || (diff.added.length === 0 && diff.removed.length === 0)) + return [ + "The available subagents have changed. This list supersedes the previous available subagent list.", + render(current), + ].join("\n") + return [ + ...(diff.added.length === 0 + ? [] + : ["New subagents are available in addition to those previously listed:", ...entries(diff.added)]), + ...(diff.removed.length === 0 + ? [] + : [ + `The following subagent IDs are no longer available and must not be used: ${diff.removed.map((agent) => agent.id).join(", ")}.`, + ]), + ].join("\n") +} + +export interface Interface { + readonly load: (agent: AgentV2.Selection) => Effect.Effect +} + +export class Service extends Context.Service()("@opencode/v2/SubagentGuidance") {} + +const layer = Layer.effect( + Service, + Effect.gen(function* () { + const agents = yield* AgentV2.Service + + return Service.of({ + load: Effect.fn("SubagentGuidance.load")(function* (selection) { + const selected = selection.info + if (!selected) return Instructions.empty + const available = (yield* agents.list()) + .filter( + (agent) => + agent.mode !== "primary" && + !agent.hidden && + PermissionV2.evaluate("subagent", agent.id, selected.permissions).effect !== "deny", + ) + .map((agent) => ({ id: agent.id, description: agent.description })) + .toSorted((a, b) => a.id.localeCompare(b.id)) + return Instructions.make>({ + key: Instructions.Key.make("core/subagent-guidance"), + codec: Schema.toCodecJson(Schema.Array(Summary)), + read: Effect.succeed(available.length === 0 ? Instructions.removed : available), + render: { + initial: render, + changed: update, + removed: () => "Subagent guidance is no longer available. Do not use any previously listed subagent.", + }, + }) + }), + }) + }), +) + +export const node = makeLocationNode({ service: Service, layer, deps: [AgentV2.node] }) diff --git a/packages/core/src/session/runner/llm.ts b/packages/core/src/session/runner/llm.ts index 932470d869c7..e45cf31c1d88 100644 --- a/packages/core/src/session/runner/llm.ts +++ b/packages/core/src/session/runner/llm.ts @@ -14,6 +14,7 @@ import { SessionError } from "@opencode-ai/schema/session-error" import { Money } from "@opencode-ai/schema/money" import { Cause, Effect, Exit, Fiber, FiberSet, Layer, Option, Semaphore, Stream } from "effect" import { AgentV2 } from "../../agent" +import { SubagentGuidance } from "../../agent/guidance" import { Database } from "../../database/database" import { EventV2 } from "../../event" import { Location } from "../../location" @@ -92,6 +93,7 @@ const layer = Layer.effect( const store = yield* SessionStore.Service const location = yield* Location.Service const builtins = yield* InstructionBuiltIns.Service + const subagentGuidance = yield* SubagentGuidance.Service const discovery = yield* InstructionDiscovery.Service const skillGuidance = yield* SkillGuidance.Service const referenceGuidance = yield* ReferenceGuidance.Service @@ -145,6 +147,7 @@ const layer = Layer.effect( Effect.all( [ builtins.load(), + subagentGuidance.load(agent), discovery.load(), skillGuidance.load(agent), referenceGuidance.load(), @@ -569,6 +572,7 @@ export const node = makeLocationNode({ EventV2.node, llmClient, AgentV2.node, + SubagentGuidance.node, ToolRegistry.node, SessionRunnerModel.node, SessionStore.node, diff --git a/packages/core/test/agent-guidance.test.ts b/packages/core/test/agent-guidance.test.ts new file mode 100644 index 000000000000..54189ebcaa73 --- /dev/null +++ b/packages/core/test/agent-guidance.test.ts @@ -0,0 +1,94 @@ +import { describe, expect } from "bun:test" +import { Effect, Layer } from "effect" +import { AgentV2 } from "@opencode-ai/core/agent" +import { SubagentGuidance } from "@opencode-ai/core/agent/guidance" +import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" +import { it } from "./lib/effect" +import { readInitial, readUpdate } from "./lib/instructions" + +const info = (id: string, input: Partial = {}) => + AgentV2.Info.make({ ...AgentV2.Info.empty(AgentV2.ID.make(id)), ...input }) + +const explore = info("explore", { mode: "subagent", description: "Search the codebase" }) +const reviewer = info("reviewer", { mode: "subagent", description: "Review code changes" }) +const hidden = info("hidden", { mode: "subagent", hidden: true, description: "Internal agent" }) +const primary = info("plan", { mode: "primary", description: "Plan changes" }) + +const layer = (list: () => AgentV2.Info[]) => + AppNodeBuilder.build(SubagentGuidance.node, [ + [AgentV2.node, Layer.mock(AgentV2.Service, { list: () => Effect.succeed(list()) })], + ]) + +describe("SubagentGuidance", () => { + it.effect("lists only visible subagents permitted for the selected agent", () => { + const selected = info("build", { + mode: "primary", + permissions: [ + { action: "subagent", resource: "*", effect: "deny" }, + { action: "subagent", resource: "explore", effect: "allow" }, + ], + }) + return Effect.gen(function* () { + const guidance = yield* SubagentGuidance.Service + const initialized = yield* guidance.load({ id: selected.id, info: selected }).pipe(Effect.flatMap(readInitial)) + + expect(initialized.text).toBe( + [ + "Use the subagent tool to delegate work only to the agents listed below.", + "", + " ", + " explore", + " Search the codebase", + " ", + "", + ].join("\n"), + ) + expect(initialized.text).not.toContain("reviewer") + expect(initialized.text).not.toContain("hidden") + expect(initialized.text).not.toContain("plan") + }).pipe(Effect.provide(layer(() => [reviewer, primary, hidden, explore]))) + }) + + it.effect("tracks permission and catalog changes as instruction updates", () => { + const selected = info("build", { mode: "primary" }) + let agents = [explore] + return Effect.gen(function* () { + const guidance = yield* SubagentGuidance.Service + const initialized = yield* guidance.load({ id: selected.id, info: selected }).pipe(Effect.flatMap(readInitial)) + + agents = [reviewer] + const updated = yield* guidance + .load({ id: selected.id, info: selected }) + .pipe(Effect.flatMap((instructions) => readUpdate(instructions, initialized))) + expect(updated.text).toBe( + [ + "New subagents are available in addition to those previously listed:", + " ", + " reviewer", + " Review code changes", + " ", + "The following subagent IDs are no longer available and must not be used: explore.", + ].join("\n"), + ) + + agents = [] + expect( + yield* guidance + .load({ id: selected.id, info: selected }) + .pipe(Effect.flatMap((instructions) => readUpdate(instructions, updated))), + ).toMatchObject({ + text: "Subagent guidance is no longer available. Do not use any previously listed subagent.", + }) + }).pipe(Effect.provide(layer(() => agents))) + }) + + it.effect("omits guidance when the selected agent is unresolved", () => + Effect.gen(function* () { + const guidance = yield* SubagentGuidance.Service + expect( + (yield* guidance.load({ id: AgentV2.ID.make("missing"), info: undefined }).pipe(Effect.flatMap(readInitial))) + .text, + ).toBe("") + }).pipe(Effect.provide(layer(() => [explore]))), + ) +}) diff --git a/packages/core/test/tool-subagent.test.ts b/packages/core/test/tool-subagent.test.ts index 56e3587162d6..ad9d703c0953 100644 --- a/packages/core/test/tool-subagent.test.ts +++ b/packages/core/test/tool-subagent.test.ts @@ -1,5 +1,5 @@ import { describe, expect } from "bun:test" -import { DateTime, Effect, Fiber, Layer, Schema, Stream } from "effect" +import { Effect, Fiber, Layer, Schema, Stream } from "effect" import { Money } from "@opencode-ai/schema/money" import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" import { LayerNode } from "@opencode-ai/core/effect/layer-node" @@ -105,7 +105,7 @@ const layer = AppNodeBuilder.build( const it = testEffect(layer) -const withSubagent = (location: Location.Ref) => +const withSubagent = (location: Location.Ref, permissions: AgentV2.Info["permissions"] = []) => Effect.gen(function* () { const locations = yield* LocationServiceMap.Service yield* PluginSupervisor.Service.use((supervisor) => supervisor.flush).pipe(Effect.provide(locations.get(location))) @@ -114,7 +114,7 @@ const withSubagent = (location: Location.Ref) => // The caller identity used by executeTool; subagent permission asserts against it. draft.update(toolIdentity.agent, (agent) => { agent.mode = "primary" - agent.permissions.push({ action: "*", resource: "*", effect: "allow" }) + agent.permissions.push({ action: "*", resource: "*", effect: "allow" }, ...permissions) }) draft.update(AgentV2.ID.make("reviewer"), (agent) => { agent.mode = "subagent" @@ -216,6 +216,56 @@ describe("SubagentTool", () => { ), ) + it.live("enforces target-specific subagent permissions before creating a child session", () => + Effect.acquireRelease( + Effect.promise(() => tmpdir()), + (dir) => Effect.promise(() => dir[Symbol.asyncDispose]()), + ).pipe( + Effect.flatMap((dir) => + Effect.gen(function* () { + const location = Location.Ref.make({ directory: AbsolutePath.make(dir.path) }) + const sessions = yield* SessionV2.Service + const parent = yield* sessions.create({ location, model: parentModel }) + yield* withSubagent(parent.location, [ + { action: SubagentTool.name, resource: "*", effect: "deny" }, + { action: SubagentTool.name, resource: "reviewer", effect: "allow" }, + ]) + const locations = yield* LocationServiceMap.Service + const registry = yield* ToolRegistry.Service.pipe(Effect.provide(locations.get(parent.location))) + yield* waitForTool(registry, SubagentTool.name) + + expect( + yield* executeTool(registry, { + sessionID: parent.id, + ...toolIdentity, + call: { + type: "tool-call", + id: "call-denied-subagent", + name: SubagentTool.name, + input: { agent: "fallback", description: "denied", prompt: "do not run" }, + }, + }), + ).toEqual({ type: "error", value: "Subagent denied: fallback" }) + expect((yield* sessions.list({ parentID: parent.id })).data).toHaveLength(0) + + expect( + yield* settleTool(registry, { + sessionID: parent.id, + ...toolIdentity, + call: { + type: "tool-call", + id: "call-allowed-subagent", + name: SubagentTool.name, + input: { agent: "reviewer", description: "allowed", prompt: "review this" }, + }, + }), + ).toMatchObject({ output: { structured: { status: "completed", output: childText } } }) + expect((yield* sessions.list({ parentID: parent.id })).data).toHaveLength(1) + }), + ), + ), + ) + it.live("returns child runner failures as tool errors", () => Effect.acquireRelease( Effect.promise(() => tmpdir()),