From b329f3a16bcd461ae7a1b9a7551ca99a0bd391a6 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 21 Jul 2026 15:35:22 +0530 Subject: [PATCH 1/5] fix(core): seed tool-execution timeout on always-on boot path (#5027) Move the [agent].agent_timeout_secs seed into the ungated INFRA: Once block of register_domain_subscribers, so channel-less / web-chat-only cores (where start_channels is skipped) still honour the configured tool-execution timeout from the first tool call. The seed is DomainSet-independent, so even a minimal DomainSet::none() boot applies it. Add a channel-less regression test. --- src/core/jsonrpc.rs | 16 +++++++++++ src/core/jsonrpc_tests.rs | 56 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/src/core/jsonrpc.rs b/src/core/jsonrpc.rs index 2b87bdcb2d..d47e8d0bca 100644 --- a/src/core/jsonrpc.rs +++ b/src/core/jsonrpc.rs @@ -1906,6 +1906,22 @@ fn register_domain_subscribers( // executes a tool. crate::openhuman::tokenjuice::install_from_config(&config); + // Seed the live tool-execution timeout from the persisted `[agent]` config + // so a user-configured value (Settings → Agent OS access → Action timeout) + // is in effect from the first tool call. `OPENHUMAN_TOOL_TIMEOUT_SECS`, when + // set, still overrides this inside `set_tool_timeout_secs`. This lives on + // the always-on boot path (not `channels::runtime::startup::start_channels`, + // which is skipped for channel-less / web-chat-only cores) so the timeout + // seeds for every install regardless of DomainSet — it is DomainSet- + // independent process-global state, not a gated subscriber (#5027). + let effective_timeout = + crate::openhuman::tool_timeout::set_tool_timeout_secs(config.agent.agent_timeout_secs); + log::debug!( + "[tool_timeout] seeded tool-execution timeout from config: configured={}s effective={}s", + config.agent.agent_timeout_secs, + effective_timeout + ); + // Seed the scheduler-gate signed-out override from the on-disk session. // Without this, a sidecar that boots with no stored JWT would happily // spin up cron / channel loops and fire LLM requests that all 401. diff --git a/src/core/jsonrpc_tests.rs b/src/core/jsonrpc_tests.rs index c711fc3ace..7f0255ee37 100644 --- a/src/core/jsonrpc_tests.rs +++ b/src/core/jsonrpc_tests.rs @@ -78,6 +78,62 @@ fn domain_subscriber_plan_harness_gates_by_owning_group() { assert!(!plan.mcp, "harness must skip mcp_registry bus init"); } +/// #5027 — the tool-execution timeout must be seeded on the always-on core boot +/// path (`register_domain_subscribers`, inside the ungated `INFRA: Once` block), +/// NOT inside `channels::runtime::startup::start_channels`, which is skipped for +/// channel-less / web-chat-only cores (and when `OPENHUMAN_DISABLE_CHANNEL_LISTENERS` +/// is set). A minimal `DomainSet::none()` must still seed, because the INFRA block +/// is DomainSet-independent. +/// +/// The timeout atomic and the `INFRA: Once` are process-global, so this is the +/// SOLE `register_domain_subscribers` seed assertion in the binary: a second caller +/// would find `INFRA` already consumed and observe no re-seed, so a second such test +/// would be order-dependent and flaky. Serialized via `TEST_ENV_LOCK` with +/// `OPENHUMAN_TOOL_TIMEOUT_SECS` cleared so the operator env override cannot mask the +/// config-derived value. Runs under a tokio runtime like the real boot paths — the +/// INFRA block calls `subscribe_global`, which `tokio::spawn`s when the global bus is +/// already initialized by another test in the binary. +#[tokio::test] +async fn tool_timeout_seeds_on_channelless_core_boot() { + let _lock = crate::openhuman::config::TEST_ENV_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); + let restore = std::env::var_os("OPENHUMAN_TOOL_TIMEOUT_SECS"); + std::env::remove_var("OPENHUMAN_TOOL_TIMEOUT_SECS"); + + // Distinctive, in-range (1..=3600) value so the assertion can only pass on a + // real seed, never on the default. Channel-less: `channels_config` stays empty, + // which is exactly the config for which `start_channels` is skipped. + let mut config = crate::openhuman::config::Config::default(); + config.agent.agent_timeout_secs = 1234; + assert!( + config.channels_config.active_channel.is_none(), + "test premise: channel-less config, so start_channels would be skipped" + ); + + let tmp = tempfile::tempdir().expect("tempdir"); + // Minimal DomainSet — INFRA (and thus the timeout seed) is DomainSet-independent, + // so even `none()` must seed. `embedded_core = true` skips the standalone + // process-exit shutdown subscriber. + super::register_domain_subscribers( + tmp.path().to_path_buf(), + config, + true, + crate::core::runtime::DomainSet::none(), + ); + + assert_eq!( + crate::openhuman::tool_timeout::tool_execution_timeout_secs(), + 1234, + "channel-less core boot must seed the tool-execution timeout from [agent].agent_timeout_secs" + ); + + match restore { + Some(v) => std::env::set_var("OPENHUMAN_TOOL_TIMEOUT_SECS", v), + None => std::env::remove_var("OPENHUMAN_TOOL_TIMEOUT_SECS"), + } +} + struct EnvVarGuard { old_values: Vec<(&'static str, Option)>, _lock: MutexGuard<'static, ()>, From e81d657a4b2e6045d486def4043e80d2eaf867b7 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 21 Jul 2026 15:36:20 +0530 Subject: [PATCH 2/5] refactor(channels): drop tool-timeout seed from start_channels (#5027) The seed now lives on the always-on core boot path in core::jsonrpc::register_domain_subscribers (previous commit). Leave a #5003-style NOTE pointing at the new home and explaining why start_channels is the wrong place (skipped for channel-less cores). --- src/openhuman/channels/runtime/startup.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/openhuman/channels/runtime/startup.rs b/src/openhuman/channels/runtime/startup.rs index 8cd8ecfbfc..8c142049b3 100644 --- a/src/openhuman/channels/runtime/startup.rs +++ b/src/openhuman/channels/runtime/startup.rs @@ -276,17 +276,12 @@ pub async fn start_channels(mut config: Config) -> Result<()> { config.workspace_dir.clone(), config.action_dir.clone(), ); - // Seed the live tool-execution timeout from the persisted `[agent]` config so - // a user-configured value (Settings → Agent OS access → Action timeout) is in - // effect from the first tool call. `OPENHUMAN_TOOL_TIMEOUT_SECS`, when set, - // still overrides this inside `set_tool_timeout_secs`. - let effective_timeout = - crate::openhuman::tool_timeout::set_tool_timeout_secs(config.agent.agent_timeout_secs); - tracing::debug!( - configured = config.agent.agent_timeout_secs, - effective = effective_timeout, - "[startup] seeded tool-execution timeout from config" - ); + // NOTE: the live tool-execution timeout seed is done in + // `core::jsonrpc::register_domain_subscribers` (unconditional core boot), NOT + // here — `start_channels` is skipped when no channel is configured or + // `OPENHUMAN_DISABLE_CHANNEL_LISTENERS` is set, which would otherwise leave + // channel-less / web-chat-only cores running the default timeout instead of the + // user-configured `[agent].agent_timeout_secs` (#5027). // Phase 1 of #1401: audit logger is wired with defaults so emission paths // are exercised at runtime. A follow-up promotes `SecurityConfig` (and // therefore the `audit` knob) onto the runtime `Config` schema so users From 11394080f03b7344befd0173a88297ab0fc1c7ce Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 21 Jul 2026 15:38:03 +0530 Subject: [PATCH 3/5] refactor(skills): retire dead register_workflow_cleanup_subscriber no-op (#5027) The legacy no-op has been superseded by ensure_triggered_workflow_subscriber (already registered on the always-on path in core::jsonrpc). Delete its call in start_channels, the empty fn + safe-noop test in skills::bus, and the stub in skills::stub. Sweep the tool_timeout README to name the seed's new home (core::jsonrpc::register_domain_subscribers). --- src/openhuman/channels/runtime/startup.rs | 1 - src/openhuman/skills/bus.rs | 10 ---------- src/openhuman/skills/stub.rs | 5 +---- src/openhuman/tool_timeout/README.md | 4 ++-- 4 files changed, 3 insertions(+), 17 deletions(-) diff --git a/src/openhuman/channels/runtime/startup.rs b/src/openhuman/channels/runtime/startup.rs index 8c142049b3..7e98177ab2 100644 --- a/src/openhuman/channels/runtime/startup.rs +++ b/src/openhuman/channels/runtime/startup.rs @@ -156,7 +156,6 @@ pub async fn start_channels(mut config: Config) -> Result<()> { let bus = event_bus::init_global(DEFAULT_CAPACITY); let _tracing_handle = bus.subscribe(Arc::new(TracingSubscriber)); crate::openhuman::health::bus::register_health_subscriber(); - crate::openhuman::skills::bus::register_workflow_cleanup_subscriber(); crate::openhuman::memory_conversations::register_conversation_persistence_subscriber( config.workspace_dir.clone(), ); diff --git a/src/openhuman/skills/bus.rs b/src/openhuman/skills/bus.rs index 7ea4f84f0a..73d551eb8a 100644 --- a/src/openhuman/skills/bus.rs +++ b/src/openhuman/skills/bus.rs @@ -255,10 +255,6 @@ pub fn ensure_triggered_workflow_subscriber(workspace: &std::path::Path) { }); } -/// Legacy no-op retained while call-sites migrate to -/// [`register_triggered_workflow_subscriber`]. Safe to call multiple times. -pub fn register_workflow_cleanup_subscriber() {} - #[cfg(test)] mod tests { use super::*; @@ -446,10 +442,4 @@ mod tests { }; assert!(idx.matching_workflows(&event).is_empty()); } - - #[test] - fn register_skill_cleanup_subscriber_is_a_safe_noop() { - register_workflow_cleanup_subscriber(); - register_workflow_cleanup_subscriber(); - } } diff --git a/src/openhuman/skills/stub.rs b/src/openhuman/skills/stub.rs index a250923198..c09d9b9bcc 100644 --- a/src/openhuman/skills/stub.rs +++ b/src/openhuman/skills/stub.rs @@ -82,7 +82,7 @@ pub mod registry { } // --------------------------------------------------------------------------- -// bus::{ensure_triggered_workflow_subscriber, register_workflow_cleanup_subscriber} +// bus::ensure_triggered_workflow_subscriber // --------------------------------------------------------------------------- pub mod bus { @@ -90,9 +90,6 @@ pub mod bus { pub fn ensure_triggered_workflow_subscriber(_workspace: &std::path::Path) { log::debug!("[skills-stub] ensure_triggered_workflow_subscriber skipped (skills disabled)"); } - - /// No-op: no skill run directories exist to clean up. - pub fn register_workflow_cleanup_subscriber() {} } // NOTE: no `tools` module here. The `pub use skills::tools::*` glob in diff --git a/src/openhuman/tool_timeout/README.md b/src/openhuman/tool_timeout/README.md index 3f3e0f091c..a6a343942a 100644 --- a/src/openhuman/tool_timeout/README.md +++ b/src/openhuman/tool_timeout/README.md @@ -7,7 +7,7 @@ Process-wide wall-clock timeout policy for tool execution (the node/tool runtime Highest precedence first: 1. `OPENHUMAN_TOOL_TIMEOUT_SECS` environment variable — operator override. When set to a valid value (`1..=3600`) it always wins; config pushes are ignored while it is present. -2. The persisted config value (`[agent].agent_timeout_secs`), pushed in via `set_tool_timeout_secs` at startup and on every `config.update_agent_settings` RPC. +2. The persisted config value (`[agent].agent_timeout_secs`), pushed in via `set_tool_timeout_secs` at startup (from `core::jsonrpc::register_domain_subscribers`, the always-on core boot path) and on every `config.update_agent_settings` RPC. 3. The built-in `DEFAULT_TIMEOUT_SECS` (`120`) default. ## Responsibilities @@ -54,7 +54,7 @@ The global timeout governs **non-scripting** tools only — a hung network/MCP c - `src/openhuman/tools/impl/system/{shell,node_exec,npm_exec}.rs` — scripting tools: unbounded by default, explicit `timeout_secs` via `explicit_call_timeout_*`. - `src/openhuman/agent/tools/delegate.rs` — bounds the delegated provider chat call with `tool_execution_timeout_secs`. - `src/openhuman/config/ops.rs` — `apply_agent_settings` calls `set_tool_timeout_secs` after persisting; `get_agent_settings` reports `effective_timeout_secs` / `env_override`. -- `src/openhuman/channels/runtime/startup.rs` — seeds the runtime value from config at core boot. +- `src/core/jsonrpc.rs` — `register_domain_subscribers` seeds the runtime value from config on the always-on core boot path (its ungated `INFRA: Once` block), so channel-less / web-chat-only cores get the configured timeout too (#5027). - `src/openhuman/agent/harness/harness_gap_tests.rs` — pins `parse_tool_timeout_secs` default/boundary behaviour. ## Notes / gotchas From e61aaed8c488f6e6e4cd69221ac074771d99b4ca Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 21 Jul 2026 17:51:58 +0530 Subject: [PATCH 4/5] fix(core): re-seed tool timeout on every boot, not just first (#5027) Move the tool-execution-timeout seed out of the process-global INFRA:Once block in register_domain_subscribers. bootstrap_core_runtime re-runs on an in-process core restart (CoreProcessHandle::restart -> ensure_running, and reset_local_data / Clear-Local-Data) with a freshly reloaded Config, but the Once is already consumed, so the previous config's timeout stayed pinned until Settings was re-saved. set_tool_timeout_secs is idempotent (atomic store honouring the env override), so re-seeding on every call re-applies the current config each boot at no cost. --- src/core/jsonrpc.rs | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/src/core/jsonrpc.rs b/src/core/jsonrpc.rs index d47e8d0bca..4b4244eab4 100644 --- a/src/core/jsonrpc.rs +++ b/src/core/jsonrpc.rs @@ -1882,6 +1882,31 @@ fn register_domain_subscribers( .insert(group) } + // Seed the live tool-execution timeout from the persisted `[agent]` config + // so a user-configured value (Settings → Agent OS access → Action timeout) + // is in effect from the first tool call. `OPENHUMAN_TOOL_TIMEOUT_SECS`, when + // set, still overrides this inside `set_tool_timeout_secs`. This lives on + // the always-on boot path (not `channels::runtime::startup::start_channels`, + // which is skipped for channel-less / web-chat-only cores) so the timeout + // seeds for every install regardless of DomainSet — it is DomainSet- + // independent process-global state, not a gated subscriber (#5027). + // + // Deliberately OUTSIDE the `INFRA: Once` below: `bootstrap_core_runtime` + // re-runs on an in-process core restart (`CoreProcessHandle::restart` → + // `ensure_running`, and `reset_local_data`/Clear-Local-Data) with a freshly + // reloaded `Config`, but `INFRA` is already consumed — so a seed gated by it + // would leave the process-global timeout pinned to the previous config's + // value until Settings is re-saved. `set_tool_timeout_secs` is idempotent + // (a plain atomic store honouring the env override), so re-seeding on every + // call is correct and cheap and re-applies the current config each boot. + let effective_timeout = + crate::openhuman::tool_timeout::set_tool_timeout_secs(config.agent.agent_timeout_secs); + log::debug!( + "[tool_timeout] seeded tool-execution timeout from config: configured={}s effective={}s", + config.agent.agent_timeout_secs, + effective_timeout + ); + // Ungated core/platform infra — health, scheduler-gate, TokenJuice // content-router, session-token seeding, the SessionExpired handler, and // service restart/shutdown. These are DomainSet-independent, so they run @@ -1906,22 +1931,6 @@ fn register_domain_subscribers( // executes a tool. crate::openhuman::tokenjuice::install_from_config(&config); - // Seed the live tool-execution timeout from the persisted `[agent]` config - // so a user-configured value (Settings → Agent OS access → Action timeout) - // is in effect from the first tool call. `OPENHUMAN_TOOL_TIMEOUT_SECS`, when - // set, still overrides this inside `set_tool_timeout_secs`. This lives on - // the always-on boot path (not `channels::runtime::startup::start_channels`, - // which is skipped for channel-less / web-chat-only cores) so the timeout - // seeds for every install regardless of DomainSet — it is DomainSet- - // independent process-global state, not a gated subscriber (#5027). - let effective_timeout = - crate::openhuman::tool_timeout::set_tool_timeout_secs(config.agent.agent_timeout_secs); - log::debug!( - "[tool_timeout] seeded tool-execution timeout from config: configured={}s effective={}s", - config.agent.agent_timeout_secs, - effective_timeout - ); - // Seed the scheduler-gate signed-out override from the on-disk session. // Without this, a sidecar that boots with no stored JWT would happily // spin up cron / channel loops and fire LLM requests that all 401. From 849c4767b251b2a738a2eae19729b73e036afec2 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 21 Jul 2026 17:52:07 +0530 Subject: [PATCH 5/5] test(core): panic-safe RAII env guard for channelless seed test (#5027) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the channelless-boot seed test's manual TEST_ENV_LOCK acquisition + end-of-test env restore with an EnvVarGuard (new remove_many) so a panicking assertion still restores OPENHUMAN_TOOL_TIMEOUT_SECS on unwind — a shared-lock sibling can no longer inherit the cleared var. Refresh the doc comment to match the seed now living just before INFRA:Once (re-runs every boot), not inside it. --- src/core/jsonrpc_tests.rs | 58 ++++++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/src/core/jsonrpc_tests.rs b/src/core/jsonrpc_tests.rs index 7f0255ee37..06e005226b 100644 --- a/src/core/jsonrpc_tests.rs +++ b/src/core/jsonrpc_tests.rs @@ -79,27 +79,27 @@ fn domain_subscriber_plan_harness_gates_by_owning_group() { } /// #5027 — the tool-execution timeout must be seeded on the always-on core boot -/// path (`register_domain_subscribers`, inside the ungated `INFRA: Once` block), -/// NOT inside `channels::runtime::startup::start_channels`, which is skipped for +/// path (`register_domain_subscribers`), NOT inside +/// `channels::runtime::startup::start_channels`, which is skipped for /// channel-less / web-chat-only cores (and when `OPENHUMAN_DISABLE_CHANNEL_LISTENERS` -/// is set). A minimal `DomainSet::none()` must still seed, because the INFRA block -/// is DomainSet-independent. +/// is set). A minimal `DomainSet::none()` must still seed, because the seed is +/// DomainSet-independent. /// -/// The timeout atomic and the `INFRA: Once` are process-global, so this is the -/// SOLE `register_domain_subscribers` seed assertion in the binary: a second caller -/// would find `INFRA` already consumed and observe no re-seed, so a second such test -/// would be order-dependent and flaky. Serialized via `TEST_ENV_LOCK` with -/// `OPENHUMAN_TOOL_TIMEOUT_SECS` cleared so the operator env override cannot mask the -/// config-derived value. Runs under a tokio runtime like the real boot paths — the -/// INFRA block calls `subscribe_global`, which `tokio::spawn`s when the global bus is -/// already initialized by another test in the binary. +/// The seed sits just *before* the ungated `INFRA: Once` block, so it re-runs on +/// every `register_domain_subscribers` call (each `bootstrap_core_runtime`), +/// re-applying the freshly reloaded config on an in-process restart — a seed gated +/// by the process-global `Once` would only fire on the first boot. `TEST_ENV_LOCK` +/// (via `EnvVarGuard`) serializes with `OPENHUMAN_TOOL_TIMEOUT_SECS` cleared so the +/// operator env override cannot mask the config-derived value. Runs under a tokio +/// runtime like the real boot paths — the INFRA block calls `subscribe_global`, +/// which `tokio::spawn`s when the global bus is already initialized by another test +/// in the binary. #[tokio::test] async fn tool_timeout_seeds_on_channelless_core_boot() { - let _lock = crate::openhuman::config::TEST_ENV_LOCK - .lock() - .unwrap_or_else(|e| e.into_inner()); - let restore = std::env::var_os("OPENHUMAN_TOOL_TIMEOUT_SECS"); - std::env::remove_var("OPENHUMAN_TOOL_TIMEOUT_SECS"); + // Clear the operator override behind a panic-safe RAII guard: if any assertion + // below panics, `Drop` still restores the previous value, so sibling tests that + // share `TEST_ENV_LOCK` never inherit the cleared var. + let _env = EnvVarGuard::remove_many(vec!["OPENHUMAN_TOOL_TIMEOUT_SECS"]); // Distinctive, in-range (1..=3600) value so the assertion can only pass on a // real seed, never on the default. Channel-less: `channels_config` stays empty, @@ -127,11 +127,6 @@ async fn tool_timeout_seeds_on_channelless_core_boot() { 1234, "channel-less core boot must seed the tool-execution timeout from [agent].agent_timeout_secs" ); - - match restore { - Some(v) => std::env::set_var("OPENHUMAN_TOOL_TIMEOUT_SECS", v), - None => std::env::remove_var("OPENHUMAN_TOOL_TIMEOUT_SECS"), - } } struct EnvVarGuard { @@ -155,6 +150,25 @@ impl EnvVarGuard { _lock: lock, } } + + /// Remove the named vars (capturing their prior values) for the guard's + /// lifetime, restoring each on `Drop`. Mirrors [`set_many`] for tests that + /// need an env var *absent* rather than set to a fixed value. + fn remove_many(keys: Vec<&'static str>) -> Self { + let lock = crate::openhuman::config::TEST_ENV_LOCK + .lock() + .expect("test env lock poisoned"); + let mut old_values = Vec::with_capacity(keys.len()); + for key in keys { + let old = std::env::var_os(key); + std::env::remove_var(key); + old_values.push((key, old)); + } + Self { + old_values, + _lock: lock, + } + } } impl Drop for EnvVarGuard {