fix(agents): stop dropping an agent's reply when another agent posts first (#757)

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

@@ -0,0 +1,194 @@
+/**
+ * Regression tests for the "who posted?" family of bugs (#757).
+ *
+ * The BYO wrapper decides whether to echo its CLI output by asking the server
+ * for recent pod messages and looking for one it believes it wrote. Three
+ * separate defects made that answer wrong:
+ *
+ *  1. The wrapper could only test `isBot`, i.e. "did ANY bot post?", so in a
+ *     multi-agent pod whichever agent finished second had its whole reply
+ *     silently dropped. The fix is a server-computed `self` flag — the client
+ *     cannot derive its own bot username safely (LEGACY_AGENT_MAP is
+ *     server-only and instanceId is owner-scoped at install).
+ *  2. The Mongo fallback path populated `userId` WITHOUT `isBot`, so every
+ *     message on that path reported `isBot: false` and self-post detection was
+ *     dead outright — guaranteed double-posting whenever PG was unavailable.
+ *  3. The PG path fabricated `isBot` from a username substring test, so a
+ *     human named "talbot" read as a bot and suppressed an agent's reply.
+ *
+ * Plus the sibling in hasRecentDuplicateUrls: it matched ANY author, so a
+ * human posting a link silenced the agent's next heartbeat post.
+ */
+
+const AgentMessageService = require('../../../services/agentMessageService');
+const Message = require('../../../models/Message');
+const PGMessage = require('../../../models/pg/Message');
+
+jest.mock('../../../models/Message');
+jest.mock('../../../models/pg/Message', () => ({ findByPodId: jest.fn() }));
+
+const POD = 'pod-1';
+const SELF = 'agent-user-1';
+const OTHER = 'agent-user-2';
+
+// Mongo path: Message.find(...).sort(...).limit(...).populate(...).lean()
+const mockMongoChain = (rows) => {
+  const populate = jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue(rows) });
+  const limit = jest.fn().mockReturnValue({ populate });
+  const sort = jest.fn().mockReturnValue({ limit });
+  Message.find.mockReturnValue({ sort });
+  return { populate };
+};
+
+describe('getRecentMessages — PG path', () => {
+  const origPgHost = process.env.PG_HOST;
+  beforeEach(() => {
+    jest.clearAllMocks();
+    process.env.PG_HOST = 'localhost';
+  });
+  afterAll(() => {
+    if (origPgHost === undefined) delete process.env.PG_HOST;
+    else process.env.PG_HOST = origPgHost;
+  });
+
+  test('stamps an explicit `self` boolean on every message when the caller identifies itself', async () => {
+    PGMessage.findByPodId.mockResolvedValue([
+      {
+        id: 1, user_id: SELF, username: 'me-bot', is_bot: true, content: 'mine', 
+      },
+      {
+        id: 2, user_id: OTHER, username: 'other-bot', is_bot: true, content: 'theirs', 
+      },
+    ]);
+
+    const out = await AgentMessageService.getRecentMessages(POD, 10, SELF);
+
+    expect(out.map((m) => m.self)).toEqual([true, false]);
+  });
+
+  // Its absence is how a client detects a server too old to compute `self`.
+  test('omits `self` when no caller identity is given', async () => {
+    PGMessage.findByPodId.mockResolvedValue([
+      {
+        id: 1, user_id: SELF, username: 'me-bot', is_bot: true, content: 'mine', 
+      },
+    ]);
+
+    const out = await AgentMessageService.getRecentMessages(POD, 10);
+
+    expect(out[0]).not.toHaveProperty('self');
+  });
+
+  test('a human whose name merely ENDS IN "bot" is not reported as a bot', async () => {
+    // The old substring heuristic ('-bot' / endsWith 'bot' / 'openclaw-')
+    // misread these as bots, and isBot gates the wrapper's echo suppression.
+    PGMessage.findByPodId.mockResolvedValue([
+      {
+        id: 1, user_id: 'u1', username: 'talbot', is_bot: false, content: 'hi', 
+      },
+      {
+        id: 2, user_id: 'u2', username: 'abbot', is_bot: false, content: 'hi', 
+      },
+      {
+        id: 3, user_id: 'u3', username: 'openclaw-aria', is_bot: false, content: 'hi', 
+      },
+      {
+        id: 4, user_id: 'u4', username: 'real-bot', is_bot: true, content: 'hi', 
+      },
+    ]);
+
+    const out = await AgentMessageService.getRecentMessages(POD, 10);
+
+    expect(out.map((m) => m.isBot)).toEqual([false, false, false, true]);
+  });
+});
+
+describe('getRecentMessages — Mongo fallback path', () => {
+  const origPgHost = process.env.PG_HOST;
+  beforeEach(() => {
+    jest.clearAllMocks();
+    delete process.env.PG_HOST; // force the Mongo branch
+  });
+  afterAll(() => {
+    if (origPgHost === undefined) delete process.env.PG_HOST;
+    else process.env.PG_HOST = origPgHost;
+  });
+
+  // Without isBot in the projection, detection is dead and every turn double-posts.
+  test('selects isBot in the populate projection', async () => {
+    const { populate } = mockMongoChain([]);
+
+    await AgentMessageService.getRecentMessages(POD, 10);
+
+    expect(populate).toHaveBeenCalledWith('userId', expect.stringContaining('isBot'));
+  });
+
+  test('reports isBot truthfully for a bot author', async () => {
+    mockMongoChain([
+      { _id: 'm1', content: 'x', userId: { _id: SELF, username: 'me-bot', isBot: true } },
+    ]);
+
+    const out = await AgentMessageService.getRecentMessages(POD, 10);
+
+    expect(out[0].isBot).toBe(true);
+  });
+
+  test('stamps `self` on the Mongo path too', async () => {
+    mockMongoChain([
+      { _id: 'm1', content: 'mine', userId: { _id: SELF, username: 'me-bot', isBot: true } },
+      { _id: 'm2', content: 'theirs', userId: { _id: OTHER, username: 'other-bot', isBot: true } },
+    ]);
+
+    const out = await AgentMessageService.getRecentMessages(POD, 10, SELF);
+
+    // .reverse() is applied on this path, so assert by author rather than order.
+    const mine = out.find((m) => String(m.userId._id) === SELF);
+    const theirs = out.find((m) => String(m.userId._id) === OTHER);
+    expect(mine.self).toBe(true);
+    expect(theirs.self).toBe(false);
+  });
+});
+
+describe('hasRecentDuplicateUrls — author scoping', () => {
+  const origPgHost = process.env.PG_HOST;
+  beforeEach(() => {
+    jest.clearAllMocks();
+    process.env.PG_HOST = 'localhost';
+  });
+  afterAll(() => {
+    if (origPgHost === undefined) delete process.env.PG_HOST;
+    else process.env.PG_HOST = origPgHost;
+  });
+
+  test('a link posted by SOMEONE ELSE no longer suppresses this author', async () => {
+    PGMessage.findByPodId.mockResolvedValue([
+      {
+        id: 1, user_id: OTHER, username: 'human', is_bot: false, content: 'look: https://x.com/a', 
+      },
+    ]);
+
+    const dup = await AgentMessageService.hasRecentDuplicateUrls({
+      podId: POD,
+      content: 'https://x.com/a',
+      authorUserId: SELF,
+    });
+
+    expect(dup).toBe(false);
+  });
+
+  test("the author's OWN recent duplicate is still caught", async () => {
+    PGMessage.findByPodId.mockResolvedValue([
+      {
+        id: 1, user_id: SELF, username: 'me-bot', is_bot: true, content: 'look: https://x.com/a', 
+      },
+    ]);
+
+    const dup = await AgentMessageService.hasRecentDuplicateUrls({
+      podId: POD,
+      content: 'https://x.com/a',
+      authorUserId: SELF,
+    });
+
+    expect(dup).toBe(true);
+  });
+});

backend/routes/agentsRuntime.ts

@@ -1103,7 +1103,8 @@ router.get(
       }
 
       const limit = Math.min(Math.max(parseInt(req.query.limit, 10) || 20, 1), 50);
-      const messages = await AgentMessageService.getRecentMessages(podId, limit);
+      // Same server-computed `self` flag as the runtime-token route (#757).
+      const messages = await AgentMessageService.getRecentMessages(podId, limit, user._id);
       return res.json({ messages });
     } catch (error: any) {
       console.error('Error fetching bot messages:', error);
@@ -1316,7 +1317,13 @@ router.get('/pods/:podId/messages', agentRuntimeAuth, async (req: any, res: any)
     }
 
     const limit = Math.min(Math.max(parseInt(req.query.limit, 10) || 20, 1), 50);
-    const messages = await AgentMessageService.getRecentMessages(podId, limit);
+    // Pass the caller's own user id so each message carries a server-computed
+    // `self` flag. The wrapper uses it to tell "I posted via my tool" from
+    // "some OTHER agent posted while I was thinking" — the latter used to
+    // silently swallow this agent's reply in any multi-agent pod (#757).
+    const messages = await AgentMessageService.getRecentMessages(
+      podId, limit, req.agentUser?._id,
+    );
 
     return res.json({ messages });
   } catch (error: any) {

backend/services/agentMessageService.ts

@@ -146,6 +146,11 @@ interface MessageNormalized {
   userId: { _id: unknown; username: string; profilePicture?: string };
   username: string;
   isBot?: boolean;
+  // Present ONLY when the caller identified itself via `selfUserId`. An
+  // explicit boolean (never undefined) so a client can tell "this server
+  // computes self" apart from "this server is too old to know" — see
+  // getRecentMessages.
+  self?: boolean;
   createdAt: Date | string;
   metadata?: MetadataDoc;
   profile_picture?: string;
@@ -441,18 +446,35 @@ class AgentMessageService {
     });
   }
 
+  /**
+   * "Has this author already shared these exact links recently?" — used to stop
+   * a heartbeat agent re-posting the same digest every tick.
+   *
+   * `authorUserId` scopes the lookback to that author's own messages. Without
+   * it the check matched ANY author, so a human dropping a link into the pod
+   * suppressed the agent's next post — the agent was silenced for someone
+   * else's message (same class of bug as #757).
+   */
   static async hasRecentDuplicateUrls(options: {
     podId: unknown;
     content: unknown;
     lookback?: number;
+    authorUserId?: unknown;
   }): Promise<boolean> {
-    const { podId, content, lookback = 10 } = options;
+    const {
+      podId, content, lookback = 10, authorUserId,
+    } = options;
     const urls = AgentMessageService.extractUrls(content);
     if (urls.length === 0) return false;
     try {
       const recent = await AgentMessageService.getRecentMessages(podId, lookback);
       if (!Array.isArray(recent) || recent.length === 0) return false;
-      const recentText = recent.map((m) => String(m?.content || '')).join('\n');
+      const authorId = authorUserId ? String(authorUserId) : null;
+      const mine = authorId
+        ? recent.filter((m) => String(m?.userId?._id ?? '') === authorId)
+        : recent;
+      if (mine.length === 0) return false;
+      const recentText = mine.map((m) => String(m?.content || '')).join('\n');
       const recentUrls = new Set(AgentMessageService.extractUrls(recentText));
       return urls.every((url) => recentUrls.has(url));
     } catch {
@@ -1079,7 +1101,11 @@ class AgentMessageService {
     }
 
     if (isHeartbeatEvent) {
-      const isDuplicate = await AgentMessageService.hasRecentDuplicateUrls({ podId, content: sanitizedContent });
+      const isDuplicate = await AgentMessageService.hasRecentDuplicateUrls({
+        podId,
+        content: sanitizedContent,
+        authorUserId: agentUser?._id,
+      });
       if (isDuplicate) {
         AgentMessageService.logMessageLifecycle('skipped', {
           agentName,
@@ -1524,11 +1550,32 @@ class AgentMessageService {
     return cleaned;
   }
 
-  static async getRecentMessages(podId: unknown, limit = 20): Promise<MessageNormalized[]> {
+  /**
+   * Recent pod messages, normalized across the PG and Mongo read paths.
+   *
+   * `selfUserId` — when the caller is a participant (an agent runtime asking
+   * "did *I* post during my turn?"), pass its user id and every returned
+   * message carries an explicit `self` boolean. Answering that server-side is
+   * deliberate: the client cannot safely derive its own bot username, because
+   * `resolveAgentType`'s LEGACY_AGENT_MAP is server-only and `instanceId` is
+   * owner-scoped and assigned at install. A client that guesses wrong silently
+   * double-posts. Omit the argument and no `self` key is emitted at all, which
+   * is how a client detects an older server (see #757).
+   */
+  static async getRecentMessages(
+    podId: unknown,
+    limit = 20,
+    selfUserId?: unknown,
+  ): Promise<MessageNormalized[]> {
     if (!podId) {
       throw new Error('podId is required');
     }
 
+    const selfId = selfUserId ? String(selfUserId) : null;
+    const withSelf = (authorId: unknown): { self?: boolean } => (
+      selfId ? { self: String(authorId ?? '') === selfId } : {}
+    );
+
     if (PGMessage && process.env.PG_HOST) {
       try {
         const messages: Array<Record<string, unknown>> = await (PGMessage as {
@@ -1537,18 +1584,12 @@ class AgentMessageService {
 
         return messages.map((msg) => {
           const username = (msg.username as string) || 'Unknown';
-          const isBot =
-            msg.is_bot === true
-            || msg.is_bot === 'true'
-            || (() => {
-              const lower = username.toLowerCase();
-              return (
-                lower.includes('-bot')
-                || lower.includes('_bot')
-                || lower.endsWith('bot')
-                || lower.startsWith('openclaw-')
-              );
-            })();
+          // Trust the mirrored `is_bot` column only. This used to fall back to
+          // a username substring test ('-bot', endsWith 'bot', 'openclaw-'),
+          // which misclassified any human named e.g. "talbot" or "abbot" as a
+          // bot — and `isBot` gates the wrapper's echo suppression, so a false
+          // positive silently ate that agent's reply.
+          const isBot = msg.is_bot === true || msg.is_bot === 'true';
           return {
             _id: msg.id,
             id: msg.id,
@@ -1561,6 +1602,7 @@ class AgentMessageService {
             },
             username,
             isBot,
+            ...withSelf(msg.user_id),
             createdAt: msg.created_at as Date,
           };
         });
@@ -1572,7 +1614,11 @@ class AgentMessageService {
     const messages: Array<Record<string, unknown>> = await Message.find({ podId })
       .sort({ createdAt: -1 })
       .limit(limit)
-      .populate('userId', 'username profilePicture')
+      // `isBot` MUST stay in this projection: it is read below, and omitting it
+      // made every message on this fallback path report isBot:false, which
+      // disabled the wrapper's self-post detection outright (guaranteed
+      // double-posting whenever PG was unavailable).
+      .populate('userId', 'username profilePicture isBot')
       .lean();
 
     return messages.reverse().map((msg) => {
@@ -1586,6 +1632,7 @@ class AgentMessageService {
         userId: userId || { username: 'Unknown' },
         username,
         isBot: userId?.isBot === true,
+        ...withSelf(userId?._id),
         createdAt: msg.createdAt as Date,
       };
     }) as MessageNormalized[];