fix: correct route Name/Action lookup for late-registered sub-routers

Author: mallardduck

pkg/teapot/router.go

@@ -606,6 +606,12 @@ func (r *Router) propagateRoute(pathPrefix, namePrefix string, rt *core.Route) {
 		r.nameIndex[newRoute.Name] = newRoute
 	}
 
+	// Also track in directRoutes and dispatchers for findMatchingRoute
+	dispatcherKey := newRoute.Method + ":" + newRoute.ChiPattern
+	if len(newRoute.QueryMatchers) == 0 {
+		r.directRoutes[dispatcherKey] = newRoute
+	}
+
 	// Propagate further up if this router also has parents
 	for _, p := range r.parents {
 		p.router.propagateRoute(p.pathPrefix, p.namePrefix, newRoute)
@@ -700,36 +706,78 @@ func (r *Router) SubRouter(prefix string) *Router {
 // findMatchingRoute manually matches a request against registered routes
 // This is used as a fallback when Chi's RouteContext isn't available (e.g., in global middleware)
 func (r *Router) findMatchingRoute(method, path string) *core.Route {
+	type candidate struct {
+		route   *core.Route
+		pattern string
+	}
+	var matches []candidate
+
 	// Check all direct routes
 	for key, route := range r.directRoutes {
 		if strings.HasPrefix(key, method+":") {
 			pattern := strings.TrimPrefix(key, method+":")
 			if r.matchPattern(pattern, path) {
-				return route
+				matches = append(matches, candidate{route: route, pattern: pattern})
 			}
 		}
 	}
 
-	// Check dispatcher routes (return fallback route for query-multiplexed)
+	// Check dispatcher routes
 	for key, disp := range r.dispatchers {
 		if strings.HasPrefix(key, method+":") {
 			pattern := strings.TrimPrefix(key, method+":")
 			if r.matchPattern(pattern, path) {
-				// Return fallback route (no query matchers)
+				// For dispatchers, we want the fallback route for name/action resolution in middleware
+				// when we don't know the query params yet.
+				var fallback *core.Route
 				for _, rt := range disp.Routes {
 					if len(rt.QueryMatchers) == 0 {
-						return rt
+						fallback = rt
+						break
 					}
 				}
-				// If no fallback, return first route
-				if len(disp.Routes) > 0 {
-					return disp.Routes[0]
+				if fallback == nil && len(disp.Routes) > 0 {
+					fallback = disp.Routes[0]
+				}
+				if fallback != nil {
+					matches = append(matches, candidate{route: fallback, pattern: pattern})
+				}
+			}
+		}
+	}
+
+	if len(matches) == 0 {
+		return nil
+	}
+
+	// If multiple matches, prioritize literal matches (no {param} or *)
+	if len(matches) > 1 {
+		bestIdx := 0
+		bestScore := -1
+
+		for i, match := range matches {
+			score := 0
+			if !strings.ContainsAny(match.pattern, "{}*") {
+				score = 100 // Exact literal match
+			} else {
+				// Count literal parts (non-parameters)
+				parts := strings.Split(match.pattern, "/")
+				for _, p := range parts {
+					if p != "" && !strings.ContainsAny(p, "{}*") {
+						score++
+					}
 				}
 			}
+
+			if score > bestScore {
+				bestScore = score
+				bestIdx = i
+			}
 		}
+		return matches[bestIdx].route
 	}
 
-	return nil
+	return matches[0].route
 }
 
 // matchPattern checks if a Chi pattern matches a path

tests/subrouter_resolution_test.go

@@ -0,0 +1,91 @@
+package tests
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/mallardduck/teapot-router/pkg/teapot"
+)
+
+func TestSubRouterResolutionBug(t *testing.T) {
+	t.Run("subrouter with parameter shadows literal", func(t *testing.T) {
+		r := teapot.New()
+
+		// Global middleware that checks route name/action
+		r.Use(teapot.RouteContextMiddleware(r))
+
+		var resolvedName string
+		var resolvedAction string
+		r.Use(func(next http.Handler) http.Handler {
+			return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+				next.ServeHTTP(w, req)
+				resolvedName = teapot.GetRouteName(req)
+				resolvedAction = teapot.GetAction(req)
+			})
+		})
+		r.GET("/{greedy}", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		}))
+
+		// Sub-router mounted at /api
+		api := teapot.New()
+
+		// Route with parameter at first position
+		api.GET("/{id}", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		})).Name("api.show").Action("ShowAction")
+
+		// Literal route that should match /api/users
+		api.GET("/users", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		})).Name("api.users").Action("UsersAction")
+
+		r.Mount("/api", api)
+		r.Finalize()
+
+		// Request to /api/users
+		req := httptest.NewRequest("GET", "/api/users", nil)
+		rec := httptest.NewRecorder()
+		r.ServeHTTP(rec, req)
+
+		assert.Equal(t, http.StatusOK, rec.Code)
+		assert.Equal(t, "api.users", resolvedName, "Should resolve to api.users, not api.show")
+		assert.Equal(t, "UsersAction", resolvedAction)
+	})
+
+	t.Run("late registration with parameter shadows literal", func(t *testing.T) {
+		r := teapot.New()
+
+		// Global middleware
+		r.Use(teapot.RouteContextMiddleware(r))
+		var resolvedName string
+		r.Use(func(next http.Handler) http.Handler {
+			return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+				next.ServeHTTP(w, req)
+				resolvedName = teapot.GetRouteName(req)
+			})
+		})
+
+		// Parameter route first
+		r.GET("/{id}", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		})).Name("show")
+
+		// Literal route later
+		r.GET("/users", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		})).Name("users")
+
+		r.Finalize()
+
+		// Request to /users
+		req := httptest.NewRequest("GET", "/users", nil)
+		rec := httptest.NewRecorder()
+		r.ServeHTTP(rec, req)
+
+		assert.Equal(t, "users", resolvedName)
+	})
+}