refactor(agent-service): extract WebSocket message types into types/ws#5751
Merged
bobbai00 merged 16 commits intoJun 28, 2026
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5751 +/- ##
============================================
+ Coverage 55.27% 56.20% +0.92%
+ Complexity 2990 2978 -12
============================================
Files 1117 1120 +3
Lines 43258 43217 -41
Branches 4668 4662 -6
============================================
+ Hits 23910 24288 +378
+ Misses 17938 17510 -428
- Partials 1410 1419 +9
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is a foundational refactor in agent-service/ that centralizes shared DTO/type definitions, introduces a dedicated backend-endpoints config module, and relocates JWT/auth helpers into a focused auth/ module (with new unit tests), while keeping api/backend-api.ts as a transitional entry point.
Changes:
- Added centralized type modules under
src/types/(api.ts,metadata.ts) and re-exported them from the types barrel. - Added
src/config/endpoints.tswithgetServiceEndpoints()to centralize service base URLs. - Moved JWT/auth helpers into
src/auth/jwt.ts, updated import paths, and addedsrc/auth/jwt.test.ts.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| agent-service/src/types/metadata.ts | Introduces extracted operator-metadata type definitions. |
| agent-service/src/types/api.ts | Adds wire DTOs for backend/service payloads and agent WS protocol. |
| agent-service/src/types/index.ts | Re-exports the new metadata and api type modules. |
| agent-service/src/config/endpoints.ts | Adds a centralized service-endpoint accessor. |
| agent-service/src/auth/jwt.ts | New JWT/auth helper module (moved from api/). |
| agent-service/src/auth/jwt.test.ts | Adds unit tests for JWT helpers. |
| agent-service/src/server.ts | Updates auth import path to the new auth/jwt module. |
| agent-service/src/api/workflow-api.ts | Updates auth header helper import path. |
| agent-service/src/api/index.ts | Updates barrel export to re-export JWT helpers from auth/. |
| agent-service/src/api/backend-api.ts | Re-exports metadata types from src/types/metadata (transitional). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Jun 17, 2026
bobbai00
force-pushed
the
refactor/agent-service-foundation
branch
from
676cc26 to
c565c4a
Compare
Contributor
Automated Reviewer SuggestionsBased on the
|
bobbai00
changed the title
Yicong-Huang
requested changes
Jun 21, 2026
bobbai00
force-pushed
the
refactor/agent-service-foundation
branch
from
6ff0a71 to
9a00461
Compare
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 385 | 0.235 | 25,139/31,427/31,427 us | 🟢 -8.0% / 🟢 -10.2% |
| 🔴 | bs=100 sw=10 sl=64 | 799 | 0.488 | 122,151/157,858/157,858 us | 🔴 +5.6% / 🔴 +12.9% |
| ⚪ | bs=1000 sw=10 sl=64 | 928 | 0.566 | 1,077,487/1,128,224/1,128,224 us | ⚪ within ±5% / 🔴 -10.9% |
Baseline details
Latest main 1c580e5 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 385 tuples/sec | 402 tuples/sec | 410.82 tuples/sec | -4.2% | -6.3% |
| bs=10 sw=10 sl=64 | MB/s | 0.235 MB/s | 0.246 MB/s | 0.251 MB/s | -4.5% | -6.3% |
| bs=10 sw=10 sl=64 | p50 | 25,139 us | 23,528 us | 23,785 us | +6.8% | +5.7% |
| bs=10 sw=10 sl=64 | p95 | 31,427 us | 34,160 us | 34,980 us | -8.0% | -10.2% |
| bs=10 sw=10 sl=64 | p99 | 31,427 us | 34,160 us | 34,980 us | -8.0% | -10.2% |
| bs=100 sw=10 sl=64 | throughput | 799 tuples/sec | 823 tuples/sec | 891.94 tuples/sec | -2.9% | -10.4% |
| bs=100 sw=10 sl=64 | MB/s | 0.488 MB/s | 0.502 MB/s | 0.544 MB/s | -2.8% | -10.4% |
| bs=100 sw=10 sl=64 | p50 | 122,151 us | 119,101 us | 112,277 us | +2.6% | +8.8% |
| bs=100 sw=10 sl=64 | p95 | 157,858 us | 149,492 us | 139,802 us | +5.6% | +12.9% |
| bs=100 sw=10 sl=64 | p99 | 157,858 us | 149,492 us | 139,802 us | +5.6% | +12.9% |
| bs=1000 sw=10 sl=64 | throughput | 928 tuples/sec | 937 tuples/sec | 1,041 tuples/sec | -1.0% | -10.9% |
| bs=1000 sw=10 sl=64 | MB/s | 0.566 MB/s | 0.572 MB/s | 0.635 MB/s | -1.0% | -10.9% |
| bs=1000 sw=10 sl=64 | p50 | 1,077,487 us | 1,065,684 us | 972,714 us | +1.1% | +10.8% |
| bs=1000 sw=10 sl=64 | p95 | 1,128,224 us | 1,107,788 us | 1,023,057 us | +1.8% | +10.3% |
| bs=1000 sw=10 sl=64 | p99 | 1,128,224 us | 1,107,788 us | 1,023,057 us | +1.8% | +10.3% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,519.03,200,128000,385,0.235,25138.52,31426.93,31426.93
1,100,10,64,20,2502.14,2000,1280000,799,0.488,122150.78,157858.47,157858.47
2,1000,10,64,20,21552.44,20000,12800000,928,0.566,1077487.28,1128224.11,1128224.11
bobbai00
force-pushed
the
refactor/agent-service-foundation
branch
from
9a00461 to
2bae592
Compare
This was referenced Jun 24, 2026
bobbai00
force-pushed
the
refactor/agent-service-foundation
branch
from
2bae592 to
508e72c
Compare
bobbai00
changed the title
bobbai00
force-pushed
the
refactor/agent-service-foundation
branch
from
508e72c to
bc67f57
Compare
bobbai00
force-pushed
the
refactor/agent-service-foundation
branch
from
bc67f57 to
43f0b96
Compare
This was referenced Jun 24, 2026
Move the inline WS message definitions out of server.ts into a dedicated types/ws module, modeled as discriminated unions rather than one all-optional interface: - client.ts: WsClientRequest = WsClientRequestPrompt | WsClientRequestStopCommand - server.ts: WsServerMessage union (snapshot/step/status/completion/error/headChange) + OperatorResultSummaryWs - index.ts: barrel, re-exported from the types barrel The client->server wire discriminator changes from "message"/"stop" to "prompt"/"command" (stop becomes commandType: "stop"). The server->client discriminators are renamed to uniform nouns that name each frame's payload: init->snapshot, state->status, complete->completion (step/error unchanged), with interfaces renamed to match (WsServerSnapshotMessage / WsServerStatusMessage / WsServerCompletionMessage). server.ts parses WsClientRequest and switches on the new shapes; the frontend WS sends and the receive switch are updated in lockstep. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qFkyrpTd5PrkNBPcBeo4Q
The `completion` frame carried both the agent's final `state` and the
operator-results snapshot. That duplicated the state path: the agent's
GENERATING->AVAILABLE transition (in TexeraAgent.sendMessage's finally) was
announced *only* inside `completion`, so `completion.state` and `status`
frames were two routes driving the same client state.
Route the end-of-run state through a `status` frame instead, emitted in a
finally covering both the success and error paths, and slim `completion` to
`{ type, operatorResults }` (a pure final-results snapshot). The frontend
`completion` handler drops its state branch; the `status` handler already
applies the resting state.
Side effect: this also fixes the client staying stuck on GENERATING after an
error — the error path previously emitted no resting-state update.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012qFkyrpTd5PrkNBPcBeo4Q
Convert the per-message comments in types/ws to uniform JSDoc blocks (so they surface in IDE hovers) and give every client and server frame a brief purpose line. Mark WsServerHeadChangeMessage @deprecated: it is redundant and unused (the checkout flow that emits it is unreachable) and is slated for removal in apache#5930. Doc-only; no type or runtime change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qFkyrpTd5PrkNBPcBeo4Q
…lts on demand Operator result summaries were pushed over the socket (on snapshot, step, and completion frames), but the only frontend consumer — the per-operator chat popover — reads them from a subject that was fed solely by those pushes, and the step-frame results were never applied at all (dead payload). Most summary fields are server-side agent context that is never displayed. Move results to an on-demand pull and slim the socket to conversation + lifecycle: - Drop the `completion` frame entirely (type, server broadcast, frontend handler). Run-end is already signaled by the end-of-run `status` frame. - Drop `operatorResults` from the `snapshot` and `step` frames (and stop computing/sending them there). - The chat popover now calls fetchOperatorResults() (GET /operator-results) when it opens, pushing fresh summaries to operatorResultSummaries$. - Remove the dead result-annotations machinery (toggleResultAnnotations, resultAnnotationsVisible(Subject/$), getResultAnnotationsVisible) — no callers. Server frames are now: snapshot, step, status, error (plus the deprecated headChange, removed in apache#5930). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qFkyrpTd5PrkNBPcBeo4Q
Add coverage for the parts of the WS framework this PR introduces, which the existing app.handle()-based suite never exercised (it only hits HTTP routes): - server.ws.test.ts: drives the real socket via app.listen + a WebSocket client. Covers the results-free snapshot on connect, the unknown-agent error, the stop command -> STOPPING status, prompt validation and malformed -frame errors, a stubbed prompt run streaming GENERATING -> step -> resting status, and a failed run still returning to a resting status (the stuck-on-GENERATING fix). No live LLM: the agent's sendMessage is stubbed. - server.ts: add a small `_getAgentForTests` hook (mirrors `_resetAgentStoreForTests`) so tests can stub agent behavior. - agent.service.spec.ts (frontend): cover fetchOperatorResults — the on-demand REST pull that replaced the WS results push — including the failure fallback. server.ts line coverage rises from ~53% to ~70%; the remaining gaps are pre-existing (agent construction, the deprecated checkout endpoint, the startup banner) and not introduced by this PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qFkyrpTd5PrkNBPcBeo4Q
Extend the suite to exercise every reachable path in server.ts: - HTTP routes not previously covered: react-steps, system-info, operator-types, steps-by-operators, the operator-results mapping body (with a stubbed visible result), checkout (success + not-found), the empty-modelType guard, initial-settings-on-create, the non-fatal workflow-load failure, delegate-token masking, and the catch-all error handler for unknown routes. - WebSocket branches: a message for an agent that was removed mid-connection, the final-step re-broadcast, the broadcast path tolerating a websocket whose send throws, and the close handler on disconnect. - start(): boots the listening app + prints the banner, and tolerates a metadata-initialization failure (stubbed to reject). Also collapse the `if (import.meta.main) start()` entry guard to one line so the entry point is a covered statement rather than an untestable block. server.ts is now at 100% line and function coverage (116 tests, all green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qFkyrpTd5PrkNBPcBeo4Q
Rename the agent-service test files from *.test.ts to *.spec.ts to match the frontend convention and codecov.yml's `**/*.spec.ts` ignore rule, so test files stay out of the coverage denominator. Bun discovers .spec.ts the same as .test.ts; no test changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qFkyrpTd5PrkNBPcBeo4Q
When an operator's chat popover opens (the `element:chat` paper event), the editor now pulls the active agent's operator results on demand. Add a test that adds an operator, fires `element:chat` for it, and asserts fetchOperatorResults is called with the active agent id — the one line of the PR's frontend change that wasn't yet exercised. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qFkyrpTd5PrkNBPcBeo4Q
The frontend CI job runs `prettier-eslint --list-different` before the tests; a multi-line `httpMock.expectOne(...)` in the added fetchOperatorResults spec fit on one line under the frontend's wider printWidth, so the check failed and the job exited before running tests — which is also why no frontend coverage report was produced. Reformat to satisfy the check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qFkyrpTd5PrkNBPcBeo4Q
Codecov flagged two uncovered changes in this PR's frontend:
- agent.service.ts: the stop-command websocket send in stopGeneration() was
never exercised. Add a spec that injects an open mock socket and asserts the
`{ type: "command", commandType: "stop" }` frame is sent (plus the REST
fallback when no socket is open).
- workflow-editor.component.ts: the `if (activeAgentId)` branch in the chat
popover's results pull was only half-covered. Add a no-active-agent case so
both branches are exercised.
Verified against the full frontend coverage run: both lines/branches now hit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012qFkyrpTd5PrkNBPcBeo4Q
bobbai00
force-pushed
the
refactor/agent-service-foundation
branch
from
ee92952 to
11a3ff5
Compare
The WS stop command was flattened from { type: "command", commandType: "stop" }
to { type: "stop" }, but this spec still asserted the old payload, failing CI.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Yicong-Huang
approved these changes
Jun 28, 2026
Yicong-Huang
left a comment
Yicong-Huang
left a comment
Contributor
There was a problem hiding this comment.
LGTM in general, left more comments inline. A big concern is parity with frontend definitions, let's resolve them later.
Each client command / server event is now a class whose `type` discriminator
equals its class name, built with `new WsServer*Event(...)` so no `type` literal
is hand-written at call sites; receivers switch on `event.type`. This drops the
`{type->payload}` map + `CustomUnionType` helper (and `types/util.ts`).
Also addresses review feedback: moves the REST DTO `OperatorResultSummary` out
of `types/ws` into `types/execution.ts`, corrects the operator-results doc path
to `GET /agents/:id/operator-results`, clarifies the `headChange` deprecation
note (unused by the UI but still reachable via `/checkout`), and clears the
`waitFor` timer in the WS spec. Frontend sender/handler and tests updated to the
new class-name wire tags.
…target broadcastToAgent -> broadcastToAgentClients and sendEvent -> sendEventToClient, so both names reflect that events go to the agent's connected WebSocket clients (the agent is the server-side object, not the recipient).
… / event) Makes the server-side WS surface speak one language: a connected consumer is a "client". Renames TexeraAgent's connection API getWebsockets/addWebsocket/ removeWebsocket -> getClients/addClient/removeClient (private `websockets` -> `clients`), and aligns the barrel comment and log strings to the command/event vocabulary the message classes already use.
bobbai00
added a commit
to bobbai00/texera
that referenced
this pull request
Jun 28, 2026
…#5930) ### What changes were proposed in this PR? Removes the step-checkout feature from agent-service, which was wired end-to-end but unreachable — nothing in the product ever invokes the frontend `checkoutStep()`, so the backend `/agents/:id/checkout` endpoint and its `headChange` WebSocket broadcast were never triggered. Removing the dead vertical slice: - `agent-service/src/server.ts` — drop the `POST /:id/checkout` route and the `headChange` / `workflowContent` members of the outgoing WebSocket-message type. - `agent-service/src/agent/texera-agent.ts` — drop `TexeraAgent.checkout()`. - `frontend/.../agent/agent.service.ts` — drop the `case "headChange"` WS handler and the unused `checkoutStep()` method. HEAD *tracking* is retained: `init`/`step` messages still carry `headId`, and the chat panel still walks the ancestor path from HEAD to render the visible step chain. Only the backward HEAD move (checkout) is removed. This branches from `main` and is independent of the type-refactor stack (apache#5751 → apache#5928/apache#5927). Note apache#5751 currently lists `headChange` as a server message; whichever merges second will drop it from the union — a trivial reconciliation. ### Any related issues, documentation, discussions? Closes apache#5942 Part of apache#5747. Independent of the stack (branches from `main`). ### How was this PR tested? agent-service: `tsc --noEmit` clean, 93/93 `bun test` pass, prettier clean. Frontend: the removed `checkoutStep()` had zero callers and the `headChange` handler is now unreachable; verified no remaining `checkout`/`headChange` references and no orphaned imports. ### 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>
bobbai00
added a commit
to bobbai00/texera
that referenced
this pull request
Jun 28, 2026
…config, and auth Consolidate the agent-service type/config layer. Stacked on the WS PR (apache#5751); types/execution.ts is left untouched (its redesign is a separate PR). - types/dto.ts: backend request/response DTOs (workflow persistence, compiling-service responses, WorkflowFatalError). - types/metadata.ts: operator-metadata types extracted from api/backend-api. - config/endpoints.ts: getServiceEndpoints() centralizing backend base URLs. - auth/jwt.ts: relocated from api/auth-api; update import paths. - Replace `any` with precise types across src/types/*; type ReActStep tool/message fields precisely. - Consume the centralized types at call sites (clients, server, agent utils). - Expand test coverage (jwt, workflow-system-metadata, compile/workflow clients); rename *.test.ts -> *.spec.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qFkyrpTd5PrkNBPcBeo4Q
bobbai00
added a commit
to bobbai00/texera
that referenced
this pull request
Jun 29, 2026
Reconciliation for rebasing the agent-service-execution-model stack onto origin/main, which had independently merged apache#5751 (extract WS types into types/ws as classes) and apache#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 apache#5751 flat `OperatorResultSummary` (which references the removed `PortShape` and duplicates our redesigned type), so drop it; our redesign's `OperatorResultSummary`/`OperatorExecutionSummary` supersede it. - Adopt apache#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 apache#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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
This PR refactors the agent-service ↔ frontend WebSocket messaging into a dedicated
types/wsfolder. Each frame is a small class whosetypediscriminator equals its class name (matching the main Texera WS convention, e.g.RegionUpdateEvent). Building a frame withnew WsServerStatusEvent(...)sets the wiretypefor you, so notype: "..."literal is hand-written at call sites; receiversswitchonevent.type.types/ws/client.ts— frames the frontend sends, unioned asWsClientCommand: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 asWsServerEvent: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 refactor(agent-service): remove dead checkout/headChange path #5930.GET /agents/:id/operator-results. The corresponding REST DTOOperatorResultSummarylives intypes/execution.ts(not underws/).This PR also adds unit tests and renames the
agent-servicetest files from*.test.tsto*.spec.tsto 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-serviceandfrontendall 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)