From 203307557016ea95ddabd8bdcf1b8effbfd6d5b2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 25 Apr 2026 05:38:53 +0100 Subject: [PATCH] fix(sessions): hide stale subagent child links --- CHANGELOG.md | 1 + docs/tools/subagents.md | 5 + src/agents/subagent-list.test.ts | 43 +++++ src/agents/subagent-list.ts | 25 ++- .../subagent-registry.persistence.test.ts | 2 +- src/agents/subagent-run-liveness.test.ts | 41 ++++ src/agents/subagent-run-liveness.ts | 34 +++- src/gateway/session-utils.subagent.test.ts | 178 ++++++++++++++++++ src/gateway/session-utils.ts | 69 ++++++- 9 files changed, 390 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 118c62fa10b..4220eb94662 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Sessions/subagents: stop stale ended runs and old store-only child reverse links from reappearing in `childSessions`, while keeping live descendants and recently-ended children visible. Fixes #57920. - Subagents: stop stale unended runs from counting as active or pending forever, while preserving restart-aborted recovery for recoverable child sessions. Fixes #71252. Thanks @hclsys. - Gateway/tools: allow `POST /tools/invoke` to reach plugin-backed catalog tools such as `browser` when no core implementation exists, while still preferring built-in tools for real core names. Thanks @chat2way. - Browser/security: require `operator.admin` for the `browser.request` gateway method, matching the host/browser-node control authority exposed by that route. Thanks @RichardCao. diff --git a/docs/tools/subagents.md b/docs/tools/subagents.md index 236488356fc..5de2bcc3670 100644 --- a/docs/tools/subagents.md +++ b/docs/tools/subagents.md @@ -214,6 +214,11 @@ Operational guidance: - Start child work once and wait for completion events instead of building poll loops around `sessions_list`, `sessions_history`, `/subagents list`, or `exec` sleep commands. +- `sessions_list` and `/subagents list` keep child-session relationships focused + on live work: live children remain attached, ended children stay visible for a + short recent window, and stale store-only child links are ignored after their + freshness window. This prevents old `spawnedBy` / `parentSessionKey` metadata + from resurrecting ghost children after restart. - If a child completion event arrives after you already sent the final answer, the correct follow-up is the exact silent token `NO_REPLY` / `no_reply`. diff --git a/src/agents/subagent-list.test.ts b/src/agents/subagent-list.test.ts index 6a2816540d2..111bd826502 100644 --- a/src/agents/subagent-list.test.ts +++ b/src/agents/subagent-list.test.ts @@ -115,9 +115,52 @@ describe("buildSubagentList", () => { }); expect(list.active[0]?.status).toBe("active (waiting on 1 child)"); + expect(list.active[0]?.childSessions).toEqual([ + "agent:main:subagent:orchestrator-ended:subagent:child", + ]); expect(list.recent).toEqual([]); }); + it("omits old ended descendants from child session summaries", () => { + const now = Date.now(); + const parentRun = { + runId: "run-parent-active-old-child", + childSessionKey: "agent:main:subagent:parent-active-old-child", + requesterSessionKey: "agent:main:main", + requesterDisplayKey: "main", + task: "parent active", + cleanup: "keep", + createdAt: now - 120_000, + startedAt: now - 120_000, + } satisfies SubagentRunRecord; + addSubagentRunForTests(parentRun); + addSubagentRunForTests({ + runId: "run-old-ended-child-summary", + childSessionKey: `${parentRun.childSessionKey}:subagent:old-ended-child`, + requesterSessionKey: parentRun.childSessionKey, + requesterDisplayKey: "subagent:parent-active-old-child", + task: "old ended child", + cleanup: "keep", + createdAt: now - 60 * 60_000, + startedAt: now - 59 * 60_000, + endedAt: now - 31 * 60_000, + outcome: { status: "ok" }, + }); + const cfg = { + commands: { text: true }, + channels: { whatsapp: { allowFrom: ["*"] } }, + } as OpenClawConfig; + + const list = buildSubagentList({ + cfg, + runs: [parentRun], + recentMinutes: 30, + taskMaxChars: 110, + }); + + expect(list.active[0]?.childSessions).toBeUndefined(); + }); + it("formats io and prompt/cache usage from session entries", async () => { const run = { runId: "run-usage", diff --git a/src/agents/subagent-list.ts b/src/agents/subagent-list.ts index 06dae768ff9..81112948661 100644 --- a/src/agents/subagent-list.ts +++ b/src/agents/subagent-list.ts @@ -13,14 +13,21 @@ import { } from "../shared/subagents-format.js"; import { resolveModelDisplayName, resolveModelDisplayRef } from "./model-selection-display.js"; import { subagentRuns } from "./subagent-registry-memory.js"; -import { countPendingDescendantRunsFromRuns } from "./subagent-registry-queries.js"; +import { + countActiveDescendantRunsFromRuns, + countPendingDescendantRunsFromRuns, +} from "./subagent-registry-queries.js"; import { getSubagentSessionRuntimeMs, getSubagentSessionStartedAt, } from "./subagent-registry-read.js"; import { getSubagentRunsSnapshotForRead } from "./subagent-registry-state.js"; import type { SubagentRunRecord } from "./subagent-registry.types.js"; -import { hasSubagentRunEnded, isLiveUnendedSubagentRun } from "./subagent-run-liveness.js"; +import { + hasSubagentRunEnded, + isLiveUnendedSubagentRun, + shouldKeepSubagentRunChildLink, +} from "./subagent-run-liveness.js"; export type SubagentListItem = { index: number; @@ -80,7 +87,11 @@ export function resolveSessionEntryForKey(params: { }; } -export function buildLatestSubagentRunIndex(runs: Map) { +export function buildLatestSubagentRunIndex( + runs: Map, + options?: { now?: number }, +) { + const now = options?.now ?? Date.now(); const latestByChildSessionKey = new Map(); for (const entry of runs.values()) { const childSessionKey = entry.childSessionKey?.trim(); @@ -100,6 +111,14 @@ export function buildLatestSubagentRunIndex(runs: Map if (!controllerSessionKey) { continue; } + if ( + !shouldKeepSubagentRunChildLink(entry, { + activeDescendants: countActiveDescendantRunsFromRuns(runs, childSessionKey), + now, + }) + ) { + continue; + } const existing = childSessionsByController.get(controllerSessionKey); if (existing) { existing.push(childSessionKey); diff --git a/src/agents/subagent-registry.persistence.test.ts b/src/agents/subagent-registry.persistence.test.ts index eabd2493c8e..cbe97c164ca 100644 --- a/src/agents/subagent-registry.persistence.test.ts +++ b/src/agents/subagent-registry.persistence.test.ts @@ -166,7 +166,7 @@ describe("subagent registry persistence", () => { const waitForRegistryWork = async (predicate: () => boolean | Promise) => { await vi.waitFor(async () => expect(await predicate()).toBe(true), { interval: 1, - timeout: 1_000, + timeout: 5_000, }); }; diff --git a/src/agents/subagent-run-liveness.test.ts b/src/agents/subagent-run-liveness.test.ts index 78376d14584..4bed1d2aa5e 100644 --- a/src/agents/subagent-run-liveness.test.ts +++ b/src/agents/subagent-run-liveness.test.ts @@ -1,8 +1,10 @@ import { describe, expect, it, vi } from "vitest"; import { isLiveUnendedSubagentRun, + RECENT_ENDED_SUBAGENT_CHILD_SESSION_MS, isStaleUnendedSubagentRun, STALE_UNENDED_SUBAGENT_RUN_MS, + shouldKeepSubagentRunChildLink, } from "./subagent-run-liveness.js"; describe("subagent run liveness", () => { @@ -72,4 +74,43 @@ describe("subagent run liveness", () => { vi.useRealTimers(); } }); + + it("keeps child links only while live, recently ended, or waiting on descendants", () => { + expect(shouldKeepSubagentRunChildLink({ createdAt: now - 60_000 }, { now })).toBe(true); + expect( + shouldKeepSubagentRunChildLink( + { + createdAt: now - RECENT_ENDED_SUBAGENT_CHILD_SESSION_MS - 60_000, + endedAt: now - RECENT_ENDED_SUBAGENT_CHILD_SESSION_MS + 1, + }, + { now }, + ), + ).toBe(true); + expect( + shouldKeepSubagentRunChildLink( + { + createdAt: now - RECENT_ENDED_SUBAGENT_CHILD_SESSION_MS - 60_000, + endedAt: now - RECENT_ENDED_SUBAGENT_CHILD_SESSION_MS - 1, + }, + { now }, + ), + ).toBe(false); + expect( + shouldKeepSubagentRunChildLink( + { + createdAt: now - RECENT_ENDED_SUBAGENT_CHILD_SESSION_MS - 60_000, + endedAt: now - RECENT_ENDED_SUBAGENT_CHILD_SESSION_MS - 1, + }, + { activeDescendants: 1, now }, + ), + ).toBe(true); + expect( + shouldKeepSubagentRunChildLink( + { + createdAt: now - STALE_UNENDED_SUBAGENT_RUN_MS - 1, + }, + { now }, + ), + ).toBe(false); + }); }); diff --git a/src/agents/subagent-run-liveness.ts b/src/agents/subagent-run-liveness.ts index 70a1de5fb41..2243472eb98 100644 --- a/src/agents/subagent-run-liveness.ts +++ b/src/agents/subagent-run-liveness.ts @@ -2,10 +2,13 @@ import type { SubagentRunRecord } from "./subagent-registry.types.js"; import { getSubagentSessionStartedAt } from "./subagent-session-metrics.js"; export const STALE_UNENDED_SUBAGENT_RUN_MS = 2 * 60 * 60 * 1_000; +export const RECENT_ENDED_SUBAGENT_CHILD_SESSION_MS = 30 * 60 * 1_000; const EXPLICIT_TIMEOUT_STALE_GRACE_MS = 60_000; const MIN_REALISTIC_RUN_TIMESTAMP_MS = Date.UTC(2020, 0, 1); -export function hasSubagentRunEnded(entry: Pick): boolean { +export function hasSubagentRunEnded>( + entry: T, +): entry is T & { endedAt: number } { return typeof entry.endedAt === "number" && Number.isFinite(entry.endedAt); } @@ -50,3 +53,32 @@ export function isLiveUnendedSubagentRun( ): boolean { return !hasSubagentRunEnded(entry) && !isStaleUnendedSubagentRun(entry, now); } + +export function isRecentlyEndedSubagentRun( + entry: Pick, + now = Date.now(), + recentMs = RECENT_ENDED_SUBAGENT_CHILD_SESSION_MS, +): boolean { + if (!hasSubagentRunEnded(entry)) { + return false; + } + return now - entry.endedAt <= recentMs; +} + +export function shouldKeepSubagentRunChildLink( + entry: Pick< + SubagentRunRecord, + "createdAt" | "startedAt" | "sessionStartedAt" | "endedAt" | "runTimeoutSeconds" + >, + options?: { + activeDescendants?: number; + now?: number; + }, +): boolean { + const now = options?.now ?? Date.now(); + return ( + isLiveUnendedSubagentRun(entry, now) || + (options?.activeDescendants ?? 0) > 0 || + isRecentlyEndedSubagentRun(entry, now) + ); +} diff --git a/src/gateway/session-utils.subagent.test.ts b/src/gateway/session-utils.subagent.test.ts index a8a427216a9..feba73bb0db 100644 --- a/src/gateway/session-utils.subagent.test.ts +++ b/src/gateway/session-utils.subagent.test.ts @@ -726,6 +726,184 @@ describe("listSessionsFromStore subagent metadata", () => { expect(result.sessions.map((session) => session.key)).toEqual(["agent:main:dashboard:child"]); }); + test("does not reattach stale terminal store-only child links", () => { + resetSubagentRegistryForTests({ persist: false }); + const now = Date.now(); + const staleAt = now - 2 * 60 * 60_000; + const store: Record = { + "agent:main:main": { + sessionId: "sess-main", + updatedAt: now, + } as SessionEntry, + "agent:claude:acp:done-child": { + sessionId: "sess-done-child", + updatedAt: staleAt, + spawnedBy: "agent:main:main", + status: "done", + endedAt: staleAt, + } as SessionEntry, + }; + + const all = listSessionsFromStore({ + cfg, + storePath: "/tmp/sessions.json", + store, + opts: {}, + }); + const main = all.sessions.find((session) => session.key === "agent:main:main"); + expect(main?.childSessions).toBeUndefined(); + + const filtered = listSessionsFromStore({ + cfg, + storePath: "/tmp/sessions.json", + store, + opts: { + spawnedBy: "agent:main:main", + }, + }); + expect(filtered.sessions.map((session) => session.key)).toEqual([]); + }); + + test("does not reattach stale orphan store-only child links without lifecycle fields", () => { + resetSubagentRegistryForTests({ persist: false }); + const now = Date.now(); + const staleAt = now - 2 * 60 * 60_000; + const store: Record = { + "agent:main:main": { + sessionId: "sess-main", + updatedAt: now, + } as SessionEntry, + "agent:main:subagent:orphan": { + sessionId: "sess-orphan", + updatedAt: staleAt, + parentSessionKey: "agent:main:main", + } as SessionEntry, + }; + + const all = listSessionsFromStore({ + cfg, + storePath: "/tmp/sessions.json", + store, + opts: {}, + }); + const main = all.sessions.find((session) => session.key === "agent:main:main"); + expect(main?.childSessions).toBeUndefined(); + + const filtered = listSessionsFromStore({ + cfg, + storePath: "/tmp/sessions.json", + store, + opts: { + spawnedBy: "agent:main:main", + }, + }); + expect(filtered.sessions.map((session) => session.key)).toEqual([]); + }); + + test("does not keep old ended registry runs attached as child sessions", () => { + const now = Date.now(); + const store: Record = { + "agent:main:main": { + sessionId: "sess-main", + updatedAt: now, + } as SessionEntry, + "agent:main:subagent:old-ended": { + sessionId: "sess-old-ended", + updatedAt: now - 60 * 60_000, + spawnedBy: "agent:main:main", + } as SessionEntry, + }; + + addSubagentRunForTests({ + runId: "run-old-ended", + childSessionKey: "agent:main:subagent:old-ended", + controllerSessionKey: "agent:main:main", + requesterSessionKey: "agent:main:main", + requesterDisplayKey: "main", + task: "old ended task", + cleanup: "keep", + createdAt: now - 60 * 60_000, + startedAt: now - 59 * 60_000, + endedAt: now - 31 * 60_000, + outcome: { status: "ok" }, + }); + + const all = listSessionsFromStore({ + cfg, + storePath: "/tmp/sessions.json", + store, + opts: {}, + }); + const main = all.sessions.find((session) => session.key === "agent:main:main"); + expect(main?.childSessions).toBeUndefined(); + + const filtered = listSessionsFromStore({ + cfg, + storePath: "/tmp/sessions.json", + store, + opts: { + spawnedBy: "agent:main:main", + }, + }); + expect(filtered.sessions.map((session) => session.key)).toEqual([]); + }); + + test("keeps ended parents attached while live descendants are still running", () => { + const now = Date.now(); + const parentKey = "agent:main:subagent:ended-parent"; + const childKey = "agent:main:subagent:ended-parent:subagent:live-child"; + const store: Record = { + "agent:main:main": { + sessionId: "sess-main", + updatedAt: now, + } as SessionEntry, + [parentKey]: { + sessionId: "sess-ended-parent", + updatedAt: now - 31 * 60_000, + spawnedBy: "agent:main:main", + } as SessionEntry, + [childKey]: { + sessionId: "sess-live-child", + updatedAt: now, + spawnedBy: parentKey, + } as SessionEntry, + }; + + addSubagentRunForTests({ + runId: "run-ended-parent", + childSessionKey: parentKey, + controllerSessionKey: "agent:main:main", + requesterSessionKey: "agent:main:main", + requesterDisplayKey: "main", + task: "ended parent task", + cleanup: "keep", + createdAt: now - 60 * 60_000, + startedAt: now - 59 * 60_000, + endedAt: now - 31 * 60_000, + outcome: { status: "ok" }, + }); + addSubagentRunForTests({ + runId: "run-live-child", + childSessionKey: childKey, + controllerSessionKey: parentKey, + requesterSessionKey: parentKey, + requesterDisplayKey: "ended-parent", + task: "live child task", + cleanup: "keep", + createdAt: now - 1_000, + startedAt: now - 900, + }); + + const result = listSessionsFromStore({ + cfg, + storePath: "/tmp/sessions.json", + store, + opts: {}, + }); + const main = result.sessions.find((session) => session.key === "agent:main:main"); + expect(main?.childSessions).toEqual([parentKey]); + }); + test("falls back to persisted subagent timing after run archival", () => { const now = Date.now(); const store: Record = { diff --git a/src/gateway/session-utils.ts b/src/gateway/session-utils.ts index cdbc342e928..33b2198b014 100644 --- a/src/gateway/session-utils.ts +++ b/src/gateway/session-utils.ts @@ -19,12 +19,17 @@ import { resolvePersistedSelectedModelRef, } from "../agents/model-selection.js"; import { + countActiveDescendantRuns, getSessionDisplaySubagentRunByChildSessionKey, getSubagentSessionRuntimeMs, getSubagentSessionStartedAt, listSubagentRunsForController, resolveSubagentSessionStatus, } from "../agents/subagent-registry-read.js"; +import { + RECENT_ENDED_SUBAGENT_CHILD_SESSION_MS, + shouldKeepSubagentRunChildLink, +} from "../agents/subagent-run-liveness.js"; import { listThinkingLevelOptions, resolveThinkingDefaultForModel, @@ -81,6 +86,7 @@ import type { GatewayAgentRow, GatewaySessionRow, GatewaySessionsDefaults, + SessionRunStatus, SessionsListResult, } from "./session-utils.types.js"; @@ -291,9 +297,36 @@ function resolveEstimatedSessionCostUsd(params: { return resolveNonNegativeNumber(estimated); } +const STALE_STORE_ONLY_CHILD_LINK_MS = 60 * 60 * 1_000; + +function isFinitePositiveTimestamp(value: unknown): value is number { + return typeof value === "number" && Number.isFinite(value) && value > 0; +} + +function isTerminalSessionStatus(status: unknown): status is Exclude { + return status === "done" || status === "failed" || status === "killed" || status === "timeout"; +} + +function shouldKeepStoreOnlyChildLink(entry: SessionEntry, now: number): boolean { + if (isTerminalSessionStatus(entry.status) || isFinitePositiveTimestamp(entry.endedAt)) { + const endedAt = isFinitePositiveTimestamp(entry.endedAt) ? entry.endedAt : entry.updatedAt; + return ( + isFinitePositiveTimestamp(endedAt) && now - endedAt <= RECENT_ENDED_SUBAGENT_CHILD_SESSION_MS + ); + } + if (entry.status === "running" || isFinitePositiveTimestamp(entry.startedAt)) { + return true; + } + return ( + isFinitePositiveTimestamp(entry.updatedAt) && + now - entry.updatedAt <= STALE_STORE_ONLY_CHILD_LINK_MS + ); +} + function resolveChildSessionKeys( controllerSessionKey: string, store: Record, + now = Date.now(), ): string[] | undefined { const childSessionKeys = new Set(); for (const entry of listSubagentRunsForController(controllerSessionKey)) { @@ -302,12 +335,23 @@ function resolveChildSessionKeys( continue; } const latest = getSessionDisplaySubagentRunByChildSessionKey(childSessionKey); + if (!latest) { + continue; + } const latestControllerSessionKey = normalizeOptionalString(latest?.controllerSessionKey) || normalizeOptionalString(latest?.requesterSessionKey); if (latestControllerSessionKey !== controllerSessionKey) { continue; } + if ( + !shouldKeepSubagentRunChildLink(latest, { + activeDescendants: countActiveDescendantRuns(childSessionKey), + now, + }) + ) { + continue; + } childSessionKeys.add(childSessionKey); } for (const [key, entry] of Object.entries(store)) { @@ -327,6 +371,16 @@ function resolveChildSessionKeys( if (latestControllerSessionKey !== controllerSessionKey) { continue; } + if ( + !shouldKeepSubagentRunChildLink(latest, { + activeDescendants: countActiveDescendantRuns(key), + now, + }) + ) { + continue; + } + } else if (!shouldKeepStoreOnlyChildLink(entry, now)) { + continue; } childSessionKeys.add(key); } @@ -1262,7 +1316,7 @@ export function buildGatewaySessionRow(params: { typeof totalTokens === "number" && Number.isFinite(totalTokens) && totalTokens > 0 ? true : transcriptUsage?.totalTokensFresh === true; - const childSessions = resolveChildSessionKeys(key, store); + const childSessions = resolveChildSessionKeys(key, store, now); const latestCompactionCheckpoint = resolveLatestCompactionCheckpoint(entry); const estimatedCostUsd = resolveEstimatedSessionCostUsd({ @@ -1445,9 +1499,18 @@ export function listSessionsFromStore(params: { const latestControllerSessionKey = normalizeOptionalString(latest.controllerSessionKey) || normalizeOptionalString(latest.requesterSessionKey); - return latestControllerSessionKey === spawnedBy; + return ( + latestControllerSessionKey === spawnedBy && + shouldKeepSubagentRunChildLink(latest, { + activeDescendants: countActiveDescendantRuns(key), + now, + }) + ); } - return entry?.spawnedBy === spawnedBy || entry?.parentSessionKey === spawnedBy; + return ( + shouldKeepStoreOnlyChildLink(entry, now) && + (entry?.spawnedBy === spawnedBy || entry?.parentSessionKey === spawnedBy) + ); }) .filter(([, entry]) => { if (!label) {