fix(serve): accept absent/null assistant content, max_completion_tokens alias, gate the GGUF router (supersedes #5411)

contracts/apr-serve-openai-compat-v1.yaml

@@ -1,7 +1,7 @@
 contract: apr-serve-openai-compat
 metadata:
   kind: pattern
-  version: "1.23.0"
+  version: "1.24.0"
   description: >
     OpenAI-compatible serve layer (/v1/chat/completions, /v1/completions,
     /v1/embeddings) fidelity invariants. Established by an adversarial audit
@@ -788,3 +788,15 @@ falsification_tests:
     test_harness: 'cargo test -p aprender-serve --lib embed_kind_eg3_4902'
     expected_output: "test result: ok"
     if_fails: 'An embedding model is refused by `apr serve` or answers with a vector the CLI does not produce; a generative route on it fails with something other than a 4xx naming the kind; or an image reaches the tokenizer as text and comes back as a 200 embedding of its base64 (media dropped, I-7).'
+  - id: FALSIFY-CHAT-CONTENT-OPTIONAL-5411
+    name: chat_message_content_pmat801
+    prediction: 'A chat request whose ASSISTANT message omits `content` or sends `content: null` (the tool-call turn OpenAI SDK loops echo back) parses with content "" and keeps its tool_calls; system, user and tool messages still REQUIRE content (absent or null is refused naming `content`); a non-string content is a type error for every role; ChatMessage serialization bytes are unchanged.'
+    test_harness: 'cargo test -p aprender-serve --lib chat_message_content'
+    expected_output: "test result: ok"
+    if_fails: 'An OpenAI-compatible agent loop gets HTTP 422 "missing field content" on its second turn, or a non-assistant role silently accepts a missing content.'
+  - id: FALSIFY-MAX-COMPLETION-TOKENS-5411
+    name: max_completion_tokens_alias
+    prediction: '`max_completion_tokens` fills `max_tokens` on /v1/chat/completions; `max_tokens` alone still parses; a body carrying both names is refused with serde `duplicate field` rather than silently preferring one.'
+    test_harness: 'cargo test -p aprender-serve --lib max_completion_tokens'
+    expected_output: "test result: ok"
+    if_fails: 'OpenAI SDK >= 1.40 clients lose their generation cap (the unknown key is ignored) and generation runs to the server default.'

crates/apr-cli/src/commands/serve/server.rs

@@ -423,15 +423,177 @@ mod qwen35_serve_route_tests {
     }
 }
 
+/// FALSIFY-AUTH-001 for the GGUF router: `build_gguf_router` mounts realizar's REAL
+/// router (`/v1/chat/completions`, `/v1/models`, `/health`, ...) behind the Bearer
+/// gate, so these tests drive the same seam `serve_router` serves through. The state
+/// is model-free (`AppState::demo_mock`): auth runs before any handler, so no weights
+/// are needed to prove that a request without the key never reaches one.
+#[cfg(all(test, feature = "inference"))]
+mod gguf_router_auth_tests {
+    #![allow(clippy::unwrap_used, clippy::disallowed_methods)]
+
+    use super::super::auth::AuthGate;
+    use super::*;
+    use axum::body::{to_bytes, Body};
+    use axum::http::{header, HeaderMap, Method, Request, StatusCode};
+    use serde_json::Value;
+    use tower::ServiceExt as _;
+
+    const CHAT_BODY: &str =
+        r#"{"model":"x","messages":[{"role":"user","content":"Hi"}],"max_tokens":1}"#;
+
+    fn router(gate: AuthGate) -> axum::Router {
+        let state = realizar::api::AppState::demo_mock().expect("model-free state");
+        build_gguf_router(state, &ServerConfig::default(), gate)
+    }
+
+    async fn fire(
+        router: axum::Router,
+        method: Method,
+        path: &str,
+        authorization: Option<&str>,
+    ) -> (StatusCode, HeaderMap, Value) {
+        let mut builder = Request::builder().method(method.clone()).uri(path);
+        let body = if method == Method::POST {
+            builder = builder.header(header::CONTENT_TYPE, "application/json");
+            Body::from(CHAT_BODY)
+        } else {
+            Body::empty()
+        };
+        if let Some(value) = authorization {
+            builder = builder.header(header::AUTHORIZATION, value);
+        }
+        let resp = router
+            .oneshot(builder.body(body).expect("request builds"))
+            .await
+            .expect("router answers");
+        let status = resp.status();
+        let headers = resp.headers().clone();
+        let bytes = to_bytes(resp.into_body(), 64 * 1024).await.unwrap();
+        let json = serde_json::from_slice(&bytes).unwrap_or(Value::Null);
+        (status, headers, json)
+    }
+
+    #[tokio::test]
+    async fn gguf_router_refuses_without_bearer() {
+        for (method, path) in [
+            (Method::POST, "/v1/chat/completions"),
+            (Method::GET, "/health"),
+            (Method::GET, "/v1/models"),
+        ] {
+            let (status, headers, body) = fire(
+                router(AuthGate::from_plain_key("k")),
+                method.clone(),
+                path,
+                None,
+            )
+            .await;
+            assert_eq!(status, StatusCode::UNAUTHORIZED, "{method} {path}: {body}");
+            assert_eq!(
+                body["error"].as_str(),
+                Some("unauthorized"),
+                "{method} {path}: {body}"
+            );
+            assert_eq!(
+                headers
+                    .get(header::WWW_AUTHENTICATE)
+                    .and_then(|v| v.to_str().ok()),
+                Some("Bearer"),
+                "{method} {path}: 401 must carry WWW-Authenticate: Bearer"
+            );
+        }
+    }
+
+    #[tokio::test]
+    async fn gguf_router_refuses_a_wrong_bearer() {
+        let (status, _, body) = fire(
+            router(AuthGate::from_plain_key("k")),
+            Method::POST,
+            "/v1/chat/completions",
+            Some("Bearer wrong"),
+        )
+        .await;
+        assert_eq!(status, StatusCode::UNAUTHORIZED, "{body}");
+        assert_eq!(body["error"].as_str(), Some("unauthorized"), "{body}");
+    }
+
+    #[tokio::test]
+    async fn gguf_router_admits_the_right_bearer() {
+        // The model-free state answers the chat route with its own "no model"
+        // 503 "No model available" (a 200 needs real weights); that exact answer proves the
+        // request reached the handler, so a missing route (404) or a gate refusal (401) fails.
+        let (status, _, body) = fire(
+            router(AuthGate::from_plain_key("k")),
+            Method::POST,
+            "/v1/chat/completions",
+            Some("Bearer k"),
+        )
+        .await;
+        assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE, "{body}");
+        assert!(body["error"].as_str().is_some_and(|e| e.contains("No model available")), "{body}");
+        assert_ne!(status, StatusCode::UNAUTHORIZED, "{body}");
+        let (status, _, body) = fire(
+            router(AuthGate::from_plain_key("k")),
+            Method::GET,
+            "/health",
+            Some("Bearer k"),
+        )
+        .await;
+        // A model-free state reports itself as still loading (503); the point is that
+        // the health handler answered, not the gate.
+        assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE, "{body}");
+        assert_eq!(body["status"].as_str(), Some("loading"), "{body}");
+    }
+
+    /// `AuthGate::from_env()` yields a disabled gate when neither env var is set (this test uses
+    /// `AuthGate::disabled()` directly; the `serve_router` env wiring itself is a one-line call, untested);
+    /// the seam must not change behaviour in that case (documented upstream
+    /// semantics: with no key, `from_env()` warns and serves open).
+    #[tokio::test]
+    async fn gguf_router_disabled_gate_is_open() {
+        let (status, _, body) =
+            fire(router(AuthGate::disabled()), Method::GET, "/health", None).await;
+        assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE, "{body}");
+        assert_eq!(body["status"].as_str(), Some("loading"), "{body}");
+    }
+}
+
+/// Build realizar's full router over `state` behind the Bearer gate.
+///
+/// HELIX-IDEA-009 / FALSIFY-AUTH-001: every route on the returned router is gated
+/// by `auth_gate` via `super::auth::layer`, `/health` included — the auth contract
+/// says health and metrics are NOT exempt. Behind the AWS Lambda
+/// Web Adapter that is fine (its readiness check treats any 100-499
+/// status as ready), but an ALB or a k8s httpGet probe counts a 401 as unhealthy: pass
+/// the probe the key, or `AuthGate::disabled()` to bypass.
+///
+/// The GGUF/Qwen3.5 CPU path (and every other caller of `serve_router`) was built straight from
+/// `create_router_with_config`: `APR_API_KEY_HASH` was set and
+/// ignored, and the server answered 200 without a key (observed on v0.70.3 and
+/// v0.71.4). The GPU, CUDA and APR GPU routers (`start_gguf_server_gpu_batched`,
+/// `start_gguf_server_cuda` (past its early serve_router fallback), and the APR/SafeTensors GPU starters in
+/// `handlers_include_01.rs`) are still ungated: follow-up.
+/// `serve_router` is a thin wrapper that supplies `AuthGate::from_env()`;
+/// the seam exists so the wiring is testable without a socket or model weights.
+#[cfg(feature = "inference")]
+fn build_gguf_router(
+    state: realizar::api::AppState,
+    config: &ServerConfig,
+    auth_gate: super::auth::AuthGate,
+) -> axum::Router {
+    super::auth::layer(
+        auth_gate,
+        realizar::api::create_router_with_config(state, config.router_config()),
+    )
+}
+
 /// Serve realizar's full router over `state` until Ctrl+C.
 ///
 /// The half of [`run_cpu_server`] that does not depend on which model the
 /// state holds — shared with [`start_qwen35_server`] (#3571), so the hybrid is
 /// served by the same router, banner and shutdown as every other GGUF.
 #[cfg(feature = "inference")]
 fn serve_router(state: realizar::api::AppState, config: &ServerConfig) -> Result<()> {
-    use realizar::api::create_router_with_config;
-
     // aprender#2376(8): the banner is read from the router's own table, not restated.
     // The previous hand-written list named 11 of the 31 mounted routes and omitted
     // /tokenize, /realize/*, /models and the health probes entirely, while a
@@ -441,7 +603,8 @@ fn serve_router(state: realizar::api::AppState, config: &ServerConfig) -> Result
 
     // Create realizar's full inference router (Ollama-parity endpoints).
     // --no-cors / --no-metrics must reach the router, not stop at the banner.
-    let app = create_router_with_config(state, config.router_config());
+    // FALSIFY-AUTH-001: gated by APR_API_KEY_HASH / APR_API_KEY like build_apr_cpu_router.
+    let app = build_gguf_router(state, config, super::auth::AuthGate::from_env());
 
     // Create tokio runtime and run server
     let runtime = tokio::runtime::Runtime::new()

crates/aprender-serve/src/api/mod_create_demo.rs

@@ -65,8 +65,13 @@ pub struct ChatCompletionRequest {
     pub model: String,
     /// Chat messages
     pub messages: Vec<ChatMessage>,
-    /// Maximum tokens to generate
-    #[serde(default)]
+    /// Maximum tokens to generate.
+    ///
+    /// `max_completion_tokens` is accepted as an alias: it is the newer OpenAI
+    /// name for the same cap (sent by OpenAI SDK >= 1.40 clients). A body that
+    /// carries BOTH names is refused by serde with a `duplicate field` error
+    /// rather than silently preferring one.
+    #[serde(default, alias = "max_completion_tokens")]
     pub max_tokens: Option<usize>,
     /// Sampling temperature.
     ///
@@ -182,6 +187,10 @@ pub struct ChatCompletionRequest {
     pub top_logprobs: Option<TopLogprobs>,
 }
 
+#[cfg(test)]
+#[path = "tests/max_completion_tokens_alias.rs"]
+mod max_completion_tokens_alias_tests;
+
 /// OpenAI's `response_format` (#3568 PR 4).
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 #[serde(tag = "type", rename_all = "snake_case")]
@@ -333,18 +342,28 @@ pub struct StreamOptions {
 }
 
 /// Chat message
+///
+/// Deserialization goes through [`ChatMessageWire`] (`try_from`) so that the
+/// `content` rule can depend on the sibling `role` field; serialization is the
+/// plain derive and its bytes are a contract (PMAT-801).
 #[derive(Debug, Clone, Default, Serialize, Deserialize)]
+#[serde(try_from = "ChatMessageWire")]
 pub struct ChatMessage {
     /// Role: "system", "user", "assistant", "tool"
     pub role: String,
     /// Message content.
     ///
-    /// PMAT-801: kept REQUIRED on deserialize (a request message missing
-    /// `content` is rejected 422, preserving the pre-PMAT-801 contract). The
-    /// response-side assistant tool-call message sets `content: String::new()`
-    /// directly in Rust, so it does not need a serde default. (Optional/null
-    /// content for assistant tool-call history on the REQUEST side is a
-    /// follow-up — it would require relaxing this field to Option.)
+    /// PMAT-801 follow-up: on the REQUEST side an ASSISTANT message whose
+    /// `content` key is absent or `null` reads as the empty string. OpenAI-
+    /// compatible clients (e.g. the `pmcp-agent` SDK loop) omit `content` on the
+    /// assistant tool-call turn they echo back, which the OpenAI API itself
+    /// permits; this used to be answered with HTTP 422. Every other role
+    /// (system, user, tool) still REQUIRES `content` (refused with serde's
+    /// "missing field" error as before), and a non-string `content` is still a
+    /// type error.
+    /// The response side is unchanged: the assistant tool-call message sets
+    /// `content: String::new()` directly in Rust and serializes `"content":""`
+    /// exactly as before, so the field stays a plain `String` for every reader.
     pub content: String,
     /// Optional name
     #[serde(default, skip_serializing_if = "Option::is_none")]
@@ -360,6 +379,45 @@ pub struct ChatMessage {
     pub tool_call_id: Option<String>,
 }
 
+/// Request-side wire shape of [`ChatMessage`].
+///
+/// `content` is an `Option` under `#[serde(default)]` so an absent key and a
+/// JSON `null` both read as `None` (a number or any other non-string is still a
+/// type error). The `TryFrom` conversion then decides, per role, whether `None`
+/// is acceptable.
+#[derive(Debug, Deserialize)]
+struct ChatMessageWire {
+    role: String,
+    #[serde(default)]
+    content: Option<String>,
+    #[serde(default)]
+    name: Option<String>,
+    #[serde(default)]
+    tool_calls: Option<Vec<ResponseToolCall>>,
+    #[serde(default)]
+    tool_call_id: Option<String>,
+}
+
+impl TryFrom<ChatMessageWire> for ChatMessage {
+    type Error = String;
+
+    fn try_from(wire: ChatMessageWire) -> std::result::Result<Self, Self::Error> {
+        let content = match wire.content {
+            Some(text) => text,
+            // An assistant tool-call history turn legitimately has no text.
+            None if wire.role == "assistant" => String::new(),
+            None => return Err("missing field `content`".to_string()),
+        };
+        Ok(Self {
+            role: wire.role,
+            content,
+            name: wire.name,
+            tool_calls: wire.tool_calls,
+            tool_call_id: wire.tool_call_id,
+        })
+    }
+}
+
 // ============================================================================
 // PMAT-801: OpenAI tool-calling wire types (request in, response out)
 // ============================================================================
@@ -1214,3 +1272,7 @@ pub(crate) fn default_top_k_features() -> usize {
 pub(crate) fn default_explain_method() -> String {
     "shap".to_string()
 }
+
+#[cfg(test)]
+#[path = "tests/chat_message_content_pmat801.rs"]
+mod chat_message_content_tests;