Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 62 additions & 39 deletions extensions/subagents/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ import {
USAGE_STATE_ENTRY,
type Usage,
WEB_STATUS_PUBLISH_INTERVAL_MS,
DEFAULT_READ_WAIT_SECONDS,
} from "./types.js";
import {
addUsage,
Expand Down Expand Up @@ -373,9 +374,13 @@ export class SubagentManager {
agent.activity.splice(0, removed);
agent.lastReadActivity = Math.max(0, agent.lastReadActivity - removed);
}
this.publishFooter();
}

private wakeReadWaiters(agent: ManagedSubagent): void {
if (agent.waiters.size === 0) return;
for (const waiter of agent.waiters) waiter();
agent.waiters.clear();
this.publishFooter();
}

private addTranscript(agent: ManagedSubagent, message: unknown): void {
Expand Down Expand Up @@ -481,6 +486,12 @@ export class SubagentManager {
);
break;
case "agent_end":
// Don't wake waiters here: `willRetry: false` only means this particular run won't
// auto-retry - Pi can still continue with queued follow-ups before ever reaching a
// real terminal state, so waking now can hand back a still-"working" snapshot and
// force the caller to wait a full cycle again for the actual completion. The terminal
// transition (agent_settled below, or attachRun's own handlers) wakes waiters once the
// status has actually changed.
if (event.willRetry) this.activity(agent, "waiting to retry");
break;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
case "agent_settled":
Expand All @@ -494,6 +505,7 @@ export class SubagentManager {
? `failed${agent.error ? `: ${agent.error}` : ` (${agent.lastStopReason})`}`
: "completed and is waiting for more instructions",
);
this.wakeReadWaiters(agent);
}
break;
case "auto_retry_start":
Expand Down Expand Up @@ -531,6 +543,7 @@ export class SubagentManager {
agent.status = "completed";
agent.completedAt = Date.now();
this.activity(agent, "task run settled");
this.wakeReadWaiters(agent);
}
})
.catch((error: unknown) => {
Expand All @@ -540,6 +553,7 @@ export class SubagentManager {
agent.error = error instanceof Error ? error.message : String(error);
agent.completedAt = Date.now();
this.activity(agent, `failed: ${agent.error}`);
this.wakeReadWaiters(agent);
});
}

Expand Down Expand Up @@ -650,6 +664,7 @@ export class SubagentManager {
agent.error = error instanceof Error ? error.message : String(error);
agent.completedAt = Date.now();
this.activity(agent, `${agent.status}: ${agent.error}`);
this.wakeReadWaiters(agent);
throw error;
}
}
Expand Down Expand Up @@ -768,6 +783,7 @@ export class SubagentManager {
agent.status = "terminated";
agent.completedAt = Date.now();
this.activity(agent, "terminated and released session resources");
this.wakeReadWaiters(agent);
}
this.agents.delete(id);
this.webTranscriptCursors.delete(id);
Expand Down Expand Up @@ -807,21 +823,22 @@ export class SubagentManager {
return terminalAgents.length;
}

private hasUnread(agent: ManagedSubagent): boolean {
return agent.lastReadActivity < agent.activity.length;
}

async waitForUpdates(
agents: ManagedSubagent[],
seconds: number,
signal?: AbortSignal,
): Promise<void> {
if (seconds <= 0 || agents.some((agent) => this.hasUnread(agent))) return;
// No early-return on "any unread activity": `agent.activity` still grows on every routine
// event (tool start/end, throttled streaming text, queue updates), so that check would fire
// for almost any actively-working agent between two reads and skip the wait entirely,
// defeating the whole point of it. An already-terminal agent is instead caught below by
// `running.length === 0`, which is the actual "nothing worth waiting for" case.
const running = agents.filter(
(agent) => agent.status === "creating" || agent.status === "working",
);
if (running.length === 0) return;

const waitSeconds = Math.max(seconds, DEFAULT_READ_WAIT_SECONDS);
await new Promise<void>((done) => {
let finished = false;
const finish = () => {
Expand All @@ -832,44 +849,46 @@ export class SubagentManager {
signal?.removeEventListener("abort", finish);
done();
};
const timer = setTimeout(finish, Math.min(30, seconds) * 1_000);
const timer = setTimeout(finish, waitSeconds * 1_000);
for (const agent of running) agent.waiters.add(finish);
signal?.addEventListener("abort", finish, { once: true });
});
}

read(agents: ManagedSubagent[], includeTranscript: boolean): string {
private readSummary(agent: ManagedSubagent): string {
const now = Date.now();
const metadata = [
`Model: ${agent.model}`,
`Effort: ${agent.effort}`,
`Elapsed: ${formatDuration((agent.completedAt ?? now) - agent.createdAt)}`,
`Turns: ${agent.turns}`,
`Usage: ↑${formatTokens(agent.usage.input)} ↓${formatTokens(agent.usage.output)}${agent.usage.cost.total ? ` $${agent.usage.cost.total.toFixed(4)}` : ""}`,
];
if (agent.currentTool) metadata.push(`Current tool: ${agent.currentTool}`);
if (agent.queuedSteering || agent.queuedFollowUp) {
metadata.push(
`Queued: ${agent.queuedSteering} steering, ${agent.queuedFollowUp} follow-up`,
);
}
if (agent.error) metadata.push(`Error: ${agent.error}`);

if (!isTerminalSubagentStatus(agent.status)) {
return `${metadata.join("\n")}\n\nAwaiting completion before returning assistant output.`;
}

const latest = finalAssistantText(agent);
if (!latest) return `${metadata.join("\n")}\n\nCompletion summary: (no assistant output)`;
return `${metadata.join("\n")}\n\nCompletion summary:\n${truncateChars(latest, 3_000)}`;
}

async read(agents: ManagedSubagent[], includeTranscript: boolean): Promise<string> {
if (agents.length === 0)
return "No subagents are involved in this session.";
const now = Date.now();
const sections: string[] = [];
const terminalAgents: string[] = [];
for (const agent of agents) {
const heading = `## ${statusIcon(agent.status)} ${agent.id} — ${agent.status}`;
const metadata = [
`Model: ${agent.model}`,
`Effort: ${agent.effort}`,
`Elapsed: ${formatDuration((agent.completedAt ?? now) - agent.createdAt)}`,
`Turns: ${agent.turns}`,
`Usage: ↑${formatTokens(agent.usage.input)} ↓${formatTokens(agent.usage.output)}${agent.usage.cost.total ? ` $${agent.usage.cost.total.toFixed(4)}` : ""}`,
];
if (agent.currentTool)
metadata.push(`Current tool: ${agent.currentTool}`);
if (agent.queuedSteering || agent.queuedFollowUp) {
metadata.push(
`Queued: ${agent.queuedSteering} steering, ${agent.queuedFollowUp} follow-up`,
);
}
if (agent.error) metadata.push(`Error: ${agent.error}`);

const unread = agent.activity.slice(agent.lastReadActivity);
const activity = unread.length
? unread
.map((item) => `- ${formatClock(item.timestamp)} ${item.text}`)
.join("\n")
: "- No new activity.";
agent.lastReadActivity = agent.activity.length;

let output = `${heading}\n${metadata.join("\n")}\n\nActivity since last read:\n${activity}`;
let output = `${heading}\n${this.readSummary(agent)}`;
if (includeTranscript) {
const transcript = agent.transcript
.map(
Expand All @@ -878,14 +897,18 @@ export class SubagentManager {
)
.join("\n\n");
output += `\n\nTranscript:\n${transcript || agent.streamingText || "(empty)"}`;
} else {
const latest = finalAssistantText(agent);
if (latest) output += `\n\nLatest assistant output:\n${latest}`;
}
sections.push(output);
if (this.archivedAgents.get(agent.id) === agent)
this.archivedAgents.delete(agent.id);
agent.lastReadActivity = agent.activity.length;
if (isTerminalSubagentStatus(agent.status)) terminalAgents.push(agent.id);
}

// Removing an archived agent from `archivedAgents` here (before terminate() runs) would
// make it unresolvable by id - `terminate(id, true)` looks the agent up via `getAgent`
// first and only then removes it, so let it own that removal instead.
if (terminalAgents.length > 0)
await Promise.all(terminalAgents.map((id) => this.terminate(id, true)));
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return truncateToolOutput(sections.join("\n\n---\n\n"));
}

Expand Down
10 changes: 5 additions & 5 deletions extensions/subagents/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ const ReadParams = Type.Object({
),
wait_seconds: Type.Optional(
Type.Integer({
description: `Wait for meaningful new activity before returning. Default ${DEFAULT_READ_WAIT_SECONDS}, maximum 30.`,
minimum: 0,
description: `Wait for meaningful subagent state changes before returning. Default and minimum ${DEFAULT_READ_WAIT_SECONDS}.`,
minimum: DEFAULT_READ_WAIT_SECONDS,
maximum: 30,
}),
),
Expand Down Expand Up @@ -134,7 +134,7 @@ export function registerSubagentTools(
"Create a background subagent with a chosen prompt, model, and effort",
promptGuidelines: [
"When calling subagent_create, omit model to inherit the current model unless deliberately choosing one of the exact session-available provider/model IDs listed in the system prompt; never shorten or invent a model ID.",
"After subagent_create returns, use subagent_read with its default wait roughly every 15–30 seconds while work continues; briefly tell the user about meaningful progress between polls without narrating every event.",
"After subagent_create returns, use subagent_read with wait_seconds 30 while work continues; expect a completion summary when the task transitions to completed. Re-poll only at that cadence for stalled work.",
"Wait for subagent_create to return before calling another subagent management tool for that id.",
"Use subagent_send with urgent only when the current approach must change immediately; use normal for work that can wait until the current run finishes.",
"Use subagent_terminate when delegated work is no longer needed, and clean up retained subagents before finishing when appropriate.",
Expand Down Expand Up @@ -174,7 +174,7 @@ export function registerSubagentTools(
name: "subagent_read",
label: "Read subagents",
description:
"Wait for and read meaningful subagent activity, status, output, usage, or full transcripts. Omit id to monitor all subagents.",
"Wait for meaningful subagent state updates. Completed subagents return a concise summary and are auto-released after read. Omit id to monitor all subagents.",
promptSnippet: "Read and monitor background subagent activity and output",
parameters: ReadParams,
async execute(_toolCallId, params, signal) {
Expand All @@ -187,7 +187,7 @@ export function registerSubagentTools(
if (signal?.aborted) throw new Error("Subagent read was cancelled");
return toolResult(
manager,
manager.read(agents, params.include_transcript ?? false),
await manager.read(agents, params.include_transcript ?? false),
);
},
renderCall(args, theme) {
Expand Down
2 changes: 1 addition & 1 deletion extensions/subagents/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const MAX_WEB_TRANSCRIPT_CHARS = 100_000;
export const MAX_WEB_STREAMING_CHARS = 20_000;
export const WEB_STATUS_PUBLISH_INTERVAL_MS = 1_000;
export const MAX_TOOL_OUTPUT_BYTES = 50 * 1024;
export const DEFAULT_READ_WAIT_SECONDS = 15;
export const DEFAULT_READ_WAIT_SECONDS = 30;
export const DETAIL_VIEW_LINES = 22;
export const USAGE_STATE_ENTRY = "vessup-subagent-usage";
export const SUBAGENT_SYSTEM_PROMPT = [
Expand Down
89 changes: 80 additions & 9 deletions tests/subagents.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { tmpdir } from "node:os";
import { join } from "node:path";
import test from "node:test";
import { ModelRegistry, ModelRuntime } from "@earendil-works/pi-coding-agent";
import { SubagentManager } from "../extensions/subagents/manager.ts";
import {
stringifyCompact,
truncateChars,
Expand All @@ -16,19 +17,17 @@ import {
} from "../extensions/subagents/ui.ts";
import subagentsExtension, {
abortRunningSubagentSessions,
appendBoundedStreamingText,
countsAgainstSubagentLimit,
filterModelsToScope,
inheritedSubagentModel,
isFailedStopReason,
isTerminalSubagentStatus,
MAX_WEB_STREAMING_CHARS,
parsePersistedUsageState,
shouldArchiveTerminalSubagent,
subagentModelGuidance,
subagentModelRuntime,
} from "../extensions/subagents.ts";

import type { ManagedSubagent } from "../extensions/subagents/types.ts";
test("subagent entrypoint preserves its tool, command, and lifecycle registrations", () => {
const tools: string[] = [];
const commands: string[] = [];
Expand Down Expand Up @@ -91,6 +90,39 @@ const usage = {
},
};

function makeManagedAgent(
override: Partial<ManagedSubagent> = {},
): ManagedSubagent {
const now = Date.now();
return {
id: "worker",
prompt: "task",
cwd: "/tmp",
createdAt: now - 1_000,
updatedAt: now,
status: "completed",
model: "provider/model",
effort: "medium",
turns: 1,
queuedSteering: 0,
queuedFollowUp: 0,
activity: [{ timestamp: now - 10, text: "assistant finished" }],
lastReadActivity: 0,
transcript: [
{
timestamp: now - 5,
role: "assistant",
text: "Subagent summary of work completed.",
},
],
streamingText: "",
lastStreamActivityAt: 0,
usage: usage,
waiters: new Set(),
...override,
};
}

test("compact formatting handles non-JSON values and preserves Unicode code points", () => {
assert.equal(stringifyCompact(undefined), "undefined");
assert.equal(stringifyCompact(Symbol("value")), "Symbol(value)");
Expand Down Expand Up @@ -355,13 +387,52 @@ test("subagent model guidance exposes exact choices and inheritance", () => {
assert.match(guidance, /Never shorten, generalize, or invent a model ID/);
});

test("streaming subagent output remains bounded to its newest text", () => {
const prefix = "a".repeat(MAX_WEB_STREAMING_CHARS - 2);
assert.equal(appendBoundedStreamingText(prefix, "bc"), `${prefix}bc`);
assert.equal(
appendBoundedStreamingText(prefix, "012345"),
`${prefix.slice(4)}012345`,
test("subagent read returns a concise completion summary and auto-releases terminal agents", async () => {
const manager = new SubagentManager({
events: { emit() {} },
} as never);
const agent = makeManagedAgent();

(manager as { agents: Map<string, ManagedSubagent> }).agents.set(
agent.id,
agent,
);

const output = await manager.read([agent], false);

assert.ok(output.includes("Completion summary:"));
assert.equal(output.includes("Activity since last read:"), false);
assert.equal(manager.list().length, 0);
});

test("subagent read includes transcript only when requested", async () => {
const manager = new SubagentManager({
events: { emit() {} },
} as never);
const withTranscript = makeManagedAgent({ id: "detailed" });

(manager as { agents: Map<string, ManagedSubagent> }).agents.set(
withTranscript.id,
withTranscript,
);

const without = await manager.read([withTranscript], false);
assert.equal(without.includes("Transcript:"), false);

// Re-insert a completed agent for the detailed-read assertion.
(manager as { agents: Map<string, ManagedSubagent> }).agents.set(
withTranscript.id,
{
...withTranscript,
lastReadActivity: 0,
status: "completed",
waiters: new Set(),
},
);

const withTranscriptOutput = await manager.read([withTranscript], true);
assert.ok(withTranscriptOutput.includes("Transcript:"));
assert.ok(withTranscriptOutput.includes(withTranscript.transcript[0]?.text));
});

test("persisted usage checkpoints reject malformed data", () => {
Expand Down