From cedfa041077c853df956bf57b07f5af4c5e6f363 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:38:58 -0700 Subject: [PATCH] test(mentions): pin the human-handle mechanism and put a budget on the wake frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TASK-074. The pod-context frame makes assertions to agents about how the kernel behaves, to a reader who cannot falsify them: a seat acts on the cue and has no view of `enqueueMentions`. Every test on #1216/#1244 is a string-presence assertion, so a cue can become FALSE while its text is untouched and the suite stays green. Two files, both mutation-checked against the pre-existing 113. **Claim 4 — "the handle is necessary and not sufficient; nothing pushes."** `agentMentionService.humansAreNotWoken.test.js`, 6 cases, each negative paired with a control: - a human @handle enqueues no AgentEvent of any type; the same sentence to an installed seat does; one message naming both routes only to the seat. - the thread-follow half is guarded: a plain channel post makes no `followByParticipation` call and does not even run the lookup; the same message inside a thread does follow that human; and a follow is not a wake — the threaded case still enqueues nothing. Blind-mutation baseline, run with the new file REMOVED, per @pod-architect's method on #1249: | mutation | pre-existing 113 | with this file | |---|---|---| | enqueue a chat.mention per resolved human handle (TASK-070b answered "push it") | **113 green** | 4 red | | hoist `resolveHumanMentionUserIds` out of `if (threadRootId)` | **113 green** | 1 red | Both are the realistic future edit, not a crude break. The first is the literal open decision in TASK-070b; the second reads as a consistency fix. **The frame's own size.** `agentMentionService.frameBudget.test.js` measures the rendered `chat.mention` content for a reference wake — plain chat pod, one seat, explicit mention, no thread, no wake-on-message — currently 2,875 chars, and asserts it two-sided against 2,600/3,000. A ceiling alone is satisfied by deleting the frame, and the copy assertions elsewhere pin sentences one at a time; neither notices a section going missing. Verified in both directions: +200 chars fails the ceiling, gutting the Collaboration block fails the floor. Not a cap. Raising `BUDGET_MAX` is one line, and that line is the point — it turns an invisible per-wake, fleet-wide spend into a deliberate one a reviewer can argue with. **#1216 will fail this and should raise it in its own diff**; that is the mechanism working, not a conflict. **Two corrections to the task row I filed, both found by running it.** Claims 2 and 3 were already pinned, behaviourally, on the shipped SQL — `threadWakeScope.test.js` runs `effectiveFollowerIds` against pg-mem with the real DDL, 24 cases. Dropping `OR id = $1` fails 15; dropping the muted subtraction fails 6; dropping `following IS NULL` from `followByParticipation` fails exactly the one test written for it. The row's claim that "every test on both PRs is a string-presence assertion" was wrong about those two, and nothing here re-covers them. And #1244 is NOT on main — it merged into #1216's branch, which is still open. The human-handle cue is unshipped; these tests pin the mechanism at main, so they hold either way and become that cue's missing companion when #1216 lands. 122/122 green across all seven agentMentionService suites on Node 22. Co-Authored-By: Claude Opus 5 --- .../agentMentionService.frameBudget.test.js | 120 +++++++++++++ ...ntMentionService.humansAreNotWoken.test.js | 162 ++++++++++++++++++ 2 files changed, 282 insertions(+) create mode 100644 backend/__tests__/unit/services/agentMentionService.frameBudget.test.js create mode 100644 backend/__tests__/unit/services/agentMentionService.humansAreNotWoken.test.js 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 000000000..3c40633af --- /dev/null +++ b/backend/__tests__/unit/services/agentMentionService.frameBudget.test.js @@ -0,0 +1,120 @@ +/** + * 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. +const BUDGET_MAX = 3000; +const BUDGET_MIN = 2600; + +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 000000000..061c95fa6 --- /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); + }); +});