From 0e044a3b3a4502bcd80f34e9e845f394accfc89c Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 25 Aug 2026 01:05:01 -0700 Subject: [PATCH 01/10] feat(agents): the three-verb cue tells agents how to CHOOSE, not just what the fields do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sam's ask (57672) was "teach agents when to use reply, or in thread, or quote." #1176 shipped the mechanics — what a plain post, `replyToMessageId` and `threadRootId` each do — and that is the other question. A description of three fields does not answer a choice, so an agent that has read the whole paragraph still re-derives which verb its next message wants, every time, from field semantics. Adds @ux-lead's decision rule (57678), close to their phrasing on purpose: 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. It is written as a test the agent applies to its own draft rather than as three more facts. The trailing clause is load-bearing: without it the rule reads as three mutually exclusive branches and an agent concludes it must pick between quoting and threading, when the two fields are independent. Verified rather than taken on the copy's word — ux-lead's framing says each verb "says who is woken", and that claim is checkable. It holds: threadWakeScopeService.narrowToThread scopes ambient thread activity to the thread's effective followers and can only NARROW an already-computed opt-in list, so "wakes followers only" is the real behaviour, not aspirational. Three tests in the existing inline-cue suite, pinning the decision rule rather than the paragraph around it — the cue ships as one opaque string, so "the frame mentions threads" stays green on the mechanics clauses alone. The third is a control proving the assertions can tell the two halves apart. Probe: replacing the rule with a mechanics-only tail reddens exactly the two behavioural tests and leaves the control green. Suite 110 passed; tsc clean for this file. Co-Authored-By: Claude Opus 5 --- .../unit/services/agentMentionService.test.js | 49 +++++++++++++++++++ backend/services/agentMentionService.ts | 21 +++++++- 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/backend/__tests__/unit/services/agentMentionService.test.js b/backend/__tests__/unit/services/agentMentionService.test.js index 703733e08..21ca432a9 100644 --- a/backend/__tests__/unit/services/agentMentionService.test.js +++ b/backend/__tests__/unit/services/agentMentionService.test.js @@ -705,6 +705,55 @@ 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'); + }); + + 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'); + }); + }); + // 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 1f9081cbf..ae4c261f8 100644 --- a/backend/services/agentMentionService.ts +++ b/backend/services/agentMentionService.ts @@ -495,7 +495,26 @@ const formatPodContextFrame = (podId: string): string => `continue under your own root with threadRootId; attachments are for genuine artifacts ` + `(files, images, documents), never for the rest of your message. ` + `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. + `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.]`; // Cross-runtime consultation cue. Companion to the pod-context frame // above; same rationale (inline cue beats structured metadata per From ae623c4e7b031acd25e17a047e7c48a5002dc2bc Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 25 Aug 2026 01:30:43 -0700 Subject: [PATCH 02/10] fix(agents): the overflow cue must not send substance where nobody wakes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @sprint-review (57706) found the hole in the prose-overflow sentence, and it is the expensive kind — the cue was obeyable and wrong. "Post your headline to the channel, continue under your own root" reads as license to make the top-level message a pointer. It cannot be. `effectiveFollowerIds` derives `participants` as authors only — `SELECT DISTINCT user_id FROM messages WHERE thread_root_id = $1 OR id = $1`. 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 every peer. An agent following the cue literally broadcasts a title and writes the substance where zero agents are woken. Two clauses close it, both naming kernel mechanisms rather than preferences: the top-level message must stand alone (the channel post is the only delivery the room is guaranteed), 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 target, enrolling them for the ambient remainder. The comment recording the pre-ship verification is corrected too. "Wakes followers only" was true and insufficient: 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. Confirming a predicate is not confirming its extension. Four guards, including a control that pins the exact unqualified sentence that shipped before this — so a revert reddens rather than passing on the shared "thread, not an attachment" phrase. Negative control: dropping the two clauses reddens exactly 2 of 69, the other 67 stay green. Co-Authored-By: Claude Opus 5 --- .../unit/services/agentMentionService.test.js | 61 +++++++++++++++++++ backend/services/agentMentionService.ts | 35 ++++++++++- 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/backend/__tests__/unit/services/agentMentionService.test.js b/backend/__tests__/unit/services/agentMentionService.test.js index 21ca432a9..319408c29 100644 --- a/backend/__tests__/unit/services/agentMentionService.test.js +++ b/backend/__tests__/unit/services/agentMentionService.test.js @@ -754,6 +754,67 @@ describe('AgentMentionService', () => { }); }); + // 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('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 ae4c261f8..9015bee1e 100644 --- a/backend/services/agentMentionService.ts +++ b/backend/services/agentMentionService.ts @@ -491,9 +491,31 @@ 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. So addressing once in a thread buys delivery of the rest. + `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 it enrols them for the rest of 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. ` + // The clauses above describe what each verb DOES. Sam's ask (57672) was @@ -512,6 +534,13 @@ const formatPodContextFrame = (podId: string): string => // 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.]`; From 0f093f9a90ea0356547777a1b8799dc18020745f Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 25 Aug 2026 01:34:15 -0700 Subject: [PATCH 03/10] fix(agents): name what threading does, not only what follows from it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @sprint-review (57707): "continue in-thread" reads as RELOCATING a message when it is actually UN-ADDRESSING it. That is the intuition behind the mistake, and the two clauses added in the previous commit do not correct it — they state mechanisms, and a mechanism does not dislodge a wrong model. One sentence, guarded separately so a future trim cannot read it as a flourish on clauses that already "cover it". It is the only line in the frame that tells an agent threading REMOVES something rather than moving it. Co-Authored-By: Claude Opus 5 --- .../__tests__/unit/services/agentMentionService.test.js | 9 +++++++++ backend/services/agentMentionService.ts | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/backend/__tests__/unit/services/agentMentionService.test.js b/backend/__tests__/unit/services/agentMentionService.test.js index 319408c29..2808fdc88 100644 --- a/backend/__tests__/unit/services/agentMentionService.test.js +++ b/backend/__tests__/unit/services/agentMentionService.test.js @@ -800,6 +800,15 @@ describe('AgentMentionService', () => { expect(content).toContain('addressing is never scoped by the thread'); }); + 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 diff --git a/backend/services/agentMentionService.ts b/backend/services/agentMentionService.ts index 9015bee1e..c6683d967 100644 --- a/backend/services/agentMentionService.ts +++ b/backend/services/agentMentionService.ts @@ -516,6 +516,11 @@ const formatPodContextFrame = (podId: string): string => `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 it 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. ` + // The clauses above describe what each verb DOES. Sam's ask (57672) was From 2b073ff48b1c628f73bc15ebf9178a234713c55f Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 25 Aug 2026 01:40:59 -0700 Subject: [PATCH 04/10] fix(agents): the @mention escape does not survive a mute, and the cue said it did MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @sprint-review (58348). The clause added two commits ago promised that addressing a peer inside a thread "enrols them for the rest of it", flat. It does not when they have muted the thread: `followByParticipation` writes only `WHERE thread_user_state.following IS NULL`, and `effectiveFollowerIds` subtracts `muted` last, so an explicit mute survives both paths. The mention itself still wakes them — addressing outranks a mute, by design. What fails is the subscription, which is exactly the half the cue was selling. Their diagnosis is the reusable part and it is the same shape as the bug it corrects: I checked that the write HAPPENS and not the condition it is guarded on. `followByParticipation`'s own docstring names the case outright ("muting a thread and then being mentioned in it is the ordinary case, not an edge one") and I read past it. Co-Authored-By: Claude Opus 5 --- .../unit/services/agentMentionService.test.js | 10 ++++++++++ backend/services/agentMentionService.ts | 10 ++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/backend/__tests__/unit/services/agentMentionService.test.js b/backend/__tests__/unit/services/agentMentionService.test.js index 2808fdc88..679da6472 100644 --- a/backend/__tests__/unit/services/agentMentionService.test.js +++ b/backend/__tests__/unit/services/agentMentionService.test.js @@ -800,6 +800,16 @@ describe('AgentMentionService', () => { 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 diff --git a/backend/services/agentMentionService.ts b/backend/services/agentMentionService.ts index c6683d967..5995269af 100644 --- a/backend/services/agentMentionService.ts +++ b/backend/services/agentMentionService.ts @@ -511,11 +511,17 @@ const formatPodContextFrame = (podId: string): string => // 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. So addressing once in a thread buys delivery of the rest. + // 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 it enrols them for the rest of it. ` + + `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 From 29fee26152107cd104d4c1459e2673c9c00d2aec Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 26 Aug 2026 00:12:12 -0700 Subject: [PATCH 05/10] feat(agents): a human is addressed by handle, and the frame never said so (#1244) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(agents): a human is addressed by handle, and the frame never said so Sam observed 2026-08-25 that seats write about him by name and nothing routes. The pod-context frame taught three addressing verbs — plain post, replyToMessageId, threadRootId — and all three move attention between AGENTS. None reaches a person, and the paragraph never said so, so an agent that had read it correctly could still conclude that naming a human was a way of addressing one. Verified rather than assumed, because the cue is only worth shipping if the escape it teaches actually works: - activityService.ts:517-521 builds `mentionNeedle = '@' + lowerUsername` and sets `isMention` from `content.includes(needle)`; :591 is the `mentions` filter that reads it. Substring on the literal handle. - resolveHumanMentionUserIds (agentMentionService.ts:1033) extracts handles from `[a-z0-9_-]` after an `@`, anchored and case-insensitive. So `@handle` surfaces in the human's mentions filter and a bare name matches neither test. The failure is silent — nothing errors, the message posts, no attention routes — which is why the cue names the outcome and not just the prescription. This is the human-facing twin of the gap ADR-018 D6.3 closed for bots: a message plainly ABOUT someone still has to be addressed TO them before anything routes. There the fix was a missing implicit-reply wake; here only the author can supply the handle. Deliberately teaches the escape and not a heuristic. Whether a bare name SHOULD route is an open decision (TASK-070b) precisely because name matching is fuzzy — every message about Sam is not for Sam — so the cue must not imply that writing the name is enough. Tests pin the two halves separately (prescription, and the silent-failure outcome) plus a control built from the pre-change clauses most likely to keep a loose assertion green: the frame already contains "human" twice and "@" many times. Mutation-checked — softening "A bare name notifies nobody" fails the second test and leaves the other two green. Stacked on #1216, which edits the same frame string; based on its head rather than main so the two clauses do not conflict. Co-Authored-By: Claude Opus 5 * fix(cue): the handle is necessary, not sufficient — state the ceiling too @sprint-review's review of #1244: every clause about the failure was precise and nothing stated the ceiling of the remedy, so an agent reads "a bare name notifies nobody" as "and the @handle notifies somebody". It does not. Humans have no AgentEvent delivery row, so the handle buys the `isMention` flag on the activity feed — a pull surface ADR-017 keeps off the push channel. Re-derived the narrower half myself rather than borrowing it: `resolveHumanMentionUserIds` is called only inside `if (threadRootId)` (:1743), so a plain channel post gets the flag alone and not even the thread follow. That would have been a new false model replacing an old one, and harder to catch — the message now looks correctly addressed while the seat sits waiting on an answer nobody was told to give. Two assertions, both mutation-checked; the control gains the same pair. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- .../unit/services/agentMentionService.test.js | 66 +++++++++++++++++++ backend/services/agentMentionService.ts | 37 ++++++++++- 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/backend/__tests__/unit/services/agentMentionService.test.js b/backend/__tests__/unit/services/agentMentionService.test.js index 679da6472..b006766fb 100644 --- a/backend/__tests__/unit/services/agentMentionService.test.js +++ b/backend/__tests__/unit/services/agentMentionService.test.js @@ -754,6 +754,72 @@ describe('AgentMentionService', () => { }); }); + // 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 diff --git a/backend/services/agentMentionService.ts b/backend/services/agentMentionService.ts index 5995269af..927faee88 100644 --- a/backend/services/agentMentionService.ts +++ b/backend/services/agentMentionService.ts @@ -554,7 +554,42 @@ const formatPodContextFrame = (podId: string): string => // 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.]`; + `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` is called only + // inside `if (threadRootId)` (:1743), so a plain channel post gets the flag + // alone. 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 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 06/10] 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); + }); +}); From 09eadb78cbeedff29750b83fe8e99550918fde49 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 26 Aug 2026 07:28:31 -0700 Subject: [PATCH 07/10] test(mentions): carry #1265's budget and raise it for the three-verbs clause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @sprint-review sharpened the merge-order note correctly: order was necessary, not sufficient. `BUDGET_MAX` lives only on #1265's branch, so this PR could not raise a constant it did not have — which meant a bulk press turned `main` red in EITHER order (this first, then #1265 lands on an over-budget frame; #1265 first, then this one lands red). Merging #1265's branch here removes the ordering hazard instead of documenting it. The raise now travels with the growth that caused it, so this PR is safe to merge in any order, and #1265 stays mergeable on its own. The band is 3550/4100, kept as tight around the new 3,935-character reference as 2600/3000 was around 2,877. Leaving MIN at 2,600 would have let a third of the frame disappear without failing — the exact hole the lower bound was added to close. What the fleet buys for the extra ~1,058 characters (+37%), per the constant's own instruction to state the trade: the three addressing verbs, spelled out. Agents were choosing between plain post / replyToMessageId / threadRootId with no statement of what each one does to attention, and picking wrong in both directions — broadcasting what should have been threaded, and threading what needed a ping. 135 passing across `agentMentionService`. Co-Authored-By: Claude Opus 5 --- .../services/agentMentionService.frameBudget.test.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/backend/__tests__/unit/services/agentMentionService.frameBudget.test.js b/backend/__tests__/unit/services/agentMentionService.frameBudget.test.js index 3c40633af..2345af258 100644 --- a/backend/__tests__/unit/services/agentMentionService.frameBudget.test.js +++ b/backend/__tests__/unit/services/agentMentionService.frameBudget.test.js @@ -52,8 +52,13 @@ const AgentEvent = require('../../../models/AgentEvent'); // @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; +// 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({ From 4bb0e6d9bc54e0e233607222d0eb652531c2d9fb Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 26 Aug 2026 07:45:12 -0700 Subject: [PATCH 08/10] docs(mentions): cite the call site by symbol, not by line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @sprint-review caught that this comment's `:1743` had drifted to `:1773` — my own #1265 merge moved the call and left the citation pointing 30 lines short. Inside the paragraph arguing that claims decay, which is a fair place to be caught. Their call was that it is not worth a push of its own, and for a line-number correction I agree. This is not that: a raw line number in a comment is a citation that expires on the next edit above it, so fixing the number restores the same defect for the next person. `resolveHumanMentionUserIds` has exactly one call site and it is inside the `if (threadRootId)` branch of `enqueueMentions` — both of which survive an edit that moves the line. The reason for the change is left in the comment, so the next author sees why the form is a symbol rather than a number and does not helpfully convert it back. Comment-only; the budget test measures string literals on non-comment lines, so the frame is unchanged. 135 passing across `agentMentionService`. Co-Authored-By: Claude Opus 5 --- backend/services/agentMentionService.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/backend/services/agentMentionService.ts b/backend/services/agentMentionService.ts index 4d52cfdf1..8bfcf64a2 100644 --- a/backend/services/agentMentionService.ts +++ b/backend/services/agentMentionService.ts @@ -578,9 +578,12 @@ const formatPodContextFrame = (podId: string): string => // 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` is called only - // inside `if (threadRootId)` (:1743), so a plain channel post gets the flag - // alone. That surface is PULL — ADR-017's only-interrupter rule reserves + // 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. From f22418a00571f70d8c512b2b0fea1ba35917eada Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 26 Aug 2026 08:06:04 -0700 Subject: [PATCH 09/10] style(mentions): reflow the over-long comment line left by the citation fix Comment-only. 4bb0e6d9 replaced the stale `:1743` citation but left one line running well past the wrap the rest of the paragraph keeps. Co-Authored-By: Claude Opus 5 --- backend/services/agentMentionService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/services/agentMentionService.ts b/backend/services/agentMentionService.ts index 8bfcf64a2..81ad6cba7 100644 --- a/backend/services/agentMentionService.ts +++ b/backend/services/agentMentionService.ts @@ -583,8 +583,8 @@ const formatPodContextFrame = (podId: string): string => // 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 + // 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 ` + From 2663ef2ad401fe5cd01acc4ff94f04fbdba32a78 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 29 Aug 2026 01:38:54 -0700 Subject: [PATCH 10/10] test(cues): give the mechanics half of the three-verb cue a live reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review gap in this PR, found by mutation on 2026-08-29. This PR pinned the CHOOSING half of the three-verb cue against the live frame and left the MECHANICS half unread. The existing control test is not a reader of it: `mechanicsOnly` is a literal in the test file asserted against itself, which is the right shape for proving the choosing assertions discriminate and the wrong shape for noticing that the cue changed. Measured before: deleting any mechanics clause from the live cue left all 140 tests green. So did INVERTING one — rewriting the frame to tell every woken agent that a threaded continuation "pings every member of the pod, loudly", which is the exact opposite of what `effectiveFollowerIds` does. Deleting a choosing clause reds 1, so the instrument worked and the gap was one half of one sentence. Adds three tests that read the live frame: names all three verbs states that replyToMessageId pings the author it addresses states that threadRootId does NOT ping, and never claims it does The third excludes the contradiction as well as asserting the negative, because a cue can carry both sentences at once. Mutation table, each anchor asserted at exactly one occurrence before applying, each restored after: drop the threadRootId defining clause was 140 pass -> now 1 FAILED drop "its author is pinged" was 140 pass -> now 2 FAILED drop the three-verb opener was 140 pass -> now 1 FAILED invert to "pings every member of the pod" was 140 pass -> now 1 FAILED copy-edit: colon -> semicolon, reworded 143 pass (unchanged) copy-edit: reword the plain-post clause 143 pass (unchanged) The two copy-edit controls are the point: these are clause-level rather than whole-paragraph, so ordinary editing does not red the build while a claim reversal does. One assertion was tightened after its own mutation came back green. `toContain('threadRootId')` passes even when the clause defining that verb is deleted, because the name appears again later in the same frame ("continue the detail under your own root with threadRootId"). It now asserts 'continues a thread', which reds. A bare name match on a string that repeats is not a reader of the sentence you meant. Suite: 140 -> 143, 8 suites, all passing. Co-Authored-By: Claude Opus 5 --- .../unit/services/agentMentionService.test.js | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/backend/__tests__/unit/services/agentMentionService.test.js b/backend/__tests__/unit/services/agentMentionService.test.js index b006766fb..8bcf2e084 100644 --- a/backend/__tests__/unit/services/agentMentionService.test.js +++ b/backend/__tests__/unit/services/agentMentionService.test.js @@ -745,6 +745,49 @@ describe('AgentMentionService', () => { 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 '