fix: Reject non-terminal catch-all routes in pages and app routers

[JaredStowell]

Fixes a routing bug where non-terminal catch-all segments were accepted during route discovery and then overmatched at runtime.

Before this change, paths like pages/[...slug]/edit/index.tsx were converted into the invalid internal pattern /:slug+/edit, and the matcher would return as soon as it saw :slug+ or :slug*. That caused the route to match URLs it should never match, including both /foo and /foo/edit.

This change fixes that in both the Pages Router and App Router.

What changed

• Reject non-terminal catch-all segments during Pages Router route discovery
• Reject non-terminal catch-all segments during App Router route discovery
• Add defensive matcher guards so malformed internal patterns like /:slug+/edit and /:slug*/edit do not overmatch
• Preserve valid App Router behavior when only transparent segments like route groups follow on disk
• Extend routing tests to cover invalid discovery, malformed matcher inputs, API routes, fallback behavior, and route-group edge cases

Details

Pages Router

• fileToRoute() now rejects [...param] and [[...param]] unless they are terminal
• matchPattern() now treats non-terminal :param+ and :param* as invalid instead of returning early

App Router

• fileToAppRoute() now rejects non-terminal catch-all segments based on visible URL segments, not raw filesystem depth
• Route groups and slots remain transparent, so cases like app/[...slug]/(admin)/page.tsx still resolve to valid /:slug+
• matchPattern() now rejects malformed non-terminal internal catch-all patterns

Tests

Added regression coverage for:

• Pages non-terminal catch-all discovery rejection
• Pages non-terminal optional catch-all discovery rejection
• Pages matcher rejection of malformed /:slug+/edit
• Pages matcher rejection of malformed /:slug*/edit
• Pages fallback to later valid routes after malformed entries
• API router rejection of invalid non-terminal catch-all routes
• App non-terminal catch-all page rejection
• App non-terminal optional catch-all route-handler rejection
• App non-terminal catch-all rejection when static suffix is behind a route group
• App acceptance of terminal catch-all routes followed only by transparent route groups
• App matcher rejection of malformed internal catch-all patterns
• App fallback to later valid routes after malformed entries

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/vinext@401

commit: 47bf197

[james-elicx]

/bigbonk review

[ask-bonk]

Good fix overall. The bug is real and the approach is sound: reject non-terminal catch-all segments at discovery time, add defensive guards in the matchers, and consolidate the duplicate segment-to-route-part conversion into a shared convertSegmentsToRouteParts helper. The test coverage is thorough and well-structured.

Parity gap: pages-server-entry.ts was missed

packages/vinext/src/entries/pages-server-entry.ts has its own copy of matchPattern (line 318) that was not patched with the same non-terminal catch-all guard. This is the Pages Router production server entry. Without the fix there, production Pages Router builds can still overmatch on malformed non-terminal catch-all patterns.

The fix is the same one-liner in both the + branch (line 324) and the * branch (line 331):

if (i !== patternParts.length - 1) return null;

The corresponding snapshot at line ~15015 in entry-templates.test.ts.snap will also need updating.

Per AGENTS.md:

When fixing a bug in any of these files, check whether the same bug exists in the others. Do not leave known bugs as "follow-ups" — fix them in the same PR.

[ask-bonk]

Review submitted successfully. Here's a summary of what I found:

Review Summary

Verdict: Request changes - one parity gap needs fixing.

What's good

• The bug fix is correct: non-terminal catch-all segments like pages/[...slug]/edit/index.tsx were producing invalid internal patterns (/:slug+/edit) that overmatched at runtime.
• The refactor extracting convertSegmentsToRouteParts() eliminates three copies of the same segment conversion logic, which is what allowed the bug to exist in the first place.
• hasRemainingVisibleSegments() is a smart design for the App Router, correctly treating route groups and parallel slots as transparent while rejecting actual visible segments after a catch-all.
• Test coverage is thorough: 350 new lines covering Pages Router, App Router, API routes, slot subroutes, intercept targets, route groups, and matcher fallback behavior.

What needs fixing

• packages/vinext/src/entries/pages-server-entry.ts has its own copy of matchPattern (line 318) that was not patched with the same guard. This is the Pages Router production server entry. Without the fix, production builds still overmatch. The same one-liner (if (i !== patternParts.length - 1) return null;) needs adding at lines 324 and 331, and the corresponding snapshot needs updating.

github run

[james-elicx]

/bigbonk re-review

[ask-bonk]

Review Summary

This is a clean, well-structured fix for a real routing correctness bug. All CI checks pass. The previous review's concern about pages-server-entry.ts being missed has been addressed — the guard is present at lines 325 and 333, and the snapshot is updated.

What's good

• Root cause addressed at both layers. Non-terminal catch-all segments are rejected at discovery time (preventing malformed patterns from ever being created) and also guarded in the matcher (defense-in-depth against malformed patterns reaching the matcher through other paths).
• convertSegmentsToRouteParts consolidation. Three copies of the same segment-to-route-part logic in app-router.ts (in fileToAppRoute, discoverSlotSubRoutes, and computeInterceptTarget) are now a single shared function. This eliminates the code duplication that allowed the bug to exist — a fix in one copy would have left the others vulnerable.
• hasRemainingVisibleSegments is the right abstraction for the App Router. It correctly treats route groups (group) and parallel slots @slot as transparent, so [...slug]/(admin)/page.tsx is accepted (catch-all is terminal in URL-space) while [...slug]/(admin)/edit/page.tsx is rejected.
• All four matchPattern copies are patched. pages-router.ts, app-router.ts, app-rsc-entry.ts, and pages-server-entry.ts all have the guard. The entry template snapshots are updated to match.
• Thorough test coverage — 380 new lines covering Pages Router, App Router, API routes, parallel slots, intercept targets, route groups, and matcher fallback behavior. Good edge case coverage for the transparent-segment logic.

Minor observations (non-blocking)

1. patternParts in test helpers is dead data. The Route interface doesn't have a patternParts field (the matcher derives it from pattern.split("/")), and makeTestAppRoute adds it via intersection type but the matcher never reads it. This is harmless but slightly misleading — a reader might think the matcher uses the pre-split array. Not worth blocking on.

2. isDynamic / params locals in fileToAppRoute are now redundant. After the refactor, params and isDynamic are declared, then immediately overwritten from convertedRoute. You could destructure directly instead of the declare-then-spread pattern. Again, purely cosmetic.

LGTM — nice fix.

[ask-bonk]

Nit: patternParts isn't a field on the Route interface and isn't used by matchRoute / matchPattern (which derives pattern parts from pattern.split("/") internally). It's harmless since you're casting via as Route, but it adds noise that might mislead a reader into thinking the matcher consumes a pre-split array.

Same applies to the other test route objects at lines 224, 237, 244.

[ask-bonk]

Nit: the params and isDynamic locals declared at lines 334-335 are now immediately overwritten from convertedRoute. You could simplify to direct destructuring:

Suggested change

	const { urlSegments, params: routeParams, isDynamic: routeIsDynamic } = convertedRoute;
	const { urlSegments, params, isDynamic } = convertedRoute;

(and remove the declarations at lines 334-335)

[ask-bonk]

Review submitted. Here's a summary:

Approved — this is a solid fix. The bug was real (non-terminal catch-all segments like pages/[...slug]/edit/index.tsx producing invalid internal patterns that overmatched at runtime), and the fix addresses it at both layers:

1. Discovery time — invalid routes are rejected before patterns are created
2. Matcher defense-in-depth — malformed patterns that somehow reach the matcher return null instead of overmatching

Key strengths:

• The convertSegmentsToRouteParts consolidation eliminates three copies of the same logic in app-router.ts
• hasRemainingVisibleSegments correctly handles App Router's transparent segments (route groups, parallel slots)
• All four matchPattern copies are patched (including pages-server-entry.ts which was flagged in the earlier review)
• 380 lines of new test coverage with good edge cases

I left two minor non-blocking nits about dead patternParts fields in test helpers and a small simplification opportunity in fileToAppRoute.

github run