refactor(routes): drop prod-dead compile_routes; document disabled-shadow (§11 tail review)

Two review findings.

1. After R3 every production rebuild path uses `compile_effective_routes`;
   `compile_routes` (the old app-only compile) was called only by its own
   two unit tests, and its "runs in build_app" doc was stale — so the
   lenient-skip + disabled-drop regression guards no longer covered the
   live path. Remove `compile_routes` + its re-export, fold the lenient-skip
   rationale onto `compile_one`, and port both guards to
   `compile_effective_routes` (app-only rows are just depth-0 effective
   rows). Add a third guard pinning nearest-owner shadowing at the pure-
   compile layer. Fix the stale `compile_routes` mention in apply_service.

2. Document the disabled + inherited semantic: the `enabled` filter runs
   before the shadow check, so a disabled own-route does not claim its
   binding and an enabled ancestor template at the same path falls through
   ("disabled = absent", §4.3). Recorded on `compile_effective_routes` and
   in design §4.5 — the corollary (can't 404 an inherited route by
   disabling a same-path own route) points at the deferred per-app opt-out.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

crates/manager-core/src/apply_service.rs

@@ -2111,8 +2111,8 @@ impl ApplyService {
         for r in &bundle.routes {
             reject_reserved_path(&r.path)?;
             // Structural validity (parity with the interactive route API):
-            // a route whose path/host doesn't parse would otherwise be
-            // written and then silently dropped by `compile_routes`.
+            // a route whose path/host doesn't parse would otherwise be written
+            // and then silently dropped by the route-table compile.
             pattern::parse_path(r.path_kind, &r.path)
                 .map_err(|e| ApplyError::Invalid(format!("route path `{}`: {e}", r.path)))?;
             pattern::parse_host(r.host_kind, &r.host, r.host_param_name.as_deref())

crates/manager-core/src/lib.rs

@@ -218,7 +218,9 @@ pub use repo::{
     ExecutionLogRepository, NewScript, PostgresExecutionLogRepository, PostgresScriptRepository,
     RepoResolver, ScriptPatch, ScriptRepository, ScriptRepositoryError,
 };
-pub use route_admin::{compile_routes, rebuild_route_table, route_admin_router, RouteAdminState};
+pub use route_admin::{
+    compile_effective_routes, rebuild_route_table, route_admin_router, RouteAdminState,
+};
 pub use route_repo::{NewRoute, PostgresRouteRepository, RouteRepository};
 pub use sandbox::{CeilingError, SandboxCeiling};
 pub use secrets_api::{secrets_router, SecretsApiError, SecretsState};

crates/manager-core/src/route_admin.rs

@@ -411,35 +411,6 @@ pub async fn rebuild_route_table(
     Ok(())
 }
 
-/// Compile stored route rows into the in-memory match table.
-///
-/// **Lenient by design (H1).** A row that fails to parse is *skipped with
-/// a warning*, not propagated as an error. The motivating case: a path
-/// that was valid when created but became reserved under a later, stricter
-/// validation (e.g. the case-insensitive reserved-prefix check) — but this
-/// also covers any other parse failure. A single un-compilable legacy row
-/// must never take down the entire data plane: this function runs at
-/// startup (where a hard error aborts boot) and on every table rebuild
-/// after a route edit (where it would fail an unrelated CRUD op). A skipped
-/// route simply doesn't match; the warning tells the operator to delete or
-/// fix it (and migration 0044 sweeps the reserved-path offenders on
-/// upgrade).
-#[must_use]
-pub fn compile_routes(rows: &[Route]) -> Vec<CompiledRoute> {
-    rows.iter()
-        // A disabled route (§4.3) is dropped from the match table entirely, so
-        // a request to it 404s indistinguishably from an absent route.
-        .filter(|r| r.enabled)
-        .filter_map(|r| {
-            // A group-owned route TEMPLATE (`app_id` None) has no single app to
-            // compile into — it is materialized per descendant by the effective
-            // rebuild (`compile_effective_routes`), not this app-only path.
-            let app_id = r.app_id?;
-            compile_one(r, app_id)
-        })
-        .collect()
-}
-
 /// Compile the §11 tail effective expansion into per-app match slices, applying
 /// **nearest-owner-wins shadowing**: for a given app, if the same binding tuple
 /// (method + host + path) is owned at more than one chain level — e.g. the app
@@ -450,6 +421,18 @@ pub fn compile_routes(rows: &[Route]) -> Vec<CompiledRoute> {
 /// app's `/users/:id` + the group's `/users/admin`) coexist and the matcher's
 /// existing precedence picks per request.
 ///
+/// This is the single production entry point: app-only installs feed it routes
+/// at `depth 0` (one per app, no templates), so it subsumes the former app-only
+/// compile path. Disabled routes are dropped (§4.3) and un-compilable rows
+/// skipped-with-warning (see [`compile_one`]).
+///
+/// **Disabled + inherited (§4.3 semantic):** the `enabled` filter runs *before*
+/// the shadow check, so a **disabled** own-route does NOT claim its binding — an
+/// enabled ancestor-group template at the same binding then falls through and
+/// serves (disabled = "indistinguishable from absent"). The corollary: a
+/// descendant cannot 404 an inherited route by disabling a same-path own route;
+/// a true per-app opt-out is deferred (§4.5).
+///
 /// Requires `rows` ordered by `(effective_app_id, depth ASC)` — which
 /// [`RouteRepository::list_effective`] guarantees — so first-seen is nearest.
 #[must_use]
@@ -457,6 +440,8 @@ pub fn compile_effective_routes(rows: &[EffectiveRoute]) -> Vec<CompiledRoute> {
     let mut seen: std::collections::HashSet<(AppId, String)> = std::collections::HashSet::new();
     let mut out = Vec::new();
     for er in rows {
+        // A disabled route (§4.3) is dropped from the match table entirely, so a
+        // request to it 404s indistinguishably from an absent route.
         if !er.route.enabled {
             continue;
         }
@@ -495,8 +480,18 @@ fn binding_key(r: &Route) -> String {
 
 /// Parse one stored route into a `CompiledRoute` bound to `app_id` (the
 /// effective app — its own for app routes, the firing descendant for a group
-/// template). Lenient: an un-compilable row is skipped-with-warning, never an
-/// error (it must not take down the data plane or fail an unrelated rebuild).
+/// template).
+///
+/// **Lenient by design (H1).** A row that fails to parse is *skipped with a
+/// warning*, not propagated as an error. The motivating case: a path that was
+/// valid when created but became reserved under a later, stricter validation
+/// (the case-insensitive reserved-prefix check) — but this also covers any
+/// other parse failure. A single un-compilable row must never take down the
+/// data plane: this runs at startup (where a hard error aborts boot) and on
+/// every table rebuild after an edit (where it would fail an unrelated CRUD
+/// op). A skipped route simply doesn't match; the warning tells the operator to
+/// delete or fix it (and migration 0044 sweeps the reserved-path offenders on
+/// upgrade).
 fn compile_one(r: &Route, app_id: AppId) -> Option<CompiledRoute> {
     match compile_route(r, app_id) {
         Ok(compiled) => Some(compiled),
@@ -734,18 +729,28 @@ mod tests {
         }
     }
 
+    /// Wrap an app-owned route as its own `depth 0` effective row — the shape
+    /// `list_effective` produces for an app-only install (no group templates).
+    fn eff(route: &Route) -> EffectiveRoute {
+        EffectiveRoute {
+            effective_app_id: route.app_id.expect("test route is app-owned"),
+            depth: 0,
+            route: route.clone(),
+        }
+    }
+
     #[test]
-    fn compile_routes_skips_uncompilable_rows_instead_of_failing() {
+    fn compile_effective_skips_uncompilable_rows_instead_of_failing() {
         // H1 regression guard: a stored route whose path is now reserved
         // (creatable before the case-insensitive reserved-prefix fix) must
         // be skipped, not abort the whole compile — otherwise one legacy
-        // row bricks startup (`compile_routes` runs in `build_app`).
+        // row bricks startup (the effective rebuild runs in `build_app`).
         let good_a = route_with_path("/ok");
         let bad = route_with_path("/API/v2/x"); // now reserved, case-insensitive
         let good_b = route_with_path("/items");
-        let rows = vec![good_a.clone(), bad.clone(), good_b.clone()];
+        let rows = [eff(&good_a), eff(&bad), eff(&good_b)];
 
-        let compiled = compile_routes(&rows);
+        let compiled = compile_effective_routes(&rows);
 
         let ids: Vec<Uuid> = compiled.iter().map(|c| c.route_id).collect();
         assert_eq!(compiled.len(), 2, "the reserved row must be dropped");
@@ -764,8 +769,53 @@ mod tests {
         let active = route_with_path("/on");
         let mut disabled = route_with_path("/off");
         disabled.enabled = false;
-        let compiled = compile_routes(&[active.clone(), disabled.clone()]);
+        let compiled = compile_effective_routes(&[eff(&active), eff(&disabled)]);
         let ids: Vec<Uuid> = compiled.iter().map(|c| c.route_id).collect();
         assert_eq!(ids, vec![active.id], "only the enabled route compiles");
     }
+
+    #[test]
+    fn nearest_owner_shadows_identical_binding() {
+        // An app's own route (depth 0) and an ancestor-group template (depth 1)
+        // with the SAME binding collapse to the nearer one; a different binding
+        // coexists. Mirrors the live `group_route_templates` integration test
+        // at the pure-compile layer.
+        let app = AppId::from(Uuid::new_v4());
+        let own = route_with_path("/x"); // app's own /x
+        let mut template = route_with_path("/x"); // group template, same binding
+        template.app_id = None;
+        template.group_id = Some(picloud_shared::GroupId::from(Uuid::new_v4()));
+        let other_template = route_with_path("/y"); // group template, different path
+
+        let rows = [
+            EffectiveRoute {
+                effective_app_id: app,
+                depth: 0,
+                route: own.clone(),
+            },
+            EffectiveRoute {
+                effective_app_id: app,
+                depth: 1,
+                route: template.clone(),
+            },
+            EffectiveRoute {
+                effective_app_id: app,
+                depth: 1,
+                route: other_template.clone(),
+            },
+        ];
+        let ids: Vec<Uuid> = compile_effective_routes(&rows)
+            .iter()
+            .map(|c| c.route_id)
+            .collect();
+        assert!(ids.contains(&own.id), "the app's own /x must win");
+        assert!(
+            !ids.contains(&template.id),
+            "the inherited /x template must be shadowed by the app's own"
+        );
+        assert!(
+            ids.contains(&other_template.id),
+            "a non-identical /y template must coexist"
+        );
+    }
 }