Skip to content

Commit a24d1d1

Browse files
bobbai00claude
andauthored
refactor(agent-service): extract WebSocket message types into types/ws (#5751)
### What changes were proposed in this PR? This PR refactors the agent-service ↔ frontend WebSocket messaging into a dedicated **`types/ws`** folder. Each frame is a small class whose `type` discriminator equals its class name (matching the main Texera WS convention, e.g. `RegionUpdateEvent`). Building a frame with `new WsServerStatusEvent(...)` sets the wire `type` for you, so no `type: "..."` literal is hand-written at call sites; receivers `switch` on `event.type`. - `types/ws/client.ts` — frames the frontend sends, unioned as `WsClientCommand`: - `WsClientPromptCommand` — a user prompt for the agent to run. - `WsClientStopCommand` — stop the in-flight run (payload-less). - `types/ws/server.ts` — frames agent-service sends back, unioned as `WsServerEvent`: - `WsServerSnapshotEvent` — full state, sent once when a client connects. - `WsServerStepEvent` — one step, streamed live as the agent runs. - `WsServerStatusEvent` — a lifecycle change (GENERATING / AVAILABLE / STOPPING). - `WsServerErrorEvent` — an error to surface to the user. - `WsServerHeadChangeEvent` — unused by the frontend/UI (still reachable via `/agents/:id/checkout`); slated for removal in #5930. - Operator result summaries are no longer pushed over WebSocket — they are pulled on demand via `GET /agents/:id/operator-results`. The corresponding REST DTO `OperatorResultSummary` lives in `types/execution.ts` (not under `ws/`). This PR also adds unit tests and renames the `agent-service` test files from `*.test.ts` to `*.spec.ts` to align with the frontend's naming convention. ### Any related issues, documentation, discussions? Closes #5749 ### How was this PR tested? Added/updated unit tests in both `agent-service` and `frontend` all pass; a local end-to-end run was also verified. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent de6c31a commit a24d1d1

18 files changed

Lines changed: 860 additions & 178 deletions

agent-service/src/agent/texera-agent.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ type ReActStepCallback = (step: ReActStep) => void;
7979
* workflow being edited (`WorkflowState`), cached operator execution results
8080
* (`WorkflowResultState`), and the tool surface exposed to the LLM. Each call
8181
* to `sendMessage` drives one multi-step generation via the Vercel AI SDK,
82-
* streaming step updates to subscribed websockets.
82+
* streaming step updates to subscribed clients.
8383
*/
8484
export class TexeraAgent {
8585
readonly agentId: string;
@@ -95,7 +95,7 @@ export class TexeraAgent {
9595
private stepCounter = 0;
9696
private workflowResultState: WorkflowResultState;
9797

98-
private websockets: Set<any> = new Set();
98+
private clients: Set<any> = new Set();
9999

100100
private model: LanguageModel;
101101
private systemPrompt: string;
@@ -266,16 +266,16 @@ export class TexeraAgent {
266266
return this.workflowResultState;
267267
}
268268

269-
getWebsockets(): Set<any> {
270-
return this.websockets;
269+
getClients(): Set<any> {
270+
return this.clients;
271271
}
272272

273-
addWebsocket(ws: any): void {
274-
this.websockets.add(ws);
273+
addClient(ws: any): void {
274+
this.clients.add(ws);
275275
}
276276

277-
removeWebsocket(ws: any): void {
278-
this.websockets.delete(ws);
277+
removeClient(ws: any): void {
278+
this.clients.delete(ws);
279279
}
280280

281281
getReActSteps(): ReActStep[] {
@@ -831,7 +831,7 @@ export class TexeraAgent {
831831

832832
this.workflowState.destroy();
833833

834-
this.websockets.clear();
834+
this.clients.clear();
835835

836836
this.reActStepsByMessageId.clear();
837837
this.stepsById.clear();

agent-service/src/agent/tools/result-formatting.test.ts renamed to agent-service/src/agent/tools/result-formatting.spec.ts

File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Lines changed: 157 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@
1717
* under the License.
1818
*/
1919

20-
import { beforeEach, describe, expect, test } from "bun:test";
21-
import { buildApp, _resetAgentStoreForTests } from "./server";
20+
import { beforeEach, describe, expect, spyOn, test } from "bun:test";
21+
import { buildApp, start, _resetAgentStoreForTests, _getAgentForTests } from "./server";
22+
import { WorkflowSystemMetadata } from "./agent/util/workflow-system-metadata";
2223
import { env } from "./config/env";
2324

2425
const API = env.API_PREFIX;
@@ -249,3 +250,157 @@ describe(`PATCH ${API}/agents/:id/settings`, () => {
249250
expect(reread.toolTimeoutSeconds).toBe(30);
250251
});
251252
});
253+
254+
describe("agent creation edge cases", () => {
255+
test("rejects an empty modelType", async () => {
256+
// The body schema accepts any string, so the handler's own guard runs.
257+
const res = await postJson(`${API}/agents`, { modelType: "" }, { Authorization: `Bearer ${TOKEN}` });
258+
expect(res.status).toBe(400);
259+
expect((await readJson<{ error: string }>(res)).error).toContain("modelType");
260+
});
261+
262+
test("applies initial settings supplied at creation time", async () => {
263+
const res = await createAgent({ settings: { maxSteps: 9, toolTimeoutSeconds: 12 } });
264+
expect(res.status).toBe(200);
265+
const body = await readJson<{ settings: { maxSteps: number; toolTimeoutSeconds: number } }>(res);
266+
expect(body.settings.maxSteps).toBe(9);
267+
expect(body.settings.toolTimeoutSeconds).toBe(12);
268+
});
269+
270+
test("creates the agent even when the workflow load fails (non-fatal)", async () => {
271+
// retrieveWorkflow targets the (unavailable) dashboard service; the failure
272+
// is caught and the agent is still created.
273+
const res = await createAgent({ workflowId: 123 });
274+
expect(res.status).toBe(200);
275+
});
276+
277+
test("masks the delegate token in agent info", async () => {
278+
const id = (await readJson<{ id: string }>(await createAgent())).id;
279+
_getAgentForTests(id)!.setDelegateConfig({
280+
userToken: "super-secret",
281+
userInfo: { uid: 1, email: "tester@example.com" },
282+
workflowId: 5,
283+
workflowName: "My Flow",
284+
computingUnitId: 2,
285+
} as any);
286+
287+
const info = await readJson<{ delegate?: { userToken: string; workflowName: string } }>(
288+
await getJson(`${API}/agents/${id}`)
289+
);
290+
expect(info.delegate?.userToken).toBe("***");
291+
expect(info.delegate?.workflowName).toBe("My Flow");
292+
});
293+
});
294+
295+
describe("agent read routes", () => {
296+
let id: string;
297+
beforeEach(async () => {
298+
id = (await readJson<{ id: string }>(await createAgent())).id;
299+
});
300+
301+
test("GET /:id/react-steps returns steps and state", async () => {
302+
const body = await readJson<{ steps: unknown[]; state: string }>(await getJson(`${API}/agents/${id}/react-steps`));
303+
expect(Array.isArray(body.steps)).toBe(true);
304+
expect(body.state).toBe("AVAILABLE");
305+
});
306+
307+
test("GET /:id/system-info responds", async () => {
308+
const res = await getJson(`${API}/agents/${id}/system-info`);
309+
expect(res.status).toBe(200);
310+
});
311+
312+
test("GET /:id/operator-types returns a list", async () => {
313+
const res = await getJson(`${API}/agents/${id}/operator-types`);
314+
expect(res.status).toBe(200);
315+
expect(Array.isArray(await readJson(res))).toBe(true);
316+
});
317+
318+
test("POST /:id/steps-by-operators returns steps", async () => {
319+
const res = await postJson(`${API}/agents/${id}/steps-by-operators`, { operatorIds: [] });
320+
expect(res.status).toBe(200);
321+
expect(Array.isArray((await readJson<{ steps: unknown[] }>(res)).steps)).toBe(true);
322+
});
323+
324+
test("GET /:id/operator-results maps the visible operator results", async () => {
325+
const agent = _getAgentForTests(id)!;
326+
(agent as any).getWorkflowResultState = () => ({
327+
getAllVisible: () =>
328+
new Map([
329+
[
330+
"op-1",
331+
{
332+
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: {},
343+
},
344+
},
345+
],
346+
]),
347+
});
348+
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);
354+
});
355+
});
356+
357+
describe("checkout route", () => {
358+
test("broadcasts and survives a websocket whose send throws", async () => {
359+
const id = (await readJson<{ id: string }>(await createAgent())).id;
360+
const agent = _getAgentForTests(id)!;
361+
(agent as any).checkout = () => true;
362+
(agent as any).getAllSteps = () => [];
363+
// A failing socket must be dropped inside broadcastToAgentClients, not crash the request.
364+
agent.addClient({
365+
send: () => {
366+
throw new Error("send failed");
367+
},
368+
} as any);
369+
370+
const res = await postJson(`${API}/agents/${id}/checkout`, { stepId: "step-1" });
371+
expect(res.status).toBe(200);
372+
expect((await readJson<{ headId: string }>(res)).headId).toBe("step-1");
373+
});
374+
375+
test("returns 500 when the step cannot be found", async () => {
376+
const id = (await readJson<{ id: string }>(await createAgent())).id;
377+
(_getAgentForTests(id) as any).checkout = () => false;
378+
const res = await postJson(`${API}/agents/${id}/checkout`, { stepId: "missing" });
379+
expect(res.status).toBe(500);
380+
});
381+
});
382+
383+
describe("non-router routes", () => {
384+
test("unknown routes fall through to the catch-all error handler", async () => {
385+
const res = await getJson("/no-such-route");
386+
expect(res.status).toBe(500);
387+
});
388+
});
389+
390+
describe("start()", () => {
391+
test("boots a listening app and prints the startup banner", async () => {
392+
const booted = await start();
393+
expect(typeof booted.server?.port).toBe("number");
394+
await booted.stop();
395+
});
396+
397+
test("tolerates a metadata-initialization failure", async () => {
398+
const spy = spyOn(WorkflowSystemMetadata, "initializeGlobal").mockImplementation(async () => {
399+
throw new Error("metadata unavailable");
400+
});
401+
const booted = await start();
402+
await booted.stop();
403+
expect(spy).toHaveBeenCalled();
404+
spy.mockRestore();
405+
});
406+
});

0 commit comments

Comments
 (0)