refactor(modules): extract agent-to-agent as registry-based module
Last extraction of Phase 3. Moves inter-agent messaging + create_agent +
destination projection into src/modules/agent-to-agent/. Core retains:
- `channel_type === 'agent'` dispatch in delivery.ts, guarded by
hasTable('agent_destinations') + dynamic import into module.
- Channel-permission ACL in delivery.ts, guarded by hasTable, with
inlined SQL (no module import from core).
- writeDestinations call in container-runner.ts, guarded by hasTable +
dynamic import into module.
- createMessagingGroupAgent's destination side effect in db/messaging-groups.ts,
guarded by hasTable. This is a documented transitional tier violation
(core imports from optional module), analogous to src/access.ts.
Migration `004-agent-destinations.ts` renamed to `module-agent-to-agent-
destinations.ts` preserving `name: 'agent-destinations'` so existing DBs
don't re-run it.
delivery.ts: 600 → 449 lines. handleSystemAction's last switch case gone
(just registry + default log-and-drop). notifyAgent helper removed (only
create_agent used it).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,135 +0,0 @@
|
||||
/**
|
||||
* Per-agent destination map + ACL.
|
||||
*
|
||||
* Each row means: agent `agent_group_id` is allowed to send messages to
|
||||
* target (`target_type`, `target_id`), and refers to it locally as `local_name`.
|
||||
*
|
||||
* Names are local to each source agent — they exist only inside that agent's
|
||||
* namespace. The host uses this table both for routing (resolve name → ID)
|
||||
* and for permission checks (row exists ⇒ authorized).
|
||||
*/
|
||||
/**
|
||||
* ⚠️ DESTINATION PROJECTION INVARIANT — READ BEFORE ADDING NEW CALL SITES.
|
||||
*
|
||||
* `agent_destinations` in the central DB is the source of truth, but the
|
||||
* agent-runner container reads its destinations from a per-session
|
||||
* projection in `inbound.db`. That projection is written by
|
||||
* `writeDestinations(agentGroupId, sessionId)` in session-manager.ts.
|
||||
*
|
||||
* `spawnContainer` calls `writeDestinations` on every container wake, so a
|
||||
* fresh container always sees the latest destinations. BUT: a container
|
||||
* that is ALREADY running when you mutate the central table will keep
|
||||
* serving the stale projection until its next wake — the central write
|
||||
* does not propagate automatically.
|
||||
*
|
||||
* **Therefore: every time you call `createDestination` / `deleteDestination` /
|
||||
* `deleteAllDestinationsTouching` from code that runs while an agent's
|
||||
* container may be alive, you MUST also call `writeDestinations(agentGroupId,
|
||||
* sessionId)` for each affected session.** Forgetting this manifests as
|
||||
* "dropped: unknown destination" errors at send_message time.
|
||||
*
|
||||
* Affected call sites today (keep this list honest if you add more):
|
||||
* - src/delivery.ts::handleSystemAction case 'create_agent'
|
||||
* - src/db/messaging-groups.ts::createMessagingGroupAgent
|
||||
*/
|
||||
import type { AgentDestination } from '../types.js';
|
||||
import { getDb } from './connection.js';
|
||||
|
||||
/**
|
||||
* ⚠️ Caller responsibility: after this returns, call
|
||||
* `writeDestinations(row.agent_group_id, <sessionId>)` for each active
|
||||
* session of that agent group so the change propagates to the running
|
||||
* container's inbound.db. See the top-of-file invariant.
|
||||
*/
|
||||
export function createDestination(row: AgentDestination): void {
|
||||
getDb()
|
||||
.prepare(
|
||||
`INSERT INTO agent_destinations (agent_group_id, local_name, target_type, target_id, created_at)
|
||||
VALUES (@agent_group_id, @local_name, @target_type, @target_id, @created_at)`,
|
||||
)
|
||||
.run(row);
|
||||
}
|
||||
|
||||
export function getDestinations(agentGroupId: string): AgentDestination[] {
|
||||
return getDb()
|
||||
.prepare('SELECT * FROM agent_destinations WHERE agent_group_id = ?')
|
||||
.all(agentGroupId) as AgentDestination[];
|
||||
}
|
||||
|
||||
export function getDestinationByName(agentGroupId: string, localName: string): AgentDestination | undefined {
|
||||
return getDb()
|
||||
.prepare('SELECT * FROM agent_destinations WHERE agent_group_id = ? AND local_name = ?')
|
||||
.get(agentGroupId, localName) as AgentDestination | undefined;
|
||||
}
|
||||
|
||||
/** Reverse lookup: what does this agent call the given target? */
|
||||
export function getDestinationByTarget(
|
||||
agentGroupId: string,
|
||||
targetType: 'channel' | 'agent',
|
||||
targetId: string,
|
||||
): AgentDestination | undefined {
|
||||
return getDb()
|
||||
.prepare('SELECT * FROM agent_destinations WHERE agent_group_id = ? AND target_type = ? AND target_id = ?')
|
||||
.get(agentGroupId, targetType, targetId) as AgentDestination | undefined;
|
||||
}
|
||||
|
||||
/** Permission check: can this agent send to this target? */
|
||||
export function hasDestination(agentGroupId: string, targetType: 'channel' | 'agent', targetId: string): boolean {
|
||||
const row = getDb()
|
||||
.prepare('SELECT 1 FROM agent_destinations WHERE agent_group_id = ? AND target_type = ? AND target_id = ? LIMIT 1')
|
||||
.get(agentGroupId, targetType, targetId);
|
||||
return !!row;
|
||||
}
|
||||
|
||||
/**
|
||||
* ⚠️ Caller responsibility: after this returns, call
|
||||
* `writeDestinations(agentGroupId, <sessionId>)` for each active session
|
||||
* so the deletion propagates to the running container's inbound.db.
|
||||
*/
|
||||
export function deleteDestination(agentGroupId: string, localName: string): void {
|
||||
getDb()
|
||||
.prepare('DELETE FROM agent_destinations WHERE agent_group_id = ? AND local_name = ?')
|
||||
.run(agentGroupId, localName);
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete every destination row where this agent group is either the owner
|
||||
* or the target. Used when tearing down a dev agent after a swap request
|
||||
* completes/rolls-back — drops the bidirectional destinations in one call.
|
||||
*
|
||||
* ⚠️ Caller responsibility: not only does `agentGroupId`'s own session
|
||||
* projection need a refresh, but ALSO every OTHER agent group that had
|
||||
* `agentGroupId` as a destination target. Use `getDestinationReferencers`
|
||||
* below to find them BEFORE calling this (the rows are gone afterwards).
|
||||
*/
|
||||
export function deleteAllDestinationsTouching(agentGroupId: string): void {
|
||||
getDb()
|
||||
.prepare('DELETE FROM agent_destinations WHERE agent_group_id = ? OR (target_type = ? AND target_id = ?)')
|
||||
.run(agentGroupId, 'agent', agentGroupId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the list of agent_group_ids that currently have a destination
|
||||
* row pointing at `targetAgentGroupId`. Call this BEFORE
|
||||
* `deleteAllDestinationsTouching` if you need to know whose session
|
||||
* projections to refresh after the delete — the rows are gone once the
|
||||
* delete runs.
|
||||
*/
|
||||
export function getDestinationReferencers(targetAgentGroupId: string): string[] {
|
||||
const rows = getDb()
|
||||
.prepare(
|
||||
"SELECT DISTINCT agent_group_id FROM agent_destinations WHERE target_type = 'agent' AND target_id = ? AND agent_group_id != ?",
|
||||
)
|
||||
.all(targetAgentGroupId, targetAgentGroupId) as Array<{ agent_group_id: string }>;
|
||||
return rows.map((r) => r.agent_group_id);
|
||||
}
|
||||
|
||||
/** Normalize a human-readable name into a lowercase, dash-separated identifier. */
|
||||
export function normalizeName(name: string): string {
|
||||
return (
|
||||
name
|
||||
.toLowerCase()
|
||||
.replace(/[^a-z0-9]+/g, '-')
|
||||
.replace(/^-+|-+$/g, '') || 'unnamed'
|
||||
);
|
||||
}
|
||||
@@ -230,7 +230,7 @@ describe('messaging group agents', () => {
|
||||
});
|
||||
|
||||
it('auto-creates an agent_destinations row for the wiring', async () => {
|
||||
const { getDestinationByTarget, getDestinations } = await import('./agent-destinations.js');
|
||||
const { getDestinationByTarget, getDestinations } = await import('../modules/agent-to-agent/db/agent-destinations.js');
|
||||
createMessagingGroupAgent(mga());
|
||||
|
||||
const dest = getDestinationByTarget('ag-1', 'channel', 'mg-1');
|
||||
@@ -240,7 +240,7 @@ describe('messaging group agents', () => {
|
||||
});
|
||||
|
||||
it('does not duplicate destination row on re-wiring', async () => {
|
||||
const { getDestinations } = await import('./agent-destinations.js');
|
||||
const { getDestinations } = await import('../modules/agent-to-agent/db/agent-destinations.js');
|
||||
createMessagingGroupAgent(mga());
|
||||
// Re-create the same wiring throws (PK unique), but even if we got the
|
||||
// row in some other way (e.g. via createDestination directly followed
|
||||
@@ -251,7 +251,7 @@ describe('messaging group agents', () => {
|
||||
});
|
||||
|
||||
it('breaks local_name collisions within an agent group', async () => {
|
||||
const { getDestinations } = await import('./agent-destinations.js');
|
||||
const { getDestinations } = await import('../modules/agent-to-agent/db/agent-destinations.js');
|
||||
// Two messaging groups with the same `name` wired to the same agent
|
||||
// should get distinct local_names (gen, gen-2).
|
||||
createMessagingGroupAgent(mga());
|
||||
|
||||
@@ -1,11 +1,20 @@
|
||||
import type { MessagingGroup, MessagingGroupAgent } from '../types.js';
|
||||
// Transitional tier violation: core imports from optional agent-to-agent module.
|
||||
// `createMessagingGroupAgent` auto-creates a destination row on wiring — the
|
||||
// two concerns are currently bundled. When agent-to-agent isn't installed,
|
||||
// the table doesn't exist and this import chain remains dormant because
|
||||
// `createMessagingGroupAgent` is only called from setup/admin paths that
|
||||
// also only run when wiring channels to agents (which implicitly requires
|
||||
// agent-to-agent for the destination ACL to mean anything). A cleaner split
|
||||
// (or making the destination side effect module-owned) is tracked in the
|
||||
// refactor plan.
|
||||
import {
|
||||
createDestination,
|
||||
getDestinationByName,
|
||||
getDestinationByTarget,
|
||||
normalizeName,
|
||||
} from './agent-destinations.js';
|
||||
import { getDb } from './connection.js';
|
||||
} from '../modules/agent-to-agent/db/agent-destinations.js';
|
||||
import { getDb, hasTable } from './connection.js';
|
||||
|
||||
// ── Messaging Groups ──
|
||||
|
||||
@@ -84,21 +93,27 @@ export function createMessagingGroupAgent(mga: MessagingGroupAgent): void {
|
||||
.run(mga);
|
||||
|
||||
// Auto-create an agent_destinations row so delivery's ACL doesn't block
|
||||
// outbound messages that target this chat.
|
||||
// outbound messages that target this chat. Guarded: when the agent-to-agent
|
||||
// module isn't installed the table doesn't exist — skip silently. Without
|
||||
// the module, the ACL check in delivery is also skipped (same guard), so
|
||||
// channel sends still work.
|
||||
//
|
||||
// ⚠️ DESTINATION PROJECTION NOTE: this function only writes the central
|
||||
// `agent_destinations` row. It does NOT project into any running
|
||||
// agent's session inbound.db (see top-of-file invariant in
|
||||
// src/db/agent-destinations.ts). In practice this is fine because the
|
||||
// only real callers are one-shot setup scripts (setup/register.ts,
|
||||
// scripts/init-first-agent.ts, /manage-channels skill) that run in a
|
||||
// separate process from the host. Any already-running container for
|
||||
// `mga.agent_group_id` will keep serving the stale projection until
|
||||
// its next wake (idle timeout or next inbound message) at which
|
||||
// point spawnContainer's writeDestinations call refreshes from central.
|
||||
// If you call this from code that runs INSIDE the host process and
|
||||
// need the refresh to happen immediately, explicitly call
|
||||
// `writeDestinations(mga.agent_group_id, <sessionId>)` afterwards.
|
||||
// src/modules/agent-to-agent/db/agent-destinations.ts). In practice this
|
||||
// is fine because the only real callers are one-shot setup scripts
|
||||
// (setup/register.ts, scripts/init-first-agent.ts, /manage-channels
|
||||
// skill) that run in a separate process from the host. Any already-
|
||||
// running container for `mga.agent_group_id` will keep serving the
|
||||
// stale projection until its next wake (idle timeout or next inbound
|
||||
// message) at which point spawnContainer's writeDestinations call
|
||||
// refreshes from central. If you call this from code that runs INSIDE
|
||||
// the host process and need the refresh to happen immediately,
|
||||
// explicitly call the module's `writeDestinations(mga.agent_group_id,
|
||||
// <sessionId>)` afterwards.
|
||||
if (!hasTable(getDb(), 'agent_destinations')) return;
|
||||
|
||||
const existing = getDestinationByTarget(mga.agent_group_id, 'channel', mga.messaging_group_id);
|
||||
if (existing) return;
|
||||
|
||||
|
||||
@@ -3,7 +3,7 @@ import type Database from 'better-sqlite3';
|
||||
import { log } from '../../log.js';
|
||||
import { migration001 } from './001-initial.js';
|
||||
import { migration002 } from './002-chat-sdk-state.js';
|
||||
import { migration004 } from './004-agent-destinations.js';
|
||||
import { moduleAgentToAgentDestinations } from './module-agent-to-agent-destinations.js';
|
||||
import { migration008 } from './008-dropped-messages.js';
|
||||
import { migration009 } from './009-drop-pending-credentials.js';
|
||||
import { moduleApprovalsPendingApprovals } from './module-approvals-pending-approvals.js';
|
||||
@@ -19,7 +19,7 @@ const migrations: Migration[] = [
|
||||
migration001,
|
||||
migration002,
|
||||
moduleApprovalsPendingApprovals,
|
||||
migration004,
|
||||
moduleAgentToAgentDestinations,
|
||||
moduleApprovalsTitleOptions,
|
||||
migration008,
|
||||
migration009,
|
||||
|
||||
@@ -15,7 +15,10 @@ import type { Migration } from './index.js';
|
||||
* while admin calls the child "worker-1". The (agent_group_id, local_name)
|
||||
* PK enforces uniqueness within a single agent's namespace only.
|
||||
*/
|
||||
export const migration004: Migration = {
|
||||
// Retains the original `name` ('agent-destinations') so existing DBs that
|
||||
// already recorded this migration under that name don't re-run it. The
|
||||
// module- prefix lives on the filename / export identifier only.
|
||||
export const moduleAgentToAgentDestinations: Migration = {
|
||||
version: 4,
|
||||
name: 'agent-destinations',
|
||||
up(db: Database.Database) {
|
||||
Reference in New Issue
Block a user