From a0665d9786224a23daac52022bc7bea23c1cc7a3 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Thu, 10 Sep 2026 11:26:27 +0200 Subject: [PATCH 1/2] refactor: clarify tool input coercion --- .../devframe/src/__tests__/tool-input.test.ts | 28 ++++++++++ packages/devframe/src/client/webmcp.ts | 4 +- packages/devframe/src/internal/index.ts | 8 +-- .../src/node/__tests__/agent-args.test.ts | 29 ---------- packages/devframe/src/node/agent-args.ts | 53 ------------------- packages/devframe/src/node/host-agent.ts | 4 +- packages/devframe/src/tool-input.ts | 50 +++++++++++++++++ packages/hub/src/node/host-commands.ts | 4 +- .../tsnapi/devframe/internal.snapshot.d.ts | 3 ++ .../tsnapi/devframe/internal.snapshot.js | 1 + 10 files changed, 92 insertions(+), 92 deletions(-) create mode 100644 packages/devframe/src/__tests__/tool-input.test.ts delete mode 100644 packages/devframe/src/node/__tests__/agent-args.test.ts delete mode 100644 packages/devframe/src/node/agent-args.ts create mode 100644 packages/devframe/src/tool-input.ts diff --git a/packages/devframe/src/__tests__/tool-input.test.ts b/packages/devframe/src/__tests__/tool-input.test.ts new file mode 100644 index 000000000..0aa563b38 --- /dev/null +++ b/packages/devframe/src/__tests__/tool-input.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from 'vitest' +import { toolInputToCommandArgs, toolInputToRpcArgs } from '../tool-input' + +describe('tool input positional arguments', () => { + it('passes arrays through and maps argN keys using the declared count', () => { + expect(toolInputToRpcArgs([1, 2], 2)).toEqual([1, 2]) + expect(toolInputToRpcArgs({ arg0: 'a', arg1: 'b' }, 2)).toEqual(['a', 'b']) + }) + + it('collects contiguous argN keys without a declared count', () => { + expect(toolInputToRpcArgs({ arg0: 1, arg1: 2 })).toEqual([1, 2]) + }) + + it('treats null, undefined, and empty objects as zero-argument calls', () => { + expect(toolInputToRpcArgs(undefined)).toEqual([]) + expect(toolInputToRpcArgs(null, 1)).toEqual([]) + expect(toolInputToRpcArgs({})).toEqual([]) + }) + + it('preserves undeclared RPC input as one argument', () => { + const input = { name: 'devframe' } + expect(toolInputToRpcArgs(input)).toEqual([input]) + }) + + it('drops undeclared command input', () => { + expect(toolInputToCommandArgs({ name: 'devframe' })).toEqual([]) + }) +}) diff --git a/packages/devframe/src/client/webmcp.ts b/packages/devframe/src/client/webmcp.ts index eca351ab9..24f6f3129 100644 --- a/packages/devframe/src/client/webmcp.ts +++ b/packages/devframe/src/client/webmcp.ts @@ -4,7 +4,7 @@ import { toAgentToolName } from 'devframe/utils/agent-tool-name' // Pure, browser-safe projections shared with the node-side MCP adapter, so // the WebMCP surface cannot drift from the MCP one. import { argsToJsonSchema } from '../adapters/mcp/to-json-schema' -import { coerceAgentPositionalArgs } from '../node/agent-args' +import { toolInputToRpcArgs } from '../tool-input' /** * Result a WebMCP tool's `execute` resolves with; mirrors the MCP @@ -195,7 +195,7 @@ async function executeRpcTool( args: Record, ): Promise { try { - const positional = coerceAgentPositionalArgs(args, def.args as readonly unknown[] | undefined, 'wrap') + const positional = toolInputToRpcArgs(args, def.args?.length) const handler = await getRpcHandler(def, context) const result = await handler(...positional) return { content: [{ type: 'text', text: stringifyResult(result) }] } diff --git a/packages/devframe/src/internal/index.ts b/packages/devframe/src/internal/index.ts index 8ee71534e..40651280c 100644 --- a/packages/devframe/src/internal/index.ts +++ b/packages/devframe/src/internal/index.ts @@ -9,8 +9,8 @@ // session/auth wiring the instance shell's own binding uses. // - `DevframeAgentHost`: the agent host implementation the hub composes into // its own commands host. -// - `coerceAgentPositionalArgs`: positional-arg coercion the hub applies when -// invoking agent tools as commands. +// - `toolInputToCommandArgs`: positional-argument conversion the hub applies +// when invoking tool-backed commands. // - `registerDevframeInstance` / `listLiveDevframeInstances`: the instance // registry: a custom host advertises itself; a devtool (the inspect plugin's // Instances tab, the connector) enumerates what's running. @@ -40,8 +40,6 @@ export { loadAutoMcpAdapter, normalizeBasePath, resolveBasePath, resolveMcpConfig } from '../adapters/_shared' export type { ResolvedMcpConfig } from '../adapters/_shared' export { resolveClientAssets } from '../client-assets' -export { coerceAgentPositionalArgs } from '../node/agent-args' -export type { AgentArgsFallback } from '../node/agent-args' export { diagnostics } from '../node/diagnostics' export { DevframeAgentHost } from '../node/host-agent' export * from '../node/host-h3' @@ -64,3 +62,5 @@ export type { ContextRpcServer, CreateContextRpcServerOptions } from '../node/rp export { normalizeHttpServerUrl } from '../node/utils' export { createRpcWireCodec, peekRpcWireFrame } from '../rpc/wire-codec' export type { RpcWireCodec } from '../rpc/wire-codec' +export { coerceAgentPositionalArgs, toolInputToCommandArgs } from '../tool-input' +export type { AgentArgsFallback } from '../tool-input' diff --git a/packages/devframe/src/node/__tests__/agent-args.test.ts b/packages/devframe/src/node/__tests__/agent-args.test.ts deleted file mode 100644 index fdde8e5ae..000000000 --- a/packages/devframe/src/node/__tests__/agent-args.test.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { coerceAgentPositionalArgs } from '../agent-args' - -describe('coerceAgentPositionalArgs', () => { - const schema = {} as unknown - - it('passes arrays through and maps argN keys onto declared schemas', () => { - expect(coerceAgentPositionalArgs([1, 2], [schema, schema])).toEqual([1, 2]) - expect(coerceAgentPositionalArgs({ arg0: 'a', arg1: 'b' }, [schema, schema])).toEqual(['a', 'b']) - }) - - it('collects argN keys even without schemas', () => { - expect(coerceAgentPositionalArgs({ arg0: 1, arg1: 2 }, undefined)).toEqual([1, 2]) - }) - - it('treats null/undefined and empty objects as zero-argument calls', () => { - expect(coerceAgentPositionalArgs(undefined, undefined)).toEqual([]) - expect(coerceAgentPositionalArgs(null, [schema])).toEqual([]) - expect(coerceAgentPositionalArgs({}, undefined)).toEqual([]) - }) - - it('follows the fallback for undeclared object payload', () => { - const payload = { name: 'devframe' } - // RPC-backed tools: an untyped RPC may take one raw object. - expect(coerceAgentPositionalArgs(payload, undefined, 'wrap')).toEqual([payload]) - // Command-backed tools: positional params come solely from declared schemas. - expect(coerceAgentPositionalArgs(payload, undefined, 'drop')).toEqual([]) - }) -}) diff --git a/packages/devframe/src/node/agent-args.ts b/packages/devframe/src/node/agent-args.ts deleted file mode 100644 index 7c913eaa8..000000000 --- a/packages/devframe/src/node/agent-args.ts +++ /dev/null @@ -1,53 +0,0 @@ -/** - * How {@link coerceAgentPositionalArgs} treats an args object that carries - * neither declared schemas nor `arg0`/`arg1`/… keys: - * - * - `'wrap'`: pass the object itself as the single positional argument. - * RPC-backed tools use this: an untyped RPC may take one raw object. - * - `'drop'`: call with zero arguments. Command-backed tools use this: - * a handler's positional parameters come solely from its declared - * `agent.args` schemas, so undeclared payload is ignored. - */ -export type AgentArgsFallback = 'wrap' | 'drop' - -/** - * Map the args payload an agent surface receives (MCP sends an object - * keyed `arg0`/`arg1`/…, matching the schema the adapter advertises) onto - * a handler's positional parameters. Shared by the agent host's RPC - * bridge and the hub's command-derived tools so the coercion cannot - * drift between them. - * - * - an array passes through as-is - * - `null`/`undefined` become a zero-argument call - * - with declared schemas, each schema reads its own `argN` key, in order - * - without schemas, `arg0`/`arg1`/… keys are collected when present - * - an empty object becomes a zero-argument call - * - anything else follows the {@link AgentArgsFallback} - */ -export function coerceAgentPositionalArgs( - args: unknown, - schemas: readonly unknown[] | undefined, - fallback: AgentArgsFallback = 'wrap', -): unknown[] { - if (Array.isArray(args)) - return args - if (args === undefined || args === null) - return [] - if (typeof args === 'object') { - const obj = args as Record - if (schemas && schemas.length) - return schemas.map((_, i) => obj[`arg${i}`]) - if ('arg0' in obj) { - const out: unknown[] = [] - let i = 0 - while (`arg${i}` in obj) { - out.push(obj[`arg${i}`]) - i++ - } - return out - } - if (Object.keys(obj).length === 0) - return [] - } - return fallback === 'drop' ? [] : [args] -} diff --git a/packages/devframe/src/node/host-agent.ts b/packages/devframe/src/node/host-agent.ts index 04236fb85..0c5c65d9d 100644 --- a/packages/devframe/src/node/host-agent.ts +++ b/packages/devframe/src/node/host-agent.ts @@ -17,7 +17,7 @@ import type { } from 'devframe/types' import { createEventEmitter } from 'devframe/utils/events' import { DEVFRAME_EVENTS } from '../events' -import { coerceAgentPositionalArgs } from './agent-args' +import { toolInputToRpcArgs } from '../tool-input' import { diagnostics } from './diagnostics' interface RegisteredTool { @@ -184,7 +184,7 @@ export class DevframeAgentHost implements DevframeAgentHostType { // (what the MCP adapter sends after flattening), or a plain array. // An untyped RPC may take a single raw object, so undeclared object // payload wraps into one positional argument. - const positional = coerceAgentPositionalArgs(args, rpcDef.args as readonly unknown[] | undefined, 'wrap') + const positional = toolInputToRpcArgs(args, rpcDef.args?.length) return await this.context.rpc.invokeLocal(id as any, ...(positional as any)) } diff --git a/packages/devframe/src/tool-input.ts b/packages/devframe/src/tool-input.ts new file mode 100644 index 000000000..5512f2d13 --- /dev/null +++ b/packages/devframe/src/tool-input.ts @@ -0,0 +1,50 @@ +/** + * Convert an object-shaped tool input into positional arguments. + * + * Tool schemas expose positional parameters as `arg0`, `arg1`, and so on. + * Arrays pass through for callers that already provide positional arguments. + */ +function collectPositionalArgs(input: unknown, argumentCount: number | undefined): unknown[] | undefined { + if (Array.isArray(input)) + return input + if (input === undefined || input === null) + return [] + if (typeof input !== 'object') + return undefined + + const record = input as Record + if (argumentCount) + return Array.from({ length: argumentCount }, (_, index) => record[`arg${index}`]) + if ('arg0' in record) { + const positional: unknown[] = [] + while (`arg${positional.length}` in record) + positional.push(record[`arg${positional.length}`]) + return positional + } + return Object.keys(record).length === 0 ? [] : undefined +} + +/** Convert tool input for an RPC, preserving an untyped payload as arg 0. */ +export function toolInputToRpcArgs(input: unknown, argumentCount?: number): unknown[] { + return collectPositionalArgs(input, argumentCount) ?? [input] +} + +/** Convert tool input for a command, whose arguments must be declared. */ +export function toolInputToCommandArgs(input: unknown, argumentCount?: number): unknown[] { + return collectPositionalArgs(input, argumentCount) ?? [] +} + +/** @deprecated Use {@link toolInputToRpcArgs} or {@link toolInputToCommandArgs}. */ +export type AgentArgsFallback = 'wrap' | 'drop' + +/** @deprecated Use {@link toolInputToRpcArgs} or {@link toolInputToCommandArgs}. */ +export function coerceAgentPositionalArgs( + input: unknown, + schemas: readonly unknown[] | undefined, + fallback: AgentArgsFallback = 'wrap', +): unknown[] { + const argumentCount = schemas?.length + return fallback === 'drop' + ? toolInputToCommandArgs(input, argumentCount) + : toolInputToRpcArgs(input, argumentCount) +} diff --git a/packages/hub/src/node/host-commands.ts b/packages/hub/src/node/host-commands.ts index a5872a482..45b957639 100644 --- a/packages/hub/src/node/host-commands.ts +++ b/packages/hub/src/node/host-commands.ts @@ -6,7 +6,7 @@ import type { DevframeServerCommandInput, } from '../types/commands' import type { DevframeHubContext } from './context' -import { coerceAgentPositionalArgs } from 'devframe/internal' +import { toolInputToCommandArgs } from 'devframe/internal' import { createEventEmitter } from 'devframe/utils/events' import { HUB_EVENTS } from '../events' import { diagnostics } from './diagnostics' @@ -193,7 +193,7 @@ export class DevframeCommandsHost implements DevframeCommandsHostType { * declared `agent.args` schemas; undeclared payload is dropped. */ handler: async (args: unknown) => - this.execute(command.id, ...coerceAgentPositionalArgs(args, agent.args, 'drop')), + this.execute(command.id, ...toolInputToCommandArgs(args, agent.args?.length)), }) } for (const child of command.children ?? []) diff --git a/tests/__snapshots__/tsnapi/devframe/internal.snapshot.d.ts b/tests/__snapshots__/tsnapi/devframe/internal.snapshot.d.ts index 9c08b4699..4e01745e3 100644 --- a/tests/__snapshots__/tsnapi/devframe/internal.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/devframe/internal.snapshot.d.ts @@ -15,6 +15,7 @@ export interface RpcWireCodec { // #endregion // #region Types +/** @deprecated */ export type AgentArgsFallback = 'wrap' | 'drop'; // #endregion @@ -48,6 +49,7 @@ export declare class DevframeAgentHost implements DevframeAgentHost$1 { // #endregion // #region Functions +/** @deprecated */ export declare function coerceAgentPositionalArgs(_: unknown, _: readonly unknown[] | undefined, _?: AgentArgsFallback): unknown[]; export declare function createH3DevframeHost(_: CreateH3DevframeHostOptions): DevframeHost; export declare function createRpcWireCodec(_?: ReadonlyMap>): RpcWireCodec; @@ -58,6 +60,7 @@ export declare function peekRpcWireFrame(_: string): { i?: string; }; export declare function resolveClientAssets(_: DevframeDefinition): StaticAssetsSource | undefined; +export declare function toolInputToCommandArgs(_: unknown, _?: number): unknown[]; // #endregion // #region Variables diff --git a/tests/__snapshots__/tsnapi/devframe/internal.snapshot.js b/tests/__snapshots__/tsnapi/devframe/internal.snapshot.js index 2ca8861ea..1dc5bcb58 100644 --- a/tests/__snapshots__/tsnapi/devframe/internal.snapshot.js +++ b/tests/__snapshots__/tsnapi/devframe/internal.snapshot.js @@ -21,4 +21,5 @@ export { resolveClientAssets } export { resolveInstanceRegister } export { resolveMcpConfig } export { samePath } +export { toolInputToCommandArgs } // #endregion \ No newline at end of file From d3993cec1a7cbb347d30aed1b59d5e800453eaaa Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Thu, 10 Sep 2026 11:35:00 +0200 Subject: [PATCH 2/2] fix: honor explicit zero argument count --- packages/devframe/src/__tests__/tool-input.test.ts | 1 + packages/devframe/src/tool-input.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/devframe/src/__tests__/tool-input.test.ts b/packages/devframe/src/__tests__/tool-input.test.ts index 0aa563b38..90961ad22 100644 --- a/packages/devframe/src/__tests__/tool-input.test.ts +++ b/packages/devframe/src/__tests__/tool-input.test.ts @@ -5,6 +5,7 @@ describe('tool input positional arguments', () => { it('passes arrays through and maps argN keys using the declared count', () => { expect(toolInputToRpcArgs([1, 2], 2)).toEqual([1, 2]) expect(toolInputToRpcArgs({ arg0: 'a', arg1: 'b' }, 2)).toEqual(['a', 'b']) + expect(toolInputToRpcArgs({ arg0: 'a' }, 0)).toEqual([]) }) it('collects contiguous argN keys without a declared count', () => { diff --git a/packages/devframe/src/tool-input.ts b/packages/devframe/src/tool-input.ts index 5512f2d13..a15f5bd48 100644 --- a/packages/devframe/src/tool-input.ts +++ b/packages/devframe/src/tool-input.ts @@ -13,7 +13,7 @@ function collectPositionalArgs(input: unknown, argumentCount: number | undefined return undefined const record = input as Record - if (argumentCount) + if (argumentCount != null) return Array.from({ length: argumentCount }, (_, index) => record[`arg${index}`]) if ('arg0' in record) { const positional: unknown[] = []