refactor(subagents): share run target resolution

2026-02-19 18:39:20 -05:00 · 2026-02-18 18:09:20 +00:00
parent 4e7182c4af
commit bec94449eb
4 changed files with 260 additions and 138 deletions
--- a/src/agents/tools/subagents-tool.ts
+++ b/src/agents/tools/subagents-tool.ts
@@ -1,6 +1,12 @@
 import crypto from "node:crypto";
 import { Type } from "@sinclair/typebox";
 import { clearSessionQueues } from "../../auto-reply/reply/queue.js";
+import {
+  resolveSubagentLabel,
+  resolveSubagentTargetFromRuns,
+  sortSubagentRuns,
+  type SubagentTargetResolution,
+} from "../../auto-reply/reply/subagents-utils.js";
 import { loadConfig } from "../../config/config.js";
 import type { SessionEntry } from "../../config/sessions.js";
 import { loadSessionStore, resolveStorePath, updateSessionStore } from "../../config/sessions.js";
@@ -63,16 +69,6 @@ type ResolvedRequesterKey = {
  callerIsSubagent: boolean;
 };

-type TargetResolution = {
-  entry?: SubagentRunRecord;
-  error?: string;
-};
-
-function resolveRunLabel(entry: SubagentRunRecord, fallback = "subagent") {
-  const raw = entry.label?.trim() || entry.task?.trim() || "";
-  return raw || fallback;
-}
-
 function resolveRunStatus(entry: SubagentRunRecord) {
  if (!entry.endedAt) {
    return "running";
@@ -87,14 +83,6 @@ function resolveRunStatus(entry: SubagentRunRecord) {
  return status;
 }

-function sortRuns(runs: SubagentRunRecord[]) {
-  return [...runs].toSorted((a, b) => {
-    const aTime = a.startedAt ?? a.createdAt ?? 0;
-    const bTime = b.startedAt ?? b.createdAt ?? 0;
-    return bTime - aTime;
-  });
-}
-
 function resolveModelRef(entry?: SessionEntry) {
  const model = typeof entry?.model === "string" ? entry.model.trim() : "";
  const provider = typeof entry?.modelProvider === "string" ? entry.modelProvider.trim() : "";
@@ -143,59 +131,22 @@ function resolveSubagentTarget(
  runs: SubagentRunRecord[],
  token: string | undefined,
  options?: { recentMinutes?: number },
-): TargetResolution {
-  const trimmed = token?.trim();
-  if (!trimmed) {
-    return { error: "Missing subagent target." };
-  }
-  const sorted = sortRuns(runs);
-  const recentMinutes = options?.recentMinutes ?? DEFAULT_RECENT_MINUTES;
-  const recentCutoff = Date.now() - recentMinutes * 60_000;
-  const numericOrder = [
-    ...sorted.filter((entry) => !entry.endedAt),
-    ...sorted.filter((entry) => !!entry.endedAt && (entry.endedAt ?? 0) >= recentCutoff),
-  ];
-  if (trimmed === "last") {
-    return { entry: sorted[0] };
-  }
-  if (/^\d+$/.test(trimmed)) {
-    const idx = Number.parseInt(trimmed, 10);
-    if (!Number.isFinite(idx) || idx <= 0 || idx > numericOrder.length) {
-      return { error: `Invalid subagent index: ${trimmed}` };
-    }
-    return { entry: numericOrder[idx - 1] };
-  }
-  if (trimmed.includes(":")) {
-    const bySessionKey = sorted.find((entry) => entry.childSessionKey === trimmed);
-    return bySessionKey
-      ? { entry: bySessionKey }
-      : { error: `Unknown subagent session: ${trimmed}` };
-  }
-  const lowered = trimmed.toLowerCase();
-  const byExactLabel = sorted.filter((entry) => resolveRunLabel(entry).toLowerCase() === lowered);
-  if (byExactLabel.length === 1) {
-    return { entry: byExactLabel[0] };
-  }
-  if (byExactLabel.length > 1) {
-    return { error: `Ambiguous subagent label: ${trimmed}` };
-  }
-  const byLabelPrefix = sorted.filter((entry) =>
-    resolveRunLabel(entry).toLowerCase().startsWith(lowered),
-  );
-  if (byLabelPrefix.length === 1) {
-    return { entry: byLabelPrefix[0] };
-  }
-  if (byLabelPrefix.length > 1) {
-    return { error: `Ambiguous subagent label prefix: ${trimmed}` };
-  }
-  const byRunIdPrefix = sorted.filter((entry) => entry.runId.startsWith(trimmed));
-  if (byRunIdPrefix.length === 1) {
-    return { entry: byRunIdPrefix[0] };
-  }
-  if (byRunIdPrefix.length > 1) {
-    return { error: `Ambiguous subagent run id prefix: ${trimmed}` };
-  }
-  return { error: `Unknown subagent target: ${trimmed}` };
+): SubagentTargetResolution {
+  return resolveSubagentTargetFromRuns({
+    runs,
+    token,
+    recentWindowMinutes: options?.recentMinutes ?? DEFAULT_RECENT_MINUTES,
+    label: (entry) => resolveSubagentLabel(entry),
+    errors: {
+      missingTarget: "Missing subagent target.",
+      invalidIndex: (value) => `Invalid subagent index: ${value}`,
+      unknownSession: (value) => `Unknown subagent session: ${value}`,
+      ambiguousLabel: (value) => `Ambiguous subagent label: ${value}`,
+      ambiguousLabelPrefix: (value) => `Ambiguous subagent label prefix: ${value}`,
+      ambiguousRunIdPrefix: (value) => `Ambiguous subagent run id prefix: ${value}`,
+      unknownTarget: (value) => `Unknown subagent target: ${value}`,
+    },
+  });
 }

 function resolveStorePathForKey(
@@ -346,7 +297,7 @@ async function cascadeKillChildren(params: {
      });
      if (stopResult.killed) {
        killed += 1;
-        labels.push(resolveRunLabel(run));
+        labels.push(resolveSubagentLabel(run));
      }
    }

@@ -401,7 +352,7 @@ export function createSubagentsTool(opts?: { agentSessionKey?: string }): AnyAge
        cfg,
        agentSessionKey: opts?.agentSessionKey,
      });
-      const runs = sortRuns(listSubagentRunsForRequester(requester.requesterSessionKey));
+      const runs = sortSubagentRuns(listSubagentRunsForRequester(requester.requesterSessionKey));
      const recentMinutesRaw = readNumberParam(params, "recentMinutes");
      const recentMinutes = recentMinutesRaw
        ? Math.max(1, Math.min(MAX_RECENT_MINUTES, Math.floor(recentMinutesRaw)))
@@ -423,7 +374,7 @@ export function createSubagentsTool(opts?: { agentSessionKey?: string }): AnyAge
          const usageText = formatTokenUsageDisplay(sessionEntry);
          const status = resolveRunStatus(entry);
          const runtime = formatDurationCompact(runtimeMs);
-          const label = truncateLine(resolveRunLabel(entry), 48);
+          const label = truncateLine(resolveSubagentLabel(entry), 48);
          const task = truncateLine(entry.task.trim(), 72);
          const line = `${index}. ${label} (${resolveModelDisplay(sessionEntry, entry.model)}, ${runtime}${usageText ? `, ${usageText}` : ""}) ${status}${task.toLowerCase() !== label.toLowerCase() ? ` - ${task}` : ""}`;
          const baseView = {
@@ -483,7 +434,7 @@ export function createSubagentsTool(opts?: { agentSessionKey?: string }): AnyAge
              const stopResult = await killSubagentRun({ cfg, entry, cache });
              if (stopResult.killed) {
                killed += 1;
-                killedLabels.push(resolveRunLabel(entry));
+                killedLabels.push(resolveSubagentLabel(entry));
              }
            }

@@ -543,7 +494,7 @@ export function createSubagentsTool(opts?: { agentSessionKey?: string }): AnyAge
            target,
            runId: resolved.entry.runId,
            sessionKey: resolved.entry.childSessionKey,
-            text: `${resolveRunLabel(resolved.entry)} is already finished.`,
+            text: `${resolveSubagentLabel(resolved.entry)} is already finished.`,
          });
        }
        const cascadeText =
@@ -556,12 +507,12 @@ export function createSubagentsTool(opts?: { agentSessionKey?: string }): AnyAge
          target,
          runId: resolved.entry.runId,
          sessionKey: resolved.entry.childSessionKey,
-          label: resolveRunLabel(resolved.entry),
+          label: resolveSubagentLabel(resolved.entry),
          cascadeKilled: cascade.killed,
          cascadeLabels: cascade.killed > 0 ? cascade.labels : undefined,
          text: stopResult.killed
-            ? `killed ${resolveRunLabel(resolved.entry)}${cascadeText}.`
-            : `killed ${cascade.killed} descendant${cascade.killed === 1 ? "" : "s"} of ${resolveRunLabel(resolved.entry)}.`,
+            ? `killed ${resolveSubagentLabel(resolved.entry)}${cascadeText}.`
+            : `killed ${cascade.killed} descendant${cascade.killed === 1 ? "" : "s"} of ${resolveSubagentLabel(resolved.entry)}.`,
        });
      }
      if (action === "steer") {
@@ -591,7 +542,7 @@ export function createSubagentsTool(opts?: { agentSessionKey?: string }): AnyAge
            target,
            runId: resolved.entry.runId,
            sessionKey: resolved.entry.childSessionKey,
-            text: `${resolveRunLabel(resolved.entry)} is already finished.`,
+            text: `${resolveSubagentLabel(resolved.entry)} is already finished.`,
          });
        }
        if (
@@ -714,8 +665,8 @@ export function createSubagentsTool(opts?: { agentSessionKey?: string }): AnyAge
          sessionKey: resolved.entry.childSessionKey,
          sessionId,
          mode: "restart",
-          label: resolveRunLabel(resolved.entry),
-          text: `steered ${resolveRunLabel(resolved.entry)}.`,
+          label: resolveSubagentLabel(resolved.entry),
+          text: `steered ${resolveSubagentLabel(resolved.entry)}.`,
        });
      }
      return jsonResult({
--- a/src/auto-reply/reply/commands-subagents.ts
+++ b/src/auto-reply/reply/commands-subagents.ts
@@ -37,12 +37,13 @@ import { INTERNAL_MESSAGE_CHANNEL } from "../../utils/message-channel.js";
 import { stopSubagentsForRequester } from "./abort.js";
 import type { CommandHandler } from "./commands-types.js";
 import { clearSessionQueues } from "./queue.js";
-import { formatRunLabel, formatRunStatus, sortSubagentRuns } from "./subagents-utils.js";
-
-type SubagentTargetResolution = {
-  entry?: SubagentRunRecord;
-  error?: string;
-};
+import {
+  formatRunLabel,
+  formatRunStatus,
+  resolveSubagentTargetFromRuns,
+  type SubagentTargetResolution,
+  sortSubagentRuns,
+} from "./subagents-utils.js";

 const COMMAND = "/subagents";
 const COMMAND_KILL = "/kill";
@@ -138,56 +139,21 @@ function resolveSubagentTarget(
  runs: SubagentRunRecord[],
  token: string | undefined,
 ): SubagentTargetResolution {
-  const trimmed = token?.trim();
-  if (!trimmed) {
-    return { error: "Missing subagent id." };
-  }
-  if (trimmed === "last") {
-    const sorted = sortSubagentRuns(runs);
-    return { entry: sorted[0] };
-  }
-  const sorted = sortSubagentRuns(runs);
-  const recentCutoff = Date.now() - RECENT_WINDOW_MINUTES * 60_000;
-  const numericOrder = [
-    ...sorted.filter((entry) => !entry.endedAt),
-    ...sorted.filter((entry) => !!entry.endedAt && (entry.endedAt ?? 0) >= recentCutoff),
-  ];
-  if (/^\d+$/.test(trimmed)) {
-    const idx = Number.parseInt(trimmed, 10);
-    if (!Number.isFinite(idx) || idx <= 0 || idx > numericOrder.length) {
-      return { error: `Invalid subagent index: ${trimmed}` };
-    }
-    return { entry: numericOrder[idx - 1] };
-  }
-  if (trimmed.includes(":")) {
-    const match = runs.find((entry) => entry.childSessionKey === trimmed);
-    return match ? { entry: match } : { error: `Unknown subagent session: ${trimmed}` };
-  }
-  const lowered = trimmed.toLowerCase();
-  const byLabel = runs.filter((entry) => formatRunLabel(entry).toLowerCase() === lowered);
-  if (byLabel.length === 1) {
-    return { entry: byLabel[0] };
-  }
-  if (byLabel.length > 1) {
-    return { error: `Ambiguous subagent label: ${trimmed}` };
-  }
-  const byLabelPrefix = runs.filter((entry) =>
-    formatRunLabel(entry).toLowerCase().startsWith(lowered),
-  );
-  if (byLabelPrefix.length === 1) {
-    return { entry: byLabelPrefix[0] };
-  }
-  if (byLabelPrefix.length > 1) {
-    return { error: `Ambiguous subagent label prefix: ${trimmed}` };
-  }
-  const byRunId = runs.filter((entry) => entry.runId.startsWith(trimmed));
-  if (byRunId.length === 1) {
-    return { entry: byRunId[0] };
-  }
-  if (byRunId.length > 1) {
-    return { error: `Ambiguous run id prefix: ${trimmed}` };
-  }
-  return { error: `Unknown subagent id: ${trimmed}` };
+  return resolveSubagentTargetFromRuns({
+    runs,
+    token,
+    recentWindowMinutes: RECENT_WINDOW_MINUTES,
+    label: (entry) => formatRunLabel(entry),
+    errors: {
+      missingTarget: "Missing subagent id.",
+      invalidIndex: (value) => `Invalid subagent index: ${value}`,
+      unknownSession: (value) => `Unknown subagent session: ${value}`,
+      ambiguousLabel: (value) => `Ambiguous subagent label: ${value}`,
+      ambiguousLabelPrefix: (value) => `Ambiguous subagent label prefix: ${value}`,
+      ambiguousRunIdPrefix: (value) => `Ambiguous run id prefix: ${value}`,
+      unknownTarget: (value) => `Unknown subagent id: ${value}`,
+    },
+  });
 }

 function buildSubagentsHelp() {
--- a/src/auto-reply/reply/subagents-utils.test.ts
+++ b/src/auto-reply/reply/subagents-utils.test.ts
@@ -0,0 +1,132 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import type { SubagentRunRecord } from "../../agents/subagent-registry.js";
+import {
+  resolveSubagentLabel,
+  resolveSubagentTargetFromRuns,
+  sortSubagentRuns,
+} from "./subagents-utils.js";
+
+const NOW_MS = 1_700_000_000_000;
+
+function makeRun(overrides: Partial<SubagentRunRecord>): SubagentRunRecord {
+  const id = overrides.runId ?? "run-default";
+  return {
+    runId: id,
+    childSessionKey: overrides.childSessionKey ?? `agent:main:subagent:${id}`,
+    requesterSessionKey: overrides.requesterSessionKey ?? "agent:main:main",
+    requesterDisplayKey: overrides.requesterDisplayKey ?? "main",
+    task: overrides.task ?? "default task",
+    cleanup: overrides.cleanup ?? "keep",
+    createdAt: overrides.createdAt ?? NOW_MS - 2_000,
+    ...overrides,
+  };
+}
+
+function resolveTarget(runs: SubagentRunRecord[], token: string | undefined) {
+  return resolveSubagentTargetFromRuns({
+    runs,
+    token,
+    recentWindowMinutes: 30,
+    label: (entry) => resolveSubagentLabel(entry),
+    errors: {
+      missingTarget: "missing",
+      invalidIndex: (value) => `invalid:${value}`,
+      unknownSession: (value) => `unknown-session:${value}`,
+      ambiguousLabel: (value) => `ambiguous-label:${value}`,
+      ambiguousLabelPrefix: (value) => `ambiguous-prefix:${value}`,
+      ambiguousRunIdPrefix: (value) => `ambiguous-run:${value}`,
+      unknownTarget: (value) => `unknown:${value}`,
+    },
+  });
+}
+
+describe("subagents utils", () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it("resolves subagent label with fallback", () => {
+    expect(resolveSubagentLabel(makeRun({ label: "  runner " }))).toBe("runner");
+    expect(resolveSubagentLabel(makeRun({ label: " ", task: "  task value " }))).toBe("task value");
+    expect(resolveSubagentLabel(makeRun({ label: " ", task: " " }), "fallback")).toBe("fallback");
+  });
+
+  it("sorts by startedAt then createdAt descending", () => {
+    const sorted = sortSubagentRuns([
+      makeRun({ runId: "a", createdAt: 10 }),
+      makeRun({ runId: "b", startedAt: 15, createdAt: 5 }),
+      makeRun({ runId: "c", startedAt: 12, createdAt: 20 }),
+    ]);
+    expect(sorted.map((entry) => entry.runId)).toEqual(["b", "c", "a"]);
+  });
+
+  it("selects last from sorted runs", () => {
+    const runs = [
+      makeRun({ runId: "old", createdAt: NOW_MS - 2_000 }),
+      makeRun({ runId: "new", createdAt: NOW_MS - 500 }),
+    ];
+    const resolved = resolveTarget(runs, " last ");
+    expect(resolved.entry?.runId).toBe("new");
+  });
+
+  it("resolves numeric index from running then recent finished order", () => {
+    vi.spyOn(Date, "now").mockReturnValue(NOW_MS);
+    const runs = [
+      makeRun({
+        runId: "running",
+        label: "running",
+        createdAt: NOW_MS - 8_000,
+      }),
+      makeRun({
+        runId: "recent-finished",
+        label: "recent",
+        createdAt: NOW_MS - 6_000,
+        endedAt: NOW_MS - 60_000,
+      }),
+      makeRun({
+        runId: "old-finished",
+        label: "old",
+        createdAt: NOW_MS - 7_000,
+        endedAt: NOW_MS - 2 * 60 * 60 * 1_000,
+      }),
+    ];
+
+    expect(resolveTarget(runs, "1").entry?.runId).toBe("running");
+    expect(resolveTarget(runs, "2").entry?.runId).toBe("recent-finished");
+    expect(resolveTarget(runs, "3").error).toBe("invalid:3");
+  });
+
+  it("resolves session key target and unknown session errors", () => {
+    const run = makeRun({ runId: "abc123", childSessionKey: "agent:beta:subagent:xyz" });
+    expect(resolveTarget([run], "agent:beta:subagent:xyz").entry?.runId).toBe("abc123");
+    expect(resolveTarget([run], "agent:beta:subagent:missing").error).toBe(
+      "unknown-session:agent:beta:subagent:missing",
+    );
+  });
+
+  it("resolves exact label, prefix, run-id prefix and ambiguity errors", () => {
+    const runs = [
+      makeRun({ runId: "run-alpha-1", label: "Alpha Core" }),
+      makeRun({ runId: "run-alpha-2", label: "Alpha Orbit" }),
+      makeRun({ runId: "run-beta-1", label: "Beta Worker" }),
+    ];
+
+    expect(resolveTarget(runs, "beta worker").entry?.runId).toBe("run-beta-1");
+    expect(resolveTarget(runs, "beta").entry?.runId).toBe("run-beta-1");
+    expect(resolveTarget(runs, "run-beta").entry?.runId).toBe("run-beta-1");
+
+    expect(resolveTarget(runs, "alpha core").entry?.runId).toBe("run-alpha-1");
+    expect(resolveTarget(runs, "alpha").error).toBe("ambiguous-prefix:alpha");
+    expect(resolveTarget(runs, "run-alpha").error).toBe("ambiguous-run:run-alpha");
+    expect(resolveTarget(runs, "missing").error).toBe("unknown:missing");
+    expect(resolveTarget(runs, undefined).error).toBe("missing");
+  });
+
+  it("returns ambiguous exact label error before prefix/run id matching", () => {
+    const runs = [
+      makeRun({ runId: "run-a", label: "dup" }),
+      makeRun({ runId: "run-b", label: "dup" }),
+    ];
+    expect(resolveTarget(runs, "dup").error).toBe("ambiguous-label:dup");
+  });
+});
--- a/src/auto-reply/reply/subagents-utils.ts
+++ b/src/auto-reply/reply/subagents-utils.ts
@@ -30,3 +30,76 @@ export function sortSubagentRuns(runs: SubagentRunRecord[]) {
    return bTime - aTime;
  });
 }
+
+export type SubagentTargetResolution = {
+  entry?: SubagentRunRecord;
+  error?: string;
+};
+
+export function resolveSubagentTargetFromRuns(params: {
+  runs: SubagentRunRecord[];
+  token: string | undefined;
+  recentWindowMinutes: number;
+  label: (entry: SubagentRunRecord) => string;
+  errors: {
+    missingTarget: string;
+    invalidIndex: (value: string) => string;
+    unknownSession: (value: string) => string;
+    ambiguousLabel: (value: string) => string;
+    ambiguousLabelPrefix: (value: string) => string;
+    ambiguousRunIdPrefix: (value: string) => string;
+    unknownTarget: (value: string) => string;
+  };
+}): SubagentTargetResolution {
+  const trimmed = params.token?.trim();
+  if (!trimmed) {
+    return { error: params.errors.missingTarget };
+  }
+  const sorted = sortSubagentRuns(params.runs);
+  if (trimmed === "last") {
+    return { entry: sorted[0] };
+  }
+  const recentCutoff = Date.now() - params.recentWindowMinutes * 60_000;
+  const numericOrder = [
+    ...sorted.filter((entry) => !entry.endedAt),
+    ...sorted.filter((entry) => !!entry.endedAt && (entry.endedAt ?? 0) >= recentCutoff),
+  ];
+  if (/^\d+$/.test(trimmed)) {
+    const idx = Number.parseInt(trimmed, 10);
+    if (!Number.isFinite(idx) || idx <= 0 || idx > numericOrder.length) {
+      return { error: params.errors.invalidIndex(trimmed) };
+    }
+    return { entry: numericOrder[idx - 1] };
+  }
+  if (trimmed.includes(":")) {
+    const bySessionKey = sorted.find((entry) => entry.childSessionKey === trimmed);
+    return bySessionKey
+      ? { entry: bySessionKey }
+      : { error: params.errors.unknownSession(trimmed) };
+  }
+  const lowered = trimmed.toLowerCase();
+  const byExactLabel = sorted.filter((entry) => params.label(entry).toLowerCase() === lowered);
+  if (byExactLabel.length === 1) {
+    return { entry: byExactLabel[0] };
+  }
+  if (byExactLabel.length > 1) {
+    return { error: params.errors.ambiguousLabel(trimmed) };
+  }
+  const byLabelPrefix = sorted.filter((entry) =>
+    params.label(entry).toLowerCase().startsWith(lowered),
+  );
+  if (byLabelPrefix.length === 1) {
+    return { entry: byLabelPrefix[0] };
+  }
+  if (byLabelPrefix.length > 1) {
+    return { error: params.errors.ambiguousLabelPrefix(trimmed) };
+  }
+  const byRunIdPrefix = sorted.filter((entry) => entry.runId.startsWith(trimmed));
+  if (byRunIdPrefix.length === 1) {
+    return { entry: byRunIdPrefix[0] };
+  }
+  if (byRunIdPrefix.length > 1) {
+    return { error: params.errors.ambiguousRunIdPrefix(trimmed) };
+  }
+  return { error: params.errors.unknownTarget(trimmed) };
+}