From bec94449eb419b65074f8aae62851020e34e93e1 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 18 Feb 2026 18:09:20 +0000 Subject: [PATCH] refactor(subagents): share run target resolution --- src/agents/tools/subagents-tool.ts | 115 +++++----------- src/auto-reply/reply/commands-subagents.ts | 78 ++++------- src/auto-reply/reply/subagents-utils.test.ts | 132 +++++++++++++++++++ src/auto-reply/reply/subagents-utils.ts | 73 ++++++++++ 4 files changed, 260 insertions(+), 138 deletions(-) create mode 100644 src/auto-reply/reply/subagents-utils.test.ts diff --git a/src/agents/tools/subagents-tool.ts b/src/agents/tools/subagents-tool.ts index d3e1cca617..bf88212d6a 100644 --- 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({ diff --git a/src/auto-reply/reply/commands-subagents.ts b/src/auto-reply/reply/commands-subagents.ts index 25cc41bdff..3bc99467e2 100644 --- 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() { diff --git a/src/auto-reply/reply/subagents-utils.test.ts b/src/auto-reply/reply/subagents-utils.test.ts new file mode 100644 index 0000000000..8a25428314 --- /dev/null +++ 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 { + 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"); + }); +}); diff --git a/src/auto-reply/reply/subagents-utils.ts b/src/auto-reply/reply/subagents-utils.ts index 1c5ecba118..a83c424bd2 100644 --- 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) }; +}