Fix federated chunk fetch: URL residue + project-scope routing (todo #153)

CHANGELOG.md

@@ -48,6 +48,10 @@ finalized in place with a date — no renaming/migration step needed.
 
 - **Serve auto-recovers LMDB storage-format corruption with a sequential wipe + rebuild.** After the arroy 0.5→0.8 / heed 0.20→0.22 major upgrades, every repo whose on-disk database was written by the previous binary failed its symbol rebuild with `MDB_BAD_VALSIZE: Unsupported size of key/DB name/data, or wrong DUPFIXED size` (observed on all C# repos after deploy). When a symbol rebuild now fails with that error class, serve wipes the repo's DB directory — closing the LMDB envs first via the same eviction sequence `remove_repo` uses, with the same bounded retry for transient Windows lock holders — and force-reindexes it through the existing force-reindex machinery, whose store-open path recreates everything on the new formats. Recoveries are queued and processed strictly **one repo at a time**: each rebuild runs a full CPU-bound embed pass, so parallel recoveries would thrash the machine. Read-only repos are skipped with a pointer to the owning writer. This closes the gap the tantivy FTS graceful reset (above) already covered on the FTS side — the vector/symbol stores now self-heal the same upgrade boundary instead of staying red until an operator force-reindexes by hand.
 
+### Fixed
+
+- **Federated chunk fetch works again — URL residue and project-scope routing (todo #153).** Two independent defects: (1) the peer URL for a `chunk_ref` fetch was built by replacing `{id` without the closing brace, so the constructed path carried a stray `}` (`/chunk/2058%7D`) that real peers answered with 400 Bad Request — axum's `{id}` parameter happily swallowed the stray brace into the captured value, which is exactly why the mock-based tests never caught it; the replacement now covers the full `{id}` placeholder, pinned by a test whose route echoes back the exact path it was hit on. (2) `get_chunk` with `project=<peer>/<alias>` plus a plain `chunk_id` died in local routing with "Unknown alias" — mounted remote projects are now routed through the same federated fetch search uses (local aliases still win a name clash), so both `chunk_ref` and `project=`+`chunk_id` forms work against remote peers.
+
 ## [1.3.19]
 
 ### Changed

src/federation/mod.rs

@@ -402,7 +402,7 @@ impl FederationClient {
     ) -> Outcome<serde_json::Value> {
         let mut url = Self::peer_url(
             peer,
-            &crate::constants::CHUNK_PATH.replace("{id", &chunk_id.to_string()),
+            &crate::constants::CHUNK_PATH.replace("{id}", &chunk_id.to_string()),
         );
         // Scope the lookup: prefer a single-project scope (`project=<alias>`)
         // so the multi-repo peer can disambiguate the chunk_id; fall back to
@@ -1285,6 +1285,51 @@ mod tests {
         );
     }
 
+    #[tokio::test]
+    #[serial_test::serial]
+    async fn get_chunk_requests_the_exact_chunk_path_without_placeholder_residue() {
+        // Regression (todo #153): the URL template is `/chunk/{id}`; a
+        // placeholder replacement that missed the closing brace produced
+        // `/chunk/2058%7D`. Axum's `{id}` param happily swallowed the stray
+        // `}` into the captured value, so mock-based route tests passed while
+        // real peers answered 400 Bad Request on the mangled id. The route
+        // below echoes back the EXACT path it was hit on, so any residue in
+        // the constructed URL fails the assertion.
+        let seen = std::sync::Arc::new(tokio::sync::Mutex::new(String::new()));
+        let seen_clone = seen.clone();
+        let router = axum::Router::new().route(
+            "/chunk/{id}",
+            axum::routing::get(move |uri: axum::http::Uri| {
+                let seen = seen_clone.clone();
+                async move {
+                    *seen.lock().await = uri.path().to_string();
+                    axum::Json(serde_json::json!({
+                        "chunk_id": 2058,
+                        "content": "ok",
+                        "path": "kb/x.md",
+                        "start_line": 1,
+                        "end_line": 2
+                    }))
+                }
+            }),
+        );
+        let addr = spawn_test_server(router).await;
+
+        let client = FederationClient::new().unwrap();
+        let outcome = client
+            .get_chunk(&peer(format!("http://{addr}")), Some("bynder"), 2058, None)
+            .await;
+        assert!(
+            matches!(outcome, Outcome::Ok(_)),
+            "the peer must answer the clean path"
+        );
+        assert_eq!(
+            *seen.lock().await,
+            "/chunk/2058",
+            "the constructed URL must carry the bare chunk id — no placeholder residue"
+        );
+    }
+
     #[tokio::test]
     #[serial_test::serial]
     async fn get_chunk_persistent_503_reports_cold_start_hint() {

src/mcp/get_chunk.rs

@@ -34,6 +34,28 @@ impl CodesearchService {
                 .await;
         }
 
+        // Federated mounts: `project=<peer>/<alias>` routes to the peer's own
+        // project, exactly like search's project-level federation — chunk_ids
+        // are peer-local, so the fetch reuses the chunk_ref path with a
+        // synthetic "<peer>/<alias>:<id>" ref. Local aliases ALWAYS win a name
+        // clash: only route remotely when the name is not a local project.
+        if let Some(proj) = request.project.as_deref() {
+            let cfg = self.federation_config();
+            if cfg.resolve(proj).is_none() {
+                if let Some(crate::db_discovery::repos::Target::RemoteProject {
+                    peer_name,
+                    peer: _,
+                    remote_alias,
+                }) = cfg.resolve_remote_project(proj)
+                {
+                    let chunk_ref = format!("{peer_name}/{remote_alias}:{}", request.chunk_id);
+                    return self
+                        .federated_get_chunk(&chunk_ref, request.context_lines)
+                        .await;
+                }
+            }
+        }
+
         // In multi-repo serve mode, require explicit project or group scope.
         // Unscoped get_chunk would fan-out over all repos, opening all DBs unnecessarily.
         // Consistent with search/find/explore which also require scope.

src/serve/tests.rs

@@ -30,6 +30,62 @@ fn rest_service_drop_does_not_touch_active_sessions() {
     );
 }
 
+#[tokio::test]
+#[serial]
+async fn get_chunk_routes_mounted_remote_projects_through_federation() {
+    // todo #153: `get_chunk(project="<peer>/<alias>", chunk_id=…)` must route
+    // through the federated fetch exactly like search's project-level
+    // federation, instead of dying in local routing with "Unknown alias".
+    // The peer URL here is unreachable, so the correctly routed answer is the
+    // federation failure message — "Unknown alias" means the routing did not
+    // happen.
+    let mut config = ReposConfig::default();
+    config.remotes.insert(
+        "cloud".to_string(),
+        crate::db_discovery::repos::RemotePeer {
+            url: "http://127.0.0.1:1".to_string(),
+            api_key: "test-key".to_string(),
+            group: None,
+            timeout_secs: None,
+        },
+    );
+    config.remote_mounts.push("cloud/bynder".to_string());
+    // Hermetic config: persist to a temp file and pass the override, so
+    // `reload_if_changed` reads THIS config — not the developer's real
+    // ~/.codesearch/repos.json (which would leak real peers into the test).
+    let tmp = tempfile::tempdir().unwrap();
+    let config_file = tmp.path().join("repos.json");
+    config.save_to(&config_file).unwrap();
+    let state = std::sync::Arc::new(ServeState::new(config, Some(config_file)));
+    let service = crate::mcp::CodesearchService::new_for_serve(state).unwrap();
+
+    let _env =
+        crate::testing::EnvRestore::set(&[(crate::constants::REMOTE_PEER_RETRY_BACKOFF_ENV, "1")]);
+    let req = crate::mcp::types::GetChunkRequest {
+        chunk_id: 2058,
+        chunk_ref: None,
+        context_lines: None,
+        project: Some("cloud/bynder".to_string()),
+        group: None,
+    };
+    let res = service
+        .get_chunk(rmcp::handler::server::wrapper::Parameters(req))
+        .await
+        .expect("handler must not error");
+    let text = match res.content.first() {
+        Some(rmcp::model::ContentBlock::Text(t)) => t.text.clone(),
+        other => panic!("expected text content, got {other:?}"),
+    };
+    assert!(
+        text.contains("Could not fetch chunk from remote peer 'cloud'"),
+        "get_chunk must route mounted remote projects to the peer, got: {text}"
+    );
+    assert!(
+        !text.contains("Unknown alias"),
+        "a mounted remote project is not a local alias — routing failed: {text}"
+    );
+}
+
 #[test]
 fn tracked_session_drop_balances_active_sessions() {
     // A genuine MCP session increments on connect and the serve factory