fix(runtime): add subprocess timeout config for claude-code driver

The claude-code driver hardcodes its per-message turn timeout inside
ClaudeCodeDriver and exposed no operator-facing knob, so long-running
CC subprocess turns (large prompt-caches, deep tool chains) hit the
internal default with no escape hatch. Adds a public config surface,
honored today only by the claude-code driver, designed so future
subprocess drivers can opt in without re-shaping the API.

Public surface
- DriverConfig.subprocess_timeout_secs: Option<u64> (llm_driver.rs)
- OPENFANG_SUBPROCESS_TIMEOUT_SECS env var (drivers/mod.rs)
- Precedence in create_driver(): env var > config field > driver default

Naming rationale
- Field/env are scope-flavored, not semantic, on purpose: the name
  telegraphs that HTTP providers (default/Anthropic, openai, bedrock,
  qwen-code) accept-but-silently-ignore the field today. A semantic
  name (message_timeout_secs) would have invited the same silent-no-op
  footgun on those providers.
- Driver-internal field in claude_code.rs intentionally kept as
  message_timeout_secs — it's not on the public boundary and the
  semantic name accurately describes what it stores.

Tests (drivers/mod.rs)
- default_when_unset: no env, no config -> driver default
- config_set: config field flows through
- env_overrides_config: env var wins over config (construction-only
  assertion; trait-object opacity prevents reading the value back)
- malformed_env_falls_through: unparseable env silently falls through
  to config, matching the .parse::<u64>().ok() chain in production
- All four tests scrub OPENFANG_SUBPROCESS_TIMEOUT_SECS pre/post to
  avoid cross-test pollution

Mechanical pass-throughs
- 12 x DriverConfig { .. } test fixtures in drivers/mod.rs gain
  subprocess_timeout_secs: None
- routes.rs (1), kernel.rs (6), agent_loop.rs (2): same pass-through
  fills in DriverConfig literals; no logic touched

Forward-compat note
- A NOTE block in drivers/mod.rs flags the scope-vs-implementation
  gap so the next contributor adding a subprocess driver knows
  exactly where to wire the config in.

Validated end-to-end against a live daemon: dry-run + full deploy
(deploy-local.sh, all 7 phases) + post-swap agent_send round-trip
through the claude-code dispatch path.
This commit is contained in:
Ben Hoverter
2026-04-27 23:17:41 -07:00
parent 4ae2961b1c
commit 79aa34c77a
5 changed files with 135 additions and 4 deletions
+1
View File
@@ -7698,6 +7698,7 @@ pub async fn test_provider(
Some(base_url) Some(base_url)
}, },
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
match openfang_runtime::drivers::create_driver(&driver_config) { match openfang_runtime::drivers::create_driver(&driver_config) {
+6
View File
@@ -660,6 +660,7 @@ impl OpenFangKernel {
.cloned() .cloned()
}), }),
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
// Primary driver failure is non-fatal: the dashboard should remain accessible // Primary driver failure is non-fatal: the dashboard should remain accessible
// even if the LLM provider is misconfigured. Users can fix config via dashboard. // even if the LLM provider is misconfigured. Users can fix config via dashboard.
@@ -683,6 +684,7 @@ impl OpenFangKernel {
.map(|z: zeroize::Zeroizing<String>| z.to_string()), .map(|z: zeroize::Zeroizing<String>| z.to_string()),
base_url: config.provider_urls.get(provider).cloned(), base_url: config.provider_urls.get(provider).cloned(),
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
match drivers::create_driver(&auto_config) { match drivers::create_driver(&auto_config) {
Ok(d) => { Ok(d) => {
@@ -731,6 +733,7 @@ impl OpenFangKernel {
.clone() .clone()
.or_else(|| config.provider_urls.get(&fb.provider).cloned()), .or_else(|| config.provider_urls.get(&fb.provider).cloned()),
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
match drivers::create_driver(&fb_config) { match drivers::create_driver(&fb_config) {
Ok(d) => { Ok(d) => {
@@ -5025,6 +5028,7 @@ impl OpenFangKernel {
api_key, api_key,
base_url, base_url,
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
match drivers::create_driver(&driver_config) { match drivers::create_driver(&driver_config) {
@@ -5101,6 +5105,7 @@ impl OpenFangKernel {
.or_else(|| dm.base_url.clone()) .or_else(|| dm.base_url.clone())
.or_else(|| self.lookup_provider_url(&fb_provider)), .or_else(|| self.lookup_provider_url(&fb_provider)),
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
match drivers::create_driver(&config) { match drivers::create_driver(&config) {
Ok(d) => chain.push((d, strip_provider_prefix(&fb_model_name, &fb_provider))), Ok(d) => chain.push((d, strip_provider_prefix(&fb_model_name, &fb_provider))),
@@ -5131,6 +5136,7 @@ impl OpenFangKernel {
.clone() .clone()
.or_else(|| self.lookup_provider_url(&fb.provider)), .or_else(|| self.lookup_provider_url(&fb.provider)),
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
match drivers::create_driver(&fb_config) { match drivers::create_driver(&fb_config) {
Ok(d) => { Ok(d) => {
@@ -1143,6 +1143,7 @@ async fn call_with_retry(
api_key, api_key,
base_url: fb.base_url.clone(), base_url: fb.base_url.clone(),
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let fb_driver = match crate::drivers::create_driver(&fb_config) { let fb_driver = match crate::drivers::create_driver(&fb_config) {
Ok(d) => d, Ok(d) => d,
@@ -1326,6 +1327,7 @@ async fn stream_with_retry(
api_key, api_key,
base_url: fb.base_url.clone(), base_url: fb.base_url.clone(),
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let fb_driver = match crate::drivers::create_driver(&fb_config) { let fb_driver = match crate::drivers::create_driver(&fb_config) {
Ok(d) => d, Ok(d) => d,
+104 -4
View File
@@ -325,10 +325,24 @@ pub fn create_driver(config: &DriverConfig) -> Result<Arc<dyn LlmDriver>, LlmErr
// Claude Code CLI — subprocess-based, no API key needed // Claude Code CLI — subprocess-based, no API key needed
if provider == "claude-code" { if provider == "claude-code" {
let cli_path = config.base_url.clone(); let cli_path = config.base_url.clone();
return Ok(Arc::new(claude_code::ClaudeCodeDriver::new( // Timeout precedence (highest wins):
cli_path, // 1. OPENFANG_SUBPROCESS_TIMEOUT_SECS env var (no-rebuild override for emergencies)
config.skip_permissions, // 2. DriverConfig.subprocess_timeout_secs (config.toml-driven)
))); // 3. Driver default (currently 300s, set inside ClaudeCodeDriver::new)
// NOTE: The field and env var are scope-named to apply to any subprocess
// driver, but today only `provider = "claude-code"` reads them. Other
// drivers accept the field silently (forward-compat); future subprocess
// drivers (qwen-code, etc.) will opt in here individually.
let timeout = std::env::var("OPENFANG_SUBPROCESS_TIMEOUT_SECS")
.ok()
.and_then(|s| s.parse::<u64>().ok())
.or(config.subprocess_timeout_secs);
return Ok(Arc::new(match timeout {
Some(secs) => {
claude_code::ClaudeCodeDriver::with_timeout(cli_path, config.skip_permissions, secs)
}
None => claude_code::ClaudeCodeDriver::new(cli_path, config.skip_permissions),
}));
} }
// Qwen Code CLI — subprocess-based, uses Qwen OAuth (free tier) // Qwen Code CLI — subprocess-based, uses Qwen OAuth (free tier)
@@ -648,6 +662,7 @@ mod tests {
api_key: Some("test".to_string()), api_key: Some("test".to_string()),
base_url: Some("http://localhost:9999/v1".to_string()), base_url: Some("http://localhost:9999/v1".to_string()),
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let driver = create_driver(&config); let driver = create_driver(&config);
assert!(driver.is_ok()); assert!(driver.is_ok());
@@ -660,6 +675,7 @@ mod tests {
api_key: None, api_key: None,
base_url: None, base_url: None,
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let driver = create_driver(&config); let driver = create_driver(&config);
assert!(driver.is_err()); assert!(driver.is_err());
@@ -779,6 +795,7 @@ mod tests {
api_key: None, api_key: None,
base_url: None, base_url: None,
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let driver = create_driver(&config); let driver = create_driver(&config);
assert!( assert!(
@@ -795,6 +812,7 @@ mod tests {
api_key: None, api_key: None,
base_url: None, base_url: None,
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let driver = create_driver(&config); let driver = create_driver(&config);
assert!(driver.is_err()); assert!(driver.is_err());
@@ -810,6 +828,7 @@ mod tests {
api_key: None, // picked up from env via provider_defaults api_key: None, // picked up from env via provider_defaults
base_url: None, base_url: None,
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let driver = create_driver(&config); let driver = create_driver(&config);
assert!( assert!(
@@ -827,6 +846,7 @@ mod tests {
api_key: None, api_key: None,
base_url: None, base_url: None,
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let driver = create_driver(&config); let driver = create_driver(&config);
assert!(driver.is_err()); assert!(driver.is_err());
@@ -842,6 +862,7 @@ mod tests {
api_key: None, api_key: None,
base_url: None, base_url: None,
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let result = create_driver(&config); let result = create_driver(&config);
assert!(result.is_err()); assert!(result.is_err());
@@ -870,6 +891,7 @@ mod tests {
api_key: Some("explicit-key".to_string()), api_key: Some("explicit-key".to_string()),
base_url: Some("https://api.example.com/v1".to_string()), base_url: Some("https://api.example.com/v1".to_string()),
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let driver = create_driver(&config); let driver = create_driver(&config);
assert!(driver.is_ok()); assert!(driver.is_ok());
@@ -897,6 +919,7 @@ mod tests {
api_key: Some("test-azure-key".to_string()), api_key: Some("test-azure-key".to_string()),
base_url: Some("https://myresource.openai.azure.com/openai/deployments".to_string()), base_url: Some("https://myresource.openai.azure.com/openai/deployments".to_string()),
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let driver = create_driver(&config); let driver = create_driver(&config);
assert!(driver.is_ok(), "Azure driver with key + URL should succeed"); assert!(driver.is_ok(), "Azure driver with key + URL should succeed");
@@ -909,6 +932,7 @@ mod tests {
api_key: None, api_key: None,
base_url: Some("https://myresource.openai.azure.com/openai/deployments".to_string()), base_url: Some("https://myresource.openai.azure.com/openai/deployments".to_string()),
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let result = create_driver(&config); let result = create_driver(&config);
assert!(result.is_err(), "Azure driver without key should error"); assert!(result.is_err(), "Azure driver without key should error");
@@ -927,6 +951,7 @@ mod tests {
api_key: Some("test-azure-key".to_string()), api_key: Some("test-azure-key".to_string()),
base_url: None, base_url: None,
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let result = create_driver(&config); let result = create_driver(&config);
assert!(result.is_err(), "Azure driver without URL should error"); assert!(result.is_err(), "Azure driver without URL should error");
@@ -945,6 +970,7 @@ mod tests {
api_key: Some("test-azure-key".to_string()), api_key: Some("test-azure-key".to_string()),
base_url: Some("https://myresource.openai.azure.com/openai/deployments".to_string()), base_url: Some("https://myresource.openai.azure.com/openai/deployments".to_string()),
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
let driver = create_driver(&config); let driver = create_driver(&config);
assert!( assert!(
@@ -969,6 +995,7 @@ mod tests {
api_key: Some("test-bedrock-api-key".to_string()), api_key: Some("test-bedrock-api-key".to_string()),
base_url: None, base_url: None,
skip_permissions: true, skip_permissions: true,
subprocess_timeout_secs: None,
}; };
// Should succeed because api_key is provided // Should succeed because api_key is provided
let driver = create_driver(&config); let driver = create_driver(&config);
@@ -977,4 +1004,77 @@ mod tests {
"Bedrock with explicit api_key should construct successfully" "Bedrock with explicit api_key should construct successfully"
); );
} }
#[test]
fn test_claude_code_driver_constructs_with_default_timeout() {
// No timeout in config and no env override → driver uses its built-in default.
std::env::remove_var("OPENFANG_SUBPROCESS_TIMEOUT_SECS");
let config = DriverConfig {
provider: "claude-code".to_string(),
api_key: None,
base_url: None,
skip_permissions: true,
subprocess_timeout_secs: None,
};
let driver = create_driver(&config);
assert!(driver.is_ok(), "claude-code driver should construct");
}
#[test]
fn test_claude_code_driver_constructs_with_config_timeout() {
// Timeout set via config field → with_timeout path is exercised.
std::env::remove_var("OPENFANG_SUBPROCESS_TIMEOUT_SECS");
let config = DriverConfig {
provider: "claude-code".to_string(),
api_key: None,
base_url: None,
skip_permissions: true,
subprocess_timeout_secs: Some(480),
};
let driver = create_driver(&config);
assert!(
driver.is_ok(),
"claude-code driver should construct with custom timeout"
);
}
#[test]
fn test_claude_code_driver_constructs_with_env_timeout_override() {
// Env var present → wins over config field. We can't read the timeout off the
// trait object here, but at minimum the construction path must not panic
// when both are set and the env var parses cleanly.
std::env::set_var("OPENFANG_SUBPROCESS_TIMEOUT_SECS", "600");
let config = DriverConfig {
provider: "claude-code".to_string(),
api_key: None,
base_url: None,
skip_permissions: true,
subprocess_timeout_secs: Some(120),
};
let driver = create_driver(&config);
std::env::remove_var("OPENFANG_SUBPROCESS_TIMEOUT_SECS");
assert!(
driver.is_ok(),
"claude-code driver should construct when env override is set"
);
}
#[test]
fn test_claude_code_driver_ignores_unparseable_env_timeout() {
// Garbage env var → falls through to config field, doesn't error.
std::env::set_var("OPENFANG_SUBPROCESS_TIMEOUT_SECS", "not-a-number");
let config = DriverConfig {
provider: "claude-code".to_string(),
api_key: None,
base_url: None,
skip_permissions: true,
subprocess_timeout_secs: Some(420),
};
let driver = create_driver(&config);
std::env::remove_var("OPENFANG_SUBPROCESS_TIMEOUT_SECS");
assert!(
driver.is_ok(),
"unparseable env override should fall through to config field"
);
}
} }
+22
View File
@@ -188,6 +188,27 @@ pub struct DriverConfig {
/// restricts what agents can do, making this safe. /// restricts what agents can do, making this safe.
#[serde(default = "default_skip_permissions")] #[serde(default = "default_skip_permissions")]
pub skip_permissions: bool, pub skip_permissions: bool,
/// Per-message subprocess turn timeout in seconds.
///
/// Caps how long the runtime will wait for a single CLI subprocess turn
/// (one message round-trip) before killing the process and reporting a
/// timeout failure. When unset, the driver's own default is used
/// (currently 300s). Long-context Opus calls with heavy tool surfaces
/// routinely take >4 minutes, so users running large prompts may want
/// to bump this to 480600s.
///
/// Can also be overridden at runtime via the
/// `OPENFANG_SUBPROCESS_TIMEOUT_SECS` env var, which wins over both
/// this field and the driver default.
///
/// **Scope:** Currently only honored by `provider = "claude-code"`.
/// Other providers (`default`, `qwen-code`, `openai`, `bedrock`, etc.)
/// accept the field for forward-compatibility but silently ignore it
/// today. As additional subprocess-based drivers are added, they will
/// opt in to this field individually.
#[serde(default)]
pub subprocess_timeout_secs: Option<u64>,
} }
fn default_skip_permissions() -> bool { fn default_skip_permissions() -> bool {
@@ -202,6 +223,7 @@ impl std::fmt::Debug for DriverConfig {
.field("api_key", &self.api_key.as_ref().map(|_| "<redacted>")) .field("api_key", &self.api_key.as_ref().map(|_| "<redacted>"))
.field("base_url", &self.base_url) .field("base_url", &self.base_url)
.field("skip_permissions", &self.skip_permissions) .field("skip_permissions", &self.skip_permissions)
.field("subprocess_timeout_secs", &self.subprocess_timeout_secs)
.finish() .finish()
} }
} }