mirror of
https://github.com/qwibitai/nanoclaw.git
synced 2026-07-06 18:52:03 +08:00
Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 1912419137 | |||
| c97548db60 |
@@ -207,6 +207,20 @@ export function getExpiredAwaitingReasonApprovals(nowIso: string): PendingApprov
|
||||
.all(nowIso) as PendingApproval[];
|
||||
}
|
||||
|
||||
/**
|
||||
* Module approvals (session-scoped) that sat unanswered past their expiry —
|
||||
* the timeout sweep's set. Scoped to `session_id IS NOT NULL` so OneCLI
|
||||
* credential rows (session-less, resolved in-memory by the gateway callback)
|
||||
* are never swept here even though they also carry an `expires_at`.
|
||||
*/
|
||||
export function getExpiredPendingApprovals(nowIso: string): PendingApproval[] {
|
||||
return getDb()
|
||||
.prepare(
|
||||
"SELECT * FROM pending_approvals WHERE status = 'pending' AND session_id IS NOT NULL AND expires_at IS NOT NULL AND expires_at <= ?",
|
||||
)
|
||||
.all(nowIso) as PendingApproval[];
|
||||
}
|
||||
|
||||
export function deletePendingApproval(approvalId: string): void {
|
||||
getDb().prepare('DELETE FROM pending_approvals WHERE approval_id = ?').run(approvalId);
|
||||
}
|
||||
|
||||
@@ -164,6 +164,18 @@ async function sweep(): Promise<void> {
|
||||
}
|
||||
// MODULE-HOOK:approvals-reason-sweep:end
|
||||
|
||||
// Finalize module approvals that timed out unanswered — the pending row
|
||||
// would otherwise strand the requesting agent forever. Central-DB scan, once
|
||||
// per tick.
|
||||
// MODULE-HOOK:approvals-timeout-sweep:start
|
||||
try {
|
||||
const { sweepExpiredModuleApprovals } = await import('./modules/approvals/index.js');
|
||||
await sweepExpiredModuleApprovals();
|
||||
} catch (err) {
|
||||
log.error('Module-approval timeout sweep failed', { err });
|
||||
}
|
||||
// MODULE-HOOK:approvals-timeout-sweep:end
|
||||
|
||||
setTimeout(sweep, SWEEP_INTERVAL_MS);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,11 +1,12 @@
|
||||
/**
|
||||
* Shared "finalize a rejected approval" path.
|
||||
* Shared "finalize a resolved approval" path.
|
||||
*
|
||||
* Three entry points land here so they relay one message and clean up
|
||||
* Four entry points land here so they relay one message and clean up
|
||||
* identically:
|
||||
* 1. The instant Reject button (response-handler.ts)
|
||||
* 2. A captured Reject-with-reason reply (reason-capture.ts)
|
||||
* 3. The host-sweep ghost finalizer (reason-capture.ts, via host-sweep)
|
||||
* 4. The host-sweep timeout finalizer (timeout-sweep.ts, via host-sweep)
|
||||
*
|
||||
* Kept in its own leaf file so both response-handler.ts and reason-capture.ts
|
||||
* can import it without an import cycle (finalize → primitive only).
|
||||
@@ -36,6 +37,37 @@ export async function finalizeReject(
|
||||
? `Your ${approval.action} request was rejected by admin: "${reason}"`
|
||||
: `Your ${approval.action} request was rejected by admin.`;
|
||||
|
||||
writeAgentNote(session, text);
|
||||
|
||||
log.info('Approval rejected', {
|
||||
approvalId: approval.approval_id,
|
||||
action: approval.action,
|
||||
userId,
|
||||
withReason: reason !== undefined,
|
||||
});
|
||||
|
||||
await finalizeResolution(approval, session, userId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Finalize a module approval that no admin ever answered — its expiry elapsed.
|
||||
* Same cleanup as a plain reject (drop the row, fire resolved callbacks, wake
|
||||
* the container) but tells the agent the request timed out rather than that an
|
||||
* admin rejected it, since no admin acted.
|
||||
*/
|
||||
export async function finalizeTimeout(approval: PendingApproval, session: Session): Promise<void> {
|
||||
writeAgentNote(session, `Your ${approval.action} request timed out waiting for admin approval.`);
|
||||
|
||||
log.info('Approval timed out', {
|
||||
approvalId: approval.approval_id,
|
||||
action: approval.action,
|
||||
});
|
||||
|
||||
await finalizeResolution(approval, session, '');
|
||||
}
|
||||
|
||||
/** Relay a one-line system note to the requesting agent's session. */
|
||||
function writeAgentNote(session: Session, text: string): void {
|
||||
writeSessionMessage(session.agent_group_id, session.id, {
|
||||
id: `appr-note-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`,
|
||||
kind: 'chat',
|
||||
@@ -45,14 +77,10 @@ export async function finalizeReject(
|
||||
threadId: null,
|
||||
content: JSON.stringify({ text, sender: 'system', senderId: 'system' }),
|
||||
});
|
||||
}
|
||||
|
||||
log.info('Approval rejected', {
|
||||
approvalId: approval.approval_id,
|
||||
action: approval.action,
|
||||
userId,
|
||||
withReason: reason !== undefined,
|
||||
});
|
||||
|
||||
/** Drop the pending row, fire approval-resolved callbacks, and wake the container. */
|
||||
async function finalizeResolution(approval: PendingApproval, session: Session, userId: string): Promise<void> {
|
||||
deletePendingApproval(approval.approval_id);
|
||||
await notifyApprovalResolved({ approval, session, outcome: 'reject', userId });
|
||||
await wakeContainer(session);
|
||||
|
||||
@@ -33,6 +33,8 @@ export type { ApprovalHandler, ApprovalHandlerContext, RequestApprovalOptions }
|
||||
// Host-sweep hook for ghosted "Reject with reason…" holds. The re-export also
|
||||
// loads reason-capture.js, registering its message-interceptor on import.
|
||||
export { sweepAwaitingReasonRejects } from './reason-capture.js';
|
||||
// Host-sweep hook for module approvals that timed out unanswered.
|
||||
export { sweepExpiredModuleApprovals } from './timeout-sweep.js';
|
||||
|
||||
registerResponseHandler(handleApprovalsResponse);
|
||||
|
||||
|
||||
@@ -0,0 +1,175 @@
|
||||
/**
|
||||
* Delivery-failure handling for module approvals (requestApproval).
|
||||
*
|
||||
* The row is created before the card is delivered. If delivery throws — or no
|
||||
* delivery adapter is wired — the row must be dropped so the requesting agent
|
||||
* hears the real failure immediately, instead of the row sitting until the
|
||||
* 7-day expiry sweep clears it with a misleading "timed out" message. Mirrors
|
||||
* the sender/channel delivery-failure tests.
|
||||
*/
|
||||
import fs from 'fs';
|
||||
import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { initTestDb, closeDb, runMigrations } from '../../db/index.js';
|
||||
import { createAgentGroup } from '../../db/agent-groups.js';
|
||||
import { createMessagingGroup } from '../../db/messaging-groups.js';
|
||||
import { createSession } from '../../db/sessions.js';
|
||||
import { upsertUser } from '../permissions/db/users.js';
|
||||
import { grantRole } from '../permissions/db/user-roles.js';
|
||||
import type { Session } from '../../types.js';
|
||||
|
||||
// Delivery adapter is swapped per-test via `adapterValue`.
|
||||
const deliverMock = vi.fn().mockResolvedValue('plat-msg-id');
|
||||
let adapterValue: { deliver: typeof deliverMock } | null = { deliver: deliverMock };
|
||||
vi.mock('../../delivery.js', () => ({
|
||||
getDeliveryAdapter: () => adapterValue,
|
||||
}));
|
||||
|
||||
// notifyAgent writes to the session inbound.db and wakes the container; stub
|
||||
// both so the failure path doesn't need a real session folder or docker.
|
||||
const writeSessionMessageMock = vi.fn();
|
||||
vi.mock('../../session-manager.js', () => ({
|
||||
writeSessionMessage: (...args: unknown[]) => writeSessionMessageMock(...args),
|
||||
}));
|
||||
const wakeContainerMock = vi.fn().mockResolvedValue(undefined);
|
||||
vi.mock('../../container-runner.js', () => ({
|
||||
wakeContainer: (...args: unknown[]) => wakeContainerMock(...args),
|
||||
isContainerRunning: () => false,
|
||||
getActiveContainerCount: () => 0,
|
||||
killContainer: vi.fn(),
|
||||
}));
|
||||
|
||||
// Resolve the approver's DM from the user_dms table instead of a real openDM RPC.
|
||||
vi.mock('../permissions/user-dm.js', () => ({
|
||||
ensureUserDm: vi.fn(async (userId: string) => {
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
return getDb()
|
||||
.prepare(
|
||||
`SELECT mg.* FROM messaging_groups mg
|
||||
JOIN user_dms ud ON ud.messaging_group_id = mg.id
|
||||
WHERE ud.user_id = ?`,
|
||||
)
|
||||
.get(userId);
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock('../../config.js', async () => {
|
||||
const actual = await vi.importActual('../../config.js');
|
||||
return { ...actual, DATA_DIR: '/tmp/nanoclaw-test-primitive-approval' };
|
||||
});
|
||||
|
||||
const TEST_DIR = '/tmp/nanoclaw-test-primitive-approval';
|
||||
|
||||
function now() {
|
||||
return new Date().toISOString();
|
||||
}
|
||||
|
||||
function pendingCount(db: { prepare: (sql: string) => { get: () => unknown } }) {
|
||||
return (db.prepare('SELECT COUNT(*) AS c FROM pending_approvals').get() as { c: number }).c;
|
||||
}
|
||||
|
||||
const SESSION: Session = {
|
||||
id: 'sess-1',
|
||||
agent_group_id: 'ag-1',
|
||||
messaging_group_id: 'mg-chat',
|
||||
thread_id: null,
|
||||
agent_provider: null,
|
||||
status: 'active',
|
||||
container_status: 'stopped',
|
||||
last_active: now(),
|
||||
created_at: now(),
|
||||
};
|
||||
|
||||
async function requestApprovalOnce() {
|
||||
const { requestApproval } = await import('./primitive.js');
|
||||
await requestApproval({
|
||||
session: SESSION,
|
||||
agentName: 'Agent',
|
||||
action: 'install_packages',
|
||||
payload: { packages: ['jq'] },
|
||||
title: 'Install packages?',
|
||||
question: 'The agent wants to install: jq',
|
||||
});
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
if (fs.existsSync(TEST_DIR)) fs.rmSync(TEST_DIR, { recursive: true });
|
||||
fs.mkdirSync(TEST_DIR, { recursive: true });
|
||||
const db = initTestDb();
|
||||
runMigrations(db);
|
||||
|
||||
createAgentGroup({ id: 'ag-1', name: 'Agent', folder: 'agent', agent_provider: null, created_at: now() });
|
||||
createMessagingGroup({
|
||||
id: 'mg-chat',
|
||||
channel_type: 'telegram',
|
||||
platform_id: 'chat-123',
|
||||
name: 'Group Chat',
|
||||
is_group: 1,
|
||||
unknown_sender_policy: 'public',
|
||||
created_at: now(),
|
||||
});
|
||||
createSession(SESSION);
|
||||
|
||||
// Owner + their DM messaging group — the approver requestApproval delivers to.
|
||||
upsertUser({ id: 'telegram:owner', kind: 'telegram', display_name: 'Owner', created_at: now() });
|
||||
grantRole({ user_id: 'telegram:owner', role: 'owner', agent_group_id: null, granted_by: null, granted_at: now() });
|
||||
createMessagingGroup({
|
||||
id: 'mg-dm-owner',
|
||||
channel_type: 'telegram',
|
||||
platform_id: 'dm-owner',
|
||||
name: 'Owner DM',
|
||||
is_group: 0,
|
||||
unknown_sender_policy: 'public',
|
||||
created_at: now(),
|
||||
});
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
getDb()
|
||||
.prepare(
|
||||
`INSERT INTO user_dms (user_id, channel_type, messaging_group_id, resolved_at)
|
||||
VALUES (?, ?, ?, ?)`,
|
||||
)
|
||||
.run('telegram:owner', 'telegram', 'mg-dm-owner', now());
|
||||
|
||||
adapterValue = { deliver: deliverMock };
|
||||
deliverMock.mockClear();
|
||||
writeSessionMessageMock.mockClear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
closeDb();
|
||||
if (fs.existsSync(TEST_DIR)) fs.rmSync(TEST_DIR, { recursive: true });
|
||||
});
|
||||
|
||||
describe('requestApproval delivery failure', () => {
|
||||
it('keeps the pending row when the card delivers', async () => {
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
await requestApprovalOnce();
|
||||
|
||||
expect(deliverMock).toHaveBeenCalledTimes(1);
|
||||
expect(pendingCount(getDb())).toBe(1);
|
||||
});
|
||||
|
||||
it('drops the pending row when card delivery throws', async () => {
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
deliverMock.mockRejectedValueOnce(new Error('delivery boom'));
|
||||
|
||||
await requestApprovalOnce();
|
||||
|
||||
expect(deliverMock).toHaveBeenCalledTimes(1);
|
||||
// Row rolled back so the agent isn't blocked for 7 days behind a lost card...
|
||||
expect(pendingCount(getDb())).toBe(0);
|
||||
// ...and it was told about the failure now.
|
||||
expect(writeSessionMessageMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('drops the pending row when no delivery adapter is wired', async () => {
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
adapterValue = null;
|
||||
|
||||
await requestApprovalOnce();
|
||||
|
||||
expect(deliverMock).not.toHaveBeenCalled();
|
||||
expect(pendingCount(getDb())).toBe(0);
|
||||
expect(writeSessionMessageMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -23,7 +23,7 @@
|
||||
*/
|
||||
import { normalizeOptions, type RawOption } from '../../channels/ask-question.js';
|
||||
import { getMessagingGroup } from '../../db/messaging-groups.js';
|
||||
import { createPendingApproval, getSession } from '../../db/sessions.js';
|
||||
import { createPendingApproval, deletePendingApproval, getSession } from '../../db/sessions.js';
|
||||
import { getDeliveryAdapter } from '../../delivery.js';
|
||||
import { wakeContainer } from '../../container-runner.js';
|
||||
import { log } from '../../log.js';
|
||||
@@ -39,6 +39,15 @@ import { ensureUserDm } from '../permissions/user-dm.js';
|
||||
*/
|
||||
export const REJECT_WITH_REASON_VALUE = 'reject_with_reason';
|
||||
|
||||
/**
|
||||
* How long a module approval waits for an admin before the host sweep
|
||||
* finalizes it as a timeout. Without this a card that's never answered (or
|
||||
* whose delivery silently fell on the floor) leaves a `status='pending'` row
|
||||
* that blocks the requesting agent forever. Reuses the `expires_at` column;
|
||||
* the awaiting-reason hold overwrites it with its own short window on transition.
|
||||
*/
|
||||
export const MODULE_APPROVAL_TIMEOUT_MS = 7 * 24 * 60 * 60 * 1000;
|
||||
|
||||
/**
|
||||
* Three-button approval UI. Plain Reject is the instant fast path; "Reject with
|
||||
* reason…" opts into the reason-capture flow. Shared by every module approval
|
||||
@@ -248,32 +257,43 @@ export async function requestApproval(opts: RequestApprovalOptions): Promise<voi
|
||||
action,
|
||||
payload: JSON.stringify(payload),
|
||||
created_at: new Date().toISOString(),
|
||||
expires_at: new Date(Date.now() + MODULE_APPROVAL_TIMEOUT_MS).toISOString(),
|
||||
title,
|
||||
options_json: JSON.stringify(normalizedOptions),
|
||||
approver_user_id: approverUserId ?? null,
|
||||
});
|
||||
|
||||
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,
|
||||
}),
|
||||
);
|
||||
} 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;
|
||||
}
|
||||
if (!adapter) {
|
||||
// No delivery adapter means the card can never be sent, so drop the row we
|
||||
// just created — otherwise it blocks the agent until the expiry sweep, with
|
||||
// a misleading "timed out" instead of the real cause.
|
||||
deletePendingApproval(approvalId);
|
||||
log.error('Failed to deliver approval card — no delivery adapter is wired', { action, approvalId });
|
||||
notifyAgent(session, `${action} failed: could not deliver approval request to ${target.userId}.`);
|
||||
return;
|
||||
}
|
||||
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,
|
||||
}),
|
||||
);
|
||||
} catch (err) {
|
||||
// Delivery failed → drop the orphan row (matching the sender/channel paths)
|
||||
// so the agent hears the real failure now, not a timeout in 7 days.
|
||||
deletePendingApproval(approvalId);
|
||||
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 });
|
||||
|
||||
@@ -277,3 +277,42 @@ describe('plain reject (regression)', () => {
|
||||
expect(lastRelayedText()).toBe('Your install_packages request was rejected by admin.');
|
||||
});
|
||||
});
|
||||
|
||||
/** Seed a pending module approval carrying an explicit expiry. */
|
||||
function seedApprovalWithExpiry(approvalId: string, expiresAt: string, action = 'create_agent'): void {
|
||||
createPendingApproval({
|
||||
approval_id: approvalId,
|
||||
session_id: 'sess-1',
|
||||
request_id: approvalId,
|
||||
action,
|
||||
payload: JSON.stringify({ name: 'child' }),
|
||||
created_at: now(),
|
||||
expires_at: expiresAt,
|
||||
title: 'Approval',
|
||||
options_json: JSON.stringify([]),
|
||||
});
|
||||
}
|
||||
|
||||
describe('module-approval timeout sweep', () => {
|
||||
it('finalizes an expired pending approval and notifies the agent', async () => {
|
||||
const { sweepExpiredModuleApprovals } = await import('./timeout-sweep.js');
|
||||
const { wakeContainer } = await import('../../container-runner.js');
|
||||
seedApprovalWithExpiry('appr-timeout', new Date(Date.now() - 1000).toISOString(), 'install_packages');
|
||||
|
||||
await sweepExpiredModuleApprovals();
|
||||
|
||||
expect(getPendingApproval('appr-timeout')).toBeUndefined();
|
||||
expect(lastRelayedText()).toBe('Your install_packages request timed out waiting for admin approval.');
|
||||
expect(vi.mocked(wakeContainer)).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('leaves a not-yet-expired pending approval untouched', async () => {
|
||||
const { sweepExpiredModuleApprovals } = await import('./timeout-sweep.js');
|
||||
seedApprovalWithExpiry('appr-fresh', new Date(Date.now() + 60_000).toISOString());
|
||||
|
||||
await sweepExpiredModuleApprovals();
|
||||
|
||||
expect(getPendingApproval('appr-fresh')?.status).toBe('pending');
|
||||
expect(vi.mocked(writeSessionMessage)).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
/**
|
||||
* Module-approval timeout sweep.
|
||||
*
|
||||
* Module approvals (create_agent, install_packages, add_mcp_server, …) are
|
||||
* created with a `status='pending'` row and a ~7-day `expires_at` (see
|
||||
* MODULE_APPROVAL_TIMEOUT_MS in primitive.ts). If the card is never answered —
|
||||
* or its delivery silently fell on the floor — the row would otherwise live
|
||||
* forever and permanently block the requesting agent behind an unresolved
|
||||
* approval. This sweep, run once per host-sweep tick, finalizes any such row
|
||||
* as a timeout via the shared finalize path so the agent always gets a
|
||||
* decision.
|
||||
*
|
||||
* Scope: `getExpiredPendingApprovals` filters to session-scoped rows, so
|
||||
* session-less OneCLI credential approvals (resolved in-memory by the gateway
|
||||
* callback) are never touched here even though they carry their own expiry.
|
||||
*/
|
||||
import { deletePendingApproval, getExpiredPendingApprovals, getSession } from '../../db/sessions.js';
|
||||
import { log } from '../../log.js';
|
||||
import { finalizeTimeout } from './finalize.js';
|
||||
|
||||
/**
|
||||
* Host-sweep finalizer: any module approval whose expiry elapsed is finalized
|
||||
* as a timeout. Called once per sweep tick.
|
||||
*/
|
||||
export async function sweepExpiredModuleApprovals(): Promise<void> {
|
||||
const rows = getExpiredPendingApprovals(new Date().toISOString());
|
||||
for (const approval of rows) {
|
||||
const session = approval.session_id ? getSession(approval.session_id) : null;
|
||||
if (!session) {
|
||||
// The requesting session is gone — nothing to notify; just drop the row.
|
||||
deletePendingApproval(approval.approval_id);
|
||||
continue;
|
||||
}
|
||||
await finalizeTimeout(approval, session);
|
||||
}
|
||||
}
|
||||
@@ -176,6 +176,22 @@ describe('unknown-channel registration flow', () => {
|
||||
expect(count).toBe(1);
|
||||
});
|
||||
|
||||
it('drops the pending row when card delivery fails (no permanent block)', async () => {
|
||||
const { routeInbound } = await import('../../router.js');
|
||||
deliverMock.mockRejectedValueOnce(new Error('delivery boom'));
|
||||
|
||||
await routeInbound(groupMention('chat-fail'));
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
|
||||
// Delivery was attempted but threw...
|
||||
expect(deliverMock).toHaveBeenCalledTimes(1);
|
||||
// ...and the row was rolled back so the dedup gate lets a future attempt
|
||||
// through instead of blocking the channel forever behind a lost card.
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
const count = (getDb().prepare('SELECT COUNT(*) AS c FROM pending_channel_approvals').get() as { c: number }).c;
|
||||
expect(count).toBe(0);
|
||||
});
|
||||
|
||||
it('dedups a second mention while the card is pending', async () => {
|
||||
const { routeInbound } = await import('../../router.js');
|
||||
await routeInbound(groupMention('chat-busy'));
|
||||
|
||||
@@ -54,7 +54,11 @@ import { log } from '../../log.js';
|
||||
import type { InboundEvent } from '../../channels/adapter.js';
|
||||
import type { AgentGroup } from '../../types.js';
|
||||
import { pickApprovalDelivery, pickApprover } from '../approvals/primitive.js';
|
||||
import { createPendingChannelApproval, hasInFlightChannelApproval } from './db/pending-channel-approvals.js';
|
||||
import {
|
||||
createPendingChannelApproval,
|
||||
deletePendingChannelApproval,
|
||||
hasInFlightChannelApproval,
|
||||
} from './db/pending-channel-approvals.js';
|
||||
import { hasAdminPrivilege } from './db/user-roles.js';
|
||||
|
||||
// ── Value constants (response handler in index.ts parses these) ──
|
||||
@@ -218,7 +222,11 @@ export async function requestChannelApproval(input: RequestChannelApprovalInput)
|
||||
|
||||
const adapter = getDeliveryAdapter();
|
||||
if (!adapter) {
|
||||
log.error('Channel registration row created but no delivery adapter is wired', { messagingGroupId });
|
||||
// Without a delivery adapter the card can't be sent, so drop the row we
|
||||
// just created. Leaving it would block the channel forever behind the
|
||||
// dedup gate (matches the header contract: adapter-missing → no row).
|
||||
deletePendingChannelApproval(messagingGroupId);
|
||||
log.error('Channel registration dropped — no delivery adapter is wired', { messagingGroupId });
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -242,6 +250,10 @@ export async function requestChannelApproval(input: RequestChannelApprovalInput)
|
||||
approver: delivery.userId,
|
||||
});
|
||||
} catch (err) {
|
||||
// Delivery failed — drop the row so the dedup gate doesn't permanently
|
||||
// block this channel behind a card that never arrived (matches the header
|
||||
// contract: delivery failure → no row, a future attempt retries).
|
||||
deletePendingChannelApproval(messagingGroupId);
|
||||
log.error('Channel registration card delivery failed', { messagingGroupId, err });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -175,6 +175,22 @@ describe('unknown-sender request_approval flow', () => {
|
||||
expect(rows).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('drops the pending row when card delivery fails (no permanent block)', async () => {
|
||||
const { routeInbound } = await import('../../router.js');
|
||||
deliverMock.mockRejectedValueOnce(new Error('delivery boom'));
|
||||
|
||||
await routeInbound(stranger('hi there'));
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
|
||||
// Delivery was attempted but threw...
|
||||
expect(deliverMock).toHaveBeenCalledTimes(1);
|
||||
// ...and the row was rolled back so the dedup gate lets a future attempt
|
||||
// through instead of stranding the sender forever behind a lost card.
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
const count = (getDb().prepare('SELECT COUNT(*) AS c FROM pending_sender_approvals').get() as { c: number }).c;
|
||||
expect(count).toBe(0);
|
||||
});
|
||||
|
||||
it('dedups a second message from the same stranger while pending', async () => {
|
||||
const { routeInbound } = await import('../../router.js');
|
||||
await routeInbound(stranger('hello'));
|
||||
|
||||
@@ -32,7 +32,11 @@ import { getDeliveryAdapter } from '../../delivery.js';
|
||||
import { log } from '../../log.js';
|
||||
import type { InboundEvent } from '../../channels/adapter.js';
|
||||
import { pickApprovalDelivery, pickApprover } from '../approvals/primitive.js';
|
||||
import { createPendingSenderApproval, hasInFlightSenderApproval } from './db/pending-sender-approvals.js';
|
||||
import {
|
||||
createPendingSenderApproval,
|
||||
deletePendingSenderApproval,
|
||||
hasInFlightSenderApproval,
|
||||
} from './db/pending-sender-approvals.js';
|
||||
|
||||
const APPROVAL_OPTIONS: RawOption[] = [
|
||||
{ label: 'Allow', selectedLabel: '✅ Allowed', value: 'approve' },
|
||||
@@ -109,10 +113,11 @@ export async function requestSenderApproval(input: RequestSenderApprovalInput):
|
||||
|
||||
const adapter = getDeliveryAdapter();
|
||||
if (!adapter) {
|
||||
// Without a delivery adapter, the card can't be sent. Log + leave the
|
||||
// row in place so the admin can see it via DB or manual tooling; the
|
||||
// dedup gate will suppress further cards until it's cleared.
|
||||
log.error('Unknown-sender approval row created but no delivery adapter is wired', {
|
||||
// Without a delivery adapter the card can't be sent, so drop the row we
|
||||
// just created. Leaving it would strand the sender forever behind the
|
||||
// dedup gate (matches the header contract: adapter-missing → no row).
|
||||
deletePendingSenderApproval(approvalId);
|
||||
log.error('Unknown-sender approval dropped — no delivery adapter is wired', {
|
||||
approvalId,
|
||||
});
|
||||
return;
|
||||
@@ -140,6 +145,10 @@ export async function requestSenderApproval(input: RequestSenderApprovalInput):
|
||||
agentGroupId,
|
||||
});
|
||||
} catch (err) {
|
||||
// Delivery failed — drop the row so the dedup gate doesn't permanently
|
||||
// block this sender behind a card that never arrived (matches the header
|
||||
// contract: delivery failure → no row, a future attempt retries).
|
||||
deletePendingSenderApproval(approvalId);
|
||||
log.error('Unknown-sender approval card delivery failed', {
|
||||
approvalId,
|
||||
err,
|
||||
|
||||
Reference in New Issue
Block a user