Repository navigation
fix(answers): an addressed agent that lost the claim still answers the person who asked - #2274
Conversation
…e person who asked QA walk 5 (2026-10-16): "Give your agent its first task" missed a stranger's first answer and completed only on the second. Since #2269 an agent a person @-mentioned answers even when another agent (Scout) won the claim race, so it holds no claim, and it posts no reply edge. The answer rule (agentAnswerService.humansAnsweredBy) counted only those two, so the answer answered nobody. The same reader decides attention: an unclaimed addressed answer that @mentions the asker would open a needs-you row again. The native run names the message it is answering, answeringMessageId (its trigger; CTO's side, #2273). This is the consumer side: - humansAnsweredBy counts that message's author, only when the message is in the post's own pod. A claim is taken in the pod and a reply edge is checked at write (TASK-278); an id is not, so it is checked here. - postMessage carries it to completeFirstTaskOnAnswer, and through _postToTarget and PGMessage.create to recordMentionedUsers. It is never stored and never a reply edge, so answers are not threaded. - It is server-set only: the CAP route builds postMessage's options explicitly, and every MCP tool posts through that route. A test pins that a request body's answeringMessageId never reaches postMessage. Tests: the answer rule counts the trigger with no claim, refuses one from another pod, reads a claimed trigger once, and ignores a non-message id; with the real rule, an addressed agent that lost the claim completes TASK-002 with its first answer, and without the trigger it does not; attention opens no row for that answer, and does for a trigger from another pod; the field reaches create as its eighth argument and the attention writer, never the INSERT; the CAP body is refused. Mutations: ignoring the trigger, dropping the pod check, and dropping the field at attention, the model, the first-task check or the route each red an arm. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0157AG2wo8mJYqKezXNs48Ya
|
Second read of the consumer side at Checked by reading
Checked by running a trial merge of
Nothing blocking from me. 🤖 Generated with Claude Code |
lilyshen0722
left a comment
There was a problem hiding this comment.
SECURITY-GATE: PASS @ 441d25f. Independent security read (Cofounder operator session with a separate reviewer; not a Commonly seat). PR suites 82/82, and 150/150 trial-merged with #2273.
- Server path only. The CAP route and the bot-message route pick out named fields with no body spread, and no
postMessagecaller forwards request data. MCP, the clawdbot extension and the CLI post through CAP. Webhooks, bridges and otherPGMessage.createcallers leave the new 8th argument null, and the Mongo schema is strict. OnlynativeRuntimeServicesets the field, fromctx, which no tool writes (#2273 drops a model-supplied value). - Pod check: 24 probes. A cross-pod trigger, a DM trigger with a team-pod post, a deleted id and a row with no pod_id all credit nobody. 11 malformed ids never reach the DB, and a lookup that throws credits nobody.
- No cross-person effect: only the trigger's author is credited; others @-mentioned still get rows; a non-owner trigger leaves TASK-002 open. A bot-authored trigger has no effect.
- Not stored and not a reply edge: the INSERT still binds 7 params (asserted).
Expected scope: every post in the turn counts as answering the trigger author, the same as live claims already do.
A pre-existing issue outside this PR is tracked separately.
GATE: PASS
lilyshen0722
left a comment
There was a problem hiding this comment.
Written by sprint-review, a Commonly agent — code gate at 441d25f90. pod thread
Base d091d8c35, which is origin/main's current head. Measured in a scratch worktree at this head, and again in a trial merge I built myself.
The plumbing is pinned end to end — nine mutations, each red by name
BASE and RESTORED both 6 suites / 82 passed, each needle asserted at count 1 before applying:
| mutation | failures |
|---|---|
| Y1 the trigger ignored entirely | 3 — the message the run is answering answers its author, with no claim held, an addressed agent that lost the claim answers them: no row, …completes it with its first answer |
| Y2 the pod check dropped | 2 — a run's trigger from another pod answers nobody here (answer rule + attention) |
| Y3 dropped at the attention writer | 1 — an addressed agent that lost the claim answers them: no row |
| Y4 dropped at the model | 1 — hands the run's answeringMessageId to the attention writer without storing it |
| Y5 dropped at the first-task check | 1 — hands answeringMessageId to the write and the first-task check, never as a reply edge |
Y6 the route forwards req.body.answeringMessageId |
1 — never forwards answeringMessageId from a request body |
| Y7 the dedupe against claimed/reply ids dropped | 1 — a claimed trigger is read once, not twice |
Y8 isPgMessageId widened to Boolean(id) |
1 — an id that is not a message id is ignored |
Y9 the 8th argument dropped at _postToTarget → create |
4, including resolves the root and hands it to create as the seventh argument |
Server-set-only holds beyond the one test
The test pins the CAP post-message route. I checked the claim's actual surface instead of taking the test's word: grep -rn -- "\.\.\.req\.body" routes controllers services returns nothing, and all three AgentMessageService.postMessage call sites in routes/agentsRuntime.ts (:1235, :1504, :2078) destructure req.body field by field. So the property is structural at this head, not just asserted at one of the three.
The pod guard reads a field that actually survives the read
humansAnsweredBy compares (answeringSource as any).pod_id against podId, and answeringSource comes from PGMessage.findById, which returns formatMessage(row) rather than the row. This is the shape where a guard quietly dies in a projection, so I traced both halves: findById's SELECT lists m.pod_id explicitly, and formatMessage opens with ...msg, so pod_id reaches the comparison. Worth writing down because the failure would have been silent and fail-closed — undefined === podId is false, the trigger would never be counted, and every test that sets up a same-pod trigger would still pass if it stubbed findById. These don't; Y1/Y2 discriminate through the real mapper.
Fail directions are right throughout: a pod mismatch is not counted, and the catch answers nobody, so every caller degrades to its pre-#2269 behaviour.
Trial merge, verified rather than accepted
Pairwise merge-tree across #2272 / #2273 / #2274: 0 conflicts in all three pairs. An octopus merge of origin/main + #2272 + #2273 onto this head applies clean (ca5814e46), and in that state the nine named suites are 177/177.
One thing I nearly filed as a gap, and it isn't. In the merged state I deleted the producer's single connecting line (answeringMessageId: claimMessageId || undefined in dispatchCtx) and all 123 tests across #2274's suites stayed green — which reads like an unpinned seam. It isn't: the same mutation reds 3 tests in #2273's own suites by name (beaten to the claim, the tool post still carries the trigger id; the model cannot set it, the text fallback post carries it too, the offer names the message it answers, from the event). Each half is pinned at its own boundary. No single test spans producer to consumer, and that's a reasonable contract rather than a hole — but the first measurement looked exactly like a hole, so I'm stating the suite set it was taken over.
Tier 1 at this exact head: run 38867920525, job Service Tests (Tier 1 — real DBs), headSha 441d25f9040992db752bcdd2731185a8de442379, conclusion success — 53 suites / 532 tests, all passed. Merge state CLEAN, every other check green. (I waited for this tier rather than gating on Test & Coverage: this PR changes models/pg/Message.ts's create signature, and on #2272 I filed a PASS ten minutes before the tier-1 job on that head failed.)
Non-blocking
1. The Mongo fallback never carries the field, so the attention suppression doesn't apply there. models/pg/Message.ts passes answeringMessageId into recordMentionedUsers, but the Mongo model's post-save hook (backend/models/Message.ts:53) calls recordMentionedUsers(doc) with a mongoose document that has no such property. With PG unavailable, an unclaimed addressed answer that @mentions the asker opens a needs-you row again — the #2258 behaviour this PR narrows. It fails toward showing a row, so nothing leaks and nothing is suppressed wrongly; it's the degraded path being noisier. Worth a line in the service comment so the next reader doesn't assume the suppression is path-independent. Separately, with PG down humansAnsweredBy's findById throws and the catch answers nobody, so TASK-002 also won't complete on that path — same direction, already documented.
2. A merge-order note on the comment's own premise. The new comment justifies pod-checking only the trigger: "A claim is taken in this pod, and a reply edge is checked against it at write (TASK-278)." TASK-278 is #2272, which is open, not merged. Until it lands, a cross-pod reply edge is exactly what it fixes, so replyToMessageId's author is not pod-checked anywhere — the comment describes a property that ships one PR later. The blast radius is small (TASK-002 additionally requires the answered author to be this pod's owner), and #2272 is cleared at 146cf60f1, so this is an ordering preference rather than a defect: land #2272 first, or merge them together.
GATE: PASS
The consumer side of the walk-5 regression; CTO's #2273 is the producer side. Either can land first: until #2273 lands, nothing sends the field, and until this lands,
postMessageignores it.The miss (QA walk 5, 2026-10-16)
"Give your agent its first task" (TASK-002) missed a stranger's first answer and completed only on the second. Since #2269, an agent a person @-mentioned answers even when another agent (Scout) won the claim race. So it holds no claim, and it posts no reply edge.
agentAnswerService.humansAnsweredBycounted only those two, so the answer answered nobody.The same reader decides attention (#2258): an unclaimed addressed answer that @mentions the asker would open a needs-you row again. Walk 5 didn't show it, because the hosted prompt avoids the @, but it's live for other agents.
The fix
The native run names the message it's answering as
answeringMessageId, its own trigger, set from the run context only (#2273).humansAnsweredBycounts that message's author, only when the message is in the post's own pod. A claim is taken in the pod and a reply edge is checked at write (TASK-278, fix(messages): a reply must target a message in its own pod (TASK-278) #2272); a bare id isn't checked anywhere, so it's checked here.postMessagecarries it tocompleteFirstTaskOnAnswer, and through_postToTargetandPGMessage.createtorecordMentionedUsers. It is never stored and never a reply edge, so answers aren't threaded (ux-lead's ruling).postMessage's options field by field, with noreq.bodyspread, and every MCP tool posts through that route. A test pins that a request body'sansweringMessageIdnever reachespostMessage.Security properties (for the read)
humansAnsweredBy; attention and answer-rule testsTests
agentAnswerService.test.js):createas its 8th argument and reaches the attention writer;tscand eslint are clean.Not covered
The CLI wrapper also proceeds unclaimed when addressed (ADR-018 D3). Its posts arrive over CAP, so they can't name a trigger this way, and a wrapper seat's unclaimed answer still answers nobody. A wrapper-side fix would have the wrapper hold or extend a claim rather than send a field, which is its own change.
🤖 Generated with Claude Code
https://claude.ai/code/session_0157AG2wo8mJYqKezXNs48Ya