fix: ignore moved subagent children on stale parents

2026-05-02 23:00:20 +00:00 · 2026-03-24 18:45:46 -05:00
parent 33e9e485b8
commit e48a0b80a8
7 changed files with 377 additions and 1 deletions
--- a/src/agents/openclaw-tools.sessions.test.ts
+++ b/src/agents/openclaw-tools.sessions.test.ts
@@ -1068,6 +1068,94 @@ describe("sessions tools", () => {
    expect(details.text).not.toContain("active (waiting on 2 children)");
  });

+  it("subagents list does not keep childSessions attached to a stale older parent", async () => {
+    resetSubagentRegistryForTests();
+    const now = Date.now();
+    const oldParentKey = "agent:main:subagent:old-parent";
+    const newParentKey = "agent:main:subagent:new-parent";
+    const childKey = "agent:main:subagent:shared-child";
+
+    addSubagentRunForTests({
+      runId: "run-old-parent",
+      childSessionKey: oldParentKey,
+      requesterSessionKey: "agent:main:main",
+      requesterDisplayKey: "main",
+      task: "old parent task",
+      cleanup: "keep",
+      createdAt: now - 10_000,
+      startedAt: now - 9_000,
+    });
+    addSubagentRunForTests({
+      runId: "run-new-parent",
+      childSessionKey: newParentKey,
+      requesterSessionKey: "agent:main:main",
+      requesterDisplayKey: "main",
+      task: "new parent task",
+      cleanup: "keep",
+      createdAt: now - 8_000,
+      startedAt: now - 7_000,
+    });
+    addSubagentRunForTests({
+      runId: "run-shared-child-stale-parent",
+      childSessionKey: childKey,
+      requesterSessionKey: oldParentKey,
+      requesterDisplayKey: oldParentKey,
+      controllerSessionKey: oldParentKey,
+      task: "shared child stale parent",
+      cleanup: "keep",
+      createdAt: now - 6_000,
+      startedAt: now - 5_000,
+      endedAt: now - 4_000,
+      outcome: { status: "ok" },
+    });
+    addSubagentRunForTests({
+      runId: "run-shared-child-current-parent",
+      childSessionKey: childKey,
+      requesterSessionKey: newParentKey,
+      requesterDisplayKey: newParentKey,
+      controllerSessionKey: newParentKey,
+      task: "shared child current parent",
+      cleanup: "keep",
+      createdAt: now - 2_000,
+      startedAt: now - 1_500,
+    });
+
+    const tool = createOpenClawTools({
+      agentSessionKey: "agent:main:main",
+    }).find((candidate) => candidate.name === "subagents");
+    expect(tool).toBeDefined();
+    if (!tool) {
+      throw new Error("missing subagents tool");
+    }
+
+    const result = await tool.execute("call-subagents-list-stale-parent", { action: "list" });
+    const details = result.details as {
+      status?: string;
+      active?: Array<{
+        runId?: string;
+        childSessions?: string[];
+        pendingDescendants?: number;
+        status?: string;
+      }>;
+    };
+
+    expect(details.status).toBe("ok");
+    const oldParent = details.active?.find((entry) => entry.runId === "run-old-parent");
+    const newParent = details.active?.find((entry) => entry.runId === "run-new-parent");
+    expect(oldParent).toMatchObject({
+      runId: "run-old-parent",
+      pendingDescendants: 0,
+      status: "running",
+    });
+    expect(oldParent?.childSessions).toBeUndefined();
+    expect(newParent).toMatchObject({
+      runId: "run-new-parent",
+      childSessions: [childKey],
+      pendingDescendants: 1,
+      status: "active (waiting on 1 child)",
+    });
+  });
+
  it("subagents list dedupes stale rows for the same child session", async () => {
    resetSubagentRegistryForTests();
    const now = Date.now();
--- a/src/agents/subagent-control.test.ts
+++ b/src/agents/subagent-control.test.ts
@@ -620,6 +620,105 @@ describe("killControlledSubagentRun", () => {
    });
    expect(getSubagentRunByChildSessionKey(leafSessionKey)?.endedAt).toBeTypeOf("number");
  });
+
+  it("does not cascade through a child session that moved to a newer parent", async () => {
+    const oldParentSessionKey = "agent:main:subagent:old-parent";
+    const newParentSessionKey = "agent:main:subagent:new-parent";
+    const childSessionKey = "agent:main:subagent:shared-child";
+    const leafSessionKey = `${childSessionKey}:subagent:leaf`;
+
+    addSubagentRunForTests({
+      runId: "run-old-parent-current",
+      childSessionKey: oldParentSessionKey,
+      controllerSessionKey: "agent:main:main",
+      requesterSessionKey: "agent:main:main",
+      requesterDisplayKey: "main",
+      task: "old parent task",
+      cleanup: "keep",
+      createdAt: Date.now() - 8_000,
+      startedAt: Date.now() - 7_000,
+      endedAt: Date.now() - 6_000,
+      outcome: { status: "ok" },
+    });
+    addSubagentRunForTests({
+      runId: "run-new-parent-current",
+      childSessionKey: newParentSessionKey,
+      controllerSessionKey: "agent:main:main",
+      requesterSessionKey: "agent:main:main",
+      requesterDisplayKey: "main",
+      task: "new parent task",
+      cleanup: "keep",
+      createdAt: Date.now() - 5_000,
+      startedAt: Date.now() - 4_000,
+    });
+    addSubagentRunForTests({
+      runId: "run-child-stale-old-parent",
+      childSessionKey,
+      controllerSessionKey: oldParentSessionKey,
+      requesterSessionKey: oldParentSessionKey,
+      requesterDisplayKey: oldParentSessionKey,
+      task: "stale shared child task",
+      cleanup: "keep",
+      createdAt: Date.now() - 4_000,
+      startedAt: Date.now() - 3_500,
+      endedAt: Date.now() - 3_000,
+      outcome: { status: "ok" },
+    });
+    addSubagentRunForTests({
+      runId: "run-child-current-new-parent",
+      childSessionKey,
+      controllerSessionKey: newParentSessionKey,
+      requesterSessionKey: newParentSessionKey,
+      requesterDisplayKey: newParentSessionKey,
+      task: "current shared child task",
+      cleanup: "keep",
+      createdAt: Date.now() - 2_000,
+      startedAt: Date.now() - 1_500,
+    });
+    addSubagentRunForTests({
+      runId: "run-leaf-active",
+      childSessionKey: leafSessionKey,
+      controllerSessionKey: childSessionKey,
+      requesterSessionKey: childSessionKey,
+      requesterDisplayKey: childSessionKey,
+      task: "leaf task",
+      cleanup: "keep",
+      createdAt: Date.now() - 1_000,
+      startedAt: Date.now() - 900,
+    });
+
+    const result = await killControlledSubagentRun({
+      cfg: {} as OpenClawConfig,
+      controller: {
+        controllerSessionKey: "agent:main:main",
+        callerSessionKey: "agent:main:main",
+        callerIsSubagent: false,
+        controlScope: "children",
+      },
+      entry: {
+        runId: "run-old-parent-current",
+        childSessionKey: oldParentSessionKey,
+        requesterSessionKey: "agent:main:main",
+        requesterDisplayKey: "main",
+        controllerSessionKey: "agent:main:main",
+        task: "old parent task",
+        cleanup: "keep",
+        createdAt: Date.now() - 8_000,
+        startedAt: Date.now() - 7_000,
+        endedAt: Date.now() - 6_000,
+        outcome: { status: "ok" },
+      },
+    });
+
+    expect(result).toEqual({
+      status: "done",
+      runId: "run-old-parent-current",
+      sessionKey: oldParentSessionKey,
+      label: "old parent task",
+      text: "old parent task is already finished.",
+    });
+    expect(getSubagentRunByChildSessionKey(leafSessionKey)?.endedAt).toBeUndefined();
+  });
 });

 describe("killAllControlledSubagentRuns", () => {
--- a/src/agents/subagent-control.ts
+++ b/src/agents/subagent-control.ts
@@ -298,7 +298,15 @@ export function buildSubagentList(params: {
    });
    const childSessions = Array.from(
      new Set(
-        listSubagentRunsForController(entry.childSessionKey).map((run) => run.childSessionKey),
+        listSubagentRunsForController(entry.childSessionKey)
+          .map((run) => run.childSessionKey?.trim())
+          .filter((childSessionKey): childSessionKey is string => Boolean(childSessionKey))
+          .filter((childSessionKey) => {
+            const latest = getLatestSubagentRunByChildSessionKey(childSessionKey);
+            const latestControllerSessionKey =
+              latest?.controllerSessionKey?.trim() || latest?.requesterSessionKey?.trim();
+            return latestControllerSessionKey === entry.childSessionKey;
+          }),
      ),
    );
    const runtime = formatDurationCompact(runtimeMs);
@@ -419,6 +427,16 @@ async function cascadeKillChildren(params: {
    if (!childKey) {
      continue;
    }
+    const latest = getLatestSubagentRunByChildSessionKey(childKey);
+    const latestControllerSessionKey =
+      latest?.controllerSessionKey?.trim() || latest?.requesterSessionKey?.trim();
+    if (
+      !latest ||
+      latest.runId !== run.runId ||
+      latestControllerSessionKey !== params.parentChildSessionKey
+    ) {
+      continue;
+    }
    const existing = childRunsBySessionKey.get(childKey);
    if (!existing || run.createdAt >= existing.createdAt) {
      childRunsBySessionKey.set(childKey, run);
--- a/src/agents/subagent-registry-queries.test.ts
+++ b/src/agents/subagent-registry-queries.test.ts
@@ -144,6 +144,44 @@ describe("subagent registry query regressions", () => {
    expect(countPendingDescendantRunsFromRuns(runs, parentSessionKey)).toBe(2);
  });

+  it("ignores stale older parent rows when a child session moved to a newer controller", () => {
+    const oldParentSessionKey = "agent:main:subagent:old-parent";
+    const newParentSessionKey = "agent:main:subagent:new-parent";
+    const childSessionKey = "agent:main:subagent:shared-child";
+    const runs = toRunMap([
+      makeRun({
+        runId: "run-old-parent",
+        childSessionKey: oldParentSessionKey,
+        requesterSessionKey: "agent:main:main",
+        createdAt: 100,
+      }),
+      makeRun({
+        runId: "run-new-parent",
+        childSessionKey: newParentSessionKey,
+        requesterSessionKey: "agent:main:main",
+        createdAt: 200,
+      }),
+      makeRun({
+        runId: "run-child-stale-parent",
+        childSessionKey,
+        requesterSessionKey: oldParentSessionKey,
+        controllerSessionKey: oldParentSessionKey,
+        createdAt: 300,
+        endedAt: 350,
+      }),
+      makeRun({
+        runId: "run-child-current-parent",
+        childSessionKey,
+        requesterSessionKey: newParentSessionKey,
+        controllerSessionKey: newParentSessionKey,
+        createdAt: 400,
+      }),
+    ]);
+
+    expect(countPendingDescendantRunsFromRuns(runs, oldParentSessionKey)).toBe(0);
+    expect(countPendingDescendantRunsFromRuns(runs, newParentSessionKey)).toBe(1);
+  });
+
  it("regression excluding current run, countPendingDescendantRunsExcludingRun keeps sibling gating intact", () => {
    // Regression guard: excluding the currently announcing run must not hide sibling pending work.
    const runs = toRunMap([
--- a/src/agents/subagent-registry-queries.ts
+++ b/src/agents/subagent-registry-queries.ts
@@ -188,6 +188,14 @@ function forEachDescendantRun(
      }
    }
    for (const [runId, entry] of latestByChildSessionKey.values()) {
+      const latestForChildSession = findLatestRunForChildSession(runs, entry.childSessionKey);
+      if (
+        !latestForChildSession ||
+        latestForChildSession.runId !== runId ||
+        latestForChildSession.requesterSessionKey !== requester
+      ) {
+        continue;
+      }
      visitor(runId, entry);
      const childKey = entry.childSessionKey.trim();
      if (!childKey || visited.has(childKey)) {
--- a/src/auto-reply/reply/abort.test.ts
+++ b/src/auto-reply/reply/abort.test.ts
@@ -14,6 +14,7 @@ import {
  resolveAbortCutoffFromContext,
  resolveSessionEntryForKey,
  setAbortMemory,
+  stopSubagentsForRequester,
  shouldSkipMessageByAbortCutoff,
  tryFastAbortFromMessage,
 } from "./abort.js";
@@ -37,10 +38,15 @@ const subagentRegistryMocks = vi.hoisted(() => ({
  listSubagentRunsForRequester: vi.fn<(requesterSessionKey: string) => SubagentRunRecord[]>(
    () => [],
  ),
+  getLatestSubagentRunByChildSessionKey: vi.fn<
+    (childSessionKey: string) => SubagentRunRecord | null
+  >(() => null),
  markSubagentRunTerminated: vi.fn(() => 1),
 }));

 vi.mock("../../agents/subagent-registry.js", () => ({
+  getLatestSubagentRunByChildSessionKey:
+    subagentRegistryMocks.getLatestSubagentRunByChildSessionKey,
  listSubagentRunsForRequester: subagentRegistryMocks.listSubagentRunsForRequester,
  listSubagentRunsForController: subagentRegistryMocks.listSubagentRunsForRequester,
  markSubagentRunTerminated: subagentRegistryMocks.markSubagentRunTerminated,
@@ -172,6 +178,8 @@ describe("abort detection", () => {
          cancelSession: acpManagerMocks.cancelSession,
        }) as never) as never,
      abortEmbeddedPiRun: () => true,
+      getLatestSubagentRunByChildSessionKey:
+        subagentRegistryMocks.getLatestSubagentRunByChildSessionKey,
      listSubagentRunsForController: subagentRegistryMocks.listSubagentRunsForRequester,
      markSubagentRunTerminated: subagentRegistryMocks.markSubagentRunTerminated,
    });
@@ -189,6 +197,7 @@ describe("abort detection", () => {
    commandQueueMocks.clearCommandLane.mockClear().mockReturnValue(1);
    acpManagerMocks.resolveSession.mockReset().mockReturnValue({ kind: "none" });
    acpManagerMocks.cancelSession.mockReset().mockResolvedValue(undefined);
+    subagentRegistryMocks.getLatestSubagentRunByChildSessionKey.mockReset().mockReturnValue(null);
  });

  it("triggerBodyNormalized extracts /stop from RawBody for abort detection", async () => {
@@ -746,6 +755,36 @@ describe("abort detection", () => {
        },
      ])
      .mockReturnValueOnce([]);
+    subagentRegistryMocks.getLatestSubagentRunByChildSessionKey.mockImplementation(
+      (childSessionKey) => {
+        if (childSessionKey === depth1Key) {
+          return {
+            runId: "run-current-parent",
+            childSessionKey: depth1Key,
+            requesterSessionKey: sessionKey,
+            requesterDisplayKey: "telegram:parent",
+            task: "current orchestrator",
+            cleanup: "keep",
+            createdAt: now - 1_000,
+            startedAt: now - 900,
+            endedAt: now - 500,
+            outcome: { status: "ok" },
+          } as SubagentRunRecord;
+        }
+        if (childSessionKey === depth2Key) {
+          return {
+            runId: "run-active-child",
+            childSessionKey: depth2Key,
+            requesterSessionKey: depth1Key,
+            requesterDisplayKey: depth1Key,
+            task: "leaf worker",
+            cleanup: "keep",
+            createdAt: now - 400,
+          } as SubagentRunRecord;
+        }
+        return null;
+      },
+    );

    const result = await runStopCommand({
      cfg,
@@ -760,4 +799,77 @@ describe("abort detection", () => {
      expect.objectContaining({ runId: "run-active-child", childSessionKey: depth2Key }),
    );
  });
+
+  it("stopSubagentsForRequester does not traverse a child that moved to a newer parent", async () => {
+    subagentRegistryMocks.listSubagentRunsForRequester.mockClear();
+    subagentRegistryMocks.markSubagentRunTerminated.mockClear();
+    const oldParentKey = "agent:main:subagent:old-parent";
+    const newParentKey = "agent:main:subagent:new-parent";
+    const childKey = "agent:main:subagent:shared-child";
+    const leafKey = `${childKey}:subagent:leaf`;
+    const now = Date.now();
+
+    subagentRegistryMocks.listSubagentRunsForRequester
+      .mockReturnValueOnce([
+        {
+          runId: "run-shared-child-stale-parent",
+          childSessionKey: childKey,
+          requesterSessionKey: oldParentKey,
+          controllerSessionKey: oldParentKey,
+          requesterDisplayKey: oldParentKey,
+          task: "shared child stale parent",
+          cleanup: "keep",
+          createdAt: now - 2_000,
+          endedAt: now - 1_000,
+          outcome: { status: "ok" },
+        },
+      ])
+      .mockReturnValueOnce([
+        {
+          runId: "run-leaf-active",
+          childSessionKey: leafKey,
+          requesterSessionKey: childKey,
+          controllerSessionKey: childKey,
+          requesterDisplayKey: childKey,
+          task: "leaf worker",
+          cleanup: "keep",
+          createdAt: now - 500,
+        },
+      ]);
+    subagentRegistryMocks.getLatestSubagentRunByChildSessionKey.mockImplementation((sessionKey) => {
+      if (sessionKey === childKey) {
+        return {
+          runId: "run-shared-child-current-parent",
+          childSessionKey: childKey,
+          requesterSessionKey: newParentKey,
+          controllerSessionKey: newParentKey,
+          requesterDisplayKey: newParentKey,
+          task: "shared child current parent",
+          cleanup: "keep",
+          createdAt: now - 250,
+        } as SubagentRunRecord;
+      }
+      if (sessionKey === leafKey) {
+        return {
+          runId: "run-leaf-active",
+          childSessionKey: leafKey,
+          requesterSessionKey: childKey,
+          controllerSessionKey: childKey,
+          requesterDisplayKey: childKey,
+          task: "leaf worker",
+          cleanup: "keep",
+          createdAt: now - 500,
+        } as SubagentRunRecord;
+      }
+      return null;
+    });
+
+    const result = stopSubagentsForRequester({
+      cfg: {} as OpenClawConfig,
+      requesterSessionKey: oldParentKey,
+    });
+
+    expect(result).toEqual({ stopped: 0 });
+    expect(subagentRegistryMocks.markSubagentRunTerminated).not.toHaveBeenCalled();
+  });
 });
--- a/src/auto-reply/reply/abort.ts
+++ b/src/auto-reply/reply/abort.ts
@@ -2,6 +2,7 @@ import { getAcpSessionManager } from "../../acp/control-plane/manager.js";
 import { resolveSessionAgentId } from "../../agents/agent-scope.js";
 import { abortEmbeddedPiRun } from "../../agents/pi-embedded.js";
 import {
+  getLatestSubagentRunByChildSessionKey,
  listSubagentRunsForController,
  markSubagentRunTerminated,
 } from "../../agents/subagent-registry.js";
@@ -51,6 +52,7 @@ export {
 const defaultAbortDeps = {
  getAcpSessionManager,
  abortEmbeddedPiRun,
+  getLatestSubagentRunByChildSessionKey,
  listSubagentRunsForController,
  markSubagentRunTerminated,
 };
@@ -64,6 +66,9 @@ export const __testing = {
    abortDeps.getAcpSessionManager =
      deps?.getAcpSessionManager ?? defaultAbortDeps.getAcpSessionManager;
    abortDeps.abortEmbeddedPiRun = deps?.abortEmbeddedPiRun ?? defaultAbortDeps.abortEmbeddedPiRun;
+    abortDeps.getLatestSubagentRunByChildSessionKey =
+      deps?.getLatestSubagentRunByChildSessionKey ??
+      defaultAbortDeps.getLatestSubagentRunByChildSessionKey;
    abortDeps.listSubagentRunsForController =
      deps?.listSubagentRunsForController ?? defaultAbortDeps.listSubagentRunsForController;
    abortDeps.markSubagentRunTerminated =
@@ -72,6 +77,8 @@ export const __testing = {
  resetDepsForTests(): void {
    abortDeps.getAcpSessionManager = defaultAbortDeps.getAcpSessionManager;
    abortDeps.abortEmbeddedPiRun = defaultAbortDeps.abortEmbeddedPiRun;
+    abortDeps.getLatestSubagentRunByChildSessionKey =
+      defaultAbortDeps.getLatestSubagentRunByChildSessionKey;
    abortDeps.listSubagentRunsForController = defaultAbortDeps.listSubagentRunsForController;
    abortDeps.markSubagentRunTerminated = defaultAbortDeps.markSubagentRunTerminated;
  },
@@ -143,6 +150,12 @@ export function stopSubagentsForRequester(params: {
    if (!childKey) {
      continue;
    }
+    const latest = abortDeps.getLatestSubagentRunByChildSessionKey(childKey);
+    const latestControllerSessionKey =
+      latest?.controllerSessionKey?.trim() || latest?.requesterSessionKey?.trim();
+    if (!latest || latest.runId !== run.runId || latestControllerSessionKey !== requesterKey) {
+      continue;
+    }
    const existing = dedupedRunsByChildKey.get(childKey);
    if (!existing || run.createdAt >= existing.createdAt) {
      dedupedRunsByChildKey.set(childKey, run);