fix: cap loader redirect hops and reject cycles

[SebTardif]

What Problem This Solves

Fixes an issue where consumers whose loaders return redirect() in a cycle (A to B to A) or a long chain would hang the tab or exhaust the heap. Navigation (handleLocation) and preloadRoute / preloadLocation both followed loader redirects with no hop budget and no seen-location set.

Why This Change Was Made

Both follow paths now share one internal budget: at most 10 hops, and a location may appear only once. A cycle or an 11th hop throws a clear Error (Redirect cycle detected or Redirect hop limit of 10 exceeded) and publishes status: "error". Finite chains such as A to B to C still resolve. No public exports, types, history adapter shape, or loader signature changed.

User Impact

A misconfigured loader cycle fails fast instead of locking the UI. Finite redirect chains still land on the destination. Preload uses the same rules so warming the cache cannot spin either.

Evidence

Live node against the built dist/index.js on this branch (8c45e1b), macOS 26.6.1, Node v26.7.0:

$ node /tmp/proof-uirouter-redirect.mjs
CHAIN success /gamma gamma
CYCLE_NAV Redirect cycle detected: /alpha -> /beta -> /alpha.
CYCLE_NAV_MS 0
CYCLE_NAV_STATUS error
HOP Redirect hop limit of 10 exceeded while following /hop10 -> /hop11.
CYCLE_PRELOAD Redirect cycle detected: /alpha -> /beta -> /alpha.
PRELOAD_CHAIN gamma success { label: 'preloaded' }

Before this patch, the same A to B to A navigate grew until the process hit FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory (about 23s).

pnpm run check passed locally (format, build, typecheck, lint, 25 tests, pack/import).

The unbounded follow was introduced in f047b64 (2026-06-20, refactor: finalize router match loading). Adjacent but different: #20 (stale navigation cancellation). Same bug class in other routers: TanStack/router#8046.

Real behavior proof

• Behavior or issue addressed: Loader redirect() cycles and uncapped hop chains no longer hang navigate or preloadRoute. A to B to A throws. A to B to C still lands on C.

• Real environment tested: macOS 26.6.1 arm64, Node v26.7.0, @openclaw/uirouter built from fix/redirect-hop-limit at /tmp/oc-impl-uirouter-redirect (8c45e1b).

• Exact steps or command run after this patch:

  cd /tmp/oc-impl-uirouter-redirect
  node /tmp/proof-uirouter-redirect.mjs

• Evidence after fix: terminal output from the patched dist/index.js:

  $ node /tmp/proof-uirouter-redirect.mjs
  CHAIN success /gamma gamma
  CYCLE_NAV Redirect cycle detected: /alpha -> /beta -> /alpha.
  CYCLE_NAV_MS 0
  CYCLE_NAV_STATUS error
  HOP Redirect hop limit of 10 exceeded while following /hop10 -> /hop11.
  CYCLE_PRELOAD Redirect cycle detected: /alpha -> /beta -> /alpha.
  PRELOAD_CHAIN gamma success { label: 'preloaded' }

• Observed result after fix: A to B to C navigation and preload both resolve to gamma. A to B to A rejects in 0ms with Redirect cycle detected: /alpha -> /beta -> /alpha. and router status error. An 11-hop chain rejects with Redirect hop limit of 10 exceeded while following /hop10 -> /hop11.

• What was not tested: Browser history adapter clicks, a running OpenClaw UI shell, and custom hop limits (the cap is fixed at 10).

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: blocked before merge. Reviewed September 11, 2026, 10:49 AM ET / 14:49 UTC (Revision 17).

ClawSweeper review

What this changes

Adds shared cycle detection and a ten-hop limit to loader redirects during navigation and cache preloading, with documentation and regression tests.

Merge readiness

⛔ Blocked before merge - 3 items remain

This remains a useful, focused fix: current main and v0.1.1 still follow redirects without a bound. The supplied runtime evidence supports the implementation, and no blocking code defect was found.

Priority: P2
Reviewed head: ef429488c6398954bab38b2142185f3e23c555f7
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused implementation with relevant real-package proof and regression coverage; compatibility acceptance remains separate from patch quality.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The contributor's Node transcript exercises the built router through navigation and preload, showing successful short chains and rejection of cycles and excess hops; the proof revision's router source matches this head.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The contributor's Node transcript exercises the built router through navigation and preload, showing successful short chains and rejection of cycles and excess hops; the proof revision's router source matches this head.
Evidence reviewed	8 items	Current main still needs the fix: Both navigation and preload recursively follow loader redirects without a hop counter or visited-location guard. Latest release retains the same behavior: GitHub identifies v0.1.1 as the latest release. Its src/router.ts blob and fetched main's blob are both 3b6f2e628c952c7b86e2ce1fa731098aaf540707, so the released router also lacks this guard. Introduced implementation and compatibility boundary: The verified merge-base-to-head patch adds one shared guard, used by navigation and preload. It rejects the eleventh redirect even when that chain would otherwise terminate successfully; README and CHANGELOG describe the new ceiling.
Findings	None	None.
Security	None	None.

How this fits together

The standalone UI router consumes application route definitions, loader results, and history locations. It follows redirects and exposes active-route or cached-match state for applications to present.

flowchart TD
  A[Navigation or preload request] --> B[Route matching and loading]
  B --> C{Loader redirects?}
  C -->|No| D[Active route or cached result]
  C -->|Yes| E{Repeated location or over ten hops?}
  E -->|No| B
  E -->|Yes| F[Reject redirect chain]

Loading

Decision needed

Question	Recommendation
Should this package reject all redirect chains beyond ten hops, including previously successful finite chains?	Approve the ten-hop ceiling: Accept the documented safety limit and explicitly tell affected consumers to shorten longer redirect chains before upgrading.

Why: The implementation and runtime proof establish the behavior, but cannot establish which existing consumer chains the package must continue supporting.

Before merge

• Resolve merge risk (P1) - Existing consumers with terminating chains longer than ten redirects will receive navigation or preload errors after upgrading; acceptance of that ceiling and the guidance for affected consumers remain unresolved.
• Complete next step (P2) - Record maintainer approval of the ten-hop ceiling and upgrade guidance, or specify the alternative bounded policy required before merge.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test growth	Production +57 net lines; tests +144 net lines	Production growth implements the shared guard and its propagation through both redirect paths.
Focused regression coverage	5 tests added	Coverage includes successful navigation and preload chains, cycles in both paths, and navigation hop exhaustion.

Merge-risk options

Maintainer options:

1. Accept the documented ceiling (recommended)
   Record approval of the ten-hop ceiling and upgrade guidance for consumers whose finite chains exceed it.
2. Resolve longer-chain support first
   Pause landing until maintainers specify a compatible bounded policy if existing consumers require more than ten redirects.

Technical review

Best possible solution:

Use one bounded redirect policy for navigation and preload, with an approved ceiling and explicit upgrade guidance for longer-chain consumers.

Do we have a high-confidence way to reproduce the issue?

Yes: two loaders redirecting to each other enter unbounded follow paths on current main. Source establishes the mechanism, and the contributor supplies a before/after Node trace; this read-only review did not execute it.

Is this the best way to solve the issue?

Yes for the implementation approach: one shared guard covers both follow paths without widening the public API. The precise ten-hop compatibility policy still needs approval.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against ea06377b0e80.

Labels

Label justifications:

• P2: This is a bounded router reliability fix for cyclic or excessive loader redirects.
• merge-risk: 🚨 compatibility: The fixed ceiling changes previously successful finite chains longer than ten hops into errors.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The contributor's Node transcript exercises the built router through navigation and preload, showing successful short chains and rejection of cycles and excess hops; the proof revision's router source matches this head.
• proof: sufficient: Contributor real behavior proof is sufficient. The contributor's Node transcript exercises the built router through navigation and preload, showing successful short chains and rejection of cycles and excess hops; the proof revision's router source matches this head.

Evidence

What I checked:

• Current main still needs the fix: Both navigation and preload recursively follow loader redirects without a hop counter or visited-location guard. (src/router.ts:321, ea06377b0e80)
• Latest release retains the same behavior: GitHub identifies v0.1.1 as the latest release. Its src/router.ts blob and fetched main's blob are both 3b6f2e628c952c7b86e2ce1fa731098aaf540707, so the released router also lacks this guard. (src/router.ts, f5ce7c0d7c04)
• Introduced implementation and compatibility boundary: The verified merge-base-to-head patch adds one shared guard, used by navigation and preload. It rejects the eleventh redirect even when that chain would otherwise terminate successfully; README and CHANGELOG describe the new ceiling. (src/router.ts:67, ef429488c639)
• Real package execution proof: The captured PR body reports macOS arm64 and Node v26.7.0 execution against built dist/index.js: short navigation and preload chains reach gamma, navigation cycles reject with error state, preload cycles reject, and an eleven-hop navigation rejects. GitHub's contents endpoint confirms the proof revision's router blob is 76dcb1f4f03ccce8d2f019ee0e9139359cd83f59, identical to the reviewed head. The body snapshot is supplied in full under sourceRevision 42f58712740cbd5c886f373059b41f3b7ac481ad2071786d5ca2c802a726ed4f. (src/router.ts, 8c45e1b347a4)
• Prior review continuity: The previous completed review inspected the same head, recorded no findings, and requested a maintainer decision on the ten-hop ceiling and upgrade guidance. The supplied discussion and current GitHub reviews contain no disposition resolving that specific choice. (ef429488c639)
• Related work does not supersede this patch: The repository PR listing identifies no merged redirect-limit replacement. fix(router): cancel stale navigation when returning to active route #20 fixes stale navigation cancellation; the supplied bodies for fix: use lastContext in the history listener #24 and fix: do not expire Infinity gcTime via setTimeout overflow #25 describe separate history-context and timer-overflow defects.

Likely related people:

• unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
• shakkernerd: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Record the maintainer's decision on the ten-hop ceiling and upgrade guidance for affected longer-chain consumers.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (16 earlier review cycles; latest 8 shown)

• reviewed 2026-08-25T19:12:10.040Z sha 8c45e1b :: needs maintainer review before merge. :: none
• reviewed 2026-08-28T12:42:10.181Z sha 8c45e1b :: needs maintainer review before merge. :: none
• reviewed 2026-08-28T20:14:32.568Z sha ef42948 :: needs maintainer review before merge. :: none
• reviewed 2026-08-29T02:09:48.305Z sha ef42948 :: needs maintainer review before merge. :: none
• reviewed 2026-08-29T08:12:54.842Z sha ef42948 :: needs maintainer review before merge. :: none
• reviewed 2026-08-29T17:58:04.473Z sha ef42948 :: needs maintainer review before merge. :: none
• reviewed 2026-08-29T23:54:27.752Z sha ef42948 :: needs maintainer review before merge. :: none
• reviewed 2026-09-05T01:16:45.352Z sha ef42948 :: blocked before merge. :: none