Skip to content

Commit 6365906

Browse files
refactor(director): resolve provider and fleet state without host closures (#1044)
* feat(director): resolve getProviderId and getLiveFleetCount without host closures (CL-7918) Decision record: - getProviderId -> reactor-supplied. Retry stamping needs the live source id so mid-session /model switches remap (xAI bare-429 retry vs abort). Neither static config (loses remap liveness) nor BaseEnv.sources (carries definitions, not the live selected id) suffice. The director now seeds from options.provider.providerName and refreshes the stamped id from each inference completion's source.sourceId; BaseEnv.sources remains the fallback only until the first completion stamps it. - getLiveFleetCount -> static config allowIdleWithFleet (default false). Live counts only ever gated one boolean (non-zero allows terminal wait/reply with open tasks); the exact count was never consumed. Rejected: BaseEnv-derived (no fleet state there) and reactor-supplied (the reactor owns no fleet-lane registry; a live count closure just re-adds the host hook this lane removes). TUI chat sets allowIdleWithFleet: true (fleet lanes may appear mid-session); exec and the default keep the historical nudge-while-open behavior. Static true is strictly more permissive than the old count>0 gate once a fleet lane drains, which is the safe direction for terminal wait. Both closures removed from ChatDirectorOptions. No new env key. Mid-session /model remap and idle-with-fleet behavior covered by src/director.test.ts (remap abort->retry across a source switch; idle-with-fleet terminal wait without nudge spend). * fix(director): ignore empty source ids in live source tracking (#1056) * fix(director): ignore empty source ids in live source tracking An empty-string source id on a completion or cycle source no longer clobbers the learned source id used to stamp retry decisions. * docs(director): align cycle-source comment with override precedence (#1057) * feat(director): liven idle-with-fleet flag and decouple host from workflow seam typing (#1048) * feat(director): liven idle-with-fleet flag and decouple host from workflow seam typing Summary: optionalize the host-to-runtime workflow seam so the host degrades gracefully against directors without workflow support; replace the static idle-with-fleet seed with a live narrow setter driven by fleet-wake publisher transitions. Verification: bun run check passes (7393 tests, 0 fail). * fix(tui): re-sync idle-with-fleet flag on director rebuilds (#1058)
1 parent 71d9052 commit 6365906

11 files changed

Lines changed: 621 additions & 67 deletions

File tree

‎src/agent/director.test.ts‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,3 +366,172 @@ describe("ChatDirector inference-error recovery (CL-6910)", () => {
366366
);
367367
});
368368
});
369+
370+
// CL-7973: the director's live source id (which stamps retry decisions so a
371+
// mid-session /model switch remaps the xAI short-429 handling) is observable
372+
// only through the retry policy it hands to each infer action. An xAI-gated
373+
// capacity error retries when the tracked id is an xAI source and aborts
374+
// otherwise, so driving tracking events then invoking the attached policy
375+
// reads the tracked id without reaching into privates.
376+
type LiveRetryPolicy = (situation: {
377+
attempt: number;
378+
elapsedMs: number;
379+
error: { category: "protocol_mismatch"; message: string };
380+
}) => Promise<{ kind: string }> | { kind: string };
381+
382+
function textCompletion(sourceId?: string): ReactorInboundEvent {
383+
const turn = {
384+
role: "assistant",
385+
model: "test",
386+
timestamp: 0,
387+
content: [{ type: "text", text: "done work" }],
388+
};
389+
const event: Record<string, unknown> = {
390+
type: "inference.done",
391+
turn,
392+
usage: { input: 0, output: 0 },
393+
};
394+
if (sourceId !== undefined)
395+
event["source"] = { sourceId, provider: "p", model: "test" };
396+
return event as unknown as ReactorInboundEvent;
397+
}
398+
399+
function stateWithCycleSource(sourceId: string): ReactorState {
400+
return {
401+
turns: [],
402+
lastCycleSource: { sourceId, provider: "p", model: "test" },
403+
} as unknown as ReactorState;
404+
}
405+
406+
async function liveRetryPolicy(
407+
director: ReturnType<typeof createChatDirector>,
408+
capabilities: ReactorCapabilities,
409+
): Promise<LiveRetryPolicy> {
410+
await director.decide(toolOnlyTurn("source-probe"), mockState, capabilities);
411+
const actions = actionsArray(
412+
await director.decide(
413+
toolDoneEvent("source-probe"),
414+
mockState,
415+
capabilities,
416+
),
417+
);
418+
const infer = actions.find((a) => a.type === "infer") as
419+
| { type: "infer"; options?: { retryPolicy?: LiveRetryPolicy } }
420+
| undefined;
421+
if (infer?.options?.retryPolicy === undefined)
422+
throw new Error("expected an infer action carrying the live retry policy");
423+
return infer.options.retryPolicy;
424+
}
425+
426+
async function isXaiStamped(policy: LiveRetryPolicy): Promise<boolean> {
427+
const decision = await policy({
428+
attempt: 1,
429+
elapsedMs: 0,
430+
error: {
431+
category: "protocol_mismatch",
432+
message: "The model is currently at capacity",
433+
},
434+
});
435+
return decision.kind === "retry";
436+
}
437+
438+
describe("ChatDirector live source-id tracking (CL-7973)", () => {
439+
test("a sourceless or empty-string completion never wipes the learned id", async () => {
440+
const director = createChatDirector("system", [], {
441+
onTasksChange: () => undefined,
442+
provider: { providerName: "test-provider" },
443+
});
444+
const capabilities = makeCapabilities();
445+
const policy = await liveRetryPolicy(director, capabilities);
446+
447+
// The seed id is not an xAI source, so the capacity error aborts.
448+
expect(await isXaiStamped(policy)).toBe(false);
449+
450+
// A completion stamps the source that served it.
451+
await director.decide(
452+
textCompletion("xai/learned"),
453+
mockState,
454+
capabilities,
455+
);
456+
expect(await isXaiStamped(policy)).toBe(true);
457+
458+
// A completion carrying no source keeps the learned id.
459+
await director.decide(textCompletion(), mockState, capabilities);
460+
expect(await isXaiStamped(policy)).toBe(true);
461+
462+
// An empty-string source id never clobbers the learned id.
463+
await director.decide(textCompletion(""), mockState, capabilities);
464+
expect(await isXaiStamped(policy)).toBe(true);
465+
466+
// An empty-string cycle source never clobbers it either.
467+
await director.decide(
468+
toolDoneEvent("empty-cycle"),
469+
stateWithCycleSource(""),
470+
capabilities,
471+
);
472+
expect(await isXaiStamped(policy)).toBe(true);
473+
});
474+
475+
test("a cycle source remaps tracking on a non-inference event", async () => {
476+
const director = createChatDirector("system", [], {
477+
onTasksChange: () => undefined,
478+
provider: { providerName: "test-provider" },
479+
});
480+
const capabilities = makeCapabilities();
481+
const policy = await liveRetryPolicy(director, capabilities);
482+
expect(await isXaiStamped(policy)).toBe(false);
483+
484+
// tool.done carries no source of its own; the harness's cycle source
485+
// covers the turn and remaps tracking from it.
486+
await director.decide(
487+
toolDoneEvent("cycle-remap"),
488+
stateWithCycleSource("xai/cycle"),
489+
capabilities,
490+
);
491+
expect(await isXaiStamped(policy)).toBe(true);
492+
});
493+
494+
test("a drained fleet capitulates to the terminal action after the nudge budget", async () => {
495+
const director = createChatDirector("system", [], {
496+
onTasksChange: () => undefined,
497+
provider: { providerName: "test-provider" },
498+
});
499+
director.restoreTasks([{ id: "t1", title: "keep going", status: "todo" }]);
500+
const capabilities = makeCapabilities();
501+
502+
// Without the idle-with-fleet allowance (drained fleet), a terminal base
503+
// action with open tasks re-infers with the open-task nudge a bounded
504+
// number of times, then lets the terminal action through — the accepted
505+
// loss stays locked in rather than resuming the nudge.
506+
for (let i = 0; i < 3; i++) {
507+
const actions = actionsArray(
508+
await director.decide(textCompletion(), mockState, capabilities),
509+
);
510+
expect(actions.some((a) => a.type === "infer")).toBe(true);
511+
expect(actions.some((a) => a.type === "reply")).toBe(false);
512+
}
513+
const terminal = actionsArray(
514+
await director.decide(textCompletion(), mockState, capabilities),
515+
);
516+
expect(terminal.some((a) => a.type === "infer")).toBe(false);
517+
expect(terminal.some((a) => a.type === "reply")).toBe(true);
518+
});
519+
520+
test("a cycle source wins over a contradictory event source", async () => {
521+
const director = createChatDirector("system", [], {
522+
onTasksChange: () => undefined,
523+
provider: { providerName: "test-provider" },
524+
});
525+
const capabilities = makeCapabilities();
526+
const policy = await liveRetryPolicy(director, capabilities);
527+
528+
// The harness's call-start snapshot is authoritative over the event's
529+
// own stamp, so when both are present the cycle source defines tracking.
530+
await director.decide(
531+
textCompletion("other/default"),
532+
stateWithCycleSource("xai/cycle"),
533+
capabilities,
534+
);
535+
expect(await isXaiStamped(policy)).toBe(true);
536+
});
537+
});

‎src/agent/director.ts‎

Lines changed: 77 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -434,19 +434,39 @@ export interface ChatDirectorOptions {
434434
requestContinuation?: (() => void) | undefined;
435435
provider?: { providerName: string; model?: string } | undefined;
436436
/**
437-
* Live catalog provider id for retry stamping. Resolved on each retry
438-
* decision so mid-session `/model` switches remapping without rebuilding
439-
* the agent. When set, preferred over static `provider.providerName`.
437+
* CL-7918 decisions (both former closures removed, no new env key):
438+
*
439+
* - getProviderId → reactor-supplied. The retry policy needs the *live*
440+
* source id per retry so mid-session /model switches remap retry stamping
441+
* (bare-429 xAI remap). A BaseEnv-derived id goes stale at the first
442+
* switch and only refreshes on rebuild; a static config id can never
443+
* remap. The reactor already learns the live id on every inference
444+
* completion, so it tracks currentSourceId itself (seeded from the session
445+
* providerName) and hands the policy a getter over it.
446+
* Accepted residual gap: retries during the single inference that first
447+
* uses a switched model still stamp the previous id — the director learns
448+
* the new id from that inference's completion event.
449+
*
450+
* - getLiveFleetCount → seeded config + live narrow setter
451+
* (setAllowIdleWithFleet below). The count is genuinely external (subagent
452+
* lane statuses the reactor never sees — its own tasks only carry
453+
* todo/doing/done/cancelled), so neither BaseEnv-derived nor
454+
* reactor-supplied can reproduce its liveness. Idle-with-fleet itself is
455+
* unchanged (fleet-running TUI sessions allow the terminal wait); the
456+
* fleet-wake publisher drives the setter on count transitions, so a
457+
* drained fleet resumes the open-task nudge.
440458
*/
441-
getProviderId?: (() => string | undefined) | undefined;
442459
/** Explicit retry policy; when set, skips the default Corbits policy. */
443460
retryPolicy?: RetryPolicy | undefined;
444461
/**
445-
* Live `status === "running"` fleet-lane count. When greater than zero the
446-
* director allows a terminal wait/reply with open tasks (idle-with-fleet).
447-
* Omitted or 0 keeps the open-task nudge. Exec omits this.
462+
* Initial idle-with-fleet allowance (CL-7918 replacement for the former
463+
* getLiveFleetCount closure). When true the director allows a terminal
464+
* wait/reply with open tasks; when omitted or false it keeps the open-task
465+
* nudge. The TUI seeds this (fleet lanes may appear mid-session); exec
466+
* omits it. The live fleet-wake publisher then keeps it current through
467+
* setAllowIdleWithFleet, so a drained fleet resumes the nudge.
448468
*/
449-
getLiveFleetCount?: (() => number) | undefined;
469+
allowIdleWithFleet?: boolean | undefined;
450470
}
451471

452472
// The constructor takes the resolved ModelFamilyPolicy rather than the raw
@@ -455,6 +475,8 @@ type ChatDirectorImplOptions = Omit<ChatDirectorOptions, "provider"> & {
455475
modelFamilyPolicy?: ModelFamilyPolicy | undefined;
456476
/** Provider-stamped retry policy (xAI short 429 remapping needs providerId). */
457477
retryPolicy?: RetryPolicy | undefined;
478+
/** Session-construction providerName: seeds currentSourceId pre-completion. */
479+
sessionProviderName?: string | undefined;
458480
};
459481

460482
class ChatDirectorImpl extends DefaultDirector {
@@ -489,7 +511,13 @@ class ChatDirectorImpl extends DefaultDirector {
489511
private readonly compaction: CompactionGovernor;
490512
private readonly modelFamilyPolicy: ModelFamilyPolicy;
491513
private readonly retryPolicy: RetryPolicy;
492-
private readonly getLiveFleetCount: (() => number) | undefined;
514+
// CL-7918: reactor-supplied live source id for retry stamping (replaces the
515+
// former getProviderId closure). Seeded from the session providerName and
516+
// refreshed on every inference completion, so mid-session /model switches
517+
// remap without rebuilding the agent.
518+
private currentSourceId: string | undefined;
519+
/** CL-7918 live replacement for the former getLiveFleetCount closure. */
520+
private allowIdleWithFleet: boolean;
493521
// Consecutive assistant turns that contain tool calls and no text. Reset on
494522
// any turn with text and on every fresh user message — a weak model that
495523
// spins in place on one thread of tool calls still converges to the
@@ -536,14 +564,28 @@ class ChatDirectorImpl extends DefaultDirector {
536564
toolDefinitions,
537565
);
538566
this.modelFamilyPolicy = familyPolicy;
539-
this.retryPolicy = options.retryPolicy ?? createCorbitsRetryPolicy();
540-
this.getLiveFleetCount = options.getLiveFleetCount;
567+
// CL-7918: the default policy stamps the live source id per retry decision
568+
// via a getter over currentSourceId (seeded from the session provider,
569+
// refreshed on each inference completion) — no host closure needed. An
570+
// explicit policy still skips this entirely.
571+
this.currentSourceId = options.sessionProviderName;
572+
this.retryPolicy =
573+
options.retryPolicy ??
574+
createCorbitsRetryPolicy({ providerId: () => this.currentSourceId });
575+
this.allowIdleWithFleet = options.allowIdleWithFleet === true;
541576
}
542577

543578
setWorkflowCoordinator(coordinator: WorkflowCoordinator | undefined): void {
544579
this.workflowCoordinator = coordinator;
545580
}
546581

582+
// Narrow live setter for the idle-with-fleet allowance (CL-7972): the
583+
// fleet-wake publisher drives this on fleet-count transitions, so a drained
584+
// fleet resumes the open-task nudge instead of holding the seeded value.
585+
setAllowIdleWithFleet(value: boolean): void {
586+
this.allowIdleWithFleet = value;
587+
}
588+
547589
updateToolDefinitions(toolDefinitions: ToolDefinition[]): void {
548590
const before = toolSetDigest(this._toolDefinitions);
549591
const after = toolSetDigest(toolDefinitions);
@@ -951,6 +993,16 @@ class ChatDirectorImpl extends DefaultDirector {
951993
// prefers provider usage when present.
952994
const turns = state.turns ?? [];
953995
this.compaction.syncFromTurns(turns);
996+
// CL-7918: reactor-supplied live source id (replaces getProviderId). The
997+
// completion stamps the source that served it, so a mid-session /model
998+
// switch remaps retry stamping from the next completion on; the harness's
999+
// lastCycleSource is the call-start snapshot and wins on conflict.
1000+
if (event.type === "inference.done") {
1001+
const served = event.source?.sourceId;
1002+
if (served !== undefined && served !== "") this.currentSourceId = served;
1003+
}
1004+
const cycled = state.lastCycleSource?.sourceId;
1005+
if (cycled !== undefined && cycled !== "") this.currentSourceId = cycled;
9541006
if (onTurnBoundary(event)) {
9551007
this.compaction.noteInferenceDone(event, turns);
9561008
}
@@ -1051,7 +1103,11 @@ class ChatDirectorImpl extends DefaultDirector {
10511103
(a) => a.type === "wait" || a.type === "reply",
10521104
);
10531105
if (hasTerminal) {
1054-
if ((this.getLiveFleetCount?.() ?? 0) > 0) {
1106+
// CL-7918 live idle-with-fleet allowance (replaces the former
1107+
// getLiveFleetCount closure): seeded at construction, then kept
1108+
// current by the fleet-wake publisher. TUI seeds true (fleet lanes
1109+
// may appear mid-session); exec omits it and keeps the nudge.
1110+
if (this.allowIdleWithFleet) {
10551111
return base;
10561112
}
10571113
if (this.idleTerminationNudges < MAX_OPEN_TASK_NUDGES) {
@@ -1086,25 +1142,21 @@ export function createChatDirector(
10861142
toolDefinitions: ToolDefinition[],
10871143
options: ChatDirectorOptions,
10881144
): ChatDirector {
1089-
const { provider, getProviderId, retryPolicy, ...rest } = options;
1145+
const { provider, retryPolicy, ...rest } = options;
10901146
return new ChatDirectorImpl(systemPrompt, toolDefinitions, {
10911147
...rest,
10921148
// `provider` is raw {providerName, model} input; the constructor wants
10931149
// the resolved ModelFamilyPolicy, not the input it was resolved from.
10941150
modelFamilyPolicy:
10951151
provider !== undefined ? resolveModelFamilyPolicy(provider) : undefined,
1152+
// CL-7918: seed the reactor-tracked live source id (replaces
1153+
// getProviderId). The impl refreshes it on every inference completion so
1154+
// mid-session `/model` switches remap retry stamping.
1155+
sessionProviderName: provider?.providerName,
10961156
// Stamp provider id onto retry errors so known-xAI short 429s remap.
1097-
// Prefer an explicit policy, then a live getter (mid-session `/model`),
1098-
// then the bootstrap providerName.
1099-
retryPolicy:
1100-
retryPolicy ??
1101-
createCorbitsRetryPolicy(
1102-
getProviderId !== undefined
1103-
? { providerId: getProviderId }
1104-
: provider !== undefined
1105-
? { providerId: provider.providerName }
1106-
: undefined,
1107-
),
1157+
// Prefer an explicit policy; otherwise the impl builds the default policy
1158+
// over its live source-id tracker.
1159+
retryPolicy,
11081160
});
11091161
}
11101162

@@ -1129,6 +1181,7 @@ export function hydrateTasksFromTurns(turns: ConversationTurn[]): Task[] {
11291181
export interface ChatDirector extends ReactorDirector {
11301182
updateToolDefinitions(toolDefinitions: ToolDefinition[]): void;
11311183
setWorkflowCoordinator(coordinator: WorkflowCoordinator | undefined): void;
1184+
setAllowIdleWithFleet(value: boolean): void;
11321185
getTasks(): Task[];
11331186
restoreTasks(tasks: Task[]): void;
11341187
getContextEstimate(): { tokens: number; isEstimate: boolean };

0 commit comments

Comments
 (0)