Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 68 additions & 17 deletions src/modules/approvals/primitive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,26 +195,77 @@ export async function requestApproval(opts: RequestApprovalOptions): Promise<voi

const adapter = getDeliveryAdapter();
if (adapter) {
try {
await adapter.deliver(
target.messagingGroup.channel_type,
target.messagingGroup.platform_id,
null,
'chat-sdk',
JSON.stringify({
type: 'ask_question',
questionId: approvalId,
title,
question,
options: APPROVAL_OPTIONS,
}),
const cardPayload = JSON.stringify({
type: 'ask_question',
questionId: approvalId,
title,
question,
options: APPROVAL_OPTIONS,
});

// Belt-and-suspenders delivery: send the card to the picked DM AND to
// the originating messaging group (the chat the agent was active in
// when it requested the approval). The DM is the right primary
// destination for privacy and routing reasons, but a stale user_dms
// cache row — pointing at an abandoned 1:1 the operator no longer
// watches — can quietly route the card into the void. Posting to the
// origin chat too means the operator sees it where they're already
// paying attention. Click authorization is enforced in the response
// handler (verifies the clicker is an eligible approver), so the
// wider audience doesn't widen the action surface.
const targets: { channel_type: string; platform_id: string; label: string }[] = [];
targets.push({
channel_type: target.messagingGroup.channel_type,
platform_id: target.messagingGroup.platform_id,
label: `DM (${target.userId})`,
});
if (session.messaging_group_id && session.messaging_group_id !== target.messagingGroup.id) {
const origin = getMessagingGroup(session.messaging_group_id);
if (
origin &&
// Avoid double-send if origin and DM resolve to the same physical
// chat (e.g. session is in the user's 1:1 DM already).
!(
origin.channel_type === target.messagingGroup.channel_type &&
origin.platform_id === target.messagingGroup.platform_id
)
) {
targets.push({
channel_type: origin.channel_type,
platform_id: origin.platform_id,
label: `origin (${origin.name ?? origin.id})`,
});
}
}

let anyDelivered = false;
for (const t of targets) {
try {
await adapter.deliver(t.channel_type, t.platform_id, null, 'chat-sdk', cardPayload);
anyDelivered = true;
} catch (err) {
log.warn('Approval card delivery to one target failed', {
action,
approvalId,
target: t.label,
err,
});
}
}
if (!anyDelivered) {
notifyAgent(
session,
`${action} failed: could not deliver approval card to any target (${targets.map((t) => t.label).join(', ')}).`,
);
} 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;
}
}

log.info('Approval requested', { action, approvalId, agentName, approver: target.userId });
log.info('Approval requested', {
action,
approvalId,
agentName,
approver: target.userId,
deliveredTo: session.messaging_group_id ? 'dm+origin' : 'dm',
});
}
27 changes: 26 additions & 1 deletion src/modules/approvals/response-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { log } from '../../log.js';
import { writeSessionMessage } from '../../session-manager.js';
import type { PendingApproval } from '../../types.js';
import { ONECLI_ACTION, resolveOneCLIApproval } from './onecli-approvals.js';
import { getApprovalHandler } from './primitive.js';
import { getApprovalHandler, pickApprover } from './primitive.js';

export async function handleApprovalsResponse(payload: ResponsePayload): Promise<boolean> {
// OneCLI credential approvals — resolved via in-memory Promise first.
Expand Down Expand Up @@ -57,6 +57,31 @@ async function handleRegisteredApproval(
return;
}

// Click authorization: the responder must be in the approver list for this
// session's agent group. We deliver the card to multiple targets (DM +
// origin chat) for visibility, which means non-admin chat members might
// see the buttons. This check prevents them from approving on the admin's
// behalf. The userId comes from the channel adapter's response payload —
// for a button-click in a chat, that's the clicker's namespaced user id.
// Empty userId (legacy or untrusted responder) is rejected.
if (!userId) {
log.warn('Approval click had no userId — ignoring', {
approvalId: approval.approval_id,
action: approval.action,
});
return;
}
const approvers = pickApprover(session.agent_group_id);
if (!approvers.includes(userId)) {
log.warn('Approval click from non-approver — ignoring', {
approvalId: approval.approval_id,
action: approval.action,
clicker: userId,
approvers,
});
return;
}

const notify = (text: string): void => {
writeSessionMessage(session.agent_group_id, session.id, {
id: `appr-note-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`,
Expand Down
Loading