diff --git a/crates/ghostlight-dungeon/Cargo.toml b/crates/ghostlight-dungeon/Cargo.toml index ff99b00..1923bca 100644 --- a/crates/ghostlight-dungeon/Cargo.toml +++ b/crates/ghostlight-dungeon/Cargo.toml @@ -42,5 +42,6 @@ uuid.workspace = true zeroize.workspace = true [dev-dependencies] +ghostlight = { path = "../ghostlight", features = ["test-support"] } tempfile = "3" tower = { version = "0.5", features = ["util"] } diff --git a/crates/ghostlight-dungeon/src/play.rs b/crates/ghostlight-dungeon/src/play.rs index b8a340f..b35a6b3 100644 --- a/crates/ghostlight-dungeon/src/play.rs +++ b/crates/ghostlight-dungeon/src/play.rs @@ -1758,11 +1758,12 @@ impl PlayTable { async fn close_with_fault(&self, turn: &mut PlayTurn, detail: String) -> Result<(), PlayError> { turn.state = PlayTurnState::Closed; // R2: the one place a fault closes a turn, so the one place it is - // logged, whole. The local inference port builds its detail from the - // fault kind and never from the transport error's text (tests pin - // that for each local failure class). The connector and SDK ports - // build details from their own error text, which this line does not - // scrub. + // logged, whole. An `InferenceFault`'s detail is a `&'static str`, so + // no inference port (local, connector or SDK) can have put an + // endpoint, request or reply text into it; the type guarantees that, + // not this line. The details of the other failures that reach here + // (kernel, request build, snapshot) are their callers' own error + // text, and nothing here constrains them. tracing::warn!(turn_id = %turn.turn_id, detail = %detail, "play turn closed on a fault"); turn.fault = Some(detail); // PA.f173: a fault can close a turn that was `AwaitingPlayer`, and @@ -3303,7 +3304,7 @@ pub(crate) mod tests { /// exercise `world.play`'s own staleness comparison against a question /// that actually opened, rather than one hand-built. pub(crate) struct ScriptedPort { - queue: StdMutex>>, + queue: StdMutex>>, seen: StdMutex>, } @@ -8226,18 +8227,15 @@ pub(crate) mod tests { "gpt-5.6-sol".into(), ) .unwrap(); - let inference = ghostlight::open_inference( - None, - None, - Some(ghostlight::LocalBinding { - endpoint, - model_prefix: "local/".into(), - caller_runtime_id: "play-test-runtime".into(), - response_timeout: timeout, - }), - &["local/play"], - ) - .unwrap(); + let binding = ghostlight::LocalBinding { + endpoint, + model_prefix: "local/".into(), + caller_runtime_id: "play-test-runtime".into(), + }; + let inference = match timeout { + Some(timeout) => ghostlight::open_local_port_with_timeout(binding, timeout).unwrap(), + None => ghostlight::open_inference(None, None, Some(binding), &["local/play"]).unwrap(), + }; let table = PlayTable::new( fixture.world.clone(), personas, @@ -8262,6 +8260,7 @@ pub(crate) mod tests { account: &str, turn_number: u128, kind: &str, + reply_canaries: &[&str], ) { let (logs, _guard) = capture_logs(); let directory = tempfile::tempdir().unwrap(); @@ -8277,7 +8276,10 @@ pub(crate) mod tests { let log = logs.text(); let debug = format!("{view:?} {:?}", table.store.lock().await.current().unwrap()); assert!(stored.contains(kind), "the error kind is kept: {stored}"); - for canary in [endpoint.to_string(), endpoint.ip().to_string(), endpoint.port().to_string(), "chat/completions".to_owned()] { + let canaries = [endpoint.to_string(), endpoint.ip().to_string(), endpoint.port().to_string(), "chat/completions".to_owned()] + .into_iter() + .chain(reply_canaries.iter().map(|canary| (*canary).to_owned())); + for canary in canaries { for (where_, text) in [("stored fault", &stored), ("warn line", &log), ("card row", &row), ("debug", &debug)] { assert!(!text.contains(&canary), "{canary} leaked into the {where_}: {text}"); } @@ -8291,7 +8293,7 @@ pub(crate) mod tests { let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); listener.local_addr().unwrap() }; - assert_faulted_turn_names_no_endpoint(endpoint, None, "player-fault-canary", 431, "refused the connection").await; + assert_faulted_turn_names_no_endpoint(endpoint, None, "player-fault-canary", 431, "refused the connection", &[]).await; } /// The whole path with a timeout, the leg a Bonsai stall takes: the @@ -8314,6 +8316,71 @@ pub(crate) mod tests { "player-fault-timeout", 432, "timed out", + &[], + ) + .await; + } + + /// Reads one HTTP request (headers and the declared body) off `stream`. + fn read_http_request(stream: &mut std::net::TcpStream) { + use std::io::Read; + let mut buf = Vec::new(); + let mut chunk = [0u8; 8192]; + loop { + let Ok(read) = stream.read(&mut chunk) else { break }; + if read == 0 { + break; + } + buf.extend_from_slice(&chunk[..read]); + let text = String::from_utf8_lossy(&buf).to_string(); + if let Some(split) = text.find("\r\n\r\n") { + let len = text[..split] + .to_ascii_lowercase() + .lines() + .find_map(|l| l.strip_prefix("content-length:").map(|v| v.trim().parse::().unwrap())) + .unwrap_or(0); + if buf.len() >= split + 4 + len { + break; + } + } + } + } + + /// Loopback server that answers every request with a `200` whose whole + /// body is `body`. + fn replying_with(body: &'static str) -> std::net::SocketAddr { + use std::io::Write; + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + std::thread::spawn(move || { + for stream in listener.incoming() { + let Ok(mut stream) = stream else { return }; + read_http_request(&mut stream); + let _ = write!( + stream, + "HTTP/1.1 200 OK\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{}", + body.len(), + body + ); + } + }); + addr + } + + /// The malformed-reply leg through the whole path: a whole body that is + /// not the declared shape closes the turn on a fault, and serde's text + /// for it, which echoes the reply's own value, reaches the stored fault, + /// the warn line, the card row and Debug nowhere. + #[tokio::test] + async fn a_malformed_local_reply_closes_the_turn_without_echoing_it() { + let endpoint = replying_with("\\"CANARY-reply-5d02c8\\""); + assert_faulted_turn_names_no_endpoint( + endpoint, + None, + "player-fault-malformed", + 433, + "not the declared shape", + &["CANARY-reply-5d02c8"], ) .await; } @@ -8323,7 +8390,7 @@ pub(crate) mod tests { /// answers each later one with an `end_turn` tool call. Returns its /// address and the count of connections seen. fn closing_then_ending(drops: usize) -> (std::net::SocketAddr, Arc) { - use std::io::{Read, Write}; + use std::io::Write; use std::sync::atomic::{AtomicUsize, Ordering}; let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); let addr = listener.local_addr().unwrap(); @@ -8337,26 +8404,7 @@ pub(crate) mod tests { drop(stream); continue; } - let mut buf = Vec::new(); - let mut chunk = [0u8; 8192]; - loop { - let Ok(read) = stream.read(&mut chunk) else { break }; - if read == 0 { - break; - } - buf.extend_from_slice(&chunk[..read]); - let text = String::from_utf8_lossy(&buf).to_string(); - if let Some(split) = text.find("\r\n\r\n") { - let len = text[..split] - .to_ascii_lowercase() - .lines() - .find_map(|l| l.strip_prefix("content-length:").map(|v| v.trim().parse::().unwrap())) - .unwrap_or(0); - if buf.len() >= split + 4 + len { - break; - } - } - } + read_http_request(&mut stream); let body = serde_json::json!({ "id": format!("resp-{n}"), "choices": [{"message": {"role": "assistant", "content": null, diff --git a/crates/ghostlight-dungeon/src/runtime.rs b/crates/ghostlight-dungeon/src/runtime.rs index dfbb870..6f83252 100644 --- a/crates/ghostlight-dungeon/src/runtime.rs +++ b/crates/ghostlight-dungeon/src/runtime.rs @@ -576,7 +576,6 @@ fn open_controller( model_prefix: std::env::var("GHOSTLIGHT_LOCAL_MODEL_PREFIX") .unwrap_or_else(|_| DEFAULT_LOCAL_MODEL_PREFIX.into()), caller_runtime_id: runtime_id.to_owned(), - response_timeout: None, }), Err(_) => None, }; diff --git a/crates/ghostlight/Cargo.toml b/crates/ghostlight/Cargo.toml index 1d66013..765334b 100644 --- a/crates/ghostlight/Cargo.toml +++ b/crates/ghostlight/Cargo.toml @@ -22,5 +22,10 @@ tracing.workspace = true uuid.workspace = true zeroize.workspace = true +[features] +# Exposes `open_local_port_with_timeout` to a consumer's tests. Production +# builds never enable it. +test-support = [] + [dev-dependencies] tempfile = "3" diff --git a/crates/ghostlight/src/controllers.rs b/crates/ghostlight/src/controllers.rs index f369bd6..64ec485 100644 --- a/crates/ghostlight/src/controllers.rs +++ b/crates/ghostlight/src/controllers.rs @@ -199,7 +199,12 @@ impl InferenceOutput { pub struct InferenceFault { disposition: InferenceFaultDisposition, class: InferenceFaultClass, - detail: String, + /// Fixed text chosen by the port that built the fault. It is a + /// `&'static str`, so no constructor can be handed text that was + /// computed from a request, a reply, an endpoint or an error's own + /// `Display`: a fault names what kind of trouble it is and never carries + /// an input value. The type enforces this for every port. + detail: &'static str, } /// What kind of trouble a fault is, as a typed value: the one place a reader @@ -236,37 +241,37 @@ pub enum InferenceFaultDisposition { } impl InferenceFault { - pub(super) fn new(detail: impl Into) -> Self { + pub(super) fn new(detail: &'static str) -> Self { Self { disposition: InferenceFaultDisposition::RecoveryRequired, class: InferenceFaultClass::Unclassified, - detail: detail.into(), + detail: detail, } } /// Transport trouble the owning flow may retry. - pub fn retryable(detail: impl Into) -> Self { + pub fn retryable(detail: &'static str) -> Self { Self { disposition: InferenceFaultDisposition::Retryable, class: InferenceFaultClass::Unclassified, - detail: detail.into(), + detail: detail, } } /// The one disposition that makes `ControllerError::requires_quarantine` /// true for an `Inference` fault: the provider's answer cannot be trusted, /// so the tick driver stops rather than commits. - pub fn integrity_violation(detail: impl Into) -> Self { + pub fn integrity_violation(detail: &'static str) -> Self { Self { disposition: InferenceFaultDisposition::IntegrityViolation, class: InferenceFaultClass::Unclassified, - detail: detail.into(), + detail: detail, } } /// The non-quarantining failure: this purpose cannot complete, but a /// sibling cell in the same tick is unaffected. - pub fn recovery_required(detail: impl Into) -> Self { + pub fn recovery_required(detail: &'static str) -> Self { Self::new(detail) } @@ -457,7 +462,8 @@ impl PreparedInference { request: InferenceRequest, ) -> Result { let request_bytes = - serde_json::to_vec(&request).map_err(|error| InferenceFault::new(error.to_string()))?; + serde_json::to_vec(&request) + .map_err(|_| InferenceFault::new("an inference request could not be serialized"))?; let purpose = request.purpose; let invocation = CodexTransportInvocation::new( caller_runtime_id, @@ -465,7 +471,7 @@ impl PreparedInference { Sha256::digest(request_bytes).into(), request.provider, ) - .map_err(|error| InferenceFault::new(error.to_string()))?; + .map_err(|_| InferenceFault::new("an inference invocation could not be built"))?; Ok(PreparedInference { purpose, invocation, @@ -500,7 +506,7 @@ impl CodexConnectorInferencePort { MAX_CONNECTOR_FRAME_BYTES, Some(RESPONSE_TIMEOUT), ) - .map_err(|error| InferenceFault::new(error.to_string()))?; + .map_err(|_| InferenceFault::new("the CodexConnector client could not be built"))?; Ok(Self { client, caller_runtime_id, @@ -513,10 +519,11 @@ impl CodexConnectorInferencePort { caller_runtime_id: impl Into, ) -> Result { let bytes = Zeroizing::new( - std::fs::read(path.as_ref()).map_err(|error| InferenceFault::new(error.to_string()))?, + std::fs::read(path.as_ref()) + .map_err(|_| InferenceFault::new("the CodexConnector key file could not be read"))?, ); let raw = std::str::from_utf8(bytes.as_slice()) - .map_err(|error| InferenceFault::new(error.to_string()))?; + .map_err(|_| InferenceFault::new("the CodexConnector key file is not UTF-8"))?; let key = raw.trim_end_matches(['\r', '\n']); if key.is_empty() || key.len() != raw.trim().len() { return Err(InferenceFault::new( @@ -537,42 +544,51 @@ impl CodexConnectorInferencePort { .execute(&request.invocation) .map_err(|error| match error { CodexConnectorClientError::Connection(_) => { - InferenceFault::retryable(error.to_string()) + InferenceFault::retryable("the CodexConnector connection failed") } CodexConnectorClientError::InvalidConfig | CodexConnectorClientError::FrameSize | CodexConnectorClientError::Encoding | CodexConnectorClientError::Transport(_) => { - InferenceFault::integrity_violation(error.to_string()) + InferenceFault::integrity_violation("the CodexConnector transport failed") } })?; let (events, receipt) = match result.disposition { CodexTransportDisposition::Refused(reason) => { - let detail = format!("CodexConnector refused request: {reason:?}"); return Err(match reason { - CodexRefusal::InFlight | CodexRefusal::Capacity => { - InferenceFault::retryable(detail) + CodexRefusal::InFlight => { + InferenceFault::retryable("CodexConnector refused request: in flight") } - CodexRefusal::Expired | CodexRefusal::Indeterminate => { - InferenceFault::new(detail) + CodexRefusal::Capacity => { + InferenceFault::retryable("CodexConnector refused request: at capacity") } - CodexRefusal::IdentitySubstitution - | CodexRefusal::ProviderDigestSubstitution - | CodexRefusal::Policy - | CodexRefusal::ReplayConflict - | CodexRefusal::Malformed => InferenceFault::integrity_violation(detail), + CodexRefusal::Expired => { + InferenceFault::new("CodexConnector refused request: expired") + } + CodexRefusal::Indeterminate => { + InferenceFault::new("CodexConnector refused request: indeterminate") + } + CodexRefusal::IdentitySubstitution => InferenceFault::integrity_violation( + "CodexConnector refused request: identity substitution", + ), + CodexRefusal::ProviderDigestSubstitution => InferenceFault::integrity_violation( + "CodexConnector refused request: provider digest substitution", + ), + CodexRefusal::Policy => { + InferenceFault::integrity_violation("CodexConnector refused request: policy") + } + CodexRefusal::ReplayConflict => InferenceFault::integrity_violation( + "CodexConnector refused request: replay conflict", + ), + CodexRefusal::Malformed => InferenceFault::integrity_violation( + "CodexConnector refused request: malformed", + ), }); } CodexTransportDisposition::Transported { events, receipt } => (events, receipt), }; - if let CodexTransportOutcome::Failed { - failure_kind, - message, - } = &receipt.outcome - { - return Err(InferenceFault::new(format!( - "Codex provider failed ({failure_kind}): {message}" - ))); + if let CodexTransportOutcome::Failed { .. } = &receipt.outcome { + return Err(InferenceFault::new("the Codex provider reported a failed outcome")); } let events = events .into_iter() @@ -590,7 +606,7 @@ impl CodexConnectorInferencePort { }) .collect(); let receipt_bytes = rmp_serde::to_vec_named(&receipt) - .map_err(|error| InferenceFault::new(error.to_string()))?; + .map_err(|_| InferenceFault::new("the CodexConnector receipt could not be encoded"))?; Ok(InferenceOutput { events, receipt_digest: format!("sha256:{:x}", Sha256::digest(&receipt_bytes)), @@ -612,14 +628,14 @@ impl InferencePort for CodexConnectorInferencePort { let port = self.clone(); tokio::task::spawn_blocking(move || port.execute(request)) .await - .map_err(|error| InferenceFault::new(error.to_string()))? + .map_err(|_| InferenceFault::new("the CodexConnector worker task failed"))? } } pub(super) fn unix_ms() -> Result { let millis = SystemTime::now() .duration_since(UNIX_EPOCH) - .map_err(|error| InferenceFault::new(error.to_string()))? + .map_err(|_| InferenceFault::new("the system clock is before the Unix epoch"))? .as_millis(); u64::try_from(millis).map_err(|_| InferenceFault::new("system time exceeds u64 milliseconds")) } @@ -6833,6 +6849,34 @@ fn purpose_name(purpose: InferencePurpose) -> &'static str { #[cfg(test)] mod tests { + /// The connector port's share of the no-input-text rule: a key-file path + /// and a runtime identity each arrive as a canary, and the fault names + /// only what failed. (The legs that need a live connector, a refusal or a + /// failed provider outcome, are fixed text by the fault type.) + #[test] + fn a_connector_fault_carries_no_text_it_was_handed() { + const CANARY: &str = "CANARY-connector-2c4d7a"; + let endpoint: SocketAddr = "127.0.0.1:9".parse().unwrap(); + let directory = tempfile::tempdir().unwrap(); + let not_utf8 = directory.path().join(CANARY); + std::fs::write(¬_utf8, [0xff_u8, 0xfe]).unwrap(); + let faults = [ + CodexConnectorInferencePort::from_secret_file(endpoint, directory.path().join("absent").join(CANARY), "runtime") + .err() + .expect("a missing key file opened"), + CodexConnectorInferencePort::from_secret_file(endpoint, ¬_utf8, "runtime") + .err() + .expect("a key file that is not UTF-8 opened"), + CodexConnectorInferencePort::new(endpoint, "key".into(), format!(" {CANARY} ")) + .err() + .expect("a runtime identity with whitespace opened"), + ]; + for fault in faults { + let rendered = format!("{fault} | {fault:?}"); + assert!(!rendered.contains(CANARY), "{CANARY} leaked into {rendered}"); + } + } + /// The decision lanes' tools go to the same provider under the same /// strict rules as the patch catalog; the interpreter's fixed tools and a /// granted catalog entry are checked offline here for the same reason. @@ -11445,7 +11489,6 @@ mod tests { endpoint: local_endpoint, model_prefix: local_model_prefix, caller_runtime_id: runtime_id, - response_timeout: None, }), &models.each(), ) diff --git a/crates/ghostlight/src/lib.rs b/crates/ghostlight/src/lib.rs index 6576eab..68a60f5 100644 --- a/crates/ghostlight/src/lib.rs +++ b/crates/ghostlight/src/lib.rs @@ -97,6 +97,8 @@ pub use elaboration::{ }; pub use lens::{Lens, LensWeights}; pub use local_inference::{DEFAULT_LOCAL_MODEL_PREFIX, LocalBinding}; +#[cfg(feature = "test-support")] +pub use local_inference::open_local_port_with_timeout; pub(crate) use mailbox::ElaborationPort; pub use mailbox::{ConsumerPort, ControllerPort, MailboxError, PlayPort, SeedPort, WorldMailbox}; pub use patch::{ diff --git a/crates/ghostlight/src/local_inference.rs b/crates/ghostlight/src/local_inference.rs index c55ecc8..3cd420e 100644 --- a/crates/ghostlight/src/local_inference.rs +++ b/crates/ghostlight/src/local_inference.rs @@ -31,10 +31,6 @@ pub struct LocalBinding { pub endpoint: SocketAddr, pub model_prefix: String, pub caller_runtime_id: String, - /// How long one request may take, reply body included. `None` is - /// `RESPONSE_TIMEOUT`; a consumer or test sets it to reach the timeout - /// path without waiting that out. - pub response_timeout: Option, } const LOCAL_RECEIPT_SCHEMA: &str = "ghostlight.local_inference_receipt.v1"; @@ -135,6 +131,10 @@ pub(super) struct LocalInferencePort { endpoint: SocketAddr, prefix: String, caller_runtime_id: String, + /// Read only by the tests that pin which timeout a constructor resolves + /// to; the live timeout is the client's. + #[cfg(test)] + response_timeout: std::time::Duration, } impl LocalInferencePort { @@ -154,8 +154,10 @@ impl LocalInferencePort { Self::with_timeout(endpoint, prefix, caller_runtime_id, RESPONSE_TIMEOUT) } - /// `new` with the response timeout named, so a test can reach the timeout - /// path without waiting out `RESPONSE_TIMEOUT`. + /// `new` with the response timeout named. `new` is the only production + /// caller and passes `RESPONSE_TIMEOUT`, so production has one timeout + /// owner; the other callers are tests that reach the timeout path without + /// waiting `RESPONSE_TIMEOUT` out. fn with_timeout( endpoint: SocketAddr, prefix: impl Into, @@ -175,6 +177,8 @@ impl LocalInferencePort { endpoint, prefix: prefix.into(), caller_runtime_id: caller_runtime_id.into(), + #[cfg(test)] + response_timeout: timeout, }) } @@ -188,7 +192,7 @@ impl LocalInferencePort { .map_err(send_fault)?; let status = response.status(); if !status.is_success() { - let detail = format!("the local inference endpoint returned {status}"); + let detail = "the local inference endpoint returned a non-success status"; let class = InferenceFaultClass::Status(status.as_u16()); return Err(if status.as_u16() == 429 || status.as_u16() == 503 { InferenceFault::retryable(detail) @@ -268,11 +272,8 @@ fn lower_request(prepared: &PreparedInference, prefix: &str) -> Result Result, ControllerOpenError> { + Ok(Arc::new(local_port(binding)?)) +} + +/// The same port with a named response timeout, for a consumer's tests that +/// reach the timeout path without waiting `RESPONSE_TIMEOUT` out. It exists +/// only under the `test-support` feature, which production builds never +/// enable, so `LocalBinding` carries no timeout and a release has exactly +/// one: `RESPONSE_TIMEOUT`. +#[cfg(feature = "test-support")] +pub fn open_local_port_with_timeout( + binding: LocalBinding, + timeout: std::time::Duration, ) -> Result, ControllerOpenError> { Ok(Arc::new(LocalInferencePort::with_timeout( binding.endpoint, binding.model_prefix, binding.caller_runtime_id, - binding.response_timeout.unwrap_or(RESPONSE_TIMEOUT), + timeout, )?)) } +fn local_port(binding: LocalBinding) -> Result { + LocalInferencePort::new(binding.endpoint, binding.model_prefix, binding.caller_runtime_id) +} + #[cfg(test)] mod tests { use super::*; @@ -805,7 +823,6 @@ mod tests { endpoint: "93.184.216.34:443".parse().unwrap(), model_prefix: DEFAULT_LOCAL_MODEL_PREFIX.into(), caller_runtime_id: TEST_RUNTIME.into(), - response_timeout: None, }), &[TEST_MODEL], ) @@ -912,7 +929,6 @@ mod tests { endpoint: "127.0.0.1:1".parse().unwrap(), model_prefix: "claude".into(), caller_runtime_id: TEST_RUNTIME.into(), - response_timeout: None, }), &["claude-opus-5"], ) @@ -1195,7 +1211,6 @@ mod tests { endpoint: "127.0.0.1:1".parse().unwrap(), model_prefix: String::new(), caller_runtime_id: TEST_RUNTIME.into(), - response_timeout: None, }), &[TEST_MODEL], ) @@ -1659,6 +1674,48 @@ mod tests { assert_eq!(fault.disposition(), InferenceFaultDisposition::IntegrityViolation, "{fault:?}"); } + /// The malformed-body leg of the no-input-text rule: serde's `Display` + /// for a type mismatch echoes the offending value, so a detail built from + /// it carries the reply's own text. The canary is a value inside a whole + /// body that fails to deserialize. + #[tokio::test] + async fn a_malformed_body_carries_neither_its_own_text_nor_the_endpoint() { + const CANARY: &str = "CANARY-reply-7f3e91"; + let addr = ScriptedResponder::start(vec![(200, format!("\"{CANARY}\""))]).await.endpoint(); + let fault = infer_once(&port(addr)).await.expect_err("a malformed body produced an output"); + assert_eq!(fault.class(), InferenceFaultClass::BadReply, "{fault:?}"); + let rendered = format!("{fault} | {fault:?}"); + for canary in [CANARY.to_owned(), addr.to_string(), addr.ip().to_string()] { + assert!(!rendered.contains(&canary), "{canary} leaked into {rendered}"); + } + } + + /// The production default is `RESPONSE_TIMEOUT` (a cold model load lives + /// inside one request) at the constructor and at the port a binding + /// opens: a shorter default turns every cold start into a Timeout fault + /// that is never retried. + #[test] + fn the_production_default_timeout_is_response_timeout_at_both_resolving_sites() { + assert_eq!(RESPONSE_TIMEOUT, std::time::Duration::from_secs(900)); + let endpoint: SocketAddr = "127.0.0.1:9".parse().unwrap(); + let constructed = { + let _guard = client_build_lock().lock().expect("the client-build lock is never poisoned"); + LocalInferencePort::new(endpoint, DEFAULT_LOCAL_MODEL_PREFIX, TEST_RUNTIME) + .expect("a loopback endpoint opens") + }; + assert_eq!(constructed.response_timeout, RESPONSE_TIMEOUT); + let opened = { + let _guard = client_build_lock().lock().expect("the client-build lock is never poisoned"); + local_port(LocalBinding { + endpoint, + model_prefix: DEFAULT_LOCAL_MODEL_PREFIX.into(), + caller_runtime_id: TEST_RUNTIME.into(), + }) + .expect("a loopback binding opens") + }; + assert_eq!(opened.response_timeout, RESPONSE_TIMEOUT); + } + /// The timeout leg of the no-endpoint rule: reqwest's `Display` for a /// timeout ends with the request URL, so a detail built from it names the /// canary endpoint this test bound. Covers both a stalled send and a diff --git a/crates/ghostlight/src/sdk_inference.rs b/crates/ghostlight/src/sdk_inference.rs index 59eddee..da7894f 100644 --- a/crates/ghostlight/src/sdk_inference.rs +++ b/crates/ghostlight/src/sdk_inference.rs @@ -146,8 +146,27 @@ pub(super) enum SidecarFaultReason { } impl SidecarFaultReason { - fn into_fault(self, detail: String) -> InferenceFault { - let detail = format!("SDK sidecar reported {self:?}: {detail}"); + /// The sidecar's own `detail` text is never carried: the reason names the + /// failure and the fault's text is fixed per reason. + fn into_fault(self) -> InferenceFault { + let detail = match self { + Self::RateLimited => "SDK sidecar reported rate limited", + Self::Overloaded => "SDK sidecar reported overloaded", + Self::ServerError => "SDK sidecar reported a server error", + Self::ApiTimeout => "SDK sidecar reported an API timeout", + Self::AuthenticationFailed => "SDK sidecar reported authentication failed", + Self::OrgNotAllowed => "SDK sidecar reported organization not allowed", + Self::BillingError => "SDK sidecar reported a billing error", + Self::InvalidRequest => "SDK sidecar reported an invalid request", + Self::ModelNotFound => "SDK sidecar reported model not found", + Self::MaxOutputTokens => "SDK sidecar reported max output tokens", + Self::MaxBudgetUsd => "SDK sidecar reported max budget", + Self::ExecutionError => "SDK sidecar reported an execution error", + Self::Unknown => "SDK sidecar reported an unknown failure", + Self::ProtocolViolation => "SDK sidecar reported a protocol violation", + Self::ToolRegistrationFailed => "SDK sidecar reported tool registration failed", + Self::TurnCapRefused => "SDK sidecar reported the turn cap refused", + }; match self { Self::RateLimited | Self::Overloaded | Self::ServerError | Self::ApiTimeout => { InferenceFault::retryable(detail) @@ -468,7 +487,7 @@ impl SdkInferencePort { } Err(error) => { let _ = self.link.restart().await; - return Err(InferenceFault::new(error.to_string())); + return Err(InferenceFault::new("the tool-result owner refused a tool call")); } } } @@ -482,10 +501,10 @@ impl SdkInferencePort { SidecarFrame::Fault { query_id: seen, reason, - detail, + .. } if seen == query_id => { let _ = self.link.restart().await; - return Err(reason.into_fault(detail)); + return Err(reason.into_fault()); } _ => { let _ = self.link.restart().await; @@ -501,13 +520,11 @@ impl SdkInferencePort { fn link_fault(error: SidecarLinkError) -> InferenceFault { match error { SidecarLinkError::Closed => { - InferenceFault::new("the SDK sidecar exited during a query".to_string()) + InferenceFault::new("the SDK sidecar exited during a query") } - SidecarLinkError::Spawn(detail) => { - InferenceFault::new(format!("the SDK sidecar could not start: {detail}")) - } - SidecarLinkError::Codec(detail) => { - InferenceFault::integrity_violation(format!("SDK sidecar framing: {detail}")) + SidecarLinkError::Spawn(_) => InferenceFault::new("the SDK sidecar could not start"), + SidecarLinkError::Codec(_) => { + InferenceFault::integrity_violation("the SDK sidecar's framing was invalid") } } } @@ -526,9 +543,9 @@ pub(super) fn lower_query( if let Some(effort) = request.reasoning_effort.as_deref() && !SDK_EFFORT_LEVELS.contains(&effort) { - return Err(InferenceFault::integrity_violation(format!( - "reasoning effort `{effort}` has no SDK counterpart" - ))); + return Err(InferenceFault::integrity_violation( + "the requested reasoning effort has no SDK counterpart", + )); } let mut items = request.input.iter(); let Some(CodexInputItem::UserText { text: prompt }) = items.next() else { @@ -641,7 +658,7 @@ fn assemble_output( total_cost_usd_estimate: material.total_cost_usd_estimate, }; let receipt_bytes = rmp_serde::to_vec_named(&receipt) - .map_err(|error| InferenceFault::new(error.to_string()))?; + .map_err(|_| InferenceFault::new("the SDK receipt could not be encoded"))?; Ok(InferenceOutput { events: lowered, receipt_digest: format!("sha256:{:x}", Sha256::digest(&receipt_bytes)), @@ -763,9 +780,9 @@ impl InferencePort for RoutedInferencePort { let model = request.provider_model().to_owned(); match self.route(&model) { Some(port) => port.prepare(request), - None => Err(InferenceFault::integrity_violation(format!( - "no configured inference backend claims the model `{model}`" - ))), + None => Err(InferenceFault::integrity_violation( + "no configured inference backend claims the requested model", + )), } } @@ -773,9 +790,9 @@ impl InferencePort for RoutedInferencePort { let model = request.invocation.request.model.clone(); match self.route(&model) { Some(port) => port.infer(request).await, - None => Err(InferenceFault::integrity_violation(format!( - "no configured inference backend claims the model `{model}`" - ))), + None => Err(InferenceFault::integrity_violation( + "no configured inference backend claims the requested model", + )), } } @@ -1482,7 +1499,7 @@ mod tests { fn soul_every_sidecar_reason_carries_its_named_disposition() { use SidecarFaultReason::*; for reason in [RateLimited, Overloaded, ServerError, ApiTimeout] { - let fault = reason.into_fault("detail".into()); + let fault = reason.into_fault(); assert!( !fault.requires_recovery() && !fault.integrity_was_violated(), "{reason:?} was not retryable" @@ -1500,13 +1517,13 @@ mod tests { Unknown, ] { assert!( - reason.into_fault("detail".into()).requires_recovery(), + reason.into_fault().requires_recovery(), "{reason:?} was not recovery-required" ); } for reason in [ProtocolViolation, ToolRegistrationFailed, TurnCapRefused] { assert!( - reason.into_fault("detail".into()).integrity_was_violated(), + reason.into_fault().integrity_was_violated(), "{reason:?} did not violate integrity" ); } @@ -1684,6 +1701,91 @@ mod tests { assert!(fault.integrity_was_violated(), "{fault:?}"); } + fn assert_names_no(fault: &InferenceFault, canary: &str) { + let rendered = format!("{fault} | {fault:?}"); + assert!(!rendered.contains(canary), "{canary} leaked into {rendered}"); + } + + /// The SDK port's share of the no-input-text rule: text the sidecar sent, + /// the link's own error text, a request's effort and a request's model + /// name each arrive as a canary, and none reaches the fault. The fault + /// type carries only fixed text, so this fails if a port ever routes a + /// computed string into it (through `Box::leak`, say). + #[tokio::test] + async fn an_sdk_fault_carries_no_text_it_was_handed() { + const CANARY: &str = "CANARY-sdk-91b6d0"; + + // The sidecar's own detail, for every reason it may name. + for reason in [ + SidecarFaultReason::RateLimited, + SidecarFaultReason::Overloaded, + SidecarFaultReason::ServerError, + SidecarFaultReason::ApiTimeout, + SidecarFaultReason::AuthenticationFailed, + SidecarFaultReason::OrgNotAllowed, + SidecarFaultReason::BillingError, + SidecarFaultReason::InvalidRequest, + SidecarFaultReason::ModelNotFound, + SidecarFaultReason::MaxOutputTokens, + SidecarFaultReason::MaxBudgetUsd, + SidecarFaultReason::ExecutionError, + SidecarFaultReason::Unknown, + SidecarFaultReason::ProtocolViolation, + SidecarFaultReason::ToolRegistrationFailed, + SidecarFaultReason::TurnCapRefused, + ] { + let link = ScriptedLink::new(vec![SidecarFrame::Fault { + query_id: 1, + reason, + detail: CANARY.into(), + }]); + let port = SdkInferencePort::new(link, TEST_RUNTIME); + let prepared = prose_prepared(&port, "Say something true."); + let fault = port.infer(prepared).await.expect_err("a fault frame produced an output"); + assert_names_no(&fault, CANARY); + } + + // The link's own error text. + assert_names_no(&link_fault(SidecarLinkError::Spawn(CANARY.into())), CANARY); + assert_names_no(&link_fault(SidecarLinkError::Codec(CANARY.into())), CANARY); + + // An effort the SDK has no counterpart for. + let port = SdkInferencePort::new(ScriptedLink::new(Vec::new()), TEST_RUNTIME); + let mut prepared = prose_prepared(&port, "Say something true."); + prepared.invocation.request.reasoning_effort = Some(CANARY.into()); + let fault = lower_query(1, &prepared, 1).expect_err("an unmapped effort lowered"); + assert_names_no(&fault, CANARY); + + // A model no backend claims, prepared and inferred through the router. + let directory = tempfile::tempdir().unwrap(); + let entry = directory.path().join("main.js"); + std::fs::write(&entry, "// sidecar").unwrap(); + let routed = open_inference( + None, + Some(SdkBinding { + sidecar_entry: entry, + caller_runtime_id: TEST_RUNTIME.into(), + model_prefix: DEFAULT_SDK_MODEL_PREFIX.into(), + }), + None, + &models(TEST_MODEL).each(), + ) + .expect("the SDK model routes"); + let request = tool_request( + CommandId::new(), + 0, + InferencePurpose::Persona, + CANARY, + "Respond only in natural prose.", + vec![CodexInputItem::UserText { text: "Say something true.".into() }], + Vec::::new(), + RequestShape { max_output_tokens: 1_200, parallel_tool_calls: false }, + ) + .expect("the request builds"); + let fault = routed.prepare(request).err().expect("an unroutable model prepared"); + assert_names_no(&fault, CANARY); + } + /// Soul: the lowering's own gate table, every row. #[test] fn soul_lower_query_refuses_the_shapes_the_gate_table_names() {