diff --git a/docs/superpowers/context/2026-07-07-client-tools-continuation-handoff.md b/docs/superpowers/context/2026-07-07-client-tools-continuation-handoff.md new file mode 100644 index 000000000..9db8dfc4c --- /dev/null +++ b/docs/superpowers/context/2026-07-07-client-tools-continuation-handoff.md @@ -0,0 +1,174 @@ +# Client-Tool Continuation Hardening — Session Handoff + +**Date:** 2026-07-07 +**Branch:** `blove/recursing-chaplygin-f20d49` (git worktree) +**Design spec (source of truth):** `docs/superpowers/specs/2026-07-07-client-tool-continuation-architecture-design.md` (Revision 4) + +This is a session-continuation handoff. A fresh agent can read this plus the design spec and carry the work forward. The prompt below is self-contained — paste it into a new session. + +--- + +``` +You are continuing a design-and-implementation effort on the Threadplane Angular +agent framework (repo: /Users/blove/repos/angular-agent-framework, working on +branch blove/recursing-chaplygin-f20d49, a git worktree). A prior session +completed the research and design; your job is to carry it forward. Start by +reading the design spec — it is the authoritative source of truth: + + docs/superpowers/specs/2026-07-07-client-tool-continuation-architecture-design.md + (Revision 4) + +=== MISSION === +Threadplane is "the Angular final mile": productized chat/threads/generative-UI +for LangGraph/AG-UI/A2UI-backed enterprise agents. Product strategy: a low-level +protocol/provider layer (Hashbrown) owns AG-UI provider + event primitives + +browser LLM tool execution; Threadplane owns the runtime-neutral `Agent` contract, +chat UX, and client-tool orchestration. The `Agent` contract is the stability +boundary (README.md:152) and the migration seam. NEVER move agent-runtime +orchestration into the low-level layer. + +This effort hardens and extends Threadplane's CLIENT-TOOL CONTINUATION model — +how a client-declared tool call (action/view/ask) is executed in the browser and +the run is continued. The decision (already made, do not relitigate): KEEP the +explicit, protocol-visible continuation (backend ends the run via middleware +`routeAfterAgent -> __end__`; browser detects `pending()` from a signal; browser +`resolve()`s by appending a tool message + starting a new run). Do NOT adopt +hidden client-side recursion — it fights server-authoritative checkpoints. Add +ergonomics (auto-execute, followUp, abort, batching, durable dedup) as opt-in, +additive, default-preserving changes behind the contract. + +=== RESOLVED DECISIONS (do not reopen; full rationale in the spec) === +1. Continuation model stays explicit/protocol-visible/server-authoritative. +2. Suppress-continuation ("followUp:false") = a NEW optional capability method + `settle(id, result)` (record + buffer, no run), NOT a flag on resolve(). + `resolve` = settle + flush the group's buffered ToolMessages in ONE run. +3. Batching = per assistant-turn tool-call group (today N pending function tools + = N racing runs, a latent bug; characterization tests must lock the CURRENT + N-runs behavior, and batching FIXES it later). +4. Crash policy = fail-closed by default + per-tool `idempotent:true` opt-out. +5. Max-turns guard = configurable, default 10, surfaced when hit. +6. Durable dedup = BACKEND-AUTHORITATIVE, not a heavy browser store. A server + `toolCallId` idempotency guard in @threadplane/middleware, shipped with a + Postgres implementation (mirrors the `(tenant_id, event_id)` ON CONFLICT DO + NOTHING pattern from the enterprise persistence platform at + /Users/blove/repos/Intelligence, applied to tool calls). Tiers: Tier 0 = no + store (today) / Tier 1 = server guard (idempotent continuation + reload + reconcile, ships M3) / Tier 2 = claim-before-execute (at-most-once for + non-idempotent browser tools, ships M3.5). Browser marker = fallback only. + Honest limit: true exactly-once of a browser side effect needs the handler to + pass its own idempotency key downstream. +7. Postgres table: nullable `tenant_id` column now, single-tenant PK + (thread_id, tool_call_id) for first ship; tenant_id joins the PK later via an + additive migration behind a config flag. +8. Shared-core extraction (PR 2) = ONLY the pure `pending` predicate + (`selectPendingClientToolCalls`); adapters keep their own resolvedIds signal + and result-application (AG-UI mutates a WritableSignal; LangGraph layers + applyClientResult over a read-only projection). + +=== MILESTONE ROADMAP (spec §10) === +M0 test hardening (scriptable fake AG-UI agent + characterization tests) <- NEXT +M1 extract the pure `pending` predicate (PR 2) +M2 AbortSignal in the handler context +M3 backend durable dedup Tier 1 + Postgres impl in @threadplane/middleware +M3.5 Tier 2 claim-before-execute (latency-validate first) +M4 settle() + batch-per-group (depends on M3 — terminal results need the guard + to be reload-safe; do NOT ship settle before the guard) +M5 max-turns guard + opt-in continuation policy + typed tool-call lifecycle +M6 Hashbrown-provider -> Agent bridge (future) + +=== IMMEDIATE NEXT STEPS === +1. Read the spec fully. +2. Write the M0 implementation plan to + docs/superpowers/plans/2026-07-07-client-tools-m0-test-hardening-plan.md + (repo convention: specs/ = design, plans/ = executable how). Use the + writing-plans skill. The plan's scope is exactly spec §11 ("First PR scope"). +3. Execute PR 1 = M0. Use test-driven / characterization discipline and the + verification-before-completion skill. PR 1 is TESTS + TEST INFRA ONLY: + ZERO production behavior change. + +M0 concretely (see spec §11 for the full checklist): + a. Extend libs/ag-ui/src/lib/testing/fake-agent.ts into a scriptable stream: + script TOOL_CALL_START/ARGS/END (no result), RUN_ERROR, STATE_SNAPSHOT, + CUSTOM/on_interrupt; and MULTI-STEP continuation by branching run() on the + presence of an appended role:'tool' ToolMessage in history (models + backend-ends -> browser resolves -> backend continues). Deterministic + (setTimeout cadence, cancellable; no clocks/Math.random). Default (no + script) behavior identical to today. Align with the existing shared testing + surface (@threadplane/chat/testing FakeAgentConfig; langgraph + provideFakeAgent + FakeStreamTransport) — extend, don't fork. + b. Characterization tests, BOTH adapters (AG-UI + LangGraph parity): single + pending -> 1 run; multiple pending -> N runs (one per resolve, CURRENT + behavior — do NOT assert batching); resolve-one-doesn't-disturb-others; + handler throw -> `Error: ` tool message + run still issued; arg-validation + fail -> `Error: invalid arguments: ...`, handler not called, run still issued; + view auto-ack; ask via render event; pending empty while isLoading; + server-resolved call excluded. + c. Chat-component integration test driving action/view/ask through the + coordinator using the scriptable fake (not just the hand-built FakeCap). + Then STOP for review before starting M1. + +=== HARD CONSTRAINTS (apply to all work) === +- Preserve the runtime-neutral `Agent` contract as the stability boundary. All + new behavior is additive and opt-in; defaults reproduce today's behavior. +- PR 1: zero runtime diff. `git diff` should touch only *.spec.ts and testing/ + scaffolding. Do NOT modify client-tools.ts resolve()/pending(), the executor, + execute.ts, the coordinator, or adapter runtime code. +- No new dependencies without explicit approval. The Postgres driver (M3) is a + dependency of @threadplane/middleware ONLY (a Node/backend package) — NEVER of + the browser libs. +- No new public exports without running `npm run generate-api-docs` and committing. + Prefer keeping M0 test scaffolding internal to spec/test files. +- Patch-only releases: never bump @threadplane/* to 0.1.0; increment patch. +- Deterministic, local tests only (no network, no live LLM in unit tests). +- NEVER reference external frameworks (hashbrown / copilotkit / chatgpt / claude) + in code, comments, or commit/PR text. Spec/plan markdown is the only sanctioned + place. The architecture is independently arrived at. +- Do not relitigate the resolved decisions above. + +=== REPO / ENVIRONMENT GOTCHAS === +- Fresh worktree: postinstall may be skipped. If unit tests can't resolve deps, + run the generate-public-key script and copy missing node_modules packages + (e.g. katex) from the main checkout before `npx nx test chat`. +- Lint gate: CI tolerates warnings but fails on ERRORS. Check with + ` | grep -cE ' error '`, not the exit code. +- Only Vercel is a required check on main; lint/test/build can land broken and + only break the release path. Verify tests locally; don't trust CI gating. +- Test targets: `npx nx test ag-ui`, `npx nx test langgraph`, `npx nx test chat`. +- Kill orphaned dev servers on :4200/:2024 before any e2e (not needed for unit). + +=== KEY SOURCE REFERENCES === +Runtime (read; modify only per the milestone you're on): + libs/chat/src/lib/agent/agent.ts (Agent contract) + libs/chat/src/lib/client-tools/client-tools-capability.ts (pending/resolve iface) + libs/chat/src/lib/client-tools/client-tool-executor.ts (13-34, auto-run) + libs/chat/src/lib/client-tools/execute.ts (executeFunctionTool 22-34) + libs/chat/src/lib/client-tools/client-tools-coordinator.ts + libs/chat/src/lib/client-tools/tools.ts (action/view/ask authoring) + libs/ag-ui/src/lib/client-tools.ts (pending 63-72, resolve 74-115) + libs/ag-ui/src/lib/reducer.ts (TOOL_CALL_* 186/213/230/247, + RUN_FINISHED 111) + libs/ag-ui/src/lib/testing/fake-agent.ts (FakeAgent.run ~47) + libs/langgraph/src/lib/client-tools.ts (pending 108-118, + resolve 129-171, + mergeClientTools 60-68) + libs/middleware/src/langgraph/middleware.ts (routeAfterAgent 90-98) +Existing specs to extend: + libs/ag-ui/src/lib/client-tools.spec.ts + libs/langgraph/src/lib/client-tools.spec.ts + libs/chat/src/lib/compositions/chat/chat.component.client-tools.spec.ts +Prior design context (specs/): + 2026-06-08-client-tools-design.md (the current client-tools design) + 2026-04-21-chat-runtime-decoupling-design.md (why the neutral contract exists) + 2026-04-25-events-on-agent-contract-design.md (state-on-signals invariant) +Prior-art reference (external, for §7 only): + /Users/blove/repos/Intelligence (@cpki/source) — Postgres event log + + (tenant_id,event_id) ON CONFLICT DO NOTHING; built persistence but NOT the + per-toolCallId execution guard (the layer M3 adds). + +=== WORKING STYLE === +Use skills: writing-plans (before the M0 plan), test-driven-development and +verification-before-completion (during M0), requesting-code-review before +declaring PR 1 done. Package versions: @threadplane/* at 0.0.55, middleware 0.0.2. +Report which files changed and paste passing test output before claiming done. +Stop after PR 1 for human review; do not proceed to M1 unprompted. +``` diff --git a/docs/superpowers/plans/2026-07-07-client-tools-m0-test-hardening-plan.md b/docs/superpowers/plans/2026-07-07-client-tools-m0-test-hardening-plan.md new file mode 100644 index 000000000..5b9ee16c4 --- /dev/null +++ b/docs/superpowers/plans/2026-07-07-client-tools-m0-test-hardening-plan.md @@ -0,0 +1,212 @@ +# Client Tools M0 Test Hardening Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add deterministic client-tool continuation test infrastructure and characterization coverage for today's Threadplane behavior without changing production runtime code. + +**Architecture:** Extend the existing AG-UI `FakeAgent` test double into a deterministic scriptable stream while keeping its default canned-token behavior unchanged. Characterization tests stay at the adapter capability and chat coordinator boundaries so later PRs can refactor or change behavior with red tests that prove the current contract. + +**Tech Stack:** Angular signals, Vitest, RxJS, Nx (`npx nx test ag-ui`, `npx nx test langgraph`, `npx nx test chat`), existing Threadplane testing helpers. + +--- + +## Scope Guard + +PR 1 is M0 only. Allowed changes are `*.spec.ts` files and testing scaffolding under existing `testing/` surfaces. Do not edit runtime client-tool code such as `libs/chat/src/lib/client-tools/client-tools-coordinator.ts`, `libs/chat/src/lib/client-tools/client-tool-executor.ts`, `libs/chat/src/lib/client-tools/execute.ts`, `libs/ag-ui/src/lib/client-tools.ts`, or `libs/langgraph/src/lib/client-tools.ts`. + +Characterize current behavior, including the known multi-tool behavior: multiple pending function tools currently issue one continuation run per `resolve()`. + +## File Structure + +- Modify: `libs/ag-ui/src/lib/testing/fake-agent.ts` + - Add test-only script types and scripted event emission to the existing fake. + - Preserve the no-script constructor path byte-for-byte in observable behavior: token stream, reasoning stream, cadence, cancellation, and public `FakeAgent` export remain compatible. + - Add deterministic message ids for scripted events; avoid clocks and randomness when a script is supplied. +- Modify: `libs/ag-ui/src/lib/testing/fake-agent.spec.ts` + - Add red/green tests for scripted `TOOL_CALL_START` / `TOOL_CALL_ARGS` / `TOOL_CALL_END`, `RUN_ERROR`, `STATE_SNAPSHOT`, `CUSTOM` `on_interrupt`, continuation branching on appended `role: 'tool'` history, and cancellation. +- Modify: `libs/ag-ui/src/lib/client-tools.spec.ts` + - Add missing characterization cases for current adapter behavior: multiple pending -> N runs, resolving one leaves others pending, pending excludes server-resolved calls, and existing error/run behavior remains covered. +- Modify: `libs/langgraph/src/lib/client-tools.spec.ts` + - Mirror the AG-UI characterization cases for LangGraph: multiple pending -> N submits, resolving one leaves others pending, pending excludes server-resolved calls, and existing error/run behavior remains covered. +- Modify: `libs/chat/src/lib/compositions/chat/chat.component.client-tools.spec.ts` + - Keep the existing hand-built capability tests as focused unit coverage. + - Add one integration path using the scriptable fake-derived capability and the real coordinator to drive action, view auto-ack, and ask render-result resolution. + +## Task 1: Scriptable AG-UI FakeAgent + +**Files:** +- Modify: `libs/ag-ui/src/lib/testing/fake-agent.ts` +- Test: `libs/ag-ui/src/lib/testing/fake-agent.spec.ts` + +- [x] **Step 1: Write failing tests for scripted tool-call emission** + +Add tests that construct: + +```ts +const agent = new FakeAgent({ + delayMs: 0, + script: [{ + when: 'initial', + events: [ + { type: 'TOOL_CALL_START', toolCallId: 'tool-1', toolCallName: 'get_weather', parentMessageId: 'assistant-1' }, + { type: 'TOOL_CALL_ARGS', toolCallId: 'tool-1', delta: '{"city":"SF"}' }, + { type: 'TOOL_CALL_END', toolCallId: 'tool-1' }, + ], + }], +}); +``` + +Assert the collected event types include `RUN_STARTED`, the three tool-call events, and `RUN_FINISHED` in order, with the input `threadId` and `runId` copied into run lifecycle events. + +- [x] **Step 2: Run the red test** + +Run: `npx nx test ag-ui --runInBand --testFile=libs/ag-ui/src/lib/testing/fake-agent.spec.ts` + +Expected: fail because `script` is not supported by `FakeAgent`. + +- [x] **Step 3: Implement the minimal script runner** + +Extend `FakeAgent` constructor options with an optional `script` array. Add internal helpers that choose the matching script branch, wrap it with `RUN_STARTED` / `RUN_FINISHED`, and emit with the existing cancellable `setTimeout` cadence. Keep no-script behavior unchanged. + +- [x] **Step 4: Run the green test** + +Run: `npx nx test ag-ui --runInBand --testFile=libs/ag-ui/src/lib/testing/fake-agent.spec.ts` + +Expected: pass. + +- [x] **Step 5: Add red tests for continuation branching and non-tool events** + +Add tests for: + +```ts +const agent = new FakeAgent({ + delayMs: 0, + script: [ + { when: 'initial', events: [{ type: 'TOOL_CALL_START', toolCallId: 'tool-1', toolCallName: 'get_weather' }, { type: 'TOOL_CALL_END', toolCallId: 'tool-1' }] }, + { when: { toolMessageFor: 'tool-1' }, events: [{ type: 'TEXT_MESSAGE_CONTENT', delta: 'continued' }] }, + ], +}); +``` + +Also cover `RUN_ERROR`, `STATE_SNAPSHOT`, and `CUSTOM` with `name: 'on_interrupt'`. + +- [x] **Step 6: Implement only the missing fake-agent script shapes** + +Add conversion for the script event shapes to AG-UI `BaseEvent` objects. Do not add new production exports or new dependencies. + +- [x] **Step 7: Re-run fake-agent tests** + +Run: `npx nx test ag-ui --runInBand --testFile=libs/ag-ui/src/lib/testing/fake-agent.spec.ts` + +Expected: pass. + +## Task 2: Adapter Characterization Tests + +**Files:** +- Modify: `libs/ag-ui/src/lib/client-tools.spec.ts` +- Modify: `libs/langgraph/src/lib/client-tools.spec.ts` + +- [x] **Step 1: Add AG-UI red tests for current multiple-resolve behavior** + +Use `createClientToolsCapability()` with two catalog-matching pending tool calls. Resolve both calls separately and assert `source.runAgent` is called twice. This intentionally locks current behavior, not planned batching. + +- [x] **Step 2: Run the AG-UI red/characterization check** + +Run: `npx nx test ag-ui --runInBand --testFile=libs/ag-ui/src/lib/client-tools.spec.ts` + +Expected: if the current behavior is already present, the test may pass immediately; if it fails, inspect whether the assertion is incorrect before changing test-only code. Do not change runtime code. + +- [x] **Step 3: Add missing AG-UI characterization tests** + +Add or refine tests for resolving one call leaving the other pending, `pending()` returning empty while loading, and server-resolved calls being excluded by `result !== undefined`. + +- [x] **Step 4: Mirror tests in LangGraph** + +Use `createClientToolsCapability()` with `makeStore()` and `makeSubmitFn()`. Resolve two pending calls and assert two `submitFn` calls. Assert one resolved call leaves the other pending and server-resolved calls are excluded. + +- [x] **Step 5: Run adapter tests** + +Run: + +```bash +npx nx test ag-ui --runInBand --testFile=libs/ag-ui/src/lib/client-tools.spec.ts +npx nx test langgraph --runInBand --testFile=libs/langgraph/src/lib/client-tools.spec.ts +``` + +Expected: pass. + +## Task 3: Chat Component Coordinator Integration + +**Files:** +- Modify: `libs/chat/src/lib/compositions/chat/chat.component.client-tools.spec.ts` + +- [x] **Step 1: Write a failing integration test using a fake-agent-backed capability** + +Build a local test harness in the spec that exposes `pending`, `setCatalog`, and `resolve`, and drives pending calls from a script-like sequence. Connect it to `ChatComponent` with the real `clientToolRegistry`. + +The test should: + +1. Install registry with an `action`, a `view`, and an `ask`. +2. Emit a pending action tool and assert the coordinator resolves it with the handler result. +3. Emit a pending view tool and assert the coordinator auto-acks with `{ shown: true }`. +4. Emit a pending ask tool, call `onClientToolEvent({ type: 'result', elementKey: askName, value })`, and assert the coordinator resolves the ask. + +- [x] **Step 2: Run the chat red/characterization check** + +Run: `npx nx test chat --runInBand --testFile=libs/chat/src/lib/compositions/chat/chat.component.client-tools.spec.ts` + +Expected: fail until the harness is wired correctly; no production changes are allowed. + +- [x] **Step 3: Implement only the spec harness needed for the integration** + +Add local spec helpers only. Do not move helpers to public API unless explicitly approved. + +- [x] **Step 4: Run the chat integration test** + +Run: `npx nx test chat --runInBand --testFile=libs/chat/src/lib/compositions/chat/chat.component.client-tools.spec.ts` + +Expected: pass. + +## Task 4: Full Verification and Diff Audit + +**Files:** +- No new files beyond this plan unless a test-only helper is required. + +- [x] **Step 1: Run project-scoped test targets** + +Run: + +```bash +npx nx test ag-ui +npx nx test langgraph +npx nx test chat +``` + +Expected: all pass. + +- [x] **Step 2: Run relevant lint checks and count errors** + +Run: + +```bash +npx nx lint ag-ui 2>&1 | tee /tmp/threadplane-ag-ui-lint.log; grep -cE ' error ' /tmp/threadplane-ag-ui-lint.log +npx nx lint langgraph 2>&1 | tee /tmp/threadplane-langgraph-lint.log; grep -cE ' error ' /tmp/threadplane-langgraph-lint.log +npx nx lint chat 2>&1 | tee /tmp/threadplane-chat-lint.log; grep -cE ' error ' /tmp/threadplane-chat-lint.log +``` + +Expected: each grep count is `0`. + +- [x] **Step 3: Audit diff scope** + +Run: + +```bash +git diff --name-only +git diff --stat +``` + +Expected: changed files are limited to `*.spec.ts`, testing scaffolding, and this plan markdown. No runtime client-tool implementation files are changed. + +- [x] **Step 4: Stop for human review** + +Report changed files, verification output, and any commands that could not run. Do not start M1. diff --git a/docs/superpowers/specs/2026-07-07-client-tool-continuation-architecture-design.md b/docs/superpowers/specs/2026-07-07-client-tool-continuation-architecture-design.md new file mode 100644 index 000000000..05946e873 --- /dev/null +++ b/docs/superpowers/specs/2026-07-07-client-tool-continuation-architecture-design.md @@ -0,0 +1,331 @@ +# Client-Tool Continuation Architecture — Explicit vs Recursive Follow-Up — Design + +**Date:** 2026-07-07 +**Status:** Research + recommendation (no code changes yet). **Revision 4** — settles the last schema/API-shaping items: Tier 1 (server idempotency guard) ships in M3, Tier 2 (claim-before-execute) splits to M3.5; the Postgres table ships a nullable `tenant_id` column with a single-tenant PK now (`tenant_id` joins the PK later via additive migration). Builds on Revision 3, which folded in: `settle()` for suppress-continuation, batch-per-tool-call-group, fail-closed crash policy with per-tool idempotent opt-out, max-turns default 10, and the **backend-authoritative durable-dedup model** (server-side `toolCallId` idempotency guard in `@threadplane/middleware`, shipped with a Postgres implementation; browser marker demoted to a fallback). +**Scope:** Decide whether Threadplane's client-tool continuation model should keep its explicit, protocol-visible contract, adopt a hidden recursive follow-up loop (as seen in comparable browser-agent frameworks), or move to a hybrid. Covers the runtime-neutral `Agent` contract, the `ClientToolsCapability`, both adapters (`@threadplane/ag-ui`, `@threadplane/langgraph`), the LangGraph middleware, and the Hashbrown↔Threadplane layering strategy. + +> **Naming note:** This spec discusses external frameworks by name for comparison (a React browser-agent framework, a low-level Angular protocol layer, and an enterprise agent-persistence platform whose Postgres event-log design is cited as prior art). Per the repo rule, those names must **never** appear in shipped code, comments, or commit/PR text — the architecture here is independently arrived at. Spec/plan markdown is the only sanctioned place to name them. + +--- + +## 1. TL;DR recommendation + +**Keep the explicit, protocol-visible continuation contract. Harden it with tests first. Introduce every behavior-changing knob through an explicitly defined contract change — never a silent default. Make the backend the durable authority; don't build a heavy client-side store.** + +1. **Preserve the primitive as-is for now.** `ClientToolsCapability.pending()` and `resolve(id, result)` keep their current signatures and behavior through the test-hardening phase. The backend continues to *end the run* on a client-only tool call (`routeAfterAgent → __end__`); the browser detects the pending call from a signal and resumes with an appended tool message + a new run. +2. **Add ergonomics as clearly-scoped later PRs, each with a defined contract:** an optional `settle()` capability method (suppress-continuation, §5a), batched continuation per tool-call group (§5d), an `AbortSignal` in the handler context (§5b), a max-turns guard (§5e), and durable dedup as a **backend-authoritative** model (§7). None ship in PR 1. +3. **Model recursive auto-continuation as a coordinator *policy*, not hidden adapter behavior.** Adapters stay thin. "Keep re-running until the model stops calling tools" becomes an explicit, inspectable, opt-in strategy. +4. **Durable dedup lives in the backend, not the browser.** The backend event log / LangGraph checkpoint already dedups the happy path. Exactly-once is enforced by a **server-side `toolCallId` idempotency guard in `@threadplane/middleware`, shipped with a Postgres implementation** (the same `ON CONFLICT DO NOTHING` pattern proven in the enterprise persistence platform, applied to tool calls). A minimal browser marker is only a fallback for backends without the guard. +5. **Ownership split:** the low-level protocol/provider/event layer (Hashbrown) owns AG-UI provider + event primitives + browser LLM tool execution; Threadplane owns the runtime-neutral `Agent` contract, chat UX, and the client-tool orchestration/continuation policy. The `Agent` contract is the migration seam. + +Why not full hidden recursion as a default: against a stateful, server-authoritative backend a client that silently splices tool results into the authoritative message array and re-runs fights the backend's checkpoint model, risks double-execution on reload (§7), has no natural max-turns cap, and reverses the deliberate `2026-06-08-client-tools-design` decision to reserve `interrupt()` for approvals. + +--- + +## 2. Behavior status legend + +Every behavior is tagged: **[TODAY]** implemented now · **[CHARACTERIZE]** PR 1 tests lock current behavior, no code change · **[CHANGE]** a later PR changes behavior/contract · **[FUTURE]** optional, needs its own design. + +### 2a. Master table — current vs proposed + +| Behavior | Status | Where | +| --- | --- | --- | +| `pending` computed signal gated on `!isLoading` + catalog name + `result===undefined` + `!resolvedIds` | **[TODAY]** | `ag-ui/client-tools.ts:63-72`, `langgraph/client-tools.ts:108-118` (identical predicate) | +| `resolve(id, result)` writes local result, appends `role:'tool'` message, **always starts one run** | **[TODAY]** | `ag-ui/client-tools.ts:74-115`, `langgraph/client-tools.ts:129-171` | +| Function tools auto-execute; coordinator calls `resolve` per tool | **[TODAY]** | `client-tool-executor.ts:17-33` | +| Each `resolve()` starts its own run → N pending function tools = **N runs** | **[TODAY]** | executor loop + per-resolve `runAgent`/`submitFn` | +| Handler throw → `Error: …` tool message; run still issued | **[TODAY]** | `execute.ts:28-33` | +| Argument validation fail → `Error: invalid arguments: …`; handler not called; run still issued | **[TODAY]** | `execute.ts:22-27` | +| In-memory dedup (`inFlight` + `resolvedIds` + `result===undefined` backstop) | **[TODAY]** | `client-tool-executor.ts:16`, adapter `resolvedIds` | +| Backend ends run on client-only tool call (`routeAfterAgent → __end__`) | **[TODAY]** | `middleware/src/langgraph/middleware.ts:90-98` | +| Characterization tests, both adapters + chat component; scriptable fake AG-UI agent | **[CHARACTERIZE]** | PR 1 (§9) | +| Extract the pure `pending` predicate into a shared chat helper | **[CHANGE — refactor]** | PR 2 (§6) | +| `settle(id, result)` optional capability method (record + buffer, no run) | **[CHANGE — contract]** | §5a | +| Batched continuation per tool-call group (N settles → 1 run) | **[CHANGE]** | §5d | +| `AbortSignal` in the function-tool handler context | **[CHANGE]** | §5b | +| Max-turns guard (default 10, configurable, surfaced) | **[CHANGE]** | §5e | +| Backend `toolCallId` idempotency guard in `@threadplane/middleware` (Postgres impl shipped) | **[CHANGE]** | §7 | +| Claim-before-execute for non-idempotent browser tools (fail-closed; `idempotent:true` opts out) | **[CHANGE]** | §7 | +| Minimal browser fail-closed marker (fallback when no server guard) | **[CHANGE]** | §7 | +| Recursive auto-continuation as an opt-in coordinator policy | **[FUTURE]** | later PR | +| Hashbrown-provider → `Agent` bridge | **[FUTURE]** | §8, M6 | + +--- + +## 3. Where the two models actually differ + +Both approaches ultimately "append a `role:'tool'` message → run again." The differences are **who drives the loop**, **whether it is protocol-visible**, and **who is authoritative over history**. + +### 3a. Threadplane today — explicit, protocol-visible, server-authoritative **[TODAY]** + +- **Backend** (`middleware.ts`): binds the client catalog so the model *can* call client tools, but `routeAfterAgent` routes a client-only call to `__end__` — the run ends with the tool call **unresolved and no `ToolMessage`**. +- **Adapter** (`client-tools.ts`): `pending` = `!isLoading && catalogName(tc) && tc.result===undefined && !resolvedIds.has(tc.id)`. `resolve(id, result)` marks resolved, writes the local `ToolCall` (freezes the card), appends a `role:'tool'` message, and **starts one new run**. +- **Chat coordinator**: decides *when* to resolve — the executor auto-runs `action()` tools; `view()` auto-acks; `ask()` resolves from the mounted component's value. + +The multi-step loop is **emergent**; the backend stays authoritative over persisted history and checkpoints. + +### 3b. The comparable recursive model — hidden, client-authoritative + +Mutual recursion (`runAgent` ⇄ `processAgentResult`), no max-turns cap; scans returned messages, splices results into the authoritative array, recurses unless aborted or `followUp:false`. No backend cooperation needed. Optimized for browser-running agents where the client can be authoritative. + +### 3c. The decisive difference + +| Dimension | Explicit (Threadplane) | Hidden recursive | +| --- | --- | --- | +| Loop driver | Backend ends → signal → coordinator resolves → new run | Runtime recurses client-side | +| Protocol visibility | Visible: the graph genuinely pauses | Invisible: client patches history + re-runs | +| Authoritative history | **Server / checkpoints** | **Client message array** | +| Backend requirement | Must route client calls to `__end__` | None | +| Termination | Emergent (no more pending) | Implicit; no max-turns | +| Fit | Stateful enterprise agents, HITL approvals, audit | Browser agents, dumb backends | + +Threadplane's target (`README.md`/`gtm.md`: "the Angular final mile" for "LangGraph/AG-UI/A2UI-backed enterprise agent workflows") is where server-authoritative, protocol-visible continuation is the correct default. + +--- + +## 4. Answers to the five questions + +**Q1 — Keep the explicit contract? → Yes.** Server-authoritative, checkpoint-safe, inspectable, composes with `interrupt()` approvals and time-travel; the advertised stability boundary (`README.md:152`); honors the `state on signals, events on events$` invariant. + +**Q2 — Hidden recursive follow-up? → No, not as adapter behavior.** Fights server checkpoints, risks reload double-execution, no max-turns, reverses `2026-06-08`. Right for browser agents (the low-level layer's domain), wrong as a Threadplane-wide default. + +**Q3 — Hybrid? → Yes.** Keep the protocol-visible primitive; add the recursive model's *ergonomics* as opt-in coordinator policy over an explicitly-extended contract. + +**Q4 — Where? → `@threadplane/chat` coordinator** for orchestration/policy; **`@threadplane/middleware`** for the server-side idempotency guard (§7); adapters stay thin. A narrow shared helper (§6) absorbs only the pure predicate. + +**Q5 — Hashbrown vs Threadplane.** Hashbrown owns provider/event/protocol primitives + browser LLM tool execution; Threadplane owns the `Agent` contract, chat UX, and continuation policy. Seam = the `Agent` contract. + +--- + +## 5. The behavior-changing knobs, defined + +All are **[CHANGE]** — none in PR 1. + +### 5a. `settle()` — suppress-continuation **[CHANGE — contract]** *(decided)* + +**Problem.** Both adapters' `resolve()` always call `runAgent`/`submitFn` (`ag-ui:114`, `langgraph:170`). A terminal tool (`followUp:false`) cannot avoid a continuation run without a contract change. + +**Decision — a separate optional method** (`settle`), chosen over an optional flag on `resolve` so terminal-ness is explicit at the call site and a capability that doesn't implement it fails loudly rather than silently continuing: + +```ts +interface ClientToolsCapability { + setCatalog(specs: readonly ClientToolSpec[]): void; + readonly pending: Signal; + /** Record the result + append the ToolMessage to the group buffer AND flush + * the buffer in one continuation run. (Unchanged for single-tool callers.) */ + resolve(id: string, result: ClientToolResult): void; + /** Record the result + append the ToolMessage to the group buffer, but do + * NOT start a run. Optional: capabilities that omit it fall back to today's + * behavior (the coordinator warns when it cannot honor followUp:false). */ + settle?(id: string, result: ClientToolResult): void; +} +``` + +**Semantics.** `settle` = record local result (freeze card) + buffer the ToolMessage; no run. `resolve` = `settle` + flush the whole buffered group in one run. For a single-tool group with an empty prior buffer, `resolve` is byte-for-byte today's behavior — so existing tests and existing consumer capabilities are unaffected. `settle` is **optional** on the interface; a capability lacking it can't suppress a run, and the coordinator emits a dev warning when a `followUp:false` tool can't be honored. + +**Transport caveat (documented, narrowed).** With today's transports a tool result reaches the backend *only* by issuing a run (AG-UI's `addMessage` ships on the next `runAgent`; LangGraph's ToolMessage rides inside the `submitFn` payload). So a **fully-terminal group** (every tool `followUp:false` → all `settle`, no `resolve`) records results **client-side only**; they are not persisted to the backend thread. A **mixed group** (any tool wants follow-up) still issues one run, so the terminal tools' results *do* get persisted along the way. Fully-terminal results are made reload-safe by §7 (they won't re-execute), so this caveat is bounded. + +### 5b. `AbortSignal` in the handler context **[CHANGE]** + +Today `executeFunctionTool(def, tc.args)` passes args only (`execute.ts:29`); `stop()` can't cancel an in-flight client tool. Thread an `AbortSignal` (owned by the executor, aborted on `stop()`/coordinator swap/registry change) into a second handler argument — `handler(args, { signal })`. Additive. An aborted handler must **not** `resolve`/`settle` and must **not** start a continuation run. + +### 5c. Durable duplicate-execution prevention **[CHANGE]** — see §7 + +The full model. In-memory `inFlight`/`resolvedIds` are per-instance and lost on reload; §7 defines the backend-authoritative replacement. + +### 5d. Batched continuation per tool-call group **[CHANGE]** *(decided)* + +**Correction to Revision 1.** Today the executor calls `resolve` per settled tool, and each `resolve` starts its own run — so **N simultaneously-pending function tools produce N runs on the same thread** (`client-tool-executor.ts:28-31`). The `2026-06-08` "batch into a single re-run" was never implemented. + +**Decision — batch per assistant-turn tool-call group.** The coordinator tracks the group of client tool calls emitted by one assistant turn; as each settles it calls `settle(id, result)`; when the group is complete it calls `resolve(lastId, lastResult)` to flush all buffered ToolMessages in **one** run. This matches the LLM contract (a turn's tool calls must all be answered before the model responds) and fixes the racing-runs bug. A slow/hung handler blocks its group — which is correct; `stop()`/abort (§5b) handles the hung case. A **fully-terminal** group flushes via `settle` on every tool and issues no run. + +### 5e. Max-turns guard **[CHANGE]** *(decided)* + +**Decision — configurable, default 10 consecutive client-tool continuation rounds.** Legit multi-step flows are 1–3 rounds; 10 catches runaway loops without tripping real usage. When hit: stop continuing, surface a diagnostic (log + an error state on the run), never silently swallow. Lives in the coordinator policy. + +--- + +## 6. Shared-core extraction — narrow scope **[CHANGE — refactor]** + +Only the predicate is genuinely shared. Extract exactly this pure function into `@threadplane/chat`: + +```ts +export function selectPendingClientToolCalls(input: { + isLoading: boolean; + toolCalls: readonly ToolCall[]; + catalogNames: ReadonlySet; + resolvedIds: ReadonlySet; +}): readonly ToolCall[]; +``` + +**Do not** move into chat: the `resolvedIds` *signal* (each adapter owns its own), result application (AG-UI mutates a `WritableSignal` at `ag-ui:88-98`; LangGraph layers `applyClientResult` over a read-only projection at `langgraph:147-150`), or the transport-specific append+rerun. Behavior-preserving; **PR 2**. + +--- + +## 7. Durable duplicate-execution — backend-authoritative design **[CHANGE]** + +The prior draft proposed a full client-side result **ledger**. Investigation of the enterprise persistence platform (`@cpki/source`) showed the stronger, proven pattern: a **server-authoritative Postgres event log with `(tenant_id, event_id)` `ON CONFLICT DO NOTHING` idempotency**, Redis as a non-authoritative hot cache, and reconnect rehydrating a `MESSAGES_SNAPSHOT` from Postgres. That platform built *thread persistence + event idempotency* but **not** a per-tool-call execution guard — which is exactly the missing top layer this section specifies, applying its `event_id` idempotency pattern to `toolCallId`. + +### 7a. Principle: the backend is the durable authority; don't rebuild the log + +LangGraph *is* a checkpointed event log; an AG-UI enterprise backend *is* that Postgres log. Threadplane must not rebuild it. Durable dedup is layered on top of it, server-side. + +### 7b. The honest guarantee boundary + +A server-side guard **cannot** make a browser handler's side effect exactly-once by itself — the handler runs in the browser before the server sees any result, so a guard on inbound results can't undo a duplicate charge. True exactly-once of a non-idempotent browser effect requires either (a) the handler passing its own idempotency key downstream, or (b) **claim-before-execute** (the client durably claims *before* running the handler). We therefore ship tiers, each explicit about what it guarantees and what it costs. + +### 7c. Guarantee tiers + +- **Tier 0 — no store (default; today's behavior).** In-memory `inFlight`/`resolvedIds` + the backend happy path: a persisted `TOOL_CALL_RESULT`, rehydrated on reload, sets `result` so `pending` never resurfaces the call. Sufficient for idempotent / non-critical tools (`view`, navigation, UI updates). No new machinery. +- **Tier 1 — server `toolCallId` idempotency guard (Postgres, shipped in M3).** When a continuation run delivers a ToolMessage for `toolCallId X`, the middleware records `(threadId, X)` with `ON CONFLICT DO NOTHING`. This makes **continuation idempotent** — a re-delivered result never double-processes downstream server logic — and provides an **authoritative "already executed" record** the client reads on reload to avoid re-surfacing `X`. Closes the happy-path and re-delivery cases exactly-once *at the server*. Does **not** by itself stop a browser re-execution in the crash window. +- **Tier 2 — claim-before-execute (for non-idempotent browser effects).** Before running a guarded handler the client durably claims `(threadId, X)='executing'` via the Tier-1 guard, runs the handler, then records the result. On reload: record `done` → skip + (optionally) re-deliver; record `executing` with no result (crash mid-handler) → **fail-closed** (surface an interrupted-error tool result); no record → run. Bounds browser re-execution to **at-most-once**. Costs one durable round trip before execution — so it applies only to tools that need it (see policy). + +**Honest limit.** Even Tier 2 is at-most-once, not exactly-once, for the *downstream* effect; genuine exactly-once needs the handler to pass an idempotency key to its own service. The framework gives at-most-once (fail-closed) or effectively-once *delivery* of an already-computed result — and documents this plainly. + +### 7d. Crash policy — fail-closed + per-tool idempotent opt-out *(decided)* + +Default for guarded tools: on reload, an `executing` record with no result → **fail-closed** (interrupted-error result, do not re-run) — safe for non-idempotent side effects. A tool declared `idempotent: true` opts out and may re-execute (at-least-once). At-most-once by default; at-least-once when the author opts in. + +### 7e. Config layering (reconciles fail-closed-default with the round-trip cost) + +- **No store wired → Tier 0.** Exactly today's behavior. Default for OSS / simple apps. +- **Postgres guard wired (enterprise) → Tier 1 on.** Continuation is idempotent; reload reconciles against the guard. Within this mode, a tool is **guarded/fail-closed by default** and `idempotent: true` opts a tool out of the Tier-2 claim path (it simply re-runs, no round trip). So the pre-execution round trip is paid only for non-idempotent tools in guard-enabled apps — never for the common idempotent case, and never for apps that don't opt in. + +### 7f. Where it lives, and the Postgres shipment + +- **Server guard + Postgres implementation:** `@threadplane/middleware` (where `routeAfterAgent` already lives). Ship a Postgres-backed `ClientToolExecutionStore` out of the box. +- **Injectable interface:** `ClientToolExecutionStore` with an **in-memory** implementation (default, Tier 0/1 without persistence) and the **Postgres** implementation (Tier 1/2). Consumers can supply their own (e.g., an existing enterprise Postgres log). +- **Browser marker (demoted to fallback):** a minimal, fail-closed, per-`toolCallId` marker for backends that expose no server guard. Non-authoritative; smallest possible; behind the same injectable boundary. This supersedes Revision 2's "ship sessionStorage as the blessed provider" — sessionStorage is now only one possible browser-fallback impl, not the primary path. + +Proposed schema (first ship: nullable `tenant_id` column, single-tenant PK; mirrors the enterprise `event_id` pattern applied to tool calls): + +```sql +CREATE TABLE threadplane_client_tool_executions ( + tenant_id text, -- nullable now; joins the PK later (see below) + thread_id text NOT NULL, + tool_call_id text NOT NULL, + status text NOT NULL, -- 'executing' | 'done' | 'failed' + result jsonb, -- serialized ClientToolResult when 'done' + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now(), + PRIMARY KEY (thread_id, tool_call_id) +); +-- claim: INSERT ... ON CONFLICT (thread_id, tool_call_id) DO NOTHING RETURNING *; +-- inserted row → 'claimed'; conflict → read existing row's status/result +-- record: UPDATE ... SET status='done', result=$, updated_at=now() WHERE (thread_id, tool_call_id)=($,$) +``` + +**Tenant scoping (settled).** Ship the nullable `tenant_id` column from day one so the table is never rewritten. Keep the PK `(thread_id, tool_call_id)` for the first ship (single-tenant-safe because `thread_id` is globally unique). When a multi-tenant consumer needs isolation, an **additive migration** behind a config flag promotes the PK to `(tenant_id, thread_id, tool_call_id)` and threads `tenant_id` through `claim`/`record`/`lookup`. No first-ship multi-tenancy work. + +Interface sketch: + +```ts +export interface ClientToolExecutionStore { + /** Atomically claim a tool-call execution. Returns the prior state if any. */ + claim(key: { threadId: string; toolCallId: string }): + Promise<'claimed' | { status: 'executing' } | { status: 'done'; result: ClientToolResult }>; + record(key: { threadId: string; toolCallId: string }, result: ClientToolResult): Promise; + /** Reload reconciliation: statuses/results for a thread's pending tool calls. */ + lookup(threadId: string, toolCallIds: readonly string[]): + Promise>; +} +``` + +--- + +## 8. Migration story: Hashbrown → Threadplane + +Developers start on the low-level layer (browser agents, LLM-driven frontend tools) and **migrate into Threadplane** for enterprise/production chat. Additive, gated by the `Agent` contract. + +1. **Provider stays** — keeps producing AG-UI events/provider primitives. +2. **Bridge to the contract** — a thin, independently-authored adapter turns the provider's events into `Agent` (mirroring `to-agent.ts`). +3. **Adopt the chat surface** — swap bespoke UI for `@threadplane/chat` compositions. +4. **Frontend tools port 1:1** — browser action tools → `action()`; render-only → `view()`; HITL → `ask()`. The coordinator policy supplies auto-execute + (opt-in) auto-continue — server-authoritative when the backend is a stateful graph, client-driven only where the provider is genuinely client-authoritative. +5. **Escalate to enterprise backends without rewriting UI** — chat consumes `Agent`, so swapping the provider requires no template changes (`README.md:152`). + +**Protecting existing `@threadplane/ag-ui` / `@threadplane/langgraph` users** (hard constraint): +- Through PR 1 and PR 2: `pending()`/`resolve()`/`action()`/`view()`/`ask()` unchanged in signature and behavior. +- Later [CHANGE] PRs are additive and default-preserving: `settle?` is optional; the handler `signal` is an optional 2nd arg; the durable guard is off unless a store is wired; batching/max-turns/recursive policy are opt-in. Each carries its §5/§7 migration note. +- No package moves, no dependency additions in `@threadplane/chat`. The Postgres driver is a dependency of `@threadplane/middleware` only (a Node/backend package), never of the browser libs. + +--- + +## 9. Risk analysis + +### 9a. Risks of keeping explicit continuation (status quo) + +| Risk | Severity | Mitigation | +| --- | --- | --- | +| Requires backend cooperation (`routeAfterAgent → __end__`). | Medium | Ship + document the middleware; dev-mode warning when a catalog tool is called but the run didn't end. | +| No single place enforces max-turns or a global follow-up policy. | Medium | Coordinator policy (§5e). | +| **Ephemeral in-memory dedup** — reload can re-execute a not-yet-persisted tool. | **High** | §7 backend-authoritative guard (Tier 1/2). | +| No `AbortSignal` to handlers. | High | §5b. | +| **N pending function tools → N racing runs.** | Medium | §5d batch-per-group. | +| Duplicated `pending` predicate across adapters. | Low | §6 narrow extraction. | + +### 9b. Risks of hidden recursive follow-up + +| Risk | Severity | Notes | +| --- | --- | --- | +| **Client becomes orchestration-authoritative** — fights server checkpoints on stateful backends. | **Critical** | *Unverified prior context:* an internal issue where a frontend store pushing state via `threads.updateState` raced the client-tool resume loop. Mechanism is concrete; re-confirm with a live-LLM test before relying on it. | +| No max-turns cap → infinite loop. | High | *Unverified prior context:* an aimock fixture-ordering bug produced an infinite continuation loop. Any auto-loop needs §5e. | +| Opaque vs a rendered `pending` signal; tension with the no-duplication invariant. | Medium | Keep the primitive signal-shaped; make any loop an explicit policy. | +| Breaks protocol-visible semantics → wrong for HITL approvals, audit. | High | Reserve `interrupt()` for approvals. | +| Reverses the documented `2026-06-08` decision. | Medium | No justification found. | + +### 9c. Why the hybrid dominates + +It captures the recursive model's *ergonomics* as a chat-layer policy over the protocol-visible primitive, preserves the enterprise-correct properties (server-authoritative, inspectable, checkpoint-safe, `interrupt()`-for-approvals), and fixes real current gaps (backend-authoritative dedup, abort, racing runs) — each through a defined contract, not a silent default. + +--- + +## 10. Milestone plan + +- **M0 — Test hardening (no behavior change).** Scriptable fake AG-UI agent; characterization tests; AG-UI/LangGraph parity; chat integration for action/view/ask. → **PR 1** (§11). Gates everything else. +- **M1 — Narrow shared-core extraction.** Pure `selectPendingClientToolCalls` only (§6). → **PR 2**. +- **M2 — `AbortSignal` in the handler context** (§5b). Additive. +- **M3 — Backend-authoritative durable dedup, Tier 1, shipped with Postgres** (§7): `ClientToolExecutionStore` interface + in-memory default + **Postgres implementation** in `@threadplane/middleware`; Tier-1 server `toolCallId` idempotency guard wired into the continuation path; reload reconciliation. Nullable `tenant_id` column, single-tenant PK. +- **M3.5 — Tier-2 claim-before-execute** (§7c): the pre-execution durable claim for non-idempotent browser tools (fail-closed on crash; `idempotent:true` opts out). Split from M3 so its extra round trip is latency-validated on a real deployment before it joins the default guarded path. +- **M4 — `settle()` + batch-per-group** (§5a, §5d), with the transport caveat documented, now reload-safe via M3. Coordinator group-tracking + `settle?` in both adapters. +- **M5 — Opt-in continuation policy + max-turns guard** (§5e); typed tool-call lifecycle surfaced to view/ask; cockpit example exercising abort + terminal (`settle`) tools. +- **M6 (FUTURE).** Hashbrown-provider → `Agent` bridge (§8). + +**Sequencing note:** M4 (`settle`/`followUp:false`) depends on M3 — a terminal tool whose result isn't persisted must be reload-safe, which only the durable guard provides. Do not ship `settle` before the guard. + +--- + +## 11. First PR scope — tests and characterization only + +**PR 1 = M0. No production behavior change. No contract change.** + +1. Extend `libs/ag-ui/src/lib/testing/fake-agent.ts` into a scriptable stream (`TOOL_CALL_START/ARGS/END`, `RUN_ERROR`, `STATE_SNAPSHOT`, `CUSTOM`/`on_interrupt`; branch on the presence of a `ToolMessage` in history to model backend-ends → resumes → continues). Deterministic (`setTimeout` cadence, cancellable; no clocks/`Math.random`). +2. Characterization tests locking **today's** behavior across both adapters: + - single pending → one continuation run; + - **multiple pending → one run *per resolve*** (N runs — current behavior; do **not** assert a single batched run); + - resolving one call does not disturb others; + - handler throw → `Error: …` tool message + a run is still issued; + - argument-validation fail → `Error: invalid arguments: …`, handler not called, run still issued; + - `view` auto-ack; `ask` resolves via the render event; + - `pending` empty while `isLoading`; server-resolved tool call skipped. +3. Chat-component integration test driving `action`/`view`/`ask` through the scriptable fake. + +**Out of PR 1** (each has a defined [CHANGE] design): shared-core extraction (PR 2), `AbortSignal` (M2), durable guard + Postgres (M3), `settle`/batching (M4), recursive policy / max-turns (M5). If a reviewer wants any sooner, it lands as its own PR carrying its §5/§7 migration note. + +Acceptance: new tests green on both adapters; **zero diff in runtime behavior** (characterization proves it); lint clean (`grep -cE ' error '`); no new public exports; no dependency changes. + +--- + +## 12. Resolved decisions & remaining open items + +**Resolved (Revision 3):** +1. **Suppress-continuation API** → separate optional **`settle()`** method (§5a), not a flag. +2. **Batching** → **batch per assistant-turn tool-call group** (§5d), fixing the N-racing-runs bug. +3. **Crash policy** → **fail-closed by default + per-tool `idempotent: true` opt-out** (§7d). +4. **Max-turns** → **configurable, default 10**, surfaced when hit (§5e). +5. **Durable dedup home & storage** → **backend-authoritative**; server `toolCallId` idempotency guard in `@threadplane/middleware`, **shipped with a Postgres implementation** (§7); browser marker demoted to a fallback (supersedes Revision 2's sessionStorage-first). +6. **Tier-2 timing** → **Tier 1 ships in M3; Tier 2 (claim-before-execute) splits to M3.5**, latency-validated on a real deployment before joining the default guarded path (§10). +7. **Tenant scoping** → **nullable `tenant_id` column now, single-tenant PK `(thread_id, tool_call_id)` for first ship**; `tenant_id` joins the PK later via an additive migration behind a config flag (§7f). + +**Remaining open items (non-blocking, M4+):** +1. **Batching mixed-inflight edge** — confirm the group-completion signal (all client tool calls in an assistant turn have settled) is derivable from `toolCallIds` on the assistant message; verify against a real multi-tool-call stream. (M4 concern.) +2. **Hashbrown-provider bridge (M6)** — reuse `to-agent`'s reducer or a bespoke adapter? Depends on the provider's event shape; defer. diff --git a/libs/ag-ui/src/lib/client-tools.spec.ts b/libs/ag-ui/src/lib/client-tools.spec.ts index 9739f318b..1f31b95e2 100644 --- a/libs/ag-ui/src/lib/client-tools.spec.ts +++ b/libs/ag-ui/src/lib/client-tools.spec.ts @@ -184,6 +184,46 @@ describe('createClientToolsCapability', () => { expect(tc?.error).toBeUndefined(); }); + it('resolve(ok) does not affect other pending calls', () => { + 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' }, + { id: 'c2', name: 'get_weather', args: {}, status: 'complete' }, + ]); + + expect(cap.pending().map((tc) => tc.id)).toEqual(['c1', 'c2']); + cap.resolve('c1', { ok: true, value: { temp: 70 } }); + + expect(cap.pending().map((tc) => tc.id)).toEqual(['c2']); + expect(store.toolCalls().find((tc) => tc.id === 'c2')?.result).toBeUndefined(); + }); + + it('resolving multiple pending calls issues one run per resolve', 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' }, + { id: 'c2', name: 'get_weather', args: {}, status: 'complete' }, + ]); + + cap.resolve('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.addMessage.mock.calls.map(([message]) => ( + message as { toolCallId: string } + ).toolCallId)).toEqual(['c1', 'c2']); + }); + // ---- resolve — error result ------------------------------------------------ it('resolve(error) writes { error } result + error + status=error onto the store tool call', () => { diff --git a/libs/ag-ui/src/lib/testing/fake-agent.spec.ts b/libs/ag-ui/src/lib/testing/fake-agent.spec.ts index ce9131c58..9deab39db 100644 --- a/libs/ag-ui/src/lib/testing/fake-agent.spec.ts +++ b/libs/ag-ui/src/lib/testing/fake-agent.spec.ts @@ -94,3 +94,135 @@ describe('FakeAgent — reasoningTokens', () => { expect(types).not.toContain('REASONING_MESSAGE_END'); }); }); + +describe('FakeAgent — scripted streams', () => { + it('emits scripted tool-call events between RUN_STARTED and RUN_FINISHED', async () => { + const agent = new FakeAgent({ + delayMs: 0, + script: [{ + when: 'initial', + events: [ + { + type: 'TOOL_CALL_START', + toolCallId: 'tool-1', + toolCallName: 'get_weather', + parentMessageId: 'assistant-1', + }, + { type: 'TOOL_CALL_ARGS', toolCallId: 'tool-1', delta: '{"city":"SF"}' }, + { type: 'TOOL_CALL_END', toolCallId: 'tool-1' }, + ], + }], + }); + + const events = await lastValueFrom( + agent.run({ ...minimalInput, threadId: 'thread-script', runId: 'run-script' }).pipe(toArray()), + ); + + expect(events.map((e) => e.type)).toEqual([ + EventType.RUN_STARTED, + 'TOOL_CALL_START', + 'TOOL_CALL_ARGS', + 'TOOL_CALL_END', + EventType.RUN_FINISHED, + ]); + expect(events[0]).toMatchObject({ threadId: 'thread-script', runId: 'run-script' }); + expect(events[1]).toMatchObject({ + toolCallId: 'tool-1', + toolCallName: 'get_weather', + parentMessageId: 'assistant-1', + }); + expect(events[4]).toMatchObject({ threadId: 'thread-script', runId: 'run-script' }); + }); + + it('selects a continuation branch when history contains a matching tool message', async () => { + const agent = new FakeAgent({ + delayMs: 0, + script: [ + { + when: 'initial', + events: [ + { type: 'TOOL_CALL_START', toolCallId: 'tool-1', toolCallName: 'get_weather' } as BaseEvent, + { type: 'TOOL_CALL_END', toolCallId: 'tool-1' } as BaseEvent, + ], + }, + { + when: { toolMessageFor: 'tool-1' }, + events: [ + { type: EventType.TEXT_MESSAGE_START, messageId: 'assistant-2', role: 'assistant' } as BaseEvent, + { type: EventType.TEXT_MESSAGE_CONTENT, messageId: 'assistant-2', delta: 'continued' } as BaseEvent, + { type: EventType.TEXT_MESSAGE_END, messageId: 'assistant-2' } as BaseEvent, + ], + }, + ], + }); + + const events = await lastValueFrom( + agent.run({ + ...minimalInput, + messages: [{ role: 'tool', toolCallId: 'tool-1', content: '{"temp":70}' }] as never, + }).pipe(toArray()), + ); + + expect(events.map((e) => e.type)).toEqual([ + EventType.RUN_STARTED, + EventType.TEXT_MESSAGE_START, + EventType.TEXT_MESSAGE_CONTENT, + EventType.TEXT_MESSAGE_END, + EventType.RUN_FINISHED, + ]); + expect(events[2]).toMatchObject({ delta: 'continued' }); + }); + + it('emits scripted RUN_ERROR, STATE_SNAPSHOT, and CUSTOM interrupt events', async () => { + const interrupt = { kind: 'approval', amount: 42 }; + const agent = new FakeAgent({ + delayMs: 0, + script: [{ + when: 'initial', + events: [ + { type: EventType.STATE_SNAPSHOT, snapshot: { fresh: 1 } } as BaseEvent, + { type: EventType.CUSTOM, name: 'on_interrupt', value: interrupt } as BaseEvent, + { type: EventType.RUN_ERROR, message: 'boom' } as BaseEvent, + ], + }], + }); + + const events = await lastValueFrom(agent.run(minimalInput).pipe(toArray())); + + expect(events.map((e) => e.type)).toEqual([ + EventType.RUN_STARTED, + EventType.STATE_SNAPSHOT, + EventType.CUSTOM, + EventType.RUN_ERROR, + EventType.RUN_FINISHED, + ]); + expect(events[1]).toMatchObject({ snapshot: { fresh: 1 } }); + expect(events[2]).toMatchObject({ name: 'on_interrupt', value: interrupt }); + expect(events[3]).toMatchObject({ message: 'boom' }); + }); + + it('cancels scripted emissions when unsubscribed', async () => { + vi.useFakeTimers(); + const agent = new FakeAgent({ + delayMs: 100, + script: [{ + when: 'initial', + events: [ + { type: 'TOOL_CALL_START', toolCallId: 'tool-1', toolCallName: 'get_weather' } as BaseEvent, + { type: 'TOOL_CALL_ARGS', toolCallId: 'tool-1', delta: '{"city":"SF"}' } as BaseEvent, + { type: 'TOOL_CALL_END', toolCallId: 'tool-1' } as BaseEvent, + ], + }], + }); + const seen: BaseEvent[] = []; + + const sub = agent.run(minimalInput).subscribe((e) => seen.push(e)); + vi.advanceTimersByTime(50); + sub.unsubscribe(); + vi.advanceTimersByTime(1000); + + expect(seen).toHaveLength(1); + expect(seen[0].type).toBe(EventType.RUN_STARTED); + vi.useRealTimers(); + }); +}); diff --git a/libs/ag-ui/src/lib/testing/fake-agent.ts b/libs/ag-ui/src/lib/testing/fake-agent.ts index 393b55bc6..47603c0ed 100644 --- a/libs/ag-ui/src/lib/testing/fake-agent.ts +++ b/libs/ag-ui/src/lib/testing/fake-agent.ts @@ -8,6 +8,15 @@ import { } from '@ag-ui/client'; import { Observable } from 'rxjs'; +type FakeAgentScriptWhen = + | 'initial' + | { toolMessageFor: string }; + +interface FakeAgentScriptBranch { + when: FakeAgentScriptWhen; + events: readonly BaseEvent[]; +} + /** * In-process AG-UI agent that emits a canned streaming response. * @@ -29,11 +38,15 @@ export class FakeAgent extends AbstractAgent { /** Milliseconds between successive token emissions. */ private readonly delayMs: number; + /** Optional deterministic event branches for tests that need exact streams. */ + private readonly script: readonly FakeAgentScriptBranch[]; + constructor(opts: { tokens?: string[]; /** Optional reasoning chunks emitted before the text reply. */ reasoningTokens?: string[]; delayMs?: number; + script?: readonly FakeAgentScriptBranch[]; } = {}) { super(); this.tokens = opts.tokens ?? [ @@ -42,9 +55,13 @@ export class FakeAgent extends AbstractAgent { ]; this.reasoningTokens = opts.reasoningTokens ?? []; this.delayMs = opts.delayMs ?? 60; + this.script = opts.script ?? []; } run(input: RunAgentInput): Observable { + const scripted = this.scriptedSequence(input); + if (scripted) return this.emitSequence(scripted, 30); + const tokens = this.tokens; const reasoningTokens = this.reasoningTokens; const delayMs = this.delayMs; @@ -69,6 +86,22 @@ export class FakeAgent extends AbstractAgent { sequence.push({ type: EventType.TEXT_MESSAGE_END, messageId } as BaseEvent); sequence.push({ type: EventType.RUN_FINISHED, threadId: input.threadId, runId: input.runId } as BaseEvent); + return this.emitSequence(sequence, 30); + } + + private scriptedSequence(input: RunAgentInput): BaseEvent[] | undefined { + if (this.script.length === 0) return undefined; + const branch = this.script.find((candidate) => matchesBranch(candidate.when, input)); + if (!branch) return undefined; + return [ + { type: EventType.RUN_STARTED, threadId: input.threadId, runId: input.runId } as BaseEvent, + ...branch.events, + { type: EventType.RUN_FINISHED, threadId: input.threadId, runId: input.runId } as BaseEvent, + ]; + } + + private emitSequence(sequence: readonly BaseEvent[], initialDelayMs: number): Observable { + const delayMs = this.delayMs; return new Observable((observer) => { let cancelled = false; let timer: ReturnType | undefined; @@ -86,7 +119,7 @@ export class FakeAgent extends AbstractAgent { timer = setTimeout(emitNext, delayMs); }; - timer = setTimeout(emitNext, 30); + timer = setTimeout(emitNext, initialDelayMs); return () => { cancelled = true; @@ -95,3 +128,25 @@ export class FakeAgent extends AbstractAgent { }); } } + +function matchesBranch(when: FakeAgentScriptWhen, input: RunAgentInput): boolean { + if (when === 'initial') return !hasToolMessages(input); + return hasToolMessageFor(input, when.toolMessageFor); +} + +function hasToolMessages(input: RunAgentInput): boolean { + return (input.messages ?? []).some((message) => isToolMessage(message)); +} + +function hasToolMessageFor(input: RunAgentInput, toolCallId: string): boolean { + return (input.messages ?? []).some((message) => { + if (!isToolMessage(message)) return false; + const record = message as Record; + return record['toolCallId'] === toolCallId || record['tool_call_id'] === toolCallId; + }); +} + +function isToolMessage(message: unknown): boolean { + const record = message as Record; + return record['role'] === 'tool' || record['type'] === 'tool'; +} 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 d6556746c..35611dcd6 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 @@ -66,6 +66,16 @@ const weatherRegistry = tools({ ), }); +const failingRegistry = tools({ + explode: action( + 'Throw from the handler', + z.object({ value: z.string() }), + async () => { + throw new Error('handler exploded'); + }, + ), +}); + // ── tests ───────────────────────────────────────────────────────────────────── describe('startClientToolExecutor()', () => { @@ -92,6 +102,51 @@ describe('startClientToolExecutor()', () => { expect(resolve).toHaveBeenCalledWith('c1', { ok: true, value: { temp: 70, city: 'SF' } }); }); + it('normalizes handler throws to an error result', async () => { + const { pending, resolve, capability } = makeFakeCapability(); + const agent = makeFakeAgent(capability); + + TestBed.runInInjectionContext(() => { + startClientToolExecutor(agent, failingRegistry); + }); + + pending.set([{ id: 'e1', name: 'explode', args: { value: 'x' }, status: 'complete' }]); + TestBed.flushEffects(); + await drainMicrotasks(); + + expect(resolve).toHaveBeenCalledWith('e1', { + ok: false, + error: 'handler exploded', + }); + }); + + it('normalizes invalid arguments to an error result without calling the handler', async () => { + const handler = vi.fn(async (a: { city: string }) => ({ temp: 70, city: a.city })); + const registry = tools({ + get_weather: action( + 'Get the weather for a city', + z.object({ city: z.string() }), + handler, + ), + }); + const { pending, resolve, capability } = makeFakeCapability(); + const agent = makeFakeAgent(capability); + + TestBed.runInInjectionContext(() => { + startClientToolExecutor(agent, registry); + }); + + pending.set([{ id: 'bad-args', name: 'get_weather', args: { city: 123 }, status: 'complete' }]); + TestBed.flushEffects(); + await drainMicrotasks(); + + expect(handler).not.toHaveBeenCalled(); + expect(resolve).toHaveBeenCalledOnce(); + expect(resolve.mock.calls[0][0]).toBe('bad-args'); + expect(resolve.mock.calls[0][1].ok).toBe(false); + expect((resolve.mock.calls[0][1] as { error: string }).error).toContain('invalid arguments:'); + }); + it('does not double-resolve the same call id (in-flight guard)', async () => { const { pending, resolve, capability } = makeFakeCapability(); const agent = makeFakeAgent(capability); diff --git a/libs/chat/src/lib/compositions/chat/chat.component.client-tools.spec.ts b/libs/chat/src/lib/compositions/chat/chat.component.client-tools.spec.ts index 169018f5c..014d527bd 100644 --- a/libs/chat/src/lib/compositions/chat/chat.component.client-tools.spec.ts +++ b/libs/chat/src/lib/compositions/chat/chat.component.client-tools.spec.ts @@ -5,7 +5,7 @@ import { Injector, runInInjectionContext, signal, type WritableSignal } from '@a import { z } from 'zod/v4'; import { ChatComponent } from './chat.component'; import { mockAgent, type MockAgent } from '../../testing/mock-agent'; -import { tools, action, ask } from '../../client-tools/tools'; +import { tools, action, view, ask } from '../../client-tools/tools'; import type { ClientToolsCapability, ClientToolResult } from '../../client-tools/client-tools-capability'; import type { ClientToolSpec } from '../../client-tools/to-json-schema'; import type { ToolCall } from '../../agent/tool-call'; @@ -31,6 +31,7 @@ function setSignalInput(sig: unknown, value: T): void { } class FakeAskComponent {} +class FakeViewComponent {} interface FakeCap { pending: WritableSignal; @@ -54,12 +55,41 @@ function agentWithClientTools(cap: ClientToolsCapability): MockAgent { return agent; } +interface ScriptedAgent { + agent: MockAgent; + emitPending(call: ToolCall): void; + resolve: ReturnType>; + setCatalog: ReturnType>; +} + +function makeScriptedClientToolAgent(): ScriptedAgent { + const pending = signal([]); + const resolve = vi.fn<[string, ClientToolResult], void>((id) => { + pending.update((calls) => calls.filter((call) => call.id !== id)); + }); + const setCatalog = vi.fn<[readonly ClientToolSpec[]], void>(); + const capability: ClientToolsCapability = { setCatalog, pending, resolve }; + return { + agent: agentWithClientTools(capability), + emitPending(call: ToolCall): void { + pending.set([call]); + }, + resolve, + setCatalog, + }; +} + const clientToolRegistry = tools({ get_weather: action( 'Get the weather', z.object({ city: z.string() }), async (a) => ({ temp: 70, city: a.city }), ), + weather_card: view( + 'Show weather details', + z.object({ city: z.string() }), + FakeViewComponent as never, + ), confirm: ask( 'Confirm an action', z.object({}), @@ -94,6 +124,7 @@ describe('ChatComponent — client-tools wiring', () => { expect(cap.setCatalog).toHaveBeenCalledOnce(); const names = (cap.setCatalog.mock.calls[0][0] as ClientToolSpec[]).map((s) => s.name); expect(names).toContain('get_weather'); + expect(names).toContain('weather_card'); expect(names).toContain('confirm'); }); @@ -103,7 +134,9 @@ describe('ChatComponent — client-tools wiring', () => { setSignalInput(c.clientTools, clientToolRegistry); setSignalInput(c.agent, agentWithClientTools(makeFakeCapability().capability)); // ask-kind tool is a view component → excluded; function tools are not. + expect(c.viewToolNames()).toContain('weather_card'); expect(c.viewToolNames()).toContain('confirm'); + expect(c.excludedToolNames()).toContain('weather_card'); expect(c.excludedToolNames()).toContain('confirm'); expect(c.viewToolNames()).not.toContain('get_weather'); }); @@ -159,6 +192,64 @@ describe('ChatComponent — client-tools wiring', () => { expect(cap.resolve).toHaveBeenCalledWith('a1', { ok: true, value: { confirmed: true } }); }); + it('drives action, view, and ask client tools through the coordinator with a scripted agent', async () => { + const scripted = makeScriptedClientToolAgent(); + let comp!: ChatComponent; + runInInjectionContext(injector, () => { + comp = new ChatComponent(); + setSignalInput(comp.clientTools, clientToolRegistry); + setSignalInput(comp.agent, scripted.agent); + TestBed.flushEffects(); + }); + await drainMicrotasks(); + + expect(scripted.setCatalog).toHaveBeenCalledOnce(); + + scripted.emitPending({ + id: 'script-action', + name: 'get_weather', + args: { city: 'SF' }, + status: 'complete', + }); + TestBed.flushEffects(); + await drainMicrotasks(); + + expect(scripted.resolve).toHaveBeenCalledWith('script-action', { + ok: true, + value: { temp: 70, city: 'SF' }, + }); + + scripted.emitPending({ + id: 'script-view', + name: 'weather_card', + args: { city: 'LA' }, + status: 'complete', + }); + TestBed.flushEffects(); + + expect(scripted.resolve).toHaveBeenCalledWith('script-view', { + ok: true, + value: { shown: true }, + }); + + scripted.emitPending({ + id: 'script-ask', + name: 'confirm', + args: {}, + status: 'complete', + }); + (comp as unknown as { onClientToolEvent: (e: unknown) => void }).onClientToolEvent({ + type: 'result', + value: { confirmed: true }, + elementKey: 'confirm', + }); + + expect(scripted.resolve).toHaveBeenCalledWith('script-ask', { + ok: true, + value: { confirmed: true }, + }); + }); + it('is a no-op when no clientTools registry is provided', async () => { const cap = makeFakeCapability(); runInInjectionContext(injector, () => { diff --git a/libs/langgraph/src/lib/client-tools.spec.ts b/libs/langgraph/src/lib/client-tools.spec.ts index eb245fbf1..33e596ff6 100644 --- a/libs/langgraph/src/lib/client-tools.spec.ts +++ b/libs/langgraph/src/lib/client-tools.spec.ts @@ -266,6 +266,28 @@ describe('createClientToolsCapability', () => { expect(remaining[0].id).toBe('c2'); }); + it('resolving multiple pending calls issues one submit per resolve', 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' }, + { id: 'c2', name: 'get_weather', args: {}, status: 'complete' }, + ]); + + cap.resolve('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']); + }); + // ── resolve — error result ────────────────────────────────────────────────── it('resolve(error) issues a run whose tool message content contains the error', async () => {