From 384a206737c88ce79da0c0edb02e49ebe2ae2b9a Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 2 Jun 2026 06:50:03 +0200 Subject: [PATCH] refactor(task): replaced numeric-prefix ids with name-first agent output ids - Changed `AgentOutputManager` to use requested names verbatim, adding `-2`/`-3` suffixes only on repeats (e.g. `Anna`, `Anna-2`). - Renamed main agent id from `0-Main` to `Main`; nested ids now use dot notation without numeric prefix (e.g. `Parent.Child`). - Updated task widget to render dotted hierarchy as `Parent>Child` breadcrumb without leading index. - Resume scan now tracks seen names instead of a counter to avoid clobbering prior outputs. --- docs/blob-artifact-architecture.md | 2 +- docs/tools/eval.md | 10 +-- docs/tools/irc.md | 4 +- docs/tools/task.md | 2 +- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/async/job-manager.ts | 6 +- .../coding-agent/src/prompts/tools/irc.md | 12 +-- .../coding-agent/src/prompts/tools/task.md | 2 +- .../src/registry/agent-registry.ts | 2 +- packages/coding-agent/src/sdk.ts | 4 +- .../coding-agent/src/session/agent-session.ts | 2 +- .../coding-agent/src/task/output-manager.ts | 88 +++++++++---------- packages/coding-agent/src/task/render.ts | 11 +-- packages/coding-agent/src/tools/index.ts | 2 +- .../test/task/output-manager.test.ts | 64 ++++++++++++++ .../test/task/render-nested-live.test.ts | 28 +++--- 16 files changed, 146 insertions(+), 94 deletions(-) create mode 100644 packages/coding-agent/test/task/output-manager.test.ts diff --git a/docs/blob-artifact-architecture.md b/docs/blob-artifact-architecture.md index 7a3f2a19a..9233f3b90 100644 --- a/docs/blob-artifact-architecture.md +++ b/docs/blob-artifact-architecture.md @@ -76,7 +76,7 @@ Non-persistent sessions without an adopted manager can store `saveArtifact(...)` ### Agent output IDs (`agent://`) -`AgentOutputManager` allocates IDs for subagent outputs as `-` (optionally nested under parent prefix, e.g. `0-Parent.1-Child`). It scans existing `.md` files on initialization to continue from the next index on resume. +`AgentOutputManager` allocates IDs for subagent outputs from the requested name, used verbatim the first time and suffixed (`-2`, `-3`, …) only when the same name repeats (e.g. `Anna`, `Anna-2`). Nested outputs are grouped under the parent prefix (e.g. `Parent.Child`). It scans existing `.md` files on initialization so a resumed session never reuses a name that would clobber a prior output. ## Persistence dataflow diff --git a/docs/tools/eval.md b/docs/tools/eval.md index 442c6e46c..835730249 100644 --- a/docs/tools/eval.md +++ b/docs/tools/eval.md @@ -132,15 +132,15 @@ Implemented in `packages/coding-agent/src/eval/js/worker-core.ts`, `packages/cod - `read`, `write`, `append`, `sort`, `uniq`, `counter`, `diff`, `tree`, `env`, `output` - `tool.(args)` proxy for arbitrary session tool calls - `llm(prompt, opts?)` for oneshot, stateless LLM calls (see _Oneshot LLM helper_ below) - - `agent(prompt, opts?)` for a single subagent call, plus JS-only `parallel()` / `pipeline()` bounded-pool helpers (see _Subagent helper_ below) + - `agent(prompt, opts?)` for a single subagent call, plus `parallel()` / `pipeline()` bounded-pool helpers (see _Subagent helper_ below) - JS helpers that touch the host/runtime boundary are async and `await`able; pure text helpers (`sort`, `uniq`, `counter`) return synchronously but may still be safely awaited. - JS helper signatures use a trailing options object rather than Python keyword arguments: - `await read(path, { offset?, limit? })` - `await tree(path = ".", { maxDepth?, hidden? })` - `sort(text, { reverse?, unique? })`, `uniq(text, { count? })`, `counter(items, { limit?, reverse? })` - `await agent(prompt, { agentType?, model?, context?, label?, schema? })` - - `await parallel([() => agent("a"), () => agent("b")], { concurrency? })` - - `await pipeline(items, stage1, stage2, { concurrency? })` + - `await parallel([() => agent("a"), () => agent("b")])` + - `await pipeline(items, stage1, stage2)` - `display(value)` behavior: - plain objects/arrays become JSON outputs - `{ type: "image", data, mimeType }` becomes an image output @@ -199,7 +199,7 @@ Both runtimes expose `agent()` — a single subagent invocation routed through ` - `context` supplies shared background; `label` controls the `agent://` output label prefix. - `schema` passes a JSON Schema to the subagent structured-output path. When present, the helper parses the final JSON text and returns an object. - Spawn restrictions use `session.getSessionSpawns()` exactly like the `task` tool. Eval-driven subagent recursion is capped at depth 3. -- JS also exposes `parallel(thunks, { concurrency })` and `pipeline(items, ...stages, { concurrency })`; both use a bounded async pool with default concurrency 4, max 16, preserve item order, and propagate rejections. +- JS and Python both expose `parallel(thunks)` and `pipeline(items, ...stages)`; both use a bounded async/threaded pool whose width tracks the `task.maxConcurrency` setting (the same ceiling the `task` tool uses; `0` = run every item at once), preserve item order, and propagate rejections. The width is fetched live from the host via the `__concurrency__` bridge, so the helpers no longer take a `concurrency` argument. - Errors surface as exceptions: unknown or disabled agent, disallowed spawn, recursion cap, subagent failure, or invalid structured output all fail the eval cell. ### Multi-language call behavior @@ -245,7 +245,7 @@ A single tool call can mix Python and JS cells. Persistence is per language runt - Output truncation window: 50KB default (`DEFAULT_MAX_BYTES` in `packages/coding-agent/src/session/streaming-output.ts`) - Output line cap inside truncation helpers: 3000 lines (`DEFAULT_MAX_LINES` in `packages/coding-agent/src/session/streaming-output.ts`) - Streaming tail buffer for live updates: `DEFAULT_MAX_BYTES * 2` = 100KB (`packages/coding-agent/src/tools/eval.ts`) -- JS `parallel()` / `pipeline()` helper concurrency default: 4; maximum: 16 +- JS/Python `parallel()` / `pipeline()` helper pool width: the `task.maxConcurrency` setting (default 32; `0` = unbounded), resolved live via the `__concurrency__` bridge (`packages/coding-agent/src/eval/concurrency-bridge.ts`) - Eval-driven `agent()` recursion cap: task depth 3 (`EVAL_AGENT_MAX_DEPTH`) - Python retained kernel idle timeout: 5 minutes (`IDLE_TIMEOUT_MS` in `packages/coding-agent/src/eval/py/executor.ts`) - Python retained kernel cap: 4 sessions (`MAX_KERNEL_SESSIONS` in `packages/coding-agent/src/eval/py/executor.ts`) diff --git a/docs/tools/irc.md b/docs/tools/irc.md index 98996443b..bf2ff70a3 100644 --- a/docs/tools/irc.md +++ b/docs/tools/irc.md @@ -28,7 +28,7 @@ | Field | Type | Required | Description | | --- | --- | --- | --- | | `op` | `"send"` | Yes | Sends one message to one peer or to `"all"`. | -| `to` | `string` | Yes | Peer id such as `0-Main`, or `"all"` for broadcast. Whitespace is trimmed. | +| `to` | `string` | Yes | Peer id such as `Main`, or `"all"` for broadcast. Whitespace is trimmed. | | `message` | `string` | Yes | Message body. Whitespace is trimmed; empty-after-trim is rejected. | | `awaitReply` | `boolean` | No | Wait for prose replies. Defaults to `true` for direct messages and `false` for `to: "all"`. | @@ -44,7 +44,7 @@ ## Flow 1. `IrcTool.createIf` only constructs the tool when `irc.enabled` is on and the session has both an `AgentRegistry` and `getAgentId` (`packages/coding-agent/src/tools/irc.ts`). -2. Tool discovery adds another gate in `packages/coding-agent/src/tools/index.ts`: if the caller is `0-Main` and `async.enabled` is off, `irc` is hidden because the main agent cannot talk to concurrent peers in sync mode. +2. Tool discovery adds another gate in `packages/coding-agent/src/tools/index.ts`: if the caller is `Main` and `async.enabled` is off, `irc` is hidden because the main agent cannot talk to concurrent peers in sync mode. 3. `execute` resolves the process-global registry and sender id. Missing either returns a text error result instead of throwing. 4. `op: "list"` calls `registry.listVisibleTo(senderId)`, which exposes every other agent in flat namespace whose status is `running` or `idle` (`packages/coding-agent/src/registry/agent-registry.ts`). 5. `list` formats human-readable lines and returns `channels` as `['all', ...peerIds]`. These are logical targets only; there is no channel join state. diff --git a/docs/tools/task.md b/docs/tools/task.md index 245824836..98026680c 100644 --- a/docs/tools/task.md +++ b/docs/tools/task.md @@ -222,4 +222,4 @@ Artifacts and side channels: - Branch-mode merge temporarily stashes the parent repo before cherry-picking task branches. A stash-pop conflict is treated as merge failure and leaves recovery state behind. - Patch-mode only applies combined root patches if every successful task produced a patch and `git.patch.canApplyText(...)` succeeds. - Nested git repos are handled separately from the root repo. They are copied into isolated worktrees, diffed independently, and merged later with `applyNestedPatches(...)` because parent git cannot track their file-level changes. -- `agent://` ids are numeric-prefixed (`0-Task`, `1-Task`, nested like `0-Parent.0-Child`) by `AgentOutputManager`; this is what prevents artifact collisions across repeated or nested task invocations. +- `agent://` ids are name-based (`Task` first, `Task-2`/`Task-3` only when the name repeats, nested like `Parent.Child`) by `AgentOutputManager`; this is what prevents artifact collisions across repeated or nested task invocations. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 8a4679dbb..8c58cccfe 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -9,6 +9,7 @@ ### Changed - Changed the eval `parallel()` / `pipeline()` helpers to drop their `concurrency` argument and run as wide as a `task` tool batch. The worker-pool ceiling now tracks the `task.maxConcurrency` setting (default 32; `0` = run every item at once) resolved live from the host via a new `__concurrency__` bridge, instead of the old per-call `concurrency` option that defaulted to 4 and capped at 16. This stops eval fan-outs from under-using the parallelism the session is configured for. +- Changed subagent / `agent://` output ids from a numeric prefix scheme (`0-Anna`, `1-Bob`, nested `0-Anna.1-Bob`) to a name-first scheme: the requested name is used verbatim and a `-2`/`-3`/… suffix is added only when the same name recurs within a session (`Anna`, `Anna-2`, `Anna-3`). Nested ids stay grouped under the parent (`Anna.Bob`) and the live task widget renders them as `Anna>Bob`. The main agent's IRC id is now `Main` (was `0-Main`). `AgentOutputManager` still scans existing `.md` outputs on resume so it never reuses a name that would clobber a prior output. - Changed resuming a session that belongs to a different project to switch the process into that project's working directory. `pi --resume` and the in-session `/resume` picker now `chdir` into the resumed session's `cwd` and re-scope every cwd-derived input — project dir, **project settings** (`.claude/settings.yml`, `.omp/settings.json`, path-scoped `enabledModels`/`disabledProviders`), plugin roots, capabilities, slash commands, and the ssh tool — so tools, discovery, configuration, and commands all follow the resumed project. The `SessionManager` adopts the resumed session's own `cwd`/session directory on load (rolled back if the switch fails). ### Fixed diff --git a/packages/coding-agent/src/async/job-manager.ts b/packages/coding-agent/src/async/job-manager.ts index bde46c4b1..e1ef6a365 100644 --- a/packages/coding-agent/src/async/job-manager.ts +++ b/packages/coding-agent/src/async/job-manager.ts @@ -17,8 +17,8 @@ export interface AsyncJob { resultText?: string; errorText?: string; /** - * Registry id of the agent that registered the job (e.g. "0-Main", - * "3-AuthLoader"). Used by scoped cancel/list APIs so a subagent's teardown + * Registry id of the agent that registered the job (e.g. "Main", + * "AuthLoader"). Used by scoped cancel/list APIs so a subagent's teardown * does not cancel its parent's jobs. Undefined for callers that don't * supply an id (e.g. legacy tests, SDK consumers without an agent context). */ @@ -58,7 +58,7 @@ export interface AsyncJobRegisterOptions { /** * Filter applied to job query/cancel APIs. With `ownerId`, results are * restricted to jobs registered by that agent (registry id from - * `AgentRegistry`, e.g. "0-Main", "3-AuthLoader"). + * `AgentRegistry`, e.g. "Main", "AuthLoader"). */ export interface AsyncJobFilter { ownerId?: string; diff --git a/packages/coding-agent/src/prompts/tools/irc.md b/packages/coding-agent/src/prompts/tools/irc.md index e29d0b5a5..8dbeda10c 100644 --- a/packages/coding-agent/src/prompts/tools/irc.md +++ b/packages/coding-agent/src/prompts/tools/irc.md @@ -1,7 +1,7 @@ Sends short text messages to other live agents in this process and receives their prose replies. -- The main agent is addressable as `0-Main`. Subagents reuse their task id (e.g. `0-AuthLoader`). +- The main agent is addressable as `Main`. Subagents reuse their task id (e.g. `AuthLoader`, or `AuthLoader-2` when the name repeats). - `op: "list"` returns the current set of visible peers. Use it before sending if you are not sure who is live. - `op: "send"` delivers `message` to `to`. `to` may be a specific id or `"all"` to broadcast. - The recipient generates the reply via an ephemeral side-channel turn that uses their current model, system prompt, and history — it does **not** wait for the recipient's main loop to be free, so it is safe to IRC an agent that is currently inside a long-running tool call. @@ -10,7 +10,7 @@ Sends short text messages to other live agents in this process and receives thei You SHOULD reach for `irc` proactively when continuing alone is wasteful or wrong. When in doubt, prefer messaging. -- **Unexpected state.** You hit something the original task did not describe — a missing file, a config that contradicts the assignment, an API behaving differently than you were told, a tool failing in a way that suggests the spec is wrong. DM `0-Main` (or the spawning agent) for guidance instead of guessing. +- **Unexpected state.** You hit something the original task did not describe — a missing file, a config that contradicts the assignment, an API behaving differently than you were told, a tool failing in a way that suggests the spec is wrong. DM `Main` (or the spawning agent) for guidance instead of guessing. - **Blocked by another agent.** A peer holds the file/branch/resource you need, has already started the change you are about to make, or owns a decision you depend on. DM that peer (or broadcast to discover who) before duplicating or stepping on work. - **Decision points outside your scope.** A genuine fork in the road that the assignment did not pre-decide (e.g. which of two viable APIs to use, whether to refactor adjacent code). Ask the requester rather than picking unilaterally. - **Coordination opportunities.** You realize a peer's in-flight work would benefit from yours, or vice-versa. @@ -25,7 +25,7 @@ These rules apply to both sending and replying. - **Use IRC, not terminal tools, to learn about peers.** Do not `grep` artifacts, read other sessions' JSONL files, or shell-poke around to figure out what another agent is doing. DM them — they have the live answer and you do not. - **One round-trip is enough.** Replies arrive synchronously when the recipient is reachable. Do not follow up with "did you get my message?" — they did. If `delivered` is empty or the result was `failed`, the peer is unavailable; move on or report the blocker, do not retry in a loop. - **Stay terse.** A DM is a chat message, not a memo. One question per send when you can. Share file paths and artifacts via `local://` / `memory://` / `artifact://` URLs instead of pasting blobs. -- **Address peers by id.** Use the exact id from `op: "list"` (e.g. `0-AuthLoader`, `0-Main`). Do not invent friendly names. +- **Address peers by id.** Use the exact id from `op: "list"` (e.g. `AuthLoader`, `Main`). Do not invent friendly names. - **Do not IRC for things a tool would answer.** If a `read`, `grep`, or build command would resolve the question, do that first. - **When you receive an IRC message, answer it before continuing.** The recipient injects the question + your auto-reply into your history; address it directly, do not repeat it back to the user. @@ -39,11 +39,11 @@ These rules apply to both sending and replying. # List peers `{"op": "list"}` # Direct message to the main agent (waits for prose reply) -`{"op": "send", "to": "0-Main", "message": "Should I prefer JWT or session cookies for the auth flow?"}` +`{"op": "send", "to": "Main", "message": "Should I prefer JWT or session cookies for the auth flow?"}` # Unexpected state — ask the originator -`{"op": "send", "to": "0-Main", "message": "Assignment says edit src/auth/jwt.ts but the file does not exist. Is the new path src/server/auth/jwt.ts?"}` +`{"op": "send", "to": "Main", "message": "Assignment says edit src/auth/jwt.ts but the file does not exist. Is the new path src/server/auth/jwt.ts?"}` # Blocked by a peer — ask them directly -`{"op": "send", "to": "0-AuthLoader", "message": "Are you still touching src/server/auth.ts? I need to add a 401 path; OK to proceed or should I wait?"}` +`{"op": "send", "to": "AuthLoader", "message": "Are you still touching src/server/auth.ts? I need to add a 401 path; OK to proceed or should I wait?"}` # Broadcast to discover who owns something (no replies, just informs them) `{"op": "send", "to": "all", "message": "About to refactor src/server/middleware/*. Anyone already in there?", "awaitReply": false}` diff --git a/packages/coding-agent/src/prompts/tools/task.md b/packages/coding-agent/src/prompts/tools/task.md index c8b422b48..252b79381 100644 --- a/packages/coding-agent/src/prompts/tools/task.md +++ b/packages/coding-agent/src/prompts/tools/task.md @@ -2,7 +2,7 @@ Launches subagents to parallelize workflows. {{#if asyncEnabled}} - Results are delivered automatically when complete. -- The tool result lists the assigned task ids (e.g. `0-AuthLoader`) — those are the live agent ids. +- The tool result lists the assigned task ids (e.g. `AuthLoader`) — those are the live agent ids. {{#if ircEnabled}} - Coordinate with running tasks via `irc` using those ids. `job cancel` terminates a task and **cannot carry a message** — only use it for stalled/abandoned work. - If genuinely blocked on completion, wait with `job poll`; otherwise keep working. diff --git a/packages/coding-agent/src/registry/agent-registry.ts b/packages/coding-agent/src/registry/agent-registry.ts index ae4abe19c..271b9d90b 100644 --- a/packages/coding-agent/src/registry/agent-registry.ts +++ b/packages/coding-agent/src/registry/agent-registry.ts @@ -8,7 +8,7 @@ import type { AgentSession } from "../session/agent-session"; -export const MAIN_AGENT_ID = "0-Main"; +export const MAIN_AGENT_ID = "Main"; export type AgentStatus = "running" | "idle" | "completed" | "aborted"; export type AgentKind = "main" | "sub"; diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 5b2b7f85d..7f5140be9 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -323,13 +323,13 @@ export interface CreateAgentSessionOptions { parentHindsightSessionState?: HindsightSessionState; /** Parent Mnemopi state to alias for subagent memory tools. */ parentMnemopiSessionState?: MnemopiSessionState; - /** Pre-allocated agent identity for IRC routing. Default: "0-Main" for top-level, parentTaskPrefix-derived for sub. */ + /** Pre-allocated agent identity for IRC routing. Default: "Main" for top-level, parentTaskPrefix-derived for sub. */ agentId?: string; /** Display name for the agent in IRC. Default: "main" or "sub". */ agentDisplayName?: string; /** Optional shared agent registry for IRC routing. Default: AgentRegistry.global(). */ agentRegistry?: AgentRegistry; - /** Parent task ID prefix for nested artifact naming (e.g., "6-Extensions") */ + /** Parent task ID prefix for nested artifact naming (e.g., "Extensions") */ parentTaskPrefix?: string; /** Inherited eval executor session id for subagents sharing parent eval state. */ parentEvalSessionId?: string; diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index ea38e483d..6654b4285 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -358,7 +358,7 @@ export interface AgentSessionConfig { * **MUST NOT** dispose it on their own teardown. */ ownedAsyncJobManager?: AsyncJobManager; - /** Agent identity (registry id like "0-Main" or "3-Alice") used for IRC routing. */ + /** Agent identity (registry id like "Main" or "Alice") used for IRC routing. */ agentId?: string; /** Shared agent registry (for forwarding IRC observations to the main session UI). */ agentRegistry?: AgentRegistry; diff --git a/packages/coding-agent/src/task/output-manager.ts b/packages/coding-agent/src/task/output-manager.ts index cc64ff402..46d6593c0 100644 --- a/packages/coding-agent/src/task/output-manager.ts +++ b/packages/coding-agent/src/task/output-manager.ts @@ -1,29 +1,28 @@ /** * Session-scoped manager for agent output IDs. * - * Ensures unique output IDs across task tool invocations within a session. - * Prefixes each ID with a sequential number (e.g., "0-AuthProvider", "1-AuthApi"). - * If a parent prefix is provided, IDs are nested (e.g., "0-Auth.1-Subtask"). + * Keeps every subagent output id unique within a session without polluting the + * common case with bookkeeping. A requested name is used verbatim the first + * time it appears; only a *repeated* name gets a numeric suffix to disambiguate + * it (e.g. "Anna", "Anna-2", "Anna-3"). When a parent prefix is configured, ids + * are nested under it (e.g. "Anna.Bob") so hierarchical outputs stay grouped. * - * This enables reliable agent:// URL resolution and prevents artifact collisions. + * This enables reliable agent:// URL resolution and prevents artifact + * collisions across repeated or nested task invocations. */ import * as fs from "node:fs/promises"; -function escapeRegExp(value: string): string { - return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); -} - /** * Manages agent output ID allocation to ensure uniqueness. * - * Each allocated ID gets a numeric prefix based on allocation order. - * If configured with a parent prefix, the numeric prefix is appended after - * the parent (e.g., "0-Parent.0-Child"). - * On resume, scans existing files to find the next available index. + * The first allocation of a given name keeps the name as-is; subsequent + * allocations of the same name get a `-2`, `-3`, … suffix. On resume, scans + * existing output files so previously written outputs are never overwritten. */ export class AgentOutputManager { - #nextId = 0; #initialized = false; + /** Final ids already handed out, relative to this manager's scope. */ + readonly #taken = new Set(); readonly #getArtifactsDir: () => string | null; readonly #parentPrefix: string | undefined; @@ -33,8 +32,8 @@ export class AgentOutputManager { } /** - * Scan existing agent output files to find the next available ID. - * This ensures we don't overwrite outputs when resuming a session. + * Seed the taken-id set from output files already on disk so a resumed + * session never reuses a name that would clobber a prior subagent's output. */ async #ensureInitialized(): Promise { if (this.#initialized) return; @@ -50,31 +49,41 @@ export class AgentOutputManager { return; // Directory doesn't exist yet } - const pattern = this.#parentPrefix - ? new RegExp(`^${escapeRegExp(this.#parentPrefix)}\\.(\\d+)-.*\\.md$`) - : /^(\d+)-.*\.md$/; - - let maxId = -1; + const prefix = this.#parentPrefix ? `${this.#parentPrefix}.` : ""; for (const file of files) { - const match = file.match(pattern); - if (match) { - const id = Number.parseInt(match[1], 10); - if (id > maxId) maxId = id; + if (!file.endsWith(".md")) continue; + let rest = file.slice(0, -3); // drop ".md" + if (prefix) { + if (!rest.startsWith(prefix)) continue; + rest = rest.slice(prefix.length); } + // Requested ids never contain "."; a dot marks a nested child, so this + // manager only owns the first segment of whatever remains. + const dot = rest.indexOf("."); + const segment = dot === -1 ? rest : rest.slice(0, dot); + if (segment) this.#taken.add(segment); } - this.#nextId = maxId + 1; + } + + /** Pick the first free name (base, then `base-2`, `base-3`, …) and reserve it. */ + #allocateUnique(id: string): string { + let candidate = id; + for (let n = 2; this.#taken.has(candidate); n++) { + candidate = `${id}-${n}`; + } + this.#taken.add(candidate); + return this.#parentPrefix ? `${this.#parentPrefix}.${candidate}` : candidate; } /** - * Allocate a unique ID with numeric prefix. + * Allocate a unique ID. * - * @param id Requested ID (e.g., "AuthProvider") - * @returns Unique ID with prefix (e.g., "0-AuthProvider") + * @param id Requested ID (e.g., "Anna") + * @returns Unique ID ("Anna" first, then "Anna-2", "Anna-3", …) */ async allocate(id: string): Promise { await this.#ensureInitialized(); - const prefix = this.#parentPrefix ? `${this.#parentPrefix}.` : ""; - return `${prefix}${this.#nextId++}-${id}`; + return this.#allocateUnique(id); } /** @@ -85,23 +94,6 @@ export class AgentOutputManager { */ async allocateBatch(ids: string[]): Promise { await this.#ensureInitialized(); - const prefix = this.#parentPrefix ? `${this.#parentPrefix}.` : ""; - return ids.map(id => `${prefix}${this.#nextId++}-${id}`); - } - - /** - * Get the next ID that would be allocated (without allocating). - */ - async peekNextIndex(): Promise { - await this.#ensureInitialized(); - return this.#nextId; - } - - /** - * Reset state (primarily for testing). - */ - reset(): void { - this.#nextId = 0; - this.#initialized = false; + return ids.map(id => this.#allocateUnique(id)); } } diff --git a/packages/coding-agent/src/task/render.ts b/packages/coding-agent/src/task/render.ts index a7473d986..5babac27e 100644 --- a/packages/coding-agent/src/task/render.ts +++ b/packages/coding-agent/src/task/render.ts @@ -128,15 +128,10 @@ function formatJsonScalar(value: unknown, _theme: Theme): string { } function formatTaskId(id: string): string { + // Ids are name-based (e.g. "Anna", "Anna-2"); a "." separates nesting levels + // (e.g. "Anna.Bob"). Render the hierarchy with a ">" breadcrumb. const segments = id.split("."); - if (segments.length < 2) return id; - - const parsed = segments.map(segment => segment.match(/^(\d+)-(.+)$/)); - if (parsed.some(match => !match)) return id; - - const indices = parsed.map(match => match![1]).join("."); - const labels = parsed.map(match => match![2]).join(">"); - return `${indices} ${labels}`; + return segments.length < 2 ? id : segments.join(">"); } const MISSING_YIELD_WARNING_PREFIX = "SYSTEM WARNING: Subagent exited without calling yield tool"; diff --git a/packages/coding-agent/src/tools/index.ts b/packages/coding-agent/src/tools/index.ts index 973823980..8467b7349 100644 --- a/packages/coding-agent/src/tools/index.ts +++ b/packages/coding-agent/src/tools/index.ts @@ -159,7 +159,7 @@ export interface ToolSession { getHindsightSessionState?: () => HindsightSessionState | undefined; /** Get Mnemopi runtime state for this agent session. */ getMnemopiSessionState?: () => MnemopiSessionState | undefined; - /** Agent identity used for IRC routing. Returns the registry id (e.g. "0-Main", "0-AuthLoader"). */ + /** Agent identity used for IRC routing. Returns the registry id (e.g. "Main", "AuthLoader"). */ getAgentId?: () => string | null; /** Look up a registered tool by name (used by the eval js backend's tool bridge). */ getToolByName?: (name: string) => AgentTool | undefined; diff --git a/packages/coding-agent/test/task/output-manager.test.ts b/packages/coding-agent/test/task/output-manager.test.ts new file mode 100644 index 000000000..50fef2440 --- /dev/null +++ b/packages/coding-agent/test/task/output-manager.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from "bun:test"; +import * as path from "node:path"; +import { AgentOutputManager } from "@oh-my-pi/pi-coding-agent/task/output-manager"; +import { TempDir } from "@oh-my-pi/pi-utils"; + +// Contract: subagent output ids are the requested name, used verbatim the first +// time and suffixed (`-2`, `-3`, …) only when the same name recurs. A parent +// prefix nests ids under it. On resume the manager scans existing `.md` outputs +// so it never reuses a name that would clobber a previously written output. + +describe("AgentOutputManager", () => { + it("uses the requested name verbatim and suffixes only on repeat", async () => { + const mgr = new AgentOutputManager(() => null); + + expect(await mgr.allocate("Anna")).toBe("Anna"); + expect(await mgr.allocate("Anna")).toBe("Anna-2"); + expect(await mgr.allocate("Anna")).toBe("Anna-3"); + // A distinct name is untouched — no prefix, no suffix. + expect(await mgr.allocate("Bob")).toBe("Bob"); + }); + + it("de-duplicates within a batch while preserving order", async () => { + const mgr = new AgentOutputManager(() => null); + + expect(await mgr.allocateBatch(["Auth", "Auth", "Api", "Auth"])).toEqual(["Auth", "Auth-2", "Api", "Auth-3"]); + }); + + it("nests ids under a parent prefix and still suffixes repeats", async () => { + const mgr = new AgentOutputManager(() => null, { parentPrefix: "Anna" }); + + expect(await mgr.allocate("Bob")).toBe("Anna.Bob"); + expect(await mgr.allocate("Bob")).toBe("Anna.Bob-2"); + expect(await mgr.allocate("Carol")).toBe("Anna.Carol"); + }); + + it("scans existing output files so a resume never clobbers prior outputs", async () => { + using tmp = TempDir.createSync("@omp-output-manager-"); + const dir = tmp.path(); + await Bun.write(path.join(dir, "Anna.md"), "prior"); + await Bun.write(path.join(dir, "Anna-2.md"), "prior"); + // Unrelated tool artifacts (numeric `.log` ids) must not be mistaken for names. + await Bun.write(path.join(dir, "7.bash.log"), "noise"); + + const mgr = new AgentOutputManager(() => dir); + + expect(await mgr.allocate("Anna")).toBe("Anna-3"); + // A name with no file on disk is still pristine. + expect(await mgr.allocate("Bob")).toBe("Bob"); + }); + + it("only counts files within its own prefix scope on resume", async () => { + using tmp = TempDir.createSync("@omp-output-manager-"); + const dir = tmp.path(); + await Bun.write(path.join(dir, "Anna.Bob.md"), "child"); + await Bun.write(path.join(dir, "Anna.Bob.Carol.md"), "grandchild"); + // A different parent's child must be ignored by Anna's manager. + await Bun.write(path.join(dir, "Other.Bob.md"), "elsewhere"); + + const mgr = new AgentOutputManager(() => dir, { parentPrefix: "Anna" }); + + expect(await mgr.allocate("Bob")).toBe("Anna.Bob-2"); + expect(await mgr.allocate("Dave")).toBe("Anna.Dave"); + }); +}); diff --git a/packages/coding-agent/test/task/render-nested-live.test.ts b/packages/coding-agent/test/task/render-nested-live.test.ts index 9fc3ddb33..fac5e4431 100644 --- a/packages/coding-agent/test/task/render-nested-live.test.ts +++ b/packages/coding-agent/test/task/render-nested-live.test.ts @@ -99,15 +99,15 @@ describe("task renderer: nested live rendering", () => { it("renders completed nested task results stored in extractedToolData.task while parent is in-progress", async () => { const parent = makeRunningProgress({ - id: "1-Parent", + id: "Parent", recentTools: [{ tool: "task", args: "", endMs: Date.now() }], extractedToolData: { task: [ { projectAgentsDir: null, results: [ - makeCompletedSubResult("1-Parent.0-AlphaSub", "Alpha child"), - makeCompletedSubResult("1-Parent.1-BetaSub", "Beta child"), + makeCompletedSubResult("Parent.AlphaSub", "Alpha child"), + makeCompletedSubResult("Parent.BetaSub", "Beta child"), ], totalDurationMs: 1000, } satisfies TaskToolDetails, @@ -119,12 +119,12 @@ describe("task renderer: nested live rendering", () => { // Parent label is intact. expect(text).toContain("Parent Level 1 work"); - // Both nested completed children labels surface (formatTaskId collapses - // dotted ids → "1.0 Parent>AlphaSub"). + // Both nested completed children labels surface (formatTaskId renders the + // dotted hierarchy as a "Parent>AlphaSub" breadcrumb). expect(text).toContain("Alpha child"); expect(text).toContain("Beta child"); - expect(text).toContain("1.0 Parent>AlphaSub"); - expect(text).toContain("1.1 Parent>BetaSub"); + expect(text).toContain("Parent>AlphaSub"); + expect(text).toContain("Parent>BetaSub"); }); it("renders the in-flight nested task snapshot (progress[]) before the call ends", async () => { @@ -133,12 +133,12 @@ describe("task renderer: nested live rendering", () => { results: [], totalDurationMs: 0, progress: [ - makeRunningSubProgress("2-Parent.0-GammaSub", "Gamma child running"), - makeRunningSubProgress("2-Parent.1-DeltaSub", "Delta child running"), + makeRunningSubProgress("Parent.GammaSub", "Gamma child running"), + makeRunningSubProgress("Parent.DeltaSub", "Delta child running"), ], }; const parent = makeRunningProgress({ - id: "2-Parent", + id: "Parent", currentTool: "task", currentToolStartMs: Date.now(), inflightTaskDetails: inflight, @@ -149,8 +149,8 @@ describe("task renderer: nested live rendering", () => { expect(text).toContain("Parent Level 1 work"); expect(text).toContain("Gamma child running"); expect(text).toContain("Delta child running"); - expect(text).toContain("2.0 Parent>GammaSub"); - expect(text).toContain("2.1 Parent>DeltaSub"); + expect(text).toContain("Parent>GammaSub"); + expect(text).toContain("Parent>DeltaSub"); }); it("combines completed and in-flight nested snapshots in one tree", async () => { @@ -160,7 +160,7 @@ describe("task renderer: nested live rendering", () => { task: [ { projectAgentsDir: null, - results: [makeCompletedSubResult("3.0-EpsilonSub", "Epsilon done")], + results: [makeCompletedSubResult("Parent.EpsilonSub", "Epsilon done")], totalDurationMs: 1000, } satisfies TaskToolDetails, ], @@ -169,7 +169,7 @@ describe("task renderer: nested live rendering", () => { projectAgentsDir: null, results: [], totalDurationMs: 0, - progress: [makeRunningSubProgress("3.1-ZetaSub", "Zeta running")], + progress: [makeRunningSubProgress("Parent.ZetaSub", "Zeta running")], }, });