Repository navigation
refactor(runtime): extract tool-call argument projections #4937
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b1b4684
cab8050
ed86051
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,232 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| /** | ||
| * The call-data boundary for one tool execution: the argument views a call | ||
| * produces. | ||
| * | ||
| * Extracted from `executeTool()` so the ownership of each rule is named | ||
| * instead of living inline in a thousand-line method. This module owns | ||
| * *construction only* — admission, dispatch identity, publication timing and | ||
| * persistence stay in `executeTool()`. | ||
| */ | ||
|
|
||
| import { computerUseModelCallArgs } from '@maka/core/computer-use'; | ||
|
|
||
| /** The recursively frozen, cycle-rejecting argument snapshot. */ | ||
| export function snapshotToolArgs(value: unknown): unknown { | ||
| return snapshotJsonValue(value, new WeakSet<object>()); | ||
| } | ||
|
|
||
| function snapshotJsonValue(value: unknown, seen: WeakSet<object>): unknown { | ||
| if (value === null || typeof value !== 'object') return value; | ||
| if (seen.has(value)) throw new Error('Tool arguments must not contain cycles'); | ||
| seen.add(value); | ||
| if (Array.isArray(value)) { | ||
| return Object.freeze(value.map((entry) => snapshotJsonValue(entry, seen))); | ||
| } | ||
| const output: Record<string, unknown> = {}; | ||
| for (const key of Object.keys(value)) { | ||
| const descriptor = Object.getOwnPropertyDescriptor(value, key); | ||
| if (!descriptor || !('value' in descriptor)) { | ||
| throw new Error(`Tool argument ${key} must be a plain data property`); | ||
| } | ||
| output[key] = snapshotJsonValue(descriptor.value, seen); | ||
| } | ||
| return Object.freeze(output); | ||
| } | ||
|
|
||
| /** | ||
| * Validates call arguments against the tool's declared schema, accepting the | ||
| * schema shapes the runtime already supports (zod `safeParseAsync`/`safeParse`, | ||
| * a `validate` callable, or a standard-schema `~standard.validate`). A tool | ||
| * without a usable schema declares nothing and validates nothing. | ||
| */ | ||
| export async function validateDeclaredToolArgs(parameters: unknown, args: unknown): Promise<void> { | ||
| if (!parameters || (typeof parameters !== 'object' && typeof parameters !== 'function')) { | ||
| return; | ||
| } | ||
| const schema = parameters as { | ||
| safeParseAsync?: ( | ||
| value: unknown, | ||
| ) => PromiseLike<{ success: true; data: unknown } | { success: false; error: unknown }>; | ||
| safeParse?: ( | ||
| value: unknown, | ||
| ) => { success: true; data: unknown } | { success: false; error: unknown }; | ||
| validate?: ( | ||
| value: unknown, | ||
| ) => | ||
| | { success: true; value: unknown } | ||
| | { success: false; error: unknown } | ||
| | PromiseLike<{ success: true; value: unknown } | { success: false; error: unknown }>; | ||
| '~standard'?: { | ||
| validate?: ( | ||
| value: unknown, | ||
| ) => | ||
| | { value: unknown } | ||
| | { issues: readonly unknown[] } | ||
| | PromiseLike<{ value: unknown } | { issues: readonly unknown[] }>; | ||
| }; | ||
| }; | ||
|
|
||
| if (typeof schema.safeParseAsync === 'function') { | ||
| const parsed = await schema.safeParseAsync(args); | ||
| if (parsed.success) return; | ||
| throw parsed.error; | ||
| } | ||
| if (typeof schema.safeParse === 'function') { | ||
| const parsed = schema.safeParse(args); | ||
| if (parsed.success) return; | ||
| throw parsed.error; | ||
| } | ||
| if (typeof schema.validate === 'function') { | ||
| const parsed = await schema.validate(args); | ||
| if (parsed.success) return; | ||
| throw parsed.error; | ||
| } | ||
| if (typeof schema['~standard']?.validate === 'function') { | ||
| const parsed = await schema['~standard'].validate(args); | ||
| if ('value' in parsed) return; | ||
| throw new Error('Tool arguments failed declared schema validation', { cause: parsed.issues }); | ||
| } | ||
| } | ||
|
|
||
| /** Permission-projection context handed to a tool's `permissionArgs` hook. */ | ||
| export interface ToolPermissionArgsContext { | ||
| sessionId: string; | ||
| turnId: string; | ||
| toolCallId: string; | ||
| } | ||
|
|
||
| /** Input to {@link buildToolCallArgs}. */ | ||
| export interface ToolCallArgsInput { | ||
| toolName: string; | ||
| /** Declared argument schema; validation runs against the execution snapshot. */ | ||
| parameters: unknown; | ||
| /** Category hint, selecting the Computer Use persisted/model projection. */ | ||
| categoryHint?: string | undefined; | ||
| /** | ||
| * The tool's permission-oriented projection. When present, it receives a | ||
| * private mutable clone of the execution args and its result is | ||
| * re-snapshotted — exactly as the inline region did. | ||
| */ | ||
| permissionArgs?: ((args: never, context: ToolPermissionArgsContext) => unknown) | undefined; | ||
| /** The canonical execution input, snapshotted synchronously at entry. */ | ||
| executionArgs: unknown; | ||
| sessionId: string; | ||
| turnId: string; | ||
| toolCallId: string; | ||
| /** | ||
| * Direct-only nested rejection skips validation and permission projection | ||
| * entirely — the call never reaches the tool. | ||
| */ | ||
| directOnlyRejected: boolean; | ||
| /** | ||
| * An unavailable sandbox-boundary surface retains its current validation | ||
| * exception, so validation is deferred while every other rule still applies. | ||
| */ | ||
| validationDeferred: boolean; | ||
| } | ||
|
|
||
| /** | ||
| * The three argument views of one tool call. Each has one owner and one job: | ||
| * | ||
| * - `executionArgs` — canonical execution input. Consumers receive private | ||
| * mutable clones; the view itself stays frozen. | ||
| * - `permissionArgs` — the tool's permission-oriented projection, also read | ||
| * by downstream policy/signature logic. | ||
| * - `persistedArgs` — what the `tool_start` event and the durable call data | ||
| * record. Persistence and the model replay share this one canonical | ||
| * tool-dialect projection: the transcript message and the `function_call` | ||
| * record the model reads back are derived from the `tool_start` event, so | ||
| * there is no separate model-facing record to keep in sync. | ||
| */ | ||
| export interface ToolCallArgs { | ||
| readonly executionArgs: unknown; | ||
| readonly permissionArgs: unknown; | ||
| readonly persistedArgs: unknown; | ||
| } | ||
|
|
||
| export interface ToolCallArgsAndProjectionError extends ToolCallArgs { | ||
| /** | ||
| * First validation or permission-projection failure. The caller routes it | ||
| * to the existing refusal path; the builder never throws for it. | ||
| */ | ||
| readonly permissionArgsError: unknown; | ||
| } | ||
|
|
||
| /** | ||
| * The named argument-view construction operation: validation, permission | ||
| * projection, and the persisted view, in the order and with the guards the | ||
| * inline region in `executeTool()` established. | ||
| * | ||
| * The args written into the `tool_start` event and the durable ledger are the | ||
| * record of the call the model reads back on its next turn | ||
| * (`model-history.ts` replays `event.content.args`). | ||
| * | ||
| * Computer Use used the host's approval summary there. That projection exists | ||
| * to decide and display a permission: it renames `window_id` to `windowId`, | ||
| * adds `approvalClass` and `rememberForTurnAllowed`, and drops every argument | ||
| * it does not need. On the real ToolRuntime a model that sent | ||
| * {action:'press_key', app, window_id, observation_id, element_id, | ||
| * text:'cmd+s'} read back {action, approvalClass, rememberForTurnAllowed, | ||
| * app, windowId, observationId} — a key the tool rejects, two fields it never | ||
| * sent, no element, and a press_key with no key. It then went on calling it | ||
| * that way. | ||
| * | ||
| * The permission prompt still reads the permission view, and the approval | ||
| * scope key is still computed from the raw call, so the projection only | ||
| * changes what is written down. `computerUseModelCallArgs` keeps the same | ||
| * privacy rule — screen-derived and user-typed values are reduced to a shape | ||
| * — and speaks the tool's own argument names. | ||
| */ | ||
| export async function buildToolCallArgs( | ||
| input: ToolCallArgsInput, | ||
| ): Promise<ToolCallArgsAndProjectionError> { | ||
| let permissionArgs = input.executionArgs; | ||
| let permissionArgsError: unknown; | ||
| if (!input.directOnlyRejected) { | ||
| try { | ||
| if (!input.validationDeferred) { | ||
| await validateDeclaredToolArgs(input.parameters, input.executionArgs); | ||
| } | ||
| permissionArgs = input.permissionArgs | ||
| ? snapshotToolArgs( | ||
| input.permissionArgs(structuredClone(input.executionArgs) as never, { | ||
| sessionId: input.sessionId, | ||
| turnId: input.turnId, | ||
| toolCallId: input.toolCallId, | ||
| }), | ||
| ) | ||
| : input.executionArgs; | ||
| } catch (error) { | ||
| permissionArgsError = error; | ||
| } | ||
| } | ||
| const persistedArgs = | ||
| input.categoryHint === 'computer_use' | ||
| ? snapshotToolArgs(computerUseModelCallArgs(permissionArgs)) | ||
| : permissionArgs; | ||
| return { | ||
| executionArgs: input.executionArgs, | ||
| permissionArgs, | ||
| permissionArgsError, | ||
| persistedArgs, | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,7 +71,7 @@ import type { | |
| UserQuestionResponse, | ||
| UserQuestionResult, | ||
| } from '@maka/core/user-question'; | ||
| import { computerUseModelCallArgs } from '@maka/core/computer-use'; | ||
| import { buildToolCallArgs, snapshotToolArgs } from './tool-call-snapshot.js'; | ||
| import type { SessionHeader } from '@maka/core/session'; | ||
| import type { ToolInvocationRecord } from '@maka/core/usage-stats/types'; | ||
| import { redactSecrets } from '@maka/core/redaction'; | ||
|
|
@@ -1089,39 +1089,31 @@ export class ToolRuntime { | |
| ? `Tool ${tool.name} is direct-only and cannot run inside exec.` | ||
| : undefined; | ||
| const admissionFailure = directOnlyFailure ?? this.admitToolForStep(tool, stepId); | ||
| let permissionArgs = executionArgs; | ||
| let permissionArgsError: unknown; | ||
| if (directOnlyFailure === undefined) { | ||
| try { | ||
| // An unavailable sandbox-boundary surface rejects the expansion before | ||
| // interpreting it, including legacy shapes whose validation stays deferred. | ||
| const sandboxBoundaryUnavailable = | ||
| tool.name === 'request_sandbox_boundary' && | ||
| !this.interactionRun() && | ||
| (!this.input.createSandboxBoundaryRequest || !this.input.settleSandboxBoundaryRequest); | ||
| if (!sandboxBoundaryUnavailable) { | ||
| await validateDeclaredToolArgs(tool.parameters, executionArgs); | ||
| } | ||
| permissionArgs = tool.permissionArgs | ||
| ? snapshotToolArgs( | ||
| tool.permissionArgs(structuredClone(executionArgs) as never, { | ||
| sessionId: this.input.sessionId, | ||
| turnId, | ||
| toolCallId: toolUseId, | ||
| }), | ||
| ) | ||
| : executionArgs; | ||
| } catch (error) { | ||
| permissionArgsError = error; | ||
| } | ||
| } | ||
| // Permission args are a policy/UI projection. Persistence and model replay | ||
| // share one canonical tool-dialect projection; Computer Use also strips | ||
| // screen-derived and user-typed values at this boundary. | ||
| const persistedArgs = | ||
| tool.categoryHint === 'computer_use' | ||
| ? snapshotToolArgs(computerUseModelCallArgs(permissionArgs)) | ||
| : permissionArgs; | ||
| // An unavailable sandbox-boundary surface cannot carry the expansion the | ||
| // tool requests, and that availability contract holds even when an older | ||
| // caller sends a legacy shape — validation stays deferred for it. | ||
| const sandboxBoundaryUnavailable = | ||
| tool.name === 'request_sandbox_boundary' && | ||
| !this.interactionRun() && | ||
| (!this.input.createSandboxBoundaryRequest || !this.input.settleSandboxBoundaryRequest); | ||
| const callArgs = await buildToolCallArgs({ | ||
| toolName: tool.name, | ||
| parameters: tool.parameters, | ||
| categoryHint: tool.categoryHint, | ||
| permissionArgs: tool.permissionArgs, | ||
| // Upstream renamed rawExecutionArgs to executionArgs after this | ||
| // refactor; buildToolCallArgs passes it through unchanged, and the | ||
| // entry snapshot above already holds that same value. | ||
|
Comment on lines
+1104
to
+1106
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| executionArgs, | ||
| sessionId: this.input.sessionId, | ||
| turnId, | ||
| toolCallId: toolUseId, | ||
| directOnlyRejected: directOnlyFailure !== undefined, | ||
| validationDeferred: sandboxBoundaryUnavailable, | ||
| }); | ||
| const permissionArgs = callArgs.permissionArgs; | ||
| const persistedArgs = callArgs.persistedArgs; | ||
| const permissionArgsError = callArgs.permissionArgsError; | ||
| const now = this.input.now(); | ||
| const trace = this.input.getRunTrace?.() ?? null; | ||
| const runId = this.input.runId; | ||
|
|
@@ -2915,55 +2907,6 @@ export class ToolRuntime { | |
| } | ||
| } | ||
|
|
||
| async function validateDeclaredToolArgs(parameters: unknown, args: unknown): Promise<void> { | ||
| if (!parameters || (typeof parameters !== 'object' && typeof parameters !== 'function')) { | ||
| return; | ||
| } | ||
| const schema = parameters as { | ||
| safeParseAsync?: ( | ||
| value: unknown, | ||
| ) => PromiseLike<{ success: true; data: unknown } | { success: false; error: unknown }>; | ||
| safeParse?: ( | ||
| value: unknown, | ||
| ) => { success: true; data: unknown } | { success: false; error: unknown }; | ||
| validate?: ( | ||
| value: unknown, | ||
| ) => | ||
| | { success: true; value: unknown } | ||
| | { success: false; error: unknown } | ||
| | PromiseLike<{ success: true; value: unknown } | { success: false; error: unknown }>; | ||
| '~standard'?: { | ||
| validate?: ( | ||
| value: unknown, | ||
| ) => | ||
| | { value: unknown } | ||
| | { issues: readonly unknown[] } | ||
| | PromiseLike<{ value: unknown } | { issues: readonly unknown[] }>; | ||
| }; | ||
| }; | ||
|
|
||
| if (typeof schema.safeParseAsync === 'function') { | ||
| const parsed = await schema.safeParseAsync(args); | ||
| if (parsed.success) return; | ||
| throw parsed.error; | ||
| } | ||
| if (typeof schema.safeParse === 'function') { | ||
| const parsed = schema.safeParse(args); | ||
| if (parsed.success) return; | ||
| throw parsed.error; | ||
| } | ||
| if (typeof schema.validate === 'function') { | ||
| const parsed = await schema.validate(args); | ||
| if (parsed.success) return; | ||
| throw parsed.error; | ||
| } | ||
| if (typeof schema['~standard']?.validate === 'function') { | ||
| const parsed = await schema['~standard'].validate(args); | ||
| if ('value' in parsed) return; | ||
| throw new Error('Tool arguments failed declared schema validation', { cause: parsed.issues }); | ||
| } | ||
| } | ||
|
|
||
| function isInteractionControlError(error: unknown): boolean { | ||
| return ( | ||
| error instanceof RuntimeInteractionAdmissionRejectedError || | ||
|
|
@@ -3542,25 +3485,3 @@ function byteLength(value: unknown): number { | |
| const text = typeof value === 'string' ? value : JSON.stringify(value ?? null); | ||
| return Buffer.byteLength(text, 'utf8'); | ||
| } | ||
|
|
||
| function snapshotToolArgs(value: unknown): unknown { | ||
| return snapshotJsonValue(value, new WeakSet<object>()); | ||
| } | ||
|
|
||
| function snapshotJsonValue(value: unknown, seen: WeakSet<object>): unknown { | ||
| if (value === null || typeof value !== 'object') return value; | ||
| if (seen.has(value)) throw new Error('Tool arguments must not contain cycles'); | ||
| seen.add(value); | ||
| if (Array.isArray(value)) { | ||
| return Object.freeze(value.map((entry) => snapshotJsonValue(entry, seen))); | ||
| } | ||
| const output: Record<string, unknown> = {}; | ||
| for (const key of Object.keys(value)) { | ||
| const descriptor = Object.getOwnPropertyDescriptor(value, key); | ||
| if (!descriptor || !('value' in descriptor)) { | ||
| throw new Error(`Tool argument ${key} must be a plain data property`); | ||
| } | ||
| output[key] = snapshotJsonValue(descriptor.value, seen); | ||
| } | ||
| return Object.freeze(output); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toolNameis declared here and passed by the one caller (tool-runtime.ts:1100), butbuildToolCallArgsnever reads it — grep the module: the name appears only on this line. The same is true of the returnedexecutionArgs: the caller destructures three fields and not that one (tool-runtime.ts:1114-1116), andvalidateDeclaredToolArgsis exported at line 61 while its only consumer is this module itself (line 207).None of these change behaviour, and I am not asking for a new consumer. The reason to fix them is that this module's stated purpose is to name a boundary and say who owns what, so a field that nothing reads is the one thing it cannot afford to carry — a reader has to decide whether
toolNameis part of the contract or was left over. Either drop them, or say in the docstring why they are there.