diff --git a/packages/cli/src/cli/agent-relay-mcp.startup.test.ts b/packages/cli/src/cli/agent-relay-mcp.startup.test.ts index b9ca08a9c..0cca4c066 100644 --- a/packages/cli/src/cli/agent-relay-mcp.startup.test.ts +++ b/packages/cli/src/cli/agent-relay-mcp.startup.test.ts @@ -481,12 +481,25 @@ describe('createAgentRelayMcpServer', () => { }, ]); + // Delegation identity rides on the spawn action so the agent record + // carries it from birth. Without this the only durable identity a + // consumer can find is whatever it can guess from the agent's name, and a + // multi-token project slug is not recoverable from a name at all. + const identityMetadata = { + organization: 'AgentWorkforce', + project: 'chief-delegation-governance', + workstream: 'dispatch-contract', + role: 'worker', + reportsTo: 'chief-delegation-governance-dispatch-lead', + }; + const spawnResult = await server.tools.get('spawn')?.handler({ name: 'FleetWorker', cli: 'codex', task: 'Implement a fix', channel: 'general', target_node: 'node-a', + metadata: identityMetadata, }); expect(spawnResult.structuredContent.invocation).toEqual({ invocationId: 'inv_1', @@ -496,6 +509,7 @@ describe('createAgentRelayMcpServer', () => { cli: 'codex', task: 'Implement a fix', target_node: 'node-a', + metadata: identityMetadata, channels: ['general'], }, }); diff --git a/packages/cli/src/cli/agent-relay-mcp.test.ts b/packages/cli/src/cli/agent-relay-mcp.test.ts index 65cbe617c..b30b5e9f1 100644 --- a/packages/cli/src/cli/agent-relay-mcp.test.ts +++ b/packages/cli/src/cli/agent-relay-mcp.test.ts @@ -37,6 +37,186 @@ describe('registerAgentWithRebind', () => { }); }); + const IDENTITY = { + organization: 'AgentWorkforce', + project: 'chief-delegation-governance', + workstream: 'dispatch-contract', + role: 'lead', + reportsTo: 'chief-khaliq', + }; + + const strictSession = () => ({ + workspaceKey: 'rk_live_test', + agentToken: 'at_live_existing', + agentName: 'WorkerA', + agents: new Map([['WorkerA', { agentName: 'WorkerA', agentToken: 'at_live_existing' }]]), + }); + + /** + * A relay that actually stores what it is given, so a test can read the + * record back instead of trusting that the call was made. Asserting only + * "registerOrRotate was called with metadata" would repeat the mistake this + * whole change is about: the parameter being passed is not the field + * landing. `persists: false` models the broken platform. + */ + function fakeRelay({ persists = true }: { persists?: boolean } = {}) { + // The platform writes its own block; a verifier must ignore it. + const records = new Map>([['WorkerA', { fleet: { nodeId: 'node_x' } }]]); + const registerOrRotate = vi.fn(async (input: any) => { + if (persists && input.metadata) { + records.set(input.name, { ...records.get(input.name), ...input.metadata }); + } + return { id: 'agent_123', name: input.name, token: 'at_live_rotated', status: 'online' }; + }); + const list = vi.fn(async () => [...records].map(([name, metadata]) => ({ name, metadata }))); + return { agents: { registerOrRotate, list }, records }; + } + + it('writes supplied metadata through and proves it landed on the record', async () => { + // The short-circuit exists to avoid handing back a dead token. It must not + // also swallow a write: a caller supplying metadata is asking for the + // agent record to change, and returning a cached token discarded that + // silently — success, no warnings, record untouched. + const relay = fakeRelay(); + + const payload = await registerAgentWithRebind({ + session: strictSession(), + setSession: vi.fn(), + getRelay: () => relay as never, + name: 'WorkerA', + metadata: IDENTITY, + verifyMetadata: true, + strictAgentName: true, + preferredAgentName: 'WorkerA', + }); + + expect(relay.agents.registerOrRotate).toHaveBeenCalledOnce(); + + // The round trip, not just the call: read the record back and assert the + // fields are actually there. + const [record] = (await relay.agents.list()).filter((a) => a.name === 'WorkerA'); + expect(record.metadata).toMatchObject(IDENTITY); + expect(record.metadata.fleet).toEqual({ nodeId: 'node_x' }, 'must not clobber platform keys'); + + expect(payload.metadata_verified).toBe(true); + expect(payload.warnings).toEqual([]); + }); + + it('says so loudly when the platform accepts metadata and does not persist it', async () => { + // This is the exact defect being fixed, reproduced: the write is accepted, + // the response looks like success, and the record is untouched. A + // passthrough that fails this way is worse than none, because it looks + // like it worked. It must never again be reported as a clean success. + const relay = fakeRelay({ persists: false }); + + const payload = await registerAgentWithRebind({ + session: strictSession(), + setSession: vi.fn(), + getRelay: () => relay as never, + name: 'WorkerA', + metadata: IDENTITY, + verifyMetadata: true, + strictAgentName: true, + preferredAgentName: 'WorkerA', + }); + + expect(payload.metadata_verified).toBe(false); + expect(payload.warnings).toHaveLength(1); + expect(payload.warnings[0]).toContain('was not persisted'); + expect(payload.warnings[0]).toContain('organization'); + expect(payload.warnings[0]).toContain('Treat this registration as unattributed'); + }); + + it('reports unverified rather than throwing when the record cannot be read back', async () => { + // The registration itself succeeded; claiming it failed would be its own + // kind of lie. But it must not be reported as verified either. + const relay = fakeRelay(); + relay.agents.list = vi.fn(async () => { + throw new Error('workspace unreachable'); + }) as never; + + const payload = await registerAgentWithRebind({ + session: strictSession(), + setSession: vi.fn(), + getRelay: () => relay as never, + name: 'WorkerA', + metadata: IDENTITY, + verifyMetadata: true, + strictAgentName: true, + preferredAgentName: 'WorkerA', + }); + + expect(payload.token).toBe('at_live_rotated'); + expect(payload.metadata_verified).toBe(false); + expect(payload.warnings[0]).toContain('could not read the record back'); + expect(payload.warnings[0]).toContain('workspace unreachable'); + }); + + it('writes a supplied persona through, and claims no metadata verification', async () => { + const relay = fakeRelay(); + + const payload = await registerAgentWithRebind({ + session: strictSession(), + setSession: vi.fn(), + getRelay: () => relay as never, + name: 'WorkerA', + persona: 'Accountable lead for chief-delegation-governance', + strictAgentName: true, + preferredAgentName: 'WorkerA', + }); + + expect(relay.agents.registerOrRotate).toHaveBeenCalledOnce(); + // No metadata was supplied, so there is nothing to verify and no read-back + // cost is paid. + expect(relay.agents.list).not.toHaveBeenCalled(); + expect(payload.metadata_verified).toBeUndefined(); + }); + + it("reports 'unchecked' rather than success when nobody verified the write", async () => { + // Verification costs a workspace listing, so the per-spawn `{model}` hint + // does not pay for it. But "nobody looked" must not be reported as "it is + // there" — collapsing those two is the same error as the silent discard. + const relay = fakeRelay({ persists: false }); + + const payload = await registerAgentWithRebind({ + session: strictSession(), + setSession: vi.fn(), + getRelay: () => relay as never, + name: 'WorkerA', + metadata: { model: 'gpt-5' }, + strictAgentName: true, + preferredAgentName: 'WorkerA', + }); + + expect(payload.metadata_verified).toBe('unchecked'); + expect(relay.agents.list).not.toHaveBeenCalled(); + // Not a warning: nothing is known to be wrong. The claim is simply scoped. + expect(payload.warnings).toEqual([]); + }); + + it('still short-circuits when the caller only wants a token', async () => { + // The original behaviour has to survive: a bare re-registration with no + // write to make should not rotate the token for nothing. + const registerOrRotate = vi.fn(); + + const payload = await registerAgentWithRebind({ + session: { + workspaceKey: 'rk_live_test', + agentToken: 'at_live_existing', + agentName: 'WorkerA', + agents: new Map([['WorkerA', { agentName: 'WorkerA', agentToken: 'at_live_existing' }]]), + }, + setSession: vi.fn(), + getRelay: () => ({ agents: { registerOrRotate } }) as never, + name: 'WorkerA', + strictAgentName: true, + preferredAgentName: 'WorkerA', + }); + + expect(registerOrRotate).not.toHaveBeenCalled(); + expect(payload).toMatchObject({ token: 'at_live_existing' }); + }); + it('re-registers when the strict-named identity was dropped from the agents map', async () => { // After an `agent_token_invalid` recovery, the active token is null and // the identity is missing from session.agents. The short-circuit must diff --git a/packages/cli/src/cli/agent-relay-mcp.ts b/packages/cli/src/cli/agent-relay-mcp.ts index 10b5ff401..59932e2b8 100644 --- a/packages/cli/src/cli/agent-relay-mcp.ts +++ b/packages/cli/src/cli/agent-relay-mcp.ts @@ -111,6 +111,12 @@ type RegisterAgentWithRebindArgs = { type?: AgentType; persona?: string; metadata?: Record; + /** + * Read the agent record back and confirm the supplied metadata persisted. + * Costs a workspace listing, so callers writing durable identity opt in and + * the per-spawn `{model}` hint does not pay for it. + */ + verifyMetadata?: boolean; strictAgentName?: boolean; preferredAgentName?: string | null; forcedAgentType?: AgentType; @@ -336,6 +342,7 @@ export async function registerAgentWithRebind({ type, persona, metadata, + verifyMetadata, strictAgentName, preferredAgentName, forcedAgentType, @@ -358,7 +365,16 @@ export async function registerAgentWithRebind({ ); } - if (session.agentToken && effectiveName && strictAgentName) { + // A caller supplying metadata or a persona is asking for a write, not for a + // token. The short-circuit below hands back a cached token without calling + // upstream, which silently discarded that write: the call returned success + // with no warnings and the agent record was never touched. Anything that + // reads identity off the record — the fleet dashboard, an org chart, a + // delegation gate — then sees nothing and falls back to guessing from the + // agent's name. Fall through so the write actually happens. + const wantsRecordWrite = metadata !== undefined || persona !== undefined; + + if (session.agentToken && effectiveName && strictAgentName && !wantsRecordWrite) { // If the session tracks per-identity agents, only short-circuit when the // strict-named identity is still registered. After an `agent_token_invalid` // recovery the entry is dropped from the map, which lets this fall through @@ -385,13 +401,94 @@ export async function registerAgentWithRebind({ const reboundName = result.name?.trim() ? result.name : effectiveName; setSession({ agentToken: result.token, agentName: reboundName }); + // A registration response carries {id, name, token, status, createdAt} and + // nothing else — `normalizeAgentRegistration` drops everything not in that + // shape. So the response cannot tell a caller whether its metadata landed, + // which is precisely why the discard this fixes went unnoticed: success and + // silent-failure are the same bytes. + // + // `metadata_verified` closes that gap without ever claiming more than is + // known. It is machine-readable so a dispatcher writing durable identity can + // fail closed on it — a passthrough that quietly does nothing is a worse bug + // than no passthrough, because it looks like it worked. + // + // Verification costs a full workspace listing, so it is opt-in rather than + // automatic: `add_agent` sends `metadata: {model}` on every spawn as a + // broker hint, and making each of those refetch every agent in the workspace + // would be a bad trade. Unverified is reported as the literal string + // 'unchecked' rather than omitted or defaulted to false — "nobody looked" is + // a different claim from "it is not there", and collapsing them is the same + // class of error as the silent discard itself. + let metadataVerified: boolean | 'unchecked' | undefined; + if (metadata) { + if (verifyMetadata) { + const verification = await verifyMetadataLanded(relay, reboundName, metadata); + metadataVerified = verification.verified; + if (!verification.verified) warnings.push(verification.warning); + } else { + metadataVerified = 'unchecked'; + } + } + return { ...result, registered_name: reboundName, + ...(metadataVerified === undefined ? {} : { metadata_verified: metadataVerified }), warnings, }; } +/** + * Read the agent record back and confirm the supplied metadata is on it. + * + * Compares only the keys the caller sent; the platform adds its own (a `fleet` + * block, for one) and those are none of our business. A read that fails is + * reported as unverified rather than thrown — the registration itself + * succeeded, and claiming otherwise would be its own kind of lie. + */ +async function verifyMetadataLanded( + relay: RelayCastLike, + name: string, + metadata: Record +): Promise<{ verified: boolean; warning: string }> { + const keys = Object.keys(metadata); + if (keys.length === 0) return { verified: true, warning: '' }; + + let agents: unknown[]; + try { + agents = await relay.agents.list(); + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + return { + verified: false, + warning: `Registered "${name}", but could not read the record back to confirm metadata landed: ${detail}`, + }; + } + + const record = agents.find((agent) => (agent as { name?: string } | null)?.name === name) as + | { metadata?: Record } + | undefined; + + if (!record) { + return { + verified: false, + warning: `Registered "${name}", but the agent is not in the workspace listing, so its metadata could not be confirmed.`, + }; + } + + const stored = record.metadata ?? {}; + const missing = keys.filter((key) => JSON.stringify(stored[key]) !== JSON.stringify(metadata[key])); + if (missing.length === 0) return { verified: true, warning: '' }; + + return { + verified: false, + warning: + `Registered "${name}", but the metadata was not persisted: ${missing.join(', ')} ` + + `${missing.length === 1 ? 'is' : 'are'} missing or different on the record. ` + + `Treat this registration as unattributed.`, + }; +} + function registerAgentRelayTools( server: McpServer, getRelay: () => RelayCastLike, @@ -531,11 +628,19 @@ function registerAgentRelayTools( .record(z.string(), z.unknown()) .optional() .describe('Key-value metadata to attach to the agent'), + verify_metadata: z + .boolean() + .optional() + .describe( + 'Read the agent record back and confirm the metadata persisted. Costs a ' + + 'workspace listing. Use when writing durable identity you intend to rely on; ' + + 'the response reports metadata_verified as true, false, or "unchecked".' + ), }, outputSchema: jsonResult, annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: true, openWorldHint: true }, }, - async ({ name, type, persona, metadata }: any) => { + async ({ name, type, persona, metadata, verify_metadata }: any) => { const payload = await registerAgentWithRebind({ session: getSession(), setSession, @@ -544,6 +649,7 @@ function registerAgentRelayTools( type, persona, metadata, + verifyMetadata: verify_metadata, strictAgentName, preferredAgentName: preferredAgentName ?? null, forcedAgentType, @@ -697,6 +803,14 @@ function registerAgentRelayTools( model: z.string().optional().describe('Model powering the worker'), session_ref: z.string().optional().describe('Session reference for resumable spawns'), target_node: z.string().optional().describe('Optional target fleet node name'), + metadata: z + .record(z.string(), z.unknown()) + .optional() + .describe( + 'Key-value metadata attached to the spawned agent record. Use for durable ' + + 'delegation identity — organization, project, workstream, role, reportsTo — ' + + 'so consumers read it off the record instead of guessing from the name.' + ), ...identityOverrideInputShape, }, outputSchema: jsonResult, @@ -707,7 +821,7 @@ function registerAgentRelayTools( openWorldHint: true, }, }, - async ({ name, cli, task, channel, channels, model, session_ref, target_node, as }) => { + async ({ name, cli, task, channel, channels, model, session_ref, target_node, metadata, as }) => { const actions = getAgentClient(as).actions; if (!actions) { throw new Error('spawn requires an agent-scoped Relaycast actions client.'); @@ -719,6 +833,7 @@ function registerAgentRelayTools( ...(model ? { model } : {}), ...(session_ref ? { session_ref } : {}), ...(target_node ? { target_node } : {}), + ...(metadata ? { metadata } : {}), ...((channels ?? (channel ? [channel] : undefined)) ? { channels: channels ?? [channel] } : {}), }; return jsonContent({ invocation: await actions.invoke('spawn', actionInput) });