From e0b56627be5e182172f9f6d1d346dee349687d6e Mon Sep 17 00:00:00 2001 From: Tyler Yust Date: Wed, 4 Mar 2026 17:11:16 -0800 Subject: [PATCH] Fix nested subagent announce routing and waiting status --- src/agents/openclaw-tools.sessions.test.ts | 7 +- src/agents/subagent-announce.timeout.test.ts | 71 ++++++++++- src/agents/subagent-announce.ts | 5 +- src/agents/subagent-registry-queries.test.ts | 120 ++++++++++++++++++- src/agents/subagent-registry-queries.ts | 10 +- src/agents/tools/subagents-tool.ts | 28 +++-- 6 files changed, 215 insertions(+), 26 deletions(-) diff --git a/src/agents/openclaw-tools.sessions.test.ts b/src/agents/openclaw-tools.sessions.test.ts index 36c1f420af4..23a12cb49db 100644 --- a/src/agents/openclaw-tools.sessions.test.ts +++ b/src/agents/openclaw-tools.sessions.test.ts @@ -914,8 +914,9 @@ describe("sessions tools", () => { const result = await tool.execute("call-subagents-list-orchestrator", { action: "list" }); const details = result.details as { status?: string; - active?: Array<{ runId?: string; status?: string }>; + active?: Array<{ runId?: string; status?: string; pendingDescendants?: number }>; recent?: Array<{ runId?: string }>; + text?: string; }; expect(details.status).toBe("ok"); @@ -923,11 +924,13 @@ describe("sessions tools", () => { expect.arrayContaining([ expect.objectContaining({ runId: "run-orchestrator-ended", - status: "active", + status: "active (waiting on 1 child)", + pendingDescendants: 1, }), ]), ); expect(details.recent?.find((entry) => entry.runId === "run-orchestrator-ended")).toBeFalsy(); + expect(details.text).toContain("active (waiting on 1 child)"); }); it("subagents list usage separates io tokens from prompt/cache", async () => { diff --git a/src/agents/subagent-announce.timeout.test.ts b/src/agents/subagent-announce.timeout.test.ts index 755ab2c1556..64a783f4d85 100644 --- a/src/agents/subagent-announce.timeout.test.ts +++ b/src/agents/subagent-announce.timeout.test.ts @@ -15,6 +15,13 @@ let configOverride: ReturnType<(typeof import("../config/config.js"))["loadConfi scope: "per-sender", }, }; +let requesterDepthResolver: (sessionKey?: string) => number = () => 0; +let subagentSessionRunActive = true; +let shouldIgnorePostCompletion = false; +let fallbackRequesterResolution: { + requesterSessionKey: string; + requesterOrigin?: { channel?: string; to?: string; accountId?: string }; +} | null = null; vi.mock("../gateway/call.js", () => ({ callGateway: vi.fn(async (request: GatewayCall) => { @@ -42,7 +49,7 @@ vi.mock("../config/sessions.js", () => ({ })); vi.mock("./subagent-depth.js", () => ({ - getSubagentDepthFromSessionStore: () => 0, + getSubagentDepthFromSessionStore: (sessionKey?: string) => requesterDepthResolver(sessionKey), })); vi.mock("./pi-embedded.js", () => ({ @@ -55,8 +62,9 @@ vi.mock("./subagent-registry.js", () => ({ countActiveDescendantRuns: () => 0, countPendingDescendantRuns: () => 0, listSubagentRunsForRequester: () => [], - isSubagentSessionRunActive: () => true, - resolveRequesterForChildSession: () => null, + isSubagentSessionRunActive: () => subagentSessionRunActive, + shouldIgnorePostCompletionAnnounceForSession: () => shouldIgnorePostCompletion, + resolveRequesterForChildSession: () => fallbackRequesterResolution, })); import { runSubagentAnnounceFlow } from "./subagent-announce.js"; @@ -115,6 +123,10 @@ describe("subagent announce timeout config", () => { configOverride = { session: defaultSessionConfig, }; + requesterDepthResolver = () => 0; + subagentSessionRunActive = true; + shouldIgnorePostCompletion = false; + fallbackRequesterResolution = null; }); it("uses 60s timeout by default for direct announce agent call", async () => { @@ -151,4 +163,57 @@ describe("subagent announce timeout config", () => { ); expect(completionDirectAgentCall?.timeoutMs).toBe(90_000); }); + + it("routes child announce back to ended parent subagent session when parent session still exists", async () => { + const parentSessionKey = "agent:main:subagent:parent"; + requesterDepthResolver = (sessionKey?: string) => + sessionKey === parentSessionKey ? 1 : sessionKey?.includes(":subagent:") ? 1 : 0; + subagentSessionRunActive = false; + shouldIgnorePostCompletion = false; + fallbackRequesterResolution = { + requesterSessionKey: "agent:main:main", + requesterOrigin: { channel: "discord", to: "chan-main", accountId: "acct-main" }, + }; + // No sessionId on purpose: existence in store should still count as alive. + sessionStore[parentSessionKey] = { updatedAt: Date.now() }; + + await runAnnounceFlowForTest("run-parent-route", { + requesterSessionKey: parentSessionKey, + requesterDisplayKey: parentSessionKey, + childSessionKey: `${parentSessionKey}:subagent:child`, + }); + + const directAgentCall = findGatewayCall( + (call) => call.method === "agent" && call.expectFinal === true, + ); + expect(directAgentCall?.params?.sessionKey).toBe(parentSessionKey); + expect(directAgentCall?.params?.deliver).toBe(false); + }); + + it("falls back to grandparent only when ended parent subagent session is missing", async () => { + const parentSessionKey = "agent:main:subagent:parent-missing"; + requesterDepthResolver = (sessionKey?: string) => + sessionKey === parentSessionKey ? 1 : sessionKey?.includes(":subagent:") ? 1 : 0; + subagentSessionRunActive = false; + shouldIgnorePostCompletion = false; + fallbackRequesterResolution = { + requesterSessionKey: "agent:main:main", + requesterOrigin: { channel: "discord", to: "chan-main", accountId: "acct-main" }, + }; + + await runAnnounceFlowForTest("run-parent-fallback", { + requesterSessionKey: parentSessionKey, + requesterDisplayKey: parentSessionKey, + childSessionKey: `${parentSessionKey}:subagent:child`, + }); + + const directAgentCall = findGatewayCall( + (call) => call.method === "agent" && call.expectFinal === true, + ); + expect(directAgentCall?.params?.sessionKey).toBe("agent:main:main"); + expect(directAgentCall?.params?.deliver).toBe(true); + expect(directAgentCall?.params?.channel).toBe("discord"); + expect(directAgentCall?.params?.to).toBe("chan-main"); + expect(directAgentCall?.params?.accountId).toBe("acct-main"); + }); }); diff --git a/src/agents/subagent-announce.ts b/src/agents/subagent-announce.ts index ea502e6a4c5..9c0db6d78ca 100644 --- a/src/agents/subagent-announce.ts +++ b/src/agents/subagent-announce.ts @@ -1260,10 +1260,7 @@ export async function runSubagentAnnounceFlow(params: { // 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(); + const parentSessionAlive = Boolean(parentSessionEntry); if (!parentSessionAlive) { // Parent session is truly gone — fallback to grandparent diff --git a/src/agents/subagent-registry-queries.test.ts b/src/agents/subagent-registry-queries.test.ts index ee5f1694855..4deaee4d8b5 100644 --- a/src/agents/subagent-registry-queries.test.ts +++ b/src/agents/subagent-registry-queries.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { countPendingDescendantRunsExcludingRunFromRuns, countPendingDescendantRunsFromRuns, + resolveRequesterForChildSessionFromRuns, shouldIgnorePostCompletionAnnounceForSessionFromRuns, } from "./subagent-registry-queries.js"; import type { SubagentRunRecord } from "./subagent-registry.types.js"; @@ -139,8 +140,9 @@ describe("subagent registry query regressions", () => { ).toBe(1); }); - it("regression post-completion gating, run-mode sessions ignore late announces once the latest run is ended", () => { - // Regression guard: late descendant announces must not reopen completed run-mode sessions. + it("regression post-completion gating, run-mode sessions ignore late announces after cleanup completes", () => { + // Regression guard: late descendant announces must not reopen run-mode sessions + // once their own completion cleanup has fully finished. const childSessionKey = "agent:main:subagent:orchestrator"; const runs = toRunMap([ makeRun({ @@ -149,6 +151,7 @@ describe("subagent registry query regressions", () => { requesterSessionKey: "agent:main:main", createdAt: 1, endedAt: 10, + cleanupCompletedAt: 11, spawnMode: "run", }), makeRun({ @@ -157,6 +160,7 @@ describe("subagent registry query regressions", () => { requesterSessionKey: "agent:main:main", createdAt: 2, endedAt: 20, + cleanupCompletedAt: 21, spawnMode: "run", }), ]); @@ -164,6 +168,118 @@ describe("subagent registry query regressions", () => { expect(shouldIgnorePostCompletionAnnounceForSessionFromRuns(runs, childSessionKey)).toBe(true); }); + it("keeps run-mode orchestrators announce-eligible while waiting on child completions", () => { + const parentSessionKey = "agent:main:subagent:orchestrator"; + const childOneSessionKey = `${parentSessionKey}:subagent:child-one`; + const childTwoSessionKey = `${parentSessionKey}:subagent:child-two`; + + const runs = toRunMap([ + makeRun({ + runId: "run-parent", + childSessionKey: parentSessionKey, + requesterSessionKey: "agent:main:main", + createdAt: 1, + endedAt: 100, + cleanupCompletedAt: undefined, + spawnMode: "run", + }), + makeRun({ + runId: "run-child-one", + childSessionKey: childOneSessionKey, + requesterSessionKey: parentSessionKey, + createdAt: 2, + endedAt: 110, + cleanupCompletedAt: undefined, + }), + makeRun({ + runId: "run-child-two", + childSessionKey: childTwoSessionKey, + requesterSessionKey: parentSessionKey, + createdAt: 3, + endedAt: 111, + cleanupCompletedAt: undefined, + }), + ]); + + expect(resolveRequesterForChildSessionFromRuns(runs, childOneSessionKey)).toMatchObject({ + requesterSessionKey: parentSessionKey, + }); + expect(resolveRequesterForChildSessionFromRuns(runs, childTwoSessionKey)).toMatchObject({ + requesterSessionKey: parentSessionKey, + }); + expect(shouldIgnorePostCompletionAnnounceForSessionFromRuns(runs, parentSessionKey)).toBe( + false, + ); + + runs.set( + "run-child-one", + makeRun({ + runId: "run-child-one", + childSessionKey: childOneSessionKey, + requesterSessionKey: parentSessionKey, + createdAt: 2, + endedAt: 110, + cleanupCompletedAt: 120, + }), + ); + runs.set( + "run-child-two", + makeRun({ + runId: "run-child-two", + childSessionKey: childTwoSessionKey, + requesterSessionKey: parentSessionKey, + createdAt: 3, + endedAt: 111, + cleanupCompletedAt: 121, + }), + ); + + const childThreeSessionKey = `${parentSessionKey}:subagent:child-three`; + runs.set( + "run-child-three", + makeRun({ + runId: "run-child-three", + childSessionKey: childThreeSessionKey, + requesterSessionKey: parentSessionKey, + createdAt: 4, + }), + ); + + expect(resolveRequesterForChildSessionFromRuns(runs, childThreeSessionKey)).toMatchObject({ + requesterSessionKey: parentSessionKey, + }); + expect(shouldIgnorePostCompletionAnnounceForSessionFromRuns(runs, parentSessionKey)).toBe( + false, + ); + + runs.set( + "run-child-three", + makeRun({ + runId: "run-child-three", + childSessionKey: childThreeSessionKey, + requesterSessionKey: parentSessionKey, + createdAt: 4, + endedAt: 122, + cleanupCompletedAt: 123, + }), + ); + + runs.set( + "run-parent", + makeRun({ + runId: "run-parent", + childSessionKey: parentSessionKey, + requesterSessionKey: "agent:main:main", + createdAt: 1, + endedAt: 100, + cleanupCompletedAt: 130, + spawnMode: "run", + }), + ); + + expect(shouldIgnorePostCompletionAnnounceForSessionFromRuns(runs, parentSessionKey)).toBe(true); + }); + it("regression post-completion gating, session-mode sessions keep accepting follow-up announces", () => { // Regression guard: persistent session-mode orchestrators must continue receiving child completions. const childSessionKey = "agent:main:subagent:orchestrator-session"; diff --git a/src/agents/subagent-registry-queries.ts b/src/agents/subagent-registry-queries.ts index 852ac8ba840..74e5b807c69 100644 --- a/src/agents/subagent-registry-queries.ts +++ b/src/agents/subagent-registry-queries.ts @@ -82,9 +82,13 @@ export function shouldIgnorePostCompletionAnnounceForSessionFromRuns( if (latest.spawnMode === "session") { return false; } - // Run-mode subagent sessions should not process new descendant completion - // traffic after their own run has already ended. - return typeof latest.endedAt === "number"; + // Run-mode sessions should only ignore late descendant completion traffic + // once the run has fully completed its own cleanup/announce flow. + return ( + typeof latest.endedAt === "number" && + typeof latest.cleanupCompletedAt === "number" && + latest.cleanupCompletedAt >= latest.endedAt + ); } export function countActiveRunsForSessionFromRuns( diff --git a/src/agents/tools/subagents-tool.ts b/src/agents/tools/subagents-tool.ts index bd52e597b28..fd1fe2dd7d5 100644 --- a/src/agents/tools/subagents-tool.ts +++ b/src/agents/tools/subagents-tool.ts @@ -71,9 +71,11 @@ type ResolvedRequesterKey = { callerIsSubagent: boolean; }; -function resolveRunStatus(entry: SubagentRunRecord, options?: { hasPendingDescendants?: boolean }) { - if (options?.hasPendingDescendants) { - return "active"; +function resolveRunStatus(entry: SubagentRunRecord, options?: { pendingDescendants?: number }) { + const pendingDescendants = Math.max(0, options?.pendingDescendants ?? 0); + if (pendingDescendants > 0) { + const childLabel = pendingDescendants === 1 ? "child" : "children"; + return `active (waiting on ${pendingDescendants} ${childLabel})`; } if (!entry.endedAt) { return "running"; @@ -369,14 +371,14 @@ export function createSubagentsTool(opts?: { agentSessionKey?: string }): AnyAge const recentCutoff = now - recentMinutes * 60_000; const cache = new Map>(); - const pendingDescendantCache = new Map(); - const hasPendingDescendants = (sessionKey: string) => { + const pendingDescendantCache = new Map(); + const pendingDescendantCount = (sessionKey: string) => { if (pendingDescendantCache.has(sessionKey)) { - return pendingDescendantCache.get(sessionKey) === true; + return pendingDescendantCache.get(sessionKey) ?? 0; } - const hasPending = countPendingDescendantRuns(sessionKey) > 0; - pendingDescendantCache.set(sessionKey, hasPending); - return hasPending; + const pending = Math.max(0, countPendingDescendantRuns(sessionKey)); + pendingDescendantCache.set(sessionKey, pending); + return pending; }; let index = 1; @@ -388,8 +390,9 @@ export function createSubagentsTool(opts?: { agentSessionKey?: string }): AnyAge }).entry; const totalTokens = resolveTotalTokens(sessionEntry); const usageText = formatTokenUsageDisplay(sessionEntry); + const pendingDescendants = pendingDescendantCount(entry.childSessionKey); const status = resolveRunStatus(entry, { - hasPendingDescendants: hasPendingDescendants(entry.childSessionKey), + pendingDescendants, }); const runtime = formatDurationCompact(runtimeMs); const label = truncateLine(resolveSubagentLabel(entry), 48); @@ -402,6 +405,7 @@ export function createSubagentsTool(opts?: { agentSessionKey?: string }): AnyAge label, task, status, + pendingDescendants, runtime, runtimeMs, model: resolveModelRef(sessionEntry) || entry.model, @@ -412,13 +416,13 @@ export function createSubagentsTool(opts?: { agentSessionKey?: string }): AnyAge return { line, view: entry.endedAt ? { ...baseView, endedAt: entry.endedAt } : baseView }; }; const active = runs - .filter((entry) => !entry.endedAt || hasPendingDescendants(entry.childSessionKey)) + .filter((entry) => !entry.endedAt || pendingDescendantCount(entry.childSessionKey) > 0) .map((entry) => buildListEntry(entry, now - (entry.startedAt ?? entry.createdAt))); const recent = runs .filter( (entry) => !!entry.endedAt && - !hasPendingDescendants(entry.childSessionKey) && + pendingDescendantCount(entry.childSessionKey) === 0 && (entry.endedAt ?? 0) >= recentCutoff, ) .map((entry) =>