mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
* fix(gateway): serve Responses API under /api/v1/ prefix (#2201) The OpenAI Responses API was only reachable at `/v1/responses`, which broke the otherwise consistent `/api/...` prefix used by every other IronClaw HTTP surface. Callers expecting `/api/v1/responses` got a 404. This routes both paths through the same handlers: - `/api/v1/responses` + `/api/v1/responses/{id}` — canonical paths - `/v1/responses` + `/v1/responses/{id}` — retained as backward-compat aliases for clients already configured against the legacy path Also updates the web gateway CLAUDE.md route table, the USER_MANAGEMENT_API.md reference, and the module docstring for responses_api.rs so documentation points at the canonical prefix. Regression test: tests/responses_api_path_prefix.rs drives the full router via `start_server` and asserts that POST/GET on both the canonical and legacy paths reach the handler (400 from the handler for bad inputs, not 404 from the router) and that both paths enforce bearer auth (401 without a token). This follows the "Test Through the Caller, Not Just the Helper" rule so a future router edit that drops either path fails the test rather than silently regressing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(gateway): address PR #2748 review feedback - Extend both_paths_require_auth to cover GET /responses/{id} on both canonical and legacy paths. - Align USER_MANAGEMENT_API.md Responses API examples with the current handler behavior (only "default" model accepted). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: address PR #2748 reviewer nits - Change the "Go ahead with the transfer" Responses API request example to use "model": "default". The handler rejects any other value, so copying the old example verbatim would 400. - Expand the Error Format section to document that the Responses API returns an OpenAI-compatible JSON envelope ({"error": {...}}) rather than the plain-text body used by every other endpoint. Add 429 to the status-code table for Responses API rate limiting. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: address PR #2748 Copilot review nits on docs + test cleanup - Correct the documented Responses API 429 error type from `rate_limit_exceeded` to `rate_limit_error` to match what `create_response_handler` actually emits. - Clarify that the JSON error envelope covers handler-generated errors; missing/invalid bearer token (401) and auth-path 503 are returned by the shared gateway auth middleware as plain text. - Add a `ServerGuard` RAII helper in the Responses API path-prefix integration test that takes `state.shutdown_tx` on startup and sends `()` on drop, so each test tears its `axum::serve` task down instead of leaking it for the rest of the process. Update the six test callers to bind the guard. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
221 lines
7.2 KiB
Rust
221 lines
7.2 KiB
Rust
//! Regression tests for the OpenAI Responses API route prefix
|
|
//! (see ironclaw#2201).
|
|
//!
|
|
//! The canonical path is `/api/v1/responses`; the legacy `/v1/responses`
|
|
//! path is retained as an alias for backward compatibility. Both must
|
|
//! reach `create_response_handler` / `get_response_handler` and produce
|
|
//! identical behavior.
|
|
//!
|
|
//! These tests drive the full router via `start_server` rather than
|
|
//! calling the handler in isolation — per `.claude/rules/testing.md`
|
|
//! ("Test Through the Caller, Not Just the Helper"), the regression
|
|
//! coverage has to exercise the router wiring, otherwise a future
|
|
//! rename / removal of one path silently loses the coverage.
|
|
|
|
use std::net::SocketAddr;
|
|
use std::sync::Arc;
|
|
use std::time::Duration;
|
|
|
|
use ironclaw::channels::web::auth::MultiAuthState;
|
|
use ironclaw::channels::web::platform::router::start_server;
|
|
use ironclaw::channels::web::platform::state::GatewayState;
|
|
use ironclaw::channels::web::test_helpers::TestGatewayBuilder;
|
|
use tokio::sync::oneshot;
|
|
|
|
const AUTH_TOKEN: &str = "test-responses-api-token";
|
|
|
|
/// RAII guard that shuts the gateway test server down when dropped,
|
|
/// even on early returns or panics. Without this, every `#[tokio::test]`
|
|
/// would leak its spawned `axum::serve` task for the remainder of the
|
|
/// test process.
|
|
struct ServerGuard {
|
|
shutdown: Option<oneshot::Sender<()>>,
|
|
}
|
|
|
|
impl Drop for ServerGuard {
|
|
fn drop(&mut self) {
|
|
if let Some(tx) = self.shutdown.take() {
|
|
// Best-effort: the receiver may already be gone if the
|
|
// serve task exited for another reason. Either way, we've
|
|
// released our half of the channel.
|
|
let _ = tx.send(());
|
|
}
|
|
}
|
|
}
|
|
|
|
async fn start_test_server() -> (SocketAddr, Arc<GatewayState>, ServerGuard) {
|
|
let state = TestGatewayBuilder::new().user_id("test-user").build();
|
|
let auth = MultiAuthState::single(AUTH_TOKEN.to_string(), "test-user".to_string());
|
|
let addr: SocketAddr = "127.0.0.1:0"
|
|
.parse()
|
|
.expect("hard-coded address must parse");
|
|
let bound = start_server(addr, state.clone(), auth.into())
|
|
.await
|
|
.expect("start gateway test server");
|
|
let shutdown = state.shutdown_tx.write().await.take();
|
|
(bound, state, ServerGuard { shutdown })
|
|
}
|
|
|
|
fn client() -> reqwest::Client {
|
|
reqwest::Client::builder()
|
|
.timeout(Duration::from_secs(5))
|
|
.build()
|
|
.expect("build test http client")
|
|
}
|
|
|
|
/// POST `/api/v1/responses` must route to `create_response_handler` —
|
|
/// not 404. We send a deliberately invalid `model` so the handler
|
|
/// short-circuits with 400 before touching the agent loop; the important
|
|
/// assertion is "the route exists".
|
|
#[tokio::test]
|
|
async fn canonical_post_path_routes_to_handler() {
|
|
let (addr, _state, _guard) = start_test_server().await;
|
|
let url = format!("http://{}/api/v1/responses", addr);
|
|
|
|
let resp = client()
|
|
.post(&url)
|
|
.bearer_auth(AUTH_TOKEN)
|
|
.json(&serde_json::json!({
|
|
"model": "not-a-real-model",
|
|
"input": "hello",
|
|
}))
|
|
.send()
|
|
.await
|
|
.expect("POST /api/v1/responses");
|
|
|
|
// The handler rejects non-"default" models with 400, which proves the
|
|
// request reached `create_response_handler` rather than the router's
|
|
// fallback 404. A 404 here would mean the route isn't registered.
|
|
assert_eq!(
|
|
resp.status(),
|
|
400,
|
|
"expected 400 from handler, got {}: {}",
|
|
resp.status(),
|
|
resp.text().await.unwrap_or_default()
|
|
);
|
|
}
|
|
|
|
/// Legacy alias `POST /v1/responses` must still route to the same
|
|
/// handler (backward compatibility with clients that were configured
|
|
/// against the pre-#2201 path).
|
|
#[tokio::test]
|
|
async fn legacy_post_path_still_routes_to_handler() {
|
|
let (addr, _state, _guard) = start_test_server().await;
|
|
let url = format!("http://{}/v1/responses", addr);
|
|
|
|
let resp = client()
|
|
.post(&url)
|
|
.bearer_auth(AUTH_TOKEN)
|
|
.json(&serde_json::json!({
|
|
"model": "not-a-real-model",
|
|
"input": "hello",
|
|
}))
|
|
.send()
|
|
.await
|
|
.expect("POST /v1/responses");
|
|
|
|
assert_eq!(
|
|
resp.status(),
|
|
400,
|
|
"legacy path must reach handler, got {}: {}",
|
|
resp.status(),
|
|
resp.text().await.unwrap_or_default()
|
|
);
|
|
}
|
|
|
|
/// GET `/api/v1/responses/{id}` with a malformed id must return 400
|
|
/// from the handler (invalid response ID) — proving the route is
|
|
/// registered and the path parameter is reaching the handler.
|
|
#[tokio::test]
|
|
async fn canonical_get_path_routes_to_handler() {
|
|
let (addr, _state, _guard) = start_test_server().await;
|
|
let url = format!("http://{}/api/v1/responses/not_a_valid_id", addr);
|
|
|
|
let resp = client()
|
|
.get(&url)
|
|
.bearer_auth(AUTH_TOKEN)
|
|
.send()
|
|
.await
|
|
.expect("GET /api/v1/responses/{id}");
|
|
|
|
assert_eq!(
|
|
resp.status(),
|
|
400,
|
|
"expected 400 from handler for malformed id, got {}: {}",
|
|
resp.status(),
|
|
resp.text().await.unwrap_or_default()
|
|
);
|
|
}
|
|
|
|
/// GET `/v1/responses/{id}` (legacy alias) must also route to the same
|
|
/// handler.
|
|
#[tokio::test]
|
|
async fn legacy_get_path_still_routes_to_handler() {
|
|
let (addr, _state, _guard) = start_test_server().await;
|
|
let url = format!("http://{}/v1/responses/not_a_valid_id", addr);
|
|
|
|
let resp = client()
|
|
.get(&url)
|
|
.bearer_auth(AUTH_TOKEN)
|
|
.send()
|
|
.await
|
|
.expect("GET /v1/responses/{id}");
|
|
|
|
assert_eq!(
|
|
resp.status(),
|
|
400,
|
|
"legacy path must reach handler, got {}: {}",
|
|
resp.status(),
|
|
resp.text().await.unwrap_or_default()
|
|
);
|
|
}
|
|
|
|
/// Both paths must enforce bearer-token auth. A missing token should
|
|
/// return 401, not 404 (which would indicate the route is missing).
|
|
#[tokio::test]
|
|
async fn both_paths_require_auth() {
|
|
let (addr, _state, _guard) = start_test_server().await;
|
|
|
|
for path in ["/api/v1/responses", "/v1/responses"] {
|
|
let url = format!("http://{}{}", addr, path);
|
|
let resp = client()
|
|
.post(&url)
|
|
.json(&serde_json::json!({ "model": "default", "input": "hi" }))
|
|
.send()
|
|
.await
|
|
.unwrap_or_else(|e| panic!("POST {path}: {e}"));
|
|
assert_eq!(
|
|
resp.status(),
|
|
401,
|
|
"{path} should return 401 without a token, got {}",
|
|
resp.status()
|
|
);
|
|
}
|
|
}
|
|
|
|
/// Both GET item paths (`/api/v1/responses/{id}` and `/v1/responses/{id}`)
|
|
/// must also enforce bearer-token auth. A missing token should return 401,
|
|
/// not 404 — the auth middleware has to apply to legacy aliases as well.
|
|
#[tokio::test]
|
|
async fn both_get_paths_require_auth() {
|
|
let (addr, _state, _guard) = start_test_server().await;
|
|
|
|
for path in [
|
|
"/api/v1/responses/not_a_valid_id",
|
|
"/v1/responses/not_a_valid_id",
|
|
] {
|
|
let url = format!("http://{}{}", addr, path);
|
|
let resp = client()
|
|
.get(&url)
|
|
.send()
|
|
.await
|
|
.unwrap_or_else(|e| panic!("GET {path}: {e}"));
|
|
assert_eq!(
|
|
resp.status(),
|
|
401,
|
|
"{path} should return 401 without a token, got {}",
|
|
resp.status()
|
|
);
|
|
}
|
|
}
|