You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "bobbai00 (via GitHub)" <gi...@apache.org> on 2026/06/17 05:29:22 UTC
[PR] refactor(agent-service): centralize types, endpoint config, and auth module [texera]
bobbai00 opened a new pull request, #5751:
URL: https://github.com/apache/texera/pull/5751
### What changes were proposed in this PR?
Foundation slice of the agent-service reorganization (no runtime behavior change):
- Add `src/types/api.ts` (wire DTOs) and `src/types/metadata.ts` (operator-metadata types extracted from `api/backend-api.ts`); export both from the types barrel.
- Add `src/config/endpoints.ts` exposing `getServiceEndpoints()`.
- Move `src/api/auth-api.ts` → `src/auth/jwt.ts` (content unchanged) and add `src/auth/jwt.test.ts`; update auth import paths.
- Keep `api/backend-api.ts` transitional: it re-exports the metadata types from `../types/metadata` and retains `getBackendConfig`/`fetchOperatorMetadata`, which the follow-up clients PR relocates.
The `types/agent.ts`/`types/workflow.ts` reshaping is deferred to the PR that also updates `server.ts`/`workflow-state.ts`, since those renames are coupled. This is the first of three PRs that split #5564 into single-topic changes per review feedback.
### Any related issues, documentation, discussions?
Closes #5748
Part of #5747
### How was this PR tested?
`bunx tsc --noEmit`, `bun test` (101 pass / 0 fail, including the new `auth/jwt.test.ts`), and `prettier --check` all pass in `agent-service`.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.8 (1M context)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): extract WebSocket message types into types/ws [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3487127776
##########
frontend/src/app/workspace/service/agent/agent.service.ts:
##########
@@ -1311,7 +1274,10 @@ export class AgentService {
}
/**
- * Fetch operator results from the backend (fallback if WebSocket data not available).
+ * Pull the agent's latest operator result summaries from the backend and push
+ * them to `operatorResultSummaries$`. Called on demand when the UI needs to
+ * show results (e.g. opening an operator's popover); results are no longer
+ * pushed over the WebSocket.
*/
Review Comment:
This doc comment says operator results are no longer pushed over the WebSocket, but the service still consumes `message.operatorResults` in the legacy `headChange` WS frame. Either clarify that the “no longer pushed” guarantee applies to snapshot/step/status frames only, or remove `operatorResults` from `headChange` as part of the deprecation cleanup.
##########
agent-service/src/server.ts:
##########
@@ -523,21 +492,23 @@ export function buildApp() {
return;
}
- let msg: WsMessage;
+ let msg: WsClientRequest;
try {
- msg = typeof messageData === "string" ? JSON.parse(messageData) : (messageData as WsMessage);
+ msg = typeof messageData === "string" ? JSON.parse(messageData) : (messageData as WsClientRequest);
} catch {
ws.send(JSON.stringify({ type: "error", error: "Invalid message format" }));
return;
}
- if (msg.type === "stop") {
- agent.stop();
- broadcastToAgent(agentId, { type: "state", state: "STOPPING" });
+ if (msg.type === "command") {
+ if (msg.commandType === "stop") {
+ agent.stop();
+ broadcastToAgent(agentId, { type: "status", state: AgentState.STOPPING });
+ }
return;
}
Review Comment:
The WS handler silently ignores unknown `commandType` values (and any unexpected `msg.type` values), which makes client/server mismatches hard to diagnose and can lead to no-op behavior on bad input. Since frames are parsed from untrusted JSON, it would be safer to validate the discriminator(s) at runtime and return an explicit error frame for unknown types/commands.
##########
agent-service/src/types/ws/server.ts:
##########
@@ -0,0 +1,110 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Server -> client WebSocket frames for this service's protocol
+// (`/agents/:id/react`). Modeled as a discriminated union on `type`, so each
+// message kind declares exactly the fields it sends.
+
+import type { AgentState, ReActStep } from "../agent";
+import type { WorkflowContent } from "../workflow";
+
+/**
+ * Wire projection of one operator's execution result, summarized for the
+ * client: counts and a small record sample instead of full payloads.
+ */
+export interface OperatorResultSummaryWs {
+ state: string;
+ inputTuples: number;
+ outputTuples: number;
+ inputPortShapes?: { portIndex: number; rows: number; columns: number }[];
+ outputColumns?: number;
+ error?: string;
+ warnings?: string[];
+ consoleLogCount?: number;
+ totalRowCount?: number;
+ sampleRecords?: Record<string, unknown>[];
+ resultStatistics?: Record<string, string>;
+}
+
+/** Per-operator result summaries, keyed by operator id. */
+type OperatorResults = Record<string, OperatorResultSummaryWs>;
+
+/** Shared discriminator base; every server frame sets a unique `type`. */
+interface WsServerMessageBase {
+ type: "snapshot" | "step" | "status" | "error" | "headChange";
+}
+
+/**
+ * Full state pushed once when a client connects: the agent's current lifecycle
+ * state, the complete step list, and the HEAD pointer. Operator results are not
+ * included — they are pulled on demand via `GET /operator-results`.
+ */
+export interface WsServerSnapshotMessage extends WsServerMessageBase {
+ type: "snapshot";
+ state: AgentState;
+ steps: ReActStep[];
+ headId: string;
+}
+
+/**
+ * A single ReAct step, streamed live as the agent runs.
+ */
+export interface WsServerStepMessage extends WsServerMessageBase {
+ type: "step";
+ step: ReActStep;
+}
+
+/**
+ * An agent lifecycle transition (e.g. GENERATING when a run starts, the resting
+ * state when it ends, STOPPING on stop).
+ */
+export interface WsServerStatusMessage extends WsServerMessageBase {
+ type: "status";
+ state: AgentState;
+}
+
+/** An error surfaced to the client (agent not found, bad request, failed run). */
+export interface WsServerErrorMessage extends WsServerMessageBase {
+ type: "error";
+ error: string;
+}
+
+/**
+ * Emitted after a checkout: HEAD moved, carrying the full step list and the
+ * workflow snapshot at the new head.
+ *
+ * @deprecated Redundant and unused — the checkout flow that produces this frame
+ * is unreachable in the product (nothing invokes the client's `checkoutStep()`).
+ * Scheduled for removal (see #5930); do not build new code on it.
+ */
+export interface WsServerHeadChangeMessage extends WsServerMessageBase {
+ type: "headChange";
+ headId: string;
+ steps: ReActStep[];
+ workflowContent?: WorkflowContent;
+ operatorResults: OperatorResults;
Review Comment:
`WsServerHeadChangeMessage` still includes `operatorResults`, which contradicts the PR description/issue goal that operator result summaries are no longer pushed over the WebSocket. If `headChange` is truly deprecated/unreachable, consider removing `operatorResults` from this frame (and from the checkout broadcast + frontend handler) so the protocol is consistent and the results transport is purely HTTP.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): extract WebSocket message types into types/ws [texera]
Posted by "bobbai00 (via GitHub)" <gi...@apache.org>.
bobbai00 commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3470455686
##########
agent-service/src/types/ws.ts:
##########
@@ -0,0 +1,55 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// WebSocket frames for this service's own protocol (/agents/:id/react):
+// inbound client messages and the outbound streaming updates it pushes back.
+
+import type { ReActStep } from "./agent";
+import type { WorkflowContent } from "./workflow";
+
+export interface WsMessage {
+ type: "message" | "stop";
Review Comment:
fixed
##########
agent-service/src/types/ws.ts:
##########
@@ -0,0 +1,55 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// WebSocket frames for this service's own protocol (/agents/:id/react):
+// inbound client messages and the outbound streaming updates it pushes back.
+
+import type { ReActStep } from "./agent";
+import type { WorkflowContent } from "./workflow";
+
+export interface WsMessage {
+ type: "message" | "stop";
+ content?: string;
+ messageSource?: "chat" | "feedback";
+}
+
+export interface OperatorResultSummaryWs {
+ state: string;
+ inputTuples: number;
+ outputTuples: number;
+ inputPortShapes?: { portIndex: number; rows: number; columns: number }[];
+ outputColumns?: number;
+ error?: string;
+ warnings?: string[];
+ consoleLogCount?: number;
+ totalRowCount?: number;
+ sampleRecords?: Record<string, unknown>[];
+ resultStatistics?: Record<string, string>;
+}
+
+export interface WsOutgoingMessage {
+ type: "step" | "state" | "error" | "complete" | "init" | "headChange";
+ step?: ReActStep;
+ state?: string;
+ error?: string;
+ steps?: ReActStep[];
+ headId?: string;
+ operatorResults?: Record<string, OperatorResultSummaryWs>;
+ workflowContent?: WorkflowContent;
+}
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): extract WebSocket message types into types/ws [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3487129988
##########
agent-service/src/types/ws/server.ts:
##########
@@ -0,0 +1,110 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Server -> client WebSocket frames for this service's protocol
+// (`/agents/:id/react`). Modeled as a discriminated union on `type`, so each
+// message kind declares exactly the fields it sends.
+
+import type { AgentState, ReActStep } from "../agent";
+import type { WorkflowContent } from "../workflow";
+
+/**
+ * Wire projection of one operator's execution result, summarized for the
+ * client: counts and a small record sample instead of full payloads.
+ */
+export interface OperatorResultSummaryWs {
+ state: string;
+ inputTuples: number;
+ outputTuples: number;
+ inputPortShapes?: { portIndex: number; rows: number; columns: number }[];
+ outputColumns?: number;
+ error?: string;
+ warnings?: string[];
+ consoleLogCount?: number;
+ totalRowCount?: number;
+ sampleRecords?: Record<string, unknown>[];
+ resultStatistics?: Record<string, string>;
+}
+
+/** Per-operator result summaries, keyed by operator id. */
+type OperatorResults = Record<string, OperatorResultSummaryWs>;
+
+/** Shared discriminator base; every server frame sets a unique `type`. */
+interface WsServerMessageBase {
+ type: "snapshot" | "step" | "status" | "error" | "headChange";
+}
+
+/**
+ * Full state pushed once when a client connects: the agent's current lifecycle
+ * state, the complete step list, and the HEAD pointer. Operator results are not
+ * included — they are pulled on demand via `GET /operator-results`.
+ */
+export interface WsServerSnapshotMessage extends WsServerMessageBase {
+ type: "snapshot";
+ state: AgentState;
+ steps: ReActStep[];
+ headId: string;
+}
+
+/**
+ * A single ReAct step, streamed live as the agent runs.
+ */
+export interface WsServerStepMessage extends WsServerMessageBase {
+ type: "step";
+ step: ReActStep;
+}
+
+/**
+ * An agent lifecycle transition (e.g. GENERATING when a run starts, the resting
+ * state when it ends, STOPPING on stop).
+ */
+export interface WsServerStatusMessage extends WsServerMessageBase {
+ type: "status";
+ state: AgentState;
+}
+
+/** An error surfaced to the client (agent not found, bad request, failed run). */
+export interface WsServerErrorMessage extends WsServerMessageBase {
+ type: "error";
+ error: string;
+}
+
+/**
+ * Emitted after a checkout: HEAD moved, carrying the full step list and the
+ * workflow snapshot at the new head.
+ *
+ * @deprecated Redundant and unused — the checkout flow that produces this frame
+ * is unreachable in the product (nothing invokes the client's `checkoutStep()`).
+ * Scheduled for removal (see #5930); do not build new code on it.
+ */
+export interface WsServerHeadChangeMessage extends WsServerMessageBase {
+ type: "headChange";
Review Comment:
1. I don't think we need `type`. just define message A, B, C and then `WsMessage = A | B | C`
2. make them read only.
##########
agent-service/src/types/ws/client.ts:
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Client -> server WebSocket frames for this service's protocol
+// (`/agents/:id/react`). Modeled as a discriminated union on `type`, so each
+// request kind carries only its own fields.
+
+/** Shared discriminator base; every client request sets a unique `type`. */
+interface WsClientRequestBase {
+ type: "prompt" | "command";
+}
+
+/**
+ * A user prompt for the agent to run. `messageSource` notes where it
+ * originated (interactive chat vs. an operator feedback action).
+ */
+export interface WsClientRequestPrompt extends WsClientRequestBase {
Review Comment:
change to `XXXRequest`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): centralize types, endpoint config, and auth module [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5751:
URL: https://github.com/apache/texera/pull/5751#issuecomment-4762901622
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- No candidates found from `git blame` history.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): extract WebSocket message types into types/ws [texera]
Posted by "bobbai00 (via GitHub)" <gi...@apache.org>.
bobbai00 commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3487548026
##########
agent-service/src/types/ws/client.ts:
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Client -> server WebSocket frames for this service's protocol
+// (`/agents/:id/react`). Modeled as a discriminated union on `type`, so each
+// request kind carries only its own fields.
+
+/** Shared discriminator base; every client request sets a unique `type`. */
+interface WsClientRequestBase {
+ type: "prompt" | "command";
+}
+
+/**
+ * A user prompt for the agent to run. `messageSource` notes where it
+ * originated (interactive chat vs. an operator feedback action).
+ */
+export interface WsClientRequestPrompt extends WsClientRequestBase {
Review Comment:
changed to `XXXCommand`
##########
agent-service/src/types/ws/server.ts:
##########
@@ -0,0 +1,110 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Server -> client WebSocket frames for this service's protocol
+// (`/agents/:id/react`). Modeled as a discriminated union on `type`, so each
+// message kind declares exactly the fields it sends.
+
+import type { AgentState, ReActStep } from "../agent";
+import type { WorkflowContent } from "../workflow";
+
+/**
+ * Wire projection of one operator's execution result, summarized for the
+ * client: counts and a small record sample instead of full payloads.
+ */
+export interface OperatorResultSummaryWs {
+ state: string;
+ inputTuples: number;
+ outputTuples: number;
+ inputPortShapes?: { portIndex: number; rows: number; columns: number }[];
+ outputColumns?: number;
+ error?: string;
+ warnings?: string[];
+ consoleLogCount?: number;
+ totalRowCount?: number;
+ sampleRecords?: Record<string, unknown>[];
+ resultStatistics?: Record<string, string>;
+}
+
+/** Per-operator result summaries, keyed by operator id. */
+type OperatorResults = Record<string, OperatorResultSummaryWs>;
+
+/** Shared discriminator base; every server frame sets a unique `type`. */
+interface WsServerMessageBase {
+ type: "snapshot" | "step" | "status" | "error" | "headChange";
+}
+
+/**
+ * Full state pushed once when a client connects: the agent's current lifecycle
+ * state, the complete step list, and the HEAD pointer. Operator results are not
+ * included — they are pulled on demand via `GET /operator-results`.
+ */
+export interface WsServerSnapshotMessage extends WsServerMessageBase {
+ type: "snapshot";
+ state: AgentState;
+ steps: ReActStep[];
+ headId: string;
+}
+
+/**
+ * A single ReAct step, streamed live as the agent runs.
+ */
+export interface WsServerStepMessage extends WsServerMessageBase {
+ type: "step";
+ step: ReActStep;
+}
+
+/**
+ * An agent lifecycle transition (e.g. GENERATING when a run starts, the resting
+ * state when it ends, STOPPING on stop).
+ */
+export interface WsServerStatusMessage extends WsServerMessageBase {
+ type: "status";
+ state: AgentState;
+}
+
+/** An error surfaced to the client (agent not found, bad request, failed run). */
+export interface WsServerErrorMessage extends WsServerMessageBase {
+ type: "error";
+ error: string;
+}
+
+/**
+ * Emitted after a checkout: HEAD moved, carrying the full step list and the
+ * workflow snapshot at the new head.
+ *
+ * @deprecated Redundant and unused — the checkout flow that produces this frame
+ * is unreachable in the product (nothing invokes the client's `checkoutStep()`).
+ * Scheduled for removal (see #5930); do not build new code on it.
+ */
+export interface WsServerHeadChangeMessage extends WsServerMessageBase {
+ type: "headChange";
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): centralize types, endpoint config, and auth module [texera]
Posted by "bobbai00 (via GitHub)" <gi...@apache.org>.
bobbai00 commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3447769626
##########
agent-service/src/auth/jwt.test.ts:
##########
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { describe, expect, test } from "bun:test";
+import { extractUserFromToken, validateToken, createAuthHeaders } from "./jwt";
+
+function makeToken(payload: Record<string, unknown>): string {
+ const encode = (o: Record<string, unknown>) => Buffer.from(JSON.stringify(o)).toString("base64");
+ return `${encode({ alg: "none", typ: "JWT" })}.${encode(payload)}.signature`;
Review Comment:
Done in 5293215b: `makeToken` now encodes the segments with base64url instead of base64, and there is an added case whose payload contains `-`/`_` to pin the url-safe decoding path.
##########
agent-service/src/auth/jwt.test.ts:
##########
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { describe, expect, test } from "bun:test";
+import { extractUserFromToken, validateToken, createAuthHeaders } from "./jwt";
Review Comment:
Added in 5293215b: a new `extractBearerToken` describe block covers a valid Bearer header, case-insensitive scheme, a non-Bearer scheme, a missing token, and an absent header.
##########
agent-service/src/auth/jwt.test.ts:
##########
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { describe, expect, test } from "bun:test";
+import { extractUserFromToken, validateToken, createAuthHeaders } from "./jwt";
+
+function makeToken(payload: Record<string, unknown>): string {
+ const encode = (o: Record<string, unknown>) => Buffer.from(JSON.stringify(o)).toString("base64");
+ return `${encode({ alg: "none", typ: "JWT" })}.${encode(payload)}.signature`;
+}
+
+const nowSeconds = () => Math.floor(Date.now() / 1000);
+
+describe("extractUserFromToken", () => {
+ test("maps JWT claims onto a UserInfo", () => {
+ const token = makeToken({ userId: 7, sub: "alice", email: "alice@example.com", role: "ADMIN" });
+ expect(extractUserFromToken(token)).toEqual({ uid: 7, name: "alice", email: "alice@example.com", role: "ADMIN" });
+ });
+
+ test("defaults missing email and role", () => {
+ const token = makeToken({ userId: 1, sub: "bob" });
+ expect(extractUserFromToken(token)).toEqual({ uid: 1, name: "bob", email: "", role: "REGULAR" });
+ });
+
+ test("throws on a malformed token", () => {
+ expect(() => extractUserFromToken("not-a-jwt")).toThrow("Failed to decode JWT");
+ });
+});
+
+describe("validateToken", () => {
+ test("accepts a token expiring in the future", () => {
+ expect(validateToken(makeToken({ sub: "a", exp: nowSeconds() + 3600 }))).toBe(true);
+ });
+
+ test("rejects an expired token", () => {
+ expect(validateToken(makeToken({ sub: "a", exp: nowSeconds() - 3600 }))).toBe(false);
+ });
+
+ test("treats a token without exp as valid", () => {
+ expect(validateToken(makeToken({ sub: "a" }))).toBe(true);
+ });
+
+ test("rejects a malformed token", () => {
+ expect(validateToken("garbage")).toBe(false);
+ });
+});
+
+describe("createAuthHeaders", () => {
+ test("builds bearer auth headers", () => {
+ expect(createAuthHeaders("tok")).toEqual({ Authorization: "Bearer tok", "Content-Type": "application/json" });
+ });
+});
Review Comment:
Added in 5293215b: see the new `extractBearerToken` tests (valid Bearer, case-insensitive scheme, wrong scheme, missing token, absent header).
##########
agent-service/src/types/api.ts:
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Wire DTOs: request/response bodies exchanged with backend services and the
+// WebSocket frames this service sends to its own clients. Distinct from domain
+// types (workflow.ts, execution.ts, agent.ts) which model in-memory state.
+
+import type { WorkflowContent, OperatorPortSchemaMap } from "./workflow";
+import type { ReActStep } from "./agent";
+
+// --- Dashboard Service: workflow persistence ---
+
+export interface Workflow {
+ wid: number;
+ name: string;
+ description?: string;
+ content: WorkflowContent;
+ creationTime?: number;
+ lastModifiedTime?: number;
+ isPublished?: boolean;
+}
+
+export interface WorkflowPersistRequest {
+ wid?: number;
+ name: string;
+ description?: string;
+ content: string;
+ isPublic?: boolean;
+}
+
+// --- Workflow Compiling Service ---
+
+export interface WorkflowFatalError {
+ message: string;
+ details: string;
+ operatorId: string;
+ workerId: string;
+ type: { name: string };
+ timestamp: { nanos: number; seconds: number };
+}
+
+export interface WorkflowCompilationResponse {
+ physicalPlan?: any;
+ operatorOutputSchemas: Record<string, OperatorPortSchemaMap>;
+ operatorErrors: Record<string, WorkflowFatalError>;
+}
Review Comment:
Resolved in afae9957: `types/api.ts` was split into `types/wire.ts` (backend DTOs) and `types/ws.ts` (this service's WebSocket frames). The duplicate `WorkflowFatalError` is gone — there is now a single definition in `types/wire.ts`, corrected to match the backend proto (`workflowruntimestate.proto`): `type` is the `FatalErrorType` enum name (string), with `details`/`operatorId`/`workerId`/`timestamp` optional. `api/compile-api.ts` and its consumers (`context-utils.ts`, `texera-agent.ts`) now import that single canonical type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): extract WebSocket message types into types/ws [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3487592867
##########
agent-service/src/types/ws/client.ts:
##########
@@ -0,0 +1,42 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Client -> server WebSocket frames for this service's protocol
+
+import type { CustomUnionType } from "../util";
+
+/**
+ * Send a prompt to agent to start the ReAct loop
+ */
+export interface WsClientPromptCommand extends Readonly<{
+ content: string;
+ messageSource?: "chat" | "feedback";
+}> {}
+
+/**
+ * Stop the agent's current ReAct loop
+ */
+export interface WsClientStopCommand extends Readonly<{}> {}
+
+export type WsClientCommandTypeMap = {
+ prompt: WsClientPromptCommand;
+ stop: WsClientStopCommand;
+};
+
+export type WsClientCommand = CustomUnionType<WsClientCommandTypeMap>;
Review Comment:
The PR description/linked issue says the client stop request should be a discriminated "command" frame (e.g. { type: "command", commandType: "stop" }), but the extracted WS types define stop as its own discriminator (type: "stop") and there is no commandType. This mismatch can confuse protocol consumers and makes it unclear what the canonical wire format is.
##########
agent-service/src/types/ws/server.ts:
##########
@@ -0,0 +1,94 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Server -> client WebSocket frames for this service's protocol
+
+import type { AgentState, ReActStep } from "../agent";
+import type { WorkflowContent } from "../workflow";
+import type { CustomUnionType } from "../util";
+
+/**
+ * Wire projection of one operator's execution result, summarized for the
+ * client: counts and a small record sample instead of full payloads.
+ */
+export interface OperatorResultSummaryWs {
+ state: string;
+ inputTuples: number;
+ outputTuples: number;
+ inputPortShapes?: { portIndex: number; rows: number; columns: number }[];
+ outputColumns?: number;
+ error?: string;
+ warnings?: string[];
+ consoleLogCount?: number;
+ totalRowCount?: number;
+ sampleRecords?: Record<string, unknown>[];
+ resultStatistics?: Record<string, string>;
+}
+
+/**
+ * Full state pushed once when a client connects: the agent's current lifecycle
+ * state, the complete step list, and the HEAD pointer. Operator results are not
+ * included — they are pulled on demand via `GET /operator-results`.
+ */
+export interface WsServerSnapshotEvent extends Readonly<{
+ state: AgentState;
+ steps: ReActStep[];
+ headId: string;
+}> {}
+
+/** A single ReAct step, streamed live as the agent runs. */
+export interface WsServerStepEvent extends Readonly<{
+ step: ReActStep;
+}> {}
+
+/**
+ * An agent lifecycle transition (e.g. GENERATING when a run starts, the resting
+ * state when it ends, STOPPING on stop).
+ */
+export interface WsServerStatusEvent extends Readonly<{
+ state: AgentState;
+}> {}
+
+/** An error surfaced to the client (agent not found, bad request, failed run). */
+export interface WsServerErrorEvent extends Readonly<{
+ error: string;
+}> {}
+
+/**
+ * Emitted after a checkout: HEAD moved, carrying the full step list and the
+ * workflow snapshot at the new head.
+ *
+ * @deprecated Redundant and unused. TODO: remove this message and related caller logics.
+ */
Review Comment:
The headChange event is annotated as "unused", but this PR still keeps a reachable `/agents/:id/checkout` route (and tests) that broadcasts headChange. If the intent is “unused by the frontend/UI”, clarify that to avoid misleading future cleanup work.
##########
agent-service/src/server.ts:
##########
@@ -39,7 +39,8 @@ import type {
AgentSettingsApi,
ReActStep,
} from "./types/agent";
-import { OperatorResultSerializationMode } from "./types/agent";
+import { AgentState, OperatorResultSerializationMode } from "./types/agent";
+import type { WsClientCommand, WsServerEvent, OperatorResultSummaryWs } from "./types/ws";
Review Comment:
`OperatorResultSummaryWs` is imported from `types/ws`, but it’s used as the payload type for the HTTP `GET /agents/:id/operator-results` response. Keeping REST DTOs under a `ws/` namespace (and with a `Ws` suffix) makes the API surface harder to navigate and can lead to accidental coupling to WebSocket-only types.
##########
agent-service/src/server.ws.spec.ts:
##########
@@ -0,0 +1,323 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Exercises the /agents/:id/react WebSocket protocol end to end: the snapshot
+// sent on connect, the status lifecycle frames, the stop command, the prompt
+// request (with a stubbed run), and the error paths. These drive the real
+// socket via app.listen + a WebSocket client, since app.handle() does not
+// perform WS upgrades.
+
+import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test } from "bun:test";
+import { buildApp, _resetAgentStoreForTests, _getAgentForTests } from "./server";
+import { env } from "./config/env";
+
+const API = env.API_PREFIX;
+
+let app: ReturnType<typeof buildApp>;
+let port: number;
+const openSockets: WebSocket[] = [];
+
+function mintTestToken(): string {
+ const header = Buffer.from(JSON.stringify({ alg: "HS256", typ: "JWT" })).toString("base64url");
+ const payload = Buffer.from(
+ JSON.stringify({
+ sub: "tester",
+ userId: 1,
+ email: "tester@example.com",
+ role: "REGULAR",
+ exp: Math.floor(Date.now() / 1000) + 3600,
+ })
+ ).toString("base64url");
+ return `${header}.${payload}.test-signature`;
+}
+
+const TOKEN = mintTestToken();
+
+async function createAgent(): Promise<string> {
+ const res = await app.handle(
+ new Request(`http://localhost${API}/agents`, {
+ method: "POST",
+ headers: { "Content-Type": "application/json", Authorization: `Bearer ${TOKEN}` },
+ body: JSON.stringify({ modelType: "test-model" }),
+ })
+ );
+ const body = (await res.json()) as { id: string };
+ return body.id;
+}
+
+interface Collector {
+ waitFor(predicate: (m: any) => boolean, timeoutMs?: number): Promise<any>;
+}
+
+// Attaches a message listener immediately (before `open`) so no frame — not even
+// the snapshot the server sends on connect — is missed, then resolves waiters
+// from a buffer.
+function collect(ws: WebSocket): Collector {
+ const buffer: any[] = [];
+ const waiters: { predicate: (m: any) => boolean; resolve: (m: any) => void }[] = [];
+ ws.addEventListener("message", ev => {
+ let data: any;
+ try {
+ data = JSON.parse(ev.data as string);
+ } catch {
+ return;
+ }
+ buffer.push(data);
+ const i = waiters.findIndex(w => w.predicate(data));
+ if (i >= 0) {
+ waiters[i].resolve(data);
+ waiters.splice(i, 1);
+ }
+ });
+ return {
+ waitFor(predicate, timeoutMs = 2000) {
+ const found = buffer.find(predicate);
+ if (found) return Promise.resolve(found);
+ return new Promise((resolve, reject) => {
+ const w = { predicate, resolve };
+ waiters.push(w);
+ setTimeout(() => {
+ const idx = waiters.indexOf(w);
+ if (idx >= 0) {
+ waiters.splice(idx, 1);
+ reject(new Error("timed out waiting for a matching WS frame"));
+ }
+ }, timeoutMs);
+ });
+ },
Review Comment:
`waitFor()` installs a timeout but never clears it when the awaited frame arrives. That leaves stray timers running, which can slow the suite and (depending on the runner) keep the event loop alive longer than necessary.
##########
agent-service/src/types/ws/server.ts:
##########
@@ -0,0 +1,94 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Server -> client WebSocket frames for this service's protocol
+
+import type { AgentState, ReActStep } from "../agent";
+import type { WorkflowContent } from "../workflow";
+import type { CustomUnionType } from "../util";
+
+/**
+ * Wire projection of one operator's execution result, summarized for the
+ * client: counts and a small record sample instead of full payloads.
+ */
+export interface OperatorResultSummaryWs {
+ state: string;
+ inputTuples: number;
+ outputTuples: number;
+ inputPortShapes?: { portIndex: number; rows: number; columns: number }[];
+ outputColumns?: number;
+ error?: string;
+ warnings?: string[];
+ consoleLogCount?: number;
+ totalRowCount?: number;
+ sampleRecords?: Record<string, unknown>[];
+ resultStatistics?: Record<string, string>;
+}
+
+/**
+ * Full state pushed once when a client connects: the agent's current lifecycle
+ * state, the complete step list, and the HEAD pointer. Operator results are not
+ * included — they are pulled on demand via `GET /operator-results`.
+ */
Review Comment:
This comment says operator results are pulled via `GET /operator-results`, but the actual HTTP route is `GET /agents/:id/operator-results` (under the API prefix). Keeping the correct path in the protocol docs helps avoid client integration mistakes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): centralize types, endpoint config, and auth module in agent-service [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5751:
URL: https://github.com/apache/texera/pull/5751#issuecomment-4786276919
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 2 better · 🔴 3 worse · ⚪ 10 noise (<±5%) · 0 without baseline
> Compared against main `1c580e5` benchmarked on **this same runner**, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.
[Dashboard](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/28077835125)
| | 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% |
<details><summary>Baseline details</summary>
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% |
</details>
<details><summary>Raw CSV</summary>
```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
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): extract WebSocket message types into types/ws [texera]
Posted by "bobbai00 (via GitHub)" <gi...@apache.org>.
bobbai00 commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3487547323
##########
agent-service/src/server.ts:
##########
@@ -523,21 +492,23 @@ export function buildApp() {
return;
}
- let msg: WsMessage;
+ let msg: WsClientRequest;
try {
- msg = typeof messageData === "string" ? JSON.parse(messageData) : (messageData as WsMessage);
+ msg = typeof messageData === "string" ? JSON.parse(messageData) : (messageData as WsClientRequest);
} catch {
ws.send(JSON.stringify({ type: "error", error: "Invalid message format" }));
return;
}
- if (msg.type === "stop") {
- agent.stop();
- broadcastToAgent(agentId, { type: "state", state: "STOPPING" });
+ if (msg.type === "command") {
+ if (msg.commandType === "stop") {
+ agent.stop();
+ broadcastToAgent(agentId, { type: "status", state: AgentState.STOPPING });
+ }
return;
}
Review Comment:
fixed
##########
agent-service/src/types/ws/server.ts:
##########
@@ -0,0 +1,110 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Server -> client WebSocket frames for this service's protocol
+// (`/agents/:id/react`). Modeled as a discriminated union on `type`, so each
+// message kind declares exactly the fields it sends.
+
+import type { AgentState, ReActStep } from "../agent";
+import type { WorkflowContent } from "../workflow";
+
+/**
+ * Wire projection of one operator's execution result, summarized for the
+ * client: counts and a small record sample instead of full payloads.
+ */
+export interface OperatorResultSummaryWs {
+ state: string;
+ inputTuples: number;
+ outputTuples: number;
+ inputPortShapes?: { portIndex: number; rows: number; columns: number }[];
+ outputColumns?: number;
+ error?: string;
+ warnings?: string[];
+ consoleLogCount?: number;
+ totalRowCount?: number;
+ sampleRecords?: Record<string, unknown>[];
+ resultStatistics?: Record<string, string>;
+}
+
+/** Per-operator result summaries, keyed by operator id. */
+type OperatorResults = Record<string, OperatorResultSummaryWs>;
+
+/** Shared discriminator base; every server frame sets a unique `type`. */
+interface WsServerMessageBase {
+ type: "snapshot" | "step" | "status" | "error" | "headChange";
+}
+
+/**
+ * Full state pushed once when a client connects: the agent's current lifecycle
+ * state, the complete step list, and the HEAD pointer. Operator results are not
+ * included — they are pulled on demand via `GET /operator-results`.
+ */
+export interface WsServerSnapshotMessage extends WsServerMessageBase {
+ type: "snapshot";
+ state: AgentState;
+ steps: ReActStep[];
+ headId: string;
+}
+
+/**
+ * A single ReAct step, streamed live as the agent runs.
+ */
+export interface WsServerStepMessage extends WsServerMessageBase {
+ type: "step";
+ step: ReActStep;
+}
+
+/**
+ * An agent lifecycle transition (e.g. GENERATING when a run starts, the resting
+ * state when it ends, STOPPING on stop).
+ */
+export interface WsServerStatusMessage extends WsServerMessageBase {
+ type: "status";
+ state: AgentState;
+}
+
+/** An error surfaced to the client (agent not found, bad request, failed run). */
+export interface WsServerErrorMessage extends WsServerMessageBase {
+ type: "error";
+ error: string;
+}
+
+/**
+ * Emitted after a checkout: HEAD moved, carrying the full step list and the
+ * workflow snapshot at the new head.
+ *
+ * @deprecated Redundant and unused — the checkout flow that produces this frame
+ * is unreachable in the product (nothing invokes the client's `checkoutStep()`).
+ * Scheduled for removal (see #5930); do not build new code on it.
+ */
+export interface WsServerHeadChangeMessage extends WsServerMessageBase {
+ type: "headChange";
+ headId: string;
+ steps: ReActStep[];
+ workflowContent?: WorkflowContent;
+ operatorResults: OperatorResults;
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): centralize types, endpoint config, and auth module in agent-service [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3448994813
##########
agent-service/src/types/ws.ts:
##########
@@ -0,0 +1,55 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// WebSocket frames for this service's own protocol (/agents/:id/react):
+// inbound client messages and the outbound streaming updates it pushes back.
+
+import type { ReActStep } from "./agent";
+import type { WorkflowContent } from "./workflow";
+
+export interface WsMessage {
+ type: "message" | "stop";
Review Comment:
rename to `prompt` and `kill-command`
##########
agent-service/src/types/ws.ts:
##########
@@ -0,0 +1,55 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// WebSocket frames for this service's own protocol (/agents/:id/react):
+// inbound client messages and the outbound streaming updates it pushes back.
+
+import type { ReActStep } from "./agent";
+import type { WorkflowContent } from "./workflow";
+
+export interface WsMessage {
+ type: "message" | "stop";
+ content?: string;
+ messageSource?: "chat" | "feedback";
+}
+
+export interface OperatorResultSummaryWs {
+ state: string;
+ inputTuples: number;
+ outputTuples: number;
+ inputPortShapes?: { portIndex: number; rows: number; columns: number }[];
Review Comment:
inputPortsDataShape?
##########
agent-service/src/types/ws.ts:
##########
@@ -0,0 +1,55 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// WebSocket frames for this service's own protocol (/agents/:id/react):
+// inbound client messages and the outbound streaming updates it pushes back.
+
+import type { ReActStep } from "./agent";
+import type { WorkflowContent } from "./workflow";
+
+export interface WsMessage {
+ type: "message" | "stop";
+ content?: string;
+ messageSource?: "chat" | "feedback";
+}
+
+export interface OperatorResultSummaryWs {
+ state: string;
+ inputTuples: number;
+ outputTuples: number;
+ inputPortShapes?: { portIndex: number; rows: number; columns: number }[];
+ outputColumns?: number;
+ error?: string;
+ warnings?: string[];
+ consoleLogCount?: number;
+ totalRowCount?: number;
+ sampleRecords?: Record<string, unknown>[];
+ resultStatistics?: Record<string, string>;
+}
+
+export interface WsOutgoingMessage {
+ type: "step" | "state" | "error" | "complete" | "init" | "headChange";
+ step?: ReActStep;
+ state?: string;
+ error?: string;
+ steps?: ReActStep[];
+ headId?: string;
+ operatorResults?: Record<string, OperatorResultSummaryWs>;
+ workflowContent?: WorkflowContent;
+}
Review Comment:
you need to seperate them into their own definition. don't mix them together.
##########
agent-service/src/types/api.ts:
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Wire DTOs: request/response bodies exchanged with backend services and the
+// WebSocket frames this service sends to its own clients. Distinct from domain
+// types (workflow.ts, execution.ts, agent.ts) which model in-memory state.
+
+import type { WorkflowContent, OperatorPortSchemaMap } from "./workflow";
+import type { ReActStep } from "./agent";
+
+// --- Dashboard Service: workflow persistence ---
+
+export interface Workflow {
+ wid: number;
+ name: string;
+ description?: string;
+ content: WorkflowContent;
+ creationTime?: number;
+ lastModifiedTime?: number;
+ isPublished?: boolean;
+}
+
+export interface WorkflowPersistRequest {
+ wid?: number;
+ name: string;
+ description?: string;
+ content: string;
+ isPublic?: boolean;
+}
+
+// --- Workflow Compiling Service ---
+
+export interface WorkflowFatalError {
+ message: string;
+ details: string;
+ operatorId: string;
+ workerId: string;
+ type: { name: string };
+ timestamp: { nanos: number; seconds: number };
+}
+
+export interface WorkflowCompilationResponse {
+ physicalPlan?: any;
+ operatorOutputSchemas: Record<string, OperatorPortSchemaMap>;
+ operatorErrors: Record<string, WorkflowFatalError>;
+}
Review Comment:
let's make sure this is aligned with our gui naming
##########
agent-service/src/types/ws.ts:
##########
@@ -0,0 +1,55 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// WebSocket frames for this service's own protocol (/agents/:id/react):
+// inbound client messages and the outbound streaming updates it pushes back.
+
+import type { ReActStep } from "./agent";
+import type { WorkflowContent } from "./workflow";
+
+export interface WsMessage {
+ type: "message" | "stop";
Review Comment:
inherit on interface, define `kill-command` seperately
##########
agent-service/src/types/ws.ts:
##########
@@ -0,0 +1,55 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// WebSocket frames for this service's own protocol (/agents/:id/react):
+// inbound client messages and the outbound streaming updates it pushes back.
+
+import type { ReActStep } from "./agent";
+import type { WorkflowContent } from "./workflow";
+
+export interface WsMessage {
+ type: "message" | "stop";
+ content?: string;
+ messageSource?: "chat" | "feedback";
+}
+
+export interface OperatorResultSummaryWs {
+ state: string;
+ inputTuples: number;
+ outputTuples: number;
+ inputPortShapes?: { portIndex: number; rows: number; columns: number }[];
Review Comment:
should this be output?
##########
agent-service/src/auth/jwt.test.ts:
##########
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { describe, expect, test } from "bun:test";
+import { extractUserFromToken, validateToken, createAuthHeaders } from "./jwt";
+
+function makeToken(payload: Record<string, unknown>): string {
+ const encode = (o: Record<string, unknown>) => Buffer.from(JSON.stringify(o)).toString("base64");
+ return `${encode({ alg: "none", typ: "JWT" })}.${encode(payload)}.signature`;
Review Comment:
check the encoding on the sign side (Auth service)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): extract WebSocket message types into types/ws [texera]
Posted by "bobbai00 (via GitHub)" <gi...@apache.org>.
bobbai00 merged PR #5751:
URL: https://github.com/apache/texera/pull/5751
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): centralize types, endpoint config, and auth module [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #5751:
URL: https://github.com/apache/texera/pull/5751#issuecomment-4726384948
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/5751?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 53.05%. Comparing base ([`07724d5`](https://app.codecov.io/gh/apache/texera/commit/07724d54fda8b1425489fa31143c5a15738578ba?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`676cc26`](https://app.codecov.io/gh/apache/texera/commit/676cc266356ce462a4f1087bfbcc05ed1414a30c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 14 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #5751 +/- ##
============================================
+ Coverage 53.04% 53.05% +0.01%
Complexity 2654 2654
============================================
Files 1094 1094
Lines 42295 42296 +1
Branches 4545 4545
============================================
+ Hits 22434 22439 +5
+ Misses 18545 18541 -4
Partials 1316 1316
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/5751/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/5751/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.44% <ø> (ø)` | | Carriedforward from [07724d5](https://app.codecov.io/gh/apache/texera/commit/07724d54fda8b1425489fa31143c5a15738578ba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/5751/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.50% <100.00%> (+0.14%)` | :arrow_up: | |
| [amber](https://app.codecov.io/gh/apache/texera/pull/5751/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.34% <ø> (ø)` | | Carriedforward from [07724d5](https://app.codecov.io/gh/apache/texera/commit/07724d54fda8b1425489fa31143c5a15738578ba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/5751/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `1.65% <ø> (ø)` | | Carriedforward from [07724d5](https://app.codecov.io/gh/apache/texera/commit/07724d54fda8b1425489fa31143c5a15738578ba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/5751/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.71% <ø> (ø)` | | Carriedforward from [07724d5](https://app.codecov.io/gh/apache/texera/commit/07724d54fda8b1425489fa31143c5a15738578ba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/5751/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.06% <ø> (ø)` | | Carriedforward from [07724d5](https://app.codecov.io/gh/apache/texera/commit/07724d54fda8b1425489fa31143c5a15738578ba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/5751/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `47.93% <ø> (ø)` | | Carriedforward from [07724d5](https://app.codecov.io/gh/apache/texera/commit/07724d54fda8b1425489fa31143c5a15738578ba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/5751/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `89.77% <ø> (ø)` | | Carriedforward from [07724d5](https://app.codecov.io/gh/apache/texera/commit/07724d54fda8b1425489fa31143c5a15738578ba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [python](https://app.codecov.io/gh/apache/texera/pull/5751/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.73% <ø> (ø)` | | Carriedforward from [07724d5](https://app.codecov.io/gh/apache/texera/commit/07724d54fda8b1425489fa31143c5a15738578ba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/5751/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.69% <ø> (ø)` | | Carriedforward from [07724d5](https://app.codecov.io/gh/apache/texera/commit/07724d54fda8b1425489fa31143c5a15738578ba?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/5751?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): extract WebSocket message types into types/ws [texera]
Posted by "bobbai00 (via GitHub)" <gi...@apache.org>.
bobbai00 commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3487554865
##########
frontend/src/app/workspace/service/agent/agent.service.ts:
##########
@@ -1311,7 +1274,10 @@ export class AgentService {
}
/**
- * Fetch operator results from the backend (fallback if WebSocket data not available).
+ * Pull the agent's latest operator result summaries from the backend and push
+ * them to `operatorResultSummaries$`. Called on demand when the UI needs to
+ * show results (e.g. opening an operator's popover); results are no longer
+ * pushed over the WebSocket.
*/
Review Comment:
Done — removed operatorResults from the headChange frame entirely (the WsServerHeadChangeEvent type, the /checkout broadcast, and the frontend handler), so results are now pull-only over HTTP and the doc
comment holds.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): extract WebSocket message types into types/ws [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3487588533
##########
agent-service/src/server.ts:
##########
@@ -464,7 +433,7 @@ function getOperatorResultSummaries(agent: TexeraAgent): Record<string, Operator
return results;
}
-function broadcastToAgent(agentId: string, message: WsOutgoingMessage): void {
+function broadcastToAgent(agentId: string, message: WsServerEvent): void {
Review Comment:
update `message` name as well?
##########
agent-service/src/server.ts:
##########
@@ -504,14 +473,13 @@ export function buildApp() {
agent.addWebsocket(ws);
- const initMessage: WsOutgoingMessage = {
- type: "init",
+ const snapshotMessage: WsServerEvent = {
+ type: "snapshot",
Review Comment:
still has type?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): extract WebSocket message types into types/ws [texera]
Posted by "bobbai00 (via GitHub)" <gi...@apache.org>.
bobbai00 commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3487668482
##########
agent-service/src/server.ts:
##########
@@ -504,14 +473,13 @@ export function buildApp() {
agent.addWebsocket(ws);
- const initMessage: WsOutgoingMessage = {
- type: "init",
+ const snapshotMessage: WsServerEvent = {
+ type: "snapshot",
Review Comment:
it has to as WS need to distinguish between jsons
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): centralize types, endpoint config, and auth module [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3426022416
##########
agent-service/src/auth/jwt.test.ts:
##########
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { describe, expect, test } from "bun:test";
+import { extractUserFromToken, validateToken, createAuthHeaders } from "./jwt";
Review Comment:
`extractBearerToken` is used by `server.ts` for auth, but the new JWT unit tests don't cover it. Adding a few cases (valid bearer, wrong scheme, missing token) would protect this parsing logic from regressions.
##########
agent-service/src/auth/jwt.test.ts:
##########
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { describe, expect, test } from "bun:test";
+import { extractUserFromToken, validateToken, createAuthHeaders } from "./jwt";
+
+function makeToken(payload: Record<string, unknown>): string {
+ const encode = (o: Record<string, unknown>) => Buffer.from(JSON.stringify(o)).toString("base64");
+ return `${encode({ alg: "none", typ: "JWT" })}.${encode(payload)}.signature`;
Review Comment:
The JWT helper tests currently build tokens with base64 encoding, but the service (e.g., `server.test.ts`) uses base64url for JWT segments. Using base64url here would better match real tokens and would catch decoding issues around `-`/`_` characters and padding.
##########
agent-service/src/auth/jwt.test.ts:
##########
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { describe, expect, test } from "bun:test";
+import { extractUserFromToken, validateToken, createAuthHeaders } from "./jwt";
+
+function makeToken(payload: Record<string, unknown>): string {
+ const encode = (o: Record<string, unknown>) => Buffer.from(JSON.stringify(o)).toString("base64");
+ return `${encode({ alg: "none", typ: "JWT" })}.${encode(payload)}.signature`;
+}
+
+const nowSeconds = () => Math.floor(Date.now() / 1000);
+
+describe("extractUserFromToken", () => {
+ test("maps JWT claims onto a UserInfo", () => {
+ const token = makeToken({ userId: 7, sub: "alice", email: "alice@example.com", role: "ADMIN" });
+ expect(extractUserFromToken(token)).toEqual({ uid: 7, name: "alice", email: "alice@example.com", role: "ADMIN" });
+ });
+
+ test("defaults missing email and role", () => {
+ const token = makeToken({ userId: 1, sub: "bob" });
+ expect(extractUserFromToken(token)).toEqual({ uid: 1, name: "bob", email: "", role: "REGULAR" });
+ });
+
+ test("throws on a malformed token", () => {
+ expect(() => extractUserFromToken("not-a-jwt")).toThrow("Failed to decode JWT");
+ });
+});
+
+describe("validateToken", () => {
+ test("accepts a token expiring in the future", () => {
+ expect(validateToken(makeToken({ sub: "a", exp: nowSeconds() + 3600 }))).toBe(true);
+ });
+
+ test("rejects an expired token", () => {
+ expect(validateToken(makeToken({ sub: "a", exp: nowSeconds() - 3600 }))).toBe(false);
+ });
+
+ test("treats a token without exp as valid", () => {
+ expect(validateToken(makeToken({ sub: "a" }))).toBe(true);
+ });
+
+ test("rejects a malformed token", () => {
+ expect(validateToken("garbage")).toBe(false);
+ });
+});
+
+describe("createAuthHeaders", () => {
+ test("builds bearer auth headers", () => {
+ expect(createAuthHeaders("tok")).toEqual({ Authorization: "Bearer tok", "Content-Type": "application/json" });
+ });
+});
Review Comment:
Add basic coverage for `extractBearerToken` (valid bearer, invalid scheme, missing token) so the auth header parsing behavior is pinned down alongside the other JWT helpers.
##########
agent-service/src/types/api.ts:
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Wire DTOs: request/response bodies exchanged with backend services and the
+// WebSocket frames this service sends to its own clients. Distinct from domain
+// types (workflow.ts, execution.ts, agent.ts) which model in-memory state.
+
+import type { WorkflowContent, OperatorPortSchemaMap } from "./workflow";
+import type { ReActStep } from "./agent";
+
+// --- Dashboard Service: workflow persistence ---
+
+export interface Workflow {
+ wid: number;
+ name: string;
+ description?: string;
+ content: WorkflowContent;
+ creationTime?: number;
+ lastModifiedTime?: number;
+ isPublished?: boolean;
+}
+
+export interface WorkflowPersistRequest {
+ wid?: number;
+ name: string;
+ description?: string;
+ content: string;
+ isPublic?: boolean;
+}
+
+// --- Workflow Compiling Service ---
+
+export interface WorkflowFatalError {
+ message: string;
+ details: string;
+ operatorId: string;
+ workerId: string;
+ type: { name: string };
+ timestamp: { nanos: number; seconds: number };
+}
+
+export interface WorkflowCompilationResponse {
+ physicalPlan?: any;
+ operatorOutputSchemas: Record<string, OperatorPortSchemaMap>;
+ operatorErrors: Record<string, WorkflowFatalError>;
+}
Review Comment:
`WorkflowFatalError` here has the *frontend WebSocket* shape (message/details/workerId/type.name/timestamp), but this section is labeled "Workflow Compiling Service" and the existing compile client (`src/api/compile-api.ts`) uses a different/simpler error shape. Keeping two incompatible `WorkflowFatalError` DTOs will be confusing and can lead to incorrect imports later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(agent-service): extract WebSocket message types into types/ws [texera]
Posted by "bobbai00 (via GitHub)" <gi...@apache.org>.
bobbai00 commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3470462347
##########
agent-service/src/types/ws.ts:
##########
@@ -0,0 +1,55 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// WebSocket frames for this service's own protocol (/agents/:id/react):
+// inbound client messages and the outbound streaming updates it pushes back.
+
+import type { ReActStep } from "./agent";
+import type { WorkflowContent } from "./workflow";
+
+export interface WsMessage {
+ type: "message" | "stop";
+ content?: string;
+ messageSource?: "chat" | "feedback";
+}
+
+export interface OperatorResultSummaryWs {
+ state: string;
+ inputTuples: number;
+ outputTuples: number;
+ inputPortShapes?: { portIndex: number; rows: number; columns: number }[];
Review Comment:
this will be refactored in later's PR #5927
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org