diff --git a/docs/tools/task.md b/docs/tools/task.md index 4bdf27a99..61be00580 100644 --- a/docs/tools/task.md +++ b/docs/tools/task.md @@ -90,7 +90,7 @@ Artifacts and side channels: 10. Artifacts dir comes from the parent session file when available, otherwise a temp dir. When the session is executing an approved plan, the plan reference is handed to the subagent. 11. Non-isolated spawns call `runSubprocess(...)` directly with parent cwd; isolated spawns run inside the isolation workspace, then commit to a branch (`mergeMode === "branch"`) or capture a patch, and always clean up the workspace. 12. `runSubprocess(...)` creates a child agent session with an isolated settings snapshot (forcing `async.enabled = false` and `bash.autoBackground.enabled = false` — subagents are internally synchronous), child `agentId` equal to the allocated id, child internal URL router/`AgentOutputManager`, output schema, the shared `context` (batch calls) in the system prompt's `CONTEXT` section, and the IRC peer roster in the system prompt. -13. Child tool availability: explicit `agent.tools` if provided; auto-add `task` when the agent has `spawns` and depth allows; strip `task` at `task.maxRecursionDepth`; ensure `irc` is present in explicit tool lists; expand `exec` to `eval` + `bash`; strip parent-owned `todo`. +13. Child tool availability: explicit `agent.tools` if provided; auto-add `task` when the agent has `spawns` and depth allows; strip `task` at `task.maxRecursionDepth`; ensure `irc` is present in explicit tool lists; expand `exec` to `eval` + `bash`; strip parent-owned `todo` — unless the spawn is prewalk-armed, whose plan nudge + todo gate need the child to commit its own todo list before the model hand-off. 14. The child must finish through the hidden `yield` tool; up to 3 reminder prompts, the last forcing `toolChoice = yield` when supported. `finalizeSubprocessOutput(...)` reconciles raw text, `yield` payloads, structured schemas, `report_finding` data, and abort states. 15. End-of-run lifecycle (keep-alive, in `runSubprocess`'s finalizer): - hard abort (caller signal / wall-clock / budget) → registry status `aborted`, session disposed — terminal; diff --git a/docs/tools/todo.md b/docs/tools/todo.md index c59c6efa8..2c2abac06 100644 --- a/docs/tools/todo.md +++ b/docs/tools/todo.md @@ -150,5 +150,5 @@ The same file also exposes non-tool helpers used by `/todo`: - plain `todo` calls survive in transcript tool-result details; - `/todo` command edits additionally append `customType: "user_todo_edit"` entries and inject a visible-to-model `` developer message describing the manual edit. - On session resume, `AgentSession.#syncTodoPhasesFromBranch()` strips `completed` and `abandoned` tasks before restoring the cached list. The `/todo` command works around that by reading the latest transcript/custom-entry state so historical done/dropped tasks still appear to the user. -- Tool availability is gated by `todo.enabled`, and the registry excludes it when `includeYield` is enabled (`packages/coding-agent/src/tools/index.ts`). -- Subagents do not inherit `todo`; `packages/coding-agent/src/task/executor.ts` filters it out as a parent-owned tool. +- Tool availability is gated by `todo.enabled`, and the registry excludes it when `includeYield` is enabled unless the session is prewalk-armed (`packages/coding-agent/src/tools/index.ts`). +- Subagents do not inherit `todo`; `packages/coding-agent/src/task/executor.ts` also filters it from the active set as a parent-owned tool. Exception (both layers): prewalk-armed subagents keep it — the prewalk plan nudge and todo gate require the child to commit its own todo list before the hand-off. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 680145e53..af88c4d6b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Added -- Added per-agent prewalk for subagents: a `prewalk` frontmatter field (`true` = hand off to the default prewalk target, a string = custom target model pattern) and a `task.agentPrewalk` settings override toggled per agent from the `/agents` dashboard with `P`. The bundled generic `task` agent ships with prewalk enabled by default (skipped when the target resolves to the subagent's own starting model, and never armed for plan-mode spawns). +- Added per-agent prewalk for subagents: a `prewalk` frontmatter field (`true` = hand off to the default prewalk target, a string = custom target model pattern) and a `task.agentPrewalk` settings override toggled per agent from the `/agents` dashboard with `P`. The bundled generic `task` agent ships with prewalk enabled by default (skipped when the target resolves to the subagent's own starting model, and never armed for plan-mode spawns). Prewalk-armed subagents keep the normally parent-owned `todo` tool so the plan-nudge → todo → hand-off flow works, and the prewalk todo gate now keys on the active tool set instead of the registry so a deactivated todo tool can no longer stall the switch. ### Changed diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 92722e32b..df558a675 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -1750,7 +1750,7 @@ export class AgentSession { #prewalkPlanInjected = false; /** True once any successful `todo` call landed — opens the prewalk * trigger gate: the switch fires at the first edit/write AFTER the todo - * list exists (sessions without a todo tool skip the gate). */ + * list exists (sessions without an ACTIVE todo tool skip the gate). */ #prewalkTodoSeen = false; #planYolo: PlanYolo | undefined; #planYoloPreviousTools: string[] | undefined; @@ -2270,12 +2270,14 @@ export class AgentSession { // todo list from it and start" — so the switch waits until a todo list // exists AND the model has actually started implementing (first // edit/write). The todo call itself never triggers: firing there handed - // the fast model the whole implementation cold. Sessions without a todo - // tool skip the gate. + // the fast model the whole implementation cold. The gate keys on the + // ACTIVE tool set, not the registry: a registered-but-deactivated todo + // (e.g. a restricted active-tool slate) is uncallable and would + // deadlock the switch. if (context.toolResults.some(result => result.toolName === "todo")) { this.#prewalkTodoSeen = true; } - const todoGateOpen = this.#prewalkTodoSeen || !this.#toolRegistry.has("todo"); + const todoGateOpen = this.#prewalkTodoSeen || !this.getActiveToolNames().includes("todo"); const action = todoGateOpen ? context.toolResults.find(result => PREWALK_ACTION_TOOLS[result.toolName]) : undefined; diff --git a/packages/coding-agent/src/task/executor.ts b/packages/coding-agent/src/task/executor.ts index 8f3f33a39..c91a2f277 100644 --- a/packages/coding-agent/src/task/executor.ts +++ b/packages/coding-agent/src/task/executor.ts @@ -2575,9 +2575,12 @@ export async function runSubprocess(options: ExecutorOptions): Promise !prewalk && name === "todo"; const subagentToolNames = session.getActiveToolNames(); - const parentOwnedToolNames = new Set(["todo"]); - const filteredSubagentTools = subagentToolNames.filter(name => !parentOwnedToolNames.has(name)); + const filteredSubagentTools = subagentToolNames.filter(name => !isParentOwnedTool(name)); if (filteredSubagentTools.length !== subagentToolNames.length) { await awaitAbortable(session.setActiveToolsByName(filteredSubagentTools)); } @@ -2635,7 +2638,7 @@ export async function runSubprocess(options: ExecutorOptions): Promise session.getActiveToolNames(), getAllTools: () => session.getAllToolNames(), setActiveTools: (toolNames: string[]) => - session.setActiveToolsByName(toolNames.filter(name => !parentOwnedToolNames.has(name))), + session.setActiveToolsByName(toolNames.filter(name => !isParentOwnedTool(name))), getCommands: () => getSessionSlashCommands(session), setModel: model => runExtensionSetModel(session, model), getThinkingLevel: () => session.thinkingLevel, diff --git a/packages/coding-agent/src/task/index.ts b/packages/coding-agent/src/task/index.ts index d3f9d03aa..3f8276137 100644 --- a/packages/coding-agent/src/task/index.ts +++ b/packages/coding-agent/src/task/index.ts @@ -206,7 +206,6 @@ function renderDescription( agents: renderedAgents, spawningDisabled, defaultAgent: spawnPolicy.defaultAgent, - allowedAgentsText: spawnPolicy.allowedPromptText, isolationEnabled, batchEnabled, asyncEnabled, diff --git a/packages/coding-agent/src/tools/index.ts b/packages/coding-agent/src/tools/index.ts index 75ee5f75f..1f4529058 100644 --- a/packages/coding-agent/src/tools/index.ts +++ b/packages/coding-agent/src/tools/index.ts @@ -205,6 +205,9 @@ export interface ToolSession { outputSchema?: unknown; /** Whether to include the yield tool by default */ requireYieldTool?: boolean; + /** Session starts with a prewalk hand-off armed. Keeps `todo` in yield-gated + * (subagent) registries: the prewalk plan nudge + todo gate need it. */ + prewalkArmed?: boolean; /** Task recursion depth (0 = top-level, 1 = first child, etc.) */ taskDepth?: number; /** Get shared eval executor session ID. Subagents inherit this to share JS/Python/Ruby/Julia state. */ @@ -612,7 +615,8 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P if (name === "launch") return session.settings.get("launch.enabled"); if (name === "eval") return allowEval; if (name === "debug") return session.settings.get("debug.enabled"); - if (name === "todo") return !includeYield && session.settings.get("todo.enabled"); + if (name === "todo") + return (!includeYield || session.prewalkArmed === true) && session.settings.get("todo.enabled"); if (name === "glob") return session.settings.get("glob.enabled"); if (name === "grep") return session.settings.get("grep.enabled"); if (name === "github") return session.settings.get("github.enabled"); diff --git a/packages/coding-agent/test/agent-session-prewalk.test.ts b/packages/coding-agent/test/agent-session-prewalk.test.ts index 94a903870..ba76c3a9e 100644 --- a/packages/coding-agent/test/agent-session-prewalk.test.ts +++ b/packages/coding-agent/test/agent-session-prewalk.test.ts @@ -291,6 +291,55 @@ describe("AgentSession prewalk", () => { ]); expect(session.model?.id).toBe(target.id); }); + it("skips the todo gate when todo is registered but not active (subagent-style restricted slates)", async () => { + // Regression: the gate used to key on the tool REGISTRY, so a session + // whose active-tool slate excluded `todo` (subagents strip it) while the + // registry still contained it could never open the gate — the model + // cannot call an inactive tool — and prewalk never fired. + const primary = modelOrThrow("claude-sonnet-4-5"); + const target = modelOrThrow("claude-sonnet-4-6"); + const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml")); + + // Turn 1: read-only (nudge injected after). Turn 2: write — first + // edit/write must switch immediately; no todo call is possible. + const mock = createMockModel({ + responses: [toolCall("t1", "record"), toolCall("t2", "write"), { content: ["done"] }], + }); + const requested: string[] = []; + const agent = new Agent({ + getApiKey: () => "test-key", + initialState: { + model: primary, + systemPrompt: ["Test"], + // Active slate excludes todo; the session toolRegistry still has it. + tools: [recordTool as AgentTool, writeTool as AgentTool], + messages: [], + thinkingLevel: Effort.Medium, + }, + convertToLlm, + streamFn: (model, context, options) => { + requested.push(`${model.provider}/${model.id}`); + return mock.stream(model, context, options); + }, + }); + session = new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(), + settings: Settings.isolated({ "compaction.enabled": false }), + modelRegistry, + toolRegistry, + prewalk: { target }, + }); + + await session.prompt("do the task"); + + expect(requested).toEqual([ + `${primary.provider}/${primary.id}`, + `${primary.provider}/${primary.id}`, + `${target.provider}/${target.id}`, + ]); + expect(session.model?.id).toBe(target.id); + }); it("armPrewalk (the /prewalk slash command) pre-arms the switch for the very next edit/write", async () => { const primary = modelOrThrow("claude-sonnet-4-5"); diff --git a/packages/coding-agent/test/task/executor-prewalk.test.ts b/packages/coding-agent/test/task/executor-prewalk.test.ts index f7b14ba11..bea38ca71 100644 --- a/packages/coding-agent/test/task/executor-prewalk.test.ts +++ b/packages/coding-agent/test/task/executor-prewalk.test.ts @@ -24,16 +24,19 @@ import type { AgentDefinition, SingleResult } from "@oh-my-pi/pi-coding-agent/ta import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; import { EventBus } from "@oh-my-pi/pi-coding-agent/utils/event-bus"; -function yieldEmittingSession(): AgentSession { +function yieldEmittingSession(initialTools: string[] = ["read", "yield"]): AgentSession { const listeners: Array<(event: AgentSessionEvent) => void> = []; + let activeTools = initialTools; const session = { state: { messages: [] }, agent: { state: { systemPrompt: ["test"] } }, model: undefined, extensionRunner: undefined, sessionManager: { appendSessionInit: () => {} }, - getActiveToolNames: () => ["read", "yield"], - setActiveToolsByName: async (_toolNames: string[]) => {}, + getActiveToolNames: () => activeTools, + setActiveToolsByName: async (toolNames: string[]) => { + activeTools = toolNames; + }, subscribe: (listener: (event: AgentSessionEvent) => void) => { listeners.push(listener); return () => { @@ -206,6 +209,36 @@ describe("runSubprocess per-agent prewalk", () => { expect(result.exitCode).toBe(0); expect(spy.mock.calls[0]?.[0]?.prewalk).toBeUndefined(); }); + it("keeps the todo tool active for a prewalk-armed subagent (the todo gate needs it)", async () => { + const session = yieldEmittingSession(["read", "todo", "yield"]); + vi.spyOn(sdkModule, "createAgentSession").mockResolvedValue(createSessionResult(session)); + + const result = await runSubprocess({ + ...baseOptions("subagent-prewalk-todo-kept", Settings.isolated()), + agent: { + ...baseAgent, + model: [`${primary.provider}/${primary.id}`], + prewalk: `${target.provider}/${target.id}`, + }, + }); + + expect(result.exitCode).toBe(0); + expect(session.getActiveToolNames()).toContain("todo"); + }); + + it("strips the parent-owned todo tool from non-prewalk subagents", async () => { + const session = yieldEmittingSession(["read", "todo", "yield"]); + vi.spyOn(sdkModule, "createAgentSession").mockResolvedValue(createSessionResult(session)); + + const result = await runSubprocess({ + ...baseOptions("subagent-no-prewalk-todo-stripped", Settings.isolated()), + agent: { ...baseAgent, model: [`${primary.provider}/${primary.id}`] }, + }); + + expect(result.exitCode).toBe(0); + expect(session.getActiveToolNames()).not.toContain("todo"); + expect(session.getActiveToolNames()).toContain("read"); + }); }); // Plan-mode spawns are read-only exploration: the task tool must strip a // prewalk-enabled agent definition before spawning so the hidden