Skip to content

fix(core): Shift existing rules on create to avoid order conflict#28722

Merged
cstuncsik merged 4 commits into
masterfrom
iam-560-fix-order-conflict-when-adding-instance-role-mapping-rule
Apr 23, 2026
Merged

fix(core): Shift existing rules on create to avoid order conflict#28722
cstuncsik merged 4 commits into
masterfrom
iam-560-fix-order-conflict-when-adding-instance-role-mapping-rule

Conversation

@cstuncsik
Copy link
Copy Markdown
Contributor

@cstuncsik cstuncsik commented Apr 20, 2026

Summary

Fixes a 409 conflict when creating a role mapping rule that lands on an already-occupied order. The backend now splices the new rule into the requested position and atomically renumbers the sequence using the existing two-phase applyOrder transaction. Omitting order appends the rule; out-of-range values clamp to the end of the list.

How to verify in the browser

Prerequisites: n8n running locally with SAML or OIDC provisioning licensed (N8N_LICENSE_ACTIVATION_KEY or dev license), and an SSO config that uses Rules in n8n for role assignment.

  1. Sign in as an owner and open Settings → SSO → Provisioning.
  2. Under Instance role rules, click Add rule, set an expression (e.g. $claims.admin === true) and a role, then save the SSO config.
  3. Click Add rule again — add another instance rule on top of the one you just created.
  4. Save the SSO config.
    • Before this PR: red error toast "A role mapping rule already exists with type 'instance' and order 0. Use a different order value."
    • After this PR: the rule is saved. Reload the page and confirm both rules are listed in the intended order with sequential priorities (1., 2.).
  5. Drag a new local rule above an existing one, save, reload — the dragged rule should land at the top and the other rules should shift down without errors.
  6. Delete the middle rule, add a new one, save — no conflict, priorities remain contiguous.
  7. Repeat steps 2–4 for Project role rules (if enabled) — same behaviour.

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/IAM-560

Review / Merge checklist

  • I have seen this code, I have run this code, and I take responsibility for this code.
  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with Backport to Beta, Backport to Stable, or Backport to v1 (if the PR is an urgent fix that needs to be backported)

🤖 PR Summary generated by AI

Creating a role mapping rule at an occupied order previously failed
with a 409 conflict. The create path now splices the new rule into
the target position and uses the existing two-phase applyOrder
transaction to renumber contiguously. The create DTO accepts an
optional order (append when omitted). Frontend omits order on create
and serialises create calls so the backend appends in the user's
intended order.
@n8n-assistant n8n-assistant Bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Apr 20, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Bundle Report

Changes will decrease total bundle size by 6.91kB (-0.02%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
editor-ui-esm 45.77MB -6.91kB (-0.02%) ⬇️

Affected Assets, Files, and Routes:

view changes for bundle: editor-ui-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
assets/constants-*.js 240 bytes 3.14MB 0.01%
assets/index-*.js -456 bytes 1.31MB -0.03%
assets/ParameterInputList-*.js 405 bytes 1.27MB 0.03%
assets/users.store-*.js 2.99kB 1.06MB 0.28%
assets/core-*.js -135 bytes 628.2kB -0.02%
assets/useCanvasMapping-*.js 451 bytes 463.75kB 0.1%
assets/InstanceAiView-*.js 835 bytes 352.42kB 0.24%
assets/RunData-*.js 195 bytes 346.06kB 0.06%
assets/ParameterInputList-*.css -158 bytes 208.11kB -0.08%
assets/table-*.js -25.24kB 153.35kB -14.13%
assets/usePostMessageHandler-*.js 826 bytes 137.88kB 0.6%
assets/NodeView-*.js 200 bytes 137.44kB 0.15%
assets/useRootStore-*.js -175 bytes 131.07kB -0.13%
assets/WorkflowLayout-*.js -38 bytes 127.89kB -0.03%
assets/router-*.js -7 bytes 119.81kB -0.01%
assets/canvas.eventBus-*.js 167 bytes 117.56kB 0.14%
assets/SettingsSso-*.js 641 bytes 107.01kB 0.6%
assets/NodeCreator-*.js 292 bytes 104.33kB 0.28%
assets/useCanvasOperations-*.js 83 bytes 95.46kB 0.09%
assets/VirtualSchema-*.js 38 bytes 94.5kB 0.04%
assets/NodeSettings-*.js 128 bytes 84.81kB 0.15%
assets/CanvasRunWorkflowButton-*.js 188 bytes 78.46kB 0.24%
assets/ProjectSettings-*.js 164 bytes 74.23kB 0.22%
assets/TriggerPanel-*.js 9 bytes 59.26kB 0.02%
assets/useLogsTreeExpand-*.js -54 bytes 41.19kB -0.13%
assets/NodeDetailsViewV2-*.js -18 bytes 38.01kB -0.05%
assets/usePushConnection-*.js -44 bytes 31.5kB -0.14%
assets/useRunWorkflow-*.js -44 bytes 27.82kB -0.16%
assets/checkbox-*.js (New) 26.24kB 26.24kB 100.0% 🚀
assets/assistant.store-*.js -46 bytes 19.32kB -0.24%
assets/SettingsLogStreamingView-*.js -43 bytes 17.44kB -0.25%
assets/InstanceAiOptinModal-*.js -12.93kB 11.82kB -52.25%
assets/useActions-*.js -49 bytes 10.36kB -0.47%
assets/pushConnection.store-*.js 38 bytes 10.14kB 0.38%
assets/OAuthConsentView-*.js -14 bytes 9.99kB -0.14%
assets/InstanceAiOptinModal-*.css -3.18kB 9.28kB -25.53%
assets/usePinnedData-*.js -130 bytes 9.0kB -1.42%
assets/WorkflowPreview-*.js 213 bytes 8.18kB 2.67%
assets/ContactAdministratorToInstall-*.js -44 bytes 5.87kB -0.74%
assets/useExecutionDebugging-*.js -217 bytes 5.41kB -3.86%
assets/nodeIcon-*.js -26 bytes 4.7kB -0.55%
assets/CredentialIcon-*.js 472 bytes 3.89kB 13.82% ⚠️
assets/NodeIcon-*.js 702 bytes 3.84kB 22.39% ⚠️
assets/DemoLayout-*.js 735 bytes 3.51kB 26.52% ⚠️
assets/useCalloutHelpers-*.js -48 bytes 3.39kB -1.4%
assets/useExpressionResolveCtx-*.js -55 bytes 1.63kB -3.27%

Files in assets/SettingsSso-*.js:

  • ./src/features/settings/sso/provisioning/composables/useRoleMappingRules.ts → Total Size: 3.8kB

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...so/provisioning/composables/useRoleMappingRules.ts 87.50% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

cstuncsik and others added 2 commits April 20, 2026 17:37
Adds integration coverage for inserting at a middle index (existing
tests only covered head, tail, and over-end clamping) and for the
instance/project order independence on (type, order). Adds Zod-level
tests for the create DTO, including the new non-negative order rule.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 20, 2026

Performance Comparison

Comparing currentlatest master14-day baseline

Memory consumption baseline with starter plan resources

Metric Current Latest Master Baseline (avg) vs Master vs Baseline Status
memory-heap-used-baseline 114.47 MB 114.27 MB 114.47 MB (σ 0.24) +0.2% -0.0%
memory-rss-baseline 290.73 MB 348.40 MB 289.80 MB (σ 40.90) -16.6% +0.3%

docker-stats

Metric Current Latest Master Baseline (avg) vs Master vs Baseline Status
docker-image-size-n8n 1269.76 MB 1269.76 MB 1273.60 MB (σ 10.49) +0.0% -0.3%
docker-image-size-runners 388.00 MB 386.00 MB 392.13 MB (σ 11.18) +0.5% -1.1%

Idle baseline with Instance AI module loaded

Metric Current Latest Master Baseline (avg) vs Master vs Baseline Status
instance-ai-heap-used-baseline 188.02 MB 186.50 MB 186.43 MB (σ 0.25) +0.8% +0.9% 🔴
instance-ai-rss-baseline 398.34 MB 392.12 MB 368.35 MB (σ 22.82) +1.6% +8.1% ⚠️
How to read this table
  • Current: This PR's value (or latest master if PR perf tests haven't run)
  • Latest Master: Most recent nightly master measurement
  • Baseline: Rolling 14-day average from master
  • vs Master: PR impact (current vs latest master)
  • vs Baseline: Drift from baseline (current vs rolling avg)
  • Status: ✅ within 1σ | ⚠️ 1-2σ | 🔴 >2σ regression

Omitting order on create broke reordering when a new local rule was
dragged above existing persisted rules — the backend would append
instead of respecting the user-intended position. The backend now
safely shifts existing rules on order collision, so the frontend can
always send the local order. Creates remain sequential to avoid
concurrent temp-order collisions.
@cstuncsik cstuncsik marked this pull request as ready for review April 21, 2026 07:28
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 8 files

Architecture diagram
sequenceDiagram
    participant UI as Browser / Frontend
    participant API as API Route / Controller
    participant Service as RoleMappingRuleService
    participant DB as Database (TypeORM)

    Note over UI, DB: Rule Creation & Atomic Reordering Flow

    UI->>UI: Prepare local changes (Delete, Update, Create)
    
    par Deletes & Updates
        UI->>API: DELETE /role-mapping-rule/:id
        UI->>API: PATCH /role-mapping-rule/:id
    end

    loop NEW: Sequential Create (Prevent Race Conditions)
        UI->>API: POST /role-mapping-rule { order, ... }
        API->>Service: create(dto)

        Service->>DB: Fetch existing rules for type (e.g., 'instance')
        DB-->>Service: List of rules [A(0), B(1)]

        Service->>Service: NEW: Clamp requested order to [0, length]
        Service->>Service: NEW: Calculate tempOrder (max + 1)

        Service->>DB: NEW: Insert new rule with tempOrder
        Note right of DB: Avoids unique constraint conflict<br/>before reordering
        DB-->>Service: Rule C saved

        Service->>Service: CHANGED: Splice C into ID list at target index
        
        Service->>DB: Start Transaction (applyOrder)
        Note over Service, DB: Two-phase update to renumber sequence [0...N]
        DB->>DB: Phase 1: Move rules to temporary sequence
        DB->>DB: Phase 2: Assign final sequential orders
        DB-->>Service: Commit

        Service->>DB: Reload created rule
        DB-->>Service: Rule C with final order
        Service-->>API: Return Rule
        API-->>UI: 201 Created
    end

    alt Invalid Order Input
        UI->>API: POST /role-mapping-rule { order: -1 }
        API-->>UI: 400 Bad Request (Zod Validation)
    end
Loading

@cstuncsik cstuncsik requested review from a team, BGZStephen, afitzek, guillaumejacquart and phyllis-noester and removed request for a team April 21, 2026 13:55
Copy link
Copy Markdown
Contributor

@afitzek afitzek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 22, 2026

Merging this PR will improve performance by 17.57%

⚡ 1 improved benchmark
✅ 31 untouched benchmarks
⏩ 20 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
vm: Simple Property - small data 2.2 ms 1.9 ms +17.57%

Comparing iam-560-fix-order-conflict-when-adding-instance-role-mapping-rule (88c2f68) with master (08949a1)

Open in CodSpeed

Footnotes

  1. 20 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions
Copy link
Copy Markdown
Contributor

Instance AI Workflow Eval Results

8/8 built | 8/27 passed (29%)

Workflow Build Passed
Create a workflow that handles contact form submissions via a webhook. 3/5
Get all the Linear issues created in the last 2 weeks. Filter them for 0/5
Every day, get the posts made in the past day on 3 different Slack cha 2/5
Create a form that collects: name, email, company, and interest level 2/2
Every day, fetch all open GitHub issues from repository 'acme-corp/bac 0/2
Create a workflow that receives webhook notifications with a JSON body 0/3
Fetch the latest posts from the JSONPlaceholder API (GET https://jsonp 1/3
Every hour, check the current weather for London, New York, and Tokyo 0/2
Failure details

partial-action-failure [builder_issue]

The workflow crashed entirely when the Telegram Notification node returned a 400 error ('Bad Request: chat not found'). The workflow has no error handling (e.g., no 'Continue on Fail' setting on the T

invalid-email [builder_issue]

The workflow crashed entirely when the Auto-Reply Email node encountered the invalid email address 'not-an-email'. The error 'Invalid email address (item 0)' caused the entire workflow execution to fa

happy-path [builder_issue]

The workflow errored at the Format Slack Message node with 'Invalid or unexpected token', and Post to Slack never ran. The root cause is in the Filter Last 14 Days node: it compared issue createdAt da

multi-team-creator [builder_issue]

The workflow failed due to multiple cascading issues. First, the Filter Last 14 Days node returned {__empty: true} because all 4 issues have createdAt dates in early-to-mid January 2025, and the cut

no-cross-team-issues [builder_issue]

The workflow errors at the Format Slack Message node with 'Invalid or unexpected token', producing no output and preventing the Post to Slack node from running. The error occurs despite the node code

unknown-creator [builder_issue]

The workflow crashed at the 'Format Slack Message' node with 'Invalid or unexpected token', producing no output. The root cause is a date comparison issue in 'Filter Last 14 Days': all four issues hav

api-error [builder_issue]

The workflow has no error handling configured. When the Linear API returned an authentication error, the 'Fetch Linear Issues' node threw an exception ('Authentication required: invalid or missing API

high-volume

Error: The operation was aborted due to timeout

channel-not-found [builder_issue]

The workflow crashed entirely when 'Get #product Messages' returned a 404 error. There is no error handling — no try/catch, no error branch, no continueOnFail setting on the node. The workflow has a l

insufficient-permissions [builder_issue]

The workflow crashed with 'Slack error response: not_in_channel' when Get #product Messages returned an error. No error handling exists in the workflow — there is no Try/Catch, no IF node to check for

happy-path [builder_issue]

The workflow failed to execute. The 'Split Issues' node has an empty 'fieldToSplitOut' parameter, which is required. This caused the execution to fail immediately with 'The workflow has issues and can

no-bugs [builder_issue]

The workflow failed to execute cleanly. The 'Split Issues' node has a missing required parameter ('Fields To Split Out' is empty), which caused the execution to fail with the error: 'The workflow has

high-priority [builder_issue]

The Route by Level Switch node correctly evaluated the 'high' condition and produced output on its first output port, but the Send Teams Message node never ran. The connections JSON shows that 'Route

medium-priority [builder_issue]

The Switch node ('Route by Level') correctly identified the 'medium' level and routed output to its second output (index 1, labeled 'medium'). However, the connections JSON shows 'Route by Level' has

low-priority [builder_issue]

The Switch node correctly routed the 'low' level notification to its third output (index 2), but the connections JSON shows 'Route by Level' has an empty 'main' array: "main": []. This means no node

happy-path [builder_issue]

The workflow has a critical data flow failure. The Fetch Posts HTTP Request node returned the response as a raw Buffer (binary data) instead of parsed JSON objects. As a result, the Filter Out qui Tit

all-filtered [builder_issue]

The workflow failed to handle the all-filtered-out case correctly. The Fetch Posts node returned the mock data as a raw Buffer (binary) rather than parsed JSON objects, so the Filter Out qui Titles no

happy-path [builder_issue]

The workflow failed to execute. The pre-analysis flags a builder issue: 'Log to Airtable' has an invalid Airtable Table ID — the value 'Weather Logs' is a plain text name, not a valid Airtable Table I

no-alerts [builder_issue]

The workflow failed to execute due to a builder configuration issue. The 'Log to Airtable' node has an invalid Airtable Table ID ('Weather Logs' is a display name, not a valid table ID which should be

@cstuncsik cstuncsik added this pull request to the merge queue Apr 23, 2026
Merged via the queue into master with commit c55b95f Apr 23, 2026
146 of 152 checks passed
@cstuncsik cstuncsik deleted the iam-560-fix-order-conflict-when-adding-instance-role-mapping-rule branch April 23, 2026 09:12
@n8n-assistant n8n-assistant Bot mentioned this pull request Apr 28, 2026
@n8n-assistant
Copy link
Copy Markdown
Contributor

n8n-assistant Bot commented Apr 28, 2026

Got released with n8n@2.19.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team Released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants