From 8f558e6ab97b6f681071d2b67c22f5a914f64a44 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Wed, 27 May 2026 18:12:03 +0200 Subject: [PATCH] mr-668: named-field path-param structs for nested cluster routes (green) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Path` deserializes one path-param value positionally. Single-mode flat routes (`/branches/{branch}`, `/commits/{commit_id}`) have one capture; multi-mode nested routes (`/graphs/{graph_id}/branches/{branch}`, `/graphs/{graph_id}/commits/{commit_id}`) have two — axum 0.8 propagates the outer capture into nested handlers. Same handler, two different shapes; the multi-mode shape 500s with "Wrong number of path arguments. Expected 1 but got 2." Symptomatic fix: change to `Path<(String, String)>` and ignore the first element. Breaks again the moment we add another nest layer (e.g. tenant in Cloud mode). Correct-by-design fix: named-field structs deserialized by name from axum's path-param map. Each handler picks only the fields it needs. Stable across single / multi / future-cloud nest depths because deserialization is by field name, not position. * New `BranchPath { branch: String }` (file-local to lib.rs) * New `CommitPath { commit_id: String }` * `server_branch_delete` extractor → `Path` * `server_commit_show` extractor → `Path` Closes the "handler path-extractor type is positional and breaks when route nesting changes" class. Red test from the previous commit turns green. All 77 server tests pass (single-mode branch delete + commit show, plus new multi-mode coverage). Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/omnigraph-server/src/lib.rs | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index 4aa20f9..b56e208 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -52,6 +52,7 @@ pub use policy::{ PolicyAction, PolicyCompiler, PolicyConfig, PolicyDecision, PolicyEngine, PolicyExpectation, PolicyRequest, PolicyTestConfig, }; +use serde::Deserialize; use serde_json::Value; use sha2::{Digest, Sha256}; use subtle::ConstantTimeEq; @@ -2035,6 +2036,20 @@ async fn server_branch_create( })) } +/// Path-param shape for [`server_branch_delete`]. Named-field +/// deserialization (rather than `Path` or `Path<(String,)>`) +/// keeps the extractor stable across single-mode flat routes and +/// multi-mode nested routes: the `{branch}` capture is picked by +/// name and any other captures in scope (e.g. `{graph_id}` in +/// multi-mode) are ignored without breaking deserialization. +/// +/// Closes the "handler path-extractor type is positional and breaks +/// when route nesting changes" class. +#[derive(Deserialize)] +struct BranchPath { + branch: String, +} + #[utoipa::path( delete, path = "/branches/{branch}", @@ -2061,7 +2076,7 @@ async fn server_branch_delete( State(state): State, Extension(handle): Extension>, actor: Option>, - Path(branch): Path, + Path(BranchPath { branch }): Path, ) -> std::result::Result, ApiError> { let actor_arc = actor .as_ref() @@ -2201,6 +2216,13 @@ async fn server_commit_list( })) } +/// Path-param shape for [`server_commit_show`]. See [`BranchPath`] +/// for the design rationale — same pattern, different field name. +#[derive(Deserialize)] +struct CommitPath { + commit_id: String, +} + #[utoipa::path( get, path = "/commits/{commit_id}", @@ -2217,6 +2239,7 @@ async fn server_commit_list( ), security(("bearer_token" = [])), )] + /// Get a single commit. /// /// Returns the commit's manifest version, parent commit(s), and creation @@ -2224,7 +2247,7 @@ async fn server_commit_list( async fn server_commit_show( Extension(handle): Extension>, actor: Option>, - Path(commit_id): Path, + Path(CommitPath { commit_id }): Path, ) -> std::result::Result, ApiError> { authorize_request( actor.as_ref().map(|Extension(actor)| actor),