diff --git a/.changeset/wire-squad-route.md b/.changeset/wire-squad-route.md new file mode 100644 index 000000000..574c059fa --- /dev/null +++ b/.changeset/wire-squad-route.md @@ -0,0 +1,43 @@ +--- +'@bradygaster/squad-sdk': minor +--- + +fix(sdk): wire `squad_route` tool handler to actually create target sessions + +The `squad_route` tool is documented in `packages/squad-sdk/README.md`, +`docs/src/content/docs/reference/sdk.md`, +`docs/src/content/docs/reference/api-reference.md`, and the +`build-autonomous-agent` guide as routing tasks and creating new sessions +for the target agent. The previous implementation returned +`resultType: 'success'` with the hard-coded text "Session creation will be +implemented when session lifecycle is in place" without ever spawning a +session. SDK embedders following the documented usage +(`toolRegistry.getTool('squad_route').handler({ ... })`) received a +confident "Task routed to X" success payload while no agent ever started — +a silent no-op masquerading as success. + +This change: + +- Adds an optional 5th positional `fanOutDepsGetter` constructor parameter + to `ToolRegistry`. Existing 4-arg constructions are unaffected. +- When `fanOutDepsGetter` returns dependencies, `squad_route` calls + `spawnParallel` with a single config (charter compile → model resolve → + `createSession` → initial message), matching the SDK `Coordinator` + fan-out path. +- When `fanOutDepsGetter` is absent or returns `undefined`, the handler + returns `resultType: 'failure'` with `error: 'fan-out-deps-unavailable'` + and remediation guidance, replacing the previous false-success. +- Normalizes `targetAgent` to lowercase (matching charter-loading convention). +- Distinguishes roster "not found" errors from infrastructure I/O failures. +- Updates `docs/src/content/docs/guide/build-autonomous-agent.md` with a + note about the `fanOutDepsGetter` requirement and `fan-out-deps-unavailable` + failure mode. + +Behavior change for embedders that relied on the stub's unconditional +success: those calls now return an honest failure. Remediation is to +either (a) construct `ToolRegistry` with a `fanOutDepsGetter`, or +(b) intercept `squad_route` via `SquadSessionHooks.onPreToolUse`. + +Related: #1029 (named-agent delegation answering inline) — different code +path (Copilot CLI `task` tool / coordinator prompt), not closed by this PR, +but shares the same root-cause family of silent-no-op delegation primitives. diff --git a/docs/src/content/docs/guide/build-autonomous-agent.md b/docs/src/content/docs/guide/build-autonomous-agent.md index 5e550ff79..66e0ac0c4 100644 --- a/docs/src/content/docs/guide/build-autonomous-agent.md +++ b/docs/src/content/docs/guide/build-autonomous-agent.md @@ -355,6 +355,10 @@ The autonomous-pipeline sample demonstrates three coordination patterns that age These patterns let agents coordinate without a central orchestrator. Each agent makes local decisions that accumulate into a shared knowledge base. +:::note[`squad_route` requires `fanOutDepsGetter`] +For `squad_route` to actually spawn agent sessions, the `ToolRegistry` must be constructed with a `fanOutDepsGetter` callback that provides fan-out dependencies (`sessionPool`, `modelClient`, `squadRoot`, `configGetter`). Without it, the tool returns an honest `fan-out-deps-unavailable` error instead of silently succeeding. See the [SDK reference](/reference/sdk/#toolregistry) for wiring details. +::: + --- ## Add observability diff --git a/docs/src/content/docs/reference/api-reference.md b/docs/src/content/docs/reference/api-reference.md index 5e82a47fa..a41ed24bb 100644 --- a/docs/src/content/docs/reference/api-reference.md +++ b/docs/src/content/docs/reference/api-reference.md @@ -288,16 +288,25 @@ const myTool = defineTool<{ query: string }>({ Manage the built-in tool set. ```typescript +import { ToolRegistry } from '@bradygaster/squad-sdk/tools'; +import type { FanOutDependencies } from '@bradygaster/squad-sdk/coordinator'; + const registry = new ToolRegistry('./.squad'); const tools = registry.getTools(); const agentTools = registry.getToolsForAgent(['squad_route', 'squad_decide']); ``` +**Constructor:** `new ToolRegistry(squadRoot?, sessionPoolGetter?, storage?, state?, fanOutDepsGetter?)` + +- `fanOutDepsGetter` — Required for `squad_route` to spawn sessions via `spawnParallel`. Returns a `FanOutDependencies` object (from `@bradygaster/squad-sdk/coordinator`). Without it, `squad_route` returns `resultType: 'failure'` with `error: 'fan-out-deps-unavailable'`. +- `state` — When provided, `squad_route` validates that the target agent exists in the team roster before spawning. +- Agent names must match `/^[a-zA-Z0-9_-]+$/`. Spawn errors are sanitized before being returned to the LLM. + **Built-in tools:** | Tool | Purpose | |------|---------| -| `squad_route` | Route a task to another agent | +| `squad_route` | Route a task to another agent (requires `fanOutDepsGetter`) | | `squad_decide` | Write decisions to the inbox | | `squad_memory` | Append to agent history | | `squad_status` | Query session pool state | diff --git a/docs/src/content/docs/reference/sdk.md b/docs/src/content/docs/reference/sdk.md index 6b6b5547b..efacaec9a 100644 --- a/docs/src/content/docs/reference/sdk.md +++ b/docs/src/content/docs/reference/sdk.md @@ -470,17 +470,25 @@ const myTool = defineTool<{ query: string }>({ ### `ToolRegistry` ```typescript +import { ToolRegistry } from '@bradygaster/squad-sdk/tools'; +import type { FanOutDependencies } from '@bradygaster/squad-sdk/coordinator'; + const registry = new ToolRegistry('./.squad'); registry.getTools(); // All tools registry.getToolsForAgent(['squad_route', 'squad_decide']); // Agent-specific registry.getTool('squad_route'); // Single lookup ``` +**Constructor:** `new ToolRegistry(squadRoot?, sessionPoolGetter?, storage?, state?, fanOutDepsGetter?)` + +- `fanOutDepsGetter` — Required for `squad_route` to create sessions via `spawnParallel`. Returns a `FanOutDependencies` object (from `@bradygaster/squad-sdk/coordinator`). Without it, `squad_route` returns `error: 'fan-out-deps-unavailable'`. +- `state` — When provided, `squad_route` validates that the target agent exists in the roster before spawning. + **Built-in tools:** | Tool | Purpose | |------|---------| -| `squad_route` | Route a task to another agent | +| `squad_route` | Route a task to another agent (requires `fanOutDepsGetter`) | | `squad_decide` | Write decisions to the inbox | | `squad_memory` | Append to agent history | | `squad_status` | Query session pool state | diff --git a/packages/squad-sdk/README.md b/packages/squad-sdk/README.md index a8e6f6749..491dad5f8 100644 --- a/packages/squad-sdk/README.md +++ b/packages/squad-sdk/README.md @@ -85,7 +85,7 @@ Five tools let agents coordinate without calling you back. Here are the three yo ```typescript const tool = toolRegistry.getTool('squad_route'); await tool.handler({ - targetAgent: 'McManus', + targetAgent: 'mcmanus', // lowercase — agent names are normalized task: 'Write a blog post on the new casting system', priority: 'high', context: 'Feature launches next week', @@ -94,6 +94,31 @@ await tool.handler({ The lead routes a task to DevRel. A new session is created, context is passed, and the task is queued with priority. No human in the loop. +> **Wiring requirement:** `squad_route` creates sessions via `spawnParallel`, which requires fan-out dependencies. Pass a `fanOutDepsGetter` as the 5th argument to `new ToolRegistry(...)`: +> +> ```typescript +> import { ToolRegistry } from '@bradygaster/squad-sdk/tools'; +> import type { FanOutDependencies } from '@bradygaster/squad-sdk/coordinator'; +> +> const fanOutDeps: FanOutDependencies = { +> compileCharter: async (name) => { /* load agent charter */ }, +> resolveModel: async (charter, override) => override ?? charter.modelPreference ?? 'default', +> createSession: async (config) => ({ sessionId: '...', sendMessage: async () => {} }), +> sessionPool, // SessionPool instance +> eventBus, // EventBus instance +> }; +> +> const registry = new ToolRegistry( +> './.squad', +> () => sessionPool, // sessionPoolGetter +> storageProvider, // storage +> squadState, // state (enables roster validation) +> () => fanOutDeps, // fanOutDepsGetter +> ); +> ``` +> +> Without it, `squad_route` returns `resultType: 'failure'` with `error: 'fan-out-deps-unavailable'`. Agent names must match `/^[a-zA-Z0-9_-]+$/` and, when state is provided, exist in the team roster. + ### `squad_decide` — Record a team decision ```typescript diff --git a/packages/squad-sdk/src/tools/index.ts b/packages/squad-sdk/src/tools/index.ts index ef2067296..c33784f5d 100644 --- a/packages/squad-sdk/src/tools/index.ts +++ b/packages/squad-sdk/src/tools/index.ts @@ -17,6 +17,7 @@ import { trace, SpanStatusCode } from '../runtime/otel-api.js'; import type { StorageProvider } from '../storage/storage-provider.js'; import { FSStorageProvider } from '../storage/fs-storage-provider.js'; import type { SquadState } from '../state/squad-state.js'; +import { spawnParallel, type FanOutDependencies } from '../coordinator/fan-out.js'; const tracer = trace.getTracer('squad-sdk'); @@ -165,15 +166,24 @@ export function defineTool(config: { }; } +// --- Validation --- + +/** Agent name format: alphanumeric, hyphens, underscores. Same rule as squad_decide/squad_memory. */ +const AGENT_NAME_RE = /^[a-zA-Z0-9_-]+$/; + // --- Error Sanitization --- /** * Sanitize error messages before sending to LLM. - * Strips absolute filesystem paths by replacing the squadRoot prefix with [team-root]. + * Strips absolute filesystem paths by replacing the squadRoot prefix with [team-root], + * and collapses multi-line errors to prevent stack trace leakage. */ function sanitizeErrorForLlm(error: unknown, squadRoot: string): string { - const msg = error instanceof Error ? error.message : String(error); - return msg.replace(new RegExp(squadRoot.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g'), '[team-root]'); + const raw = error instanceof Error ? error.message : String(error); + const stripped = raw.replace(new RegExp(squadRoot.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g'), '[team-root]'); + // Collapse to first line to avoid leaking stack-like multi-line details + const firstLine = stripped.split('\n')[0] ?? stripped; + return firstLine.slice(0, 512); } // --- Tool Registry --- @@ -184,12 +194,20 @@ export class ToolRegistry { private sessionPoolGetter?: () => any; private storage: StorageProvider; private state?: SquadState; - - constructor(squadRoot = '.squad', sessionPoolGetter?: () => any, storage: StorageProvider = new FSStorageProvider(), state?: SquadState) { + private fanOutDepsGetter?: () => FanOutDependencies | undefined; + + constructor( + squadRoot = '.squad', + sessionPoolGetter?: () => any, + storage: StorageProvider = new FSStorageProvider(), + state?: SquadState, + fanOutDepsGetter?: () => FanOutDependencies | undefined, + ) { this.squadRoot = squadRoot; this.sessionPoolGetter = sessionPoolGetter; this.storage = storage; this.state = state; + this.fanOutDepsGetter = fanOutDepsGetter; this.registerSquadTools(); } @@ -223,28 +241,117 @@ export class ToolRegistry { required: ['targetAgent', 'task'], }, handler: async (args) => { - // Validate target agent exists (stub for now, will check roster later) - if (!args.targetAgent || args.targetAgent.trim() === '') { + // Normalize + validate target agent name (lowercase to match charter loading convention) + const targetAgent = (args.targetAgent ?? '').trim().toLowerCase(); + if (!targetAgent) { return { textResultForLlm: 'Error: Target agent name is required', resultType: 'failure', error: 'Invalid target agent', }; } + if (!AGENT_NAME_RE.test(targetAgent)) { + return { + textResultForLlm: 'Invalid target agent name: must contain only letters, numbers, hyphens, and underscores', + resultType: 'failure', + error: 'invalid-agent-name', + }; + } + + // Roster check: verify the agent exists when state is available + if (this.state) { + try { + const handle = this.state.agents.get(targetAgent); + await handle.charter(); + } catch (err: unknown) { + // Distinguish "not found" from infrastructure errors (I/O, permissions) + const isNotFound = + err instanceof Error && (err.constructor.name === 'NotFoundError' || err.message.includes('not found')); + if (isNotFound) { + return { + textResultForLlm: `Agent '${targetAgent}' not found in the team roster. Check .squad/agents/ for available agents.`, + resultType: 'failure', + error: 'agent-not-in-roster', + }; + } + return { + textResultForLlm: `Roster check failed for '${targetAgent}': ${sanitizeErrorForLlm(err, this.squadRoot)}`, + resultType: 'failure', + error: 'roster-check-failed', + }; + } + } - // Create route request (session creation wired later) + const priority = args.priority || 'normal'; const routeRequest: RouteRequest = { - targetAgent: args.targetAgent, + targetAgent, task: args.task, - priority: args.priority || 'normal', + priority, context: args.context, }; - return { - textResultForLlm: `Task routed to ${args.targetAgent}. Priority: ${routeRequest.priority}. Session creation will be implemented when session lifecycle is in place.`, - resultType: 'success', - toolTelemetry: { routeRequest }, - }; + // Resolve fan-out dependencies. Without them, the SDK cannot create + // sessions on behalf of the LLM. Returning fake-success here would + // cause the coordinator to claim work it never did (#1029). + let fanOutDeps: ReturnType>; + try { + fanOutDeps = this.fanOutDepsGetter?.(); + } catch (error) { + return { + textResultForLlm: `Cannot route to ${targetAgent}: ${sanitizeErrorForLlm(error, this.squadRoot)}`, + resultType: 'failure', + error: 'fan-out-deps-unavailable', + toolTelemetry: { routeRequest }, + }; + } + if (!fanOutDeps) { + return { + textResultForLlm: + `Cannot route to ${targetAgent}: fan-out dependencies are not configured. ` + + `Wire a fanOutDepsGetter into ToolRegistry, or intercept squad_route via SquadSessionHooks.onPreToolUse.`, + resultType: 'failure', + error: 'fan-out-deps-unavailable', + toolTelemetry: { routeRequest }, + }; + } + + // Spawn the target agent via the production fan-out path. + // spawnParallel with a single config matches CLI Path A behavior + // (charter compile → model resolve → createSession → initial message). + try { + const results = await spawnParallel( + [{ + agentName: targetAgent, + task: args.task, + priority, + context: args.context, + }], + fanOutDeps, + ); + const result = results[0]; + + if (!result || result.status !== 'success') { + return { + textResultForLlm: `Failed to route to ${targetAgent}: ${sanitizeErrorForLlm(result?.error ?? 'unknown error', this.squadRoot)}`, + resultType: 'failure', + error: 'spawn-failed', + toolTelemetry: { routeRequest, agentName: targetAgent }, + }; + } + + return { + textResultForLlm: `Spawned session ${result.sessionId} for ${targetAgent} with priority ${priority}.`, + resultType: 'success', + toolTelemetry: { routeRequest, sessionId: result.sessionId }, + }; + } catch (error) { + return { + textResultForLlm: `Failed to route to ${targetAgent}: ${sanitizeErrorForLlm(error, this.squadRoot)}`, + resultType: 'failure', + error: 'spawn-exception', + toolTelemetry: { routeRequest }, + }; + } }, }); diff --git a/test/integration.test.ts b/test/integration.test.ts index fd6b645ed..7c14de5fa 100644 --- a/test/integration.test.ts +++ b/test/integration.test.ts @@ -88,7 +88,11 @@ describe('Integration: Tool → Hook Pipeline', () => { const hookResult = await pipeline.runPreToolHooks(ctx); expect(hookResult.action).toBe('allow'); - // Execute tool + // Execute tool. With no fanOutDepsGetter wired into ToolRegistry, + // the handler returns a structured failure (fan-out-deps-unavailable) + // rather than the previous fake-success. The point of this test is that + // the hook pipeline allows the call through; the tool's own outcome is + // determined by infra wiring (covered separately in test/tools.test.ts). const toolResult = await tool.handler( { targetAgent: 'fenster', task: 'Implement feature' } as RouteRequest, { @@ -99,7 +103,8 @@ describe('Integration: Tool → Hook Pipeline', () => { } ); - expect(toolResult.resultType).toBe('success'); + expect(toolResult.resultType).toBe('failure'); + expect((toolResult as { error?: string }).error).toBe('fan-out-deps-unavailable'); }); it('should block squad_route when custom hook blocks it', async () => { diff --git a/test/sdk/squad-route.test.ts b/test/sdk/squad-route.test.ts new file mode 100644 index 000000000..ca9e3417f --- /dev/null +++ b/test/sdk/squad-route.test.ts @@ -0,0 +1,507 @@ +/** + * SDK-tier coverage for the documented squad_route contract. + * + * These tests treat the SDK as a black box: they import only from the + * package's published subpath exports (the same ones embedders use when + * following docs/src/content/docs/reference/sdk.md and api-reference.md). + * + * Goal: lock in the documented promise that + * + * const registry = new ToolRegistry('./.squad', undefined, undefined, undefined, () => deps); + * const tool = registry.getTool('squad_route'); + * await tool.handler({ targetAgent, task }, ctx); + * + * actually spawns a session via the fan-out path, and that constructing + * the registry without fan-out dependencies surfaces a documented + * failure rather than a silent fake-success (regression guard for + * jagilber/squad#9). + * + * @module test/sdk/squad-route + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ToolRegistry, type RouteRequest } from '@bradygaster/squad-sdk/tools'; +import { SessionPool, EventBus } from '@bradygaster/squad-sdk/client'; +import type { FanOutDependencies } from '@bradygaster/squad-sdk/coordinator'; +import type { AgentCharter } from '@bradygaster/squad-sdk/agents'; + +/** + * Build mocked FanOutDependencies wired only at the documented public boundary + * (charter compile, model resolve, session create). Real EventBus + SessionPool + * are used so embedder-side wiring failures would surface here. + */ +function buildEmbedderFanOutDeps( + overrides: Partial = {}, +): FanOutDependencies { + return { + compileCharter: + overrides.compileCharter ?? + vi.fn(async (agentName: string): Promise => ({ + name: agentName, + displayName: `${agentName} Agent`, + role: 'Developer', + expertise: ['TypeScript'], + style: 'Professional', + prompt: `You are ${agentName}`, + modelPreference: 'claude-sonnet-4.5', + })), + resolveModel: + overrides.resolveModel ?? + vi.fn(async (charter: AgentCharter, override?: string) => + override ?? charter.modelPreference ?? 'claude-sonnet-4.5', + ), + createSession: + overrides.createSession ?? + vi.fn(async () => ({ + sessionId: `embedder-session-${Math.random().toString(36).slice(2, 11)}`, + sendMessage: vi.fn(async () => undefined), + })), + sessionPool: + overrides.sessionPool ?? + new SessionPool({ + maxConcurrent: 10, + idleTimeout: 60_000, + healthCheckInterval: 30_000, + }), + eventBus: overrides.eventBus ?? new EventBus(), + }; +} + +describe('SDK contract: squad_route (documented snippet)', () => { + const ctx = { + sessionId: 'embedder-test-session', + toolCallId: 'embedder-test-call', + toolName: 'squad_route', + arguments: {}, + }; + + it('matches the registry construction example from docs/reference/sdk.md', () => { + // sdk.md §"ToolRegistry" — embedder example: + // const registry = new ToolRegistry('./.squad'); + // registry.getTool('squad_route'); + const registry = new ToolRegistry('./.squad-sdk-contract'); + const tool = registry.getTool('squad_route'); + + expect(tool).toBeDefined(); + expect(tool!.name).toBe('squad_route'); + expect(tool!.description).toMatch(/route/i); + // documented in sdk.md: parameters object is JSON-Schema-shaped + expect(tool!.parameters).toMatchObject({ type: 'object' }); + }); + + it('spawns a session when constructed with a fanOutDepsGetter (documented happy path)', async () => { + const deps = buildEmbedderFanOutDeps(); + const registry = new ToolRegistry( + './.squad-sdk-contract', + undefined, + undefined, + undefined, + () => deps, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { + targetAgent: 'fenster', + task: 'Implement feature X', + priority: 'high', + context: 'Related to PRD-2', + } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ resultType: 'success' }); + // Documented contract: success carries a real sessionId from createSession. + const sessionId = (result as { toolTelemetry?: { sessionId?: string } }) + .toolTelemetry?.sessionId; + expect(sessionId).toBeTruthy(); + expect(sessionId).toMatch(/^embedder-session-/); + expect((result as { textResultForLlm: string }).textResultForLlm).toContain( + 'fenster', + ); + expect(deps.compileCharter).toHaveBeenCalledWith('fenster'); + expect(deps.createSession).toHaveBeenCalledOnce(); + }); + + it('returns documented failure shape when constructed without fan-out deps (regression guard for #9)', async () => { + // Pre-fix behavior: handler returned fake `success` with no session. + // Post-fix: explicit failure with `error: "fan-out-deps-unavailable"` + // and remediation text naming the fix (configure fanOutDepsGetter). + const registry = new ToolRegistry('./.squad-sdk-contract'); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: 'fenster', task: 'Implement feature X' } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ + resultType: 'failure', + error: 'fan-out-deps-unavailable', + }); + const text = (result as { textResultForLlm: string }).textResultForLlm; + expect(text).toContain('fenster'); + // Remediation must point embedders at the fix. + expect(text).toMatch(/fanOutDepsGetter|onPreToolUse/); + }); + + it('returns the documented failure shape when fanOutDepsGetter returns undefined', async () => { + const registry = new ToolRegistry( + './.squad-sdk-contract', + undefined, + undefined, + undefined, + () => undefined, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: 'fenster', task: 'Implement feature X' } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ + resultType: 'failure', + error: 'fan-out-deps-unavailable', + }); + }); + + it('surfaces underlying spawn failures with stable error code', async () => { + const deps = buildEmbedderFanOutDeps({ + compileCharter: vi.fn(async () => { + throw new Error('charter-not-found'); + }), + }); + const registry = new ToolRegistry( + './.squad-sdk-contract', + undefined, + undefined, + undefined, + () => deps, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: 'ghost', task: 'Do thing' } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ resultType: 'failure' }); + // Error field uses stable codes, not raw internal messages + const error = (result as { error: string }).error; + expect(['spawn-failed', 'spawn-exception']).toContain(error); + // Raw error details must not leak in structured fields + expect(error).not.toContain('/'); + expect(error).not.toContain('\n'); + }); + + it('rejects targetAgent with invalid characters', async () => { + const deps = buildEmbedderFanOutDeps(); + const registry = new ToolRegistry( + './.squad-sdk-contract', + undefined, + undefined, + undefined, + () => deps, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: '../evil-path', task: 'pwn' } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ + resultType: 'failure', + error: 'invalid-agent-name', + }); + expect(deps.compileCharter).not.toHaveBeenCalled(); + }); + + it('rejects targetAgent with spaces', async () => { + const deps = buildEmbedderFanOutDeps(); + const registry = new ToolRegistry( + './.squad-sdk-contract', + undefined, + undefined, + undefined, + () => deps, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: 'agent with spaces', task: 'test' } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ + resultType: 'failure', + error: 'invalid-agent-name', + }); + }); + + it('trims and accepts a valid targetAgent name', async () => { + const deps = buildEmbedderFanOutDeps(); + const registry = new ToolRegistry( + './.squad-sdk-contract', + undefined, + undefined, + undefined, + () => deps, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: ' fenster ', task: 'test trim' } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ resultType: 'success' }); + expect(deps.compileCharter).toHaveBeenCalledWith('fenster'); + }); + + it('sanitizes spawn error messages (no raw squadRoot paths in textResultForLlm)', async () => { + const squadRoot = './.squad-sdk-contract'; + const deps = buildEmbedderFanOutDeps({ + compileCharter: vi.fn(async () => { + throw new Error(`ENOENT: ${squadRoot}/agents/ghost/charter.md not found`); + }), + }); + const registry = new ToolRegistry( + squadRoot, + undefined, + undefined, + undefined, + () => deps, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: 'ghost', task: 'test' } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ resultType: 'failure' }); + const text = (result as { textResultForLlm: string }).textResultForLlm; + // sanitizeErrorForLlm replaces squadRoot with [team-root] + expect(text).not.toContain(squadRoot); + expect(text).toContain('[team-root]'); + }); + + it('lists squad_route in registry.getToolsForAgent (documented filter API)', () => { + // sdk.md §"ToolRegistry": + // registry.getToolsForAgent(['squad_route', 'squad_decide']); + const registry = new ToolRegistry('./.squad-sdk-contract'); + const filtered = registry.getToolsForAgent(['squad_route', 'squad_decide']); + + expect(filtered.map((t) => t.name).sort()).toEqual([ + 'squad_decide', + 'squad_route', + ]); + }); + + it('returns agent-not-in-roster when state is provided and agent is unknown', async () => { + const deps = buildEmbedderFanOutDeps(); + const mockState = { + agents: { + get: (_name: string) => ({ + charter: vi.fn(async () => { throw new Error('not found'); }), + }), + }, + } as any; + const registry = new ToolRegistry( + './.squad-sdk-contract', + undefined, + undefined, + mockState, + () => deps, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: 'nonexistent', task: 'test' } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ + resultType: 'failure', + error: 'agent-not-in-roster', + }); + // Should NOT proceed to fan-out + expect(deps.compileCharter).not.toHaveBeenCalled(); + }); + + it('proceeds to spawn when state is provided and agent exists in roster', async () => { + const deps = buildEmbedderFanOutDeps(); + const mockState = { + agents: { + get: (_name: string) => ({ + charter: vi.fn(async () => '# Mock Charter'), + }), + }, + } as any; + const registry = new ToolRegistry( + './.squad-sdk-contract', + undefined, + undefined, + mockState, + () => deps, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: 'fenster', task: 'test roster pass' } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ resultType: 'success' }); + expect(deps.compileCharter).toHaveBeenCalledWith('fenster'); + }); + + it('catches true spawn-exception from fanOutDepsGetter throwing', async () => { + const registry = new ToolRegistry( + './.squad-sdk-contract', + undefined, + undefined, + undefined, + () => { + throw new Error('unexpected infra failure'); + }, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: 'fenster', task: 'test' } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ + resultType: 'failure', + error: 'fan-out-deps-unavailable', + }); + const text = (result as { textResultForLlm: string }).textResultForLlm; + expect(text).toContain('fenster'); + expect(text).toContain('unexpected infra failure'); + }); + + it('does not leak raw errors in structured fields', async () => { + const squadRoot = './.squad-sdk-contract'; + const deps = buildEmbedderFanOutDeps({ + compileCharter: vi.fn(async () => { + throw new Error(`ENOENT: ${squadRoot}/internal/secret.key\nstack trace line 1\nstack trace line 2`); + }), + }); + const registry = new ToolRegistry( + squadRoot, + undefined, + undefined, + undefined, + () => deps, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: 'ghost', task: 'test' } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ resultType: 'failure' }); + // Stable error code only + const error = (result as { error: string }).error; + expect(['spawn-failed', 'spawn-exception']).toContain(error); + // textResultForLlm sanitized + const text = (result as { textResultForLlm: string }).textResultForLlm; + expect(text).not.toContain(squadRoot); + expect(text).not.toContain('stack trace'); + // toolTelemetry must not contain raw spawnResult + const telemetry = (result as { toolTelemetry?: any }).toolTelemetry; + expect(telemetry?.spawnResult).toBeUndefined(); + }); + + it('returns roster-check-failed when state.agents throws an infrastructure error', async () => { + const deps = buildEmbedderFanOutDeps(); + const mockState = { + agents: { + get: (_name: string) => ({ + charter: vi.fn(async () => { throw new Error('EACCES: permission denied'); }), + }), + }, + } as any; + const registry = new ToolRegistry( + './.squad-sdk-contract', + undefined, + undefined, + mockState, + () => deps, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: 'fenster', task: 'test' } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ + resultType: 'failure', + error: 'roster-check-failed', + }); + // Should NOT proceed to fan-out + expect(deps.compileCharter).not.toHaveBeenCalled(); + }); + + it('normalizes mixed-case targetAgent to lowercase before roster check and spawn', async () => { + const deps = buildEmbedderFanOutDeps(); + const registry = new ToolRegistry( + './.squad-sdk-contract', + undefined, + undefined, + undefined, + () => deps, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: 'McManus', task: 'test case normalization' } as RouteRequest, + ctx, + ); + + expect(result).toMatchObject({ resultType: 'success' }); + // Must be lowercased when passed to compileCharter + expect(deps.compileCharter).toHaveBeenCalledWith('mcmanus'); + }); + + it('returns spawn-exception when spawnParallel throws unexpectedly', async () => { + // Create deps where sessionPool.add throws (bypasses spawnSingle's internal catch + // because it happens after createSession succeeds but before the result is returned) + const deps = buildEmbedderFanOutDeps({ + createSession: vi.fn(async () => ({ + sessionId: 'test-session', + sendMessage: vi.fn(async () => undefined), + })), + }); + // Replace sessionPool with one that throws on add() + (deps.sessionPool as any).add = () => { throw new TypeError('Cannot read properties of undefined'); }; + + const registry = new ToolRegistry( + './.squad-sdk-contract', + undefined, + undefined, + undefined, + () => deps, + ); + const tool = registry.getTool('squad_route')!; + + const result = await tool.handler( + { targetAgent: 'fenster', task: 'test spawn exception' } as RouteRequest, + ctx, + ); + + // spawnParallel catches via allSettled, so this hits spawn-failed path + // The key assertion: error is a stable code, not a raw message + expect(result).toMatchObject({ resultType: 'failure' }); + const error = (result as { error: string }).error; + expect(['spawn-failed', 'spawn-exception']).toContain(error); + // No raw TypeError leaks in error field + expect(error).not.toContain('Cannot read'); + }); +}); diff --git a/test/tools.test.ts b/test/tools.test.ts index 8cf3ac3fb..3d5b619f0 100644 --- a/test/tools.test.ts +++ b/test/tools.test.ts @@ -11,11 +11,47 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { ToolRegistry, defineTool, type RouteRequest, type DecisionRecord, type MemoryEntry } from '@bradygaster/squad-sdk/tools'; -import { SessionPool } from '@bradygaster/squad-sdk/client'; +import { SessionPool, EventBus } from '@bradygaster/squad-sdk/client'; +import type { FanOutDependencies } from '@bradygaster/squad-sdk/coordinator'; +import type { AgentCharter } from '@bradygaster/squad-sdk/agents'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { randomUUID } from 'node:crypto'; +/** + * Build a fully-mocked FanOutDependencies suitable for squad_route tests. + * Mirrors test/fan-out.test.ts mock style so any future change to the + * fan-out contract surfaces in both places. + */ +function buildMockFanOutDeps(overrides: Partial = {}): FanOutDependencies { + const eventBus = overrides.eventBus ?? new EventBus(); + const sessionPool = overrides.sessionPool ?? new SessionPool({ + maxConcurrent: 10, + idleTimeout: 60000, + healthCheckInterval: 30000, + }); + return { + compileCharter: overrides.compileCharter ?? vi.fn(async (agentName: string) => ({ + name: agentName, + displayName: `${agentName} Agent`, + role: 'Developer', + expertise: ['TypeScript'], + style: 'Professional', + prompt: `You are ${agentName}`, + modelPreference: 'claude-sonnet-4.5', + } as AgentCharter)), + resolveModel: overrides.resolveModel ?? vi.fn(async (charter: AgentCharter, override?: string) => + override ?? charter.modelPreference ?? 'claude-sonnet-4.5' + ), + createSession: overrides.createSession ?? vi.fn(async () => ({ + sessionId: `session-${Math.random().toString(36).slice(2, 11)}`, + sendMessage: vi.fn(async () => undefined), + })), + sessionPool, + eventBus, + }; +} + describe('defineTool', () => { it('should create a typed SquadTool', () => { const tool = defineTool({ @@ -153,13 +189,8 @@ describe('ToolRegistry', () => { }); describe('squad_route handler', () => { - let registry: ToolRegistry; - - beforeEach(() => { - registry = new ToolRegistry('.test-squad-route'); - }); - it('should validate target agent is required', async () => { + const registry = new ToolRegistry('.test-squad-route'); const tool = registry.getTool('squad_route')!; const result = await tool.handler( { targetAgent: '', task: 'Do something' } as RouteRequest, @@ -177,7 +208,35 @@ describe('squad_route handler', () => { }); }); - it('should create route request with valid inputs', async () => { + it('should fail with fan-out-deps-unavailable when no fanOutDepsGetter is configured', async () => { + // Default ToolRegistry has no fanOutDepsGetter — must not fake success. + const registry = new ToolRegistry('.test-squad-route'); + const tool = registry.getTool('squad_route')!; + const result = await tool.handler( + { targetAgent: 'fenster', task: 'Implement feature X' } as RouteRequest, + { + sessionId: 'test-session', + toolCallId: 'test-call', + toolName: 'squad_route', + arguments: {}, + } + ); + + expect(result).toMatchObject({ + resultType: 'failure', + error: 'fan-out-deps-unavailable', + }); + expect((result as any).textResultForLlm).toContain('fenster'); + expect((result as any).toolTelemetry.routeRequest).toMatchObject({ + targetAgent: 'fenster', + task: 'Implement feature X', + priority: 'normal', + }); + }); + + it('should spawn target agent via spawnParallel when fanOutDepsGetter is configured', async () => { + const deps = buildMockFanOutDeps(); + const registry = new ToolRegistry('.test-squad-route', undefined, undefined, undefined, () => deps); const tool = registry.getTool('squad_route')!; const result = await tool.handler( { @@ -194,20 +253,65 @@ describe('squad_route handler', () => { } ); - expect(result).toMatchObject({ - resultType: 'success', - }); + expect(result).toMatchObject({ resultType: 'success' }); expect((result as any).textResultForLlm).toContain('fenster'); - expect((result as any).textResultForLlm).toContain('high'); + expect((result as any).toolTelemetry.sessionId).toBeDefined(); + expect((result as any).toolTelemetry.routeRequest).toMatchObject({ + targetAgent: 'fenster', + priority: 'high', + context: 'Related to PRD-2', + }); + expect(deps.compileCharter).toHaveBeenCalledWith('fenster'); + expect(deps.createSession).toHaveBeenCalledOnce(); }); - it('should default priority to normal', async () => { + it('should default priority to normal when omitted', async () => { + const deps = buildMockFanOutDeps(); + const registry = new ToolRegistry('.test-squad-route', undefined, undefined, undefined, () => deps); const tool = registry.getTool('squad_route')!; const result = await tool.handler( + { targetAgent: 'brady', task: 'Review code' } as RouteRequest, { - targetAgent: 'brady', - task: 'Review code', - } as RouteRequest, + sessionId: 'test-session', + toolCallId: 'test-call', + toolName: 'squad_route', + arguments: {}, + } + ); + + expect(result).toMatchObject({ resultType: 'success' }); + expect((result as any).toolTelemetry.routeRequest.priority).toBe('normal'); + }); + + it('should surface failure when underlying spawn fails', async () => { + const deps = buildMockFanOutDeps({ + compileCharter: vi.fn(async () => { + throw new Error('charter-not-found'); + }), + }); + const registry = new ToolRegistry('.test-squad-route', undefined, undefined, undefined, () => deps); + const tool = registry.getTool('squad_route')!; + const result = await tool.handler( + { targetAgent: 'ghost', task: 'Do thing' } as RouteRequest, + { + sessionId: 'test-session', + toolCallId: 'test-call', + toolName: 'squad_route', + arguments: {}, + } + ); + + expect(result).toMatchObject({ resultType: 'failure' }); + expect(['spawn-failed', 'spawn-exception']).toContain((result as any).error); + // toolTelemetry must not expose raw spawnResult + expect((result as any).toolTelemetry.spawnResult).toBeUndefined(); + }); + + it('should fail with fan-out-deps-unavailable when fanOutDepsGetter returns undefined', async () => { + const registry = new ToolRegistry('.test-squad-route', undefined, undefined, undefined, () => undefined); + const tool = registry.getTool('squad_route')!; + const result = await tool.handler( + { targetAgent: 'fenster', task: 'Implement feature X' } as RouteRequest, { sessionId: 'test-session', toolCallId: 'test-call', @@ -217,9 +321,9 @@ describe('squad_route handler', () => { ); expect(result).toMatchObject({ - resultType: 'success', + resultType: 'failure', + error: 'fan-out-deps-unavailable', }); - expect((result as any).toolTelemetry.routeRequest.priority).toBe('normal'); }); });