From 6931ca7035fe26e6aec1c1b78a08340274d752e7 Mon Sep 17 00:00:00 2001 From: Operative-001 Date: Mon, 16 Feb 2026 13:21:19 +0100 Subject: [PATCH] fix(subagent): route nested announce to parent even when parent run ended MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a depth-2 subagent (Birdie) completes and its parent (Newton) is a depth-1 subagent, the announce should go to Newton, not bypass to the grandparent (Jaris). Previously, isSubagentSessionRunActive(Newton) returned false because Newton's agent turn completed after spawning Birdie. This triggered the fallback to grandparent even though Newton's SESSION was still alive and waiting for child results. Now we only fallback to grandparent if the parent SESSION is actually deleted (no sessionId in session store). If the parent session exists, we inject into it even if the current run has ended — this starts a new agent turn to process the child result. Fixes #18037 Test Plan: - Added regression test: routes to parent when run ended but session alive - Added regression test: falls back to grandparent only when session deleted --- .../subagent-announce.format.e2e.test.ts | 98 +++++++++++++++++++ src/agents/subagent-announce.ts | 40 +++++--- 2 files changed, 126 insertions(+), 12 deletions(-) diff --git a/src/agents/subagent-announce.format.e2e.test.ts b/src/agents/subagent-announce.format.e2e.test.ts index c8fb45d05d5..39592e7e7f0 100644 --- a/src/agents/subagent-announce.format.e2e.test.ts +++ b/src/agents/subagent-announce.format.e2e.test.ts @@ -732,4 +732,102 @@ describe("subagent announce formatting", () => { expect(call?.params?.channel).toBe("bluebubbles"); expect(call?.params?.to).toBe("bluebubbles:chat_guid:123"); }); + + it("routes to parent subagent when parent run ended but session still exists (#18037)", async () => { + // Scenario: Newton (depth-1) spawns Birdie (depth-2). Newton's agent turn ends + // after spawning but Newton's SESSION still exists (waiting for Birdie's result). + // Birdie completes → Birdie's announce should go to Newton, NOT to Jaris (depth-0). + const { runSubagentAnnounceFlow } = await import("./subagent-announce.js"); + embeddedRunMock.isEmbeddedPiRunActive.mockReturnValue(false); + embeddedRunMock.isEmbeddedPiRunStreaming.mockReturnValue(false); + + // Parent's run has ended (no active run) + subagentRegistryMock.isSubagentSessionRunActive.mockReturnValue(false); + // BUT parent session still exists in the store + sessionStore = { + "agent:main:subagent:newton": { + sessionId: "newton-session-id-alive", + inputTokens: 100, + outputTokens: 50, + }, + "agent:main:subagent:newton:subagent:birdie": { + sessionId: "birdie-session-id", + inputTokens: 20, + outputTokens: 10, + }, + }; + // Fallback would be available to Jaris (grandparent) + subagentRegistryMock.resolveRequesterForChildSession.mockReturnValue({ + requesterSessionKey: "agent:main:main", + requesterOrigin: { channel: "discord" }, + }); + + const didAnnounce = await runSubagentAnnounceFlow({ + childSessionKey: "agent:main:subagent:newton:subagent:birdie", + childRunId: "run-birdie", + requesterSessionKey: "agent:main:subagent:newton", + requesterDisplayKey: "subagent:newton", + task: "QA the outline", + timeoutMs: 1000, + cleanup: "keep", + waitForCompletion: false, + startedAt: 10, + endedAt: 20, + outcome: { status: "ok" }, + }); + + expect(didAnnounce).toBe(true); + // Verify announce went to Newton (the parent), NOT to Jaris (grandparent fallback) + const call = agentSpy.mock.calls[0]?.[0] as { params?: Record }; + expect(call?.params?.sessionKey).toBe("agent:main:subagent:newton"); + // deliver=false because Newton is a subagent (internal injection) + expect(call?.params?.deliver).toBe(false); + // Should NOT have used the grandparent fallback + expect(call?.params?.sessionKey).not.toBe("agent:main:main"); + }); + + it("falls back to grandparent only when parent session is deleted (#18037)", async () => { + // Scenario: Parent session was cleaned up. Only then should we fallback. + const { runSubagentAnnounceFlow } = await import("./subagent-announce.js"); + embeddedRunMock.isEmbeddedPiRunActive.mockReturnValue(false); + embeddedRunMock.isEmbeddedPiRunStreaming.mockReturnValue(false); + + // Parent's run ended AND session is gone + subagentRegistryMock.isSubagentSessionRunActive.mockReturnValue(false); + // Parent session does NOT exist (was deleted) + sessionStore = { + "agent:main:subagent:birdie": { + sessionId: "birdie-session-id", + inputTokens: 20, + outputTokens: 10, + }, + // Newton's entry is MISSING (session was deleted) + }; + subagentRegistryMock.resolveRequesterForChildSession.mockReturnValue({ + requesterSessionKey: "agent:main:main", + requesterOrigin: { channel: "discord", accountId: "jaris-account" }, + }); + + const didAnnounce = await runSubagentAnnounceFlow({ + childSessionKey: "agent:main:subagent:birdie", + childRunId: "run-birdie-orphan", + requesterSessionKey: "agent:main:subagent:newton", + requesterDisplayKey: "subagent:newton", + task: "QA task", + timeoutMs: 1000, + cleanup: "keep", + waitForCompletion: false, + startedAt: 10, + endedAt: 20, + outcome: { status: "ok" }, + }); + + expect(didAnnounce).toBe(true); + // Verify announce fell back to Jaris (grandparent) since Newton is gone + const call = agentSpy.mock.calls[0]?.[0] as { params?: Record }; + expect(call?.params?.sessionKey).toBe("agent:main:main"); + // deliver=true because Jaris is main (user-facing) + expect(call?.params?.deliver).toBe(true); + expect(call?.params?.channel).toBe("discord"); + }); }); diff --git a/src/agents/subagent-announce.ts b/src/agents/subagent-announce.ts index db53e3eced6..8a5588014ae 100644 --- a/src/agents/subagent-announce.ts +++ b/src/agents/subagent-announce.ts @@ -508,23 +508,39 @@ export async function runSubagentAnnounceFlow(params: { let requesterIsSubagent = requesterDepth >= 1; // If the requester subagent has already finished, bubble the announce to its // requester (typically main) so descendant completion is not silently lost. + // BUT: only fallback if the parent SESSION is deleted, not just if the current + // run ended. A parent waiting for child results has no active run but should + // still receive the announce — injecting will start a new agent turn. if (requesterIsSubagent) { const { isSubagentSessionRunActive, resolveRequesterForChildSession } = await import("./subagent-registry.js"); if (!isSubagentSessionRunActive(targetRequesterSessionKey)) { - const fallback = resolveRequesterForChildSession(targetRequesterSessionKey); - if (!fallback?.requesterSessionKey) { - // Without a requester fallback we cannot safely deliver this nested - // completion. Keep cleanup retryable so a later registry restore can - // recover and re-announce instead of silently dropping the result. - shouldDeleteChildSession = false; - return false; + // Parent run has ended. Check if parent SESSION still exists. + // If it does, the parent may be waiting for child results — inject there. + const parentSessionEntry = loadSessionEntryByKey(targetRequesterSessionKey); + const parentSessionAlive = + parentSessionEntry && + typeof parentSessionEntry.sessionId === "string" && + parentSessionEntry.sessionId.trim(); + + if (!parentSessionAlive) { + // Parent session is truly gone — fallback to grandparent + const fallback = resolveRequesterForChildSession(targetRequesterSessionKey); + if (!fallback?.requesterSessionKey) { + // Without a requester fallback we cannot safely deliver this nested + // completion. Keep cleanup retryable so a later registry restore can + // recover and re-announce instead of silently dropping the result. + shouldDeleteChildSession = false; + return false; + } + targetRequesterSessionKey = fallback.requesterSessionKey; + targetRequesterOrigin = + normalizeDeliveryContext(fallback.requesterOrigin) ?? targetRequesterOrigin; + requesterDepth = getSubagentDepthFromSessionStore(targetRequesterSessionKey); + requesterIsSubagent = requesterDepth >= 1; } - targetRequesterSessionKey = fallback.requesterSessionKey; - targetRequesterOrigin = - normalizeDeliveryContext(fallback.requesterOrigin) ?? targetRequesterOrigin; - requesterDepth = getSubagentDepthFromSessionStore(targetRequesterSessionKey); - requesterIsSubagent = requesterDepth >= 1; + // If parent session is alive (just has no active run), continue with parent + // as target. Injecting the announce will start a new agent turn for processing. } }