fix(agents): a suppressed runtime failure must not conclude an agent-dm

[lilyshen0722]

Found by @sprint-review while checking whether rule 19's audience clause was over-fitted to one site. It is not — this is the second instance and the worse one, because here the direction is wrong too, not just the audience.

The chain

runtime model chain exhausted / tool note relayed
  -> isRuntimeModelFailure | isRuntimeToolFailureNote  (:949, :957)
  -> sanitizedContent = ''   + console.warn            operator-only
  -> !sanitizedContent path                            (:1124)
       -> recordAgentDmConclusion  fires               ADR-012 §4
       -> return { success: true, skipped: true, reason: 'silent_or_empty' }

Two ways to reach '' and they mean opposite things. An intentional NO_REPLY is the agent deciding the exchange is over. A suppressed runtime failure is the agent never having produced a reply at all.

Collapsing them cost more than a wrong label. The empty path fires ADR-012 §4's recordAgentDmConclusion, which writes a system_exchanges entry for both peers whose takeaway is the sender's preceding message. So a model-chain exhaustion wrote "this conversation concluded" into the record, attributing a takeaway the agent never reached. The caller got the same silent_or_empty a deliberate silence returns, and the only honest trace was a console.warn — a surface no agent can read.

That is the rule-19 shape twice over: the guard fails toward the silent outcome, and its one signal is addressed to an audience that is not the party owed the outcome.

The fix

Track why the content emptied, then discriminate at the empty path:

arrival	concludes?	reason
bare NO_REPLY	yes	silent_or_empty
whitespace / no content	yes	silent_or_empty
model chain exhausted	no	runtime_model_failure_suppressed
tool-failure note	no	runtime_tool_failure_suppressed

Still success: true, skipped: true — the caller did nothing wrong, and a 500 here would turn a degraded model chain into a broken endpoint. Both existing silent_or_empty assertions (clawdbot-e2e.test.js:732, :748) are genuine silences and keep the old value; no assertion inverted.

Evidence

The CONTROL is the half that matters: a bare NO_REPLY and whitespace-only content must still conclude, or the fix has traded one collapse for another.

Mutation-checked per rule 1 — reverting just the discrimination branch reddens 4 of 7, and both CONTROL cases stay green:

✕ model chain exhausted does not fire the agent-dm conclusion trigger
✕ failover summary does not fire the agent-dm conclusion trigger
✕ tool-status note does not fire the agent-dm conclusion trigger
✕ and says which failure it was, not "silent"
✓ logs the suppressed content for the operator, as before
✓ a bare NO_REPLY fires the trigger and reports silent_or_empty
✓ and so does genuinely empty content

14 green with the chatNoise sibling suite.

[lilyshen0722]

The exemplar is in the same function, 160 lines down — @sprint-review.

:1112, the phantom-upload guard:

sanitizedContent += `\n\n⚠️ _(system note: ... no matching attachment was found ...)_`;
console.warn(`[agent-msg] phantom-upload-directive from agent=... `);

Both audiences, deliberately. The runtime-failure suppression at :949/:957 warns only. Same file, same function, both answers to who hears a failure, and nothing pointing between them — rule 19's five-independent-coin-flips observation landing inside a single function rather than across five files.

The asymmetry is defensible and should not be "fixed" by symmetry. The phantom guard has a message to annotate; the suppression would have to manufacture a post, which is exactly the ~30-minute-per-pod flood it exists to stop. So the answer differs for a real reason. What was never defensible is that the reason lived nowhere: the second author could not see the first had already decided this question. Added a pointer in both directions recording the difference and its grounds.

Left undecided, flagged rather than shipped. This fix stops a suppressed failure from writing a false conclusion, but the peer in an agent-dm still just gets nothing — and there, unlike a heartbeat pod, someone is explicitly waiting on a reply they asked for. A scoped version of the exemplar would be: post a minimal visible note only when the suppressed message was answering a direct wake, and stay silent on heartbeat-driven posts. That changes what agents observe about each other, so it is a kernel-surface call, not a drive-by. Not in this PR.

93 tests green across the agentMessageService suites.

[lilyshen0722]

Verified the whole chain at d331b16d, including the part I only inferred when I raised it.

recordAgentDmConclusion (systemExchangeTriggers.ts:173) does what the PR body says: gated on podIsAgentDm, then "Write to BOTH peers", with the speaker's takeaway derived from findPreviousNonSilentMessage and the listener getting a @peer:-prefixed copy. So an exhausted model chain really was writing a conclusion entry into both agents' system_exchanges with a takeaway the agent never reached. The fix is correct and minimal.

Reproduced the mutation claim exactly: baseline 7 passed; deleting just the if (suppressedFailure) branch reddens 4, and both CONTROLs (a bare NO_REPLY fires the trigger, and so does genuinely empty content) stay green. The CONTROLs are the right load-bearing half.

One gap: there is a third arrival at '' and this fix does not reach it. sanitizeAgentContent returns '' for a bare runtime artifact at :1862 — BARE_RUNTIME_ARTIFACTS = new Set(['RGCTX']), whose own comment describes it as "a wrapper artifact ... runtime noise" added "only after observing it as a wrapper artifact in production". That path leaves suppressedFailure null, so it falls through to the ADR-012 §4 trigger. Measured at this head, against the PR's own harness:

RGCTX     reason=silent_or_empty  concludes=true
NO_REPLY  reason=silent_or_empty  concludes=true

Byte-identical. The same laundering the PR fixes, one branch over, inside the function the PR edits.

I am not certain it is a defect, and the uncertainty is the argument. TASK-042 describes the wrapper as having emitted RGCTX "instead of a silent reply", which reads as malformed silence — in which case concluding is right. But nothing in the artifact distinguishes "the agent chose silence and the wrapper mangled it" from "the wrapper ate a real reply", and that is precisely the ambiguity this PR argues must not be resolved in favour of a positive semantic event.

Either resolution is fine by me:

1. Extend suppressedFailure to the artifact floor (reason: 'runtime_artifact_suppressed'), or
2. Leave the behaviour and state in the new comment why the artifact floor is a decision while the other two are failures.

What should not ship is the third case being unmentioned next to a comment that says there are two ways to arrive here — a future reader will take that enumeration as complete. Same class as the "two readers only" fix on #1219 an hour ago.

Everything else here is right, and the reason-string split is the correct shape. Not blocking.

[lilyshen0722]

Follow-up on the item flagged-not-acted in the PR body ("emit a minimal visible note only when the suppressed message was answering a direct wake"). Two things worth pinning before that becomes a design ticket, both checked at origin/main 8a674ac3.

1. "Answering a direct wake" is not decidable at :949/:957. PostMessageOptions is:

agentName, podId, content, metadata, messageType, payload,
instanceId, displayName, replyToMessageId, threadRootId, installationConfig

There is no event id, mention id, or wake provenance in it. And replyToMessageId is not a usable proxy — #1226 establishes the opposite: an in-thread post deliberately carries no reply edge, which is the whole reason threadRootId exists as a separate field. So the wake-scoped version needs a new field threaded from the wake path (agentEventService / agentMentionService) through all ten non-test callers of postMessage, not a local change.

2. The pod-type-scoped version is already free, and the premise is already in the tree. recordAgentDmConclusion's hot-path gate carries the heartbeat/waiting split as a load-bearing comment:

Hot-path gate: bail before User.find for non-DM pods. Heartbeats in team pods regularly emit NO_REPLY-only posts; this short-circuit keeps that path cheap.

That is the same distinction the flagged item wants ("unlike a heartbeat pod, someone is explicitly waiting"), already written down and already relied on. The empty path can call podIsAgentDm exactly as the conclusion trigger does.

So the two framings are very different sizes of change, and the PR body currently reads as one item. Worth splitting: dm-scoped visible note is small and local; wake-scoped visible note is a kernel-surface change with new plumbing. My read is the first is what the flag actually wants.

Also confirming the suite claim: 93 passed across 10 agentMessageService suites at d331b16d.

And a note on the asymmetry defence in the PR body — it is stronger than "defensible", it is structural. :1112 is sanitizedContent += ..., so the phantom guard can annotate only because a message survived. The suppression path emptied the string and has nothing to append to. The two answers could not have matched, whatever the second author preferred. Worth saying that way in the pointer comment, since "defensible" invites someone to re-litigate it and "structurally impossible" does not.

[lilyshen0722]

Gate: PASS at 8fd4b3d0a — re-gated after six days stalled; previous gate was d331b16d.

The only delta since that gate is one docs-only commit, +15 lines of comment in agentMessageService.ts and no behaviour change. I verified its claims rather than waving it through, since a comment that describes a guard is exactly the kind of assertion nothing else in the tree checks.

Both hold at this head:

• "This pair warns only." Correct. The suppression path does console.warn at the two classification sites and then :1162 returns { success: true, skipped: true, reason: 'runtime_<kind>_suppressed' }. Nothing is posted, and nothing is appended to sanitizedContent.
• "it appends a VISIBLE system note to the message and warns." Correct — :1128 appends the ⚠️ system note to sanitizedContent, :1131 warns. Both audiences, as stated.

The asymmetry the comment defends is real and the defence is the right one: the phantom guard has a message to annotate, the suppression path would have to manufacture one, and manufacturing one is the flood the suppression exists to prevent. Recording why two adjacent guards answer the same question differently is worth more than the fifteen lines it costs — a later reader reconciling them by symmetry is precisely the failure mode, and now they can't do it accidentally.

One non-blocking note, offered because it is the same genus as the finding this comment came from. "the phantom-upload guard ~160 lines down" is a line-offset locator; the real distance at this head is ~175 lines and it moves with every edit between the two. A symbol name (phantoms.length > 0, or the phantom-upload-directive warn tag, which is greppable and unique) survives edits that a line count does not. Same reasoning as #1385 — the locator that still resolves after the file changes is the one worth writing. Not worth another push on its own; fold it in if this branch moves again.

Nothing else changed. The chain I verified at d331b16d is unaffected.