From c5828e059107ab3095227d1b54537cb742293379 Mon Sep 17 00:00:00 2001 From: alexion Date: Sat, 1 Aug 2026 22:24:04 -0400 Subject: [PATCH] feat(pi): label subagent work items --- .../pi/extensions/subagents/config.test.ts | 3 +- .../agents/pi/extensions/subagents/index.ts | 14 ++- .../pi/extensions/subagents/runner.test.ts | 6 +- .../agents/pi/extensions/subagents/runner.ts | 2 +- .../agents/pi/extensions/subagents/status.ts | 19 +++- .../extensions/subagents/supervisor.test.ts | 86 ++++++++++++++++++- .../pi/extensions/subagents/supervisor.ts | 36 ++++++-- .../agents/pi/extensions/subagents/types.ts | 16 ++-- modules/agents/pi/extensions/subagents/ui.ts | 3 +- 9 files changed, 157 insertions(+), 28 deletions(-) diff --git a/modules/agents/pi/extensions/subagents/config.test.ts b/modules/agents/pi/extensions/subagents/config.test.ts index 679cc91..2acda97 100644 --- a/modules/agents/pi/extensions/subagents/config.test.ts +++ b/modules/agents/pi/extensions/subagents/config.test.ts @@ -115,9 +115,10 @@ test("named spawn resolves overrides, frontmatter, config, and defaults", () => const config = loadConfig(cwd, true, diag, agentDir); const agents = loadAgents(cwd, true, diag, agentDir); - const resolved = resolveSpawn({ agent: "review", prompt: "check this", thinking: "low" }, config, agents); + const resolved = resolveSpawn({ agent: "review", prompt: "check this", label: "Review migration", thinking: "low" }, config, agents); assert.equal(resolved.prompt, "check this"); + assert.equal(resolved.label, "Review migration"); assert.equal(resolved.context, "independent"); assert.equal(resolved.model, "inherit"); assert.equal(resolved.thinking, "low"); diff --git a/modules/agents/pi/extensions/subagents/index.ts b/modules/agents/pi/extensions/subagents/index.ts index bb194e1..47d0600 100644 --- a/modules/agents/pi/extensions/subagents/index.ts +++ b/modules/agents/pi/extensions/subagents/index.ts @@ -4,6 +4,7 @@ import { loadAgents } from "./agents.ts"; import { loadConfig, resolveSpawn, type Diagnostics } from "./config.ts"; import { SubprocessRpcRunner } from "./runner.ts"; import { Supervisor } from "./supervisor.ts"; +import { milestoneNotification } from "./status.ts"; import type { SpawnRequest, SubagentStatus } from "./types.ts"; import { widget } from "./ui.ts"; @@ -23,7 +24,11 @@ export default function subagents(pi: ExtensionAPI) { supervisor = new Supervisor(new SubprocessRpcRunner(), cwd, { maxConcurrent: config.maxConcurrent, recentTerminalTtlMs: config.recentTerminalTtlMs, - onMilestone: (status, event) => pi.appendEntry("subagent_milestone", { event, status }), + onMilestone: (status, event) => { + pi.appendEntry("subagent_milestone", { event, status }); + const notification = milestoneNotification(status, event); + if (notification) ctx.ui?.notify?.(notification.message, notification.level); + }, onChange: (statuses) => { lastStatuses = statuses; updateUi(ctx, config.ui.enabled); @@ -51,6 +56,7 @@ export default function subagents(pi: ExtensionAPI) { description: "Start one ad hoc independent subagent and return immediately with its child id", parameters: Type.Object({ prompt: Type.String({ description: "Prompt for the delegated subagent" }), + label: Type.Optional(Type.String({ description: "Human-readable label for this work item" })), agent: Type.Optional(Type.String({ description: "Named agent definition to use" })), context: Type.Optional(Type.Union([Type.Literal("independent"), Type.Literal("fork")])), model: Type.Optional(Type.String({ description: "Optional model selector for the child" })), @@ -59,7 +65,7 @@ export default function subagents(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { const accepted = getSupervisor(ctx).spawn(resolve(ctx, params as SpawnRequest)); - ctx.ui?.notify?.(`Started subagent ${accepted.id}`, "info"); + ctx.ui?.notify?.(`Started subagent ${accepted.label}`, "info"); return textResult(accepted); }, }); @@ -72,6 +78,7 @@ export default function subagents(pi: ExtensionAPI) { subagents: Type.Array( Type.Object({ prompt: Type.String({ description: "Prompt for the delegated subagent" }), + label: Type.Optional(Type.String({ description: "Human-readable label for this work item" })), agent: Type.Optional(Type.String({ description: "Named agent definition to use" })), context: Type.Optional(Type.Union([Type.Literal("independent"), Type.Literal("fork")])), model: Type.Optional(Type.String({ description: "Optional model selector for the child" })), @@ -164,7 +171,7 @@ export default function subagents(pi: ExtensionAPI) { description: "Start an ad hoc independent subagent", handler: async (args, ctx) => { const accepted = getSupervisor(ctx).spawn(resolve(ctx, parseSpawnArgs(args))); - ctx.ui.notify(`Started subagent ${accepted.id}`, "info"); + ctx.ui.notify(`Started subagent ${accepted.label}`, "info"); }, }); @@ -250,6 +257,7 @@ function parseSpawnArgs(args: string): SpawnRequest { const flag = parts.shift(); const value = parts.shift(); if (flag === "--agent") request.agent = value; + else if (flag === "--label") request.label = value; else if (flag === "--context" && (value === "independent" || value === "fork")) request.context = value; else if (flag === "--tools") request.tools = value; else if (flag === "--model") request.model = value; diff --git a/modules/agents/pi/extensions/subagents/runner.test.ts b/modules/agents/pi/extensions/subagents/runner.test.ts index cd5ca2f..4a13680 100644 --- a/modules/agents/pi/extensions/subagents/runner.test.ts +++ b/modules/agents/pi/extensions/subagents/runner.test.ts @@ -51,14 +51,18 @@ test("child RPC process disables discovery while explicitly loading subagents ex const { SubprocessRpcRunner } = await import("./runner.ts"); const runner = new SubprocessRpcRunner(); - await runner.start("child-1", { prompt: "work" }, "/tmp", events()); + await runner.start("child-1", { prompt: "work", label: "Review migration" }, "/tmp", events()); assert.equal(spawn.mock.callCount(), 1); const args = calls[0].args; const noExtensionsIndex = args.indexOf("--no-extensions"); const extensionIndex = args.indexOf("--extension"); + const nameIndex = args.indexOf("--name"); + assert.notEqual(noExtensionsIndex, -1, "child args keep automatic extension discovery disabled"); + assert.notEqual(nameIndex, -1, "child args include a process name"); + assert.equal(args[nameIndex + 1], "subagent Review migration"); assert.notEqual(extensionIndex, -1, "child args explicitly load the subagents extension entry"); assert.equal(args[extensionIndex + 1], fileURLToPath(new URL("./index.ts", import.meta.url))); assert.ok(noExtensionsIndex < extensionIndex); diff --git a/modules/agents/pi/extensions/subagents/runner.ts b/modules/agents/pi/extensions/subagents/runner.ts index 7804f7b..5291f0f 100644 --- a/modules/agents/pi/extensions/subagents/runner.ts +++ b/modules/agents/pi/extensions/subagents/runner.ts @@ -157,7 +157,7 @@ class RpcChildHandle implements ChildHandle { export class SubprocessRpcRunner implements ChildRunner { async start(id: string, request: SpawnRequest, cwd: string, events: RunnerEvents): Promise { - const args = [process.argv[1], "--mode", "rpc", "--no-extensions", "--extension", subagentsExtensionPath(), "--name", `subagent ${id}`, ...contextArgs(request), ...toolArgs(request), ...modelArgs(request)]; + const args = [process.argv[1], "--mode", "rpc", "--no-extensions", "--extension", subagentsExtensionPath(), "--name", `subagent ${request.label ?? id}`, ...contextArgs(request), ...toolArgs(request), ...modelArgs(request)]; const child = spawn(process.execPath, args, { cwd, env: childEnvironment(), diff --git a/modules/agents/pi/extensions/subagents/status.ts b/modules/agents/pi/extensions/subagents/status.ts index 76df5e7..1831b4e 100644 --- a/modules/agents/pi/extensions/subagents/status.ts +++ b/modules/agents/pi/extensions/subagents/status.ts @@ -1,4 +1,5 @@ -import type { ChildRecord, SpawnAccepted, SubagentResult, SubagentStatus } from "./types.ts"; +import { SUBAGENT_STATES, SUBAGENT_TERMINAL_STATES } from "./types.ts"; +import type { ChildRecord, SpawnAccepted, SubagentResult, SubagentState, SubagentStatus } from "./types.ts"; export function toAccepted(status: SubagentStatus): SpawnAccepted { return { @@ -17,9 +18,10 @@ export function cloneStatus(status: SubagentStatus): SubagentStatus { export function cloneResult(record: ChildRecord): SubagentResult { const status = cloneStatus(record.status); - const terminal = ["completed", "failed", "cancelled", "timed_out", "orphaned"].includes(status.state); + const terminal = isTerminalState(status.state); return { id: status.id, + label: status.label, state: status.state, running: !terminal, resultAvailable: status.resultAvailable, @@ -30,6 +32,19 @@ export function cloneResult(record: ChildRecord): SubagentResult { }; } +export function isTerminalState(state: SubagentState): boolean { + return (SUBAGENT_TERMINAL_STATES as readonly string[]).includes(state); +} + +export function milestoneNotification(status: SubagentStatus, event: string): { message: string; level: "info" | "error" } | undefined { + if (!isSubagentState(event) || !isTerminalState(event)) return undefined; + return { message: `Subagent ${status.label} ${event}`, level: event === "completed" ? "info" : "error" }; +} + +export function isSubagentState(value: string): value is SubagentState { + return (SUBAGENT_STATES as readonly string[]).includes(value); +} + export function elapsedMs(status: Pick): number { const start = Date.parse(status.startedAt); const end = status.completedAt ? Date.parse(status.completedAt) : Date.now(); diff --git a/modules/agents/pi/extensions/subagents/supervisor.test.ts b/modules/agents/pi/extensions/subagents/supervisor.test.ts index edaca0c..8b1e69b 100644 --- a/modules/agents/pi/extensions/subagents/supervisor.test.ts +++ b/modules/agents/pi/extensions/subagents/supervisor.test.ts @@ -1,7 +1,9 @@ import assert from "node:assert/strict"; import test from "node:test"; +import { milestoneNotification } from "./status.ts"; import { Supervisor } from "./supervisor.ts"; import type { ChildHandle, ChildRunner, RunnerEvents, SpawnRequest } from "./types.ts"; +import { widget } from "./ui.ts"; class FakeHandle implements ChildHandle { cancelCalls = 0; @@ -110,6 +112,77 @@ test("completed children ignore later cancel", async () => { assert.equal(runner.starts[0].handle.cancelCalls, 0); }); +test("explicit labels are reused across accepted status list and result surfaces", async () => { + const runner = new FakeRunner(); + const supervisor = new Supervisor(runner, "/tmp"); + const label = "Review risky migration"; + + const accepted = supervisor.spawn({ prompt: "inspect the migration plan", label } as SpawnRequest & { label: string }); + await sleep(0); + runner.starts[0].events.completed("done", "agent_settled"); + + assert.deepEqual( + { + accepted: accepted.label, + status: supervisor.status(accepted.id).label, + list: supervisor.list().find((status) => status.id === accepted.id)?.label, + result: (supervisor.result(accepted.id) as { label?: string }).label, + }, + { + accepted: label, + status: label, + list: label, + result: label, + }, + ); +}); + +test("ad hoc fallback labels are prompt-derived and reused by widget and result surfaces", async () => { + const runner = new FakeRunner(); + const supervisor = new Supervisor(runner, "/tmp"); + const prompt = " Audit\n\tguest enablement plan "; + const label = "Audit guest enablement plan"; + + const accepted = supervisor.spawn({ prompt }); + await sleep(0); + runner.starts[0].events.completed("done", "agent_settled"); + const statuses = supervisor.list(); + const inspectorLines = widget(statuses, true)().render(240); + + assert.deepEqual( + { + accepted: accepted.label, + childRequest: runner.starts[0].request.label, + status: supervisor.status(accepted.id).label, + list: statuses.find((status) => status.id === accepted.id)?.label, + result: supervisor.result(accepted.id).label, + }, + { + accepted: label, + childRequest: label, + status: label, + list: label, + result: label, + }, + ); + assert.ok(inspectorLines.some((line) => line.includes(`${accepted.id} ${label} independent completed`)), inspectorLines.join("\n")); + assert.doesNotMatch(accepted.label, /^ad-hoc sg-/u); +}); + +test("milestone notifications use the stored label", async () => { + const runner = new FakeRunner(); + const supervisor = new Supervisor(runner, "/tmp"); + const accepted = supervisor.spawn({ prompt: "work", label: "Review migration" }); + await sleep(0); + runner.starts[0].events.completed("done", "agent_settled"); + + assert.deepEqual(milestoneNotification(supervisor.status(accepted.id), "completed"), { + message: "Subagent Review migration completed", + level: "info", + }); + assert.equal(milestoneNotification(supervisor.status(accepted.id), "running"), undefined); +}); + test("shutdown clears recent terminal expiry timer", async () => { const runner = new FakeRunner(); let changes = 0; @@ -128,17 +201,22 @@ test("shutdown clears recent terminal expiry timer", async () => { assert.equal(changes, afterShutdown); }); -test("batch spawn returns accepted ids and per-entry failures", async () => { +test("batch spawn returns explicit labels on accepted child requests and statuses while preserving failures", async () => { const runner = new FakeRunner(); const supervisor = new Supervisor(runner, "/tmp"); - const result = supervisor.spawnBatch([{ prompt: "one" }, { prompt: "" }, { prompt: "two" }]); + const result = supervisor.spawnBatch([ + { prompt: "one", label: "Review docs" }, + { prompt: "" }, + { prompt: "two", label: "Check tests" }, + ]); await sleep(0); - assert.equal(result.accepted.length, 2); + assert.deepEqual(result.accepted.map((accepted) => accepted.label), ["Review docs", "Check tests"]); assert.equal(result.failed.length, 1); assert.equal(result.failed[0].index, 1); - assert.equal(runner.starts.length, 2); + assert.deepEqual(runner.starts.map((start) => start.request.label), ["Review docs", "Check tests"]); + assert.deepEqual(result.accepted.map((accepted) => supervisor.status(accepted.id).label), ["Review docs", "Check tests"]); }); test("maxConcurrent preserves queued records", async () => { diff --git a/modules/agents/pi/extensions/subagents/supervisor.ts b/modules/agents/pi/extensions/subagents/supervisor.ts index 702aa46..75a9b08 100644 --- a/modules/agents/pi/extensions/subagents/supervisor.ts +++ b/modules/agents/pi/extensions/subagents/supervisor.ts @@ -11,7 +11,7 @@ import type { SubagentWaitMode, SubagentWaitResult, } from "./types.ts"; -import { cloneResult, cloneStatus, toAccepted } from "./status.ts"; +import { cloneResult, cloneStatus, isTerminalState, toAccepted } from "./status.ts"; interface RunningChild { record: ChildRecord; @@ -157,7 +157,7 @@ export class Supervisor { const now = new Date().toISOString(); const status: SubagentStatus = { id, - label: request.agent ?? `ad-hoc ${id}`, + label: deriveLabel(request, id), agent: request.agent, adHoc: !request.agent, context: this.resolveContext(request.context), @@ -172,7 +172,7 @@ export class Supervisor { lastEventAt: now, resultAvailable: false, }; - const child: RunningChild = { record: { status }, request: { ...request, prompt, context: status.context, tools: status.tools } }; + const child: RunningChild = { record: { status }, request: { ...request, prompt, label: status.label, context: status.context, tools: status.tools } }; this.children.set(id, child); this.emitMilestone(child, "accepted"); this.queue.push(child); @@ -418,6 +418,32 @@ export class Supervisor { } } -function isTerminal(state: SubagentStatus["state"]): boolean { - return ["completed", "failed", "cancelled", "timed_out", "orphaned"].includes(state); +function deriveLabel(request: SpawnRequest, id: string): string { + const explicit = normalizeLabel(request.label); + if (explicit) return explicit; + const agent = normalizeLabel(request.agent); + if (agent) return agent; + return promptLabel(request.prompt) ?? `ad-hoc ${id}`; +} + +function promptLabel(prompt: string): string | undefined { + const normalized = normalizeLabel(prompt); + if (!normalized) return undefined; + return truncateLabel(normalized); +} + +function normalizeLabel(value: unknown): string | undefined { + if (typeof value !== "string") return undefined; + const normalized = value.replace(/\s+/gu, " ").trim(); + return normalized || undefined; +} + +function truncateLabel(label: string): string { + const maxLength = 80; + if (label.length <= maxLength) return label; + return `${label.slice(0, maxLength - 1).trimEnd()}…`; +} + +function isTerminal(state: SubagentStatus["state"]): boolean { + return isTerminalState(state); } diff --git a/modules/agents/pi/extensions/subagents/types.ts b/modules/agents/pi/extensions/subagents/types.ts index 14a5143..2274ccb 100644 --- a/modules/agents/pi/extensions/subagents/types.ts +++ b/modules/agents/pi/extensions/subagents/types.ts @@ -1,15 +1,9 @@ export type ContextMode = "independent" | "fork"; -export type SubagentState = - | "queued" - | "starting" - | "running" - | "settling" - | "completed" - | "failed" - | "cancelled" - | "timed_out" - | "orphaned"; +export const SUBAGENT_STATES = ["queued", "starting", "running", "settling", "completed", "failed", "cancelled", "timed_out", "orphaned"] as const; +export const SUBAGENT_TERMINAL_STATES = ["completed", "failed", "cancelled", "timed_out", "orphaned"] as const; + +export type SubagentState = (typeof SUBAGENT_STATES)[number]; export interface ToolProfile { activeTools: string[] | null; @@ -17,6 +11,7 @@ export interface ToolProfile { export interface SpawnRequest { prompt: string; + label?: string; context?: ContextMode; agent?: string; model?: string; @@ -60,6 +55,7 @@ export interface SubagentStatus { export interface SubagentResult { id: string; + label: string; state: SubagentState; running: boolean; resultAvailable: boolean; diff --git a/modules/agents/pi/extensions/subagents/ui.ts b/modules/agents/pi/extensions/subagents/ui.ts index b466096..aef20f7 100644 --- a/modules/agents/pi/extensions/subagents/ui.ts +++ b/modules/agents/pi/extensions/subagents/ui.ts @@ -1,9 +1,10 @@ +import { isTerminalState } from "./status.ts"; import type { SubagentStatus } from "./types.ts"; export function renderSummary(statuses: SubagentStatus[]): string[] { const running = statuses.filter((status) => ["starting", "running", "settling"].includes(status.state)).length; const queued = statuses.filter((status) => status.state === "queued").length; - const terminal = statuses.filter((status) => ["completed", "failed", "cancelled", "timed_out", "orphaned"].includes(status.state)).length; + const terminal = statuses.filter((status) => isTerminalState(status.state)).length; if (running === 0 && queued === 0 && terminal === 0) return []; return [`subagents: ${running} running · ${queued} queued · ${terminal} recent`]; }