Wire live task updates and simplify createChatDirector's options - #408
Merged
Conversation
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.
…hydrate parser 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.
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.
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
bun run typecheckbun run buildbun run test(4184/4185 pass; the one failure,src/agent/lsp-availability.test.ts, is a pre-existing environment-dependent check unrelated to this change — it asserts a language server binary is installed in the sandbox)