fix(coding-agent): stabilized subagent HUD ordering by parent and spawn index
- Added `parentToolCallId` and `index` fields to subagent session records and propagated them from task lifecycle and progress events. - Reworked subagent ordering to sort by parent-group order, spawn index, and stable creation order so out-of-order updates no longer reshuffled active HUD rows. - Updated task execution to pass spawn indices through sync and async paths, switched the `tool.task` icon to Octicons tasklist, and added a registry test for out-of-order progress ordering.
This commit is contained in:
@@ -27,6 +27,7 @@
|
||||
- Changed `/debug dump-next-request` path handling to expand `~` and resolve relative paths against the current working directory
|
||||
- Changed the task tool's TUI block: the header now shows the task dispatch glyph (`tool.task`) while agents are in flight instead of a spinner (async spawns return immediately, so a spinner misread the call as blocking), and per-agent rows use one static dot for every state — completed rows keep the same dot and settle from accent to the plain foreground color instead of switching to a different status glyph
|
||||
- Compressed the `eval`, `browser`, `read`, and `irc` tool prompts (6156 → 4932 tokens o200k, −20%): deduplicated claims across sections, tightened helper reference descriptions, trimmed redundant examples; input grammar, examples, and template conditionals (`py`/`js`/`spawns`, read display modes) unchanged
|
||||
- Changed the Nerd Font `tool.task` icon to the Octicons tasklist glyph.
|
||||
|
||||
### Removed
|
||||
|
||||
|
||||
@@ -10,6 +10,8 @@ export interface ObservableSession {
|
||||
description?: string;
|
||||
status: "active" | "completed" | "failed" | "aborted";
|
||||
sessionFile?: string;
|
||||
parentToolCallId?: string;
|
||||
index?: number;
|
||||
lastUpdate: number;
|
||||
/** Latest progress snapshot from the subagent executor */
|
||||
progress?: AgentProgress;
|
||||
@@ -26,6 +28,9 @@ export class SessionObserverRegistry {
|
||||
#sessions = new Map<string, ObservableSession>();
|
||||
#listeners = new Set<() => void>();
|
||||
#eventBusUnsubscribers: Array<() => void> = [];
|
||||
#sortOrderById = new Map<string, number>();
|
||||
#parentSortOrderById = new Map<string, number>();
|
||||
#nextSortOrder = 0;
|
||||
|
||||
/** Add a change listener. Returns unsubscribe function. */
|
||||
onChange(cb: () => void): () => void {
|
||||
@@ -37,8 +42,34 @@ export class SessionObserverRegistry {
|
||||
for (const cb of this.#listeners) cb();
|
||||
}
|
||||
|
||||
#ensureSortOrder(id: string): number {
|
||||
const existing = this.#sortOrderById.get(id);
|
||||
if (existing !== undefined) return existing;
|
||||
const order = this.#nextSortOrder++;
|
||||
this.#sortOrderById.set(id, order);
|
||||
return order;
|
||||
}
|
||||
|
||||
#ensureParentSortOrder(parentToolCallId: string | undefined, order: number): void {
|
||||
if (!parentToolCallId) return;
|
||||
if (this.#parentSortOrderById.has(parentToolCallId)) return;
|
||||
this.#parentSortOrderById.set(parentToolCallId, order);
|
||||
}
|
||||
|
||||
#getStableOrder(session: ObservableSession): number {
|
||||
return this.#sortOrderById.get(session.id) ?? Number.MAX_SAFE_INTEGER;
|
||||
}
|
||||
|
||||
#getGroupOrder(session: ObservableSession): number {
|
||||
const parentOrder = session.parentToolCallId
|
||||
? this.#parentSortOrderById.get(session.parentToolCallId)
|
||||
: undefined;
|
||||
return parentOrder ?? this.#getStableOrder(session);
|
||||
}
|
||||
|
||||
setMainSession(sessionFile?: string): void {
|
||||
const existing = this.#sessions.get("main");
|
||||
this.#ensureSortOrder("main");
|
||||
this.#sessions.set("main", {
|
||||
id: "main",
|
||||
kind: "main",
|
||||
@@ -53,9 +84,18 @@ export class SessionObserverRegistry {
|
||||
getSessions(): ObservableSession[] {
|
||||
const sessions = [...this.#sessions.values()];
|
||||
sessions.sort((a, b) => {
|
||||
if (a.kind === "main") return -1;
|
||||
if (b.kind === "main") return 1;
|
||||
return a.lastUpdate - b.lastUpdate;
|
||||
if (a.kind === "main" && b.kind !== "main") return -1;
|
||||
if (b.kind === "main" && a.kind !== "main") return 1;
|
||||
if (a.kind === "main" || b.kind === "main") return 0;
|
||||
|
||||
const groupDiff = this.#getGroupOrder(a) - this.#getGroupOrder(b);
|
||||
if (groupDiff !== 0) return groupDiff;
|
||||
|
||||
const aIndex = a.index ?? Number.MAX_SAFE_INTEGER;
|
||||
const bIndex = b.index ?? Number.MAX_SAFE_INTEGER;
|
||||
if (aIndex !== bIndex) return aIndex - bIndex;
|
||||
|
||||
return this.#getStableOrder(a) - this.#getStableOrder(b);
|
||||
});
|
||||
return sessions;
|
||||
}
|
||||
@@ -71,6 +111,9 @@ export class SessionObserverRegistry {
|
||||
/** Clear all tracked sessions (e.g. on session switch). Keeps EventBus subscriptions and listeners. */
|
||||
resetSessions(): void {
|
||||
this.#sessions.clear();
|
||||
this.#sortOrderById.clear();
|
||||
this.#parentSortOrderById.clear();
|
||||
this.#nextSortOrder = 0;
|
||||
this.#notifyListeners();
|
||||
}
|
||||
|
||||
@@ -78,6 +121,9 @@ export class SessionObserverRegistry {
|
||||
for (const unsub of this.#eventBusUnsubscribers) unsub();
|
||||
this.#eventBusUnsubscribers = [];
|
||||
this.#sessions.clear();
|
||||
this.#sortOrderById.clear();
|
||||
this.#parentSortOrderById.clear();
|
||||
this.#nextSortOrder = 0;
|
||||
this.#listeners.clear();
|
||||
}
|
||||
|
||||
@@ -92,10 +138,14 @@ export class SessionObserverRegistry {
|
||||
const status = STATUS_MAP[payload.status];
|
||||
if (!status) return;
|
||||
|
||||
const sortOrder = this.#ensureSortOrder(payload.id);
|
||||
this.#ensureParentSortOrder(payload.parentToolCallId, sortOrder);
|
||||
const existing = this.#sessions.get(payload.id);
|
||||
if (existing) {
|
||||
existing.status = status;
|
||||
existing.lastUpdate = Date.now();
|
||||
existing.index = payload.index;
|
||||
existing.parentToolCallId = payload.parentToolCallId ?? existing.parentToolCallId;
|
||||
if (payload.description) existing.description = payload.description;
|
||||
if (payload.sessionFile) existing.sessionFile = payload.sessionFile;
|
||||
} else {
|
||||
@@ -107,6 +157,8 @@ export class SessionObserverRegistry {
|
||||
description: payload.description,
|
||||
status,
|
||||
sessionFile: payload.sessionFile,
|
||||
parentToolCallId: payload.parentToolCallId,
|
||||
index: payload.index,
|
||||
lastUpdate: Date.now(),
|
||||
});
|
||||
}
|
||||
@@ -121,8 +173,12 @@ export class SessionObserverRegistry {
|
||||
const id = progress.id;
|
||||
const existing = this.#sessions.get(id);
|
||||
|
||||
const sortOrder = this.#ensureSortOrder(id);
|
||||
this.#ensureParentSortOrder(payload.parentToolCallId, sortOrder);
|
||||
if (existing) {
|
||||
existing.lastUpdate = Date.now();
|
||||
existing.index = payload.index;
|
||||
existing.parentToolCallId = payload.parentToolCallId ?? existing.parentToolCallId;
|
||||
existing.progress = progress;
|
||||
if (progress.description) existing.description = progress.description;
|
||||
if (payload.sessionFile) existing.sessionFile = payload.sessionFile;
|
||||
@@ -135,6 +191,8 @@ export class SessionObserverRegistry {
|
||||
description: progress.description,
|
||||
status: "active",
|
||||
sessionFile: payload.sessionFile,
|
||||
parentToolCallId: payload.parentToolCallId,
|
||||
index: payload.index,
|
||||
lastUpdate: Date.now(),
|
||||
progress,
|
||||
});
|
||||
|
||||
@@ -715,7 +715,7 @@ const NERD_SYMBOLS: SymbolMap = {
|
||||
"tool.debug": "\uEAD8",
|
||||
"tool.mcp": "\uEB2D",
|
||||
"tool.job": "\uEBA2",
|
||||
"tool.task": "\uEA7E",
|
||||
"tool.task": "\uf4a0",
|
||||
"tool.todo": "\uEAB3",
|
||||
"tool.memory": "\uEACE",
|
||||
"tool.ask": "\uEAC7",
|
||||
|
||||
@@ -698,7 +698,14 @@ export class TaskTool implements AgentTool<TaskToolSchemaInstance, TaskToolDetai
|
||||
buildDetails("running", ownJobId) as unknown as Record<string, unknown>,
|
||||
);
|
||||
try {
|
||||
const result = await this.#executeSync(toolCallId, spawnParams, runSignal, undefined, agentId);
|
||||
const result = await this.#executeSync(
|
||||
toolCallId,
|
||||
spawnParams,
|
||||
runSignal,
|
||||
undefined,
|
||||
agentId,
|
||||
progress.index,
|
||||
);
|
||||
const finalText = result.content.find(part => part.type === "text")?.text ?? "(no output)";
|
||||
const singleResult = result.details?.results[0];
|
||||
// A missing result means the sync path failed at the tool level
|
||||
@@ -781,7 +788,14 @@ export class TaskTool implements AgentTool<TaskToolSchemaInstance, TaskToolDetai
|
||||
if (spawnItems.length === 1) {
|
||||
await semaphore.acquire();
|
||||
try {
|
||||
return await this.#executeSync(toolCallId, spawnParamsFor(params, spawnItems[0]), signal, onUpdate);
|
||||
return await this.#executeSync(
|
||||
toolCallId,
|
||||
spawnParamsFor(params, spawnItems[0]),
|
||||
signal,
|
||||
onUpdate,
|
||||
undefined,
|
||||
0,
|
||||
);
|
||||
} finally {
|
||||
semaphore.release();
|
||||
}
|
||||
@@ -818,7 +832,14 @@ export class TaskTool implements AgentTool<TaskToolSchemaInstance, TaskToolDetai
|
||||
}
|
||||
}
|
||||
: undefined;
|
||||
return await this.#executeSync(toolCallId, spawnParamsFor(params, item), workerSignal, itemOnUpdate);
|
||||
return await this.#executeSync(
|
||||
toolCallId,
|
||||
spawnParamsFor(params, item),
|
||||
workerSignal,
|
||||
itemOnUpdate,
|
||||
undefined,
|
||||
index,
|
||||
);
|
||||
} finally {
|
||||
semaphore.release();
|
||||
}
|
||||
@@ -875,8 +896,9 @@ export class TaskTool implements AgentTool<TaskToolSchemaInstance, TaskToolDetai
|
||||
signal?: AbortSignal,
|
||||
onUpdate?: AgentToolUpdateCallback<TaskToolDetails>,
|
||||
preAllocatedId?: string,
|
||||
spawnIndex = 0,
|
||||
): Promise<AgentToolResult<TaskToolDetails>> {
|
||||
return this.#runSpawn(toolCallId, params, signal, onUpdate, preAllocatedId);
|
||||
return this.#runSpawn(toolCallId, params, signal, onUpdate, preAllocatedId, spawnIndex);
|
||||
}
|
||||
|
||||
/** Spawn a fresh subagent and run it to completion. */
|
||||
@@ -886,6 +908,7 @@ export class TaskTool implements AgentTool<TaskToolSchemaInstance, TaskToolDetai
|
||||
signal?: AbortSignal,
|
||||
onUpdate?: AgentToolUpdateCallback<TaskToolDetails>,
|
||||
preAllocatedId?: string,
|
||||
spawnIndex = 0,
|
||||
): Promise<AgentToolResult<TaskToolDetails>> {
|
||||
const startTime = Date.now();
|
||||
const { agents, projectAgentsDir } = await discoverAgents(this.session.cwd);
|
||||
@@ -1070,7 +1093,7 @@ export class TaskTool implements AgentTool<TaskToolSchemaInstance, TaskToolDetai
|
||||
|
||||
// Progress tracking for the single agent
|
||||
let latestProgress: AgentProgress = {
|
||||
index: 0,
|
||||
index: spawnIndex,
|
||||
id: agentId,
|
||||
agent: agentName,
|
||||
agentSource: agent.source,
|
||||
@@ -1120,7 +1143,7 @@ export class TaskTool implements AgentTool<TaskToolSchemaInstance, TaskToolDetai
|
||||
context: sharedContext,
|
||||
planReference,
|
||||
description: params.description,
|
||||
index: 0,
|
||||
index: spawnIndex,
|
||||
parentToolCallId: toolCallId,
|
||||
id: agentId,
|
||||
taskDepth,
|
||||
@@ -1226,7 +1249,7 @@ export class TaskTool implements AgentTool<TaskToolSchemaInstance, TaskToolDetai
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
return {
|
||||
index: 0,
|
||||
index: spawnIndex,
|
||||
id: agentId,
|
||||
agent: agent.name,
|
||||
agentSource: agent.source,
|
||||
|
||||
@@ -5,9 +5,19 @@
|
||||
*/
|
||||
import { beforeAll, describe, expect, it } from "bun:test";
|
||||
import { renderSubagentHudLines } from "@oh-my-pi/pi-coding-agent/modes/interactive-mode";
|
||||
import type { ObservableSession } from "@oh-my-pi/pi-coding-agent/modes/session-observer-registry";
|
||||
import {
|
||||
type ObservableSession,
|
||||
SessionObserverRegistry,
|
||||
} from "@oh-my-pi/pi-coding-agent/modes/session-observer-registry";
|
||||
import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
|
||||
import type { AgentProgress } from "@oh-my-pi/pi-coding-agent/task";
|
||||
import {
|
||||
type AgentProgress,
|
||||
type SubagentLifecyclePayload,
|
||||
type SubagentProgressPayload,
|
||||
TASK_SUBAGENT_LIFECYCLE_CHANNEL,
|
||||
TASK_SUBAGENT_PROGRESS_CHANNEL,
|
||||
} from "@oh-my-pi/pi-coding-agent/task";
|
||||
import { EventBus } from "@oh-my-pi/pi-coding-agent/utils/event-bus";
|
||||
|
||||
function makeSession(overrides: Partial<ObservableSession> & { id: string }): ObservableSession {
|
||||
return {
|
||||
@@ -37,6 +47,29 @@ function makeProgress(overrides: Partial<AgentProgress> & { id: string }): Agent
|
||||
};
|
||||
}
|
||||
|
||||
function makeLifecycle(id: string, index: number, description: string): SubagentLifecyclePayload {
|
||||
return {
|
||||
id,
|
||||
index,
|
||||
agent: "task",
|
||||
agentSource: "bundled",
|
||||
description,
|
||||
status: "started",
|
||||
parentToolCallId: "tool-call",
|
||||
};
|
||||
}
|
||||
|
||||
function makeProgressPayload(id: string, index: number, description: string): SubagentProgressPayload {
|
||||
return {
|
||||
index,
|
||||
agent: "task",
|
||||
agentSource: "bundled",
|
||||
task: description,
|
||||
parentToolCallId: "tool-call",
|
||||
progress: makeProgress({ id, index, description, task: description }),
|
||||
};
|
||||
}
|
||||
|
||||
function render(sessions: ObservableSession[], columns = 120): string {
|
||||
return Bun.stripANSI(renderSubagentHudLines(sessions, columns).join("\n"));
|
||||
}
|
||||
@@ -90,4 +123,41 @@ describe("subagent HUD lines", () => {
|
||||
expect(Bun.stringWidth(line)).toBeLessThanOrEqual(60);
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps subagent registry order stable while progress arrives out of order", () => {
|
||||
const eventBus = new EventBus();
|
||||
const registry = new SessionObserverRegistry();
|
||||
registry.subscribeToEventBus(eventBus);
|
||||
const activeIds = () =>
|
||||
registry
|
||||
.getSessions()
|
||||
.filter(session => session.kind === "subagent" && session.status === "active")
|
||||
.map(session => session.id);
|
||||
|
||||
eventBus.emit(
|
||||
TASK_SUBAGENT_LIFECYCLE_CHANNEL,
|
||||
makeLifecycle("BlastRadius", 1, "Survey id-keyed downstream consumers"),
|
||||
);
|
||||
eventBus.emit(
|
||||
TASK_SUBAGENT_LIFECYCLE_CHANNEL,
|
||||
makeLifecycle("SelectorSurfaces", 0, "Map model-selector resolution surfaces"),
|
||||
);
|
||||
eventBus.emit(
|
||||
TASK_SUBAGENT_LIFECYCLE_CHANNEL,
|
||||
makeLifecycle("VariantsSurvey", 2, "Survey tier-variant ids across catalog"),
|
||||
);
|
||||
|
||||
expect(activeIds()).toEqual(["SelectorSurfaces", "BlastRadius", "VariantsSurvey"]);
|
||||
|
||||
eventBus.emit(
|
||||
TASK_SUBAGENT_PROGRESS_CHANNEL,
|
||||
makeProgressPayload("VariantsSurvey", 2, "Survey tier-variant ids across catalog"),
|
||||
);
|
||||
eventBus.emit(
|
||||
TASK_SUBAGENT_PROGRESS_CHANNEL,
|
||||
makeProgressPayload("BlastRadius", 1, "Survey id-keyed downstream consumers"),
|
||||
);
|
||||
|
||||
expect(activeIds()).toEqual(["SelectorSurfaces", "BlastRadius", "VariantsSurvey"]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user