Skip to content

fix(answers): an addressed agent that lost the claim still answers the person who asked - #2274

Merged
lilyshen0722 merged 1 commit into
mainfrom
growth/addressed-answer-credit
Oct 16, 2026
Merged

lilyshen0722 merged 1 commit into
mainfrom
growth/addressed-answer-credit

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

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, postMessage ignores 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.humansAnsweredBy counted 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).

  • 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, 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.
  • postMessage carries it to completeFirstTaskOnAnswer, and through _postToTarget and PGMessage.create to recordMentionedUsers. It is never stored and never a reply edge, so answers aren't threaded (ux-lead's ruling).
  • It's server-set only. The CAP route builds postMessage's options field by field, with no req.body spread, and every MCP tool posts through that route. A test pins that a request body's answeringMessageId never reaches postMessage.

Security properties (for the read)

property where
honoured only from the native runtime's server path CAP route test; #2273 ignores model tool args
a message in another pod (or another person's DM, which is another pod) can't complete a task or suppress a row the pod check in humansAnsweredBy; attention and answer-rule tests
a forged id can't close someone else's TASK-002 not forgeable (server-set); and TASK-002 needs the answered author to be this pod's owner, from a message in this pod

Tests

  • Answer rule (new agentAnswerService.test.js):
    • the trigger is counted with no claim held;
    • a trigger from another pod is refused;
    • a claimed trigger is read once;
    • a non-message id is ignored.
  • TASK-002 with the real rule: an addressed agent that lost the claim completes the card with its first answer; without the trigger it doesn't (the walk-5 miss).
  • Attention: that answer opens no row, and a trigger from another pod does open one.
  • Plumbing:
    • the field reaches create as its 8th argument and reaches the attention writer;
    • the INSERT still carries 7 parameters;
    • the CAP body is refused.
  • Mutations, each turning a test red:
    • the trigger ignored;
    • the pod check dropped;
    • the field dropped at attention, at the model, at the first-task check, or forwarded by the route.
  • Ran locally on Node 22: 39 suites, 415/415 (agent message, attention, starter task, PG message, runtime routes, answer rule, activity queue, message controller). tsc and eslint are clean.
  • Trial merge of main + fix(messages): a reply must target a message in its own pod (TASK-278) #2272 + feat(native): every post names the message the turn answers; hosted prompt says it cannot change a task's status #2273 + this: clean, and 118/118 on the answer, attention, starter-task, PG-message, thread-writes, runtime-route and native-claims suites.

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

…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
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Second read of the consumer side at 441d25f9 (CTO, producer side is #2273). This is not a gate; there's no verdict line on purpose.

Checked by reading

  • Pod check reads a real value. humansAnsweredBy compares answeringSource.pod_id with podId. PGMessage.findById selects m.pod_id, and formatMessage spreads the raw row before adding its camelCase fields, so pod_id is present on the formatted result.
  • Threaded explicitly and never stored. postMessage → _postToTarget → PGMessage.create's 8th argument → recordMentionedUsers, and separately → completeFirstTaskOnAnswer. No spread anywhere, it's not a column, and it's not passed to the INSERT.
  • Id handling. The id is digit-checked like the others, and deduped against claimed and reply ids before the extra lookup. A failure still answers nobody.
  • CAP route. The new route arm pins that a body answeringMessageId is dropped.

Checked by running a trial merge of main + #2272 (146cf60f) + #2273 (5dd2915a) + #2274 (441d25f9). It merges cleanly (Message.ts and agentMessageService.ts auto-merge).

  • Tier 1 against real Postgres 16 and Mongo 7: threading.*, pgMessages, messages, messageClaimHandoff, agentsRuntime.* — 75/75. This matters because fix(messages): a reply must target a message in its own pod (TASK-278) #2272 changed the same create()'s SQL, and pg-mem would not have shown an interaction (checklist rule 55).
  • Unit: models, controllers, agentMessageService, agentAnswerService, attentionItemService, starterTask, nativeRuntime, thread, runtime and hosted routes — 1149/1149 across 114 suites.
  • Type-check: the 50 pre-existing errors in standalone backend scripts, none new.

Nothing blocking from me.

🤖 Generated with Claude Code

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 postMessage caller forwards request data. MCP, the clawdbot extension and the CLI post through CAP. Webhooks, bridges and other PGMessage.create callers leave the new 8th argument null, and the Mongo schema is strict. Only nativeRuntimeService sets the field, from ctx, 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 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@lilyshen0722
lilyshen0722 added this pull request to the merge queue Oct 16, 2026
Merged via the queue into main with commit 95f7d4e Oct 16, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant