From 9475d94ea4ad8041920e330e503a1b497602e685 Mon Sep 17 00:00:00 2001 From: likiosliu Date: Wed, 22 Jul 2026 23:07:14 +0800 Subject: [PATCH] feat(sicore-a2a-mcp): support multiple named A2A keys selected by alias One adapter process could previously bind only a single A2A key, so switching Siclaw agents meant editing config and restarting. Passing the key as a tool argument is not an option: credentials must never flow through the model context. Add configuration-side named keys, selected at call time by alias: - New SICLAW_A2A_KEYS env: JSON {alias: key}. Aliases match ^[a-z0-9][a-z0-9_-]{0,31}$. The single-key SICLAW_A2A_KEY / SICLAW_A2A_KEY_FILE form is unchanged and maps to the reserved "default" alias; the two forms may be combined. SICLAW_AGENT_ID still pins the default key and is rejected alongside SICLAW_A2A_KEYS. Every key is self-resolved once at startup and any failure aborts boot (fail-fast). - Every tool gains an optional "agent" alias argument (never a key). Tool descriptions are populated at startup with the configured aliases and their resolved agent ids. With one agent "agent" is optional; with several, a create/list call that omits it errors with the alias list rather than guessing. - task_id -> alias is tracked in process for the server lifetime, so wait/get/cancel auto-route to the creating key. A mismatched "agent" argument is overridden by the recorded creator and noted in the result. siclaw_list_tasks without "agent" aggregates across every key, tags each task with its alias, and rebuilds the map after a restart. Keys stay in configuration only: never a tool parameter, never logged, never present in an error message (errors carry aliases and agent ids). Route the new logic through an AgentRouter; keep SicoreA2aClient per-key. Existing tests updated to the router API; add router unit tests, multi-key routing tool tests, and a two-alias stdio e2e cold-start smoke. --- mcp/sicore-a2a-adapter/README.md | 175 ++++++++-- mcp/sicore-a2a-adapter/src/a2a-client.ts | 7 +- mcp/sicore-a2a-adapter/src/config.test.ts | 91 ++++- mcp/sicore-a2a-adapter/src/config.ts | 103 +++++- mcp/sicore-a2a-adapter/src/index.ts | 36 +- mcp/sicore-a2a-adapter/src/router.test.ts | 71 ++++ mcp/sicore-a2a-adapter/src/router.ts | 111 ++++++ mcp/sicore-a2a-adapter/src/server.test.ts | 7 +- mcp/sicore-a2a-adapter/src/server.ts | 32 +- mcp/sicore-a2a-adapter/src/stdio.e2e.test.ts | 68 ++++ mcp/sicore-a2a-adapter/src/tools.test.ts | 162 ++++++++- mcp/sicore-a2a-adapter/src/tools.ts | 350 +++++++++++++------ 12 files changed, 1025 insertions(+), 188 deletions(-) create mode 100644 mcp/sicore-a2a-adapter/src/router.test.ts create mode 100644 mcp/sicore-a2a-adapter/src/router.ts diff --git a/mcp/sicore-a2a-adapter/README.md b/mcp/sicore-a2a-adapter/README.md index f41e0617..1f190cb4 100644 --- a/mcp/sicore-a2a-adapter/README.md +++ b/mcp/sicore-a2a-adapter/README.md @@ -1,6 +1,7 @@ # Sicore A2A MCP Adapter -A minimal local stdio MCP server that lets Codex or Claude Code use one existing Siclaw agent through Sicore's A2A API. +A minimal local stdio MCP server that lets Codex or Claude Code drive one or more +existing Siclaw agents through Sicore's A2A API. The adapter is intentionally thin: @@ -8,15 +9,88 @@ The adapter is intentionally thin: Codex / Claude Code --stdio MCP--> this adapter --HTTPS A2A--> Sicore --> Siclaw Runtime --> the agent's existing AgentBox ``` -It does not hold cluster credentials, choose a Siclaw agent, or execute infrastructure tools. One adapter process is fixed to one `SICLAW_AGENT_ID` and one Sicore A2A key. +It does not hold cluster credentials, choose what a Siclaw agent does, or execute +infrastructure tools. + +## Multiple agents, one adapter + +Each Sicore A2A key is bound to exactly one Siclaw agent. Historically one adapter +process meant one key meant one agent, so switching agents meant editing config and +restarting. + +This adapter can hold several **named keys** at once. You give each key a short +alias in configuration; the model selects an agent by passing that alias as the +`agent` tool argument. The alias is the only agent selector that ever crosses the +model boundary. + +### Design + +- **Keys live only in configuration.** `SICLAW_A2A_KEYS` is a JSON object of + `{alias: key}`. The key material never appears in a tool schema, a tool + argument, a log line, or an error message. Errors name the *alias* and the + resolved *agent id*, never the key. +- **The model selects by alias, never by key.** Every tool takes an optional + `agent` argument constrained to the alias pattern `^[a-z0-9][a-z0-9_-]{0,31}$`. + This is the fix for the failure mode where a credential would otherwise have to + be pasted into the chat to change which agent answers. +- **Fail fast at startup.** Each configured key is self-resolved once at boot via + `GET /api/v1/a2a/self` (the same mechanism the single-key form already used). If + any key fails to resolve, the process exits with an error that names the failing + alias — no key is ever partially usable and silent. +- **No guessing.** With a single configured agent, `agent` may be omitted. With + several, a create/list call that omits `agent` returns an explicit error listing + the available aliases rather than picking one. +- **Task ownership is tracked, so follow-ups auto-route.** `task_id` and + `context_id` are per-key server-side resources. The adapter records which alias + created each `task_id` (in process memory, for the server's lifetime). + `siclaw_wait_task` / `siclaw_get_task` / `siclaw_cancel_task` therefore route to + the creating key automatically — you usually only pass `agent` on + `siclaw_investigate`. If a task op passes an `agent` that disagrees with the + recorded creator, the recorded creator wins and the result carries a + `routing_note` recording the override. +- **Aggregated recovery.** `siclaw_list_tasks` without `agent` (and with several + agents configured) queries every key and tags each task with its owning alias, + re-establishing the `task_id -> alias` map after a client restart. Passing + `agent` scopes the listing to that one agent and supports paging. + +### Backward compatibility + +The original single-key environment variables keep working unchanged and map to +the reserved alias `default`: + +- `SICLAW_A2A_KEY` / `SICLAW_A2A_KEY_FILE` alone → one agent named `default`. +- `SICLAW_AGENT_ID` still pins the `default` agent (for an older Sicore without + `/self`, or as a cross-check). It applies only to the single-key form; combining + it with `SICLAW_A2A_KEYS` is a configuration error, because each named key + resolves its own agent. +- The single-key form and `SICLAW_A2A_KEYS` may be combined; the single key takes + the `default` slot and a `default` alias inside `SICLAW_A2A_KEYS` is rejected as + a collision. ## Tools -- `siclaw_investigate`: create or continue an investigation and wait up to 50 seconds. Longer investigations continue server-side and are watched with `siclaw_wait_task`. -- `siclaw_wait_task`: wait up to 50 seconds on an existing task without submitting another investigation. Use it repeatedly as the same-turn watchdog. -- `siclaw_get_task`: get one immediate compact snapshot; terminal snapshots include the full result. +- `siclaw_investigate`: create or continue an investigation and wait up to 50 + seconds. Longer investigations continue server-side and are watched with + `siclaw_wait_task`. +- `siclaw_wait_task`: wait up to 50 seconds on an existing task without submitting + another investigation. Use it repeatedly as the same-turn watchdog. +- `siclaw_get_task`: get one immediate compact snapshot; terminal snapshots + include the full result. - `siclaw_cancel_task`: cancel a non-terminal task. -- `siclaw_list_tasks`: recover/list tasks scoped to the same agent and API key. +- `siclaw_list_tasks`: recover/list tasks; aggregates across all agents when + `agent` is omitted. + +Every tool takes an optional `agent` alias. Its description is populated at startup +with the configured aliases and their resolved agent ids, so the model does not +have to guess which alias maps to which agent. + +## Security + +- The key exists only in configuration/env (or a `0600` key file). It is never a + tool parameter, never logged, and never echoed in an error message. +- The model-visible surface carries aliases and resolved agent ids only. +- Diagnostic output on stderr reports the endpoint origin and `alias=agentId` + pairs, never key material. ## Build and test @@ -33,17 +107,26 @@ Requires Node.js 22.19 or newer. Required: - `SICORE_URL`: Sicore base URL, for example `https://sicore.example.com`. -- exactly one of: - - `SICLAW_A2A_KEY_FILE`: path to a file containing the key. On Unix it must have mode `0600` or stricter. - - `SICLAW_A2A_KEY`: direct environment fallback for ephemeral testing. +- at least one key, from any combination of: + - `SICLAW_A2A_KEYS`: JSON object of named keys, e.g. `{"sre":"sk-a","kb":"sk-b"}`. + Alias must match `^[a-z0-9][a-z0-9_-]{0,31}$`. + - `SICLAW_A2A_KEY_FILE`: path to a file containing a single key (the `default` + alias). On Unix it must have mode `0600` or stricter. + - `SICLAW_A2A_KEY`: direct single-key environment fallback for ephemeral testing + (the `default` alias). Optional: -- `SICLAW_AGENT_ID`: the agent UUID bound to the A2A key. Keys are per-agent, so by default the adapter resolves the agent from the key at startup (`GET /api/v1/a2a/self`). Set it explicitly to pin the agent as a cross-check, or when talking to an older Sicore without the `/self` endpoint. -- `SICLAW_A2A_TIMEOUT_MS`: network-operation timeout, default `30000`. Bounded GET retries share this same budget. +- `SICLAW_AGENT_ID`: the agent UUID bound to the single `default` key. By default + the adapter resolves the agent from each key at startup (`GET /api/v1/a2a/self`). + Set it to pin the `default` agent as a cross-check or for an older Sicore without + `/self`. It cannot be combined with `SICLAW_A2A_KEYS`. +- `SICLAW_A2A_TIMEOUT_MS`: network-operation timeout, default `30000`. Bounded GET + retries share this same budget. - `SICLAW_A2A_POLL_INTERVAL_MS`: task polling interval, default `3000`. -The key must never be passed as a command-line argument or committed to a project MCP config. Prefer a private key file outside the repository: +Keys must never be passed as a command-line argument or committed to a project MCP +config. Prefer a private key file outside the repository for the single-key form: ```bash mkdir -p ~/.config/siclaw @@ -54,25 +137,40 @@ unset SICLAW_TEST_KEY chmod 600 ~/.config/siclaw/test-a2a-key ``` +For `SICLAW_A2A_KEYS`, keep the JSON in a private env file the MCP client loads, +not in a shared/committed config. + ## Run directly +Single key: + ```bash SICORE_URL=https://sicore.example.com \ SICLAW_A2A_KEY_FILE=~/.config/siclaw/test-a2a-key \ node dist/index.js ``` -The process speaks MCP over stdout. Diagnostic startup messages go only to stderr and never include the key. +Multiple named keys: + +```bash +SICORE_URL=https://sicore.example.com \ +SICLAW_A2A_KEYS='{"sre":"sk-a","kb":"sk-b"}' \ +node dist/index.js +``` + +The process speaks MCP over stdout. Diagnostic startup messages go only to stderr +and never include a key. ## Codex -Use absolute paths so the MCP process does not depend on the current project directory: +Use absolute paths so the MCP process does not depend on the current project +directory: ```bash codex mcp add \ --env SICORE_URL=https://sicore.example.com \ - --env SICLAW_A2A_KEY_FILE=/absolute/path/to/test-a2a-key \ - siclaw-test -- node /absolute/path/to/sicore-a2a-adapter/dist/index.js + --env SICLAW_A2A_KEYS='{"sre":"sk-a","kb":"sk-b"}' \ + siclaw -- node /absolute/path/to/sicore-a2a-adapter/dist/index.js ``` ## Claude Code @@ -80,19 +178,22 @@ codex mcp add \ ```bash claude mcp add -s user \ -e SICORE_URL=https://sicore.example.com \ - -e SICLAW_A2A_KEY_FILE=/absolute/path/to/test-a2a-key \ - siclaw-test -- node /absolute/path/to/sicore-a2a-adapter/dist/index.js + -e SICLAW_A2A_KEYS='{"sre":"sk-a","kb":"sk-b"}' \ + siclaw -- node /absolute/path/to/sicore-a2a-adapter/dist/index.js ``` -Create one named MCP server per Siclaw agent/key pair. The model-visible tool schemas intentionally contain no `agent_id` parameter, and the adapter derives the agent from the key itself (add `-e SICLAW_AGENT_ID=` to pin it explicitly or for an older Sicore). +The model-visible tool schemas contain no key parameter and no `agent_id` +parameter — only an `agent` alias. With one key you can keep the original +single-key form and omit `agent`; with several, the model passes the alias. ## Watchdog behavior -Sicore owns the durable background task. The local adapter stays stateless and -does not run a persistent daemon. When `siclaw_investigate` returns a working -task, the MCP client should keep the current turn open and call -`siclaw_wait_task` with the same `task_id` until the task is terminal, the user -asks it to stop, or an overall investigation deadline is exhausted. +Sicore owns the durable background task. The local adapter stays stateless across +restarts (the only in-memory state is the `task_id -> alias` map, which +`siclaw_list_tasks` rebuilds). When `siclaw_investigate` returns a working task, +the MCP client should keep the current turn open and call `siclaw_wait_task` with +the same `task_id` until the task is terminal, the user asks it to stop, or an +overall investigation deadline is exhausted. Working responses contain only the latest status, timestamp, and accumulated character count. The growing partial report is deliberately withheld so each @@ -106,9 +207,25 @@ previous conversation. ## Failure behavior -- A2A `message:send` is never automatically retried, avoiding duplicate investigation tasks. -- Idempotent task reads retry transient timeouts, connection failures, malformed responses, HTTP 408/429, and HTTP 500/502/503/504 at most twice with short backoff. Retries stay inside the read or watchdog deadline. +- A2A `message:send` is never automatically retried, avoiding duplicate + investigation tasks. +- Idempotent task reads retry transient timeouts, connection failures, malformed + responses, HTTP 408/429, and HTTP 500/502/503/504 at most twice with short + backoff. Retries stay inside the read or watchdog deadline. - Task cancellation is never automatically retried. -- If a bounded wait expires, the tool returns a compact working task; call `siclaw_wait_task` again instead of resubmitting the question. -- HTTP 401/403/429/503 and A2A error reasons are returned as MCP tool errors without request headers or key material. -- Adapter restarts do not lose server-side tasks; recover them with `siclaw_list_tasks` using the same key. +- If a bounded wait expires, the tool returns a compact working task; call + `siclaw_wait_task` again instead of resubmitting the question. +- HTTP 401/403/429/503 and A2A error reasons are returned as MCP tool errors + without request headers or key material. +- Adapter restarts do not lose server-side tasks; recover them with + `siclaw_list_tasks` using the same keys. + +## Relation to the remote Sicore MCP endpoint + +Sicore also exposes a first-party remote MCP endpoint (`/api/v1/mcp`, stateless +streamable HTTP). That endpoint is out of scope for this adapter: each remote MCP +client configuration carries its own `Authorization` header, so multi-agent use +there is just multiple client configurations, one per agent — no alias multiplexing +is needed. This local stdio adapter exists for clients that inject credentials from +env/files rather than per-request headers, and it is where named-key aliasing +applies. diff --git a/mcp/sicore-a2a-adapter/src/a2a-client.ts b/mcp/sicore-a2a-adapter/src/a2a-client.ts index d2689280..da9606d1 100644 --- a/mcp/sicore-a2a-adapter/src/a2a-client.ts +++ b/mcp/sicore-a2a-adapter/src/a2a-client.ts @@ -1,5 +1,8 @@ import { setTimeout as delay } from "node:timers/promises"; -import type { AdapterConfig, ResolvedAdapterConfig } from "./config.js"; +import type { ResolvedAdapterConfig } from "./config.js"; + +/** The subset of config resolveAgentId needs; each named key supplies its own. */ +export type AgentResolveInput = Pick; export const TERMINAL_A2A_STATES = new Set([ "TASK_STATE_COMPLETED", @@ -278,7 +281,7 @@ async function retryIdempotentRead( // from copying the agent UUID into client config. Older Sicore deployments // without the endpoint require SICLAW_AGENT_ID to be set explicitly. export async function resolveAgentId( - config: AdapterConfig, + config: AgentResolveInput, fetchImpl: typeof fetch = globalThis.fetch, ): Promise { let payload: { agentId?: unknown } | null; diff --git a/mcp/sicore-a2a-adapter/src/config.test.ts b/mcp/sicore-a2a-adapter/src/config.test.ts index 18e92e35..a9df50fa 100644 --- a/mcp/sicore-a2a-adapter/src/config.test.ts +++ b/mcp/sicore-a2a-adapter/src/config.test.ts @@ -20,18 +20,97 @@ function baseEnv(extra: NodeJS.ProcessEnv = {}): NodeJS.ProcessEnv { } describe("loadConfig", () => { - it("loads and normalizes environment configuration", () => { + it("loads and normalizes the single-key form as the default alias", () => { expect(loadConfig(baseEnv())).toEqual({ baseUrl: "https://sicore.example.com", - agentId: "agent-1", - apiKey: "test-key", + keys: [{ alias: "default", apiKey: "test-key", agentId: "agent-1" }], requestTimeoutMs: 30_000, pollIntervalMs: 3_000, }); }); it("treats SICLAW_AGENT_ID as optional for key self-resolution", () => { - expect(loadConfig(baseEnv({ SICLAW_AGENT_ID: undefined })).agentId).toBeUndefined(); + expect(loadConfig(baseEnv({ SICLAW_AGENT_ID: undefined })).keys).toEqual([ + { alias: "default", apiKey: "test-key", agentId: undefined }, + ]); + }); + + it("parses SICLAW_A2A_KEYS into named keys in declaration order", () => { + const env = baseEnv({ + SICLAW_A2A_KEY: undefined, + SICLAW_AGENT_ID: undefined, + SICLAW_A2A_KEYS: '{"sre":"sk-a","kb":"sk-b"}', + }); + expect(loadConfig(env).keys).toEqual([ + { alias: "sre", apiKey: "sk-a" }, + { alias: "kb", apiKey: "sk-b" }, + ]); + }); + + it("merges the single key as default alongside named keys", () => { + const env = baseEnv({ + SICLAW_AGENT_ID: undefined, + SICLAW_A2A_KEYS: '{"kb":"sk-b"}', + }); + expect(loadConfig(env).keys).toEqual([ + { alias: "default", apiKey: "test-key", agentId: undefined }, + { alias: "kb", apiKey: "sk-b" }, + ]); + }); + + it("rejects a named alias colliding with the single-key default", () => { + const env = baseEnv({ + SICLAW_AGENT_ID: undefined, + SICLAW_A2A_KEYS: '{"default":"sk-b"}', + }); + expect(() => loadConfig(env)).toThrow(/collides with the single-key/); + }); + + it("rejects invalid alias names", () => { + const env = baseEnv({ + SICLAW_A2A_KEY: undefined, + SICLAW_AGENT_ID: undefined, + SICLAW_A2A_KEYS: '{"SRE":"sk-a"}', + }); + expect(() => loadConfig(env)).toThrow(/is invalid/); + }); + + it("rejects SICLAW_AGENT_ID combined with SICLAW_A2A_KEYS", () => { + const env = baseEnv({ + SICLAW_A2A_KEY: undefined, + SICLAW_A2A_KEYS: '{"sre":"sk-a"}', + }); + expect(() => loadConfig(env)).toThrow(/cannot be combined with SICLAW_A2A_KEYS/); + }); + + it("rejects malformed SICLAW_A2A_KEYS JSON without echoing values", () => { + const env = baseEnv({ + SICLAW_A2A_KEY: undefined, + SICLAW_AGENT_ID: undefined, + SICLAW_A2A_KEYS: "not-json", + }); + expect(() => loadConfig(env)).toThrow(/must be a JSON object/); + }); + + it("rejects a non-string key value without echoing it", () => { + const env = baseEnv({ + SICLAW_A2A_KEY: undefined, + SICLAW_AGENT_ID: undefined, + SICLAW_A2A_KEYS: '{"sre":123}', + }); + try { + loadConfig(env); + throw new Error("expected ConfigError"); + } catch (error) { + expect(error).toBeInstanceOf(ConfigError); + expect((error as Error).message).toContain("sre"); + expect((error as Error).message).not.toContain("123"); + } + }); + + it("requires at least one key", () => { + const env = baseEnv({ SICLAW_A2A_KEY: undefined, SICLAW_AGENT_ID: undefined }); + expect(() => loadConfig(env)).toThrow(/at least one key/); }); it("allows HTTP only for loopback testing", () => { @@ -47,8 +126,8 @@ describe("loadConfig", () => { const keyFile = join(dir, "key"); writeFileSync(keyFile, "file-key\n", { mode: 0o600 }); chmodSync(keyFile, 0o600); - const env = baseEnv({ SICLAW_A2A_KEY: undefined, SICLAW_A2A_KEY_FILE: keyFile }); - expect(loadConfig(env).apiKey).toBe("file-key"); + const env = baseEnv({ SICLAW_A2A_KEY: undefined, SICLAW_AGENT_ID: undefined, SICLAW_A2A_KEY_FILE: keyFile }); + expect(loadConfig(env).keys).toEqual([{ alias: "default", apiKey: "file-key", agentId: undefined }]); }); it.runIf(process.platform !== "win32")("rejects a group-readable key file", () => { diff --git a/mcp/sicore-a2a-adapter/src/config.ts b/mcp/sicore-a2a-adapter/src/config.ts index 5845dd87..63b92636 100644 --- a/mcp/sicore-a2a-adapter/src/config.ts +++ b/mcp/sicore-a2a-adapter/src/config.ts @@ -1,15 +1,31 @@ import { readFileSync, statSync } from "node:fs"; +/** One named A2A key. `agentId` is set only when pinned via SICLAW_AGENT_ID on the single-key path. */ +export interface NamedKey { + alias: string; + apiKey: string; + agentId?: string; +} + export interface AdapterConfig { baseUrl: string; - /** Absent when the agent should be resolved from the key via GET /api/v1/a2a/self. */ - agentId?: string; + keys: NamedKey[]; + requestTimeoutMs: number; + pollIntervalMs: number; +} + +/** Per-key config handed to one SicoreA2aClient after its agent is resolved. */ +export interface ResolvedAdapterConfig { + baseUrl: string; + agentId: string; apiKey: string; requestTimeoutMs: number; pollIntervalMs: number; } -export type ResolvedAdapterConfig = AdapterConfig & { agentId: string }; +export const ALIAS_PATTERN = "^[a-z0-9][a-z0-9_-]{0,31}$"; +const ALIAS_RE = new RegExp(ALIAS_PATTERN); +const DEFAULT_ALIAS = "default"; export class ConfigError extends Error { constructor(message: string) { @@ -84,7 +100,10 @@ function loadKeyFromFile(path: string): string { return key; } -function loadApiKey(env: NodeJS.ProcessEnv): string { +// The single-key form (SICLAW_A2A_KEY / SICLAW_A2A_KEY_FILE) is the original, +// backward-compatible way to configure exactly one agent. It maps to the +// reserved "default" alias so old configs keep working untouched. +function loadSingleKey(env: NodeJS.ProcessEnv): string | undefined { const direct = env.SICLAW_A2A_KEY?.trim(); const keyFile = env.SICLAW_A2A_KEY_FILE?.trim(); if (direct && keyFile) { @@ -92,18 +111,82 @@ function loadApiKey(env: NodeJS.ProcessEnv): string { } if (keyFile) return loadKeyFromFile(keyFile); if (direct) return direct; - throw new ConfigError("SICLAW_A2A_KEY or SICLAW_A2A_KEY_FILE is required"); + return undefined; } -export function loadConfig(env: NodeJS.ProcessEnv = process.env): AdapterConfig { - const agentId = env.SICLAW_AGENT_ID?.trim() || undefined; - if (agentId && Buffer.byteLength(agentId, "utf8") > 255) { +function parseKeysJson(raw: string): Array<[string, string]> { + let obj: unknown; + try { + obj = JSON.parse(raw); + } catch { + throw new ConfigError('SICLAW_A2A_KEYS must be a JSON object mapping alias to key, e.g. {"sre":"sk-..."}'); + } + if (obj === null || typeof obj !== "object" || Array.isArray(obj)) { + throw new ConfigError('SICLAW_A2A_KEYS must be a JSON object mapping alias to key, e.g. {"sre":"sk-..."}'); + } + const entries: Array<[string, string]> = []; + for (const [alias, value] of Object.entries(obj)) { + if (typeof value !== "string" || !value.trim()) { + // Do not echo the value; a malformed key must never surface in an error. + throw new ConfigError(`SICLAW_A2A_KEYS["${alias}"] must be a non-empty string key`); + } + entries.push([alias, value.trim()]); + } + if (entries.length === 0) { + throw new ConfigError("SICLAW_A2A_KEYS must contain at least one alias"); + } + return entries; +} + +function loadKeys(env: NodeJS.ProcessEnv): NamedKey[] { + const single = loadSingleKey(env); + const multiRaw = env.SICLAW_A2A_KEYS?.trim(); + const pinnedAgentId = env.SICLAW_AGENT_ID?.trim() || undefined; + if (pinnedAgentId && Buffer.byteLength(pinnedAgentId, "utf8") > 255) { throw new ConfigError("SICLAW_AGENT_ID must be 255 bytes or less"); } + + const byAlias = new Map(); + + if (single !== undefined) { + // SICLAW_AGENT_ID only pins the default single key. Every SICLAW_A2A_KEYS + // entry resolves its own agent at startup, so pinning one id there is + // ambiguous and rejected below. + byAlias.set(DEFAULT_ALIAS, { alias: DEFAULT_ALIAS, apiKey: single, agentId: pinnedAgentId }); + } + + if (multiRaw) { + if (pinnedAgentId) { + throw new ConfigError( + "SICLAW_AGENT_ID cannot be combined with SICLAW_A2A_KEYS; each named key resolves its own agent", + ); + } + for (const [alias, key] of parseKeysJson(multiRaw)) { + if (!ALIAS_RE.test(alias)) { + throw new ConfigError(`SICLAW_A2A_KEYS alias "${alias}" is invalid; must match ${ALIAS_PATTERN}`); + } + if (byAlias.has(alias)) { + // The only possible collision is with the "default" single key. + throw new ConfigError( + `SICLAW_A2A_KEYS alias "${alias}" collides with the single-key SICLAW_A2A_KEY/SICLAW_A2A_KEY_FILE (reserved as "default"); remove one`, + ); + } + byAlias.set(alias, { alias, apiKey: key }); + } + } + + if (byAlias.size === 0) { + throw new ConfigError( + "Configure at least one key via SICLAW_A2A_KEYS, SICLAW_A2A_KEY, or SICLAW_A2A_KEY_FILE", + ); + } + return [...byAlias.values()]; +} + +export function loadConfig(env: NodeJS.ProcessEnv = process.env): AdapterConfig { return { baseUrl: normalizeBaseUrl(required(env, "SICORE_URL")), - agentId, - apiKey: loadApiKey(env), + keys: loadKeys(env), requestTimeoutMs: parseBoundedInteger(env, "SICLAW_A2A_TIMEOUT_MS", 30_000, 1_000, 120_000), pollIntervalMs: parseBoundedInteger(env, "SICLAW_A2A_POLL_INTERVAL_MS", 3_000, 500, 5_000), }; diff --git a/mcp/sicore-a2a-adapter/src/index.ts b/mcp/sicore-a2a-adapter/src/index.ts index b6c69327..df3a3136 100644 --- a/mcp/sicore-a2a-adapter/src/index.ts +++ b/mcp/sicore-a2a-adapter/src/index.ts @@ -1,15 +1,41 @@ #!/usr/bin/env node import { resolveAgentId, SicoreA2aClient } from "./a2a-client.js"; -import { loadConfig } from "./config.js"; +import { loadConfig, type AdapterConfig, type NamedKey } from "./config.js"; +import { AgentRouter, type AgentEntry } from "./router.js"; import { serveStdio } from "./server.js"; +async function buildEntry(config: AdapterConfig, key: NamedKey): Promise { + const shared = { + baseUrl: config.baseUrl, + apiKey: key.apiKey, + requestTimeoutMs: config.requestTimeoutMs, + pollIntervalMs: config.pollIntervalMs, + }; + let agentId: string; + try { + agentId = key.agentId ?? await resolveAgentId(shared); + } catch (error) { + // Prefix with the alias (never the key) so the operator knows which entry failed. + throw new Error(`agent "${key.alias}": ${error instanceof Error ? error.message : String(error)}`); + } + const client = new SicoreA2aClient({ ...shared, agentId }); + return { alias: key.alias, agentId, api: client }; +} + async function main(): Promise { const config = loadConfig(); - const agentId = config.agentId ?? await resolveAgentId(config); - const server = await serveStdio(new SicoreA2aClient({ ...config, agentId })); + const entries: AgentEntry[] = []; + for (const key of config.keys) { + entries.push(await buildEntry(config, key)); + } + const router = new AgentRouter(entries); + const server = await serveStdio(router); + + const agentList = entries.map((entry) => `${entry.alias}=${entry.agentId}`).join(", "); + const resolvedFromKey = config.keys.some((key) => !key.agentId); process.stderr.write( - `[sicore-a2a-mcp] ready agent=${agentId} endpoint=${new URL(config.baseUrl).origin}` - + `${config.agentId ? "" : " (agent resolved from key)"}\n`, + `[sicore-a2a-mcp] ready endpoint=${new URL(config.baseUrl).origin} agents=[${agentList}]` + + `${resolvedFromKey ? " (some agents resolved from key)" : ""}\n`, ); const shutdown = async (): Promise => { diff --git a/mcp/sicore-a2a-adapter/src/router.test.ts b/mcp/sicore-a2a-adapter/src/router.test.ts new file mode 100644 index 00000000..3824395e --- /dev/null +++ b/mcp/sicore-a2a-adapter/src/router.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it, vi } from "vitest"; +import type { SiclawA2aApi } from "./a2a-client.js"; +import { AgentRouter, RoutingError } from "./router.js"; + +function fakeApi(): SiclawA2aApi { + return { + sendMessage: vi.fn(), + getTask: vi.fn(), + cancelTask: vi.fn(), + listTasks: vi.fn(), + waitForTask: vi.fn(), + } as unknown as SiclawA2aApi; +} + +function router(aliases: string[]): AgentRouter { + return new AgentRouter(aliases.map((alias) => ({ alias, agentId: `agent-${alias}`, api: fakeApi() }))); +} + +describe("AgentRouter", () => { + it("rejects an empty or duplicated configuration", () => { + expect(() => new AgentRouter([])).toThrow(RoutingError); + expect(() => router(["sre", "sre"])).toThrow(/Duplicate agent alias/); + }); + + it("preserves alias order and describes agents by id, never a key", () => { + const r = router(["sre", "kb"]); + expect(r.aliases).toEqual(["sre", "kb"]); + expect(r.isSingle).toBe(false); + expect(r.describeAgents()).toBe("sre = agent-sre, kb = agent-kb"); + }); + + it("makes the agent optional for a single configured key", () => { + const r = router(["default"]); + expect(r.selectExplicit(undefined).alias).toBe("default"); + expect(r.selectExplicit("default").alias).toBe("default"); + expect(() => r.selectExplicit("other")).toThrow(/Unknown agent alias "other"/); + }); + + it("refuses to guess an agent for a create call when several exist", () => { + const r = router(["sre", "kb"]); + expect(() => r.selectExplicit(undefined)).toThrow(/Multiple Siclaw agents/); + expect(r.selectExplicit("kb").alias).toBe("kb"); + }); + + it("routes a task to its recorded creator regardless of the argument", () => { + const r = router(["sre", "kb"]); + r.remember("t1", "kb"); + expect(r.selectForTask("t1", undefined)).toEqual({ entry: expect.objectContaining({ alias: "kb" }) }); + + const mismatched = r.selectForTask("t1", "sre"); + expect(mismatched.entry.alias).toBe("kb"); + expect(mismatched.note).toMatch(/Routed to agent "kb".*ignored agent="sre"/); + }); + + it("still validates a bogus argument even when the mapping wins", () => { + const r = router(["sre", "kb"]); + r.remember("t1", "kb"); + expect(() => r.selectForTask("t1", "ghost")).toThrow(/Unknown agent alias "ghost"/); + }); + + it("requires an agent for an untracked task under multiple keys", () => { + const r = router(["sre", "kb"]); + expect(() => r.selectForTask("t9", undefined)).toThrow(/was not created in this session/); + expect(r.selectForTask("t9", "sre").entry.alias).toBe("sre"); + }); + + it("uses the sole agent for an untracked task under a single key", () => { + const r = router(["default"]); + expect(r.selectForTask("t9", undefined).entry.alias).toBe("default"); + }); +}); diff --git a/mcp/sicore-a2a-adapter/src/router.ts b/mcp/sicore-a2a-adapter/src/router.ts new file mode 100644 index 00000000..a70f7935 --- /dev/null +++ b/mcp/sicore-a2a-adapter/src/router.ts @@ -0,0 +1,111 @@ +import type { SiclawA2aApi } from "./a2a-client.js"; + +export interface AgentEntry { + alias: string; + agentId: string; + api: SiclawA2aApi; +} + +export class RoutingError extends Error { + constructor(message: string) { + super(message); + this.name = "RoutingError"; + } +} + +export interface TaskRoute { + entry: AgentEntry; + /** Set when the caller's `agent` argument was overridden by the recorded creator. */ + note?: string; +} + +// AgentRouter maps user-facing aliases to A2A clients and routes tool calls. +// task_id and context_id are per-key server resources, so the router also +// remembers which alias created each task_id (process-local, for the MCP +// server's lifetime). Errors never carry key material, only aliases and the +// agent ids resolved at startup. +export class AgentRouter { + private readonly entries: Map; + private readonly order: string[]; + private readonly taskAlias = new Map(); + + constructor(entries: AgentEntry[]) { + if (entries.length === 0) throw new RoutingError("At least one agent must be configured"); + this.entries = new Map(); + for (const entry of entries) { + if (this.entries.has(entry.alias)) { + throw new RoutingError(`Duplicate agent alias "${entry.alias}"`); + } + this.entries.set(entry.alias, entry); + } + this.order = entries.map((entry) => entry.alias); + } + + get aliases(): string[] { + return [...this.order]; + } + + get isSingle(): boolean { + return this.order.length === 1; + } + + listEntries(): AgentEntry[] { + return this.order.map((alias) => this.entries.get(alias)!); + } + + /** "sre = , kb = " for tool descriptions and error messages. */ + describeAgents(): string { + return this.order.map((alias) => `${alias} = ${this.entries.get(alias)!.agentId}`).join(", "); + } + + requireAlias(alias: string): AgentEntry { + const entry = this.entries.get(alias); + if (!entry) { + throw new RoutingError(`Unknown agent alias "${alias}". Configured agents: ${this.describeAgents()}`); + } + return entry; + } + + /** + * Pick the agent for a create/list call. With a single configured agent the + * argument is optional; with several, an absent argument is an error rather + * than a guess. + */ + selectExplicit(alias: string | undefined): AgentEntry { + if (alias !== undefined) return this.requireAlias(alias); + if (this.isSingle) return this.entries.get(this.order[0])!; + throw new RoutingError( + `Multiple Siclaw agents are configured; pass "agent" to choose one. Configured agents: ${this.describeAgents()}`, + ); + } + + /** + * Pick the agent for a task-scoped call (wait/get/cancel). The recorded + * creator wins over the caller's argument, because task_id belongs to the key + * that created it; when they disagree the returned note records the override. + */ + selectForTask(taskId: string, alias: string | undefined): TaskRoute { + const mapped = this.taskAlias.get(taskId); + if (mapped) { + const entry = this.entries.get(mapped)!; + if (alias !== undefined && alias !== mapped) { + this.requireAlias(alias); // validate for a clean message; mapping still wins + return { + entry, + note: `Routed to agent "${mapped}" that created this task; ignored agent="${alias}".`, + }; + } + return { entry }; + } + if (alias !== undefined) return { entry: this.requireAlias(alias) }; + if (this.isSingle) return { entry: this.entries.get(this.order[0])! }; + throw new RoutingError( + `Task "${taskId}" was not created in this session and multiple agents are configured; ` + + `pass "agent", or run siclaw_list_tasks first. Configured agents: ${this.describeAgents()}`, + ); + } + + remember(taskId: string, alias: string): void { + if (taskId) this.taskAlias.set(taskId, alias); + } +} diff --git a/mcp/sicore-a2a-adapter/src/server.test.ts b/mcp/sicore-a2a-adapter/src/server.test.ts index c847415f..affaa987 100644 --- a/mcp/sicore-a2a-adapter/src/server.test.ts +++ b/mcp/sicore-a2a-adapter/src/server.test.ts @@ -2,6 +2,7 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; import { afterEach, describe, expect, it, vi } from "vitest"; import type { SiclawA2aApi, SiclawTask } from "./a2a-client.js"; +import { AgentRouter } from "./router.js"; import { createMcpServer } from "./server.js"; const closeAfter: Array<{ close(): Promise }> = []; @@ -34,10 +35,14 @@ function fakeApi(): SiclawA2aApi { }; } +function singleRouter(api: SiclawA2aApi): AgentRouter { + return new AgentRouter([{ alias: "default", agentId: "agent-default", api }]); +} + describe("MCP server", () => { it("completes MCP initialization, lists tools, and calls Siclaw through the adapter", async () => { const api = fakeApi(); - const server = createMcpServer(api); + const server = createMcpServer(singleRouter(api)); const client = new Client({ name: "test-client", version: "1.0.0" }); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); closeAfter.push(client, server); diff --git a/mcp/sicore-a2a-adapter/src/server.ts b/mcp/sicore-a2a-adapter/src/server.ts index b65ccd66..c122898b 100644 --- a/mcp/sicore-a2a-adapter/src/server.ts +++ b/mcp/sicore-a2a-adapter/src/server.ts @@ -1,32 +1,44 @@ import { Server } from "@modelcontextprotocol/sdk/server/index.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; import { CallToolRequestSchema, ListToolsRequestSchema } from "@modelcontextprotocol/sdk/types.js"; -import type { SiclawA2aApi } from "./a2a-client.js"; -import { createToolHandler, TOOL_DEFINITIONS } from "./tools.js"; +import type { AgentRouter } from "./router.js"; +import { buildToolDefinitions, createToolHandler } from "./tools.js"; -export const SERVER_INSTRUCTIONS = [ +const BASE_INSTRUCTIONS = [ "Use siclaw_investigate for operational questions that require the configured Siclaw SRE agent.", "When it returns a non-terminal task, keep the current turn open and call siclaw_wait_task with the same task_id until terminal, unless the user requests fire-and-forget, asks to stop, or the overall investigation deadline is exhausted.", "Never resubmit the same question merely because a task is still working.", "Use siclaw_list_tasks to recover server-side tasks after a client restart.", -].join(" "); +]; -export function createMcpServer(api: SiclawA2aApi): Server { +export function buildInstructions(router: AgentRouter): string { + const lines = [...BASE_INSTRUCTIONS]; + if (!router.isSingle) { + lines.push( + `Multiple Siclaw agents are configured; pass the "agent" argument (an alias, never a key) to choose one: ${router.describeAgents()}.`, + "Task waits, snapshots, and cancels auto-route to the agent that created the task, so you usually only need \"agent\" on siclaw_investigate.", + ); + } + return lines.join(" "); +} + +export function createMcpServer(router: AgentRouter): Server { const server = new Server( { name: "sicore-a2a-mcp-adapter", version: "0.1.0" }, - { capabilities: { tools: {} }, instructions: SERVER_INSTRUCTIONS }, + { capabilities: { tools: {} }, instructions: buildInstructions(router) }, ); - const handleTool = createToolHandler(api); + const handleTool = createToolHandler(router); + const tools = buildToolDefinitions(router); - server.setRequestHandler(ListToolsRequestSchema, async () => ({ tools: [...TOOL_DEFINITIONS] })); + server.setRequestHandler(ListToolsRequestSchema, async () => ({ tools: [...tools] })); server.setRequestHandler(CallToolRequestSchema, async (request) => ( await handleTool(request.params.name, request.params.arguments ?? {}) as any )); return server; } -export async function serveStdio(api: SiclawA2aApi): Promise { - const server = createMcpServer(api); +export async function serveStdio(router: AgentRouter): Promise { + const server = createMcpServer(router); await server.connect(new StdioServerTransport()); return server; } diff --git a/mcp/sicore-a2a-adapter/src/stdio.e2e.test.ts b/mcp/sicore-a2a-adapter/src/stdio.e2e.test.ts index 6b6dce2d..73a73369 100644 --- a/mcp/sicore-a2a-adapter/src/stdio.e2e.test.ts +++ b/mcp/sicore-a2a-adapter/src/stdio.e2e.test.ts @@ -121,4 +121,72 @@ describe("stdio process", () => { expect(paths[0]).toBe("/api/v1/a2a/self"); expect(paths).toContain("/api/v1/a2a/agents/agent-resolved/message:send"); }); + + it("resolves several named keys, injects aliases into tool descriptions, and routes by alias", async () => { + const requests: Array<{ path: string; auth: string }> = []; + const mock = createServer(async (request, response) => { + const auth = request.headers.authorization ?? ""; + const path = request.url ?? ""; + requests.push({ path, auth }); + for await (const _chunk of request) { /* drain */ } + response.writeHead(200, { "content-type": "application/a2a+json" }); + if (path === "/api/v1/a2a/self") { + const agentId = auth === "Bearer secret-key-sre" + ? "agent-sre-id" + : auth === "Bearer secret-key-kb" ? "agent-kb-id" : "agent-unknown"; + response.end(JSON.stringify({ agentId })); + return; + } + response.end(JSON.stringify({ task: completedTask() })); + }); + httpServers.push(mock); + const port = await listen(mock); + + const adapterEntrypoint = fileURLToPath(new URL("./index.ts", import.meta.url)); + const transport = new StdioClientTransport({ + command: process.execPath, + args: ["--import", "tsx", adapterEntrypoint], + env: { + SICORE_URL: `http://127.0.0.1:${port}`, + SICLAW_A2A_KEYS: JSON.stringify({ sre: "secret-key-sre", kb: "secret-key-kb" }), + }, + stderr: "pipe", + }); + let stderr = ""; + transport.stderr?.on("data", (chunk) => { stderr += String(chunk); }); + const client = new Client({ name: "stdio-e2e-multi", version: "1.0.0" }); + clients.push(client); + await client.connect(transport); + + // Both keys are self-resolved once at startup. + const selfCalls = requests.filter((entry) => entry.path === "/api/v1/a2a/self"); + expect(selfCalls.map((entry) => entry.auth).sort()).toEqual([ + "Bearer secret-key-kb", + "Bearer secret-key-sre", + ]); + + const listed = await client.listTools(); + const investigate = listed.tools.find((tool) => tool.name === "siclaw_investigate")!; + expect(investigate.description).toContain("sre = agent-sre-id"); + expect(investigate.description).toContain("kb = agent-kb-id"); + // The description advertises aliases, never the keys. + expect(investigate.description).not.toContain("secret-key"); + expect(investigate.inputSchema.properties).toHaveProperty("agent"); + expect(investigate.inputSchema.properties).not.toHaveProperty("key"); + + const result = await client.callTool({ + name: "siclaw_investigate", + arguments: { question: "check kb", agent: "kb", wait_seconds: 0 }, + }); + expect(result.isError).not.toBe(true); + expect(result.structuredContent).toMatchObject({ task_id: "task-e2e", agent: "kb" }); + const sendCall = requests.find((entry) => entry.path.endsWith("/message:send")); + expect(sendCall?.path).toBe("/api/v1/a2a/agents/agent-kb-id/message:send"); + expect(sendCall?.auth).toBe("Bearer secret-key-kb"); + + // The diagnostic ready line reports agent ids, never key material. + expect(stderr).toContain("agents=["); + expect(stderr).toContain("agent-kb-id"); + expect(stderr).not.toContain("secret-key"); + }); }); diff --git a/mcp/sicore-a2a-adapter/src/tools.test.ts b/mcp/sicore-a2a-adapter/src/tools.test.ts index e0e4fcdf..fa79f5ef 100644 --- a/mcp/sicore-a2a-adapter/src/tools.test.ts +++ b/mcp/sicore-a2a-adapter/src/tools.test.ts @@ -1,10 +1,11 @@ import { describe, expect, it, vi } from "vitest"; import type { SiclawA2aApi, SiclawTask } from "./a2a-client.js"; -import { createToolHandler, TOOL_DEFINITIONS } from "./tools.js"; +import { AgentRouter } from "./router.js"; +import { buildToolDefinitions, createToolHandler } from "./tools.js"; -function task(state: SiclawTask["state"] = "working"): SiclawTask { +function task(state: SiclawTask["state"] = "working", taskId = "task-1"): SiclawTask { return { - task_id: "task-1", + task_id: taskId, context_id: "context-1", state, a2a_state: state === "completed" ? "TASK_STATE_COMPLETED" : "TASK_STATE_WORKING", @@ -16,34 +17,60 @@ function task(state: SiclawTask["state"] = "working"): SiclawTask { }; } -function fakeApi(): SiclawA2aApi { +function fakeApi(overrides: Partial = {}): SiclawA2aApi { return { sendMessage: vi.fn(async () => task()), getTask: vi.fn(async () => task("completed")), cancelTask: vi.fn(async () => ({ ...task(), state: "canceled", a2a_state: "TASK_STATE_CANCELED", is_terminal: true })), listTasks: vi.fn(async () => ({ tasks: [task()], total_size: 1, page_size: 20, next_page_token: null })), waitForTask: vi.fn(async () => task("completed")), + ...overrides, }; } +function router(entries: Array<[string, SiclawA2aApi]>): AgentRouter { + return new AgentRouter(entries.map(([alias, api]) => ({ alias, agentId: `agent-${alias}`, api }))); +} + +function singleRouter(api: SiclawA2aApi = fakeApi()): AgentRouter { + return router([["default", api]]); +} + describe("tool contract", () => { - it("keeps the configured agent out of every model-visible input schema", () => { - expect(TOOL_DEFINITIONS.map((tool) => tool.name)).toEqual([ + it("keeps agent aliases, never a key parameter, in every model-visible input schema", () => { + const defs = buildToolDefinitions(singleRouter()); + expect(defs.map((tool) => tool.name)).toEqual([ "siclaw_investigate", "siclaw_wait_task", "siclaw_get_task", "siclaw_cancel_task", "siclaw_list_tasks", ]); - for (const tool of TOOL_DEFINITIONS) { - expect(tool.inputSchema.properties).not.toHaveProperty("agent_id"); + for (const tool of defs) { + const properties = tool.inputSchema.properties as Record; + expect(properties).not.toHaveProperty("agent_id"); + expect(properties).not.toHaveProperty("key"); + expect(properties).not.toHaveProperty("api_key"); + expect(properties).toHaveProperty("agent"); expect(tool.inputSchema.additionalProperties).toBe(false); + const required = (tool.inputSchema as { required?: string[] }).required ?? []; + expect(required).not.toContain("agent"); } }); - it("submits and waits for a bounded investigation", async () => { + it("injects the configured aliases and agent ids into descriptions", () => { + const defs = buildToolDefinitions(router([["sre", fakeApi()], ["kb", fakeApi()]])); + const investigate = defs.find((tool) => tool.name === "siclaw_investigate")!; + expect(investigate.description).toContain("sre = agent-sre"); + expect(investigate.description).toContain("kb = agent-kb"); + expect(investigate.description).toMatch(/must pass "agent"/); + const list = defs.find((tool) => tool.name === "siclaw_list_tasks")!; + expect(list.description).toMatch(/aggregates tasks from every configured agent/); + }); + + it("submits and waits for a bounded investigation and tags the agent", async () => { const api = fakeApi(); - const handle = createToolHandler(api); + const handle = createToolHandler(singleRouter(api)); const result = await handle("siclaw_investigate", { question: "check node", context_id: "context-1", @@ -52,12 +79,12 @@ describe("tool contract", () => { expect(api.sendMessage).toHaveBeenCalledWith("check node", "context-1"); expect(api.waitForTask).toHaveBeenCalledWith("task-1", 5); expect(result.isError).toBeUndefined(); - expect(result.structuredContent).toMatchObject({ state: "completed", result: "root cause" }); + expect(result.structuredContent).toMatchObject({ state: "completed", result: "root cause", agent: "default" }); }); it("maps simple list status to the A2A enum", async () => { const api = fakeApi(); - const handle = createToolHandler(api); + const handle = createToolHandler(singleRouter(api)); const result = await handle("siclaw_list_tasks", { status: "working", page_size: 10 }); expect(api.listTasks).toHaveBeenCalledWith({ contextId: undefined, @@ -70,14 +97,14 @@ describe("tool contract", () => { it("waits on an existing task without submitting another investigation", async () => { const api = fakeApi(); - const handle = createToolHandler(api); + const handle = createToolHandler(singleRouter(api)); const result = await handle("siclaw_wait_task", { task_id: "task-1", wait_seconds: 45, }); expect(api.waitForTask).toHaveBeenCalledWith("task-1", 45); expect(api.sendMessage).not.toHaveBeenCalled(); - expect(result.structuredContent).toMatchObject({ state: "completed", result: "root cause" }); + expect(result.structuredContent).toMatchObject({ state: "completed", result: "root cause", agent: "default" }); }); it("keeps a working response compact until the terminal result", async () => { @@ -87,7 +114,7 @@ describe("tool contract", () => { result: "partial evidence that should not be repeated", updated_at: "2026-07-18T00:00:00.000Z", }); - const result = await createToolHandler(api)("siclaw_get_task", { task_id: "task-1" }); + const result = await createToolHandler(singleRouter(api))("siclaw_get_task", { task_id: "task-1" }); expect(result.content[0].text).not.toContain("partial evidence that should not be repeated"); expect(result.content[0].text).toContain("siclaw_wait_task"); expect(result.structuredContent).toMatchObject({ @@ -100,7 +127,7 @@ describe("tool contract", () => { it("keeps the submitted task_id when the bounded wait fails after submission", async () => { const api = fakeApi(); vi.mocked(api.waitForTask).mockRejectedValue(new Error("Sicore A2A request timed out")); - const result = await createToolHandler(api)("siclaw_investigate", { question: "check node" }); + const result = await createToolHandler(singleRouter(api))("siclaw_investigate", { question: "check node" }); expect(api.sendMessage).toHaveBeenCalledOnce(); expect(result.isError).toBeUndefined(); @@ -115,20 +142,20 @@ describe("tool contract", () => { }); it("returns MCP tool errors for invalid arguments", async () => { - const result = await createToolHandler(fakeApi())("siclaw_get_task", {}); + const result = await createToolHandler(singleRouter())("siclaw_get_task", {}); expect(result.isError).toBe(true); expect(result.content[0].text).toMatch(/task_id/); }); it("keeps waits below common MCP client request timeouts", async () => { - const result = await createToolHandler(fakeApi())("siclaw_investigate", { + const result = await createToolHandler(singleRouter())("siclaw_investigate", { question: "check node", wait_seconds: 51, }); expect(result.isError).toBe(true); expect(result.content[0].text).toMatch(/between 0 and 50/); - const waitResult = await createToolHandler(fakeApi())("siclaw_wait_task", { + const waitResult = await createToolHandler(singleRouter())("siclaw_wait_task", { task_id: "task-1", wait_seconds: 51, }); @@ -136,3 +163,100 @@ describe("tool contract", () => { expect(waitResult.content[0].text).toMatch(/between 1 and 50/); }); }); + +describe("multi-key routing", () => { + it("refuses to guess when several agents are configured and none is named", async () => { + const handle = createToolHandler(router([["sre", fakeApi()], ["kb", fakeApi()]])); + const result = await handle("siclaw_investigate", { question: "check node" }); + expect(result.isError).toBe(true); + expect(result.content[0].text).toMatch(/Multiple Siclaw agents/); + expect(result.content[0].text).toContain("sre = agent-sre"); + expect(result.content[0].text).toContain("kb = agent-kb"); + }); + + it("rejects an unknown alias and lists valid ones without leaking a key", async () => { + const handle = createToolHandler(router([["sre", fakeApi()], ["kb", fakeApi()]])); + const result = await handle("siclaw_investigate", { question: "check node", agent: "nope" }); + expect(result.isError).toBe(true); + expect(result.content[0].text).toMatch(/Unknown agent alias "nope"/); + expect(result.content[0].text).toContain("sre = agent-sre"); + }); + + it("routes a named investigation to that agent only", async () => { + const sre = fakeApi(); + const kb = fakeApi(); + const handle = createToolHandler(router([["sre", sre], ["kb", kb]])); + const result = await handle("siclaw_investigate", { question: "check node", agent: "kb", wait_seconds: 0 }); + expect(kb.sendMessage).toHaveBeenCalledWith("check node", undefined); + expect(sre.sendMessage).not.toHaveBeenCalled(); + expect(result.structuredContent).toMatchObject({ agent: "kb" }); + }); + + it("auto-routes a follow-up task op to the agent that created the task", async () => { + const sre = fakeApi(); + const kb = fakeApi(); + const handle = createToolHandler(router([["sre", sre], ["kb", kb]])); + await handle("siclaw_investigate", { question: "check node", agent: "kb", wait_seconds: 0 }); + + const waited = await handle("siclaw_wait_task", { task_id: "task-1", wait_seconds: 5 }); + expect(kb.waitForTask).toHaveBeenCalledWith("task-1", 5); + expect(sre.waitForTask).not.toHaveBeenCalled(); + expect(waited.structuredContent).toMatchObject({ agent: "kb" }); + expect(waited.structuredContent).not.toHaveProperty("routing_note"); + }); + + it("honors the recorded creator over a mismatched agent argument and notes the override", async () => { + const sre = fakeApi(); + const kb = fakeApi(); + const handle = createToolHandler(router([["sre", sre], ["kb", kb]])); + await handle("siclaw_investigate", { question: "check node", agent: "kb", wait_seconds: 0 }); + + const got = await handle("siclaw_get_task", { task_id: "task-1", agent: "sre" }); + expect(kb.getTask).toHaveBeenCalledWith("task-1"); + expect(sre.getTask).not.toHaveBeenCalled(); + expect(got.structuredContent).toMatchObject({ agent: "kb" }); + expect((got.structuredContent as { routing_note?: string }).routing_note) + .toMatch(/Routed to agent "kb".*ignored agent="sre"/); + expect(got.content[0].text).toContain("routing_note:"); + }); + + it("refuses an untracked task op that cannot be attributed to one agent", async () => { + const handle = createToolHandler(router([["sre", fakeApi()], ["kb", fakeApi()]])); + const result = await handle("siclaw_cancel_task", { task_id: "ghost" }); + expect(result.isError).toBe(true); + expect(result.content[0].text).toMatch(/was not created in this session/); + }); + + it("aggregates list_tasks across every agent and tags each row", async () => { + const sre = fakeApi({ + listTasks: vi.fn(async () => ({ tasks: [task("working", "sre-1")], total_size: 1, page_size: 20, next_page_token: 20 })), + }); + const kb = fakeApi({ + listTasks: vi.fn(async () => ({ tasks: [task("completed", "kb-1")], total_size: 1, page_size: 20, next_page_token: null })), + }); + const handle = createToolHandler(router([["sre", sre], ["kb", kb]])); + const result = await handle("siclaw_list_tasks", {}); + expect(result.structuredContent).toMatchObject({ total_size: 2, next_page_token: null, truncated_agents: ["sre"] }); + const tasks = (result.structuredContent as { tasks: Array<{ task_id: string; agent: string }> }).tasks; + expect(tasks).toEqual(expect.arrayContaining([ + expect.objectContaining({ task_id: "sre-1", agent: "sre" }), + expect.objectContaining({ task_id: "kb-1", agent: "kb" }), + ])); + expect(result.content[0].text).toContain("agent=sre"); + expect(result.content[0].text).toContain("More tasks exist for agents: sre"); + }); + + it("recovers and remembers ownership from a per-agent list", async () => { + const sre = fakeApi(); + const kb = fakeApi({ + listTasks: vi.fn(async () => ({ tasks: [task("working", "kb-9")], total_size: 1, page_size: 20, next_page_token: null })), + }); + const handle = createToolHandler(router([["sre", sre], ["kb", kb]])); + await handle("siclaw_list_tasks", { agent: "kb" }); + + const waited = await handle("siclaw_wait_task", { task_id: "kb-9", wait_seconds: 5 }); + expect(kb.waitForTask).toHaveBeenCalledWith("kb-9", 5); + expect(sre.waitForTask).not.toHaveBeenCalled(); + expect(waited.structuredContent).toMatchObject({ agent: "kb" }); + }); +}); diff --git a/mcp/sicore-a2a-adapter/src/tools.ts b/mcp/sicore-a2a-adapter/src/tools.ts index 4ea8677e..1595b8fb 100644 --- a/mcp/sicore-a2a-adapter/src/tools.ts +++ b/mcp/sicore-a2a-adapter/src/tools.ts @@ -1,4 +1,6 @@ -import type { SiclawA2aApi, SiclawTask, SiclawTaskList } from "./a2a-client.js"; +import type { SiclawTask } from "./a2a-client.js"; +import { ALIAS_PATTERN } from "./config.js"; +import type { AgentRouter } from "./router.js"; const STATUS_TO_A2A: Record = { submitted: "TASK_STATE_SUBMITTED", @@ -9,97 +11,139 @@ const STATUS_TO_A2A: Record = { rejected: "TASK_STATE_REJECTED", }; -export const TOOL_DEFINITIONS = [ - { - name: "siclaw_investigate", - description: "Ask the configured Siclaw SRE agent to investigate an operational question. This creates an asynchronous Sicore A2A task. Reuse context_id to continue the same investigation. If the returned task is not terminal, do not submit it again: call siclaw_wait_task until it finishes unless the user explicitly requested fire-and-forget. The configured A2A key fixes which Siclaw agent is used.", - inputSchema: { - type: "object", - properties: { - question: { - type: "string", - minLength: 1, - description: "The concrete operational question for Siclaw, including relevant target names and time window when known.", - }, - context_id: { - type: "string", - minLength: 1, - maxLength: 255, - description: "Optional context_id from a prior task to continue the same Siclaw investigation session.", - }, - wait_seconds: { - type: "integer", - minimum: 0, - maximum: 50, - default: 20, - description: "How long to wait for a terminal result before returning the task as working. The 50-second ceiling stays below common MCP client request timeouts; use siclaw_wait_task later when it is still running.", +function agentHint(router: AgentRouter): string { + if (router.isSingle) { + return `Only one Siclaw agent is configured (${router.describeAgents()}); "agent" may be omitted.`; + } + return `Configured Siclaw agents (pass one as "agent"): ${router.describeAgents()}.`; +} + +function agentProperty(router: AgentRouter) { + return { + type: "string", + pattern: ALIAS_PATTERN, + description: + `Which configured Siclaw agent to use, selected by alias (never a key). ${agentHint(router)}`, + } as const; +} + +export function buildToolDefinitions(router: AgentRouter) { + const agent = agentProperty(router); + const multi = !router.isSingle; + const hint = agentHint(router); + const requireAgent = multi ? ` You must pass "agent" because more than one agent is configured.` : ""; + const routeNote = ` If "agent" is omitted, the adapter routes to the agent that created the task.`; + const listNote = multi + ? ` Without "agent" it aggregates tasks from every configured agent and tags each with its alias; with "agent" it lists only that agent's tasks.` + : ` Tasks are tagged with the configured agent's alias.`; + + return [ + { + name: "siclaw_investigate", + description: + "Ask the configured Siclaw SRE agent to investigate an operational question. This creates an asynchronous Sicore A2A task. Reuse context_id to continue the same investigation. If the returned task is not terminal, do not submit it again: call siclaw_wait_task until it finishes unless the user explicitly requested fire-and-forget. The A2A key selected by \"agent\" fixes which Siclaw agent is used." + + ` ${hint}${requireAgent}`, + inputSchema: { + type: "object", + properties: { + question: { + type: "string", + minLength: 1, + description: "The concrete operational question for Siclaw, including relevant target names and time window when known.", + }, + agent, + context_id: { + type: "string", + minLength: 1, + maxLength: 255, + description: "Optional context_id from a prior task to continue the same Siclaw investigation session. It belongs to the same agent that created it.", + }, + wait_seconds: { + type: "integer", + minimum: 0, + maximum: 50, + default: 20, + description: "How long to wait for a terminal result before returning the task as working. The 50-second ceiling stays below common MCP client request timeouts; use siclaw_wait_task later when it is still running.", + }, }, + required: ["question"], + additionalProperties: false, }, - required: ["question"], - additionalProperties: false, }, - }, - { - name: "siclaw_wait_task", - description: "Wait for an existing Siclaw investigation without creating another task. Use this as the same-turn watchdog: call it repeatedly while the task is non-terminal, unless the user asks to stop or the overall investigation deadline is exhausted. Working responses are compact; the full report is returned once the task reaches a terminal state.", - inputSchema: { - type: "object", - properties: { - task_id: { type: "string", minLength: 1, maxLength: 255 }, - wait_seconds: { - type: "integer", - minimum: 1, - maximum: 50, - default: 45, - description: "Bounded watchdog wait. Keep this below the MCP client's request timeout.", + { + name: "siclaw_wait_task", + description: + "Wait for an existing Siclaw investigation without creating another task. Use this as the same-turn watchdog: call it repeatedly while the task is non-terminal, unless the user asks to stop or the overall investigation deadline is exhausted. Working responses are compact; the full report is returned once the task reaches a terminal state." + + ` ${hint}${routeNote}`, + inputSchema: { + type: "object", + properties: { + task_id: { type: "string", minLength: 1, maxLength: 255 }, + agent, + wait_seconds: { + type: "integer", + minimum: 1, + maximum: 50, + default: 45, + description: "Bounded watchdog wait. Keep this below the MCP client's request timeout.", + }, }, + required: ["task_id"], + additionalProperties: false, }, - required: ["task_id"], - additionalProperties: false, }, - }, - { - name: "siclaw_get_task", - description: "Get one immediate snapshot of a Siclaw investigation task created with this same A2A key. Use siclaw_wait_task instead when actively waiting for completion.", - inputSchema: { - type: "object", - properties: { - task_id: { type: "string", minLength: 1, maxLength: 255 }, + { + name: "siclaw_get_task", + description: + "Get one immediate snapshot of a Siclaw investigation task. Use siclaw_wait_task instead when actively waiting for completion." + + ` ${hint}${routeNote}`, + inputSchema: { + type: "object", + properties: { + task_id: { type: "string", minLength: 1, maxLength: 255 }, + agent, + }, + required: ["task_id"], + additionalProperties: false, }, - required: ["task_id"], - additionalProperties: false, }, - }, - { - name: "siclaw_cancel_task", - description: "Cancel a non-terminal Siclaw investigation task created with this same A2A key.", - inputSchema: { - type: "object", - properties: { - task_id: { type: "string", minLength: 1, maxLength: 255 }, + { + name: "siclaw_cancel_task", + description: + "Cancel a non-terminal Siclaw investigation task." + + ` ${hint}${routeNote}`, + inputSchema: { + type: "object", + properties: { + task_id: { type: "string", minLength: 1, maxLength: 255 }, + agent, + }, + required: ["task_id"], + additionalProperties: false, }, - required: ["task_id"], - additionalProperties: false, }, - }, - { - name: "siclaw_list_tasks", - description: "List Siclaw investigation tasks scoped to this configured agent and A2A key. Use this to recover task IDs after a local client restart.", - inputSchema: { - type: "object", - properties: { - context_id: { type: "string", minLength: 1, maxLength: 255 }, - status: { - type: "string", - enum: ["submitted", "working", "completed", "failed", "canceled", "rejected"], + { + name: "siclaw_list_tasks", + description: + "List Siclaw investigation tasks. Use this to recover task IDs after a local client restart." + + ` ${hint}${listNote}`, + inputSchema: { + type: "object", + properties: { + agent, + context_id: { type: "string", minLength: 1, maxLength: 255 }, + status: { + type: "string", + enum: ["submitted", "working", "completed", "failed", "canceled", "rejected"], + }, + page_size: { type: "integer", minimum: 1, maximum: 100, default: 20 }, + page_token: { type: "integer", minimum: 0, default: 0 }, }, - page_size: { type: "integer", minimum: 1, maximum: 100, default: 20 }, - page_token: { type: "integer", minimum: 0, default: 0 }, + additionalProperties: false, }, - additionalProperties: false, }, - }, -] as const; + ] as const; +} type ToolResult = { isError?: boolean; @@ -108,14 +152,17 @@ type ToolResult = { }; type TaskToolView = Omit & { + agent: string; result?: string | null; progress_chars: number; + routing_note?: string; wait_error?: string; }; -function taskView(task: SiclawTask, includeTerminalResult: boolean): TaskToolView { +function taskView(task: SiclawTask, alias: string, includeTerminalResult: boolean): TaskToolView { const { result, ...summary } = task; return { + agent: alias, ...summary, progress_chars: result?.length ?? 0, ...(includeTerminalResult && task.is_terminal ? { result } : {}), @@ -155,14 +202,21 @@ function intArg( return value as number; } -function taskText(task: SiclawTask, waitError?: string): string { +interface TaskAnnotations { + waitError?: string; + routingNote?: string; +} + +function taskText(task: SiclawTask, alias: string, notes: TaskAnnotations = {}): string { const lines = [ `Siclaw task ${task.task_id}: ${task.state}`, + `agent: ${alias}`, `context_id: ${task.context_id}`, ]; if (task.updated_at) lines.push(`updated_at: ${task.updated_at}`); if (task.status_message) lines.push(`status: ${task.status_message}`); - if (waitError) lines.push(`wait_error: ${waitError} (status polling failed, but the task was already created)`); + if (notes.routingNote) lines.push(`routing_note: ${notes.routingNote}`); + if (notes.waitError) lines.push(`wait_error: ${notes.waitError} (status polling failed, but the task was already created)`); if (task.is_terminal && task.result) lines.push("", task.result); if (!task.is_terminal) { const progressChars = task.result?.length ?? 0; @@ -172,70 +226,154 @@ function taskText(task: SiclawTask, waitError?: string): string { return lines.join("\n"); } -function taskResult(task: SiclawTask, waitError?: string): ToolResult { - const view = taskView(task, true); - if (waitError) view.wait_error = waitError; +function taskResult(task: SiclawTask, alias: string, notes: TaskAnnotations = {}): ToolResult { + const view = taskView(task, alias, true); + if (notes.routingNote) view.routing_note = notes.routingNote; + if (notes.waitError) view.wait_error = notes.waitError; return { - content: [{ type: "text", text: taskText(task, waitError) }], + content: [{ type: "text", text: taskText(task, alias, notes) }], structuredContent: view as unknown as Record, }; } -function listText(list: SiclawTaskList): string { - if (list.tasks.length === 0) return "No Siclaw tasks matched this query."; - const rows = list.tasks.map((task) => `${task.task_id}\t${task.state}\tcontext=${task.context_id}`); - return [`Siclaw tasks (${list.tasks.length}/${list.total_size}):`, ...rows].join("\n"); +interface TaggedTask { + task: SiclawTask; + alias: string; +} + +function listResult( + tagged: TaggedTask[], + meta: { totalSize: number; pageSize: number; nextPageToken: number | null; truncatedAgents?: string[] }, +): ToolResult { + const lines: string[] = []; + if (tagged.length === 0) { + lines.push("No Siclaw tasks matched this query."); + } else { + lines.push(`Siclaw tasks (${tagged.length}/${meta.totalSize}):`); + for (const { task, alias } of tagged) { + lines.push(`${task.task_id}\t${task.state}\tagent=${alias}\tcontext=${task.context_id}`); + } + } + if (meta.truncatedAgents && meta.truncatedAgents.length > 0) { + lines.push( + "", + `More tasks exist for agents: ${meta.truncatedAgents.join(", ")}. ` + + "Query siclaw_list_tasks again with that agent and page_token to page through them.", + ); + } + return { + content: [{ type: "text", text: lines.join("\n") }], + structuredContent: { + tasks: tagged.map(({ task, alias }) => taskView(task, alias, false)), + total_size: meta.totalSize, + page_size: meta.pageSize, + next_page_token: meta.nextPageToken, + ...(meta.truncatedAgents && meta.truncatedAgents.length > 0 + ? { truncated_agents: meta.truncatedAgents } + : {}), + } as unknown as Record, + }; } -export function createToolHandler(api: SiclawA2aApi) { +export function createToolHandler(router: AgentRouter) { return async (name: string, rawArgs: unknown): Promise => { try { const args = argsRecord(rawArgs ?? {}); + const agentArg = stringArg(args, "agent"); + if (name === "siclaw_investigate") { + const entry = router.selectExplicit(agentArg); const question = stringArg(args, "question", true)!; if (Buffer.byteLength(question, "utf8") > 512 * 1024) { throw new Error("question must be 512 KiB or less"); } const contextId = stringArg(args, "context_id"); const waitSeconds = intArg(args, "wait_seconds", 20, 0, 50); - const submitted = await api.sendMessage(question, contextId); - if (waitSeconds === 0 || submitted.is_terminal) return taskResult(submitted); + const submitted = await entry.api.sendMessage(question, contextId); + router.remember(submitted.task_id, entry.alias); + if (waitSeconds === 0 || submitted.is_terminal) return taskResult(submitted, entry.alias); try { - return taskResult(await api.waitForTask(submitted.task_id, waitSeconds)); + const done = await entry.api.waitForTask(submitted.task_id, waitSeconds); + router.remember(done.task_id, entry.alias); + return taskResult(done, entry.alias); } catch (error) { // The task exists server-side at this point; a plain error would drop the // task_id and invite the client to resubmit a duplicate investigation. - return taskResult(submitted, error instanceof Error ? error.message : String(error)); + return taskResult(submitted, entry.alias, { + waitError: error instanceof Error ? error.message : String(error), + }); } } + if (name === "siclaw_get_task") { - return taskResult(await api.getTask(stringArg(args, "task_id", true)!)); + const taskId = stringArg(args, "task_id", true)!; + const { entry, note } = router.selectForTask(taskId, agentArg); + const task = await entry.api.getTask(taskId); + router.remember(task.task_id, entry.alias); + return taskResult(task, entry.alias, { routingNote: note }); } + if (name === "siclaw_wait_task") { const taskId = stringArg(args, "task_id", true)!; const waitSeconds = intArg(args, "wait_seconds", 45, 1, 50); - return taskResult(await api.waitForTask(taskId, waitSeconds)); + const { entry, note } = router.selectForTask(taskId, agentArg); + const task = await entry.api.waitForTask(taskId, waitSeconds); + router.remember(task.task_id, entry.alias); + return taskResult(task, entry.alias, { routingNote: note }); } + if (name === "siclaw_cancel_task") { - return taskResult(await api.cancelTask(stringArg(args, "task_id", true)!)); + const taskId = stringArg(args, "task_id", true)!; + const { entry, note } = router.selectForTask(taskId, agentArg); + const task = await entry.api.cancelTask(taskId); + router.remember(task.task_id, entry.alias); + return taskResult(task, entry.alias, { routingNote: note }); } + if (name === "siclaw_list_tasks") { const status = stringArg(args, "status"); if (status && !STATUS_TO_A2A[status]) throw new Error("status is invalid"); - const list = await api.listTasks({ + const options = { contextId: stringArg(args, "context_id"), status: status ? STATUS_TO_A2A[status] : undefined, pageSize: intArg(args, "page_size", 20, 1, 100), pageToken: intArg(args, "page_token", 0, 0, Number.MAX_SAFE_INTEGER), - }); - return { - content: [{ type: "text", text: listText(list) }], - structuredContent: { - ...list, - tasks: list.tasks.map((task) => taskView(task, false)), - } as unknown as Record, }; + + // Aggregate across every agent only when none is named and several exist. + if (agentArg === undefined && !router.isSingle) { + const perAgent = await Promise.all( + router.listEntries().map(async (entry) => ({ entry, list: await entry.api.listTasks(options) })), + ); + const tagged: TaggedTask[] = []; + const truncatedAgents: string[] = []; + let totalSize = 0; + for (const { entry, list } of perAgent) { + for (const task of list.tasks) { + router.remember(task.task_id, entry.alias); + tagged.push({ task, alias: entry.alias }); + } + totalSize += list.total_size; + if (list.next_page_token !== null) truncatedAgents.push(entry.alias); + } + // A combined cursor across agents would be meaningless; page per agent instead. + return listResult(tagged, { + totalSize, + pageSize: options.pageSize, + nextPageToken: null, + truncatedAgents, + }); + } + + const entry = router.selectExplicit(agentArg); + const list = await entry.api.listTasks(options); + for (const task of list.tasks) router.remember(task.task_id, entry.alias); + return listResult( + list.tasks.map((task) => ({ task, alias: entry.alias })), + { totalSize: list.total_size, pageSize: list.page_size, nextPageToken: list.next_page_token }, + ); } + throw new Error(`Unknown tool: ${name}`); } catch (error) { const message = error instanceof Error ? error.message : String(error);