Merge pull request #1932 from Koshkoshinsk/main
v2: Fix Discord approval card bugs
This commit is contained in:
@@ -56,6 +56,8 @@ export interface InboundEvent {
|
|||||||
* See InboundMessage.isMention for the full explanation.
|
* See InboundMessage.isMention for the full explanation.
|
||||||
*/
|
*/
|
||||||
isMention?: boolean;
|
isMention?: boolean;
|
||||||
|
/** True when the source is a group/channel thread, false for DMs. */
|
||||||
|
isGroup?: boolean;
|
||||||
};
|
};
|
||||||
replyTo?: DeliveryAddress;
|
replyTo?: DeliveryAddress;
|
||||||
}
|
}
|
||||||
@@ -81,6 +83,8 @@ export interface InboundMessage {
|
|||||||
* router falls back to text-match against agent_group_name.
|
* router falls back to text-match against agent_group_name.
|
||||||
*/
|
*/
|
||||||
isMention?: boolean;
|
isMention?: boolean;
|
||||||
|
/** True when the source is a group/channel thread, false for DMs. */
|
||||||
|
isGroup?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** A file attachment to deliver alongside a message. */
|
/** A file attachment to deliver alongside a message. */
|
||||||
|
|||||||
@@ -125,7 +125,7 @@ export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter
|
|||||||
let setupConfig: ChannelSetup;
|
let setupConfig: ChannelSetup;
|
||||||
let gatewayAbort: AbortController | null = null;
|
let gatewayAbort: AbortController | null = null;
|
||||||
|
|
||||||
async function messageToInbound(message: ChatMessage, isMention: boolean): Promise<InboundMessage> {
|
async function messageToInbound(message: ChatMessage, isMention: boolean, isGroup?: boolean): Promise<InboundMessage> {
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
const serialized = message.toJSON() as Record<string, any>;
|
const serialized = message.toJSON() as Record<string, any>;
|
||||||
|
|
||||||
@@ -182,6 +182,7 @@ export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter
|
|||||||
content: serialized,
|
content: serialized,
|
||||||
timestamp: message.metadata.dateSent.toISOString(),
|
timestamp: message.metadata.dateSent.toISOString(),
|
||||||
isMention,
|
isMention,
|
||||||
|
isGroup,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -215,13 +216,13 @@ export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter
|
|||||||
// wirings still fire on in-thread mentions.
|
// wirings still fire on in-thread mentions.
|
||||||
chat.onSubscribedMessage(async (thread, message) => {
|
chat.onSubscribedMessage(async (thread, message) => {
|
||||||
const channelId = adapter.channelIdFromThreadId(thread.id);
|
const channelId = adapter.channelIdFromThreadId(thread.id);
|
||||||
await setupConfig.onInbound(channelId, thread.id, await messageToInbound(message, message.isMention === true));
|
await setupConfig.onInbound(channelId, thread.id, await messageToInbound(message, message.isMention === true, true));
|
||||||
});
|
});
|
||||||
|
|
||||||
// @mention in an unsubscribed thread — SDK-confirmed bot mention.
|
// @mention in an unsubscribed thread — SDK-confirmed bot mention.
|
||||||
chat.onNewMention(async (thread, message) => {
|
chat.onNewMention(async (thread, message) => {
|
||||||
const channelId = adapter.channelIdFromThreadId(thread.id);
|
const channelId = adapter.channelIdFromThreadId(thread.id);
|
||||||
await setupConfig.onInbound(channelId, thread.id, await messageToInbound(message, true));
|
await setupConfig.onInbound(channelId, thread.id, await messageToInbound(message, true, true));
|
||||||
});
|
});
|
||||||
|
|
||||||
// DMs — by definition addressed to the bot. Thread id flows through
|
// DMs — by definition addressed to the bot. Thread id flows through
|
||||||
@@ -236,7 +237,7 @@ export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter
|
|||||||
sender: (message.author as any)?.fullName ?? (message.author as any)?.userId ?? 'unknown',
|
sender: (message.author as any)?.fullName ?? (message.author as any)?.userId ?? 'unknown',
|
||||||
threadId: thread.id,
|
threadId: thread.id,
|
||||||
});
|
});
|
||||||
await setupConfig.onInbound(channelId, thread.id, await messageToInbound(message, true));
|
await setupConfig.onInbound(channelId, thread.id, await messageToInbound(message, true, false));
|
||||||
});
|
});
|
||||||
|
|
||||||
// Plain messages in unsubscribed threads.
|
// Plain messages in unsubscribed threads.
|
||||||
@@ -251,7 +252,7 @@ export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter
|
|||||||
// flood gate.
|
// flood gate.
|
||||||
chat.onNewMessage(/./, async (thread, message) => {
|
chat.onNewMessage(/./, async (thread, message) => {
|
||||||
const channelId = adapter.channelIdFromThreadId(thread.id);
|
const channelId = adapter.channelIdFromThreadId(thread.id);
|
||||||
await setupConfig.onInbound(channelId, thread.id, await messageToInbound(message, false));
|
await setupConfig.onInbound(channelId, thread.id, await messageToInbound(message, false, true));
|
||||||
});
|
});
|
||||||
|
|
||||||
// Handle button clicks (ask_user_question)
|
// Handle button clicks (ask_user_question)
|
||||||
@@ -530,7 +531,10 @@ async function handleForwardedEvent(
|
|||||||
// type 3 = MessageComponent (button/select)
|
// type 3 = MessageComponent (button/select)
|
||||||
if (interaction.type === 3) {
|
if (interaction.type === 3) {
|
||||||
const customId = (interaction.data as Record<string, unknown>)?.custom_id as string;
|
const customId = (interaction.data as Record<string, unknown>)?.custom_id as string;
|
||||||
const user = (interaction.member as Record<string, unknown>)?.user as Record<string, string> | undefined;
|
// In guilds the clicker is at interaction.member.user; in DMs it's interaction.user directly.
|
||||||
|
const user =
|
||||||
|
((interaction.member as Record<string, unknown>)?.user as Record<string, string> | undefined) ??
|
||||||
|
(interaction.user as Record<string, string> | undefined);
|
||||||
const interactionId = interaction.id as string;
|
const interactionId = interaction.id as string;
|
||||||
const interactionToken = interaction.token as string;
|
const interactionToken = interaction.token as string;
|
||||||
|
|
||||||
|
|||||||
27
src/db/migrations/013-approval-render-metadata.ts
Normal file
27
src/db/migrations/013-approval-render-metadata.ts
Normal file
@@ -0,0 +1,27 @@
|
|||||||
|
/**
|
||||||
|
* Persist ask_question render metadata (title + options_json) on
|
||||||
|
* `pending_channel_approvals` and `pending_sender_approvals`, mirroring the
|
||||||
|
* columns migration 003 / module-approvals-title-options added to
|
||||||
|
* `pending_approvals`.
|
||||||
|
*
|
||||||
|
* Before this, `getAskQuestionRender` hardcoded the title + option labels
|
||||||
|
* for these two tables in the DB-access layer — duplicating wording that
|
||||||
|
* also lived in the approval modules and causing a visible drift between
|
||||||
|
* the initial card title ("📣 Bot mentioned in new chat" / "💬 New direct
|
||||||
|
* message", chosen per event) and the post-click render ("📣 Channel
|
||||||
|
* registration", constant). Storing the render metadata alongside the row
|
||||||
|
* lets both sides read from the same source.
|
||||||
|
*/
|
||||||
|
import type Database from 'better-sqlite3';
|
||||||
|
import type { Migration } from './index.js';
|
||||||
|
|
||||||
|
export const migration013: Migration = {
|
||||||
|
version: 13,
|
||||||
|
name: 'approval-render-metadata',
|
||||||
|
up(db: Database.Database) {
|
||||||
|
db.exec(`ALTER TABLE pending_channel_approvals ADD COLUMN title TEXT NOT NULL DEFAULT ''`);
|
||||||
|
db.exec(`ALTER TABLE pending_channel_approvals ADD COLUMN options_json TEXT NOT NULL DEFAULT '[]'`);
|
||||||
|
db.exec(`ALTER TABLE pending_sender_approvals ADD COLUMN title TEXT NOT NULL DEFAULT ''`);
|
||||||
|
db.exec(`ALTER TABLE pending_sender_approvals ADD COLUMN options_json TEXT NOT NULL DEFAULT '[]'`);
|
||||||
|
},
|
||||||
|
};
|
||||||
@@ -9,6 +9,7 @@ import { migration009 } from './009-drop-pending-credentials.js';
|
|||||||
import { migration010 } from './010-engage-modes.js';
|
import { migration010 } from './010-engage-modes.js';
|
||||||
import { migration011 } from './011-pending-sender-approvals.js';
|
import { migration011 } from './011-pending-sender-approvals.js';
|
||||||
import { migration012 } from './012-channel-registration.js';
|
import { migration012 } from './012-channel-registration.js';
|
||||||
|
import { migration013 } from './013-approval-render-metadata.js';
|
||||||
import { moduleApprovalsPendingApprovals } from './module-approvals-pending-approvals.js';
|
import { moduleApprovalsPendingApprovals } from './module-approvals-pending-approvals.js';
|
||||||
import { moduleApprovalsTitleOptions } from './module-approvals-title-options.js';
|
import { moduleApprovalsTitleOptions } from './module-approvals-title-options.js';
|
||||||
|
|
||||||
@@ -29,6 +30,7 @@ const migrations: Migration[] = [
|
|||||||
migration010,
|
migration010,
|
||||||
migration011,
|
migration011,
|
||||||
migration012,
|
migration012,
|
||||||
|
migration013,
|
||||||
];
|
];
|
||||||
|
|
||||||
export function runMigrations(db: Database.Database): void {
|
export function runMigrations(db: Database.Database): void {
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import type { PendingApproval, PendingQuestion, Session } from '../types.js';
|
import type { PendingApproval, PendingQuestion, Session } from '../types.js';
|
||||||
import { getDb } from './connection.js';
|
import { getDb, hasTable } from './connection.js';
|
||||||
|
|
||||||
// ── Sessions ──
|
// ── Sessions ──
|
||||||
|
|
||||||
@@ -205,6 +205,23 @@ export function getAskQuestionRender(
|
|||||||
const a = getDb().prepare('SELECT title, options_json FROM pending_approvals WHERE approval_id = ?').get(id) as
|
const a = getDb().prepare('SELECT title, options_json FROM pending_approvals WHERE approval_id = ?').get(id) as
|
||||||
| { title: string; options_json: string }
|
| { title: string; options_json: string }
|
||||||
| undefined;
|
| undefined;
|
||||||
if (!a || !a.title) return undefined;
|
if (a?.title) return { title: a.title, options: JSON.parse(a.options_json) };
|
||||||
return { title: a.title, options: JSON.parse(a.options_json) };
|
|
||||||
|
// Channel-registration + unknown-sender approvals persist title/options_json
|
||||||
|
// the same way pending_approvals does — just SELECT and return.
|
||||||
|
if (hasTable(getDb(), 'pending_channel_approvals')) {
|
||||||
|
const c = getDb()
|
||||||
|
.prepare('SELECT title, options_json FROM pending_channel_approvals WHERE messaging_group_id = ?')
|
||||||
|
.get(id) as { title: string; options_json: string } | undefined;
|
||||||
|
if (c?.title) return { title: c.title, options: JSON.parse(c.options_json) };
|
||||||
|
}
|
||||||
|
|
||||||
|
if (hasTable(getDb(), 'pending_sender_approvals')) {
|
||||||
|
const s = getDb().prepare('SELECT title, options_json FROM pending_sender_approvals WHERE id = ?').get(id) as
|
||||||
|
| { title: string; options_json: string }
|
||||||
|
| undefined;
|
||||||
|
if (s?.title) return { title: s.title, options: JSON.parse(s.options_json) };
|
||||||
|
}
|
||||||
|
|
||||||
|
return undefined;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -85,6 +85,7 @@ async function main(): Promise<void> {
|
|||||||
content: JSON.stringify(message.content),
|
content: JSON.stringify(message.content),
|
||||||
timestamp: message.timestamp,
|
timestamp: message.timestamp,
|
||||||
isMention: message.isMention,
|
isMention: message.isMention,
|
||||||
|
isGroup: message.isGroup,
|
||||||
},
|
},
|
||||||
}).catch((err) => {
|
}).catch((err) => {
|
||||||
log.error('Failed to route inbound message', { channelType: adapter.channelType, err });
|
log.error('Failed to route inbound message', { channelType: adapter.channelType, err });
|
||||||
|
|||||||
@@ -101,13 +101,26 @@ export async function requestChannelApproval(input: RequestChannelApprovalInput)
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const originName = originMg?.name ?? originMg?.platform_id ?? 'an unfamiliar chat';
|
const isGroup = event.message?.isGroup ?? originMg?.is_group === 1;
|
||||||
const isGroup = originMg?.is_group === 1;
|
|
||||||
|
// Extract sender name from the event content for a human-readable card.
|
||||||
|
let senderName: string | undefined;
|
||||||
|
try {
|
||||||
|
const parsed = JSON.parse(event.message.content) as Record<string, unknown>;
|
||||||
|
senderName = (parsed.senderName ?? parsed.sender) as string | undefined;
|
||||||
|
} catch {
|
||||||
|
// non-critical — fall through to generic wording
|
||||||
|
}
|
||||||
|
|
||||||
const title = isGroup ? '📣 Bot mentioned in new chat' : '💬 New direct message';
|
const title = isGroup ? '📣 Bot mentioned in new chat' : '💬 New direct message';
|
||||||
const question = isGroup
|
const question = isGroup
|
||||||
? `Your agent was mentioned in ${originName} on ${originChannelType}. Wire it to ${target.name} and let it engage?`
|
? senderName
|
||||||
: `Someone DM'd your agent on ${originChannelType} (${originName}). Wire it to ${target.name} and let it respond?`;
|
? `${senderName} mentioned your agent in a ${originChannelType} channel. Wire it to ${target.name} and let it engage?`
|
||||||
|
: `Your agent was mentioned in a ${originChannelType} channel. Wire it to ${target.name} and let it engage?`
|
||||||
|
: senderName
|
||||||
|
? `${senderName} DM'd your agent on ${originChannelType}. Wire it to ${target.name} and let it respond?`
|
||||||
|
: `Someone DM'd your agent on ${originChannelType}. Wire it to ${target.name} and let it respond?`;
|
||||||
|
const options = normalizeOptions(APPROVAL_OPTIONS);
|
||||||
|
|
||||||
createPendingChannelApproval({
|
createPendingChannelApproval({
|
||||||
messaging_group_id: messagingGroupId,
|
messaging_group_id: messagingGroupId,
|
||||||
@@ -115,6 +128,8 @@ export async function requestChannelApproval(input: RequestChannelApprovalInput)
|
|||||||
original_message: JSON.stringify(event),
|
original_message: JSON.stringify(event),
|
||||||
approver_user_id: delivery.userId,
|
approver_user_id: delivery.userId,
|
||||||
created_at: new Date().toISOString(),
|
created_at: new Date().toISOString(),
|
||||||
|
title,
|
||||||
|
options_json: JSON.stringify(options),
|
||||||
});
|
});
|
||||||
|
|
||||||
const adapter = getDeliveryAdapter();
|
const adapter = getDeliveryAdapter();
|
||||||
@@ -139,7 +154,7 @@ export async function requestChannelApproval(input: RequestChannelApprovalInput)
|
|||||||
questionId: messagingGroupId,
|
questionId: messagingGroupId,
|
||||||
title,
|
title,
|
||||||
question,
|
question,
|
||||||
options: normalizeOptions(APPROVAL_OPTIONS),
|
options,
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
log.info('Channel registration card delivered', {
|
log.info('Channel registration card delivered', {
|
||||||
|
|||||||
@@ -17,6 +17,10 @@ export interface PendingChannelApproval {
|
|||||||
original_message: string;
|
original_message: string;
|
||||||
approver_user_id: string;
|
approver_user_id: string;
|
||||||
created_at: string;
|
created_at: string;
|
||||||
|
/** Card title shown at creation and re-used by getAskQuestionRender on click. */
|
||||||
|
title: string;
|
||||||
|
/** Normalized options (JSON-encoded NormalizedOption[]) — same shape persisted on pending_approvals. */
|
||||||
|
options_json: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function createPendingChannelApproval(row: PendingChannelApproval): void {
|
export function createPendingChannelApproval(row: PendingChannelApproval): void {
|
||||||
@@ -24,11 +28,11 @@ export function createPendingChannelApproval(row: PendingChannelApproval): void
|
|||||||
.prepare(
|
.prepare(
|
||||||
`INSERT INTO pending_channel_approvals (
|
`INSERT INTO pending_channel_approvals (
|
||||||
messaging_group_id, agent_group_id, original_message,
|
messaging_group_id, agent_group_id, original_message,
|
||||||
approver_user_id, created_at
|
approver_user_id, created_at, title, options_json
|
||||||
)
|
)
|
||||||
VALUES (
|
VALUES (
|
||||||
@messaging_group_id, @agent_group_id, @original_message,
|
@messaging_group_id, @agent_group_id, @original_message,
|
||||||
@approver_user_id, @created_at
|
@approver_user_id, @created_at, @title, @options_json
|
||||||
)`,
|
)`,
|
||||||
)
|
)
|
||||||
.run(row);
|
.run(row);
|
||||||
|
|||||||
@@ -19,6 +19,10 @@ export interface PendingSenderApproval {
|
|||||||
original_message: string;
|
original_message: string;
|
||||||
approver_user_id: string;
|
approver_user_id: string;
|
||||||
created_at: string;
|
created_at: string;
|
||||||
|
/** Card title shown at creation and re-used by getAskQuestionRender on click. */
|
||||||
|
title: string;
|
||||||
|
/** Normalized options (JSON-encoded NormalizedOption[]) — same shape persisted on pending_approvals. */
|
||||||
|
options_json: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function createPendingSenderApproval(row: PendingSenderApproval): void {
|
export function createPendingSenderApproval(row: PendingSenderApproval): void {
|
||||||
@@ -26,11 +30,13 @@ export function createPendingSenderApproval(row: PendingSenderApproval): void {
|
|||||||
.prepare(
|
.prepare(
|
||||||
`INSERT INTO pending_sender_approvals (
|
`INSERT INTO pending_sender_approvals (
|
||||||
id, messaging_group_id, agent_group_id, sender_identity,
|
id, messaging_group_id, agent_group_id, sender_identity,
|
||||||
sender_name, original_message, approver_user_id, created_at
|
sender_name, original_message, approver_user_id, created_at,
|
||||||
|
title, options_json
|
||||||
)
|
)
|
||||||
VALUES (
|
VALUES (
|
||||||
@id, @messaging_group_id, @agent_group_id, @sender_identity,
|
@id, @messaging_group_id, @agent_group_id, @sender_identity,
|
||||||
@sender_name, @original_message, @approver_user_id, @created_at
|
@sender_name, @original_message, @approver_user_id, @created_at,
|
||||||
|
@title, @options_json
|
||||||
)`,
|
)`,
|
||||||
)
|
)
|
||||||
.run(row);
|
.run(row);
|
||||||
|
|||||||
@@ -88,10 +88,11 @@ export async function requestSenderApproval(input: RequestSenderApprovalInput):
|
|||||||
|
|
||||||
const approvalId = generateId();
|
const approvalId = generateId();
|
||||||
const senderDisplay = senderName && senderName.length > 0 ? senderName : senderIdentity;
|
const senderDisplay = senderName && senderName.length > 0 ? senderName : senderIdentity;
|
||||||
const originName = originMg?.name ?? originMg?.platform_id ?? 'an unfamiliar chat';
|
const originName = originMg?.name ?? `a ${originChannelType} channel`;
|
||||||
|
|
||||||
const title = '👤 New sender';
|
const title = '👤 New sender';
|
||||||
const question = `${senderDisplay} wants to talk to your agent in ${originName}. Allow?`;
|
const question = `${senderDisplay} wants to talk to your agent in ${originName}. Allow?`;
|
||||||
|
const options = normalizeOptions(APPROVAL_OPTIONS);
|
||||||
|
|
||||||
createPendingSenderApproval({
|
createPendingSenderApproval({
|
||||||
id: approvalId,
|
id: approvalId,
|
||||||
@@ -102,6 +103,8 @@ export async function requestSenderApproval(input: RequestSenderApprovalInput):
|
|||||||
original_message: JSON.stringify(event),
|
original_message: JSON.stringify(event),
|
||||||
approver_user_id: target.userId,
|
approver_user_id: target.userId,
|
||||||
created_at: new Date().toISOString(),
|
created_at: new Date().toISOString(),
|
||||||
|
title,
|
||||||
|
options_json: JSON.stringify(options),
|
||||||
});
|
});
|
||||||
|
|
||||||
const adapter = getDeliveryAdapter();
|
const adapter = getDeliveryAdapter();
|
||||||
@@ -126,7 +129,7 @@ export async function requestSenderApproval(input: RequestSenderApprovalInput):
|
|||||||
questionId: approvalId,
|
questionId: approvalId,
|
||||||
title,
|
title,
|
||||||
question,
|
question,
|
||||||
options: APPROVAL_OPTIONS,
|
options,
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
log.info('Unknown-sender approval card delivered', {
|
log.info('Unknown-sender approval card delivered', {
|
||||||
|
|||||||
@@ -170,7 +170,7 @@ export async function routeInbound(event: InboundEvent): Promise<void> {
|
|||||||
channel_type: event.channelType,
|
channel_type: event.channelType,
|
||||||
platform_id: event.platformId,
|
platform_id: event.platformId,
|
||||||
name: null,
|
name: null,
|
||||||
is_group: 0,
|
is_group: event.message.isGroup ? 1 : 0,
|
||||||
unknown_sender_policy: 'request_approval',
|
unknown_sender_policy: 'request_approval',
|
||||||
denied_at: null,
|
denied_at: null,
|
||||||
created_at: new Date().toISOString(),
|
created_at: new Date().toISOString(),
|
||||||
|
|||||||
Reference in New Issue
Block a user