Skip to content

Commit 336cfd5

Browse files
Merge pull request #408 from corbitsdev/cl-5708-wire-live-task-updates
Wire live task updates and simplify createChatDirector's options
2 parents 11d4c0e + 1351f05 commit 336cfd5

14 files changed

Lines changed: 396 additions & 250 deletions

src/agent/director.test.ts

Lines changed: 7 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -100,18 +100,7 @@ describe("ChatDirector tool-only loop protection", () => {
100100
const providerlessPolicy = { providerName: "test-provider" };
101101

102102
test("nudges once at the family threshold, after pending tools execute", async () => {
103-
const director = createChatDirector(
104-
"system",
105-
[],
106-
undefined,
107-
undefined,
108-
undefined,
109-
undefined,
110-
undefined,
111-
undefined,
112-
undefined,
113-
providerlessPolicy,
114-
);
103+
const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: providerlessPolicy });
115104
const capabilities = makeCapabilities();
116105

117106
// Default family nudges at 12 consecutive tool-only turns.
@@ -122,18 +111,7 @@ describe("ChatDirector tool-only loop protection", () => {
122111
});
123112

124113
test("the nudge is one-shot — it does not repeat on the next tool-only turn", async () => {
125-
const director = createChatDirector(
126-
"system",
127-
[],
128-
undefined,
129-
undefined,
130-
undefined,
131-
undefined,
132-
undefined,
133-
undefined,
134-
undefined,
135-
providerlessPolicy,
136-
);
114+
const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: providerlessPolicy });
137115
const capabilities = makeCapabilities();
138116

139117
await runToolOnlyStreak(director, capabilities, 12);
@@ -144,18 +122,7 @@ describe("ChatDirector tool-only loop protection", () => {
144122
});
145123

146124
test("pauses and stops issuing infers at the family pause threshold", async () => {
147-
const director = createChatDirector(
148-
"system",
149-
[],
150-
undefined,
151-
undefined,
152-
undefined,
153-
undefined,
154-
undefined,
155-
undefined,
156-
undefined,
157-
providerlessPolicy,
158-
);
125+
const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: providerlessPolicy });
159126
const capabilities = makeCapabilities();
160127

161128
// Default family pauses at 20 consecutive tool-only turns.
@@ -169,18 +136,7 @@ describe("ChatDirector tool-only loop protection", () => {
169136
});
170137

171138
test("resumes after the operator sends a new message", async () => {
172-
const director = createChatDirector(
173-
"system",
174-
[],
175-
undefined,
176-
undefined,
177-
undefined,
178-
undefined,
179-
undefined,
180-
undefined,
181-
undefined,
182-
providerlessPolicy,
183-
);
139+
const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: providerlessPolicy });
184140
const capabilities = makeCapabilities();
185141

186142
await runToolOnlyStreak(director, capabilities, 20);
@@ -191,18 +147,7 @@ describe("ChatDirector tool-only loop protection", () => {
191147
});
192148

193149
test("a dismissed ask_operator counts toward the streak like any other tool-only turn", async () => {
194-
const director = createChatDirector(
195-
"system",
196-
[],
197-
undefined,
198-
undefined,
199-
undefined,
200-
undefined,
201-
undefined,
202-
undefined,
203-
undefined,
204-
providerlessPolicy,
205-
);
150+
const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: providerlessPolicy });
206151
const capabilities = makeCapabilities();
207152

208153
// 11 ordinary tool-only turns, then a turn whose only tool call is a
@@ -252,18 +197,7 @@ describe("ChatDirector tool-only loop protection", () => {
252197
});
253198

254199
test("a busy-but-progressing session (text interleaved with tools) never trips", async () => {
255-
const director = createChatDirector(
256-
"system",
257-
[],
258-
undefined,
259-
undefined,
260-
undefined,
261-
undefined,
262-
undefined,
263-
undefined,
264-
undefined,
265-
providerlessPolicy,
266-
);
200+
const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: providerlessPolicy });
267201
const capabilities = makeCapabilities();
268202

269203
let lastActions: ReactorAction[] = [];
@@ -278,18 +212,7 @@ describe("ChatDirector tool-only loop protection", () => {
278212
});
279213

280214
test("grok's tightened thresholds fire earlier than the default family", async () => {
281-
const director = createChatDirector(
282-
"system",
283-
[],
284-
undefined,
285-
undefined,
286-
undefined,
287-
undefined,
288-
undefined,
289-
undefined,
290-
undefined,
291-
{ providerName: "xai/default", model: "grok-4.5" },
292-
);
215+
const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: { providerName: "xai/default", model: "grok-4.5" } });
293216
const capabilities = makeCapabilities();
294217

295218
// Grok nudges at 6, well below the default family's 12.

src/agent/director.ts

Lines changed: 82 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,40 @@ function isCodeFile(path: string): boolean {
303303
return CODE_FILE_EXT.test(path);
304304
}
305305

306+
// Single implementation of "what does a manage_tasks tool call do to the
307+
// task list", shared by the live decide() loop below and hydrateTasksFromTurns.
308+
// Task state is owned by the director, not by the tool: manage_tasks's
309+
// handler (src/agent/tools.ts) performs no side effect of its own — it
310+
// parses the same arguments and returns a fixed "Tasks updated." string. The
311+
// tool_call is therefore the authoritative event, and applying it here does
312+
// not need to wait on a tool_result the handler never varies.
313+
// Returns null when the call is not manage_tasks or its arguments don't
314+
// parse, so callers can distinguish "no valid manage_tasks call here" from
315+
// "a valid call that happened to be a no-op" — the latter still counts as an
316+
// update for onTasksChange purposes.
317+
function applyManageTasksToolCall(tasks: Task[], block: { name: string; arguments: unknown }): Task[] | null {
318+
if (block.name !== "manage_tasks") return null;
319+
const taskArgs = parseManageTasksArgs(block.arguments);
320+
return taskArgs !== null ? applyManageTasks(tasks, taskArgs) : null;
321+
}
322+
323+
export type ChatDirectorOptions = {
324+
taskClassifier?: ((message: string, metadata: SessionMetadata) => Promise<TaskBoundary>) | undefined;
325+
onActivateTools?: ((names: string[]) => void) | undefined;
326+
inactivityTimeoutMs?: number | undefined;
327+
totalTimeoutMs?: number | undefined;
328+
workflowCoordinator?: WorkflowCoordinator | undefined;
329+
onTasksChange: (tasks: Task[]) => void;
330+
requestContinuation?: (() => void) | undefined;
331+
provider?: { providerName: string; model?: string } | undefined;
332+
};
333+
334+
// The constructor takes the resolved ModelFamilyPolicy rather than the raw
335+
// `provider` input the factory function accepts and resolves on its behalf.
336+
type ChatDirectorImplOptions = Omit<ChatDirectorOptions, "provider"> & {
337+
modelFamilyPolicy?: ModelFamilyPolicy | undefined;
338+
};
339+
306340
class ChatDirectorImpl extends DefaultDirector {
307341
private readonly workflowCalls = new Map<string, { name: string; args: unknown }>();
308342
private readonly lspTriggerCalls = new Set<string>();
@@ -341,29 +375,18 @@ class ChatDirectorImpl extends DefaultDirector {
341375
private pendingToolOnlyNudge = false;
342376
private pausedForToolOnly = false;
343377

344-
constructor(
345-
systemPrompt: string,
346-
toolDefinitions: ToolDefinition[],
347-
taskClassifier?: (message: string, metadata: SessionMetadata) => Promise<TaskBoundary>,
348-
onActivateTools?: (names: string[]) => void,
349-
inactivityTimeoutMs?: number,
350-
totalTimeoutMs?: number,
351-
workflowCoordinator?: WorkflowCoordinator,
352-
onTasksChange?: (tasks: Task[]) => void,
353-
requestContinuation?: () => void,
354-
modelFamilyPolicy?: ModelFamilyPolicy,
355-
) {
378+
constructor(systemPrompt: string, toolDefinitions: ToolDefinition[], options: ChatDirectorImplOptions) {
356379
super(systemPrompt, toolDefinitions, {});
357380
this._systemPrompt = systemPrompt;
358381
this._toolDefinitions = toolDefinitions;
359-
this.inactivityTimeoutMs = inactivityTimeoutMs;
360-
this.totalTimeoutMs = totalTimeoutMs;
361-
this.taskClassifier = taskClassifier;
362-
this.onActivateTools = onActivateTools;
363-
this.workflowCoordinator = workflowCoordinator;
364-
this.onTasksChange = onTasksChange;
365-
this.compaction = createCompactionGovernor(requestContinuation, systemPrompt, toolDefinitions);
366-
this.modelFamilyPolicy = modelFamilyPolicy ?? resolveModelFamilyPolicy({ providerName: "" });
382+
this.inactivityTimeoutMs = options.inactivityTimeoutMs;
383+
this.totalTimeoutMs = options.totalTimeoutMs;
384+
this.taskClassifier = options.taskClassifier;
385+
this.onActivateTools = options.onActivateTools;
386+
this.workflowCoordinator = options.workflowCoordinator;
387+
this.onTasksChange = options.onTasksChange;
388+
this.compaction = createCompactionGovernor(options.requestContinuation, systemPrompt, toolDefinitions);
389+
this.modelFamilyPolicy = options.modelFamilyPolicy ?? resolveModelFamilyPolicy({ providerName: "" });
367390
}
368391

369392
setWorkflowCoordinator(coordinator: WorkflowCoordinator | undefined): void {
@@ -386,6 +409,15 @@ class ChatDirectorImpl extends DefaultDirector {
386409
return [...this.tasks];
387410
}
388411

412+
// A resumed session's task list lives in the transcript, not in the freshly
413+
// constructed director. Without this the chrome panel would read an empty
414+
// list until the model happened to call manage_tasks again, disagreeing
415+
// with the task block already painted in the transcript.
416+
restoreTasks(tasks: Task[]): void {
417+
this.tasks = [...tasks];
418+
this.onTasksChange?.(this.tasks);
419+
}
420+
389421
// The status bar's context meter falls back to this when a provider omits
390422
// or zeroes usage on the latest turn — a local lower-then-corrected bound
391423
// beats displaying a number the provider never actually reported.
@@ -617,9 +649,9 @@ class ChatDirectorImpl extends DefaultDirector {
617649
for (const block of event.turn.content) {
618650
if (block.type !== "tool_call") continue;
619651
if (block.name === "manage_tasks") {
620-
const taskArgs = parseManageTasksArgs(block.arguments);
621-
if (taskArgs !== null) {
622-
this.tasks = applyManageTasks(this.tasks, taskArgs);
652+
const next = applyManageTasksToolCall(this.tasks, block);
653+
if (next !== null) {
654+
this.tasks = next;
623655
this.onTasksChange?.(this.tasks);
624656
}
625657
} else if (block.name === "read_file" || block.name === "edit_file") {
@@ -775,27 +807,33 @@ class ChatDirectorImpl extends DefaultDirector {
775807
export function createChatDirector(
776808
systemPrompt: string,
777809
toolDefinitions: ToolDefinition[],
778-
taskClassifier?: (message: string, metadata: SessionMetadata) => Promise<TaskBoundary>,
779-
onActivateTools?: (names: string[]) => void,
780-
inactivityTimeoutMs?: number,
781-
totalTimeoutMs?: number,
782-
workflowCoordinator?: WorkflowCoordinator,
783-
onTasksChange?: (tasks: Task[]) => void,
784-
requestContinuation?: () => void,
785-
provider?: { providerName: string; model?: string },
810+
options: ChatDirectorOptions,
786811
): ChatDirector {
787-
return new ChatDirectorImpl(
788-
systemPrompt,
789-
toolDefinitions,
790-
taskClassifier,
791-
onActivateTools,
792-
inactivityTimeoutMs,
793-
totalTimeoutMs,
794-
workflowCoordinator,
795-
onTasksChange,
796-
requestContinuation,
797-
provider !== undefined ? resolveModelFamilyPolicy(provider) : undefined,
798-
);
812+
const { provider, ...rest } = options;
813+
return new ChatDirectorImpl(systemPrompt, toolDefinitions, {
814+
...rest,
815+
// `provider` is raw {providerName, model} input; the constructor wants
816+
// the resolved ModelFamilyPolicy, not the input it was resolved from.
817+
modelFamilyPolicy: provider !== undefined ? resolveModelFamilyPolicy(provider) : undefined,
818+
});
819+
}
820+
821+
// Uses the same applyManageTasksToolCall a live session's decide() loop uses,
822+
// so hydrate necessarily reaches the same task state live decide() would
823+
// have produced from this transcript: the tool_call is the authoritative
824+
// event (see applyManageTasksToolCall), and there is only the one function
825+
// that knows how to turn a manage_tasks call into a task list.
826+
export function hydrateTasksFromTurns(turns: ConversationTurn[]): Task[] {
827+
let tasks: Task[] = [];
828+
for (const turn of turns) {
829+
if (turn.role !== "assistant") continue;
830+
for (const block of turn.content) {
831+
if (block.type !== "tool_call") continue;
832+
const next = applyManageTasksToolCall(tasks, block);
833+
if (next !== null) tasks = next;
834+
}
835+
}
836+
return tasks;
799837
}
800838

801839
export interface ChatDirector extends ReactorDirector {
@@ -804,5 +842,6 @@ export interface ChatDirector extends ReactorDirector {
804842
setGoalGovernor(goal: GoalGovernor | undefined): void;
805843
getGoalGovernor(): GoalGovernor | undefined;
806844
getTasks(): Task[];
845+
restoreTasks(tasks: Task[]): void;
807846
getContextEstimate(): { tokens: number; isEstimate: boolean };
808847
}

0 commit comments

Comments
 (0)