diff --git a/backend/__tests__/unit/services/agentMentionService.frameBudget.test.js b/backend/__tests__/unit/services/agentMentionService.frameBudget.test.js new file mode 100644 index 0000000000..2345af2586 --- /dev/null +++ b/backend/__tests__/unit/services/agentMentionService.frameBudget.test.js @@ -0,0 +1,125 @@ +/** + * The wake frame has a size, and nobody was paying for it (TASK-074). + * + * Every clause added to `formatPodContextFrame` and its companions is prepended + * to `chat.mention.payload.content` for EVERY mention to EVERY agent, forever. + * That is the correct place for a kernel affordance — the inline cue is the one + * thing a model will not deprioritize — and it is also the reason the cost is + * invisible: each clause is a few hundred characters in a diff nobody measures, + * and the total is paid per wake, fleet-wide. + * + * Measured while gating #1216/#1244: the pod-context frame alone went 1,639 → + * 2,269 (+38%) → ~2,697 (+65%) across one two-PR stack. No review comment + * mentioned the size, because there was no number to compare against. + * + * This is not a cap. It is a budget: raising it is a one-line change, and that + * one line is the entire point — it turns an invisible spend into a deliberate + * one that a reviewer can see and argue with. A PR that legitimately needs more + * room should raise the ceiling and say why in its body. + * + * Two-sided on purpose. A ceiling alone is satisfied by deleting the frame, and + * the copy assertions elsewhere in this suite pin individual sentences rather + * than the whole. The floor catches a frame that silently lost a section. + */ + +jest.mock('../../../services/agentEventService', () => ({ enqueue: jest.fn() })); +jest.mock('../../../models/AgentRegistry', () => ({ + AgentInstallation: { find: jest.fn(), findOne: jest.fn() }, +})); +jest.mock('../../../models/AgentProfile', () => ({ find: jest.fn() })); +jest.mock('../../../models/Pod', () => ({ findById: jest.fn(), find: jest.fn() })); +jest.mock('../../../models/User', () => ({ find: jest.fn(), findById: jest.fn() })); +jest.mock('../../../services/chatSummarizerService', () => ({ + constructor: { getLatestPodSummary: jest.fn() }, + summarizePodMessages: jest.fn(), +})); +jest.mock('../../../models/AgentEvent', () => ({ countDocuments: jest.fn() })); +jest.mock('../../../services/welcomeWakeService', () => ({ maybeFireWelcomeWake: jest.fn() })); +jest.mock('../../../models/pg/Message', () => ({ findById: jest.fn(async () => null) })); +jest.mock('../../../models/pg/ThreadUserState', () => ({ + followByParticipation: jest.fn().mockResolvedValue(true), +})); + +const AgentMentionService = require('../../../services/agentMentionService'); +const AgentEventService = require('../../../services/agentEventService'); +const { AgentInstallation } = require('../../../models/AgentRegistry'); +const AgentProfile = require('../../../models/AgentProfile'); +const Pod = require('../../../models/Pod'); +const User = require('../../../models/User'); +const AgentEvent = require('../../../models/AgentEvent'); + +// The reference wake: a plain chat pod, one installed seat, an explicit +// @mention, no thread, no wake-on-message opt-in. Deliberately the SMALLEST +// real frame — a collaborative pod and a wake-on-message seat both add more, so +// a budget measured here is a floor on what the fleet actually pays. +// Raised from 3000/2600 by the three-verbs clause on this branch. The band is +// kept as tight around the new reference (3,935) as the original was around +// 2,877 — leaving MIN at 2,600 would let a third of the frame vanish silently, +// which is the failure the lower bound exists to catch. What the fleet buys for +// the extra ~1,058 characters is in the PR body. +const BUDGET_MAX = 4100; +const BUDGET_MIN = 3550; + +const referenceWake = async () => { + AgentInstallation.find.mockReturnValue({ + lean: jest.fn().mockResolvedValue([ + { agentName: 'seat-a', instanceId: 'default', displayName: 'Seat A' }, + ]), + }); + AgentProfile.find.mockReturnValue({ lean: jest.fn().mockResolvedValue([]) }); + User.find.mockReturnValue({ select: jest.fn().mockReturnThis(), lean: jest.fn().mockResolvedValue([]) }); + User.findById.mockImplementation(() => ({ + select: jest.fn().mockReturnThis(), + lean: jest.fn().mockResolvedValue({ _id: 'user-1', isBot: false }), + })); + Pod.findById.mockReturnValue({ + select: jest.fn().mockReturnValue({ + lean: jest.fn().mockResolvedValue({ type: 'chat', members: ['user-1', 'bot-1'] }), + }), + lean: jest.fn().mockResolvedValue({ _id: 'pod-1', type: 'chat', members: ['user-1', 'bot-1'] }), + }); + AgentEvent.countDocuments.mockResolvedValue(0); + + await AgentMentionService.enqueueMentions({ + podId: 'pod-1', + userId: 'user-1', + username: 'alice', + message: { id: 'm-1', content: 'hi @seat-a' }, + }); + const call = AgentEventService.enqueue.mock.calls.find(([a]) => a.type === 'chat.mention'); + return call[0].payload.content; +}; + +beforeEach(() => { jest.clearAllMocks(); }); + +describe('wake frame size budget', () => { + test('the reference wake stays inside its character budget', async () => { + const content = await referenceWake(); + + // If this fails you added a clause. That is allowed — raise BUDGET_MAX in + // the same diff and say in the PR body what the fleet is buying, so the + // trade is on the record instead of inside a paragraph. + expect(content.length).toBeLessThanOrEqual(BUDGET_MAX); + }); + + test('and has not silently lost a section', async () => { + const content = await referenceWake(); + + // The other half of a budget. A ceiling on its own is satisfied by an + // empty frame, and the copy assertions in this suite pin sentences one at + // a time — none of them notices a whole section going missing. + expect(content.length).toBeGreaterThanOrEqual(BUDGET_MIN); + }); + + test('the sections that make up the cost are all present', async () => { + // Named so a budget failure is diagnosable: the number alone says the + // frame grew, not where. These are the four bracketed blocks a reference + // wake carries. + const content = await referenceWake(); + + expect(content).toContain('[Pod context:'); + expect(content).toContain('[Trigger:'); + expect(content).toContain('[Collaboration:'); + expect(content).toContain('[Reply mechanics:'); + }); +}); diff --git a/backend/__tests__/unit/services/agentMentionService.humansAreNotWoken.test.js b/backend/__tests__/unit/services/agentMentionService.humansAreNotWoken.test.js new file mode 100644 index 0000000000..061c95fa6d --- /dev/null +++ b/backend/__tests__/unit/services/agentMentionService.humansAreNotWoken.test.js @@ -0,0 +1,162 @@ +/** + * A human @handle routes nothing (TASK-070a / TASK-074, follow-up to #1244). + * + * #1244 added a paragraph to the pod-context frame telling every agent that a + * human's handle "is necessary and not sufficient — it flags the message in a + * mentions filter the human pulls; nothing pushes". That is an assertion about + * the kernel, made to a reader who cannot check it: an agent acts on the cue + * and has no view of `enqueueMentions`. + * + * #1244's three tests pin the SENTENCE (`toContain('nothing pushes')`). They go + * red if someone rewords the cue and stay green if someone makes it false. This + * file pins the other half: the behaviour the sentence describes. + * + * Two mechanisms, stated separately because they can break separately: + * + * 1. A handle that resolves to a human enqueues no `AgentEvent` of any type. + * `enqueueMentions` has no human delivery branch at all — the handle is + * filtered into `humanMentionHandles` and never reaches an enqueue. The + * realistic future edit is TASK-070b (should a bare name route?): an + * implementer who answers "yes, and push it" makes the cue a lie taught on + * every wake, fleet-wide, and nothing in the suite objects. + * + * 2. Even the thread-follow half is narrower than the cue's readers assume: + * `resolveHumanMentionUserIds` is called only inside `if (threadRootId)`, + * so a plain channel post materialises no state for the mentioned human. + * Hoisting that call out of the guard is the consistency fix that looks + * correct and quietly widens what a handle does. + * + * Every negative here is paired with a control, per the house rule: an + * assertion that nothing was enqueued is worthless from a harness that cannot + * enqueue anything. + */ + +jest.mock('../../../services/agentEventService', () => ({ enqueue: jest.fn() })); +jest.mock('../../../models/AgentRegistry', () => ({ + AgentInstallation: { find: jest.fn(), findOne: jest.fn() }, +})); +jest.mock('../../../models/AgentProfile', () => ({ find: jest.fn() })); +jest.mock('../../../models/Pod', () => ({ findById: jest.fn(), find: jest.fn() })); +jest.mock('../../../models/User', () => ({ find: jest.fn(), findById: jest.fn() })); +jest.mock('../../../services/chatSummarizerService', () => ({ + constructor: { getLatestPodSummary: jest.fn() }, + summarizePodMessages: jest.fn(), +})); +jest.mock('../../../models/AgentEvent', () => ({ countDocuments: jest.fn() })); +jest.mock('../../../services/welcomeWakeService', () => ({ maybeFireWelcomeWake: jest.fn() })); +jest.mock('../../../models/pg/Message', () => ({ findById: jest.fn(async () => null) })); +jest.mock('../../../models/pg/ThreadUserState', () => ({ + followByParticipation: jest.fn().mockResolvedValue(true), +})); + +const AgentMentionService = require('../../../services/agentMentionService'); +const AgentEventService = require('../../../services/agentEventService'); +const { AgentInstallation } = require('../../../models/AgentRegistry'); +const AgentProfile = require('../../../models/AgentProfile'); +const Pod = require('../../../models/Pod'); +const User = require('../../../models/User'); +const AgentEvent = require('../../../models/AgentEvent'); +const ThreadUserState = require('../../../models/pg/ThreadUserState'); + +const SEAT = { agentName: 'seat-a', instanceId: 'default', displayName: 'Seat A' }; +const HUMAN = { _id: 'user-sam', username: 'sam' }; + +const mockInstallations = (installations) => { + AgentInstallation.find.mockReturnValue({ lean: jest.fn().mockResolvedValue(installations) }); + AgentProfile.find.mockReturnValue({ lean: jest.fn().mockResolvedValue([]) }); +}; + +// The human-handle resolver. MOCKED DELIBERATELY and load-bearing: `sam` is a +// real non-bot row inside the pod, so anything that hands this handle to a +// delivery path WILL find a user to deliver to. A harness where the lookup +// returns nothing would pass every negative below for the wrong reason. +const mockUserLookup = () => { + User.find.mockReturnValue({ + select: jest.fn().mockReturnThis(), + lean: jest.fn().mockResolvedValue([HUMAN]), + }); + User.findById.mockImplementation(() => ({ + select: jest.fn().mockReturnThis(), + lean: jest.fn().mockResolvedValue({ _id: 'user-1', isBot: false }), + })); +}; + +const enqueued = () => AgentEventService.enqueue.mock.calls.map(([a]) => a); +const mentions = () => enqueued().filter((a) => a.type === 'chat.mention'); + +const send = (message, extra = {}) => AgentMentionService.enqueueMentions({ + podId: 'pod-1', userId: 'user-1', username: 'alice', message, ...extra, +}); + +beforeEach(() => { + jest.clearAllMocks(); + mockUserLookup(); + Pod.findById.mockReturnValue({ + select: jest.fn().mockReturnValue({ + lean: jest.fn().mockResolvedValue({ type: 'chat', members: ['user-1', 'user-sam', 'bot-1'] }), + }), + lean: jest.fn().mockResolvedValue({ + _id: 'pod-1', type: 'chat', members: ['user-1', 'user-sam', 'bot-1'], + }), + }); + AgentEvent.countDocuments.mockResolvedValue(0); + mockInstallations([SEAT]); +}); + +describe('a human handle is not a delivery target', () => { + test('@handle for a human enqueues no event of any kind', async () => { + await send({ id: 'm-1', content: 'can you decide this @sam' }); + + // Not "no chat.mention" — no event at all. A future human-push branch is + // as likely to invent a type as to reuse this one. + expect(enqueued()).toHaveLength(0); + }); + + test('CONTROL: the same sentence addressed to an installed agent DOES enqueue', async () => { + await send({ id: 'm-2', content: 'can you decide this @seat-a' }); + + const got = mentions(); + expect(got).toHaveLength(1); + expect(got[0]).toMatchObject({ agentName: 'seat-a', type: 'chat.mention' }); + }); + + test('one message naming both routes to the agent only', async () => { + // The discriminating case. A human-push branch added beside the agent one + // leaves every single-target fixture above intact — it only shows up when + // both appear at once and the count stops being 1. + await send({ id: 'm-3', content: '@seat-a please answer, @sam to press' }); + + expect(enqueued()).toHaveLength(1); + expect(enqueued()[0]).toMatchObject({ agentName: 'seat-a' }); + }); +}); + +describe('the thread-follow half is guarded by threadRootId', () => { + test('a plain channel post materialises no thread state for the mentioned human', async () => { + await send({ id: 'm-4', content: 'over to you @sam' }); + + expect(ThreadUserState.followByParticipation).not.toHaveBeenCalled(); + // And it declined to make the lookup at all, rather than making it and + // finding nobody — the guard is on the call, not on the result. + expect(User.find).not.toHaveBeenCalled(); + }); + + test('CONTROL: the same message inside a thread DOES follow that human', async () => { + await send({ + id: 'm-5', content: 'over to you @sam', thread_root_id: 101, threadRootId: 101, + }); + + expect(ThreadUserState.followByParticipation).toHaveBeenCalledWith(101, 'user-sam', 'pod-1'); + }); + + test('a follow is not a wake — the threaded case still enqueues no event', async () => { + // Both halves of the cue's ceiling in one assertion: the handle bought a + // pull-surface row, and nothing was pushed. + await send({ + id: 'm-6', content: 'over to you @sam', thread_root_id: 101, threadRootId: 101, + }); + + expect(ThreadUserState.followByParticipation).toHaveBeenCalled(); + expect(enqueued()).toHaveLength(0); + }); +}); diff --git a/backend/__tests__/unit/services/agentMentionService.test.js b/backend/__tests__/unit/services/agentMentionService.test.js index 703733e086..8bcf2e0844 100644 --- a/backend/__tests__/unit/services/agentMentionService.test.js +++ b/backend/__tests__/unit/services/agentMentionService.test.js @@ -705,6 +705,244 @@ describe('AgentMentionService', () => { expect(ev.payload.content).toContain('Hi @nova'); }); + // The three-verb cue has two halves and #1176 shipped only one. + // + // It put the MECHANICS into the frame — what a plain post, + // `replyToMessageId` and `threadRootId` each do. Sam's ask (57672) was + // narrower and different: teach agents *when* to use each. A description + // of three fields does not answer a choice, so an agent that read the + // paragraph still re-derived which verb its own next message wanted, + // every time. + // + // These pin the DECISION RULE, not the paragraph around it. The cue is + // delivered as one opaque string, so "the frame mentions threads" is not + // evidence the choosing half survived an edit — the mechanics clauses + // keep that assertion green by themselves. The third test is the control + // that proves these can tell the two halves apart. + describe('three-verb cue teaches choosing, not just mechanics', () => { + const frame = async () => { + setupForAgent({ agentName: 'openclaw', instanceId: 'nova', displayName: 'Nova' }); + await AgentMentionService.enqueueMentions({ + podId: 'pod-verbs-1', + message: { content: 'Hi @nova', id: 'msg-verbs-1' }, + userId: 'user-1', + username: 'sam', + }); + return lastPayload().payload.content; + }; + + test('carries the decision rule for all three verbs', async () => { + const content = await frame(); + expect(content).toContain('if your message answers one person, reply'); + expect(content).toContain('if it continues a topic, thread'); + expect(content).toContain('if it starts one, post'); + }); + + test('says a reply inside a thread still addresses its author', async () => { + // Without this the rule reads as three mutually exclusive branches, + // and an agent concludes it must choose between quoting and + // threading. The two fields are independent. + expect(await frame()).toContain('A reply inside a thread is allowed and still addresses its author'); + }); + + // The MECHANICS half had no reader at all until now, and the control + // below is not one: it asserts against its own literal, which is the + // right shape for proving the choosing assertions discriminate and the + // wrong shape for noticing that the cue changed. + // + // Found by mutation on 2026-08-29. Deleting any mechanics clause from + // the live cue — or INVERTING one, so the frame tells every agent that + // a threaded continuation pings the whole pod — left all 140 tests + // green. The choosing half reds on the same treatment, so the gap was + // one half of one sentence, not the suite. + // + // These read the live frame. They are deliberately clause-level rather + // than a whole-paragraph match, so ordinary copy-editing does not red + // the build while a claim reversal does. + test('names all three verbs in the live frame', async () => { + const content = await frame(); + expect(content).toContain('a plain post broadcasts to the channel'); + expect(content).toContain('replyToMessageId quotes and ADDRESSES a message'); + // NOT a bare `toContain('threadRootId')` — the name appears again + // later in the same frame ("continue the detail under your own root + // with threadRootId"), so the bare form stays green when the clause + // that DEFINES the verb is deleted. Verified by mutation: bare passed, + // this reds. + expect(content).toContain('continues a thread'); + }); + + test('states that replyToMessageId pings the author it addresses', async () => { + // An agent that believes a quote is silent uses it for asides, and + // the person quoted is woken every time. + expect(await frame()).toContain('its author is pinged'); + }); + + test('states that threadRootId does NOT ping, and never claims it does', async () => { + // This is the claim the frame gets WRONG most expensively if it + // drifts: `effectiveFollowerIds` is precisely why a threaded + // continuation is quiet, and an agent told otherwise stops threading + // at all. Asserting the negative alone is not enough — a cue can + // carry both sentences — so the contradiction is excluded too. + const content = await frame(); + expect(content).toContain('WITHOUT pinging anyone'); + expect(content).not.toMatch(/threadRootId[^.]*pings (every|all|the pod)/i); + }); + + test('control: the mechanics half alone does not satisfy the assertions above', () => { + const mechanicsOnly = 'a plain post broadcasts to the channel; replyToMessageId quotes ' + + 'and ADDRESSES a message — its author is pinged; threadRootId continues a thread ' + + 'WITHOUT pinging anyone — followers see it, the channel stays uncluttered.'; + expect(mechanicsOnly).not.toContain('if your message answers one person, reply'); + expect(mechanicsOnly).not.toContain('if it starts one, post'); + }); + }); + + // Humans are addressed by handle, and only by handle (TASK-070a). + // + // Sam observed 2026-08-25 that seats write about him by name and nothing + // routes. The frame taught three verbs, all of which move attention + // between AGENTS; none of them reach a person, and the paragraph never + // said so. An agent that had read it correctly could still conclude that + // naming a human was a way of addressing one. + // + // The control matters more than usual here. This frame already contains + // the word "human" twice (the token-misattribution clause) and the + // literal "@" many times, so a loose assertion stays green on a frame + // that has lost the routing rule entirely. + describe('human-handle cue', () => { + const frame = async () => { + setupForAgent({ agentName: 'openclaw', instanceId: 'nova', displayName: 'Nova' }); + await AgentMentionService.enqueueMentions({ + podId: 'pod-human-1', + message: { content: 'Hi @nova', id: 'msg-human-1' }, + userId: 'user-1', + username: 'sam', + }); + return lastPayload().payload.content; + }; + + test('tells the agent to @mention a handle when it needs a human', async () => { + expect(await frame()).toContain('@mention their handle'); + }); + + test('states that a bare name reaches no one', async () => { + // The failure is silent — nothing errors, the message posts, and no + // attention routes — so the cue has to name the outcome, not just + // prescribe the handle. Without this an agent reads the rule as a + // style preference it may skip when the name reads more naturally. + const content = await frame(); + expect(content).toContain('A bare name reaches no one'); + expect(content).toContain('matched on the literal @handle'); + }); + + test('states the handle is necessary and not sufficient', async () => { + // Without this the cue teaches, by contrast with "reaches no one", + // that the handle DOES notify. It does not: humans get no AgentEvent + // row, so the ceiling is a pull surface. An agent that believes it + // has notified Sam stops working and waits — a new false model, and + // harder to spot than the old one because the message now looks + // correctly addressed. + const content = await frame(); + expect(content).toContain('necessary and not sufficient'); + expect(content).toContain('nothing pushes'); + }); + + test('control: the pre-TASK-070 frame does not satisfy the assertions above', () => { + // Verbatim from the clauses that shipped before this change, and they + // are the ones most likely to keep a sloppy assertion green: both + // mention humans, and the second is entirely about @handles. + const beforeTaskO70 = 'Never post through an operator\'s CLI profile (`commonly pod send`) ' + + 'or a human user\'s token — that misattributes your words to a human. ' + + 'replyToMessageId quotes and ADDRESSES a message — its author is pinged. ' + + 'Rule of thumb: if your message answers one person, reply.'; + expect(beforeTaskO70).not.toContain('@mention their handle'); + expect(beforeTaskO70).not.toContain('A bare name reaches no one'); + expect(beforeTaskO70).not.toContain('matched on the literal @handle'); + expect(beforeTaskO70).not.toContain('necessary and not sufficient'); + expect(beforeTaskO70).not.toContain('nothing pushes'); + }); + }); + + // The overflow rule, and the qualifier without which it is harmful. + // + // @sprint-review (57706) traced it to `effectiveFollowerIds`, whose + // `participants` CTE is `SELECT DISTINCT user_id FROM messages WHERE + // thread_root_id = $1 OR id = $1` — authors only. A thread you just + // opened has exactly one author, you, so narrowToThread empties the wake + // list for every peer. "Post your headline, continue under your own + // root" therefore licenses broadcasting a title and writing the + // substance where nothing wakes. + // + // These pin the two clauses that make the rule safe. Both name a kernel + // mechanism rather than a preference, which is why they are worth a + // guard: standing alone is required because the channel post is the only + // delivery the room is guaranteed, and the @mention escape works because + // the mention path runs BEFORE this narrowing and then writes + // `following IS TRUE` for the target. + describe('prose-overflow rule carries its follower-set qualifier', () => { + const frame = async () => { + setupForAgent({ agentName: 'openclaw', instanceId: 'nova', displayName: 'Nova' }); + await AgentMentionService.enqueueMentions({ + podId: 'pod-overflow-1', + message: { content: 'Hi @nova', id: 'msg-overflow-1' }, + userId: 'user-1', + username: 'sam', + }); + return lastPayload().payload.content; + }; + + test('still routes prose overflow to a thread rather than an attachment', async () => { + const content = await frame(); + expect(content).toContain('Prose overflow goes in a thread, not an attachment'); + expect(content).toContain('never for the rest of your message'); + }); + + test('requires the top-level message to stand alone', async () => { + const content = await frame(); + expect(content).toContain('Your top-level message must stand alone'); + expect(content).toContain("a fresh thread's followers are its authors"); + }); + + test('offers the @mention escape for a peer who needs the continuation', async () => { + const content = await frame(); + expect(content).toContain('@mention them in the threaded message'); + expect(content).toContain('addressing is never scoped by the thread'); + }); + + test('does not promise the @mention enrols a peer who muted the thread', async () => { + // @sprint-review (58348). The first draft said addressing "enrols + // them for the rest of it" flat. `followByParticipation` writes only + // `WHERE following IS NULL` and `effectiveFollowerIds` subtracts + // muted last, so a mute survives both — the mention wakes them, + // nothing subscribes them. Checking that the write happens is not + // checking the condition it is guarded on. + expect(await frame()).toContain('unless they have muted it'); + }); + + test('names what threading actually does to a message', async () => { + // The mechanism sentences say what happens; this one corrects the + // intuition that produces the mistake. Guarded separately because a + // future trim would read it as a flourish on top of clauses that + // already "cover it" — it is the only line that tells an agent + // threading REMOVES something rather than moving it. + expect(await frame()).toContain('Threading does not relocate your message, it un-addresses it'); + }); + + test('control: the unqualified overflow rule fails the two assertions above', () => { + // The exact sentence that shipped before 57706. If a future edit + // reverts to it, the two tests above must go red — this proves they + // can tell the qualified rule from the bare one rather than both + // passing on the shared "thread, not an attachment" phrase. + const unqualified = 'Prose overflow goes in a thread, not an attachment: post your ' + + 'headline to the channel, continue under your own root with threadRootId; ' + + 'attachments are for genuine artifacts (files, images, documents), never for ' + + 'the rest of your message.'; + expect(unqualified).toContain('Prose overflow goes in a thread, not an attachment'); + expect(unqualified).not.toContain('Your top-level message must stand alone'); + expect(unqualified).not.toContain('addressing is never scoped by the thread'); + }); + }); + // Author/age frame. The envelope has always carried `username` and // `createdAt`; the model only ever sees `payload.content`, so they // were invisible to their only reader (four sprint agents spent diff --git a/backend/services/agentMentionService.ts b/backend/services/agentMentionService.ts index 3809ddcca1..82b41a147d 100644 --- a/backend/services/agentMentionService.ts +++ b/backend/services/agentMentionService.ts @@ -491,11 +491,108 @@ const formatPodContextFrame = (podId: string): string => `threadRootId (the thread root's message id, in the same post body) continues a thread ` + `WITHOUT pinging anyone — followers see it, the channel stays uncluttered. ` + `Quote and thread are independent: use both fields to quote someone inside a thread. ` + - `Prose overflow goes in a thread, not an attachment: post your headline to the channel, ` + - `continue under your own root with threadRootId; attachments are for genuine artifacts ` + - `(files, images, documents), never for the rest of your message. ` + + `Prose overflow goes in a thread, not an attachment: post the POINT to the channel, ` + + `continue the detail under your own root with threadRootId; attachments are for genuine ` + + `artifacts (files, images, documents), never for the rest of your message. ` + + // @sprint-review (57706) found the hole in the sentence above as first + // drafted, and it was the expensive kind: "post your headline, continue + // under your own root" reads as license to make the top-level message a + // POINTER. It cannot be. `effectiveFollowerIds` derives participants as + // `SELECT DISTINCT user_id FROM messages WHERE thread_root_id = $1 OR id + // = $1` — AUTHORS ONLY. At the instant you open a thread under your own + // root you are its only author, so you are its only follower, and + // narrowToThread empties the wake list for everyone else. An agent + // obeying the unqualified cue would broadcast a title and write the + // substance where nothing wakes. + // + // Two clauses close it, and both are mechanisms already in the kernel + // rather than anything this cue asks for: the top-level message stands + // alone (the room is guaranteed to get it), and an @mention inside the + // thread reaches a named peer regardless of scope — the mention path runs + // before this narrowing, and `followMentionedThreadUsers` then writes + // `following IS TRUE` for that peer, enrolling them for the ambient + // remainder — unless they have MUTED it. @sprint-review (58348) caught the + // overclaim: `followByParticipation` writes only `WHERE following IS NULL`, + // and `effectiveFollowerIds` subtracts muted last, so a mute survives both. + // The mention still wakes them (addressing outranks a mute); it just does + // not subscribe them. The first version of this clause checked that the + // write happens and not the condition it is guarded on. + `Your top-level message must stand alone — a fresh thread's followers are its authors, ` + + `so the moment you open one you are its only follower and the continuation is ambient to ` + + `everyone else. If a specific peer needs the detail, @mention them in the threaded ` + + `message: addressing is never scoped by the thread, and unless they have muted it that ` + + `also enrols them for the rest of it. ` + + // @sprint-review's (57707) compression of both clauses, and the reason it + // earns a line of its own: the two sentences above state mechanisms, and a + // mechanism does not correct a wrong intuition. The wrong intuition is that + // threading MOVES a message. It does not — it removes its address. + `Threading does not relocate your message, it un-addresses it. ` + `Every message you read carries thread_root_id (null = not in a thread), and adding ` + - `?threadRootId= to your messages read returns just that thread.]`; + `?threadRootId= to your messages read returns just that thread. ` + + // The clauses above describe what each verb DOES. Sam's ask (57672) was + // "teach agents WHEN to use reply, or in thread, or quote" — and a + // mechanics description does not answer a choice. This sentence is the + // half that was missing: without it an agent that has read the paragraph + // still re-derives which verb its own next message wants, every time, + // from field semantics. + // + // Copy is @ux-lead's (57678), kept close to their phrasing on purpose. It + // is written as a test the agent applies to its own draft — does this + // answer one person, continue a topic, or start one — rather than as three + // more facts about the fields. + // + // Verified before shipping rather than taken on the copy's word: "wakes + // followers only" is true. threadWakeScopeService.narrowToThread scopes + // ambient thread activity to the thread's effective followers, and can + // only NARROW an already-computed opt-in list. + // + // That verification was true and insufficient, which is the lesson worth + // keeping: it confirmed the SET the wake is narrowed to and never asked + // what that set CONTAINS on the path the cue tells agents to take. For a + // thread you just opened, it contains you and nobody else. Confirming a + // predicate is not confirming its extension — see the overflow comment + // below for the hole it left open. + `Rule of thumb: if your message answers one person, reply; if it continues ` + + `a topic, thread; if it starts one, post. A reply inside a thread is allowed ` + + `and still addresses its author. ` + + // Humans are addressed by handle, and ONLY by handle (TASK-070a, Sam + // observed 2026-08-25). Every verb above routes attention between agents; + // none of them reach a person. A human's attention is matched on the + // literal `@handle` — activityService's `mentions` filter tests + // `content.includes('@' + username)`, and resolveHumanMentionUserIds + // extracts handles from `[a-z0-9_-]` after an `@`. A bare name matches + // neither, so "Sam should decide this" is addressed to nobody. + // + // This is the human-facing twin of the gap ADR-018 D6.3 closed for bots: a + // message that is plainly ABOUT someone still has to be addressed TO them + // before anything routes. The bot version was a missing implicit-reply + // wake; this one is a missing handle, and only the author can supply it. + // + // Deliberately teaches the escape and not a heuristic. Whether a bare name + // SHOULD route is an open decision (TASK-070b) precisely because matching + // names is fuzzy — every message about Sam is not for Sam — so the cue + // must not imply that writing the name is enough. + // + // The handle is NECESSARY, not sufficient, and the cue has to say both or + // it installs a fresh false model in place of the old one (@sprint-review + // on #1244). Humans have no AgentEvent delivery row — `enqueueMentions` + // never enqueues for a person — so an @handle buys the `isMention` flag on + // the activity feed and nothing else. Even the thread-follow half is + // narrower than it reads: `resolveHumanMentionUserIds` has exactly one call + // site, inside the `if (threadRootId)` branch of `enqueueMentions`, so a + // plain channel post gets the flag alone. Cited by symbol and branch rather + // than by line: this comment carried `:1743` and the #1265 merge moved the + // call to `:1773` without touching either — a stale citation inside the + // paragraph about claims decaying. That surface is PULL — ADR-017's + // only-interrupter rule reserves push for the escalation envelope — so "I mentioned them" is never "they + // know", and an agent that stops there has blocked itself on a filter + // nobody may have opened. + `When you need a HUMAN — a decision, a merge press, an answer only they ` + + `have — @mention their handle. A bare name reaches no one: human attention ` + + `is matched on the literal @handle, so "Sam should decide this" is addressed ` + + `to nobody. The handle is necessary and not sufficient — it flags the message ` + + `in a mentions filter the human pulls; nothing pushes. Say plainly what you ` + + `need, and never treat a mention as an answer received.]`; // Cross-runtime consultation cue. Companion to the pod-context frame // above; same rationale (inline cue beats structured metadata per