Skip to content

Commit 1a2bbb2

Browse files
bobbai00claude
andcommitted
refactor(agent-service): reconcile rebased stack with origin/main
Reconciliation for rebasing the agent-service-execution-model stack onto origin/main, which had independently merged #5751 (extract WS types into types/ws as classes) and #5930 (remove the dead checkout/headChange path). Our stack redesigns the same WS layer as interface discriminated unions, so the two diverge structurally. Changes: - Restore the execution-model redesign in types/execution.ts: the 3-way merge re-appended main's #5751 flat `OperatorResultSummary` (which references the removed `PortShape` and duplicates our redesigned type), so drop it; our redesign's `OperatorResultSummary`/`OperatorExecutionSummary` supersede it. - Adopt #5751 in server.ts: the TexeraAgent websocket surface was renamed `websockets` -> `clients` (getClients/addClient/removeClient) by the auto-merge in texera-agent.ts; align the server call sites, restore the `_getAgentForTests` test accessor, and reject unknown ws message types with an error frame (matching main's handler) instead of silently ignoring them. - Adopt #5930: drop the checkout/headChange dead path that our stack still carried — the POST /:id/checkout route + headChange broadcast and the startup-banner token in server.ts, `WsServerHeadChangeMessage` (and its now orphaned WorkflowContent import) in types/ws/server.ts, and the `case "headChange"` handler + `checkoutStep()` in the frontend agent.service. (TexeraAgent.checkout() was already removed by the auto-merge.) - Reconcile tests to the new discriminated-union ws protocol: rewrite server.ws.spec.ts (init/step/state/complete/error frames; prompt/command client frames; operatorResults streamed on init/complete), update the server.spec.ts operator-results test to the OperatorExecutionSummary shape, and fix the frontend stopGeneration assertion to the {type:"command", commandType:"stop"} wire format. agent-service: tsc --noEmit clean, 143/143 bun test pass, prettier clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2e572ed commit 1a2bbb2

7 files changed

Lines changed: 75 additions & 153 deletions

File tree

agent-service/src/server.spec.ts

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -323,34 +323,33 @@ describe("agent read routes", () => {
323323

324324
test("GET /:id/operator-results maps the visible operator results", async () => {
325325
const agent = _getAgentForTests(id)!;
326+
// The route returns each operator's OperatorExecutionSummary verbatim.
326327
(agent as any).getWorkflowResultState = () => ({
327328
getAllVisible: () =>
328329
new Map([
329330
[
330331
"op-1",
331332
{
332333
operatorInfo: {
333-
state: "COMPLETED",
334-
inputTuples: 1,
335-
outputTuples: 2,
336-
inputPortShapes: [],
337-
result: [{ a: 1 }],
338-
error: undefined,
339-
warnings: [],
340-
consoleLogs: [],
341-
totalRowCount: 2,
342-
resultStatistics: {},
334+
state: "Completed",
335+
errorMessages: [],
336+
resultSummary: {
337+
resultMode: "table",
338+
sampleTuples: [{ a: 1 }],
339+
totalRowCount: 2,
340+
},
343341
},
344342
},
345343
],
346344
]),
347345
});
348346

349-
const body = await readJson<{ results: Record<string, { outputTuples: number; outputColumns: number }> }>(
350-
await getJson(`${API}/agents/${id}/operator-results`)
351-
);
352-
expect(body.results["op-1"].outputTuples).toBe(2);
353-
expect(body.results["op-1"].outputColumns).toBe(1);
347+
const body = await readJson<{
348+
results: Record<string, { state: string; resultSummary: { totalRowCount: number; sampleTuples: unknown[] } }>;
349+
}>(await getJson(`${API}/agents/${id}/operator-results`));
350+
expect(body.results["op-1"].state).toBe("Completed");
351+
expect(body.results["op-1"].resultSummary.totalRowCount).toBe(2);
352+
expect(body.results["op-1"].resultSummary.sampleTuples).toEqual([{ a: 1 }]);
354353
});
355354
});
356355

agent-service/src/server.ts

Lines changed: 15 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -309,31 +309,6 @@ const agentsRouter = new Elysia({ prefix: "/agents" })
309309
return { status: "cleared" };
310310
})
311311

312-
.post("/:id/checkout", ({ params: { id }, body }) => {
313-
const agent = getAgent(id);
314-
const { stepId } = body as { stepId: string };
315-
if (!stepId) throw new Error("stepId is required");
316-
317-
const success = agent.checkout(stepId);
318-
if (!success) throw new Error(`Step ${stepId} not found or checkout failed`);
319-
320-
const allSteps = agent.getAllSteps();
321-
const workflowContent = agent.getWorkflowState().getWorkflowContent();
322-
323-
broadcastToAgent(id, {
324-
type: "headChange",
325-
headId: stepId,
326-
steps: allSteps,
327-
workflowContent,
328-
operatorResults: getOperatorResultSummaries(agent),
329-
});
330-
331-
return {
332-
status: "checked out",
333-
headId: stepId,
334-
};
335-
})
336-
337312
.get("/:id/operator-types", ({ params: { id } }) => {
338313
const agent = getAgent(id);
339314
const metadataStore = agent.getMetadataStore();
@@ -426,12 +401,12 @@ function broadcastToAgent(agentId: string, message: WsServerMessage): void {
426401
if (!agent) return;
427402

428403
const jsonMessage = JSON.stringify(message);
429-
for (const ws of agent.getWebsockets()) {
404+
for (const ws of agent.getClients()) {
430405
try {
431406
ws.send(jsonMessage);
432407
} catch (error) {
433408
wsLog.error({ agentId, err: error }, "failed to send message to client");
434-
agent.removeWebsocket(ws);
409+
agent.removeClient(ws);
435410
}
436411
}
437412
}
@@ -459,7 +434,7 @@ export function buildApp() {
459434
return;
460435
}
461436

462-
agent.addWebsocket(ws);
437+
agent.addClient(ws);
463438

464439
const initMessage: WsServerInitMessage = {
465440
type: "init",
@@ -488,6 +463,12 @@ export function buildApp() {
488463
return;
489464
}
490465

466+
if (msg.type !== "command" && msg.type !== "prompt") {
467+
const unknownType = (msg as { type?: string }).type;
468+
ws.send(JSON.stringify({ type: "error", error: `Unknown message type: ${unknownType}` }));
469+
return;
470+
}
471+
491472
if (msg.type === "command") {
492473
if (msg.commandType === "stop") {
493474
agent.stop();
@@ -546,7 +527,7 @@ export function buildApp() {
546527

547528
const agent = agentStore.get(agentId);
548529
if (agent) {
549-
agent.removeWebsocket(ws);
530+
agent.removeClient(ws);
550531
}
551532
},
552533
})
@@ -564,6 +545,10 @@ export function _resetAgentStoreForTests(): void {
564545
agentCounter = 0;
565546
}
566547

548+
export function _getAgentForTests(agentId: string): TexeraAgent | undefined {
549+
return agentStore.get(agentId);
550+
}
551+
567552
function printStartupMessage(app: ReturnType<typeof buildApp>) {
568553
const LINE = "=".repeat(60);
569554
console.log(LINE);
@@ -591,7 +576,7 @@ function printStartupMessage(app: ReturnType<typeof buildApp>) {
591576
}
592577
console.log(" Send: { type: 'prompt', content: '...' }");
593578
console.log(" Send: { type: 'command', commandType: 'stop' }");
594-
console.log(" Recv: { type: 'step' | 'state' | 'complete' | 'error' | 'init' | 'headChange', ... }");
579+
console.log(" Recv: { type: 'step' | 'state' | 'complete' | 'error' | 'init', ... }");
595580
}
596581

597582
console.log("");

agent-service/src/server.ws.spec.ts

Lines changed: 43 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@
1717
* under the License.
1818
*/
1919

20-
// Exercises the /agents/:id/react WebSocket protocol end to end: the snapshot
21-
// sent on connect, the status lifecycle frames, the stop command, the prompt
22-
// request (with a stubbed run), and the error paths. These drive the real
23-
// socket via app.listen + a WebSocket client, since app.handle() does not
24-
// perform WS upgrades.
20+
// Exercises the /agents/:id/react WebSocket protocol end to end: the init
21+
// snapshot sent on connect, the state lifecycle frames, the stop command, the
22+
// prompt request (with a stubbed run), and the error paths. Frames use the
23+
// discriminated-union message model (type: "init" | "step" | "state" |
24+
// "complete" | "error" for server frames; "prompt" | "command" for client
25+
// frames). These drive the real socket via app.listen + a WebSocket client,
26+
// since app.handle() does not perform WS upgrades.
2527

2628
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test } from "bun:test";
2729
import { buildApp, _resetAgentStoreForTests, _getAgentForTests } from "./server";
@@ -150,74 +152,75 @@ afterEach(() => {
150152
});
151153

152154
describe(`WS ${API}/agents/:id/react`, () => {
153-
test("sends a results-free snapshot frame on connect", async () => {
155+
test("sends an init snapshot frame on connect", async () => {
154156
const id = await createAgent();
155157
const { ws, messages } = connect(id);
156158
await waitOpen(ws);
157159

158-
const snapshot = await messages.waitFor(m => m.type === "WsServerSnapshotEvent");
160+
const snapshot = await messages.waitFor(m => m.type === "init");
159161
expect(snapshot.state).toBe("AVAILABLE");
160162
expect(Array.isArray(snapshot.steps)).toBe(true);
161163
expect(typeof snapshot.headId).toBe("string");
162-
// Results are pulled on demand, never pushed on the snapshot.
163-
expect("operatorResults" in snapshot).toBe(false);
164+
// The init frame carries the current per-operator execution summaries.
165+
expect("operatorResults" in snapshot).toBe(true);
166+
expect(typeof snapshot.operatorResults).toBe("object");
164167
});
165168

166169
test("errors and closes when connecting to an unknown agent", async () => {
167170
const { messages } = connect("agent-does-not-exist");
168-
const err = await messages.waitFor(m => m.type === "WsServerErrorEvent");
171+
const err = await messages.waitFor(m => m.type === "error");
169172
expect(err.error).toBe("Agent not found");
170173
});
171174

172-
test("a stop command broadcasts a STOPPING status frame", async () => {
175+
test("a stop command broadcasts a STOPPING state frame", async () => {
173176
const id = await createAgent();
174177
const { ws, messages } = connect(id);
175178
await waitOpen(ws);
176-
await messages.waitFor(m => m.type === "WsServerSnapshotEvent");
179+
await messages.waitFor(m => m.type === "init");
177180

178-
ws.send(JSON.stringify({ type: "WsClientStopCommand" }));
181+
ws.send(JSON.stringify({ type: "command", commandType: "stop" }));
179182

180-
const status = await messages.waitFor(m => m.type === "WsServerStatusEvent");
183+
const status = await messages.waitFor(m => m.type === "state");
181184
expect(status.state).toBe("STOPPING");
182185
});
183186

184187
test("a prompt with empty content yields an error frame", async () => {
185188
const id = await createAgent();
186189
const { ws, messages } = connect(id);
187190
await waitOpen(ws);
188-
await messages.waitFor(m => m.type === "WsServerSnapshotEvent");
191+
await messages.waitFor(m => m.type === "init");
189192

190-
ws.send(JSON.stringify({ type: "WsClientPromptCommand", content: "" }));
193+
ws.send(JSON.stringify({ type: "prompt", content: "" }));
191194

192-
const err = await messages.waitFor(m => m.type === "WsServerErrorEvent");
195+
const err = await messages.waitFor(m => m.type === "error");
193196
expect(err.error).toBe("Message content is required");
194197
});
195198

196199
test("a malformed (non-JSON) frame yields an error frame", async () => {
197200
const id = await createAgent();
198201
const { ws, messages } = connect(id);
199202
await waitOpen(ws);
200-
await messages.waitFor(m => m.type === "WsServerSnapshotEvent");
203+
await messages.waitFor(m => m.type === "init");
201204

202205
ws.send("this is not json");
203206

204-
const err = await messages.waitFor(m => m.type === "WsServerErrorEvent");
207+
const err = await messages.waitFor(m => m.type === "error");
205208
expect(err.error).toBe("Invalid message format");
206209
});
207210

208211
test("an unknown message type yields an error frame", async () => {
209212
const id = await createAgent();
210213
const { ws, messages } = connect(id);
211214
await waitOpen(ws);
212-
await messages.waitFor(m => m.type === "WsServerSnapshotEvent");
215+
await messages.waitFor(m => m.type === "init");
213216

214217
ws.send(JSON.stringify({ type: "bogus" }));
215218

216-
const err = await messages.waitFor(m => m.type === "WsServerErrorEvent");
219+
const err = await messages.waitFor(m => m.type === "error");
217220
expect(err.error).toBe("Unknown message type: bogus");
218221
});
219222

220-
test("a prompt run streams GENERATING -> step -> resting status (no result frames)", async () => {
223+
test("a prompt run streams GENERATING -> step -> complete", async () => {
221224
const id = await createAgent();
222225

223226
// Stub the agent's run so no live LLM is needed: emit one ending step via
@@ -259,22 +262,25 @@ describe(`WS ${API}/agents/:id/react`, () => {
259262

260263
const { ws, messages } = connect(id);
261264
await waitOpen(ws);
262-
await messages.waitFor(m => m.type === "WsServerSnapshotEvent");
265+
await messages.waitFor(m => m.type === "init");
263266

264-
ws.send(JSON.stringify({ type: "WsClientPromptCommand", content: "hello" }));
267+
ws.send(JSON.stringify({ type: "prompt", content: "hello" }));
265268

266-
const generating = await messages.waitFor(m => m.type === "WsServerStatusEvent" && m.state === "GENERATING");
269+
const generating = await messages.waitFor(m => m.type === "state" && m.state === "GENERATING");
267270
expect(generating.state).toBe("GENERATING");
268271

269-
const step = await messages.waitFor(m => m.type === "WsServerStepEvent");
272+
const step = await messages.waitFor(m => m.type === "step");
270273
expect(step.step.content).toBe("done");
274+
// A step with no tool calls carries no operatorResults.
271275
expect("operatorResults" in step).toBe(false);
272276

273-
const resting = await messages.waitFor(m => m.type === "WsServerStatusEvent" && m.state === "AVAILABLE");
274-
expect(resting.state).toBe("AVAILABLE");
277+
// The run ends with a terminal `complete` frame carrying the resting state.
278+
const complete = await messages.waitFor(m => m.type === "complete");
279+
expect(complete.state).toBe("AVAILABLE");
280+
expect("operatorResults" in complete).toBe(true);
275281
});
276282

277-
test("a failed run emits an error frame and still returns to a resting status", async () => {
283+
test("a failed run emits an error frame", async () => {
278284
const id = await createAgent();
279285

280286
const agent = _getAgentForTests(id)!;
@@ -284,40 +290,35 @@ describe(`WS ${API}/agents/:id/react`, () => {
284290

285291
const { ws, messages } = connect(id);
286292
await waitOpen(ws);
287-
await messages.waitFor(m => m.type === "WsServerSnapshotEvent");
293+
await messages.waitFor(m => m.type === "init");
288294

289-
ws.send(JSON.stringify({ type: "WsClientPromptCommand", content: "hello" }));
295+
ws.send(JSON.stringify({ type: "prompt", content: "hello" }));
290296

291-
await messages.waitFor(m => m.type === "WsServerStatusEvent" && m.state === "GENERATING");
297+
await messages.waitFor(m => m.type === "state" && m.state === "GENERATING");
292298

293-
const err = await messages.waitFor(m => m.type === "WsServerErrorEvent");
299+
const err = await messages.waitFor(m => m.type === "error");
294300
expect(err.error).toBe("boom");
295-
296-
// The end-of-run status frame must still fire after a failure, so the client
297-
// is not left stuck on GENERATING.
298-
const resting = await messages.waitFor(m => m.type === "WsServerStatusEvent" && m.state === "AVAILABLE");
299-
expect(resting.state).toBe("AVAILABLE");
300301
});
301302

302303
test("a message for an agent that no longer exists yields an error frame", async () => {
303304
const id = await createAgent();
304305
const { ws, messages } = connect(id);
305306
await waitOpen(ws);
306-
await messages.waitFor(m => m.type === "WsServerSnapshotEvent");
307+
await messages.waitFor(m => m.type === "init");
307308

308309
// Drop the agent while the socket stays open; the message handler re-looks it up.
309310
_resetAgentStoreForTests();
310-
ws.send(JSON.stringify({ type: "WsClientPromptCommand", content: "hello" }));
311+
ws.send(JSON.stringify({ type: "prompt", content: "hello" }));
311312

312-
const err = await messages.waitFor(m => m.type === "WsServerErrorEvent");
313+
const err = await messages.waitFor(m => m.type === "error");
313314
expect(err.error).toBe("Agent not found");
314315
});
315316

316317
test("runs the close handler when the client disconnects", async () => {
317318
const id = await createAgent();
318319
const { ws, messages } = connect(id);
319320
await waitOpen(ws);
320-
await messages.waitFor(m => m.type === "WsServerSnapshotEvent");
321+
await messages.waitFor(m => m.type === "init");
321322

322323
const closed = new Promise<void>(resolve => ws.addEventListener("close", () => resolve(), { once: true }));
323324
ws.close();

agent-service/src/types/execution.ts

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -95,22 +95,3 @@ export interface WorkflowExecutionSummary {
9595
// empty means none.
9696
errors: string[];
9797
}
98-
99-
/**
100-
* Wire projection of one operator's execution result, summarized for the
101-
* client: counts and a small record sample instead of full payloads. Returned
102-
* by the REST route `GET /agents/:id/operator-results`.
103-
*/
104-
export interface OperatorResultSummary {
105-
state: string;
106-
inputTuples: number;
107-
outputTuples: number;
108-
inputPortShapes?: PortShape[];
109-
outputColumns?: number;
110-
error?: string;
111-
warnings?: string[];
112-
consoleLogCount?: number;
113-
totalRowCount?: number;
114-
sampleRecords?: Record<string, unknown>[];
115-
resultStatistics?: Record<string, string>;
116-
}

0 commit comments

Comments
 (0)