Skip to content

fix(diff-engine): scope addConnection duplicate check to sourceIndex (#738)#750

Open
SAY-5 wants to merge 1 commit intoczlonkowski:mainfrom
SAY-5:fix/addconnection-source-index-738
Open

fix(diff-engine): scope addConnection duplicate check to sourceIndex (#738)#750
SAY-5 wants to merge 1 commit intoczlonkowski:mainfrom
SAY-5:fix/addconnection-source-index-738

Conversation

@SAY-5
Copy link
Copy Markdown

@SAY-5 SAY-5 commented Apr 27, 2026

Closes #738.

What

validateAddConnection checked uniqueness on (source, sourceOutput, target) only, scanning the entire existing: Connection[][] array
across all sourceIndex slots. Two outputs of a Switch node fanning
into the same target node — e.g. several branches sharing a
fallback / error handler, exactly the scenario in the bug report — got
rejected with Connection already exists even though
(source, sourceIndex=N, target) wasn't actually a duplicate.

Fix

validateAddConnection now:

Tests

tests/unit/services/workflow-diff-engine.test.ts:

  • New should allow connecting different sourceIndex outputs to the same target (#738): pre-wires Slack -> HTTP Request at
    sourceIndex=0; addConnection at sourceIndex=1 is accepted; the
    resulting workflow has both indices populated.
  • New should still reject duplicate connections at the same sourceIndex (#738): re-adding the same edge at the same index still
    errors with Connection already exists and now includes
    sourceIndex=0 in the message.
  • The pre-existing should reject duplicate connections continues to
    pass — the default-sourceIndex-0 path is unchanged.

Verification

…zlonkowski#738)

Closes czlonkowski#738.

`validateAddConnection` checked uniqueness on (source, sourceOutput,
target) only, scanning the entire `existing: Connection[][]` array
across all sourceIndex slots. Two outputs of a Switch node fanning
into the same target node — e.g. several branches sharing a
fallback / error handler, the original bug report's setup — got
rejected with 'Connection already exists' even though
(source, sourceIndex=N, target) wasn't actually a duplicate.

Fix: resolve sourceIndex via resolveSmartParameters (same helper
applyAddConnection / rewire already use, which honours sourceIndex,
case=, branch=, and the numeric-sourceOutput remap from czlonkowski#537/czlonkowski#659)
and only check the existing[sourceIndex] slot. The error message
now also reports the offending sourceIndex so callers can spot a
genuine duplicate vs. an unrelated index.

Tests
- New 'should allow connecting different sourceIndex outputs to the
  same target (czlonkowski#738)': pre-wire Slack -> HTTP at index 0, addConnection
  at index 1, expect success and both indices populated.
- New 'should still reject duplicate connections at the same
  sourceIndex (czlonkowski#738)': re-add the same edge at the same index,
  expect 'Connection already exists' AND 'sourceIndex=0' in the
  message.
- Existing 'should reject duplicate connections' still passes — the
  default sourceIndex 0 path is unchanged.

Verified locally: `npx vitest run tests/unit/services/workflow-diff-engine.test.ts -t addConnection`
runs 7 passing tests; reverting the validation block to the old
(sourceIndex-blind) version makes both new czlonkowski#738 tests fail.

Signed-off-by: SAY-5 <say.apm35@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

addConnection throws "Connection already exists" when connecting multiple Switch outputs to same target node

1 participant