[go_router] Fix chained top-level redirects not being fully resolved (#11108)

Fixes flutter/flutter#178984

The `onEnter` refactor (commit `9ec29b6d23`, PR #8339) split the unified `redirect()` method into `applyTopLegacyRedirect()` (top-level, runs once) and `redirect()` (route-level only). This introduced two regressions:

1. **Top-level redirect chains broken**: `applyTopLegacyRedirect()` returned after one hop — a chain like `/ → /a → /b` stopped at `/a` instead of resolving to `/b`.
2. **Route-level → top-level chains broken**: `processRouteLevelRedirect()` recursed into `redirect()` without re-evaluating the top-level redirect on the new location.

### Fix

Unify top-level and route-level redirects back into `redirect()` as the single entry point for all redirect processing, while keeping the separation of concerns via `applyTopLegacyRedirect()` as an internal helper.

> **Note:** An earlier revision of this PR (v1) kept the split but added recursive calls in both methods. Based on [reviewer feedback](#11108 (comment)), the approach was refactored to move `applyTopLegacyRedirect()` inside `redirect()` so the caller (`parseRouteInformationWithDependencies`) doesn't need to know about the two-phase process.

#### Before (broken)

```mermaid
sequenceDiagram
    participant Parser as parseRouteInformationWithDependencies
    participant TopRedir as applyTopLegacyRedirect
    participant Redir as redirect (route-level only)

    Parser->>TopRedir: / (one hop only)
    TopRedir-->>Parser: /a (stops here ✗)
    Parser->>Redir: /a
    Redir->>Redir: route-level redirects
    Note right of Redir: No top-level re-evaluation
    Redir-->>Parser: result
```

#### After (fix)

```mermaid
sequenceDiagram
    participant Parser as parseRouteInformationWithDependencies
    participant Redir as redirect (unified)
    participant TopRedir as applyTopLegacyRedirect

    Parser->>Redir: / (initial matches)
    Redir->>TopRedir: / → top-level redirect chain
    TopRedir->>TopRedir: / → /a → /b (self-recursive)
    TopRedir-->>Redir: /b (fully resolved ✓)
    Redir->>Redir: route-level redirects on /b
    Note right of Redir: If route-level changes location,<br/>recurse → top-level re-evaluated
    Redir-->>Parser: final result
```

#### Key changes

- **`configuration.dart` — `redirect()`**: Now calls `applyTopLegacyRedirect()` first at every cycle, then processes route-level redirects on the post-top-level result. Route-level `_processRouteLevelRedirects` extracted as a helper.
- **`configuration.dart` — `applyTopLegacyRedirect()`**: Self-recursive to fully resolve top-level chains. No functional change from v1.
- **`parser.dart` — `parseRouteInformationWithDependencies()`**: Simplified — no longer calls `applyTopLegacyRedirect` separately. Just passes initial matches to `_navigate()`.
- **`parser.dart` — `_navigate()`**: Removed `preSharedHistory` parameter. Added `context.mounted` guard in the result `.then()` to protect the relocated async boundary.

Both fixes share the existing `redirectHistory` for loop detection and respect `redirectLimit`. The `onEnter` system is completely unaffected — it runs before redirects in the pipeline.

### Tests

- **19 redirect chain tests** (`redirect_chain_test.dart`): top-level chains, async chains, loop detection (including loop-to-initial), route→top cross-type chains, **async cross-type chains** (async top→route, async route→sync top, sync route→async top), **context disposal** during async top-level and route-level redirects, redirect limit boundary (exact limit succeeds, limit+1 fails), shared limit across redirect types.
- **3 onEnter interaction tests** (`on_enter_test.dart`): onEnter called once when chains resolve, onEnter block prevents redirect evaluation.
- **Full suite**: 418 tests pass, 0 regressions.

## Pre-Review Checklist

[^1]: This PR uses `pending_changelogs/` for versioning and changelog, following the go_router batch release process.

Author: davidmigloz

packages/go_router/lib/src/configuration.dart

@@ -249,7 +249,7 @@ class RouteConfiguration {
 
   /// Legacy top level page redirect.
   ///
-  /// This is handled via [applyTopLegacyRedirect] and runs at most once per navigation.
+  /// This is handled via [applyTopLegacyRedirect] inside [redirect].
   GoRouterRedirect get topRedirect => _routingConfig.value.redirect;
 
   /// Top level page on enter.
@@ -401,80 +401,121 @@ class RouteConfiguration {
     return const <RouteMatchBase>[];
   }
 
-  /// Processes route-level redirects by returning a new [RouteMatchList] representing the new location.
+  /// Processes all redirects (top-level and route-level) and returns the
+  /// final [RouteMatchList].
   ///
-  /// This method now handles ONLY route-level redirects.
-  /// Top-level redirects are handled by applyTopLegacyRedirect.
+  /// Top-level redirects are evaluated first via [applyTopLegacyRedirect],
+  /// then route-level redirects run on the resulting match list. If a
+  /// route-level redirect changes the location, this method recurses —
+  /// re-evaluating top-level redirects on the new location naturally.
   FutureOr<RouteMatchList> redirect(
     BuildContext context,
     FutureOr<RouteMatchList> prevMatchListFuture, {
     required List<RouteMatchList> redirectHistory,
   }) {
     FutureOr<RouteMatchList> processRedirect(RouteMatchList prevMatchList) {
-      final prevLocation = prevMatchList.uri.toString();
-
-      FutureOr<RouteMatchList> processRouteLevelRedirect(
-        String? routeRedirectLocation,
-      ) {
-        if (routeRedirectLocation != null &&
-            routeRedirectLocation != prevLocation) {
-          final RouteMatchList newMatch = _getNewMatches(
-            routeRedirectLocation,
-            prevMatchList.uri,
-            redirectHistory,
-          );
+      // Step 1: Apply top-level redirect first (self-recursive for chains).
+      final FutureOr<RouteMatchList> afterTopLevel = applyTopLegacyRedirect(
+        context,
+        prevMatchList,
+        redirectHistory: redirectHistory,
+      );
 
-          if (newMatch.isError) {
-            return newMatch;
-          }
-          return redirect(context, newMatch, redirectHistory: redirectHistory);
-        }
-        return prevMatchList;
+      // Step 2: Then apply route-level redirects on the post-top-level result.
+      if (afterTopLevel is RouteMatchList) {
+        return _processRouteLevelRedirects(
+          context,
+          afterTopLevel,
+          redirectHistory,
+        );
       }
-
-      final routeMatches = <RouteMatchBase>[];
-      prevMatchList.visitRouteMatches((RouteMatchBase match) {
-        if (match.route.redirect != null) {
-          routeMatches.add(match);
+      return afterTopLevel.then<RouteMatchList>((RouteMatchList ml) {
+        if (!context.mounted) {
+          return ml;
         }
-        return true;
+        return _processRouteLevelRedirects(context, ml, redirectHistory);
       });
+    }
+
+    if (prevMatchListFuture is RouteMatchList) {
+      return processRedirect(prevMatchListFuture);
+    }
+    return prevMatchListFuture.then<RouteMatchList>(processRedirect);
+  }
 
-      try {
-        final FutureOr<String?> routeLevelRedirectResult =
-            _getRouteLevelRedirect(context, prevMatchList, routeMatches, 0);
+  /// Processes route-level redirects on [matchList].
+  ///
+  /// If a route-level redirect changes the location, recurses back into
+  /// [redirect] which will re-evaluate top-level redirects on the new path.
+  FutureOr<RouteMatchList> _processRouteLevelRedirects(
+    BuildContext context,
+    RouteMatchList matchList,
+    List<RouteMatchList> redirectHistory,
+  ) {
+    final prevLocation = matchList.uri.toString();
 
-        if (routeLevelRedirectResult is String?) {
-          return processRouteLevelRedirect(routeLevelRedirectResult);
-        }
-        return routeLevelRedirectResult
-            .then<RouteMatchList>(processRouteLevelRedirect)
-            .catchError((Object error) {
-              final GoException goException = error is GoException
-                  ? error
-                  : GoException('Exception during route redirect: $error');
-              return _errorRouteMatchList(
-                prevMatchList.uri,
-                goException,
-                extra: prevMatchList.extra,
-              );
-            });
-      } catch (exception) {
-        final GoException goException = exception is GoException
-            ? exception
-            : GoException('Exception during route redirect: $exception');
-        return _errorRouteMatchList(
-          prevMatchList.uri,
-          goException,
-          extra: prevMatchList.extra,
+    FutureOr<RouteMatchList> processRouteLevelRedirect(
+      String? routeRedirectLocation,
+    ) {
+      if (routeRedirectLocation != null &&
+          routeRedirectLocation != prevLocation) {
+        final RouteMatchList newMatch = _getNewMatches(
+          routeRedirectLocation,
+          matchList.uri,
+          redirectHistory,
         );
+
+        if (newMatch.isError) {
+          return newMatch;
+        }
+        // Recurse into redirect — top-level will be re-evaluated at the
+        // top of the next cycle.
+        return redirect(context, newMatch, redirectHistory: redirectHistory);
       }
+      return matchList;
     }
 
-    if (prevMatchListFuture is RouteMatchList) {
-      return processRedirect(prevMatchListFuture);
+    final routeMatches = <RouteMatchBase>[];
+    matchList.visitRouteMatches((RouteMatchBase match) {
+      if (match.route.redirect != null) {
+        routeMatches.add(match);
+      }
+      return true;
+    });
+
+    try {
+      final FutureOr<String?> routeLevelRedirectResult = _getRouteLevelRedirect(
+        context,
+        matchList,
+        routeMatches,
+        0,
+      );
+
+      if (routeLevelRedirectResult is String?) {
+        return processRouteLevelRedirect(routeLevelRedirectResult);
+      }
+      return routeLevelRedirectResult
+          .then<RouteMatchList>(processRouteLevelRedirect)
+          .catchError((Object error) {
+            final GoException goException = error is GoException
+                ? error
+                : GoException('Exception during route redirect: $error');
+            return _errorRouteMatchList(
+              matchList.uri,
+              goException,
+              extra: matchList.extra,
+            );
+          });
+    } catch (exception) {
+      final GoException goException = exception is GoException
+          ? exception
+          : GoException('Exception during route redirect: $exception');
+      return _errorRouteMatchList(
+        matchList.uri,
+        goException,
+        extra: matchList.extra,
+      );
     }
-    return prevMatchListFuture.then<RouteMatchList>(processRedirect);
   }
 
   /// Applies the legacy top-level redirect to [prevMatchList] and returns the
@@ -484,9 +525,10 @@ class RouteConfiguration {
   ///
   /// Shares [redirectHistory] with later route-level redirects for proper loop detection.
   ///
-  /// Note: Legacy top-level redirect is executed at most once per navigation,
-  /// before route-level redirects. It does not re-evaluate if it redirects to
-  /// a location that would itself trigger another top-level redirect.
+  /// Recursively re-evaluates the top-level redirect when it produces a new
+  /// location, so that top-level redirect chains (e.g. `/` → `/a` → `/b`) are
+  /// fully resolved. Loop detection and redirect limit are enforced via the
+  /// shared [redirectHistory].
   FutureOr<RouteMatchList> applyTopLegacyRedirect(
     BuildContext context,
     RouteMatchList prevMatchList, {
@@ -500,7 +542,15 @@ class RouteConfiguration {
           prevMatchList.uri,
           redirectHistory,
         );
-        return newMatch;
+        if (newMatch.isError) {
+          return newMatch;
+        }
+        // Recursively re-evaluate the top-level redirect on the new location.
+        return applyTopLegacyRedirect(
+          context,
+          newMatch,
+          redirectHistory: redirectHistory,
+        );
       }
       return prevMatchList;
     }

packages/go_router/lib/src/parser.dart

@@ -111,54 +111,22 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> {
     );
 
     // ALL navigation types now go through onEnter, and if allowed,
-    // legacy top-level redirect runs, then route-level redirects.
+    // redirect() handles both top-level and route-level redirects.
     return _onEnterHandler.handleTopOnEnter(
       context: context,
       routeInformation: effectiveRoute,
       infoState: infoState,
       onCanEnter: () {
-        // Compose legacy top-level redirect here (one shared cycle/history).
         final RouteMatchList initialMatches = configuration.findMatch(
           effectiveRoute.uri,
           extra: infoState.extra,
         );
-        final redirectHistory = <RouteMatchList>[];
-
-        final FutureOr<RouteMatchList> afterLegacy = configuration
-            .applyTopLegacyRedirect(
-              context,
-              initialMatches,
-              redirectHistory: redirectHistory,
-            );
-
-        if (afterLegacy is RouteMatchList) {
-          return _navigate(
-            effectiveRoute,
-            context,
-            infoState,
-            startingMatches: afterLegacy,
-            preSharedHistory: redirectHistory,
-          );
-        }
-        return afterLegacy.then((RouteMatchList ml) {
-          if (!context.mounted) {
-            return _lastMatchList ??
-                _OnEnterHandler._errorRouteMatchList(
-                  effectiveRoute.uri,
-                  GoException(
-                    'Navigation aborted because the router context was disposed.',
-                  ),
-                  extra: infoState.extra,
-                );
-          }
-          return _navigate(
-            effectiveRoute,
-            context,
-            infoState,
-            startingMatches: ml,
-            preSharedHistory: redirectHistory,
-          );
-        });
+        return _navigate(
+          effectiveRoute,
+          context,
+          infoState,
+          startingMatches: initialMatches,
+        );
       },
       onCanNotEnter: () {
         // If blocked, stay on the current route by restoring the last known good configuration.
@@ -198,19 +166,17 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> {
     BuildContext context,
     RouteInfoState infoState, {
     FutureOr<RouteMatchList>? startingMatches,
-    List<RouteMatchList>? preSharedHistory,
   }) {
     // If we weren't given matches, compute them here. The URI has already been
     // normalized at the parser entry point.
     final FutureOr<RouteMatchList> baseMatches =
         startingMatches ??
         configuration.findMatch(routeInformation.uri, extra: infoState.extra);
 
-    // History may be shared with the legacy step done in onEnter.
-    final List<RouteMatchList> redirectHistory =
-        preSharedHistory ?? <RouteMatchList>[];
+    final redirectHistory = <RouteMatchList>[];
 
-    FutureOr<RouteMatchList> afterRouteLevel(FutureOr<RouteMatchList> base) {
+    // redirect() handles both top-level and route-level redirects.
+    FutureOr<RouteMatchList> applyRedirects(FutureOr<RouteMatchList> base) {
       if (base is RouteMatchList) {
         return configuration.redirect(
           context,
@@ -222,27 +188,33 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> {
         if (!context.mounted) {
           return ml;
         }
-        final FutureOr<RouteMatchList> step = configuration.redirect(
+        return configuration.redirect(
           context,
           ml,
           redirectHistory: redirectHistory,
         );
-        return step;
       });
     }
 
-    // Only route-level redirects from here on out.
-    final FutureOr<RouteMatchList> redirected = afterRouteLevel(baseMatches);
+    final FutureOr<RouteMatchList> redirected = applyRedirects(baseMatches);
 
     return debugParserFuture =
         (redirected is RouteMatchList
                 ? SynchronousFuture<RouteMatchList>(redirected)
                 : redirected)
             .then((RouteMatchList matchList) {
+              // Guard against context disposal during async redirects.
+              if (!context.mounted) {
+                return _lastMatchList ??
+                    _OnEnterHandler._errorRouteMatchList(
+                      routeInformation.uri,
+                      GoException(
+                        'Navigation aborted because the router context was disposed.',
+                      ),
+                      extra: infoState.extra,
+                    );
+              }
               if (matchList.isError && onParserException != null) {
-                if (!context.mounted) {
-                  return matchList;
-                }
                 return onParserException!(context, matchList);
               }
 

packages/go_router/pending_changelogs/fix_chained_redirect_regression.yaml

@@ -0,0 +1,4 @@
+changelog: |
+  - Fixes chained top-level redirects not being fully resolved (e.g. `/ → /a → /b` stopping at `/a`).
+  - Fixes route-level redirects not triggering top-level redirect re-evaluation on the new location.
+version: patch