From b452368b3bc6f3dc60d8f967552671cd39d7d4fd Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Thu, 9 Jul 2026 22:01:31 -0400 Subject: [PATCH] refactor(core): simplify tool admission flow (#36180) --- packages/codemode/codemode.md | 25 ++-- packages/core/src/session/run-coordinator.ts | 14 +-- packages/core/src/session/runner/llm.ts | 61 +-------- packages/core/src/tool/AGENTS.md | 2 +- packages/core/src/tool/execute.ts | 6 +- packages/core/src/tool/registry.ts | 34 ++--- packages/core/test/lib/tool.ts | 18 +-- packages/core/test/plugin.test.ts | 13 +- .../core/test/session-instructions.test.ts | 2 +- .../test/session-runner-tool-registry.test.ts | 117 +++++------------- packages/core/test/session-runner.test.ts | 51 ++++---- packages/core/test/tool-patch.test.ts | 16 +-- packages/core/test/tool-question.test.ts | 2 +- packages/core/test/tool-subagent.test.ts | 6 +- specs/v2/session.md | 2 +- specs/v2/tools.md | 7 +- 16 files changed, 110 insertions(+), 266 deletions(-) diff --git a/packages/codemode/codemode.md b/packages/codemode/codemode.md index 440271dc06..8a93beca51 100644 --- a/packages/codemode/codemode.md +++ b/packages/codemode/codemode.md @@ -95,7 +95,8 @@ CodeMode is integrated into V2 through `packages/core/src/tool/registry.ts` and normally. - When visible deferred tools exist, Core reserves and materializes one `execute` tool. Grouped deferred tools become CodeMode namespaces instead of flattened model-facing names. -- Each nested call checks that its captured registration is still current before dispatching it. +- Nested calls execute the registered `Tool` values captured for the model request; later registrations affect later + requests. - Authorization and side-effect ordering remain responsibilities of the leaf tool. Catalog visibility is not execution authorization. - Structured child output enters the interpreter. File parts are collected host-side and attached to the outer result. @@ -126,18 +127,18 @@ represent accurately rather than guessing semantics. ## Decisions and Rationale -| Decision | Rationale | -| --- | --- | -| Keep an owned tree-walking interpreter. | The product need is bounded tool orchestration, not arbitrary JavaScript. Owning the language surface keeps authority and behavior explicit. | -| Treat schemas as the model-facing interface. | Signatures drive correct calls; Effect Schema also provides the runtime validation boundary, while JSON Schema supports adapter interoperability. | -| Keep authority host-owned. | CodeMode can only confine programs to supplied tools. The host chooses those tools, and each tool enforces its own authorization and side-effect policy. | -| Use progressive catalog disclosure plus search. | Large tool sets should not consume the prompt, but every namespace must remain discoverable and speculative search calls should remain valid. | -| Start tool promises eagerly and supervise them. | This preserves normal call-time parallelism while giving each call run-once settlement and interruption safety. | -| Keep files outside the sandbox value space. | Models should compose structured data without routing binary payloads through generated code or context. | +| Decision | Rationale | +| -------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Keep an owned tree-walking interpreter. | The product need is bounded tool orchestration, not arbitrary JavaScript. Owning the language surface keeps authority and behavior explicit. | +| Treat schemas as the model-facing interface. | Signatures drive correct calls; Effect Schema also provides the runtime validation boundary, while JSON Schema supports adapter interoperability. | +| Keep authority host-owned. | CodeMode can only confine programs to supplied tools. The host chooses those tools, and each tool enforces its own authorization and side-effect policy. | +| Use progressive catalog disclosure plus search. | Large tool sets should not consume the prompt, but every namespace must remain discoverable and speculative search calls should remain valid. | +| Start tool promises eagerly and supervise them. | This preserves normal call-time parallelism while giving each call run-once settlement and interruption safety. | +| Keep files outside the sandbox value space. | Models should compose structured data without routing binary payloads through generated code or context. | | Treat `execute` as the model-facing invocation boundary. | Nested calls are implementation details of one orchestration program. Reusing the outer context and bounding only the final result preserves complete intermediate data without inventing durable child-call identities. | -| Return expected failures as data. | Models need actionable diagnostics without exposing private host causes; host interruption and defects must still propagate correctly. | -| Leave execution-limit defaults to hosts. | Appropriate budgets depend on the surrounding product and its own cancellation, retention, and output-bounding policies. | -| Skip unsupported OpenAPI operations. | Incorrect parameter encoding, authentication, or transport behavior is worse than a precise `skipped` reason. | +| Return expected failures as data. | Models need actionable diagnostics without exposing private host causes; host interruption and defects must still propagate correctly. | +| Leave execution-limit defaults to hosts. | Appropriate budgets depend on the surrounding product and its own cancellation, retention, and output-bounding policies. | +| Skip unsupported OpenAPI operations. | Incorrect parameter encoding, authentication, or transport behavior is worse than a precise `skipped` reason. | ## Remaining Work diff --git a/packages/core/src/session/run-coordinator.ts b/packages/core/src/session/run-coordinator.ts index 1550280164..c0876c7203 100644 --- a/packages/core/src/session/run-coordinator.ts +++ b/packages/core/src/session/run-coordinator.ts @@ -28,7 +28,6 @@ type Execution = { owner?: Fiber.Fiber pendingWake: boolean stopping: boolean - settling: boolean interruptionReason?: Reason } @@ -74,7 +73,6 @@ export const make = (options: { done: Deferred.makeUnsafe(), pendingWake: false, stopping: false, - settling: false, } executions.set(key, execution) // The leading yield lets `owner` be assigned before the drain can settle, and keeps @@ -86,7 +84,7 @@ export const make = (options: { Effect.andThen(loop(key, execution, force)), Effect.onExit((exit) => Effect.sync(() => { - execution.settling = true + execution.owner = undefined }).pipe(Effect.andThen(options.settled?.(key, exit, execution.interruptionReason) ?? Effect.void)), ), Effect.onExit((exit) => Effect.sync(() => settle(key, execution, exit))), @@ -106,14 +104,14 @@ export const make = (options: { } const run = (key: Key): Effect.Effect => - Effect.uninterruptibleMask((restore) => { + Effect.suspend(() => { const execution = executions.get(key) if (execution !== undefined) { // A stopping execution refuses joiners: wait out its cleanup, then run fresh. - if (execution.stopping) return restore(Deferred.await(execution.done).pipe(Effect.andThen(run(key)))) - return restore(Deferred.await(execution.done)) + if (execution.stopping) return Deferred.await(execution.done).pipe(Effect.andThen(run(key))) + return Deferred.await(execution.done) } - return restore(Deferred.await(start(key, true).done)) + return Deferred.await(start(key, true).done) }) const wake = (key: Key) => @@ -129,7 +127,7 @@ export const make = (options: { const interrupt = (key: Key, reason?: Reason): Effect.Effect => Effect.suspend(() => { const execution = executions.get(key) - if (execution?.owner === undefined || execution.stopping || execution.settling) return Effect.void + if (execution?.owner === undefined || execution.stopping) return Effect.void execution.stopping = true execution.pendingWake = false execution.interruptionReason = reason diff --git a/packages/core/src/session/runner/llm.ts b/packages/core/src/session/runner/llm.ts index 556c1168dc..33d3fc2766 100644 --- a/packages/core/src/session/runner/llm.ts +++ b/packages/core/src/session/runner/llm.ts @@ -80,53 +80,8 @@ export function calculateCost(costs: ModelV2.Info["cost"], tokens: StepTokens) { } /** - * Runs one durable coding-agent Session until it settles. - * - * Keep this as orchestration over smaller collaborators rather than rebuilding the legacy - * `SessionPrompt` monolith. Implement the unchecked items in small reviewed slices: - * - * - Session ownership and controls - * - [x] Coordinate one local active drain per Session; explicit resumes join and prompt wakeups coalesce. - * - [ ] Replace local ownership with durable multi-node ownership when clustered. - * - [x] Publish durable historical execution lifecycle and bounded retry observations. - * - [ ] Honor interruption and reject stale work after runtime attachment replacement. - * - [x] Honor optional agent step limits. - * - [ ] Bound repeated identical tool calls (provider retries are bounded). - * - * - Runtime context assembly - * - Track V1 runtime-context parity canonically in `specs/v2/session.md`. - * - * - One step - * - [x] Translate every projected V2 Session message variant into canonical - * `@opencode-ai/llm` messages. - * - [ ] Resolve policy-filtered built-in, MCP, plugin, and structured-output tool definitions. - * - [x] Stream exactly one `llm.stream(request)` call per attempt. - * - [x] Persist assistant text and usage events incrementally as they arrive. - * - [ ] Persist snapshots, patches, and retry notices incrementally as they arrive. - * - [x] Persist reasoning, provider errors, and tool-call events incrementally as they arrive. - * - * - Tool settlement and continuation - * - [x] Durably record each tool call before side effects begin. - * - [x] Authorize and execute recorded local calls through a core-owned registry hook. - * - [x] Persist typed success, failure, and provider-executed tool outcomes. - * - [x] Start each recorded local call eagerly and await all settlements before continuation. - * - [ ] Add scoped runtime context, progress updates, attachment normalization, - * plugins, and cancellation settlement. - * - [x] Reload projected history and start the next explicit step after local tool results. - * - [x] Continue for durable user steering accepted during an active step. - * - [ ] Continue for compaction or another continuation condition when required. - * - * - Post-run maintenance - * - [ ] Settle final status and expose durable output events to replayable consumers. - * - [ ] Coalesce streamed deltas and add covering projected-history indexes. - * - [ ] Update title, summaries, compaction state, and cleanup in bounded background work. - * - * Use `llm.stream(request)` for each attempt. Keep tool execution and continuation here. - * Durable continuation recovery remains a separate future slice with an explicit retry policy. - * - * The current slice loads V2 history, translates it, resolves a model through a core service, and persists one - * step. Registry definitions are advertised, local tool calls are settled durably, and an - * explicit loop starts the next step after local settlement. Configured agent step limits bound the loop. + * Runs one durable coding-agent Session until it settles. Each step reloads projected history, + * materializes tools, makes one model request, and settles local calls before continuation. */ const layer = Layer.effect( @@ -239,9 +194,7 @@ const layer = Layer.effect( const entries = yield* SessionHistory.entriesForRunner(db, session.id, checkpoint.baselineSeq) const context = entries.map((entry) => entry.message) const isLastStep = agentInfo.steps !== undefined && currentStep >= agentInfo.steps - const toolMaterialization = isLastStep - ? undefined - : yield* tools.materialize({ permissions: agentInfo.permissions, model }) + const toolMaterialization = isLastStep ? undefined : yield* tools.materialize(agentInfo.permissions) const promptCacheKey = /^ses_[0-9a-f]{64}$/.test(session.id) ? session.id.slice(4) : session.id const request = LLM.request({ model, @@ -457,8 +410,8 @@ const layer = Layer.effect( const toolsInterrupted = settledCauses.some(Cause.hasInterrupts) const userDeclined = settledCauses.some(isUserDeclined) + if (settled._tag === "Failure") yield* FiberSet.clear(toolFibers) if (userDeclined || streamInterrupted || toolsInterrupted) { - yield* FiberSet.clear(toolFibers) yield* serialized(publisher.failUnsettledTools({ type: "aborted", message: "Tool execution interrupted" })) yield* serialized(publisher.failAssistant({ type: "aborted", message: "Step interrupted" })) } @@ -509,9 +462,7 @@ const layer = Layer.effect( const stepFailure = publisher.stepFailure() const stepSettlement = publisher.stepSettlement() - const stepEndedCleanly = - !streamInterrupted && !toolsInterrupted && infraError === undefined && !providerFailed && !stepFailure - if (stepSettlement && stepEndedCleanly) yield* publishStepEnd(stepSettlement) + if (stepSettlement && !stepFailure) yield* publishStepEnd(stepSettlement) if (stepFailure) yield* serialized(publisher.publishStepFailure(stepSettlement ? stepUsage(stepSettlement) : undefined)) @@ -523,7 +474,7 @@ const layer = Layer.effect( if (stepFailure) return yield* new StepFailedError({ error: stepFailure }) return { _tag: "Completed", - needsContinuation: !providerFailed && needsContinuation, + needsContinuation, step: currentStep, } as const }), diff --git a/packages/core/src/tool/AGENTS.md b/packages/core/src/tool/AGENTS.md index 0b3f9c3628..9bfd05f1a4 100644 --- a/packages/core/src/tool/AGENTS.md +++ b/packages/core/src/tool/AGENTS.md @@ -35,7 +35,7 @@ Registrations are scoped: - The latest active same-placement registration wins. - Closing any registration removes only that registration and reveals the next active one. -- An invocation captures the effective tool once settlement starts. +- Each model request captures the effective tools it advertises; later registration changes affect later requests. `ToolRegistry.Service` is Location-scoped. Do not make the registry process-global or construct a separate application-tool service for each Location. diff --git a/packages/core/src/tool/execute.ts b/packages/core/src/tool/execute.ts index d9769c396e..35990f3842 100644 --- a/packages/core/src/tool/execute.ts +++ b/packages/core/src/tool/execute.ts @@ -36,19 +36,19 @@ type CollectedFiles = { readonly files: Array } -export interface Registration { +interface Registration { readonly tool: AnyTool readonly name: string readonly group?: string } -export const create = (options: { readonly registrations: ReadonlyMap }) => { +export const create = (registrations: ReadonlyMap) => { const runtime = ( invoke: (name: string, registration: Registration, input: unknown) => Effect.Effect, hooks?: CodeMode.ToolCallHooks, ) => { const tools: Record | Record>> = {} - for (const [name, registration] of options.registrations) { + for (const [name, registration] of registrations) { const child = definition(name, registration.tool) const value = Tool.make({ description: child.description, diff --git a/packages/core/src/tool/registry.ts b/packages/core/src/tool/registry.ts index 1588c81005..0429d7ba44 100644 --- a/packages/core/src/tool/registry.ts +++ b/packages/core/src/tool/registry.ts @@ -25,7 +25,7 @@ export type ExecuteInput = { } export interface Interface { - readonly materialize: (input: MaterializeInput) => Effect.Effect + readonly materialize: (permissions?: PermissionV2.Ruleset) => Effect.Effect /** Internal registration capability exposed publicly only through Tools.Service. */ readonly register: ( tools: Readonly>, @@ -33,11 +33,6 @@ export interface Interface { ) => Effect.Effect } -export interface MaterializeInput { - readonly model: { readonly id: string; readonly provider: string } - readonly permissions?: PermissionV2.Ruleset -} - export interface Materialization { readonly definitions: ReadonlyArray readonly settle: (input: ExecuteInput) => Effect.Effect @@ -171,27 +166,20 @@ const registryLayer = Layer.effect( }), ) }), - materialize: Effect.fn("ToolRegistry.materialize")(function* (input) { - const registrations = new Map() + materialize: Effect.fn("ToolRegistry.materialize")(function* (permissions) { + const direct = new Map() + const deferred = new Map() + const rules = permissions ?? [] for (const [name, entries] of local) { const registration = entries.at(-1)?.registration - if (registration) registrations.set(name, registration) + if (!registration) continue + if (registration.deferred && !Flag.CODEMODE_ENABLED) continue + if (whollyDisabled(permission(registration.tool, name), rules)) continue + if (registration.deferred) deferred.set(name, registration) + else direct.set(name, registration) } - for (const [name, registration] of registrations) { - if ( - (registration.deferred && !Flag.CODEMODE_ENABLED) || - whollyDisabled(permission(registration.tool, name), input.permissions ?? []) - ) - registrations.delete(name) - } - const direct = new Map(Array.from(registrations).filter(([, registration]) => !registration.deferred)) - const deferred = new Map(Array.from(registrations).filter(([, registration]) => registration.deferred)) const execute = - deferred.size > 0 && !whollyDisabled("execute", input.permissions ?? []) - ? ExecuteTool.create({ - registrations: deferred, - }) - : undefined + deferred.size > 0 && !whollyDisabled("execute", rules) ? ExecuteTool.create(deferred) : undefined return { definitions: [ ...Array.from(direct, ([name, registration]) => definition(name, registration.tool)), diff --git a/packages/core/test/lib/tool.ts b/packages/core/test/lib/tool.ts index c09d1e6296..10d68c7fa3 100644 --- a/packages/core/test/lib/tool.ts +++ b/packages/core/test/lib/tool.ts @@ -13,14 +13,8 @@ export const toolIdentity = { assistantMessageID: SessionMessage.ID.make("msg_tool_test"), } -// Default fixture model: a non-OpenAI provider, so edit and write are the materialized edit tools. -export const testModel: ToolRegistry.MaterializeInput["model"] = { id: "claude-test", provider: "anthropic" } - -export const toolDefinitions = ( - registry: ToolRegistry.Interface, - permissions?: PermissionV2.Ruleset, - model = testModel, -) => registry.materialize({ permissions, model }).pipe(Effect.map((materialized) => materialized.definitions)) +export const toolDefinitions = (registry: ToolRegistry.Interface, permissions?: PermissionV2.Ruleset) => + registry.materialize(permissions).pipe(Effect.map((materialized) => materialized.definitions)) export function waitForTool( registry: ToolRegistry.Interface, @@ -76,8 +70,8 @@ export const registerToolPlugin = (plugin: { yield* plugin.effect(context) }) -export const settleTool = (registry: ToolRegistry.Interface, input: ToolRegistry.ExecuteInput, model = testModel) => - registry.materialize({ model }).pipe(Effect.flatMap((materialized) => materialized.settle(input))) +export const settleTool = (registry: ToolRegistry.Interface, input: ToolRegistry.ExecuteInput) => + registry.materialize().pipe(Effect.flatMap((materialized) => materialized.settle(input))) -export const executeTool = (registry: ToolRegistry.Interface, input: ToolRegistry.ExecuteInput, model = testModel) => - settleTool(registry, input, model).pipe(Effect.map((settlement) => settlement.result)) +export const executeTool = (registry: ToolRegistry.Interface, input: ToolRegistry.ExecuteInput) => + settleTool(registry, input).pipe(Effect.map((settlement) => settlement.result)) diff --git a/packages/core/test/plugin.test.ts b/packages/core/test/plugin.test.ts index 5b91a336ad..a21dc25479 100644 --- a/packages/core/test/plugin.test.ts +++ b/packages/core/test/plugin.test.ts @@ -12,7 +12,6 @@ import { SessionMessage } from "@opencode-ai/core/session/message" import { Tool } from "@opencode-ai/core/tool/tool" import { ToolRegistry } from "@opencode-ai/core/tool/registry" import { testEffect } from "./lib/effect" -import { testModel } from "./lib/tool" import { PluginTestLayer } from "./plugin/fixture" const it = testEffect(PluginTestLayer) @@ -255,14 +254,10 @@ describe("PluginV2", () => { }) yield* plugins.activate([plugin]) - expect((yield* registry.materialize({ model: testModel })).definitions.map((tool) => tool.name)).toContain( - "plugin_tool", - ) + expect((yield* registry.materialize()).definitions.map((tool) => tool.name)).toContain("plugin_tool") yield* plugins.activate([]) - expect((yield* registry.materialize({ model: testModel })).definitions.map((tool) => tool.name)).not.toContain( - "plugin_tool", - ) + expect((yield* registry.materialize()).definitions.map((tool) => tool.name)).not.toContain("plugin_tool") }), ) @@ -291,7 +286,7 @@ describe("PluginV2", () => { yield* plugins.activate([plugin]) - expect((yield* registry.materialize({ model: testModel })).definitions.map((tool) => tool.name)).toEqual([ + expect((yield* registry.materialize()).definitions.map((tool) => tool.name)).toEqual([ "plain", "context_7_look_up", "execute", @@ -350,7 +345,7 @@ describe("PluginV2", () => { yield* plugins.activate([plugin]) - const materialized = yield* registry.materialize({ model: testModel }) + const materialized = yield* registry.materialize() const settlement = yield* materialized.settle({ sessionID: SessionV2.ID.make("ses_hooks"), agent: AgentV2.ID.make("build"), diff --git a/packages/core/test/session-instructions.test.ts b/packages/core/test/session-instructions.test.ts index 34439836a4..6faa0ce52c 100644 --- a/packages/core/test/session-instructions.test.ts +++ b/packages/core/test/session-instructions.test.ts @@ -34,7 +34,7 @@ import { ToolRegistry } from "@opencode-ai/core/tool/registry" import { tempLocationLayer } from "./fixture/location" import { makeLocationNode } from "@opencode-ai/core/effect/app-node" import { testEffect } from "./lib/effect" -import { registerToolPlugin, settleTool, testModel } from "./lib/tool" +import { registerToolPlugin, settleTool } from "./lib/tool" const readToolNode = makeLocationNode({ name: "test/read-tool-plugin", diff --git a/packages/core/test/session-runner-tool-registry.test.ts b/packages/core/test/session-runner-tool-registry.test.ts index a1c11583a4..8c14b2aebf 100644 --- a/packages/core/test/session-runner-tool-registry.test.ts +++ b/packages/core/test/session-runner-tool-registry.test.ts @@ -7,7 +7,7 @@ import { SessionV2 } from "@opencode-ai/core/session" import { SessionMessage } from "@opencode-ai/core/session/message" import { ToolOutputStore } from "@opencode-ai/core/tool-output-store" import { ToolRegistry } from "@opencode-ai/core/tool/registry" -import { executeTool, settleTool, testModel, toolDefinitions } from "./lib/tool" +import { executeTool, settleTool, toolDefinitions } from "./lib/tool" import { Cause, Deferred, Effect, Exit, Fiber, Layer, Option, Schema, SchemaGetter, SchemaIssue, Scope } from "effect" import { testEffect } from "./lib/effect" @@ -52,6 +52,15 @@ const make = (permission?: string) => { return permission ? Tool.withPermission(tool, permission) : tool } +const constant = (text: string) => + Tool.make({ + description: "Return text", + input: Schema.Struct({ text: Schema.String }), + output: Schema.Struct({ text: Schema.String }), + execute: () => Effect.succeed({ text }), + toModelOutput: ({ output }) => [{ type: "text" as const, text: output.text }], + }) + describe("ToolRegistry", () => { it.effect("filters disabled tools with edit aliases and ordered wildcard precedence", () => Effect.gen(function* () { @@ -82,25 +91,6 @@ describe("ToolRegistry", () => { }), ) - it.effect("materializes all permission-eligible edit tools before request policy", () => - Effect.gen(function* () { - const service = yield* ToolRegistry.Service - yield* service.register({ - read: make(), - edit: make("edit"), - write: make("edit"), - patch: make("edit"), - }) - const names = (model: ToolRegistry.MaterializeInput["model"]) => - service - .materialize({ model }) - .pipe(Effect.map((materialized) => materialized.definitions.map((tool) => tool.name))) - - expect(yield* names({ id: "gpt-5", provider: "openai" })).toEqual(["read", "edit", "write", "patch"]) - expect(yield* names({ id: "claude-sonnet-4", provider: "anthropic" })).toEqual(["read", "edit", "write", "patch"]) - }), - ) - it.effect("keeps permission decoration isolated between registrations", () => Effect.gen(function* () { const service = yield* ToolRegistry.Service @@ -117,7 +107,7 @@ describe("ToolRegistry", () => { }), ) - it.effect("reuses model definitions across provider turns", () => + it.effect("reuses model definitions across requests", () => Effect.gen(function* () { const service = yield* ToolRegistry.Service yield* service.register({ echo: make() }) @@ -196,7 +186,7 @@ describe("ToolRegistry", () => { }), }) expect( - yield* service.materialize({ model: testModel }).pipe( + yield* service.materialize().pipe( Effect.flatMap((materialized) => materialized.settle({ sessionID, @@ -214,7 +204,7 @@ describe("ToolRegistry", () => { Effect.gen(function* () { const service = yield* ToolRegistry.Service yield* service.register({ echo: make() }) - const materialized = yield* service.materialize({ model: testModel }) + const materialized = yield* service.materialize() const exit = yield* materialized.settle(call("echo", "call-retention-failure")).pipe(Effect.exit) expect(Exit.isFailure(exit)).toBe(true) @@ -340,54 +330,34 @@ describe("ToolRegistry", () => { }), ) - it.effect("executes the unchanged registration advertised for a provider turn", () => - Effect.gen(function* () { - const service = yield* ToolRegistry.Service - yield* service.register({ echo: make() }) - const materialized = yield* service.materialize({ model: testModel }) - - expect((yield* materialized.settle(call("echo"))).result).toEqual({ type: "text", value: "echo" }) - }), - ) - - it.effect("executes the advertised registration after it is removed", () => + it.effect("executes the tool advertised in a model request", () => Effect.gen(function* () { const service = yield* ToolRegistry.Service const scope = yield* Scope.make() - yield* service.register({ echo: make() }).pipe(Scope.provide(scope)) - const materialized = yield* service.materialize({ model: testModel }) + yield* service.register({ echo: constant("advertised") }).pipe(Scope.provide(scope)) + const request = yield* service.materialize() yield* Scope.close(scope, Exit.void) + yield* service.register({ echo: constant("replacement") }) - expect((yield* materialized.settle(call("echo"))).result).toEqual({ type: "text", value: "echo" }) + expect((yield* request.settle(call("echo"))).result).toEqual({ type: "text", value: "advertised" }) + expect(yield* executeTool(service, call("echo"))).toEqual({ type: "text", value: "replacement" }) }), ) - it.effect("executes each registration advertised for a provider turn after replacement", () => + it.effect("reveals the previous registration after an overlay closes", () => Effect.gen(function* () { const service = yield* ToolRegistry.Service - yield* service.register({ first: make(), second: make() }) - const materialized = yield* service.materialize({ model: testModel }) - yield* service.register({ first: make() }) - - expect((yield* materialized.settle(call("first"))).result).toEqual({ type: "text", value: "first" }) - expect((yield* materialized.settle(call("second"))).result).toEqual({ type: "text", value: "second" }) - }), - ) - - it.effect("executes an advertised overlay after the previous registration is revealed", () => - Effect.gen(function* () { - const service = yield* ToolRegistry.Service - yield* service.register({ echo: make() }) + yield* service.register({ echo: constant("base") }) const overlay = yield* Scope.make() - yield* service.register({ echo: make() }).pipe(Scope.provide(overlay)) - const materialized = yield* service.materialize({ model: testModel }) - yield* Scope.close(overlay, Exit.void) + yield* service.register({ echo: constant("overlay") }).pipe(Scope.provide(overlay)) - expect((yield* materialized.settle(call("echo"))).result).toEqual({ type: "text", value: "echo" }) + expect(yield* executeTool(service, call("echo"))).toEqual({ type: "text", value: "overlay" }) + yield* Scope.close(overlay, Exit.void) + expect(yield* executeTool(service, call("echo"))).toEqual({ type: "text", value: "base" }) }), ) - it.effect("executes deferred registrations from the advertised generation", () => + it.effect("executes deferred tools advertised in a model request", () => Effect.gen(function* () { const service = yield* ToolRegistry.Service const executed: string[] = [] @@ -405,7 +375,7 @@ describe("ToolRegistry", () => { { deferred: true }, ) .pipe(Scope.provide(scope)) - const materialized = yield* service.materialize({ model: testModel }) + const materialized = yield* service.materialize() yield* Scope.close(scope, Exit.void) yield* service.register( { @@ -425,41 +395,12 @@ describe("ToolRegistry", () => { type: "tool-call", id: "call-execute", name: "execute", - input: { code: 'return await tools.echo({ text: "admitted" })' }, + input: { code: 'return await tools.echo({ text: "request" })' }, }, }) expect(settlement.result).toMatchObject({ type: "text" }) - expect(executed).toEqual(["old:admitted"]) - }), - ) - - it.effect("keeps captured execution running after registration mutation", () => - Effect.gen(function* () { - const service = yield* ToolRegistry.Service - const started = yield* Deferred.make() - const release = yield* Deferred.make() - const scope = yield* Scope.make() - yield* service - .register({ - echo: Tool.make({ - description: "Echo text", - input: Schema.Struct({ text: Schema.String }), - output: Schema.Struct({ text: Schema.String }), - execute: ({ text }) => - Deferred.succeed(started, undefined).pipe(Effect.andThen(Deferred.await(release)), Effect.as({ text })), - toModelOutput: ({ output }) => [{ type: "text", text: output.text }], - }), - }) - .pipe(Scope.provide(scope)) - const materialized = yield* service.materialize({ model: testModel }) - const settlement = yield* materialized.settle(call("echo")).pipe(Effect.forkChild) - yield* Deferred.await(started) - yield* Scope.close(scope, Exit.void) - yield* service.register({ echo: make() }) - yield* Deferred.succeed(release, undefined) - - expect(yield* Fiber.join(settlement)).toMatchObject({ result: { type: "text", value: "echo" } }) + expect(executed).toEqual(["old:request"]) }), ) }) diff --git a/packages/core/test/session-runner.test.ts b/packages/core/test/session-runner.test.ts index c1aae6333d..cf6b636a5b 100644 --- a/packages/core/test/session-runner.test.ts +++ b/packages/core/test/session-runner.test.ts @@ -788,19 +788,19 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("executes parallel tool calls against the generation admitted before a registry reload", () => + it.effect("executes the tool advertised before a registry reload", () => Effect.gen(function* () { const session = yield* setup const registry = yield* ToolRegistry.Service const scope = yield* Scope.make() - const generation: string[] = [] + const executions: string[] = [] yield* registry .register({ reloaded: Tool.make({ - description: "Record the admitted generation", + description: "Record the advertised tool", input: Schema.Struct({}), - output: Schema.Struct({ generation: Schema.String }), - execute: () => Effect.sync(() => generation.push("admitted")).pipe(Effect.as({ generation: "admitted" })), + output: Schema.Struct({ value: Schema.String }), + execute: () => Effect.sync(() => executions.push("advertised")).pipe(Effect.as({ value: "advertised" })), }), }) .pipe(Scope.provide(scope)) @@ -808,8 +808,7 @@ describe("SessionRunnerLLM", () => { responses = [ [ LLMEvent.stepStart({ index: 0 }), - LLMEvent.toolCall({ id: "call-reloaded-1", name: "reloaded", input: {} }), - LLMEvent.toolCall({ id: "call-reloaded-2", name: "reloaded", input: {} }), + LLMEvent.toolCall({ id: "call-reloaded", name: "reloaded", input: {} }), LLMEvent.stepFinish({ index: 0, reason: "tool-calls" }), LLMEvent.finish({ reason: "tool-calls" }), ], @@ -823,17 +822,16 @@ describe("SessionRunnerLLM", () => { yield* Scope.close(scope, Exit.void) yield* registry.register({ reloaded: Tool.make({ - description: "Record the replacement generation", + description: "Record the replacement tool", input: Schema.Struct({}), - output: Schema.Struct({ generation: Schema.String }), - execute: () => - Effect.sync(() => generation.push("replacement")).pipe(Effect.as({ generation: "replacement" })), + output: Schema.Struct({ value: Schema.String }), + execute: () => Effect.sync(() => executions.push("replacement")).pipe(Effect.as({ value: "replacement" })), }), }) yield* Deferred.succeed(streamGate, undefined) yield* Fiber.join(run) - expect(generation).toEqual(["admitted", "admitted"]) + expect(executions).toEqual(["advertised"]) expect(yield* session.context(sessionID)).toMatchObject([ { type: "user", text: "Use the reloaded tool" }, { @@ -841,13 +839,8 @@ describe("SessionRunnerLLM", () => { content: [ { type: "tool", - id: "call-reloaded-1", - state: { status: "completed", structured: { generation: "admitted" } }, - }, - { - type: "tool", - id: "call-reloaded-2", - state: { status: "completed", structured: { generation: "admitted" } }, + id: "call-reloaded", + state: { status: "completed", structured: { value: "advertised" } }, }, ], }, @@ -855,7 +848,7 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("starts a real runner turn after default prompt recording", () => + it.effect("starts a real runner step after default prompt recording", () => Effect.gen(function* () { const session = yield* setup @@ -919,7 +912,7 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("retries the first provider turn after system context becomes available", () => + it.effect("retries the first request after system context becomes available", () => Effect.gen(function* () { const session = yield* setup const { db } = yield* Database.Service @@ -2093,7 +2086,7 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("reloads a model switch before a tool-driven continuation turn", () => + it.effect("reloads a model switch before a tool-driven continuation step", () => Effect.gen(function* () { const session = yield* setup const events = yield* EventV2.Service @@ -2122,7 +2115,7 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("restores durable reasoning provider metadata in a second-turn request", () => + it.effect("restores durable reasoning provider metadata in the next request", () => Effect.gen(function* () { const session = yield* setup yield* admit(session, "Think first") @@ -2194,7 +2187,7 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("replays durable provider-executed tool results inline in a second-turn request", () => + it.effect("replays durable provider-executed tool results inline in the next request", () => Effect.gen(function* () { const session = yield* setup yield* admit(session, "Search first") @@ -2406,7 +2399,7 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("steers an active provider turn with newly recorded prompts", () => + it.effect("steers an active step with newly recorded prompts", () => Effect.gen(function* () { const session = yield* setup yield* admit(session, "Start working") @@ -2626,7 +2619,7 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("coalesces multiple active steering prompts into one continuation turn", () => + it.effect("coalesces multiple active steering prompts into one continuation step", () => Effect.gen(function* () { const session = yield* setup yield* admit(session, "Start working") @@ -2653,7 +2646,7 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("runs steering input accepted while the active provider turn fails", () => + it.effect("runs steering input accepted while the active step fails", () => Effect.gen(function* () { const session = yield* setup yield* admit(session, "Start working") @@ -3290,7 +3283,7 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("durably fails blocked local tools when a provider turn is interrupted", () => + it.effect("durably fails blocked local tools when a step is interrupted", () => Effect.gen(function* () { const session = yield* setup yield* admit(session, "Interrupt blocked tool") @@ -3346,7 +3339,7 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("interrupts a blocked provider turn without local tool execution", () => + it.effect("interrupts a blocked step without local tool execution", () => Effect.gen(function* () { const session = yield* setup yield* admit(session, "Interrupt provider") diff --git a/packages/core/test/tool-patch.test.ts b/packages/core/test/tool-patch.test.ts index 43b20d0cbf..2da29fd8d1 100644 --- a/packages/core/test/tool-patch.test.ts +++ b/packages/core/test/tool-patch.test.ts @@ -135,9 +135,6 @@ const call = (patchText: string, id = "call-patch") => ({ call: { type: "tool-call" as const, id, name: "patch", input: { patchText } }, }) -// patch is only materialized for OpenAI/GPT models. -const model = { id: "gpt-5", provider: "openai" } - const exists = (target: string) => Effect.promise(() => fs.stat(target).then( @@ -161,15 +158,12 @@ describe("PatchTool", () => { Effect.andThen( withTool(tmp.path, (registry) => Effect.gen(function* () { - expect((yield* toolDefinitions(registry, undefined, model)).map((tool) => tool.name)).toEqual([ - "patch", - ]) + expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["patch"]) const settled = yield* settleTool( registry, call( "*** Begin Patch\n*** Add File: nested/new.txt\n+created\n*** Update File: update.txt\n@@\n-before\n+after\n*** Delete File: remove.txt\n*** End Patch", ), - model, ) expect(settled.result).toEqual({ type: "text", @@ -239,7 +233,6 @@ describe("PatchTool", () => { call( "*** Begin Patch\n*** Add File: created.txt\n+created\n*** Update File: old.txt\n*** Move to: moved.txt\n@@\n-before\n+after\n*** End Patch", ), - model, ), ).toEqual({ type: "error", value: "patch moves are not supported yet" }) expect(yield* exists(path.join(tmp.path, "created.txt"))).toBe(false) @@ -267,7 +260,6 @@ describe("PatchTool", () => { yield* executeTool( registry, call(`*** Begin Patch\n*** Update File: ${target}\n@@\n-before\n+after\n*** End Patch`), - model, ), ).toMatchObject({ type: "text" }) expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"]) @@ -304,7 +296,6 @@ describe("PatchTool", () => { call( `*** Begin Patch\n*** Update File: ${first}\n@@\n-before\n+after\n*** Update File: ${second}\n@@\n-before\n+after\n*** End Patch`, ), - model, ), ).toMatchObject({ type: "text" }) expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"]) @@ -336,7 +327,6 @@ describe("PatchTool", () => { call( "*** Begin Patch\n*** Add File: created.txt\n+created\n*** Update File: missing.txt\n@@\n-before\n+after\n*** End Patch", ), - model, ), ).toEqual({ type: "error", value: "Unable to apply patch at missing.txt" }) expect(yield* exists(path.join(tmp.path, "created.txt"))).toBe(false) @@ -361,7 +351,6 @@ describe("PatchTool", () => { yield* executeTool( registry, call("*** Begin Patch\n*** Add File: existing.txt\n+replacement\n*** End Patch"), - model, ), ).toEqual({ type: "error", value: "Unable to apply patch at existing.txt" }) expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("sentinel\n") @@ -387,7 +376,6 @@ describe("PatchTool", () => { yield* executeTool( registry, call("*** Begin Patch\n*** Add File: appeared.txt\n+replacement\n*** End Patch"), - model, ), ).toEqual({ type: "error", value: "Unable to apply patch at appeared.txt" }) expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("winner\n") @@ -415,7 +403,6 @@ describe("PatchTool", () => { yield* executeTool( registry, call("*** Begin Patch\n*** Delete File: first.txt\n*** Delete File: second.txt\n*** End Patch"), - model, ).pipe(Effect.exit), ), ).toBe(true) @@ -447,7 +434,6 @@ describe("PatchTool", () => { const run = yield* executeTool( registry, call("*** Begin Patch\n*** Delete File: first.txt\n*** Delete File: second.txt\n*** End Patch"), - model, ).pipe(Effect.forkChild) yield* Deferred.await(removeStarted!) const interrupt = yield* Fiber.interrupt(run).pipe(Effect.forkChild) diff --git a/packages/core/test/tool-question.test.ts b/packages/core/test/tool-question.test.ts index 63b6f3a912..ef6bfdc54a 100644 --- a/packages/core/test/tool-question.test.ts +++ b/packages/core/test/tool-question.test.ts @@ -79,7 +79,7 @@ const it = testEffect( ) describe("QuestionTool", () => { - it.effect("omits a denied built-in question and terminally settles a stale call", () => + it.effect("omits a catalog-denied question and enforces its leaf permission", () => Effect.gen(function* () { captured = undefined deny = true diff --git a/packages/core/test/tool-subagent.test.ts b/packages/core/test/tool-subagent.test.ts index 661ef5d411..56e3587162 100644 --- a/packages/core/test/tool-subagent.test.ts +++ b/packages/core/test/tool-subagent.test.ts @@ -27,7 +27,7 @@ import { ToolRegistry } from "@opencode-ai/core/tool/registry" import { ToolOutputStore } from "@opencode-ai/core/tool-output-store" import { tmpdir } from "./fixture/tmpdir" import { testEffect } from "./lib/effect" -import { executeTool, settleTool, testModel, toolIdentity, waitForTool } from "./lib/tool" +import { executeTool, settleTool, toolIdentity, waitForTool } from "./lib/tool" const childText = "child final response" const childModel = ModelV2.Ref.make({ id: ModelV2.ID.make("child"), providerID: ProviderV2.ID.make("test") }) @@ -146,9 +146,7 @@ describe("SubagentTool", () => { const locations = yield* LocationServiceMap.Service const registry = yield* ToolRegistry.Service.pipe(Effect.provide(locations.get(parent.location))) yield* waitForTool(registry, SubagentTool.name) - expect((yield* registry.materialize({ model: testModel })).definitions.map((tool) => tool.name)).toContain( - SubagentTool.name, - ) + expect((yield* registry.materialize()).definitions.map((tool) => tool.name)).toContain(SubagentTool.name) expect( yield* executeTool(registry, { sessionID: parent.id, diff --git a/specs/v2/session.md b/specs/v2/session.md index 511ece8cfc..37188ff95c 100644 --- a/specs/v2/session.md +++ b/specs/v2/session.md @@ -215,7 +215,7 @@ Event replay owner claims are separate from clustered Session execution ownershi ## Current Tool Registry Slice -Each Location-scoped `ToolRegistry` stores scoped tool registrations, materializes definitions, and owns lookup and settlement. Built-ins and plugins contribute through the same `Tools.Service.register(...)` path. Closing a contribution scope removes its definition and rebuilds the advertised catalog. Trusted tool executors capture and perform authorization; the registry applies catalog visibility filtering, decodes input, invokes the retained handler, validates output, and settles failures as typed tool-result errors. +The Session runner materializes Location tools for each model request and persists their settlements. Registration, authorization, execution, and failure semantics are canonical in [Tools](./tools.md). When a Session omits `agent`, both execution and permission evaluation use the default `build` agent. A caller must not observe `build` model behavior while permission checks silently evaluate an empty no-agent policy. diff --git a/specs/v2/tools.md b/specs/v2/tools.md index 6be14d2f7b..68343079f6 100644 --- a/specs/v2/tools.md +++ b/specs/v2/tools.md @@ -148,7 +148,7 @@ Invalid input never invokes the tool. Invalid output never produces a successful `toModelOutput` is pure and total. When omitted, the encoded output remains structured output; an encoded string is also projected as text. Projection does not receive invocation identity because presentation depends only on validated input and output. -Step materialization captures the effective registration identity for each advertised name without retaining its handler. Settlement rejects the call as stale if that registration was removed or replaced, including when closing an overlay reveals the previously effective registration. The current handler is captured only after this check; removing or replacing its registration afterward does not affect the running invocation. +Each model request captures the effective registered `Tool` value for every advertised name. Settlement executes those captured values; later registration changes affect later requests. ## Output Bounding @@ -165,7 +165,7 @@ Outcomes remain distinct: - `ToolFailure` is an expected model-visible failure. - Interruption cancels the invocation and is not a tool result. - Unexpected typed errors and defects follow the runner's operational failure policy. -- Unknown, invalid, and stale calls become explicit model-visible settlement errors without invoking a handler. +- Unknown and invalid calls become explicit model-visible settlement errors without invoking a handler. Leaf tools translate only errors they deliberately classify as recoverable. Broad cause-catching around an executor is invalid because it consumes interruption and defects. @@ -175,8 +175,7 @@ Leaf tools translate only errors they deliberately classify as recoverable. Broa - **Codec boundary:** execution observes decoded input; projection observes encoded output. - **Durable identity:** invocation-owned records use the exact Session, agent, assistant message, and call IDs supplied by the runner. - **Scoped registration:** closing a Scope removes exactly its registration and reveals any prior active overlay. -- **Captured execution:** registration changes cannot alter an invocation after effective lookup. -- **Stale rejection:** a call never executes a registration other than the one advertised for its step. +- **Captured execution:** a call executes the registered `Tool` value advertised in its model request. - **Storage encapsulation:** domain output does not change according to model-output bounding or retention policy. ## Follow-Up