Skip to content

Commit 01d810a

Browse files
refactor(director): surface task and tool changes as reactor events (#1042)
* feat(chat): replace onTasksChange/onActivateTools with reactor events Hosts subscribe to custom.chat.tasks.changed and custom.chat.tools.activate on the agent stream; TUI repaints the task panel, exec activates tools. Fixes CL-7916 * refactor(chat): share reactor event-subscriber helper across sinks (#1054) Extract the duplicated tasks-changed/tools-activate subscriber blocks into handleChatDirectorEvent, log invalid payloads at debug level instead of dropping them silently, and clear queued task-change notifications when a turn throws. * chore(deadcode): drop stale usage allowlist, cover eval fixtures fetchCodexUsage/fetchCodexModels/fetchXaiUsage deleted by #1031; four completion-harness fixtures loaded by path from script.json. * chore(deadcode): update keeper test for post-trim allowlist The CL-6815 trim removed the three usage-formatter exports; the scoped exemption keeper now pins the two remaining constants flags plus the negative sibling probe.
1 parent 6365906 commit 01d810a

20 files changed

Lines changed: 461 additions & 195 deletions

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+
}

0 commit comments

Comments
 (0)