diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f0eea7e..bb592816 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,16 @@ Packages without a separate changelog are covered by the cross-package notes bel ## [Unreleased] +### Fixed + +- Removing an agent that has authored messages now succeeds. The agent row is + retained so its messages keep their author, while the name is freed for reuse + and the old credential stops authenticating. +- A removed or released agent is no longer a delivery target: its channel and + direct-message memberships are cleared with it. +- A released agent can no longer be revived to `active` by a late heartbeat, and + id-scoped updates no longer resolve to a released row. + ## [8.0.2] - 2026-08-15 ### Fixed diff --git a/packages/engine/CHANGELOG.md b/packages/engine/CHANGELOG.md index 35bcd28d..9d793174 100644 --- a/packages/engine/CHANGELOG.md +++ b/packages/engine/CHANGELOG.md @@ -9,6 +9,17 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht ## [Unreleased] +### Fixed + +- Removing an agent that has authored messages now succeeds. The agent row is + retained so its messages keep their author, while the name is freed for reuse + and the old credential stops authenticating. +- A removed or released agent is no longer a delivery target: its channel and + direct-message memberships are cleared with it. +- A released agent can no longer be revived to `active` by a late heartbeat, and + id-scoped updates no longer resolve to a released row. + + ## [8.0.1] - 2026-08-14 ### Fixed diff --git a/packages/engine/src/__tests__/conformance/deleteAgentRoute.test.ts b/packages/engine/src/__tests__/conformance/deleteAgentRoute.test.ts new file mode 100644 index 00000000..86240e9b --- /dev/null +++ b/packages/engine/src/__tests__/conformance/deleteAgentRoute.test.ts @@ -0,0 +1,113 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { and, eq } from 'drizzle-orm'; +import { createWorkspace, makeNodeStack, registerAgent, type TestStack } from './harness.js'; +import { agents, channelMembers, messages } from '../../db/schema.js'; + +/** + * `DELETE /v1/agents/:name` -> `agentEngine.deleteAgent` was the last release + * path still doing a bare DELETE, after `relaycast#309` fixed the local release + * path and `relaycast#330` fixed the node-completed one. + * + * Four FKs reference `agents.id` with no ON DELETE action + * (`messages.agent_id`, `channels.created_by`, `files.uploaded_by`, + * `webhooks.created_by`), so the delete is refused for any agent that has ever + * spoken — and the refusal reaches the operator as raw SQL with the row id in + * it, observed in production as: + * + * Failed query: delete from "agents" where "agents"."id" = ? params: 2144… + * + * This route is what older CLIs call, so it stays reachable after the other two + * fixes ship. + */ +describe('DELETE /v1/agents/:name preserves attributed history', () => { + let stack: TestStack; + + beforeEach(() => { + stack = makeNodeStack(); + }); + + afterEach(() => stack.close()); + + async function post(token: string, text: string) { + const res = await stack.app.request('/v1/channels/general/messages', { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${token}` }, + body: JSON.stringify({ text }), + }); + expect(res.status).toBe(201); + } + + function removeAgent(workspaceKey: string, name: string) { + return stack.app.request(`/v1/agents/${name}`, { + method: 'DELETE', + headers: { authorization: `Bearer ${workspaceKey}` }, + }); + } + + // MUST-FIRE: fails before the fix — the bare DELETE is refused by the FK. + it('removes an agent that has authored messages, keeping the attribution', async () => { + const ws = await createWorkspace(stack.app, 'route-delete-with-history'); + const target = await registerAgent(stack.app, ws.workspaceKey, 'talkative'); + await post(target.token, 'this message must keep its author'); + + const res = await removeAgent(ws.workspaceKey, target.name); + expect(res.status).toBeLessThan(300); + + // The name is the scarce resource and must be free immediately. + expect( + await stack.runtime.deps.db + .select() + .from(agents) + .where(and(eq(agents.workspaceId, ws.workspaceId), eq(agents.name, target.name))), + ).toHaveLength(0); + + // The row survives as a tombstone so history keeps its author. + const [tombstone] = await stack.runtime.deps.db + .select({ name: agents.name, status: agents.status }) + .from(agents) + .where(eq(agents.id, target.agentId)); + expect(tombstone).toMatchObject({ + name: `${target.name}#released-${target.agentId}`, + status: 'released', + }); + + // The tombstone must not stay subscribed: `channel_members` cascades on + // DELETE, and an UPDATE does not fire that cascade, so a released agent + // would otherwise remain a delivery target. + expect( + await stack.runtime.deps.db + .select() + .from(channelMembers) + .where(eq(channelMembers.agentId, target.agentId)), + ).toHaveLength(0); + + // Attribution intact; the old credential is dead; the name is reusable. + expect( + await stack.runtime.deps.db.select().from(messages).where(eq(messages.agentId, target.agentId)), + ).toHaveLength(1); + const reuse = await stack.app.request('/v1/channels/general/messages', { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${target.token}` }, + body: JSON.stringify({ text: 'should be rejected' }), + }); + expect(reuse.status).toBeGreaterThanOrEqual(400); + const successor = await registerAgent(stack.app, ws.workspaceKey, target.name); + expect(successor.agentId).not.toBe(target.agentId); + }); + + // MUST-NOT-FIRE: the silent case must behave identically, so the fix cannot + // pass by treating one class of agent specially. + it('removes an agent that never spoke through the same route', async () => { + const ws = await createWorkspace(stack.app, 'route-delete-no-history'); + const target = await registerAgent(stack.app, ws.workspaceKey, 'silent'); + + const res = await removeAgent(ws.workspaceKey, target.name); + expect(res.status).toBeLessThan(300); + expect( + await stack.runtime.deps.db + .select() + .from(agents) + .where(and(eq(agents.workspaceId, ws.workspaceId), eq(agents.name, target.name))), + ).toHaveLength(0); + }); +}); diff --git a/packages/engine/src/engine/action.ts b/packages/engine/src/engine/action.ts index 2bbe7897..59ebb506 100644 --- a/packages/engine/src/engine/action.ts +++ b/packages/engine/src/engine/action.ts @@ -1,6 +1,6 @@ import { and, asc, eq, inArray, isNotNull, isNull, lte, or, sql } from 'drizzle-orm'; import type { getDb } from '../db/index.js'; -import { actions, actionInvocations, agents, agentNodeBindings, nodes } from '../db/schema.js'; +import { actions, actionInvocations, agents, agentNodeBindings, channelMembers, dmParticipants, nodes } from '../db/schema.js'; import { generateId } from './snowflake.js'; import { RELEASED_AGENT_STATUS, releasedAgentName } from './agent.js'; import { randomHex, sha256Hex } from '../lib/crypto.js'; @@ -779,6 +779,15 @@ async function dispatchRelease(args: { eq(agents.id, agent.id), invocationIsOpen, ))); + // `channel_members` and `dm_participants` cascade on DELETE; the + // tombstone's UPDATE does not fire that cascade, so drop the memberships + // in the SAME atomic unit or the released agent stays a delivery target. + writes.push(writeDb + .delete(channelMembers) + .where(eq(channelMembers.agentId, agent.id))); + writes.push(writeDb + .delete(dmParticipants) + .where(eq(dmParticipants.agentId, agent.id))); writes.push(writeDb .delete(nodes) .where(and( @@ -1383,6 +1392,11 @@ async function applyReleaseCompletionEffect( lastSeen: new Date(), }) .where(and(eq(agents.workspaceId, workspaceId), eq(agents.id, agent.id))); + // `channel_members` and `dm_participants` reference `agents.id` ON DELETE + // CASCADE; an UPDATE does not fire that cascade, so drop the memberships + // explicitly or the released agent stays a delivery target. + await db.delete(channelMembers).where(eq(channelMembers.agentId, agent.id)); + await db.delete(dmParticipants).where(eq(dmParticipants.agentId, agent.id)); const implicitNodeId = `node_direct_${agent.id}`; await db.delete(nodes).where(and(eq(nodes.workspaceId, workspaceId), eq(nodes.id, implicitNodeId))); } else { diff --git a/packages/engine/src/engine/agent.ts b/packages/engine/src/engine/agent.ts index 781f3926..b9fe652e 100644 --- a/packages/engine/src/engine/agent.ts +++ b/packages/engine/src/engine/agent.ts @@ -1,6 +1,6 @@ import { eq, and, gt, lt, ne, inArray, sql } from 'drizzle-orm'; import type { getDb } from '../db/index.js'; -import { agents, agentNodeBindings, channels, channelMembers, actions, deliveries, nodes } from '../db/schema.js'; +import { agents, agentNodeBindings, channels, channelMembers, dmParticipants, actions, deliveries, nodes } from '../db/schema.js'; import { randomHex, sha256Hex } from '../lib/crypto.js'; import { generateId } from './snowflake.js'; import { codedError } from '../lib/httpError.js'; @@ -451,7 +451,15 @@ export async function updateAgentById( const [updated] = await db .update(agents) .set(setClause) - .where(and(eq(agents.workspaceId, workspaceId), eq(agents.id, agentId))) + // A released row is a tombstone kept only so history stays attributable — + // it is not a live agent and must not be updatable, matching the roster and + // presence reads that already exclude it. Without this, an id-scoped write + // against a released agent silently succeeds against the tombstone. + .where(and( + eq(agents.workspaceId, workspaceId), + eq(agents.id, agentId), + ne(agents.status, RELEASED_AGENT_STATUS), + )) .returning(); if (!updated) return null; @@ -527,8 +535,55 @@ export async function deleteAgent(db: Db, workspaceId: string, name: string) { if (!agent) return false; - await db.delete(agents).where(eq(agents.id, agent.id)); - await db.delete(nodes).where(eq(nodes.id, directNodeIdForAgent(agent.id))); + // Tombstone rather than DELETE, matching both release paths + // (`dispatchRelease` -> `completeLocally`, and `applyReleaseCompletionEffect`). + // Four FKs reference `agents.id` with no ON DELETE action — + // `messages.agent_id`, `channels.created_by`, `files.uploaded_by`, + // `webhooks.created_by` — so a bare DELETE is refused for any agent that has + // ever spoken, and the caller sees the raw SQL failure with the row id in it. + // Renaming frees the unique `(workspace_id, name)` immediately while every FK + // target stays valid and every message keeps its sender. + const releasedName = releasedAgentName(agent.name, agent.id); + // The row survives, so its credential must not. `token_hash` is NOT NULL + // UNIQUE and cannot be cleared, so rotate it to a value nobody holds. + const releasedTokenHash = await sha256Hex(`released:${agent.id}:${randomHex(16)}`); + const releasedAt = new Date(); + // One atomic unit: a partial apply would leave the agent renamed and + // credential-rotated while still a channel member — reachable by delivery + // under a name its owner no longer knows. + await runAtomicWrites(db, (writeDb) => { + const writes: AtomicWrite[] = []; + writes.push(writeDb + .update(agents) + .set({ + name: releasedName, + handle: `@${releasedName}`, + status: RELEASED_AGENT_STATUS, + tokenHash: releasedTokenHash, + locationType: 'self_connected', + locationNodeId: null, + lastSeen: releasedAt, + // Same `release` shape both release paths write, so an audit does not + // have to know which path released the agent. + metadata: sql`json_patch(COALESCE(${agents.metadata}, '{}'), ${JSON.stringify({ + release: { + reason: 'agent removed', + released_at: releasedAt.toISOString(), + previous_name: agent.name, + }, + })})`, + }) + .where(eq(agents.id, agent.id))); + // `channel_members` and `dm_participants` cascade on DELETE; an UPDATE does + // not fire that cascade, so a released agent would stay a delivery target. + writes.push(writeDb.delete(channelMembers).where(eq(channelMembers.agentId, agent.id))); + writes.push(writeDb.delete(dmParticipants).where(eq(dmParticipants.agentId, agent.id))); + // Release the node binding so the host's active-agent count is not held by + // a tombstone, matching the release paths. + writes.push(writeDb.delete(agentNodeBindings).where(eq(agentNodeBindings.agentId, agent.id))); + writes.push(writeDb.delete(nodes).where(eq(nodes.id, directNodeIdForAgent(agent.id)))); + return writes; + }); return true; } @@ -536,7 +591,10 @@ export async function touchLastSeen(db: Db, agentId: string): Promise { await db .update(agents) .set({ lastSeen: new Date(), status: 'active' }) - .where(eq(agents.id, agentId)); + // Auth schedules this without awaiting it, so a heartbeat racing a release + // can land after the tombstone and flip the row back to `active` — reviving + // a released agent as a live roster member. A tombstone is terminal. + .where(and(eq(agents.id, agentId), ne(agents.status, RELEASED_AGENT_STATUS))); } export async function sweepStaleAgents(db: Db, workspaceId?: string): Promise {