From 2f2f7a9db51239e41594307b4a52c783bfc0065b Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Thu, 29 Oct 2026 12:08:29 +0100 Subject: [PATCH] claude-connector: pre-spawn failures bill zero, model ids and prices are fuses, check runs serve's readiness, serve errors name fields Co-Authored-By: Claude Sonnet 5.5 --- crates/claude-connector/src/backend.rs | 60 +++-- crates/claude-connector/src/backend/tests.rs | 49 +++- crates/claude-connector/src/daemon.rs | 235 ++++++++++++++++-- crates/claude-connector/src/lib.rs | 2 +- crates/claude-connector/src/main.rs | 7 +- crates/claude-connector/src/stream.rs | 18 +- .../claude-connector/tests/config_document.rs | 231 ++++++++++++++++- crates/model-connector/src/serve.rs | 29 ++- .../model-connector/tests/reasoning_state.rs | 8 +- 9 files changed, 561 insertions(+), 78 deletions(-) diff --git a/crates/claude-connector/src/backend.rs b/crates/claude-connector/src/backend.rs index 2b27368..c5a8f81 100644 --- a/crates/claude-connector/src/backend.rs +++ b/crates/claude-connector/src/backend.rs @@ -110,12 +110,7 @@ impl ClaudeCodeBackend { /// The one pin check: `ready` and every spawn use it, and nothing caches /// its answer. fn binary_matches_pin(&self) -> bool { - let Ok(mut file) = File::open(&self.config.claude_binary) else { - return false; - }; - let mut hasher = Sha256::new(); - io::copy(&mut file, &mut hasher).is_ok() - && <[u8; 32]>::from(hasher.finalize()) == self.config.claude_binary_sha256 + binary_matches_pin(&self.config) } fn run(&self, request: &ProviderRequest) -> (Vec, TransportOutcome) { @@ -137,13 +132,13 @@ impl ClaudeCodeBackend { ); }; if !self.binary_matches_pin() { - return failure(Failure::unbilled( + return failure(Failure::never_reached( FailureKind::Provider, "claude-code: the claude binary does not match its pin", )); } let Ok(prompt) = PromptFile::create(&self.config.prompt_dir, &request.instructions) else { - return failure(Failure::unbilled( + return failure(Failure::never_reached( FailureKind::Provider, "claude-code: the system prompt could not be staged", )); @@ -159,7 +154,7 @@ impl ClaudeCodeBackend { stdin_bytes(plan.blocks()), ); let Ok(mut child) = spawned else { - return failure(Failure::unbilled( + return failure(Failure::never_reached( FailureKind::Provider, "claude-code: the child could not be spawned", )); @@ -224,12 +219,8 @@ impl ClaudeCodeBackend { } impl ProviderBackend for ClaudeCodeBackend { - fn ready(&self) -> bool { - self.config.work_dir.is_dir() - && self.config.config_dir.is_dir() - && self.config.prompt_dir.is_dir() - && self.tokens.read().is_some() - && self.binary_matches_pin() + fn ready(&self) -> Result<(), &'static str> { + readiness(&self.config, self.tokens.as_ref()) } fn provider(&self) -> &str { @@ -256,19 +247,19 @@ fn read_stream( } } StdoutRead::Eof => { - return Err(Failure::unbilled( + return Err(Failure::cost_unknown( FailureKind::Provider, "claude-code: the stream ended without a result", )); } StdoutRead::Timeout => { - return Err(Failure::unbilled( + return Err(Failure::cost_unknown( FailureKind::Timeout, "claude-code: the deadline passed before a result", )); } StdoutRead::Breach => { - return Err(Failure::unbilled( + return Err(Failure::cost_unknown( FailureKind::Provider, "claude-code: the output passed its size cap", )); @@ -280,6 +271,39 @@ fn read_stream( finished } +fn binary_matches_pin(config: &ClaudeCodeConfig) -> bool { + let Ok(mut file) = File::open(&config.claude_binary) else { + return false; + }; + let mut hasher = Sha256::new(); + io::copy(&mut file, &mut hasher).is_ok() + && <[u8; 32]>::from(hasher.finalize()) == config.claude_binary_sha256 +} + +/// Whether the backend can take work, naming the first config field that +/// cannot be used. `config check` and `serve` both ask this. +pub(crate) fn readiness( + config: &ClaudeCodeConfig, + tokens: &dyn TokenSource, +) -> Result<(), &'static str> { + if !config.work_dir.is_dir() { + return Err("work_dir"); + } + if !config.config_dir.is_dir() { + return Err("config_dir"); + } + if !config.prompt_dir.is_dir() { + return Err("prompt_dir"); + } + if tokens.read().is_none() { + return Err("token_credential"); + } + if !binary_matches_pin(config) { + return Err("claude_binary_sha256"); + } + Ok(()) +} + fn zero_usage() -> Usage { Usage { input_tokens: 0, diff --git a/crates/claude-connector/src/backend/tests.rs b/crates/claude-connector/src/backend/tests.rs index 5a8939d..86130b7 100644 --- a/crates/claude-connector/src/backend/tests.rs +++ b/crates/claude-connector/src/backend/tests.rs @@ -633,7 +633,7 @@ fn ready_checks_the_pinned_binary_digest() { std::fs::create_dir_all(&cfg).unwrap(); let binary = dir.path().join("claude"); - let ready = |sha: [u8; 32], binary: &Path, work: &Path, prompts: &Path, token: bool| { + let ready_field = |sha: [u8; 32], binary: &Path, work: &Path, prompts: &Path, token: bool| { let mut config = config(dir.path(), INSTANCE, limits()); config.claude_binary = binary.to_path_buf(); config.claude_binary_sha256 = sha; @@ -643,8 +643,27 @@ fn ready_checks_the_pinned_binary_digest() { let tokens = Box::new(FixedToken(token.then_some("t"))); ready_backend(config, tokens).ready() }; + let ready = |sha: [u8; 32], binary: &Path, work: &Path, prompts: &Path, token: bool| { + ready_field(sha, binary, work, prompts, token).is_ok() + }; let prompts = dir.path().join("prompts"); let pin = pin_of(PINNED); + assert_eq!( + ready_field([0; 32], &binary, &work, &prompts, true), + Err("claude_binary_sha256") + ); + assert_eq!( + ready_field(pin, &binary, &dir.path().join("absent"), &prompts, true), + Err("work_dir") + ); + assert_eq!( + ready_field(pin, &binary, &work, &dir.path().join("absent"), true), + Err("prompt_dir") + ); + assert_eq!( + ready_field(pin, &binary, &work, &prompts, false), + Err("token_credential") + ); assert!(ready(pin, &binary, &work, &prompts, true)); assert!(!ready([0; 32], &binary, &work, &prompts, true), "other pin"); @@ -702,7 +721,7 @@ fn token_comes_from_the_credential_file_not_the_environment() { let mut door = Door::new(); // The variable is set and the file is absent: nothing to give the child. - assert!(!backend.ready()); + assert_eq!(backend.ready(), Err("token_credential")); let (claim, _) = door.begin(request(vec![user("hi")])); let (kind, message, usage) = failure_of(&backend.execute(claim.call())); assert_eq!(kind, FailureKind::Authentication); @@ -717,7 +736,7 @@ fn token_comes_from_the_credential_file_not_the_environment() { format!("{CANARY_TOKEN}\n"), ) .unwrap(); - assert!(backend.ready()); + assert_eq!(backend.ready(), Ok(())); let mut door = Door::new(); let (claim, _) = door.begin(request(vec![user("hi")])); let result = backend.execute(claim.call()); @@ -730,7 +749,7 @@ fn token_comes_from_the_credential_file_not_the_environment() { // An empty credential is no token. std::fs::write(credentials.join("claude-token"), "\n").unwrap(); - assert!(!backend.ready()); + assert_eq!(backend.ready(), Err("token_credential")); } #[test] @@ -1369,9 +1388,11 @@ fn a_port_that_cannot_spawn_is_a_provider_failure() { ); let mut door = Door::new(); let (claim, _) = door.begin(request(vec![user("hi")])); - let (kind, message, usage) = failure_of(&backend.execute(claim.call())); + let result = backend.execute(claim.call()); + let (kind, message, usage) = failure_of(&result); assert_eq!(kind, FailureKind::Provider); - assert_eq!(usage, None); + assert_eq!(usage, Some(usage_zero()), "a child that never ran bills nothing"); + assert_eq!(parts(&result).1.cost_micro_usd, 0); assert!( !message.contains(dir.path().to_str().unwrap()), "no path in a message" @@ -1619,12 +1640,14 @@ fn prompt_file_lives_in_prompt_dir_exclusively() { Some(rig.dir.path().join("prompts").as_path()) ); - // A prompt directory that cannot take the file fails staging: the failure - // is the unbilled constructor's, and no child runs. + // A prompt directory that cannot take the file fails staging: nothing + // reached the provider, so it bills nothing, and no child runs. let rig = rig_for(INSTANCE, vec![ok_script(SESSION, "x")], &[SESSION], b""); std::fs::remove_dir_all(rig.dir.path().join("prompts")).unwrap(); - let (kind, _, usage) = failure_of(&rig.run(request(vec![user("hi")]))); - assert_eq!((kind, usage), (FailureKind::Provider, None)); + let result = rig.run(request(vec![user("hi")])); + let (kind, _, usage) = failure_of(&result); + assert_eq!((kind, usage), (FailureKind::Provider, Some(usage_zero()))); + assert_eq!(parts(&result).1.cost_micro_usd, 0); assert_eq!(rig.spawns(), 0); } @@ -1638,8 +1661,10 @@ fn pin_is_checked_before_every_spawn() { // The binary is swapped after readiness: the next call never spawns. std::fs::write(&binary, b"swapped bytes").unwrap(); - let (kind, message, usage) = failure_of(&rig.run(request(vec![user("hi")]))); - assert_eq!((kind, usage), (FailureKind::Provider, None)); + let result = rig.run(request(vec![user("hi")])); + let (kind, message, usage) = failure_of(&result); + assert_eq!((kind, usage), (FailureKind::Provider, Some(usage_zero()))); + assert_eq!(parts(&result).1.cost_micro_usd, 0, "billed nothing, not the reservation"); assert!(!message.contains(rig.dir.path().to_str().unwrap())); assert_eq!(rig.spawns(), 1, "no child for a binary off its pin"); assert!(!rig.backend.binary_matches_pin()); diff --git a/crates/claude-connector/src/daemon.rs b/crates/claude-connector/src/daemon.rs index 8debb4b..5a6e592 100644 --- a/crates/claude-connector/src/daemon.rs +++ b/crates/claude-connector/src/daemon.rs @@ -11,11 +11,15 @@ use std::time::Duration; use cultcache_rs::{CultCache, DatabaseEntry, SingleFileMessagePackBackingStore}; use model_connector::{ - CallerConfig, MIN_PAYLOAD_BYTES, ModelPrice, ServeError, ServeLimits, load_admissions, serve, + CallerAdmission, CallerConfig, MIN_PAYLOAD_BYTES, ModelPrice, ServeError, ServeLimits, + ServiceError, load_admissions, serve, }; use thiserror::Error; -use crate::{ClaudeCodeBackend, ClaudeCodeConfig, ClaudeCodeLimits, StderrSink}; +use crate::backend::readiness; +use crate::{ + ClaudeCodeBackend, ClaudeCodeConfig, ClaudeCodeLimits, CredentialTokenSource, StderrSink, +}; const CONFIG_KEY: &str = "runtime"; const MODEL_PREFIX: &str = "claude-"; @@ -81,8 +85,39 @@ pub enum ConfigError { PromptDir, #[error("the listener could not be bound")] Listen, - #[error(transparent)] - Serve(#[from] ServeError), + /// Something the document names exists but cannot be used now. + #[error("the configured {0} cannot be used")] + Unusable(&'static str), + /// The serve loop failed after it started. + #[error("the serve loop failed at {0}")] + Serving(&'static str), +} + +/// The kit's serve errors carry stored paths, ids and numbers; none of that +/// text is shown. Each maps to the configuration field or stage it names. +impl From for ConfigError { + fn from(error: ServeError) -> Self { + match error { + ServeError::InvalidConfig(field) | ServeError::BackendUnready(field) => { + Self::Unusable(field) + } + ServeError::FrameBound { .. } => Self::Invalid("max_frame_bytes"), + ServeError::ConnectionKey(_) => Self::Unusable("connection_key_file"), + ServeError::Service(error) => match error { + ServiceError::ReplayStore(_) => Self::Unusable("replay_store"), + ServiceError::UnpricedModel(_) => Self::Invalid("prices"), + ServiceError::InvalidAdmission + | ServiceError::DuplicateCaller + | ServiceError::SharedCallerKey => Self::Invalid("callers"), + _ => Self::Serving("service"), + }, + ServeError::ServicePoisoned => Self::Serving("service"), + ServeError::Listen(_) => Self::Serving("listener"), + ServeError::Thread(_) | ServeError::Connection(_) => Self::Serving("connection"), + ServeError::FrameSize | ServeError::FrameEncoding => Self::Serving("frame"), + ServeError::Clock => Self::Serving("clock"), + } + } } impl ClaudeConnectorConfig { @@ -162,7 +197,11 @@ impl ClaudeConnectorConfig { return Err(ConfigError::Invalid("allowed_models")); } } - if !self.prices.keys().all(|model| is_canonical_model(model)) { + if !self + .prices + .iter() + .all(|(model, price)| is_canonical_model(model) && is_fully_priced(price)) + { return Err(ConfigError::Invalid("prices")); } Ok(listen) @@ -200,14 +239,45 @@ fn is_trimmed_nonempty(text: &str) -> bool { !text.is_empty() && text.trim() == text } -/// A model id the CLI echoes back unchanged: aliases are refused because the -/// stream's `init.model` must equal the requested model. +/// A model id the CLI echoes back unchanged: `claude---` +/// with an optional `-YYYYMMDD` suffix, the family lowercase letters. Aliases +/// and bare prefixes are refused because the stream's `init.model` must equal +/// the requested model. fn is_canonical_model(model: &str) -> bool { - model.starts_with(MODEL_PREFIX) - && model.len() > MODEL_PREFIX.len() - && model - .bytes() - .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'.' | b'_')) + let Some(rest) = model.strip_prefix(MODEL_PREFIX) else { + return false; + }; + let digits = |part: &str, most: usize| { + (1..=most).contains(&part.len()) && part.bytes().all(|byte| byte.is_ascii_digit()) + }; + let mut parts = rest.split('-'); + let family = parts.next().unwrap_or(""); + let major = parts.next().unwrap_or(""); + let minor = parts.next().unwrap_or(""); + let date = parts.next(); + !family.is_empty() + && family.bytes().all(|byte| byte.is_ascii_lowercase()) + && digits(major, 2) + && digits(minor, 2) + && date.is_none_or(|date| date.len() == 8 && digits(date, 8)) + && parts.next().is_none() +} + +/// A price is a fuse only if every component it bills is nonzero: a zero rate +/// lets a call cost nothing against the caller's ceiling. +fn is_fully_priced(price: &ModelPrice) -> bool { + let long = price.long_context.map(|long| long.rates); + [Some(price.rates), long].into_iter().flatten().all(|rates| { + [ + rates.input, + rates.cache_write_short, + rates.cache_write_long, + rates.cache_read, + rates.output, + ] + .iter() + .all(|rate| *rate > 0) + }) } /// One file name under `$CREDENTIALS_DIRECTORY`, never a path. @@ -286,24 +356,49 @@ pub fn check_prompt_dir(dir: &Path) -> Result<(), ConfigError> { /// The child's stderr is the log's business by size only: its bytes are never /// written anywhere the connector controls. -struct LogSink; +struct LogSink(std::sync::Mutex); -impl StderrSink for LogSink { +impl StderrSink for LogSink { fn stderr_tail(&self, tail: &[u8]) { - eprintln!("claude-connector: child stderr, {} bytes", tail.len()); + if let Ok(mut log) = self.0.lock() { + let _ = writeln!(log, "claude-connector: child stderr, {} bytes", tail.len()); + } } } +/// Everything `serve` needs true before it binds, in one place so that +/// `config check` passes exactly what `serve` will start: the prompt +/// directory, the other directories, the credential, the pin, the connection +/// key and the frame and connection bounds. Returns the admitted callers. +fn readiness_of(config: &ClaudeConnectorConfig) -> Result, ConfigError> { + config.validate()?; + #[cfg(unix)] + check_prompt_dir(&config.prompt_dir)?; + let admissions = load_admissions(&config.callers)?; + config.serve_limits().validate(&admissions)?; + let backend = config.backend_config(); + readiness( + &backend, + &CredentialTokenSource::from_environment(backend.token_credential.clone()), + ) + .map_err(ConfigError::Unusable)?; + Ok(admissions) +} + +/// Whether `serve` would start with this document and this environment. +pub fn check_ready(config: &ClaudeConnectorConfig) -> Result<(), ConfigError> { + readiness_of(config).map(drop) +} + /// Loads the document and serves until the listener fails. #[cfg(unix)] pub fn serve_config(path: &Path) -> Result<(), ConfigError> { let config = load_config(path)?; + let admissions = readiness_of(&config)?; let listen = config.validate()?; - check_prompt_dir(&config.prompt_dir)?; - let admissions = load_admissions(&config.callers)?; let backend = Arc::new(ClaudeCodeBackend::new( config.backend_config(), - Box::new(LogSink), + Box::new(LogSink(std::sync::Mutex::new(std::io::stderr()))), )); let listener = TcpListener::bind(listen).map_err(|_| ConfigError::Listen)?; serve( @@ -316,3 +411,107 @@ pub fn serve_config(path: &Path) -> Result<(), ConfigError> { )?; Ok(()) } + +#[cfg(test)] +mod tests { + use std::sync::{Arc, Mutex}; + + use super::*; + + /// A log that a test can read back. + #[derive(Clone, Default)] + struct Captured(Arc>>); + + impl std::io::Write for Captured { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(bytes); + Ok(bytes.len()) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + #[test] + fn the_log_gets_the_size_of_the_childs_stderr_and_never_its_bytes() { + let log = Captured::default(); + let sink = LogSink(Mutex::new(log.clone())); + sink.stderr_tail(b"canary-stderr-text, 22 bytes"); + let written = String::from_utf8(log.0.lock().unwrap().clone()).unwrap(); + assert_eq!(written, "claude-connector: child stderr, 28 bytes +"); + assert!(!written.contains("canary")); + } + + #[cfg(unix)] + mod prompt_dir { + use std::os::unix::fs::{PermissionsExt, chown, symlink}; + use std::path::PathBuf; + + use super::super::check_prompt_dir; + + struct Dir(PathBuf); + + impl Dir { + fn new(name: &str) -> Self { + let path = std::env::temp_dir() + .join(format!("claude-prompt-dir-{}-{name}", std::process::id())); + let _ = std::fs::remove_dir_all(&path); + std::fs::create_dir_all(&path).unwrap(); + Self(path) + } + fn mode(&self, mode: u32) { + std::fs::set_permissions(&self.0, std::fs::Permissions::from_mode(mode)).unwrap(); + } + } + + impl Drop for Dir { + fn drop(&mut self) { + let _ = std::fs::set_permissions(&self.0, std::fs::Permissions::from_mode(0o700)); + let _ = std::fs::remove_dir_all(&self.0); + } + } + + #[test] + fn only_mode_0700_passes() { + let dir = Dir::new("modes"); + dir.mode(0o700); + assert!(check_prompt_dir(&dir.0).is_ok()); + // Each of these leaves the owner able to enter, or adds a bit + // beyond the owner's three; all are refused. + for mode in [ + 0o500, 0o300, 0o100, 0o600, 0o770, 0o707, 0o750, 0o755, 0o1700, 0o2700, 0o4700, + ] { + dir.mode(mode); + assert!(check_prompt_dir(&dir.0).is_err(), "{mode:o}"); + } + } + + #[test] + fn a_link_to_a_good_directory_is_refused() { + let dir = Dir::new("link-target"); + dir.mode(0o700); + let link = std::env::temp_dir() + .join(format!("claude-prompt-dir-{}-link", std::process::id())); + let _ = std::fs::remove_file(&link); + symlink(&dir.0, &link).unwrap(); + let refused = check_prompt_dir(&link).is_err(); + std::fs::remove_file(&link).unwrap(); + assert!(refused); + } + + #[test] + fn a_directory_owned_by_another_user_is_refused() { + // SAFETY: geteuid has no preconditions and cannot fail. + if unsafe { libc::geteuid() } != 0 { + eprintln!("skipped: only root can give a directory away"); + return; + } + let dir = Dir::new("owner"); + dir.mode(0o700); + assert!(check_prompt_dir(&dir.0).is_ok()); + chown(&dir.0, Some(1234), None).unwrap(); + assert!(check_prompt_dir(&dir.0).is_err()); + } + } +} diff --git a/crates/claude-connector/src/lib.rs b/crates/claude-connector/src/lib.rs index 2ca22bf..ef3e404 100644 --- a/crates/claude-connector/src/lib.rs +++ b/crates/claude-connector/src/lib.rs @@ -13,7 +13,7 @@ mod stream; pub use backend::{ClaudeCodeBackend, CredentialTokenSource, StderrSink, TokenSource}; pub use config::{ClaudeCodeConfig, ClaudeCodeLimits}; -pub use daemon::{ClaudeConnectorConfig, ConfigError, load_config, write_config}; +pub use daemon::{ClaudeConnectorConfig, ConfigError, check_ready, load_config, write_config}; #[cfg(unix)] pub use daemon::{check_prompt_dir, serve_config}; pub use handle::{FsSessionStore, SessionStore}; diff --git a/crates/claude-connector/src/main.rs b/crates/claude-connector/src/main.rs index f4b7004..ece39cc 100644 --- a/crates/claude-connector/src/main.rs +++ b/crates/claude-connector/src/main.rs @@ -12,7 +12,9 @@ use std::collections::BTreeMap; use std::path::PathBuf; use std::process::ExitCode; -use claude_connector::{ClaudeConnectorConfig, ConfigError, load_config, write_config}; +use claude_connector::{ + ClaudeConnectorConfig, ConfigError, check_ready, load_config, write_config, +}; use model_connector::{CallerConfig, ModelPrice, Rates, frame_bytes_for_payload}; fn main() -> ExitCode { @@ -59,7 +61,8 @@ fn serve(_: &str) -> Result<(), Failure> { fn check(path: &str) -> Result<(), Failure> { let config = load_config(path.as_ref())?; - println!("claude connector configuration is valid"); + check_ready(&config)?; + println!("claude connector configuration is valid and ready to serve"); println!("callers: {}", config.callers.len()); println!("allowed_models: {}", config.callers[0].allowed_models.len()); println!("prices: {}", config.prices.len()); diff --git a/crates/claude-connector/src/stream.rs b/crates/claude-connector/src/stream.rs index e323662..e99935f 100644 --- a/crates/claude-connector/src/stream.rs +++ b/crates/claude-connector/src/stream.rs @@ -8,7 +8,8 @@ use serde::Deserialize; use serde_json::Value; /// A call that cannot complete, with the stage that failed. `usage` is `None` -/// unless a result event reported it. +/// (the kit bills its reservation) unless a result event reported it or the +/// call never reached the provider (zero). #[derive(Debug, PartialEq, Eq)] pub(crate) struct Failure { pub kind: FailureKind, @@ -17,13 +18,24 @@ pub(crate) struct Failure { } impl Failure { - pub(crate) fn unbilled(kind: FailureKind, message: &'static str) -> Self { + /// A failure after the child ran: what the provider billed is unknown, so + /// the kit bills its reservation. + pub(crate) fn cost_unknown(kind: FailureKind, message: &'static str) -> Self { Self { kind, message, usage: None, } } + + /// A failure before anything was sent to the provider: it bills nothing. + pub(crate) fn never_reached(kind: FailureKind, message: &'static str) -> Self { + Self { + kind, + message, + usage: Some(sum_usage(std::iter::empty())), + } + } } /// What the stream said, once its result event arrived. @@ -252,7 +264,7 @@ impl<'a> StreamState<'a> { } fn fail(kind: FailureKind, message: &'static str) -> Step { - Step::Done(Box::new(Err(Failure::unbilled(kind, message)))) + Step::Done(Box::new(Err(Failure::cost_unknown(kind, message)))) } /// What this turn consumed, summed over every model it used (harness side diff --git a/crates/claude-connector/tests/config_document.rs b/crates/claude-connector/tests/config_document.rs index 2bc14f2..39f377d 100644 --- a/crates/claude-connector/tests/config_document.rs +++ b/crates/claude-connector/tests/config_document.rs @@ -15,7 +15,7 @@ use cultcache_rs::{ use model_connector::{CallerConfig, MIN_PAYLOAD_BYTES, ModelPrice, Rates}; const CANARY: &str = "canary-9f3b7c1d"; -const OPUS: &str = "claude-opus-5"; +const OPUS: &str = "claude-opus-5-1"; const HAIKU: &str = "claude-haiku-5-5"; /// An edit that should make the valid document refuse. @@ -262,6 +262,48 @@ fn config_refuses_aliases_unpriced_models_and_second_callers() { c.callers[0].allowed_models = vec!["claude-".to_string()]; }), ), + ( + "family only", + Box::new(|c| { + c.callers[0].allowed_models = vec!["claude-haiku".to_string()]; + c.prices.insert("claude-haiku".to_string(), price()); + }), + ), + ( + "latest alias", + Box::new(|c| { + c.callers[0].allowed_models = vec!["claude-sonnet-latest".to_string()]; + c.prices.insert("claude-sonnet-latest".to_string(), price()); + }), + ), + ( + "no minor", + Box::new(|c| { + c.callers[0].allowed_models = vec!["claude-opus-5".to_string()]; + c.prices.insert("claude-opus-5".to_string(), price()); + }), + ), + ( + "uppercase family", + Box::new(|c| { + c.callers[0].allowed_models = vec!["claude-Opus-5-1".to_string()]; + c.prices.insert("claude-Opus-5-1".to_string(), price()); + }), + ), + ( + "short date", + Box::new(|c| { + c.callers[0].allowed_models = vec!["claude-opus-5-1-2026".to_string()]; + c.prices.insert("claude-opus-5-1-2026".to_string(), price()); + }), + ), + ( + "trailing part", + Box::new(|c| { + c.callers[0].allowed_models = vec!["claude-opus-5-1-20260101-x".to_string()]; + c.prices.insert("claude-opus-5-1-20260101-x".to_string(), price()); + }), + ), ( "unpriced allowed", Box::new(|c| { @@ -295,6 +337,12 @@ fn config_refuses_aliases_unpriced_models_and_second_callers() { "allowed_models", "allowed_models", "allowed_models", + "allowed_models", + "allowed_models", + "allowed_models", + "allowed_models", + "allowed_models", + "allowed_models", "callers", "callers", ]; @@ -305,6 +353,44 @@ fn config_refuses_aliases_unpriced_models_and_second_callers() { } } +#[test] +fn a_dated_canonical_id_is_allowed() { + let dated = "claude-opus-5-1-20260101".to_string(); + let mut config = valid(); + config.callers[0].allowed_models = vec![dated.clone()]; + config.prices.insert(dated, price()); + config.validate().unwrap(); +} + +#[test] +fn a_price_with_a_zero_billed_component_is_refused() { + let zeroed: Vec<(&str, fn(&mut Rates))> = vec![ + ("input", |r| r.input = 0), + ("cache_write_short", |r| r.cache_write_short = 0), + ("cache_write_long", |r| r.cache_write_long = 0), + ("cache_read", |r| r.cache_read = 0), + ("output", |r| r.output = 0), + ]; + for (name, zero) in &zeroed { + let mut config = valid(); + zero(&mut config.prices.get_mut(HAIKU).unwrap().rates); + assert_eq!(refused(&config), "prices", "{name}"); + + let mut config = valid(); + let mut long = price().rates; + zero(&mut long); + config.prices.get_mut(HAIKU).unwrap().long_context = Some(model_connector::LongContext { + above_prompt_tokens: 200_000, + rates: long, + }); + assert_eq!(refused(&config), "prices", "long context {name}"); + } + // A zero overhead is not a rate. + let mut config = valid(); + config.prices.get_mut(HAIKU).unwrap().overhead_tokens = 0; + config.validate().unwrap(); +} + #[test] fn config_refuses_exposed_listeners_relative_paths_and_empty_bounds() { let cases: Vec<(&str, &str, Change)> = vec![ @@ -522,7 +608,7 @@ fn init(scratch: &Scratch, flags: &[String]) -> Output { } #[test] -fn config_init_then_check_reports_counts_only() { +fn config_init_writes_the_flags_into_the_document() { let scratch = Scratch::new(); let made = init(&scratch, &init_flags(&scratch, &[])); assert!(made.status.success(), "{}", shown(&made)); @@ -532,20 +618,155 @@ fn config_init_then_check_reports_counts_only() { assert_eq!(config.prices[HAIKU].rates.cache_write_long, 1_250_000); assert_eq!(config.prices[HAIKU].overhead_tokens, 100); assert_eq!(config.claude_binary_sha256, [0xab; 32]); +} +/// A scratch directory in which `serve` would start: every directory, the +/// pinned binary, the connection key and the credential exist. Returns the +/// `config init` flags and the credential directory. +#[cfg(unix)] +fn prepare(scratch: &Scratch) -> (Vec, PathBuf) { + use sha2::{Digest, Sha256}; + use std::os::unix::fs::PermissionsExt; + + for name in ["work", "home", "prompts", "credentials"] { + std::fs::create_dir_all(scratch.path(name)).unwrap(); + } + std::fs::set_permissions( + scratch.path("prompts"), + std::fs::Permissions::from_mode(0o700), + ) + .unwrap(); + std::fs::write(scratch.path("credentials/claude-token"), CANARY).unwrap(); + std::fs::write(scratch.path("caller.key"), "caller-secret").unwrap(); + std::fs::write(scratch.path("claude"), b"pinned claude").unwrap(); + let sha: String = Sha256::digest(b"pinned claude") + .iter() + .map(|byte| format!("{byte:02x}")) + .collect(); + let mut flags = init_flags(scratch, &[]); + let at = flags.iter().position(|flag| flag == "--claude-sha256").unwrap(); + flags[at + 1] = sha; + (flags, scratch.path("credentials")) +} + +#[cfg(unix)] +fn check_and_serve(config: &Path, credentials: Option<&Path>) -> (Output, Output) { + let envs: Vec<(&str, &Path)> = credentials + .map(|dir| ("CREDENTIALS_DIRECTORY", dir)) + .into_iter() + .collect(); + let path = config.to_str().unwrap(); + ( + run(&["config", "check", path], &envs), + run(&["serve", "--config", path], &envs), + ) +} + +#[cfg(unix)] +#[test] +fn check_passes_a_document_serve_can_start_and_reports_counts_only() { + let scratch = Scratch::new(); + let (flags, credentials) = prepare(&scratch); + let made = init(&scratch, &flags); + assert!(made.status.success(), "{}", shown(&made)); let checked = run( &[ "config", "check", scratch.path("connector.cc").to_str().unwrap(), ], - &[], + &[("CREDENTIALS_DIRECTORY", &credentials)], ); - assert!(checked.status.success()); + assert!(checked.status.success(), "{}", shown(&checked)); let text = shown(&checked); assert!(text.contains("callers: 1") && text.contains("allowed_models: 1")); assert!(text.contains("prices: 1")); - assert!(!text.contains("persona-a") && !text.contains(HAIKU)); + for secret in ["persona-a", HAIKU, CANARY, "caller-secret"] { + assert!(!text.contains(secret), "{text}"); + } +} + +#[cfg(unix)] +#[test] +fn check_refuses_exactly_what_serve_refuses_and_names_the_field() { + use std::os::unix::fs::PermissionsExt; + + type Break = fn(&Scratch, &Path); + let cases: Vec<(&str, &str, Break)> = vec![ + ("open prompt dir", "prompt directory", |s, _| { + std::fs::set_permissions(s.path("prompts"), std::fs::Permissions::from_mode(0o755)) + .unwrap(); + }), + ("no key file", "connection_key_file", |s, _| { + std::fs::remove_file(s.path("caller.key")).unwrap(); + }), + ("binary off its pin", "claude_binary_sha256", |s, _| { + std::fs::write(s.path("claude"), b"other bytes").unwrap(); + }), + ("no credential", "token_credential", |_, credentials| { + std::fs::remove_file(credentials.join("claude-token")).unwrap(); + }), + ("no work dir", "work_dir", |s, _| { + std::fs::remove_dir(s.path("work")).unwrap(); + }), + ("no config dir", "config_dir", |s, _| { + std::fs::remove_dir(s.path("home")).unwrap(); + }), + ("frame too small", "max_frame_bytes", |s, _| { + let path = s.path("connector.cc"); + let mut config = load_config(&path).unwrap(); + config.max_frame_bytes = 4099; + write_config(&path, &config).unwrap(); + }), + ("too few connections", "connection capacity", |s, _| { + let path = s.path("connector.cc"); + let mut config = load_config(&path).unwrap(); + config.max_connections = 1; + write_config(&path, &config).unwrap(); + }), + ]; + for (name, field, break_it) in cases { + let scratch = Scratch::new(); + let (flags, credentials) = prepare(&scratch); + let made = init(&scratch, &flags); + assert!(made.status.success(), "{name}: {}", shown(&made)); + break_it(&scratch, &credentials); + let (checked, served) = check_and_serve(&scratch.path("connector.cc"), Some(&credentials)); + assert!(!checked.status.success(), "{name} check passed"); + assert!(!served.status.success(), "{name} serve started"); + assert!(shown(&checked).contains(field), "{name}: {}", shown(&checked)); + assert_eq!(shown(&checked), shown(&served), "{name}"); + for secret in [CANARY, "caller-secret", "other bytes"] { + assert!(!shown(&checked).contains(secret), "{name}"); + } + } + + // No credential directory at all is the same refusal. + let scratch = Scratch::new(); + let (flags, _) = prepare(&scratch); + assert!(init(&scratch, &flags).status.success()); + let (checked, served) = check_and_serve(&scratch.path("connector.cc"), None); + assert!(!checked.status.success() && !served.status.success()); + assert!(shown(&checked).contains("token_credential")); + assert_eq!(shown(&checked), shown(&served)); +} + +#[cfg(unix)] +#[test] +fn serve_names_a_corrupt_replay_store_without_its_path() { + let scratch = Scratch::new(); + let (mut flags, credentials) = prepare(&scratch); + let store = scratch.path(&format!("{CANARY}-store/replay.cc")); + std::fs::create_dir_all(store.parent().unwrap()).unwrap(); + std::fs::write(&store, b"not a cultcache store").unwrap(); + let at = flags.iter().position(|flag| flag == "--replay-store").unwrap(); + flags[at + 1] = store.to_string_lossy().into_owned(); + assert!(init(&scratch, &flags).status.success()); + let (_, served) = check_and_serve(&scratch.path("connector.cc"), Some(&credentials)); + assert!(!served.status.success()); + let text = shown(&served); + assert!(text.contains("replay_store"), "{text}"); + assert!(!text.contains(CANARY) && !text.contains("replay.cc"), "{text}"); } #[test] diff --git a/crates/model-connector/src/serve.rs b/crates/model-connector/src/serve.rs index 6d87f6e..fe8f950 100644 --- a/crates/model-connector/src/serve.rs +++ b/crates/model-connector/src/serve.rs @@ -23,10 +23,11 @@ use crate::service::{ /// The only provider seam of the connector kit. A connector binary implements /// it over one model provider; the kit names no provider. pub trait ProviderBackend: Send + Sync { - /// Whether the provider can take work. `serve` consults it once, before it - /// opens the replay store or accepts a connection; the request path does - /// not consult it. - fn ready(&self) -> bool; + /// Whether the provider can take work; `Err` names the backend field that + /// is not ready, never a value. `serve` consults it once, before it opens + /// the replay store or accepts a connection; the request path does not + /// consult it. + fn ready(&self) -> Result<(), &'static str>; /// The tag of the provider this backend fronts. The service owns it from /// `serve` on: it names the provider on every receipt and decides which /// reasoning state is foreign. @@ -59,7 +60,7 @@ impl ServeLimits { /// largest whose sealed frame, at its worst-case encoding, fits the frame. /// An admitted payload therefore always fits, and `complete` is given the /// same frame bound to fit every result in. - fn validate(&self, admissions: &[CallerAdmission]) -> Result<(), ServeError> { + pub fn validate(&self, admissions: &[CallerAdmission]) -> Result<(), ServeError> { if self.max_frame_bytes == 0 || self.max_frame_bytes > u32::MAX as usize || self.max_connections == 0 @@ -171,9 +172,7 @@ pub fn serve( { return Err(ServeError::InvalidConfig("listener address")); } - if !backend.ready() { - return Err(ServeError::BackendUnready); - } + backend.ready().map_err(ServeError::BackendUnready)?; let service = Arc::new(Mutex::new(TransportService::open( replay_store, backend.provider(), @@ -322,8 +321,8 @@ pub enum ServeError { ConnectionKey(#[source] std::io::Error), #[error(transparent)] Service(#[from] ServiceError), - #[error("provider backend is not ready")] - BackendUnready, + #[error("provider backend is not ready: {0}")] + BackendUnready(&'static str), #[error("connector service lock was poisoned")] ServicePoisoned, #[error("connector listener failed")] @@ -379,9 +378,9 @@ mod tests { } impl ProviderBackend for CountingBackend { - fn ready(&self) -> bool { + fn ready(&self) -> Result<(), &'static str> { self.ready_calls.fetch_add(1, Ordering::SeqCst); - self.ready + self.ready.then_some(()).ok_or("fixture") } fn provider(&self) -> &str { @@ -710,7 +709,7 @@ mod tests { &store.path, backend.clone(), ); - assert!(matches!(result, Some(Err(ServeError::BackendUnready)))); + assert!(matches!(result, Some(Err(ServeError::BackendUnready("fixture"))))); assert_eq!(backend.executions.load(Ordering::SeqCst), 0); assert!( !store.path.exists(), @@ -1016,8 +1015,8 @@ mod tests { } impl ProviderBackend for SizedBackend { - fn ready(&self) -> bool { - true + fn ready(&self) -> Result<(), &'static str> { + Ok(()) } fn provider(&self) -> &str { diff --git a/crates/model-connector/tests/reasoning_state.rs b/crates/model-connector/tests/reasoning_state.rs index f42e0f1..8c9c4af 100644 --- a/crates/model-connector/tests/reasoning_state.rs +++ b/crates/model-connector/tests/reasoning_state.rs @@ -38,8 +38,8 @@ impl RecordingBackend { } impl ProviderBackend for RecordingBackend { - fn ready(&self) -> bool { - true + fn ready(&self) -> Result<(), &'static str> { + Ok(()) } fn provider(&self) -> &str { @@ -291,8 +291,8 @@ struct TamperingBackend { } impl ProviderBackend for TamperingBackend { - fn ready(&self) -> bool { - true + fn ready(&self) -> Result<(), &'static str> { + Ok(()) } fn provider(&self) -> &str {