Skip to content

Commit 48b2d6f

Browse files
TheGreatAxiosTheGreatAxios
committed
CL-7919: decide workflowCoordinator/taskClassifier shape, remove both from ChatDirectorOptions
Decision: - workflow coordination is host-owned. WorkflowHost owns the runtime lifecycle (start/resume/reset/persist), which is load-bearing, so the WorkflowCoordinator instance stays host-owned and reaches the director only through the narrow setWorkflowCoordinator live-object seam (the only path production ever used; the constructor option had zero suppliers). All director consult sites (directive, idle rails, gate, handleToolDone) unchanged. - task-boundary classification is neither a host closure nor native director behavior. The taskClassifier seam had zero production suppliers (assembleChatAgent funnel never passes it; TUI and exec omit it), so the decide()-time new-task path was inert in product and its removal keeps TUI behavior byte-identical. The pure classifier (classifyTaskBoundary Tier-1 heuristics + caller-supplied Tier-2) stays in session/compactor.ts as host-free library. Rejected: - tools the director calls (loop-internal automation must not mount model-visible surface), - BaseEnv handles (live non-serializable objects are not config; no new env keys), - moving decide()-time directive/idle/gate rails out of the director (they are the loop), - keeping the constructor option (dead duplicate of the setter blocking zero-closure), - native heuristics-only classification in the director (would newly arm new-task envelopes in the TUI), - native full-LLM-tier classification (decide() has no side-channel inference handle; adding one would be a new host closure). Zero-closure: both closures removed from ChatDirectorOptions. Remaining closures (onActivateTools, onTasksChange, requestContinuation, getProviderId, getLiveFleetCount) belong to sibling lanes CL-7916/7917/7918; this branch merges last (7916->7917->7918->7919). Tests: migrated tests/unit/workflows-director.test.ts (8 sites) from the constructor option to setWorkflowCoordinator; replaced the injected- classifier test with a no-checkpoint normal-infer pin; added setter attach/detach directive tests. bun run check green (7385 pass, 0 fail).
1 parent 174d7e3 commit 48b2d6f

3 files changed

Lines changed: 110 additions & 76 deletions

File tree

‎src/agent/director.ts‎

Lines changed: 18 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,7 @@ import type {
1414
ConversationTurn,
1515
RetryPolicy,
1616
} from "@intx/types/runtime";
17-
import {
18-
type SessionMetadata,
19-
type TaskBoundary,
20-
isCompactSpacerEchoTurn,
21-
} from "../session/compactor.js";
17+
import { isCompactSpacerEchoTurn } from "../session/compactor.js";
2218
import type { WorkflowCoordinator } from "../workflows/coordinator.js";
2319
import {
2420
createCompactionGovernor,
@@ -423,13 +419,23 @@ function applyManageTasksToolCall(
423419
}
424420

425421
export interface ChatDirectorOptions {
426-
taskClassifier?:
427-
| ((message: string, metadata: SessionMetadata) => Promise<TaskBoundary>)
428-
| undefined;
422+
// CL-7919: task-boundary classification and workflow coordination are not
423+
// host-injected closures. Classification's pure core lives in
424+
// session/compactor.ts (classifyTaskBoundary); the director runs no
425+
// decide()-time classification — the taskClassifier seam had zero
426+
// production suppliers, and a native heuristics-only hook would newly arm
427+
// new-task envelopes in the TUI. Coordination is host-owned (WorkflowHost
428+
// owns the runtime lifecycle: start/resume/reset/persist) and reaches the
429+
// director only through setWorkflowCoordinator, the narrow live-object
430+
// seam below — never through options. Rejected: tools the director calls
431+
// (loop-internal automation must not mount model-visible surface),
432+
// BaseEnv handles (live non-serializable objects are not config), moving
433+
// decide()-time directive/idle/gate rails out of the director (they are
434+
// the loop), and keeping the constructor option (dead duplicate of the
435+
// setter that keeps a host closure in options).
429436
onActivateTools?: ((names: string[]) => void) | undefined;
430437
inactivityTimeoutMs?: number | undefined;
431438
totalTimeoutMs?: number | undefined;
432-
workflowCoordinator?: WorkflowCoordinator | undefined;
433439
onTasksChange: (tasks: Task[]) => void;
434440
requestContinuation?: (() => void) | undefined;
435441
provider?: { providerName: string; model?: string } | undefined;
@@ -465,13 +471,13 @@ class ChatDirectorImpl extends DefaultDirector {
465471
private readonly lspTriggerCalls = new Set<string>();
466472
private readonly askOperatorCalls = new Set<string>();
467473
private readonly onActivateTools: ((names: string[]) => void) | undefined;
468-
private readonly taskClassifier:
469-
| ((message: string, metadata: SessionMetadata) => Promise<TaskBoundary>)
470-
| undefined;
471474
private readonly _systemPrompt: string;
472475
private _toolDefinitions: ToolDefinition[];
473476
private inactivityTimeoutMs: number | undefined;
474477
private totalTimeoutMs: number | undefined;
478+
// CL-7919: host-owned live object, attached via setWorkflowCoordinator
479+
// (WorkflowHost owns the runtime lifecycle). Consulted, never constructed
480+
// here; deliberately not a constructor option.
475481
private workflowCoordinator: WorkflowCoordinator | undefined;
476482
private workflowIdleTurns = 0;
477483
private idleTerminationNudges = 0;
@@ -482,10 +488,6 @@ class ChatDirectorImpl extends DefaultDirector {
482488
private operatorJustResponded = false;
483489
private tasks: Task[] = [];
484490
private readonly onTasksChange: ((tasks: Task[]) => void) | undefined;
485-
private turnCount = 0;
486-
private currentTaskLabel: string | undefined;
487-
private lastTaskSummary: string | undefined;
488-
private startedAt = Date.now();
489491
private readonly compaction: CompactionGovernor;
490492
private readonly modelFamilyPolicy: ModelFamilyPolicy;
491493
private readonly retryPolicy: RetryPolicy;
@@ -526,9 +528,7 @@ class ChatDirectorImpl extends DefaultDirector {
526528
this._toolDefinitions = toolDefinitions;
527529
this.inactivityTimeoutMs = options.inactivityTimeoutMs;
528530
this.totalTimeoutMs = options.totalTimeoutMs;
529-
this.taskClassifier = options.taskClassifier;
530531
this.onActivateTools = options.onActivateTools;
531-
this.workflowCoordinator = options.workflowCoordinator;
532532
this.onTasksChange = options.onTasksChange;
533533
this.compaction = createCompactionGovernor(
534534
options.requestContinuation,
@@ -771,52 +771,7 @@ class ChatDirectorImpl extends DefaultDirector {
771771
}
772772
if (onTurnBoundary(event)) this.inferenceRecoveries = 0;
773773

774-
if (
775-
event.type === "message.received" &&
776-
this.taskClassifier !== undefined
777-
) {
778-
const message = event.message;
779-
const content =
780-
typeof message.content === "string" ? message.content : "";
781-
const metadata: SessionMetadata = {
782-
turnCount: this.turnCount,
783-
currentTaskLabel: this.currentTaskLabel,
784-
lastTaskSummary: this.lastTaskSummary,
785-
minutesElapsed: Math.floor((Date.now() - this.startedAt) / 60000),
786-
toolCallCount: 0,
787-
};
788-
789-
try {
790-
const boundary = await this.taskClassifier(content, metadata);
791-
if (boundary.kind === "new_task") {
792-
this.currentTaskLabel = undefined;
793-
794-
const envelope =
795-
this.lastTaskSummary !== undefined
796-
? `\n--- Compacted prior context ---\n${this.lastTaskSummary}\n---` +
797-
`\n\nNew task starting now. Prior context summarized above.\n`
798-
: "\n--- Context cleared for new task ---\n";
799-
800-
return [
801-
capabilities.checkpoint(`new-task: ${boundary.reason}`),
802-
capabilities.infer(
803-
withEphemeralNudge(
804-
{
805-
systemPrompt: this._systemPrompt,
806-
tools: this._toolDefinitions,
807-
},
808-
envelope,
809-
),
810-
),
811-
];
812-
}
813-
} catch {
814-
// Classifier failure should not break the session. Fall through to infer.
815-
}
816-
}
817-
818774
if (onTurnBoundary(event)) {
819-
this.turnCount++;
820775
const hasToolCalls = event.turn.content.some(
821776
(b) => b.type === "tool_call",
822777
);

‎src/director.test.ts‎

Lines changed: 84 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import {
99
LEGACY_COMPACT_SPACER_TEXT,
1010
compactorNoOpFloor,
1111
} from "./session/compactor.js";
12-
import type { SessionMetadata, TaskBoundary } from "./session/compactor.js";
1312
import {
1413
validateActions,
1514
type ExtendedInferenceOptions,
@@ -1353,12 +1352,11 @@ describe("updateToolDefinitions rewrites infer tools", () => {
13531352
await toolset.dispose();
13541353
});
13551354

1356-
test("the new-task path also carries the current tools", async () => {
1357-
const classifier = async (_msg: string, _meta: SessionMetadata) =>
1358-
({ kind: "new_task" as const, reason: "pivot" }) as TaskBoundary;
1355+
// CL-7919: the taskClassifier host closure is gone, so a plain message
1356+
// flows to normal inference with no new-task checkpoint or envelope.
1357+
test("a message with no classifier configured takes the normal infer path", async () => {
13591358
const director = createChatDirector("base-prompt", [], {
13601359
onTasksChange: () => undefined,
1361-
taskClassifier: classifier,
13621360
});
13631361
director.updateToolDefinitions([lateTool]);
13641362

@@ -1373,6 +1371,87 @@ describe("updateToolDefinitions rewrites infer tools", () => {
13731371
| undefined;
13741372
expect(inferAction).toBeDefined();
13751373
expect(inferToolNames(inferAction)).toContain("mcp__acme__list_issues");
1374+
expect(actions.some((a) => a.type === "checkpoint")).toBe(false);
1375+
});
1376+
});
1377+
1378+
describe("CL-7919 coordinator shape", () => {
1379+
const makeMessageReceivedEvent = (content: string) =>
1380+
({
1381+
type: "message.received",
1382+
message: { role: "user", content },
1383+
}) as unknown as ReactorInboundEvent;
1384+
const capabilitiesWithInferArgs: ReactorCapabilities = {
1385+
...mockCapabilities,
1386+
infer: (opts) =>
1387+
({ type: "infer", options: opts }) as unknown as ReactorAction,
1388+
};
1389+
const inferEphemeralText = (
1390+
action: ReactorAction | undefined,
1391+
): string | undefined => {
1392+
if (action?.type !== "infer") return undefined;
1393+
const turns = (action.options as { ephemeralTurns?: unknown } | undefined)
1394+
?.ephemeralTurns;
1395+
if (!Array.isArray(turns) || turns.length === 0) return undefined;
1396+
const first = turns[0] as { content?: { text?: string }[] };
1397+
return first.content?.[0]?.text;
1398+
};
1399+
1400+
// CL-7919: coordination is host-owned and reaches the director only
1401+
// through setWorkflowCoordinator — the constructor takes no coordinator.
1402+
// Attaching a live coordinator injects its directive into the next infer.
1403+
test("setWorkflowCoordinator attaches live coordination to the loop", async () => {
1404+
const { WorkflowRuntime } = await import("./workflows/runtime.js");
1405+
const { WorkflowCoordinator } = await import("./workflows/coordinator.js");
1406+
const workflow = {
1407+
name: "shape",
1408+
description: "setter seam",
1409+
steps: [{ id: "a", label: "A" }],
1410+
};
1411+
const runtime = new WorkflowRuntime(new Map(), () => workflow);
1412+
runtime.start(workflow);
1413+
const director = createChatDirector("base-prompt", [], {
1414+
onTasksChange: () => undefined,
1415+
});
1416+
director.setWorkflowCoordinator(new WorkflowCoordinator(runtime));
1417+
1418+
const actions = actionsArray(
1419+
await director.decide(
1420+
makeMessageReceivedEvent("hello"),
1421+
mockState,
1422+
capabilitiesWithInferArgs,
1423+
),
1424+
);
1425+
const infer = actions.find((a) => a.type === "infer");
1426+
expect(inferEphemeralText(infer)).toContain("[WORKFLOW STEP 1/1: A]");
1427+
});
1428+
1429+
// Detaching restores the plain loop: no directive once cleared.
1430+
test("clearing the coordinator removes the directive", async () => {
1431+
const { WorkflowRuntime } = await import("./workflows/runtime.js");
1432+
const { WorkflowCoordinator } = await import("./workflows/coordinator.js");
1433+
const workflow = {
1434+
name: "shape",
1435+
description: "setter seam",
1436+
steps: [{ id: "a", label: "A" }],
1437+
};
1438+
const runtime = new WorkflowRuntime(new Map(), () => workflow);
1439+
runtime.start(workflow);
1440+
const director = createChatDirector("base-prompt", [], {
1441+
onTasksChange: () => undefined,
1442+
});
1443+
director.setWorkflowCoordinator(new WorkflowCoordinator(runtime));
1444+
director.setWorkflowCoordinator(undefined);
1445+
1446+
const actions = actionsArray(
1447+
await director.decide(
1448+
makeMessageReceivedEvent("hello"),
1449+
mockState,
1450+
capabilitiesWithInferArgs,
1451+
),
1452+
);
1453+
const infer = actions.find((a) => a.type === "infer");
1454+
expect(inferEphemeralText(infer)).toBeUndefined();
13761455
});
13771456
});
13781457

‎tests/unit/workflows-director.test.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ test("the active step directive is injected into the inferred system prompt", as
109109
const coordinator = new WorkflowCoordinator(runtime);
110110
const director = createChatDirector("BASE PROMPT", [], {
111111
onTasksChange: () => undefined,
112-
workflowCoordinator: coordinator,
113112
});
113+
director.setWorkflowCoordinator(coordinator);
114114

115115
const event: ReactorInboundEvent = {
116116
type: "message.received",
@@ -147,8 +147,8 @@ test("a submit_output tool call with the current step id advances the runtime th
147147
const coordinator = new WorkflowCoordinator(runtime);
148148
const director = createChatDirector("BASE", [], {
149149
onTasksChange: () => undefined,
150-
workflowCoordinator: coordinator,
151150
});
151+
director.setWorkflowCoordinator(coordinator);
152152
const caps = makeCapabilities();
153153

154154
const turn: ReactorInboundEvent = {
@@ -189,8 +189,8 @@ test("a stale submit_output does not skip ahead through the director", async ()
189189
const coordinator = new WorkflowCoordinator(runtime);
190190
const director = createChatDirector("BASE", [], {
191191
onTasksChange: () => undefined,
192-
workflowCoordinator: coordinator,
193192
});
193+
director.setWorkflowCoordinator(coordinator);
194194
const caps = makeCapabilities();
195195

196196
const turn: ReactorInboundEvent = {
@@ -280,8 +280,8 @@ test("auto-continuation fires on reply() as well as wait() after a text turn", a
280280
const coordinator = new WorkflowCoordinator(runtime);
281281
const director = createChatDirector("BASE", [], {
282282
onTasksChange: () => undefined,
283-
workflowCoordinator: coordinator,
284283
});
284+
director.setWorkflowCoordinator(coordinator);
285285
const caps = makeCapabilities();
286286

287287
// Simulate a text-only inference turn (no tool calls).
@@ -356,8 +356,8 @@ test("a content-free workflow turn with open tasks nudges toward submit_output",
356356
const coordinator = new WorkflowCoordinator(runtime);
357357
const director = createChatDirector("BASE", [], {
358358
onTasksChange: () => undefined,
359-
workflowCoordinator: coordinator,
360359
});
360+
director.setWorkflowCoordinator(coordinator);
361361
const caps = makeCapabilities();
362362

363363
await director.decide(manageTasksTurn("doing"), state, caps);
@@ -386,8 +386,8 @@ test("open tasks do not defeat the workflow stuck-cutoff after 3 idle turns", as
386386
const coordinator = new WorkflowCoordinator(runtime);
387387
const director = createChatDirector("BASE", [], {
388388
onTasksChange: () => undefined,
389-
workflowCoordinator: coordinator,
390389
});
390+
director.setWorkflowCoordinator(coordinator);
391391
const caps = makeCapabilities();
392392

393393
await director.decide(manageTasksTurn("doing"), state, caps);
@@ -411,8 +411,8 @@ test("auto-continuation falls back after 3 consecutive text-only turns", async (
411411
const coordinator = new WorkflowCoordinator(runtime);
412412
const director = createChatDirector("BASE", [], {
413413
onTasksChange: () => undefined,
414-
workflowCoordinator: coordinator,
415414
});
415+
director.setWorkflowCoordinator(coordinator);
416416
const caps = makeCapabilities();
417417

418418
await director.decide(textTurn("text 1"), state, caps);
@@ -436,8 +436,8 @@ test("after spacer echo-cap a non-gate workflow step does not empty-settle", asy
436436
const coordinator = new WorkflowCoordinator(runtime);
437437
const director = createChatDirector("BASE", [], {
438438
onTasksChange: () => undefined,
439-
workflowCoordinator: coordinator,
440439
});
440+
director.setWorkflowCoordinator(coordinator);
441441
const caps = makeCapabilities();
442442

443443
for (let i = 0; i < 2; i++) {

0 commit comments

Comments
 (0)