From 8d3cfe7c1e50205ae7a7babd99fbb8665d3a6627 Mon Sep 17 00:00:00 2001 From: Wei Deng Date: Sun, 18 Oct 2026 16:11:10 -0700 Subject: [PATCH 1/9] feat: make unattended runs configurable Benchmark harnesses such as Harbor run S-Code with no one present, often against self-hosted models. Add settings for those runs. Every default keeps today's behavior. - Turn limits move into configuration: turn.max_model_calls, max_tool_calls, max_elapsed_seconds, model_stream_idle_seconds, max_cost_micros and max_total_tokens, with S_CODE_TURN_* overrides. The defaults are the previous built-in values. - turn.disabled_tools (S_CODE_TURN_DISABLED_TOOLS) removes tools from the model's tool list and rejects them if they are called anyway, including from Code Mode. - model.connect_timeout_seconds, read_timeout_seconds and request_timeout_seconds (S_CODE_MODEL_*_TIMEOUT_SECONDS) bound requests to endpoints set in the configuration file or environment, between 1 and 3600 seconds. The defaults stay 10, 45 and 180 seconds. - model.merge_system_messages (S_CODE_MODEL_MERGE_SYSTEM_MESSAGES) sends an OpenAI-compatible request's system messages, in order, as one leading message, for chat templates such as Qwen3.6's that reject later ones. It is off by default and, like the timeouts, applies to endpoints set in the configuration file or environment. - A new --raw-events-jsonl output mode emits every raw event as versioned JSON Lines. In that mode a question for the user ends the run instead of waiting for --timeout. Other output modes are unchanged. Refs #129 Signed-off-by: Wei Deng --- crates/cli/src/args.rs | 11 +- crates/cli/src/commands/automation.rs | 271 ++++++++++++---- crates/cli/src/main.rs | 19 ++ crates/config/src/lib.rs | 329 ++++++++++++++++++++ crates/daemon/src/code_mode.rs | 27 ++ crates/daemon/src/lib.rs | 130 +++++++- crates/daemon/src/main.rs | 60 +++- crates/model-gateway/src/lib.rs | 213 ++++++++++++- docs-site/content/guides/model-endpoints.md | 20 ++ docs/guides/model-endpoints.md | 20 ++ 10 files changed, 1000 insertions(+), 100 deletions(-) diff --git a/crates/cli/src/args.rs b/crates/cli/src/args.rs index 38f1f69..5e28993 100644 --- a/crates/cli/src/args.rs +++ b/crates/cli/src/args.rs @@ -25,6 +25,7 @@ pub(crate) enum OutputMode { Text, Jsonl, StreamJson, + RawEventsJsonl, } #[derive(Debug, Default, PartialEq, Eq)] @@ -133,7 +134,8 @@ Options: --network Request network access for `sandbox` --yes Confirm an explicit sandbox request in automation --jsonl Emit versioned JSON Lines - --stream-json Emit raw versioned event JSON Lines + --stream-json Emit legacy-compatible streaming JSON Lines + --raw-events-jsonl Emit every raw LiveEvent as versioned JSON Lines --output-last-message --output-schema Require a JSON response matching the schema --timeout Non-interactive timeout (default: 600) @@ -323,6 +325,13 @@ pub(crate) fn parse_args(values: impl IntoIterator) -> Result { + if parsed.output_mode != OutputMode::Text { + return Err(anyhow!("choose only one machine output mode")); + } + parsed.output_mode = OutputMode::RawEventsJsonl; + parsed.print = true; + } "--output-last-message" => { parsed.output_last_message = Some(PathBuf::from( args.next() diff --git a/crates/cli/src/commands/automation.rs b/crates/cli/src/commands/automation.rs index 13d0f32..7d8f54e 100644 --- a/crates/cli/src/commands/automation.rs +++ b/crates/cli/src/commands/automation.rs @@ -3,6 +3,7 @@ use crate::{ api::{Api, encode}, args::OutputMode, commands::interactive::{drain_sse, value_text}, + state::LiveEvent, }; use anyhow::{Result, anyhow}; use futures_util::StreamExt; @@ -37,9 +38,40 @@ fn emit_json(value: Value) -> Result<()> { Ok(()) } -fn is_tool_limit_question(kind: &str, payload: &Value) -> bool { - kind == "question.required" - && payload.get("source").and_then(Value::as_str) == Some("tool_call_limit") +fn stream_record(event: &LiveEvent) -> Value { + json!({ + "schema_version": "1", + "sequence": event.id, + "timestamp": event.timestamp, + "kind": event.kind, + "session_id": event.session_id, + "turn_id": event.turn_id, + "item_id": event.item_id, + "payload": event.payload, + }) +} + +fn turn_started_record(output_mode: OutputMode, session: &Id, turn: &Id) -> Option { + matches!(output_mode, OutputMode::Jsonl | OutputMode::StreamJson).then(|| { + json!({ + "schema_version": "1", + "type": "turn.started", + "session_id": session, + "turn_id": turn, + }) + }) +} + +fn lifecycle_record( + output_mode: OutputMode, + event: &LiveEvent, + legacy_record: Value, +) -> Option { + match output_mode { + OutputMode::Text => None, + OutputMode::Jsonl | OutputMode::StreamJson => Some(legacy_record), + OutputMode::RawEventsJsonl => Some(stream_record(event)), + } } fn tool_limit_resume_message(session: &Id) -> String { @@ -49,6 +81,24 @@ fn tool_limit_resume_message(session: &Id) -> String { ) } +fn question_required_message( + kind: &str, + payload: &Value, + session: &Id, + output_mode: OutputMode, +) -> Option { + if kind != "question.required" { + return None; + } + if payload.get("source").and_then(Value::as_str) == Some("tool_call_limit") { + return Some(tool_limit_resume_message(session)); + } + // Other output modes keep waiting for the turn as before; raw-events + // consumers run unattended and need the run to end instead. + (output_mode == OutputMode::RawEventsJsonl) + .then(|| "the turn requested interactive input; rerun without --print".into()) +} + pub(crate) async fn print_turn( api: &Api, session: &Id, @@ -69,13 +119,8 @@ pub(crate) async fn print_turn( let mut buffer = Vec::new(); let mut wrote = false; let mut answer = String::new(); - if output_mode != OutputMode::Text { - emit_json(json!({ - "schema_version": "1", - "type": "turn.started", - "session_id": session, - "turn_id": turn, - }))?; + if let Some(record) = turn_started_record(output_mode, session, turn) { + emit_json(record)?; } while let Some(chunk) = bytes.next().await { buffer.extend_from_slice(&chunk?); @@ -83,18 +128,24 @@ pub(crate) async fn print_turn( if event.session_id.as_ref() != Some(session) || event.turn_id.as_ref() != Some(turn) { continue; } - if is_tool_limit_question(&event.kind, &event.payload) { - if output_mode != OutputMode::Text { - emit_json(json!({ + if let Some(message) = + question_required_message(&event.kind, &event.payload, session, output_mode) + { + if let Some(record) = lifecycle_record( + output_mode, + &event, + json!({ "schema_version": "1", "type": "question.required", "session_id": session, "turn_id": turn, "item_id": event.item_id, "payload": event.payload, - }))?; + }), + ) { + emit_json(record)?; } - return Err(anyhow!(tool_limit_resume_message(session))); + return Err(anyhow!(message)); } match event.kind.as_str() { "model.delta" => { @@ -113,29 +164,26 @@ pub(crate) async fn print_turn( "item_id": event.item_id, "delta": text, }))?, - OutputMode::StreamJson => emit_json(json!({ - "schema_version": "1", - "sequence": event.id, - "timestamp": event.timestamp, - "kind": event.kind, - "session_id": event.session_id, - "turn_id": event.turn_id, - "item_id": event.item_id, - "payload": event.payload, - }))?, + OutputMode::StreamJson | OutputMode::RawEventsJsonl => { + emit_json(stream_record(&event))? + } } wrote |= !text.is_empty(); } "approval.required" => { - if output_mode != OutputMode::Text { - emit_json(json!({ + if let Some(record) = lifecycle_record( + output_mode, + &event, + json!({ "schema_version": "1", "type": "approval.required", "session_id": session, "turn_id": turn, "item_id": event.item_id, "payload": event.payload, - }))?; + }), + ) { + emit_json(record)?; } if let Some(id) = event.payload["approval_id"].as_str() { match api.approval(id, false, ApprovalScope::Once).await { @@ -175,28 +223,25 @@ pub(crate) async fn print_turn( "turn_id": turn, "message": answer, }))?, - OutputMode::StreamJson => emit_json(json!({ - "schema_version": "1", - "sequence": event.id, - "timestamp": event.timestamp, - "kind": event.kind, - "session_id": event.session_id, - "turn_id": event.turn_id, - "item_id": event.item_id, - "payload": event.payload, - }))?, + OutputMode::StreamJson | OutputMode::RawEventsJsonl => { + emit_json(stream_record(&event))? + } } return Ok(answer); } "turn.failed" => { - if output_mode != OutputMode::Text { - emit_json(json!({ + if let Some(record) = lifecycle_record( + output_mode, + &event, + json!({ "schema_version": "1", "type": "turn.failed", "session_id": session, "turn_id": turn, "error_code": event.payload["error_code"], - }))?; + }), + ) { + emit_json(record)?; } return Err(anyhow!( "turn failed: {}", @@ -206,26 +251,27 @@ pub(crate) async fn print_turn( )); } "turn.cancelled" => { - if output_mode != OutputMode::Text { - emit_json(json!({ + if let Some(record) = lifecycle_record( + output_mode, + &event, + json!({ "schema_version": "1", "type": "turn.cancelled", "session_id": session, "turn_id": turn, - }))?; + }), + ) { + emit_json(record)?; } return Err(anyhow!("turn was cancelled").context(CliExitStatus::Cancelled)); } - _ if output_mode == OutputMode::StreamJson => emit_json(json!({ - "schema_version": "1", - "sequence": event.id, - "timestamp": event.timestamp, - "kind": event.kind, - "session_id": event.session_id, - "turn_id": event.turn_id, - "item_id": event.item_id, - "payload": event.payload, - }))?, + _ if matches!( + output_mode, + OutputMode::StreamJson | OutputMode::RawEventsJsonl + ) => + { + emit_json(stream_record(&event))? + } _ => {} } } @@ -235,24 +281,113 @@ pub(crate) async fn print_turn( #[cfg(test)] mod tests { - use super::{is_tool_limit_question, tool_limit_resume_message}; + use super::{ + lifecycle_record, question_required_message, stream_record, tool_limit_resume_message, + turn_started_record, + }; + use crate::{args::OutputMode, state::LiveEvent}; + use chrono::Utc; use s_code_protocol::Id; use serde_json::json; #[test] - fn only_tool_limit_questions_interrupt_noninteractive_output() { - assert!(is_tool_limit_question( - "question.required", - &json!({"source":"tool_call_limit"}) - )); - assert!(!is_tool_limit_question( - "question.required", - &json!({"source":"mcp_elicitation"}) - )); - assert!(!is_tool_limit_question( - "turn.failed", - &json!({"source":"tool_call_limit"}) - )); + fn stream_records_preserve_terminal_limit_details() { + let event = LiveEvent { + id: 7, + timestamp: Utc::now(), + kind: "turn.failed".into(), + payload: json!({ + "error_code": "turn_elapsed_timeout", + "reason": "turn elapsed-time limit reached", + }), + session_id: Some(Id("session".into())), + turn_id: Some(Id("turn".into())), + item_id: Some(Id("item".into())), + }; + let record = stream_record(&event); + assert_eq!(record["kind"], "turn.failed"); + assert_eq!(record["payload"], event.payload); + assert!(record.get("type").is_none()); + } + + #[test] + fn raw_events_are_additive_to_the_legacy_stream_json_contract() { + let session = Id("session".into()); + let turn = Id("turn".into()); + let started = turn_started_record(OutputMode::StreamJson, &session, &turn).unwrap(); + assert_eq!( + started, + json!({ + "schema_version": "1", + "type": "turn.started", + "session_id": "session", + "turn_id": "turn", + }) + ); + assert!(turn_started_record(OutputMode::RawEventsJsonl, &session, &turn).is_none()); + + let event = LiveEvent { + id: 8, + timestamp: Utc::now(), + kind: "turn.failed".into(), + payload: json!({"error_code":"turn_elapsed_timeout", "reason":"elapsed"}), + session_id: Some(session), + turn_id: Some(turn), + item_id: None, + }; + let legacy = json!({ + "schema_version": "1", + "type": "turn.failed", + "session_id": "session", + "turn_id": "turn", + "error_code": "turn_elapsed_timeout", + }); + assert_eq!( + lifecycle_record(OutputMode::StreamJson, &event, legacy.clone()), + Some(legacy) + ); + let raw = lifecycle_record(OutputMode::RawEventsJsonl, &event, json!({})).unwrap(); + assert_eq!(raw["kind"], "turn.failed"); + assert_eq!(raw["payload"], event.payload); + assert!(raw.get("type").is_none()); + } + + #[test] + fn questions_interrupt_only_where_unattended_output_needs_it() { + let session = Id("ses_123".into()); + let tool_limit = json!({"source":"tool_call_limit"}); + let elicitation = json!({"source":"mcp_elicitation"}); + for mode in [ + OutputMode::Text, + OutputMode::Jsonl, + OutputMode::StreamJson, + OutputMode::RawEventsJsonl, + ] { + assert_eq!( + question_required_message("question.required", &tool_limit, &session, mode) + .as_deref(), + Some(tool_limit_resume_message(&session).as_str()) + ); + assert!( + question_required_message("turn.failed", &tool_limit, &session, mode).is_none() + ); + } + for mode in [OutputMode::Text, OutputMode::Jsonl, OutputMode::StreamJson] { + assert!( + question_required_message("question.required", &elicitation, &session, mode) + .is_none() + ); + } + assert_eq!( + question_required_message( + "question.required", + &elicitation, + &session, + OutputMode::RawEventsJsonl, + ) + .as_deref(), + Some("the turn requested interactive input; rerun without --print") + ); assert!( tool_limit_resume_message(&Id("ses_123".into())) .contains("s-code --resume=ses_123` without `--print") diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 7899678..f6934b0 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -2673,6 +2673,14 @@ mod tests { #[test] fn rejects_conflicting_or_unsafe_argument_modes() { assert!(parse_args(["--continue", "--resume"].into_iter().map(str::to_owned)).is_err()); + assert!( + parse_args( + ["--stream-json", "--raw-events-jsonl"] + .into_iter() + .map(str::to_owned) + ) + .is_err() + ); assert!( parse_args( ["--permission-mode", "unrestricted"] @@ -2706,6 +2714,17 @@ mod tests { assert!(exec.ephemeral); assert_eq!(exec.prompt.as_deref(), Some("fix tests")); + let raw_events = parse_args( + ["exec", "--raw-events-jsonl", "fix", "tests"] + .into_iter() + .map(str::to_owned), + ) + .unwrap() + .unwrap(); + assert_eq!(raw_events.output_mode, OutputMode::RawEventsJsonl); + assert!(raw_events.print); + assert_eq!(raw_events.prompt.as_deref(), Some("fix tests")); + let review = parse_args( ["review", "--base", "main", "focus", "on", "auth"] .into_iter() diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index ae7bc78..992dd85 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -510,6 +510,7 @@ pub struct RootConfig { pub daemon: DaemonConfig, pub runner: RunnerProcessConfig, pub client: ClientConfig, + pub turn: TurnConfig, pub model: ModelConfig, pub connectors: ConnectorConfig, pub mcp: McpConfig, @@ -524,6 +525,7 @@ impl Default for RootConfig { daemon: DaemonConfig::default(), runner: RunnerProcessConfig::default(), client: ClientConfig::default(), + turn: TurnConfig::default(), model: ModelConfig::default(), connectors: ConnectorConfig::default(), mcp: McpConfig::default(), @@ -531,6 +533,33 @@ impl Default for RootConfig { } } +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(default, deny_unknown_fields)] +pub struct TurnConfig { + pub max_model_calls: u32, + pub max_tool_calls: u32, + pub max_elapsed_seconds: u64, + pub model_stream_idle_seconds: u64, + pub max_cost_micros: u64, + pub max_total_tokens: u64, + /// Tools omitted from model requests and rejected if called anyway. + pub disabled_tools: Vec, +} + +impl Default for TurnConfig { + fn default() -> Self { + Self { + max_model_calls: 48, + max_tool_calls: 64, + max_elapsed_seconds: 1_800, + model_stream_idle_seconds: 60, + max_cost_micros: 5_000_000, + max_total_tokens: 1_000_000, + disabled_tools: Vec::new(), + } + } +} + #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(default, deny_unknown_fields)] pub struct DaemonConfig { @@ -666,6 +695,15 @@ pub struct ModelConfig { pub provider: String, pub base_url: Option, pub credential_handle: Option, + /// Seconds allowed to open a model connection. + pub connect_timeout_seconds: u64, + /// Longest wait, in seconds, for the next bytes of a model response. + pub read_timeout_seconds: u64, + /// Seconds allowed for one model request, including its streamed response. + pub request_timeout_seconds: u64, + /// Send OpenAI-compatible system messages as one leading message, for chat + /// templates that reject a system message after the first. + pub merge_system_messages: bool, pub endpoints: Vec, pub editing_mode: EditingMode, pub editing_overrides: BTreeMap, @@ -688,6 +726,10 @@ impl Default for ModelConfig { provider: "openai_compatible".into(), base_url: Some(DEFAULT_MODEL_API_BASE_URL.into()), credential_handle: None, + connect_timeout_seconds: 10, + read_timeout_seconds: 45, + request_timeout_seconds: 180, + merge_system_messages: false, endpoints: Vec::new(), editing_mode: EditingMode::Auto, editing_overrides: BTreeMap::new(), @@ -1002,6 +1044,7 @@ impl ConfigLoader { #[derive(Clone, Copy)] enum EnvKind { String, + StringList, Bool, U64, } @@ -1081,6 +1124,61 @@ const ENV_MAPPINGS: &[EnvMapping] = &[ path: "model.credential_handle", kind: EnvKind::String, }, + EnvMapping { + env: "S_CODE_MODEL_CONNECT_TIMEOUT_SECONDS", + path: "model.connect_timeout_seconds", + kind: EnvKind::U64, + }, + EnvMapping { + env: "S_CODE_MODEL_READ_TIMEOUT_SECONDS", + path: "model.read_timeout_seconds", + kind: EnvKind::U64, + }, + EnvMapping { + env: "S_CODE_MODEL_REQUEST_TIMEOUT_SECONDS", + path: "model.request_timeout_seconds", + kind: EnvKind::U64, + }, + EnvMapping { + env: "S_CODE_MODEL_MERGE_SYSTEM_MESSAGES", + path: "model.merge_system_messages", + kind: EnvKind::Bool, + }, + EnvMapping { + env: "S_CODE_TURN_MAX_MODEL_CALLS", + path: "turn.max_model_calls", + kind: EnvKind::U64, + }, + EnvMapping { + env: "S_CODE_TURN_MAX_TOOL_CALLS", + path: "turn.max_tool_calls", + kind: EnvKind::U64, + }, + EnvMapping { + env: "S_CODE_TURN_MAX_ELAPSED_SECONDS", + path: "turn.max_elapsed_seconds", + kind: EnvKind::U64, + }, + EnvMapping { + env: "S_CODE_TURN_MODEL_STREAM_IDLE_SECONDS", + path: "turn.model_stream_idle_seconds", + kind: EnvKind::U64, + }, + EnvMapping { + env: "S_CODE_TURN_MAX_COST_MICROS", + path: "turn.max_cost_micros", + kind: EnvKind::U64, + }, + EnvMapping { + env: "S_CODE_TURN_MAX_TOTAL_TOKENS", + path: "turn.max_total_tokens", + kind: EnvKind::U64, + }, + EnvMapping { + env: "S_CODE_TURN_DISABLED_TOOLS", + path: "turn.disabled_tools", + kind: EnvKind::StringList, + }, EnvMapping { env: "S_CODE_MCP_ENABLED", path: "mcp.enabled", @@ -1266,6 +1364,23 @@ const SENSITIVE_PATHS: &[&str] = &[ fn parse_env(value: &str, kind: EnvKind) -> Result { match kind { EnvKind::String => Ok(Value::String(value.into())), + EnvKind::StringList => { + if value.is_empty() { + return Ok(Value::Array(Vec::new())); + } + let values = value + .split(',') + .map(str::trim) + .map(|item| { + if item.is_empty() { + Err("expected a comma-separated list without empty items".into()) + } else { + Ok(Value::String(item.into())) + } + }) + .collect::, String>>()?; + Ok(Value::Array(values)) + } EnvKind::Bool => match value.to_ascii_lowercase().as_str() { "1" | "true" | "yes" | "on" => Ok(Value::Bool(true)), "0" | "false" | "no" | "off" => Ok(Value::Bool(false)), @@ -1284,6 +1399,60 @@ fn validate(config: &RootConfig, component: Component) -> Result<(), ConfigError } match component { Component::Daemon => { + if !(1..=10_000).contains(&config.turn.max_model_calls) { + return Err(ConfigError::Invalid( + "turn.max_model_calls must be between 1 and 10000".into(), + )); + } + if !(1..=10_000).contains(&config.turn.max_tool_calls) { + return Err(ConfigError::Invalid( + "turn.max_tool_calls must be between 1 and 10000".into(), + )); + } + if !(1..=86_400).contains(&config.turn.max_elapsed_seconds) { + return Err(ConfigError::Invalid( + "turn.max_elapsed_seconds must be between 1 and 86400".into(), + )); + } + if !(1..=3_600).contains(&config.turn.model_stream_idle_seconds) { + return Err(ConfigError::Invalid( + "turn.model_stream_idle_seconds must be between 1 and 3600".into(), + )); + } + if !(1..=1_000_000_000_000).contains(&config.turn.max_cost_micros) { + return Err(ConfigError::Invalid( + "turn.max_cost_micros must be between 1 and 1000000000000".into(), + )); + } + if !(1..=1_000_000_000).contains(&config.turn.max_total_tokens) { + return Err(ConfigError::Invalid( + "turn.max_total_tokens must be between 1 and 1000000000".into(), + )); + } + if config.turn.disabled_tools.len() > 128 { + return Err(ConfigError::Invalid( + "turn.disabled_tools must contain at most 128 names".into(), + )); + } + let mut disabled_tools = std::collections::BTreeSet::new(); + for tool in &config.turn.disabled_tools { + if tool.is_empty() + || tool.len() > 128 + || !tool.bytes().all(|byte| { + byte.is_ascii_alphanumeric() || matches!(byte, b'_' | b'-' | b'.') + }) + { + return Err(ConfigError::Invalid( + "turn.disabled_tools entries must be 1..128 safe tool-name characters" + .into(), + )); + } + if !disabled_tools.insert(tool) { + return Err(ConfigError::Invalid( + "turn.disabled_tools must not contain duplicate names".into(), + )); + } + } let listen = config.daemon.listen.parse::().map_err(|_| { ConfigError::Invalid("daemon.listen must be a socket address".into()) })?; @@ -1460,6 +1629,26 @@ fn validate(config: &RootConfig, component: Component) -> Result<(), ConfigError for endpoint in &config.model.endpoints { secure_model_base_url(&endpoint.base_url, "model.endpoints[].base_url")?; } + for (name, seconds) in [ + ( + "model.connect_timeout_seconds", + config.model.connect_timeout_seconds, + ), + ( + "model.read_timeout_seconds", + config.model.read_timeout_seconds, + ), + ( + "model.request_timeout_seconds", + config.model.request_timeout_seconds, + ), + ] { + if !(1..=3_600).contains(&seconds) { + return Err(ConfigError::Invalid(format!( + "{name} must be between 1 and 3600" + ))); + } + } let servicenow = [ config.connectors.servicenow_api_base.is_some(), config.connectors.servicenow_table.is_some(), @@ -2051,6 +2240,95 @@ fn url_contains_password(value: &str) -> bool { mod tests { use super::*; + #[test] + fn turn_limits_are_environment_configurable_and_bounded() { + let configured = ConfigLoader::new() + .with_environment([ + ("S_CODE_TURN_MAX_MODEL_CALLS", "100"), + ("S_CODE_TURN_MAX_TOOL_CALLS", "256"), + ("S_CODE_TURN_MAX_ELAPSED_SECONDS", "3600"), + ("S_CODE_TURN_MODEL_STREAM_IDLE_SECONDS", "300"), + ("S_CODE_TURN_MAX_COST_MICROS", "1000000000"), + ("S_CODE_TURN_MAX_TOTAL_TOKENS", "10000000"), + ( + "S_CODE_TURN_DISABLED_TOOLS", + "web_open,pdf_read,pdf_view,request_user_input", + ), + ]) + .load(Component::Daemon) + .unwrap() + .config + .turn; + assert_eq!(configured.max_model_calls, 100); + assert_eq!(configured.max_tool_calls, 256); + assert_eq!(configured.max_elapsed_seconds, 3_600); + assert_eq!(configured.model_stream_idle_seconds, 300); + assert_eq!(configured.max_cost_micros, 1_000_000_000); + assert_eq!(configured.max_total_tokens, 10_000_000); + assert_eq!( + configured.disabled_tools, + ["web_open", "pdf_read", "pdf_view", "request_user_input"].map(str::to_owned) + ); + + for (environment, value, expected) in [ + ("S_CODE_TURN_MAX_MODEL_CALLS", "0", "turn.max_model_calls"), + ("S_CODE_TURN_MAX_TOOL_CALLS", "10001", "turn.max_tool_calls"), + ( + "S_CODE_TURN_MAX_ELAPSED_SECONDS", + "86401", + "turn.max_elapsed_seconds", + ), + ( + "S_CODE_TURN_MODEL_STREAM_IDLE_SECONDS", + "3601", + "turn.model_stream_idle_seconds", + ), + ( + "S_CODE_TURN_MAX_COST_MICROS", + "1000000000001", + "turn.max_cost_micros", + ), + ( + "S_CODE_TURN_MAX_TOTAL_TOKENS", + "1000000001", + "turn.max_total_tokens", + ), + ] { + let error = ConfigLoader::new() + .with_environment([(environment, value)]) + .load(Component::Daemon) + .unwrap_err(); + assert!( + error.to_string().contains(expected), + "unexpected error: {error}" + ); + } + + for value in [ + "web_open,,pdf_read", + "web_open,web_open", + "request user input", + "mcp/server/tool", + ] { + let error = ConfigLoader::new() + .with_environment([("S_CODE_TURN_DISABLED_TOOLS", value)]) + .load(Component::Daemon) + .unwrap_err(); + assert!( + error.to_string().contains("turn.disabled_tools") + || error.to_string().contains("comma-separated list"), + "unexpected error for {value}: {error}" + ); + } + + let defaults = ConfigLoader::new() + .load(Component::Daemon) + .unwrap() + .config + .turn; + assert!(defaults.disabled_tools.is_empty()); + } + #[test] fn editing_modes_load_from_real_toml_and_reject_unknown_modes() { let directory = tempfile::tempdir().unwrap(); @@ -2525,6 +2803,57 @@ storage_encryption_key_id = "storage-key-1" ); } + #[test] + fn model_request_timeouts_are_configurable_and_bounded() { + let defaults = ConfigLoader::new().load(Component::Daemon).unwrap(); + assert_eq!( + ( + defaults.config.model.connect_timeout_seconds, + defaults.config.model.read_timeout_seconds, + defaults.config.model.request_timeout_seconds, + ), + (10, 45, 180) + ); + + let configured = ConfigLoader::new() + .with_environment([ + ("S_CODE_MODEL_CONNECT_TIMEOUT_SECONDS", "20"), + ("S_CODE_MODEL_READ_TIMEOUT_SECONDS", "600"), + ("S_CODE_MODEL_REQUEST_TIMEOUT_SECONDS", "3600"), + ]) + .load(Component::Daemon) + .unwrap(); + assert_eq!(configured.config.model.connect_timeout_seconds, 20); + assert_eq!(configured.config.model.read_timeout_seconds, 600); + assert_eq!(configured.config.model.request_timeout_seconds, 3_600); + + for (name, value) in [ + ("S_CODE_MODEL_CONNECT_TIMEOUT_SECONDS", "0"), + ("S_CODE_MODEL_READ_TIMEOUT_SECONDS", "3601"), + ("S_CODE_MODEL_REQUEST_TIMEOUT_SECONDS", "0"), + ] { + let error = ConfigLoader::new() + .with_environment([(name, value)]) + .load(Component::Daemon) + .unwrap_err(); + assert!( + error.to_string().contains("must be between 1 and 3600"), + "{name}={value}: {error}" + ); + } + } + + #[test] + fn system_message_merging_is_opt_in() { + let defaults = ConfigLoader::new().load(Component::Daemon).unwrap(); + assert!(!defaults.config.model.merge_system_messages); + let configured = ConfigLoader::new() + .with_environment([("S_CODE_MODEL_MERGE_SYSTEM_MESSAGES", "true")]) + .load(Component::Daemon) + .unwrap(); + assert!(configured.config.model.merge_system_messages); + } + #[test] fn model_provider_selection_is_explicit_and_atomic() { let effective = ConfigLoader::new() diff --git a/crates/daemon/src/code_mode.rs b/crates/daemon/src/code_mode.rs index de27099..de910ac 100644 --- a/crates/daemon/src/code_mode.rs +++ b/crates/daemon/src/code_mode.rs @@ -460,6 +460,10 @@ impl DaemonToolExecutor { "{tool} is not available in the {:?} tool profile", self.profile ); + ensure!( + self.state.tool_enabled(tool), + "{tool} is disabled by daemon configuration" + ); let admitted = self .state .store @@ -783,6 +787,29 @@ mod tests { ); } + #[tokio::test] + async fn code_mode_children_reject_disabled_tools() { + let (_dir, mut executor) = fixture().await; + executor.state = executor + .state + .with_disabled_tools(&["read_file".to_owned()]); + let parent = parent(&executor).await; + let error = executor + .code_mode_child( + &parent.request.id, + "read_file", + json!({"path":"a.txt"}), + &CancellationToken::new(), + ) + .await + .unwrap_err(); + assert!( + error + .to_string() + .contains("read_file is disabled by daemon configuration") + ); + } + #[cfg(any(target_os = "macos", target_os = "linux"))] #[tokio::test] async fn code_mode_full_session_keeps_workspace_and_read_only_boundaries() { diff --git a/crates/daemon/src/lib.rs b/crates/daemon/src/lib.rs index 6ea6f39..a3abf2a 100644 --- a/crates/daemon/src/lib.rs +++ b/crates/daemon/src/lib.rs @@ -179,6 +179,8 @@ pub struct AppState { editing_profiles: editing::EditingProfiles, code_mode_enabled: bool, code_mode_slots: Arc, + turn_limits: TurnLimits, + disabled_tools: Arc>, model_credentials_available: bool, storage_protection: Arc, runtime_scopes: RuntimeScopes, @@ -1307,6 +1309,8 @@ impl AppState { editing_profiles: editing::EditingProfiles::default(), code_mode_enabled: false, code_mode_slots: Arc::new(tokio::sync::Semaphore::new(4)), + turn_limits: TurnLimits::default(), + disabled_tools: Arc::new(BTreeSet::new()), model_credentials_available: false, storage_protection: "test_plaintext".into(), runtime_scopes: RuntimeScopes::default(), @@ -1335,6 +1339,28 @@ impl AppState { self } + pub fn with_turn_limits(mut self, config: &s_code_config::TurnConfig) -> Self { + self.turn_limits = TurnLimits { + max_model_calls: config.max_model_calls, + max_tool_calls: config.max_tool_calls, + max_elapsed_seconds: config.max_elapsed_seconds, + model_stream_idle_seconds: config.model_stream_idle_seconds, + max_cost_micros: config.max_cost_micros, + max_total_tokens: config.max_total_tokens, + }; + self + } + + pub fn with_disabled_tools(mut self, tools: &[String]) -> Self { + self.disabled_tools = Arc::new(tools.iter().cloned().collect()); + self.refresh_connector_executor(); + self + } + + fn tool_enabled(&self, tool: &str) -> bool { + !self.disabled_tools.contains(tool) + } + pub fn with_local_provider_setup( mut self, path: std::path::PathBuf, @@ -1705,6 +1731,7 @@ impl AppState { ci: self.ci_connector.clone(), security_events: self.security_event_connector.clone(), mcp: self.mcp_registry.clone(), + disabled_tools: self.disabled_tools.clone(), events: self.event_publisher(), })); } @@ -17737,11 +17764,15 @@ async fn execute_turn( purpose: "agent", }), executor, - TurnLimits::default(), + state.turn_limits.clone(), ) .with_observer(observer); let mut tools = if session.mode == s_code_protocol::SessionMode::Chat { - vec![start_work_definition()] + state + .tool_enabled("start_work") + .then(start_work_definition) + .into_iter() + .collect() } else { tools_for_profile(&state, profile) }; @@ -18651,6 +18682,7 @@ struct DeferredToolMatch { fn search_tool_catalog( registry: &McpRegistry, + disabled_tools: &BTreeSet, input: &ToolSearchArgs, ) -> Result, String> { let query = input.query.trim().to_ascii_lowercase(); @@ -18685,6 +18717,7 @@ fn search_tool_catalog( source: "built_in".into(), }), ) + .filter(|candidate| !disabled_tools.contains(&candidate.name)) .filter_map(|mut candidate| { let name = candidate.name.to_ascii_lowercase(); let description = candidate.description.to_ascii_lowercase(); @@ -18723,6 +18756,7 @@ struct ConnectorToolExecutor { ci: Option>, security_events: Option>, mcp: McpRegistry, + disabled_tools: Arc>, events: EventPublisher, } @@ -18992,7 +19026,7 @@ impl ExternalToolExecutor for ConnectorToolExecutor { if call.request.tool == "tool_search" { let input: ToolSearchArgs = serde_json::from_value(call.request.arguments.clone()) .map_err(|error| format!("invalid Tool Search request: {error}"))?; - let definitions = search_tool_catalog(&self.mcp, &input)?; + let definitions = search_tool_catalog(&self.mcp, &self.disabled_tools, &input)?; return Ok(Some(serde_json::json!({ "query": input.query, "matched": definitions.len(), @@ -20791,6 +20825,11 @@ impl AgentToolExecutor for DaemonToolExecutor { arguments: serde_json::Value, cancellation: &CancellationToken, ) -> PreparedAgentToolCall { + if !self.state.tool_enabled(tool) { + return PreparedAgentToolCall::resolved(AgentToolResult::Failed { + error: format!("{tool} is disabled by daemon configuration"), + }); + } // `start_work` is a Chat->Work transition governed by the session-mode // checks below, not by the read-only tool policy, so it is exempt here. if tool != "start_work" && !profile_allows_tool(self.profile, tool) { @@ -21371,6 +21410,7 @@ fn available_tools(state: &AppState) -> Vec { if state.code_mode_enabled { tools.push(code_mode::definition()); } + tools.retain(|tool| state.tool_enabled(&tool.name)); tools } @@ -22338,6 +22378,27 @@ impl IntoResponse for ApiError { mod tests { use super::*; + #[tokio::test] + async fn configured_turn_limits_reach_agent_runs() { + let state = AppState::new("secret", Store::in_memory().await.unwrap(), 0).with_turn_limits( + &s_code_config::TurnConfig { + max_model_calls: 100, + max_tool_calls: 256, + max_elapsed_seconds: 3_600, + model_stream_idle_seconds: 300, + max_cost_micros: 1_000_000_000, + max_total_tokens: 10_000_000, + disabled_tools: Vec::new(), + }, + ); + assert_eq!(state.turn_limits.max_model_calls, 100); + assert_eq!(state.turn_limits.max_tool_calls, 256); + assert_eq!(state.turn_limits.max_elapsed_seconds, 3_600); + assert_eq!(state.turn_limits.model_stream_idle_seconds, 300); + assert_eq!(state.turn_limits.max_cost_micros, 1_000_000_000); + assert_eq!(state.turn_limits.max_total_tokens, 10_000_000); + } + #[test] fn audit_csv_cells_neutralize_spreadsheet_formulas() { assert_eq!( @@ -34779,6 +34840,7 @@ mod tests { let matches = search_tool_catalog( &state.mcp_registry, + &state.disabled_tools, &ToolSearchArgs { query: "commit approved diff".into(), limit: 8, @@ -34794,6 +34856,68 @@ mod tests { ); } + #[tokio::test] + async fn configured_tools_are_neither_advertised_nor_dispatched() { + let disabled = + ["web_open", "pdf_read", "pdf_view", "request_user_input"].map(str::to_owned); + let store = Store::in_memory().await.unwrap(); + let state = AppState::new("secret", store.clone(), 0).with_disabled_tools(&disabled); + let names = tools_for_profile(&state, ToolProfile::Default) + .into_iter() + .map(|tool| tool.name) + .collect::>(); + for tool in &disabled { + assert!( + !names.contains(tool), + "disabled tool was advertised: {tool}" + ); + } + + let workspace = tempfile::tempdir().unwrap(); + let scope = Scope { + organization_id: Id("org".into()), + team_id: Id("team".into()), + actor_id: Id("user".into()), + goal_id: None, + task_id: None, + }; + let session = store + .create_session(CreateSession { + mode: s_code_protocol::SessionMode::Work, + scope: scope.clone(), + workspace_uri: url::Url::from_directory_path(workspace.path()) + .unwrap() + .to_string(), + title: "Disabled tools".into(), + model: "mock".into(), + }) + .await + .unwrap(); + let turn = store.create_turn(&scope, &session.id).await.unwrap(); + let executor = DaemonToolExecutor { + state, + session_id: session.id, + turn_id: turn.id, + scope, + profile: ToolProfile::Default, + }; + for tool in disabled { + let result = executor + .execute( + &format!("disabled-{tool}"), + &tool, + serde_json::json!({}), + &CancellationToken::new(), + ) + .await; + assert!(matches!( + result, + AgentToolResult::Failed { error } + if error == format!("{tool} is disabled by daemon configuration") + )); + } + } + #[tokio::test] async fn accept_edits_auto_approves_only_policy_allowed_file_changes() { let workspace = tempfile::tempdir().unwrap(); diff --git a/crates/daemon/src/main.rs b/crates/daemon/src/main.rs index 44f15de..68c13a1 100644 --- a/crates/daemon/src/main.rs +++ b/crates/daemon/src/main.rs @@ -23,7 +23,7 @@ use s_code_mcp_client::{ }; use s_code_model_gateway::{ AnthropicMessages, EnvironmentCredentials, GeminiGenerateContent, GovernedModelRouter, - ModelProvider, OpenAiCompatible, RoutedModelEndpoint, + HttpTimeouts, ModelProvider, OpenAiCompatible, RoutedModelEndpoint, }; use s_code_policy::PolicyTrustStore; use s_code_protocol::{DaemonSettings, Id, Scope}; @@ -633,28 +633,50 @@ fn configured_model_provider( base_url: String, credential_handle: Option, credentials: std::sync::Arc, + timeouts: HttpTimeouts, + merge_system_messages: bool, ) -> std::sync::Arc { match provider { "openai_compatible" => match credential_handle { - Some(handle) => { - std::sync::Arc::new(OpenAiCompatible::new(base_url, handle, credentials)) - } - None => std::sync::Arc::new(OpenAiCompatible::without_auth(base_url, credentials)), + Some(handle) => std::sync::Arc::new( + OpenAiCompatible::new(base_url, handle, credentials) + .with_timeouts(timeouts) + .with_merged_system_messages(merge_system_messages), + ), + None => std::sync::Arc::new( + OpenAiCompatible::without_auth(base_url, credentials) + .with_timeouts(timeouts) + .with_merged_system_messages(merge_system_messages), + ), }, - "anthropic" => std::sync::Arc::new(AnthropicMessages::new( - base_url, - credential_handle.expect("native provider credential handle"), - credentials, - )), - "gemini" => std::sync::Arc::new(GeminiGenerateContent::new( - base_url, - credential_handle.expect("native provider credential handle"), - credentials, - )), + "anthropic" => std::sync::Arc::new( + AnthropicMessages::new( + base_url, + credential_handle.expect("native provider credential handle"), + credentials, + ) + .with_timeouts(timeouts), + ), + "gemini" => std::sync::Arc::new( + GeminiGenerateContent::new( + base_url, + credential_handle.expect("native provider credential handle"), + credentials, + ) + .with_timeouts(timeouts), + ), _ => unreachable!("validated model provider"), } } +fn model_http_timeouts(config: &ModelConfig) -> HttpTimeouts { + HttpTimeouts { + connect: std::time::Duration::from_secs(config.connect_timeout_seconds), + read: std::time::Duration::from_secs(config.read_timeout_seconds), + total: std::time::Duration::from_secs(config.request_timeout_seconds), + } +} + fn default_model_credential_handle(provider: &str) -> String { match provider { "anthropic" => "ANTHROPIC_API_KEY".into(), @@ -1332,6 +1354,8 @@ async fn daemon_main() -> Result<(), Box> { .with_device_id(device_id) .with_revoked_team_grants(revoked_team_grants) .with_model_editing(&config.model) + .with_turn_limits(&config.turn) + .with_disabled_tools(&config.turn.disabled_tools) .with_model_credentials_available(model_credentials_available) .with_storage_protection(storage_protection); let mut connector_approval_verifier = None; @@ -1660,6 +1684,8 @@ async fn daemon_main() -> Result<(), Box> { ); } let credentials = std::sync::Arc::new(EnvironmentCredentials); + let model_timeouts = model_http_timeouts(&config.model); + let merge_system_messages = config.model.merge_system_messages; if !config.model.endpoints.is_empty() { let endpoints = config .model @@ -1679,6 +1705,8 @@ async fn daemon_main() -> Result<(), Box> { endpoint.base_url, credential_handle, credentials.clone(), + model_timeouts, + merge_system_messages, ), } }) @@ -1696,6 +1724,8 @@ async fn daemon_main() -> Result<(), Box> { base_url, credential_handle, credentials, + model_timeouts, + merge_system_messages, )); } if development_auth diff --git a/crates/model-gateway/src/lib.rs b/crates/model-gateway/src/lib.rs index 1357406..1703ecf 100644 --- a/crates/model-gateway/src/lib.rs +++ b/crates/model-gateway/src/lib.rs @@ -323,6 +323,7 @@ pub struct OpenAiCompatible { base_url: String, credential_handle: Option, credentials: Arc, + merge_system_messages: bool, } pub struct AnthropicMessages { @@ -339,14 +340,35 @@ pub struct GeminiGenerateContent { credentials: Arc, } -fn http_client() -> reqwest::Client { +/// Time limits for each model HTTP request. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub struct HttpTimeouts { + /// Time allowed to open the connection. + pub connect: Duration, + /// Longest wait for the next bytes of a response. + pub read: Duration, + /// Time allowed for the whole request, including a streamed response. + pub total: Duration, +} + +impl Default for HttpTimeouts { + fn default() -> Self { + Self { + connect: Duration::from_secs(10), + read: Duration::from_secs(45), + total: Duration::from_secs(180), + } + } +} + +fn http_client(timeouts: HttpTimeouts) -> reqwest::Client { reqwest::Client::builder() - .connect_timeout(Duration::from_secs(10)) - .read_timeout(Duration::from_secs(45)) - .timeout(Duration::from_secs(180)) + .connect_timeout(timeouts.connect) + .read_timeout(timeouts.read) + .timeout(timeouts.total) .redirect(reqwest::redirect::Policy::none()) .build() - .expect("static model gateway HTTP configuration must be valid") + .expect("model gateway HTTP configuration must be valid") } impl OpenAiCompatible { @@ -356,10 +378,11 @@ impl OpenAiCompatible { credentials: Arc, ) -> Self { Self { - client: http_client(), + client: http_client(HttpTimeouts::default()), base_url: base_url.into().trim_end_matches('/').into(), credential_handle: Some(credential_handle.into()), credentials, + merge_system_messages: false, } } @@ -368,19 +391,38 @@ impl OpenAiCompatible { credentials: Arc, ) -> Self { Self { - client: http_client(), + client: http_client(HttpTimeouts::default()), base_url: base_url.into().trim_end_matches('/').into(), credential_handle: None, credentials, + merge_system_messages: false, } } + + /// Send every system message, in order, as one leading system message. + /// Some chat templates, including Qwen3.6's, reject a system message + /// after the first one. + pub fn with_merged_system_messages(mut self, merge: bool) -> Self { + self.merge_system_messages = merge; + self + } + + /// Use these request time limits instead of the defaults. + pub fn with_timeouts(mut self, timeouts: HttpTimeouts) -> Self { + self.client = http_client(timeouts); + self + } } #[async_trait] impl ModelProvider for OpenAiCompatible { async fn stream(&self, request: ModelRequest) -> Result { let tools: Vec = request.tools.iter().map(|t| serde_json::json!({"type":"function","function":{"name":t.name,"description":t.description,"parameters":t.parameters}})).collect(); - let messages = openai_messages(&request.messages, &request.transient_tool_media)?; + let messages = openai_messages( + &request.messages, + &request.transient_tool_media, + self.merge_system_messages, + )?; let mut builder = self .client .post(format!("{}/chat/completions", self.base_url)); @@ -407,12 +449,18 @@ impl AnthropicMessages { credentials: Arc, ) -> Self { Self { - client: http_client(), + client: http_client(HttpTimeouts::default()), base_url: base_url.into().trim_end_matches('/').into(), credential_handle: credential_handle.into(), credentials, } } + + /// Use these request time limits instead of the defaults. + pub fn with_timeouts(mut self, timeouts: HttpTimeouts) -> Self { + self.client = http_client(timeouts); + self + } } #[async_trait] @@ -463,12 +511,18 @@ impl GeminiGenerateContent { credentials: Arc, ) -> Self { Self { - client: http_client(), + client: http_client(HttpTimeouts::default()), base_url: base_url.into().trim_end_matches('/').into(), credential_handle: credential_handle.into(), credentials, } } + + /// Use these request time limits instead of the defaults. + pub fn with_timeouts(mut self, timeouts: HttpTimeouts) -> Self { + self.client = http_client(timeouts); + self + } } #[async_trait] @@ -756,12 +810,21 @@ fn safe_call_label(call_id: &str) -> String { fn openai_messages( messages: &[ModelMessage], media: &BTreeMap>, + merge_system_messages: bool, ) -> Result, GatewayError> { validate_transient_tool_media(messages, media)?; + // When asked, send every system message, in order, as one leading system + // message, as the Anthropic and Gemini adapters always do. + let mut system = Vec::new(); let mut converted = Vec::new(); let mut index = 0usize; while index < messages.len() { let message = &messages[index]; + if merge_system_messages && message.role == "system" { + system.push(text_content(&message.content)?); + index += 1; + continue; + } if message.role == "tool" { let mut image_parts = Vec::new(); while index < messages.len() && messages[index].role == "tool" { @@ -838,6 +901,12 @@ fn openai_messages( }); index += 1; } + if !system.is_empty() { + converted.insert( + 0, + serde_json::json!({"role": "system", "content": system.join("\n\n")}), + ); + } Ok(converted) } @@ -1457,7 +1526,7 @@ mod tests { #[test] fn openai_media_contract_keeps_parallel_tool_results_contiguous() { let (messages, media) = media_fixture(); - let converted = openai_messages(&messages, &media).unwrap(); + let converted = openai_messages(&messages, &media, false).unwrap(); assert_eq!( converted, @@ -1602,7 +1671,7 @@ mod tests { )]), ] { assert!(matches!( - openai_messages(&messages, &media), + openai_messages(&messages, &media, false), Err(GatewayError::InvalidResponse(_)) )); } @@ -1621,7 +1690,8 @@ mod tests { }], }), }; - let output = openai_messages(std::slice::from_ref(&message), &BTreeMap::new()).unwrap(); + let output = + openai_messages(std::slice::from_ref(&message), &BTreeMap::new(), false).unwrap(); assert_eq!( output[0]["tool_calls"][0], serde_json::json!({ @@ -1638,6 +1708,64 @@ mod tests { } } + #[test] + fn openai_messages_merge_system_messages_only_when_asked() { + let messages = vec![ + ModelMessage { + role: "system".into(), + content: json!("work"), + }, + ModelMessage { + role: "system".into(), + content: json!({"instructions": "cite sources"}), + }, + ModelMessage { + role: "user".into(), + content: json!("fix the build"), + }, + ModelMessage { + role: "assistant".into(), + content: json!("checking"), + }, + ModelMessage { + role: "system".into(), + content: json!("background terminal finished"), + }, + ModelMessage { + role: "user".into(), + content: json!("continue"), + }, + ]; + + let preserved = openai_messages(&messages, &BTreeMap::new(), false).unwrap(); + assert_eq!( + preserved + .iter() + .map(|message| message["role"].as_str().unwrap()) + .collect::>(), + ["system", "system", "user", "assistant", "system", "user"] + ); + assert_eq!( + preserved[1]["content"], + json!("{\"instructions\":\"cite sources\"}") + ); + + let converted = openai_messages(&messages, &BTreeMap::new(), true).unwrap(); + + assert_eq!( + converted, + vec![ + json!({ + "role": "system", + "content": "work\n\n{\"instructions\":\"cite sources\"}\n\nbackground terminal finished", + }), + json!({"role": "user", "content": "fix the build"}), + json!({"role": "assistant", "content": "checking"}), + json!({"role": "user", "content": "continue"}), + ] + ); + } + #[test] fn parses_text_delta() { let events = parse_sse_line( @@ -2072,6 +2200,65 @@ mod tests { server.abort(); } + #[tokio::test] + async fn openai_stream_uses_configured_read_timeout() { + let app = Router::new().route( + "/chat/completions", + post(|| async { + let first = futures_util::stream::once(async { + Ok::<_, Infallible>(Bytes::from_static( + b"data: {\"choices\":[{\"delta\":{\"content\":\"ok\"}}]}\n\n", + )) + }); + Response::builder() + .status(StatusCode::OK) + .header("content-type", "text/event-stream") + .body(Body::from_stream( + first.chain(futures_util::stream::pending()), + )) + .unwrap() + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + let provider = + OpenAiCompatible::without_auth(format!("http://{address}"), Arc::new(FixedCredentials)) + .with_timeouts(HttpTimeouts { + read: Duration::from_millis(200), + ..HttpTimeouts::default() + }); + + let started = std::time::Instant::now(); + let events = provider + .stream(ModelRequest { + model: "default".into(), + temperature: 0.0, + messages: vec![ModelMessage { + role: "user".into(), + content: Value::String("hello".into()), + }], + tools: Vec::new(), + max_output_tokens: 32, + routing: None, + transient_tool_media: BTreeMap::new(), + }) + .await + .unwrap() + .collect::>() + .await; + + // The default 45-second read timeout would hold the stream open far longer. + assert!(started.elapsed() < Duration::from_secs(10)); + assert!( + events + .iter() + .any(|event| matches!(event, Ok(ModelEvent::TextDelta { text }) if text == "ok")) + ); + assert!(matches!(events.last(), Some(Err(_)))); + server.abort(); + } + #[tokio::test] async fn anthropic_http_contract_translates_tools_history_usage_and_stream() { type Captured = Arc>>; diff --git a/docs-site/content/guides/model-endpoints.md b/docs-site/content/guides/model-endpoints.md index 7a74f61..54f2f60 100644 --- a/docs-site/content/guides/model-endpoints.md +++ b/docs-site/content/guides/model-endpoints.md @@ -106,6 +106,26 @@ Configuration stores `MODEL_API_KEY` as a handle, not the value. If a running service predates a configuration change, restart it so the new endpoint and environment are loaded. +A model request fails if it cannot connect within 10 seconds, receives no new +response data for 45 seconds, or takes longer than 180 seconds in total. Slow +self-hosted models can need more time. Set `model.connect_timeout_seconds`, +`model.read_timeout_seconds` and `model.request_timeout_seconds`, or +`S_CODE_MODEL_CONNECT_TIMEOUT_SECONDS`, `S_CODE_MODEL_READ_TIMEOUT_SECONDS` and +`S_CODE_MODEL_REQUEST_TIMEOUT_SECONDS`, to between 1 and 3600 seconds. These +limits apply to endpoints in the configuration file or environment, not to +connections saved through guided setup. Each turn also has its own limits: it +fails after `turn.model_stream_idle_seconds` (default 60) without model output +and after `turn.max_elapsed_seconds` (default 1800) in total. To wait longer, +raise those too, or set `S_CODE_TURN_MODEL_STREAM_IDLE_SECONDS` and +`S_CODE_TURN_MAX_ELAPSED_SECONDS`. + +Some self-hosted chat templates, including Qwen3.6's, reject any system message +after the first one. For those endpoints, set `model.merge_system_messages = +true` or `S_CODE_MODEL_MERGE_SYSTEM_MESSAGES=1` to send every system message, in +order, as one leading message. S-Code always does this for Anthropic and Gemini. +Like the timeouts, this setting applies only to endpoints in the configuration +file or environment. + ## Development server The S-Code repository includes a development API Server that exposes an diff --git a/docs/guides/model-endpoints.md b/docs/guides/model-endpoints.md index 7a74f61..54f2f60 100644 --- a/docs/guides/model-endpoints.md +++ b/docs/guides/model-endpoints.md @@ -106,6 +106,26 @@ Configuration stores `MODEL_API_KEY` as a handle, not the value. If a running service predates a configuration change, restart it so the new endpoint and environment are loaded. +A model request fails if it cannot connect within 10 seconds, receives no new +response data for 45 seconds, or takes longer than 180 seconds in total. Slow +self-hosted models can need more time. Set `model.connect_timeout_seconds`, +`model.read_timeout_seconds` and `model.request_timeout_seconds`, or +`S_CODE_MODEL_CONNECT_TIMEOUT_SECONDS`, `S_CODE_MODEL_READ_TIMEOUT_SECONDS` and +`S_CODE_MODEL_REQUEST_TIMEOUT_SECONDS`, to between 1 and 3600 seconds. These +limits apply to endpoints in the configuration file or environment, not to +connections saved through guided setup. Each turn also has its own limits: it +fails after `turn.model_stream_idle_seconds` (default 60) without model output +and after `turn.max_elapsed_seconds` (default 1800) in total. To wait longer, +raise those too, or set `S_CODE_TURN_MODEL_STREAM_IDLE_SECONDS` and +`S_CODE_TURN_MAX_ELAPSED_SECONDS`. + +Some self-hosted chat templates, including Qwen3.6's, reject any system message +after the first one. For those endpoints, set `model.merge_system_messages = +true` or `S_CODE_MODEL_MERGE_SYSTEM_MESSAGES=1` to send every system message, in +order, as one leading message. S-Code always does this for Anthropic and Gemini. +Like the timeouts, this setting applies only to endpoints in the configuration +file or environment. + ## Development server The S-Code repository includes a development API Server that exposes an From d535cb537b84e12119e408e257f930d033560ddd Mon Sep 17 00:00:00 2001 From: Wei Deng Date: Sun, 18 Oct 2026 16:11:10 -0700 Subject: [PATCH 2/9] feat(harbor): build a pinned Linux runtime bundle Add integrations/harbor/build-bundle.sh. It refuses a checkout with any uncommitted or untracked file and builds the Linux x86-64 s-code, s-code-cli, s-code-daemon and s-code-pdf-worker binaries from HEAD. It then runs their self-tests and writes a gzip tar archive with a SHA-256 sidecar. The archive's manifest records the source commit and tree, the lockfile digests, the tool versions, and each file's digest, size and mode. integrations/harbor/runtime.py validates an archive against its SHA-256 digest and manifest without extracting or running anything. It uses only the Python standard library. Refs #129 Signed-off-by: Wei Deng --- integrations/__init__.py | 1 + integrations/harbor/__init__.py | 1 + integrations/harbor/build-bundle.sh | 347 +++++++++++++++++ integrations/harbor/runtime.py | 558 ++++++++++++++++++++++++++++ tests/test-harbor-bundle.py | 278 ++++++++++++++ 5 files changed, 1185 insertions(+) create mode 100644 integrations/__init__.py create mode 100644 integrations/harbor/__init__.py create mode 100755 integrations/harbor/build-bundle.sh create mode 100644 integrations/harbor/runtime.py create mode 100644 tests/test-harbor-bundle.py diff --git a/integrations/__init__.py b/integrations/__init__.py new file mode 100644 index 0000000..46cbf97 --- /dev/null +++ b/integrations/__init__.py @@ -0,0 +1 @@ +"""Optional integrations for running S-Code in external systems.""" diff --git a/integrations/harbor/__init__.py b/integrations/harbor/__init__.py new file mode 100644 index 0000000..e5428f0 --- /dev/null +++ b/integrations/harbor/__init__.py @@ -0,0 +1 @@ +"""Harbor adapter for S-Code.""" diff --git a/integrations/harbor/build-bundle.sh b/integrations/harbor/build-bundle.sh new file mode 100755 index 0000000..f36fc69 --- /dev/null +++ b/integrations/harbor/build-bundle.sh @@ -0,0 +1,347 @@ +#!/bin/sh +# Build the pinned, flat x86_64-glibc runtime bundle consumed by Harbor. +set -eu + +usage() { + cat <<'EOF' +Usage: integrations/harbor/build-bundle.sh + +Build a normalized S-Code runtime archive for x86_64 Linux with glibc. +The Git worktree must be clean and all pinned build tools must already exist. + +Environment: + S_CODE_HARBOR_OUTPUT_DIR Output directory (default: .work/harbor) + CARGO_TARGET_DIR Cargo build directory + (default: .work/harbor/cargo-target) + +Outputs: + s-code--x86_64-unknown-linux-gnu.tar.gz + s-code--x86_64-unknown-linux-gnu.tar.gz.sha256 +EOF +} + +if [ "$#" -eq 1 ]; then + case "$1" in + --help|-h) usage; exit 0 ;; + *) usage >&2; exit 2 ;; + esac +fi +[ "$#" -eq 0 ] || { usage >&2; exit 2; } + +fail() { + echo "build-bundle: $*" >&2 + exit 1 +} + +ROOT="$(CDPATH= cd -- "$(dirname -- "$0")/../.." && pwd -P)" +[ "$(git -C "$ROOT" rev-parse --show-toplevel 2>/dev/null)" = "$ROOT" ] || + fail "the script must run from its S-Code Git checkout" + +[ "$(uname -s)" = Linux ] || fail "bundles can only be built on Linux" +[ "$(uname -m)" = x86_64 ] || fail "bundles require the x86_64 architecture" +TARGET=x86_64-unknown-linux-gnu + +for required_command in \ + cargo cmp cut find getconf git install mkdir mktemp mv node npm python3 rustc sed uname +do + command -v "$required_command" >/dev/null 2>&1 || + fail "missing required command: $required_command" +done + +GLIBC_VERSION="$(getconf GNU_LIBC_VERSION 2>/dev/null || true)" +python3 - "$GLIBC_VERSION" <<'PY' || fail "a detectable glibc runtime is required" +import re +import sys + +raise SystemExit(re.fullmatch(r"glibc [0-9]+(?:\.[0-9]+)+", sys.argv[1]) is None) +PY + +PYTHON_VERSION="$(python3 -c 'import platform; print(platform.python_version())')" +python3 -c 'import sys; raise SystemExit(sys.version_info < (3, 9))' || + fail "Python 3.9 or newer is required" + +PINNED_RUST="$(sed -n 's/^channel = "\([^"]*\)"/\1/p' "$ROOT/rust-toolchain.toml")" +[ -n "$PINNED_RUST" ] || fail "rust-toolchain.toml does not pin a Rust version" +RUSTC_VERSION="$(rustc --version)" +case "$RUSTC_VERSION" in + "rustc $PINNED_RUST "*) ;; + *) fail "rustc $PINNED_RUST is required (found: $RUSTC_VERSION)" ;; +esac +CARGO_VERSION="$(cargo --version)" +RUST_HOST="$(rustc -vV | sed -n 's/^host: //p')" +[ "$RUST_HOST" = "$TARGET" ] || + fail "the Rust host must be $TARGET (found: ${RUST_HOST:-unknown})" + +PINNED_NODE="$(sed -n \ + 's/^[[:space:]]*SOURCE_NODE_VERSION=\([0-9][0-9.]*\)$/\1/p' \ + "$ROOT/scripts/source-dependencies.sh")" +[ -n "$PINNED_NODE" ] || + fail "scripts/source-dependencies.sh does not pin a Node.js version" +NODE_VERSION="$(node --version)" +[ "$NODE_VERSION" = "v$PINNED_NODE" ] || + fail "Node.js $PINNED_NODE is required (found: $NODE_VERSION)" +NPM_VERSION="$(npm --version)" +ZLIB_VERSION="$(python3 -c 'import zlib; print(zlib.ZLIB_VERSION)')" + +DIRTY="$(git -C "$ROOT" status --porcelain=v1 --untracked-files=all)" +[ -z "$DIRTY" ] || fail "Git worktree must be clean before building" +COMMIT="$(git -C "$ROOT" rev-parse --verify HEAD)" +TREE="$(git -C "$ROOT" rev-parse --verify 'HEAD^{tree}')" +SHORT_COMMIT="$(printf '%s' "$COMMIT" | cut -c 1-12)" + +OUTPUT_DIR="${S_CODE_HARBOR_OUTPUT_DIR:-$ROOT/.work/harbor}" +case "$OUTPUT_DIR" in + /*) ;; + *) OUTPUT_DIR="$ROOT/$OUTPUT_DIR" ;; +esac +case "$OUTPUT_DIR" in + /|"$ROOT") fail "output directory must be a dedicated directory" ;; +esac +if [ -n "${HOME:-}" ] && [ "$OUTPUT_DIR" = "$HOME" ]; then + fail "output directory must not be the home directory" +fi +[ ! -L "$OUTPUT_DIR" ] || fail "output directory must not be a symlink" +mkdir -p "$OUTPUT_DIR" +OUTPUT_DIR="$(CDPATH= cd -- "$OUTPUT_DIR" && pwd -P)" + +CARGO_BUILD_DIR="${CARGO_TARGET_DIR:-$OUTPUT_DIR/cargo-target}" +case "$CARGO_BUILD_DIR" in + /*) ;; + *) CARGO_BUILD_DIR="$ROOT/$CARGO_BUILD_DIR" ;; +esac +case "$CARGO_BUILD_DIR" in + /|"$ROOT") fail "Cargo target must be a dedicated build directory" ;; +esac +if [ -n "${HOME:-}" ] && [ "$CARGO_BUILD_DIR" = "$HOME" ]; then + fail "Cargo target must not be the home directory" +fi +export CARGO_TARGET_DIR="$CARGO_BUILD_DIR" + +STAGE_DIR="$(mktemp -d "$OUTPUT_DIR/.bundle-stage.XXXXXX")" +ARCHIVE_TMP="" +SIDECAR_TMP="" +cleanup() { + cleanup_status=$? + trap - EXIT HUP INT TERM + if [ -n "$STAGE_DIR" ] && [ -d "$STAGE_DIR" ]; then + find "$STAGE_DIR" -depth -delete 2>/dev/null || true + fi + for temporary in "$ARCHIVE_TMP" "$SIDECAR_TMP"; do + [ -n "$temporary" ] && [ -f "$temporary" ] && + find "$temporary" -type f -delete 2>/dev/null || true + done + exit "$cleanup_status" +} +trap cleanup EXIT HUP INT TERM + +export LC_ALL=C TZ=UTC SOURCE_DATE_EPOCH=0 +# Do not let caller-specific paths leak into Rust payloads. Unit separator is +# Cargo's documented encoded-rustflags delimiter and safely handles spaces. +RUSTFLAG_SEPARATOR="$(printf '\037')" +export CARGO_ENCODED_RUSTFLAGS="--remap-path-prefix=$ROOT=/usr/src/s-code${RUSTFLAG_SEPARATOR}\ +--remap-path-prefix=${HOME:-/nonexistent}=/home/s-code" +unset RUSTFLAGS + +echo "Building web bundle with Node.js $PINNED_NODE..." +npm ci --prefix "$ROOT/web" --no-audit --no-fund +npm run build --prefix "$ROOT/web" +git -C "$ROOT" diff --quiet -- web/app.js || + fail "web/app.js was stale; commit the regenerated bundle before building" + +echo "Building S-Code runtime with Rust $PINNED_RUST..." +cargo build --locked --release --manifest-path "$ROOT/Cargo.toml" \ + -p s-code-daemon -p s-code-cli + +for runtime_file in s-code-daemon s-code-cli s-code-pdf-worker; do + source_file="$CARGO_TARGET_DIR/release/$runtime_file" + [ -x "$source_file" ] || fail "build did not produce $runtime_file" + install -m 0755 "$source_file" "$STAGE_DIR/$runtime_file" +done +install -m 0755 "$ROOT/scripts/s-code" "$STAGE_DIR/s-code" + +"$STAGE_DIR/s-code-daemon" --self-test >/dev/null +"$STAGE_DIR/s-code-cli" --self-test >/dev/null +"$STAGE_DIR/s-code-pdf-worker" --self-test >/dev/null +"$STAGE_DIR/s-code" --help >/dev/null + +python3 - \ + "$STAGE_DIR" "$COMMIT" "$TREE" "$TARGET" \ + "$ROOT/Cargo.lock" "$ROOT/web/package-lock.json" \ + "$RUSTC_VERSION" "$CARGO_VERSION" "$NODE_VERSION" "$NPM_VERSION" \ + "$PYTHON_VERSION" "$ZLIB_VERSION" "$GLIBC_VERSION" <<'PY' +import hashlib +import json +import os +from pathlib import Path +import stat +import sys + +( + stage_arg, commit, tree, target, cargo_lock_arg, web_lock_arg, + rustc_version, cargo_version, node_version, npm_version, + python_version, zlib_version, glibc_version, +) = sys.argv[1:] +stage = Path(stage_arg) + +def digest(path): + value = hashlib.sha256() + with path.open("rb") as stream: + for block in iter(lambda: stream.read(1024 * 1024), b""): + value.update(block) + return value.hexdigest() + +runtime_names = ( + "s-code", + "s-code-cli", + "s-code-daemon", + "s-code-pdf-worker", +) +files = {} +for name in runtime_names: + path = stage / name + metadata = path.lstat() + if not stat.S_ISREG(metadata.st_mode) or stat.S_IMODE(metadata.st_mode) != 0o755: + raise SystemExit(f"invalid staged runtime file: {name}") + files[name] = { + "mode": "0755", + "sha256": digest(path), + "size": metadata.st_size, + } + +manifest = { + "build": { + "cargo": cargo_version, + "glibc": glibc_version, + "node": node_version, + "npm": npm_version, + "python": python_version, + "rustc": rustc_version, + "zlib": zlib_version, + }, + "files": files, + "format": "s-code-harbor-bundle", + "schema_version": 1, + "source": { + "commit": commit, + "dirty": False, + "locks": { + "Cargo.lock": digest(Path(cargo_lock_arg)), + "web/package-lock.json": digest(Path(web_lock_arg)), + }, + "tree": tree, + }, + "target": target, +} +(stage / "manifest.json").write_text( + json.dumps(manifest, ensure_ascii=True, separators=(",", ":"), sort_keys=True) + "\n", + encoding="utf-8", +) +os.chmod(stage / "manifest.json", 0o644) +PY + +ARCHIVE_NAME="s-code-$SHORT_COMMIT-$TARGET.tar.gz" +ARCHIVE_PATH="$OUTPUT_DIR/$ARCHIVE_NAME" +ARCHIVE_TMP="$OUTPUT_DIR/.$ARCHIVE_NAME.tmp.$$" +[ ! -e "$ARCHIVE_TMP" ] && [ ! -L "$ARCHIVE_TMP" ] || + fail "temporary archive path already exists" + +python3 - "$STAGE_DIR" "$ARCHIVE_TMP" <<'PY' +import gzip +from pathlib import Path +import tarfile +import sys + +stage = Path(sys.argv[1]) +output = Path(sys.argv[2]) +names = ( + "manifest.json", + "s-code", + "s-code-cli", + "s-code-daemon", + "s-code-pdf-worker", +) + +with output.open("xb") as raw_output: + with gzip.GzipFile( + filename="", mode="wb", compresslevel=9, fileobj=raw_output, mtime=0 + ) as compressed: + with tarfile.open(fileobj=compressed, mode="w|", format=tarfile.USTAR_FORMAT) as archive: + for name in names: + path = stage / name + metadata = path.stat() + member = tarfile.TarInfo(name) + member.size = metadata.st_size + member.mode = 0o644 if name == "manifest.json" else 0o755 + member.mtime = 0 + member.uid = 0 + member.gid = 0 + member.uname = "" + member.gname = "" + with path.open("rb") as source: + archive.addfile(member, source) +PY +chmod 0644 "$ARCHIVE_TMP" + +ARCHIVE_SHA256="$(python3 - "$ARCHIVE_TMP" <<'PY' +import hashlib +from pathlib import Path +import sys + +digest = hashlib.sha256() +with Path(sys.argv[1]).open("rb") as stream: + for block in iter(lambda: stream.read(1024 * 1024), b""): + digest.update(block) +print(digest.hexdigest()) +PY +)" + +PYTHONDONTWRITEBYTECODE=1 python3 "$ROOT/integrations/harbor/runtime.py" \ + "$ARCHIVE_TMP" "$ARCHIVE_SHA256" >/dev/null + +if [ -e "$ARCHIVE_PATH" ] || [ -L "$ARCHIVE_PATH" ]; then + [ ! -L "$ARCHIVE_PATH" ] || fail "refusing existing archive symlink: $ARCHIVE_PATH" + EXISTING_SHA256="$(python3 - "$ARCHIVE_PATH" <<'PY' +import hashlib +from pathlib import Path +import sys + +digest = hashlib.sha256() +with Path(sys.argv[1]).open("rb") as stream: + for block in iter(lambda: stream.read(1024 * 1024), b""): + digest.update(block) +print(digest.hexdigest()) +PY +)" + [ "$EXISTING_SHA256" = "$ARCHIVE_SHA256" ] || + fail "existing archive differs for the same source commit: $ARCHIVE_PATH" + find "$ARCHIVE_TMP" -type f -delete + ARCHIVE_TMP="" +else + mv "$ARCHIVE_TMP" "$ARCHIVE_PATH" + ARCHIVE_TMP="" +fi + +SIDECAR_PATH="$ARCHIVE_PATH.sha256" +SIDECAR_TMP="$OUTPUT_DIR/.$ARCHIVE_NAME.sha256.tmp.$$" +[ ! -e "$SIDECAR_TMP" ] && [ ! -L "$SIDECAR_TMP" ] || + fail "temporary sidecar path already exists" +printf '%s %s\n' "$ARCHIVE_SHA256" "$ARCHIVE_NAME" > "$SIDECAR_TMP" +chmod 0644 "$SIDECAR_TMP" +if [ -e "$SIDECAR_PATH" ] || [ -L "$SIDECAR_PATH" ]; then + [ ! -L "$SIDECAR_PATH" ] || fail "refusing existing sidecar symlink: $SIDECAR_PATH" + cmp -s "$SIDECAR_TMP" "$SIDECAR_PATH" || + fail "existing digest sidecar differs: $SIDECAR_PATH" + find "$SIDECAR_TMP" -type f -delete + SIDECAR_TMP="" +else + mv "$SIDECAR_TMP" "$SIDECAR_PATH" + SIDECAR_TMP="" +fi + +# The selected output directory need not be Git-ignored, but tracked sources +# must remain byte-for-byte identical to the tree recorded in the manifest. +git -C "$ROOT" diff --quiet && git -C "$ROOT" diff --cached --quiet || + fail "build modified the tracked Git worktree" + +echo "Bundle: $ARCHIVE_PATH" +echo "SHA-256: $ARCHIVE_SHA256" diff --git a/integrations/harbor/runtime.py b/integrations/harbor/runtime.py new file mode 100644 index 0000000..fcc16b9 --- /dev/null +++ b/integrations/harbor/runtime.py @@ -0,0 +1,558 @@ +"""Validation helpers for the pinned S-Code Harbor runtime bundle. + +The bundle is intentionally small and rigid. It is a gzip-compressed, flat +tar archive containing ``manifest.json`` and exactly the four runtime files in +``REQUIRED_RUNTIME_FILES``. Callers must supply the archive's SHA-256 digest; +the outer digest is never embedded in the archive itself. + +This module uses only the Python standard library and supports Python 3.9+. +Bundle validation does not extract or execute any archive member. +""" + +from __future__ import annotations + +import argparse +import gzip +import hashlib +import hmac +import json +import os +import platform +import re +import stat +import tarfile +import tempfile +from contextlib import contextmanager +from dataclasses import dataclass +from pathlib import Path, PurePosixPath +from types import MappingProxyType +from typing import BinaryIO, Dict, Iterator, Mapping, Sequence, Tuple, Union + + +BUNDLE_FORMAT = "s-code-harbor-bundle" +BUNDLE_SCHEMA_VERSION = 1 +SUPPORTED_TARGET = "x86_64-unknown-linux-gnu" +MANIFEST_NAME = "manifest.json" +REQUIRED_RUNTIME_FILES: Tuple[str, ...] = ( + "s-code", + "s-code-cli", + "s-code-daemon", + "s-code-pdf-worker", +) + +_EXPECTED_ARCHIVE_MEMBERS = (MANIFEST_NAME,) + REQUIRED_RUNTIME_FILES +_EXPECTED_MEMBER_MODES = MappingProxyType( + {MANIFEST_NAME: 0o644, **{name: 0o755 for name in REQUIRED_RUNTIME_FILES}} +) +_SHA256_RE = re.compile(r"[0-9a-f]{64}\Z") +_GIT_OBJECT_RE = re.compile(r"(?:[0-9a-f]{40}|[0-9a-f]{64})\Z") +_GLIBC_RE = re.compile(r"glibc [0-9]+(?:\.[0-9]+)+\Z") +_TOOL_KEYS = ("cargo", "node", "npm", "python", "rustc", "zlib") +_LOCK_KEYS = ("Cargo.lock", "web/package-lock.json") +_MAX_ARCHIVE_BYTES = 512 * 1024 * 1024 +_MAX_MANIFEST_BYTES = 64 * 1024 +_MAX_MEMBER_BYTES = 512 * 1024 * 1024 +_MAX_UNPACKED_BYTES = 1024 * 1024 * 1024 +_MAX_TAR_STREAM_BYTES = _MAX_UNPACKED_BYTES + 10 * 1024 * 1024 +_COPY_CHUNK_BYTES = 1024 * 1024 +_NORMALIZED_GZIP_HEADER = b"\x1f\x8b\x08\x00\x00\x00\x00\x00\x02\xff" + + +class BundleValidationError(ValueError): + """Raised when a Harbor runtime bundle fails closed validation.""" + + +@dataclass(frozen=True) +class BundleFile: + """Validated metadata for one runtime payload file.""" + + sha256: str + size: int + mode: int + + +@dataclass(frozen=True) +class BundleManifest: + """Validated bundle metadata plus its caller-supplied outer digest.""" + + schema_version: int + bundle_format: str + source_commit: str + source_tree: str + source_dirty: bool + target: str + glibc_version: str + lock_sha256s: Mapping[str, str] + tool_versions: Mapping[str, str] + files: Mapping[str, BundleFile] + archive_sha256: str + + +PathLike = Union[str, os.PathLike] + + +def validated_sha256(value: str, field: str = "sha256") -> str: + """Return a canonical lowercase SHA-256 string or raise.""" + + if not isinstance(value, str) or _SHA256_RE.fullmatch(value) is None: + raise BundleValidationError( + f"{field} must be exactly 64 lowercase hexadecimal characters" + ) + return value + + +def validate_sha256(value: str, field: str = "sha256") -> str: + """Compatibility spelling for :func:`validated_sha256`.""" + + return validated_sha256(value, field) + + +def _open_regular_file(path: PathLike) -> BinaryIO: + try: + flags = os.O_RDONLY + if hasattr(os, "O_CLOEXEC"): + flags |= os.O_CLOEXEC + if hasattr(os, "O_NONBLOCK"): + flags |= os.O_NONBLOCK + if hasattr(os, "O_NOFOLLOW"): + flags |= os.O_NOFOLLOW + descriptor = os.open(os.fspath(path), flags) + except (OSError, TypeError, ValueError) as error: + raise BundleValidationError(f"cannot open bundle as a regular file: {error}") from error + + try: + metadata = os.fstat(descriptor) + if not stat.S_ISREG(metadata.st_mode): + raise BundleValidationError("bundle path must name a regular file") + return os.fdopen(descriptor, "rb") + except Exception: + os.close(descriptor) + raise + + +def sha256_file(path: PathLike) -> str: + """Hash a regular, non-symlink file without loading it into memory.""" + + digest = hashlib.sha256() + with _open_regular_file(path) as source: + for block in iter(lambda: source.read(_COPY_CHUNK_BYTES), b""): + digest.update(block) + return digest.hexdigest() + + +@contextmanager +def _verified_snapshot( + path: PathLike, expected_sha256: str +) -> Iterator[Tuple[BinaryIO, str]]: + """Copy and authenticate one stable archive snapshot. + + Validation reads the snapshot rather than reopening the caller's path, so + a concurrent path replacement cannot change bytes between hashing and tar + inspection. + """ + + expected = validated_sha256(expected_sha256, "expected_sha256") + digest = hashlib.sha256() + total = 0 + snapshot = tempfile.SpooledTemporaryFile(max_size=32 * 1024 * 1024, mode="w+b") + try: + with _open_regular_file(path) as source: + while True: + block = source.read(_COPY_CHUNK_BYTES) + if not block: + break + total += len(block) + if total > _MAX_ARCHIVE_BYTES: + raise BundleValidationError( + f"bundle exceeds {_MAX_ARCHIVE_BYTES} compressed bytes" + ) + digest.update(block) + snapshot.write(block) + actual = digest.hexdigest() + if not hmac.compare_digest(actual, expected): + raise BundleValidationError( + f"bundle SHA-256 mismatch: expected {expected}, got {actual}" + ) + snapshot.seek(0) + yield snapshot, actual + finally: + snapshot.close() + + +def _require_exact_keys(value: object, expected: Sequence[str], field: str) -> Dict[str, object]: + if not isinstance(value, dict): + raise BundleValidationError(f"{field} must be a JSON object") + actual = set(value) + wanted = set(expected) + if actual != wanted: + missing = sorted(wanted - actual) + unexpected = sorted(actual - wanted) + details = [] + if missing: + details.append(f"missing {missing}") + if unexpected: + details.append(f"unexpected {unexpected}") + raise BundleValidationError(f"{field} has invalid keys ({'; '.join(details)})") + return value + + +def _reject_duplicate_keys(pairs: Sequence[Tuple[str, object]]) -> Dict[str, object]: + result: Dict[str, object] = {} + for key, value in pairs: + if key in result: + raise BundleValidationError(f"manifest contains duplicate JSON key {key!r}") + result[key] = value + return result + + +def _reject_json_constant(value: str) -> object: + raise BundleValidationError(f"manifest contains invalid JSON constant {value!r}") + + +def _manifest_text(value: object, field: str) -> str: + if not isinstance(value, str) or not value or len(value) > 512: + raise BundleValidationError(f"{field} must be a non-empty string of at most 512 characters") + if any(ord(character) < 0x20 or ord(character) > 0x7E for character in value): + raise BundleValidationError(f"{field} must contain printable ASCII without newlines") + return value + + +def _parse_manifest( + manifest_bytes: bytes, archive_sha256: str, expected_target: str +) -> BundleManifest: + try: + manifest_text = manifest_bytes.decode("utf-8") + except UnicodeDecodeError as error: + raise BundleValidationError("manifest.json must be UTF-8") from error + try: + value = json.loads( + manifest_text, + object_pairs_hook=_reject_duplicate_keys, + parse_constant=_reject_json_constant, + ) + except BundleValidationError: + raise + except (json.JSONDecodeError, TypeError, ValueError) as error: + raise BundleValidationError(f"manifest.json is not strict JSON: {error}") from error + + root = _require_exact_keys( + value, + ("schema_version", "format", "target", "source", "build", "files"), + "manifest", + ) + if type(root["schema_version"]) is not int or root["schema_version"] != BUNDLE_SCHEMA_VERSION: + raise BundleValidationError( + f"manifest.schema_version must be {BUNDLE_SCHEMA_VERSION}" + ) + if root["format"] != BUNDLE_FORMAT: + raise BundleValidationError(f"manifest.format must be {BUNDLE_FORMAT!r}") + if root["target"] != expected_target: + raise BundleValidationError( + f"manifest.target must be {expected_target!r}, got {root['target']!r}" + ) + + source = _require_exact_keys( + root["source"], ("commit", "tree", "dirty", "locks"), "manifest.source" + ) + commit = source["commit"] + tree = source["tree"] + if not isinstance(commit, str) or _GIT_OBJECT_RE.fullmatch(commit) is None: + raise BundleValidationError("manifest.source.commit must be a lowercase Git object ID") + if not isinstance(tree, str) or _GIT_OBJECT_RE.fullmatch(tree) is None: + raise BundleValidationError("manifest.source.tree must be a lowercase Git object ID") + if len(commit) != len(tree): + raise BundleValidationError("manifest source commit and tree use different hash formats") + if source["dirty"] is not False: + raise BundleValidationError("manifest.source.dirty must be false") + locks = _require_exact_keys(source["locks"], _LOCK_KEYS, "manifest.source.locks") + lock_sha256s = { + name: validated_sha256(locks[name], f"manifest.source.locks.{name}") + for name in _LOCK_KEYS + } + + build = _require_exact_keys( + root["build"], _TOOL_KEYS + ("glibc",), "manifest.build" + ) + tool_versions = { + name: _manifest_text(build[name], f"manifest.build.{name}") for name in _TOOL_KEYS + } + glibc_version = _manifest_text(build["glibc"], "manifest.build.glibc") + if _GLIBC_RE.fullmatch(glibc_version) is None: + raise BundleValidationError( + "manifest.build.glibc must have the canonical form 'glibc '" + ) + + raw_files = _require_exact_keys(root["files"], REQUIRED_RUNTIME_FILES, "manifest.files") + files: Dict[str, BundleFile] = {} + for name in REQUIRED_RUNTIME_FILES: + raw_metadata = _require_exact_keys( + raw_files[name], ("sha256", "size", "mode"), f"manifest.files.{name}" + ) + digest = validated_sha256(raw_metadata["sha256"], f"manifest.files.{name}.sha256") + size = raw_metadata["size"] + if type(size) is not int or size <= 0 or size > _MAX_MEMBER_BYTES: + raise BundleValidationError( + f"manifest.files.{name}.size must be an integer from 1 to {_MAX_MEMBER_BYTES}" + ) + if raw_metadata["mode"] != "0755": + raise BundleValidationError(f"manifest.files.{name}.mode must be '0755'") + files[name] = BundleFile(sha256=digest, size=size, mode=0o755) + + return BundleManifest( + schema_version=BUNDLE_SCHEMA_VERSION, + bundle_format=BUNDLE_FORMAT, + source_commit=commit, + source_tree=tree, + source_dirty=False, + target=expected_target, + glibc_version=glibc_version, + lock_sha256s=MappingProxyType(lock_sha256s), + tool_versions=MappingProxyType(tool_versions), + files=MappingProxyType(files), + archive_sha256=archive_sha256, + ) + + +def _validate_member_header(member: tarfile.TarInfo) -> None: + name = member.name + path = PurePosixPath(name) + if ( + not name + or "\\" in name + or path.is_absolute() + or len(path.parts) != 1 + or path.name in ("", ".", "..") + ): + raise BundleValidationError(f"unsafe archive member path {name!r}") + if name not in _EXPECTED_ARCHIVE_MEMBERS: + raise BundleValidationError(f"unexpected archive member {name!r}") + if not member.isfile() or member.type not in (tarfile.REGTYPE, tarfile.AREGTYPE): + raise BundleValidationError(f"archive member {name!r} must be a regular file") + if member.linkname: + raise BundleValidationError(f"archive member {name!r} must not have a link target") + if member.pax_headers: + raise BundleValidationError(f"archive member {name!r} must not use PAX headers") + if getattr(member, "sparse", None): + raise BundleValidationError(f"archive member {name!r} must not be sparse") + if member.uid != 0 or member.gid != 0 or member.uname or member.gname: + raise BundleValidationError(f"archive member {name!r} has non-normalized ownership") + if member.mtime != 0: + raise BundleValidationError(f"archive member {name!r} has non-normalized mtime") + expected_mode = _EXPECTED_MEMBER_MODES[name] + if member.mode != expected_mode: + raise BundleValidationError( + f"archive member {name!r} mode must be {expected_mode:04o}, got {member.mode:04o}" + ) + maximum = _MAX_MANIFEST_BYTES if name == MANIFEST_NAME else _MAX_MEMBER_BYTES + if member.size <= 0 or member.size > maximum: + raise BundleValidationError( + f"archive member {name!r} size must be from 1 to {maximum} bytes" + ) + + +def _read_member(archive: tarfile.TarFile, member: tarfile.TarInfo, maximum: int) -> bytes: + stream = archive.extractfile(member) + if stream is None: + raise BundleValidationError(f"cannot read archive member {member.name!r}") + value = stream.read(maximum + 1) + if len(value) != member.size: + raise BundleValidationError(f"archive member {member.name!r} is truncated or oversized") + return value + + +def _hash_member(archive: tarfile.TarFile, member: tarfile.TarInfo) -> Tuple[str, int]: + stream = archive.extractfile(member) + if stream is None: + raise BundleValidationError(f"cannot read archive member {member.name!r}") + digest = hashlib.sha256() + size = 0 + while True: + block = stream.read(_COPY_CHUNK_BYTES) + if not block: + break + size += len(block) + if size > member.size: + raise BundleValidationError(f"archive member {member.name!r} exceeds its header size") + digest.update(block) + if size != member.size: + raise BundleValidationError(f"archive member {member.name!r} is truncated") + return digest.hexdigest(), size + + +class _LimitedReader: + """Bound decompressed bytes consumed by tarfile's streaming reader.""" + + def __init__(self, source: BinaryIO, limit: int) -> None: + self._source = source + self._limit = limit + self._count = 0 + + def read(self, size: int = -1) -> bytes: + remaining = self._limit - self._count + if remaining < 0: + raise BundleValidationError("decompressed tar stream exceeds its size limit") + if size < 0 or size > remaining + 1: + size = remaining + 1 + value = self._source.read(size) + self._count += len(value) + if self._count > self._limit: + raise BundleValidationError("decompressed tar stream exceeds its size limit") + return value + + @property + def count(self) -> int: + return self._count + + +def _validate_snapshot( + snapshot: BinaryIO, archive_sha256: str, expected_target: str +) -> BundleManifest: + try: + snapshot.seek(0) + if snapshot.read(len(_NORMALIZED_GZIP_HEADER)) != _NORMALIZED_GZIP_HEADER: + raise BundleValidationError("bundle does not have a normalized gzip header") + snapshot.seek(0) + with gzip.GzipFile(fileobj=snapshot, mode="rb") as compressed: + limited = _LimitedReader(compressed, _MAX_TAR_STREAM_BYTES) + with tarfile.open( + fileobj=limited, mode="r|", format=tarfile.USTAR_FORMAT, ignore_zeros=True + ) as archive: + by_name: Dict[str, tarfile.TarInfo] = {} + observed: Dict[str, Tuple[str, int]] = {} + manifest_bytes = None + unpacked_size = 0 + next_header_offset = 0 + for member in archive: + if ( + member.offset != next_header_offset + or member.offset_data != member.offset + tarfile.BLOCKSIZE + ): + raise BundleValidationError( + "archive contains hidden or non-normalized tar headers" + ) + if member.name in by_name: + raise BundleValidationError(f"duplicate archive member {member.name!r}") + _validate_member_header(member) + by_name[member.name] = member + unpacked_size += member.size + if unpacked_size > _MAX_UNPACKED_BYTES: + raise BundleValidationError( + f"archive exceeds {_MAX_UNPACKED_BYTES} unpacked bytes" + ) + if member.name == MANIFEST_NAME: + manifest_bytes = _read_member( + archive, member, _MAX_MANIFEST_BYTES + ) + else: + observed[member.name] = _hash_member(archive, member) + padded_size = ( + (member.size + tarfile.BLOCKSIZE - 1) + // tarfile.BLOCKSIZE + * tarfile.BLOCKSIZE + ) + next_header_offset = member.offset_data + padded_size + + minimum_end = next_header_offset + 2 * tarfile.BLOCKSIZE + normalized_stream_size = ( + (minimum_end + tarfile.RECORDSIZE - 1) + // tarfile.RECORDSIZE + * tarfile.RECORDSIZE + ) + if limited.count != normalized_stream_size: + raise BundleValidationError( + "archive has non-normalized tar padding or trailing data" + ) + + if set(by_name) != set(_EXPECTED_ARCHIVE_MEMBERS): + missing = set(_EXPECTED_ARCHIVE_MEMBERS) - set(by_name) + if missing: + raise BundleValidationError( + f"archive is missing members {sorted(missing)}" + ) + raise BundleValidationError( + "archive must contain exactly manifest.json and the four runtime files" + ) + if manifest_bytes is None: + raise BundleValidationError("archive is missing manifest.json") + manifest = _parse_manifest( + manifest_bytes, archive_sha256, expected_target + ) + + for name in REQUIRED_RUNTIME_FILES: + member = by_name[name] + metadata = manifest.files[name] + digest, size = observed[name] + if member.size != metadata.size or size != metadata.size: + raise BundleValidationError( + f"archive member {name!r} size does not match manifest" + ) + if member.mode != metadata.mode: + raise BundleValidationError( + f"archive member {name!r} mode does not match manifest" + ) + if not hmac.compare_digest(digest, metadata.sha256): + raise BundleValidationError( + f"archive member {name!r} content does not match manifest" + ) + return manifest + except BundleValidationError: + raise + except (tarfile.TarError, EOFError, OSError, ValueError) as error: + raise BundleValidationError(f"invalid gzip tar bundle: {error}") from error + + +def validate_bundle( + bundle_path: PathLike, + expected_sha256: str, + expected_target: str = SUPPORTED_TARGET, +) -> BundleManifest: + """Authenticate and strictly validate a bundle without extracting it. + + ``expected_sha256`` must come from a trusted adapter configuration, not + from the bundle or its adjacent sidecar. The only supported payload target + is x86_64 Linux with glibc; callers cannot weaken that check by supplying a + different ``expected_target``. + """ + + if expected_target != SUPPORTED_TARGET: + raise BundleValidationError( + f"unsupported expected target {expected_target!r}; only {SUPPORTED_TARGET!r} is allowed" + ) + with _verified_snapshot(bundle_path, expected_sha256) as (snapshot, archive_sha256): + return _validate_snapshot(snapshot, archive_sha256, expected_target) + + +def assert_supported_runtime() -> str: + """Fail closed unless the current machine is x86_64 Linux with glibc. + + Controllers may validate a bundle on another OS, so this check is explicit + rather than part of :func:`validate_bundle`. Invoke it in the task + container before installing or executing the runtime payload. + """ + + if platform.system() != "Linux" or platform.machine() != "x86_64": + raise BundleValidationError("S-Code Harbor bundles require x86_64 Linux") + try: + glibc_version = os.confstr("CS_GNU_LIBC_VERSION") + except (AttributeError, OSError, ValueError): + glibc_version = None + if not glibc_version or _GLIBC_RE.fullmatch(glibc_version) is None: + raise BundleValidationError("S-Code Harbor bundles require detectable glibc") + return glibc_version + + +def _main(arguments: Sequence[str]) -> int: + parser = argparse.ArgumentParser(description="Validate a pinned S-Code Harbor bundle") + parser.add_argument("bundle", type=Path) + parser.add_argument("sha256", help="trusted lowercase SHA-256 of the complete archive") + options = parser.parse_args(arguments) + manifest = validate_bundle(options.bundle, options.sha256) + print( + f"validated {options.bundle}: {manifest.source_commit} " + f"{manifest.target} {manifest.archive_sha256}" + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(_main(os.sys.argv[1:])) diff --git a/tests/test-harbor-bundle.py b/tests/test-harbor-bundle.py new file mode 100644 index 0000000..7c9a857 --- /dev/null +++ b/tests/test-harbor-bundle.py @@ -0,0 +1,278 @@ +#!/usr/bin/env python3 +"""Focused contract and adversarial tests for the Harbor runtime bundle.""" + +import gzip +import hashlib +import importlib.util +import io +import json +from pathlib import Path +import subprocess +import sys +import tarfile +import tempfile +import unittest +from unittest import mock + + +ROOT = Path(__file__).resolve().parents[1] +RUNTIME_PATH = ROOT / "integrations/harbor/runtime.py" +BUILDER_PATH = ROOT / "integrations/harbor/build-bundle.sh" +SPEC = importlib.util.spec_from_file_location("harbor_bundle_runtime", RUNTIME_PATH) +runtime = importlib.util.module_from_spec(SPEC) +sys.modules[SPEC.name] = runtime +SPEC.loader.exec_module(runtime) + + +class BundleTests(unittest.TestCase): + def setUp(self): + temporary = tempfile.TemporaryDirectory(prefix="s-code-harbor-bundle-test.") + self.addCleanup(temporary.cleanup) + self.directory = Path(temporary.name) + self.counter = 0 + self.payloads = { + name: f"fixture:{name}\n".encode() for name in runtime.REQUIRED_RUNTIME_FILES + } + + def manifest(self): + return { + "build": { + "cargo": "cargo 1.89.0", + "glibc": "glibc 2.36", + "node": "v22.23.2", + "npm": "10.9.2", + "python": "3.12.0", + "rustc": "rustc 1.89.0", + "zlib": "1.2.13", + }, + "files": { + name: { + "mode": "0755", + "sha256": hashlib.sha256(contents).hexdigest(), + "size": len(contents), + } + for name, contents in self.payloads.items() + }, + "format": runtime.BUNDLE_FORMAT, + "schema_version": runtime.BUNDLE_SCHEMA_VERSION, + "source": { + "commit": "a" * 40, + "dirty": False, + "locks": { + "Cargo.lock": "c" * 64, + "web/package-lock.json": "d" * 64, + }, + "tree": "b" * 40, + }, + "target": runtime.SUPPORTED_TARGET, + } + + def manifest_bytes(self, value=None): + if value is None: + value = self.manifest() + return ( + json.dumps(value, ensure_ascii=True, separators=(",", ":"), sort_keys=True) + + "\n" + ).encode() + + def entries(self, manifest_bytes=None): + if manifest_bytes is None: + manifest_bytes = self.manifest_bytes() + return [ + (runtime.MANIFEST_NAME, manifest_bytes, 0o644, tarfile.REGTYPE, ""), + *[ + (name, contents, 0o755, tarfile.REGTYPE, "") + for name, contents in self.payloads.items() + ], + ] + + def archive(self, entries=None, *, gzip_mtime=0): + if entries is None: + entries = self.entries() + self.counter += 1 + path = self.directory / f"bundle-{self.counter}.tar.gz" + with path.open("xb") as raw_output: + with gzip.GzipFile( + filename="", + mode="wb", + compresslevel=9, + fileobj=raw_output, + mtime=gzip_mtime, + ) as compressed: + with tarfile.open( + fileobj=compressed, mode="w|", format=tarfile.USTAR_FORMAT + ) as output: + for name, contents, mode, kind, linkname in entries: + member = tarfile.TarInfo(name) + member.mode = mode + member.mtime = 0 + member.uid = 0 + member.gid = 0 + member.uname = "" + member.gname = "" + member.type = kind + member.linkname = linkname + member.size = len(contents) if kind == tarfile.REGTYPE else 0 + source = io.BytesIO(contents) if kind == tarfile.REGTYPE else None + output.addfile(member, source) + digest = hashlib.sha256(path.read_bytes()).hexdigest() + return path, digest + + def assert_rejected(self, path, digest): + with self.assertRaises(runtime.BundleValidationError): + runtime.validate_bundle(path, digest) + + def test_valid_bundle_returns_authenticated_manifest(self): + path, digest = self.archive() + manifest = runtime.validate_bundle(path, digest) + self.assertEqual(manifest.archive_sha256, digest) + self.assertEqual(manifest.bundle_format, runtime.BUNDLE_FORMAT) + self.assertEqual(manifest.source_commit, "a" * 40) + self.assertEqual(manifest.source_tree, "b" * 40) + self.assertFalse(manifest.source_dirty) + self.assertEqual(manifest.target, runtime.SUPPORTED_TARGET) + self.assertEqual(manifest.glibc_version, "glibc 2.36") + self.assertEqual(tuple(manifest.files), runtime.REQUIRED_RUNTIME_FILES) + self.assertEqual(manifest.files["s-code"].mode, 0o755) + + def test_outer_digest_and_payload_tampering_are_rejected(self): + path, digest = self.archive() + self.assert_rejected(path, "0" * 64) + for malformed in ("short", "A" * 64): + self.assert_rejected(path, malformed) + + entries = self.entries() + name, _, mode, kind, linkname = entries[1] + entries[1] = (name, b"tampered\n", mode, kind, linkname) + tampered_path, tampered_digest = self.archive(entries) + self.assert_rejected(tampered_path, tampered_digest) + + def test_traversal_unexpected_and_duplicate_members_are_rejected(self): + for unsafe_name in ("../escape", "/absolute", "unexpected"): + with self.subTest(name=unsafe_name): + entries = self.entries() + entries.append((unsafe_name, b"x", 0o644, tarfile.REGTYPE, "")) + self.assert_rejected(*self.archive(entries)) + + entries = self.entries() + entries.append(entries[1]) + self.assert_rejected(*self.archive(entries)) + + def test_links_nonregular_members_and_unsafe_modes_are_rejected(self): + for kind, linkname in ((tarfile.SYMTYPE, "s-code-cli"), (tarfile.DIRTYPE, "")): + with self.subTest(kind=kind): + entries = self.entries() + entries[1] = ("s-code", b"", 0o755, kind, linkname) + self.assert_rejected(*self.archive(entries)) + + entries = self.entries() + name, contents, _, kind, linkname = entries[1] + entries[1] = (name, contents, 0o4755, kind, linkname) + self.assert_rejected(*self.archive(entries)) + + def test_malformed_duplicate_key_and_unsafe_manifests_are_rejected(self): + valid = self.manifest_bytes() + duplicate_key = valid.replace( + b'{"build":', b'{"schema_version":1,"build":', 1 + ) + variants = [b"{not-json", duplicate_key] + + unsafe = self.manifest() + unsafe["source"]["dirty"] = True + variants.append(self.manifest_bytes(unsafe)) + unsafe = self.manifest() + unsafe["target"] = "aarch64-unknown-linux-gnu" + variants.append(self.manifest_bytes(unsafe)) + unsafe = self.manifest() + unsafe["files"]["s-code"]["mode"] = "0777" + variants.append(self.manifest_bytes(unsafe)) + + for manifest_bytes in variants: + with self.subTest(manifest=manifest_bytes[:30]): + self.assert_rejected(*self.archive(self.entries(manifest_bytes))) + + def test_size_bounds_reject_cheap_oversized_fixtures(self): + path, digest = self.archive() + for field in ("_MAX_ARCHIVE_BYTES", "_MAX_UNPACKED_BYTES"): + with self.subTest(field=field), mock.patch.object(runtime, field, 1): + self.assert_rejected(path, digest) + + oversized_manifest = b" " * (runtime._MAX_MANIFEST_BYTES + 1) + self.assert_rejected(*self.archive(self.entries(oversized_manifest))) + + # A tar header can claim a huge member without allocating its contents. + member = tarfile.TarInfo("s-code") + member.mode = 0o755 + member.mtime = member.uid = member.gid = 0 + member.size = runtime._MAX_MEMBER_BYTES + 1 + raw_tar = member.tobuf(format=tarfile.USTAR_FORMAT) + b"\0" * tarfile.RECORDSIZE + self.counter += 1 + huge_header = self.directory / f"bundle-{self.counter}.tar.gz" + with huge_header.open("xb") as raw_output: + with gzip.GzipFile( + filename="", mode="wb", fileobj=raw_output, mtime=0, compresslevel=9 + ) as compressed: + compressed.write(raw_tar) + self.assert_rejected(huge_header, hashlib.sha256(huge_header.read_bytes()).hexdigest()) + + def test_non_normalized_archive_and_outer_file_types_are_rejected(self): + self.assert_rejected(*self.archive(gzip_mtime=1)) + path, digest = self.archive() + link = self.directory / "bundle-link.tar.gz" + link.symlink_to(path) + self.assert_rejected(link, digest) + self.assert_rejected(self.directory, digest) + + def test_validator_cli_and_help(self): + path, digest = self.archive() + valid = subprocess.run( + [sys.executable, str(RUNTIME_PATH), str(path), digest], + text=True, + capture_output=True, + timeout=10, + ) + self.assertEqual(valid.returncode, 0, valid.stdout + valid.stderr) + self.assertIn("validated", valid.stdout) + + help_result = subprocess.run( + [sys.executable, str(RUNTIME_PATH), "--help"], + text=True, + capture_output=True, + timeout=10, + ) + self.assertEqual(help_result.returncode, 0, help_result.stderr) + self.assertIn("Validate a pinned S-Code Harbor bundle", help_result.stdout) + + invalid = subprocess.run( + [sys.executable, str(RUNTIME_PATH), str(path), "0" * 64], + text=True, + capture_output=True, + timeout=10, + ) + self.assertNotEqual(invalid.returncode, 0) + self.assertIn("SHA-256 mismatch", invalid.stderr) + + def test_builder_help_and_argument_rejection(self): + help_result = subprocess.run( + [str(BUILDER_PATH), "--help"], + text=True, + capture_output=True, + timeout=10, + ) + self.assertEqual(help_result.returncode, 0, help_result.stderr) + self.assertIn("Usage: integrations/harbor/build-bundle.sh", help_result.stdout) + self.assertIn("x86_64-unknown-linux-gnu.tar.gz.sha256", help_result.stdout) + + for arguments in (("--unknown",), ("--help", "extra")): + with self.subTest(arguments=arguments): + rejected = subprocess.run( + [str(BUILDER_PATH), *arguments], + text=True, + capture_output=True, + timeout=10, + ) + self.assertEqual(rejected.returncode, 2) + + +if __name__ == "__main__": + unittest.main() From 4e8445d12d629a2c37ea620791812065b10d40bd Mon Sep 17 00:00:00 2001 From: Wei Deng Date: Sun, 18 Oct 2026 16:11:10 -0700 Subject: [PATCH 3/9] feat(harbor): add the Harbor 0.17 adapter integrations.harbor.agent:SCodeAgent runs one non-interactive S-Code turn per Harbor trial against an OpenAI-compatible endpoint. It is pinned to harbor==0.17.0. - Install: validates the bundle, uploads it, checks its digest inside the task container, and runs each binary's self-test with fresh S-Code state. - Run: uses Full permission mode and --raw-events-jsonl. It disables the browser, PDF and interactive-input tools and merges system messages. It keeps S-Code's limits inside the task's Harbor deadline. - Results: keeps events.jsonl, stderr.log, final.txt, provenance.json and daemon.log in the trial's agent logs. A run that never gets a model response is raised as Harbor's NetworkConnectionError or UnknownApiError, so it counts as an infrastructure error and the verifier still runs. Refs #129 Signed-off-by: Wei Deng --- integrations/harbor/agent.py | 1120 ++++++++++++++++++++++++++ integrations/harbor/requirements.txt | 2 + tests/test-harbor-adapter.py | 1033 ++++++++++++++++++++++++ 3 files changed, 2155 insertions(+) create mode 100644 integrations/harbor/agent.py create mode 100644 integrations/harbor/requirements.txt create mode 100644 tests/test-harbor-adapter.py diff --git a/integrations/harbor/agent.py b/integrations/harbor/agent.py new file mode 100644 index 0000000..6993895 --- /dev/null +++ b/integrations/harbor/agent.py @@ -0,0 +1,1120 @@ +"""Pinned Harbor 0.17 adapter for one isolated S-Code run. + +The adapter deliberately has a small surface: a local, checksum-pinned runtime +bundle; one OpenAI-compatible model endpoint; and one credential environment +handle supplied through Harbor's agent environment. It does not download +software, retry a turn, resume a session, persist state, or claim ATIF support. +""" + +from __future__ import annotations + +import asyncio +import json +import math +import re +import shlex +import shutil +import tempfile +from importlib.metadata import version as distribution_version +from ipaddress import ip_address +from pathlib import Path +from typing import Any, override +from urllib.parse import urlsplit + +from harbor.agents.installed.base import ( + BaseInstalledAgent, + NetworkConnectionError, + NonZeroAgentExitCodeError, + UnknownApiError, + with_prompt_template, +) +from harbor.environments.base import BaseEnvironment +from harbor.models.agent.context import AgentContext +from harbor.models.task.config import TaskConfig as HarborTaskConfig +from harbor.models.trial.config import TrialConfig +from harbor.models.trial.paths import EnvironmentPaths + +from .runtime import BundleManifest, validate_bundle, validate_sha256 + + +_SUPPORTED_HARBOR_VERSION = "0.17.0" +_ADAPTER_VERSION = "0.1.0" +_CREDENTIAL_ENV_RE = re.compile(r"^[A-Z_][A-Z0-9_]*$") +_SENSITIVE_ENV_TOKEN_RE = re.compile( + r"KEY|SECRET|TOKEN|PASSWORD|CREDENTIAL|AUTH", + re.IGNORECASE, +) +_EXECUTABLE_MEMBERS = ( + "s-code", + "s-code-cli", + "s-code-daemon", + "s-code-pdf-worker", +) +_MAX_CALL_LIMIT = 10_000 +_MAX_IDLE_SEC = 3_600 +_MAX_MODEL_TIMEOUT_SEC = 3_600 +_MAX_COST_MICROS = 1_000_000_000_000 +_MAX_TOTAL_TOKENS = 1_000_000_000 +_MIN_HARBOR_TIMEOUT_SEC = 180 +_MAX_HARBOR_TIMEOUT_SEC = 86_400 +_MODEL_TIMEOUT_MARGIN_SEC = 150 +_CORE_TIMEOUT_MARGIN_SEC = 120 +_CLI_TIMEOUT_MARGIN_SEC = 90 +_CLEANUP_SERVER_TIMEOUT_SEC = 10 +_CLEANUP_CLIENT_TIMEOUT_SEC = 15 +_DISABLED_TOOLS = ( + "web_open", + "pdf_read", + "pdf_view", + "request_user_input", +) + +_INSTALL_DIR = "/installed-agent/s-code" +_PRIVATE_DIR = "/installed-agent/s-code-private" +_BUNDLE_ARCHIVE_PATH = "/installed-agent/s-code-runtime.tar.gz" +_RUNNER_PATH = "/installed-agent/s-code-harbor-runner" +_PROMPT_PATH = f"{_PRIVATE_DIR}/instruction.txt" +_PROVENANCE_SEED_PATH = f"{_PRIVATE_DIR}/provenance.seed.json" +_EXIT_STATUS_PATH = f"{_PRIVATE_DIR}/exit.status" + +_EVENTS_PATH = (EnvironmentPaths.agent_dir / "events.jsonl").as_posix() +_STDERR_PATH = (EnvironmentPaths.agent_dir / "stderr.log").as_posix() +_FINAL_PATH = (EnvironmentPaths.agent_dir / "final.txt").as_posix() +_PROVENANCE_PATH = (EnvironmentPaths.agent_dir / "provenance.json").as_posix() +_DAEMON_LOG_PATH = (EnvironmentPaths.agent_dir / "daemon.log").as_posix() +_STATE_DAEMON_LOG_PATH = f"{_PRIVATE_DIR}/state/logs/daemon.log" + + +# The prompt is intentionally read from an uploaded file. Harbor therefore +# logs only this fixed runner path, never task text or a credential value. The +# S-Code CLI currently accepts its prompt positionally, so the runner performs +# the final data-to-argv handoff without invoking eval or a nested shell. +_RUNNER = f"""#!/bin/sh +set -u +umask 077 + +events={shlex.quote(_EVENTS_PATH)} +stderr_log={shlex.quote(_STDERR_PATH)} +final={shlex.quote(_FINAL_PATH)} +provenance={shlex.quote(_PROVENANCE_PATH)} +provenance_seed={shlex.quote(_PROVENANCE_SEED_PATH)} +prompt={shlex.quote(_PROMPT_PATH)} +exit_status={shlex.quote(_EXIT_STATUS_PATH)} +daemon_log={shlex.quote(_DAEMON_LOG_PATH)} + +record_exit() {{ + recorded_status=$1 + cleanup_status=$2 + runner_status=$3 + provenance_tmp="$provenance.tmp" + if sed \ + -e "s/\\\"daemon_cleanup_exit_code\\\": null/\\\"daemon_cleanup_exit_code\\\": $cleanup_status/" \ + -e "s/\\\"exit_code\\\": null/\\\"exit_code\\\": $recorded_status/" \ + -e "s/\\\"runner_exit_code\\\": null/\\\"runner_exit_code\\\": $runner_status/" \ + "$provenance_seed" > "$provenance_tmp" + then + mv "$provenance_tmp" "$provenance" + else + rm -f "$provenance_tmp" + fi + status_tmp="$exit_status.tmp" + if printf '%s %s %s\n' \ + "$recorded_status" "$cleanup_status" "$runner_status" > "$status_tmp" + then + mv "$status_tmp" "$exit_status" + else + rm -f "$status_tmp" + fi +}} + +fail() {{ + printf '%s\n' "$1" >> "$stderr_log" + record_exit 1 0 1 + tail -c 65536 "$stderr_log" >&2 2>/dev/null || true + exit 1 +}} + +: > "$events" || exit 1 +: > "$stderr_log" || exit 1 +: > "$final" || fail "S-Code could not create its final-output log" +cp "$provenance_seed" "$provenance" || fail "S-Code provenance data is unavailable" + +[ -f "$prompt" ] && [ ! -L "$prompt" ] || fail "S-Code prompt data is unavailable" +for directory in \ + {shlex.quote(_PRIVATE_DIR + "/home")} \ + {shlex.quote(_PRIVATE_DIR + "/state")} \ + {shlex.quote(_PRIVATE_DIR + "/run")} +do + [ ! -e "$directory" ] && [ ! -L "$directory" ] || \ + fail "S-Code refused non-fresh trial state" + mkdir -m 700 "$directory" || fail "S-Code could not create private trial state" +done + +# Appending then removing a non-newline sentinel preserves trailing newlines in +# the uploaded instruction despite POSIX command-substitution trimming. +instruction=$(cat "$prompt"; printf x) || fail "S-Code could not read prompt data" +instruction=${{instruction%x}} + +status=0 +{shlex.quote(_INSTALL_DIR + "/s-code")} exec \ + --raw-events-jsonl \ + --ephemeral \ + --permission-mode full \ + --timeout "$S_CODE_HARBOR_CLI_TIMEOUT_SEC" \ + --output-last-message "$final" \ + -- "$instruction" \ + < /dev/null \ + > "$events" 2> "$stderr_log" || status=$? + +cleanup_status=0 +{shlex.quote(_INSTALL_DIR + "/s-code")} stop \ + >> "$stderr_log" 2>&1 || cleanup_status=$? + +# Keep the daemon's own log with the trial; it records why model requests failed. +cp {shlex.quote(_STATE_DAEMON_LOG_PATH)} "$daemon_log" 2>/dev/null || true + +runner_status=$status +record_exit "$status" "$cleanup_status" "$runner_status" +chmod 600 "$events" "$stderr_log" "$final" "$provenance" "$daemon_log" 2>/dev/null || true + +if [ "$runner_status" -ne 0 ]; then + # Feed only S-Code's own bounded stderr tail back to Harbor so + # BaseInstalledAgent can classify the non-zero exit correctly. The complete + # stderr remains in /logs/agent/stderr.log. + tail -c 65536 "$stderr_log" >&2 2>/dev/null || true +fi +exit "$runner_status" +""" + + +def _positive_bounded_int( + value: object, + *, + name: str, + maximum: int, + minimum: int = 1, +) -> int: + if isinstance(value, bool) or not isinstance(value, int): + raise TypeError(f"{name} must be an integer") + if not minimum <= value <= maximum: + raise ValueError(f"{name} must be between {minimum} and {maximum}") + return value + + +def _model_base_url(value: object) -> str: + if not isinstance(value, str) or not value or value != value.strip(): + raise ValueError("model_base_url must be a non-empty absolute URL") + if any(character.isspace() for character in value): + raise ValueError("model_base_url must not contain whitespace") + parsed = urlsplit(value) + if parsed.scheme not in {"http", "https"} or not parsed.hostname: + raise ValueError("model_base_url must be an absolute HTTP(S) URL") + try: + parsed.port + except ValueError as exc: + raise ValueError("model_base_url has an invalid port") from exc + if parsed.username is not None or parsed.password is not None: + raise ValueError("model_base_url must not contain credentials") + if parsed.query or parsed.fragment: + raise ValueError("model_base_url must not contain a query or fragment") + loopback = parsed.hostname.casefold() == "localhost" + if not loopback: + try: + loopback = ip_address(parsed.hostname).is_loopback + except ValueError: + pass + if parsed.scheme != "https" and not loopback: + raise ValueError("model_base_url must use HTTPS or loopback HTTP") + return value + + +def _model_name(value: object) -> str: + if not isinstance(value, str) or not value.strip(): + raise ValueError("model_name is required") + if value != value.strip() or "\x00" in value or "\n" in value or "\r" in value: + raise ValueError("model_name must be a single non-empty identifier") + return value + + +def _credential_env(value: object) -> str: + if not isinstance(value, str) or not _CREDENTIAL_ENV_RE.fullmatch(value): + raise ValueError("credential_env must be an uppercase environment-variable name") + if _SENSITIVE_ENV_TOKEN_RE.search(value) is None: + raise ValueError( + "credential_env must contain KEY, SECRET, TOKEN, PASSWORD, " + "CREDENTIAL, or AUTH so Harbor treats its value as sensitive" + ) + return value + + +def _harbor_version() -> str: + actual = distribution_version("harbor") + if actual != _SUPPORTED_HARBOR_VERSION: + raise RuntimeError( + "S-Code's Harbor adapter requires harbor==" + f"{_SUPPORTED_HARBOR_VERSION}; found {actual}" + ) + return actual + + +def _resolved_harbor_agent_timeout_sec(environment: BaseEnvironment) -> int: + """Reproduce Harbor 0.17's effective single-step agent deadline.""" + + trial_path = environment.trial_paths.config_path + task_path = environment.environment_dir.parent / "task.toml" + try: + trial = TrialConfig.model_validate_json(trial_path.read_text(encoding="utf-8")) + task = HarborTaskConfig.model_validate_toml( + task_path.read_text(encoding="utf-8") + ) + except (OSError, UnicodeError, ValueError) as exc: + raise RuntimeError("could not resolve Harbor's agent deadline") from exc + if task.steps: + raise ValueError("SCodeAgent does not support multi-step Harbor tasks") + + base = trial.agent.override_timeout_sec or task.agent.timeout_sec + if base is None: + raise ValueError("Harbor task must define an agent timeout") + maximum = trial.agent.max_timeout_sec or float("inf") + multiplier = ( + trial.agent_timeout_multiplier + if trial.agent_timeout_multiplier is not None + else trial.timeout_multiplier + ) + resolved = min(base, maximum) * multiplier + if not math.isfinite(resolved): + raise ValueError("Harbor resolved a non-finite agent timeout") + timeout = math.floor(resolved) + if not _MIN_HARBOR_TIMEOUT_SEC <= timeout <= _MAX_HARBOR_TIMEOUT_SEC: + raise ValueError( + "Harbor's resolved agent timeout must be between " + f"{_MIN_HARBOR_TIMEOUT_SEC} and {_MAX_HARBOR_TIMEOUT_SEC} seconds" + ) + return timeout + + +def _runtime_preflight_command(required_glibc: str) -> str: + """Return a POSIX-shell preflight with numeric dotted-version comparison.""" + + required = required_glibc.removeprefix("glibc ") + return f"""set -eu +for required_command in \ + cat chmod chown cp getconf gzip head id mkdir mv rm sed sha256sum sleep stat tail tar uname wc +do + command -v "$required_command" >/dev/null || {{ + echo "S-Code requires command: $required_command" >&2 + exit 1 + }} +done +[ -x /usr/bin/nohup ] || [ -x /bin/nohup ] || {{ + echo 'S-Code requires /usr/bin/nohup or /bin/nohup' >&2 + exit 1 +}} +[ "$(uname -s)" = Linux ] || {{ echo 'S-Code requires Linux' >&2; exit 1; }} +[ "$(uname -m)" = x86_64 ] || {{ echo 'S-Code requires x86_64' >&2; exit 1; }} +actual_glibc=$(getconf GNU_LIBC_VERSION 2>/dev/null) || {{ + echo 'S-Code requires detectable glibc' >&2 + exit 1 +}} +case "$actual_glibc" in + 'glibc '*) actual_glibc=${{actual_glibc#glibc }} ;; + *) echo 'S-Code requires detectable glibc' >&2; exit 1 ;; +esac +version_at_least() {{ + actual_left=$1 + required_left=$2 + while [ -n "$actual_left$required_left" ]; do + case "$actual_left" in + *.*) actual_part=${{actual_left%%.*}}; actual_left=${{actual_left#*.}} ;; + '') actual_part=0 ;; + *) actual_part=$actual_left; actual_left= ;; + esac + case "$required_left" in + *.*) required_part=${{required_left%%.*}}; required_left=${{required_left#*.}} ;; + '') required_part=0 ;; + *) required_part=$required_left; required_left= ;; + esac + case "$actual_part:$required_part" in + *[!0-9:]*) return 2 ;; + esac + [ "$actual_part" -gt "$required_part" ] && return 0 + [ "$actual_part" -lt "$required_part" ] && return 1 + done + return 0 +}} +if ! version_at_least "$actual_glibc" {shlex.quote(required)}; then + echo "S-Code bundle requires glibc >= {required}; found $actual_glibc" >&2 + exit 1 +fi +test ! -e {shlex.quote(_INSTALL_DIR)} +test ! -L {shlex.quote(_INSTALL_DIR)} +test ! -e {shlex.quote(_PRIVATE_DIR)} +test ! -L {shlex.quote(_PRIVATE_DIR)} +test ! -e {shlex.quote(_BUNDLE_ARCHIVE_PATH)} +test ! -L {shlex.quote(_BUNDLE_ARCHIVE_PATH)} +test ! -e {shlex.quote(_RUNNER_PATH)} +test ! -L {shlex.quote(_RUNNER_PATH)} +mkdir -m 0755 {shlex.quote(_INSTALL_DIR)} +mkdir -m 0700 {shlex.quote(_PRIVATE_DIR)} +""" + + +def _fallback_terminal(exit_code: int | None) -> tuple[str, str | None]: + if exit_code == 0: + return "failed", None + if exit_code == 124: + return "timed_out", "cli_timeout" + if exit_code == 130: + return "cancelled", None + return "failed", None + + +def _classify_event_stream(path: Path, exit_code: int | None) -> dict[str, Any]: + """Classify a completed run from S-Code's JSONL terminal evidence.""" + + status, limit_kind = _fallback_terminal(exit_code) + failure_kind: str | None = None + terminal: dict[str, Any] | None = None + usage: dict[str, int] = {} + model_requests = {"accepted": 0, "rejected": 0, "connection_error": 0} + records = 0 + invalid_line: int | None = None + + try: + source = path.open("rb") + except OSError: + return { + "status": status, + "limit_kind": limit_kind, + "failure_kind": failure_kind, + "terminal_event": None, + "usage": {}, + "model_requests": model_requests, + "event_stream": { + "validation": "unavailable", + "records": 0, + "error": "events.jsonl could not be read", + }, + } + + with source: + for line_number, line in enumerate(source, start=1): + if not line.strip(): + invalid_line = invalid_line or line_number + continue + try: + event = json.loads(line) + except (json.JSONDecodeError, UnicodeError): + invalid_line = invalid_line or line_number + continue + if not isinstance(event, dict): + invalid_line = invalid_line or line_number + continue + records += 1 + kind = event.get("kind") or event.get("type") + if not isinstance(kind, str): + continue + payload = event.get("payload") + if not isinstance(payload, dict): + payload = {} + error_code = event.get("error_code") or payload.get("error_code") + if not isinstance(error_code, str): + error_code = None + reason = payload.get("reason") + if not isinstance(reason, str): + reason = None + if kind == "privacy.request.finished": + request = payload.get("request") + outcome = request.get("status") if isinstance(request, dict) else None + if outcome in model_requests: + model_requests[outcome] += 1 + + usage_keys = ( + ("input_units", "input_tokens"), + ("output_units", "output_tokens"), + ("input_tokens", "input_tokens"), + ("output_tokens", "output_tokens"), + ("model_calls", "model_calls"), + ("tool_calls", "tool_calls"), + ) + if kind == "turn.usage" or kind.startswith("turn."): + for source_key, target_key in usage_keys: + value = payload.get(source_key) + if isinstance(value, int) and not isinstance(value, bool) and value >= 0: + usage[target_key] = value + + candidate: tuple[str, str | None] | None = None + candidate_failure: str | None = None + if kind == "question.required" and payload.get("source") == "tool_call_limit": + error_code = "tool_call_limit" + candidate = ("limit_stopped", "tool_call_limit") + elif kind == "turn.awaiting_input" and error_code == "tool_call_limit": + candidate = ("limit_stopped", "tool_call_limit") + elif kind in {"turn.awaiting_input", "question.required"}: + error_code = error_code or "input_required" + candidate = ("failed", None) + candidate_failure = "input_required" + elif kind in {"turn.awaiting_approval", "approval.required"}: + error_code = error_code or "approval_required" + candidate = ("failed", None) + candidate_failure = "approval_required" + elif kind == "turn.completed": + candidate = ("completed", None) + elif kind == "turn.cancelled": + candidate = ("cancelled", None) + elif kind == "turn.failed": + if error_code == "tool_call_limit": + candidate = ("limit_stopped", "tool_call_limit") + elif error_code == "model_stream_idle_timeout": + candidate = ("timed_out", "model_stream_idle_timeout") + elif error_code == "turn_elapsed_timeout": + candidate = ("timed_out", "turn_elapsed_timeout") + elif reason == "model-call limit reached": + candidate = ("limit_stopped", "model_call_limit") + else: + candidate = ("failed", None) + + if candidate is not None: + status, limit_kind = candidate + failure_kind = candidate_failure + terminal = { + "line": line_number, + "kind": kind, + "error_code": error_code, + } + sequence = event.get("sequence") + if isinstance(sequence, int) and not isinstance(sequence, bool): + terminal["sequence"] = sequence + + # A run that never received a model response measures the endpoint, not + # S-Code, so name the cause and let run() report it as infrastructure. + if status != "completed" and failure_kind is None and model_requests["accepted"] == 0: + if model_requests["connection_error"]: + failure_kind = "model_unreachable" + elif model_requests["rejected"]: + failure_kind = "model_rejected" + + if invalid_line is not None: + validation = "invalid" + error = f"invalid JSONL record at line {invalid_line}" + elif terminal is None: + validation = "incomplete" + error = "no terminal event was found" + else: + validation = "valid" + error = None + return { + "status": status, + "limit_kind": limit_kind, + "failure_kind": failure_kind, + "terminal_event": terminal, + "usage": usage, + "model_requests": model_requests, + "event_stream": { + "validation": validation, + "records": records, + "error": error, + }, + } + + +def _parse_exit_status( + path: Path, +) -> tuple[int | None, int | None, int | None]: + try: + if path.stat().st_size > 32: + return None, None, None + value = path.read_text(encoding="ascii") + except (OSError, UnicodeError): + return None, None, None + match = re.fullmatch( + r"([0-9]{1,3}) ([0-9]{1,3}) ([0-9]{1,3})\n?", + value, + ) + if match is None: + return None, None, None + exit_code, cleanup_code, runner_code = (int(part) for part in match.groups()) + if exit_code > 255 or cleanup_code > 255 or runner_code > 255: + return None, None, None + return exit_code, cleanup_code, runner_code + + +def _manifest_provenance(manifest: BundleManifest) -> dict[str, Any]: + return { + "schema_version": manifest.schema_version, + "bundle_format": manifest.bundle_format, + "source_commit": manifest.source_commit, + "source_tree": manifest.source_tree, + "source_dirty": manifest.source_dirty, + "target": manifest.target, + "glibc_version": manifest.glibc_version, + "lock_sha256s": dict(manifest.lock_sha256s), + "tool_versions": dict(manifest.tool_versions), + "files": { + name: { + "sha256": metadata.sha256, + "size": metadata.size, + "mode": f"{metadata.mode:04o}", + } + for name, metadata in manifest.files.items() + }, + "archive_sha256": manifest.archive_sha256, + } + + +async def _upload_text( + environment: BaseEnvironment, + contents: str, + target_path: str, + *, + mode: int, +) -> None: + with tempfile.TemporaryDirectory(prefix="s-code-harbor-upload-") as directory: + source = Path(directory) / "payload" + # Encode explicitly so a Windows Harbor host cannot translate LF to + # CRLF and alter the prompt, JSON evidence, or POSIX runner bytes. + source.write_bytes(contents.encode("utf-8")) + source.chmod(mode) + await environment.upload_file(source, target_path) + + +class SCodeAgent(BaseInstalledAgent): + """Run exactly one isolated, non-interactive S-Code turn in Harbor.""" + + SUPPORTS_ATIF = False + SUPPORTS_WINDOWS = False + + def __init__( + self, + logs_dir: Path, + model_name: str | None = None, + *, + bundle_path: str | Path, + bundle_sha256: str, + model_base_url: str, + credential_env: str, + max_model_calls: int = 100, + max_tool_calls: int = 256, + model_stream_idle_seconds: int = 300, + model_connect_timeout_sec: int = 10, + model_read_timeout_sec: int = 600, + model_request_timeout_sec: int = 3_600, + max_cost_micros: int = 5_000_000, + max_total_tokens: int = 10_000_000, + extra_env: dict[str, str] | None = None, + **kwargs: Any, + ) -> None: + self._harbor_version = _harbor_version() + self._bundle_path = Path(bundle_path).expanduser().resolve(strict=True) + if not self._bundle_path.is_file(): + raise ValueError("bundle_path must name a local regular file") + self._bundle_sha256 = validate_sha256(bundle_sha256, field="bundle_sha256") + self._model_base_url = _model_base_url(model_base_url) + self._credential_env = _credential_env(credential_env) + self._model_name = _model_name(model_name) + self._max_model_calls = _positive_bounded_int( + max_model_calls, name="max_model_calls", maximum=_MAX_CALL_LIMIT + ) + self._max_tool_calls = _positive_bounded_int( + max_tool_calls, name="max_tool_calls", maximum=_MAX_CALL_LIMIT + ) + self._requested_model_stream_idle_seconds = _positive_bounded_int( + model_stream_idle_seconds, + name="model_stream_idle_seconds", + maximum=_MAX_IDLE_SEC, + ) + # Self-hosted models can stream slowly and go quiet while vLLM buffers + # array-valued tool arguments, so the gateway's interactive defaults + # (45 s between bytes, 180 s per request) are too short here. + self._requested_model_connect_timeout_sec = _positive_bounded_int( + model_connect_timeout_sec, + name="model_connect_timeout_sec", + maximum=_MAX_MODEL_TIMEOUT_SEC, + ) + self._requested_model_read_timeout_sec = _positive_bounded_int( + model_read_timeout_sec, + name="model_read_timeout_sec", + maximum=_MAX_MODEL_TIMEOUT_SEC, + ) + self._requested_model_request_timeout_sec = _positive_bounded_int( + model_request_timeout_sec, + name="model_request_timeout_sec", + maximum=_MAX_MODEL_TIMEOUT_SEC, + ) + self._max_cost_micros = _positive_bounded_int( + max_cost_micros, + name="max_cost_micros", + maximum=_MAX_COST_MICROS, + ) + self._max_total_tokens = _positive_bounded_int( + max_total_tokens, + name="max_total_tokens", + maximum=_MAX_TOTAL_TOKENS, + ) + # Harbor's real per-task deadline becomes available through the + # environment in install(). These placeholder values keep setup + # deterministic; install() replaces them before the run phase. + self._set_effective_timeouts(3_600) + + supplied_env = dict(extra_env or {}) + credential = supplied_env.get(self._credential_env) + if not isinstance(credential, str) or not credential: + raise ValueError( + "credential_env must name a non-empty value supplied through " + "Harbor's agent env/extra_env" + ) + managed_env = { + "S_CODE_MODEL_PROVIDER": "openai_compatible", + "S_CODE_MODEL_BASE_URL": self._model_base_url, + "S_CODE_MODEL_CREDENTIAL_HANDLE": self._credential_env, + "S_CODE_MODEL": self._model_name, + "S_CODE_MODEL_CONNECT_TIMEOUT_SECONDS": str( + self._model_connect_timeout_sec + ), + "S_CODE_MODEL_READ_TIMEOUT_SECONDS": str(self._model_read_timeout_sec), + "S_CODE_MODEL_REQUEST_TIMEOUT_SECONDS": str( + self._model_request_timeout_sec + ), + # Qwen3.6's chat template, like others, rejects later system messages. + "S_CODE_MODEL_MERGE_SYSTEM_MESSAGES": "true", + "S_CODE_HOME": f"{_PRIVATE_DIR}/home", + "S_CODE_STATE_DIR": f"{_PRIVATE_DIR}/state", + "S_CODE_RUNTIME_DIR": f"{_PRIVATE_DIR}/run", + "S_CODE_TURN_MAX_MODEL_CALLS": str(self._max_model_calls), + "S_CODE_TURN_MAX_TOOL_CALLS": str(self._max_tool_calls), + "S_CODE_TURN_MAX_ELAPSED_SECONDS": str(self._core_timeout_sec), + "S_CODE_TURN_MODEL_STREAM_IDLE_SECONDS": str( + self._model_stream_idle_seconds + ), + "S_CODE_TURN_MAX_COST_MICROS": str(self._max_cost_micros), + "S_CODE_TURN_MAX_TOTAL_TOKENS": str(self._max_total_tokens), + "S_CODE_TURN_DISABLED_TOOLS": ",".join(_DISABLED_TOOLS), + "S_CODE_HARBOR_CLI_TIMEOUT_SEC": str(self._cli_timeout_sec), + "NO_COLOR": "1", + "TERM": "dumb", + } + for name, expected in managed_env.items(): + if name in supplied_env and supplied_env[name] != expected: + raise ValueError(f"{name} is managed by SCodeAgent and cannot be overridden") + supplied_env.update(managed_env) + + # Validation is repeated immediately before upload in install(); doing it + # here makes configuration errors fail before Harbor starts a container. + self._manifest = validate_bundle(self._bundle_path, self._bundle_sha256) + self._agent_owner: str | None = None + self._has_run = False + super().__init__( + logs_dir=logs_dir, + model_name=self._model_name, + version=_ADAPTER_VERSION, + extra_env=supplied_env, + **kwargs, + ) + + def _set_effective_timeouts(self, harbor_timeout_sec: int) -> None: + self._harbor_timeout_sec = harbor_timeout_sec + self._model_request_timeout_sec = min( + self._requested_model_request_timeout_sec, + harbor_timeout_sec - _MODEL_TIMEOUT_MARGIN_SEC, + ) + self._model_connect_timeout_sec = min( + self._requested_model_connect_timeout_sec, + self._model_request_timeout_sec, + ) + self._model_read_timeout_sec = min( + self._requested_model_read_timeout_sec, + self._model_request_timeout_sec, + ) + self._model_stream_idle_seconds = min( + self._requested_model_stream_idle_seconds, + self._model_request_timeout_sec, + ) + self._core_timeout_sec = harbor_timeout_sec - _CORE_TIMEOUT_MARGIN_SEC + self._cli_timeout_sec = harbor_timeout_sec - _CLI_TIMEOUT_MARGIN_SEC + + def _effective_timeout_env(self) -> dict[str, str]: + return { + "S_CODE_MODEL_CONNECT_TIMEOUT_SECONDS": str( + self._model_connect_timeout_sec + ), + "S_CODE_MODEL_READ_TIMEOUT_SECONDS": str(self._model_read_timeout_sec), + "S_CODE_MODEL_REQUEST_TIMEOUT_SECONDS": str( + self._model_request_timeout_sec + ), + "S_CODE_TURN_MAX_ELAPSED_SECONDS": str(self._core_timeout_sec), + "S_CODE_TURN_MODEL_STREAM_IDLE_SECONDS": str( + self._model_stream_idle_seconds + ), + "S_CODE_HARBOR_CLI_TIMEOUT_SEC": str(self._cli_timeout_sec), + } + + @staticmethod + @override + def name() -> str: + return "s-code" + + async def _resolve_agent_owner(self, environment: BaseEnvironment) -> None: + result = await self.exec_as_agent( + environment, + command="command -v id >/dev/null && id -u && id -g", + ) + lines = (result.stdout or "").splitlines() + if len(lines) != 2 or any(re.fullmatch(r"[0-9]+", line) is None for line in lines): + raise RuntimeError("could not resolve the Harbor agent user's numeric uid:gid") + uid, gid = (int(line) for line in lines) + if uid > 4_294_967_295 or gid > 4_294_967_295: + raise RuntimeError("Harbor agent uid:gid is outside the supported range") + self._agent_owner = f"{uid}:{gid}" + + def _owner_command(self, *paths: str) -> str: + if self._agent_owner is None: + raise RuntimeError("Harbor agent ownership was not resolved") + owner = shlex.quote(self._agent_owner) + quoted_paths = " ".join(shlex.quote(path) for path in paths) + return f"chown -- {owner} {quoted_paths}" + + @override + async def install(self, environment: BaseEnvironment) -> None: + self._set_effective_timeouts(_resolved_harbor_agent_timeout_sec(environment)) + self._extra_env.update(self._effective_timeout_env()) + await self._resolve_agent_owner(environment) + await self.exec_as_root( + environment, + command=_runtime_preflight_command(self._manifest.glibc_version), + ) + + with tempfile.TemporaryDirectory(prefix="s-code-harbor-bundle-") as directory: + # Authenticate, validate, and upload the same private snapshot. A + # concurrent replacement of the configured path cannot create a + # validation/upload race. + snapshot = Path(directory) / "bundle.tar.gz" + shutil.copyfile(self._bundle_path, snapshot) + current_manifest = validate_bundle(snapshot, self._bundle_sha256) + if current_manifest != self._manifest: + raise ValueError("bundle changed after adapter construction") + await environment.upload_file(snapshot, _BUNDLE_ARCHIVE_PATH) + + await _upload_text(environment, _RUNNER, _RUNNER_PATH, mode=0o755) + owner_private = self._owner_command(_PRIVATE_DIR) + executables = " ".join( + shlex.quote(f"{_INSTALL_DIR}/{name}") for name in _EXECUTABLE_MEMBERS + ) + await self.exec_as_root( + environment, + command=( + "set -eu; " + f"printf '%s %s\\n' {shlex.quote(self._bundle_sha256)} " + f"{shlex.quote(_BUNDLE_ARCHIVE_PATH)} | sha256sum -c - >/dev/null; " + f"tar -xzf {shlex.quote(_BUNDLE_ARCHIVE_PATH)} " + f"-C {shlex.quote(_INSTALL_DIR)}; " + f"rm -f {shlex.quote(_BUNDLE_ARCHIVE_PATH)}; " + f"chmod 0644 {shlex.quote(_INSTALL_DIR + '/manifest.json')}; " + f"chmod 0755 {executables}; " + f"chmod 0755 {shlex.quote(_RUNNER_PATH)}; " + f"chmod 0700 {shlex.quote(_PRIVATE_DIR)}; " + f"{owner_private}; " + f"{shlex.quote(_INSTALL_DIR + '/s-code')} --help >/dev/null; " + f"{shlex.quote(_INSTALL_DIR + '/s-code-cli')} --self-test >/dev/null; " + f"{shlex.quote(_INSTALL_DIR + '/s-code-daemon')} --self-test >/dev/null; " + f"{shlex.quote(_INSTALL_DIR + '/s-code-pdf-worker')} --self-test >/dev/null" + ), + ) + + def _provenance(self) -> dict[str, Any]: + return { + "schema_version": "1", + "adapter": { + "name": self.name(), + "version": _ADAPTER_VERSION, + "harbor_version": self._harbor_version, + "supports_atif": False, + }, + "bundle": _manifest_provenance(self._manifest), + "model": { + "provider": "openai_compatible", + "name": self._model_name, + "base_url": self._model_base_url, + "credential_env": self._credential_env, + }, + "execution": { + "attempts": 1, + "ephemeral": True, + "permission_mode": "full", + "output_mode": "raw_events_jsonl", + "max_model_calls": self._max_model_calls, + "max_tool_calls": self._max_tool_calls, + "disabled_tools": list(_DISABLED_TOOLS), + "model_stream_idle_seconds": self._model_stream_idle_seconds, + "model_connect_timeout_sec": self._model_connect_timeout_sec, + "model_read_timeout_sec": self._model_read_timeout_sec, + "model_request_timeout_sec": self._model_request_timeout_sec, + "max_cost_micros": self._max_cost_micros, + "cost_metering_available": False, + "max_total_tokens": self._max_total_tokens, + "core_timeout_sec": self._core_timeout_sec, + "cli_timeout_sec": self._cli_timeout_sec, + "harbor_timeout_sec": self._harbor_timeout_sec, + "daemon_cleanup_exit_code": None, + "exit_code": None, + "post_run_cleanup_exit_code": None, + "post_run_cleanup_status": "pending", + "runner_exit_code": None, + }, + "result": { + "status": "pending", + "limit_kind": None, + "failure_kind": None, + "terminal_event": None, + "event_stream": { + "validation": "pending", + "records": 0, + "error": None, + }, + }, + "artifacts": { + "events": _EVENTS_PATH, + "stderr": _STDERR_PATH, + "final": _FINAL_PATH, + "provenance": _PROVENANCE_PATH, + "daemon_log": _DAEMON_LOG_PATH, + }, + } + + async def _finalize_provenance( + self, + environment: BaseEnvironment, + context: AgentContext, + *, + process_succeeded: bool, + post_cleanup_code: int | None, + post_cleanup_status: str, + ) -> dict[str, Any]: + with tempfile.TemporaryDirectory(prefix="s-code-harbor-results-") as directory: + local = Path(directory) + status_path = local / "exit.status" + events_path = local / "events.jsonl" + + try: + await environment.download_file(_EXIT_STATUS_PATH, status_path) + except Exception: + exit_code, cleanup_code, runner_code = ( + (0, None, 0) if process_succeeded else (None, None, None) + ) + status_validation = "unavailable" + else: + recorded_exit, cleanup_code, runner_code = _parse_exit_status(status_path) + if recorded_exit is None or runner_code is None: + exit_code, runner_code = ( + (0, 0) if process_succeeded else (None, None) + ) + status_validation = "invalid" + elif ( + recorded_exit != runner_code + or process_succeeded != (runner_code == 0) + ): + exit_code = recorded_exit + status_validation = "inconsistent" + else: + exit_code = recorded_exit + status_validation = "valid" + + try: + await environment.download_file(_EVENTS_PATH, events_path) + except Exception: + pass + result = _classify_event_stream(events_path, exit_code) + result["process_status_validation"] = status_validation + usage = result["usage"] + if "input_tokens" in usage: + context.n_input_tokens = usage["input_tokens"] + if "output_tokens" in usage: + context.n_output_tokens = usage["output_tokens"] + if context.metadata is not None: + context.metadata["s_code"]["result_status"] = result["status"] + context.metadata["s_code"]["usage"] = usage + + provenance = self._provenance() + provenance["execution"]["exit_code"] = exit_code + provenance["execution"]["daemon_cleanup_exit_code"] = cleanup_code + provenance["execution"]["post_run_cleanup_exit_code"] = post_cleanup_code + provenance["execution"]["post_run_cleanup_status"] = post_cleanup_status + provenance["execution"]["runner_exit_code"] = runner_code + provenance["result"] = result + contents = json.dumps( + provenance, + ensure_ascii=False, + indent=2, + sort_keys=True, + ) + "\n" + await _upload_text( + environment, + contents, + _PROVENANCE_PATH, + mode=0o600, + ) + return result + + async def _bounded_cancel_cleanup(self, environment: BaseEnvironment) -> None: + command = ( + "status=0; " + f"{shlex.quote(_INSTALL_DIR + '/s-code')} stop >/dev/null 2>&1 || status=$?; " + f"cp {shlex.quote(_STATE_DAEMON_LOG_PATH)} " + f"{shlex.quote(_DAEMON_LOG_PATH)} 2>/dev/null || true; " + f"chmod 600 {shlex.quote(_DAEMON_LOG_PATH)} 2>/dev/null || true; " + 'exit "$status"' + ) + try: + await asyncio.wait_for( + environment.exec( + command=command, + timeout_sec=_CLEANUP_SERVER_TIMEOUT_SEC, + ), + timeout=_CLEANUP_CLIENT_TIMEOUT_SEC, + ) + except BaseException: + # The original Harbor cancellation remains authoritative. + pass + + @with_prompt_template + @override + async def run( + self, + instruction: str, + environment: BaseEnvironment, + context: AgentContext, + ) -> None: + if self._has_run: + raise RuntimeError("SCodeAgent supports exactly one run per trial") + self._has_run = True + if not isinstance(instruction, str) or not instruction or "\x00" in instruction: + raise ValueError("instruction must be non-empty text without NUL bytes") + + provenance = json.dumps( + self._provenance(), + ensure_ascii=False, + indent=2, + sort_keys=True, + ) + "\n" + await _upload_text( + environment, + instruction, + _PROMPT_PATH, + mode=0o600, + ) + await _upload_text( + environment, + provenance, + _PROVENANCE_SEED_PATH, + mode=0o600, + ) + owner_inputs = self._owner_command( + _PROMPT_PATH, + _PROVENANCE_SEED_PATH, + ) + await self.exec_as_root( + environment, + command=( + f"chmod 0600 {shlex.quote(_PROMPT_PATH)} " + f"{shlex.quote(_PROVENANCE_SEED_PATH)}; " + f"{owner_inputs}" + ), + ) + + context.metadata = { + "s_code": { + "events": _EVENTS_PATH, + "stderr": _STDERR_PATH, + "final": _FINAL_PATH, + "provenance": _PROVENANCE_PATH, + "daemon_log": _DAEMON_LOG_PATH, + "supports_atif": False, + } + } + + execution_error: BaseException | None = None + try: + await self.exec_as_agent( + environment, + command=shlex.quote(_RUNNER_PATH), + ) + except BaseException as error: + execution_error = error + + if isinstance(execution_error, asyncio.CancelledError): + await self._bounded_cancel_cleanup(environment) + raise execution_error + if execution_error is not None and not isinstance(execution_error, Exception): + raise execution_error + + post_cleanup_error: Exception | None = None + post_cleanup_code: int | None = None + try: + cleanup_result = await asyncio.wait_for( + environment.exec( + command=f"{shlex.quote(_INSTALL_DIR + '/s-code')} stop", + timeout_sec=_CLEANUP_SERVER_TIMEOUT_SEC, + ), + timeout=_CLEANUP_CLIENT_TIMEOUT_SEC, + ) + post_cleanup_code = cleanup_result.return_code + post_cleanup_status = ( + "succeeded" if post_cleanup_code == 0 else "failed" + ) + except asyncio.CancelledError: + # A deadline or cancellation during cleanup stays a cancellation, so + # Harbor still records it as one. + await self._bounded_cancel_cleanup(environment) + raise + except Exception as error: + post_cleanup_error = error + post_cleanup_status = "error" + + finalized_result: dict[str, Any] | None = None + finalization_error: Exception | None = None + try: + finalized_result = await self._finalize_provenance( + environment, + context, + process_succeeded=execution_error is None, + post_cleanup_code=post_cleanup_code, + post_cleanup_status=post_cleanup_status, + ) + except Exception as error: + finalization_error = error + self.logger.error("S-Code could not finalize Harbor provenance") + + if post_cleanup_error is not None: + raise NonZeroAgentExitCodeError( + "S-Code daemon cleanup could not confirm quiescence" + ) from post_cleanup_error + if post_cleanup_code != 0: + raise NonZeroAgentExitCodeError( + "S-Code daemon cleanup failed; task state is not quiescent" + ) + if finalized_result is not None and ( + finalized_result["process_status_validation"] == "valid" + and finalized_result["event_stream"]["validation"] == "valid" + ): + failure_kind = finalized_result["failure_kind"] + if failure_kind == "model_unreachable": + raise NetworkConnectionError( + "S-Code did not receive a response from the model endpoint" + ) from execution_error + if failure_kind == "model_rejected": + raise UnknownApiError( + "The model endpoint rejected every S-Code request" + ) from execution_error + if execution_error is not None: + raise execution_error + if finalization_error is not None: + raise NonZeroAgentExitCodeError( + "S-Code could not validate and preserve its completed run" + ) from finalization_error + if finalized_result is None or ( + finalized_result["process_status_validation"] != "valid" + or finalized_result["event_stream"]["validation"] != "valid" + or finalized_result["status"] != "completed" + ): + raise NonZeroAgentExitCodeError( + "S-Code reported success without valid completed-run evidence" + ) diff --git a/integrations/harbor/requirements.txt b/integrations/harbor/requirements.txt new file mode 100644 index 0000000..47291c3 --- /dev/null +++ b/integrations/harbor/requirements.txt @@ -0,0 +1,2 @@ +# Keep the custom-agent API reproducible; newer Harbor releases may change it. +harbor==0.17.0 diff --git a/tests/test-harbor-adapter.py b/tests/test-harbor-adapter.py new file mode 100644 index 0000000..e19a201 --- /dev/null +++ b/tests/test-harbor-adapter.py @@ -0,0 +1,1033 @@ +#!/usr/bin/env python3 +"""Focused standard-library tests for the pinned Harbor adapter.""" + +from __future__ import annotations + +import asyncio +import json +import os +from pathlib import Path, PurePosixPath +import shutil +import subprocess +import sys +import tempfile +import tomllib +import types +import unittest +from types import SimpleNamespace +from unittest import mock + + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT)) + + +class FakeNonZeroExit(RuntimeError): + """Stand-in for Harbor's non-zero process exception.""" + + +class FakeNetworkConnectionError(FakeNonZeroExit): + """Stand-in for Harbor's transport-failure exception.""" + + +class FakeUnknownApiError(FakeNonZeroExit): + """Stand-in for Harbor's unclassified API-response exception.""" + + +class FakeLogger: + def __init__(self) -> None: + self.errors: list[str] = [] + + def error(self, message: str) -> None: + self.errors.append(message) + + +class FakeProcess: + def __init__( + self, + return_code: int = 0, + *, + stdout: str = "", + stderr: str = "", + ) -> None: + self.return_code = return_code + self.stdout = stdout + self.stderr = stderr + + +class FakeAgentContext: + def __init__(self) -> None: + self.metadata = None + self.n_input_tokens = None + self.n_output_tokens = None + + +class FakeTrialConfig: + @classmethod + def model_validate_json(cls, value: str) -> SimpleNamespace: + data = json.loads(value) + agent = data.get("agent", {}) + return SimpleNamespace( + agent=SimpleNamespace( + override_timeout_sec=agent.get("override_timeout_sec"), + max_timeout_sec=agent.get("max_timeout_sec"), + ), + timeout_multiplier=data.get("timeout_multiplier", 1.0), + agent_timeout_multiplier=data.get("agent_timeout_multiplier"), + ) + + +class FakeTaskConfig: + @classmethod + def model_validate_toml(cls, value: str) -> SimpleNamespace: + data = tomllib.loads(value) + return SimpleNamespace( + agent=SimpleNamespace(timeout_sec=data.get("agent", {}).get("timeout_sec")), + steps=data.get("steps", []), + ) + + +class FakeBaseInstalledAgent: + def __init__( + self, + *, + logs_dir: Path, + model_name: str, + version: str, + extra_env: dict[str, str], + **kwargs: object, + ) -> None: + self.base_init = { + "logs_dir": logs_dir, + "model_name": model_name, + "version": version, + "extra_env": dict(extra_env), + "kwargs": dict(kwargs), + } + self._extra_env = dict(extra_env) + self.logger = FakeLogger() + + async def exec_as_agent( + self, + environment: "FakeEnvironment", + *, + command: str, + timeout_sec: int | None = None, + ) -> FakeProcess: + result = await environment.exec(command=command, timeout_sec=timeout_sec) + if result.return_code != 0: + raise FakeNonZeroExit(result.return_code) + return result + + async def exec_as_root( + self, + environment: "FakeEnvironment", + *, + command: str, + timeout_sec: int | None = None, + ) -> FakeProcess: + result = await environment.exec(command=command, timeout_sec=timeout_sec) + if result.return_code != 0: + raise FakeNonZeroExit(result.return_code) + return result + + +def _identity_decorator(function): + return function + + +def _install_harbor_stubs() -> None: + modules = { + name: types.ModuleType(name) + for name in ( + "harbor", + "harbor.agents", + "harbor.agents.installed", + "harbor.agents.installed.base", + "harbor.environments", + "harbor.environments.base", + "harbor.models", + "harbor.models.agent", + "harbor.models.agent.context", + "harbor.models.task", + "harbor.models.task.config", + "harbor.models.trial", + "harbor.models.trial.config", + "harbor.models.trial.paths", + ) + } + for name, module in modules.items(): + if name.rpartition(".")[2] not in {"base", "context", "paths"}: + module.__path__ = [] + sys.modules[name] = module + modules["harbor.agents.installed.base"].BaseInstalledAgent = ( + FakeBaseInstalledAgent + ) + modules["harbor.agents.installed.base"].NonZeroAgentExitCodeError = ( + FakeNonZeroExit + ) + modules["harbor.agents.installed.base"].NetworkConnectionError = ( + FakeNetworkConnectionError + ) + modules["harbor.agents.installed.base"].UnknownApiError = FakeUnknownApiError + modules["harbor.agents.installed.base"].with_prompt_template = ( + _identity_decorator + ) + modules["harbor.environments.base"].BaseEnvironment = object + modules["harbor.models.agent.context"].AgentContext = FakeAgentContext + modules["harbor.models.task.config"].TaskConfig = FakeTaskConfig + modules["harbor.models.trial.config"].TrialConfig = FakeTrialConfig + modules["harbor.models.trial.paths"].EnvironmentPaths = SimpleNamespace( + agent_dir=PurePosixPath("/logs/agent") + ) + + +_install_harbor_stubs() +from integrations.harbor import agent # noqa: E402 + + +class FakeEnvironment: + def __init__( + self, + *, + runner_return_code: int = 0, + cleanup_return_code: int = 0, + ) -> None: + self.runner_return_code = runner_return_code + self.cleanup_return_code = cleanup_return_code + self.commands: list[tuple[str, int | None]] = [] + self.uploads: list[tuple[str, bytes, int]] = [] + self.remote_files: dict[str, bytes] = {} + self.runner_error: BaseException | None = None + self.cleanup_error: BaseException | None = None + self.cancel_cleanup_error: BaseException | None = None + self._metadata = Path(tempfile.mkdtemp(prefix="s-code-harbor-fake-metadata.")) + metadata = self._metadata + self.environment_dir = metadata / "task" / "environment" + self.environment_dir.mkdir(parents=True) + (self.environment_dir.parent / "task.toml").write_text( + "[agent]\ntimeout_sec = 3600.0\n", + encoding="utf-8", + ) + trial_dir = metadata / "trial" + trial_dir.mkdir() + config_path = trial_dir / "config.json" + config_path.write_text( + json.dumps( + { + "timeout_multiplier": 1.0, + "agent_timeout_multiplier": None, + "agent": { + "override_timeout_sec": None, + "max_timeout_sec": None, + }, + } + ), + encoding="utf-8", + ) + self.trial_paths = SimpleNamespace(config_path=config_path) + + def __del__(self) -> None: + shutil.rmtree(self._metadata, ignore_errors=True) + + def configure_deadline( + self, + *, + task_timeout: float = 3_600.0, + override_timeout: float | None = None, + max_timeout: float | None = None, + timeout_multiplier: float = 1.0, + agent_timeout_multiplier: float | None = None, + ) -> None: + (self.environment_dir.parent / "task.toml").write_text( + f"[agent]\ntimeout_sec = {task_timeout}\n", + encoding="utf-8", + ) + self.trial_paths.config_path.write_text( + json.dumps( + { + "timeout_multiplier": timeout_multiplier, + "agent_timeout_multiplier": agent_timeout_multiplier, + "agent": { + "override_timeout_sec": override_timeout, + "max_timeout_sec": max_timeout, + }, + } + ), + encoding="utf-8", + ) + + async def exec( + self, + *, + command: str, + timeout_sec: int | None = None, + ) -> FakeProcess: + self.commands.append((command, timeout_sec)) + if command == "command -v id >/dev/null && id -u && id -g": + return FakeProcess(stdout="1234\n5678\n") + if command == agent._RUNNER_PATH: + if self.runner_error is not None: + raise self.runner_error + return FakeProcess(return_code=self.runner_return_code) + if command.startswith("status=0; "): + if self.cancel_cleanup_error is not None: + raise self.cancel_cleanup_error + return FakeProcess() + if command == f"{agent._INSTALL_DIR}/s-code stop": + if self.cleanup_error is not None: + raise self.cleanup_error + return FakeProcess(return_code=self.cleanup_return_code) + return FakeProcess() + + async def upload_file(self, source: Path, target: str) -> None: + source_path = Path(source) + contents = source_path.read_bytes() + mode = source_path.stat().st_mode & 0o777 + self.uploads.append((target, contents, mode)) + self.remote_files[target] = contents + + async def download_file(self, source: str, target: Path) -> None: + if source not in self.remote_files: + raise FileNotFoundError(source) + Path(target).write_bytes(self.remote_files[source]) + + +def _manifest(archive_sha256: str) -> SimpleNamespace: + files = { + name: SimpleNamespace(sha256=(str(index) * 64), size=index, mode=0o755) + for index, name in enumerate(agent._EXECUTABLE_MEMBERS, start=1) + } + return SimpleNamespace( + schema_version=1, + bundle_format="s-code-harbor-bundle", + source_commit="a" * 40, + source_tree="b" * 40, + source_dirty=False, + target="x86_64-unknown-linux-gnu", + glibc_version="glibc 2.36", + lock_sha256s={ + "Cargo.lock": "c" * 64, + "web/package-lock.json": "d" * 64, + }, + tool_versions={ + "cargo": "cargo 1.89.0", + "node": "v22.23.2", + "npm": "10.9.2", + "python": "3.12.0", + "rustc": "rustc 1.89.0", + "zlib": "1.3.1", + }, + files=files, + archive_sha256=archive_sha256, + ) + + +class AdapterTests(unittest.TestCase): + def setUp(self) -> None: + temporary = tempfile.TemporaryDirectory(prefix="s-code-harbor-adapter.") + self.addCleanup(temporary.cleanup) + self.directory = Path(temporary.name) + self.bundle = self.directory / "runtime.tar.gz" + self.bundle.write_bytes(b"authenticated archive fixture\n") + self.bundle_sha256 = "e" * 64 + self.manifest = _manifest(self.bundle_sha256) + + version_patcher = mock.patch.object( + agent, "distribution_version", return_value="0.17.0" + ) + validate_patcher = mock.patch.object( + agent, "validate_bundle", return_value=self.manifest + ) + self.version = version_patcher.start() + self.validate = validate_patcher.start() + self.addCleanup(version_patcher.stop) + self.addCleanup(validate_patcher.stop) + + def new_agent(self, **overrides: object) -> agent.SCodeAgent: + arguments: dict[str, object] = { + "logs_dir": self.directory / "logs", + "model_name": "fixture-model", + "bundle_path": self.bundle, + "bundle_sha256": self.bundle_sha256, + "model_base_url": "https://models.example/v1", + "credential_env": "S_CODE_HARBOR_API_KEY", + "extra_env": {"S_CODE_HARBOR_API_KEY": "top-secret-fixture"}, + } + arguments.update(overrides) + return agent.SCodeAgent(**arguments) + + def write_events(self, *events: object) -> Path: + path = self.directory / "events.jsonl" + path.write_text( + "".join(json.dumps(event) + "\n" for event in events), + encoding="utf-8", + ) + return path + + def test_exact_harbor_version_is_required(self) -> None: + self.new_agent() + self.version.assert_called_with("harbor") + + self.version.return_value = "0.17.1" + with self.assertRaisesRegex(RuntimeError, r"harbor==0\.17\.0; found 0\.17\.1"): + self.new_agent() + + def test_integer_configuration_bounds_and_types(self) -> None: + invalid_values = ( + ("max_model_calls", 0), + ("max_model_calls", 10_001), + ("max_tool_calls", 0), + ("max_tool_calls", 10_001), + ("model_stream_idle_seconds", 0), + ("model_stream_idle_seconds", 3_601), + ("model_connect_timeout_sec", 0), + ("model_connect_timeout_sec", 3_601), + ("model_read_timeout_sec", 0), + ("model_read_timeout_sec", 3_601), + ("model_request_timeout_sec", 0), + ("model_request_timeout_sec", 3_601), + ("max_cost_micros", 0), + ("max_cost_micros", 1_000_000_000_001), + ("max_total_tokens", 0), + ("max_total_tokens", 1_000_000_001), + ) + for name, value in invalid_values: + with self.subTest(name=name, value=value): + with self.assertRaises(ValueError): + self.new_agent(**{name: value}) + + for name in ( + "max_model_calls", + "max_tool_calls", + "model_stream_idle_seconds", + "model_connect_timeout_sec", + "model_read_timeout_sec", + "model_request_timeout_sec", + "max_cost_micros", + "max_total_tokens", + ): + with self.subTest(name=name, value=True): + with self.assertRaises(TypeError): + self.new_agent(**{name: True}) + + bounded = self.new_agent( + max_model_calls=10_000, + max_tool_calls=10_000, + model_stream_idle_seconds=3_600, + model_connect_timeout_sec=3_600, + model_read_timeout_sec=3_600, + model_request_timeout_sec=3_600, + max_cost_micros=1_000_000_000_000, + max_total_tokens=1_000_000_000, + ) + self.assertEqual(bounded._model_connect_timeout_sec, 3_450) + self.assertEqual(bounded._core_timeout_sec, 3_480) + self.assertEqual(bounded._cli_timeout_sec, 3_510) + + def test_url_and_credential_validation(self) -> None: + for url in ( + "https://models.example/v1", + "http://localhost:8000/v1", + "http://127.0.0.1:8000/v1", + "http://[::1]:8000/v1", + ): + with self.subTest(valid=url): + self.assertEqual(agent._model_base_url(url), url) + + for url in ( + "http://models.example/v1", + "https://user:pass@models.example/v1", + "https://models.example/v1?secret=yes", + "https://models.example/v1#fragment", + "ftp://models.example/v1", + " https://models.example/v1", + "https://models.example:bad/v1", + ): + with self.subTest(invalid=url): + with self.assertRaises(ValueError): + agent._model_base_url(url) + + for name in ( + "API_KEY", + "MODEL_SECRET", + "ACCESS_TOKEN", + "MODEL_PASSWORD", + "MODEL_CREDENTIAL", + "MODEL_AUTH", + ): + with self.subTest(valid=name): + self.assertEqual(agent._credential_env(name), name) + + for name in ( + "MODEL_API", + "model_secret", + "MODEL_secret", + "9_API_KEY", + "API-KEY", + "", + None, + ): + with self.subTest(invalid=name): + with self.assertRaises(ValueError): + agent._credential_env(name) + + with self.assertRaisesRegex(ValueError, "non-empty value"): + self.new_agent(extra_env={"S_CODE_HARBOR_API_KEY": ""}) + + def test_constructor_manages_environment_and_timeout_hierarchy(self) -> None: + instance = self.new_agent(extra_fixture="passed-through") + initialized = instance.base_init + self.assertEqual(initialized["model_name"], "fixture-model") + self.assertEqual(initialized["version"], "0.1.0") + self.assertEqual(initialized["kwargs"], {"extra_fixture": "passed-through"}) + environment = initialized["extra_env"] + self.assertEqual(environment["S_CODE_HARBOR_API_KEY"], "top-secret-fixture") + self.assertEqual(environment["S_CODE_TURN_MAX_MODEL_CALLS"], "100") + self.assertEqual(environment["S_CODE_TURN_MAX_TOOL_CALLS"], "256") + self.assertEqual(environment["S_CODE_TURN_MAX_ELAPSED_SECONDS"], "3480") + self.assertEqual(environment["S_CODE_HARBOR_CLI_TIMEOUT_SEC"], "3510") + self.assertEqual(environment["S_CODE_TURN_MAX_TOTAL_TOKENS"], "10000000") + self.assertEqual(environment["S_CODE_MODEL_CONNECT_TIMEOUT_SECONDS"], "10") + self.assertEqual(environment["S_CODE_MODEL_READ_TIMEOUT_SECONDS"], "600") + self.assertEqual(environment["S_CODE_MODEL_REQUEST_TIMEOUT_SECONDS"], "3450") + self.assertEqual(environment["S_CODE_MODEL_MERGE_SYSTEM_MESSAGES"], "true") + self.assertEqual( + environment["S_CODE_TURN_DISABLED_TOOLS"], + "web_open,pdf_read,pdf_view,request_user_input", + ) + self.assertEqual(instance._harbor_timeout_sec, 3600) + self.assertEqual(instance._core_timeout_sec, 3480) + self.assertEqual(instance._cli_timeout_sec, 3510) + + with self.assertRaisesRegex(ValueError, "managed by SCodeAgent"): + self.new_agent( + extra_env={ + "S_CODE_HARBOR_API_KEY": "top-secret-fixture", + "S_CODE_TURN_MAX_MODEL_CALLS": "999", + } + ) + + def test_harbor_deadline_resolution_matches_cap_then_multiplier(self) -> None: + environment = FakeEnvironment() + environment.configure_deadline( + task_timeout=900, + override_timeout=800, + max_timeout=700, + timeout_multiplier=3, + agent_timeout_multiplier=2, + ) + self.assertEqual( + agent._resolved_harbor_agent_timeout_sec(environment), + 1_400, + ) + + environment.configure_deadline( + task_timeout=900, + override_timeout=800, + max_timeout=700, + timeout_multiplier=3, + ) + self.assertEqual( + agent._resolved_harbor_agent_timeout_sec(environment), + 2_100, + ) + + environment.configure_deadline(task_timeout=179) + with self.assertRaisesRegex(ValueError, "between 180 and 86400"): + agent._resolved_harbor_agent_timeout_sec(environment) + + environment.trial_paths.config_path.write_text("not json", encoding="utf-8") + with self.assertRaisesRegex(RuntimeError, "could not resolve"): + agent._resolved_harbor_agent_timeout_sec(environment) + + def test_install_uses_one_archive_and_numeric_nonroot_owner(self) -> None: + instance = self.new_agent() + environment = FakeEnvironment() + environment.configure_deadline(task_timeout=600) + asyncio.run(instance.install(environment)) + + self.assertEqual(instance._agent_owner, "1234:5678") + self.assertEqual(instance._harbor_timeout_sec, 600) + self.assertEqual(instance._model_connect_timeout_sec, 10) + self.assertEqual(instance._model_read_timeout_sec, 450) + self.assertEqual(instance._model_request_timeout_sec, 450) + self.assertEqual(instance._model_stream_idle_seconds, 300) + self.assertEqual(instance._core_timeout_sec, 480) + self.assertEqual(instance._cli_timeout_sec, 510) + self.assertEqual( + instance._extra_env["S_CODE_TURN_MAX_ELAPSED_SECONDS"], "480" + ) + self.assertEqual( + instance._extra_env["S_CODE_MODEL_REQUEST_TIMEOUT_SECONDS"], "450" + ) + self.assertEqual( + instance._provenance()["execution"]["output_mode"], + "raw_events_jsonl", + ) + self.assertEqual(self.validate.call_count, 2) + targets = [target for target, _, _ in environment.uploads] + self.assertEqual(targets, [agent._BUNDLE_ARCHIVE_PATH, agent._RUNNER_PATH]) + archive_upload = environment.uploads[0] + self.assertEqual(archive_upload[1], self.bundle.read_bytes()) + runner_upload = environment.uploads[1] + self.assertEqual(runner_upload[1], agent._RUNNER.encode("utf-8")) + self.assertEqual(runner_upload[2], 0o755) + + commands = "\n".join(command for command, _ in environment.commands) + self.assertIn("getconf GNU_LIBC_VERSION", commands) + self.assertIn(self.bundle_sha256, commands) + self.assertIn("sha256sum -c -", commands) + self.assertIn( + f"tar -xzf {agent._BUNDLE_ARCHIVE_PATH} -C {agent._INSTALL_DIR}", + commands, + ) + self.assertIn(f"chown -- 1234:5678 {agent._PRIVATE_DIR}", commands) + self.assertIn("s-code --help", commands) + for executable in ("s-code-cli", "s-code-daemon", "s-code-pdf-worker"): + self.assertIn(f"{executable} --self-test", commands) + + secret = b"top-secret-fixture" + self.assertNotIn(secret.decode(), commands) + for _, contents, _ in environment.uploads: + self.assertNotIn(secret, contents) + + def _run_generated_runner( + self, + *, + exec_status: int, + cleanup_status: int, + prompt: bytes, + ) -> tuple[subprocess.CompletedProcess[bytes], Path, Path, Path]: + case_directory = Path( + tempfile.mkdtemp(prefix="runner-case.", dir=self.directory) + ) + logs = case_directory / "logs" + private = case_directory / "private" + logs.mkdir() + private.mkdir() + fake_s_code = case_directory / "fake-s-code" + calls = case_directory / "calls" + captured_prompt = case_directory / "captured-prompt" + captured_stdin = case_directory / "captured-stdin" + fake_s_code.write_text( + """#!/bin/sh +command=$1 +shift +case "$command" in + exec) + printf 'exec\\n' >> "$FAKE_CALLS" + cat > "$FAKE_CAPTURED_STDIN" + [ ! -s "$FAKE_CAPTURED_STDIN" ] || exit 98 + last= + for argument do last=$argument; done + printf '%s' "$last" > "$FAKE_CAPTURED_PROMPT" + printf '%s\\n' '{"kind":"turn.completed","payload":{}}' + printf '%s\\n' 'fixture stderr' >&2 + mkdir -p "$FAKE_STATE_DIR/logs" + printf '%s\\n' 'fixture daemon log' > "$FAKE_STATE_DIR/logs/daemon.log" + exit "$FAKE_EXEC_STATUS" + ;; + stop) + printf 'stop\\n' >> "$FAKE_CALLS" + exit "$FAKE_CLEANUP_STATUS" + ;; + *) exit 99 ;; +esac +""", + encoding="utf-8", + ) + fake_s_code.chmod(0o755) + replacements = { + f"{agent._INSTALL_DIR}/s-code": str(fake_s_code), + agent._EVENTS_PATH: str(logs / "events.jsonl"), + agent._STDERR_PATH: str(logs / "stderr.log"), + agent._FINAL_PATH: str(logs / "final.txt"), + agent._PROVENANCE_PATH: str(logs / "provenance.json"), + agent._DAEMON_LOG_PATH: str(logs / "daemon.log"), + agent._PRIVATE_DIR: str(private), + } + rendered = agent._RUNNER + for original in sorted(replacements, key=len, reverse=True): + rendered = rendered.replace(original, replacements[original]) + runner = case_directory / "runner" + runner.write_text(rendered, encoding="utf-8") + runner.chmod(0o755) + (private / "instruction.txt").write_bytes(prompt) + (private / "provenance.seed.json").write_text( + '{"daemon_cleanup_exit_code": null, "exit_code": null, ' + '"runner_exit_code": null}\n', + encoding="utf-8", + ) + + child_env = dict(os.environ) + child_env.update( + { + "FAKE_CALLS": str(calls), + "FAKE_CAPTURED_PROMPT": str(captured_prompt), + "FAKE_CAPTURED_STDIN": str(captured_stdin), + "FAKE_EXEC_STATUS": str(exec_status), + "FAKE_CLEANUP_STATUS": str(cleanup_status), + "FAKE_STATE_DIR": str(private / "state"), + "S_CODE_HARBOR_CLI_TIMEOUT_SEC": "3570", + } + ) + result = subprocess.run( + ["/bin/sh", str(runner)], + env=child_env, + input=b"parent stdin must not reach noninteractive S-Code", + check=False, + capture_output=True, + ) + return result, private / "exit.status", calls, captured_prompt + + def test_generated_runner_syntax_and_prompt_data_handling(self) -> None: + self.assertIn(" --raw-events-jsonl ", agent._RUNNER) + self.assertNotIn(" --stream-json ", agent._RUNNER) + syntax = subprocess.run( + ["/bin/sh", "-n"], + input=agent._RUNNER.encode("utf-8"), + check=False, + capture_output=True, + ) + self.assertEqual(syntax.returncode, 0, syntax.stderr.decode()) + + injection_marker = self.directory / "prompt-was-executed" + prompt = f"$(touch {injection_marker}) ; `false`\n\n".encode() + result, exit_status, calls, captured_prompt = self._run_generated_runner( + exec_status=0, + cleanup_status=7, + prompt=prompt, + ) + self.assertEqual(result.returncode, 0, result.stderr.decode()) + self.assertEqual(exit_status.read_text(encoding="ascii"), "0 7 0\n") + self.assertEqual(calls.read_text(encoding="ascii"), "exec\nstop\n") + self.assertEqual(captured_prompt.read_bytes(), prompt) + self.assertFalse(injection_marker.exists()) + daemon_log = exit_status.parent.parent / "logs" / "daemon.log" + self.assertEqual(daemon_log.read_text(encoding="utf-8"), "fixture daemon log\n") + self.assertEqual(daemon_log.stat().st_mode & 0o777, 0o600) + + result, exit_status, calls, captured_prompt = self._run_generated_runner( + exec_status=42, + cleanup_status=7, + prompt=b"second fixture\n", + ) + self.assertEqual(result.returncode, 42) + self.assertEqual(exit_status.read_text(encoding="ascii"), "42 7 42\n") + self.assertEqual(calls.read_text(encoding="ascii"), "exec\nstop\n") + self.assertEqual(captured_prompt.read_bytes(), b"second fixture\n") + + def test_event_stream_classifications(self) -> None: + completed = agent._classify_event_stream( + self.write_events( + { + "kind": "turn.usage", + "payload": { + "input_units": 12, + "output_units": 7, + "model_calls": 2, + "tool_calls": 3, + }, + }, + {"kind": "turn.completed", "sequence": 9, "payload": {}}, + ), + 0, + ) + self.assertEqual(completed["status"], "completed") + self.assertEqual( + completed["usage"], + {"input_tokens": 12, "output_tokens": 7, "model_calls": 2, "tool_calls": 3}, + ) + self.assertEqual(completed["event_stream"]["validation"], "valid") + + cases = ( + ( + {"kind": "question.required", "payload": {"source": "tool_call_limit"}}, + 1, + "limit_stopped", + "tool_call_limit", + None, + ), + ( + {"kind": "turn.failed", "payload": {"reason": "model-call limit reached"}}, + 1, + "limit_stopped", + "model_call_limit", + None, + ), + ( + {"kind": "turn.failed", "error_code": "model_stream_idle_timeout"}, + 1, + "timed_out", + "model_stream_idle_timeout", + None, + ), + ( + {"kind": "turn.failed", "payload": {"error_code": "turn_elapsed_timeout"}}, + 1, + "timed_out", + "turn_elapsed_timeout", + None, + ), + ( + {"kind": "turn.cancelled", "payload": {}}, + 130, + "cancelled", + None, + None, + ), + ( + {"kind": "question.required", "payload": {"source": "model"}}, + 1, + "failed", + None, + "input_required", + ), + ( + {"kind": "approval.required", "payload": {}}, + 1, + "failed", + None, + "approval_required", + ), + ) + for event, exit_code, status, limit_kind, failure_kind in cases: + with self.subTest(kind=event["kind"], status=status, limit_kind=limit_kind): + result = agent._classify_event_stream( + self.write_events(event), exit_code + ) + self.assertEqual(result["status"], status) + self.assertEqual(result["limit_kind"], limit_kind) + self.assertEqual(result["failure_kind"], failure_kind) + self.assertEqual(result["event_stream"]["validation"], "valid") + + malformed = self.directory / "malformed.jsonl" + malformed.write_bytes(b"not json\n") + result = agent._classify_event_stream(malformed, 0) + self.assertEqual(result["status"], "failed") + self.assertEqual(result["event_stream"]["validation"], "invalid") + self.assertEqual(result["event_stream"]["records"], 0) + self.assertNotIn("not json", result["event_stream"]["error"]) + + def test_runs_without_a_model_response_name_the_endpoint(self) -> None: + def finished(status: str) -> dict[str, object]: + return { + "kind": "privacy.request.finished", + "payload": {"request": {"status": status}}, + } + + failed = {"kind": "turn.failed", "payload": {"error_code": "agent_error"}} + cases = ( + (("connection_error",) * 3, "model_unreachable"), + (("rejected",), "model_rejected"), + (("connection_error", "rejected"), "model_unreachable"), + (("connection_error", "accepted"), None), + ) + for outcomes, failure_kind in cases: + with self.subTest(outcomes=outcomes): + result = agent._classify_event_stream( + self.write_events(*(finished(status) for status in outcomes), failed), + 1, + ) + self.assertEqual(result["status"], "failed") + self.assertEqual(result["failure_kind"], failure_kind) + self.assertEqual( + sum(result["model_requests"].values()), len(outcomes) + ) + + completed = agent._classify_event_stream( + self.write_events( + finished("connection_error"), + {"kind": "turn.completed", "payload": {}}, + ), + 0, + ) + self.assertEqual(completed["status"], "completed") + self.assertIsNone(completed["failure_kind"]) + + def _run_environment( + self, + *, + runner_return_code: int, + cleanup_return_code: int, + status: bytes, + events: bytes, + ) -> FakeEnvironment: + environment = FakeEnvironment( + runner_return_code=runner_return_code, + cleanup_return_code=cleanup_return_code, + ) + environment.remote_files[agent._EXIT_STATUS_PATH] = status + environment.remote_files[agent._EVENTS_PATH] = events + return environment + + def test_post_cleanup_failure_precedes_runner_nonzero(self) -> None: + instance = self.new_agent() + instance._agent_owner = "1234:5678" + environment = self._run_environment( + runner_return_code=42, + cleanup_return_code=7, + status=b"42 7 42\n", + events=b'{"kind":"turn.failed","payload":{"reason":"fixture"}}\n', + ) + context = FakeAgentContext() + with self.assertRaisesRegex(RuntimeError, "task state is not quiescent") as raised: + asyncio.run(instance.run("one task", environment, context)) + self.assertIs(type(raised.exception), FakeNonZeroExit) + self.assertEqual( + [command for command, _ in environment.commands][-2:], + [agent._RUNNER_PATH, f"{agent._INSTALL_DIR}/s-code stop"], + ) + self.assertIsNone(environment.commands[-2][1]) + self.assertEqual(environment.commands[-1][1], 10) + + def test_harbor_cancellation_is_preserved_after_bounded_cleanup(self) -> None: + instance = self.new_agent() + instance._agent_owner = "1234:5678" + environment = self._run_environment( + runner_return_code=0, + cleanup_return_code=0, + status=b"", + events=b"", + ) + cancellation = asyncio.CancelledError("Harbor deadline") + environment.runner_error = cancellation + environment.cancel_cleanup_error = RuntimeError("cleanup transport failed") + with self.assertRaises(asyncio.CancelledError) as raised: + asyncio.run(instance.run("one task", environment, FakeAgentContext())) + self.assertIs(raised.exception, cancellation) + self.assertTrue(environment.commands[-1][0].startswith("status=0; ")) + self.assertEqual(environment.commands[-1][1], 10) + self.assertNotIn(agent._PROVENANCE_PATH, environment.remote_files) + + def test_cancellation_during_post_run_cleanup_is_preserved(self) -> None: + instance = self.new_agent() + instance._agent_owner = "1234:5678" + environment = self._run_environment( + runner_return_code=0, + cleanup_return_code=0, + status=b"0 0 0\n", + events=b'{"kind":"turn.completed","payload":{}}\n', + ) + cancellation = asyncio.CancelledError("Harbor deadline") + environment.cleanup_error = cancellation + with self.assertRaises(asyncio.CancelledError) as raised: + asyncio.run(instance.run("one task", environment, FakeAgentContext())) + self.assertIs(raised.exception, cancellation) + self.assertTrue(environment.commands[-1][0].startswith("status=0; ")) + + def test_success_with_invalid_event_evidence_fails_closed(self) -> None: + instance = self.new_agent() + instance._agent_owner = "1234:5678" + environment = self._run_environment( + runner_return_code=0, + cleanup_return_code=0, + status=b"0 0 0\n", + events=b"malformed event\n", + ) + context = FakeAgentContext() + with self.assertRaisesRegex( + FakeNonZeroExit, "without valid completed-run evidence" + ): + asyncio.run(instance.run("one task", environment, context)) + provenance = json.loads(environment.remote_files[agent._PROVENANCE_PATH]) + self.assertEqual(provenance["result"]["event_stream"]["validation"], "invalid") + self.assertEqual(provenance["execution"]["post_run_cleanup_status"], "succeeded") + + def test_endpoint_failures_use_harbor_retryable_error_types(self) -> None: + cases = ( + ("connection_error", FakeNetworkConnectionError, "model_unreachable"), + ("rejected", FakeUnknownApiError, "model_rejected"), + ) + for request_status, expected_error, failure_kind in cases: + with self.subTest(request_status=request_status): + instance = self.new_agent() + instance._agent_owner = "1234:5678" + environment = self._run_environment( + runner_return_code=1, + cleanup_return_code=0, + status=b"1 0 1\n", + events=( + json.dumps( + { + "kind": "privacy.request.finished", + "payload": {"request": {"status": request_status}}, + } + ).encode() + + b"\n" + + b'{"kind":"turn.failed","payload":{}}\n' + ), + ) + with self.assertRaises(expected_error): + asyncio.run( + instance.run("one task", environment, FakeAgentContext()) + ) + provenance = json.loads( + environment.remote_files[agent._PROVENANCE_PATH] + ) + self.assertEqual(provenance["result"]["failure_kind"], failure_kind) + self.assertEqual( + provenance["execution"]["post_run_cleanup_status"], "succeeded" + ) + + def test_untrusted_endpoint_evidence_is_not_promoted(self) -> None: + instance = self.new_agent() + instance._agent_owner = "1234:5678" + environment = self._run_environment( + runner_return_code=1, + cleanup_return_code=0, + status=b"malformed\n", + events=( + b'{"kind":"privacy.request.finished","payload":' + b'{"request":{"status":"connection_error"}}}\n' + b'{"kind":"turn.failed","payload":{}}\n' + ), + ) + with self.assertRaises(FakeNonZeroExit) as raised: + asyncio.run(instance.run("one task", environment, FakeAgentContext())) + self.assertIs(type(raised.exception), FakeNonZeroExit) + + def test_finalization_failure_remains_a_harbor_agent_failure(self) -> None: + instance = self.new_agent() + instance._agent_owner = "1234:5678" + environment = self._run_environment( + runner_return_code=0, + cleanup_return_code=0, + status=b"0 0 0\n", + events=b'{"kind":"turn.completed","payload":{}}\n', + ) + instance._finalize_provenance = mock.AsyncMock( + side_effect=OSError("fixture finalization failure") + ) + with self.assertRaisesRegex( + FakeNonZeroExit, "could not validate and preserve" + ): + asyncio.run(instance.run("one task", environment, FakeAgentContext())) + + def test_successful_run_is_one_shot_and_records_usage(self) -> None: + instance = self.new_agent() + instance._agent_owner = "1234:5678" + environment = self._run_environment( + runner_return_code=0, + cleanup_return_code=0, + status=b"0 0 0\n", + events=( + b'{"kind":"turn.usage","payload":{"input_tokens":8,' + b'"output_tokens":5}}\n' + b'{"kind":"turn.completed","payload":{}}\n' + ), + ) + context = FakeAgentContext() + asyncio.run(instance.run("one task\n", environment, context)) + self.assertEqual(context.n_input_tokens, 8) + self.assertEqual(context.n_output_tokens, 5) + self.assertEqual(context.metadata["s_code"]["result_status"], "completed") + self.assertEqual(environment.remote_files[agent._PROMPT_PATH], b"one task\n") + + with self.assertRaisesRegex(RuntimeError, "exactly one run"): + asyncio.run(instance.run("second task", environment, context)) + + +if __name__ == "__main__": + unittest.main() From 0f982120afc59770945ecf7c64a59f43dba4ed25 Mon Sep 17 00:00:00 2001 From: Wei Deng Date: Sun, 18 Oct 2026 16:11:10 -0700 Subject: [PATCH 4/9] docs(harbor): document the adapter and run its tests in CI Add integrations/harbor/README.md: requirements, building the bundle, running a Harbor job, the agent kwargs and the files kept per trial. It also explains that the task container must reach the model endpoint itself. Route integrations/harbor and its tests to the benchmarks CI job, and run the adapter and bundle tests there. Refs #129 Signed-off-by: Wei Deng --- .github/workflows/ci.yml | 2 + integrations/harbor/README.md | 123 ++++++++++++++++++++++++++++++++++ scripts/ci-plan.py | 2 +- tests/test-ci-plan.py | 11 +++ 4 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 integrations/harbor/README.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3bb3f3f..19788ff 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -342,6 +342,8 @@ jobs: - run: npm ci --prefix tests/benchmarks/runner --no-audit --no-fund - run: python3 tests/test-harness-benchmark.py validate - run: tests/test-harness-grader-integrity.sh + - run: python3 tests/test-harbor-adapter.py + - run: python3 tests/test-harbor-bundle.py - if: needs.checks.outputs.audit == 'true' run: npm audit --prefix tests/benchmarks/runner --audit-level=high diff --git a/integrations/harbor/README.md b/integrations/harbor/README.md new file mode 100644 index 0000000..fdf33c3 --- /dev/null +++ b/integrations/harbor/README.md @@ -0,0 +1,123 @@ +# S-Code adapter for Harbor + +This adapter runs one noninteractive S-Code attempt in a Harbor-managed task +container. It targets Harbor 0.17.0 so benchmark runs use a known custom-agent +API and a checksum-verified S-Code runtime. + +## Requirements + +- Python 3.12 or newer with `harbor==0.17.0` from `requirements.txt`. +- An x86-64 glibc Linux build host and task container. +- A clean S-Code checkout with the pinned Rust 1.89.0 and Node 22.23.2 tools. +- An OpenAI-compatible model endpoint reachable from the task container. + +The Linux restriction applies to this Harbor bundle, not to S-Code generally. + +Install the pinned Harbor version in a dedicated environment. Keep it under the +git-ignored `.work/` directory so the checkout stays clean for the bundle build: + +```sh +python3.12 -m venv .work/harbor-venv +. .work/harbor-venv/bin/activate +python -m pip install -r integrations/harbor/requirements.txt +``` + +## Build the runtime bundle + +From the repository root on x86-64 glibc Linux, run: + +```sh +integrations/harbor/build-bundle.sh +``` + +The builder refuses a dirty checkout, builds from the checked-out commit, runs +the runtime self-tests, and writes these files under `.work/harbor`: + +```text +s-code--x86_64-unknown-linux-gnu.tar.gz +s-code--x86_64-unknown-linux-gnu.tar.gz.sha256 +``` + +Set `S_CODE_HARBOR_OUTPUT_DIR` to choose another output directory and +`CARGO_TARGET_DIR` to reuse a Cargo build directory. The archive contains only +`manifest.json`, `s-code`, `s-code-cli`, `s-code-daemon`, and +`s-code-pdf-worker`. + +## Run a Harbor task + +Run Harbor from the repository root with the repository on `PYTHONPATH`, so +Harbor can import the adapter, and replace the placeholders below. Write job +output under `.work/` too, so the checkout stays clean. + +S-Code calls the model from inside the task container, so the container itself +must reach the endpoint over HTTPS. `--allow-agent-host` adds the endpoint to +the agent's allowlist only for tasks with a restricted network policy; Harbor +ignores it for public tasks such as TBLite. Daytona Tier 1 and Tier 2 sandboxes +can reach only Daytona's essential-services list whatever the task allows, so a +self-hosted endpoint needs Harbor's local `docker` environment, Daytona Tier 3 +or higher, or another provider with open outbound access. + +```sh +export S_CODE_HARBOR_CREDENTIAL='' +export S_CODE_HARBOR_BUNDLE='.work/harbor/s-code--x86_64-unknown-linux-gnu.tar.gz' +export S_CODE_HARBOR_BUNDLE_SHA256='' +export PYTHONPATH="$PWD${PYTHONPATH:+:$PYTHONPATH}" + +harbor run \ + --dataset '' \ + --agent integrations.harbor.agent:SCodeAgent \ + --model '' \ + --agent-kwarg "bundle_path=${S_CODE_HARBOR_BUNDLE}" \ + --agent-kwarg "bundle_sha256=${S_CODE_HARBOR_BUNDLE_SHA256}" \ + --agent-kwarg 'model_base_url=https://models.example/v1' \ + --agent-kwarg 'credential_env=S_CODE_HARBOR_CREDENTIAL' \ + --agent-env "S_CODE_HARBOR_CREDENTIAL=${S_CODE_HARBOR_CREDENTIAL}" \ + --allow-agent-host models.example \ + --jobs-dir .work/harbor-jobs +``` + +`bundle_path`, `bundle_sha256`, `model_base_url`, `credential_env`, and the +Harbor `--model` value are required. The credential variable must have a +nonempty value, including when the endpoint does not authenticate. The adapter +passes the model name through to S-Code and records only the credential handle, +not its value. Optional agent kwargs: + +| Kwarg | Default | Meaning | +| --- | --- | --- | +| `max_model_calls` | 100 | Model calls allowed in the turn | +| `max_tool_calls` | 256 | Tool calls allowed in the turn | +| `model_stream_idle_seconds` | 300 | Longest gap between model stream events | +| `model_connect_timeout_sec` | 10 | Time allowed to open one model connection | +| `model_read_timeout_sec` | 600 | Longest gap between bytes of one model response | +| `model_request_timeout_sec` | 3600 | Time allowed for one model request | + +The adapter reads Harbor's resolved per-task agent deadline and nests the model +request, S-Code turn, CLI, cleanup, and Harbor limits inside it. Requested model +timeouts are automatically clamped for shorter tasks. + +The adapter validates the archive and its manifest locally, uploads that exact +archive, checks its SHA-256 inside the task container, extracts it, runs each +binary's self-test, and uses fresh S-Code state for the trial. The adapter +selects the additive `--raw-events-jsonl` output mode; existing `--jsonl` and +`--stream-json` output contracts are unchanged. It preserves these files in +Harbor's agent logs: + +- `events.jsonl`: every raw S-Code `LiveEvent`, as versioned JSON Lines. +- `stderr.log`: S-Code diagnostic output. +- `final.txt`: the final agent message. +- `provenance.json`: adapter, bundle, model, endpoint, and trial metadata. +- `daemon.log`: the S-Code daemon's log, including why model requests failed. + +If no model request in the run gets a successful response, provenance records +`model_unreachable` or `model_rejected`. The adapter raises Harbor's +`NetworkConnectionError` for transport failures and `UnknownApiError` for HTTP +rejections, allowing Harbor to record the infrastructure failure and still run +the verifier. + +S-Code runs in Full permission mode because the Harbor task container is the +outer filesystem and network boundary. Browser, PDF, and interactive-input tools +are disabled because they cannot complete reliably in an unattended trial. The +adapter also has S-Code send its system messages as one leading message, +because some chat templates, including Qwen3.6's, reject later ones. The +adapter performs one attempt and does not provide retries, resume, persistent +memory, or ATIF conversion. diff --git a/scripts/ci-plan.py b/scripts/ci-plan.py index cb2672d..b5a3e9e 100644 --- a/scripts/ci-plan.py +++ b/scripts/ci-plan.py @@ -54,7 +54,7 @@ def plan(paths: list[str], *, full: bool = False, public: bool = False) -> dict[ elif path.startswith("clients/jetbrains/"): if not path.endswith(".md"): flags["jetbrains"] = True - elif path.startswith("tests/benchmarks/") or path in ("tests/test-harness-benchmark.py", "tests/test-harness-grader-integrity.sh"): + elif path.startswith("tests/benchmarks/") or path.startswith("integrations/harbor/") or path in ("tests/test-harness-benchmark.py", "tests/test-harness-grader-integrity.sh", "tests/test-harbor-adapter.py", "tests/test-harbor-bundle.py"): flags["benchmarks"] = True elif path in ("s-code", "scripts/s-code", "scripts/install-from-source.sh", "scripts/source-dependencies.sh", "scripts/configure-shell-path.py", "tests/test-shell-path.py", "tests/test-source-dependencies.py", "tests/test-source-install.sh", "tests/test-source-install-real.sh", "tests/test-first-run.sh", "tests/wedged-s-code-daemon.py"): flags["install"] = flags["runtime"] = flags["rust"] = True diff --git a/tests/test-ci-plan.py b/tests/test-ci-plan.py index 995f601..0306bbb 100644 --- a/tests/test-ci-plan.py +++ b/tests/test-ci-plan.py @@ -46,6 +46,17 @@ def test_documentation_and_plugin_changes_are_isolated(self): p = ci.plan([path]) self.assertEqual({key for key in ci.JOBS if p[key]}, {expected}) + def test_harbor_adapter_changes_only_select_benchmarks(self): + for path in ( + "integrations/harbor/adapter.py", + "tests/test-harbor-adapter.py", + "tests/test-harbor-bundle.py", + ): + with self.subTest(path=path): + p = ci.plan([path]) + self.assertEqual({key for key in ci.AREAS if p[key]}, {"benchmarks"}) + self.assertEqual({key for key in ci.JOBS if p[key]}, {"benchmarks"}) + def test_runtime_changes_cover_both_supported_platforms(self): for path in ["crates/daemon/src/lib.rs", "crates/platform-runtime/src/lib.rs", "crates/storage/migrations/new.sql", "crates/model-gateway/src/lib.rs", "crates/agent-adapter/src/lib.rs", "crates/context-engine/src/lib.rs", "crates/new-runtime/src/lib.rs", "tests/model_fixture.py", "scripts/s-code"]: with self.subTest(path=path): From c0097f39c601c9f51f4af2ce8b34ead427131f2b Mon Sep 17 00:00:00 2001 From: Wei Deng Date: Sun, 18 Oct 2026 16:49:19 -0700 Subject: [PATCH 5/9] feat: make agent sampling configurable Benchmarks compare agents at fixed sampling settings. S-Code always samples at temperature 0. It starts with 8,192 output tokens per model call and doubles that after a cut-off response, up to 32,768. Add model.temperature (S_CODE_MODEL_TEMPERATURE, 0 to 2) and model.max_output_tokens (S_CODE_MODEL_MAX_OUTPUT_TOKENS, 1 to 1000000). When they are unset, behavior is unchanged. An explicit output cap stays fixed instead of doubling. The Harbor adapter passes both through as the temperature and max_output_tokens kwargs and records them in provenance. Refs #129 Signed-off-by: Wei Deng --- crates/agent-core/src/lib.rs | 77 +++++++++++++++++++- crates/config/src/lib.rs | 78 +++++++++++++++++++++ crates/daemon/src/lib.rs | 17 ++++- crates/daemon/src/main.rs | 1 + crates/evals/src/main.rs | 1 + docs-site/content/guides/model-endpoints.md | 6 ++ docs/guides/model-endpoints.md | 6 ++ integrations/harbor/README.md | 2 + integrations/harbor/agent.py | 30 ++++++++ tests/test-harbor-adapter.py | 25 +++++++ 10 files changed, 240 insertions(+), 3 deletions(-) diff --git a/crates/agent-core/src/lib.rs b/crates/agent-core/src/lib.rs index bcc013b..7111e43 100644 --- a/crates/agent-core/src/lib.rs +++ b/crates/agent-core/src/lib.rs @@ -138,6 +138,10 @@ pub struct AgentRunRequest { pub messages: Vec, pub tools: Vec, pub max_output_tokens: u32, + /// Keep `max_output_tokens` as a fixed cap instead of doubling it after a + /// cut-off response. + #[serde(default)] + pub fixed_max_output_tokens: bool, #[serde(default, skip_serializing_if = "Option::is_none")] pub routing: Option, /// Tool media for the lifetime of this in-memory run segment. It is @@ -944,7 +948,10 @@ impl AgentRunner { matches!(reason, "length" | "max_tokens" | "max_output_tokens") }) || call_output_tokens >= u64::from(request.max_output_tokens); - if exhausted_budget && request.max_output_tokens < MAX_ADAPTIVE_OUTPUT_TOKENS { + if exhausted_budget + && !request.fixed_max_output_tokens + && request.max_output_tokens < MAX_ADAPTIVE_OUTPUT_TOKENS + { request.max_output_tokens = request .max_output_tokens .saturating_mul(2) @@ -983,6 +990,7 @@ impl AgentRunner { }); } if exhausted_output_budget + && !request.fixed_max_output_tokens && request.max_output_tokens < MAX_ADAPTIVE_OUTPUT_TOKENS { request.max_output_tokens = request @@ -2816,6 +2824,7 @@ mod tests { parameters: json!({"type":"object"}), }], max_output_tokens: 100, + fixed_max_output_tokens: false, routing: None, transient_tool_media: BTreeMap::new(), } @@ -3852,6 +3861,72 @@ mod tests { assert_eq!(requests[1].max_output_tokens, 200); } + #[tokio::test] + async fn fixed_output_cap_does_not_grow_after_an_exhausted_budget() { + struct CapturingProvider { + requests: Arc>>, + responses: Mutex>>, + } + #[async_trait] + impl ModelProvider for CapturingProvider { + async fn stream( + &self, + request: ModelRequest, + ) -> Result { + self.requests.lock().unwrap().push(request); + let events = self.responses.lock().unwrap().pop_front().unwrap(); + Ok(Box::pin(futures_util::stream::iter( + events.into_iter().map(Ok), + ))) + } + } + let requests = Arc::new(Mutex::new(Vec::new())); + let provider = Arc::new(CapturingProvider { + requests: requests.clone(), + responses: Mutex::new(VecDeque::from([ + vec![ + ModelEvent::Usage { + input_tokens: 10, + output_tokens: 100, + }, + ModelEvent::Completed { + finish_reason: Some("length".into()), + }, + ], + vec![ + ModelEvent::TextDelta { + text: "done with room".into(), + }, + ModelEvent::Completed { + finish_reason: Some("stop".into()), + }, + ], + ])), + }); + let result = AgentRunner::new( + provider, + Arc::new(FakeExecutor { + outcome: AgentToolResult::Completed { value: json!(null) }, + }), + TurnLimits::default(), + ) + .run( + AgentRunRequest { + fixed_max_output_tokens: true, + ..request() + }, + CancellationToken::new(), + ) + .await + .unwrap(); + + assert_eq!(result.status, AgentRunStatus::Completed); + let requests = requests.lock().unwrap(); + assert_eq!(requests.len(), 2); + assert_eq!(requests[0].max_output_tokens, 100); + assert_eq!(requests[1].max_output_tokens, 100); + } + #[tokio::test] async fn legitimate_answer_may_explain_provider_tool_markup() { let provider = Arc::new(FakeProvider { diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 992dd85..5cc2cfc 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -704,6 +704,11 @@ pub struct ModelConfig { /// Send OpenAI-compatible system messages as one leading message, for chat /// templates that reject a system message after the first. pub merge_system_messages: bool, + /// Sampling temperature for agent turns. Unset keeps the default of 0. + pub temperature: Option, + /// Fixed cap on output tokens per model call in agent turns. Unset starts + /// at 8192 and doubles after a cut-off response, up to 32768. + pub max_output_tokens: Option, pub endpoints: Vec, pub editing_mode: EditingMode, pub editing_overrides: BTreeMap, @@ -730,6 +735,8 @@ impl Default for ModelConfig { read_timeout_seconds: 45, request_timeout_seconds: 180, merge_system_messages: false, + temperature: None, + max_output_tokens: None, endpoints: Vec::new(), editing_mode: EditingMode::Auto, editing_overrides: BTreeMap::new(), @@ -1047,6 +1054,7 @@ enum EnvKind { StringList, Bool, U64, + F64, } struct EnvMapping { env: &'static str, @@ -1144,6 +1152,16 @@ const ENV_MAPPINGS: &[EnvMapping] = &[ path: "model.merge_system_messages", kind: EnvKind::Bool, }, + EnvMapping { + env: "S_CODE_MODEL_TEMPERATURE", + path: "model.temperature", + kind: EnvKind::F64, + }, + EnvMapping { + env: "S_CODE_MODEL_MAX_OUTPUT_TOKENS", + path: "model.max_output_tokens", + kind: EnvKind::U64, + }, EnvMapping { env: "S_CODE_TURN_MAX_MODEL_CALLS", path: "turn.max_model_calls", @@ -1390,6 +1408,12 @@ fn parse_env(value: &str, kind: EnvKind) -> Result { .parse::() .map(Value::from) .map_err(|_| "expected a non-negative integer".into()), + EnvKind::F64 => value + .parse::() + .ok() + .filter(|number| number.is_finite()) + .map(Value::from) + .ok_or_else(|| "expected a finite number".into()), } } @@ -1649,6 +1673,24 @@ fn validate(config: &RootConfig, component: Component) -> Result<(), ConfigError ))); } } + if config + .model + .temperature + .is_some_and(|temperature| !(0.0..=2.0).contains(&temperature)) + { + return Err(ConfigError::Invalid( + "model.temperature must be between 0 and 2".into(), + )); + } + if config + .model + .max_output_tokens + .is_some_and(|tokens| !(1..=1_000_000).contains(&tokens)) + { + return Err(ConfigError::Invalid( + "model.max_output_tokens must be between 1 and 1000000".into(), + )); + } let servicenow = [ config.connectors.servicenow_api_base.is_some(), config.connectors.servicenow_table.is_some(), @@ -2843,6 +2885,42 @@ storage_encryption_key_id = "storage-key-1" } } + #[test] + fn sampling_settings_are_optional_and_bounded() { + let defaults = ConfigLoader::new().load(Component::Daemon).unwrap(); + assert_eq!(defaults.config.model.temperature, None); + assert_eq!(defaults.config.model.max_output_tokens, None); + + let configured = ConfigLoader::new() + .with_environment([ + ("S_CODE_MODEL_TEMPERATURE", "0.6"), + ("S_CODE_MODEL_MAX_OUTPUT_TOKENS", "4096"), + ]) + .load(Component::Daemon) + .unwrap(); + assert_eq!(configured.config.model.temperature, Some(0.6)); + assert_eq!(configured.config.model.max_output_tokens, Some(4096)); + + for (name, value, message) in [ + ("S_CODE_MODEL_TEMPERATURE", "2.5", "between 0 and 2"), + ("S_CODE_MODEL_TEMPERATURE", "NaN", "finite number"), + ( + "S_CODE_MODEL_MAX_OUTPUT_TOKENS", + "0", + "between 1 and 1000000", + ), + ] { + let error = ConfigLoader::new() + .with_environment([(name, value)]) + .load(Component::Daemon) + .unwrap_err(); + assert!( + error.to_string().contains(message), + "{name}={value}: {error}" + ); + } + } + #[test] fn system_message_merging_is_opt_in() { let defaults = ConfigLoader::new().load(Component::Daemon).unwrap(); diff --git a/crates/daemon/src/lib.rs b/crates/daemon/src/lib.rs index a3abf2a..1e6e21a 100644 --- a/crates/daemon/src/lib.rs +++ b/crates/daemon/src/lib.rs @@ -181,6 +181,8 @@ pub struct AppState { code_mode_slots: Arc, turn_limits: TurnLimits, disabled_tools: Arc>, + model_temperature: Option, + model_max_output_tokens: Option, model_credentials_available: bool, storage_protection: Arc, runtime_scopes: RuntimeScopes, @@ -1311,6 +1313,8 @@ impl AppState { code_mode_slots: Arc::new(tokio::sync::Semaphore::new(4)), turn_limits: TurnLimits::default(), disabled_tools: Arc::new(BTreeSet::new()), + model_temperature: None, + model_max_output_tokens: None, model_credentials_available: false, storage_protection: "test_plaintext".into(), runtime_scopes: RuntimeScopes::default(), @@ -1351,6 +1355,14 @@ impl AppState { self } + pub fn with_model_sampling(mut self, config: &s_code_config::ModelConfig) -> Self { + self.model_temperature = config.temperature.map(|temperature| temperature as f32); + self.model_max_output_tokens = config + .max_output_tokens + .map(|tokens| u32::try_from(tokens).unwrap_or(u32::MAX)); + self + } + pub fn with_disabled_tools(mut self, tools: &[String]) -> Self { self.disabled_tools = Arc::new(tools.iter().cloned().collect()); self.refresh_connector_executor(); @@ -17789,10 +17801,11 @@ async fn execute_turn( } let request = AgentRunRequest { model: session.model.clone(), - temperature: 0.0, + temperature: state.model_temperature.unwrap_or(0.0), messages, tools, - max_output_tokens: 8192, + max_output_tokens: state.model_max_output_tokens.unwrap_or(8192), + fixed_max_output_tokens: state.model_max_output_tokens.is_some(), routing: routing_policy(&state, &turn.scope, &session.model).await?, transient_tool_media, }; diff --git a/crates/daemon/src/main.rs b/crates/daemon/src/main.rs index 68c13a1..9f88535 100644 --- a/crates/daemon/src/main.rs +++ b/crates/daemon/src/main.rs @@ -1354,6 +1354,7 @@ async fn daemon_main() -> Result<(), Box> { .with_device_id(device_id) .with_revoked_team_grants(revoked_team_grants) .with_model_editing(&config.model) + .with_model_sampling(&config.model) .with_turn_limits(&config.turn) .with_disabled_tools(&config.turn.disabled_tools) .with_model_credentials_available(model_credentials_available) diff --git a/crates/evals/src/main.rs b/crates/evals/src/main.rs index 5ed1757..3bac5e5 100644 --- a/crates/evals/src/main.rs +++ b/crates/evals/src/main.rs @@ -409,6 +409,7 @@ async fn run_scenario(scenario: Scenario) -> ScenarioReport { }], tools: tools(), max_output_tokens: 512, + fixed_max_output_tokens: false, routing: None, transient_tool_media: std::collections::BTreeMap::new(), }, diff --git a/docs-site/content/guides/model-endpoints.md b/docs-site/content/guides/model-endpoints.md index 54f2f60..03da513 100644 --- a/docs-site/content/guides/model-endpoints.md +++ b/docs-site/content/guides/model-endpoints.md @@ -126,6 +126,12 @@ order, as one leading message. S-Code always does this for Anthropic and Gemini. Like the timeouts, this setting applies only to endpoints in the configuration file or environment. +Agent turns sample at temperature 0 and allow 8,192 output tokens per model +call, doubling after a cut-off response up to 32,768. To match another agent's +settings, for example in a benchmark, set `model.temperature` (0 to 2) and +`model.max_output_tokens`, or `S_CODE_MODEL_TEMPERATURE` and +`S_CODE_MODEL_MAX_OUTPUT_TOKENS`. An explicit output cap stays fixed. + ## Development server The S-Code repository includes a development API Server that exposes an diff --git a/docs/guides/model-endpoints.md b/docs/guides/model-endpoints.md index 54f2f60..03da513 100644 --- a/docs/guides/model-endpoints.md +++ b/docs/guides/model-endpoints.md @@ -126,6 +126,12 @@ order, as one leading message. S-Code always does this for Anthropic and Gemini. Like the timeouts, this setting applies only to endpoints in the configuration file or environment. +Agent turns sample at temperature 0 and allow 8,192 output tokens per model +call, doubling after a cut-off response up to 32,768. To match another agent's +settings, for example in a benchmark, set `model.temperature` (0 to 2) and +`model.max_output_tokens`, or `S_CODE_MODEL_TEMPERATURE` and +`S_CODE_MODEL_MAX_OUTPUT_TOKENS`. An explicit output cap stays fixed. + ## Development server The S-Code repository includes a development API Server that exposes an diff --git a/integrations/harbor/README.md b/integrations/harbor/README.md index fdf33c3..50a6a12 100644 --- a/integrations/harbor/README.md +++ b/integrations/harbor/README.md @@ -90,6 +90,8 @@ not its value. Optional agent kwargs: | `model_connect_timeout_sec` | 10 | Time allowed to open one model connection | | `model_read_timeout_sec` | 600 | Longest gap between bytes of one model response | | `model_request_timeout_sec` | 3600 | Time allowed for one model request | +| `temperature` | S-Code's default, 0 | Sampling temperature, 0 to 2 | +| `max_output_tokens` | S-Code's default, 8192 growing to 32768 | Fixed output-token cap per model call | The adapter reads Harbor's resolved per-task agent deadline and nests the model request, S-Code turn, CLI, cleanup, and Harbor limits inside it. Requested model diff --git a/integrations/harbor/agent.py b/integrations/harbor/agent.py index 6993895..87d593c 100644 --- a/integrations/harbor/agent.py +++ b/integrations/harbor/agent.py @@ -53,6 +53,7 @@ _MAX_CALL_LIMIT = 10_000 _MAX_IDLE_SEC = 3_600 _MAX_MODEL_TIMEOUT_SEC = 3_600 +_MAX_OUTPUT_TOKENS = 1_000_000 _MAX_COST_MICROS = 1_000_000_000_000 _MAX_TOTAL_TOKENS = 1_000_000_000 _MIN_HARBOR_TIMEOUT_SEC = 180 @@ -201,6 +202,16 @@ def _positive_bounded_int( return value +def _optional_temperature(value: object) -> float | None: + if value is None: + return None + if isinstance(value, bool) or not isinstance(value, (int, float)): + raise TypeError("temperature must be a number") + if not math.isfinite(value) or not 0 <= value <= 2: + raise ValueError("temperature must be between 0 and 2") + return float(value) + + def _model_base_url(value: object) -> str: if not isinstance(value, str) or not value or value != value.strip(): raise ValueError("model_base_url must be a non-empty absolute URL") @@ -601,6 +612,8 @@ def __init__( model_request_timeout_sec: int = 3_600, max_cost_micros: int = 5_000_000, max_total_tokens: int = 10_000_000, + temperature: float | None = None, + max_output_tokens: int | None = None, extra_env: dict[str, str] | None = None, **kwargs: Any, ) -> None: @@ -651,6 +664,17 @@ def __init__( name="max_total_tokens", maximum=_MAX_TOTAL_TOKENS, ) + # Unset keeps S-Code's own sampling; benchmarks set these to match a baseline. + self._temperature = _optional_temperature(temperature) + self._max_output_tokens = ( + None + if max_output_tokens is None + else _positive_bounded_int( + max_output_tokens, + name="max_output_tokens", + maximum=_MAX_OUTPUT_TOKENS, + ) + ) # Harbor's real per-task deadline becomes available through the # environment in install(). These placeholder values keep setup # deterministic; install() replaces them before the run phase. @@ -693,6 +717,10 @@ def __init__( "NO_COLOR": "1", "TERM": "dumb", } + if self._temperature is not None: + managed_env["S_CODE_MODEL_TEMPERATURE"] = str(self._temperature) + if self._max_output_tokens is not None: + managed_env["S_CODE_MODEL_MAX_OUTPUT_TOKENS"] = str(self._max_output_tokens) for name, expected in managed_env.items(): if name in supplied_env and supplied_env[name] != expected: raise ValueError(f"{name} is managed by SCodeAgent and cannot be overridden") @@ -848,6 +876,8 @@ def _provenance(self) -> dict[str, Any]: "model_connect_timeout_sec": self._model_connect_timeout_sec, "model_read_timeout_sec": self._model_read_timeout_sec, "model_request_timeout_sec": self._model_request_timeout_sec, + "temperature": self._temperature, + "max_output_tokens": self._max_output_tokens, "max_cost_micros": self._max_cost_micros, "cost_metering_available": False, "max_total_tokens": self._max_total_tokens, diff --git a/tests/test-harbor-adapter.py b/tests/test-harbor-adapter.py index e19a201..f83f4f2 100644 --- a/tests/test-harbor-adapter.py +++ b/tests/test-harbor-adapter.py @@ -541,6 +541,31 @@ def test_harbor_deadline_resolution_matches_cap_then_multiplier(self) -> None: with self.assertRaisesRegex(RuntimeError, "could not resolve"): agent._resolved_harbor_agent_timeout_sec(environment) + def test_sampling_kwargs_are_optional_and_validated(self) -> None: + environment = self.new_agent().base_init["extra_env"] + self.assertNotIn("S_CODE_MODEL_TEMPERATURE", environment) + self.assertNotIn("S_CODE_MODEL_MAX_OUTPUT_TOKENS", environment) + + configured = self.new_agent(temperature=0.6, max_output_tokens=4096) + environment = configured.base_init["extra_env"] + self.assertEqual(environment["S_CODE_MODEL_TEMPERATURE"], "0.6") + self.assertEqual(environment["S_CODE_MODEL_MAX_OUTPUT_TOKENS"], "4096") + execution = configured._provenance()["execution"] + self.assertEqual(execution["temperature"], 0.6) + self.assertEqual(execution["max_output_tokens"], 4096) + + for kwargs, error in ( + ({"temperature": 2.5}, ValueError), + ({"temperature": float("nan")}, ValueError), + ({"temperature": True}, TypeError), + ({"temperature": "0.6"}, TypeError), + ({"max_output_tokens": 0}, ValueError), + ({"max_output_tokens": True}, TypeError), + ): + with self.subTest(kwargs=kwargs): + with self.assertRaises(error): + self.new_agent(**kwargs) + def test_install_uses_one_archive_and_numeric_nonroot_owner(self) -> None: instance = self.new_agent() environment = FakeEnvironment() From 71e08c8649f15d6adad09febc121fd3f3da71a0f Mon Sep 17 00:00:00 2001 From: Wei Deng Date: Sun, 18 Oct 2026 21:24:53 -0700 Subject: [PATCH 6/9] test(daemon): check configured sampling reaches the model request The existing tests cover the sampling settings' validation and the agent's fixed output cap, but none followed a configured value to the request the model receives. Add one: it runs a turn without the settings and checks the defaults (temperature 0, 8,192 output tokens), then with temperature 0.6 and max_output_tokens 4,096 and checks those values arrive. Refs #129 Signed-off-by: Wei Deng --- crates/daemon/src/lib.rs | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/crates/daemon/src/lib.rs b/crates/daemon/src/lib.rs index 1e6e21a..377f9cf 100644 --- a/crates/daemon/src/lib.rs +++ b/crates/daemon/src/lib.rs @@ -27379,6 +27379,62 @@ mod tests { })); } + #[tokio::test] + async fn configured_sampling_settings_reach_the_model_request() { + let workspace = tempfile::tempdir().unwrap(); + let scope = Scope { + organization_id: Id("org".into()), + team_id: Id("team".into()), + actor_id: Id("user".into()), + goal_id: None, + task_id: None, + }; + let store = Store::in_memory().await.unwrap(); + let session = store + .create_session(CreateSession { + mode: s_code_protocol::SessionMode::Work, + scope: scope.clone(), + workspace_uri: url::Url::from_directory_path(workspace.path()) + .unwrap() + .to_string(), + title: "Sampling".into(), + model: "mock".into(), + }) + .await + .unwrap(); + let configured = s_code_config::ModelConfig { + temperature: Some(0.6), + max_output_tokens: Some(4_096), + ..Default::default() + }; + let state = AppState::new("secret", store.clone(), 0); + for (state, temperature, max_output_tokens) in [ + (state.clone(), 0.0, 8_192), + (state.with_model_sampling(&configured), 0.6, 4_096), + ] { + store + .append_message(&scope, &session.id, "user", serde_json::json!("hello")) + .await + .unwrap(); + let turn = store.create_turn(&scope, &session.id).await.unwrap(); + let captured = Arc::new(StdMutex::new(None)); + run_turn( + state, + Arc::new(CapturingProvider { + request: captured.clone(), + }), + turn, + CancellationToken::new(), + ToolProfile::Default, + ) + .await + .unwrap(); + let request = captured.lock().unwrap().clone().unwrap(); + assert_eq!(request.temperature, temperature); + assert_eq!(request.max_output_tokens, max_output_tokens); + } + } + #[tokio::test] async fn scoped_memory_and_explicit_compaction_are_bounded_and_content_safe() { let workspace = tempfile::tempdir().unwrap(); From a2509ffe577a84d63b25c2fc8ba181193e341717 Mon Sep 17 00:00:00 2001 From: Wei Deng Date: Mon, 19 Oct 2026 13:53:42 -0700 Subject: [PATCH 7/9] fix(harbor): give commands the task container's environment Under Harbor, S-Code runs in Full mode, where run_command clears the environment and sets a minimal PATH and no HOME. Tools in /usr/local/bin or in Dockerfile ENV PATH directories, PYTHONPATH and JAVA_HOME were invisible to anything a command started, so scores could reflect the harness rather than S-Code. Add a Harbor-only container environment mode: - A default-off Cargo feature, harbor-container-env, that only integrations/harbor/build-bundle.sh enables. The bundle manifest records it and the adapter refuses bundles without it, so normal builds cannot activate the mode. - Before it sets anything of its own, the runner saves its environment as the agent user, minus every variable Harbor passed to the agent. The daemon loads it once at startup, only if it is a private regular file this user owns; it drops S_CODE_*, volatile shell fields and any variable holding the model credential, deletes the file and writes a summary of names and a hash. - Full-mode commands then run in that environment, with PWD set to their directory, and are found on its PATH. S-Code adds no PATH, TMPDIR or OPENSSL_CONF. Sandboxed commands, extensions and normal builds are unchanged. - credential_env must start with S_CODE_HARBOR_, and the adapter no longer sets TERM and NO_COLOR, which would otherwise reach every command. - Provenance records the verified mode with the variable names and their hash, a hash of the adapter sources in its version, and the names of unmanaged S_CODE_* variables. Refs #129 Signed-off-by: Wei Deng --- .github/workflows/ci.yml | 6 + Cargo.lock | 2 + crates/daemon/Cargo.toml | 5 + crates/daemon/src/main.rs | 125 ++++ crates/platform-runtime/Cargo.toml | 8 + .../src/container_environment.rs | 596 ++++++++++++++++++ crates/platform-runtime/src/lib.rs | 62 +- crates/tool-runtime/Cargo.toml | 5 + crates/tool-runtime/src/lib.rs | 75 ++- .../tests/harbor_container_env.rs | 69 ++ docs-site/content/guides/tools-permissions.md | 4 +- docs/guides/tools-permissions.md | 4 +- integrations/harbor/README.md | 46 +- integrations/harbor/agent.py | 175 ++++- integrations/harbor/build-bundle.sh | 11 +- integrations/harbor/runtime.py | 26 +- tests/test-harbor-adapter.py | 335 +++++++++- tests/test-harbor-bundle.py | 54 ++ 18 files changed, 1560 insertions(+), 48 deletions(-) create mode 100644 crates/platform-runtime/src/container_environment.rs create mode 100644 crates/tool-runtime/tests/harbor_container_env.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 19788ff..deae784 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -154,6 +154,12 @@ jobs: - name: Rust tests including platform security controls if: needs.checks.outputs.rust == 'true' run: cargo test --locked --workspace + - name: Harbor-only container environment feature + if: needs.checks.outputs.rust == 'true' + run: | + cargo clippy --locked -p s-code-platform-runtime -p s-code-tool-runtime -p s-code-daemon --all-targets --features harbor-container-env -- -D warnings + cargo test --locked -p s-code-platform-runtime -p s-code-tool-runtime --features harbor-container-env + cargo test --locked -p s-code-daemon --features harbor-container-env --bin s-code-daemon - name: CLI end-to-end if: needs.checks.outputs.runtime == 'true' run: tests/test-cli-e2e.sh diff --git a/Cargo.lock b/Cargo.lock index 01ae36b..da0c30c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3943,6 +3943,8 @@ dependencies = [ "rustix 1.1.4", "s-code-protocol", "serde", + "serde_json", + "sha2 0.10.9", "tempfile", "thiserror 2.0.19", "tokio", diff --git a/crates/daemon/Cargo.toml b/crates/daemon/Cargo.toml index da4c505..6e8d687 100644 --- a/crates/daemon/Cargo.toml +++ b/crates/daemon/Cargo.toml @@ -6,6 +6,11 @@ license.workspace = true publish.workspace = true default-run = "s-code-daemon" +[features] +# Harbor benchmark bundles only (integrations/harbor/build-bundle.sh): load the +# task container's command environment that the adapter captured. +harbor-container-env = ["s-code-platform-runtime/harbor-container-env"] + [dependencies] anyhow.workspace = true async-trait.workspace = true diff --git a/crates/daemon/src/main.rs b/crates/daemon/src/main.rs index 9f88535..669396b 100644 --- a/crates/daemon/src/main.rs +++ b/crates/daemon/src/main.rs @@ -25,6 +25,8 @@ use s_code_model_gateway::{ AnthropicMessages, EnvironmentCredentials, GeminiGenerateContent, GovernedModelRouter, HttpTimeouts, ModelProvider, OpenAiCompatible, RoutedModelEndpoint, }; +#[cfg(feature = "harbor-container-env")] +use s_code_platform_runtime::container_environment; use s_code_policy::PolicyTrustStore; use s_code_protocol::{DaemonSettings, Id, Scope}; use s_code_storage::{StorageError, StorageFormat, Store}; @@ -1025,6 +1027,48 @@ async fn connect_mcp_source_groups( Ok((registry, active_mcp_sources, active_mcp_http_sources)) } +/// Harbor bundles only: load the task container's command environment that the +/// adapter captured for Full-mode commands. Any missing or invalid setting stops +/// the daemon rather than running commands in the minimal environment. +#[cfg(feature = "harbor-container-env")] +fn harbor_command_environment( + variable: impl Fn(&str) -> Option, +) -> Result< + Option<( + container_environment::ContainerEnvironment, + container_environment::Summary, + )>, + String, +> { + let Some(snapshot) = variable("S_CODE_HARBOR_COMMAND_ENV_FILE") else { + return Ok(None); + }; + let required = |name: &str| { + variable(name) + .ok_or_else(|| format!("{name} must be set with S_CODE_HARBOR_COMMAND_ENV_FILE")) + }; + let summary = PathBuf::from(required("S_CODE_HARBOR_COMMAND_ENV_SUMMARY")?); + let agent_names = required("S_CODE_HARBOR_AGENT_ENV_NAMES")? + .into_string() + .map_err(|_| "S_CODE_HARBOR_AGENT_ENV_NAMES must be UTF-8".to_string())?; + let snapshot = PathBuf::from(snapshot); + if !snapshot.is_absolute() || !summary.is_absolute() { + return Err("the Harbor command environment paths must be absolute".into()); + } + let agent_names = agent_names + .split(',') + .filter(|name| !name.is_empty()) + .map(std::ffi::OsString::from) + .collect(); + // The model credential never reaches commands, even under another name. + let secret_values = variable("S_CODE_MODEL_CREDENTIAL_HANDLE") + .and_then(|handle| handle.into_string().ok()) + .and_then(|handle| variable(&handle)) + .into_iter() + .collect::>(); + container_environment::load(&snapshot, &agent_names, &secret_values, &summary).map(Some) +} + fn main() -> Result<(), Box> { if std::env::args().nth(1).as_deref() == Some("--code-mode-worker") { s_code_daemon::code_mode::worker()?; @@ -1287,6 +1331,19 @@ async fn daemon_main() -> Result<(), Box> { _ => return Err(format!("unknown argument: {command}").into()), } } + // Keep this after the argument handling above: Harbor's install step runs + // `--self-test` with these variables set before the runner has written a + // snapshot. + #[cfg(feature = "harbor-container-env")] + if let Some((environment, summary)) = harbor_command_environment(|name| std::env::var_os(name)) + .map_err(|error| format!("cannot load the Harbor command environment: {error}"))? + { + container_environment::install(environment)?; + eprintln!( + "S_CODE_HARBOR_COMMAND_ENV={} count={} sha256={}", + summary.mode, summary.count, summary.sha256 + ); + } tracing_subscriber::fmt() .with_env_filter(tracing_subscriber::EnvFilter::from_default_env()) .json() @@ -2509,6 +2566,74 @@ mod tests { ); } + #[cfg(feature = "harbor-container-env")] + #[test] + fn harbor_command_environment_needs_every_setting_and_drops_agent_names() { + let directory = tempfile::tempdir().unwrap(); + let snapshot = directory.path().join("command.env"); + let summary = directory.path().join("command-env.json"); + let (snapshot_path, summary_path) = (snapshot.to_str().unwrap(), summary.to_str().unwrap()); + let load = |values: &[(&str, &str)]| { + harbor_command_environment(|name| { + values + .iter() + .find(|(key, _)| *key == name) + .map(|(_, value)| std::ffi::OsString::from(*value)) + }) + }; + assert!(load(&[]).unwrap().is_none()); + let invalid: [(&[(&str, &str)], &str); 3] = [ + ( + &[ + ("S_CODE_HARBOR_COMMAND_ENV_FILE", snapshot_path), + ("S_CODE_HARBOR_AGENT_ENV_NAMES", ""), + ], + "S_CODE_HARBOR_COMMAND_ENV_SUMMARY must be set", + ), + ( + &[ + ("S_CODE_HARBOR_COMMAND_ENV_FILE", snapshot_path), + ("S_CODE_HARBOR_COMMAND_ENV_SUMMARY", summary_path), + ], + "S_CODE_HARBOR_AGENT_ENV_NAMES must be set", + ), + ( + &[ + ("S_CODE_HARBOR_COMMAND_ENV_FILE", "command.env"), + ("S_CODE_HARBOR_COMMAND_ENV_SUMMARY", summary_path), + ("S_CODE_HARBOR_AGENT_ENV_NAMES", ""), + ], + "must be absolute", + ), + ]; + for (values, expected) in invalid { + let error = load(values).unwrap_err(); + assert!(error.contains(expected), "{error}"); + } + + std::fs::write( + &snapshot, + b"KEPT=1\0FIRST_AGENT=2\0SECOND_AGENT=3\0COPIED_KEY=model-key-canary\0", + ) + .unwrap(); + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&snapshot, std::fs::Permissions::from_mode(0o600)).unwrap(); + } + let (environment, recorded) = load(&[ + ("S_CODE_HARBOR_COMMAND_ENV_FILE", snapshot_path), + ("S_CODE_HARBOR_COMMAND_ENV_SUMMARY", summary_path), + ("S_CODE_HARBOR_AGENT_ENV_NAMES", "FIRST_AGENT,SECOND_AGENT"), + ("S_CODE_MODEL_CREDENTIAL_HANDLE", "S_CODE_HARBOR_KEY"), + ("S_CODE_HARBOR_KEY", "model-key-canary"), + ]) + .unwrap() + .unwrap(); + assert_eq!(recorded.names, ["KEPT"]); + assert_eq!(environment.variables().count(), 1); + assert!(!snapshot.exists() && summary.exists()); + } + #[tokio::test] async fn development_preserves_an_explicit_non_legacy_model() { let store = Store::in_memory().await.unwrap(); diff --git a/crates/platform-runtime/Cargo.toml b/crates/platform-runtime/Cargo.toml index 6d3dc92..56a90da 100644 --- a/crates/platform-runtime/Cargo.toml +++ b/crates/platform-runtime/Cargo.toml @@ -5,11 +5,19 @@ edition.workspace = true license.workspace = true publish.workspace = true +[features] +# Harbor benchmark bundles only (integrations/harbor/build-bundle.sh): Full-mode +# host commands get the task container's captured environment. Off by default, +# so no other build can turn the mode on. +harbor-container-env = ["dep:serde_json", "dep:sha2"] + [dependencies] async-trait.workspace = true s-code-protocol = { path = "../protocol" } rustix.workspace = true serde.workspace = true +serde_json = { workspace = true, optional = true } +sha2 = { workspace = true, optional = true } tempfile.workspace = true thiserror.workspace = true tokio.workspace = true diff --git a/crates/platform-runtime/src/container_environment.rs b/crates/platform-runtime/src/container_environment.rs new file mode 100644 index 0000000..a90ed4e --- /dev/null +++ b/crates/platform-runtime/src/container_environment.rs @@ -0,0 +1,596 @@ +//! The command environment of a Harbor task container. +//! +//! Full-mode host commands normally start from a cleared, minimal environment. +//! Harbor benchmark bundles are built with the `harbor-container-env` feature: +//! their adapter captures the environment an ordinary command in the task +//! container gets (`env -0`, as the agent user), the daemon loads it once at +//! startup, and host commands then run in it, so a score reflects S-Code rather +//! than a PATH or HOME the task image never had. Without the feature nothing +//! can load or install an environment and [`installed`] is always `None`. + +use std::{ + collections::BTreeMap, + ffi::{OsStr, OsString}, + path::PathBuf, +}; +#[cfg(feature = "harbor-container-env")] +use std::{collections::BTreeSet, path::Path, sync::OnceLock}; + +#[cfg(all(feature = "harbor-container-env", not(unix)))] +compile_error!("the harbor-container-env feature supports only Unix task containers"); + +#[cfg(feature = "harbor-container-env")] +pub const MAX_SNAPSHOT_BYTES: usize = 1024 * 1024; +#[cfg(feature = "harbor-container-env")] +pub const MAX_SNAPSHOT_RECORDS: usize = 4096; + +/// Shell bookkeeping that describes the capture rather than a command. +#[cfg(feature = "harbor-container-env")] +const VOLATILE_NAMES: [&str; 4] = ["PWD", "OLDPWD", "SHLVL", "_"]; + +/// The variables Full-mode host commands get instead of the minimal +/// environment. `Debug` shows names only. +pub struct ContainerEnvironment { + variables: BTreeMap, +} + +impl ContainerEnvironment { + pub fn variables(&self) -> impl Iterator { + self.variables + .iter() + .map(|(name, value)| (name.as_os_str(), value.as_os_str())) + } + + /// The absolute entries of this environment's PATH in their original + /// order, so a program is found on the PATH it then runs with. + pub fn search_path(&self) -> Vec { + self.variables + .get(OsStr::new("PATH")) + .map(|paths| { + std::env::split_paths(paths) + .filter(|path| path.is_absolute()) + .collect() + }) + .unwrap_or_default() + } +} + +impl std::fmt::Debug for ContainerEnvironment { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_struct("ContainerEnvironment") + .field("names", &self.variables.keys().collect::>()) + .finish() + } +} + +#[cfg(feature = "harbor-container-env")] +static INSTALLED: OnceLock = OnceLock::new(); + +/// The environment the daemon installed for host commands, if any. +pub fn installed() -> Option<&'static ContainerEnvironment> { + #[cfg(feature = "harbor-container-env")] + { + INSTALLED.get() + } + #[cfg(not(feature = "harbor-container-env"))] + { + None + } +} + +/// Set the process-wide container environment. It can be set only once. +#[cfg(feature = "harbor-container-env")] +pub fn install(environment: ContainerEnvironment) -> Result<(), String> { + INSTALLED + .set(environment) + .map_err(|_| "the container environment is already installed".into()) +} + +/// What the daemon records about a loaded environment: names and a digest, +/// never values. +#[cfg(feature = "harbor-container-env")] +#[derive(Debug, serde::Serialize)] +pub struct Summary { + pub mode: &'static str, + pub count: usize, + pub names: Vec, + /// SHA-256 of the `NAME=VALUE` records in name order, joined by NUL. + pub sha256: String, +} + +/// Load an `env -0` snapshot, delete it, and write its summary (mode 0600). +/// The snapshot must be a regular file that only this user can read. S-Code's +/// own `S_CODE_*` variables, the agent-only names Harbor passed to the agent, +/// the shell's volatile fields and any variable holding one of +/// `secret_values` are dropped; later duplicates win. Any failure is an +/// error, so the daemon never starts commands in an environment it did not +/// fully load. +#[cfg(feature = "harbor-container-env")] +pub fn load( + snapshot: &Path, + agent_names: &BTreeSet, + secret_values: &[OsString], + summary: &Path, +) -> Result<(ContainerEnvironment, Summary), String> { + use rustix::fs::{Mode, OFlags}; + use sha2::{Digest, Sha256}; + use std::io::{Read, Write}; + use std::os::unix::{ + ffi::OsStrExt, + fs::{MetadataExt, OpenOptionsExt}, + }; + + // Open without following a link, then check what was opened. + let file = rustix::fs::open( + snapshot, + OFlags::RDONLY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .map(std::fs::File::from) + .map_err(|error| format!("cannot open {}: {error}", snapshot.display()))?; + let metadata = file + .metadata() + .map_err(|error| format!("cannot inspect {}: {error}", snapshot.display()))?; + let private = metadata.is_file() + && metadata.uid() == rustix::process::geteuid().as_raw() + && metadata.mode() & 0o077 == 0; + let mut bytes = Vec::new(); + if private { + file.take(MAX_SNAPSHOT_BYTES as u64 + 1) + .read_to_end(&mut bytes) + .map_err(|error| format!("cannot read {}: {error}", snapshot.display()))?; + } + // The snapshot can hold the agent's credentials, so remove it before any + // other check can fail. + std::fs::remove_file(snapshot) + .map_err(|error| format!("cannot delete {}: {error}", snapshot.display()))?; + if !private { + return Err(format!( + "{} must be a regular file that only its owner, this user, can access", + snapshot.display() + )); + } + if bytes.len() > MAX_SNAPSHOT_BYTES { + return Err(format!( + "{} is larger than {MAX_SNAPSHOT_BYTES} bytes", + snapshot.display() + )); + } + let mut records = bytes.split(|byte| *byte == 0).collect::>(); + if records.last().is_some_and(|record| record.is_empty()) { + records.pop(); + } + if records.len() > MAX_SNAPSHOT_RECORDS { + return Err(format!( + "{} has more than {MAX_SNAPSHOT_RECORDS} records", + snapshot.display() + )); + } + let mut variables = BTreeMap::new(); + for record in records { + let Some(separator) = record.iter().position(|byte| *byte == b'=') else { + continue; + }; + let name = OsStr::from_bytes(&record[..separator]); + let value = OsStr::from_bytes(&record[separator + 1..]); + if name.is_empty() + || name.as_bytes().starts_with(b"S_CODE_") + || VOLATILE_NAMES.iter().any(|volatile| name == *volatile) + || agent_names.contains(name) + || secret_values + .iter() + .any(|secret| !secret.is_empty() && secret.as_os_str() == value) + { + continue; + } + variables.insert(name.to_owned(), value.to_owned()); + } + + let mut digest = Sha256::new(); + for (index, (name, value)) in variables.iter().enumerate() { + if index > 0 { + digest.update([0]); + } + digest.update(name.as_bytes()); + digest.update(b"="); + digest.update(value.as_bytes()); + } + let recorded = Summary { + mode: "container", + count: variables.len(), + names: variables + .keys() + .map(|name| name.to_string_lossy().into_owned()) + .collect(), + sha256: digest + .finalize() + .iter() + .map(|byte| format!("{byte:02x}")) + .collect(), + }; + let json = serde_json::to_vec(&recorded).map_err(|error| error.to_string())?; + // Replace a stale summary instead of writing through whatever is there. + match std::fs::remove_file(summary) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => { + return Err(format!("cannot replace {}: {error}", summary.display())); + } + } + std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(summary) + .and_then(|mut file| file.write_all(&json)) + .map_err(|error| format!("cannot write {}: {error}", summary.display()))?; + Ok((ContainerEnvironment { variables }, recorded)) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Write a snapshot the way the runner does: a file only its owner can read. + #[cfg(feature = "harbor-container-env")] + fn write_snapshot(path: &std::path::Path, bytes: &[u8]) { + use std::io::Write; + use std::os::unix::fs::OpenOptionsExt; + + std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(path) + .and_then(|mut file| file.write_all(bytes)) + .unwrap(); + } + + #[cfg(any(target_os = "macos", target_os = "linux"))] + fn write_executable(path: &std::path::Path, script: &str) { + use std::os::unix::fs::PermissionsExt; + + std::fs::write(path, script).unwrap(); + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + + /// Run a Full-mode host command the way `ToolRuntime::run_host` does, + /// with an explicit container environment instead of the installed one. + #[cfg(any(target_os = "macos", target_os = "linux"))] + async fn run_host( + workspace: &std::path::Path, + program: impl AsRef, + args: &[&str], + container: Option<&ContainerEnvironment>, + ) -> crate::ProcessOutput { + crate::NativeRuntime::execute_native( + crate::ProcessSpec { + program: program.as_ref().to_string_lossy().into_owned(), + args: args.iter().map(|arg| arg.to_string()).collect(), + cwd_uri: url::Url::from_directory_path(workspace) + .unwrap() + .to_string(), + environment_handles: Default::default(), + timeout: std::time::Duration::from_secs(10), + network_enabled: true, + browser_compatible: false, + readable_root_uris: Vec::new(), + writable_root_uris: Vec::new(), + denied_read_uris: Vec::new(), + output_limit_bytes: 64 * 1024, + pinned_cwd: Some(std::sync::Arc::new(std::fs::File::open(workspace).unwrap())), + }, + true, + container, + ) + .await + .unwrap() + } + + #[cfg(not(feature = "harbor-container-env"))] + #[test] + fn normal_builds_never_have_a_container_environment() { + assert!(installed().is_none()); + } + + #[cfg(unix)] + #[test] + fn search_path_keeps_absolute_entries_in_their_order() { + let environment = ContainerEnvironment { + variables: BTreeMap::from([( + OsString::from("PATH"), + OsString::from("/opt/venv/bin:relative::/usr/local/bin:/usr/bin"), + )]), + }; + assert_eq!( + environment.search_path(), + ["/opt/venv/bin", "/usr/local/bin", "/usr/bin"].map(PathBuf::from) + ); + let without_path = ContainerEnvironment { + variables: BTreeMap::new(), + }; + assert!(without_path.search_path().is_empty()); + } + + #[cfg(any(target_os = "macos", target_os = "linux"))] + #[tokio::test] + async fn host_commands_run_in_the_container_environment_and_nothing_else() { + let workspace = tempfile::tempdir().unwrap(); + let first = tempfile::tempdir().unwrap(); + let second = tempfile::tempdir().unwrap(); + let home = tempfile::tempdir().unwrap(); + for directory in [first.path(), second.path()] { + write_executable(&directory.join("tool"), "#!/bin/sh\n"); + } + let node = first.path().join("node"); + write_executable( + &node, + "#!/bin/sh\nprintf 'TMPDIR=%s OPENSSL_CONF=%s\\n' \"${TMPDIR-unset}\" \"${OPENSSL_CONF-unset}\"\n", + ); + let path = std::env::join_paths([ + first.path(), + second.path(), + std::path::Path::new("/usr/bin"), + std::path::Path::new("/bin"), + ]) + .unwrap(); + let environment = ContainerEnvironment { + variables: BTreeMap::from([ + (OsString::from("PATH"), path.clone()), + (OsString::from("HOME"), home.path().into()), + (OsString::from("CONTAINER_MARKER"), "from the image".into()), + ]), + }; + + let output = run_host( + workspace.path(), + "/bin/sh", + &["-c", "command -v tool; printf '%s\\n' \"$HOME\""], + Some(&environment), + ) + .await; + assert_eq!(output.exit_code, Some(0), "{output:?}"); + assert_eq!( + output.stdout, + format!( + "{}\n{}\n", + first.path().join("tool").display(), + home.path().display() + ) + ); + + // A program receives the container's variables and its own PWD, and + // nothing from S-Code: no PATH rewrite, TMPDIR or daemon variable. + let printed = run_host(workspace.path(), "/usr/bin/env", &[], Some(&environment)).await; + let mut received = printed.stdout.lines().collect::>(); + received.sort_unstable(); + assert_eq!( + received, + [ + "CONTAINER_MARKER=from the image".to_string(), + format!("HOME={}", home.path().display()), + format!("PATH={}", path.to_str().unwrap()), + format!("PWD={}", workspace.path().canonicalize().unwrap().display()), + ] + ); + + // Only the minimal environment gives a program named node its own + // OPENSSL_CONF and a per-command TMPDIR. + let contained = run_host(workspace.path(), &node, &[], Some(&environment)).await; + assert_eq!(contained.stdout, "TMPDIR=unset OPENSSL_CONF=unset\n"); + let minimal = run_host(workspace.path(), &node, &[], None).await; + assert!( + minimal.stdout.contains("OPENSSL_CONF=/dev/null") && !minimal.stdout.contains("unset"), + "{minimal:?}" + ); + } + + #[cfg(any(target_os = "macos", target_os = "linux"))] + #[tokio::test] + async fn git_global_configuration_persists_through_the_container_home() { + let git = std::path::Path::new("/usr/bin/git"); + if !git.is_file() { + return; + } + let workspace = tempfile::tempdir().unwrap(); + let home = tempfile::tempdir().unwrap(); + let environment = ContainerEnvironment { + variables: BTreeMap::from([ + (OsString::from("PATH"), OsString::from("/usr/bin:/bin")), + (OsString::from("HOME"), home.path().into()), + ]), + }; + let set = run_host( + workspace.path(), + git, + &["config", "--global", "user.name", "Fixture User"], + Some(&environment), + ) + .await; + assert_eq!(set.exit_code, Some(0), "{set:?}"); + let get = run_host( + workspace.path(), + git, + &["config", "--global", "user.name"], + Some(&environment), + ) + .await; + assert_eq!(get.stdout, "Fixture User\n", "{get:?}"); + assert!(home.path().join(".gitconfig").is_file()); + } + + #[cfg(feature = "harbor-container-env")] + #[test] + fn load_keeps_container_variables_and_writes_a_summary_without_values() { + use sha2::{Digest, Sha256}; + use std::os::unix::{ffi::OsStrExt, fs::PermissionsExt}; + + let directory = tempfile::tempdir().unwrap(); + let snapshot = directory.path().join("command.env"); + let summary = directory.path().join("command-env.json"); + let records: &[&[u8]] = &[ + b"PATH=/opt/venv/bin:/usr/local/bin:/usr/bin:/bin", + b"HOME=/home/agent", + b"BINARY=\xff\xfe", + b"EQUALS=a=b=c", + b"MULTILINE=first\nsecond", + b"DUPLICATE=earlier", + b"DUPLICATE=later", + b"S_CODE_HARBOR_CREDENTIAL=s-code-canary-value", + b"S_CODE_TOKEN=s-code-token-value", + b"AGENT_ONLY=agent-canary-value", + b"COPIED_KEY=model-key-canary", + b"PWD=/app", + b"OLDPWD=/", + b"SHLVL=1", + b"_=/installed-agent/run", + b"=empty-name", + b"NO_SEPARATOR", + b"", + ]; + let mut bytes = records.join(&0); + bytes.push(0); + write_snapshot(&snapshot, &bytes); + + let (environment, recorded) = load( + &snapshot, + &BTreeSet::from([OsString::from("AGENT_ONLY")]), + &[OsString::from("model-key-canary")], + &summary, + ) + .unwrap(); + + assert!(!snapshot.exists()); + let kept: &[&[u8]] = &[ + b"BINARY=\xff\xfe", + b"DUPLICATE=later", + b"EQUALS=a=b=c", + b"HOME=/home/agent", + b"MULTILINE=first\nsecond", + b"PATH=/opt/venv/bin:/usr/local/bin:/usr/bin:/bin", + ]; + assert_eq!( + environment + .variables() + .map(|(name, value)| [name.as_bytes(), b"=", value.as_bytes()].concat()) + .collect::>(), + kept + ); + let sha256 = Sha256::digest(kept.join(&0)) + .iter() + .map(|byte| format!("{byte:02x}")) + .collect::(); + assert_eq!(recorded.sha256, sha256); + let written = std::fs::read(&summary).unwrap(); + assert_eq!( + serde_json::from_slice::(&written).unwrap(), + serde_json::json!({ + "mode": "container", + "count": 6, + "names": ["BINARY", "DUPLICATE", "EQUALS", "HOME", "MULTILINE", "PATH"], + "sha256": sha256, + }) + ); + for value in [ + b"/home/agent".as_slice(), + b"a=b=c", + b"later", + b"/opt/venv/bin", + b"canary-value", + b"token-value", + b"model-key", + ] { + assert!(!written.windows(value.len()).any(|window| window == value)); + } + assert_eq!( + std::fs::metadata(&summary).unwrap().permissions().mode() & 0o777, + 0o600 + ); + } + + #[cfg(feature = "harbor-container-env")] + #[test] + fn load_refuses_links_and_files_others_can_access() { + use std::os::unix::fs::PermissionsExt; + + let directory = tempfile::tempdir().unwrap(); + let summary = directory.path().join("command-env.json"); + let target = directory.path().join("target.env"); + write_snapshot(&target, b"PATH=/usr/bin\0"); + let link = directory.path().join("link.env"); + std::os::unix::fs::symlink(&target, &link).unwrap(); + assert!(load(&link, &BTreeSet::new(), &[], &summary).is_err()); + assert!( + target.exists(), + "the link's target is never read or removed" + ); + + let shared = directory.path().join("shared.env"); + write_snapshot(&shared, b"PATH=/usr/bin\0"); + std::fs::set_permissions(&shared, std::fs::Permissions::from_mode(0o640)).unwrap(); + let error = load(&shared, &BTreeSet::new(), &[], &summary).unwrap_err(); + assert!(error.contains("only its owner"), "{error}"); + assert!(!shared.exists()); + assert!(!summary.exists()); + } + + #[cfg(feature = "harbor-container-env")] + #[test] + fn load_accepts_the_limits_and_rejects_anything_larger() { + let directory = tempfile::tempdir().unwrap(); + let snapshot = directory.path().join("command.env"); + let summary = directory.path().join("command-env.json"); + let largest = [b"LARGE=".as_slice(), &vec![b'x'; MAX_SNAPSHOT_BYTES - 6]].concat(); + for (bytes, accepted) in [ + (largest.clone(), true), + ([largest.as_slice(), b"x"].concat(), false), + (b"A=1\0".repeat(MAX_SNAPSHOT_RECORDS), true), + (b"A=1\0".repeat(MAX_SNAPSHOT_RECORDS + 1), false), + ] { + write_snapshot(&snapshot, &bytes); + let loaded = load(&snapshot, &BTreeSet::new(), &[], &summary); + assert_eq!(loaded.is_ok(), accepted, "{loaded:?}"); + assert!(!snapshot.exists()); + } + } + + #[cfg(all( + feature = "harbor-container-env", + any(target_os = "macos", target_os = "linux") + ))] + #[tokio::test] + async fn s_code_and_agent_variables_never_reach_commands() { + let workspace = tempfile::tempdir().unwrap(); + let directory = tempfile::tempdir().unwrap(); + let snapshot = directory.path().join("command.env"); + write_snapshot( + &snapshot, + b"PATH=/usr/bin:/bin\0CONTAINER_MARKER=kept\0S_CODE_HARBOR_CREDENTIAL=s-code-canary-5c1e\0AGENT_ONLY=agent-canary-9b2d\0", + ); + let (environment, _) = load( + &snapshot, + &BTreeSet::from([OsString::from("AGENT_ONLY")]), + &[], + &directory.path().join("command-env.json"), + ) + .unwrap(); + + let output = run_host(workspace.path(), "/usr/bin/env", &[], Some(&environment)).await; + assert_eq!(output.exit_code, Some(0), "{output:?}"); + assert!( + output + .stdout + .lines() + .any(|line| line == "CONTAINER_MARKER=kept") + ); + for canary in [ + "S_CODE_", + "s-code-canary-5c1e", + "AGENT_ONLY", + "agent-canary-9b2d", + ] { + assert!(!output.stdout.contains(canary), "{output:?}"); + } + } +} diff --git a/crates/platform-runtime/src/lib.rs b/crates/platform-runtime/src/lib.rs index 62aae7e..11a79a3 100644 --- a/crates/platform-runtime/src/lib.rs +++ b/crates/platform-runtime/src/lib.rs @@ -1,6 +1,8 @@ +pub mod container_environment; pub mod sensitive_paths; use async_trait::async_trait; +use container_environment::ContainerEnvironment; use s_code_protocol::Capability; use serde::{Deserialize, Serialize}; use std::{ @@ -725,7 +727,7 @@ impl PlatformRuntime for NativeRuntime { } async fn execute(&self, spec: ProcessSpec) -> Result { - Self::execute_native(spec, false).await + Self::execute_native(spec, false, None).await } fn supports_host_execution(&self) -> bool { @@ -733,7 +735,7 @@ impl PlatformRuntime for NativeRuntime { } async fn execute_host(&self, spec: ProcessSpec) -> Result { - Self::execute_native(spec, true).await + Self::execute_native(spec, true, container_environment::installed()).await } async fn cancel_process_tree(&self, _process_id: &str) -> Result<(), RuntimeError> { @@ -744,9 +746,11 @@ impl PlatformRuntime for NativeRuntime { } impl NativeRuntime { + /// `container` replaces the minimal environment of host commands only. async fn execute_native( mut spec: ProcessSpec, host: bool, + container: Option<&ContainerEnvironment>, ) -> Result { let deadline = tokio::time::Instant::now() + spec.timeout; let cwd = Self::uri_path(&spec.cwd_uri)? @@ -768,14 +772,22 @@ impl NativeRuntime { )); } } - let sandbox_temp = tempfile::Builder::new() - .prefix("s-code-sandbox.") - .tempdir() - .map_err(|error| RuntimeError::Execution(error.to_string()))?; - let sandbox_temp_uri = url::Url::from_directory_path(sandbox_temp.path()) - .map_err(|_| RuntimeError::Execution("sandbox temp path is not absolute".into()))? - .to_string(); - spec.writable_root_uris.push(sandbox_temp_uri); + let container = container.filter(|_| host); + // A host command in a Harbor task container's environment runs like + // any other command there, without an S-Code temporary directory. + let sandbox_temp = if container.is_some() { + None + } else { + let sandbox_temp = tempfile::Builder::new() + .prefix("s-code-sandbox.") + .tempdir() + .map_err(|error| RuntimeError::Execution(error.to_string()))?; + let sandbox_temp_uri = url::Url::from_directory_path(sandbox_temp.path()) + .map_err(|_| RuntimeError::Execution("sandbox temp path is not absolute".into()))? + .to_string(); + spec.writable_root_uris.push(sandbox_temp_uri); + Some(sandbox_temp) + }; let mut command = if host { if !cfg!(any(target_os = "macos", target_os = "linux")) { return Err(RuntimeError::CapabilityUnavailable("host execution".into())); @@ -817,16 +829,26 @@ impl NativeRuntime { } #[cfg(not(unix))] command.current_dir(&cwd); - command - .env_clear() - .env("PATH", sanitized_host_extension_path(&spec.program)) - .env("TMPDIR", sandbox_temp.path()) - .kill_on_drop(true); - if std::path::Path::new(&spec.program) - .file_name() - .is_some_and(|name| name == "node") - { - command.env("OPENSSL_CONF", "/dev/null"); + command.env_clear().kill_on_drop(true); + match container { + // A Harbor task container's own command environment. S-Code adds + // no PATH, TMPDIR or OPENSSL_CONF; PWD names this command's + // directory rather than the capture's. + Some(container) => { + command.envs(container.variables()).env("PWD", &cwd); + } + None => { + command.env("PATH", sanitized_host_extension_path(&spec.program)); + if let Some(sandbox_temp) = &sandbox_temp { + command.env("TMPDIR", sandbox_temp.path()); + } + if std::path::Path::new(&spec.program) + .file_name() + .is_some_and(|name| name == "node") + { + command.env("OPENSSL_CONF", "/dev/null"); + } + } } for (name, value) in &spec.environment_handles { command.env(name, value); diff --git a/crates/tool-runtime/Cargo.toml b/crates/tool-runtime/Cargo.toml index 84f970f..5a4e40f 100644 --- a/crates/tool-runtime/Cargo.toml +++ b/crates/tool-runtime/Cargo.toml @@ -5,6 +5,11 @@ edition.workspace = true license.workspace = true publish.workspace = true +[features] +# Only for testing run_host's lookup with a loaded container environment. The +# mode itself is enabled through s-code-daemon's feature of the same name. +harbor-container-env = ["s-code-platform-runtime/harbor-container-env"] + [dependencies] ignore.workspace = true regex.workspace = true diff --git a/crates/tool-runtime/src/lib.rs b/crates/tool-runtime/src/lib.rs index 273dff0..2b93e71 100644 --- a/crates/tool-runtime/src/lib.rs +++ b/crates/tool-runtime/src/lib.rs @@ -1,4 +1,5 @@ use regex::Regex; +use s_code_platform_runtime::container_environment::{self, ContainerEnvironment}; use s_code_platform_runtime::sensitive_paths::sensitive_path; use s_code_platform_runtime::{PlatformRuntime, ProcessOutput, ProcessSpec, RuntimeError}; use serde::{Deserialize, Deserializer, Serialize}; @@ -832,13 +833,19 @@ impl ToolRuntime { } else { find_program_in( program, - self.program_search_path.iter().cloned(), + host_search_path( + container_environment::installed(), + &self.program_search_path, + ), &platform_executable_extensions(), ) .ok_or_else(|| ToolError::Invalid(format!("program not found on PATH: {program}")))? }; // Do not inherit daemon credentials or process environment. Full grants // host filesystem/network authority, not access to daemon env secrets. + // A Harbor bundle's daemon instead gives commands the task container's + // captured environment, minus S-Code's and the agent's own variables + // (s_code_platform_runtime::container_environment). self.platform .execute_host(ProcessSpec { program: executable.to_string_lossy().into_owned(), @@ -2308,6 +2315,19 @@ fn shebang_interpreter(executable: &Path, search_path: &[PathBuf]) -> Option, + daemon_search_path: &[PathBuf], +) -> Vec { + container + .map(ContainerEnvironment::search_path) + .filter(|search_path| !search_path.is_empty()) + .unwrap_or_else(|| daemon_search_path.to_vec()) +} + fn find_program_in( program: &str, paths: impl IntoIterator, @@ -2909,6 +2929,59 @@ mod tests { ); } + #[cfg(feature = "harbor-container-env")] + #[test] + fn host_lookup_uses_the_container_path_before_the_daemon_path() { + use std::os::unix::ffi::OsStrExt; + + let directory = tempfile::tempdir().unwrap(); + let [first, second, daemon] = + ["first", "second", "daemon"].map(|name| directory.path().join(name)); + for bin in [&first, &second, &daemon] { + fs::create_dir(bin).unwrap(); + fs::write(bin.join("tool"), "fixture").unwrap(); + } + let load = |snapshot: &[u8]| { + use std::os::unix::fs::PermissionsExt; + + let path = directory.path().join("command.env"); + fs::write(&path, snapshot).unwrap(); + fs::set_permissions(&path, fs::Permissions::from_mode(0o600)).unwrap(); + container_environment::load( + &path, + &Default::default(), + &[], + &directory.path().join("command-env.json"), + ) + .unwrap() + .0 + }; + let path = std::env::join_paths([Path::new("relative"), &first, &second]).unwrap(); + let container = load(&[b"PATH=", path.as_bytes(), b"\0"].concat()); + assert_eq!( + find_program_in( + "tool", + host_search_path(Some(&container), std::slice::from_ref(&daemon)), + &[] + ), + Some(first.join("tool")) + ); + + // An environment without an absolute PATH entry finds programs where + // a daemon without a container environment does. + let without_path = load(b"HOME=/home/agent\0"); + for container in [Some(&without_path), None] { + assert_eq!( + find_program_in( + "tool", + host_search_path(container, std::slice::from_ref(&daemon)), + &[] + ), + Some(daemon.join("tool")) + ); + } + } + #[cfg(unix)] #[test] fn symlinked_executable_is_staged_without_its_siblings() { diff --git a/crates/tool-runtime/tests/harbor_container_env.rs b/crates/tool-runtime/tests/harbor_container_env.rs new file mode 100644 index 0000000..2375765 --- /dev/null +++ b/crates/tool-runtime/tests/harbor_container_env.rs @@ -0,0 +1,69 @@ +//! Harbor bundles only: once the daemon installs a task container's command +//! environment, a Full-mode host command is found on that environment's PATH +//! and runs in exactly that environment. Integration tests run in their own +//! process, so installing the process-wide environment is safe here. +#![cfg(all( + feature = "harbor-container-env", + any(target_os = "macos", target_os = "linux") +))] + +use s_code_platform_runtime::{NativeRuntime, container_environment}; +use s_code_tool_runtime::ToolRuntime; +use std::{ + collections::BTreeSet, ffi::OsString, os::unix::fs::PermissionsExt, sync::Arc, time::Duration, +}; + +#[tokio::test] +async fn host_commands_use_the_installed_container_environment() { + let directory = tempfile::tempdir().unwrap(); + let bin = directory.path().join("bin"); + let home = directory.path().join("home"); + let workspace = directory.path().join("workspace"); + for path in [&bin, &home, &workspace] { + std::fs::create_dir(path).unwrap(); + } + // `probe` exists only on the container PATH, so finding it proves the + // lookup uses that PATH rather than the daemon's. + std::os::unix::fs::symlink("/usr/bin/env", bin.join("probe")).unwrap(); + + let path = format!("{}:/usr/bin:/bin", bin.display()); + let snapshot = directory.path().join("command.env"); + std::fs::write( + &snapshot, + format!( + "PATH={path}\0HOME={}\0CONTAINER_MARKER=kept\0S_CODE_CANARY=dropped\0\ + AGENT_ONLY=dropped\0PWD=/capture\0", + home.display() + ), + ) + .unwrap(); + std::fs::set_permissions(&snapshot, std::fs::Permissions::from_mode(0o600)).unwrap(); + let (environment, _) = container_environment::load( + &snapshot, + &BTreeSet::from([OsString::from("AGENT_ONLY")]), + &[], + &directory.path().join("command-env.json"), + ) + .unwrap(); + container_environment::install(environment).unwrap(); + + let workspace = workspace.canonicalize().unwrap(); + let uri = url::Url::from_directory_path(&workspace).unwrap(); + let runtime = ToolRuntime::open(uri.as_str(), Arc::new(NativeRuntime)).unwrap(); + let output = runtime + .run_host("probe", Vec::new(), Duration::from_secs(10), 64 * 1024) + .await + .unwrap(); + assert_eq!(output.exit_code, Some(0), "{}", output.stderr); + let mut variables = output.stdout.lines().collect::>(); + variables.sort_unstable(); + assert_eq!( + variables, + [ + "CONTAINER_MARKER=kept".to_string(), + format!("HOME={}", home.display()), + format!("PATH={path}"), + format!("PWD={}", workspace.display()), + ] + ); +} diff --git a/docs-site/content/guides/tools-permissions.md b/docs-site/content/guides/tools-permissions.md index f3f7da4..af7d5b4 100644 --- a/docs-site/content/guides/tools-permissions.md +++ b/docs-site/content/guides/tools-permissions.md @@ -247,7 +247,9 @@ approves local edits and commands; `run_command` runs directly on the host witho an OS sandbox, with network enabled and access outside the project. Its `sandbox_profile` and `network_enabled` arguments do not restrict this host mode. The structured file tools remain workspace-bound. The process environment is -cleared; daemon credentials are not inherited. External integration approvals, +cleared; daemon credentials are not inherited. (The Harbor benchmark bundle is +the one exception: it gives these commands the task container's environment, as +`integrations/harbor/README.md` describes.) External integration approvals, installation trust, and Plan/Review tool restrictions remain in force. Explicit hard file protections always win. If any are active, host commands, diff --git a/docs/guides/tools-permissions.md b/docs/guides/tools-permissions.md index f3f7da4..af7d5b4 100644 --- a/docs/guides/tools-permissions.md +++ b/docs/guides/tools-permissions.md @@ -247,7 +247,9 @@ approves local edits and commands; `run_command` runs directly on the host witho an OS sandbox, with network enabled and access outside the project. Its `sandbox_profile` and `network_enabled` arguments do not restrict this host mode. The structured file tools remain workspace-bound. The process environment is -cleared; daemon credentials are not inherited. External integration approvals, +cleared; daemon credentials are not inherited. (The Harbor benchmark bundle is +the one exception: it gives these commands the task container's environment, as +`integrations/harbor/README.md` describes.) External integration approvals, installation trust, and Plan/Review tool restrictions remain in force. Explicit hard file protections always win. If any are active, host commands, diff --git a/integrations/harbor/README.md b/integrations/harbor/README.md index 50a6a12..a547ad0 100644 --- a/integrations/harbor/README.md +++ b/integrations/harbor/README.md @@ -78,9 +78,10 @@ harbor run \ `bundle_path`, `bundle_sha256`, `model_base_url`, `credential_env`, and the Harbor `--model` value are required. The credential variable must have a -nonempty value, including when the endpoint does not authenticate. The adapter -passes the model name through to S-Code and records only the credential handle, -not its value. Optional agent kwargs: +nonempty value, including when the endpoint does not authenticate, and a name +that starts with `S_CODE_HARBOR_`. The adapter passes the model name through to +S-Code and records only the credential handle, not its value. Optional agent +kwargs: | Kwarg | Default | Meaning | | --- | --- | --- | @@ -108,7 +109,15 @@ Harbor's agent logs: - `stderr.log`: S-Code diagnostic output. - `final.txt`: the final agent message. - `provenance.json`: adapter, bundle, model, endpoint, and trial metadata. + `adapter.source_sha256` is what + `LC_ALL=C sh -c 'sha256sum __init__.py agent.py runtime.py | sha256sum'` + prints in this directory, and `execution.unmanaged_s_code_env` names any + other `S_CODE_*` variables passed with `--agent-env`. - `daemon.log`: the S-Code daemon's log, including why model requests failed. +- `command-env.json`: the names of the variables S-Code's commands receive and + a SHA-256 of them, never the values. The digest covers the `NAME=VALUE` + records in byte order of their names, joined by single NUL bytes with no + trailing NUL. Commands also receive `PWD`, which is not listed. If no model request in the run gets a successful response, provenance records `model_unreachable` or `model_rejected`. The adapter raises Harbor's @@ -123,3 +132,34 @@ adapter also has S-Code send its system messages as one leading message, because some chat templates, including Qwen3.6's, reject later ones. The adapter performs one attempt and does not provide retries, resume, persistent memory, or ATIF conversion. + +## Command environment + +Task images often install tools under `/usr/local/bin`, add directories with +`ENV PATH`, or set `HOME`, `JAVA_HOME` or `PYTHONPATH`. S-Code normally runs +Full-mode commands with a cleared environment and a minimal `PATH`, so the +Harbor bundle has a container environment mode instead: + +- Before it sets anything of its own, the runner saves its environment as the + agent user: the environment an ordinary Harbor command gets in the task + container, minus every `--agent-env` variable. The daemon loads that file + once at startup and deletes it. +- Full-mode commands get that environment, with `PWD` set to their working + directory, and programs are found on its `PATH`. S-Code adds no `PATH`, + `TMPDIR` or `OPENSSL_CONF` of its own. +- Commands never receive `--agent-env` variables (the credential among them), + `S_CODE_*` variables, any variable whose value is the model credential, or + the shell's `PWD`, `OLDPWD`, `SHLVL` and `_`. A task variable that shares its + name with an `--agent-env` variable is dropped as well. The daemon loads the + snapshot only if it is a regular file that no other user can read. +- `credential_env` must start with `S_CODE_HARBOR_`, so the credential is also + excluded by its prefix and cannot collide with S-Code's own settings. +- Only this bundle can enable the mode. `build-bundle.sh` builds the daemon + with the `harbor-container-env` Cargo feature, which other builds leave off, + and the adapter refuses a bundle without it. Sandboxed commands, extensions + and other S-Code builds are unchanged. + +Provenance copies the names and digest from `command-env.json` into +`execution.command_environment`, and sets `execution.host_command_env` to +`container` once that summary is read, or to `unverified` with the reason in +`execution.command_environment_error`. diff --git a/integrations/harbor/agent.py b/integrations/harbor/agent.py index 87d593c..55dba20 100644 --- a/integrations/harbor/agent.py +++ b/integrations/harbor/agent.py @@ -9,6 +9,7 @@ from __future__ import annotations import asyncio +import hashlib import json import math import re @@ -34,11 +35,16 @@ from harbor.models.trial.config import TrialConfig from harbor.models.trial.paths import EnvironmentPaths -from .runtime import BundleManifest, validate_bundle, validate_sha256 +from .runtime import ( + HARBOR_CONTAINER_ENV_FEATURE, + BundleManifest, + validate_bundle, + validate_sha256, +) _SUPPORTED_HARBOR_VERSION = "0.17.0" -_ADAPTER_VERSION = "0.1.0" +_ADAPTER_RELEASE = "0.1.0" _CREDENTIAL_ENV_RE = re.compile(r"^[A-Z_][A-Z0-9_]*$") _SENSITIVE_ENV_TOKEN_RE = re.compile( r"KEY|SECRET|TOKEN|PASSWORD|CREDENTIAL|AUTH", @@ -63,6 +69,8 @@ _CLI_TIMEOUT_MARGIN_SEC = 90 _CLEANUP_SERVER_TIMEOUT_SEC = 10 _CLEANUP_CLIENT_TIMEOUT_SEC = 15 +_MAX_COMMAND_ENV_SUMMARY_BYTES = 2 * 1024 * 1024 +_AGENT_ENV_NAMES_VAR = "S_CODE_HARBOR_AGENT_ENV_NAMES" _DISABLED_TOOLS = ( "web_open", "pdf_read", @@ -77,12 +85,17 @@ _PROMPT_PATH = f"{_PRIVATE_DIR}/instruction.txt" _PROVENANCE_SEED_PATH = f"{_PRIVATE_DIR}/provenance.seed.json" _EXIT_STATUS_PATH = f"{_PRIVATE_DIR}/exit.status" +# The runner snapshots the container environment here; the daemon loads it +# once, deletes it, and writes a names-and-hash summary. +_COMMAND_ENV_FILE_PATH = f"{_PRIVATE_DIR}/command.env" +_COMMAND_ENV_SUMMARY_PATH = f"{_PRIVATE_DIR}/run/command-env.json" _EVENTS_PATH = (EnvironmentPaths.agent_dir / "events.jsonl").as_posix() _STDERR_PATH = (EnvironmentPaths.agent_dir / "stderr.log").as_posix() _FINAL_PATH = (EnvironmentPaths.agent_dir / "final.txt").as_posix() _PROVENANCE_PATH = (EnvironmentPaths.agent_dir / "provenance.json").as_posix() _DAEMON_LOG_PATH = (EnvironmentPaths.agent_dir / "daemon.log").as_posix() +_COMMAND_ENV_PATH = (EnvironmentPaths.agent_dir / "command-env.json").as_posix() _STATE_DAEMON_LOG_PATH = f"{_PRIVATE_DIR}/state/logs/daemon.log" @@ -94,6 +107,27 @@ set -u umask 077 +# Snapshot the environment an ordinary Harbor command gets here before this +# script sets a variable of its own, leaving out every variable Harbor passed +# to the agent (the model credential among them). The daemon filters it again, +# loads it once and deletes it. +s_code_runner_captured=1 +if [ -n "${{S_CODE_HARBOR_COMMAND_ENV_FILE:-}}" ] && + rm -f "$S_CODE_HARBOR_COMMAND_ENV_FILE" && + ( + set -f + IFS=, + set -- + for s_code_runner_name in ${{S_CODE_HARBOR_AGENT_ENV_NAMES:-}}; do + set -- "$@" -u "$s_code_runner_name" + done + unset s_code_runner_name + exec env "$@" -0 + ) > "$S_CODE_HARBOR_COMMAND_ENV_FILE" +then + s_code_runner_captured=0 +fi + events={shlex.quote(_EVENTS_PATH)} stderr_log={shlex.quote(_STDERR_PATH)} final={shlex.quote(_FINAL_PATH)} @@ -102,6 +136,7 @@ prompt={shlex.quote(_PROMPT_PATH)} exit_status={shlex.quote(_EXIT_STATUS_PATH)} daemon_log={shlex.quote(_DAEMON_LOG_PATH)} +command_env={shlex.quote(_COMMAND_ENV_PATH)} record_exit() {{ recorded_status=$1 @@ -129,6 +164,7 @@ }} fail() {{ + [ -z "${{S_CODE_HARBOR_COMMAND_ENV_FILE:-}}" ] || rm -f "$S_CODE_HARBOR_COMMAND_ENV_FILE" printf '%s\n' "$1" >> "$stderr_log" record_exit 1 0 1 tail -c 65536 "$stderr_log" >&2 2>/dev/null || true @@ -156,6 +192,9 @@ instruction=$(cat "$prompt"; printf x) || fail "S-Code could not read prompt data" instruction=${{instruction%x}} +[ "$s_code_runner_captured" -eq 0 ] || + fail "S-Code could not capture the command environment" + status=0 {shlex.quote(_INSTALL_DIR + "/s-code")} exec \ --raw-events-jsonl \ @@ -171,12 +210,23 @@ {shlex.quote(_INSTALL_DIR + "/s-code")} stop \ >> "$stderr_log" 2>&1 || cleanup_status=$? +# The snapshot holds every value, so never leave one S-Code did not delete. +[ -z "${{S_CODE_HARBOR_COMMAND_ENV_FILE:-}}" ] || rm -f "$S_CODE_HARBOR_COMMAND_ENV_FILE" + # Keep the daemon's own log with the trial; it records why model requests failed. cp {shlex.quote(_STATE_DAEMON_LOG_PATH)} "$daemon_log" 2>/dev/null || true +# The command-environment summary records names and a hash, never values. +command_env_summary=${{S_CODE_HARBOR_COMMAND_ENV_SUMMARY:-}} +if [ -n "$command_env_summary" ] && [ -f "$command_env_summary" ] && + [ ! -L "$command_env_summary" ] +then + cp "$command_env_summary" "$command_env" 2>/dev/null || true +fi runner_status=$status record_exit "$status" "$cleanup_status" "$runner_status" -chmod 600 "$events" "$stderr_log" "$final" "$provenance" "$daemon_log" 2>/dev/null || true +chmod 600 "$events" "$stderr_log" "$final" "$provenance" "$daemon_log" "$command_env" \ + 2>/dev/null || true if [ "$runner_status" -ne 0 ]; then # Feed only S-Code's own bounded stderr tail back to Harbor so @@ -250,6 +300,12 @@ def _model_name(value: object) -> str: def _credential_env(value: object) -> str: if not isinstance(value, str) or not _CREDENTIAL_ENV_RE.fullmatch(value): raise ValueError("credential_env must be an uppercase environment-variable name") + if not value.startswith("S_CODE_HARBOR_"): + raise ValueError( + "credential_env must start with S_CODE_HARBOR_: S-Code keeps S_CODE_* " + "names out of its commands' environment, and this prefix cannot " + "collide with S-Code's own settings" + ) if _SENSITIVE_ENV_TOKEN_RE.search(value) is None: raise ValueError( "credential_env must contain KEY, SECRET, TOKEN, PASSWORD, " @@ -258,6 +314,12 @@ def _credential_env(value: object) -> str: return value +def _agent_env_names(env: dict[str, str]) -> str: + """Name every variable Harbor passes to the agent, for the daemon to drop.""" + + return ",".join(sorted({*env, _AGENT_ENV_NAMES_VAR})) + + def _harbor_version() -> str: actual = distribution_version("harbor") if actual != _SUPPORTED_HARBOR_VERSION: @@ -268,6 +330,29 @@ def _harbor_version() -> str: return actual +_ADAPTER_SOURCES = ("__init__.py", "agent.py", "runtime.py") + + +def _adapter_source_sha256(directory: Path) -> str: + """Hash the sources a run uses, as ``LC_ALL=C sha256sum`` of them piped to + ``sha256sum`` does.""" + + digest = hashlib.sha256() + for name in _ADAPTER_SOURCES: + path = directory / name + if not path.is_file(): + continue + source_sha256 = hashlib.sha256(path.read_bytes()).hexdigest() + digest.update(f"{source_sha256} {name}\n".encode("utf-8")) + return digest.hexdigest() + + +# The adapter decides timeouts, tools, and classification, so runs from +# different adapter code must stay distinguishable. +_ADAPTER_SOURCE_SHA256 = _adapter_source_sha256(Path(__file__).resolve().parent) +_ADAPTER_VERSION = f"{_ADAPTER_RELEASE}+{_ADAPTER_SOURCE_SHA256[:12]}" + + def _resolved_harbor_agent_timeout_sec(environment: BaseEnvironment) -> int: """Reproduce Harbor 0.17's effective single-step agent deadline.""" @@ -321,6 +406,7 @@ def _runtime_preflight_command(required_glibc: str) -> str: echo 'S-Code requires /usr/bin/nohup or /bin/nohup' >&2 exit 1 }} +env -0 >/dev/null || {{ echo 'S-Code requires env -0' >&2; exit 1; }} [ "$(uname -s)" = Linux ] || {{ echo 'S-Code requires Linux' >&2; exit 1; }} [ "$(uname -m)" = x86_64 ] || {{ echo 'S-Code requires x86_64' >&2; exit 1; }} actual_glibc=$(getconf GNU_LIBC_VERSION 2>/dev/null) || {{ @@ -550,6 +636,34 @@ def _parse_exit_status( return exit_code, cleanup_code, runner_code +def _command_environment(path: Path) -> tuple[dict[str, Any] | None, str | None]: + """Read the daemon's command-environment summary: names and a hash only.""" + + try: + if path.stat().st_size > _MAX_COMMAND_ENV_SUMMARY_BYTES: + return None, "command-env.json is too large" + summary = json.loads(path.read_bytes()) + except OSError: + return None, "command-env.json is unavailable" + except ValueError: + return None, "command-env.json is not valid JSON" + names = summary.get("names") if isinstance(summary, dict) else None + if ( + not isinstance(summary, dict) + or set(summary) != {"mode", "count", "names", "sha256"} + or summary["mode"] != "container" + or not isinstance(names, list) + or not all(isinstance(name, str) and name and "=" not in name for name in names) + or names != sorted(set(names)) + or type(summary["count"]) is not int + or summary["count"] != len(names) + or not isinstance(summary["sha256"], str) + or re.fullmatch(r"[0-9a-f]{64}", summary["sha256"]) is None + ): + return None, "command-env.json is not a valid summary" + return {"count": len(names), "names": names, "sha256": summary["sha256"]}, None + + def _manifest_provenance(manifest: BundleManifest) -> dict[str, Any]: return { "schema_version": manifest.schema_version, @@ -561,6 +675,7 @@ def _manifest_provenance(manifest: BundleManifest) -> dict[str, Any]: "glibc_version": manifest.glibc_version, "lock_sha256s": dict(manifest.lock_sha256s), "tool_versions": dict(manifest.tool_versions), + "cargo_features": list(manifest.cargo_features), "files": { name: { "sha256": metadata.sha256, @@ -687,6 +802,11 @@ def __init__( "credential_env must name a non-empty value supplied through " "Harbor's agent env/extra_env" ) + if any("," in name for name in supplied_env): + raise ValueError( + "agent env names must not contain commas; S-Code could not " + "keep them out of the environment its commands receive" + ) managed_env = { "S_CODE_MODEL_PROVIDER": "openai_compatible", "S_CODE_MODEL_BASE_URL": self._model_base_url, @@ -714,21 +834,37 @@ def __init__( "S_CODE_TURN_MAX_TOTAL_TOKENS": str(self._max_total_tokens), "S_CODE_TURN_DISABLED_TOOLS": ",".join(_DISABLED_TOOLS), "S_CODE_HARBOR_CLI_TIMEOUT_SEC": str(self._cli_timeout_sec), - "NO_COLOR": "1", - "TERM": "dumb", + "S_CODE_HARBOR_COMMAND_ENV_FILE": _COMMAND_ENV_FILE_PATH, + "S_CODE_HARBOR_COMMAND_ENV_SUMMARY": _COMMAND_ENV_SUMMARY_PATH, } if self._temperature is not None: managed_env["S_CODE_MODEL_TEMPERATURE"] = str(self._temperature) if self._max_output_tokens is not None: managed_env["S_CODE_MODEL_MAX_OUTPUT_TOKENS"] = str(self._max_output_tokens) + managed_env[_AGENT_ENV_NAMES_VAR] = _agent_env_names( + {**supplied_env, **managed_env} + ) for name, expected in managed_env.items(): if name in supplied_env and supplied_env[name] != expected: raise ValueError(f"{name} is managed by SCodeAgent and cannot be overridden") + # S-Code reads these too, so provenance names them (never their values). + self._unmanaged_s_code_env = sorted( + name + for name in supplied_env + if name.startswith("S_CODE_") + and name not in managed_env + and name != self._credential_env + ) supplied_env.update(managed_env) # Validation is repeated immediately before upload in install(); doing it # here makes configuration errors fail before Harbor starts a container. self._manifest = validate_bundle(self._bundle_path, self._bundle_sha256) + if HARBOR_CONTAINER_ENV_FEATURE not in self._manifest.cargo_features: + raise ValueError( + "bundle was built without the harbor-container-env Cargo feature; " + "rebuild it with integrations/harbor/build-bundle.sh" + ) self._agent_owner: str | None = None self._has_run = False super().__init__( @@ -805,6 +941,7 @@ def _owner_command(self, *paths: str) -> str: async def install(self, environment: BaseEnvironment) -> None: self._set_effective_timeouts(_resolved_harbor_agent_timeout_sec(environment)) self._extra_env.update(self._effective_timeout_env()) + self._extra_env[_AGENT_ENV_NAMES_VAR] = _agent_env_names(self._extra_env) await self._resolve_agent_owner(environment) await self.exec_as_root( environment, @@ -854,6 +991,7 @@ def _provenance(self) -> dict[str, Any]: "adapter": { "name": self.name(), "version": _ADAPTER_VERSION, + "source_sha256": _ADAPTER_SOURCE_SHA256, "harbor_version": self._harbor_version, "supports_atif": False, }, @@ -868,6 +1006,10 @@ def _provenance(self) -> dict[str, Any]: "attempts": 1, "ephemeral": True, "permission_mode": "full", + "host_command_env": "pending", + "command_environment": None, + "command_environment_error": "pending", + "unmanaged_s_code_env": list(self._unmanaged_s_code_env), "output_mode": "raw_events_jsonl", "max_model_calls": self._max_model_calls, "max_tool_calls": self._max_tool_calls, @@ -907,6 +1049,7 @@ def _provenance(self) -> dict[str, Any]: "final": _FINAL_PATH, "provenance": _PROVENANCE_PATH, "daemon_log": _DAEMON_LOG_PATH, + "command_env": _COMMAND_ENV_PATH, }, } @@ -923,6 +1066,7 @@ async def _finalize_provenance( local = Path(directory) status_path = local / "exit.status" events_path = local / "events.jsonl" + command_env_path = local / "command-env.json" try: await environment.download_file(_EXIT_STATUS_PATH, status_path) @@ -952,6 +1096,13 @@ async def _finalize_provenance( await environment.download_file(_EVENTS_PATH, events_path) except Exception: pass + try: + await environment.download_file(_COMMAND_ENV_PATH, command_env_path) + except Exception: + pass + command_environment, command_environment_error = _command_environment( + command_env_path + ) result = _classify_event_stream(events_path, exit_code) result["process_status_validation"] = status_validation usage = result["usage"] @@ -969,6 +1120,11 @@ async def _finalize_provenance( provenance["execution"]["post_run_cleanup_exit_code"] = post_cleanup_code provenance["execution"]["post_run_cleanup_status"] = post_cleanup_status provenance["execution"]["runner_exit_code"] = runner_code + provenance["execution"]["host_command_env"] = ( + "container" if command_environment is not None else "unverified" + ) + provenance["execution"]["command_environment"] = command_environment + provenance["execution"]["command_environment_error"] = command_environment_error provenance["result"] = result contents = json.dumps( provenance, @@ -988,9 +1144,15 @@ async def _bounded_cancel_cleanup(self, environment: BaseEnvironment) -> None: command = ( "status=0; " f"{shlex.quote(_INSTALL_DIR + '/s-code')} stop >/dev/null 2>&1 || status=$?; " + f"rm -f {shlex.quote(_COMMAND_ENV_FILE_PATH)}; " f"cp {shlex.quote(_STATE_DAEMON_LOG_PATH)} " f"{shlex.quote(_DAEMON_LOG_PATH)} 2>/dev/null || true; " - f"chmod 600 {shlex.quote(_DAEMON_LOG_PATH)} 2>/dev/null || true; " + f"if [ -f {shlex.quote(_COMMAND_ENV_SUMMARY_PATH)} ] && " + f"[ ! -L {shlex.quote(_COMMAND_ENV_SUMMARY_PATH)} ]; then " + f"cp {shlex.quote(_COMMAND_ENV_SUMMARY_PATH)} " + f"{shlex.quote(_COMMAND_ENV_PATH)} 2>/dev/null || true; fi; " + f"chmod 600 {shlex.quote(_DAEMON_LOG_PATH)} " + f"{shlex.quote(_COMMAND_ENV_PATH)} 2>/dev/null || true; " 'exit "$status"' ) try: @@ -1057,6 +1219,7 @@ async def run( "final": _FINAL_PATH, "provenance": _PROVENANCE_PATH, "daemon_log": _DAEMON_LOG_PATH, + "command_env": _COMMAND_ENV_PATH, "supports_atif": False, } } diff --git a/integrations/harbor/build-bundle.sh b/integrations/harbor/build-bundle.sh index f36fc69..331155e 100755 --- a/integrations/harbor/build-bundle.sh +++ b/integrations/harbor/build-bundle.sh @@ -8,6 +8,7 @@ Usage: integrations/harbor/build-bundle.sh Build a normalized S-Code runtime archive for x86_64 Linux with glibc. The Git worktree must be clean and all pinned build tools must already exist. +The daemon is built with the Harbor-only harbor-container-env Cargo feature. Environment: S_CODE_HARBOR_OUTPUT_DIR Output directory (default: .work/harbor) @@ -40,6 +41,9 @@ ROOT="$(CDPATH= cd -- "$(dirname -- "$0")/../.." && pwd -P)" [ "$(uname -s)" = Linux ] || fail "bundles can only be built on Linux" [ "$(uname -m)" = x86_64 ] || fail "bundles require the x86_64 architecture" TARGET=x86_64-unknown-linux-gnu +# Only this bundle enables the container environment mode for agent commands; +# the manifest records the features so the adapter can refuse other builds. +CARGO_FEATURES=s-code-daemon/harbor-container-env for required_command in \ cargo cmp cut find getconf git install mkdir mktemp mv node npm python3 rustc sed uname @@ -150,7 +154,7 @@ git -C "$ROOT" diff --quiet -- web/app.js || echo "Building S-Code runtime with Rust $PINNED_RUST..." cargo build --locked --release --manifest-path "$ROOT/Cargo.toml" \ - -p s-code-daemon -p s-code-cli + -p s-code-daemon -p s-code-cli --features "$CARGO_FEATURES" for runtime_file in s-code-daemon s-code-cli s-code-pdf-worker; do source_file="$CARGO_TARGET_DIR/release/$runtime_file" @@ -168,7 +172,7 @@ python3 - \ "$STAGE_DIR" "$COMMIT" "$TREE" "$TARGET" \ "$ROOT/Cargo.lock" "$ROOT/web/package-lock.json" \ "$RUSTC_VERSION" "$CARGO_VERSION" "$NODE_VERSION" "$NPM_VERSION" \ - "$PYTHON_VERSION" "$ZLIB_VERSION" "$GLIBC_VERSION" <<'PY' + "$PYTHON_VERSION" "$ZLIB_VERSION" "$GLIBC_VERSION" "$CARGO_FEATURES" <<'PY' import hashlib import json import os @@ -179,7 +183,7 @@ import sys ( stage_arg, commit, tree, target, cargo_lock_arg, web_lock_arg, rustc_version, cargo_version, node_version, npm_version, - python_version, zlib_version, glibc_version, + python_version, zlib_version, glibc_version, cargo_features, ) = sys.argv[1:] stage = Path(stage_arg) @@ -211,6 +215,7 @@ for name in runtime_names: manifest = { "build": { "cargo": cargo_version, + "cargo_features": sorted(cargo_features.split(",")), "glibc": glibc_version, "node": node_version, "npm": npm_version, diff --git a/integrations/harbor/runtime.py b/integrations/harbor/runtime.py index fcc16b9..8b16503 100644 --- a/integrations/harbor/runtime.py +++ b/integrations/harbor/runtime.py @@ -39,6 +39,9 @@ "s-code-daemon", "s-code-pdf-worker", ) +# The Harbor container environment mode exists only in daemons built with this +# feature; build-bundle.sh is the one build that enables it. +HARBOR_CONTAINER_ENV_FEATURE = "s-code-daemon/harbor-container-env" _EXPECTED_ARCHIVE_MEMBERS = (MANIFEST_NAME,) + REQUIRED_RUNTIME_FILES _EXPECTED_MEMBER_MODES = MappingProxyType( @@ -47,6 +50,7 @@ _SHA256_RE = re.compile(r"[0-9a-f]{64}\Z") _GIT_OBJECT_RE = re.compile(r"(?:[0-9a-f]{40}|[0-9a-f]{64})\Z") _GLIBC_RE = re.compile(r"glibc [0-9]+(?:\.[0-9]+)+\Z") +_CARGO_FEATURE_RE = re.compile(r"[A-Za-z0-9_-]+/[A-Za-z0-9_][A-Za-z0-9_+.-]*\Z") _TOOL_KEYS = ("cargo", "node", "npm", "python", "rustc", "zlib") _LOCK_KEYS = ("Cargo.lock", "web/package-lock.json") _MAX_ARCHIVE_BYTES = 512 * 1024 * 1024 @@ -84,6 +88,7 @@ class BundleManifest: glibc_version: str lock_sha256s: Mapping[str, str] tool_versions: Mapping[str, str] + cargo_features: Tuple[str, ...] files: Mapping[str, BundleFile] archive_sha256: str @@ -270,8 +275,13 @@ def _parse_manifest( for name in _LOCK_KEYS } + raw_build = root["build"] + if isinstance(raw_build, dict) and "cargo_features" not in raw_build: + # Bundles built before the field existed have no features, so the + # adapter refuses them with a rebuild message rather than a key error. + raw_build = {**raw_build, "cargo_features": []} build = _require_exact_keys( - root["build"], _TOOL_KEYS + ("glibc",), "manifest.build" + raw_build, _TOOL_KEYS + ("cargo_features", "glibc"), "manifest.build" ) tool_versions = { name: _manifest_text(build[name], f"manifest.build.{name}") for name in _TOOL_KEYS @@ -281,6 +291,19 @@ def _parse_manifest( raise BundleValidationError( "manifest.build.glibc must have the canonical form 'glibc '" ) + cargo_features = build["cargo_features"] + if ( + not isinstance(cargo_features, list) + or any( + not isinstance(feature, str) or _CARGO_FEATURE_RE.fullmatch(feature) is None + for feature in cargo_features + ) + or cargo_features != sorted(set(cargo_features)) + ): + raise BundleValidationError( + "manifest.build.cargo_features must be a sorted array of unique " + "'package/feature' names" + ) raw_files = _require_exact_keys(root["files"], REQUIRED_RUNTIME_FILES, "manifest.files") files: Dict[str, BundleFile] = {} @@ -308,6 +331,7 @@ def _parse_manifest( glibc_version=glibc_version, lock_sha256s=MappingProxyType(lock_sha256s), tool_versions=MappingProxyType(tool_versions), + cargo_features=tuple(cargo_features), files=MappingProxyType(files), archive_sha256=archive_sha256, ) diff --git a/tests/test-harbor-adapter.py b/tests/test-harbor-adapter.py index f83f4f2..e2e9df9 100644 --- a/tests/test-harbor-adapter.py +++ b/tests/test-harbor-adapter.py @@ -4,6 +4,7 @@ from __future__ import annotations import asyncio +import hashlib import json import os from pathlib import Path, PurePosixPath @@ -318,11 +319,20 @@ def _manifest(archive_sha256: str) -> SimpleNamespace: "rustc": "rustc 1.89.0", "zlib": "1.3.1", }, + cargo_features=(agent.HARBOR_CONTAINER_ENV_FEATURE,), files=files, archive_sha256=archive_sha256, ) +_COMMAND_ENV_SUMMARY = { + "count": 2, + "mode": "container", + "names": ["HOME", "PATH"], + "sha256": "f" * 64, +} + + class AdapterTests(unittest.TestCase): def setUp(self) -> None: temporary = tempfile.TemporaryDirectory(prefix="s-code-harbor-adapter.") @@ -449,22 +459,21 @@ def test_url_and_credential_validation(self) -> None: agent._model_base_url(url) for name in ( - "API_KEY", - "MODEL_SECRET", - "ACCESS_TOKEN", - "MODEL_PASSWORD", - "MODEL_CREDENTIAL", - "MODEL_AUTH", + "S_CODE_HARBOR_API_KEY", + "S_CODE_HARBOR_MODEL_SECRET", + "S_CODE_HARBOR_ACCESS_TOKEN", + "S_CODE_HARBOR_MODEL_PASSWORD", + "S_CODE_HARBOR_CREDENTIAL", + "S_CODE_HARBOR_MODEL_AUTH", ): with self.subTest(valid=name): self.assertEqual(agent._credential_env(name), name) for name in ( - "MODEL_API", - "model_secret", - "MODEL_secret", - "9_API_KEY", - "API-KEY", + "S_CODE_HARBOR_MODEL_API", + "s_code_harbor_model_secret", + "S_CODE_HARBOR_MODEL_secret", + "S_CODE_HARBOR_API-KEY", "", None, ): @@ -472,6 +481,20 @@ def test_url_and_credential_validation(self) -> None: with self.assertRaises(ValueError): agent._credential_env(name) + # Commands never receive S_CODE_* names, so the credential needs one, + # and S_CODE_HARBOR_ keeps it clear of S-Code's own settings. + for name in ( + "API_KEY", + "HARBOR_MODEL_TOKEN", + "S_CODEX_API_KEY", + "S_CODE_API_KEY", + "S_CODE_TOKEN", + "S_CODE_STORAGE_ENCRYPTION_KEY_BASE64", + ): + with self.subTest(unprefixed=name): + with self.assertRaisesRegex(ValueError, "must start with S_CODE_HARBOR_"): + agent._credential_env(name) + with self.assertRaisesRegex(ValueError, "non-empty value"): self.new_agent(extra_env={"S_CODE_HARBOR_API_KEY": ""}) @@ -479,7 +502,9 @@ def test_constructor_manages_environment_and_timeout_hierarchy(self) -> None: instance = self.new_agent(extra_fixture="passed-through") initialized = instance.base_init self.assertEqual(initialized["model_name"], "fixture-model") - self.assertEqual(initialized["version"], "0.1.0") + self.assertEqual( + initialized["version"], f"0.1.0+{agent._ADAPTER_SOURCE_SHA256[:12]}" + ) self.assertEqual(initialized["kwargs"], {"extra_fixture": "passed-through"}) environment = initialized["extra_env"] self.assertEqual(environment["S_CODE_HARBOR_API_KEY"], "top-secret-fixture") @@ -508,6 +533,96 @@ def test_constructor_manages_environment_and_timeout_hierarchy(self) -> None: } ) + def test_command_environment_settings_are_managed(self) -> None: + instance = self.new_agent( + credential_env="S_CODE_HARBOR_CANARY_API_KEY", + extra_env={ + "S_CODE_HARBOR_CANARY_API_KEY": "canary-credential-value", + "HTTPS_PROXY": "http://proxy.example:3128", + }, + ) + environment = instance.base_init["extra_env"] + self.assertEqual( + environment["S_CODE_HARBOR_COMMAND_ENV_FILE"], + "/installed-agent/s-code-private/command.env", + ) + self.assertEqual( + environment["S_CODE_HARBOR_COMMAND_ENV_SUMMARY"], + "/installed-agent/s-code-private/run/command-env.json", + ) + self.assertNotIn("TERM", environment) + self.assertNotIn("NO_COLOR", environment) + # The daemon drops every name Harbor passes to the agent, including the + # credential and agent env that is not S-Code's own. + names = environment["S_CODE_HARBOR_AGENT_ENV_NAMES"] + self.assertEqual(names.split(","), sorted(environment)) + self.assertIn("S_CODE_HARBOR_CANARY_API_KEY", names.split(",")) + self.assertIn("HTTPS_PROXY", names.split(",")) + self.assertNotIn("canary-credential-value", names) + + for name in ( + "S_CODE_HARBOR_COMMAND_ENV_FILE", + "S_CODE_HARBOR_COMMAND_ENV_SUMMARY", + "S_CODE_HARBOR_AGENT_ENV_NAMES", + ): + with self.subTest(name=name): + with self.assertRaisesRegex(ValueError, "managed by SCodeAgent"): + self.new_agent( + extra_env={ + "S_CODE_HARBOR_API_KEY": "top-secret-fixture", + name: "/tmp/override", + } + ) + with self.assertRaisesRegex(ValueError, "must not contain commas"): + self.new_agent( + extra_env={"S_CODE_HARBOR_API_KEY": "top-secret-fixture", "A,B": "x"} + ) + + def test_bundle_without_the_container_env_feature_is_refused(self) -> None: + for features in ((), ("s-code-cli/fixture",)): + with self.subTest(features=features): + self.manifest.cargo_features = features + with self.assertRaisesRegex(ValueError, "harbor-container-env"): + self.new_agent() + + def test_adapter_version_identifies_the_adapter_source(self) -> None: + sources = self.directory / "adapter-source" + sources.mkdir() + (sources / "agent.py").write_bytes(b"agent fixture\n") + (sources / "__init__.py").write_bytes(b"package fixture\n") + (sources / ".#agent.py").write_bytes(b"editor lock, not a source\n") + (sources / "README.md").write_bytes(b"not a source\n") + listing = "".join( + f"{hashlib.sha256(contents).hexdigest()} {name}\n" + for name, contents in ( + ("__init__.py", b"package fixture\n"), + ("agent.py", b"agent fixture\n"), + ) + ) + expected = hashlib.sha256(listing.encode()).hexdigest() + self.assertEqual(agent._adapter_source_sha256(sources), expected) + (sources / "agent.py").write_bytes(b"changed agent\n") + self.assertNotEqual(agent._adapter_source_sha256(sources), expected) + + adapter_directory = ROOT / "integrations" / "harbor" + self.assertEqual( + agent._ADAPTER_SOURCE_SHA256, agent._adapter_source_sha256(adapter_directory) + ) + # The README's shell recipe reproduces the digest. + for tool in ("sha256sum", "shasum -a 256"): + if shutil.which(tool.split()[0]) is None: + continue + recipe = subprocess.run( + ["/bin/sh", "-c", f"{tool} __init__.py agent.py runtime.py | {tool}"], + cwd=adapter_directory, + env={**os.environ, "LC_ALL": "C"}, + check=True, + capture_output=True, + text=True, + ) + self.assertEqual(recipe.stdout.split()[0], agent._ADAPTER_SOURCE_SHA256) + break + def test_harbor_deadline_resolution_matches_cap_then_multiplier(self) -> None: environment = FakeEnvironment() environment.configure_deadline( @@ -601,6 +716,7 @@ def test_install_uses_one_archive_and_numeric_nonroot_owner(self) -> None: commands = "\n".join(command for command, _ in environment.commands) self.assertIn("getconf GNU_LIBC_VERSION", commands) + self.assertIn("env -0 >/dev/null", commands) self.assertIn(self.bundle_sha256, commands) self.assertIn("sha256sum -c -", commands) self.assertIn( @@ -617,12 +733,25 @@ def test_install_uses_one_archive_and_numeric_nonroot_owner(self) -> None: for _, contents, _ in environment.uploads: self.assertNotIn(secret, contents) + def test_install_keeps_the_agent_env_name_list_complete(self) -> None: + instance = self.new_agent() + timeout_env = instance._effective_timeout_env + instance._effective_timeout_env = lambda: { + **timeout_env(), + "S_CODE_FIXTURE_ADDED_AT_INSTALL": "1", + } + asyncio.run(instance.install(FakeEnvironment())) + names = instance._extra_env["S_CODE_HARBOR_AGENT_ENV_NAMES"].split(",") + self.assertIn("S_CODE_FIXTURE_ADDED_AT_INSTALL", names) + self.assertEqual(names, sorted(instance._extra_env)) + def _run_generated_runner( self, *, exec_status: int, cleanup_status: int, prompt: bytes, + command_env_file: str | None = "command.env", ) -> tuple[subprocess.CompletedProcess[bytes], Path, Path, Path]: case_directory = Path( tempfile.mkdtemp(prefix="runner-case.", dir=self.directory) @@ -642,6 +771,9 @@ def _run_generated_runner( case "$command" in exec) printf 'exec\\n' >> "$FAKE_CALLS" + cp "$S_CODE_HARBOR_COMMAND_ENV_FILE" "$FAKE_CAPTURED_ENV" || exit 97 + ls -ln "$S_CODE_HARBOR_COMMAND_ENV_FILE" > "$FAKE_CAPTURED_ENV_MODE" + printf '%s\\n' "$FAKE_SUMMARY" > "$S_CODE_HARBOR_COMMAND_ENV_SUMMARY" cat > "$FAKE_CAPTURED_STDIN" [ ! -s "$FAKE_CAPTURED_STDIN" ] || exit 98 last= @@ -670,6 +802,7 @@ def _run_generated_runner( agent._FINAL_PATH: str(logs / "final.txt"), agent._PROVENANCE_PATH: str(logs / "provenance.json"), agent._DAEMON_LOG_PATH: str(logs / "daemon.log"), + agent._COMMAND_ENV_PATH: str(logs / "command-env.json"), agent._PRIVATE_DIR: str(private), } rendered = agent._RUNNER @@ -694,9 +827,27 @@ def _run_generated_runner( "FAKE_EXEC_STATUS": str(exec_status), "FAKE_CLEANUP_STATUS": str(cleanup_status), "FAKE_STATE_DIR": str(private / "state"), + "FAKE_CAPTURED_ENV": str(case_directory / "captured-env"), + "FAKE_CAPTURED_ENV_MODE": str(case_directory / "captured-env-mode"), + "FAKE_SUMMARY": json.dumps(_COMMAND_ENV_SUMMARY), + "HARBOR_TASK_MARKER": "task value\nwith a newline", + # An image variable named like one of the runner's own. + "events": "image value", + # Variables Harbor passes only to the agent. + "S_CODE_HARBOR_CANARY_API_KEY": "canary-credential-value", + "AGENT_ONLY_MARKER": "agent-only-value", + "S_CODE_HARBOR_AGENT_ENV_NAMES": ( + "AGENT_ONLY_MARKER,S_CODE_HARBOR_AGENT_ENV_NAMES," + "S_CODE_HARBOR_CANARY_API_KEY" + ), "S_CODE_HARBOR_CLI_TIMEOUT_SEC": "3570", + "S_CODE_HARBOR_COMMAND_ENV_SUMMARY": str( + private / "run" / "command-env.json" + ), } ) + if command_env_file is not None: + child_env["S_CODE_HARBOR_COMMAND_ENV_FILE"] = str(private / command_env_file) result = subprocess.run( ["/bin/sh", str(runner)], env=child_env, @@ -743,6 +894,68 @@ def test_generated_runner_syntax_and_prompt_data_handling(self) -> None: self.assertEqual(calls.read_text(encoding="ascii"), "exec\nstop\n") self.assertEqual(captured_prompt.read_bytes(), b"second fixture\n") + def test_generated_runner_captures_the_command_environment_first(self) -> None: + # The snapshot is taken before the runner assigns any variable of its + # own, and checked before S-Code starts. + self.assertLess( + agent._RUNNER.index('exec env "$@" -0'), agent._RUNNER.index("\nevents=") + ) + self.assertLess( + agent._RUNNER.index('[ "$s_code_runner_captured" -eq 0 ]'), + agent._RUNNER.index(f"{agent._INSTALL_DIR}/s-code exec"), + ) + result, exit_status, calls, _ = self._run_generated_runner( + exec_status=0, + cleanup_status=0, + prompt=b"task\n", + ) + self.assertEqual(result.returncode, 0, result.stderr.decode()) + case = exit_status.parent.parent + # S-Code saw a private, NUL-separated snapshot when it started. + records = (case / "captured-env").read_bytes().split(b"\0") + self.assertIn(b"HARBOR_TASK_MARKER=task value\nwith a newline", records) + # The image's value, not the runner's, and no agent-only variable. + self.assertIn(b"events=image value", records) + names = {record.split(b"=", 1)[0] for record in records if record} + for agent_only in ( + b"AGENT_ONLY_MARKER", + b"S_CODE_HARBOR_CANARY_API_KEY", + b"S_CODE_HARBOR_AGENT_ENV_NAMES", + ): + self.assertNotIn(agent_only, names) + self.assertNotIn(b"canary-credential-value", b"\0".join(records)) + mode = (case / "captured-env-mode").read_text(encoding="utf-8") + self.assertTrue(mode.startswith("-rw-------"), mode) + # The runner removes a snapshot the daemon did not delete. + self.assertFalse((exit_status.parent / "command.env").exists()) + summary = case / "logs" / "command-env.json" + self.assertEqual( + json.loads(summary.read_text(encoding="utf-8")), _COMMAND_ENV_SUMMARY + ) + self.assertEqual(summary.stat().st_mode & 0o777, 0o600) + + # Without a snapshot S-Code must not start, whether the capture fails + # or the adapter did not ask for one. + for command_env_file in ("missing/command.env", None): + with self.subTest(command_env_file=command_env_file): + self._assert_runner_refuses_without_snapshot(command_env_file) + + def _assert_runner_refuses_without_snapshot(self, command_env_file: str | None) -> None: + result, exit_status, calls, _ = self._run_generated_runner( + exec_status=0, + cleanup_status=0, + prompt=b"task\n", + command_env_file=command_env_file, + ) + self.assertEqual(result.returncode, 1) + self.assertFalse(calls.exists()) + self.assertEqual(exit_status.read_text(encoding="ascii"), "1 0 1\n") + stderr_log = exit_status.parent.parent / "logs" / "stderr.log" + self.assertIn( + "could not capture the command environment", + stderr_log.read_text(encoding="utf-8"), + ) + def test_event_stream_classifications(self) -> None: completed = agent._classify_event_stream( self.write_events( @@ -923,6 +1136,7 @@ def test_harbor_cancellation_is_preserved_after_bounded_cleanup(self) -> None: asyncio.run(instance.run("one task", environment, FakeAgentContext())) self.assertIs(raised.exception, cancellation) self.assertTrue(environment.commands[-1][0].startswith("status=0; ")) + self.assertIn(f"rm -f {agent._COMMAND_ENV_FILE_PATH};", environment.commands[-1][0]) self.assertEqual(environment.commands[-1][1], 10) self.assertNotIn(agent._PROVENANCE_PATH, environment.remote_files) @@ -1049,10 +1263,107 @@ def test_successful_run_is_one_shot_and_records_usage(self) -> None: self.assertEqual(context.n_output_tokens, 5) self.assertEqual(context.metadata["s_code"]["result_status"], "completed") self.assertEqual(environment.remote_files[agent._PROMPT_PATH], b"one task\n") + # No summary was preserved, so provenance says why it has none. + execution = json.loads(environment.remote_files[agent._PROVENANCE_PATH])[ + "execution" + ] + self.assertIsNone(execution["command_environment"]) + self.assertEqual(execution["host_command_env"], "unverified") + self.assertEqual( + execution["command_environment_error"], "command-env.json is unavailable" + ) with self.assertRaisesRegex(RuntimeError, "exactly one run"): asyncio.run(instance.run("second task", environment, context)) + def test_provenance_records_the_command_environment_by_name_only(self) -> None: + instance = self.new_agent( + credential_env="S_CODE_HARBOR_CANARY_API_KEY", + extra_env={ + "S_CODE_HARBOR_CANARY_API_KEY": "canary-credential-value", + "S_CODE_MCP_ENABLED": "canary-mcp-value", + "HTTPS_PROXY": "http://canary-proxy.example:3128", + }, + ) + instance._agent_owner = "1234:5678" + environment = self._run_environment( + runner_return_code=0, + cleanup_return_code=0, + status=b"0 0 0\n", + events=b'{"kind":"turn.completed","payload":{}}\n', + ) + environment.remote_files[agent._COMMAND_ENV_PATH] = json.dumps( + _COMMAND_ENV_SUMMARY + ).encode() + context = FakeAgentContext() + asyncio.run(instance.run("one task", environment, context)) + self.assertEqual( + context.metadata["s_code"]["command_env"], agent._COMMAND_ENV_PATH + ) + + recorded = environment.remote_files[agent._PROVENANCE_PATH] + provenance = json.loads(recorded) + self.assertEqual( + provenance["adapter"]["version"], + f"0.1.0+{agent._ADAPTER_SOURCE_SHA256[:12]}", + ) + self.assertEqual( + provenance["adapter"]["source_sha256"], agent._ADAPTER_SOURCE_SHA256 + ) + self.assertEqual( + provenance["bundle"]["cargo_features"], + [agent.HARBOR_CONTAINER_ENV_FEATURE], + ) + execution = provenance["execution"] + self.assertEqual(execution["host_command_env"], "container") + self.assertEqual( + execution["command_environment"], + {"count": 2, "names": ["HOME", "PATH"], "sha256": "f" * 64}, + ) + self.assertIsNone(execution["command_environment_error"]) + self.assertEqual(execution["unmanaged_s_code_env"], ["S_CODE_MCP_ENABLED"]) + self.assertEqual( + provenance["artifacts"]["command_env"], agent._COMMAND_ENV_PATH + ) + seed = environment.remote_files[agent._PROVENANCE_SEED_PATH] + for value in (b"canary-credential-value", b"canary-mcp-value", b"canary-proxy"): + self.assertNotIn(value, recorded) + self.assertNotIn(value, seed) + + def test_command_environment_summary_is_validated(self) -> None: + path = self.directory / "command-env.json" + self.assertEqual( + agent._command_environment(path), + (None, "command-env.json is unavailable"), + ) + invalid = ( + b"not json", + b"[]", + {**_COMMAND_ENV_SUMMARY, "values": {}}, + {**_COMMAND_ENV_SUMMARY, "mode": "minimal"}, + {**_COMMAND_ENV_SUMMARY, "count": 3}, + {**_COMMAND_ENV_SUMMARY, "count": True, "names": ["HOME"]}, + {**_COMMAND_ENV_SUMMARY, "names": ["PATH", "HOME"]}, + # A name carrying "=" could smuggle a value into provenance. + {**_COMMAND_ENV_SUMMARY, "names": ["HOME=/root", "PATH"]}, + {**_COMMAND_ENV_SUMMARY, "sha256": "F" * 64}, + ) + for contents in invalid: + with self.subTest(contents=contents): + if isinstance(contents, dict): + contents = json.dumps(contents).encode() + path.write_bytes(contents) + summary, error = agent._command_environment(path) + self.assertIsNone(summary) + self.assertIsInstance(error, str) + + path.write_text(json.dumps(_COMMAND_ENV_SUMMARY), encoding="utf-8") + with mock.patch.object(agent, "_MAX_COMMAND_ENV_SUMMARY_BYTES", 8): + self.assertEqual( + agent._command_environment(path), + (None, "command-env.json is too large"), + ) + if __name__ == "__main__": unittest.main() diff --git a/tests/test-harbor-bundle.py b/tests/test-harbor-bundle.py index 7c9a857..20f2eda 100644 --- a/tests/test-harbor-bundle.py +++ b/tests/test-harbor-bundle.py @@ -38,6 +38,7 @@ def manifest(self): return { "build": { "cargo": "cargo 1.89.0", + "cargo_features": [runtime.HARBOR_CONTAINER_ENV_FEATURE], "glibc": "glibc 2.36", "node": "v22.23.2", "npm": "10.9.2", @@ -132,6 +133,9 @@ def test_valid_bundle_returns_authenticated_manifest(self): self.assertFalse(manifest.source_dirty) self.assertEqual(manifest.target, runtime.SUPPORTED_TARGET) self.assertEqual(manifest.glibc_version, "glibc 2.36") + self.assertEqual( + manifest.cargo_features, (runtime.HARBOR_CONTAINER_ENV_FEATURE,) + ) self.assertEqual(tuple(manifest.files), runtime.REQUIRED_RUNTIME_FILES) self.assertEqual(manifest.files["s-code"].mode, 0o755) @@ -191,6 +195,56 @@ def test_malformed_duplicate_key_and_unsafe_manifests_are_rejected(self): with self.subTest(manifest=manifest_bytes[:30]): self.assert_rejected(*self.archive(self.entries(manifest_bytes))) + def test_cargo_features_are_parsed_strictly(self): + # The runtime only parses the features; the adapter refuses a bundle + # without the container environment feature. + for features in ( + [], + ["s-code-cli/fixture", runtime.HARBOR_CONTAINER_ENV_FEATURE], + ): + with self.subTest(valid=features): + manifest = self.manifest() + manifest["build"]["cargo_features"] = features + path, digest = self.archive(self.entries(self.manifest_bytes(manifest))) + self.assertEqual( + runtime.validate_bundle(path, digest).cargo_features, tuple(features) + ) + + # A bundle from before the field existed has no features. + missing = self.manifest() + del missing["build"]["cargo_features"] + path, digest = self.archive(self.entries(self.manifest_bytes(missing))) + self.assertEqual(runtime.validate_bundle(path, digest).cargo_features, ()) + + invalid = [] + for features in ( + runtime.HARBOR_CONTAINER_ENV_FEATURE, + ["harbor-container-env"], + [runtime.HARBOR_CONTAINER_ENV_FEATURE] * 2, + [runtime.HARBOR_CONTAINER_ENV_FEATURE, "s-code-cli/fixture"], + ["s-code-daemon/feature with space"], + [1], + ): + manifest = self.manifest() + manifest["build"]["cargo_features"] = features + invalid.append(manifest) + for manifest in invalid: + with self.subTest(invalid=manifest["build"].get("cargo_features")): + path, digest = self.archive(self.entries(self.manifest_bytes(manifest))) + with self.assertRaisesRegex(runtime.BundleValidationError, "cargo_features"): + runtime.validate_bundle(path, digest) + + def test_builder_enables_and_records_the_container_env_feature(self): + # The build itself is too slow for unit tests, so check its script. + builder = BUILDER_PATH.read_text(encoding="utf-8") + self.assertIn( + f"\nCARGO_FEATURES={runtime.HARBOR_CONTAINER_ENV_FEATURE}\n", builder + ) + self.assertIn( + '-p s-code-daemon -p s-code-cli --features "$CARGO_FEATURES"\n', builder + ) + self.assertIn('"cargo_features": sorted(cargo_features.split(",")),', builder) + def test_size_bounds_reject_cheap_oversized_fixtures(self): path, digest = self.archive() for field in ("_MAX_ARCHIVE_BYTES", "_MAX_UNPACKED_BYTES"): From b6232f3ffe29aae091f737990056e7fb807d2aa2 Mon Sep 17 00:00:00 2001 From: Wei Deng Date: Mon, 19 Oct 2026 16:47:38 -0700 Subject: [PATCH 8/9] fix: send the configured temperature exactly A configured temperature was narrowed to f32 on its way to the model, so 0.6 went out as 0.6000000238418579 while provenance recorded 0.6. Carry it as f64 through the daemon, agent-core and the gateway, and check 0.6 in the gateway test, which f32 cannot represent exactly. The Harbor adapter now refuses S_CODE_MODEL_TEMPERATURE and S_CODE_MODEL_MAX_OUTPUT_TOKENS in the agent env, so provenance records the values actually used, and the docs explain that a fixed output cap also bounds a reasoning model's hidden reasoning. Refs #129 Signed-off-by: Wei Deng --- crates/agent-core/src/lib.rs | 2 +- crates/daemon/src/lib.rs | 4 ++-- crates/evals/src/main.rs | 4 ++-- crates/model-gateway/src/lib.rs | 7 ++++--- docs-site/content/guides/model-endpoints.md | 5 ++++- docs/guides/model-endpoints.md | 5 ++++- integrations/harbor/README.md | 6 +++++- integrations/harbor/agent.py | 8 ++++++++ tests/test-harbor-adapter.py | 6 ++++++ 9 files changed, 36 insertions(+), 11 deletions(-) diff --git a/crates/agent-core/src/lib.rs b/crates/agent-core/src/lib.rs index 7111e43..61add3d 100644 --- a/crates/agent-core/src/lib.rs +++ b/crates/agent-core/src/lib.rs @@ -134,7 +134,7 @@ impl TurnMachine { #[derive(Clone, Debug, Serialize, Deserialize)] pub struct AgentRunRequest { pub model: String, - pub temperature: f32, + pub temperature: f64, pub messages: Vec, pub tools: Vec, pub max_output_tokens: u32, diff --git a/crates/daemon/src/lib.rs b/crates/daemon/src/lib.rs index 377f9cf..2402a76 100644 --- a/crates/daemon/src/lib.rs +++ b/crates/daemon/src/lib.rs @@ -181,7 +181,7 @@ pub struct AppState { code_mode_slots: Arc, turn_limits: TurnLimits, disabled_tools: Arc>, - model_temperature: Option, + model_temperature: Option, model_max_output_tokens: Option, model_credentials_available: bool, storage_protection: Arc, @@ -1356,7 +1356,7 @@ impl AppState { } pub fn with_model_sampling(mut self, config: &s_code_config::ModelConfig) -> Self { - self.model_temperature = config.temperature.map(|temperature| temperature as f32); + self.model_temperature = config.temperature; self.model_max_output_tokens = config .max_output_tokens .map(|tokens| u32::try_from(tokens).unwrap_or(u32::MAX)); diff --git a/crates/evals/src/main.rs b/crates/evals/src/main.rs index 3bac5e5..61ac806 100644 --- a/crates/evals/src/main.rs +++ b/crates/evals/src/main.rs @@ -16,7 +16,7 @@ use std::{ use tokio_util::sync::CancellationToken; const MODEL_VERSION: &str = "fixture-agent-v1"; -const TEMPERATURE: f32 = 0.0; +const TEMPERATURE: f64 = 0.0; const PROMPT_VERSION: &str = "team-eval-prompt-v1"; const TOOL_VERSION: &str = "eval-tools-v1"; const COST_PER_TOKEN_MICROS: u64 = 20; @@ -230,7 +230,7 @@ struct ScenarioReport { struct EvalReport { schema_version: u32, model_version: &'static str, - temperature: f32, + temperature: f64, prompt_version: &'static str, tool_version: &'static str, scenarios: Vec, diff --git a/crates/model-gateway/src/lib.rs b/crates/model-gateway/src/lib.rs index 1703ecf..4cbe49d 100644 --- a/crates/model-gateway/src/lib.rs +++ b/crates/model-gateway/src/lib.rs @@ -46,7 +46,7 @@ pub struct ToolDefinition { #[derive(Clone, Debug, Serialize, Deserialize)] pub struct ModelRequest { pub model: String, - pub temperature: f32, + pub temperature: f64, pub messages: Vec, pub tools: Vec, pub max_output_tokens: u32, @@ -2064,7 +2064,8 @@ mod tests { let events = provider .stream(ModelRequest { model: "enterprise-model-v1".into(), - temperature: 0.25, + // 0.6 has no exact f32 form, so this catches a narrowed value. + temperature: 0.6, messages: vec![ ModelMessage { role: "user".into(), @@ -2137,7 +2138,7 @@ mod tests { let (headers, body) = captured.lock().unwrap().take().unwrap(); assert_eq!(headers["authorization"], "Bearer short-lived-secret"); assert_eq!(body["model"], "enterprise-model-v1"); - assert_eq!(body["temperature"], 0.25); + assert_eq!(body["temperature"], 0.6); assert_eq!(body["max_tokens"], 321); assert_eq!(body["stream"], true); assert_eq!(body["stream_options"]["include_usage"], true); diff --git a/docs-site/content/guides/model-endpoints.md b/docs-site/content/guides/model-endpoints.md index 03da513..f4b8657 100644 --- a/docs-site/content/guides/model-endpoints.md +++ b/docs-site/content/guides/model-endpoints.md @@ -130,7 +130,10 @@ Agent turns sample at temperature 0 and allow 8,192 output tokens per model call, doubling after a cut-off response up to 32,768. To match another agent's settings, for example in a benchmark, set `model.temperature` (0 to 2) and `model.max_output_tokens`, or `S_CODE_MODEL_TEMPERATURE` and -`S_CODE_MODEL_MAX_OUTPUT_TOKENS`. An explicit output cap stays fixed. +`S_CODE_MODEL_MAX_OUTPUT_TOKENS`. An explicit output cap stays fixed: a +cut-off or empty response is retried at the same cap at most twice, then the +turn fails. A reasoning model's hidden reasoning counts toward the cap, so +leave room for it. ## Development server diff --git a/docs/guides/model-endpoints.md b/docs/guides/model-endpoints.md index 03da513..f4b8657 100644 --- a/docs/guides/model-endpoints.md +++ b/docs/guides/model-endpoints.md @@ -130,7 +130,10 @@ Agent turns sample at temperature 0 and allow 8,192 output tokens per model call, doubling after a cut-off response up to 32,768. To match another agent's settings, for example in a benchmark, set `model.temperature` (0 to 2) and `model.max_output_tokens`, or `S_CODE_MODEL_TEMPERATURE` and -`S_CODE_MODEL_MAX_OUTPUT_TOKENS`. An explicit output cap stays fixed. +`S_CODE_MODEL_MAX_OUTPUT_TOKENS`. An explicit output cap stays fixed: a +cut-off or empty response is retried at the same cap at most twice, then the +turn fails. A reasoning model's hidden reasoning counts toward the cap, so +leave room for it. ## Development server diff --git a/integrations/harbor/README.md b/integrations/harbor/README.md index a547ad0..f7f81a2 100644 --- a/integrations/harbor/README.md +++ b/integrations/harbor/README.md @@ -92,7 +92,11 @@ kwargs: | `model_read_timeout_sec` | 600 | Longest gap between bytes of one model response | | `model_request_timeout_sec` | 3600 | Time allowed for one model request | | `temperature` | S-Code's default, 0 | Sampling temperature, 0 to 2 | -| `max_output_tokens` | S-Code's default, 8192 growing to 32768 | Fixed output-token cap per model call | +| `max_output_tokens` | S-Code's default, 8192 growing to 32768 | Fixed output-token cap per model call; a reasoning model's hidden reasoning counts toward it | + +Set the sampling settings with these kwargs; the adapter refuses +`S_CODE_MODEL_TEMPERATURE` and `S_CODE_MODEL_MAX_OUTPUT_TOKENS` in the agent +env, so provenance always records the values used. The adapter reads Harbor's resolved per-task agent deadline and nests the model request, S-Code turn, CLI, cleanup, and Harbor limits inside it. Requested model diff --git a/integrations/harbor/agent.py b/integrations/harbor/agent.py index 55dba20..eb91cbe 100644 --- a/integrations/harbor/agent.py +++ b/integrations/harbor/agent.py @@ -841,6 +841,14 @@ def __init__( managed_env["S_CODE_MODEL_TEMPERATURE"] = str(self._temperature) if self._max_output_tokens is not None: managed_env["S_CODE_MODEL_MAX_OUTPUT_TOKENS"] = str(self._max_output_tokens) + # Provenance records the sampling settings from the kwargs, so they + # cannot also come from the agent env. + for name, kwarg in ( + ("S_CODE_MODEL_TEMPERATURE", "temperature"), + ("S_CODE_MODEL_MAX_OUTPUT_TOKENS", "max_output_tokens"), + ): + if name in supplied_env and name not in managed_env: + raise ValueError(f"set {kwarg} with the agent kwarg, not {name}") managed_env[_AGENT_ENV_NAMES_VAR] = _agent_env_names( {**supplied_env, **managed_env} ) diff --git a/tests/test-harbor-adapter.py b/tests/test-harbor-adapter.py index e2e9df9..a8414ec 100644 --- a/tests/test-harbor-adapter.py +++ b/tests/test-harbor-adapter.py @@ -669,6 +669,12 @@ def test_sampling_kwargs_are_optional_and_validated(self) -> None: self.assertEqual(execution["temperature"], 0.6) self.assertEqual(execution["max_output_tokens"], 4096) + # The agent env cannot set them behind provenance's back. + for name in ("S_CODE_MODEL_TEMPERATURE", "S_CODE_MODEL_MAX_OUTPUT_TOKENS"): + with self.subTest(agent_env=name): + with self.assertRaisesRegex(ValueError, "agent kwarg"): + self.new_agent(extra_env={"S_CODE_HARBOR_API_KEY": "key", name: "1"}) + for kwargs, error in ( ({"temperature": 2.5}, ValueError), ({"temperature": float("nan")}, ValueError), From c304734ae91808de965f77bcf1e24473c544289e Mon Sep 17 00:00:00 2001 From: Wei Deng Date: Tue, 20 Oct 2026 09:49:51 -0700 Subject: [PATCH 9/9] fix: require Harbor command environment snapshot Signed-off-by: Wei Deng --- crates/daemon/src/main.rs | 10 ++++++---- integrations/harbor/README.md | 4 +++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/daemon/src/main.rs b/crates/daemon/src/main.rs index 669396b..49fb8f1 100644 --- a/crates/daemon/src/main.rs +++ b/crates/daemon/src/main.rs @@ -1040,9 +1040,8 @@ fn harbor_command_environment( )>, String, > { - let Some(snapshot) = variable("S_CODE_HARBOR_COMMAND_ENV_FILE") else { - return Ok(None); - }; + let snapshot = variable("S_CODE_HARBOR_COMMAND_ENV_FILE") + .ok_or_else(|| "S_CODE_HARBOR_COMMAND_ENV_FILE must be set".to_string())?; let required = |name: &str| { variable(name) .ok_or_else(|| format!("{name} must be set with S_CODE_HARBOR_COMMAND_ENV_FILE")) @@ -2581,7 +2580,10 @@ mod tests { .map(|(_, value)| std::ffi::OsString::from(*value)) }) }; - assert!(load(&[]).unwrap().is_none()); + assert_eq!( + load(&[]).err().as_deref(), + Some("S_CODE_HARBOR_COMMAND_ENV_FILE must be set") + ); let invalid: [(&[(&str, &str)], &str); 3] = [ ( &[ diff --git a/integrations/harbor/README.md b/integrations/harbor/README.md index f7f81a2..527706c 100644 --- a/integrations/harbor/README.md +++ b/integrations/harbor/README.md @@ -151,13 +151,15 @@ Harbor bundle has a container environment mode instead: - Full-mode commands get that environment, with `PWD` set to their working directory, and programs are found on its `PATH`. S-Code adds no `PATH`, `TMPDIR` or `OPENSSL_CONF` of its own. -- Commands never receive `--agent-env` variables (the credential among them), +- Commands never inherit `--agent-env` variables (the credential among them), `S_CODE_*` variables, any variable whose value is the model credential, or the shell's `PWD`, `OLDPWD`, `SHLVL` and `_`. A task variable that shares its name with an `--agent-env` variable is dropped as well. The daemon loads the snapshot only if it is a regular file that no other user can read. - `credential_env` must start with `S_CODE_HARBOR_`, so the credential is also excluded by its prefix and cannot collide with S-Code's own settings. + Use a scoped, budget-limited credential because same-user commands may still + inspect the daemon's environment through Linux `/proc`. - Only this bundle can enable the mode. `build-bundle.sh` builds the daemon with the `harbor-container-env` Cargo feature, which other builds leave off, and the adapter refuses a bundle without it. Sandboxed commands, extensions