Skip to content

Commit c8cd470

Browse files
committed
Trim the reactor-events doc and cover real event unions
The ARCHITECTURE.md block explaining inference.done vs reactor.done carried a full rejected-alternatives essay on ESLint and Biome, which reads as noise once someone actually configures a linter. Cut it to the table row, one paragraph naming the distinction, and the note that this is a convention rather than an enforced constraint. The guard tests previously passed bare { type: string } literals, which only proves the string comparison works. Drive real ReactorInboundEvent and ReactorEmittedEvent members through onTurnBoundary and onReactorShutdown instead, so a change that breaks narrowing on either union is caught. Add an integration test that exercises onTurnBoundary against a real reactor run's emitted events; onReactorShutdown's shutdown path is not integration-testable because agent.close() clears stream() consumers before the queued abort event produces reactor.done, so app code can't observe it there either - the unit test covers that guard's correctness instead.
1 parent 920041c commit c8cd470

3 files changed

Lines changed: 148 additions & 39 deletions

File tree

docs/ARCHITECTURE.md

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,27 +24,14 @@ This repeats until the director emits `capabilities.done()`.
2424

2525
`inference.done` and `reactor.done` read as near-synonyms at a call site but
2626
answer different questions: "did a turn end" versus "did the reactor shut
27-
down." Three shipped defects came from code that needed a turn boundary but
28-
keyed off `reactor.done` instead: queued messages never dispatched because
29-
the send-queue drain waited for shutdown; `run.json`'s `turnsUsed` froze for
30-
an entire session because the mid-run snapshot only re-fired on shutdown;
31-
and the shell run state didn't return to idle between turns. Documentation
32-
didn't prevent the second and third instances, so code that needs to ask
33-
"did a turn end" or "did the reactor shut down" should go through the
34-
`onTurnBoundary` / `onReactorShutdown` guards in `src/agent/reactor-events.ts`
35-
rather than comparing `event.type` to a string directly — naming the
36-
question makes the right thing easier to write than the wrong one.
27+
down." Code that needs either answer should go through the `onTurnBoundary`
28+
/ `onReactorShutdown` guards in `src/agent/reactor-events.ts` rather than
29+
comparing `event.type` to a string directly — naming the question makes the
30+
right thing easier to write than the wrong one.
3731

3832
This is a convention, not an enforced constraint: nothing stops a future
3933
call site from writing `event.type === "reactor.done"` directly instead of
40-
reaching for the guard. Two enforcement routes were considered and both are
41-
out of scope here — a lint rule (`no-restricted-syntax` or similar) would
42-
mean standing up ESLint or Biome from scratch, since neither is configured
43-
anywhere in this repo, disproportionate for a Low-priority cleanup; and a
44-
type-level fix branding `event.type` would require modifying `@intx/types`
45-
or `@intx/inference`, which are vendored and off-limits. Reviewers should
46-
treat a bare `event.type === "reactor.done"` / `"inference.done"` comparison
47-
outside `reactor-events.ts` as a signal to ask why the guard wasn't used.
34+
reaching for the guard.
4835

4936
### ReactorActions
5037

src/agent/reactor-events.test.ts

Lines changed: 91 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,115 @@
11
import { describe, expect, test } from "bun:test";
2+
import type { ReactorEmittedEvent } from "@intx/inference";
3+
import type { ReactorInboundEvent } from "@intx/types/runtime";
24
import { onReactorShutdown, onTurnBoundary } from "./reactor-events.js";
35

6+
// Bare `{ type: string }` literals only prove the string comparison works.
7+
// The generic exists so the guards narrow across both `ReactorInboundEvent`
8+
// (the director-facing union, `src/agent/director.ts` / `compaction.ts`) and
9+
// `ReactorEmittedEvent` (the stream-facing union consumers see) without
10+
// redeclaring either union in `reactor-events.ts`. These tests drive real
11+
// members of both unions through the guards so a future change that breaks
12+
// narrowing on either union — e.g. a renamed variant, or the guard's
13+
// signature drifting to accept only one union — fails here instead of
14+
// surfacing as a silent `never` match downstream.
15+
16+
// `reactor.done` is emitted-only: it does not exist on `ReactorInboundEvent`
17+
// at all, so `onReactorShutdown` narrows to `never` for every director-side
18+
// event. That is exactly the distinction the doc and the guard both draw
19+
// ("did a turn end" is a question directors ask; "did the reactor shut
20+
// down" is not), and it is a fact the integration harness cannot exercise
21+
// on its own — the agent stream never hands a `ReactorInboundEvent` to
22+
// application code, only `ReactorEmittedEvent`.
23+
const inboundEvents: ReactorInboundEvent[] = [
24+
{
25+
type: "message.received",
26+
message: { role: "user", content: [{ type: "text", text: "hi" }] },
27+
} as unknown as ReactorInboundEvent,
28+
{
29+
type: "inference.done",
30+
turn: {},
31+
usage: {},
32+
source: {},
33+
} as unknown as ReactorInboundEvent,
34+
{
35+
type: "inference.error",
36+
error: {},
37+
partial: {},
38+
} as unknown as ReactorInboundEvent,
39+
{ type: "tool.done", result: {} } as unknown as ReactorInboundEvent,
40+
{
41+
type: "reactor.gate.cleared",
42+
gateId: "g1",
43+
reason: "resolved",
44+
} as unknown as ReactorInboundEvent,
45+
{ type: "abort", reason: {} } as unknown as ReactorInboundEvent,
46+
];
47+
48+
const emittedEvents: ReactorEmittedEvent[] = [
49+
{
50+
type: "message.received",
51+
seq: 0,
52+
data: { message: { role: "user", content: [{ type: "text", text: "hi" }] } },
53+
} as unknown as ReactorEmittedEvent,
54+
{ type: "inference.done", data: {} } as unknown as ReactorEmittedEvent,
55+
{ type: "reactor.done", data: {} } as unknown as ReactorEmittedEvent,
56+
{ type: "tool.done", data: {} } as unknown as ReactorEmittedEvent,
57+
];
58+
459
describe("onTurnBoundary", () => {
5-
test("true only for inference.done", () => {
6-
expect(onTurnBoundary({ type: "inference.done" })).toBe(true);
7-
expect(onTurnBoundary({ type: "reactor.done" })).toBe(false);
8-
expect(onTurnBoundary({ type: "tool.done" })).toBe(false);
60+
test("narrows ReactorInboundEvent to exactly the inference.done member", () => {
61+
const matches = inboundEvents.filter(onTurnBoundary);
62+
expect(matches.map((e) => e.type)).toEqual(["inference.done"]);
63+
// Type-level: narrowing must land on the real union member, with its
64+
// real fields, not an unrelated shape — this line fails to compile if
65+
// onTurnBoundary stops narrowing E correctly.
66+
const [narrowed] = matches;
67+
const _turn: unknown = narrowed?.turn;
68+
void _turn;
69+
});
70+
71+
test("narrows ReactorEmittedEvent to exactly the inference.done member", () => {
72+
const matches = emittedEvents.filter(onTurnBoundary);
73+
expect(matches.map((e) => e.type)).toEqual(["inference.done"]);
974
});
1075

1176
// The property all three shipped defects violated: code that gated a
1277
// turn boundary on `reactor.done` only ever saw it once, at shutdown.
1378
// A multi-turn session must trip this guard once per turn.
14-
test("fires more than once across a multi-turn session", () => {
15-
const turnEvents = [
16-
{ type: "inference.start" },
17-
{ type: "inference.done" },
18-
{ type: "tool.done" },
19-
{ type: "inference.done" },
20-
{ type: "inference.done" },
79+
test("fires more than once across a multi-turn stream of real events", () => {
80+
const turnEvents: ReactorEmittedEvent[] = [
81+
{ type: "inference.start", data: {} } as unknown as ReactorEmittedEvent,
82+
{ type: "inference.done", data: {} } as unknown as ReactorEmittedEvent,
83+
{ type: "tool.done", data: {} } as unknown as ReactorEmittedEvent,
84+
{ type: "inference.done", data: {} } as unknown as ReactorEmittedEvent,
85+
{ type: "inference.done", data: {} } as unknown as ReactorEmittedEvent,
2186
];
2287

23-
const boundaries = turnEvents.filter((event) => onTurnBoundary(event));
88+
const boundaries = turnEvents.filter(onTurnBoundary);
2489

2590
expect(boundaries.length).toBe(3);
2691
expect(boundaries.length).toBeGreaterThan(1);
2792
});
2893
});
2994

3095
describe("onReactorShutdown", () => {
31-
test("true only for reactor.done, and fires once per session", () => {
32-
const sessionEvents = [
33-
{ type: "inference.done" },
34-
{ type: "inference.done" },
35-
{ type: "inference.done" },
36-
{ type: "reactor.done" },
96+
test("never matches any ReactorInboundEvent member — reactor.done is emitted-only", () => {
97+
const matches = inboundEvents.filter(onReactorShutdown);
98+
expect(matches).toEqual([]);
99+
});
100+
101+
test("narrows ReactorEmittedEvent to exactly the reactor.done member, once per session", () => {
102+
const sessionEvents: ReactorEmittedEvent[] = [
103+
{ type: "inference.done", data: {} } as unknown as ReactorEmittedEvent,
104+
{ type: "inference.done", data: {} } as unknown as ReactorEmittedEvent,
105+
{ type: "inference.done", data: {} } as unknown as ReactorEmittedEvent,
106+
{ type: "reactor.done", data: {} } as unknown as ReactorEmittedEvent,
37107
];
38108

39-
expect(onReactorShutdown({ type: "reactor.done" })).toBe(true);
40-
expect(onReactorShutdown({ type: "inference.done" })).toBe(false);
109+
const matches = emittedEvents.filter(onReactorShutdown);
110+
expect(matches.map((e) => e.type)).toEqual(["reactor.done"]);
41111

42-
const shutdowns = sessionEvents.filter((event) => onReactorShutdown(event));
112+
const shutdowns = sessionEvents.filter(onReactorShutdown);
43113
expect(shutdowns.length).toBe(1);
44114
});
45115
});
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { describe, expect, test } from "bun:test";
2+
import type { ReactorEmittedEvent } from "@intx/inference";
3+
4+
import { onTurnBoundary } from "../../src/agent/reactor-events.js";
5+
import { createPermissionGate } from "../../src/permission/gate.js";
6+
import { closeIntegrationSession, openIntegrationSession, runUntilDone } from "./harness.js";
7+
8+
// The unit tests in `src/agent/reactor-events.test.ts` cover type-level
9+
// narrowing across both event unions and `onReactorShutdown`'s behavior.
10+
// This test asserts the property `onTurnBoundary` exists for — it matches
11+
// exactly once per turn — against a real reactor's real emitted events,
12+
// not a synthetic filtered array of hand-built literals.
13+
//
14+
// `onReactorShutdown` is not exercised here: `@intx/agent`'s `close()`
15+
// clears `stream()` consumers synchronously, before the queued abort that
16+
// produces `reactor.done` is processed, so application code attached via
17+
// `agent.stream()` cannot observe it after `close()` — the same reason
18+
// `src/session/run-sink.ts` snapshots status *before* close instead of
19+
// relying on `reactor.done` to arrive. That gap is covered by the unit
20+
// test's real `ReactorEmittedEvent` / `ReactorInboundEvent` narrowing.
21+
describe("integration — reactor-events guards", () => {
22+
test.serial("onTurnBoundary matches exactly the turn boundary, once per real turn", async () => {
23+
const session = await openIntegrationSession({
24+
permissionGate: createPermissionGate({
25+
approvals: [],
26+
interactive: false,
27+
skipPermissions: true,
28+
}),
29+
});
30+
31+
try {
32+
session.harness.scenario.replyOnce("anthropic", {
33+
toolCalls: [{ name: "write_file", args: { path: "out.txt", content: "ok\n" } }],
34+
});
35+
session.harness.scenario.replyOnce("anthropic", { text: "Done." });
36+
37+
const { events } = await runUntilDone(session, "Write out.txt with content ok.");
38+
39+
const turnBoundaries = events.filter(onTurnBoundary);
40+
41+
// One tool-call turn followed by one final-text turn: exactly two
42+
// inference.done events, despite tool.done and other events on the stream.
43+
expect(turnBoundaries.length).toBe(2);
44+
expect(turnBoundaries.every((e: ReactorEmittedEvent) => e.type === "inference.done")).toBe(
45+
true,
46+
);
47+
expect(events.some((e) => e.type === "tool.done")).toBe(true);
48+
} finally {
49+
await closeIntegrationSession(session);
50+
}
51+
});
52+
});

0 commit comments

Comments
 (0)