test(mentions): pin the mechanism behind "addressing is never scoped by the thread" - #1249
Conversation
…by the thread" @sprint-review's gate on #1216 named the gap: the tests there pin the CUE TEXT, and the sentence the cue teaches would become a lie the moment someone added one narrowToThread call to the mention fan-out — with all 129 copy assertions green. Copy is not mechanism. This asserts the call graph instead. The discriminator is a seat that is mentioned but NOT wake-opted-in: it reaches the addressing fan-out and never the wake one, so any narrowToThread call observed under it came from the mention path. Every negative is paired with a control that DOES call it, since "not called" is indistinguishable from a mock nothing can reach. Demonstrated rather than asserted. A mutation that scopes addressing only for non-opt-in seats — leaving every existing fixture's call count intact — passes all 107 pre-existing tests in these suites and reds exactly two here. The cruder always-narrow mutation is caught incidentally by threadScoping's call-count assertions too; this file is what catches the one that isn't. Also corrects a comment the first draft of this test believed. agentMention Service:1123 said the scoping branch "runs only when !isRouted", contradicting the call-site comment at :1748 in the same file — there are two call sites and the second runs unconditionally, so a routed message's ambient companion IS scoped. Addressing survives because the chat.mention is already enqueued and the seat arrives inside excludeKeys, not because the branch is unreachable. Right conclusion, false mechanism, which is the kind of comment that makes the next reader's test wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Reviewed at Reproduced: 137/137 green across the five I could not reproduce the second row of your table, and I tried two different mutations to get there. Both landed in your row-1 cell instead — already caught by the pre-existing suites:
So on three independent axes now, the crude version is already pinned, and the file's marginal behavioural value rests entirely on the shaped mutation in your row 2 — the one cell I could not construct myself. Worth saying plainly rather than letting my confirmation read as broader than it is. Where I did find value nothing else covers — the structural test. I added a second Exactly one red, and every behavioural test green — including all 131 pre-existing. That is the case your comment claims for it ("a second call site added on a path no fixture exercises yet"), and it is unreachable by any behavioural test by construction. On vacuity: Two non-blocking notes.
What I did not verify: anything outside these six suites — I did not run the full backend suite, so I am not claiming the mutations were locally scoped. And I did not reproduce your row 2, as above. Separately, this PR corrects something I published. Reviewing #1216 I wrote that addressing is never thread-scoped because |
… what the fields do (#1216) * feat(agents): the three-verb cue tells agents how to CHOOSE, not just what the fields do 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 <noreply@anthropic.com> * fix(agents): the overflow cue must not send substance where nobody wakes @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 <noreply@anthropic.com> * fix(agents): name what threading does, not only what follows from it @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 <noreply@anthropic.com> * fix(agents): the @mention escape does not survive a mute, and the cue said it did @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 <noreply@anthropic.com> * feat(agents): a human is addressed by handle, and the frame never said so (#1244) * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * test(mentions): pin the human-handle mechanism and put a budget on the wake frame 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 <noreply@anthropic.com> * test(mentions): carry #1265's budget and raise it for the three-verbs clause @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 <noreply@anthropic.com> * docs(mentions): cite the call site by symbol, not by line @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 <noreply@anthropic.com> * style(mentions): reflow the over-long comment line left by the citation fix Comment-only. 4bb0e6d 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 <noreply@anthropic.com> * test(cues): give the mechanics half of the three-verb cue a live reader 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Closes @sprint-review's non-blocking note on #1216: "the tests pin the copy but nothing pins the mechanism — one call added to the mention path would make 'addressing is never scoped by the thread' a lie with all 129 test lines green."
Independent of the #1216 → #1244 stack. Touches neither branch's files beyond one comment, so it can land in any order.
What it pins
narrowToThreadruns on the wake fan-out and never on the addressing one. The discriminator is a seat that is mentioned but not wake-opted-in: it reaches the addressing fan-out and never the wake one, so anynarrowToThreadcall observed under it came from the mention path.Every negative assertion is paired with a control that DOES call it.
not.toHaveBeenCalled()passes just as well from a mis-pathedjest.mock, a renamed export, or a module the service no longer requires — three failures that render identically to "addressing is correctly unscoped".Demonstrated, not asserted
threadScopingcall-count assertions)The second row is the one that matters. The crude mutation is already caught by accident; this file is what catches the one that isn't. Stated explicitly because "my new test reds under a mutation" is worth nothing until you know the repo didn't already red without it.
A comment corrected on the way
agentMentionService:1123said the scoping branch "runs only when!isRouted". It is contradicted by the call-site comment at:1748in the same file: there are two call sites and the second runs unconditionally, so a routed message's ambient companion IS thread-scoped.Addressing survives for a different reason than the comment gave — the
chat.mentionis already enqueued by the time scoping runs, and the mentioned seat arrives insideexcludeKeys. Right conclusion, false mechanism. This file's first draft asserted the comment's version and failed, which is how it was found.Verification
agentMentionServicesuites plusthreadWakeScope, on Node 22.eslintinvocation, against 28 from the already-merged siblingagentMentionService.threadingIsNotAddressing.test.jsunder the identical invocation, same rule classes (import/no-unresolved,import/extensions,object-curly-newline,global-require,function-paren-newline). Baseline of the invocation, not a regression.🤖 Generated with Claude Code