ShawnGarello opened a new pull request, #8981: URL: https://github.com/apache/texera/pull/8981
### What changes were proposed in this PR? When the agent calls `modifyOperator` with a source operator that doesn't exist, the tool reports an error, but the operator's properties and name have already changed and its input links are gone. `addOperator` has the same problem: a bad port index or source leaves the new operator (and links for earlier ports) in the workflow, and a retry with the same id then fails with "already exists". Both tools now run every input check (port index, source operator) first, through one small helper, `resolveInputLinks`, and only then add the operator, apply properties and name, or replace links. A failed call leaves the workflow as it was. Error messages and success results are unchanged. Why this approach (option A in #8979) and not snapshot/restore (B): it's a reorder with no new `WorkflowState` API, and a failed call now emits no change events, so nothing half-applied reaches the canvas or the 500 ms auto-persist. B would restore with `setWorkflowContent`, which emits no events, after the failed edit had already emitted some. I'm happy to switch to B if reviewers prefer it. The existing test "returns an error when an input port index is not a number" (from #7026) asserted the half-applied result: the failed modify had dropped the operator's input link. It now asserts that the link is kept and the operator isn't renamed. One side effect: `addOperator` naming the new operator as its own source used to create a self-loop link; it now returns the usual "Source operator ... not found" error. Out of scope: the model isn't told that the call failed (`isError` is always false for these results). I'll open a separate issue for that. ### Any related issues, documentation, discussions? Closes #8979 Backport: the bug is on both release branches (`workflow-crud-tools.ts` is identical on `release/v1.3` and `release/v1.2`), so I'd suggest both. On `release/v1.2` the spec file `workflow-crud-tools.spec.ts` doesn't exist yet, so that cherry-pick will need the test changes dropped or adapted. ### How was this PR tested? In `agent-service`, after `bun install --frozen-lockfile`: 1. Red: the first commit adds three tests that assert the workflow is unchanged after an `[ERROR]` result (one for `modifyOperator`, two for `addOperator`). Without the fix, `bun test` gives 310 pass, 3 fail: exactly those three. 2. Green: with the fix, `bun test` gives 313 pass, 0 fail (run twice). 3. With the fix reverted and the tests kept, exactly 4 fail: the three new tests and the updated #7026 test. 4. `bun run format:check` and `bun run typecheck` are clean. `bun test --coverage`: all new lines in `workflow-crud-tools.ts` are covered (226/227 lines; the one uncovered line is the existing `catch` in `deleteOperator`). ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5.5) -- 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]
