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

backend/__tests__/unit/services/agentMessageService.suppressedFailureIsNotAConclusion.test.js

@@ -0,0 +1,83 @@
+/**
+ * A suppressed runtime failure must not conclude an agent-dm.
+ *
+ * @sprint-review, 2026-08-25: `agentMessageService` empties `sanitizedContent`
+ * for a runtime model-failure (:949) and a tool-failure note (:957), and the
+ * empty path then returns the SAME `silent_or_empty` an intentional NO_REPLY
+ * returns. The label was the small half.
+ *
+ * The large half is that the empty path also 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
+ * being exhausted wrote "this conversation concluded" into the record, with a
+ * takeaway the agent never reached — a failure laundered into a positive
+ * semantic event, its only honest trace a `console.warn` no agent can read.
+ *
+ * These pin the discrimination in both directions. The NO_REPLY case is the
+ * control: it must still conclude, or the fix has traded one collapse for
+ * another.
+ */
+const mockRecordAgentDmConclusion = jest.fn().mockResolvedValue(undefined);
+jest.mock('../../../services/systemExchangeTriggers', () => ({
+  recordAgentDmConclusion: (...args) => mockRecordAgentDmConclusion(...args),
+}));
+
+const AgentMessageService = require('../../../services/agentMessageService');
+
+const POD = '69ef02b036b742e2e2c0c4af';
+const post = (content) => AgentMessageService.postMessage({
+  agentName: 'openclaw', instanceId: 'nova', podId: POD, content,
+});
+
+beforeEach(() => {
+  mockRecordAgentDmConclusion.mockClear();
+  jest.spyOn(console, 'warn').mockImplementation(() => {});
+});
+afterEach(() => jest.restoreAllMocks());
+
+describe('a failure concludes nothing', () => {
+  it.each([
+    ['model chain exhausted', '⚠️ Agent failed before reply: All models failed (4): openrouter/x: 401'],
+    ['failover summary', 'All models failed (3): openrouter/a: 429'],
+    ['tool-status note', '⚠️ 📝 Edit: in /workspace/MEMORY.md failed'],
+  ])('%s does not fire the agent-dm conclusion trigger', async (_label, content) => {
+    await post(content);
+    expect(mockRecordAgentDmConclusion).not.toHaveBeenCalled();
+  });
+
+  it('and says which failure it was, not "silent"', async () => {
+    const model = await post('All models failed (3): openrouter/a: 429');
+    const tool = await post('⚠️ 📝 Edit: in /workspace/MEMORY.md failed');
+    expect(model.reason).toBe('runtime_model_failure_suppressed');
+    expect(tool.reason).toBe('runtime_tool_failure_suppressed');
+    // Still skipped, still not an error — the caller did nothing wrong and a
+    // 500 here would turn a degraded model chain into a broken endpoint.
+    expect(model).toMatchObject({ success: true, skipped: true });
+    expect(tool).toMatchObject({ success: true, skipped: true });
+  });
+
+  it('logs the suppressed content for the operator, as before', async () => {
+    await post('All models failed (3): openrouter/a: 429');
+    const warned = console.warn.mock.calls.map((c) => String(c[0])).join('\n');
+    expect(warned).toContain('suppressed runtime model-failure');
+    expect(warned).toContain('All models failed');
+  });
+});
+
+describe('CONTROL: an intentional silence still concludes', () => {
+  // Without this the suite is equally consistent with a fix that simply
+  // stopped calling the trigger, which would break ADR-012 §4 outright.
+  it('a bare NO_REPLY fires the trigger and reports silent_or_empty', async () => {
+    const res = await post('NO_REPLY');
+    expect(mockRecordAgentDmConclusion).toHaveBeenCalledWith({
+      podId: POD, senderAgentName: 'openclaw', senderInstanceId: 'nova',
+    });
+    expect(res.reason).toBe('silent_or_empty');
+  });
+
+  it('and so does genuinely empty content', async () => {
+    const res = await post('   ');
+    expect(mockRecordAgentDmConclusion).toHaveBeenCalled();
+    expect(res.reason).toBe('silent_or_empty');
+  });
+});

backend/services/agentMessageService.ts

@@ -946,9 +946,25 @@ class AgentMessageService {
     // spam every pod the agent heartbeats in (~every 30 min). Treat them like a
     // silent reply (empty → skipped below) and log instead, so a degraded
     // community agent fails quietly rather than flooding chat.
+    // WHY the content emptied, kept because the empty path below cannot
+    // otherwise tell a failure from a decision. Both arrive as `''`.
+    //
+    // Read alongside the phantom-upload guard ~160 lines down, which faces the
+    // same question and answers it the other way: it appends a VISIBLE system
+    // note to the message and warns, so the pod and the operator both learn.
+    // This pair warns only. The asymmetry is defensible and is NOT a bug to
+    // "fix" by symmetry — that guard has a message to annotate, and this one
+    // would have to manufacture a post, which is the ~30-minute-per-pod flood
+    // the suppression exists to stop. What was never defensible was the two
+    // guards being written independently in one function with nothing pointing
+    // at each other, so the second author could not see the first had already
+    // decided who hears a failure. (@sprint-review, 2026-08-25.)
+    let suppressedFailure: 'model_failure' | 'tool_failure' | null = null;
+
     if (sanitizedContent && AgentMessageService.isRuntimeModelFailure(sanitizedContent)) {
       console.warn(`[agent-msg] suppressed runtime model-failure from agent=${agentName} instance=${instanceId} pod=${podId}: ${sanitizedContent.slice(0, 120)}`);
       sanitizedContent = '';
+      suppressedFailure = 'model_failure';
     }
 
     // Same treatment for gateway tool-status failure notes ("⚠️ 📝 Edit: ...
@@ -957,6 +973,7 @@ class AgentMessageService {
     if (sanitizedContent && AgentMessageService.isRuntimeToolFailureNote(sanitizedContent)) {
       console.warn(`[agent-msg] suppressed runtime tool-failure note from agent=${agentName} instance=${instanceId} pod=${podId}: ${sanitizedContent.slice(0, 120)}`);
       sanitizedContent = '';
+      suppressedFailure = 'tool_failure';
     }
 
     // Task #68: detect false-attachment claims. Agents sometimes post
@@ -1110,6 +1127,10 @@ class AgentMessageService {
             if (phantoms.length > 0) {
               const phantomList = phantoms.map((n) => `\`${n}\``).join(', ');
               sanitizedContent += `\n\n⚠️ _(system note: this message references ${phantoms.length === 1 ? 'an upload directive' : 'upload directives'} for ${phantomList} but no matching attachment was found in this pod. The agent may have typed the directive without actually calling \`commonly_attach_file\`. Check the agent's workspace.)_`;
+              // Both audiences on purpose: the note tells the pod, the warn
+              // tells the operator. The runtime-failure suppression at the top
+              // of this function deliberately does only the second — see the
+              // note there for why the two differ and why that is not drift.
               console.warn(`[agent-msg] phantom-upload-directive from agent=${agentName} instance=${instanceId} pod=${podId} — names=${JSON.stringify(phantoms)}`);
             }
           }
@@ -1122,6 +1143,26 @@ class AgentMessageService {
     }
 
     if (!sanitizedContent) {
+      // Two ways to arrive here 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 —
+      // the model chain was exhausted, or a tool note was relayed as prose.
+      //
+      // Collapsing them cost more than a wrong label. ADR-012 §4 fires here:
+      // in an agent-dm both peers get a `system_exchanges` conclusion entry
+      // whose takeaway is the SENDER's PRECEDING message. So a model-chain
+      // exhaustion used to write "this conversation concluded" into the
+      // record, for both peers, attributing a takeaway the agent never
+      // reached — a failure laundered into a positive semantic event. The
+      // caller meanwhile got the same `silent_or_empty` an intentional
+      // NO_REPLY returns, and the only trace of the failure was a
+      // `console.warn` no agent can read. (@sprint-review, 2026-08-25.)
+      //
+      // So: a failure concludes nothing, and says which failure it was.
+      if (suppressedFailure) {
+        return { success: true, skipped: true, reason: `runtime_${suppressedFailure}_suppressed` };
+      }
+
       // ADR-012 §4: agent-dm-conclusion trigger. The reply is silent
       // (NO_REPLY swallowed by sanitizeAgentContent). If the pod is an
       // agent-dm, both peers get a system_exchanges entry whose takeaway is