Synced from monorepo
Synced from monorepo Changes: - Workspace task snapshots only list incomplete backgrounded tasks - Quiet auth, LSP, and config warnings in the shell - Fix observability attributes for warm store errors, restore setup, remote tools, and preview denials - Fail closed when soak metrics are missing - Run plan-mode exit last in mixed tool batches - Allow /loop to store prompts that can terminate the loop - Make subagent maximum nesting depth configurable - Security: apply sandbox profile to the leader process that executes tools Source-Revision: 1adcd1f477870e4a97bacbd6be78c8a3bfbac46d
This commit is contained in:
parent
b41c75a578
commit
02d9359435
96 changed files with 2346 additions and 351 deletions
|
|
@ -189,6 +189,7 @@ windows = { workspace = true }
|
|||
[dev-dependencies]
|
||||
criterion = { workspace = true }
|
||||
filetime = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
xai-grok-memory = { workspace = true, features = ["test-support"] }
|
||||
xai-grok-workspace = { workspace = true, features = ["test-support"] }
|
||||
|
|
|
|||
|
|
@ -1538,6 +1538,10 @@ pub struct Config {
|
|||
/// Not remotely gated.
|
||||
#[serde(skip)]
|
||||
pub subagents_enabled: bool,
|
||||
/// Resolved max subagent nesting depth (see
|
||||
/// [`crate::config::SubagentsConfig::resolve_max_depth`]).
|
||||
#[serde(skip)]
|
||||
pub subagents_max_depth: u32,
|
||||
/// Per-subagent model ID overrides from `[subagents.models]` in config.toml.
|
||||
/// Keys are agent names, values are model IDs. Set alongside `subagents_enabled`
|
||||
/// from `SubagentsConfig::resolve()`.
|
||||
|
|
@ -1858,6 +1862,7 @@ impl Default for Config {
|
|||
cli_agents: Vec::new(),
|
||||
cli_agent_overrides: CliAgentOverrides::default(),
|
||||
subagents_enabled: true,
|
||||
subagents_max_depth: crate::config::SubagentsConfig::DEFAULT_MAX_DEPTH,
|
||||
subagent_model_overrides: std::collections::HashMap::new(),
|
||||
subagent_toggle: std::collections::HashMap::new(),
|
||||
subagent_roles: std::collections::HashMap::new(),
|
||||
|
|
@ -1887,6 +1892,12 @@ impl Default for Config {
|
|||
cfg
|
||||
}
|
||||
}
|
||||
/// Config paths read by raw-layer resolvers, not [`Config`] serde fields, so
|
||||
/// `serde_ignored` must not report them as unrecognized keys.
|
||||
const NON_SERDE_CONFIG_PATHS: &[&str] = &[
|
||||
crate::util::config::REMOTE_FETCH_CONFIG_PATH,
|
||||
crate::util::config::SLASH_COMMAND_TAGS_CONFIG_PATH,
|
||||
];
|
||||
/// Parse `[auth_provider.<name>]` tables leniently: a malformed entry warns
|
||||
/// (surfaced by `grok inspect`) and is skipped, so it fails closed for the
|
||||
/// models referencing it instead of failing the whole config.
|
||||
|
|
@ -1992,17 +2003,18 @@ impl Config {
|
|||
unused_keys.push(path.to_string());
|
||||
})
|
||||
.map_err(|e| e.to_string())?;
|
||||
let user_unused = match user_config.as_table() {
|
||||
let unrecognized_keys = match user_config.as_table() {
|
||||
Some(user_table) => unused_keys
|
||||
.into_iter()
|
||||
.filter(|path| {
|
||||
let top_level = path.split('.').next().unwrap_or(path);
|
||||
user_table.contains_key(top_level)
|
||||
})
|
||||
.filter(|path| !NON_SERDE_CONFIG_PATHS.contains(&path.as_str()))
|
||||
.collect(),
|
||||
None => Vec::new(),
|
||||
};
|
||||
Ok((config, user_unused))
|
||||
Ok((config, unrecognized_keys))
|
||||
}
|
||||
pub fn new_from_toml_cfg(raw_config: &toml::Value) -> Result<Self, String> {
|
||||
let raw_config = &Self::expand_auth_alias(raw_config);
|
||||
|
|
@ -2052,21 +2064,25 @@ impl Config {
|
|||
if let toml::Value::Table(ref mut t) = base {
|
||||
t.remove("mcp_servers");
|
||||
}
|
||||
let (mut config, user_unused) =
|
||||
let (mut config, mut unrecognized_keys) =
|
||||
Self::deserialize_collecting_unrecognized(base, &raw_without_model_sections)?;
|
||||
config.mcp_servers = parsed_mcp_servers.into_iter().collect();
|
||||
if !user_unused.is_empty() {
|
||||
let keys = user_unused.join(", ");
|
||||
tracing::warn!(
|
||||
"config has unrecognized key(s): {keys}. Run /help for config reference."
|
||||
);
|
||||
}
|
||||
config.config_models = config_models;
|
||||
config.config_warnings = config_warnings;
|
||||
config.auth_providers = auth_providers;
|
||||
config.model_providers = model_providers;
|
||||
config.config_warnings.extend(auth_provider_warnings);
|
||||
config.config_warnings.extend(model_provider_warnings);
|
||||
unrecognized_keys.sort();
|
||||
for key in unrecognized_keys {
|
||||
config.config_warnings.push(
|
||||
super::config_model_override_parse::ConfigWarning::config_key(
|
||||
key,
|
||||
super::config_model_override_parse::ConfigWarningKind::UnknownField,
|
||||
"unrecognized config key".to_owned(),
|
||||
),
|
||||
);
|
||||
}
|
||||
let declared_provider_names: std::collections::HashSet<&str> = raw_config
|
||||
.get("auth_provider")
|
||||
.and_then(toml::Value::as_table)
|
||||
|
|
@ -2161,6 +2177,13 @@ impl Config {
|
|||
self.subagent_toggle = sa.toggle;
|
||||
self.subagent_roles = sa.roles;
|
||||
self.subagent_personas = sa.personas;
|
||||
let env = std::env::var(crate::config::SubagentsConfig::ENV_MAX_DEPTH).ok();
|
||||
let remote = self
|
||||
.remote_settings
|
||||
.as_ref()
|
||||
.and_then(|r| r.subagents_max_depth);
|
||||
self.subagents_max_depth =
|
||||
crate::config::SubagentsConfig::resolve_max_depth(env.as_deref(), sa.max_depth, remote);
|
||||
}
|
||||
/// Resolve all `#[serde(skip)]` runtime fields that have resolver functions.
|
||||
///
|
||||
|
|
@ -2184,6 +2207,15 @@ impl Config {
|
|||
self.session_summary_model_override = ctx.cli_session_summary_model.map(|s| s.to_owned());
|
||||
let cli_flag = ctx.cli_subagents.unwrap_or(false);
|
||||
self.resolve_subagents(cli_flag, ctx.raw_config);
|
||||
let env = std::env::var(crate::config::SubagentsConfig::ENV_MAX_DEPTH).ok();
|
||||
let toml_max = ctx
|
||||
.raw_config
|
||||
.get("subagents")
|
||||
.and_then(|s| s.get("max_depth"))
|
||||
.and_then(|v| v.as_integer());
|
||||
let remote = ctx.remote_settings.and_then(|r| r.subagents_max_depth);
|
||||
self.subagents_max_depth =
|
||||
crate::config::SubagentsConfig::resolve_max_depth(env.as_deref(), toml_max, remote);
|
||||
let tools = crate::config::ToolsConfig::resolve(ctx.raw_config);
|
||||
self.respect_gitignore = match self.requirements.respect_gitignore.pinned() {
|
||||
Some(pinned) => pinned,
|
||||
|
|
@ -10431,6 +10463,30 @@ agent_type = "cursor"
|
|||
assert!(unused.iter().any(|k| k == "endpoint"), "got: {unused:?}");
|
||||
}
|
||||
#[test]
|
||||
fn known_non_serde_config_paths_are_not_reported_unused() {
|
||||
let unused = unused_keys_from_toml(
|
||||
r#"
|
||||
[features]
|
||||
remote_fetch = false
|
||||
not_a_real_feature = true
|
||||
[slash_command_tags]
|
||||
workflows = "new"
|
||||
"#,
|
||||
);
|
||||
assert!(
|
||||
!unused.iter().any(|k| k == "features.remote_fetch"),
|
||||
"features.remote_fetch must not be treated as a typo: {unused:?}"
|
||||
);
|
||||
assert!(
|
||||
!unused.iter().any(|k| k == "slash_command_tags"),
|
||||
"slash_command_tags is a real table: {unused:?}"
|
||||
);
|
||||
assert!(
|
||||
unused.iter().any(|k| k == "features.not_a_real_feature"),
|
||||
"real typos still surface: {unused:?}"
|
||||
);
|
||||
}
|
||||
#[test]
|
||||
fn config_warns_on_field_typos() {
|
||||
let unused = unused_keys_from_toml(
|
||||
r#"
|
||||
|
|
|
|||
|
|
@ -69,6 +69,9 @@ pub enum WarningTarget {
|
|||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
field: Option<String>,
|
||||
},
|
||||
ConfigKey {
|
||||
path: String,
|
||||
},
|
||||
}
|
||||
|
||||
impl WarningTarget {
|
||||
|
|
@ -81,6 +84,7 @@ impl WarningTarget {
|
|||
Self::AuthProvider { name, .. } => format!("auth_provider.\"{name}\""),
|
||||
Self::ModelProviderSection => "model_providers".to_owned(),
|
||||
Self::ModelProvider { id, .. } => format!("model_providers.\"{id}\""),
|
||||
Self::ConfigKey { path } => path.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -89,7 +93,10 @@ impl WarningTarget {
|
|||
Self::Model { field, .. }
|
||||
| Self::AuthProvider { field, .. }
|
||||
| Self::ModelProvider { field, .. } => field.as_deref(),
|
||||
Self::ModelSection | Self::AuthProviderSection | Self::ModelProviderSection => None,
|
||||
Self::ModelSection
|
||||
| Self::AuthProviderSection
|
||||
| Self::ModelProviderSection
|
||||
| Self::ConfigKey { .. } => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -179,6 +186,14 @@ impl ConfigWarning {
|
|||
}
|
||||
}
|
||||
|
||||
pub(crate) fn config_key(path: String, kind: ConfigWarningKind, reason: String) -> Self {
|
||||
Self {
|
||||
target: WarningTarget::ConfigKey { path },
|
||||
kind,
|
||||
reason,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn field(&self) -> Option<&str> {
|
||||
self.target.field()
|
||||
}
|
||||
|
|
@ -252,13 +267,13 @@ pub(crate) fn log_config_warnings(warnings: &[ConfigWarning]) {
|
|||
field = warning.field().unwrap_or("(entry)"),
|
||||
kind = ?warning.kind,
|
||||
reason = %warning.reason,
|
||||
"model_override: skipped invalid config"
|
||||
"config: ignored unrecognized or invalid entry"
|
||||
);
|
||||
}
|
||||
if !warnings.is_empty() {
|
||||
tracing::warn!(
|
||||
warnings = warnings.len(),
|
||||
"model_override: parsed with warnings; run `grok inspect` for details"
|
||||
"config: parsed with warnings; run `grok inspect` for details"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1325,8 +1325,6 @@ impl acp::Agent for MvpAgent {
|
|||
} else {
|
||||
self.model_state(Some(&session_id))
|
||||
};
|
||||
let (session_config_value, session_detail_value) = self
|
||||
.session_config_meta(&session_id, cwd.as_str().to_owned(), None, &models);
|
||||
let applied_tool_overrides = match self
|
||||
.session_handle_waiting_for_load(&session_id)
|
||||
.await
|
||||
|
|
@ -1349,8 +1347,13 @@ impl acp::Agent for MvpAgent {
|
|||
"feedbackEnabled": feedback_enabled,
|
||||
});
|
||||
if let Some(obj) = meta.as_object_mut() {
|
||||
obj.insert("x.ai/sessionConfig".to_string(), session_config_value);
|
||||
obj.insert("x.ai/sessionDetail".to_string(), session_detail_value);
|
||||
self.insert_session_config_meta(
|
||||
obj,
|
||||
&session_id,
|
||||
cwd.as_str().to_owned(),
|
||||
None,
|
||||
&models,
|
||||
);
|
||||
insert_applied_tool_overrides(obj, applied_tool_overrides.as_ref());
|
||||
}
|
||||
Ok(
|
||||
|
|
@ -2097,15 +2100,13 @@ impl acp::Agent for MvpAgent {
|
|||
);
|
||||
}
|
||||
let model_state = self.model_state(Some(&session_id));
|
||||
let (session_config_value, session_detail_value) = self
|
||||
.session_config_meta(
|
||||
&session_id,
|
||||
session_cwd.clone().unwrap_or_default(),
|
||||
summary.display_title_opt(),
|
||||
&model_state,
|
||||
);
|
||||
response_meta_map.insert("x.ai/sessionConfig".to_string(), session_config_value);
|
||||
response_meta_map.insert("x.ai/sessionDetail".to_string(), session_detail_value);
|
||||
self.insert_session_config_meta(
|
||||
&mut response_meta_map,
|
||||
&session_id,
|
||||
session_cwd.clone().unwrap_or_default(),
|
||||
summary.display_title_opt(),
|
||||
&model_state,
|
||||
);
|
||||
let applied_tool_overrides = {
|
||||
let cmd_tx = self
|
||||
.sessions
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
//! Inherent [`MvpAgent`] helpers (MCP/clients/gateway, settings/models, session ops, spawn).
|
||||
//! Co-located child of `mvp_agent` (`use super::*`).
|
||||
use super::*;
|
||||
use crate::auth::PreferredAuthMethod;
|
||||
use xai_grok_tools::implementations::grok_build::task::backend::SubagentBackend;
|
||||
/// `preferred` model, else catalog `current`, else first with own credentials.
|
||||
fn byok_from_models(
|
||||
|
|
@ -16,6 +17,23 @@ fn byok_from_models(
|
|||
.or_else(|| models.get(current).and_then(|m| m.own_credential()))
|
||||
.or_else(|| models.values().find_map(|m| m.own_credential()))
|
||||
}
|
||||
struct MissingSessionCtx {
|
||||
has_session_key: bool,
|
||||
has_own_credentials: bool,
|
||||
is_session_based_auth: bool,
|
||||
preferred: Option<PreferredAuthMethod>,
|
||||
}
|
||||
/// Warn only when a missing session is a real failure, not on API-key hosts.
|
||||
fn should_warn_missing_session(ctx: MissingSessionCtx) -> bool {
|
||||
if ctx.has_session_key || ctx.has_own_credentials {
|
||||
return false;
|
||||
}
|
||||
match ctx.preferred {
|
||||
Some(PreferredAuthMethod::Oidc) => true,
|
||||
Some(PreferredAuthMethod::ApiKey) => false,
|
||||
None => ctx.is_session_based_auth,
|
||||
}
|
||||
}
|
||||
impl MvpAgent {
|
||||
pub fn reload_skills_all_sessions(&self) -> usize {
|
||||
let session_ids: Vec<agent_client_protocol::SessionId> = self
|
||||
|
|
@ -1465,9 +1483,11 @@ impl MvpAgent {
|
|||
origin_client: Option<crate::http::OriginClientInfo>,
|
||||
) -> SamplingConfig {
|
||||
let preferred = self.cfg.borrow().grok_com_config.preferred_method;
|
||||
let prefers_oidc = preferred == Some(PreferredAuthMethod::Oidc);
|
||||
let is_session_based_auth = self.is_session_based_auth();
|
||||
let session = match preferred {
|
||||
Some(crate::auth::PreferredAuthMethod::ApiKey) => None,
|
||||
_ if self.is_session_based_auth() => self.auth_manager.current_or_expired(),
|
||||
Some(PreferredAuthMethod::ApiKey) => None,
|
||||
_ if is_session_based_auth => self.auth_manager.current_or_expired(),
|
||||
_ => None,
|
||||
};
|
||||
let has_session_key = session.is_some();
|
||||
|
|
@ -1475,8 +1495,7 @@ impl MvpAgent {
|
|||
model,
|
||||
session.as_ref().map(|a| a.key.as_str()),
|
||||
);
|
||||
if matches!(preferred, Some(crate::auth::PreferredAuthMethod::Oidc))
|
||||
&& !model.has_own_credentials()
|
||||
if prefers_oidc && !model.has_own_credentials()
|
||||
&& credentials.auth_type == xai_chat_state::AuthType::ApiKey
|
||||
{
|
||||
credentials.api_key = None;
|
||||
|
|
@ -1488,7 +1507,7 @@ impl MvpAgent {
|
|||
session.as_ref().map(|a| a.key.as_str()),
|
||||
);
|
||||
if !has_session_key && credentials.auth_type == xai_chat_state::AuthType::ApiKey
|
||||
&& !model.has_own_credentials() && self.is_session_based_auth()
|
||||
&& !model.has_own_credentials() && is_session_based_auth
|
||||
{
|
||||
tracing::info!(
|
||||
model = model.info().model.as_str(),
|
||||
|
|
@ -1501,7 +1520,12 @@ impl MvpAgent {
|
|||
);
|
||||
credentials.auth_type = xai_chat_state::AuthType::SessionToken;
|
||||
}
|
||||
if !has_session_key && !model.has_own_credentials() {
|
||||
if should_warn_missing_session(MissingSessionCtx {
|
||||
has_session_key,
|
||||
has_own_credentials: model.has_own_credentials(),
|
||||
is_session_based_auth,
|
||||
preferred,
|
||||
}) {
|
||||
tracing::warn!(
|
||||
model = model.info().model.as_str(),
|
||||
is_expired = self.auth_manager.is_expired(),
|
||||
|
|
@ -1922,7 +1946,7 @@ impl MvpAgent {
|
|||
subagent_presentation: RefCell::new(
|
||||
crate::agent::subagent::SubagentPresentation::new(),
|
||||
),
|
||||
monitor_event_buffer: xai_grok_tools::implementations::grok_build::task::types::MonitorEventBuffer::default(),
|
||||
monitor_event_buffer: xai_grok_tools::implementations::grok_build::monitor::types::MonitorEventBuffer::default(),
|
||||
bundle_sync_in_flight: Arc::new(std::sync::atomic::AtomicBool::new(false)),
|
||||
post_unblock_jwt_retry_in_flight: Arc::new(
|
||||
std::sync::atomic::AtomicBool::new(false),
|
||||
|
|
@ -2738,17 +2762,18 @@ impl MvpAgent {
|
|||
current_effort,
|
||||
)
|
||||
}
|
||||
/// Build the `x.ai/sessionConfig` and `x.ai/sessionDetail` `_meta` values
|
||||
/// shared by `new_session` and `load_session`, returned as
|
||||
/// `(sessionConfig, sessionDetail)`. Keeping both response paths on this one
|
||||
/// Insert the per-session `_meta` keys (`x.ai/sessionConfig`,
|
||||
/// `x.ai/sessionDetail`, `x.ai/schedulerBackgroundLoops`) shared by
|
||||
/// `new_session` and `load_session`. Keeping both response paths on this one
|
||||
/// builder stops them drifting.
|
||||
pub(super) fn session_config_meta(
|
||||
pub(super) fn insert_session_config_meta(
|
||||
&self,
|
||||
meta: &mut serde_json::Map<String, serde_json::Value>,
|
||||
session_id: &acp::SessionId,
|
||||
cwd: String,
|
||||
title: Option<String>,
|
||||
model_state: &acp::SessionModelState,
|
||||
) -> (serde_json::Value, serde_json::Value) {
|
||||
) {
|
||||
let config_options = self.session_config_options(Some(session_id), model_state);
|
||||
let detail = session_config::GrokSessionDetail::build(
|
||||
session_id.0.to_string(),
|
||||
|
|
@ -2756,7 +2781,22 @@ impl MvpAgent {
|
|||
model_state.current_model_id.0.to_string(),
|
||||
title,
|
||||
);
|
||||
(serde_json::json!({ "options": config_options }), serde_json::json!(detail))
|
||||
meta.insert(
|
||||
"x.ai/sessionConfig".to_string(),
|
||||
serde_json::json!({ "options": config_options }),
|
||||
);
|
||||
meta.insert("x.ai/sessionDetail".to_string(), serde_json::json!(detail));
|
||||
if let Some(background_loops) = self
|
||||
.sessions
|
||||
.borrow()
|
||||
.get(session_id)
|
||||
.map(|handle| handle.scheduler_background_loops)
|
||||
{
|
||||
meta.insert(
|
||||
SCHEDULER_BACKGROUND_LOOPS_META_KEY.to_string(),
|
||||
serde_json::json!(background_loops),
|
||||
);
|
||||
}
|
||||
}
|
||||
/// Seed the global sampling config with login auth when available.
|
||||
///
|
||||
|
|
@ -3708,7 +3748,7 @@ impl MvpAgent {
|
|||
let user_path = xai_grok_tools::util::grok_home::grok_home()
|
||||
.join("lsp.json");
|
||||
let project_path = tool_ctx.cwd.as_path().join(".grok").join("lsp.json");
|
||||
tracing::warn!(
|
||||
tracing::debug!(
|
||||
cwd = %tool_ctx.cwd,
|
||||
user_lsp_path = %user_path.display(),
|
||||
project_lsp_path = %project_path.display(),
|
||||
|
|
@ -3758,6 +3798,7 @@ impl MvpAgent {
|
|||
let goal_enabled = self.cfg.borrow().resolve_goal().value;
|
||||
let background_workflows_enabled = self.cfg.borrow().resolve_workflows().value;
|
||||
let subagents_enabled = self.cfg.borrow().subagents_enabled;
|
||||
let subagents_max_depth = self.cfg.borrow().subagents_max_depth;
|
||||
let ask_user_question_enabled = crate::upload::turn::parse_ask_user_question_from_meta(
|
||||
session_meta,
|
||||
)
|
||||
|
|
@ -3995,6 +4036,7 @@ impl MvpAgent {
|
|||
goal_enabled,
|
||||
background_workflows_enabled,
|
||||
subagents_enabled,
|
||||
subagents_max_depth,
|
||||
ask_user_question_enabled,
|
||||
client_hooks,
|
||||
prompt_display_cwd,
|
||||
|
|
|
|||
|
|
@ -76,8 +76,8 @@ use xai_grok_sampler::SamplerConfig as SamplingConfig;
|
|||
use crate::session::persistence::PersistenceHandle;
|
||||
use crate::session::worktree::BackgroundCopyContext;
|
||||
use crate::session::{
|
||||
ParsedPromptInfo, SessionCommand, SessionHandle, SessionLiveState, SessionThread,
|
||||
info::Info as SessionInfo, spawn_session_on_thread,
|
||||
ParsedPromptInfo, SCHEDULER_BACKGROUND_LOOPS_META_KEY, SessionCommand, SessionHandle,
|
||||
SessionLiveState, SessionThread, info::Info as SessionInfo, spawn_session_on_thread,
|
||||
};
|
||||
use crate::terminal::{AcpTerminalRunner, TerminalRunner};
|
||||
use crate::tools::ToolContext;
|
||||
|
|
@ -820,7 +820,7 @@ pub struct MvpAgent {
|
|||
/// Pushed by the `InjectNotification` handler when a turn is active and the
|
||||
/// notification has `Next` priority. Drained by the session turn loop
|
||||
/// (`inject_pending_monitor_events`) into a hidden synthetic user message.
|
||||
monitor_event_buffer: xai_grok_tools::implementations::grok_build::task::types::MonitorEventBuffer,
|
||||
monitor_event_buffer: xai_grok_tools::implementations::grok_build::monitor::types::MonitorEventBuffer,
|
||||
/// The process launch directory, captured once at construction so the
|
||||
/// deferred launch-dir init paths share one source of truth instead of each
|
||||
/// re-calling `std::env::current_dir()` (which could drift if the process
|
||||
|
|
@ -1711,6 +1711,7 @@ impl MvpAgent {
|
|||
explicitly_killed: false,
|
||||
owner_session_id: None,
|
||||
description: None,
|
||||
is_backgrounded: true,
|
||||
};
|
||||
let notification = crate::extensions::notification::SessionNotification {
|
||||
session_id: session_id.clone(),
|
||||
|
|
|
|||
|
|
@ -426,6 +426,7 @@ impl MvpAgent {
|
|||
yolo_mode,
|
||||
subagent_event_tx: self.subagent_event_tx.clone(),
|
||||
parent_depth,
|
||||
subagents_max_depth: self.cfg.borrow().subagents_max_depth,
|
||||
inference_idle_timeout_secs,
|
||||
auto_compact_threshold_tiers:
|
||||
crate::agent::subagent::AutoCompactThresholdTiers::capture(&self.cfg.borrow()),
|
||||
|
|
|
|||
|
|
@ -1167,6 +1167,7 @@ fn make_test_handle(
|
|||
std::sync::Arc::new(crate::terminal::LocalTerminalRunner),
|
||||
),
|
||||
model_id: acp::ModelId::new(model),
|
||||
scheduler_background_loops: true,
|
||||
reasoning_effort: None,
|
||||
yolo_mode: yolo,
|
||||
origin_client: client_id.map(|s| crate::http::OriginClientInfo {
|
||||
|
|
@ -1721,6 +1722,25 @@ async fn session_usage_dead_chat_state_actor_fails_closed() {
|
|||
.expect_err("dead chat-state actor");
|
||||
assert_eq!(err.code, acp::Error::internal_error().code);
|
||||
}
|
||||
/// The session responses publish the value THIS session's spawn pinned, so a
|
||||
/// client describing `/loop` fires can never contradict what the fires do.
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn session_meta_publishes_the_sessions_pinned_scheduler_background_loops() {
|
||||
let agent = build_minimal_agent_for_tests();
|
||||
let sid = acp::SessionId::new("loop-mode-sess");
|
||||
let mut handle = make_test_handle("test-model", false, None);
|
||||
handle.info.id = sid.clone();
|
||||
handle.scheduler_background_loops = false;
|
||||
agent.sessions.borrow_mut().insert(sid.clone(), handle);
|
||||
let model_state = agent.model_state(Some(&sid));
|
||||
let mut meta = serde_json::Map::new();
|
||||
agent.insert_session_config_meta(&mut meta, &sid, "/tmp".to_string(), None, &model_state);
|
||||
assert_eq!(
|
||||
meta.get(crate::session::SCHEDULER_BACKGROUND_LOOPS_META_KEY),
|
||||
Some(&serde_json::json!(false)),
|
||||
"session meta must carry the handle's pinned value"
|
||||
);
|
||||
}
|
||||
/// Build a minimal MvpAgent with pre-loaded auth for gate tests.
|
||||
fn build_agent_with_auth(auth: crate::auth::GrokAuth) -> MvpAgent {
|
||||
use crate::agent::config::Config as AgentConfig;
|
||||
|
|
|
|||
|
|
@ -381,8 +381,7 @@ pub(crate) async fn run_shell_child(
|
|||
.spawn_depth
|
||||
.unwrap_or(ctx.parent_depth + 1);
|
||||
let tools_before_policy = definition.tool_config.tools.len();
|
||||
let allow_nested_subagents =
|
||||
child_depth < xai_grok_tools::implementations::grok_build::task::MAX_SUBAGENT_DEPTH;
|
||||
let allow_nested_subagents = child_depth < ctx.subagents_max_depth;
|
||||
xai_grok_subagent_resolution::apply_child_tool_policy(
|
||||
&mut definition,
|
||||
effective_runtime.capability_mode,
|
||||
|
|
@ -1097,6 +1096,7 @@ pub(crate) async fn run_shell_child(
|
|||
ctx.goal_enabled,
|
||||
ctx.background_workflows_enabled,
|
||||
true,
|
||||
ctx.subagents_max_depth,
|
||||
ctx.ask_user_question_enabled,
|
||||
ctx.client_hooks.clone(),
|
||||
None,
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ use xai_file_utils::events::types::CancellationCategory;
|
|||
use xai_grok_agent::config::{McpInheritance, ModelOverride, PermissionMode};
|
||||
use xai_grok_sampling_types::conversation::ConversationItem;
|
||||
use xai_grok_subagent_resolution::ResumeSourceData;
|
||||
use xai_grok_tools::implementations::grok_build::monitor::types::MonitorEventBuffer;
|
||||
use xai_grok_tools::implementations::grok_build::task::coordinator::{
|
||||
ChildCompletion, ChildControl, ChildReporter, ChildRunOutput, LocalBoxFuture, StartedChild,
|
||||
SubagentProgress,
|
||||
|
|
@ -123,6 +124,7 @@ pub(crate) struct SubagentSpawnContext {
|
|||
pub yolo_mode: bool,
|
||||
pub subagent_event_tx: mpsc::UnboundedSender<SubagentEvent>,
|
||||
pub parent_depth: u32,
|
||||
pub subagents_max_depth: u32,
|
||||
/// Inference idle timeout (secs), resolved from the parent's model config at spawn-context creation time.
|
||||
pub inference_idle_timeout_secs: u64,
|
||||
/// Tier inputs for resolving `auto_compact_threshold_percent` at
|
||||
|
|
|
|||
|
|
@ -224,6 +224,9 @@ impl MemoryConfig {
|
|||
pub struct SubagentsConfig {
|
||||
/// Whether subagent support is enabled.
|
||||
pub enabled: bool,
|
||||
/// Raw `[subagents] max_depth` (i64 so out-of-range parses; clamped ≥1 at resolve).
|
||||
#[serde(default)]
|
||||
pub max_depth: Option<i64>,
|
||||
/// Per-subagent model ID overrides.
|
||||
/// Keys are agent names, values are model IDs that must exist in the
|
||||
/// available models registry. Parsed from `[subagents.models]` in config.toml.
|
||||
|
|
@ -430,13 +433,61 @@ impl SubagentsConfig {
|
|||
let roles_dir = cwd.join(".grok").join("roles");
|
||||
self.discover_roles_in_dir(&roles_dir);
|
||||
}
|
||||
pub const ENV_MAX_DEPTH: &'static str = "GROK_SUBAGENTS_MAX_DEPTH";
|
||||
pub const DEFAULT_MAX_DEPTH: u32 = 1;
|
||||
/// Clamp to `1..=u32::MAX`. Values below 1 (including 0 / negatives) warn
|
||||
/// and become 1 so nesting is never accidentally disabled.
|
||||
pub fn clamp_max_depth(raw: i64, source: &str) -> u32 {
|
||||
if raw < i64::from(Self::DEFAULT_MAX_DEPTH) {
|
||||
tracing::warn!(
|
||||
source,
|
||||
value = raw,
|
||||
"subagents max_depth < 1; clamping to 1"
|
||||
);
|
||||
Self::DEFAULT_MAX_DEPTH
|
||||
} else if raw > i64::from(u32::MAX) {
|
||||
tracing::warn!(
|
||||
source,
|
||||
value = raw,
|
||||
"subagents max_depth exceeds u32::MAX; clamping"
|
||||
);
|
||||
u32::MAX
|
||||
} else {
|
||||
raw as u32
|
||||
}
|
||||
}
|
||||
/// Precedence: env > TOML > remote > [`Self::DEFAULT_MAX_DEPTH`].
|
||||
///
|
||||
/// Depth 0 is the top-level session; a child is parent+1. Spawn is rejected
|
||||
/// when `depth >= max`. So `max = 1` allows only top-level spawns; nested
|
||||
/// spawns from a first-level subagent need `max >= 2`.
|
||||
pub fn resolve_max_depth(env: Option<&str>, config: Option<i64>, remote: Option<u32>) -> u32 {
|
||||
if let Some(raw) = env {
|
||||
match raw.trim().parse::<i64>() {
|
||||
Ok(v) => return Self::clamp_max_depth(v, "env"),
|
||||
Err(_) => {
|
||||
tracing::warn!(
|
||||
value = %raw,
|
||||
"invalid GROK_SUBAGENTS_MAX_DEPTH (expected integer); ignoring"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
if let Some(v) = config {
|
||||
return Self::clamp_max_depth(v, "config");
|
||||
}
|
||||
if let Some(v) = remote {
|
||||
return Self::clamp_max_depth(i64::from(v), "remote");
|
||||
}
|
||||
Self::DEFAULT_MAX_DEPTH
|
||||
}
|
||||
/// Resolve the final subagents config from all sources (in priority order):
|
||||
/// 1. CLI flag `--subagents` (absolute highest — always enables)
|
||||
/// 2. `GROK_SUBAGENTS` env var: `1`/`true` enables, `0`/`false` force-disables
|
||||
/// 3. Config file `[subagents]` section
|
||||
/// 4. Default (enabled)
|
||||
///
|
||||
/// Subagents are deliberately not remotely gated — only explicit local
|
||||
/// `enabled` is deliberately not remotely gated — only explicit local
|
||||
/// intent (CLI flag, `GROK_SUBAGENTS`, `[subagents] enabled`) changes
|
||||
/// the default.
|
||||
///
|
||||
|
|
|
|||
|
|
@ -1010,6 +1010,81 @@ fn subagents_config_default_enabled() {
|
|||
});
|
||||
}
|
||||
#[test]
|
||||
fn subagents_max_depth_defaults_to_one() {
|
||||
assert_eq!(
|
||||
SubagentsConfig::resolve_max_depth(None, None, None),
|
||||
SubagentsConfig::DEFAULT_MAX_DEPTH
|
||||
);
|
||||
assert_eq!(SubagentsConfig::DEFAULT_MAX_DEPTH, 1);
|
||||
}
|
||||
#[test]
|
||||
fn subagents_max_depth_env_beats_toml_and_remote() {
|
||||
assert_eq!(
|
||||
SubagentsConfig::resolve_max_depth(Some("3"), Some(2), Some(4)),
|
||||
3
|
||||
);
|
||||
}
|
||||
#[test]
|
||||
fn subagents_max_depth_toml_beats_remote() {
|
||||
assert_eq!(
|
||||
SubagentsConfig::resolve_max_depth(None, Some(2), Some(4)),
|
||||
2
|
||||
);
|
||||
}
|
||||
#[test]
|
||||
fn subagents_max_depth_remote_used_when_local_absent() {
|
||||
assert_eq!(SubagentsConfig::resolve_max_depth(None, None, Some(5)), 5);
|
||||
}
|
||||
#[test]
|
||||
fn subagents_max_depth_clamps_below_one_to_one() {
|
||||
assert_eq!(SubagentsConfig::clamp_max_depth(-3, "test"), 1);
|
||||
assert_eq!(SubagentsConfig::clamp_max_depth(0, "test"), 1);
|
||||
assert_eq!(
|
||||
SubagentsConfig::resolve_max_depth(Some("-2"), None, None),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
SubagentsConfig::resolve_max_depth(None, Some(0), Some(3)),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
SubagentsConfig::resolve_max_depth(None, None, Some(0)),
|
||||
1
|
||||
);
|
||||
}
|
||||
#[test]
|
||||
fn subagents_max_depth_invalid_env_falls_through() {
|
||||
assert_eq!(
|
||||
SubagentsConfig::resolve_max_depth(Some("not-a-number"), Some(2), None),
|
||||
2
|
||||
);
|
||||
}
|
||||
#[test]
|
||||
fn subagents_config_parses_max_depth_from_toml() {
|
||||
without_grok_subagents(|| {
|
||||
let config: toml::Value = toml::from_str("[subagents]\nmax_depth = 2\n")
|
||||
.unwrap();
|
||||
let sa = SubagentsConfig::resolve(false, &config);
|
||||
assert_eq!(sa.max_depth, Some(2));
|
||||
});
|
||||
}
|
||||
#[test]
|
||||
fn subagents_config_parses_negative_max_depth_without_dropping_section() {
|
||||
without_grok_subagents(|| {
|
||||
let config: toml::Value = toml::from_str(
|
||||
"[subagents]\nenabled = true\nmax_depth = -1\n",
|
||||
)
|
||||
.unwrap();
|
||||
let sa = SubagentsConfig::resolve(false, &config);
|
||||
assert!(sa.enabled);
|
||||
assert_eq!(sa.max_depth, Some(-1));
|
||||
assert_eq!(
|
||||
SubagentsConfig::resolve_max_depth(None, sa.max_depth, None),
|
||||
1
|
||||
);
|
||||
});
|
||||
}
|
||||
#[test]
|
||||
fn subagents_config_cli_flag_enables() {
|
||||
without_grok_subagents(|| {
|
||||
let config = toml::Value::Table(toml::map::Map::new());
|
||||
|
|
|
|||
|
|
@ -787,6 +787,14 @@ pub enum ConnectionError {
|
|||
Timeout,
|
||||
#[error("Reconnection cancelled")]
|
||||
Cancelled,
|
||||
#[error(
|
||||
"leader mode is unavailable under sandbox profile '{0}': the leader is a \
|
||||
separate, shared process this client cannot prove is confined by that \
|
||||
profile, so tools are not guaranteed to stay inside it. Disable the \
|
||||
profile at the source that selected it (CLI, env, config, or a managed \
|
||||
requirement)"
|
||||
)]
|
||||
SandboxConfinement(&'static str),
|
||||
}
|
||||
/// Handle for a connection to the leader process.
|
||||
///
|
||||
|
|
@ -1052,6 +1060,13 @@ impl LeaderReconnector {
|
|||
info!(attempt, "Reconnected to leader");
|
||||
return Ok(conn.into_channels_with_disconnect());
|
||||
}
|
||||
Err(e) if is_terminal_refusal(&e) => {
|
||||
warn!(attempt, error = %e, "Reconnection refused (terminal)");
|
||||
let _ = self.status_tx.send(ConnectionStatus::Failed {
|
||||
error: e.to_string(),
|
||||
});
|
||||
return Err(e);
|
||||
}
|
||||
Err(e) => {
|
||||
warn!(attempt, error = %e, "Reconnection attempt failed");
|
||||
if let ReconnectPolicy::Bounded { max_attempts } = policy
|
||||
|
|
@ -1364,6 +1379,10 @@ fn is_connect_level_failure(error: &ConnectionError) -> bool {
|
|||
ConnectionError::Timeout | ConnectionError::Client(ClientError::Connect(_, _))
|
||||
)
|
||||
}
|
||||
/// Policy refusals that can never succeed on reconnect retry (not zombie-evictable).
|
||||
fn is_terminal_refusal(error: &ConnectionError) -> bool {
|
||||
matches!(error, ConnectionError::SandboxConfinement(_))
|
||||
}
|
||||
/// Evict a suspected zombie leader (holds the flock but is not connectable).
|
||||
/// SIGTERM, wait, then escalate to SIGKILL if it overran the grace window.
|
||||
async fn evict_zombie_leader(pid: u32, sock_path: &Path, waited: Duration) {
|
||||
|
|
@ -1425,6 +1444,9 @@ pub async fn connect_or_spawn(
|
|||
env_urls: &LeaderEnvUrls,
|
||||
capabilities: ClientCapabilities,
|
||||
) -> Result<LeaderConnection, ConnectionError> {
|
||||
if let Some(profile) = xai_grok_sandbox::requested_confinement_profile() {
|
||||
return Err(ConnectionError::SandboxConfinement(profile));
|
||||
}
|
||||
let start = std::time::Instant::now();
|
||||
let mut lock = LeaderLock::new(&env_urls.grok_ws_url);
|
||||
let sock_path = lock.socket_path().clone();
|
||||
|
|
@ -1895,6 +1917,20 @@ mod tests {
|
|||
assert!(!is_connect_level_failure(&ConnectionError::Client(
|
||||
ClientError::ConnectionClosed
|
||||
)));
|
||||
assert!(!is_connect_level_failure(
|
||||
&ConnectionError::SandboxConfinement("strict")
|
||||
));
|
||||
}
|
||||
#[test]
|
||||
fn terminal_refusal_classification() {
|
||||
assert!(is_terminal_refusal(&ConnectionError::SandboxConfinement(
|
||||
"strict"
|
||||
)));
|
||||
assert!(!is_terminal_refusal(&ConnectionError::Timeout));
|
||||
assert!(!is_terminal_refusal(&ConnectionError::SpawnFailed(
|
||||
"boom".into()
|
||||
)));
|
||||
assert!(!is_terminal_refusal(&ConnectionError::Cancelled));
|
||||
}
|
||||
/// Per-PID eviction budget: allows `max` attempts, then denies; a PID change
|
||||
/// resets the counter so a fresh zombie gets its own budget.
|
||||
|
|
|
|||
|
|
@ -1591,6 +1591,10 @@ mod permission_auto_mode_tests;
|
|||
#[cfg(test)]
|
||||
#[path = "acp_session_tests/plan_approval_resume_tests.rs"]
|
||||
mod plan_approval_resume_tests;
|
||||
/// Mixed-batch plan.md write + exit_plan_mode snapshot.
|
||||
#[cfg(test)]
|
||||
#[path = "acp_session_tests/plan_exit_batch_barrier_tests.rs"]
|
||||
mod plan_exit_batch_barrier_tests;
|
||||
/// Plan-mode edit gate: read-only except the plan file, even under allow-all.
|
||||
#[cfg(test)]
|
||||
#[path = "acp_session_tests/plan_mode_edit_gate_tests.rs"]
|
||||
|
|
|
|||
|
|
@ -379,6 +379,7 @@ mod notification_hook_filter_tests {
|
|||
explicitly_killed: false,
|
||||
owner_session_id: None,
|
||||
description: None,
|
||||
is_backgrounded: false,
|
||||
},
|
||||
will_wake: false,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -448,7 +448,7 @@ impl SessionActor {
|
|||
let Some(buffer) = &self.tool_context.monitor_event_buffer else {
|
||||
return;
|
||||
};
|
||||
for event in xai_grok_tools::implementations::grok_build::task::types::drain_owned(
|
||||
for event in xai_grok_tools::implementations::grok_build::monitor::types::drain_owned(
|
||||
buffer,
|
||||
Some(self.session_info.id.0.as_ref()),
|
||||
) {
|
||||
|
|
@ -500,7 +500,7 @@ impl SessionActor {
|
|||
notifications: &[PendingNotification],
|
||||
task_output_tool_name: &str,
|
||||
) -> Vec<acp::ContentBlock> {
|
||||
use xai_grok_tools::implementations::grok_build::task::types::MonitorEventNotification;
|
||||
use xai_grok_tools::implementations::grok_build::monitor::types::MonitorEventNotification;
|
||||
|
||||
let completion_task_ids: std::collections::HashSet<&str> = notifications
|
||||
.iter()
|
||||
|
|
|
|||
|
|
@ -889,7 +889,7 @@ pub(super) async fn run_session(
|
|||
// Cap to prevent unbounded growth during long tool calls.
|
||||
const MAX_BUFFER_EVENTS: usize = 50;
|
||||
buffer.push_capped(
|
||||
xai_grok_tools::implementations::grok_build::task::types::MonitorEventNotification {
|
||||
xai_grok_tools::implementations::grok_build::monitor::types::MonitorEventNotification {
|
||||
task_id: task_id.clone(),
|
||||
event_text,
|
||||
// Tag with this session's id so the
|
||||
|
|
|
|||
|
|
@ -273,6 +273,7 @@ pub(crate) async fn spawn_session_actor(
|
|||
goal_enabled: bool,
|
||||
background_workflows_enabled: bool,
|
||||
subagents_enabled: bool,
|
||||
subagents_max_depth: u32,
|
||||
ask_user_question_enabled: bool,
|
||||
client_hooks: crate::extensions::hooks::ClientHooks,
|
||||
prompt_display_cwd: Option<String>,
|
||||
|
|
@ -925,6 +926,11 @@ pub(crate) async fn spawn_session_actor(
|
|||
let context_window_tokens = context_window_override
|
||||
.map(|c| c.get())
|
||||
.unwrap_or(sampling_config.context_window);
|
||||
let scheduler_background_loops = crate::util::config::resolve_scheduler_background_loops(
|
||||
remote_settings
|
||||
.as_ref()
|
||||
.and_then(|r| r.scheduler_background_loops),
|
||||
);
|
||||
let managed_gateway_tool_client = auth_manager.as_ref().map(|am| {
|
||||
xai_grok_tools::types::resources::ManagedGatewayToolClient(Arc::new(
|
||||
ShellManagedGatewayToolClient {
|
||||
|
|
@ -1003,15 +1009,12 @@ pub(crate) async fn spawn_session_actor(
|
|||
monitor_event_buffer: tool_context.monitor_event_buffer.clone(),
|
||||
user_question_tx: user_question_tx.clone(),
|
||||
subagent_depth: tool_context.subagent_depth,
|
||||
subagents_max_depth,
|
||||
session_id_str: session_info.id.0.to_string(),
|
||||
blocking_wait_depth: tool_context.blocking_wait_depth.clone(),
|
||||
respect_gitignore,
|
||||
path_not_found_hints,
|
||||
scheduler_background_loops: crate::util::config::resolve_scheduler_background_loops(
|
||||
remote_settings
|
||||
.as_ref()
|
||||
.and_then(|r| r.scheduler_background_loops),
|
||||
),
|
||||
scheduler_background_loops,
|
||||
mcp_state: mcp_state.clone(),
|
||||
managed_gateway_tool_client: managed_gateway_tool_client.clone(),
|
||||
is_non_interactive: startup_hints.non_interactive,
|
||||
|
|
@ -2085,6 +2088,7 @@ pub(crate) async fn spawn_session_actor(
|
|||
upload_failures_since_success: Arc::new(std::sync::atomic::AtomicU64::new(0)),
|
||||
tool_context: tool_context_for_handle,
|
||||
model_id: session_model_id,
|
||||
scheduler_background_loops,
|
||||
reasoning_effort: sampling_config.reasoning_effort,
|
||||
yolo_mode: session_yolo_mode,
|
||||
origin_client: origin_client.clone(),
|
||||
|
|
@ -2215,6 +2219,7 @@ pub(crate) async fn spawn_session_on_thread(
|
|||
goal_enabled: bool,
|
||||
background_workflows_enabled: bool,
|
||||
subagents_enabled: bool,
|
||||
subagents_max_depth: u32,
|
||||
ask_user_question_enabled: bool,
|
||||
client_hooks: crate::extensions::hooks::ClientHooks,
|
||||
prompt_display_cwd: Option<String>,
|
||||
|
|
@ -2387,6 +2392,7 @@ pub(crate) async fn spawn_session_on_thread(
|
|||
goal_enabled,
|
||||
background_workflows_enabled,
|
||||
subagents_enabled,
|
||||
subagents_max_depth,
|
||||
ask_user_question_enabled,
|
||||
client_hooks,
|
||||
prompt_display_cwd,
|
||||
|
|
|
|||
|
|
@ -388,6 +388,7 @@ mod stop_gate_snapshot_tests {
|
|||
explicitly_killed: false,
|
||||
owner_session_id: None,
|
||||
description: None,
|
||||
is_backgrounded: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -130,6 +130,30 @@ pub(super) fn should_intercept_exit_plan_approval(
|
|||
}
|
||||
true
|
||||
}
|
||||
/// Whether this tool call exits file-backed plan mode (not inline plan creation).
|
||||
pub(super) fn is_file_backed_exit_plan_input(tool_input: &ToolInput) -> bool {
|
||||
if matches!(tool_input, ToolInput::ExitPlanMode(_)) {
|
||||
return true;
|
||||
}
|
||||
false
|
||||
}
|
||||
pub(super) fn is_file_backed_exit_plan_kind(
|
||||
kind: Option<xai_grok_tools::types::tool::ToolKind>,
|
||||
) -> bool {
|
||||
matches!(kind, Some(xai_grok_tools::types::tool::ToolKind::ExitPlan))
|
||||
}
|
||||
/// Split ExitPlan-kind calls into the tail so they run after the rest of the batch.
|
||||
fn split_exit_plan_tail(
|
||||
calls: Vec<crate::sampling::types::ToolCallResponse>,
|
||||
kind_of: impl Fn(&str) -> Option<xai_grok_tools::types::tool::ToolKind>,
|
||||
) -> (
|
||||
Vec<crate::sampling::types::ToolCallResponse>,
|
||||
Vec<crate::sampling::types::ToolCallResponse>,
|
||||
) {
|
||||
calls
|
||||
.into_iter()
|
||||
.partition(|call| !is_file_backed_exit_plan_kind(kind_of(&call.function.name)))
|
||||
}
|
||||
/// Verdict for a tool call evaluated against the plan-mode edit gate.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(super) enum PlanEditGate {
|
||||
|
|
@ -290,10 +314,55 @@ impl SessionActor {
|
|||
}
|
||||
let mut final_result: Option<ToolLoop> = None;
|
||||
let mut deferred_followups: Vec<ConversationItem> = Vec::new();
|
||||
if tool_calls.len() > 1 {
|
||||
let kind_of = |name: &str| self.agent.borrow().tool_bridge().tool_kind(name);
|
||||
let (body, tail) = split_exit_plan_tail(tool_calls, kind_of);
|
||||
if !body.is_empty() {
|
||||
self.execute_tool_calls_batch(body, &mut deferred_followups, &mut final_result)
|
||||
.await?;
|
||||
}
|
||||
if !tail.is_empty() {
|
||||
self.execute_tool_calls_batch(tail, &mut deferred_followups, &mut final_result)
|
||||
.await?;
|
||||
}
|
||||
} else {
|
||||
self.execute_tool_calls_batch(tool_calls, &mut deferred_followups, &mut final_result)
|
||||
.await?;
|
||||
}
|
||||
{
|
||||
let _span = if !deferred_followups.is_empty() {
|
||||
Some(
|
||||
tracing::info_span!(
|
||||
"tools.deferred_followups",
|
||||
count = deferred_followups.len()
|
||||
)
|
||||
.entered(),
|
||||
)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
for chat in deferred_followups {
|
||||
self.chat_state_handle.push_user_message(chat);
|
||||
}
|
||||
}
|
||||
self.drain_pending_interjections().await;
|
||||
self.flush_pending_skill_reminders().await;
|
||||
if let Some(final_result) = final_result {
|
||||
return Ok(final_result);
|
||||
}
|
||||
Ok(ToolLoop::Continue)
|
||||
}
|
||||
/// Prepare → dispatch → post-flight. Caller owns the outer tail flush.
|
||||
async fn execute_tool_calls_batch(
|
||||
&self,
|
||||
tool_calls: Vec<crate::sampling::types::ToolCallResponse>,
|
||||
deferred_followups: &mut Vec<ConversationItem>,
|
||||
final_result: &mut Option<ToolLoop>,
|
||||
) -> Result<(), acp::Error> {
|
||||
let mut approved: Vec<PreparedToolCall> = Vec::new();
|
||||
for call in tool_calls.into_iter() {
|
||||
if final_result.is_some() {
|
||||
let message = match &final_result {
|
||||
let message = match &*final_result {
|
||||
Some(ToolLoop::PermissionReject { .. }) => {
|
||||
format!(
|
||||
"Tool execution cancelled due to earlier permission rejection for tool `{}`",
|
||||
|
|
@ -333,10 +402,7 @@ impl SessionActor {
|
|||
)
|
||||
.await;
|
||||
let call_name = call.function.name.clone();
|
||||
match self
|
||||
.prepare_tool_call(call, &mut deferred_followups)
|
||||
.await?
|
||||
{
|
||||
match self.prepare_tool_call(call, deferred_followups).await? {
|
||||
Ok(prepared) => approved.push(prepared),
|
||||
Err(tool_loop) => {
|
||||
self.events.tool_finished();
|
||||
|
|
@ -376,7 +442,7 @@ impl SessionActor {
|
|||
| ToolLoop::FollowupMessage(_)
|
||||
) && final_result.is_none()
|
||||
{
|
||||
final_result = Some(tool_loop);
|
||||
*final_result = Some(tool_loop);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -721,34 +787,13 @@ impl SessionActor {
|
|||
| ToolLoop::Cancelled
|
||||
| ToolLoop::FollowupMessage(_) => {
|
||||
if final_result.is_none() {
|
||||
final_result = Some(tool_loop);
|
||||
*final_result = Some(tool_loop);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
{
|
||||
let _span = if !deferred_followups.is_empty() {
|
||||
Some(
|
||||
tracing::info_span!(
|
||||
"tools.deferred_followups",
|
||||
count = deferred_followups.len()
|
||||
)
|
||||
.entered(),
|
||||
)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
for chat in deferred_followups {
|
||||
self.chat_state_handle.push_user_message(chat);
|
||||
}
|
||||
}
|
||||
self.drain_pending_interjections().await;
|
||||
self.flush_pending_skill_reminders().await;
|
||||
if let Some(final_result) = final_result {
|
||||
return Ok(final_result);
|
||||
}
|
||||
Ok(ToolLoop::Continue)
|
||||
Ok(())
|
||||
}
|
||||
/// Phase 1: pre-flight (MCP, args, hooks, permission, ExitPlanMode).
|
||||
pub(crate) async fn prepare_tool_call(
|
||||
|
|
@ -1237,10 +1282,11 @@ impl SessionActor {
|
|||
}
|
||||
}
|
||||
let is_exit_plan_mode = matches!(&tool_input, ToolInput::ExitPlanMode(_));
|
||||
let is_file_backed_exit = is_file_backed_exit_plan_input(&tool_input);
|
||||
let is_cursor_switch_to_agent = false;
|
||||
let is_cursor_create_plan = false;
|
||||
let plan_file_path = self.plan_mode.lock().plan_file_path().to_path_buf();
|
||||
let plan_read = if is_exit_plan_mode || is_cursor_switch_to_agent || is_cursor_create_plan {
|
||||
let plan_read = if is_file_backed_exit || is_cursor_create_plan {
|
||||
let inline_cursor_plan: Option<PlanFileRead> = None;
|
||||
if let Some(plan) = inline_cursor_plan {
|
||||
plan
|
||||
|
|
@ -2752,6 +2798,61 @@ mod execute_tool_call_parts_tests {
|
|||
}
|
||||
}
|
||||
#[cfg(test)]
|
||||
mod exit_plan_tail_predicate_tests {
|
||||
use super::{
|
||||
is_file_backed_exit_plan_input, is_file_backed_exit_plan_kind, split_exit_plan_tail,
|
||||
};
|
||||
use xai_grok_tools::types::ToolInput;
|
||||
use xai_grok_tools::types::tool::ToolKind;
|
||||
fn call(name: &str, args: &str) -> crate::sampling::types::ToolCallResponse {
|
||||
crate::sampling::types::ToolCallResponse {
|
||||
id: format!("call_{name}"),
|
||||
kind: "function".into(),
|
||||
function: crate::sampling::types::ToolCallFunction::new(name, args),
|
||||
}
|
||||
}
|
||||
/// Wire name does not matter — only [`ToolKind::ExitPlan`].
|
||||
fn kind_of(name: &str) -> Option<ToolKind> {
|
||||
match name {
|
||||
"exit_plan_mode" | "FinishPlan" => Some(ToolKind::ExitPlan),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
#[test]
|
||||
fn exit_plan_kind_is_file_backed_exit() {
|
||||
assert!(is_file_backed_exit_plan_kind(Some(ToolKind::ExitPlan)));
|
||||
assert!(!is_file_backed_exit_plan_kind(Some(ToolKind::Edit)));
|
||||
assert!(!is_file_backed_exit_plan_kind(None));
|
||||
assert!(is_file_backed_exit_plan_input(&ToolInput::ExitPlanMode(
|
||||
xai_grok_tools::implementations::grok_build::exit_plan_mode::ExitPlanModeInput {}
|
||||
)));
|
||||
}
|
||||
fn mixed(calls: Vec<crate::sampling::types::ToolCallResponse>) -> bool {
|
||||
let (body, tail) = split_exit_plan_tail(calls, kind_of);
|
||||
!body.is_empty() && !tail.is_empty()
|
||||
}
|
||||
#[test]
|
||||
fn split_puts_exit_plan_in_tail() {
|
||||
let write = call(
|
||||
"search_replace",
|
||||
r#"{"file_path":"/tmp/plan.md","old_string":"a","new_string":"b"}"#,
|
||||
);
|
||||
let exit = call("exit_plan_mode", "{}");
|
||||
let renamed_exit = call("FinishPlan", "{}");
|
||||
let create = call(
|
||||
"CreatePlan",
|
||||
r#"{"name":"p","overview":"o","plan":"plan body","todos":[]}"#,
|
||||
);
|
||||
assert!(mixed(vec![write.clone(), exit.clone()]));
|
||||
assert!(mixed(vec![exit.clone(), write.clone()]));
|
||||
assert!(mixed(vec![write.clone(), renamed_exit.clone()]));
|
||||
assert!(!mixed(vec![exit.clone()]));
|
||||
assert!(!mixed(vec![write.clone()]));
|
||||
assert!(!mixed(vec![write.clone(), create.clone()]));
|
||||
assert!(mixed(vec![write, exit, create]));
|
||||
}
|
||||
}
|
||||
#[cfg(test)]
|
||||
mod exit_plan_intercept_tests {
|
||||
use super::{PlanFileRead, classify_plan_file_read, should_intercept_exit_plan_approval};
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
//! Turn-execution concern for `SessionActor` (`handle_prompt`, turn-end,
|
||||
//! sampling loop).
|
||||
use super::*;
|
||||
use xai_grok_tools::implementations::grok_build::LoopFireMode;
|
||||
/// Synthetic tool the model calls to return its schema-constrained final answer
|
||||
/// on backends that can't constrain output natively (Messages API). Intercepted
|
||||
/// in the loop, never executed as a real tool.
|
||||
|
|
@ -334,12 +335,18 @@ impl SessionActor {
|
|||
}
|
||||
acc
|
||||
});
|
||||
let loop_fire_mode = if self.rebuild_spec.scheduler_background_loops {
|
||||
LoopFireMode::Detached
|
||||
} else {
|
||||
LoopFireMode::InSession
|
||||
};
|
||||
let prompt_blocks = match slash_commands::resolve(
|
||||
prompt_blocks,
|
||||
&slash_skills,
|
||||
availability,
|
||||
skill_rewrite,
|
||||
&named_workflows,
|
||||
loop_fire_mode,
|
||||
) {
|
||||
Ok(blocks) => blocks,
|
||||
Err(SlashCommandOutcome::Builtin(action)) => {
|
||||
|
|
@ -1402,7 +1409,7 @@ impl SessionActor {
|
|||
let Some(buffer) = &self.tool_context.monitor_event_buffer else {
|
||||
return;
|
||||
};
|
||||
let mine = xai_grok_tools::implementations::grok_build::task::types::drain_owned(
|
||||
let mine = xai_grok_tools::implementations::grok_build::monitor::types::drain_owned(
|
||||
buffer,
|
||||
Some(self.session_info.id.0.as_ref()),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -514,6 +514,7 @@ async fn genuine_user_start_consumes_deferred_completions_without_notification_t
|
|||
explicitly_killed: false,
|
||||
owner_session_id: None,
|
||||
description: None,
|
||||
is_backgrounded: false,
|
||||
},
|
||||
Some("get_command_or_subagent_output"),
|
||||
);
|
||||
|
|
@ -1667,6 +1668,7 @@ fn completed_bash_task(id: &str) -> xai_grok_tools::computer::types::TaskSnapsho
|
|||
explicitly_killed: false,
|
||||
owner_session_id: None,
|
||||
description: None,
|
||||
is_backgrounded: false,
|
||||
}
|
||||
}
|
||||
/// Real-actor coverage for the `SessionCommand::IsBusy` predicate
|
||||
|
|
|
|||
|
|
@ -0,0 +1,312 @@
|
|||
//! Mixed plan.md edit + exit: approval snapshot matches the post-edit plan body.
|
||||
|
||||
use super::support::*;
|
||||
use super::*;
|
||||
use agent_client_protocol as acp;
|
||||
use xai_grok_tools::implementations::grok_build::exit_plan_mode::ExitPlanModeExtRequest;
|
||||
|
||||
const SEED_PLAN: &str = "# OLD mixed-batch plan seed unique-c91e04";
|
||||
const NEW_PLAN: &str = "# NEW mixed-batch plan body unique-a7f3c2";
|
||||
|
||||
fn ext_response(outcome: &str) -> Arc<serde_json::value::RawValue> {
|
||||
serde_json::value::to_raw_value(&serde_json::json!({ "outcome": outcome }))
|
||||
.unwrap()
|
||||
.into()
|
||||
}
|
||||
|
||||
fn search_replace_plan(id: &str, plan_path: &str) -> ToolCallResponse {
|
||||
ToolCallResponse {
|
||||
id: id.to_string(),
|
||||
kind: "function".to_string(),
|
||||
function: crate::sampling::types::ToolCallFunction::new(
|
||||
"search_replace",
|
||||
serde_json::json!({
|
||||
"file_path": plan_path,
|
||||
"old_string": SEED_PLAN,
|
||||
"new_string": NEW_PLAN,
|
||||
})
|
||||
.to_string(),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
fn exit_plan_mode_call(id: &str) -> ToolCallResponse {
|
||||
ToolCallResponse {
|
||||
id: id.to_string(),
|
||||
kind: "function".to_string(),
|
||||
function: crate::sampling::types::ToolCallFunction::new("exit_plan_mode", "{}"),
|
||||
}
|
||||
}
|
||||
|
||||
async fn seeded_active_plan_actor_with_edit_tools() -> (
|
||||
SessionActor,
|
||||
tokio::sync::mpsc::UnboundedReceiver<xai_acp_lib::AcpClientMessage>,
|
||||
tempfile::TempDir,
|
||||
std::path::PathBuf,
|
||||
) {
|
||||
use xai_grok_tools::implementations::grok_build::enter_plan_mode::EnterPlanModeTool;
|
||||
use xai_grok_tools::implementations::grok_build::exit_plan_mode::ExitPlanModeTool;
|
||||
use xai_grok_tools::registry::types::ToolConfig;
|
||||
|
||||
let (gateway_tx, gateway_rx) =
|
||||
tokio::sync::mpsc::unbounded_channel::<xai_acp_lib::AcpClientMessage>();
|
||||
let (persistence_tx, _persistence_rx) =
|
||||
tokio::sync::mpsc::unbounded_channel::<PersistenceMsg>();
|
||||
let actor = create_test_actor(0, 256_000, 85, gateway_tx, persistence_tx).await;
|
||||
*actor.agent.borrow_mut() = test_agent_with_tools(vec![
|
||||
ToolConfig::from_id("GrokBuild:read_file"),
|
||||
ToolConfig {
|
||||
id: "GrokBuild:search_replace".into(),
|
||||
params: Some(
|
||||
serde_json::from_value(serde_json::json!({
|
||||
"skip_read_before_edit": true
|
||||
}))
|
||||
.unwrap(),
|
||||
),
|
||||
name_override: None,
|
||||
params_name_overrides: None,
|
||||
description_override: None,
|
||||
behavior_version: None,
|
||||
kind: None,
|
||||
},
|
||||
ToolConfig::for_tool::<EnterPlanModeTool>(),
|
||||
ToolConfig::for_tool::<ExitPlanModeTool>(),
|
||||
])
|
||||
.await;
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let plan_path = dir.path().join("plan.md");
|
||||
std::fs::write(&plan_path, SEED_PLAN).unwrap();
|
||||
{
|
||||
let mut tracker = actor.plan_mode.lock();
|
||||
*tracker = crate::session::plan_mode::PlanModeTracker::new(dir.path().to_path_buf());
|
||||
tracker.activate_from_tool();
|
||||
}
|
||||
actor
|
||||
.agent
|
||||
.borrow()
|
||||
.tool_bridge()
|
||||
.update_resource(xai_grok_tools::types::resources::PlanFilePath(
|
||||
plan_path.clone(),
|
||||
))
|
||||
.await;
|
||||
// Phase-2 file tools dispatch through workspace_ops; without a bound
|
||||
// session, search_replace hard-errors before writing plan.md.
|
||||
actor
|
||||
.workspace_ops
|
||||
.bind_local_session(
|
||||
&actor.session_id_string(),
|
||||
actor.tool_context.cwd.as_path().to_path_buf(),
|
||||
actor.tool_context.hunk_tracker_handle.clone(),
|
||||
actor.agent.borrow().tool_bridge().toolset(),
|
||||
None,
|
||||
)
|
||||
.expect("bind_local_session must succeed");
|
||||
|
||||
(actor, gateway_rx, dir, plan_path)
|
||||
}
|
||||
|
||||
fn spawn_exit_capture(
|
||||
mut gateway_rx: tokio::sync::mpsc::UnboundedReceiver<xai_acp_lib::AcpClientMessage>,
|
||||
) -> (
|
||||
tokio::task::JoinHandle<()>,
|
||||
std::sync::Arc<std::sync::Mutex<Option<String>>>,
|
||||
) {
|
||||
let captured = std::sync::Arc::new(std::sync::Mutex::new(None::<String>));
|
||||
let captured_for_task = captured.clone();
|
||||
let handle = tokio::task::spawn_local(async move {
|
||||
while let Some(msg) = gateway_rx.recv().await {
|
||||
match msg {
|
||||
xai_acp_lib::AcpClientMessage::ExtMethod(args) => {
|
||||
if args.request.method.as_ref() == "x.ai/exit_plan_mode" {
|
||||
let req: ExitPlanModeExtRequest =
|
||||
serde_json::from_str(args.request.params.get()).unwrap();
|
||||
*captured_for_task.lock().unwrap() = req.plan_content;
|
||||
let _ = args
|
||||
.response_tx
|
||||
.send(Ok(acp::ExtResponse::new(ext_response("approved"))));
|
||||
}
|
||||
}
|
||||
xai_acp_lib::AcpClientMessage::SessionNotification(args) => {
|
||||
let _ = args.response_tx.send(Ok(()));
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
});
|
||||
(handle, captured)
|
||||
}
|
||||
|
||||
async fn assert_mixed_batch_snapshot(write_first: bool) {
|
||||
let (actor, gateway_rx, _dir, plan_path) = seeded_active_plan_actor_with_edit_tools().await;
|
||||
let plan_path_str = plan_path.to_string_lossy().into_owned();
|
||||
let (responder, captured) = spawn_exit_capture(gateway_rx);
|
||||
|
||||
let write = search_replace_plan("call_write_plan", &plan_path_str);
|
||||
let exit = exit_plan_mode_call("call_exit_plan");
|
||||
let batch = if write_first {
|
||||
vec![write, exit]
|
||||
} else {
|
||||
vec![exit, write]
|
||||
};
|
||||
|
||||
tokio::time::timeout(
|
||||
std::time::Duration::from_secs(10),
|
||||
actor.execute_tool_calls(batch),
|
||||
)
|
||||
.await
|
||||
.expect("execute_tool_calls must not hang")
|
||||
.expect("execute_tool_calls must not error");
|
||||
|
||||
assert_eq!(std::fs::read_to_string(&plan_path).unwrap(), NEW_PLAN);
|
||||
|
||||
let snapshot = captured
|
||||
.lock()
|
||||
.unwrap()
|
||||
.clone()
|
||||
.expect("gateway must receive x.ai/exit_plan_mode with plan content");
|
||||
assert_eq!(snapshot, NEW_PLAN);
|
||||
|
||||
responder.abort();
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn mixed_write_then_exit_snapshot_sees_new_plan() {
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local.run_until(assert_mixed_batch_snapshot(true)).await;
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn mixed_exit_then_write_snapshot_sees_new_plan() {
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local.run_until(assert_mixed_batch_snapshot(false)).await;
|
||||
}
|
||||
|
||||
fn bash_call(id: &str) -> ToolCallResponse {
|
||||
ToolCallResponse {
|
||||
id: id.to_string(),
|
||||
kind: "function".to_string(),
|
||||
function: crate::sampling::types::ToolCallFunction::new(
|
||||
"run_terminal_cmd",
|
||||
r#"{"command":"echo mixed-batch-reject","description":"probe mixed-batch permission cancel"}"#,
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn mixed_permission_cancel_skips_exit_reverse_request() {
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local
|
||||
.run_until(async {
|
||||
use xai_grok_paths::AbsPathBuf;
|
||||
use xai_grok_tools::implementations::grok_build::enter_plan_mode::EnterPlanModeTool;
|
||||
use xai_grok_tools::implementations::grok_build::exit_plan_mode::ExitPlanModeTool;
|
||||
use xai_grok_tools::registry::types::ToolConfig;
|
||||
use xai_grok_workspace::permission::{ClientType, spawn_permission_manager};
|
||||
|
||||
let (gateway_tx, mut gateway_rx) =
|
||||
tokio::sync::mpsc::unbounded_channel::<xai_acp_lib::AcpClientMessage>();
|
||||
let (persistence_tx, _persistence_rx) =
|
||||
tokio::sync::mpsc::unbounded_channel::<PersistenceMsg>();
|
||||
let mut actor =
|
||||
create_test_actor(0, 256_000, 85, gateway_tx.clone(), persistence_tx).await;
|
||||
// Disable background bash so finalize does not require the
|
||||
// get_task_output / kill_task companion tools.
|
||||
*actor.agent.borrow_mut() = test_agent_with_tools(vec![
|
||||
ToolConfig {
|
||||
id: "GrokBuild:run_terminal_cmd".into(),
|
||||
params: Some(
|
||||
serde_json::from_value(serde_json::json!({
|
||||
"enabled_background": false
|
||||
}))
|
||||
.unwrap(),
|
||||
),
|
||||
name_override: None,
|
||||
params_name_overrides: None,
|
||||
description_override: None,
|
||||
behavior_version: None,
|
||||
kind: None,
|
||||
},
|
||||
ToolConfig::for_tool::<EnterPlanModeTool>(),
|
||||
ToolConfig::for_tool::<ExitPlanModeTool>(),
|
||||
])
|
||||
.await;
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let plan_path = dir.path().join("plan.md");
|
||||
std::fs::write(&plan_path, SEED_PLAN).unwrap();
|
||||
{
|
||||
let mut tracker = actor.plan_mode.lock();
|
||||
*tracker =
|
||||
crate::session::plan_mode::PlanModeTracker::new(dir.path().to_path_buf());
|
||||
tracker.activate_from_tool();
|
||||
}
|
||||
actor
|
||||
.agent
|
||||
.borrow()
|
||||
.tool_bridge()
|
||||
.update_resource(xai_grok_tools::types::resources::PlanFilePath(plan_path))
|
||||
.await;
|
||||
|
||||
let cwd = AbsPathBuf::new(std::path::PathBuf::from(actor.session_info.cwd.clone()))
|
||||
.unwrap_or_else(|_| AbsPathBuf::new(std::path::PathBuf::from("/tmp")).unwrap());
|
||||
let (perms, _ev) = spawn_permission_manager(
|
||||
actor.session_info.id.clone(),
|
||||
xai_acp_lib::AcpAgentGatewaySender::new(gateway_tx),
|
||||
cwd,
|
||||
ClientType::Generic,
|
||||
None,
|
||||
vec![],
|
||||
vec![],
|
||||
false,
|
||||
None,
|
||||
);
|
||||
actor.permissions = perms;
|
||||
|
||||
let exit_fired = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
|
||||
let exit_fired_task = exit_fired.clone();
|
||||
let responder = tokio::task::spawn_local(async move {
|
||||
while let Some(msg) = gateway_rx.recv().await {
|
||||
match msg {
|
||||
xai_acp_lib::AcpClientMessage::RequestPermission(args) => {
|
||||
let _ = args
|
||||
.response_tx
|
||||
.send(Ok(acp::RequestPermissionResponse::new(
|
||||
acp::RequestPermissionOutcome::Cancelled,
|
||||
)));
|
||||
}
|
||||
xai_acp_lib::AcpClientMessage::ExtMethod(args) => {
|
||||
if args.request.method.as_ref() == "x.ai/exit_plan_mode" {
|
||||
exit_fired_task.store(true, std::sync::atomic::Ordering::SeqCst);
|
||||
let _ = args
|
||||
.response_tx
|
||||
.send(Ok(acp::ExtResponse::new(ext_response("approved"))));
|
||||
}
|
||||
}
|
||||
xai_acp_lib::AcpClientMessage::SessionNotification(args) => {
|
||||
let _ = args.response_tx.send(Ok(()));
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
tokio::time::timeout(
|
||||
std::time::Duration::from_secs(10),
|
||||
actor.execute_tool_calls(vec![
|
||||
bash_call("call_bash_reject"),
|
||||
exit_plan_mode_call("call_exit"),
|
||||
]),
|
||||
)
|
||||
.await
|
||||
.expect("execute_tool_calls must not hang")
|
||||
.expect("execute_tool_calls must not error");
|
||||
|
||||
assert!(
|
||||
!exit_fired.load(std::sync::atomic::Ordering::SeqCst),
|
||||
"exit must not reverse-request after an earlier permission cancel"
|
||||
);
|
||||
responder.abort();
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
|
@ -51,9 +51,8 @@ use xai_grok_tools::computer::types::{AsyncFileSystem, TerminalBackend};
|
|||
use xai_grok_tools::implementations::grok_build::ask_user_question::types::UserQuestionRequest;
|
||||
use xai_grok_tools::implementations::grok_build::deploy_app::AppBuilderDeployerConfig;
|
||||
use xai_grok_tools::implementations::grok_build::image_gen::ImageGenConfig;
|
||||
use xai_grok_tools::implementations::grok_build::task::types::{
|
||||
MonitorEventBuffer, SubagentEvent, TaskModelValidator,
|
||||
};
|
||||
use xai_grok_tools::implementations::grok_build::monitor::types::MonitorEventBuffer;
|
||||
use xai_grok_tools::implementations::grok_build::task::types::{SubagentEvent, TaskModelValidator};
|
||||
use xai_grok_tools::implementations::grok_build::video_gen::VideoGenConfig;
|
||||
use xai_grok_tools::implementations::grok_build::web_fetch::WebFetchConfig;
|
||||
use xai_grok_tools::implementations::lsp::LspBackend;
|
||||
|
|
@ -122,10 +121,14 @@ pub(crate) struct AgentRebuildSpec {
|
|||
pub monitor_event_buffer: Option<MonitorEventBuffer>,
|
||||
pub user_question_tx: UnboundedSender<UserQuestionRequest>,
|
||||
pub subagent_depth: u32,
|
||||
pub subagents_max_depth: u32,
|
||||
pub session_id_str: String,
|
||||
pub blocking_wait_depth: Arc<crate::tools::tool_context::BlockingWaitState>,
|
||||
pub respect_gitignore: bool,
|
||||
pub path_not_found_hints: bool,
|
||||
/// Fire side of the scheduler mode. The spawn copies the same resolution
|
||||
/// onto [`SessionHandle::scheduler_background_loops`](crate::session::SessionHandle),
|
||||
/// which is what clients read — keep the two on one resolve.
|
||||
pub scheduler_background_loops: bool,
|
||||
pub mcp_state: Arc<tokio::sync::Mutex<crate::session::mcp_servers::McpState>>,
|
||||
pub managed_gateway_tool_client:
|
||||
|
|
@ -219,6 +222,7 @@ impl AgentRebuildSpec {
|
|||
monitor_event_buffer,
|
||||
user_question_tx,
|
||||
subagent_depth,
|
||||
subagents_max_depth,
|
||||
session_id_str,
|
||||
blocking_wait_depth,
|
||||
respect_gitignore,
|
||||
|
|
@ -327,7 +331,7 @@ impl AgentRebuildSpec {
|
|||
ChannelBackend, SubagentBackendResource,
|
||||
};
|
||||
use xai_grok_tools::implementations::grok_build::task::types::{
|
||||
SessionIdResource, SubagentDepthCounter, SubagentEventSender,
|
||||
MaxSubagentDepth, SessionIdResource, SubagentDepthCounter, SubagentEventSender,
|
||||
};
|
||||
let backend = SubagentBackendResource(Arc::new(ChannelBackend::for_session(
|
||||
event_tx.clone(),
|
||||
|
|
@ -338,6 +342,10 @@ impl AgentRebuildSpec {
|
|||
.tool_bridge()
|
||||
.update_resource(SubagentDepthCounter(*subagent_depth))
|
||||
.await;
|
||||
agent
|
||||
.tool_bridge()
|
||||
.update_resource(MaxSubagentDepth(*subagents_max_depth))
|
||||
.await;
|
||||
agent
|
||||
.tool_bridge()
|
||||
.update_resource(SessionIdResource(session_id_str.clone()))
|
||||
|
|
@ -440,6 +448,7 @@ pub(crate) fn test_rebuild_spec_default() -> Arc<AgentRebuildSpec> {
|
|||
monitor_event_buffer: None,
|
||||
user_question_tx: uq_tx,
|
||||
subagent_depth: 0,
|
||||
subagents_max_depth: xai_grok_tools::implementations::grok_build::task::MAX_SUBAGENT_DEPTH,
|
||||
session_id_str: "test-session".to_string(),
|
||||
blocking_wait_depth: Arc::new(crate::tools::tool_context::BlockingWaitState::new()),
|
||||
respect_gitignore: false,
|
||||
|
|
|
|||
|
|
@ -34,6 +34,10 @@ pub enum SessionLiveState {
|
|||
/// `Dormant` on the next disk scan.
|
||||
DeadFailed,
|
||||
}
|
||||
/// `_meta` key carrying [`SessionHandle::scheduler_background_loops`] on the
|
||||
/// `session/new` and `session/load` responses. Defined here so the shell that
|
||||
/// publishes it and the clients that read it share one spelling.
|
||||
pub const SCHEDULER_BACKGROUND_LOOPS_META_KEY: &str = "x.ai/schedulerBackgroundLoops";
|
||||
/// Handle for interacting with a session actor.
|
||||
/// Note: Permission event receivers are returned separately from `spawn_session_actor`
|
||||
/// and should be stored/managed by the caller.
|
||||
|
|
@ -108,6 +112,14 @@ pub struct SessionHandle {
|
|||
/// Per-session tracking prevents cross-client contamination in leader mode
|
||||
/// where `MvpAgent.current_model_id` is shared mutable state.
|
||||
pub model_id: acp::ModelId,
|
||||
/// Whether this session's scheduled fires run as detached background
|
||||
/// subagents. Copied from the value the spawn resolved for the session's
|
||||
/// [`AgentRebuildSpec`](crate::session::agent_rebuild::AgentRebuildSpec), so
|
||||
/// it is pinned for the session's whole life exactly like the fire side.
|
||||
/// Published to clients on the `session/new` / `session/load` response so
|
||||
/// they describe the fires this session will actually get rather than
|
||||
/// re-resolving a setting that may have flipped since spawn.
|
||||
pub scheduler_background_loops: bool,
|
||||
pub reasoning_effort: Option<ReasoningEffort>,
|
||||
/// YOLO (auto-approve) mode for this session.
|
||||
/// Per-session tracking prevents cross-client contamination in leader mode
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
use std::collections::{HashMap, HashSet};
|
||||
|
||||
use agent_client_protocol as acp;
|
||||
use xai_grok_tools::implementations::grok_build::LoopFireMode;
|
||||
use xai_grok_tools::implementations::skills::skill::format_skill_name;
|
||||
use xai_grok_tools::implementations::skills::types::SkillInfo;
|
||||
|
||||
|
|
@ -1120,6 +1121,7 @@ pub(super) fn resolve(
|
|||
availability: CommandAvailability,
|
||||
_skill_rewrite: SkillSlashRewrite,
|
||||
workflows: &[crate::session::workflow::registry::WorkflowListing],
|
||||
loop_fire_mode: LoopFireMode,
|
||||
) -> Result<Vec<acp::ContentBlock>, SlashCommandOutcome> {
|
||||
let Some((command_name, args)) = parse_slash_prefix(&prompt_blocks) else {
|
||||
return Ok(prompt_blocks);
|
||||
|
|
@ -1137,7 +1139,7 @@ pub(super) fn resolve(
|
|||
// matching arm fails loudly at the call site instead of silently
|
||||
// reusing /loop's prompt builder.
|
||||
let mut blocks = match prompt_cmd.name {
|
||||
"loop" => build_loop_prompt_blocks(args),
|
||||
"loop" => build_loop_prompt_blocks(args, loop_fire_mode),
|
||||
other => {
|
||||
unreachable!("prompt-only command /{other} has no resolver wired in resolve()")
|
||||
}
|
||||
|
|
@ -1247,7 +1249,7 @@ fn parse_slash_prefix(prompt_blocks: &[acp::ContentBlock]) -> Option<(&str, &str
|
|||
/// two front-ends can't drift. Like the pager, there is no host-side interval
|
||||
/// default: the model derives the cadence from the request and asks when none
|
||||
/// is given.
|
||||
fn build_loop_prompt_blocks(args: &str) -> Vec<acp::ContentBlock> {
|
||||
fn build_loop_prompt_blocks(args: &str, mode: LoopFireMode) -> Vec<acp::ContentBlock> {
|
||||
use xai_grok_tools::implementations::grok_build::{
|
||||
loop_schedule_instruction, loop_usage_message,
|
||||
};
|
||||
|
|
@ -1255,7 +1257,7 @@ fn build_loop_prompt_blocks(args: &str) -> Vec<acp::ContentBlock> {
|
|||
let text = if args.trim().is_empty() {
|
||||
loop_usage_message().to_string()
|
||||
} else {
|
||||
loop_schedule_instruction(args)
|
||||
loop_schedule_instruction(args, mode)
|
||||
};
|
||||
|
||||
vec![acp::ContentBlock::Text(acp::TextContent::new(text))]
|
||||
|
|
@ -1266,6 +1268,27 @@ mod tests {
|
|||
use super::*;
|
||||
use xai_grok_tools::implementations::skills::types::SkillScope;
|
||||
|
||||
/// Shadows [`super::resolve`] for the cases that route something other
|
||||
/// than `/loop`: they are indifferent to the fire mode, and pinning it
|
||||
/// here keeps a plumbing change out of every unrelated call site. Tests
|
||||
/// that care about the mode call `super::resolve` directly.
|
||||
fn resolve(
|
||||
prompt_blocks: Vec<acp::ContentBlock>,
|
||||
skills: &[SkillInfo],
|
||||
availability: CommandAvailability,
|
||||
skill_rewrite: SkillSlashRewrite,
|
||||
workflows: &[crate::session::workflow::registry::WorkflowListing],
|
||||
) -> Result<Vec<acp::ContentBlock>, SlashCommandOutcome> {
|
||||
super::resolve(
|
||||
prompt_blocks,
|
||||
skills,
|
||||
availability,
|
||||
skill_rewrite,
|
||||
workflows,
|
||||
LoopFireMode::Detached,
|
||||
)
|
||||
}
|
||||
|
||||
fn all_gated() -> CommandAvailability {
|
||||
CommandAvailability::all_enabled()
|
||||
}
|
||||
|
|
@ -1586,6 +1609,36 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_loop_expands_for_the_sessions_fire_mode() {
|
||||
let text_of = |mode| {
|
||||
let outcome = super::resolve(
|
||||
vec![text_block("/loop 1m echo hello")],
|
||||
&[],
|
||||
all_gated(),
|
||||
SkillSlashRewrite::default(),
|
||||
&[],
|
||||
mode,
|
||||
)
|
||||
.unwrap_err();
|
||||
let SlashCommandOutcome::InvokeSkill { blocks, .. } = outcome else {
|
||||
panic!("expected InvokeSkill for /loop");
|
||||
};
|
||||
let Some(acp::ContentBlock::Text(tb)) = blocks.into_iter().next() else {
|
||||
panic!("expected a text block");
|
||||
};
|
||||
tb.text
|
||||
};
|
||||
assert!(
|
||||
text_of(LoopFireMode::Detached).contains("cannot see this conversation"),
|
||||
"detached sessions must get the standalone-prompt framing"
|
||||
);
|
||||
assert!(
|
||||
text_of(LoopFireMode::InSession).contains("arrives as a new turn in this conversation"),
|
||||
"in-session sessions must get the standing-order framing"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_passthrough_preserves_original_blocks() {
|
||||
// External-harness agents: blocks are passed through verbatim.
|
||||
|
|
@ -1838,8 +1891,8 @@ mod tests {
|
|||
}
|
||||
|
||||
/// Extract the text of the first block produced by `build_loop_prompt_blocks`.
|
||||
fn loop_text(args: &str) -> String {
|
||||
match build_loop_prompt_blocks(args).into_iter().next() {
|
||||
fn loop_text(args: &str, mode: LoopFireMode) -> String {
|
||||
match build_loop_prompt_blocks(args, mode).into_iter().next() {
|
||||
Some(acp::ContentBlock::Text(t)) => t.text,
|
||||
other => panic!("expected a text block, got {other:?}"),
|
||||
}
|
||||
|
|
@ -1848,7 +1901,7 @@ mod tests {
|
|||
#[test]
|
||||
fn loop_usage_has_no_10m_default() {
|
||||
// The shell client must not advertise a silent 10m default.
|
||||
let usage = loop_text("");
|
||||
let usage = loop_text("", LoopFireMode::Detached);
|
||||
assert!(usage.contains("Usage: /loop"), "got: {usage}");
|
||||
assert!(
|
||||
!usage.contains("10m"),
|
||||
|
|
@ -1858,7 +1911,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn loop_instruction_derives_interval_without_default_or_inline_execute() {
|
||||
let instr = loop_text("every 30 minutes do x");
|
||||
let instr = loop_text("every 30 minutes do x", LoopFireMode::Detached);
|
||||
assert!(
|
||||
!instr.contains("10m"),
|
||||
"instruction must not default: {instr}"
|
||||
|
|
@ -1880,11 +1933,13 @@ mod tests {
|
|||
use xai_grok_tools::implementations::grok_build::{
|
||||
loop_schedule_instruction, loop_usage_message,
|
||||
};
|
||||
assert_eq!(loop_text(""), loop_usage_message());
|
||||
assert_eq!(
|
||||
loop_text("2h run tests"),
|
||||
loop_schedule_instruction("2h run tests")
|
||||
);
|
||||
assert_eq!(loop_text("", LoopFireMode::Detached), loop_usage_message());
|
||||
for mode in [LoopFireMode::Detached, LoopFireMode::InSession] {
|
||||
assert_eq!(
|
||||
loop_text("2h run tests", mode),
|
||||
loop_schedule_instruction("2h run tests", mode)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -3003,6 +3003,42 @@ mod tests {
|
|||
assert!(result[3].contains("resp3"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_rewind_ignores_a_malformed_middle_line() {
|
||||
let user_message_1 = acp_envelope(
|
||||
r#"{"sessionUpdate":"user_message_chunk","content":{"type":"text","text":"first"}}"#,
|
||||
);
|
||||
let agent_message_1 = acp_envelope(
|
||||
r#"{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"resp1"}}"#,
|
||||
);
|
||||
let user_message_2 = acp_envelope(
|
||||
r#"{"sessionUpdate":"user_message_chunk","content":{"type":"text","text":"second"}}"#,
|
||||
);
|
||||
let agent_message_2 = acp_envelope(
|
||||
r#"{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"resp2"}}"#,
|
||||
);
|
||||
let rewind_to_1 = xai_envelope(
|
||||
r#"{"sessionUpdate":"rewind_marker","target_prompt_index":1,"created_at":"2024-01-01"}"#,
|
||||
);
|
||||
let torn = "{ torn, unparseable jsonl line";
|
||||
|
||||
// The malformed line is kept but not counted as a prompt boundary, so
|
||||
// the rewind still drops prompt 1.
|
||||
let survivors = filter_rewind_lines(vec![
|
||||
user_message_1.as_str(),
|
||||
agent_message_1.as_str(),
|
||||
torn,
|
||||
user_message_2.as_str(),
|
||||
agent_message_2.as_str(),
|
||||
rewind_to_1.as_str(),
|
||||
]);
|
||||
|
||||
pretty_assertions::assert_eq!(
|
||||
survivors,
|
||||
vec![user_message_1.as_str(), agent_message_1.as_str(), torn]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_rewind_to_zero_clears_all() {
|
||||
let u1 = acp_envelope(
|
||||
|
|
|
|||
|
|
@ -633,7 +633,7 @@ async fn reindex_all(root_dir: &Path, storage: &dyn StorageAdapter) -> io::Resul
|
|||
&& should_skip_session(path, max_file_size)
|
||||
{
|
||||
let file_size = std::fs::metadata(path).map(|m| m.len()).unwrap_or(0);
|
||||
tracing::warn!(
|
||||
tracing::debug!(
|
||||
session_id = %session_id,
|
||||
file_size = file_size,
|
||||
max_size = max_file_size,
|
||||
|
|
|
|||
|
|
@ -7,4 +7,7 @@ pub mod bench;
|
|||
pub mod replay;
|
||||
|
||||
pub use bench::synthesize_to_target_bytes;
|
||||
pub use replay::{SessionSpec, locate_session_dir, prepare_session, sid, write_rewind_jsonl};
|
||||
pub use replay::{
|
||||
SessionSpec, expected_replay_lines, locate_session_dir, prepare_session, sid,
|
||||
write_rewind_jsonl,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -165,6 +165,12 @@ fn write_updates_jsonl(path: &Path, session_id: &str, spec: &SessionSpec) {
|
|||
std::fs::write(path, out).expect("write updates.jsonl");
|
||||
}
|
||||
|
||||
/// Replay keeps the per-turn user and agent chunks and drops ACUs; keep in sync
|
||||
/// with `write_updates_jsonl`.
|
||||
pub fn expected_replay_lines(spec: &SessionSpec) -> usize {
|
||||
spec.turns * (1 + spec.agent_chunks_per_turn)
|
||||
}
|
||||
|
||||
pub fn write_rewind_jsonl(path: &Path, spec: &SessionSpec) {
|
||||
let mut out = String::new();
|
||||
for p in 0..spec.rewind_points {
|
||||
|
|
|
|||
|
|
@ -82,6 +82,7 @@ pub(crate) static TASK_COMPLETED: LazyLock<String> = LazyLock::new(|| {
|
|||
explicitly_killed: false,
|
||||
owner_session_id: None,
|
||||
description: None,
|
||||
is_backgrounded: false,
|
||||
},
|
||||
will_wake: false,
|
||||
})
|
||||
|
|
|
|||
|
|
@ -102,6 +102,8 @@ impl TrackedTask {
|
|||
kind: self.kind,
|
||||
owner_session_id: self.owner_session_id.clone(),
|
||||
description: self.description.clone(),
|
||||
// ACP tracked tasks are only registered via run_background.
|
||||
is_backgrounded: true,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -104,6 +104,7 @@ pub(crate) fn ctx_with_toggle(toggle: HashMap<String, bool>) -> SubagentSpawnCon
|
|||
gcs_upload_method: None,
|
||||
hook_registry: None,
|
||||
parent_depth: 0,
|
||||
subagents_max_depth: xai_grok_tools::implementations::grok_build::task::MAX_SUBAGENT_DEPTH,
|
||||
inference_idle_timeout_secs: 600,
|
||||
auto_compact_threshold_tiers: crate::agent::subagent::AutoCompactThresholdTiers::default(),
|
||||
permission_handle: None,
|
||||
|
|
|
|||
|
|
@ -937,6 +937,7 @@ mod tests {
|
|||
explicitly_killed: false,
|
||||
owner_session_id: None,
|
||||
description: None,
|
||||
is_backgrounded: false,
|
||||
}
|
||||
}
|
||||
#[tokio::test]
|
||||
|
|
@ -2224,6 +2225,7 @@ mod tests {
|
|||
explicitly_killed: false,
|
||||
owner_session_id: None,
|
||||
description: None,
|
||||
is_backgrounded: false,
|
||||
}
|
||||
}
|
||||
/// Extract the auto-wake prompt text emitted on the session command channel.
|
||||
|
|
|
|||
|
|
@ -178,7 +178,7 @@ pub struct ToolContext {
|
|||
/// (`inject_pending_monitor_events`) and surfaced as ONE hidden
|
||||
/// synthetic user message before the next sampling step.
|
||||
pub monitor_event_buffer:
|
||||
Option<xai_grok_tools::implementations::grok_build::task::types::MonitorEventBuffer>,
|
||||
Option<xai_grok_tools::implementations::grok_build::monitor::types::MonitorEventBuffer>,
|
||||
pub task_completion_reservations:
|
||||
Option<xai_grok_tools::reminders::task_completion::TaskCompletionReservations>,
|
||||
pub task_wake_suppressed:
|
||||
|
|
|
|||
|
|
@ -53,6 +53,8 @@ pub fn resolve_remote_fetch_enabled() -> bool {
|
|||
}
|
||||
}
|
||||
|
||||
pub const REMOTE_FETCH_CONFIG_PATH: &str = "features.remote_fetch";
|
||||
|
||||
fn remote_fetch_value(v: &TomlValue) -> Option<bool> {
|
||||
v.get("features")?.get("remote_fetch")?.as_bool()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -112,11 +112,13 @@ pub fn resolve_tips_from_disk(
|
|||
crate::util::tips::pick_and_advance(&all, grok_home)
|
||||
}
|
||||
|
||||
pub const SLASH_COMMAND_TAGS_CONFIG_PATH: &str = "slash_command_tags";
|
||||
|
||||
/// Parse `[slash_command_tags]` from a TOML value into a name → tag map.
|
||||
/// Only string values are kept; non-string entries are ignored.
|
||||
fn slash_command_tags_from_toml(root: &TomlValue) -> std::collections::HashMap<String, String> {
|
||||
let mut out = std::collections::HashMap::new();
|
||||
if let Some(TomlValue::Table(table)) = root.get("slash_command_tags") {
|
||||
if let Some(TomlValue::Table(table)) = root.get(SLASH_COMMAND_TAGS_CONFIG_PATH) {
|
||||
for (name, value) in table {
|
||||
if let Some(tag) = value.as_str() {
|
||||
out.insert(name.clone(), tag.to_string());
|
||||
|
|
|
|||
|
|
@ -0,0 +1,39 @@
|
|||
//! Defense-in-depth: `connect_or_spawn` must refuse when a non-`off` sandbox
|
||||
//! profile was requested, before any socket discovery or leader spawn.
|
||||
//!
|
||||
//! Own binary: `set_configured_profile` writes a process-global `OnceLock` that
|
||||
//! other unit tests in this crate also set.
|
||||
|
||||
use xai_grok_shell::leader::{
|
||||
ClientCapabilities, ClientMode, ConnectionError, LeaderEnvUrls, connect_or_spawn,
|
||||
};
|
||||
|
||||
#[tokio::test]
|
||||
async fn connect_or_spawn_refuses_when_sandbox_confinement_requested() {
|
||||
xai_grok_sandbox::set_configured_profile("strict");
|
||||
|
||||
let env_urls = LeaderEnvUrls {
|
||||
// Guard returns before LeaderLock / socket paths touch the filesystem.
|
||||
grok_ws_url: "wss://test.invalid/sandbox-confinement".into(),
|
||||
grok_ws_origin: "https://test.invalid".into(),
|
||||
};
|
||||
|
||||
let err = match connect_or_spawn(
|
||||
"test-sandbox-confinement",
|
||||
ClientMode::Stdio,
|
||||
&env_urls,
|
||||
ClientCapabilities::default(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(_) => panic!(
|
||||
"confined client must not adopt or spawn a leader (connect_or_spawn returned Ok)"
|
||||
),
|
||||
Err(err) => err,
|
||||
};
|
||||
|
||||
assert!(
|
||||
matches!(err, ConnectionError::SandboxConfinement("strict")),
|
||||
"expected SandboxConfinement(\"strict\"), got {err:?}"
|
||||
);
|
||||
}
|
||||
|
|
@ -12,8 +12,10 @@
|
|||
#[global_allocator]
|
||||
static DHAT_ALLOC: dhat::Alloc = dhat::Alloc;
|
||||
|
||||
use pretty_assertions::assert_eq;
|
||||
use xai_grok_shell::session::storage::{JsonlStorageAdapter, StorageAdapter, prepare_replay_lines};
|
||||
use xai_grok_shell::session::testkit::synth::{self, SessionSpec};
|
||||
use xai_grok_test_support::env::env_parse;
|
||||
|
||||
#[cfg(feature = "dhat-heap")]
|
||||
use std::path::Path;
|
||||
|
|
@ -31,29 +33,10 @@ fn file_len(path: &std::path::Path) -> u64 {
|
|||
std::fs::metadata(path).expect("stat updates.jsonl").len()
|
||||
}
|
||||
|
||||
fn env_parse<T: std::str::FromStr>(key: &str, default: T) -> T {
|
||||
let Ok(text) = std::env::var(key) else {
|
||||
return default;
|
||||
};
|
||||
match text.parse() {
|
||||
Ok(value) => value,
|
||||
Err(_) => {
|
||||
eprintln!("[test_session_load] ignoring unparseable {key}={text:?}; using default");
|
||||
default
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn memory_spec() -> SessionSpec {
|
||||
SessionSpec::from_env_prefixed("SESSION_LOAD", SessionSpec::default())
|
||||
}
|
||||
|
||||
// Replay keeps one user chunk plus the agent chunks per turn and drops the
|
||||
// redundant ACUs, mirroring `synth::prepare_session` and `prepare_replay_lines`.
|
||||
fn expected_replayed_lines(spec: &SessionSpec) -> usize {
|
||||
spec.turns * (1 + spec.agent_chunks_per_turn)
|
||||
}
|
||||
|
||||
/// Non-ignored zero-copy guard: every replay line must borrow from the
|
||||
/// transcript, so an owned-copy regression fails here in CI.
|
||||
#[tokio::test]
|
||||
|
|
@ -78,7 +61,7 @@ async fn prepare_replay_lines_borrows_the_transcript() {
|
|||
let prepared = prepare_replay_lines(&transcript, None);
|
||||
assert_eq!(
|
||||
prepared.lines.len(),
|
||||
expected_replayed_lines(&spec),
|
||||
synth::expected_replay_lines(&spec),
|
||||
"replay line count regressed"
|
||||
);
|
||||
|
||||
|
|
@ -353,7 +336,7 @@ async fn session_load_dhat_bounded_and_freed() {
|
|||
let (info, dir) = synth::prepare_session(root.path(), cwd.path(), &opts).await;
|
||||
let updates_path = dir.join("updates.jsonl");
|
||||
let on_disk_bytes = file_len(&updates_path);
|
||||
let expected_lines = expected_replayed_lines(&opts);
|
||||
let expected_lines = synth::expected_replay_lines(&opts);
|
||||
|
||||
let budget = DhatBudget {
|
||||
warmup: env_parse("SESSION_LOAD_WARMUP", 3usize),
|
||||
|
|
@ -366,12 +349,13 @@ async fn session_load_dhat_bounded_and_freed() {
|
|||
|
||||
let adapter = JsonlStorageAdapter::with_root(root.path().to_path_buf());
|
||||
|
||||
let profiler = dhat::Profiler::builder().testing().build();
|
||||
|
||||
// Warm up before starting the profiler so its lifetime `max_bytes` covers
|
||||
// only the measured window, not a warmup transient.
|
||||
for _ in 0..budget.warmup {
|
||||
let _ = run_load_cycle(&adapter, &info, &updates_path).await;
|
||||
}
|
||||
|
||||
let profiler = dhat::Profiler::builder().testing().build();
|
||||
let window_before = dhat::HeapStats::get();
|
||||
let mut replayed_lines = 0usize;
|
||||
for _ in 0..budget.cycles {
|
||||
|
|
@ -380,9 +364,8 @@ async fn session_load_dhat_bounded_and_freed() {
|
|||
let window_after = dhat::HeapStats::get();
|
||||
drop(profiler);
|
||||
|
||||
// `max_bytes` is a running maximum over the profiler's whole life (warmup
|
||||
// included), so subtracting the post-warmup baseline yields a conservative
|
||||
// upper bound on the load peak, never an underestimate.
|
||||
// `max_bytes` spans only the measured window, so the peak over its starting
|
||||
// baseline is a true load peak rather than a warmup artifact.
|
||||
let peak_over_baseline =
|
||||
(window_after.max_bytes as u64).saturating_sub(window_before.curr_bytes as u64);
|
||||
|
||||
|
|
@ -412,6 +395,7 @@ async fn session_load_dhat_bounded_and_freed() {
|
|||
#[cfg(not(feature = "dhat-heap"))]
|
||||
mod rss {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use std::cell::RefCell;
|
||||
use std::path::PathBuf;
|
||||
|
|
@ -508,7 +492,7 @@ mod rss {
|
|||
}
|
||||
|
||||
fn pass(&self) -> bool {
|
||||
!self.measurable() || self.within_budget()
|
||||
self.measurable() && self.within_budget()
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -533,14 +517,14 @@ mod rss {
|
|||
assert!(under.within_budget());
|
||||
assert!(under.pass());
|
||||
|
||||
// An unmeasurable baseline passes vacuously.
|
||||
// Unmeasurable RSS fails the gate rather than passing vacuously.
|
||||
let unmeasurable = RssOutcome {
|
||||
baseline: None,
|
||||
peak_rss: 0,
|
||||
budget_mb: 1,
|
||||
};
|
||||
assert!(!unmeasurable.measurable());
|
||||
assert!(unmeasurable.pass());
|
||||
assert!(!unmeasurable.pass());
|
||||
}
|
||||
|
||||
fn report_summary(mode: &str, counts: serde_json::Value, on_disk_bytes: u64, o: &RssOutcome) {
|
||||
|
|
@ -566,19 +550,21 @@ mod rss {
|
|||
}
|
||||
|
||||
fn assert_bounds(label: Option<&str>, on_disk_bytes: u64, o: &RssOutcome) {
|
||||
if o.measurable() {
|
||||
let prefix = label.map(|l| format!("{l} ")).unwrap_or_default();
|
||||
assert!(
|
||||
o.within_budget(),
|
||||
"{prefix}peak RSS grew {:.1} MB over baseline while loading a {:.1} MB updates file \
|
||||
(bound {} MB)",
|
||||
o.peak_growth_bytes() as f64 / BYTES_PER_MB,
|
||||
on_disk_bytes as f64 / BYTES_PER_MB,
|
||||
o.budget_mb,
|
||||
);
|
||||
} else {
|
||||
eprintln!("[soak] RSS measurement unavailable on this platform; bound skipped");
|
||||
}
|
||||
let prefix = label.map(|l| format!("{l} ")).unwrap_or_default();
|
||||
// This soak exists to enforce a bound, so unmeasurable RSS is a failure,
|
||||
// not a silent skip.
|
||||
assert!(
|
||||
o.measurable(),
|
||||
"{prefix}RSS sampling unavailable; the soak cannot enforce a bound"
|
||||
);
|
||||
assert!(
|
||||
o.within_budget(),
|
||||
"{prefix}peak RSS grew {:.1} MB over baseline while loading a {:.1} MB updates file \
|
||||
(bound {} MB)",
|
||||
o.peak_growth_bytes() as f64 / BYTES_PER_MB,
|
||||
on_disk_bytes as f64 / BYTES_PER_MB,
|
||||
o.budget_mb,
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
|
|
@ -590,7 +576,7 @@ mod rss {
|
|||
let (info, dir) = synth::prepare_session(root.path(), cwd.path(), &opts).await;
|
||||
let updates_path = dir.join("updates.jsonl");
|
||||
let on_disk_bytes = file_len(&updates_path);
|
||||
let expected_lines = expected_replayed_lines(&opts);
|
||||
let expected_lines = synth::expected_replay_lines(&opts);
|
||||
|
||||
let budget_mb = env_parse("SESSION_LOAD_MAX_PEAK_MB", 1024u64);
|
||||
let adapter = JsonlStorageAdapter::with_root(root.path().to_path_buf());
|
||||
|
|
|
|||
|
|
@ -1,796 +0,0 @@
|
|||
//! Subagent lifecycle soak: churn spawn/run/completion/eviction and assert
|
||||
//! threads, fds, and heap/RSS reach steady state. A stub `ChildRunner` drives
|
||||
//! the real coordinator/transport.
|
||||
//!
|
||||
//! SUBAGENT_SOAK_CYCLES=20000 cargo test -p xai-grok-shell \
|
||||
//! [--features dhat-heap] --test test_subagent_soak -- --ignored --nocapture
|
||||
|
||||
#![cfg(unix)]
|
||||
|
||||
#[cfg(feature = "dhat-heap")]
|
||||
#[global_allocator]
|
||||
static DHAT_ALLOC: dhat::Alloc = dhat::Alloc;
|
||||
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
use serde::ser::SerializeMap;
|
||||
use serde::{Serialize, Serializer};
|
||||
use strum::{EnumCount, IntoEnumIterator};
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
use xai_grok_test_support::env::env_parse;
|
||||
use xai_grok_test_support::resources::{ResourceGrowth, ResourceSnapshot};
|
||||
use xai_grok_tools::implementations::grok_build::task::backend::{ChannelBackend, SubagentBackend};
|
||||
use xai_grok_tools::implementations::grok_build::task::coordinator::{
|
||||
ChildCompletion, ChildControl, ChildRunOutput, ChildRunRequest, ChildRunner, CoordinatorConfig,
|
||||
LocalBoxFuture, MAX_COMPLETED_ENTRIES, StartedChild, SubagentCoordinator, SubagentProgress,
|
||||
};
|
||||
use xai_grok_tools::implementations::grok_build::task::types::{
|
||||
SubagentDescribeOutcome, SubagentOwner, SubagentRegistryCounts, SubagentRequest,
|
||||
SubagentResult, SubagentValidateTypeOutcome,
|
||||
};
|
||||
|
||||
const PARENT_SESSION_ID: &str = "subagent-soak-parent";
|
||||
|
||||
#[derive(Clone, Copy, strum::EnumCount, strum::EnumIter)]
|
||||
enum Metric {
|
||||
Rss,
|
||||
Threads,
|
||||
Fds,
|
||||
}
|
||||
|
||||
impl Metric {
|
||||
fn label(self) -> &'static str {
|
||||
match self {
|
||||
Metric::Rss => "rss",
|
||||
Metric::Threads => "threads",
|
||||
Metric::Fds => "fds",
|
||||
}
|
||||
}
|
||||
|
||||
/// RSS reports raw bytes, so its key names the unit.
|
||||
fn summary_key(self) -> &'static str {
|
||||
match self {
|
||||
Metric::Rss => "rss_bytes",
|
||||
Metric::Threads => "threads",
|
||||
Metric::Fds => "fds",
|
||||
}
|
||||
}
|
||||
|
||||
fn unit(self) -> Option<&'static str> {
|
||||
match self {
|
||||
Metric::Rss => Some("MiB"),
|
||||
Metric::Threads | Metric::Fds => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn budget(self, bounds: &Bounds) -> f64 {
|
||||
match self {
|
||||
Metric::Rss => bounds.max_rss_growth_mib as f64,
|
||||
Metric::Threads => bounds.max_thread_growth as f64,
|
||||
Metric::Fds => bounds.max_fd_growth as f64,
|
||||
}
|
||||
}
|
||||
|
||||
/// RSS growth samples are bytes; convert to MiB for the budget comparison.
|
||||
fn growth_in_budget_unit(self, raw: usize) -> f64 {
|
||||
match self {
|
||||
Metric::Rss => bytes_to_mib(raw),
|
||||
Metric::Threads | Metric::Fds => raw as f64,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Reads a metric's field from a snapshot or a growth delta so serialization and
|
||||
/// the gates share one projection instead of repeating it.
|
||||
trait MetricValue {
|
||||
fn value_of(&self, metric: Metric) -> Option<usize>;
|
||||
}
|
||||
|
||||
impl MetricValue for ResourceSnapshot {
|
||||
fn value_of(&self, metric: Metric) -> Option<usize> {
|
||||
// Destructure so a new resource field is a compile error here, not a
|
||||
// silently dropped metric.
|
||||
let ResourceSnapshot { rss, threads, fds } = *self;
|
||||
match metric {
|
||||
Metric::Rss => rss,
|
||||
Metric::Threads => threads,
|
||||
Metric::Fds => fds,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl MetricValue for ResourceGrowth {
|
||||
fn value_of(&self, metric: Metric) -> Option<usize> {
|
||||
let ResourceGrowth { rss, threads, fds } = *self;
|
||||
match metric {
|
||||
Metric::Rss => rss,
|
||||
Metric::Threads => threads,
|
||||
Metric::Fds => fds,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn serialize_metrics<T: MetricValue, S: Serializer>(
|
||||
value: &T,
|
||||
serializer: S,
|
||||
) -> Result<S::Ok, S::Error> {
|
||||
let mut map = serializer.serialize_map(Some(Metric::COUNT))?;
|
||||
for metric in Metric::iter() {
|
||||
map.serialize_entry(metric.summary_key(), &value.value_of(metric))?;
|
||||
}
|
||||
map.end()
|
||||
}
|
||||
|
||||
fn bytes_to_mib(bytes: usize) -> f64 {
|
||||
bytes as f64 / (1024.0 * 1024.0)
|
||||
}
|
||||
|
||||
#[cfg_attr(not(feature = "dhat-heap"), allow(dead_code))]
|
||||
#[derive(Clone, Copy, Serialize)]
|
||||
struct HeapSample {
|
||||
blocks: i64,
|
||||
bytes: i64,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Serialize)]
|
||||
struct HeapMetrics {
|
||||
before: HeapSample,
|
||||
after: HeapSample,
|
||||
blocks_per_cycle: f64,
|
||||
bytes_per_cycle: f64,
|
||||
}
|
||||
|
||||
impl HeapMetrics {
|
||||
fn new(before: HeapSample, after: HeapSample, cycles: u64) -> Self {
|
||||
// `SUBAGENT_SOAK_CYCLES=0` would otherwise divide by zero and feed
|
||||
// NaN/inf into the leak gates.
|
||||
let cycles = cycles.max(1) as f64;
|
||||
Self {
|
||||
before,
|
||||
after,
|
||||
blocks_per_cycle: (after.blocks - before.blocks) as f64 / cycles,
|
||||
bytes_per_cycle: (after.bytes - before.bytes) as f64 / cycles,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
struct Bounds {
|
||||
#[serde(rename = "warmup_cycles")]
|
||||
warmup: u64,
|
||||
#[serde(rename = "measured_cycles")]
|
||||
measure: u64,
|
||||
max_thread_growth: u64,
|
||||
max_fd_growth: u64,
|
||||
max_rss_growth_mib: u64,
|
||||
max_blocks_per_cycle: f64,
|
||||
max_bytes_per_cycle: f64,
|
||||
}
|
||||
|
||||
impl Bounds {
|
||||
fn from_env() -> Self {
|
||||
Self {
|
||||
// Default warmup to the completed-entry cap so the ring is saturated
|
||||
// and the measured window observes steady-state eviction rather than
|
||||
// one-time cache fill.
|
||||
warmup: env_parse("SUBAGENT_SOAK_WARMUP", MAX_COMPLETED_ENTRIES as u64),
|
||||
measure: env_parse("SUBAGENT_SOAK_CYCLES", 512u64),
|
||||
max_thread_growth: env_parse("SUBAGENT_SOAK_MAX_THREAD_GROWTH", 32u64),
|
||||
max_fd_growth: env_parse("SUBAGENT_SOAK_MAX_FD_GROWTH", 64u64),
|
||||
max_rss_growth_mib: env_parse("SUBAGENT_SOAK_MAX_RSS_GROWTH_MIB", 256u64),
|
||||
max_blocks_per_cycle: env_parse("SUBAGENT_SOAK_MAX_BLOCKS_PER_CYCLE", 2.0f64),
|
||||
max_bytes_per_cycle: env_parse("SUBAGENT_SOAK_MAX_BYTES_PER_CYCLE", 4096.0f64),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
struct Measurement {
|
||||
#[serde(serialize_with = "serialize_metrics")]
|
||||
before: ResourceSnapshot,
|
||||
#[serde(serialize_with = "serialize_metrics")]
|
||||
after: ResourceSnapshot,
|
||||
#[serde(serialize_with = "serialize_metrics")]
|
||||
growth: ResourceGrowth,
|
||||
#[serde(serialize_with = "serialize_counts")]
|
||||
counts: SubagentRegistryCounts,
|
||||
heap: Option<HeapMetrics>,
|
||||
quiesced: bool,
|
||||
}
|
||||
|
||||
fn serialize_counts<S: Serializer>(
|
||||
counts: &SubagentRegistryCounts,
|
||||
serializer: S,
|
||||
) -> Result<S::Ok, S::Error> {
|
||||
// Exhaustive destructure so a new count field is a compile error here, not a
|
||||
// silently dropped summary key.
|
||||
let SubagentRegistryCounts {
|
||||
pending,
|
||||
active,
|
||||
completed,
|
||||
} = counts;
|
||||
let mut map = serializer.serialize_map(Some(3))?;
|
||||
map.serialize_entry("pending", pending)?;
|
||||
map.serialize_entry("active", active)?;
|
||||
map.serialize_entry("completed", completed)?;
|
||||
map.end()
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
struct Summary<'a> {
|
||||
#[serde(flatten)]
|
||||
bounds: &'a Bounds,
|
||||
#[serde(flatten)]
|
||||
measurement: &'a Measurement,
|
||||
}
|
||||
|
||||
fn heap_capture() -> Option<HeapSample> {
|
||||
#[cfg(feature = "dhat-heap")]
|
||||
{
|
||||
let stats = dhat::HeapStats::get();
|
||||
Some(HeapSample {
|
||||
blocks: stats.curr_blocks as i64,
|
||||
bytes: stats.curr_bytes as i64,
|
||||
})
|
||||
}
|
||||
#[cfg(not(feature = "dhat-heap"))]
|
||||
{
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
async fn quiesce(backend: &ChannelBackend) -> bool {
|
||||
const MAX_POLLS: usize = 200;
|
||||
const SLEEP: Duration = Duration::from_millis(5);
|
||||
for _ in 0..MAX_POLLS {
|
||||
let counts = backend.registry_counts().await;
|
||||
if counts.pending == 0 && counts.active == 0 {
|
||||
return true;
|
||||
}
|
||||
tokio::time::sleep(SLEEP).await;
|
||||
}
|
||||
let counts = backend.registry_counts().await;
|
||||
eprintln!(
|
||||
"[soak] quiesce budget expired with pending={} active={}; snapshot may be noisy",
|
||||
counts.pending, counts.active
|
||||
);
|
||||
false
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct SoakControl {
|
||||
cancellation: CancellationToken,
|
||||
}
|
||||
|
||||
impl ChildControl for SoakControl {
|
||||
type ProgressFuture = std::future::Ready<SubagentProgress>;
|
||||
|
||||
fn progress(&self) -> Self::ProgressFuture {
|
||||
std::future::ready(SubagentProgress::default())
|
||||
}
|
||||
|
||||
fn cancel(&self) {
|
||||
self.cancellation.cancel();
|
||||
}
|
||||
}
|
||||
|
||||
struct SoakRunner;
|
||||
|
||||
impl ChildRunner for SoakRunner {
|
||||
type Control = SoakControl;
|
||||
type CompletionData = ();
|
||||
type RunFuture = LocalBoxFuture<ChildRunOutput<()>>;
|
||||
type ValidateFuture = LocalBoxFuture<SubagentValidateTypeOutcome>;
|
||||
type DescribeFuture = LocalBoxFuture<SubagentDescribeOutcome>;
|
||||
|
||||
fn run(&self, run: ChildRunRequest<Self::Control>) -> Self::RunFuture {
|
||||
Box::pin(async move {
|
||||
let ChildRunRequest {
|
||||
request,
|
||||
cancellation,
|
||||
reporter,
|
||||
} = run;
|
||||
let promoted = reporter
|
||||
.started(StartedChild {
|
||||
child_session_id: request.id.clone(),
|
||||
persona: None,
|
||||
resumed_from: request.resume_from.clone(),
|
||||
child_cwd: request.cwd.clone().unwrap_or_default(),
|
||||
worktree_path: None,
|
||||
effective_model_id: "soak-model".to_owned(),
|
||||
definition_background: false,
|
||||
control: SoakControl {
|
||||
cancellation: cancellation.clone(),
|
||||
},
|
||||
})
|
||||
.await;
|
||||
if !promoted || cancellation.is_cancelled() {
|
||||
return ChildRunOutput {
|
||||
result: SubagentResult {
|
||||
success: false,
|
||||
cancelled: true,
|
||||
error: Some("cancelled before start".to_owned()),
|
||||
subagent_id: request.id.clone(),
|
||||
child_session_id: request.id,
|
||||
..Default::default()
|
||||
},
|
||||
completion_data: (),
|
||||
snapshot_ref: None,
|
||||
};
|
||||
}
|
||||
ChildRunOutput {
|
||||
result: SubagentResult {
|
||||
success: true,
|
||||
output: Arc::from("soak child output"),
|
||||
subagent_id: request.id.clone(),
|
||||
child_session_id: request.id,
|
||||
tool_calls: 1,
|
||||
turns: 1,
|
||||
..Default::default()
|
||||
},
|
||||
completion_data: (),
|
||||
snapshot_ref: None,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn validate_type(&self, _subagent_type: String, _parent: String) -> Self::ValidateFuture {
|
||||
Box::pin(std::future::ready(SubagentValidateTypeOutcome::Ok))
|
||||
}
|
||||
|
||||
fn describe_type(
|
||||
&self,
|
||||
_subagent_type: String,
|
||||
_harness_agent_type: Option<String>,
|
||||
_parent: String,
|
||||
) -> Self::DescribeFuture {
|
||||
Box::pin(std::future::ready(SubagentDescribeOutcome::Unavailable))
|
||||
}
|
||||
|
||||
fn on_completed(&self, _completion: ChildCompletion<Self::CompletionData>) {}
|
||||
}
|
||||
|
||||
fn soak_request(id: String, background: bool) -> SubagentRequest {
|
||||
SubagentRequest {
|
||||
id,
|
||||
prompt: "soak work".to_owned(),
|
||||
description: "soak child".to_owned(),
|
||||
subagent_type: "explore".to_owned(),
|
||||
parent_session_id: PARENT_SESSION_ID.to_owned(),
|
||||
parent_prompt_id: Some("soak-prompt".to_owned()),
|
||||
resume_from: None,
|
||||
cwd: None,
|
||||
runtime_overrides: Default::default(),
|
||||
run_in_background: background,
|
||||
surface_completion: true,
|
||||
await_to_completion: false,
|
||||
fork_context: false,
|
||||
owner: SubagentOwner::Task,
|
||||
cancel_token: CancellationToken::new(),
|
||||
}
|
||||
}
|
||||
|
||||
async fn run_cycle(backend: &ChannelBackend, i: u64) {
|
||||
let fg = backend
|
||||
.spawn(soak_request(format!("fg-{i}"), false))
|
||||
.await
|
||||
.expect("foreground spawn round-trips through the coordinator");
|
||||
assert!(fg.success, "cycle {i}: foreground child must complete");
|
||||
|
||||
let bg_id = format!("bg-{i}");
|
||||
let bg = backend
|
||||
.spawn(soak_request(bg_id.clone(), true))
|
||||
.await
|
||||
.expect("background spawn round-trips through the coordinator");
|
||||
assert!(bg.success, "cycle {i}: background child must complete");
|
||||
|
||||
let blocking = true;
|
||||
let timeout_ms = Some(5_000);
|
||||
let snapshot = backend.query(&bg_id, blocking, timeout_ms).await;
|
||||
assert!(
|
||||
snapshot.is_some(),
|
||||
"cycle {i}: completed subagent must be queryable"
|
||||
);
|
||||
}
|
||||
|
||||
async fn warmup(backend: &ChannelBackend, cycles: u64) -> bool {
|
||||
for i in 0..cycles {
|
||||
run_cycle(backend, i).await;
|
||||
}
|
||||
quiesce(backend).await
|
||||
}
|
||||
|
||||
async fn measure(backend: &ChannelBackend, bounds: &Bounds, warmup_quiesced: bool) -> Measurement {
|
||||
let heap_before = heap_capture();
|
||||
let before = ResourceSnapshot::capture();
|
||||
|
||||
// Continue ids past the warmup window so measured cycles use fresh entries
|
||||
// and keep exercising eviction instead of colliding with warmup ids.
|
||||
for i in bounds.warmup..(bounds.warmup + bounds.measure) {
|
||||
run_cycle(backend, i).await;
|
||||
}
|
||||
// A warmup that never drained already poisons the `before` baseline, so skip
|
||||
// the measured-window drain and report the window as not quiesced.
|
||||
let quiesced = warmup_quiesced && quiesce(backend).await;
|
||||
|
||||
let heap_after = heap_capture();
|
||||
let after = ResourceSnapshot::capture();
|
||||
let counts = backend.registry_counts().await;
|
||||
|
||||
Measurement {
|
||||
before,
|
||||
after,
|
||||
growth: after.growth_from(&before),
|
||||
counts,
|
||||
heap: heap_before
|
||||
.zip(heap_after)
|
||||
.map(|(before, after)| HeapMetrics::new(before, after, bounds.measure)),
|
||||
quiesced,
|
||||
}
|
||||
}
|
||||
|
||||
fn check_bounds(bounds: &Bounds, m: &Measurement) -> Vec<String> {
|
||||
// Drain first: a non-quiesced window has nonzero counts and noisy growth, so
|
||||
// report the quiesce failure alone; the gates below only mean anything once
|
||||
// drained.
|
||||
if !m.quiesced {
|
||||
return vec![
|
||||
"quiesce budget expired before the measured window drained; soak result is unreliable"
|
||||
.to_owned(),
|
||||
];
|
||||
}
|
||||
|
||||
let mut failures = Vec::new();
|
||||
if m.counts.pending != 0 {
|
||||
failures.push(format!(
|
||||
"no subagent may remain pending, saw {}",
|
||||
m.counts.pending
|
||||
));
|
||||
}
|
||||
if m.counts.active != 0 {
|
||||
failures.push(format!(
|
||||
"no subagent may remain active, saw {}",
|
||||
m.counts.active
|
||||
));
|
||||
}
|
||||
if m.counts.completed > MAX_COMPLETED_ENTRIES {
|
||||
failures.push(format!(
|
||||
"completed retention must stay bounded by its cap, saw {}",
|
||||
m.counts.completed
|
||||
));
|
||||
}
|
||||
|
||||
for metric in Metric::iter() {
|
||||
let Some(raw) = m.growth.value_of(metric) else {
|
||||
continue;
|
||||
};
|
||||
let growth = metric.growth_in_budget_unit(raw);
|
||||
let budget = metric.budget(bounds);
|
||||
if growth > budget {
|
||||
let unit = metric.unit().map(|u| format!(" {u}")).unwrap_or_default();
|
||||
failures.push(format!(
|
||||
"{}: grew {growth:.1}{unit} over the soak (bound {budget:.1}{unit})",
|
||||
metric.label()
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(h) = m.heap {
|
||||
let measure = bounds.measure;
|
||||
if h.blocks_per_cycle > bounds.max_blocks_per_cycle {
|
||||
failures.push(format!(
|
||||
"block-count leak: {:.3} blocks/cycle retained ({} over {measure} cycles) \
|
||||
exceeds the {} gate",
|
||||
h.blocks_per_cycle,
|
||||
h.after.blocks - h.before.blocks,
|
||||
bounds.max_blocks_per_cycle
|
||||
));
|
||||
}
|
||||
if h.bytes_per_cycle > bounds.max_bytes_per_cycle {
|
||||
failures.push(format!(
|
||||
"byte leak: {:.1} bytes/cycle retained ({} over {measure} cycles) \
|
||||
exceeds the {} gate",
|
||||
h.bytes_per_cycle,
|
||||
h.after.bytes - h.before.bytes,
|
||||
bounds.max_bytes_per_cycle
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
failures
|
||||
}
|
||||
|
||||
fn assert_bounds(bounds: &Bounds, m: &Measurement) {
|
||||
let failures = check_bounds(bounds, m);
|
||||
assert!(
|
||||
failures.is_empty(),
|
||||
"subagent soak bounds violated:\n - {}",
|
||||
failures.join("\n - ")
|
||||
);
|
||||
}
|
||||
|
||||
/// Keep this the only test in the binary that creates a `dhat::Profiler`.
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[ignore = "subagent soak; run with --ignored (SUBAGENT_SOAK_CYCLES bounds the measured window)"]
|
||||
async fn subagent_lifecycle_soak_bounds_threads_fds_and_heap() {
|
||||
#[cfg(feature = "dhat-heap")]
|
||||
let _profiler = dhat::Profiler::builder().testing().build();
|
||||
|
||||
let bounds = Bounds::from_env();
|
||||
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local
|
||||
.run_until(async move {
|
||||
let (command_tx, command_rx) = tokio::sync::mpsc::unbounded_channel();
|
||||
let config = CoordinatorConfig {
|
||||
foreground_budget: Duration::from_secs(600),
|
||||
..CoordinatorConfig::default()
|
||||
};
|
||||
tokio::task::spawn_local(
|
||||
SubagentCoordinator::new(command_rx, SoakRunner, config).run(),
|
||||
);
|
||||
let backend = ChannelBackend::new(command_tx);
|
||||
|
||||
let warmup_quiesced = warmup(&backend, bounds.warmup).await;
|
||||
let measurement = measure(&backend, &bounds, warmup_quiesced).await;
|
||||
|
||||
let summary = Summary {
|
||||
bounds: &bounds,
|
||||
measurement: &measurement,
|
||||
};
|
||||
eprintln!(
|
||||
"SUBAGENT_SOAK_SUMMARY {}",
|
||||
serde_json::to_string(&summary).expect("summary serializes")
|
||||
);
|
||||
|
||||
assert_bounds(&bounds, &measurement);
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn value_of_reads_the_matching_slot_of_snapshot_and_growth() {
|
||||
let snapshot = ResourceSnapshot {
|
||||
rss: Some(11),
|
||||
threads: Some(22),
|
||||
fds: Some(33),
|
||||
};
|
||||
assert_eq!(snapshot.value_of(Metric::Rss), Some(11));
|
||||
assert_eq!(snapshot.value_of(Metric::Threads), Some(22));
|
||||
assert_eq!(snapshot.value_of(Metric::Fds), Some(33));
|
||||
|
||||
let growth = ResourceGrowth {
|
||||
rss: Some(1),
|
||||
threads: None,
|
||||
fds: Some(3),
|
||||
};
|
||||
assert_eq!(growth.value_of(Metric::Rss), Some(1));
|
||||
assert_eq!(growth.value_of(Metric::Threads), None);
|
||||
assert_eq!(growth.value_of(Metric::Fds), Some(3));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn serialize_metrics_keys_match_summary_keys_in_order() {
|
||||
#[derive(Serialize)]
|
||||
struct Wrap(#[serde(serialize_with = "serialize_metrics")] ResourceSnapshot);
|
||||
let snapshot = ResourceSnapshot {
|
||||
rss: Some(1),
|
||||
threads: None,
|
||||
fds: Some(3),
|
||||
};
|
||||
let json = serde_json::to_string(&Wrap(snapshot)).expect("snapshot serializes");
|
||||
assert_eq!(json, r#"{"rss_bytes":1,"threads":null,"fds":3}"#);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bytes_to_mib_divides_by_1024_squared() {
|
||||
assert_eq!(bytes_to_mib(0), 0.0);
|
||||
assert_eq!(bytes_to_mib(1024 * 1024), 1.0);
|
||||
assert_eq!(bytes_to_mib(3 * 1024 * 1024), 3.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn growth_in_budget_unit_scales_only_rss() {
|
||||
assert_eq!(Metric::Rss.growth_in_budget_unit(2 * 1024 * 1024), 2.0);
|
||||
assert_eq!(Metric::Threads.growth_in_budget_unit(7), 7.0);
|
||||
assert_eq!(Metric::Fds.growth_in_budget_unit(7), 7.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn budget_reads_per_metric_bound() {
|
||||
let bounds = Bounds {
|
||||
warmup: 0,
|
||||
measure: 0,
|
||||
max_thread_growth: 3,
|
||||
max_fd_growth: 5,
|
||||
max_rss_growth_mib: 7,
|
||||
max_blocks_per_cycle: 1.0,
|
||||
max_bytes_per_cycle: 2.0,
|
||||
};
|
||||
assert_eq!(Metric::Rss.budget(&bounds), 7.0);
|
||||
assert_eq!(Metric::Threads.budget(&bounds), 3.0);
|
||||
assert_eq!(Metric::Fds.budget(&bounds), 5.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn heap_metrics_clamps_zero_cycles() {
|
||||
let before = HeapSample {
|
||||
blocks: 10,
|
||||
bytes: 100,
|
||||
};
|
||||
let after = HeapSample {
|
||||
blocks: 20,
|
||||
bytes: 400,
|
||||
};
|
||||
let heap = HeapMetrics::new(before, after, 0);
|
||||
assert!(heap.blocks_per_cycle.is_finite());
|
||||
assert!(heap.bytes_per_cycle.is_finite());
|
||||
assert_eq!(heap.blocks_per_cycle, 10.0);
|
||||
assert_eq!(heap.bytes_per_cycle, 300.0);
|
||||
}
|
||||
|
||||
fn generous_bounds() -> Bounds {
|
||||
Bounds {
|
||||
warmup: 0,
|
||||
measure: 4,
|
||||
max_thread_growth: 100,
|
||||
max_fd_growth: 100,
|
||||
max_rss_growth_mib: 100,
|
||||
max_blocks_per_cycle: 10.0,
|
||||
max_bytes_per_cycle: 10_000.0,
|
||||
}
|
||||
}
|
||||
|
||||
fn drained(growth: ResourceGrowth, heap: Option<HeapMetrics>) -> Measurement {
|
||||
Measurement {
|
||||
before: ResourceSnapshot::default(),
|
||||
after: ResourceSnapshot::default(),
|
||||
growth,
|
||||
counts: SubagentRegistryCounts {
|
||||
pending: 0,
|
||||
active: 0,
|
||||
completed: 0,
|
||||
},
|
||||
heap,
|
||||
quiesced: true,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_bounds_passes_a_clean_drained_window() {
|
||||
let m = drained(ResourceGrowth::default(), None);
|
||||
assert!(check_bounds(&generous_bounds(), &m).is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_bounds_reports_non_quiesce_first_and_alone() {
|
||||
let mut m = drained(ResourceGrowth::default(), None);
|
||||
m.quiesced = false;
|
||||
m.counts.pending = 3;
|
||||
let failures = check_bounds(&generous_bounds(), &m);
|
||||
assert_eq!(failures.len(), 1);
|
||||
assert!(failures[0].contains("quiesce"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_bounds_flags_over_budget_growth() {
|
||||
let growth = ResourceGrowth {
|
||||
rss: Some(200 * 1024 * 1024),
|
||||
threads: Some(0),
|
||||
fds: Some(0),
|
||||
};
|
||||
let failures = check_bounds(&generous_bounds(), &drained(growth, None));
|
||||
assert!(
|
||||
failures.iter().any(|f| f.starts_with("rss:")),
|
||||
"{failures:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_bounds_treats_the_budget_as_an_inclusive_max() {
|
||||
let growth = ResourceGrowth {
|
||||
rss: Some(100 * 1024 * 1024),
|
||||
threads: Some(100),
|
||||
fds: Some(100),
|
||||
};
|
||||
assert!(check_bounds(&generous_bounds(), &drained(growth, None)).is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_bounds_flags_nonzero_counts_and_heap_leak() {
|
||||
let mut m = drained(
|
||||
ResourceGrowth::default(),
|
||||
Some(HeapMetrics {
|
||||
before: HeapSample {
|
||||
blocks: 0,
|
||||
bytes: 0,
|
||||
},
|
||||
after: HeapSample {
|
||||
blocks: 0,
|
||||
bytes: 0,
|
||||
},
|
||||
blocks_per_cycle: 0.0,
|
||||
bytes_per_cycle: 1_000_000.0,
|
||||
}),
|
||||
);
|
||||
m.counts.active = 2;
|
||||
let failures = check_bounds(&generous_bounds(), &m);
|
||||
assert!(
|
||||
failures.iter().any(|f| f.contains("active")),
|
||||
"{failures:?}"
|
||||
);
|
||||
assert!(
|
||||
failures.iter().any(|f| f.contains("byte leak")),
|
||||
"{failures:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_bounds_flags_pending_while_quiesced() {
|
||||
let mut m = drained(ResourceGrowth::default(), None);
|
||||
m.counts.pending = 3;
|
||||
let failures = check_bounds(&generous_bounds(), &m);
|
||||
assert!(
|
||||
failures.iter().any(|f| f.contains("pending")),
|
||||
"{failures:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_bounds_flags_completed_over_cap() {
|
||||
let mut m = drained(ResourceGrowth::default(), None);
|
||||
m.counts.completed = MAX_COMPLETED_ENTRIES + 1;
|
||||
let failures = check_bounds(&generous_bounds(), &m);
|
||||
assert!(
|
||||
failures.iter().any(|f| f.contains("completed retention")),
|
||||
"{failures:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_bounds_flags_thread_and_fd_over_budget() {
|
||||
let growth = ResourceGrowth {
|
||||
rss: Some(0),
|
||||
threads: Some(200),
|
||||
fds: Some(200),
|
||||
};
|
||||
let failures = check_bounds(&generous_bounds(), &drained(growth, None));
|
||||
assert!(
|
||||
failures.iter().any(|f| f.starts_with("threads:")),
|
||||
"{failures:?}"
|
||||
);
|
||||
assert!(
|
||||
failures.iter().any(|f| f.starts_with("fds:")),
|
||||
"{failures:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_bounds_flags_block_count_leak() {
|
||||
let m = drained(
|
||||
ResourceGrowth::default(),
|
||||
Some(HeapMetrics {
|
||||
before: HeapSample {
|
||||
blocks: 0,
|
||||
bytes: 0,
|
||||
},
|
||||
after: HeapSample {
|
||||
blocks: 0,
|
||||
bytes: 0,
|
||||
},
|
||||
blocks_per_cycle: 50.0,
|
||||
bytes_per_cycle: 0.0,
|
||||
}),
|
||||
);
|
||||
let failures = check_bounds(&generous_bounds(), &m);
|
||||
assert!(
|
||||
failures.iter().any(|f| f.contains("block-count leak")),
|
||||
"{failures:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue