diff --git a/src/cli/crud.ts b/src/cli/crud.ts index a87d71ef8..5a5aca59e 100644 --- a/src/cli/crud.ts +++ b/src/cli/crud.ts @@ -232,6 +232,7 @@ export function registerResource(def: ResourceDef): void { if (def.operations.list) { register({ name: `${def.plural}-list`, + action: `${def.plural}.list`, description: `List all ${def.plural}.`, access: def.operations.list, resource: def.plural, @@ -244,6 +245,7 @@ export function registerResource(def: ResourceDef): void { if (def.operations.get) { register({ name: `${def.plural}-get`, + action: `${def.plural}.get`, description: `Get a ${def.name} by ID.`, access: def.operations.get, resource: def.plural, @@ -256,6 +258,7 @@ export function registerResource(def: ResourceDef): void { if (def.operations.create) { register({ name: `${def.plural}-create`, + action: `${def.plural}.create`, description: `Create a new ${def.name}.`, access: def.operations.create, resource: def.plural, @@ -267,6 +270,7 @@ export function registerResource(def: ResourceDef): void { if (def.operations.update) { register({ name: `${def.plural}-update`, + action: `${def.plural}.update`, description: `Update a ${def.name}.`, access: def.operations.update, resource: def.plural, @@ -278,6 +282,7 @@ export function registerResource(def: ResourceDef): void { if (def.operations.delete) { register({ name: `${def.plural}-delete`, + action: `${def.plural}.delete`, description: `Delete a ${def.name}.`, access: def.operations.delete, resource: def.plural, @@ -291,6 +296,7 @@ export function registerResource(def: ResourceDef): void { for (const [verb, op] of Object.entries(def.customOperations)) { register({ name: `${def.plural}-${verb.replace(/ /g, '-')}`, + action: `${def.plural}.${verb.replace(/ /g, '.')}`, description: op.description, access: op.access, resource: def.plural, diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index f0dc430cb..235f9b8c5 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -12,11 +12,27 @@ import { getSession } from '../db/sessions.js'; import { registerApprovalHandler, requestApproval } from '../modules/approvals/index.js'; import type { CallerContext, ErrorCode, RequestFrame, ResponseFrame } from './frame.js'; import { getResource } from './crud.js'; -import { lookup } from './registry.js'; +import { type CommandDef, lookup } from './registry.js'; -type DispatchOptions = { +/** + * Resolution out-param for middleware that wraps dispatch: dispatch reassigns + * `req` internally (dash-joined id fallback, group-scope auto-fill), so a + * wrapper can't see the resolved command or effective args on the frame it + * passed in. + */ +export type DispatchTrace = { + cmd?: CommandDef; + command?: string; + args?: Record; +}; + +export type DispatchOptions = { /** True when a command is being replayed after approval. */ approved?: boolean; + /** Id of the approval that authorized this replay — correlation for middleware/observers. */ + approvalId?: string; + /** Filled by dispatch with the resolved command + effective args. */ + trace?: DispatchTrace; }; export async function dispatch( @@ -48,6 +64,12 @@ export async function dispatch( } } + if (opts.trace) { + opts.trace.cmd = cmd; + opts.trace.command = req.command; + opts.trace.args = req.args; + } + if (!cmd) { return err(req.id, 'unknown-command', `no command "${req.command}"`); } @@ -102,6 +124,7 @@ export async function dispatch( fill.id = req.args.id ?? ctx.agentGroupId; } req = { ...req, args: { ...req.args, ...fill } }; + if (opts.trace) opts.trace.args = req.args; // Fail-closed pre-handler check for sessions-get: returns "not found" // regardless of whether the UUID exists in another group, preventing an @@ -192,10 +215,12 @@ export async function dispatch( } } -registerApprovalHandler('cli_command', async ({ payload, notify }) => { +registerApprovalHandler('cli_command', async ({ payload, notify, approvalId }) => { const frame = payload.frame as RequestFrame; const callerContext = parseCallerContext(payload.callerContext) ?? { caller: 'host' }; - const response = await dispatch(frame, callerContext, { approved: true }); + // approvalId rides opts so middleware/observers around dispatch can + // correlate the replay with the approval that authorized it. + const response = await dispatch(frame, callerContext, { approved: true, approvalId }); if (response.ok) { const data = typeof response.data === 'string' ? response.data : JSON.stringify(response.data, null, 2); diff --git a/src/cli/registry.ts b/src/cli/registry.ts index 8c0019d97..ca7306175 100644 --- a/src/cli/registry.ts +++ b/src/cli/registry.ts @@ -31,18 +31,30 @@ export type CommandDef = { * Custom operations return ad-hoc shapes and leave this undefined. */ generic?: 'list' | 'get'; + /** + * Canonical dotted action name, e.g. `groups.config.add-mcp-server` — for + * middleware and observability surfaces that need namespaced verbs. Stamped + * explicitly by `registerResource` (which knows verb segment boundaries); + * hand-registered commands may omit it and get `name` with dashes→dots. + */ + action: string; /** Validates `frame.args` and produces the typed handler input. Throws on invalid. */ parseArgs: (raw: Record) => TArgs; handler: (args: TArgs, ctx: CallerContext) => Promise; }; +/** `register()` input — `action` is defaulted, everything else as stored. */ +export type CommandInput = Omit, 'action'> & { + action?: string; +}; + const registry = new Map(); -export function register(def: CommandDef): void { +export function register(def: CommandInput): void { if (registry.has(def.name)) { throw new Error(`CLI command "${def.name}" already registered`); } - registry.set(def.name, def as CommandDef); + registry.set(def.name, { ...def, action: def.action ?? def.name.replace(/-/g, '.') } as CommandDef); } export function lookup(name: string): CommandDef | undefined { diff --git a/src/modules/agent-to-agent/create-agent.ts b/src/modules/agent-to-agent/create-agent.ts index b8044da39..10536f12b 100644 --- a/src/modules/agent-to-agent/create-agent.ts +++ b/src/modules/agent-to-agent/create-agent.ts @@ -112,6 +112,11 @@ export const applyCreateAgent: ApprovalHandler = async ({ session, payload, noti await performCreateAgent(name, instructions, session, sourceGroup, notify); }; +/** What creation did — the created group's ids on success, the reason on failure. */ +export type CreateAgentResult = + | { ok: true; agentGroupId: string; name: string; localName: string; folder: string } + | { ok: false; reason: string }; + /** * Core creation: writes the new agent group + bidirectional destinations and * scaffolds its filesystem, then reports via `notify`. Authorization is the @@ -126,13 +131,13 @@ async function performCreateAgent( session: Session, sourceGroup: AgentGroup, notify: (text: string) => void, -): Promise { +): Promise { const localName = normalizeName(name); // Collision in the creator's destination namespace if (getDestinationByName(sourceGroup.id, localName)) { notify(`Cannot create agent "${name}": you already have a destination named "${localName}".`); - return; + return { ok: false, reason: 'destination name collision' }; } // Derive a safe folder name, deduplicated globally across agent_groups.folder @@ -149,7 +154,7 @@ async function performCreateAgent( if (!resolvedPath.startsWith(resolvedGroupsDir + path.sep)) { notify(`Cannot create agent "${name}": invalid folder path.`); log.error('create_agent path traversal attempt', { folder, resolvedPath }); - return; + return { ok: false, reason: 'invalid folder path' }; } const agentGroupId = `ag-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; @@ -208,4 +213,5 @@ async function performCreateAgent( notify(`Agent "${localName}" created. You can now message it with ....`); log.info('Agent group created', { agentGroupId, name, localName, folder, parent: sourceGroup.id }); + return { ok: true, agentGroupId, name, localName, folder }; } diff --git a/src/modules/agent-to-agent/message-gate.test.ts b/src/modules/agent-to-agent/message-gate.test.ts index fa1a485f6..e5e9d9b88 100644 --- a/src/modules/agent-to-agent/message-gate.test.ts +++ b/src/modules/agent-to-agent/message-gate.test.ts @@ -165,6 +165,7 @@ describe('agent message policies', () => { await applyA2aMessageGate({ session: SA, userId: 'slack:dana', + approvalId: 'appr-test', notify, payload: { id: 'held-1', platform_id: B, content: JSON.stringify({ text: 'approved!' }), in_reply_to: null }, }); diff --git a/src/modules/approvals/onecli-approvals.ts b/src/modules/approvals/onecli-approvals.ts index 6d7183b88..b95b4e957 100644 --- a/src/modules/approvals/onecli-approvals.ts +++ b/src/modules/approvals/onecli-approvals.ts @@ -64,8 +64,12 @@ function shortApprovalId(): string { return `oa-${Math.random().toString(36).slice(2, 10)}`; } -/** Called from the approvals response handler when a card button is clicked. */ -export function resolveOneCLIApproval(approvalId: string, selectedOption: string): boolean { +/** + * Called from the approvals response handler when a card button is clicked. + * `userId` is the namespaced id of the clicking admin — the resolution's + * identity, recorded here (empty when the resolver is a timer/sweep). + */ +export function resolveOneCLIApproval(approvalId: string, selectedOption: string, userId: string): boolean { const state = pending.get(approvalId); if (!state) return false; pending.delete(approvalId); @@ -78,7 +82,7 @@ export function resolveOneCLIApproval(approvalId: string, selectedOption: string deletePendingApproval(approvalId); state.resolve(decision); - log.info('OneCLI approval resolved', { approvalId, decision }); + log.info('OneCLI approval resolved', { approvalId, decision, decidedBy: userId || 'system' }); return true; } diff --git a/src/modules/approvals/primitive.ts b/src/modules/approvals/primitive.ts index 2d3028a64..fab32889c 100644 --- a/src/modules/approvals/primitive.ts +++ b/src/modules/approvals/primitive.ts @@ -61,6 +61,8 @@ export interface ApprovalHandlerContext { payload: Record; /** User ID of the admin who approved. Empty string if unknown. */ userId: string; + /** pending_approvals id of the approval that authorized this run. */ + approvalId: string; /** Send a system chat message to the requesting agent's session. */ notify: (text: string) => void; } @@ -214,19 +216,31 @@ export interface RequestApprovalOptions { approverUserId?: string; } +/** + * A successfully queued hold: the minted approval id and the approver the + * card was actually delivered to — both decided in here and otherwise + * invisible to callers. Null when no hold reached an approver. + */ +export interface ApprovalHold { + approvalId: string; + /** The approver the card was actually delivered to. */ + approverUserId: string; +} + /** * Queue an approval request. Picks an approver, delivers the card to their * DM, and records the pending_approvals row. Fire-and-forget from the - * caller's perspective — the admin's response kicks off the registered - * approval handler for this action via the response dispatcher. + * caller's perspective (the returned hold is informational) — the admin's + * response kicks off the registered approval handler for this action via the + * response dispatcher. */ -export async function requestApproval(opts: RequestApprovalOptions): Promise { +export async function requestApproval(opts: RequestApprovalOptions): Promise { const { session, action, payload, title, question, agentName, approverUserId } = opts; const approvers = approverUserId ? [approverUserId] : pickApprover(session.agent_group_id); if (approvers.length === 0) { notifyAgent(session, `${action} failed: no owner or admin configured to approve.`); - return; + return null; } const originChannelType = session.messaging_group_id @@ -236,7 +250,7 @@ export async function requestApproval(opts: RequestApprovalOptions): Promise { +async function handleSenderApprovalResponse(payload: ResponsePayload): Promise { const row = getPendingSenderApproval(payload.questionId); - if (!row) return false; + if (!row) return { claimed: false }; // payload.userId is the raw platform userId (e.g. "6037840640"); namespace it // with the channel type so it matches users(id) format. Some platforms @@ -243,10 +259,18 @@ async function handleSenderApprovalResponse(payload: ResponsePayload): Promise (await handleSenderApprovalResponse(payload)).claimed); // ── Unknown-channel registration flow ── @@ -293,6 +317,23 @@ setChannelRequestGate(async (mg, event) => { await requestChannelApproval({ messagingGroupId: mg.id, event }); }); +/** + * What a channel-registration flow decided: connected (to which agent group, + * created or existing), rejected, or failed — and which admin drove it. Like + * SenderDecision, the registry consumes only `claimed`. + */ +export interface ChannelDecision { + kind: 'connected' | 'rejected' | 'failed'; + messagingGroupId: string; + approverId: string; + channelType?: string; + agentGroupId?: string; + createdAgentGroup?: boolean; + agentName?: string; + reason?: string; +} +export type ChannelApprovalResult = { claimed: boolean; decision?: ChannelDecision }; + /** * Response handler for the unknown-channel registration card. * @@ -307,9 +348,9 @@ setChannelRequestGate(async (mg, event) => { * captures the reply and creates immediately) * reject — set denied_at, delete pending row */ -async function handleChannelApprovalResponse(payload: ResponsePayload): Promise { +async function handleChannelApprovalResponse(payload: ResponsePayload): Promise { const row = getPendingChannelApproval(payload.questionId); - if (!row) return false; + if (!row) return { claimed: false }; const clickerId = payload.userId ? payload.userId.includes(':') @@ -324,9 +365,14 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise< clickerId, expectedApprover: row.approver_user_id, }); - return true; + return { claimed: true }; } const approverId = clickerId; + const decisionBase = { + messagingGroupId: row.messaging_group_id, + approverId, + channelType: payload.channelType, + }; // ── Reject / Cancel ── if (payload.value === REJECT_VALUE) { @@ -336,7 +382,7 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise< messagingGroupId: row.messaging_group_id, approverId, }); - return true; + return { claimed: true, decision: { kind: 'rejected', ...decisionBase } }; } // ── Choose existing agent — send agent-selection follow-up card ── @@ -347,11 +393,11 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise< messagingGroupId: row.messaging_group_id, approverUserId: row.approver_user_id, }); - return true; + return { claimed: true }; } const adapter = getDeliveryAdapter(); - if (!adapter) return true; + if (!adapter) return { claimed: true }; const agentGroups = getAllAgentGroups(); const options = buildAgentSelectionOptions(agentGroups, approverId); @@ -378,7 +424,7 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise< err, }); } - return true; + return { claimed: true }; } // ── Create new agent — prompt for free-text name ── @@ -389,7 +435,7 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise< messagingGroupId: row.messaging_group_id, approverUserId: row.approver_user_id, }); - return true; + return { claimed: true }; } const adapter = getDeliveryAdapter(); @@ -397,7 +443,7 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise< log.error('Channel registration: no delivery adapter for name prompt', { messagingGroupId: row.messaging_group_id, }); - return true; + return { claimed: true }; } awaitingNameInput.set(row.approver_user_id, { @@ -421,7 +467,7 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise< }); awaitingNameInput.delete(row.approver_user_id); } - return true; + return { claimed: true }; } // ── Resolve target agent group (connect to existing or create new) ── @@ -436,7 +482,10 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise< targetAgentGroupId, }); deletePendingChannelApproval(row.messaging_group_id); - return true; + return { + claimed: true, + decision: { kind: 'failed', reason: 'target agent group no longer exists', ...decisionBase }, + }; } if (!hasAdminPrivilege(approverId, targetAgentGroupId)) { log.warn('Channel registration: target agent group rejected for unauthorized approver', { @@ -444,14 +493,14 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise< targetAgentGroupId, approverId, }); - return true; + return { claimed: true }; } } else { log.warn('Channel registration: unknown response value', { messagingGroupId: row.messaging_group_id, value: payload.value, }); - return true; + return { claimed: true }; } // ── Wire + replay (shared path for connect and create) ── @@ -464,7 +513,7 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise< err, }); deletePendingChannelApproval(row.messaging_group_id); - return true; + return { claimed: true, decision: { kind: 'failed', reason: 'failed to parse stored event', ...decisionBase } }; } const isGroup = event.threadId !== null; @@ -512,22 +561,26 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise< err, }); } - return true; + return { + claimed: true, + decision: { kind: 'connected', agentGroupId: targetAgentGroupId, createdAgentGroup: false, ...decisionBase }, + }; } -registerResponseHandler(handleChannelApprovalResponse); +registerResponseHandler(async (payload) => (await handleChannelApprovalResponse(payload)).claimed); // ── Free-text name interceptor ── // Captures the next DM from an approver who clicked "Create new agent", // creates the agent immediately, wires the channel, and replays. -registerMessageInterceptor(async (event: InboundEvent): Promise => { +async function channelNameInterceptor(event: InboundEvent): Promise { const userId = extractAndUpsertUser(event); - if (!userId) return false; + if (!userId) return { claimed: false }; const pending = awaitingNameInput.get(userId); - if (!pending) return false; - if (event.channelType !== pending.dmChannelType || event.platformId !== pending.dmPlatformId) return false; + if (!pending) return { claimed: false }; + if (event.channelType !== pending.dmChannelType || event.platformId !== pending.dmPlatformId) + return { claimed: false }; awaitingNameInput.delete(userId); @@ -541,11 +594,11 @@ registerMessageInterceptor(async (event: InboundEvent): Promise => { if (!text) { log.warn('Channel registration: empty name reply, ignoring', { userId }); - return true; + return { claimed: true }; } const row = getPendingChannelApproval(pending.channelMgId); - if (!row) return true; + if (!row) return { claimed: true }; const ag = createNewAgentGroup(text); log.info('Channel registration: new agent group created', { @@ -554,6 +607,14 @@ registerMessageInterceptor(async (event: InboundEvent): Promise => { agentName: ag.name, folder: ag.folder, }); + const decisionBase = { + messagingGroupId: row.messaging_group_id, + approverId: userId, + channelType: event.channelType, + agentGroupId: ag.id, + createdAgentGroup: true, + agentName: ag.name, + }; let originalEvent: InboundEvent; try { @@ -564,7 +625,8 @@ registerMessageInterceptor(async (event: InboundEvent): Promise => { err, }); deletePendingChannelApproval(row.messaging_group_id); - return true; + // The group WAS created above — the decision descriptor must say so. + return { claimed: true, decision: { kind: 'failed', reason: 'failed to parse stored event', ...decisionBase } }; } const isGroup = originalEvent.threadId !== null; @@ -628,5 +690,7 @@ registerMessageInterceptor(async (event: InboundEvent): Promise => { .catch(() => {}); } } - return true; -}); + return { claimed: true, decision: { kind: 'connected', ...decisionBase } }; +} + +registerMessageInterceptor(async (event) => (await channelNameInterceptor(event)).claimed);