Skip to content

Commit 1e0783e

Browse files
committed
Merge remote-tracking branch 'origin/main' into cl-7919-chat-director-decide-the-shape-for-workflowcoordinator-and
2 parents 522a0a8 + 776cd01 commit 1e0783e

27 files changed

Lines changed: 1029 additions & 371 deletions

‎docs/ARCHITECTURE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ When a cycle's input tokens cross a threshold, the director compacts the inferen
177177
- **Idle (end-of-turn)** — An interactive turn can end with a reply and then sit idle with no tool batch to intercept; the governor requests a continuation at that pause and compacts when it arrives. An operator message that races the continuation still compacts first, then re-enters inference to answer it.
178178
- **Overflow recovery** — A `context_overflow` inference error would otherwise become a terminal error reply; the governor compacts and retries instead, bounded so a history the compactor cannot shrink does not loop forever. Overflow ignores hysteresis for the compact itself.
179179

180-
The compaction control flow is shaped by a reactor invariant: a `compact` action runs in its own cycle (it cannot be paired with `infer`), and **the reactor delivers no event after a compact cycle**. A director that simply emitted `compact` in place of the follow-up `infer` would leave the loop idle forever — the cause of an earlier stall. Instead the governor, after emitting `compact`, self-delivers a content-less inbound message (a host-supplied `requestContinuation` callback). That message adds no turn (`createInboundTurn` returns `null` for empty content) but re-enters the loop, where the director issues the follow-up `infer` against the freshly truncated history.
180+
The compaction control flow is shaped by a reactor invariant: a `compact` action runs in its own cycle (it cannot be paired with `infer`), and **the reactor delivers no event after a compact cycle**. A director that simply emitted `compact` in place of the follow-up `infer` would leave the loop idle forever — the cause of an earlier stall. Instead the governor pairs `compact` with an emit action (`custom.compaction.continue`), and the host answers that emission by delivering a content-less inbound message (`buildCompactionContinuationMessage()`) through the serial operation queue, generation-guarded like every other deliver. That message adds no turn (`createInboundTurn` returns `null` for empty content) but re-enters the loop, where the director issues the follow-up `infer` against the freshly truncated history. Each emission is answered at most once (a replayed duplicate of an already-answered emission is ignored), and an unsolicited continuation with neither resume flag set answers `wait` rather than burning a billable inference. The legacy `requestContinuation` closure survives only on the sub-agent path, which delivers directly with no host emit hop; idle arming keeps the two channels exclusive (closure fires and the arming flag stays false, or no closure and the flag reports the arming) so a caller honoring both cannot double-deliver.
181181

182182
Compaction replaces older turns with a structured, workflow-aware summary rather than a stats blob: sections for **What Happened / What We're Doing / Relevant Links / Action Items / Next Steps**, with the active workflow and step woven in so compacting mid-`/build` or mid-`/plan` preserves the contract. The summary is produced by a one-shot model call; on any failure it falls back to a deterministic summary so a compaction cycle never breaks the session.
183183

‎scripts/check-dead-exports.test.ts‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,12 @@ import {
2828
const repoRoot = join(import.meta.dir, "..");
2929

3030
// A probe dead export in one of the scoped files must fail the guard: the
31-
// exact-name exemptions cover only the five deferred-cleanup flags, never
32-
// the whole module.
31+
// exact-name exemptions cover only the remaining deferred-cleanup flags
32+
// (the usage-formatter flags were removed with their exports by the CL-6815
33+
// trim), never the whole module.
3334
describe("scoped exemptions", () => {
3435
test("the real allowlist covers the named flags but not a sibling probe", () => {
3536
const rules = loadAllowlist();
36-
expect(
37-
isAllowlisted(rules, "src/auth/codex/usage.ts", "fetchCodexUsage"),
38-
).toBe(true);
39-
expect(
40-
isAllowlisted(rules, "src/auth/codex/usage.ts", "fetchCodexModels"),
41-
).toBe(true);
42-
expect(isAllowlisted(rules, "src/auth/xai/usage.ts", "fetchXaiUsage")).toBe(
43-
true,
44-
);
4537
expect(
4638
isAllowlisted(
4739
rules,

‎scripts/dead-export-allowlist.txt‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,21 @@ vendor/
2626
# flags so a new dead export in these files fails the guard instead of
2727
# hiding under a whole-file exemption. Remove each entry with the export it
2828
# names when the owning cleanup lands.
29-
src/auth/codex/usage.ts: fetchCodexUsage
30-
src/auth/codex/usage.ts: fetchCodexModels
31-
src/auth/xai/usage.ts: fetchXaiUsage
3229
src/auth/codex/constants.ts: CODEX_REFRESH_SKEW_MS
3330
src/auth/codex/constants.ts: CODEX_HEADLESS_REFRESH_INTERVAL_MS
3431

3532
# Plugin fixture entry point. Loaded by file path from the fixture's plugin
3633
# manifest by the plugin-registration tests, so it has no static importers.
3734
tests/fixtures/plugins/implement-feature/src/index.ts
3835

36+
# Eval completion-harness task fixtures (CL-7932 lane). Loaded by file path
37+
# from each task's script.json by the harness runner, so they have no static
38+
# importers.
39+
evals/completion/tasks/decline-refactor/fixture/notes.ts: addNote
40+
evals/completion/tasks/stall-read/fixture/data.ts: data
41+
evals/completion/tasks/sum-fix/fixture/sum.ts: total
42+
evals/completion/tasks/version-endpoint/fixture/service.ts: handleRequest
43+
3944
# ts-prune parser false positives: bare tokens on `as const satisfies ...`
4045
# lines inside live exports. None of these is an export declaration.
4146
src/provider/reasoning-effort.ts: satisfies
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import { describe, expect, test } from "bun:test";
2+
import {
3+
CHAT_TASKS_CHANGED_EVENT,
4+
CHAT_TOOLS_ACTIVATE_EVENT,
5+
} from "./director.js";
6+
import { handleChatDirectorEvent } from "./chat-event-subscribers.js";
7+
import type { Task } from "./tasks.js";
8+
9+
function makeLog() {
10+
const calls: { message: string; fields?: Record<string, unknown> }[] = [];
11+
return {
12+
calls,
13+
log: (message: string, fields?: Record<string, unknown>): void => {
14+
calls.push(fields !== undefined ? { message, fields } : { message });
15+
},
16+
};
17+
}
18+
19+
describe("handleChatDirectorEvent", () => {
20+
test("dispatches a valid tasks-changed payload without logging", () => {
21+
const seen: Task[][] = [];
22+
const log = makeLog();
23+
const handled = handleChatDirectorEvent(
24+
{
25+
type: CHAT_TASKS_CHANGED_EVENT,
26+
data: { tasks: [{ id: "t1", title: "work", status: "doing" }] },
27+
},
28+
{
29+
onTasksChanged: (tasks) => seen.push(tasks),
30+
onToolsActivate: () => {
31+
throw new Error("unexpected tools-activate dispatch");
32+
},
33+
},
34+
log.log,
35+
);
36+
expect(handled).toBe(true);
37+
expect(seen).toEqual([[{ id: "t1", title: "work", status: "doing" }]]);
38+
expect(log.calls).toEqual([]);
39+
});
40+
41+
test("dispatches a valid tools-activate payload without logging", () => {
42+
const seen: string[][] = [];
43+
const log = makeLog();
44+
const handled = handleChatDirectorEvent(
45+
{ type: CHAT_TOOLS_ACTIVATE_EVENT, data: { names: ["lsp"] } },
46+
{
47+
onTasksChanged: () => {
48+
throw new Error("unexpected tasks-changed dispatch");
49+
},
50+
onToolsActivate: (names) => seen.push([...names]),
51+
},
52+
log.log,
53+
);
54+
expect(handled).toBe(true);
55+
expect(seen).toEqual([["lsp"]]);
56+
expect(log.calls).toEqual([]);
57+
});
58+
59+
test("drops an invalid tasks payload with a debug log naming the failure", () => {
60+
let dispatched = false;
61+
const log = makeLog();
62+
const handled = handleChatDirectorEvent(
63+
{ type: CHAT_TASKS_CHANGED_EVENT, data: { tasks: "not-a-list" } },
64+
{
65+
onTasksChanged: () => {
66+
dispatched = true;
67+
},
68+
onToolsActivate: () => {
69+
dispatched = true;
70+
},
71+
},
72+
log.log,
73+
);
74+
expect(handled).toBe(true);
75+
expect(dispatched).toBe(false);
76+
expect(log.calls).toHaveLength(1);
77+
expect(log.calls[0]?.message).toMatch(/tasks-changed/);
78+
expect(typeof log.calls[0]?.fields?.["error"]).toBe("string");
79+
});
80+
81+
test("drops an invalid tools payload with a debug log naming the failure", () => {
82+
let dispatched = false;
83+
const log = makeLog();
84+
const handled = handleChatDirectorEvent(
85+
{ type: CHAT_TOOLS_ACTIVATE_EVENT, data: { names: [42] } },
86+
{
87+
onTasksChanged: () => {
88+
dispatched = true;
89+
},
90+
onToolsActivate: () => {
91+
dispatched = true;
92+
},
93+
},
94+
log.log,
95+
);
96+
expect(handled).toBe(true);
97+
expect(dispatched).toBe(false);
98+
expect(log.calls).toHaveLength(1);
99+
expect(log.calls[0]?.message).toMatch(/tools-activate/);
100+
expect(typeof log.calls[0]?.fields?.["error"]).toBe("string");
101+
});
102+
103+
test("ignores unrelated events without logging or dispatching", () => {
104+
let dispatched = false;
105+
const log = makeLog();
106+
const handled = handleChatDirectorEvent(
107+
{ type: "inference.done", data: {} },
108+
{
109+
onTasksChanged: () => {
110+
dispatched = true;
111+
},
112+
onToolsActivate: () => {
113+
dispatched = true;
114+
},
115+
},
116+
log.log,
117+
);
118+
expect(handled).toBe(false);
119+
expect(dispatched).toBe(false);
120+
expect(log.calls).toEqual([]);
121+
});
122+
});
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/**
2+
* Shared subscriber for the chat-director reactor events. The TUI and exec
3+
* stream sinks both listen for the task-list and tool-activation events the
4+
* chat director emits in place of the former host closures; the parse,
5+
* validation, and invalid-payload handling live here so the two sinks cannot
6+
* drift apart.
7+
*/
8+
9+
import { type } from "arktype";
10+
import {
11+
CHAT_TASKS_CHANGED_EVENT,
12+
CHAT_TOOLS_ACTIVATE_EVENT,
13+
ChatTasksChangedDataSchema,
14+
ChatToolsActivateDataSchema,
15+
} from "./director.js";
16+
import type { Task } from "./tasks.js";
17+
18+
export interface ChatDirectorEventHandlers {
19+
onTasksChanged: (tasks: Task[]) => void;
20+
onToolsActivate: (names: string[]) => void;
21+
}
22+
23+
export type ChatDirectorEventDebugLog = (
24+
message: string,
25+
fields?: Record<string, unknown>,
26+
) => void;
27+
28+
/**
29+
* Dispatch one stream event to the chat-director handlers. Returns true when
30+
* the event is a chat-director event (valid or not) so sinks can fall through
31+
* to their own handling otherwise. Invalid payloads are dropped after a
32+
* debug-level log naming the failure — never silently.
33+
*/
34+
export function handleChatDirectorEvent(
35+
event: { type: string; data: unknown },
36+
handlers: ChatDirectorEventHandlers,
37+
logDebug: ChatDirectorEventDebugLog,
38+
): boolean {
39+
if (event.type === CHAT_TASKS_CHANGED_EVENT) {
40+
const parsed = ChatTasksChangedDataSchema(event.data);
41+
if (parsed instanceof type.errors) {
42+
logDebug("chat tasks-changed event dropped invalid payload: {error}", {
43+
error: parsed.summary,
44+
});
45+
return true;
46+
}
47+
handlers.onTasksChanged(parsed.tasks);
48+
return true;
49+
}
50+
if (event.type === CHAT_TOOLS_ACTIVATE_EVENT) {
51+
const parsed = ChatToolsActivateDataSchema(event.data);
52+
if (parsed instanceof type.errors) {
53+
logDebug("chat tools-activate event dropped invalid payload: {error}", {
54+
error: parsed.summary,
55+
});
56+
return true;
57+
}
58+
handlers.onToolsActivate(parsed.names);
59+
return true;
60+
}
61+
return false;
62+
}

‎src/agent/compaction.test.ts‎

Lines changed: 89 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import type {
66
ReactorInboundEvent,
77
TokenUsage,
88
} from "@intx/types/runtime";
9-
import { createCompactionGovernor } from "./compaction.js";
9+
import {
10+
COMPACTION_CONTINUATION_EVENT,
11+
createCompactionGovernor,
12+
} from "./compaction.js";
1013
import {
1114
compactionResumeDeltaFor,
1215
compactionThresholdFor,
@@ -28,6 +31,11 @@ const capabilities = {
2831
compactor,
2932
reason,
3033
}),
34+
emit: (eventType: string, data: unknown) => ({
35+
type: "emit",
36+
eventType,
37+
data,
38+
}),
3139
} as unknown as ReactorCapabilities;
3240

3341
// Distinct, non-zero cacheRead/cacheWrite so a test asserting on the total
@@ -182,15 +190,34 @@ describe("compaction governor", () => {
182190
).toBeNull();
183191
});
184192

185-
test("stays inert without a continuation channel", () => {
193+
test("expresses continuation as an emit action without a closure", () => {
186194
const governor = createCompactionGovernor(undefined);
187195
governor.noteInferenceDone(inferenceDone(overThreshold), tenTurns);
196+
const actions = governor.interceptActions(
197+
toolDone(),
198+
inferAction,
199+
capabilities,
200+
);
201+
expect(actions?.some((a) => a.type === "compact")).toBe(true);
202+
expect(actions?.some((a) => a.type === "infer")).toBe(false);
188203
expect(
189-
governor.interceptActions(toolDone(), inferAction, capabilities),
190-
).toBeNull();
191-
expect(
192-
governor.interceptOverflow(overflowError(), capabilities),
193-
).toBeNull();
204+
actions?.some(
205+
(a) =>
206+
a.type === "emit" &&
207+
"eventType" in a &&
208+
a.eventType === COMPACTION_CONTINUATION_EVENT,
209+
),
210+
).toBe(true);
211+
expect(
212+
governor
213+
.interceptOverflow(overflowError(), capabilities)
214+
?.some(
215+
(a) =>
216+
a.type === "emit" &&
217+
"eventType" in a &&
218+
a.eventType === COMPACTION_CONTINUATION_EVENT,
219+
),
220+
).toBe(true);
194221
});
195222

196223
test("recovers from context overflow a bounded number of times", () => {
@@ -241,6 +268,61 @@ describe("compaction governor", () => {
241268
).toBeNull();
242269
});
243270

271+
test("an idle over-threshold turn arms an emit continuation without a closure", () => {
272+
const governor = createCompactionGovernor(undefined);
273+
governor.noteInferenceDone(inferenceDone(overThreshold), tenTurns);
274+
275+
const terminal: ReactorAction[] = [{ type: "reply", content: "done" }];
276+
expect(governor.noteIdleTurn(inferenceDone(overThreshold), terminal)).toBe(
277+
true,
278+
);
279+
// Only arms once even if the idle turn is observed again.
280+
expect(governor.noteIdleTurn(inferenceDone(overThreshold), terminal)).toBe(
281+
false,
282+
);
283+
284+
const actions = governor.interceptIdleContinuation(
285+
emptyMessage(),
286+
capabilities,
287+
);
288+
expect(
289+
actions?.some(
290+
(a) =>
291+
a.type === "compact" &&
292+
"reason" in a &&
293+
a.reason === "context-threshold",
294+
),
295+
).toBe(true);
296+
expect(
297+
actions?.some(
298+
(a) =>
299+
a.type === "emit" &&
300+
"eventType" in a &&
301+
a.eventType === COMPACTION_CONTINUATION_EVENT,
302+
),
303+
).toBe(true);
304+
});
305+
306+
test("idle arming with a closure fires once and never returns true", () => {
307+
// Single-delivery contract: a caller that both installs the legacy closure
308+
// and honors the boolean (appending an emit action on true) must still
309+
// deliver exactly once. The closure fires; the return stays false.
310+
let continuations = 0;
311+
const governor = createCompactionGovernor(() => continuations++);
312+
governor.noteInferenceDone(inferenceDone(overThreshold), tenTurns);
313+
314+
const terminal: ReactorAction[] = [{ type: "reply", content: "done" }];
315+
expect(governor.noteIdleTurn(inferenceDone(overThreshold), terminal)).toBe(
316+
false,
317+
);
318+
expect(continuations).toBe(1);
319+
// A repeated idle turn neither refires nor arms.
320+
expect(governor.noteIdleTurn(inferenceDone(overThreshold), terminal)).toBe(
321+
false,
322+
);
323+
expect(continuations).toBe(1);
324+
});
325+
244326
// Idle compact with an empty continuation previously left postCompactInfer
245327
// unset, so resumeAfterCompact never fired and notePostCompact never ran —
246328
// the Ctx meter stayed on pre-compact lastTurnUsage until the next user turn.

0 commit comments

Comments
 (0)