bobbai00 commented on code in PR #8981:
URL: https://github.com/apache/texera/pull/8981#discussion_r4318956753
##########
agent-service/src/agent/tools/workflow-crud-tools.ts:
##########
@@ -57,6 +57,47 @@ function formatInputArgs(args: Record<string, any>): string {
return `Input: ${JSON.stringify(compact)}`;
}
+/**
+ * Checks every port index and source operator in inputOperatorIds and returns
the links to create,
+ * or the first error. It doesn't change the workflow, so callers must run it
before any mutation.
+ */
+function resolveInputLinks(
+ workflowState: WorkflowState,
+ operatorId: string,
+ inputPorts: PortDescription[],
+ inputOperatorIds: Record<string, string[]> | undefined,
+ inputInfo: string
+): { links: Omit<OperatorLink, "linkID">[] } | { error: string } {
+ const links: Omit<OperatorLink, "linkID">[] = [];
+ for (const [portIndexStr, sourceOpIds] of Object.entries(inputOperatorIds ??
{})) {
+ const targetPortIdx = parseInt(portIndexStr, 10);
+ if (isNaN(targetPortIdx) || targetPortIdx < 0) {
+ return { error: `Invalid input port index: "${portIndexStr}". Must be a
non-negative integer. ${inputInfo}` };
+ }
+ if (targetPortIdx >= inputPorts.length) {
+ return {
+ error: `Input port index ${targetPortIdx} out of range. Operator
"${operatorId}" has ${inputPorts.length} input port(s). ${inputInfo}`,
+ };
+ }
+ const targetPortId = inputPorts[targetPortIdx].portID;
+
+ for (const sourceOpId of sourceOpIds) {
+ const sourceOp = workflowState.getOperator(sourceOpId);
Review Comment:
**Advisory:**
Self-references now differ between the tools: `addOperator` naming itself
gets `Source operator "op1" not found`, while `modifyOperator` with `{"0":
["op0"]}` on `op0` stores an `op0 --> op0` self-loop and reports success (both
reproduced on this head). One guard here gives both the same clear error. Fine
as a follow-up if you'd rather keep this a pure reorder.
```suggestion
if (sourceOpId === operatorId) {
return { error: `Operator "${operatorId}" cannot be its own input
source. ${inputInfo}` };
}
const sourceOp = workflowState.getOperator(sourceOpId);
```
##########
agent-service/src/agent/tools/workflow-crud-tools.ts:
##########
@@ -57,6 +57,47 @@ function formatInputArgs(args: Record<string, any>): string {
return `Input: ${JSON.stringify(compact)}`;
}
+/**
+ * Checks every port index and source operator in inputOperatorIds and returns
the links to create,
+ * or the first error. It doesn't change the workflow, so callers must run it
before any mutation.
+ */
+function resolveInputLinks(
+ workflowState: WorkflowState,
+ operatorId: string,
+ inputPorts: PortDescription[],
+ inputOperatorIds: Record<string, string[]> | undefined,
+ inputInfo: string
+): { links: Omit<OperatorLink, "linkID">[] } | { error: string } {
+ const links: Omit<OperatorLink, "linkID">[] = [];
+ for (const [portIndexStr, sourceOpIds] of Object.entries(inputOperatorIds ??
{})) {
+ const targetPortIdx = parseInt(portIndexStr, 10);
+ if (isNaN(targetPortIdx) || targetPortIdx < 0) {
+ return { error: `Invalid input port index: "${portIndexStr}". Must be a
non-negative integer. ${inputInfo}` };
+ }
Review Comment:
**Advisory:**
`parseInt` stops at the first non-digit, so `"1abc"` or `"1.5"` links port 1
instead of erroring (reproduced: `{"1abc": ["op0"]}` linked `input-1`). This is
unchanged from `main`, but this helper is now the one place keys are parsed. A
digits-only test, like the `op\d+` check at line 167, keeps the existing
message. Optional, fine as a follow-up.
```suggestion
if (!/^\d+$/.test(portIndexStr)) {
return { error: `Invalid input port index: "${portIndexStr}". Must be
a non-negative integer. ${inputInfo}` };
}
const targetPortIdx = parseInt(portIndexStr, 10);
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
