From 7833ded4a19b633742e0aa07f392c372c5a3d26e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Mon, 20 Apr 2026 08:21:43 -0400 Subject: [PATCH] Make @mentions visible to the agents they name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A DM addressed to one agent that @-mentions another was silently dropping the second agent out of the conversation: * `dispatchAgentMessage` added the mentioned agent to the wake set, so CEO's heartbeat fired — but * `listAgentInboxUnread` only surfaced messages where the current agent was `to_agent_id` or sat in a matching channel/project/ department. Mentions were invisible, so CEO's next turn saw nothing new and returned silently. From the operator's chair this looked exactly like "CEO ignored me; 张工程师 answered instead". * `findMentionedAgentIds` had a latent bug on top: its unique-alias filter used `ids.length === 1`, but a single agent whose name matched its role registered itself twice, tripped the filter, and every `@CEO` in a scaffold with role === name was a no-op. Once inbox visibility was fixed the bug became load-bearing. Root-cause fixes: * New migration v48 adds `agent_message_mentions(message_id, agent_id, company_id, created_at)` so mention fan-out is a first-class join. * `sendAgentMessage` now takes the resolved mention set and writes the row + mentions in a single transaction — a crash between the two writes can no longer leave a message visible to the recipient but invisible to the mentioned agent. * `dispatchAgentMessage` computes the mention set exactly once via `resolveMentionedAgentIds` and hands the same list to both the DB write and `resolveMessageWakeTargets`, so persistence and wake stay in lockstep. * `listAgentInboxUnread` and `listAgentMessages` gain an `id IN (select message_id from agent_message_mentions …)` clause so the mentioned agent's inbox actually contains the message that woke it. * `findMentionedAgentIds` now stores per-alias agent IDs in a `Set` and filters on `ids.size === 1`, dropping the self-duplication bug while preserving the "refuse ambiguous aliases" semantics. * Company teardown drops `agent_message_mentions` alongside the existing `agent_message_reads` cleanup so deletes stay clean. Tests pin the new behaviour: a DM with a `@CEO` mention reaches CEO's inbox and both agents get woken, while a DM whose recipient is `__board__` never attempts to wake the human sentinel. --- src/main/database.ts | 2 +- src/main/database/agents-crud.ts | 21 ++++-- src/main/database/company-crud.ts | 7 ++ src/main/database/messages.ts | 85 +++++++++++++++++++------ src/main/database/migrations/v48-v48.ts | 46 +++++++++++++ src/main/database/schema.ts | 2 + src/main/message-dispatch.ts | 39 +++++++++--- tests/unit/message-dispatch.test.ts | 65 +++++++++++++++++++ 8 files changed, 231 insertions(+), 36 deletions(-) create mode 100644 src/main/database/migrations/v48-v48.ts diff --git a/src/main/database.ts b/src/main/database.ts index aa0c41a3..e8f884dd 100644 --- a/src/main/database.ts +++ b/src/main/database.ts @@ -14,7 +14,7 @@ export class AppDatabase extends AppDatabaseBase { // ─── Messages ────────────────────────────────────────────────────────────── - sendAgentMessage(input: { companyId: string; fromAgentId: string; toAgentId?: string | null; channel?: string; channelTargetId?: string | null; subject: string; body: string; priority?: string; parentMessageId?: string | null; attachmentsJson?: string; senderKind?: "agent" | "human" | "system" }) { return this.messages.sendAgentMessage(input); } + sendAgentMessage(input: { companyId: string; fromAgentId: string; toAgentId?: string | null; channel?: string; channelTargetId?: string | null; subject: string; body: string; priority?: string; parentMessageId?: string | null; attachmentsJson?: string; senderKind?: "agent" | "human" | "system"; mentionedAgentIds?: readonly string[] }) { return this.messages.sendAgentMessage(input); } getAgentMessage(messageId: string, companyId: string) { return this.messages.getAgentMessage(messageId, companyId); } listAgentMessages(companyId: string, opts?: { agentId?: string; messageId?: string; channel?: string; channelTargetId?: string | null; unreadOnly?: boolean; limit?: number }) { return this.messages.listAgentMessages(companyId, opts); } searchMessages(companyId: string, query: string, opts?: { channel?: string; channelTargetId?: string | null; limit?: number }) { return this.messages.searchMessages(companyId, query, opts); } diff --git a/src/main/database/agents-crud.ts b/src/main/database/agents-crud.ts index 5f177c30..57fc1f86 100644 --- a/src/main/database/agents-crud.ts +++ b/src/main/database/agents-crud.ts @@ -295,12 +295,18 @@ export function buildAgentCrudMethods( if (agents.length === 0) return []; const lowerText = text.toLowerCase(); - const aliasMap = new Map(); + // Per-alias agent set. Storing a `Set` (not an array) means the + // same agent contributing the same string via multiple fields + // (e.g. name === role === "CEO") collapses to one entry. Before + // this fix the duplicate push made the alias look non-unique, the + // unique-alias filter dropped it, and every @mention in a scaffold + // where role mirrored name silently missed. + const aliasMap = new Map>(); const registerAlias = (alias: string, agentId: string) => { const normalized = alias.trim().toLowerCase(); if (!normalized) return; - const existing = aliasMap.get(normalized) ?? []; - existing.push(agentId); + const existing = aliasMap.get(normalized) ?? new Set(); + existing.add(agentId); aliasMap.set(normalized, existing); }; for (const agent of agents) { @@ -310,8 +316,12 @@ export function buildAgentCrudMethods( } const matched = new Set(); + // Only aliases that unambiguously resolve to a single agent are + // treated as mentions. Two agents sharing a role ("CEO" appearing + // twice in the company) make `@ceo` ambiguous and we refuse to + // pick one silently. const uniqueAliases = [...aliasMap.entries()] - .filter(([, ids]) => ids.length === 1) + .filter(([, ids]) => ids.size === 1) .sort((a, b) => b[0].length - a[0].length); for (const [alias, ids] of uniqueAliases) { const pattern = `@${alias}`; @@ -319,7 +329,8 @@ export function buildAgentCrudMethods( if (idx === -1) continue; const afterIdx = idx + pattern.length; if (afterIdx >= lowerText.length || /[\s,!?.;:\])}>]/.test(lowerText[afterIdx]!)) { - matched.add(ids[0]!); + const [onlyId] = ids; + if (onlyId) matched.add(onlyId); } } return [...matched]; diff --git a/src/main/database/company-crud.ts b/src/main/database/company-crud.ts index ad380244..996899e4 100644 --- a/src/main/database/company-crud.ts +++ b/src/main/database/company-crud.ts @@ -66,6 +66,13 @@ export function buildCompanyCrudMethods( } catch (err) { if (!(err instanceof Error) || !err.message.includes("no such table")) throw err; } + try { + getDb() + .prepare("delete from agent_message_mentions where company_id = ?") + .run(companyId); + } catch (err) { + if (!(err instanceof Error) || !err.message.includes("no such table")) throw err; + } // New table cleanup (budget_incidents before budget_policies due to FK) getDb().prepare("delete from budget_incidents where company_id = ?").run(companyId); getDb().prepare("delete from budget_policies where company_id = ?").run(companyId); diff --git a/src/main/database/messages.ts b/src/main/database/messages.ts index a3df9bb0..9c0abb0a 100644 --- a/src/main/database/messages.ts +++ b/src/main/database/messages.ts @@ -18,6 +18,26 @@ export function buildMessageMethods( transaction: Transaction; }, ) { + /** Write the @mention fan-out rows for a freshly-persisted message. + * Idempotent: the primary key is (message_id, agent_id), so a retry + * driven by a steer/wake race never duplicates rows. Skipping the + * sender keeps self-mentions out of the join so an agent talking + * about itself ("cc @me …") cannot inflate its own inbox. */ + function insertMessageMentions( + messageId: string, + companyId: string, + fromAgentId: string, + mentionedAgentIds: Iterable, + createdAt: string, + ): void { + const stmt = getDb().prepare( + "insert or ignore into agent_message_mentions (message_id, agent_id, company_id, created_at) values (?, ?, ?, ?)", + ); + for (const agentId of mentionedAgentIds) { + if (!agentId || agentId === fromAgentId || agentId === BOARD_READER_ID) continue; + stmt.run(messageId, agentId, companyId, createdAt); + } + } return { sendAgentMessage(input: { companyId: string; @@ -31,6 +51,11 @@ export function buildMessageMethods( parentMessageId?: string | null; attachmentsJson?: string; senderKind?: "agent" | "human" | "system"; + /** Agent IDs parsed out of the body's @mentions. Persisted into + * `agent_message_mentions` alongside the row so the inbox queries + * can surface the message to mentioned agents without re-parsing + * every body at read time. Empty / absent = no mention fan-out. */ + mentionedAgentIds?: readonly string[]; }): string { const channel = input.channel ?? "direct"; if (channel === "direct" && !input.toAgentId) { @@ -39,25 +64,35 @@ export function buildMessageMethods( const senderKind = input.senderKind ?? (input.fromAgentId === BOARD_READER_ID ? "human" : "agent"); const id = randomUUID(); - getDb() - .prepare( - "insert into agent_messages (id, company_id, from_agent_id, to_agent_id, sender_kind, channel, channel_target_id, subject, body, priority, parent_message_id, attachments_json, created_at) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", - ) - .run( - id, - input.companyId, - input.fromAgentId, - input.toAgentId ?? null, - senderKind, - channel, - input.channelTargetId ?? null, - input.subject, - input.body, - input.priority ?? "normal", - input.parentMessageId ?? null, - input.attachmentsJson ?? "[]", - nowIso(), - ); + const createdAt = nowIso(); + // Single transaction: the row and its mention fan-out must land + // atomically, otherwise a crash between the two writes leaves a + // message visible to its recipient but silently invisible to the + // mentioned agents — which is exactly the old bug we're fixing. + deps.transaction(() => { + getDb() + .prepare( + "insert into agent_messages (id, company_id, from_agent_id, to_agent_id, sender_kind, channel, channel_target_id, subject, body, priority, parent_message_id, attachments_json, created_at) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + ) + .run( + id, + input.companyId, + input.fromAgentId, + input.toAgentId ?? null, + senderKind, + channel, + input.channelTargetId ?? null, + input.subject, + input.body, + input.priority ?? "normal", + input.parentMessageId ?? null, + input.attachmentsJson ?? "[]", + createdAt, + ); + if (input.mentionedAgentIds && input.mentionedAgentIds.length > 0) { + insertMessageMentions(id, input.companyId, input.fromAgentId, input.mentionedAgentIds, createdAt); + } + }); deps.addActivity({ companyId: input.companyId, actor: input.fromAgentId, @@ -165,8 +200,9 @@ export function buildMessageMethods( "from_agent_id = ?", "(channel = 'direct' and to_agent_id = ?)", "channel in ('company','incident')", + "id in (select message_id from agent_message_mentions where agent_id = ? and company_id = ?)", ]; - const visibilityParams: SQLInputValue[] = [opts.agentId, opts.agentId]; + const visibilityParams: SQLInputValue[] = [opts.agentId, opts.agentId, opts.agentId, companyId]; if (department) { visibilityClauses.push("(channel = 'department' and channel_target_id = ?)"); visibilityParams.push(department); @@ -306,8 +342,15 @@ export function buildMessageMethods( const visibilityClauses: string[] = [ "(channel = 'direct' and to_agent_id = ?)", "channel in ('company','incident')", + // @mention fan-out: a DM addressed to someone else still lands + // in my inbox when I'm mentioned in the body, because the + // dispatcher persisted (message_id, agent_id) into + // `agent_message_mentions` at write time. Without this clause a + // "@CEO please update" inside a human ↔ engineer chat woke CEO + // with an empty inbox and produced silence instead of a reply. + "id in (select message_id from agent_message_mentions where agent_id = ? and company_id = ?)", ]; - const visibilityParams: SQLInputValue[] = [agentId]; + const visibilityParams: SQLInputValue[] = [agentId, agentId, companyId]; if (department) { visibilityClauses.push("(channel = 'department' and channel_target_id = ?)"); visibilityParams.push(department); diff --git a/src/main/database/migrations/v48-v48.ts b/src/main/database/migrations/v48-v48.ts new file mode 100644 index 00000000..9bf6ef97 --- /dev/null +++ b/src/main/database/migrations/v48-v48.ts @@ -0,0 +1,46 @@ +import log from "electron-log/main.js"; +import type { Migration, MigrationDeps } from "./types"; + +export function createMigrationsV48V48(deps: MigrationDeps): Migration[] { + const { db } = deps; + return [ + { + // Persist @mention fan-out so inbox visibility stops depending on + // a runtime-only regex against every message body. + // + // Before v48 `dispatchAgentMessage` woke mentioned agents via + // `findMentionedAgentIds`, but `listAgentInboxUnread` filtered + // purely on `channel`, `to_agent_id`, and department/project + // membership. A mid-chat `@ceo update please` targeted at + // 张工程师 therefore woke CEO with an empty inbox — CEO's + // heartbeat fired, saw nothing new, and stayed silent while the + // DM recipient replied and the operator assumed CEO had ignored + // them. The join table makes "mentioned in a message" a first- + // class visibility path: persisted once at dispatch, joined on + // read, so every consumer (inbox projection, read-receipts, + // search) sees a single source of truth. + version: 48, + name: "agent_message_mentions", + fn: () => { + db.exec(` + CREATE TABLE IF NOT EXISTS agent_message_mentions ( + message_id TEXT NOT NULL, + agent_id TEXT NOT NULL, + company_id TEXT NOT NULL, + created_at TEXT NOT NULL, + PRIMARY KEY (message_id, agent_id) + ) + `); + db.exec(` + CREATE INDEX IF NOT EXISTS idx_agent_message_mentions_agent + ON agent_message_mentions(company_id, agent_id, created_at) + `); + db.exec(` + CREATE INDEX IF NOT EXISTS idx_agent_message_mentions_message + ON agent_message_mentions(message_id) + `); + log.info("[migration v48] agent_message_mentions join table installed"); + }, + }, + ]; +} diff --git a/src/main/database/schema.ts b/src/main/database/schema.ts index d92121b4..4f352a55 100644 --- a/src/main/database/schema.ts +++ b/src/main/database/schema.ts @@ -12,6 +12,7 @@ import { createMigrationsV44V44 } from "./migrations/v44-v44"; import { createMigrationsV45V45 } from "./migrations/v45-v45"; import { createMigrationsV46V46 } from "./migrations/v46-v46"; import { createMigrationsV47V47 } from "./migrations/v47-v47"; +import { createMigrationsV48V48 } from "./migrations/v48-v48"; import { columnExists, nowIso, @@ -199,6 +200,7 @@ export function runMigrations( ...createMigrationsV45V45(deps), ...createMigrationsV46V46(deps), ...createMigrationsV47V47(deps), + ...createMigrationsV48V48(deps), ]; for (const migration of migrations) { diff --git a/src/main/message-dispatch.ts b/src/main/message-dispatch.ts index cad1929e..ee46e5b0 100644 --- a/src/main/message-dispatch.ts +++ b/src/main/message-dispatch.ts @@ -214,9 +214,27 @@ function resolveProjectWakeTargets( return db.listActiveAgentIdsFromIds(input.companyId, ids); } +/** Resolve the agents mentioned in a message body, stripped of the + * sender and the human-reader sentinel so downstream consumers never + * have to redo those filters. Shared between wake-target resolution + * and the mention persistence path so both see the same set. */ +export function resolveMentionedAgentIds( + db: AppDatabase, + input: Pick, +): string[] { + const mentioned = db.findMentionedAgentIds(input.companyId, input.body); + const filtered: string[] = []; + for (const agentId of mentioned) { + if (!agentId || agentId === input.fromAgentId || agentId === HUMAN_SENDER_ID) continue; + filtered.push(agentId); + } + return filtered; +} + export function resolveMessageWakeTargets( db: AppDatabase, input: NormalizedAgentMessageInput, + mentionedAgentIds?: readonly string[], ): string[] { const wakeTargets = new Set(); @@ -252,12 +270,9 @@ export function resolveMessageWakeTargets( } } - // Parse @mentions in message body and wake mentioned agents - const mentionedIds = db.findMentionedAgentIds(input.companyId, input.body); - for (const mentionedId of mentionedIds) { - if (mentionedId !== input.fromAgentId) { - wakeTargets.add(mentionedId); - } + const mentioned = mentionedAgentIds ?? resolveMentionedAgentIds(db, input); + for (const mentionedId of mentioned) { + wakeTargets.add(mentionedId); } // The human participant (`__board__`) is a reader, not a wake-able @@ -282,9 +297,15 @@ export function dispatchAgentMessage( } const senderKind = normalized.fromHumanUser ? "human" as const : "agent" as const; + // Compute @mentions once per dispatch and hand them to both the DB + // writer (so `agent_message_mentions` is populated atomically with + // the row) and the wake resolver (so the wake fan-out uses the same + // set). Two lookups would waste a regex pass and allow the set to + // drift under mid-dispatch edits. + const mentionedAgentIds = resolveMentionedAgentIds(deps.db, normalized); const persistedMessage = normalized.channel === "direct" - ? { ...normalized, senderKind } - : { ...normalized, toAgentId: null, senderKind }; + ? { ...normalized, senderKind, mentionedAgentIds } + : { ...normalized, toAgentId: null, senderKind, mentionedAgentIds }; const id = deps.db.sendAgentMessage(persistedMessage); const message = deps.db.getAgentMessage(id, normalized.companyId); if (message && deps.emitEvent) { @@ -314,7 +335,7 @@ export function dispatchAgentMessage( deps.onHumanMessage?.(normalized.companyId, normalized.toAgentId, normalized.threadId, normalized.waitStateId); } - const wakeTargets = resolveMessageWakeTargets(deps.db, normalized); + const wakeTargets = resolveMessageWakeTargets(deps.db, normalized, mentionedAgentIds); const mentionResults: MentionResult[] = []; if (deps.wakeAgentIfPossible) { diff --git a/tests/unit/message-dispatch.test.ts b/tests/unit/message-dispatch.test.ts index e92a4a55..f2cc8091 100644 --- a/tests/unit/message-dispatch.test.ts +++ b/tests/unit/message-dispatch.test.ts @@ -166,6 +166,71 @@ describe("message dispatch", () => { db.close(); }); + it("routes a DM addressed to one agent into the @mentioned agent's inbox (group-chat @mention visibility)", async () => { + const profileDir = await createTempDir("agentcompany-message-mention-"); + const db = new AppDatabase(profileDir); + db.init(); + + const { companyId, ceoId, ctoId } = createScaffold(db); + const wakeAgentIfPossible = vi.fn().mockReturnValue(null); + + const result = dispatchAgentMessage({ + db, + wakeAgentIfPossible, + }, { + companyId, + fromAgentId: HUMAN_SENDER_ID, + fromHumanUser: true, + channel: "direct", + toAgentId: ctoId, + subject: "status", + body: "Hi @CEO, please share a progress update, @CTO is on infra.", + }); + + expect(result.ok).toBe(true); + if (!result.ok) throw new Error("Expected dispatch to succeed."); + expect(new Set(result.data.wakeTargets)).toEqual(new Set([ceoId, ctoId])); + + const ceoInbox = db.listAgentInboxUnread(companyId, ceoId); + expect(ceoInbox.map((entry) => entry.id)).toContain(result.data.id); + const ctoInbox = db.listAgentInboxUnread(companyId, ctoId); + expect(ctoInbox.map((entry) => entry.id)).toContain(result.data.id); + + db.close(); + }); + + it("keeps __board__ out of wake targets even when the recipient is the human reader", async () => { + const profileDir = await createTempDir("agentcompany-message-board-wake-"); + const db = new AppDatabase(profileDir); + db.init(); + + const { companyId, ceoId } = createScaffold(db); + const wakeAgentIfPossible = vi.fn().mockReturnValue(null); + + const result = dispatchAgentMessage({ + db, + wakeAgentIfPossible, + }, { + companyId, + fromAgentId: ceoId, + channel: "direct", + toAgentId: HUMAN_SENDER_ID, + subject: "report", + body: "Update ready for review.", + }); + + expect(result.ok).toBe(true); + if (!result.ok) throw new Error("Expected dispatch to succeed."); + expect(result.data.wakeTargets).not.toContain(HUMAN_SENDER_ID); + expect(wakeAgentIfPossible).not.toHaveBeenCalledWith( + HUMAN_SENDER_ID, + expect.anything(), + expect.anything(), + ); + + db.close(); + }); + it("resolves only user_reply wait states for company-wide human messages, but scoped replies resolve thread wait states", async () => { const profileDir = await createTempDir("agentcompany-message-wait-"); const db = new AppDatabase(profileDir);