From 975391b4fc1a44893d66a551ca8886e0c30d2171 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 12:25:52 -0700 Subject: [PATCH 1/5] Collapse createChatDirector's optional parameters into an options object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ten positional parameters made call sites error-prone — several same-typed optional callbacks sat next to each other with nothing but position to distinguish them. A single ChatDirectorOptions object, shared between the factory function and the class constructor, makes each call site self- describing and immune to accidental argument swaps. --- src/agent/director.test.ts | 91 +++------------------------ src/agent/director.ts | 74 ++++++++++------------ src/director.test.ts | 18 +++--- src/exec/runner.ts | 19 +++--- src/tui/runner.ts | 19 +++--- tests/integration/harness.ts | 8 +-- tests/unit/director.test.ts | 16 ++--- tests/unit/workflows-director.test.ts | 12 ++-- 8 files changed, 77 insertions(+), 180 deletions(-) diff --git a/src/agent/director.test.ts b/src/agent/director.test.ts index baa441b96..a60797c8d 100644 --- a/src/agent/director.test.ts +++ b/src/agent/director.test.ts @@ -100,18 +100,7 @@ describe("ChatDirector tool-only loop protection", () => { const providerlessPolicy = { providerName: "test-provider" }; test("nudges once at the family threshold, after pending tools execute", async () => { - const director = createChatDirector( - "system", - [], - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - providerlessPolicy, - ); + const director = createChatDirector("system", [], { provider: providerlessPolicy }); const capabilities = makeCapabilities(); // Default family nudges at 12 consecutive tool-only turns. @@ -122,18 +111,7 @@ describe("ChatDirector tool-only loop protection", () => { }); test("the nudge is one-shot — it does not repeat on the next tool-only turn", async () => { - const director = createChatDirector( - "system", - [], - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - providerlessPolicy, - ); + const director = createChatDirector("system", [], { provider: providerlessPolicy }); const capabilities = makeCapabilities(); await runToolOnlyStreak(director, capabilities, 12); @@ -144,18 +122,7 @@ describe("ChatDirector tool-only loop protection", () => { }); test("pauses and stops issuing infers at the family pause threshold", async () => { - const director = createChatDirector( - "system", - [], - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - providerlessPolicy, - ); + const director = createChatDirector("system", [], { provider: providerlessPolicy }); const capabilities = makeCapabilities(); // Default family pauses at 20 consecutive tool-only turns. @@ -169,18 +136,7 @@ describe("ChatDirector tool-only loop protection", () => { }); test("resumes after the operator sends a new message", async () => { - const director = createChatDirector( - "system", - [], - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - providerlessPolicy, - ); + const director = createChatDirector("system", [], { provider: providerlessPolicy }); const capabilities = makeCapabilities(); await runToolOnlyStreak(director, capabilities, 20); @@ -191,18 +147,7 @@ describe("ChatDirector tool-only loop protection", () => { }); test("a dismissed ask_operator counts toward the streak like any other tool-only turn", async () => { - const director = createChatDirector( - "system", - [], - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - providerlessPolicy, - ); + const director = createChatDirector("system", [], { provider: providerlessPolicy }); const capabilities = makeCapabilities(); // 11 ordinary tool-only turns, then a turn whose only tool call is a @@ -252,18 +197,7 @@ describe("ChatDirector tool-only loop protection", () => { }); test("a busy-but-progressing session (text interleaved with tools) never trips", async () => { - const director = createChatDirector( - "system", - [], - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - providerlessPolicy, - ); + const director = createChatDirector("system", [], { provider: providerlessPolicy }); const capabilities = makeCapabilities(); let lastActions: ReactorAction[] = []; @@ -278,18 +212,7 @@ describe("ChatDirector tool-only loop protection", () => { }); test("grok's tightened thresholds fire earlier than the default family", async () => { - const director = createChatDirector( - "system", - [], - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - { providerName: "xai/default", model: "grok-4.5" }, - ); + const director = createChatDirector("system", [], { provider: { providerName: "xai/default", model: "grok-4.5" } }); const capabilities = makeCapabilities(); // Grok nudges at 6, well below the default family's 12. diff --git a/src/agent/director.ts b/src/agent/director.ts index 593cfd872..b5ffe2bec 100644 --- a/src/agent/director.ts +++ b/src/agent/director.ts @@ -303,6 +303,23 @@ function isCodeFile(path: string): boolean { return CODE_FILE_EXT.test(path); } +export type ChatDirectorOptions = { + taskClassifier?: ((message: string, metadata: SessionMetadata) => Promise) | undefined; + onActivateTools?: ((names: string[]) => void) | undefined; + inactivityTimeoutMs?: number | undefined; + totalTimeoutMs?: number | undefined; + workflowCoordinator?: WorkflowCoordinator | undefined; + onTasksChange?: ((tasks: Task[]) => void) | undefined; + requestContinuation?: (() => void) | undefined; + provider?: { providerName: string; model?: string } | undefined; +}; + +// The constructor takes the resolved ModelFamilyPolicy rather than the raw +// `provider` input the factory function accepts and resolves on its behalf. +type ChatDirectorImplOptions = Omit & { + modelFamilyPolicy?: ModelFamilyPolicy | undefined; +}; + class ChatDirectorImpl extends DefaultDirector { private readonly workflowCalls = new Map(); private readonly lspTriggerCalls = new Set(); @@ -341,29 +358,18 @@ class ChatDirectorImpl extends DefaultDirector { private pendingToolOnlyNudge = false; private pausedForToolOnly = false; - constructor( - systemPrompt: string, - toolDefinitions: ToolDefinition[], - taskClassifier?: (message: string, metadata: SessionMetadata) => Promise, - onActivateTools?: (names: string[]) => void, - inactivityTimeoutMs?: number, - totalTimeoutMs?: number, - workflowCoordinator?: WorkflowCoordinator, - onTasksChange?: (tasks: Task[]) => void, - requestContinuation?: () => void, - modelFamilyPolicy?: ModelFamilyPolicy, - ) { + constructor(systemPrompt: string, toolDefinitions: ToolDefinition[], options: ChatDirectorImplOptions = {}) { super(systemPrompt, toolDefinitions, {}); this._systemPrompt = systemPrompt; this._toolDefinitions = toolDefinitions; - this.inactivityTimeoutMs = inactivityTimeoutMs; - this.totalTimeoutMs = totalTimeoutMs; - this.taskClassifier = taskClassifier; - this.onActivateTools = onActivateTools; - this.workflowCoordinator = workflowCoordinator; - this.onTasksChange = onTasksChange; - this.compaction = createCompactionGovernor(requestContinuation, systemPrompt, toolDefinitions); - this.modelFamilyPolicy = modelFamilyPolicy ?? resolveModelFamilyPolicy({ providerName: "" }); + this.inactivityTimeoutMs = options.inactivityTimeoutMs; + this.totalTimeoutMs = options.totalTimeoutMs; + this.taskClassifier = options.taskClassifier; + this.onActivateTools = options.onActivateTools; + this.workflowCoordinator = options.workflowCoordinator; + this.onTasksChange = options.onTasksChange; + this.compaction = createCompactionGovernor(options.requestContinuation, systemPrompt, toolDefinitions); + this.modelFamilyPolicy = options.modelFamilyPolicy ?? resolveModelFamilyPolicy({ providerName: "" }); } setWorkflowCoordinator(coordinator: WorkflowCoordinator | undefined): void { @@ -775,27 +781,15 @@ class ChatDirectorImpl extends DefaultDirector { export function createChatDirector( systemPrompt: string, toolDefinitions: ToolDefinition[], - taskClassifier?: (message: string, metadata: SessionMetadata) => Promise, - onActivateTools?: (names: string[]) => void, - inactivityTimeoutMs?: number, - totalTimeoutMs?: number, - workflowCoordinator?: WorkflowCoordinator, - onTasksChange?: (tasks: Task[]) => void, - requestContinuation?: () => void, - provider?: { providerName: string; model?: string }, + options: ChatDirectorOptions = {}, ): ChatDirector { - return new ChatDirectorImpl( - systemPrompt, - toolDefinitions, - taskClassifier, - onActivateTools, - inactivityTimeoutMs, - totalTimeoutMs, - workflowCoordinator, - onTasksChange, - requestContinuation, - provider !== undefined ? resolveModelFamilyPolicy(provider) : undefined, - ); + const { provider, ...rest } = options; + return new ChatDirectorImpl(systemPrompt, toolDefinitions, { + ...rest, + // `provider` is raw {providerName, model} input; the constructor wants + // the resolved ModelFamilyPolicy, not the input it was resolved from. + modelFamilyPolicy: provider !== undefined ? resolveModelFamilyPolicy(provider) : undefined, + }); } export interface ChatDirector extends ReactorDirector { diff --git a/src/director.test.ts b/src/director.test.ts index fbc85a3c1..0e4438d56 100644 --- a/src/director.test.ts +++ b/src/director.test.ts @@ -273,8 +273,10 @@ describe("chatDirector compaction", () => { test("schedules idle compaction after an over-threshold text-only reply", async () => { let continuations = 0; - const director = createChatDirector("", [], undefined, undefined, undefined, undefined, undefined, undefined, () => { - continuations++; + const director = createChatDirector("", [], { + requestContinuation: () => { + continuations++; + }, }); // One turn past createPruningCompactor's own no-op floor (session/compactor.ts), // so the arming check finds a history actually worth compacting. @@ -326,7 +328,7 @@ describe("chatDirector compaction", () => { } function chatDirectorWithContinuation(onContinuation?: () => void) { - return createChatDirector("", [], undefined, undefined, undefined, undefined, undefined, undefined, onContinuation ?? (() => {})); + return createChatDirector("", [], { requestContinuation: onContinuation ?? (() => {}) }); } test("compacts at the tool.done pause once over threshold", async () => { @@ -437,7 +439,7 @@ describe("chatDirector compaction", () => { describe("chatDirector LSP auto-activation", () => { test("reading a code file activates the lsp tool on success", async () => { const activated: string[][] = []; - const director = createChatDirector("", [], undefined, (names: string[]) => activated.push(names)); + const director = createChatDirector("", [], { onActivateTools: (names: string[]) => activated.push(names) }); await director.decide(makeInferenceDoneEvent([{ id: "c", name: "read_file", args: { path: "src/foo.ts" } }]), mockState, mockCapabilities); await director.decide(makeToolDoneEvent("c"), mockState, mockCapabilities); expect(activated).toEqual([["lsp"]]); @@ -445,7 +447,7 @@ describe("chatDirector LSP auto-activation", () => { test("editing a code file activates lsp", async () => { const activated: string[][] = []; - const director = createChatDirector("", [], undefined, (names: string[]) => activated.push(names)); + const director = createChatDirector("", [], { onActivateTools: (names: string[]) => activated.push(names) }); await director.decide(makeInferenceDoneEvent([{ id: "c", name: "edit_file", args: { path: "lib/bar.rs" } }]), mockState, mockCapabilities); await director.decide(makeToolDoneEvent("c"), mockState, mockCapabilities); expect(activated).toEqual([["lsp"]]); @@ -453,7 +455,7 @@ describe("chatDirector LSP auto-activation", () => { test("a non-code file does not activate lsp", async () => { const activated: string[][] = []; - const director = createChatDirector("", [], undefined, (names: string[]) => activated.push(names)); + const director = createChatDirector("", [], { onActivateTools: (names: string[]) => activated.push(names) }); await director.decide(makeInferenceDoneEvent([{ id: "c", name: "read_file", args: { path: "README.md" } }]), mockState, mockCapabilities); await director.decide(makeToolDoneEvent("c"), mockState, mockCapabilities); expect(activated).toEqual([]); @@ -461,7 +463,7 @@ describe("chatDirector LSP auto-activation", () => { test("a failed read does not activate lsp", async () => { const activated: string[][] = []; - const director = createChatDirector("", [], undefined, (names: string[]) => activated.push(names)); + const director = createChatDirector("", [], { onActivateTools: (names: string[]) => activated.push(names) }); await director.decide(makeInferenceDoneEvent([{ id: "c", name: "read_file", args: { path: "src/foo.ts" } }]), mockState, mockCapabilities); await director.decide(makeToolErrorEvent("c", "Error: not found"), mockState, mockCapabilities); expect(activated).toEqual([]); @@ -598,7 +600,7 @@ describe("updateToolDefinitions rewrites infer tools", () => { test("the new-task path also carries the current tools", async () => { const classifier = async (_msg: string, _meta: SessionMetadata) => ({ kind: "new_task" as const, reason: "pivot" } as TaskBoundary); - const director = createChatDirector("base-prompt", [], classifier); + const director = createChatDirector("base-prompt", [], { taskClassifier: classifier }); director.updateToolDefinitions([lateTool]); const result = await director.decide(makeMessageReceivedEvent("new thing"), mockState, capabilitiesWithInferArgs); diff --git a/src/exec/runner.ts b/src/exec/runner.ts index 585fce223..c61628d95 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -399,26 +399,21 @@ export async function runExec(config: Config): Promise { id: `${ID_PREFIX}/chat`, configSchema: type({}), factory: (_cfg, _env, agentCtx) => { - const d = createChatDirector( - agentCtx.systemPrompt, - computeAdvertised([...agentCtx.toolDefinitions]), - undefined, - (names) => { + const d = createChatDirector(agentCtx.systemPrompt, computeAdvertised([...agentCtx.toolDefinitions]), { + onActivateTools: (names) => { if (!activatedToolNames.activate(names)) return; directorHolder.instance?.updateToolDefinitions( computeAdvertised(agentToolset.dynamicRunner.currentDefinitions()), ); }, - config.inactivityTimeoutMs ?? 750_000, - config.totalTimeoutMs, - undefined, - undefined, - () => { + inactivityTimeoutMs: config.inactivityTimeoutMs ?? 750_000, + totalTimeoutMs: config.totalTimeoutMs, + requestContinuation: () => { // Compaction governor self-delivers after compact so the loop re-enters. currentAgent?.deliver(buildCompactionContinuationMessage()); }, - { providerName: config.providerName, model: config.model }, - ); + provider: { providerName: config.providerName, model: config.model }, + }); directorHolder.instance = d; return d; }, diff --git a/src/tui/runner.ts b/src/tui/runner.ts index a7e70be1a..276846ac8 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -1170,20 +1170,15 @@ export async function runTUI(initialConfig: Config): Promise { id: `${ID_PREFIX}/chat`, configSchema: type({}), factory: (_config, _env, agentCtx) => { - const d = createChatDirector( - agentCtx.systemPrompt, - computeAdvertised([...agentCtx.toolDefinitions]), - undefined, - (names) => promoteTools(names), - config.inactivityTimeoutMs ?? 750_000, - config.totalTimeoutMs, - undefined, - undefined, - () => { + const d = createChatDirector(agentCtx.systemPrompt, computeAdvertised([...agentCtx.toolDefinitions]), { + onActivateTools: (names) => promoteTools(names), + inactivityTimeoutMs: config.inactivityTimeoutMs ?? 750_000, + totalTimeoutMs: config.totalTimeoutMs, + requestContinuation: () => { enqueueAgentDeliver(() => currentAgent.deliver(buildCompactionContinuationMessage())); }, - { providerName: config.providerName, model: config.model }, - ); + provider: { providerName: config.providerName, model: config.model }, + }); d.setGoalGovernor(goalGovernor); directorHolder.instance = d; return d; diff --git a/tests/integration/harness.ts b/tests/integration/harness.ts index a41409ae3..09eb304e8 100644 --- a/tests/integration/harness.ts +++ b/tests/integration/harness.ts @@ -70,13 +70,7 @@ export async function openIntegrationSession( id: `${ID_PREFIX}/chat`, configSchema: type({}), factory: (_config, _env, agentCtx) => - createChatDirector( - agentCtx.systemPrompt, - [...agentCtx.toolDefinitions], - undefined, - undefined, - 750_000, - ), + createChatDirector(agentCtx.systemPrompt, [...agentCtx.toolDefinitions], { inactivityTimeoutMs: 750_000 }), }); const toolsFactory = defineTool({ diff --git a/tests/unit/director.test.ts b/tests/unit/director.test.ts index 820f98bba..db3d7e54f 100644 --- a/tests/unit/director.test.ts +++ b/tests/unit/director.test.ts @@ -105,9 +105,7 @@ const manyTurnsState: ReactorState = { }; function makeChatDirectorWithContinuation(onContinue: () => void) { - return createChatDirector( - "sys", [], undefined, undefined, undefined, undefined, undefined, undefined, onContinue, - ); + return createChatDirector("sys", [], { requestContinuation: onContinue }); } test("current context over threshold emits compact and a continuation request, not a dead loop", async () => { @@ -180,17 +178,13 @@ async function runToolOnlyStreak(director: ReturnType } test("a grok provider pauses the session after 10 tool-only turns, tighter than the default 20", async () => { - const grokDirector = createChatDirector( - "sys", [], undefined, undefined, undefined, undefined, undefined, undefined, undefined, - { providerName: "xai", model: "grok-4" }, - ); + const grokDirector = createChatDirector("sys", [], { provider: { providerName: "xai", model: "grok-4" } }); const grokActions = await runToolOnlyStreak(grokDirector, 10); expect(grokActions.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe(true); - const defaultDirector = createChatDirector( - "sys", [], undefined, undefined, undefined, undefined, undefined, undefined, undefined, - { providerName: "openai", model: "gpt-4" }, - ); + const defaultDirector = createChatDirector("sys", [], { + provider: { providerName: "openai", model: "gpt-4" }, + }); const defaultActions = await runToolOnlyStreak(defaultDirector, 10); expect(defaultActions.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe(false); }); diff --git a/tests/unit/workflows-director.test.ts b/tests/unit/workflows-director.test.ts index 54ac8ab54..4576bc5ac 100644 --- a/tests/unit/workflows-director.test.ts +++ b/tests/unit/workflows-director.test.ts @@ -91,7 +91,7 @@ test("the active step directive is injected into the inferred system prompt", as const runtime = new WorkflowRuntime(emptyCaps, (n) => (n === "flow" ? flow : undefined)); runtime.start(flow); const coordinator = new WorkflowCoordinator(runtime); - const director = createChatDirector("BASE PROMPT", [], undefined, undefined, undefined, undefined, coordinator); + const director = createChatDirector("BASE PROMPT", [], { workflowCoordinator: coordinator }); const event: ReactorInboundEvent = { type: "message.received", message: { role: "user", content: "go" } }; const result = await director.decide(event, state, makeCapabilities()); @@ -109,7 +109,7 @@ test("an advance_workflow tool call advances the runtime through the director", const runtime = new WorkflowRuntime(emptyCaps, (n) => (n === "flow" ? flow : undefined)); runtime.start(flow); const coordinator = new WorkflowCoordinator(runtime); - const director = createChatDirector("BASE", [], undefined, undefined, undefined, undefined, coordinator); + const director = createChatDirector("BASE", [], { workflowCoordinator: coordinator }); const caps = makeCapabilities(); const turn: ReactorInboundEvent = { @@ -153,7 +153,7 @@ test("auto-continuation fires on reply() as well as wait() after a text turn", a const runtime = new WorkflowRuntime(emptyCaps, (n) => (n === "flow" ? flow : undefined)); runtime.start(flow); const coordinator = new WorkflowCoordinator(runtime); - const director = createChatDirector("BASE", [], undefined, undefined, undefined, undefined, coordinator); + const director = createChatDirector("BASE", [], { workflowCoordinator: coordinator }); const caps = makeCapabilities(); // Simulate a text-only inference turn (no tool calls). @@ -206,7 +206,7 @@ test("a content-free workflow turn with open tasks nudges toward advance_workflo const runtime = new WorkflowRuntime(emptyCaps, (n) => (n === "flow" ? flow : undefined)); runtime.start(flow); const coordinator = new WorkflowCoordinator(runtime); - const director = createChatDirector("BASE", [], undefined, undefined, undefined, undefined, coordinator); + const director = createChatDirector("BASE", [], { workflowCoordinator: coordinator }); const caps = makeCapabilities(); await director.decide(manageTasksTurn("doing"), state, caps); @@ -229,7 +229,7 @@ test("open tasks do not defeat the workflow stuck-cutoff after 3 idle turns", as const runtime = new WorkflowRuntime(emptyCaps, (n) => (n === "flow" ? flow : undefined)); runtime.start(flow); const coordinator = new WorkflowCoordinator(runtime); - const director = createChatDirector("BASE", [], undefined, undefined, undefined, undefined, coordinator); + const director = createChatDirector("BASE", [], { workflowCoordinator: coordinator }); const caps = makeCapabilities(); await director.decide(manageTasksTurn("doing"), state, caps); @@ -247,7 +247,7 @@ test("auto-continuation falls back after 3 consecutive text-only turns", async ( const runtime = new WorkflowRuntime(emptyCaps, (n) => (n === "flow" ? flow : undefined)); runtime.start(flow); const coordinator = new WorkflowCoordinator(runtime); - const director = createChatDirector("BASE", [], undefined, undefined, undefined, undefined, coordinator); + const director = createChatDirector("BASE", [], { workflowCoordinator: coordinator }); const caps = makeCapabilities(); await director.decide(textTurn("text 1"), state, caps); From 1134a43a971712467ee7b1e2433de5e641fe6d7b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 12:32:38 -0700 Subject: [PATCH 2/5] Wire manage_tasks updates into live rendering and drop the duplicate hydrate parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TUI's chrome panel now pulls task state through onTasksChange, the same callback path onActivateTools already used, so a running session's task list updates live instead of only appearing after a resume. Exec mode wires the same callback to debug logging, since it has no equivalent live task surface today. Resumed sessions previously re-derived their task list by independently replaying manage_tasks tool calls in turns-to-blocks.ts. That replay is gone; hydrate and the live decide() loop now share one function for applying a manage_tasks call to a task list, so there is a single definition of what a transcript's task state means. The live loop already applied a call as soon as it saw the tool_call, without waiting for its tool_result — hydrate now matches that instead of gating on a successful result. --- src/agent/director.ts | 36 +++++++++++++++++-- src/director.test.ts | 35 +++++++++++++++++++ src/exec/runner.ts | 8 +++++ src/tui/runner.ts | 8 ++++- src/tui/turns-to-blocks.test.ts | 58 ++++++++++++++++++++++++++++++ src/tui/turns-to-blocks.ts | 62 +++++++++++---------------------- 6 files changed, 161 insertions(+), 46 deletions(-) create mode 100644 src/tui/turns-to-blocks.test.ts diff --git a/src/agent/director.ts b/src/agent/director.ts index b5ffe2bec..29faf3617 100644 --- a/src/agent/director.ts +++ b/src/agent/director.ts @@ -303,6 +303,18 @@ function isCodeFile(path: string): boolean { return CODE_FILE_EXT.test(path); } +// Single implementation of "what does a manage_tasks tool call do to the +// task list", shared by the live decide() loop below and hydrateTasksFromTurns. +// Returns null when the call is not manage_tasks or its arguments don't +// parse, so callers can distinguish "no valid manage_tasks call here" from +// "a valid call that happened to be a no-op" — the latter still counts as an +// update for onTasksChange purposes, matching prior behavior. +function applyManageTasksToolCall(tasks: Task[], block: { name: string; arguments: unknown }): Task[] | null { + if (block.name !== "manage_tasks") return null; + const taskArgs = parseManageTasksArgs(block.arguments); + return taskArgs !== null ? applyManageTasks(tasks, taskArgs) : null; +} + export type ChatDirectorOptions = { taskClassifier?: ((message: string, metadata: SessionMetadata) => Promise) | undefined; onActivateTools?: ((names: string[]) => void) | undefined; @@ -623,9 +635,9 @@ class ChatDirectorImpl extends DefaultDirector { for (const block of event.turn.content) { if (block.type !== "tool_call") continue; if (block.name === "manage_tasks") { - const taskArgs = parseManageTasksArgs(block.arguments); - if (taskArgs !== null) { - this.tasks = applyManageTasks(this.tasks, taskArgs); + const next = applyManageTasksToolCall(this.tasks, block); + if (next !== null) { + this.tasks = next; this.onTasksChange?.(this.tasks); } } else if (block.name === "read_file" || block.name === "edit_file") { @@ -792,6 +804,24 @@ export function createChatDirector( }); } +// Task state on hydrate is derived with the same manage_tasks-handling logic +// live sessions use (applyManageTasksToolCall), applied unconditionally on +// each tool_call regardless of whether its tool_result later errors — a +// resumed transcript's task list matches what a live session would have +// held at that point, rather than a looser hydrate-only interpretation. +export function hydrateTasksFromTurns(turns: ConversationTurn[]): Task[] { + let tasks: Task[] = []; + for (const turn of turns) { + if (turn.role !== "assistant") continue; + for (const block of turn.content) { + if (block.type !== "tool_call") continue; + const next = applyManageTasksToolCall(tasks, block); + if (next !== null) tasks = next; + } + } + return tasks; +} + export interface ChatDirector extends ReactorDirector { updateToolDefinitions(toolDefinitions: ToolDefinition[]): void; setWorkflowCoordinator(coordinator: WorkflowCoordinator | undefined): void; diff --git a/src/director.test.ts b/src/director.test.ts index 0e4438d56..6c24d8460 100644 --- a/src/director.test.ts +++ b/src/director.test.ts @@ -772,3 +772,38 @@ describe("goal continue-rule", () => { }); }); +describe("onTasksChange live wiring", () => { + test("a manage_tasks tool call invokes the wired onTasksChange with the updated task list", async () => { + const updates: Array> = []; + const director = createChatDirector("base", [], { + onTasksChange: (tasks) => updates.push(tasks), + }); + + await director.decide( + makeInferenceDoneEvent([ + { id: "m", name: "manage_tasks", args: { action: "create", tasks: [{ id: "t1", title: "work", status: "doing" }] } }, + ]), + mockState, + mockCapabilities, + ); + + expect(updates).toHaveLength(1); + expect(updates[0]).toEqual([{ id: "t1", title: "work", status: "doing" }]); + }); + + test("onTasksChange is not invoked for tool calls that are not manage_tasks", async () => { + const updates: unknown[] = []; + const director = createChatDirector("base", [], { + onTasksChange: (tasks) => updates.push(tasks), + }); + + await director.decide( + makeInferenceDoneEvent([{ id: "c", name: "read_file", args: { path: "src/foo.ts" } }]), + mockState, + mockCapabilities, + ); + + expect(updates).toHaveLength(0); + }); +}); + diff --git a/src/exec/runner.ts b/src/exec/runner.ts index c61628d95..f47864885 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -408,6 +408,14 @@ export async function runExec(config: Config): Promise { }, inactivityTimeoutMs: config.inactivityTimeoutMs ?? 750_000, totalTimeoutMs: config.totalTimeoutMs, + // Exec mode has no live task panel or task stdout output today (unlike + // the TUI's chrome zone) — debug logging is the closest match to how + // this mode already surfaces other in-session state changes. + onTasksChange: (tasks) => { + logger.debug("tasks updated: {tasks}", { + tasks: tasks.map((t) => `${t.status}:${t.title}`).join(", "), + }); + }, requestContinuation: () => { // Compaction governor self-delivers after compact so the loop re-enters. currentAgent?.deliver(buildCompactionContinuationMessage()); diff --git a/src/tui/runner.ts b/src/tui/runner.ts index 276846ac8..67ff28d0b 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -119,7 +119,7 @@ import { createSubAgentSessionStore, type SubAgentProvider } from "../subagent/i import type { InferenceSource, ToolDefinition, InboundMessage } from "@intx/types/runtime"; import { createSessionOperationQueue } from "./session-operation-queue.js"; import { setAgentSourceUnlessClosed } from "./agent-source-sync.js"; -import { createChatDirector } from "../agent/director.js"; +import { createChatDirector, hydrateTasksFromTurns } from "../agent/director.js"; import { createGoalGovernor } from "../agent/goal.js"; import { createGoalEvaluator } from "../agent/goal-evaluator.js"; import { loadGoalState, saveGoalState } from "../session/goal-state.js"; @@ -1174,6 +1174,7 @@ export async function runTUI(initialConfig: Config): Promise { onActivateTools: (names) => promoteTools(names), inactivityTimeoutMs: config.inactivityTimeoutMs ?? 750_000, totalTimeoutMs: config.totalTimeoutMs, + onTasksChange: (tasks) => emitter.emit("tasks", tasks), requestContinuation: () => { enqueueAgentDeliver(() => currentAgent.deliver(buildCompactionContinuationMessage())); }, @@ -2077,6 +2078,7 @@ export async function runTUI(initialConfig: Config): Promise { }, chrome: () => ({ goal: goalGovernor.get(), + tasks: directorHolder.instance?.getTasks() ?? null, agents: subAgentSessions.listForStrip().map((s) => ({ agentId: s.agentId, id: s.id, @@ -2090,9 +2092,11 @@ export async function runTUI(initialConfig: Config): Promise { subscribeChrome: (notify) => { const unsubscribeAgents = subAgentSessions.subscribe(notify); emitter.on("goal", notify); + emitter.on("tasks", notify); return () => { unsubscribeAgents(); emitter.off("goal", notify); + emitter.off("tasks", notify); }; }, subAgentSessions: () => subAgentSessions.list(), @@ -2267,6 +2271,8 @@ export async function runTUI(initialConfig: Config): Promise { void loadRecentTurns(workdir, RESUME_TRANSCRIPT_BLOCK_LIMIT) .then((turns) => { const blocks = turnsToContentBlocks(turns, { maxBlocks: RESUME_TRANSCRIPT_BLOCK_LIMIT }); + const tasks = hydrateTasksFromTurns(turns); + if (tasks.length > 0) blocks.unshift({ type: "tasks", tasks }); if (blocks.length > 0) emitter.emit("history.hydrate", blocks); }) .catch((err: unknown) => { diff --git a/src/tui/turns-to-blocks.test.ts b/src/tui/turns-to-blocks.test.ts new file mode 100644 index 000000000..adb24e15c --- /dev/null +++ b/src/tui/turns-to-blocks.test.ts @@ -0,0 +1,58 @@ +import { describe, test, expect } from "bun:test"; +import type { ConversationTurn } from "@intx/types/runtime"; +import { turnsToContentBlocks } from "./turns-to-blocks.js"; +import { hydrateTasksFromTurns } from "../agent/director.js"; + +function manageTasksTurn(id: string, status: "todo" | "doing" | "done"): ConversationTurn { + return { + role: "assistant", + model: "test", + timestamp: 0, + content: [ + { + type: "tool_call", + id, + name: "manage_tasks", + arguments: { action: "create", tasks: [{ id: "t1", title: "work", status }] }, + }, + ], + } as unknown as ConversationTurn; +} + +function toolResultTurn(callId: string, isError: boolean): ConversationTurn { + return { + role: "assistant", + model: "test", + timestamp: 0, + content: [{ type: "tool_result", callId, content: "ok", isError }], + } as unknown as ConversationTurn; +} + +describe("turnsToContentBlocks no longer derives tasks", () => { + test("a transcript with manage_tasks calls produces no tasks block on its own", () => { + const turns = [manageTasksTurn("m1", "doing"), toolResultTurn("m1", false)]; + const blocks = turnsToContentBlocks(turns); + expect(blocks.some((b) => b.type === "tasks")).toBe(false); + }); +}); + +describe("hydrateTasksFromTurns", () => { + test("derives the task list from manage_tasks tool calls in a transcript", () => { + const turns = [manageTasksTurn("m1", "doing"), toolResultTurn("m1", false)]; + const tasks = hydrateTasksFromTurns(turns); + expect(tasks).toEqual([{ id: "t1", title: "work", status: "doing" }]); + }); + + test("applies a manage_tasks call even when its tool_result later errors, matching live decide() behavior", () => { + const turns = [manageTasksTurn("m1", "doing"), toolResultTurn("m1", true)]; + const tasks = hydrateTasksFromTurns(turns); + expect(tasks).toEqual([{ id: "t1", title: "work", status: "doing" }]); + }); + + test("returns an empty list for a transcript with no manage_tasks calls", () => { + const turns: ConversationTurn[] = [ + { role: "user", content: [{ type: "text", text: "hi" }], timestamp: 0 } as unknown as ConversationTurn, + ]; + expect(hydrateTasksFromTurns(turns)).toEqual([]); + }); +}); diff --git a/src/tui/turns-to-blocks.ts b/src/tui/turns-to-blocks.ts index ccd35f400..d0b9e11c9 100644 --- a/src/tui/turns-to-blocks.ts +++ b/src/tui/turns-to-blocks.ts @@ -1,6 +1,6 @@ import type { ContentBlock as RuntimeContentBlock, ConversationTurn } from "@intx/types/runtime"; -import { applyManageTasks, parseManageTasksArgs, type Task } from "../agent/tasks.js"; +import type { Task } from "../agent/tasks.js"; import { validateView, type ViewNode } from "./view/index.js"; type PlanBlockStep = { file: string; action: string; reason?: string }; @@ -78,7 +78,7 @@ function stringifyToolContent(content: unknown): string { function upsertResumeBlock( blocks: ContentBlockData[], - block: { type: "plan"; steps: PlanBlockStep[] } | { type: "tasks"; tasks: Task[] }, + block: { type: "plan"; steps: PlanBlockStep[] }, ): ContentBlockData[] { const next = [...blocks]; const existing = next.findIndex((entry) => entry.type === block.type); @@ -90,7 +90,7 @@ function upsertResumeBlock( return next; } -/** Mirror live-stream tool.done handling for plan/tasks when hydrating a session. */ +/** Mirror live-stream tool.done handling for plan (submit_plan) when hydrating a session. */ function finalizeResumeToolBlocks(blocks: ContentBlockData[]): ContentBlockData[] { const callIdToCallIndex = new Map(); for (let i = 0; i < blocks.length; i += 1) { @@ -100,7 +100,6 @@ function finalizeResumeToolBlocks(blocks: ContentBlockData[]): ContentBlockData[ } } - let tasks: Task[] = []; let planSteps: PlanBlockStep[] | null = null; const indicesToRemove = new Set(); @@ -110,53 +109,32 @@ function finalizeResumeToolBlocks(blocks: ContentBlockData[]): ContentBlockData[ const callIndex = callIdToCallIndex.get(result.callId); if (callIndex === undefined) continue; const call = blocks[callIndex]; - if (call?.type !== "tool_call") continue; + if (call?.type !== "tool_call" || call.name !== "submit_plan") continue; - if (call.name === "submit_plan") { - indicesToRemove.add(callIndex); - indicesToRemove.add(i); - let steps: PlanBlockStep[] = []; - try { - const parsed = JSON.parse(call.arguments) as { - steps?: Array<{ file: string; action: string; reason?: string }>; - }; - if (Array.isArray(parsed.steps)) { - steps = parsed.steps.map((s) => ({ - file: s.file, - action: s.action, - ...(s.reason !== undefined ? { reason: s.reason } : {}), - })); - } - } catch { - /* invalid args → empty plan */ - } - planSteps = steps; - continue; - } - - if (call.name === "manage_tasks") { - indicesToRemove.add(callIndex); - indicesToRemove.add(i); - let raw: unknown; - try { - raw = JSON.parse(call.arguments); - } catch { - continue; - } - const parsed = parseManageTasksArgs(raw); - if (parsed !== null) { - tasks = applyManageTasks(tasks, parsed); + indicesToRemove.add(callIndex); + indicesToRemove.add(i); + let steps: PlanBlockStep[] = []; + try { + const parsed = JSON.parse(call.arguments) as { + steps?: Array<{ file: string; action: string; reason?: string }>; + }; + if (Array.isArray(parsed.steps)) { + steps = parsed.steps.map((s) => ({ + file: s.file, + action: s.action, + ...(s.reason !== undefined ? { reason: s.reason } : {}), + })); } + } catch { + /* invalid args → empty plan */ } + planSteps = steps; } let out = blocks.filter((_, index) => !indicesToRemove.has(index)); if (planSteps !== null) { out = upsertResumeBlock(out, { type: "plan", steps: planSteps }); } - if (tasks.length > 0) { - out = upsertResumeBlock(out, { type: "tasks", tasks }); - } return out; } From a796ab596939bc3f408318ed0985f6c4d8f30929 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 12:52:12 -0700 Subject: [PATCH 3/5] Keep resumed sessions consistent about their task list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two surfaces disagreed with each other after a resume. The transcript dropped its manage_tasks rows in favor of one aggregated task block, but only when the rows were stripped alongside submit_plan's — without that, every call reappeared as its own row beneath the summary. And the chrome panel reads the director's in-memory list, which a freshly constructed director leaves empty until the model happens to call manage_tasks again, so a resumed session showed tasks in its scrollback and none in its panel. Restoring the derived list into the director closes both: one source of task state, painted the same way everywhere. --- src/agent/director.ts | 10 ++++++++++ src/director.test.ts | 13 +++++++++++++ src/tui/runner.ts | 5 ++++- src/tui/turns-to-blocks.test.ts | 14 ++++++++++++++ src/tui/turns-to-blocks.ts | 16 ++++++++++++++-- 5 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/agent/director.ts b/src/agent/director.ts index 29faf3617..7a0aef6e6 100644 --- a/src/agent/director.ts +++ b/src/agent/director.ts @@ -404,6 +404,15 @@ class ChatDirectorImpl extends DefaultDirector { return [...this.tasks]; } + // A resumed session's task list lives in the transcript, not in the freshly + // constructed director. Without this the chrome panel would read an empty + // list until the model happened to call manage_tasks again, disagreeing + // with the task block already painted in the transcript. + restoreTasks(tasks: Task[]): void { + this.tasks = [...tasks]; + this.onTasksChange?.(this.tasks); + } + // The status bar's context meter falls back to this when a provider omits // or zeroes usage on the latest turn — a local lower-then-corrected bound // beats displaying a number the provider never actually reported. @@ -828,5 +837,6 @@ export interface ChatDirector extends ReactorDirector { setGoalGovernor(goal: GoalGovernor | undefined): void; getGoalGovernor(): GoalGovernor | undefined; getTasks(): Task[]; + restoreTasks(tasks: Task[]): void; getContextEstimate(): { tokens: number; isEstimate: boolean }; } diff --git a/src/director.test.ts b/src/director.test.ts index 6c24d8460..5a47667d4 100644 --- a/src/director.test.ts +++ b/src/director.test.ts @@ -791,6 +791,19 @@ describe("onTasksChange live wiring", () => { expect(updates[0]).toEqual([{ id: "t1", title: "work", status: "doing" }]); }); + test("restoreTasks seeds a resumed session's task list and notifies the consumer", () => { + const updates: Array> = []; + const director = createChatDirector("base", [], { + onTasksChange: (tasks) => updates.push(tasks), + }); + + const restored = [{ id: "t1", title: "from transcript", status: "doing" as const }]; + director.restoreTasks(restored); + + expect(director.getTasks()).toEqual(restored); + expect(updates).toEqual([restored]); + }); + test("onTasksChange is not invoked for tool calls that are not manage_tasks", async () => { const updates: unknown[] = []; const director = createChatDirector("base", [], { diff --git a/src/tui/runner.ts b/src/tui/runner.ts index 67ff28d0b..cbedfa1be 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -2272,7 +2272,10 @@ export async function runTUI(initialConfig: Config): Promise { .then((turns) => { const blocks = turnsToContentBlocks(turns, { maxBlocks: RESUME_TRANSCRIPT_BLOCK_LIMIT }); const tasks = hydrateTasksFromTurns(turns); - if (tasks.length > 0) blocks.unshift({ type: "tasks", tasks }); + if (tasks.length > 0) { + blocks.unshift({ type: "tasks", tasks }); + directorHolder.instance?.restoreTasks(tasks); + } if (blocks.length > 0) emitter.emit("history.hydrate", blocks); }) .catch((err: unknown) => { diff --git a/src/tui/turns-to-blocks.test.ts b/src/tui/turns-to-blocks.test.ts index adb24e15c..1b4c79be6 100644 --- a/src/tui/turns-to-blocks.test.ts +++ b/src/tui/turns-to-blocks.test.ts @@ -34,6 +34,20 @@ describe("turnsToContentBlocks no longer derives tasks", () => { const blocks = turnsToContentBlocks(turns); expect(blocks.some((b) => b.type === "tasks")).toBe(false); }); + + // The aggregated task block is unshifted separately on resume, so leaving + // the raw rows in would show every manage_tasks call twice over. + test("manage_tasks call and result rows are stripped from the resumed transcript", () => { + const turns = [ + manageTasksTurn("m1", "todo"), + toolResultTurn("m1", false), + manageTasksTurn("m2", "doing"), + toolResultTurn("m2", false), + ]; + const blocks = turnsToContentBlocks(turns); + expect(blocks.some((b) => b.type === "tool_call" && b.name === "manage_tasks")).toBe(false); + expect(blocks.some((b) => b.type === "tool_result" && b.name === "manage_tasks")).toBe(false); + }); }); describe("hydrateTasksFromTurns", () => { diff --git a/src/tui/turns-to-blocks.ts b/src/tui/turns-to-blocks.ts index d0b9e11c9..1f277b9f3 100644 --- a/src/tui/turns-to-blocks.ts +++ b/src/tui/turns-to-blocks.ts @@ -90,7 +90,12 @@ function upsertResumeBlock( return next; } -/** Mirror live-stream tool.done handling for plan (submit_plan) when hydrating a session. */ +/** + * Mirror live-stream tool.done handling when hydrating a session: submit_plan + * collapses into a single plan block, and manage_tasks rows are dropped + * entirely because the resumed task list is rendered as one aggregated block + * (see hydrateTasksFromTurns) rather than as one row per call. + */ function finalizeResumeToolBlocks(blocks: ContentBlockData[]): ContentBlockData[] { const callIdToCallIndex = new Map(); for (let i = 0; i < blocks.length; i += 1) { @@ -109,7 +114,14 @@ function finalizeResumeToolBlocks(blocks: ContentBlockData[]): ContentBlockData[ const callIndex = callIdToCallIndex.get(result.callId); if (callIndex === undefined) continue; const call = blocks[callIndex]; - if (call?.type !== "tool_call" || call.name !== "submit_plan") continue; + if (call?.type !== "tool_call") continue; + + if (call.name === "manage_tasks") { + indicesToRemove.add(callIndex); + indicesToRemove.add(i); + continue; + } + if (call.name !== "submit_plan") continue; indicesToRemove.add(callIndex); indicesToRemove.add(i); From a42f8325887fa6269305b2fce35161681e47d50a Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 13:07:35 -0700 Subject: [PATCH 4/5] Fix the resume strip/apply mismatch and make onTasksChange required MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resumed transcripts stripped a manage_tasks call's raw rows only when it had a successful tool_result, but hydrateTasksFromTurns already applies the call unconditionally on the tool_call itself — a call with an errored or missing result kept its raw rows next to the aggregated task block instead of being replaced by it. The strip now matches the apply: manage_tasks rows come out regardless of the result's outcome, because the tool_call is what the underlying tool's side-effect-free handler makes authoritative, not whatever result eventually shows up. onTasksChange moves from optional to required on ChatDirectorOptions, same motivation as CL-5709: an omitted required field is a visible gap in a caller's diff, not an invisible one. --- src/agent/director.test.ts | 14 ++++---- src/agent/director.ts | 23 ++++++++----- src/director.test.ts | 48 ++++++++++++++------------- src/prompts.test.ts | 2 +- src/tui/turns-to-blocks.test.ts | 44 ++++++++++++++++++++++++ src/tui/turns-to-blocks.ts | 24 +++++++++----- tests/integration/harness.ts | 2 +- tests/unit/director.test.ts | 5 +-- tests/unit/workflows-director.test.ts | 12 +++---- 9 files changed, 117 insertions(+), 57 deletions(-) diff --git a/src/agent/director.test.ts b/src/agent/director.test.ts index a60797c8d..44fa17cd4 100644 --- a/src/agent/director.test.ts +++ b/src/agent/director.test.ts @@ -100,7 +100,7 @@ describe("ChatDirector tool-only loop protection", () => { const providerlessPolicy = { providerName: "test-provider" }; test("nudges once at the family threshold, after pending tools execute", async () => { - const director = createChatDirector("system", [], { provider: providerlessPolicy }); + const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: providerlessPolicy }); const capabilities = makeCapabilities(); // Default family nudges at 12 consecutive tool-only turns. @@ -111,7 +111,7 @@ describe("ChatDirector tool-only loop protection", () => { }); test("the nudge is one-shot — it does not repeat on the next tool-only turn", async () => { - const director = createChatDirector("system", [], { provider: providerlessPolicy }); + const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: providerlessPolicy }); const capabilities = makeCapabilities(); await runToolOnlyStreak(director, capabilities, 12); @@ -122,7 +122,7 @@ describe("ChatDirector tool-only loop protection", () => { }); test("pauses and stops issuing infers at the family pause threshold", async () => { - const director = createChatDirector("system", [], { provider: providerlessPolicy }); + const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: providerlessPolicy }); const capabilities = makeCapabilities(); // Default family pauses at 20 consecutive tool-only turns. @@ -136,7 +136,7 @@ describe("ChatDirector tool-only loop protection", () => { }); test("resumes after the operator sends a new message", async () => { - const director = createChatDirector("system", [], { provider: providerlessPolicy }); + const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: providerlessPolicy }); const capabilities = makeCapabilities(); await runToolOnlyStreak(director, capabilities, 20); @@ -147,7 +147,7 @@ describe("ChatDirector tool-only loop protection", () => { }); test("a dismissed ask_operator counts toward the streak like any other tool-only turn", async () => { - const director = createChatDirector("system", [], { provider: providerlessPolicy }); + const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: providerlessPolicy }); const capabilities = makeCapabilities(); // 11 ordinary tool-only turns, then a turn whose only tool call is a @@ -197,7 +197,7 @@ describe("ChatDirector tool-only loop protection", () => { }); test("a busy-but-progressing session (text interleaved with tools) never trips", async () => { - const director = createChatDirector("system", [], { provider: providerlessPolicy }); + const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: providerlessPolicy }); const capabilities = makeCapabilities(); let lastActions: ReactorAction[] = []; @@ -212,7 +212,7 @@ describe("ChatDirector tool-only loop protection", () => { }); test("grok's tightened thresholds fire earlier than the default family", async () => { - const director = createChatDirector("system", [], { provider: { providerName: "xai/default", model: "grok-4.5" } }); + const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: { providerName: "xai/default", model: "grok-4.5" } }); const capabilities = makeCapabilities(); // Grok nudges at 6, well below the default family's 12. diff --git a/src/agent/director.ts b/src/agent/director.ts index 7a0aef6e6..c7d071a20 100644 --- a/src/agent/director.ts +++ b/src/agent/director.ts @@ -305,10 +305,15 @@ function isCodeFile(path: string): boolean { // Single implementation of "what does a manage_tasks tool call do to the // task list", shared by the live decide() loop below and hydrateTasksFromTurns. +// Task state is owned by the director, not by the tool: manage_tasks's +// handler (src/agent/tools.ts) performs no side effect of its own — it +// parses the same arguments and returns a fixed "Tasks updated." string. The +// tool_call is therefore the authoritative event, and applying it here does +// not need to wait on a tool_result the handler never varies. // Returns null when the call is not manage_tasks or its arguments don't // parse, so callers can distinguish "no valid manage_tasks call here" from // "a valid call that happened to be a no-op" — the latter still counts as an -// update for onTasksChange purposes, matching prior behavior. +// update for onTasksChange purposes. function applyManageTasksToolCall(tasks: Task[], block: { name: string; arguments: unknown }): Task[] | null { if (block.name !== "manage_tasks") return null; const taskArgs = parseManageTasksArgs(block.arguments); @@ -321,7 +326,7 @@ export type ChatDirectorOptions = { inactivityTimeoutMs?: number | undefined; totalTimeoutMs?: number | undefined; workflowCoordinator?: WorkflowCoordinator | undefined; - onTasksChange?: ((tasks: Task[]) => void) | undefined; + onTasksChange: (tasks: Task[]) => void; requestContinuation?: (() => void) | undefined; provider?: { providerName: string; model?: string } | undefined; }; @@ -370,7 +375,7 @@ class ChatDirectorImpl extends DefaultDirector { private pendingToolOnlyNudge = false; private pausedForToolOnly = false; - constructor(systemPrompt: string, toolDefinitions: ToolDefinition[], options: ChatDirectorImplOptions = {}) { + constructor(systemPrompt: string, toolDefinitions: ToolDefinition[], options: ChatDirectorImplOptions) { super(systemPrompt, toolDefinitions, {}); this._systemPrompt = systemPrompt; this._toolDefinitions = toolDefinitions; @@ -802,7 +807,7 @@ class ChatDirectorImpl extends DefaultDirector { export function createChatDirector( systemPrompt: string, toolDefinitions: ToolDefinition[], - options: ChatDirectorOptions = {}, + options: ChatDirectorOptions, ): ChatDirector { const { provider, ...rest } = options; return new ChatDirectorImpl(systemPrompt, toolDefinitions, { @@ -813,11 +818,11 @@ export function createChatDirector( }); } -// Task state on hydrate is derived with the same manage_tasks-handling logic -// live sessions use (applyManageTasksToolCall), applied unconditionally on -// each tool_call regardless of whether its tool_result later errors — a -// resumed transcript's task list matches what a live session would have -// held at that point, rather than a looser hydrate-only interpretation. +// Uses the same applyManageTasksToolCall a live session's decide() loop uses, +// so hydrate necessarily reaches the same task state live decide() would +// have produced from this transcript: the tool_call is the authoritative +// event (see applyManageTasksToolCall), and there is only the one function +// that knows how to turn a manage_tasks call into a task list. export function hydrateTasksFromTurns(turns: ConversationTurn[]): Task[] { let tasks: Task[] = []; for (const turn of turns) { diff --git a/src/director.test.ts b/src/director.test.ts index 5a47667d4..9cdfe3534 100644 --- a/src/director.test.ts +++ b/src/director.test.ts @@ -79,7 +79,7 @@ describe("operator declined tool calls", () => { // reactor and break further sends, and it does not re-infer off a bare // decline. test("chat director surfaces the decline and waits, keeping the reactor alive", async () => { - const director = createChatDirector("", []); + const director = createChatDirector("", [], { onTasksChange: () => {} }); const actions = actionsArray(await director.decide(makeToolErrorEvent("c", declined), mockState, mockCapabilities)); expect(hasCheckpoint(actions)).toBe(true); expect(hasDeclineReply(actions)).toBe(true); @@ -109,7 +109,7 @@ describe("open-task termination guard", () => { const hasReply = (a: ReactorAction[]): boolean => a.some((x) => x.type === "reply"); test("re-infers instead of ending the turn while a task is still open", async () => { - const director = createChatDirector("base", []); + const director = createChatDirector("base", [], { onTasksChange: () => {} }); await director.decide(manageTasksEvent("doing"), mockState, mockCapabilities); const actions = actionsArray(await director.decide(textTurn(), mockState, mockCapabilities)); @@ -118,7 +118,7 @@ describe("open-task termination guard", () => { }); test("ends the turn normally once every task is terminal", async () => { - const director = createChatDirector("base", []); + const director = createChatDirector("base", [], { onTasksChange: () => {} }); await director.decide(manageTasksEvent("done"), mockState, mockCapabilities); const actions = actionsArray(await director.decide(textTurn(), mockState, mockCapabilities)); @@ -127,7 +127,7 @@ describe("open-task termination guard", () => { }); test("stops nudging and lets the turn end after the cap of content-free attempts", async () => { - const director = createChatDirector("base", []); + const director = createChatDirector("base", [], { onTasksChange: () => {} }); await director.decide(manageTasksEvent("doing"), mockState, mockCapabilities); for (let i = 0; i < 3; i++) { @@ -142,7 +142,7 @@ describe("open-task termination guard", () => { test("empty model turn settles with an empty reply before wait", async () => { // DefaultDirector ends empty responses with bare wait; without a reply, // agent.send hangs and the TUI Working spinner sticks forever. - const director = createChatDirector("base", []); + const director = createChatDirector("base", [], { onTasksChange: () => {} }); const emptyTurn = { type: "inference.done", turn: { role: "assistant", model: "test", timestamp: 0, content: [] }, @@ -158,7 +158,7 @@ describe("open-task termination guard", () => { }); test("a declined tool with open tasks re-infers, then terminates after its cap", async () => { - const director = createChatDirector("base", []); + const director = createChatDirector("base", [], { onTasksChange: () => {} }); await director.decide(manageTasksEvent("doing"), mockState, mockCapabilities); for (let i = 0; i < 2; i++) { @@ -177,7 +177,7 @@ describe("open-task termination guard", () => { // single user turn — the budget is monotonic per inbound message, not per // tool call, so it does not matter whether a tool call happens at all. test("a no-op tool call between nudges does not reset the idle budget", async () => { - const director = createChatDirector("base", []); + const director = createChatDirector("base", [], { onTasksChange: () => {} }); await director.decide(manageTasksEvent("doing"), mockState, mockCapabilities); // Two content-free terminations spend two of the three nudges. @@ -201,7 +201,7 @@ describe("open-task termination guard", () => { }); test("a new user message resets the idle budget for the next turn", async () => { - const director = createChatDirector("base", []); + const director = createChatDirector("base", [], { onTasksChange: () => {} }); await director.decide(manageTasksEvent("doing"), mockState, mockCapabilities); for (let i = 0; i < 3; i++) { @@ -221,7 +221,7 @@ describe("open-task termination guard", () => { }); test("a successful tool call between declines does not reset the declined budget", async () => { - const director = createChatDirector("base", []); + const director = createChatDirector("base", [], { onTasksChange: () => {} }); await director.decide(manageTasksEvent("doing"), mockState, mockCapabilities); // Spend both of the declined-path nudges, with a successful tool result @@ -242,7 +242,7 @@ describe("open-task termination guard", () => { }); test("a declined tool with no open tasks surfaces the decline immediately", async () => { - const director = createChatDirector("base", []); + const director = createChatDirector("base", [], { onTasksChange: () => {} }); const actions = actionsArray(await director.decide(makeToolErrorEvent("c", declined), mockState, mockCapabilities)); expect(actions.some((a) => a.type === "reply" && "content" in a && a.content === "Tool call rejected by operator.")).toBe(true); expect(actions.some((a) => a.type === "infer")).toBe(false); @@ -274,6 +274,7 @@ describe("chatDirector compaction", () => { test("schedules idle compaction after an over-threshold text-only reply", async () => { let continuations = 0; const director = createChatDirector("", [], { + onTasksChange: () => {}, requestContinuation: () => { continuations++; }, @@ -328,7 +329,7 @@ describe("chatDirector compaction", () => { } function chatDirectorWithContinuation(onContinuation?: () => void) { - return createChatDirector("", [], { requestContinuation: onContinuation ?? (() => {}) }); + return createChatDirector("", [], { onTasksChange: () => {}, requestContinuation: onContinuation ?? (() => {}) }); } test("compacts at the tool.done pause once over threshold", async () => { @@ -439,7 +440,7 @@ describe("chatDirector compaction", () => { describe("chatDirector LSP auto-activation", () => { test("reading a code file activates the lsp tool on success", async () => { const activated: string[][] = []; - const director = createChatDirector("", [], { onActivateTools: (names: string[]) => activated.push(names) }); + const director = createChatDirector("", [], { onTasksChange: () => {}, onActivateTools: (names: string[]) => activated.push(names) }); await director.decide(makeInferenceDoneEvent([{ id: "c", name: "read_file", args: { path: "src/foo.ts" } }]), mockState, mockCapabilities); await director.decide(makeToolDoneEvent("c"), mockState, mockCapabilities); expect(activated).toEqual([["lsp"]]); @@ -447,7 +448,7 @@ describe("chatDirector LSP auto-activation", () => { test("editing a code file activates lsp", async () => { const activated: string[][] = []; - const director = createChatDirector("", [], { onActivateTools: (names: string[]) => activated.push(names) }); + const director = createChatDirector("", [], { onTasksChange: () => {}, onActivateTools: (names: string[]) => activated.push(names) }); await director.decide(makeInferenceDoneEvent([{ id: "c", name: "edit_file", args: { path: "lib/bar.rs" } }]), mockState, mockCapabilities); await director.decide(makeToolDoneEvent("c"), mockState, mockCapabilities); expect(activated).toEqual([["lsp"]]); @@ -455,7 +456,7 @@ describe("chatDirector LSP auto-activation", () => { test("a non-code file does not activate lsp", async () => { const activated: string[][] = []; - const director = createChatDirector("", [], { onActivateTools: (names: string[]) => activated.push(names) }); + const director = createChatDirector("", [], { onTasksChange: () => {}, onActivateTools: (names: string[]) => activated.push(names) }); await director.decide(makeInferenceDoneEvent([{ id: "c", name: "read_file", args: { path: "README.md" } }]), mockState, mockCapabilities); await director.decide(makeToolDoneEvent("c"), mockState, mockCapabilities); expect(activated).toEqual([]); @@ -463,7 +464,7 @@ describe("chatDirector LSP auto-activation", () => { test("a failed read does not activate lsp", async () => { const activated: string[][] = []; - const director = createChatDirector("", [], { onActivateTools: (names: string[]) => activated.push(names) }); + const director = createChatDirector("", [], { onTasksChange: () => {}, onActivateTools: (names: string[]) => activated.push(names) }); await director.decide(makeInferenceDoneEvent([{ id: "c", name: "read_file", args: { path: "src/foo.ts" } }]), mockState, mockCapabilities); await director.decide(makeToolErrorEvent("c", "Error: not found"), mockState, mockCapabilities); expect(activated).toEqual([]); @@ -495,7 +496,7 @@ describe("updateToolDefinitions rewrites infer tools", () => { }; test("a tool registered after construction is advertised on the next inference", async () => { - const director = createChatDirector("base-prompt", []); + const director = createChatDirector("base-prompt", [], { onTasksChange: () => {} }); director.updateToolDefinitions([lateTool]); const result = await director.decide(makeMessageReceivedEvent("hello"), mockState, capabilitiesWithInferArgs); @@ -508,7 +509,7 @@ describe("updateToolDefinitions rewrites infer tools", () => { // The provider cache is a prefix cache keyed on the tools array; a tool_search // between turns must not reshape it. test("wire tools are byte-identical across a turn that ran tool_search", async () => { - const director = createChatDirector("base-prompt", [lateTool]); + const director = createChatDirector("base-prompt", [lateTool], { onTasksChange: () => {} }); const before = await firstInferTools(director, makeMessageReceivedEvent("do work")); @@ -529,7 +530,7 @@ describe("updateToolDefinitions rewrites infer tools", () => { // advance_workflow is always on the wire so a workflow going active never grows // the array and busts the provider cache prefix. test("advance_workflow is advertised even with no active workflow", async () => { - const director = createChatDirector("base-prompt", []); + const director = createChatDirector("base-prompt", [], { onTasksChange: () => {} }); director.updateToolDefinitions([lateTool]); const result = await director.decide(makeMessageReceivedEvent("hello"), mockState, capabilitiesWithInferArgs); @@ -564,6 +565,7 @@ describe("updateToolDefinitions rewrites infer tools", () => { const director = createChatDirector( "base-prompt", computeAdvertised(toolset.dynamicRunner.currentDefinitions()), + { onTasksChange: () => {} }, ); // Before discovery: the MCP tool is registered (dispatchable) but not wired. @@ -600,7 +602,7 @@ describe("updateToolDefinitions rewrites infer tools", () => { test("the new-task path also carries the current tools", async () => { const classifier = async (_msg: string, _meta: SessionMetadata) => ({ kind: "new_task" as const, reason: "pivot" } as TaskBoundary); - const director = createChatDirector("base-prompt", [], { taskClassifier: classifier }); + const director = createChatDirector("base-prompt", [], { onTasksChange: () => {}, taskClassifier: classifier }); director.updateToolDefinitions([lateTool]); const result = await director.decide(makeMessageReceivedEvent("new thing"), mockState, capabilitiesWithInferArgs); @@ -656,7 +658,7 @@ describe("transient nudges", () => { }) as unknown as ReactorInboundEvent; test("open-task nudge uses ephemeralTurns, not systemPrompt", async () => { - const director = createChatDirector("stable-base", []); + const director = createChatDirector("stable-base", [], { onTasksChange: () => {} }); await director.decide(manageTasksEvent("doing"), mockState, mockCapabilities); const actions = actionsArray(await director.decide(textTurn(), mockState, mockCapabilities)); const infer = actions.find((a) => a.type === "infer"); @@ -704,7 +706,7 @@ describe("goal continue-rule", () => { test("active not-met goal rewrites a clean yield into re-infer", async () => { const { createGoalGovernor } = await import("./agent/goal.js"); - const director = createChatDirector("base", []); + const director = createChatDirector("base", [], { onTasksChange: () => {} }); const g = createGoalGovernor({ evaluate: async () => ({ met: false, reason: "tests still red" }), }); @@ -724,7 +726,7 @@ describe("goal continue-rule", () => { test("met goal leaves terminal reply and marks achieved", async () => { const { createGoalGovernor } = await import("./agent/goal.js"); - const director = createChatDirector("base", []); + const director = createChatDirector("base", [], { onTasksChange: () => {} }); const g = createGoalGovernor({ evaluate: async () => ({ met: true, reason: "green" }), }); @@ -744,7 +746,7 @@ describe("goal continue-rule", () => { test("open-task nudge still wins over goal when tasks are open", async () => { const { createGoalGovernor } = await import("./agent/goal.js"); let evals = 0; - const director = createChatDirector("base", []); + const director = createChatDirector("base", [], { onTasksChange: () => {} }); const g = createGoalGovernor({ evaluate: async () => { evals++; diff --git a/src/prompts.test.ts b/src/prompts.test.ts index de98d629e..20797b89c 100644 --- a/src/prompts.test.ts +++ b/src/prompts.test.ts @@ -21,7 +21,7 @@ const minimalToolDefinitions = [manageTasksDefinition, submitOutputDefinition]; test("buildChatSystemPrompt wires into createChatDirector without error", () => { const prompt = buildChatSystemPrompt(); - expect(() => createChatDirector(prompt, minimalToolDefinitions)).not.toThrow(); + expect(() => createChatDirector(prompt, minimalToolDefinitions, { onTasksChange: () => {} })).not.toThrow(); }); test("chat prompt orders base, then tools, then context", () => { diff --git a/src/tui/turns-to-blocks.test.ts b/src/tui/turns-to-blocks.test.ts index 1b4c79be6..28a6a0409 100644 --- a/src/tui/turns-to-blocks.test.ts +++ b/src/tui/turns-to-blocks.test.ts @@ -48,6 +48,23 @@ describe("turnsToContentBlocks no longer derives tasks", () => { expect(blocks.some((b) => b.type === "tool_call" && b.name === "manage_tasks")).toBe(false); expect(blocks.some((b) => b.type === "tool_result" && b.name === "manage_tasks")).toBe(false); }); + + // hydrateTasksFromTurns applies manage_tasks on the tool_call regardless of + // the result's outcome, so the strip must match: an errored or missing + // result must not leave the raw call/result rows behind next to the + // aggregated block runner.ts unshifts from hydrateTasksFromTurns. + test("strips a manage_tasks call whose result errored", () => { + const turns = [manageTasksTurn("m1", "doing"), toolResultTurn("m1", true)]; + const blocks = turnsToContentBlocks(turns); + expect(blocks.some((b) => b.type === "tool_call" && b.name === "manage_tasks")).toBe(false); + expect(blocks.some((b) => b.type === "tool_result" && b.name === "manage_tasks")).toBe(false); + }); + + test("strips a manage_tasks call with no result at all (interrupted turn)", () => { + const turns = [manageTasksTurn("m1", "doing")]; + const blocks = turnsToContentBlocks(turns); + expect(blocks.some((b) => b.type === "tool_call" && b.name === "manage_tasks")).toBe(false); + }); }); describe("hydrateTasksFromTurns", () => { @@ -70,3 +87,30 @@ describe("hydrateTasksFromTurns", () => { expect(hydrateTasksFromTurns(turns)).toEqual([]); }); }); + +describe("resume rendering, end to end (mirrors runner.ts's hydrate composition)", () => { + test("a manage_tasks call whose result errored shows the task exactly once", () => { + const turns = [manageTasksTurn("m1", "doing"), toolResultTurn("m1", true)]; + + const blocks = turnsToContentBlocks(turns); + const tasks = hydrateTasksFromTurns(turns); + if (tasks.length > 0) blocks.unshift({ type: "tasks", tasks }); + + const taskBlocks = blocks.filter((b) => b.type === "tasks"); + expect(taskBlocks).toHaveLength(1); + expect(taskBlocks[0]).toEqual({ type: "tasks", tasks: [{ id: "t1", title: "work", status: "doing" }] }); + expect(blocks.some((b) => b.type === "tool_call" && b.name === "manage_tasks")).toBe(false); + }); + + test("a manage_tasks call with no result at all shows the task exactly once", () => { + const turns = [manageTasksTurn("m1", "doing")]; + + const blocks = turnsToContentBlocks(turns); + const tasks = hydrateTasksFromTurns(turns); + if (tasks.length > 0) blocks.unshift({ type: "tasks", tasks }); + + const taskBlocks = blocks.filter((b) => b.type === "tasks"); + expect(taskBlocks).toHaveLength(1); + expect(blocks.some((b) => b.type === "tool_call" && b.name === "manage_tasks")).toBe(false); + }); +}); diff --git a/src/tui/turns-to-blocks.ts b/src/tui/turns-to-blocks.ts index 1f277b9f3..1f4b16a9c 100644 --- a/src/tui/turns-to-blocks.ts +++ b/src/tui/turns-to-blocks.ts @@ -98,30 +98,38 @@ function upsertResumeBlock( */ function finalizeResumeToolBlocks(blocks: ContentBlockData[]): ContentBlockData[] { const callIdToCallIndex = new Map(); + const callIdToResultIndex = new Map(); for (let i = 0; i < blocks.length; i += 1) { const block = blocks[i]; if (block?.type === "tool_call" && block.callId !== undefined) { callIdToCallIndex.set(block.callId, i); + } else if (block?.type === "tool_result") { + callIdToResultIndex.set(block.callId, i); } } let planSteps: PlanBlockStep[] | null = null; const indicesToRemove = new Set(); + // manage_tasks strips regardless of its result's outcome, matching + // applyManageTasksToolCall: the tool_call is the authoritative event, not + // whatever the (side-effect-free) handler's tool_result happens to say — + // so an errored or missing result must not leave the raw rows behind. + for (let i = 0; i < blocks.length; i += 1) { + const call = blocks[i]; + if (call?.type !== "tool_call" || call.name !== "manage_tasks") continue; + indicesToRemove.add(i); + const resultIndex = call.callId !== undefined ? callIdToResultIndex.get(call.callId) : undefined; + if (resultIndex !== undefined) indicesToRemove.add(resultIndex); + } + for (let i = 0; i < blocks.length; i += 1) { const result = blocks[i]; if (result?.type !== "tool_result" || result.isError) continue; const callIndex = callIdToCallIndex.get(result.callId); if (callIndex === undefined) continue; const call = blocks[callIndex]; - if (call?.type !== "tool_call") continue; - - if (call.name === "manage_tasks") { - indicesToRemove.add(callIndex); - indicesToRemove.add(i); - continue; - } - if (call.name !== "submit_plan") continue; + if (call?.type !== "tool_call" || call.name !== "submit_plan") continue; indicesToRemove.add(callIndex); indicesToRemove.add(i); diff --git a/tests/integration/harness.ts b/tests/integration/harness.ts index 09eb304e8..869d6f88a 100644 --- a/tests/integration/harness.ts +++ b/tests/integration/harness.ts @@ -70,7 +70,7 @@ export async function openIntegrationSession( id: `${ID_PREFIX}/chat`, configSchema: type({}), factory: (_config, _env, agentCtx) => - createChatDirector(agentCtx.systemPrompt, [...agentCtx.toolDefinitions], { inactivityTimeoutMs: 750_000 }), + createChatDirector(agentCtx.systemPrompt, [...agentCtx.toolDefinitions], { onTasksChange: () => {}, inactivityTimeoutMs: 750_000 }), }); const toolsFactory = defineTool({ diff --git a/tests/unit/director.test.ts b/tests/unit/director.test.ts index db3d7e54f..d570b59a5 100644 --- a/tests/unit/director.test.ts +++ b/tests/unit/director.test.ts @@ -105,7 +105,7 @@ const manyTurnsState: ReactorState = { }; function makeChatDirectorWithContinuation(onContinue: () => void) { - return createChatDirector("sys", [], { requestContinuation: onContinue }); + return createChatDirector("sys", [], { onTasksChange: () => {}, requestContinuation: onContinue }); } test("current context over threshold emits compact and a continuation request, not a dead loop", async () => { @@ -178,11 +178,12 @@ async function runToolOnlyStreak(director: ReturnType } test("a grok provider pauses the session after 10 tool-only turns, tighter than the default 20", async () => { - const grokDirector = createChatDirector("sys", [], { provider: { providerName: "xai", model: "grok-4" } }); + const grokDirector = createChatDirector("sys", [], { onTasksChange: () => {}, provider: { providerName: "xai", model: "grok-4" } }); const grokActions = await runToolOnlyStreak(grokDirector, 10); expect(grokActions.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe(true); const defaultDirector = createChatDirector("sys", [], { + onTasksChange: () => {}, provider: { providerName: "openai", model: "gpt-4" }, }); const defaultActions = await runToolOnlyStreak(defaultDirector, 10); diff --git a/tests/unit/workflows-director.test.ts b/tests/unit/workflows-director.test.ts index 4576bc5ac..bb1486efd 100644 --- a/tests/unit/workflows-director.test.ts +++ b/tests/unit/workflows-director.test.ts @@ -91,7 +91,7 @@ test("the active step directive is injected into the inferred system prompt", as const runtime = new WorkflowRuntime(emptyCaps, (n) => (n === "flow" ? flow : undefined)); runtime.start(flow); const coordinator = new WorkflowCoordinator(runtime); - const director = createChatDirector("BASE PROMPT", [], { workflowCoordinator: coordinator }); + const director = createChatDirector("BASE PROMPT", [], { onTasksChange: () => {}, workflowCoordinator: coordinator }); const event: ReactorInboundEvent = { type: "message.received", message: { role: "user", content: "go" } }; const result = await director.decide(event, state, makeCapabilities()); @@ -109,7 +109,7 @@ test("an advance_workflow tool call advances the runtime through the director", const runtime = new WorkflowRuntime(emptyCaps, (n) => (n === "flow" ? flow : undefined)); runtime.start(flow); const coordinator = new WorkflowCoordinator(runtime); - const director = createChatDirector("BASE", [], { workflowCoordinator: coordinator }); + const director = createChatDirector("BASE", [], { onTasksChange: () => {}, workflowCoordinator: coordinator }); const caps = makeCapabilities(); const turn: ReactorInboundEvent = { @@ -153,7 +153,7 @@ test("auto-continuation fires on reply() as well as wait() after a text turn", a const runtime = new WorkflowRuntime(emptyCaps, (n) => (n === "flow" ? flow : undefined)); runtime.start(flow); const coordinator = new WorkflowCoordinator(runtime); - const director = createChatDirector("BASE", [], { workflowCoordinator: coordinator }); + const director = createChatDirector("BASE", [], { onTasksChange: () => {}, workflowCoordinator: coordinator }); const caps = makeCapabilities(); // Simulate a text-only inference turn (no tool calls). @@ -206,7 +206,7 @@ test("a content-free workflow turn with open tasks nudges toward advance_workflo const runtime = new WorkflowRuntime(emptyCaps, (n) => (n === "flow" ? flow : undefined)); runtime.start(flow); const coordinator = new WorkflowCoordinator(runtime); - const director = createChatDirector("BASE", [], { workflowCoordinator: coordinator }); + const director = createChatDirector("BASE", [], { onTasksChange: () => {}, workflowCoordinator: coordinator }); const caps = makeCapabilities(); await director.decide(manageTasksTurn("doing"), state, caps); @@ -229,7 +229,7 @@ test("open tasks do not defeat the workflow stuck-cutoff after 3 idle turns", as const runtime = new WorkflowRuntime(emptyCaps, (n) => (n === "flow" ? flow : undefined)); runtime.start(flow); const coordinator = new WorkflowCoordinator(runtime); - const director = createChatDirector("BASE", [], { workflowCoordinator: coordinator }); + const director = createChatDirector("BASE", [], { onTasksChange: () => {}, workflowCoordinator: coordinator }); const caps = makeCapabilities(); await director.decide(manageTasksTurn("doing"), state, caps); @@ -247,7 +247,7 @@ test("auto-continuation falls back after 3 consecutive text-only turns", async ( const runtime = new WorkflowRuntime(emptyCaps, (n) => (n === "flow" ? flow : undefined)); runtime.start(flow); const coordinator = new WorkflowCoordinator(runtime); - const director = createChatDirector("BASE", [], { workflowCoordinator: coordinator }); + const director = createChatDirector("BASE", [], { onTasksChange: () => {}, workflowCoordinator: coordinator }); const caps = makeCapabilities(); await director.decide(textTurn("text 1"), state, caps); From 1351f05c7ade8495f3734b3c468e49e5962d87fe Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 13:07:42 -0700 Subject: [PATCH 5/5] Stop asking the operator to approve manage_tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit manage_tasks's handler has no side effect of its own — it parses its arguments and returns a fixed string. The task list it appears to control is actually mutated earlier, by the director's decide() loop at the tool_call event, before this tool ever executes. By the time an approval prompt for it would reach the operator, there is nothing left for a denial to undo, the same reasoning that already exempts read-only tools like lsp from approval. --- src/permission/classify.ts | 14 ++++++++++---- src/permission/gate.ts | 1 - src/permission/permission.test.ts | 19 ++++++++++++++++++- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/permission/classify.ts b/src/permission/classify.ts index f7ab32fac..5bf65e239 100644 --- a/src/permission/classify.ts +++ b/src/permission/classify.ts @@ -14,10 +14,16 @@ import type { RootsProvider } from "./worktree-roots.js"; // Read-only tools never need approval as long as they don't touch a restricted // path; they cannot change the workspace. `lsp` is included here even though // it is activated dynamically mid-session (see director.ts onActivateTools) — -// hover/definition/reference lookups are as inert as a grep. Every other posix -// tool is consequential and defaults to the "ask" tier. Catastrophic commands -// are denied earlier by the authorization plugin, so they never reach here. -const READ_ONLY_TOOLS = new Set(["read_file", "search_files", "grep", "list_dir", "lsp"]); +// hover/definition/reference lookups are as inert as a grep. `manage_tasks` is +// included for a related but distinct reason: its handler (src/agent/tools.ts) +// has no side effect of its own — the task list is mutated earlier, by the +// director's decide() loop at the tool_call event, before this tool ever +// executes (see applyManageTasksToolCall in src/agent/director.ts). By the +// time an operator would see an approval prompt for it, there is nothing left +// for a denial to prevent. Every other posix tool is consequential and +// defaults to the "ask" tier. Catastrophic commands are denied earlier by the +// authorization plugin, so they never reach here. +const READ_ONLY_TOOLS = new Set(["read_file", "search_files", "grep", "list_dir", "lsp", "manage_tasks"]); // Tools that take a single path-like argument the gate should check against // restriction (outside the workspace boundary, or writes under the session state root). diff --git a/src/permission/gate.ts b/src/permission/gate.ts index ad8c1227e..c05f085de 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -175,7 +175,6 @@ const AUTO_ALLOWED_TOOLS = new Set([ "write_file", "edit_file", "delete_file", - "manage_tasks", "manage_goal", "present", "tool_search", diff --git a/src/permission/permission.test.ts b/src/permission/permission.test.ts index 4a546738a..d3e7fed69 100644 --- a/src/permission/permission.test.ts +++ b/src/permission/permission.test.ts @@ -985,13 +985,30 @@ describe("createPermissionGate", () => { const editVerdict = await gate.evaluate({ id: "c", name: "edit_file", arguments: { path: "src/a.ts" } }); expect(editVerdict.allowed).toBe(true); // Benign built-ins a hands-off run should not stop for. - for (const name of ["manage_tasks", "present", "tool_search", "use_skill", "search_agents", "task"]) { + for (const name of ["present", "tool_search", "use_skill", "search_agents", "task"]) { const verdict = await gate.evaluate({ id: "c", name, arguments: {} }); expect(verdict.allowed).toBe(true); } expect(asked).toBe(0); }); + // manage_tasks's handler has no side effect — the task list is mutated + // earlier by the director, before this tool ever executes — so denying it + // cannot undo anything. It auto-allows unconditionally, not just in auto + // mode, unlike the tools above. + test("manage_tasks auto-allows outside auto mode too", async () => { + let asked = 0; + const gate = createPermissionGate({ + approvals: [], + requestApproval: async () => { asked++; return { allow: false }; }, + interactive: true, + skipPermissions: false, + }); + const verdict = await gate.evaluate({ id: "c", name: "manage_tasks", arguments: {} }); + expect(verdict.allowed).toBe(true); + expect(asked).toBe(0); + }); + test("auto mode routes MCP tools to the operator prompt rather than blanket-allow", async () => { let asked = 0; const gate = createPermissionGate({