diff --git a/apps/website/content/docs/chat/api/api-docs.json b/apps/website/content/docs/chat/api/api-docs.json index 7c02a0cd0..71667e296 100644 --- a/apps/website/content/docs/chat/api/api-docs.json +++ b/apps/website/content/docs/chat/api/api-docs.json @@ -6309,6 +6309,12 @@ "description": "", "optional": false }, + { + "name": "followUp", + "type": "boolean", + "description": "", + "optional": true + }, { "name": "handler", "type": "(args: any, context: FunctionToolHandlerContext) => unknown", @@ -6353,6 +6359,12 @@ "description": "", "optional": false }, + { + "name": "followUp", + "type": "boolean", + "description": "", + "optional": true + }, { "name": "kind", "type": "\"ask\"", @@ -6700,6 +6712,20 @@ ], "examples": [] }, + { + "name": "ClientToolContinuationOptions", + "kind": "interface", + "description": "Execution policy options for browser-executed function tools.", + "properties": [ + { + "name": "followUp", + "type": "boolean", + "description": "False when the tool result should be recorded without forcing a continuation.", + "optional": true + } + ], + "examples": [] + }, { "name": "ClientToolExecutionGuard", "kind": "interface", @@ -6745,6 +6771,12 @@ "kind": "interface", "description": "Execution policy options for browser-executed function tools.", "properties": [ + { + "name": "followUp", + "type": "boolean", + "description": "False when the tool result should be recorded without forcing a continuation.", + "optional": true + }, { "name": "idempotent", "type": "boolean", @@ -6824,6 +6856,12 @@ "type": "ClientToolExecutionGuard", "description": "", "optional": true + }, + { + "name": "settleToolCall", + "type": "(toolCall: ToolCall, result: ClientToolResult) => void", + "description": "", + "optional": true } ], "examples": [] @@ -6872,6 +6910,25 @@ "optional": false } ] + }, + { + "name": "settle", + "signature": "settle(toolCallId: string, result: ClientToolResult): void", + "description": "Record a client tool's result without continuing the run.", + "params": [ + { + "name": "toolCallId", + "type": "string", + "description": "", + "optional": false + }, + { + "name": "result", + "type": "ClientToolResult", + "description": "", + "optional": false + } + ] } ], "examples": [] @@ -7114,6 +7171,12 @@ "description": "", "optional": false }, + { + "name": "followUp", + "type": "boolean", + "description": "", + "optional": true + }, { "name": "handler", "type": "(args: StandardSchemaInferOutput, context: FunctionToolHandlerContext) => R | Promise", @@ -8005,6 +8068,12 @@ "description": "", "optional": false }, + { + "name": "followUp", + "type": "boolean", + "description": "", + "optional": true + }, { "name": "kind", "type": "\"view\"", @@ -8350,7 +8419,7 @@ "name": "ask", "kind": "function", "description": "Interactive (human-in-the-loop) component tool — the model fills the\ncomponent's props from the schema's output; the value the component emits\nback to the framework becomes the tool result sent to the model.\n\nThe component's signal inputs are checked against the schema output type\n(strict-but-flexible: every schema key must be a declared input with an\nassignable type; the component may declare extra inputs the schema doesn't\nfill). Author the component with `ViewProps` to derive input\nprop types directly from the schema.", - "signature": "ask(description: string, schema: S, component: AcceptComponent): AskToolDef", + "signature": "ask(description: string, schema: S, component: AcceptComponent, options: ClientToolContinuationOptions): AskToolDef", "params": [ { "name": "description", @@ -8369,6 +8438,12 @@ "type": "AcceptComponent", "description": "Angular component whose signal inputs must be compatible with\n the schema output. A type-level error is reported here when they diverge.", "optional": false + }, + { + "name": "options", + "type": "ClientToolContinuationOptions", + "description": "", + "optional": true } ], "returns": { @@ -9374,7 +9449,7 @@ "name": "view", "kind": "function", "description": "Render-only component tool — the model fills the component's props from the\nschema's output; the tool call is auto-acknowledged once the component mounts.\n\nThe component's signal inputs are checked against the schema output type\n(strict-but-flexible: every schema key must be a declared input with an\nassignable type; the component may declare extra inputs the schema doesn't fill).\nAuthor the component with `ViewProps` as the input type set to\nguarantee the shapes stay aligned.", - "signature": "view(description: string, schema: S, component: AcceptComponent): ViewToolDef", + "signature": "view(description: string, schema: S, component: AcceptComponent, options: ClientToolContinuationOptions): ViewToolDef", "params": [ { "name": "description", @@ -9393,6 +9468,12 @@ "type": "AcceptComponent", "description": "Angular component whose signal inputs must be compatible with\n the schema output. A type-level error is reported here when they diverge.", "optional": false + }, + { + "name": "options", + "type": "ClientToolContinuationOptions", + "description": "", + "optional": true } ], "returns": { diff --git a/docs/superpowers/plans/2026-07-08-client-tools-m4-settle-batching-plan.md b/docs/superpowers/plans/2026-07-08-client-tools-m4-settle-batching-plan.md new file mode 100644 index 000000000..6bc531606 --- /dev/null +++ b/docs/superpowers/plans/2026-07-08-client-tools-m4-settle-batching-plan.md @@ -0,0 +1,307 @@ +# Client Tools M4 Settle Batching Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add the optional `settle()` client-tools capability and change coordinator-driven client-tool continuation from one run per resolved tool to one run per assistant-turn tool-call group. + +**Architecture:** Adapters own transport-specific buffering: `settle(id, result)` records the local tool result and appends a buffered ToolMessage without issuing a run; `resolve(id, result)` delegates to `settle` and flushes all buffered ToolMessages in one continuation run. Chat owns orchestration: function/view/ask tools report settled results to a group tracker, and the tracker flushes once every non-terminal call in the current pending group has settled. `followUp:false` tools settle without forcing a continuation; a fully-terminal group issues no run. + +**Tech Stack:** Angular signals/effects, Vitest, Nx, existing `@threadplane/chat`, `@threadplane/ag-ui`, and `@threadplane/langgraph` packages. No new dependencies. + +--- + +## File Structure + +- Modify: `libs/chat/src/lib/client-tools/client-tools-capability.ts` + - Add optional `settle(toolCallId, result)` to the capability interface. +- Modify: `libs/chat/src/lib/client-tools/tool-def.ts` + - Extend client-tool authoring options with `followUp?: boolean`. + - Carry `followUp` on function, view, and ask definitions. +- Modify: `libs/chat/src/lib/client-tools/tools.ts` + - Accept `followUp` options for `action`, `view`, and `ask`. +- Modify: `libs/chat/src/lib/client-tools/client-tool-executor.ts` + - Add a settlement callback option so function-tool execution no longer directly chooses `resolve` for every call. +- Modify: `libs/chat/src/lib/client-tools/client-tools-coordinator.ts` + - Add assistant-turn group tracking over `cap.pending()`. + - Use `cap.settle` for intermediate/terminal results and `cap.resolve` for the group flush. + - Warn and fall back to `resolve` when `followUp:false` is requested but a capability lacks `settle`. +- Modify: `libs/ag-ui/src/lib/client-tools.ts` + - Implement `settle` and ToolMessage buffering; make `resolve` flush the buffer in one `runAgent`. +- Modify: `libs/langgraph/src/lib/client-tools.ts` + - Implement `settle` and ToolMessage buffering; make `resolve` flush the buffer in one `submitFn`. +- Modify: `libs/chat/src/lib/client-tools/index.ts` + - Export new public types/options if added. +- Modify: `libs/chat/src/public-api.ts` + - Export new public types/options and capability shape changes. +- Regenerate: `apps/website/content/docs/chat/api/api-docs.json` + - Required because public API changes. + +--- + +### Task 1: Adapter `settle()` Contract and Buffering + +**Files:** +- Test: `libs/ag-ui/src/lib/client-tools.spec.ts` +- Test: `libs/langgraph/src/lib/client-tools.spec.ts` +- Modify: `libs/chat/src/lib/client-tools/client-tools-capability.ts` +- Modify: `libs/ag-ui/src/lib/client-tools.ts` +- Modify: `libs/langgraph/src/lib/client-tools.ts` + +- [x] **Step 1: Write failing adapter tests** + +Add tests in both adapter specs: + +```ts +it('settle records a local result without issuing a run', async () => { + const sourceOrSubmit = makeSourceOrSubmit(); + const store = makeStore({ isLoading: false }); + const cap = createClientToolsCapability(sourceOrSubmit, store); + cap.setCatalog([WEATHER_SPEC]); + store.toolCallsSig.set([{ id: 'c1', name: 'get_weather', args: {}, status: 'complete' }]); + + cap.settle?.('c1', { ok: true, value: { temp: 70 } }); + await Promise.resolve(); + + expect(cap.pending()).toHaveLength(0); + expect(runSpy).not.toHaveBeenCalled(); +}); +``` + +Update the existing "resolving multiple pending calls issues one submit/run per resolve" characterization in both adapters to the new M4 behavior: + +```ts +cap.settle?.('c1', { ok: true, value: { temp: 70 } }); +cap.resolve('c2', { ok: true, value: { temp: 71 } }); +await Promise.resolve(); + +expect(runSpy).toHaveBeenCalledOnce(); +expect(flushedToolCallIds).toEqual(['c1', 'c2']); +``` + +- [x] **Step 2: Run adapter tests to verify RED** + +Run: + +```bash +npx vitest run src/lib/client-tools.spec.ts --config vite.config.mts +``` + +from `libs/ag-ui`, then from `libs/langgraph`. + +Expected: FAIL because `settle` is missing and `resolve` still flushes one run per call. + +- [x] **Step 3: Implement adapter buffering** + +In both adapters: + +- Add `settle` to `ClientToolsCapability`. +- Extract the local result write and ToolMessage construction from `resolve` into shared helpers. +- Keep a `toolMessageBuffer` array in the capability closure. +- `settle(id, result)`: + - mark `resolvedIds`; + - write the local result/error patch; + - push the transport-specific ToolMessage into `toolMessageBuffer`; + - issue no run. +- `resolve(id, result)`: + - call `settle(id, result)`; + - flush all buffered ToolMessages in one run; + - clear the buffer after building the run payload. + +- [x] **Step 4: Run adapter tests to verify GREEN** + +Run: + +```bash +npx vitest run src/lib/client-tools.spec.ts --config vite.config.mts +``` + +from `libs/ag-ui`, then from `libs/langgraph`. + +Expected: PASS. + +--- + +### Task 2: Authoring Options for `followUp` + +**Files:** +- Test: `libs/chat/src/lib/client-tools/tools.spec.ts` or nearest existing authoring spec if present +- Modify: `libs/chat/src/lib/client-tools/tool-def.ts` +- Modify: `libs/chat/src/lib/client-tools/tools.ts` + +- [x] **Step 1: Write failing authoring tests** + +Add tests covering: + +```ts +expect(action('x', z.object({}), async () => undefined, { followUp: false }).followUp).toBe(false); +expect(view('x', z.object({}), FakeComponent, { followUp: false }).followUp).toBe(false); +expect(ask('x', z.object({}), FakeComponent, { followUp: false }).followUp).toBe(false); +``` + +- [x] **Step 2: Run tests to verify RED** + +Run: + +```bash +npx vitest run src/lib/client-tools/tools.spec.ts --config vite.config.mts +``` + +Expected: FAIL because `view` and `ask` do not accept options and definitions do not carry `followUp`. + +- [x] **Step 3: Implement minimal authoring API** + +- Add `readonly followUp?: boolean` to function/view/ask definition types. +- Extend `ClientToolExecutionOptions` or introduce a shared `ClientToolContinuationOptions` so all tools can carry `followUp`. +- Keep `action`'s existing `idempotent` option intact. +- Add optional final `options` parameter to `view` and `ask`. + +- [x] **Step 4: Run tests to verify GREEN** + +Run: + +```bash +npx vitest run src/lib/client-tools/tools.spec.ts --config vite.config.mts +``` + +Expected: PASS. + +--- + +### Task 3: Coordinator Group Tracking and Flush Policy + +**Files:** +- Test: `libs/chat/src/lib/client-tools/client-tool-executor.spec.ts` +- Test: `libs/chat/src/lib/client-tools/client-tools-coordinator.spec.ts` +- Test: `libs/chat/src/lib/compositions/chat/chat.component.client-tools.spec.ts` +- Modify: `libs/chat/src/lib/client-tools/client-tool-executor.ts` +- Modify: `libs/chat/src/lib/client-tools/client-tools-coordinator.ts` + +- [x] **Step 1: Write failing coordinator/executor tests** + +Add tests that prove: + +- Two pending function tools in one `pending()` group call `settle` for the first result and `resolve` for the last result. +- A mixed group with `followUp:false` plus a default follow-up tool settles the terminal result and flushes both results in one final `resolve`. +- A fully-terminal group (`followUp:false` on every tool) calls only `settle` and never `resolve`. +- View auto-ack and ask render-result handling participate in the same group tracker. +- If `followUp:false` is requested and `cap.settle` is absent, the coordinator falls back to `resolve` and does not drop the result. +- Existing guard behavior still claims and records before group settlement. + +- [x] **Step 2: Run chat tests to verify RED** + +Run: + +```bash +npx vitest run src/lib/client-tools/client-tool-executor.spec.ts src/lib/client-tools/client-tools-coordinator.spec.ts src/lib/compositions/chat/chat.component.client-tools.spec.ts --config vite.config.mts +``` + +Expected: FAIL because the executor and coordinator still call `resolve` per result. + +- [x] **Step 3: Implement group tracker** + +- Add an internal settlement callback option to `startClientToolExecutor`. +- Keep default executor behavior backward-compatible when no callback is supplied. +- In `createClientToolsCoordinator`, maintain the current pending group as a set of pending IDs from the same `cap.pending()` emission. +- Track settled IDs/results by tool call ID. +- For each completed result: + - if every unsettled remaining call in the group is terminal, call `settle` for terminal calls and do not run; + - if this is not the last follow-up call, call `settle`; + - when the last follow-up call settles, call `resolve` for that call so the adapter flushes the buffered group once. +- Treat `followUp !== false` as default follow-up. +- Preserve ask lookup by `elementKey` and view auto-ack idempotence. + +- [x] **Step 4: Run chat tests to verify GREEN** + +Run: + +```bash +npx vitest run src/lib/client-tools/client-tool-executor.spec.ts src/lib/client-tools/client-tools-coordinator.spec.ts src/lib/compositions/chat/chat.component.client-tools.spec.ts --config vite.config.mts +``` + +Expected: PASS. + +--- + +### Task 4: Public API Docs and Verification + +**Files:** +- Modify: `libs/chat/src/lib/client-tools/index.ts` +- Modify: `libs/chat/src/public-api.ts` +- Regenerate: `apps/website/content/docs/chat/api/api-docs.json` + +- [x] **Step 1: Export public API changes** + +Export any new option types and the updated capability contract through the package's existing public surfaces. Do not export the internal coordinator factory publicly. + +- [x] **Step 2: Regenerate API docs** + +Run: + +```bash +npm run generate-api-docs +``` + +Expected: succeeds and updates chat API docs. + +- [x] **Step 3: Run focused and package verification** + +Run: + +```bash +npx nx test chat +npx nx test ag-ui +npx nx test langgraph +npx nx lint chat +npx nx lint ag-ui +npx nx lint langgraph +npx nx build chat +npx nx build ag-ui +npx nx build langgraph +npx nx run-many -t build --projects=chat,langgraph,ag-ui,render,a2ui,licensing,telemetry --configuration=production +node scripts/check-dx-coverage.mjs +git diff --check +``` + +Expected: all commands exit 0. Lint warnings are acceptable only if there are zero lint errors. + +- [x] **Step 4: Scan for forbidden external names outside approved docs** + +Run: + +```bash +(git diff --name-only; git ls-files --others --exclude-standard) | rg -v '^docs/superpowers/' | xargs rg -n 'hashbrown|copilotkit|chatgpt|claude' || true +``` + +Expected: no output. + +- [x] **Step 5: Review diff and commit** + +Run: + +```bash +git status --short +git diff --stat +git diff -- libs/chat/src/lib/client-tools libs/ag-ui/src/lib/client-tools.ts libs/langgraph/src/lib/client-tools.ts +git add docs/superpowers/plans/2026-07-08-client-tools-m4-settle-batching-plan.md libs/chat/src libs/ag-ui/src/lib/client-tools.ts libs/ag-ui/src/lib/client-tools.spec.ts libs/langgraph/src/lib/client-tools.ts libs/langgraph/src/lib/client-tools.spec.ts apps/website/content/docs/chat/api/api-docs.json +git commit -m "feat: batch client tool continuations" +``` + +Expected: commit succeeds with no external framework references in the commit message. + +--- + +## Acceptance Checklist + +- [ ] `ClientToolsCapability.settle?` exists and is optional. +- [ ] AG-UI `settle` records a local result and buffers a ToolMessage without a run. +- [ ] LangGraph `settle` records a local result and buffers a ToolMessage without a run. +- [ ] AG-UI `resolve` flushes all buffered ToolMessages in one run. +- [ ] LangGraph `resolve` flushes all buffered ToolMessages in one submit. +- [ ] Coordinator batches one assistant-turn pending group into one continuation run. +- [ ] `followUp:false` terminal tools do not force a continuation run. +- [ ] Fully-terminal groups issue no run. +- [ ] Missing `settle` capability falls back without dropping results. +- [ ] Guarded function tools still claim/record before settling or resolving. +- [ ] No new dependencies. +- [ ] API docs regenerated for public API changes. +- [ ] Verification commands above pass before PR creation. diff --git a/libs/ag-ui/src/lib/client-tools.spec.ts b/libs/ag-ui/src/lib/client-tools.spec.ts index 1f31b95e2..fb626294e 100644 --- a/libs/ag-ui/src/lib/client-tools.spec.ts +++ b/libs/ag-ui/src/lib/client-tools.spec.ts @@ -202,7 +202,24 @@ describe('createClientToolsCapability', () => { expect(store.toolCalls().find((tc) => tc.id === 'c2')?.result).toBeUndefined(); }); - it('resolving multiple pending calls issues one run per resolve', async () => { + it('settle(ok) drops the id from pending and records the result without running', async () => { + const source = makeSource(); + const store = makeStore(); + const cap = createClientToolsCapability(source, store); + cap.setCatalog([WEATHER_SPEC]); + store.isLoading.set(false); + store.toolCalls.set([{ id: 'c1', name: 'get_weather', args: {}, status: 'complete' }]); + + cap.settle?.('c1', { ok: true, value: { temp: 70 } }); + await Promise.resolve(); + + expect(cap.pending()).toHaveLength(0); + expect(store.toolCalls().find((tc) => tc.id === 'c1')?.result).toEqual({ temp: 70 }); + expect(source.addMessage).toHaveBeenCalledOnce(); + expect(source.runAgent).not.toHaveBeenCalled(); + }); + + it('settle plus resolve flushes multiple pending results in one run', async () => { const source = makeSource(); const store = makeStore(); const cap = createClientToolsCapability(source, store); @@ -213,12 +230,12 @@ describe('createClientToolsCapability', () => { { id: 'c2', name: 'get_weather', args: {}, status: 'complete' }, ]); - cap.resolve('c1', { ok: true, value: { temp: 70 } }); + cap.settle?.('c1', { ok: true, value: { temp: 70 } }); cap.resolve('c2', { ok: true, value: { temp: 71 } }); await Promise.resolve(); expect(source.addMessage).toHaveBeenCalledTimes(2); - expect(source.runAgent).toHaveBeenCalledTimes(2); + expect(source.runAgent).toHaveBeenCalledTimes(1); expect(source.addMessage.mock.calls.map(([message]) => ( message as { toolCallId: string } ).toolCallId)).toEqual(['c1', 'c2']); diff --git a/libs/ag-ui/src/lib/client-tools.ts b/libs/ag-ui/src/lib/client-tools.ts index 5e18ea1f7..25d489d1f 100644 --- a/libs/ag-ui/src/lib/client-tools.ts +++ b/libs/ag-ui/src/lib/client-tools.ts @@ -40,11 +40,14 @@ function safeStringify(v: unknown): string { * have no backend result, and haven't been resolved client-side yet — but ONLY * when the run is not in progress (isLoading===false). The backend ends the run * without emitting TOOL_CALL_RESULT for client tools, so result stays undefined. - * - resolve(id, result): marks the call as resolved, writes the outcome onto + * - settle(id, result): marks the call as resolved, writes the outcome onto * the local ToolCall in the store (so the transcript freezes: the mounted * ask component re-renders with its emitted value as props and can branch to - * a frozen state), adds a ToolMessage via source.addMessage, then re-runs - * the agent with the catalog tools attached. + * a frozen state), and adds a ToolMessage via source.addMessage without + * starting a run. + * - resolve(id, result): settles the result, then re-runs the agent with the + * catalog tools attached. Any ToolMessages previously settled into the + * source are flushed by that single run. * * Call catalogAsAgUiTools() to get the current catalog as AG-UI Tool[] for * threading into runAgent(). @@ -60,6 +63,47 @@ export function createClientToolsCapability( return catalog().map(toAgUiTool); } + function settleResult(id: string, result: ClientToolResult): void { + // Mark as resolved first so pending() drops it immediately. + resolvedIds.update((s) => new Set(s).add(id)); + + // Write the outcome onto the LOCAL ToolCall in the store. The client tool + // DID produce a result client-side, so this is semantically correct — and + // it freezes the transcript card: toToolViewSpec spreads `{...args, + // ...result, status}` into the mounted ask component, so the component + // re-renders with its own emitted value as props and can branch to a + // resolved/frozen state. The backend ToolMessage never reaches this local + // ToolCall, so without this write the card stays interactive forever. + const ok = result.ok; + const value = (result as { value: unknown }).value; + const error = (result as { error: string }).error; + store.toolCalls.update((calls) => + calls.map((tc) => + tc.id === id + ? { + ...tc, + result: ok ? value : { error }, + ...(ok ? {} : { error, status: 'error' as const }), + } + : tc, + ), + ); + + // Cast rather than rely on discriminant narrowing: consumer apps that + // compile this source with `strictNullChecks: false` don't narrow the + // ClientToolResult union in a ternary. + const content = ok + ? safeStringify(value) + : `Error: ${error}`; + + source.addMessage({ + id: `tool-${id}`, + role: 'tool', + toolCallId: id, + content, + } as Message); + } + const clientTools: ClientToolsCapability & { catalogAsAgUiTools(): Tool[] } = { setCatalog(specs: readonly ClientToolSpec[]): void { catalog.set([...specs]); @@ -76,46 +120,12 @@ export function createClientToolsCapability( }); }), - resolve(id: string, result: ClientToolResult): void { - // Mark as resolved first so pending() drops it immediately. - resolvedIds.update((s) => new Set(s).add(id)); - - // Write the outcome onto the LOCAL ToolCall in the store. The client tool - // DID produce a result client-side, so this is semantically correct — and - // it freezes the transcript card: toToolViewSpec spreads `{...args, - // ...result, status}` into the mounted ask component, so the component - // re-renders with its own emitted value as props and can branch to a - // resolved/frozen state. The backend ToolMessage never reaches this local - // ToolCall, so without this write the card stays interactive forever. - const ok = result.ok; - const value = (result as { value: unknown }).value; - const error = (result as { error: string }).error; - store.toolCalls.update((calls) => - calls.map((tc) => - tc.id === id - ? { - ...tc, - result: ok ? value : { error }, - ...(ok ? {} : { error, status: 'error' as const }), - } - : tc, - ), - ); - - // Cast rather than rely on discriminant narrowing: consumer apps that - // compile this source with `strictNullChecks: false` don't narrow the - // ClientToolResult union in a ternary. - const content = ok - ? safeStringify(value) - : `Error: ${error}`; - - source.addMessage({ - id: `tool-${id}`, - role: 'tool', - toolCallId: id, - content, - } as Message); + settle(id: string, result: ClientToolResult): void { + settleResult(id, result); + }, + resolve(id: string, result: ClientToolResult): void { + settleResult(id, result); void source.runAgent({ tools: catalogAsAgUiTools() }); }, diff --git a/libs/chat/src/lib/client-tools/client-tool-executor.spec.ts b/libs/chat/src/lib/client-tools/client-tool-executor.spec.ts index e9746bea6..ef801e527 100644 --- a/libs/chat/src/lib/client-tools/client-tool-executor.spec.ts +++ b/libs/chat/src/lib/client-tools/client-tool-executor.spec.ts @@ -125,6 +125,27 @@ describe('startClientToolExecutor()', () => { expect(resolve).toHaveBeenCalledWith('c1', { ok: true, value: { temp: 70, city: 'SF' } }); }); + it('uses a custom settlement handler when provided', async () => { + const { pending, resolve, capability } = makeFakeCapability(); + const settleToolCall = vi.fn<[ToolCall, ClientToolResult], void>(); + const agent = makeFakeAgent(capability); + + TestBed.runInInjectionContext(() => { + startClientToolExecutor(agent, weatherRegistry, { settleToolCall }); + }); + + pending.set([{ id: 'c1', name: 'get_weather', args: { city: 'SF' }, status: 'complete' }]); + TestBed.flushEffects(); + await drainMicrotasks(); + + expect(settleToolCall).toHaveBeenCalledOnce(); + expect(settleToolCall).toHaveBeenCalledWith( + { id: 'c1', name: 'get_weather', args: { city: 'SF' }, status: 'complete' }, + { ok: true, value: { temp: 70, city: 'SF' } }, + ); + expect(resolve).not.toHaveBeenCalled(); + }); + it('normalizes handler throws to an error result', async () => { const { pending, resolve, capability } = makeFakeCapability(); const agent = makeFakeAgent(capability); diff --git a/libs/chat/src/lib/client-tools/client-tool-executor.ts b/libs/chat/src/lib/client-tools/client-tool-executor.ts index 29f0b8e05..4c2ca554f 100644 --- a/libs/chat/src/lib/client-tools/client-tool-executor.ts +++ b/libs/chat/src/lib/client-tools/client-tool-executor.ts @@ -1,8 +1,9 @@ // SPDX-License-Identifier: MIT import { DestroyRef, effect, inject } from '@angular/core'; import type { Agent } from '../agent'; +import type { ToolCall } from '../agent/tool-call'; import type { ClientToolRegistry, AnyFunctionToolDef } from './tool-def'; -import type { ClientToolsCapability, ClientToolResult } from './client-tools-capability'; +import type { ClientToolResult } from './client-tools-capability'; import { executeFunctionTool } from './execute'; import { clientToolGuardFailureResult, @@ -16,6 +17,7 @@ import { /** Options for wiring automatic browser function-tool execution. */ export interface ClientToolExecutorOptions { readonly executionGuard?: ClientToolExecutionGuard; + readonly settleToolCall?: (toolCall: ToolCall, result: ClientToolResult) => void; } /** @@ -59,12 +61,13 @@ export function startClientToolExecutor( const controller = new AbortController(); inFlight.set(tc.id, controller); void runFunctionTool({ - cap, def, + toolCall: tc, rawArgs: tc.args, toolCallId: tc.id, controller, executionGuard: options.executionGuard, + settleToolCall: options.settleToolCall ?? ((toolCall, result) => cap.resolve(toolCall.id, result)), }).finally(() => { inFlight.delete(tc.id); }); @@ -73,18 +76,19 @@ export function startClientToolExecutor( } async function runFunctionTool(input: { - readonly cap: ClientToolsCapability; readonly def: AnyFunctionToolDef; + readonly toolCall: ToolCall; readonly rawArgs: unknown; readonly toolCallId: string; readonly controller: AbortController; readonly executionGuard?: ClientToolExecutionGuard; + readonly settleToolCall: (toolCall: ToolCall, result: ClientToolResult) => void; }): Promise { - const { cap, def, rawArgs, toolCallId, controller, executionGuard } = input; + const { def, toolCall, rawArgs, toolCallId, controller, executionGuard, settleToolCall } = input; const signal = controller.signal; if (!executionGuard || !shouldClaimBeforeExecute(def)) { const result = await executeFunctionTool(def, rawArgs, { signal }); - if (!signal.aborted) cap.resolve(toolCallId, result); + if (!signal.aborted) settleToolCall(toolCall, result); return; } @@ -93,7 +97,7 @@ async function runFunctionTool(input: { try { claim = await executionGuard.store.claim(key); } catch (err) { - if (!signal.aborted) cap.resolve(toolCallId, clientToolGuardFailureResult(toolCallId, err)); + if (!signal.aborted) settleToolCall(toolCall, clientToolGuardFailureResult(toolCallId, err)); return; } if (signal.aborted) return; @@ -101,34 +105,51 @@ async function runFunctionTool(input: { if (claim === 'claimed') { const result = await executeFunctionTool(def, rawArgs, { signal }); if (signal.aborted) return; - await recordOrResolveGuardFailure(executionGuard, key, result, cap, toolCallId, signal); + await recordOrResolveGuardFailure( + executionGuard, + key, + result, + toolCall, + toolCallId, + signal, + settleToolCall, + ); return; } if (claim.status === 'done') { - cap.resolve(toolCallId, claim.result); + settleToolCall(toolCall, claim.result); return; } const result = claim.status === 'failed' && claim.result ? claim.result : defaultInterruptedClientToolResult(toolCallId); - await recordOrResolveGuardFailure(executionGuard, key, result, cap, toolCallId, signal); + await recordOrResolveGuardFailure( + executionGuard, + key, + result, + toolCall, + toolCallId, + signal, + settleToolCall, + ); } async function recordOrResolveGuardFailure( executionGuard: ClientToolExecutionGuard, key: ClientToolExecutionKey, result: ClientToolResult, - cap: ClientToolsCapability, + toolCall: ToolCall, toolCallId: string, signal: AbortSignal, + settleToolCall: (toolCall: ToolCall, result: ClientToolResult) => void, ): Promise { try { await executionGuard.store.record(key, result); } catch (err) { - if (!signal.aborted) cap.resolve(toolCallId, clientToolGuardFailureResult(toolCallId, err)); + if (!signal.aborted) settleToolCall(toolCall, clientToolGuardFailureResult(toolCallId, err)); return; } - if (!signal.aborted) cap.resolve(toolCallId, result); + if (!signal.aborted) settleToolCall(toolCall, result); } diff --git a/libs/chat/src/lib/client-tools/client-tools-capability.ts b/libs/chat/src/lib/client-tools/client-tools-capability.ts index 13129bb69..c6dee3f9b 100644 --- a/libs/chat/src/lib/client-tools/client-tools-capability.ts +++ b/libs/chat/src/lib/client-tools/client-tools-capability.ts @@ -19,6 +19,8 @@ export interface ClientToolsCapability { setCatalog(specs: readonly ClientToolSpec[]): void; /** Tool calls the model made for client tools that await a client result. */ readonly pending: Signal; + /** Record a client tool's result without continuing the run. */ + settle?(toolCallId: string, result: ClientToolResult): void; /** Return a client tool's result (or error) and continue the run. */ resolve(toolCallId: string, result: ClientToolResult): void; } diff --git a/libs/chat/src/lib/client-tools/client-tools-coordinator.spec.ts b/libs/chat/src/lib/client-tools/client-tools-coordinator.spec.ts index 75f324237..ba21e6ae8 100644 --- a/libs/chat/src/lib/client-tools/client-tools-coordinator.spec.ts +++ b/libs/chat/src/lib/client-tools/client-tools-coordinator.spec.ts @@ -30,6 +30,20 @@ class FakeAskComponent {} // ── factory helpers ─────────────────────────────────────────────────────────── function makeFakeCapability() { + const pending = signal([]); + const settle = vi.fn<[string, ClientToolResult], void>(); + const resolve = vi.fn<[string, ClientToolResult], void>(); + const setCatalog = vi.fn<[readonly unknown[]], void>(); + const capability: ClientToolsCapability = { + setCatalog, + pending, + settle, + resolve, + }; + return { pending, settle, resolve, setCatalog, capability }; +} + +function makeFakeCapabilityWithoutSettle() { const pending = signal([]); const resolve = vi.fn<[string, ClientToolResult], void>(); const setCatalog = vi.fn<[readonly unknown[]], void>(); @@ -217,6 +231,118 @@ describe('createClientToolsCoordinator()', () => { expect(resolve).toHaveBeenCalledWith('f1', { ok: true, value: { temp: 72, city: 'SF' } }); }); + it('batches two pending function tools into one group flush', async () => { + const registry = tools({ + weather_a: action('Get weather A', z.object({ city: z.string() }), async (a) => `A:${a.city}`), + weather_b: action('Get weather B', z.object({ city: z.string() }), async (a) => `B:${a.city}`), + }); + const { pending, settle, resolve, capability } = makeFakeCapability(); + const agent = makeFakeAgent(capability); + const coordinator = createClientToolsCoordinator(registry); + + TestBed.runInInjectionContext(() => { + coordinator.connect(agent); + }); + + pending.set([ + { id: 'f1', name: 'weather_a', args: { city: 'SF' }, status: 'running' }, + { id: 'f2', name: 'weather_b', args: { city: 'LA' }, status: 'running' }, + ]); + TestBed.flushEffects(); + await drainMicrotasks(); + + expect(settle).toHaveBeenCalledOnce(); + expect(settle).toHaveBeenCalledWith('f1', { ok: true, value: 'A:SF' }); + expect(resolve).toHaveBeenCalledOnce(); + expect(resolve).toHaveBeenCalledWith('f2', { ok: true, value: 'B:LA' }); + }); + + it('settles terminal tools and flushes once when a mixed group completes', async () => { + const registry = tools({ + terminal_card: view( + 'Show terminal card', + z.object({ city: z.string() }), + FakeViewComponent as never, + { followUp: false }, + ), + get_weather: action( + 'Get weather', + z.object({ city: z.string() }), + async (a) => ({ temp: 72, city: a.city }), + ), + }); + const { pending, settle, resolve, capability } = makeFakeCapability(); + const agent = makeFakeAgent(capability); + const coordinator = createClientToolsCoordinator(registry); + + TestBed.runInInjectionContext(() => { + coordinator.connect(agent); + }); + + pending.set([ + { id: 'v1', name: 'terminal_card', args: { city: 'LA' }, status: 'running' }, + { id: 'f1', name: 'get_weather', args: { city: 'SF' }, status: 'running' }, + ]); + TestBed.flushEffects(); + await drainMicrotasks(); + + expect(settle).toHaveBeenCalledWith('v1', { ok: true, value: { shown: true } }); + expect(resolve).toHaveBeenCalledOnce(); + expect(resolve).toHaveBeenCalledWith('f1', { ok: true, value: { temp: 72, city: 'SF' } }); + }); + + it('settles a fully-terminal group without resolving', () => { + const registry = tools({ + terminal_card: view( + 'Show terminal card', + z.object({ city: z.string() }), + FakeViewComponent as never, + { followUp: false }, + ), + }); + const { pending, settle, resolve, capability } = makeFakeCapability(); + const agent = makeFakeAgent(capability); + const coordinator = createClientToolsCoordinator(registry); + + TestBed.runInInjectionContext(() => { + coordinator.connect(agent); + }); + + pending.set([{ id: 'v1', name: 'terminal_card', args: { city: 'LA' }, status: 'running' }]); + TestBed.flushEffects(); + + expect(settle).toHaveBeenCalledOnce(); + expect(settle).toHaveBeenCalledWith('v1', { ok: true, value: { shown: true } }); + expect(resolve).not.toHaveBeenCalled(); + }); + + it('falls back to resolve when followUp:false cannot be honored without settle', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + const registry = tools({ + terminal_card: view( + 'Show terminal card', + z.object({ city: z.string() }), + FakeViewComponent as never, + { followUp: false }, + ), + }); + const { pending, resolve, capability } = makeFakeCapabilityWithoutSettle(); + const agent = makeFakeAgent(capability); + const coordinator = createClientToolsCoordinator(registry); + + TestBed.runInInjectionContext(() => { + coordinator.connect(agent); + }); + + pending.set([{ id: 'v1', name: 'terminal_card', args: { city: 'LA' }, status: 'running' }]); + TestBed.flushEffects(); + + expect(resolve).toHaveBeenCalledOnce(); + expect(resolve).toHaveBeenCalledWith('v1', { ok: true, value: { shown: true } }); + expect(warn).toHaveBeenCalledOnce(); + warn.mockRestore(); + }); + it('passes an execution guard to function-tool execution', async () => { const { pending, resolve, capability } = makeFakeCapability(); const agent = makeFakeAgent(capability); @@ -261,6 +387,44 @@ describe('createClientToolsCoordinator()', () => { expect(resolve).toHaveBeenCalledWith('a1', { ok: true, value: { picked: 2 } }); }); + it('batches ask results with the pending group before flushing', async () => { + const registry = tools({ + confirm_booking: ask( + 'Ask user to confirm a booking', + z.object({ flight: z.string() }), + FakeAskComponent as never, + ), + get_weather: action( + 'Get weather', + z.object({ city: z.string() }), + async (a) => ({ temp: 72, city: a.city }), + ), + }); + const { pending, settle, resolve, capability } = makeFakeCapability(); + const agent = makeFakeAgent(capability); + const coordinator = createClientToolsCoordinator(registry); + + TestBed.runInInjectionContext(() => { + coordinator.connect(agent); + }); + + pending.set([ + { id: 'a1', name: 'confirm_booking', args: { flight: 'UA1' }, status: 'running' }, + { id: 'f1', name: 'get_weather', args: { city: 'SF' }, status: 'running' }, + ]); + TestBed.flushEffects(); + coordinator.handleRenderEvent(agent, { + type: 'result', + value: { confirmed: true }, + elementKey: 'confirm_booking', + }); + await drainMicrotasks(); + + expect(settle).toHaveBeenCalledWith('a1', { ok: true, value: { confirmed: true } }); + expect(resolve).toHaveBeenCalledOnce(); + expect(resolve).toHaveBeenCalledWith('f1', { ok: true, value: { temp: 72, city: 'SF' } }); + }); + it('handleRenderEvent() does NOT resolve a view tool via handleRenderEvent (views auto-ack separately)', () => { const { pending, resolve, capability } = makeFakeCapability(); const agent = makeFakeAgent(capability); diff --git a/libs/chat/src/lib/client-tools/client-tools-coordinator.ts b/libs/chat/src/lib/client-tools/client-tools-coordinator.ts index 86c66b8de..17c24fd9f 100644 --- a/libs/chat/src/lib/client-tools/client-tools-coordinator.ts +++ b/libs/chat/src/lib/client-tools/client-tools-coordinator.ts @@ -8,6 +8,7 @@ import type { ClientToolExecutionGuard } from './client-tool-execution-guard'; import type { ClientToolSpec } from './to-json-schema'; import { deriveJsonSchema } from './to-json-schema'; import { startClientToolExecutor } from './client-tool-executor'; +import type { ClientToolsCapability, ClientToolResult } from './client-tools-capability'; export interface ClientToolsCoordinator { /** Components for `view`/`ask` tools, keyed by tool name — merge into the chat `views`. */ @@ -25,6 +26,12 @@ export interface ClientToolsCoordinatorOptions { readonly executionGuard?: ClientToolExecutionGuard; } +interface PendingToolGroup { + readonly ids: ReadonlySet; + readonly hasFollowUp: boolean; + readonly settledIds: Set; +} + /** Build the catalog spec list shipped to the model. */ export function toClientToolSpecs(registry: ClientToolRegistry): ClientToolSpec[] { return Object.entries(registry).map(([name, def]) => ({ @@ -54,6 +61,62 @@ export function createClientToolsCoordinator( ): ClientToolsCoordinator { const viewRegistry = views(viewComponents(registry)); const ackedViews = new Set(); + let currentGroup: PendingToolGroup | undefined; + + function toolWantsFollowUp(tc: ToolCall): boolean { + return registry[tc.name]?.followUp !== false; + } + + function createGroup(calls: readonly ToolCall[]): PendingToolGroup { + return { + ids: new Set(calls.map((tc) => tc.id)), + hasFollowUp: calls.some(toolWantsFollowUp), + settledIds: new Set(), + }; + } + + function groupFor(cap: ClientToolsCapability, tc: ToolCall): PendingToolGroup { + if (currentGroup?.ids.has(tc.id)) return currentGroup; + const pending = cap.pending(); + const calls = pending.some((pendingCall) => pendingCall.id === tc.id) + ? pending + : [tc]; + currentGroup = createGroup(calls); + return currentGroup; + } + + function warnMissingSettle(tc: ToolCall): void { + console.warn( + `Client tool "${tc.name}" requested batched or terminal settlement, but the agent capability does not implement settle(); falling back to resolve().`, + ); + } + + function settleClientToolCall( + cap: ClientToolsCapability, + tc: ToolCall, + result: ClientToolResult, + ): void { + const group = groupFor(cap, tc); + if (group.settledIds.has(tc.id)) return; + group.settledIds.add(tc.id); + + const groupComplete = Array.from(group.ids).every((id) => group.settledIds.has(id)); + if (!cap.settle) { + if (group.ids.size > 1 || registry[tc.name]?.followUp === false) warnMissingSettle(tc); + cap.resolve(tc.id, result); + if (groupComplete) currentGroup = undefined; + return; + } + + if (groupComplete && group.hasFollowUp) { + cap.resolve(tc.id, result); + currentGroup = undefined; + return; + } + + cap.settle(tc.id, result); + if (groupComplete) currentGroup = undefined; + } return { viewRegistry, @@ -61,7 +124,10 @@ export function createClientToolsCoordinator( const cap = agent.clientTools; if (!cap) return; cap.setCatalog(toClientToolSpecs(registry)); - startClientToolExecutor(agent, registry, { executionGuard: options.executionGuard }); // function tools + startClientToolExecutor(agent, registry, { + executionGuard: options.executionGuard, + settleToolCall: (tc, result) => settleClientToolCall(cap, tc, result), + }); // function tools // Auto-ack `view` tools: they render but produce no user value. effect(() => { for (const tc of cap.pending()) { @@ -69,7 +135,7 @@ export function createClientToolsCoordinator( if (!def || def.kind !== 'view') continue; if (ackedViews.has(tc.id)) continue; ackedViews.add(tc.id); - cap.resolve(tc.id, { ok: true, value: { shown: true } }); + settleClientToolCall(cap, tc, { ok: true, value: { shown: true } }); } }); }, @@ -83,7 +149,7 @@ export function createClientToolsCoordinator( const pending = cap.pending().find( (tc: ToolCall) => tc.name === name && registry[tc.name]?.kind === 'ask', ); - if (pending) cap.resolve(pending.id, { ok: true, value: event.value }); + if (pending) settleClientToolCall(cap, pending, { ok: true, value: event.value }); }, }; } diff --git a/libs/chat/src/lib/client-tools/index.ts b/libs/chat/src/lib/client-tools/index.ts index 64770d88e..6090f49f3 100644 --- a/libs/chat/src/lib/client-tools/index.ts +++ b/libs/chat/src/lib/client-tools/index.ts @@ -1,7 +1,17 @@ // SPDX-License-Identifier: MIT export { action, view, ask, tools } from './tools'; export { deriveJsonSchema } from './to-json-schema'; -export type { ClientToolDef, ClientToolExecutionOptions, FunctionToolDef, FunctionToolHandlerContext, AnyFunctionToolDef, ViewToolDef, AskToolDef, ClientToolRegistry } from './tool-def'; +export type { + ClientToolContinuationOptions, + ClientToolDef, + ClientToolExecutionOptions, + FunctionToolDef, + FunctionToolHandlerContext, + AnyFunctionToolDef, + ViewToolDef, + AskToolDef, + ClientToolRegistry, +} from './tool-def'; export type { ClientToolSpec } from './to-json-schema'; export type { ClientToolsCapability, ClientToolResult } from './client-tools-capability'; export { validateArgs, executeFunctionTool } from './execute'; diff --git a/libs/chat/src/lib/client-tools/tool-def.ts b/libs/chat/src/lib/client-tools/tool-def.ts index 8fb89aaa3..411632a32 100644 --- a/libs/chat/src/lib/client-tools/tool-def.ts +++ b/libs/chat/src/lib/client-tools/tool-def.ts @@ -11,7 +11,13 @@ export interface FunctionToolHandlerContext { } /** Execution policy options for browser-executed function tools. */ -export interface ClientToolExecutionOptions { +export interface ClientToolContinuationOptions { + /** False when the tool result should be recorded without forcing a continuation. */ + readonly followUp?: boolean; +} + +/** Execution policy options for browser-executed function tools. */ +export interface ClientToolExecutionOptions extends ClientToolContinuationOptions { /** True when the handler may safely re-run and should skip durable pre-claims. */ readonly idempotent?: boolean; } @@ -22,6 +28,7 @@ export interface FunctionToolDef, @@ -38,6 +45,7 @@ export interface AnyFunctionToolDef { readonly kind: 'function'; readonly description: string; readonly schema: StandardSchemaV1; + readonly followUp?: boolean; readonly idempotent?: boolean; // eslint-disable-next-line @typescript-eslint/no-explicit-any -- bivariance escape hatch; see note above readonly handler: (args: any, context: FunctionToolHandlerContext) => unknown | Promise; @@ -47,6 +55,7 @@ export interface ViewToolDef; } @@ -54,6 +63,7 @@ export interface AskToolDef; } diff --git a/libs/chat/src/lib/client-tools/tools.spec.ts b/libs/chat/src/lib/client-tools/tools.spec.ts index 74d4e894b..562af5a10 100644 --- a/libs/chat/src/lib/client-tools/tools.spec.ts +++ b/libs/chat/src/lib/client-tools/tools.spec.ts @@ -30,6 +30,11 @@ describe('action()', () => { }); expect(t.kind).toBe('function'); }); + + it('carries followUp:false from options', () => { + const def = action('no follow-up', z.object({}), async () => undefined, { followUp: false }); + expect(def.followUp).toBe(false); + }); }); describe('view()', () => { @@ -42,6 +47,16 @@ describe('view()', () => { expect(def.schema).toBe(schema); expect((def as { component: unknown }).component).toBe(FakeComponent); }); + + it('carries followUp:false from options', () => { + const def = view( + 'terminal view', + z.object({}), + FakeComponent as unknown as import('@angular/core').Type, + { followUp: false }, + ); + expect(def.followUp).toBe(false); + }); }); describe('ask()', () => { @@ -54,6 +69,16 @@ describe('ask()', () => { expect(def.schema).toBe(schema); expect((def as { component: unknown }).component).toBe(FakeComponent); }); + + it('carries followUp:false from options', () => { + const def = ask( + 'terminal ask', + z.object({}), + FakeComponent as unknown as import('@angular/core').Type, + { followUp: false }, + ); + expect(def.followUp).toBe(false); + }); }); describe('tools()', () => { diff --git a/libs/chat/src/lib/client-tools/tools.ts b/libs/chat/src/lib/client-tools/tools.ts index 2c218a49b..7f7798f0a 100644 --- a/libs/chat/src/lib/client-tools/tools.ts +++ b/libs/chat/src/lib/client-tools/tools.ts @@ -4,6 +4,7 @@ import type { StandardSchemaV1, StandardSchemaInferOutput } from '@threadplane/r import type { AskToolDef, ClientToolDef, + ClientToolContinuationOptions, ClientToolExecutionOptions, FunctionToolDef, FunctionToolHandlerContext, @@ -77,8 +78,9 @@ export function view( description: string, schema: S, component: AcceptComponent, + options: ClientToolContinuationOptions = {}, ): ViewToolDef { - return { kind: 'view', description, schema, component: component as Type }; + return { kind: 'view', description, schema, component: component as Type, ...options }; } /** @@ -117,8 +119,9 @@ export function ask( description: string, schema: S, component: AcceptComponent, + options: ClientToolContinuationOptions = {}, ): AskToolDef { - return { kind: 'ask', description, schema, component: component as Type }; + return { kind: 'ask', description, schema, component: component as Type, ...options }; } /** diff --git a/libs/chat/src/public-api.ts b/libs/chat/src/public-api.ts index a5903c4f2..b86c72223 100644 --- a/libs/chat/src/public-api.ts +++ b/libs/chat/src/public-api.ts @@ -230,7 +230,7 @@ export { isPathRef, isLiteralString, isLiteralNumber, isLiteralBoolean } from '@ // Client tools (declaration API — tools/action/view/ask + JSON-schema derivation) export { tools, action, view, ask } from './lib/client-tools/tools'; export { deriveJsonSchema } from './lib/client-tools/to-json-schema'; -export type { ClientToolDef, ClientToolExecutionOptions, AnyFunctionToolDef, FunctionToolDef, FunctionToolHandlerContext, ViewToolDef, AskToolDef, ClientToolRegistry, StandardSchemaV1, StandardSchemaInferInput, StandardSchemaInferOutput } from './lib/client-tools/tool-def'; +export type { ClientToolContinuationOptions, ClientToolDef, ClientToolExecutionOptions, AnyFunctionToolDef, FunctionToolDef, FunctionToolHandlerContext, ViewToolDef, AskToolDef, ClientToolRegistry, StandardSchemaV1, StandardSchemaInferInput, StandardSchemaInferOutput } from './lib/client-tools/tool-def'; export type { ViewProps } from './lib/client-tools/component-inputs'; /** Inferred argument type for a schema (alias of StandardSchemaInferOutput). */ export type ToolArgs = import('./lib/client-tools/tool-def').StandardSchemaInferOutput; diff --git a/libs/langgraph/src/lib/client-tools.spec.ts b/libs/langgraph/src/lib/client-tools.spec.ts index 33e596ff6..7fb3e1349 100644 --- a/libs/langgraph/src/lib/client-tools.spec.ts +++ b/libs/langgraph/src/lib/client-tools.spec.ts @@ -266,7 +266,24 @@ describe('createClientToolsCapability', () => { expect(remaining[0].id).toBe('c2'); }); - it('resolving multiple pending calls issues one submit per resolve', async () => { + it('settle(ok) drops the id from pending and records the result without submitting', async () => { + const submitFn = makeSubmitFn(); + const store = makeStore({ isLoading: false }); + const cap = createClientToolsCapability(submitFn, store); + cap.setCatalog([WEATHER_SPEC]); + store.toolCallsSig.set([ + { id: 'c1', name: 'get_weather', args: {}, status: 'complete' }, + ]); + + cap.settle?.('c1', { ok: true, value: { temp: 70 } }); + await Promise.resolve(); + + expect(cap.pending()).toHaveLength(0); + expect(store.toolCalls().find((tc) => tc.id === 'c1')?.result).toEqual({ temp: 70 }); + expect(submitFn).not.toHaveBeenCalled(); + }); + + it('settle plus resolve flushes multiple pending results in one submit', async () => { const submitFn = makeSubmitFn(); const store = makeStore({ isLoading: false }); const cap = createClientToolsCapability(submitFn, store); @@ -276,16 +293,14 @@ describe('createClientToolsCapability', () => { { id: 'c2', name: 'get_weather', args: {}, status: 'complete' }, ]); - cap.resolve('c1', { ok: true, value: { temp: 70 } }); + cap.settle?.('c1', { ok: true, value: { temp: 70 } }); cap.resolve('c2', { ok: true, value: { temp: 71 } }); await Promise.resolve(); - expect(submitFn).toHaveBeenCalledTimes(2); - const payloads = (submitFn as unknown as ReturnType).mock.calls - .map(([payload]) => payload as Record); - expect(payloads.map((payload) => ( - (payload['messages'] as Record[])[0]['tool_call_id'] - ))).toEqual(['c1', 'c2']); + expect(submitFn).toHaveBeenCalledTimes(1); + const payload = (submitFn as unknown as ReturnType).mock.calls[0][0] as Record; + const messages = payload['messages'] as Record[]; + expect(messages.map((message) => message['tool_call_id'])).toEqual(['c1', 'c2']); }); // ── resolve — error result ────────────────────────────────────────────────── diff --git a/libs/langgraph/src/lib/client-tools.ts b/libs/langgraph/src/lib/client-tools.ts index c90b78190..61e5c99cf 100644 --- a/libs/langgraph/src/lib/client-tools.ts +++ b/libs/langgraph/src/lib/client-tools.ts @@ -86,8 +86,10 @@ export function mergeClientTools( * yet — but ONLY when the run is not in progress (isLoading===false). * The backend ends the run without emitting a ToolMessage result for * client tools, so `result` stays undefined on those entries. - * - resolve(id, result): marks the call as resolved, then issues a NEW - * run on the SAME thread by calling submitFn with: + * - settle(id, result): marks the call as resolved, writes the local result, + * and buffers a ToolMessage without issuing a run. + * - resolve(id, result): settles the result, then issues a NEW run on the SAME + * thread by calling submitFn with the full buffered ToolMessage group: * input: { * messages: [{ type: 'tool', role: 'tool', tool_call_id: id, content }], * client_tools: catalog(), @@ -109,6 +111,7 @@ export function createClientToolsCapability( ): ClientToolsCapability & { catalog: Signal } { const catalog = signal([]); const resolvedIds = signal>(new Set()); + const toolMessageBuffer: Array<{ type: 'tool'; role: 'tool'; tool_call_id: string; content: string }> = []; const pending = computed(() => { // Client tools are only actionable after the run ends (the backend @@ -122,6 +125,41 @@ export function createClientToolsCapability( }); }); + function settleResult(id: string, result: ClientToolResult): void { + // Mark as resolved first so pending() drops it immediately. + resolvedIds.update((s) => new Set(s).add(id)); + + // Cast rather than rely on discriminant narrowing: consumer apps that + // compile this source with `strictNullChecks: false` don't narrow the + // ClientToolResult union in a ternary. + const ok = result.ok; + const value = (result as { value: unknown }).value; + const error = (result as { error: string }).error; + + // Write the outcome onto the LOCAL ToolCall (via the adapter's override + // layer). The client tool DID produce a result client-side, so this is + // semantically correct — and it freezes the transcript card: the mounted + // ask component re-renders with its own emitted value as props and can + // branch to a resolved/frozen state. Without this, the LOCAL tool call + // never gets a result (only the backend ToolMessage does) so the card + // stays interactive forever. + store.applyClientResult(id, { + result: ok ? value : { error }, + ...(ok ? {} : { error, status: 'error' as const }), + }); + + const content = ok + ? safeStringify(value) + : `Error: ${error}`; + + // Message shape: both `type` and `role` are set for compatibility — + // the LangGraph server's add_messages coercion reads `role` (Python + // side), while the bridge's local optimistic-message path reads `type` + // (via toMessage's normalizeMessageType). This mirrors the human-message + // shape used in buildSubmitUpdate (agent.fn.ts line 732). + toolMessageBuffer.push({ type: 'tool', role: 'tool', tool_call_id: id, content }); + } + const capability: ClientToolsCapability & { catalog: Signal } = { catalog, @@ -131,46 +169,20 @@ export function createClientToolsCapability( pending, - resolve(id: string, result: ClientToolResult): void { - // Mark as resolved first so pending() drops it immediately. - resolvedIds.update((s) => new Set(s).add(id)); - - // Cast rather than rely on discriminant narrowing: consumer apps that - // compile this source with `strictNullChecks: false` don't narrow the - // ClientToolResult union in a ternary. - const ok = result.ok; - const value = (result as { value: unknown }).value; - const error = (result as { error: string }).error; - - // Write the outcome onto the LOCAL ToolCall (via the adapter's override - // layer). The client tool DID produce a result client-side, so this is - // semantically correct — and it freezes the transcript card: the mounted - // ask component re-renders with its own emitted value as props and can - // branch to a resolved/frozen state. Without this, the LOCAL tool call - // never gets a result (only the backend ToolMessage does) so the card - // stays interactive forever. - store.applyClientResult(id, { - result: ok ? value : { error }, - ...(ok ? {} : { error, status: 'error' as const }), - }); - - const content = ok - ? safeStringify(value) - : `Error: ${error}`; + settle(id: string, result: ClientToolResult): void { + settleResult(id, result); + }, + resolve(id: string, result: ClientToolResult): void { + settleResult(id, result); // Issue a new run on the same thread. LangGraph's add_messages reducer - // appends the ToolMessage to the thread state. `client_tools` is + // appends the ToolMessages to the thread state. `client_tools` is // included so the model sees the full tool catalog on the continuation. - // - // Message shape: both `type` and `role` are set for compatibility — - // the LangGraph server's add_messages coercion reads `role` (Python - // side), while the bridge's local optimistic-message path reads `type` - // (via toMessage's normalizeMessageType). This mirrors the human-message - // shape used in buildSubmitUpdate (agent.fn.ts line 732). const toolPayload = { - messages: [{ type: 'tool', role: 'tool', tool_call_id: id, content }], + messages: [...toolMessageBuffer], client_tools: catalog(), }; + toolMessageBuffer.length = 0; void submitFn(toolPayload); },