refactor: richer results + context threading at the dispatch/approval seams

Standalone seam contracts, no consumer shipped in trunk:
- CommandDef.action: canonical dotted action name, stamped by
  registerResource (verb-boundary-aware), defaulted from name otherwise
- dispatch: DispatchTrace out-param (resolved cmd + effective args) and
  approvalId on DispatchOptions for replay correlation
- requestApproval returns ApprovalHold (minted id + picked approver);
  ApprovalHandlerContext carries approvalId
- resolveOneCLIApproval takes the resolving admin's userId (logged)
- permissions response handlers return SenderApprovalResult /
  ChannelApprovalResult decision descriptors; registry coercions keep the
  claimed-boolean contract
- performCreateAgent returns CreateAgentResult

These are the integration points /add-audit composes on (next commit) —
middleware/observers wrap here without reaching into function bodies.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Moshe Krupper
2026-07-06 21:57:02 +03:00
parent b6cb53e21c
commit d413b9a839
9 changed files with 184 additions and 51 deletions
+6
View File
@@ -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,
+29 -4
View File
@@ -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<string, unknown>;
};
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);
+14 -2
View File
@@ -31,18 +31,30 @@ export type CommandDef<TArgs = unknown, TData = unknown> = {
* 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<string, unknown>) => TArgs;
handler: (args: TArgs, ctx: CallerContext) => Promise<TData>;
};
/** `register()` input — `action` is defaulted, everything else as stored. */
export type CommandInput<TArgs = unknown, TData = unknown> = Omit<CommandDef<TArgs, TData>, 'action'> & {
action?: string;
};
const registry = new Map<string, CommandDef>();
export function register<TArgs, TData>(def: CommandDef<TArgs, TData>): void {
export function register<TArgs, TData>(def: CommandInput<TArgs, TData>): 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 {
+9 -3
View File
@@ -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<void> {
): Promise<CreateAgentResult> {
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 <message to="${localName}">...</message>.`);
log.info('Agent group created', { agentGroupId, name, localName, folder, parent: sourceGroup.id });
return { ok: true, agentGroupId, name, localName, folder };
}
@@ -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 },
});
+7 -3
View File
@@ -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;
}
+21 -6
View File
@@ -61,6 +61,8 @@ export interface ApprovalHandlerContext {
payload: Record<string, unknown>;
/** 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<void> {
export async function requestApproval(opts: RequestApprovalOptions): Promise<ApprovalHold | null> {
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<voi
const target = await pickApprovalDelivery(approvers, originChannelType);
if (!target) {
notifyAgent(session, `${action} failed: no DM channel found for any eligible approver.`);
return;
return null;
}
const approvalId = `appr-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
@@ -272,9 +286,10 @@ export async function requestApproval(opts: RequestApprovalOptions): Promise<voi
} catch (err) {
log.error('Failed to deliver approval card', { action, approvalId, err });
notifyAgent(session, `${action} failed: could not deliver approval request to ${target.userId}.`);
return;
return null;
}
}
log.info('Approval requested', { action, approvalId, agentName, approver: target.userId });
return { approvalId, approverUserId: target.userId };
}
+2 -2
View File
@@ -42,7 +42,7 @@ export async function handleApprovalsResponse(payload: ResponsePayload): Promise
}
if (approval.action === ONECLI_ACTION) {
if (resolveOneCLIApproval(payload.questionId, payload.value)) {
if (resolveOneCLIApproval(payload.questionId, payload.value, namespacedUserId(payload) ?? '')) {
return true;
}
// Row exists but the in-memory resolver is gone (timer fired or the process
@@ -112,7 +112,7 @@ async function handleRegisteredApproval(
const payload = JSON.parse(approval.payload);
try {
await handler({ session, payload, userId, notify });
await handler({ session, payload, userId, approvalId: approval.approval_id, notify });
log.info('Approval handled', { approvalId: approval.approval_id, action: approval.action, userId });
} catch (err) {
log.error('Approval handler threw', { approvalId: approval.approval_id, action: approval.action, err });
+95 -31
View File
@@ -208,6 +208,22 @@ setSenderScopeGate(
},
);
/**
* What a sender-approval card click decided who was allowed/denied into
* which group, and which admin decided. The response registry consumes only
* `claimed`; the decision descriptor is for observers composed around the
* handler (and for tests).
*/
export interface SenderDecision {
approved: boolean;
senderIdentity: string;
agentGroupId: string;
messagingGroupId: string;
approverId: string;
channelType: string;
}
export type SenderApprovalResult = { claimed: boolean; decision?: SenderDecision };
/**
* Response handler for the unknown-sender approval card.
*
@@ -222,9 +238,9 @@ setSenderScopeGate(
* Deny: delete the row (no "deny list" a future message re-triggers a
* fresh card per ACTION-ITEMS item 5 "no denial persistence").
*/
async function handleSenderApprovalResponse(payload: ResponsePayload): Promise<boolean> {
async function handleSenderApprovalResponse(payload: ResponsePayload): Promise<SenderApprovalResult> {
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<b
clickerId,
expectedApprover: row.approver_user_id,
});
return true; // claim the response so it's not unclaimed-logged, but do nothing
return { claimed: true }; // claim the response so it's not unclaimed-logged, but do nothing
}
const approverId = clickerId;
const approved = payload.value === 'approve';
const decision = {
approved,
senderIdentity: row.sender_identity,
agentGroupId: row.agent_group_id,
messagingGroupId: row.messaging_group_id,
approverId,
channelType: payload.channelType,
};
if (approved) {
addMember({
@@ -272,7 +296,7 @@ async function handleSenderApprovalResponse(payload: ResponsePayload): Promise<b
} catch (err) {
log.error('Failed to replay message after sender approval', { approvalId: row.id, err });
}
return true;
return { claimed: true, decision };
}
log.info('Unknown sender denied', {
@@ -282,10 +306,10 @@ async function handleSenderApprovalResponse(payload: ResponsePayload): Promise<b
approverId,
});
deletePendingSenderApproval(row.id);
return true;
return { claimed: true, decision };
}
registerResponseHandler(handleSenderApprovalResponse);
registerResponseHandler(async (payload) => (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<boolean> {
async function handleChannelApprovalResponse(payload: ResponsePayload): Promise<ChannelApprovalResult> {
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<boolean> => {
async function channelNameInterceptor(event: InboundEvent): Promise<ChannelApprovalResult> {
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<boolean> => {
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<boolean> => {
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<boolean> => {
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<boolean> => {
.catch(() => {});
}
}
return true;
});
return { claimed: true, decision: { kind: 'connected', ...decisionBase } };
}
registerMessageInterceptor(async (event) => (await channelNameInterceptor(event)).claimed);