Parse create-run bodies strictly per admission lane

Both lanes now deserialize the raw request bytes directly instead of
round-tripping through a serde_json::Value, which silently collapsed
duplicate JSON keys to last-key-wins on the legacy manifest lane and
stripped line/column locations from manifest parse errors.

When neither lane accepts the body, attribution now recognizes a
defective manifest by its required keys, so a legacy manifest carrying a
stray workflow_version_id keeps its 400 manifest error instead of being
misrouted to a 422 run_intent_invalid describing a schema the caller
never used.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

lib/apps/fabro-server/src/server/handler/runs.rs

@@ -38,7 +38,6 @@ use fabro_workflow::command_log::{command_log_path, read_json_string_blob, read_
 use fabro_workflow::run_status::RunStatus;
 use fabro_workflow::workflow_bundle::WorkflowBundle;
 use fabro_workflow::{Error as WorkflowError, operations};
-use serde::Deserialize as _;
 use strum::VariantArray as _;
 use tokio::fs;
 use tracing::info;
@@ -536,34 +535,19 @@ async fn create_run(
     headers: HeaderMap,
     body: Bytes,
 ) -> Response {
-    let value = match serde_json::from_slice::<serde_json::Value>(&body) {
-        Ok(value) => value,
-        Err(err) => {
-            return ApiError::with_code(StatusCode::BAD_REQUEST, err.to_string(), "invalid_json")
-                .into_response();
-        }
-    };
-    let intent_error = match RunIntent::deserialize(&value) {
+    // Both lanes parse the raw bytes directly so serde_json keeps its
+    // duplicate-key rejection and line/column error locations; a JSON `Value`
+    // round-trip would silently collapse duplicate keys to last-key-wins.
+    let intent_error = match serde_json::from_slice::<RunIntent>(&body) {
         Ok(intent) => {
             return Box::pin(create_run_from_intent(state, intent, actor, headers)).await;
         }
         Err(err) => err,
     };
-    let req = match RunManifest::deserialize(&value) {
+    let req = match serde_json::from_slice::<RunManifest>(&body) {
         Ok(req) => req,
         Err(manifest_error) => {
-            if value
-                .as_object()
-                .is_some_and(|object| object.contains_key("workflow_version_id"))
-            {
-                return ApiError::with_code(
-                    StatusCode::UNPROCESSABLE_ENTITY,
-                    intent_error.to_string(),
-                    "run_intent_invalid",
-                )
-                .into_response();
-            }
-            return ApiError::bad_request(manifest_error.to_string()).into_response();
+            return create_run_parse_error(&body, &intent_error, &manifest_error);
         }
     };
     let explicit_title_supplied = req.title.is_some();
@@ -582,6 +566,42 @@ async fn create_run(
     .await
 }
 
+/// Attribute a create-run body that neither lane accepted. A body carrying
+/// any of the legacy manifest's required keys is a defective manifest even
+/// when a stray `workflow_version_id` rides along, and keeps the manifest
+/// lane's `400` contract; only an intent-shaped body gets the intent `422`.
+fn create_run_parse_error(
+    body: &[u8],
+    intent_error: &serde_json::Error,
+    manifest_error: &serde_json::Error,
+) -> Response {
+    let value = match serde_json::from_slice::<serde_json::Value>(body) {
+        Ok(value) => value,
+        Err(err) => {
+            return ApiError::with_code(StatusCode::BAD_REQUEST, err.to_string(), "invalid_json")
+                .into_response();
+        }
+    };
+    let has_key = |key: &str| {
+        value
+            .as_object()
+            .is_some_and(|object| object.contains_key(key))
+    };
+    if has_key("workflow_version_id")
+        && !has_key("version")
+        && !has_key("cwd")
+        && !has_key("workflows")
+    {
+        return ApiError::with_code(
+            StatusCode::UNPROCESSABLE_ENTITY,
+            intent_error.to_string(),
+            "run_intent_invalid",
+        )
+        .into_response();
+    }
+    ApiError::bad_request(manifest_error.to_string()).into_response()
+}
+
 async fn create_run_from_intent(
     state: Arc<AppState>,
     intent: RunIntent,

lib/apps/fabro-server/src/server/tests.rs

@@ -3757,6 +3757,45 @@ async fn post_runs_run_intent_dispatches_errors_without_changing_legacy_lane() {
     assert_eq!(legacy["lifecycle"]["status"]["kind"], "submitted");
 }
 
+#[tokio::test]
+async fn post_runs_attributes_parse_failures_and_rejects_duplicate_keys() {
+    let state = test_app_state();
+    let app = crate::test_support::build_test_router(Arc::clone(&state));
+    let post = |body: String| {
+        Request::builder()
+            .method("POST")
+            .uri(api("/runs"))
+            .header("content-type", "application/json")
+            .body(Body::from(body))
+            .unwrap()
+    };
+
+    // A defective manifest keeps the manifest lane's 400 contract even when
+    // a stray workflow_version_id rides along.
+    let mut manifest = minimal_manifest_json(MINIMAL_DOT);
+    manifest["workflow_version_id"] = json!(fabro_types::test_support::test_workflow_version_id());
+    manifest["cwd"] = json!(42);
+    let response = app
+        .clone()
+        .oneshot(post(manifest.to_string()))
+        .await
+        .unwrap();
+    let body = response_json!(response, StatusCode::BAD_REQUEST).await;
+    let detail = body["errors"][0]["detail"].as_str().unwrap();
+    assert!(detail.contains("invalid type: integer `42`"), "{detail}");
+
+    // Duplicate JSON keys are ambiguous: they must be rejected, not
+    // collapsed to last-key-wins by a Value round-trip.
+    let duplicated = format!(
+        r#"{{"version":1,"cwd":"/tmp","cwd":"/other","target":{{"path":"workflow.fabro"}},"workflows":{{"workflow.fabro":{{"source":{source},"files":{{}}}}}}}}"#,
+        source = serde_json::to_string(MINIMAL_DOT).unwrap()
+    );
+    let response = app.clone().oneshot(post(duplicated)).await.unwrap();
+    let body = response_json!(response, StatusCode::BAD_REQUEST).await;
+    let detail = body["errors"][0]["detail"].as_str().unwrap();
+    assert!(detail.contains("duplicate field"), "{detail}");
+}
+
 #[tokio::test]
 async fn post_runs_run_intent_maps_missing_version_environment_and_target_errors() {
     let state = test_app_state();