From 57248a7ca120b1083005f37a833ffebcddbd45d8 Mon Sep 17 00:00:00 2001
From: Tyler Yust <64381258+tyler6204@users.noreply.github.com>
Date: Fri, 30 Jan 2026 15:52:19 -0800
Subject: [PATCH] fix: prefer requesterOrigin over stale session entry in
 subagent announce routing (#4957)

* fix: prefer requesterOrigin over stale session entry in subagent announce routing

When a subagent finishes and announces results back, resolveAnnounceOrigin
merged the session entry (primary) with requesterOrigin (fallback). If the
session store had a stale lastChannel (e.g. whatsapp) from a previous
interaction but the user was now on a different channel (e.g. bluebubbles),
the announce would route to the wrong channel.

Swap the merge order so requesterOrigin (captured at spawn time, reflecting
the actual current channel) takes priority, with the session entry as
fallback for any missing fields.

Error before fix:
  Delivery failed (whatsapp to bluebubbles:chat_guid:...): Unknown channel: whatsapp

Adds regression test for the stale-channel scenario.

* fix: match test to exact failure scenario and improve reliability (#4957) (thanks @tyler6204)

- Remove lastTo from stale session store to match the exact mismatch scenario described in the PR
- Replace 5ms setTimeout sleeps with expect.poll for better test reliability
- Prevents flakiness on slower CI machines
---
 src/agents/subagent-announce.format.test.ts | 42 ++++++++++++++++++++-
 src/agents/subagent-announce.ts             |  5 ++-
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/src/agents/subagent-announce.format.test.ts b/src/agents/subagent-announce.format.test.ts
index 374a833fa4..0d119b599a 100644
--- a/src/agents/subagent-announce.format.test.ts
+++ b/src/agents/subagent-announce.format.test.ts
@@ -186,7 +186,7 @@ describe("subagent announce formatting", () => {
     });
 
     expect(didAnnounce).toBe(true);
-    await new Promise((r) => setTimeout(r, 5));
+    await expect.poll(() => agentSpy.mock.calls.length).toBe(1);
 
     const call = agentSpy.mock.calls[0]?.[0] as { params?: Record<string, unknown> };
     expect(call?.params?.channel).toBe("whatsapp");
@@ -299,6 +299,44 @@ describe("subagent announce formatting", () => {
     expect(call?.params?.accountId).toBe("acct-987");
   });
 
+  it("prefers requesterOrigin channel over stale session lastChannel in queued announce", async () => {
+    const { runSubagentAnnounceFlow } = await import("./subagent-announce.js");
+    embeddedRunMock.isEmbeddedPiRunActive.mockReturnValue(true);
+    embeddedRunMock.isEmbeddedPiRunStreaming.mockReturnValue(false);
+    // Session store has stale whatsapp channel, but the requesterOrigin says bluebubbles.
+    sessionStore = {
+      "agent:main:main": {
+        sessionId: "session-stale",
+        lastChannel: "whatsapp",
+        queueMode: "collect",
+        queueDebounceMs: 0,
+      },
+    };
+
+    const didAnnounce = await runSubagentAnnounceFlow({
+      childSessionKey: "agent:main:subagent:test",
+      childRunId: "run-stale-channel",
+      requesterSessionKey: "main",
+      requesterOrigin: { channel: "bluebubbles", to: "bluebubbles:chat_guid:123" },
+      requesterDisplayKey: "main",
+      task: "do thing",
+      timeoutMs: 1000,
+      cleanup: "keep",
+      waitForCompletion: false,
+      startedAt: 10,
+      endedAt: 20,
+      outcome: { status: "ok" },
+    });
+
+    expect(didAnnounce).toBe(true);
+    await expect.poll(() => agentSpy.mock.calls.length).toBe(1);
+
+    const call = agentSpy.mock.calls[0]?.[0] as { params?: Record<string, unknown> };
+    // The channel should match requesterOrigin, NOT the stale session entry.
+    expect(call?.params?.channel).toBe("bluebubbles");
+    expect(call?.params?.to).toBe("bluebubbles:chat_guid:123");
+  });
+
   it("splits collect-mode announces when accountId differs", async () => {
     const { runSubagentAnnounceFlow } = await import("./subagent-announce.js");
     embeddedRunMock.isEmbeddedPiRunActive.mockReturnValue(true);
@@ -343,7 +381,7 @@ describe("subagent announce formatting", () => {
       outcome: { status: "ok" },
     });
 
-    await new Promise((r) => setTimeout(r, 5));
+    await expect.poll(() => agentSpy.mock.calls.length).toBe(2);
 
     const accountIds = agentSpy.mock.calls.map(
       (call) => (call[0] as { params?: Record<string, unknown> }).params?.accountId,
diff --git a/src/agents/subagent-announce.ts b/src/agents/subagent-announce.ts
index 444726efc5..36dbbcf2b8 100644
--- a/src/agents/subagent-announce.ts
+++ b/src/agents/subagent-announce.ts
@@ -93,7 +93,10 @@ function resolveAnnounceOrigin(
   entry?: DeliveryContextSource,
   requesterOrigin?: DeliveryContext,
 ): DeliveryContext | undefined {
-  return mergeDeliveryContext(deliveryContextFromSession(entry), requesterOrigin);
+  // requesterOrigin (captured at spawn time) reflects the channel the user is
+  // actually on and must take priority over the session entry, which may carry
+  // stale lastChannel / lastTo values from a previous channel interaction.
+  return mergeDeliveryContext(requesterOrigin, deliveryContextFromSession(entry));
 }
 
 async function sendAnnounce(item: AnnounceQueueItem) {