From 02d9359435d0e9c20a20945679389cdce441e431 Mon Sep 17 00:00:00 2001 From: "grokkybara[bot]" <304785771+grokkybara[bot]@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:54:34 +0000 Subject: [PATCH] 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 --- Cargo.lock | 4 + SOURCE_REV | 2 +- crates/build/xai-proto-build/src/lib.rs | 4 + .../codegen/xai-grok-config-types/src/lib.rs | 3 + crates/codegen/xai-grok-pager-bin/src/main.rs | 39 ++- .../docs/user-guide/15-agent-mode.md | 2 +- .../docs/user-guide/18-sandbox.md | 8 + .../src/app/acp_handler/settings.rs | 7 + .../src/app/acp_handler/tests/mod.rs | 1 + .../acp_handler/tests/queue_and_adoption.rs | 6 + .../src/app/acp_handler/tests/settings.rs | 128 +++++++ .../app/acp_handler/tests/turn_completion.rs | 1 + .../codegen/xai-grok-pager/src/app/actions.rs | 12 + .../xai-grok-pager/src/app/agent_view/mod.rs | 9 + .../src/app/agent_view/session.rs | 1 + .../xai-grok-pager/src/app/app_view.rs | 9 + .../src/app/dispatch/dashboard.rs | 4 + .../xai-grok-pager/src/app/dispatch/prompt.rs | 6 + .../src/app/dispatch/session/lifecycle.rs | 4 + .../src/app/dispatch/session/load.rs | 2 + .../src/app/dispatch/settings/ui.rs | 23 ++ .../src/app/dispatch/task_result.rs | 13 +- .../src/app/dispatch/tests/dashboard.rs | 2 + .../src/app/dispatch/tests/mod.rs | 1 + .../src/app/dispatch/tests/session/fork.rs | 1 + .../app/dispatch/tests/session/lifecycle.rs | 11 + .../src/app/dispatch/tests/session/load.rs | 24 ++ .../src/app/dispatch/tests/session/modal.rs | 1 + .../src/app/dispatch/tests/task_result.rs | 1 + .../xai-grok-pager/src/app/effects/helpers.rs | 19 ++ .../xai-grok-pager/src/app/effects/mod.rs | 13 +- .../xai-grok-pager/src/app/event_loop.rs | 54 ++- crates/codegen/xai-grok-pager/src/app/mod.rs | 243 ++++++++++++-- .../xai-grok-pager/src/settings/registry.rs | 7 + .../src/slash/commands/loop_cmd.rs | 39 ++- crates/codegen/xai-grok-sandbox/src/lib.rs | 22 ++ crates/codegen/xai-grok-shell/Cargo.toml | 1 + .../xai-grok-shell/src/agent/config.rs | 74 ++++- .../src/agent/config_model_override_parse.rs | 21 +- .../src/agent/mvp_agent/acp_agent.rs | 27 +- .../src/agent/mvp_agent/agent_ops.rs | 70 +++- .../xai-grok-shell/src/agent/mvp_agent/mod.rs | 7 +- .../agent/mvp_agent/subagent_coordinator.rs | 1 + .../src/agent/mvp_agent/tests.rs | 20 ++ .../src/agent/subagent/handle_request.rs | 4 +- .../xai-grok-shell/src/agent/subagent/mod.rs | 2 + .../codegen/xai-grok-shell/src/config/mod.rs | 53 ++- .../xai-grok-shell/src/config/tests.rs | 75 +++++ .../codegen/xai-grok-shell/src/leader/mod.rs | 36 ++ .../xai-grok-shell/src/session/acp_session.rs | 4 + .../session/acp_session_impl/hook_dispatch.rs | 1 + .../acp_session_impl/notification_drain.rs | 4 +- .../src/session/acp_session_impl/run_loop.rs | 2 +- .../src/session/acp_session_impl/spawn.rs | 16 +- .../src/session/acp_session_impl/stop_gate.rs | 1 + .../session/acp_session_impl/tool_calls.rs | 161 +++++++-- .../src/session/acp_session_impl/turn.rs | 9 +- .../auto_wake_suppression_tests.rs | 2 + .../plan_exit_batch_barrier_tests.rs | 312 ++++++++++++++++++ .../src/session/agent_rebuild.rs | 17 +- .../xai-grok-shell/src/session/handle.rs | 12 + .../src/session/slash_commands.rs | 79 ++++- .../xai-grok-shell/src/session/storage/mod.rs | 36 ++ .../src/session/storage/search.rs | 2 +- .../src/session/testkit/synth/mod.rs | 5 +- .../src/session/testkit/synth/replay.rs | 6 + .../xai-grok-shell/src/session/wire_tags.rs | 1 + .../xai-grok-shell/src/terminal/adapter.rs | 2 + .../src/test_support/lsp_runtime.rs | 1 + .../src/tools/notification_bridge.rs | 2 + .../xai-grok-shell/src/tools/tool_context.rs | 2 +- .../src/util/config/resolve/features.rs | 2 + .../xai-grok-shell/src/util/config/tips.rs | 4 +- .../tests/test_leader_sandbox_confinement.rs | 39 +++ .../tests/test_session_load_memory.rs | 72 ++-- .../xai-grok-tools-api/src/slash_commands.rs | 143 ++++++-- crates/codegen/xai-grok-tools/Cargo.toml | 4 + crates/codegen/xai-grok-tools/src/bridge.rs | 1 + .../src/computer/local/terminal.rs | 2 + .../xai-grok-tools/src/computer/types.rs | 8 + .../src/implementations/grok_build/mod.rs | 3 +- .../grok_build/monitor/types.rs | 47 +++ .../grok_build/scheduler/create.rs | 4 +- .../grok_build/task/coordinator_state.rs | 1 - .../implementations/grok_build/task/mod.rs | 62 +++- .../implementations/grok_build/task/types.rs | 59 +--- .../grok_build/task_output/mod.rs | 1 + .../grok_build/workflow/mod.rs | 4 +- .../src/implementations/task_output/tool.rs | 1 + .../src/reminders/task_completion.rs | 25 +- .../tests/test_subagent_soak.rs | 194 +++++++++-- .../src/rpc/workspace.rs | 8 +- crates/codegen/xai-grok-workspace/src/hub.rs | 1 + .../xai-grok-workspace/src/hub_server.rs | 148 ++++++++- .../xai-computer-hub-sdk/src/connection.rs | 53 +++ .../xai-computer-hub-sdk/src/metrics.rs | 19 ++ 96 files changed, 2347 insertions(+), 352 deletions(-) create mode 100644 crates/codegen/xai-grok-shell/src/session/acp_session_tests/plan_exit_batch_barrier_tests.rs create mode 100644 crates/codegen/xai-grok-shell/tests/test_leader_sandbox_confinement.rs rename crates/codegen/{xai-grok-shell => xai-grok-tools}/tests/test_subagent_soak.rs (80%) diff --git a/Cargo.lock b/Cargo.lock index 4d7f4f2..5c3a134 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13796,6 +13796,7 @@ dependencies = [ "obfstr", "parking_lot", "portable-pty", + "pretty_assertions", "process-wrap", "prod-mc-cli-chat-proxy-types", "prost", @@ -14046,6 +14047,7 @@ dependencies = [ "command-fds", "crc32fast", "derive_more 2.1.1", + "dhat", "dirs 5.0.1", "dunce", "educe", @@ -14064,6 +14066,7 @@ dependencies = [ "nix 0.30.1", "parking_lot", "pdf_oxide", + "pretty_assertions", "pulldown-cmark", "quick-xml 0.38.3", "regex", @@ -14098,6 +14101,7 @@ dependencies = [ "xai-grok-config", "xai-grok-env", "xai-grok-sandbox", + "xai-grok-test-support", "xai-grok-tools-api", "xai-grok-version", "xai-grok-workspace-types", diff --git a/SOURCE_REV b/SOURCE_REV index c2214f4..cdc7e55 100644 --- a/SOURCE_REV +++ b/SOURCE_REV @@ -1 +1 @@ -91d8cf309110a3b879c1b8198f7525aed545dfb4 +1adcd1f477870e4a97bacbd6be78c8a3bfbac46d diff --git a/crates/build/xai-proto-build/src/lib.rs b/crates/build/xai-proto-build/src/lib.rs index 417c4b2..66613d0 100644 --- a/crates/build/xai-proto-build/src/lib.rs +++ b/crates/build/xai-proto-build/src/lib.rs @@ -49,6 +49,10 @@ impl XaiProtoBuilder { } } + pub fn btree_map>(self, paths: impl IntoIterator) -> Self { + self.map_builder(|b| paths.into_iter().fold(b, |b, path| b.btree_map(path))) + } + pub fn bytes>(self, paths: impl IntoIterator) -> Self { self.map_builder(|b| paths.into_iter().fold(b, |b, path| b.bytes(path))) } diff --git a/crates/codegen/xai-grok-config-types/src/lib.rs b/crates/codegen/xai-grok-config-types/src/lib.rs index 63408a7..66ebbe1 100644 --- a/crates/codegen/xai-grok-config-types/src/lib.rs +++ b/crates/codegen/xai-grok-config-types/src/lib.rs @@ -1003,6 +1003,9 @@ pub struct RemoteSettings { /// further override per the resolver chain. #[serde(default)] pub auto_compact_threshold_percent: Option, + /// Max subagent nesting depth (`grok_build_settings.subagents_max_depth`). + #[serde(default)] + pub subagents_max_depth: Option, /// Global system-prompt identity label. Per-model override wins; see /// `resolve_system_prompt_label`. #[serde(default)] diff --git a/crates/codegen/xai-grok-pager-bin/src/main.rs b/crates/codegen/xai-grok-pager-bin/src/main.rs index 2053e71..e4937c9 100644 --- a/crates/codegen/xai-grok-pager-bin/src/main.rs +++ b/crates/codegen/xai-grok-pager-bin/src/main.rs @@ -30,8 +30,9 @@ use std::env; use std::net::SocketAddr; use tokio_util::sync::CancellationToken; use xai_grok_pager::app::{ - AgentCmd, Command, HeadlessArgs, LeaderMgmtArgs, LeaderMgmtCommand, LeaderTargetArgs, - PagerArgs, join_early_prefetch, resolve_use_leader, + AgentCmd, Command, HeadlessArgs, LeaderMgmtArgs, LeaderMgmtCommand, LeaderMode, + LeaderTargetArgs, PagerArgs, join_early_prefetch, resolve_leader_mode, resolve_use_leader, + warn_leader_disabled_by_sandbox, }; use xai_grok_pager::app::{WorkspaceMgmtArgs, WorkspaceMgmtCommand, WorkspaceStartArgs}; use xai_grok_pager::client_identity::PAGER_CLIENT_VERSION; @@ -416,6 +417,20 @@ fn fetch_remote_settings() -> Option Result<()> { + if matches!( + &args.command, + WorkspaceMgmtCommand::Start(_) + | WorkspaceMgmtCommand::Restart(_) + | WorkspaceMgmtCommand::Resume { .. } + ) && let Some(profile) = xai_grok_sandbox::requested_confinement_profile() + { + anyhow::bail!( + "`grok workspace` start/restart/resume is unavailable under sandbox profile '{profile}': \ + those commands (re)activate shared-leader workspace exposure that this session cannot \ + prove is confined by that profile. Disable the profile at the source that selected it \ + (CLI, env, config, or a managed requirement)." + ); + } let env_override = workspace_command_env_override(); let remote_settings = if env_override.is_none() { fetch_remote_settings() @@ -526,6 +541,7 @@ async fn workspace_start( &raw_config, remote_settings.as_ref(), true, + xai_grok_sandbox::requested_confinement_profile(), ); if !use_leader { anyhow::bail!( @@ -1122,14 +1138,29 @@ async fn run_agent_command( &agent_args.mode, None | Some(AgentCmd::Stdio) | Some(AgentCmd::Headless(_)) ); - let (use_leader, policy_disable_reason) = resolve_use_leader( + let requested_confinement = xai_grok_sandbox::requested_confinement_profile(); + let LeaderMode { + use_leader, + policy_disable_reason, + disabled_by_confinement, + } = resolve_leader_mode( agent_args.leader, agent_args.no_leader, &raw_config, remote_settings.as_ref(), leader_eligible, + requested_confinement, ); - tracing::info!(use_leader, ?policy_disable_reason, "leader mode resolved"); + tracing::info!( + use_leader, + ?policy_disable_reason, + sandbox_profile = ?requested_confinement, + leader_disabled_by_sandbox = disabled_by_confinement.is_some(), + "leader mode resolved" + ); + if let Some(profile) = disabled_by_confinement { + warn_leader_disabled_by_sandbox(profile); + } let managed_install = is_managed_install( std::env::current_exe().ok(), &xai_grok_shell::util::grok_home::grok_home(), diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/15-agent-mode.md b/crates/codegen/xai-grok-pager/docs/user-guide/15-agent-mode.md index 9a81e4e..1624e69 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/15-agent-mode.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/15-agent-mode.md @@ -67,7 +67,7 @@ grok agent --always-approve serve --bind 127.0.0.1:2419 --secret | `--always-approve` | Run without interactive tool-permission prompts. Alias: `--yolo`. | | `--reauth` | Authenticate before the agent starts. | | `--agent-profile ` | Load an agent profile from a file. | -| `--leader` / `--no-leader` | Connect to a shared leader process, or force a local agent. | +| `--leader` / `--no-leader` | Connect to a shared leader process, or force a local agent. When a non-`off` sandbox profile is requested, leader mode is refused so tools stay in-process (see [Sandbox Mode](18-sandbox.md)). | --- diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/18-sandbox.md b/crates/codegen/xai-grok-pager/docs/user-guide/18-sandbox.md index c6b2e22..c5261eb 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/18-sandbox.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/18-sandbox.md @@ -151,6 +151,14 @@ The sandbox is applied to the **entire grok process** at startup using kernel pr - `bash` commands, `grep` (rg) -- child processes inherit FS restrictions automatically - Network -- on Linux, child processes can be blocked via seccomp; on macOS this is a no-op +When a non-`off` sandbox profile is **requested** (CLI, `GROK_SANDBOX`, config, or a managed requirement): + +- The agent runs **in-process**, not through the shared leader, so tool calls stay in this process when the profile is enforced. If leader mode would otherwise have been on, a one-line note at startup says so +- If a built-in profile fails to apply, Grok warns and continues without enforcement (see [Platform Support](#platform-support)), but still refuses the leader so tools are not delegated elsewhere +- `grok workspace start`, `restart`, and `resume` are unavailable; `pause`, `stop`, and `status` still work + +Disable the profile at the source that selected it to use the refused commands. + The sandbox is **irreversible** once applied. The agent cannot relax restrictions at runtime. --- diff --git a/crates/codegen/xai-grok-pager/src/app/acp_handler/settings.rs b/crates/codegen/xai-grok-pager/src/app/acp_handler/settings.rs index 7a4cd9d..6c1fa12 100644 --- a/crates/codegen/xai-grok-pager/src/app/acp_handler/settings.rs +++ b/crates/codegen/xai-grok-pager/src/app/acp_handler/settings.rs @@ -304,6 +304,13 @@ pub(super) fn handle_settings_update(notif: &acp::ExtNotification, app: &mut App } } + // `scheduler_background_loops` is deliberately absent from this handler, + // unlike the flags above. A live session's scheduled fires keep the mode + // the shell pinned when the session's actor spawned, so applying a pushed + // flip here would make `/loop` promise a runtime those fires never get. + // The per-session value arrives on the `session/new` / `session/load` + // response instead (`AgentView::scheduler_background_loops`). + // Re-resolve tips from config layers + the updated remote tips. if let Some(remote_tips) = update.tips { use xai_grok_shell::util::config::resolve_tips; diff --git a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/mod.rs b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/mod.rs index 7cb9f7f..e5eb6a1 100644 --- a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/mod.rs +++ b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/mod.rs @@ -1817,6 +1817,7 @@ pub(super) fn task_completed_notif( explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, }, will_wake, }, diff --git a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/queue_and_adoption.rs b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/queue_and_adoption.rs index 96f91c4..dfa21eb 100644 --- a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/queue_and_adoption.rs +++ b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/queue_and_adoption.rs @@ -1042,6 +1042,7 @@ restore_summary: None, restore_degree: None, running_prompt_id: Some("p-run".to_string()), + scheduler_background_loops: None, }), &mut app, ); @@ -1167,6 +1168,7 @@ restore_summary: None, restore_degree: None, running_prompt_id: Some("task-completed-abc-123".to_string()), + scheduler_background_loops: None, }), &mut app, ); @@ -1207,6 +1209,7 @@ restore_summary: None, restore_degree: None, running_prompt_id: Some(pid.to_string()), + scheduler_background_loops: None, }), &mut app, ); @@ -1244,6 +1247,7 @@ restore_summary: None, restore_degree: None, running_prompt_id: Some("p-run".to_string()), + scheduler_background_loops: None, }), &mut app, ); @@ -1277,6 +1281,7 @@ restore_summary: None, restore_degree: None, running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); @@ -2526,6 +2531,7 @@ restore_summary: None, restore_degree: None, running_prompt_id: Some("p-run".to_string()), + scheduler_background_loops: None, }), &mut app, ); diff --git a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/settings.rs b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/settings.rs index ca5bedf..a2782e3 100644 --- a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/settings.rs +++ b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/settings.rs @@ -134,6 +134,134 @@ assert!(!app.voice_mode_enabled); } + /// Build an `x.ai/settings/update` carrying only the scheduler flag. + fn scheduler_background_loops_update(value: bool) -> acp::ExtNotification { + acp::ExtNotification::new( + "x.ai/settings/update", + std::sync::Arc::from( + serde_json::value::to_raw_value(&serde_json::json!({ + "scheduler_background_loops": value + })) + .unwrap(), + ), + ) + } + + /// Drain the `/loop` instruction the pager actually stored for a session. + fn loop_instruction(app: &mut AppView, args: &str) -> String { + use crate::app::actions::Action; + + // `/loop` is `required_tools()`-gated and the registry fails closed + // until the toolset is advertised, so a bare test agent never reaches + // the command. + if let Some(agent) = app.agents.get_mut(&AgentId(0)) { + agent + .prompt + .slash_controller + .registry_mut() + .set_available_tools( + [xai_grok_tools::implementations::grok_build::SCHEDULER_CREATE_TOOL_NAME] + .into_iter() + .map(str::to_string) + .collect(), + ); + } + let effects = + crate::app::dispatch::dispatch(Action::SendPrompt(format!("/loop {args}")), app); + let blocks = effects + .iter() + .find_map(|e| match e { + Effect::SendPromptBlocks { blocks, .. } => Some(blocks), + _ => None, + }) + .expect("/loop must enqueue an instruction and drain it"); + match &blocks[0] { + acp::ContentBlock::Text(text) => text.text.clone(), + other => panic!("expected a text prompt block, got {other:?}"), + } + } + + /// `/loop`'s wording must describe THIS session's fires. The shell pins the + /// fire mode when a session's actor spawns, so a mid-session settings push + /// carrying the opposite value must not change the instruction: describing + /// detached fires as in-session drops the self-contained state those fires + /// need. + #[test] + fn loop_fire_mode_follows_session_not_later_settings_push() { + use crate::app::actions::{Action, TaskResult}; + use xai_grok_tools::implementations::grok_build::{ + LoopFireMode, loop_schedule_instruction, + }; + + let mut app = make_app_with_agent("sess-loop"); + // Seed says detached; only the session's own answer can produce the + // in-session wording asserted below. + app.scheduler_background_loops_seed = true; + crate::app::dispatch::dispatch( + Action::TaskComplete(TaskResult::SessionCreated { + agent_id: AgentId(0), + session_id: acp::SessionId::new("sess-loop"), + models: None, + scheduler_background_loops: Some(false), + }), + &mut app, + ); + + assert!(handle_ext_notification( + &scheduler_background_loops_update(true), + &mut app + )); + + assert_eq!( + loop_instruction(&mut app, "5m check ci"), + loop_schedule_instruction("5m check ci", LoopFireMode::InSession), + "a pushed flip must not re-describe fires this session already pinned" + ); + } + + /// The value is session-scoped, not frozen for the process: resuming a + /// session adopts the mode that resume's spawn pinned. + #[test] + fn loop_fire_mode_adopts_the_loaded_session_value() { + use crate::app::actions::{Action, TaskResult}; + use xai_grok_tools::implementations::grok_build::{ + LoopFireMode, loop_schedule_instruction, + }; + + let mut app = make_app_with_agent("sess-loop-load"); + // Opposite of both the seed and the pre-resume value, so only the load + // response can produce the detached wording asserted below. + app.scheduler_background_loops_seed = false; + crate::app::dispatch::dispatch( + Action::TaskComplete(TaskResult::SessionCreated { + agent_id: AgentId(0), + session_id: acp::SessionId::new("sess-loop-load"), + models: None, + scheduler_background_loops: Some(false), + }), + &mut app, + ); + crate::app::dispatch::dispatch( + Action::TaskComplete(TaskResult::SessionLoaded { + agent_id: AgentId(0), + session_id: acp::SessionId::new("sess-loop-load"), + models: None, + code_restored: false, + restore_summary: None, + restore_degree: None, + running_prompt_id: None, + scheduler_background_loops: Some(true), + }), + &mut app, + ); + + assert_eq!( + loop_instruction(&mut app, "5m check ci"), + loop_schedule_instruction("5m check ci", LoopFireMode::Detached), + "resume must adopt the value its own spawn pinned" + ); + } + #[test] fn settings_update_clearing_group_tool_verbs_reverts_to_default() { // Expected values come from the same chain the handler resolves, so the diff --git a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/turn_completion.rs b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/turn_completion.rs index f5fa74d..2bbe3ec 100644 --- a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/turn_completion.rs +++ b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/turn_completion.rs @@ -1277,6 +1277,7 @@ restore_summary: None, restore_degree: None, running_prompt_id: Some("p-run".to_string()), + scheduler_background_loops: None, }), &mut app, ); diff --git a/crates/codegen/xai-grok-pager/src/app/actions.rs b/crates/codegen/xai-grok-pager/src/app/actions.rs index 1fb61a2..292ec95 100644 --- a/crates/codegen/xai-grok-pager/src/app/actions.rs +++ b/crates/codegen/xai-grok-pager/src/app/actions.rs @@ -2163,6 +2163,12 @@ pub enum TaskResult { agent_id: AgentId, session_id: acp::SessionId, models: Option, + /// Whether this session's scheduled fires run detached, as the shell + /// resolved it at spawn (response + /// `_meta["x.ai/schedulerBackgroundLoops"]`). `None` from a shell that + /// predates the key. See + /// [`crate::app::effects::parse_session_scheduler_background_loops`]. + scheduler_background_loops: Option, }, /// Session creation failed. SessionFailed { @@ -2178,6 +2184,8 @@ pub enum TaskResult { /// Effective cwd inside the worktree (preserves subdirectory offset). session_cwd: std::path::PathBuf, models: Option, + /// See [`TaskResult::SessionCreated::scheduler_background_loops`]. + scheduler_background_loops: Option, }, /// Worktree created and session forked, but not yet loaded. /// The dispatch handler sets session_id eagerly, then emits LoadSession. @@ -2209,6 +2217,10 @@ pub enum TaskResult { /// pass the live `session/update` gate without re-rendering the user /// block (replay already rendered it). running_prompt_id: Option, + /// See [`TaskResult::SessionCreated::scheduler_background_loops`]. A + /// resumed session re-spawns its actor, so the load response carries + /// the value that spawn just pinned. + scheduler_background_loops: Option, }, /// Session load (resume) failed. SessionLoadFailed { diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/mod.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/mod.rs index 93c14f3..5f5ea13 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent_view/mod.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/mod.rs @@ -1385,6 +1385,15 @@ pub struct AgentView { /// Whether the `/share` slash command is available (mirrors /// `AppView::sharing_enabled`). Used to gate palette entries. pub sharing_enabled: bool, + /// Whether THIS session's scheduled fires run as detached background + /// subagents, as resolved by the shell when the session's actor spawned and + /// delivered on the `session/new` / `session/load` response. `/loop` reads + /// it to describe the runtime a fire will get. `None` until that response + /// lands (or against a shell that predates the key), where readers fall + /// back to `AppView::scheduler_background_loops_seed`. Deliberately NOT + /// refreshed by `x.ai/settings/update`: the fire side is pinned for the + /// session's lifetime, so a live mirror would drift out of agreement. + pub scheduler_background_loops: Option, /// Mirrors `AppView::usage_visible` (credit warning + `/usage manage`). pub billing_surface_visible: bool, /// Input flight recorder — rolling buffer of recent key events. diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/session.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/session.rs index f21d3f6..011573b 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent_view/session.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/session.rs @@ -297,6 +297,7 @@ impl AgentView { is_subagent_view: false, hit_subagent_frame_close: Default::default(), sharing_enabled: false, + scheduler_background_loops: None, billing_surface_visible: false, input_log: crate::input_log::InputRingBuffer::new(), esc_pressed_at: None, diff --git a/crates/codegen/xai-grok-pager/src/app/app_view.rs b/crates/codegen/xai-grok-pager/src/app/app_view.rs index aca4db0..73576dd 100644 --- a/crates/codegen/xai-grok-pager/src/app/app_view.rs +++ b/crates/codegen/xai-grok-pager/src/app/app_view.rs @@ -755,6 +755,13 @@ pub struct AppView { /// non-selectable headers. Gated by `GROK_SESSION_PICKER_GROUPED` env var /// or remote settings `session_picker_grouped`; defaults to `false`. pub session_picker_grouped: bool, + /// Startup-only seed for `AgentView::scheduler_background_loops`, resolved + /// once from the config layers plus the remote tier known at connect. + /// Read only until a session's own value arrives on its `session/new` / + /// `session/load` response, and by the session-less dashboard. Never + /// refreshed afterwards — the authoritative value is per session, pinned by + /// the shell when that session's actor spawned. + pub scheduler_background_loops_seed: bool, /// Whether Ctrl+C before first server activity rewinds the prompt /// back into the input box. Gated by `GROK_CANCEL_REWIND` env / /// `[features] cancel_rewind` config / remote settings flag. @@ -1555,6 +1562,7 @@ impl AppView { optimistic_prompt_echoes: std::collections::HashMap::new(), pending_running_adoptions: std::collections::HashMap::new(), session_picker_grouped: false, + scheduler_background_loops_seed: true, cancel_rewind_enabled: true, session_recap_available: false, tutorial: None, @@ -5827,6 +5835,7 @@ pub(crate) mod tests { optimistic_prompt_echoes: std::collections::HashMap::new(), pending_running_adoptions: std::collections::HashMap::new(), session_picker_grouped: false, + scheduler_background_loops_seed: true, cancel_rewind_enabled: true, session_recap_available: false, tutorial: None, diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/dashboard.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/dashboard.rs index 99dce43..091cd70 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/dashboard.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/dashboard.rs @@ -1293,6 +1293,9 @@ pub(super) fn dispatch_dashboard_dispatch_slash(app: &mut AppView, text: String) let auto_mode_gate_from_app = app.auto_mode_gate; let ask_user_question_timeout_enabled_from_app = app.ask_user_question_timeout_enabled; let voice_stt_language_from_app = app.voice_config.language.clone(); + // Dashboard commands run before any session exists, so the startup seed is + // the only answer available here. + let scheduler_background_loops_seed = app.scheduler_background_loops_seed; // Build the execution context from app-wide state. The dashboard // is session-less, so `session_id` is `None`. Offered session-less @@ -1401,6 +1404,7 @@ pub(super) fn dispatch_dashboard_dispatch_slash(app: &mut AppView, text: String) auto_mode_gate: auto_mode_gate_from_app, ask_user_question_timeout_enabled: ask_user_question_timeout_enabled_from_app, voice_stt_language: voice_stt_language_from_app, + scheduler_background_loops: scheduler_background_loops_seed, }, }; command.run(&mut ctx, invocation.args) diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/prompt.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/prompt.rs index 4932364..3bd7150 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/prompt.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/prompt.rs @@ -477,6 +477,7 @@ pub(super) fn dispatch_send_prompt_inner( // shown after the agent borrow ends so we can re-enter via the tip helper. let mut tip_send_now_after_queue = false; let voice_stt_language_from_app = app.voice_config.language.clone(); + let scheduler_background_loops_seed = app.scheduler_background_loops_seed; let login_method_id_from_app = app.login_method_id.as_ref().map(|id| id.0.to_string()); let Some(agent) = app.agents.get_mut(&id) else { return vec![]; @@ -575,6 +576,11 @@ pub(super) fn dispatch_send_prompt_inner( auto_mode_gate: auto_mode_gate_from_app, ask_user_question_timeout_enabled: ask_user_question_timeout_enabled_from_app, voice_stt_language: voice_stt_language_from_app, + // This session's own value (what its fires will actually + // do), seed only until the session response lands. + scheduler_background_loops: agent + .scheduler_background_loops + .unwrap_or(scheduler_background_loops_seed), }, }; diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/session/lifecycle.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/session/lifecycle.rs index 599900a..ae9fadf 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/session/lifecycle.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/session/lifecycle.rs @@ -801,6 +801,7 @@ pub(in crate::app::dispatch) fn handle_session_created( agent_id: AgentId, session_id: acp::SessionId, new_models: Option, + scheduler_background_loops: Option, ) -> Vec { let agent_count = app.agents.len(); let switch_hint = @@ -822,6 +823,7 @@ pub(in crate::app::dispatch) fn handle_session_created( ))); } agent.bind_session_id(session_id); + agent.scheduler_background_loops = scheduler_background_loops; if let Some(m) = new_models { app.models = Some(m).into(); agent.session.models = app.models.clone(); @@ -910,12 +912,14 @@ pub(in crate::app::dispatch) fn handle_worktree_session_created( worktree_path: std::path::PathBuf, session_cwd: std::path::PathBuf, new_models: Option, + scheduler_background_loops: Option, ) -> Vec { if let Some(agent) = app.agents.get_mut(&agent_id) { agent.session.finish_command(); agent.mark_turn_finished(); let session_id_clone = session_id.clone(); agent.bind_session_id(session_id); + agent.scheduler_background_loops = scheduler_background_loops; agent.session.cwd = session_cwd.clone(); agent.session.is_worktree = true; if let Some(m) = new_models { diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/session/load.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/session/load.rs index 45de1da..5939ba7 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/session/load.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/session/load.rs @@ -879,6 +879,7 @@ pub(in crate::app::dispatch) fn handle_session_loaded( restore_summary: Option, restore_degree: Option, running_prompt_id: Option, + scheduler_background_loops: Option, ) -> Vec { tracing::info!( "Session loaded for agent {:?} session {:?}", @@ -891,6 +892,7 @@ pub(in crate::app::dispatch) fn handle_session_loaded( } let hydrate_sid = session_id.clone(); agent.bind_session_id(session_id); + agent.scheduler_background_loops = scheduler_background_loops; agent.scrollback.end_batch(); agent.session.loading_replay = false; agent.session.restore_degree = restore_degree; diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/settings/ui.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/settings/ui.rs index 4e5a49f..79126fb 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/settings/ui.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/settings/ui.rs @@ -54,6 +54,7 @@ pub(crate) fn refresh_open_settings_modals(app: &mut AppView) { let auto_mode_gate_from_app = app.auto_mode_gate; let ask_user_question_timeout_enabled_from_app = app.ask_user_question_timeout_enabled; let voice_stt_language_from_app = app.voice_config.language.clone(); + let scheduler_background_loops_seed = app.scheduler_background_loops_seed; for agent in app.agents.values_mut() { // Walk both `Settings` and `ResetSettingsConfirm` — the // confirm dialog embeds settings state that must stay fresh @@ -92,6 +93,9 @@ pub(crate) fn refresh_open_settings_modals(app: &mut AppView) { auto_mode_gate: auto_mode_gate_from_app, ask_user_question_timeout_enabled: ask_user_question_timeout_enabled_from_app, voice_stt_language: voice_stt_language_from_app.clone(), + scheduler_background_loops: agent + .scheduler_background_loops + .unwrap_or(scheduler_background_loops_seed), }; } } @@ -191,6 +195,7 @@ pub(in crate::app::dispatch) fn dispatch_open_settings( let auto_mode_gate_from_app = app.auto_mode_gate; let ask_user_question_timeout_enabled_from_app = app.ask_user_question_timeout_enabled; let voice_stt_language_from_app = app.voice_config.language.clone(); + let scheduler_background_loops_seed = app.scheduler_background_loops_seed; let Some(agent) = app.agents.get_mut(&id) else { return effects; @@ -238,6 +243,9 @@ pub(in crate::app::dispatch) fn dispatch_open_settings( auto_mode_gate: auto_mode_gate_from_app, ask_user_question_timeout_enabled: ask_user_question_timeout_enabled_from_app, voice_stt_language: voice_stt_language_from_app, + scheduler_background_loops: agent + .scheduler_background_loops + .unwrap_or(scheduler_background_loops_seed), }; let mut state = Box::new(SettingsModalState::new( registry, @@ -650,6 +658,20 @@ fn agent_auto_mode(app: &AppView) -> bool { false } +/// Effective `scheduler_background_loops` for the active agent: the value the +/// shell pinned for that session, falling back to the startup seed while the +/// session response is still in flight (or with no agent at all). See +/// [`agent_multiline_mode`] for the no-agent fallback rationale. +fn agent_scheduler_background_loops(app: &AppView) -> bool { + if let ActiveView::Agent(id) = app.active_view + && let Some(agent) = app.agents.get(&id) + && let Some(value) = agent.scheduler_background_loops + { + return value; + } + app.scheduler_background_loops_seed +} + /// Effective `plan_mode` for the active agent /// (`pending.unwrap_or(active)`). fn agent_plan_mode(app: &AppView) -> bool { @@ -716,6 +738,7 @@ pub(crate) fn build_pager_snapshot(app: &AppView) -> crate::settings::PagerLocal auto_mode_gate: app.auto_mode_gate, ask_user_question_timeout_enabled: app.ask_user_question_timeout_enabled, voice_stt_language: app.voice_config.language.clone(), + scheduler_background_loops: agent_scheduler_background_loops(app), } } diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/task_result.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/task_result.rs index 151aeac..8de391e 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/task_result.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/task_result.rs @@ -244,7 +244,14 @@ pub(super) fn dispatch_task_result(result: TaskResult, app: &mut AppView) -> Vec agent_id, session_id, models: new_models, - } => handle_session_created(app, agent_id, session_id, new_models), + scheduler_background_loops, + } => handle_session_created( + app, + agent_id, + session_id, + new_models, + scheduler_background_loops, + ), TaskResult::SessionFailed { agent_id, error } => { handle_session_failed(app, agent_id, error) } @@ -254,6 +261,7 @@ pub(super) fn dispatch_task_result(result: TaskResult, app: &mut AppView) -> Vec worktree_path, session_cwd, models: new_models, + scheduler_background_loops, } => handle_worktree_session_created( app, agent_id, @@ -261,6 +269,7 @@ pub(super) fn dispatch_task_result(result: TaskResult, app: &mut AppView) -> Vec worktree_path, session_cwd, new_models, + scheduler_background_loops, ), TaskResult::WorktreeForked { agent_id, @@ -326,6 +335,7 @@ pub(super) fn dispatch_task_result(result: TaskResult, app: &mut AppView) -> Vec restore_summary, restore_degree, running_prompt_id, + scheduler_background_loops, } => handle_session_loaded( app, agent_id, @@ -335,6 +345,7 @@ pub(super) fn dispatch_task_result(result: TaskResult, app: &mut AppView) -> Vec restore_summary, restore_degree, running_prompt_id, + scheduler_background_loops, ), TaskResult::SessionTitleFromDisk { agent_id, title } => { if let Some(agent) = app.agents.get_mut(&agent_id) diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/dashboard.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/dashboard.rs index 7c4871d..3c72794 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/dashboard.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/dashboard.rs @@ -2071,6 +2071,7 @@ fn dashboard_deferred_plan_mode_applied_on_session_created() { agent_id: id, session_id: session_id.clone(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -5234,6 +5235,7 @@ fn dashboard_attach_roster_focuses_existing_local_agent() { agent_id: id, session_id: "local-owned".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/mod.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/mod.rs index 5131813..736039b 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/mod.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/mod.rs @@ -260,6 +260,7 @@ fn test_app() -> AppView { optimistic_prompt_echoes: std::collections::HashMap::new(), pending_running_adoptions: std::collections::HashMap::new(), session_picker_grouped: false, + scheduler_background_loops_seed: true, cancel_rewind_enabled: true, session_recap_available: false, tutorial: None, diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/fork.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/fork.rs index 94ebe65..1fc6c9e 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/fork.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/fork.rs @@ -229,6 +229,7 @@ fn fork_initiation_supersedes_open_reload_window() { restore_summary: None, restore_degree: None, running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/lifecycle.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/lifecycle.rs index f32bd40..f6b8139 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/lifecycle.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/lifecycle.rs @@ -132,6 +132,7 @@ fn session_created_sets_session_id() { agent_id: id, session_id: "new-session-123".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -175,6 +176,7 @@ fn session_created_omits_cta_catalog_when_disabled() { agent_id: id, session_id: "new-session-123".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -219,6 +221,7 @@ fn session_created_banner_advertises_resume_in_minimal_mode() { agent_id: id, session_id: "new-session-123".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -300,6 +303,7 @@ fn worktree_session_created_sets_session_and_cwd() { worktree_path: worktree_path.clone(), session_cwd: session_cwd.clone(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -358,6 +362,7 @@ fn worktree_session_preserves_subdirectory_offset() { worktree_path: worktree_root.clone(), session_cwd: session_cwd.clone(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -486,6 +491,7 @@ fn worktree_session_created_drains_queued_prompts() { worktree_path, session_cwd: PathBuf::from("/tmp/grok-worktrees/pager-abc"), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -519,6 +525,7 @@ fn session_created_drains_queued_prompts() { agent_id: id, session_id: acp::SessionId::new("sess-drain-1"), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -555,6 +562,7 @@ fn session_created_with_flag_emits_five_fetches_and_clears_flag() { agent_id: id, session_id: acp::SessionId::new("s"), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -577,6 +585,7 @@ fn session_created_without_flag_emits_no_extension_fetches() { agent_id: id, session_id: acp::SessionId::new("s"), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -940,6 +949,7 @@ fn deferred_model_switch_applied_on_session_created() { agent_id: id, session_id: session_id.clone(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -980,6 +990,7 @@ fn deferred_model_switch_applied_on_worktree_session_created() { worktree_path: PathBuf::from("/tmp/worktree"), session_cwd: PathBuf::from("/tmp/worktree"), models: None, + scheduler_background_loops: None, }), &mut app, ); diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/load.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/load.rs index 2d95f60..1a2131d 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/load.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/load.rs @@ -64,6 +64,7 @@ fn session_loaded_with_restore_shows_summary_in_scrollback() { ), restore_degree: Some(xai_grok_workspace::session::git::RestoreDegree::Full), running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); @@ -206,6 +207,7 @@ fn session_loaded_without_adoption_finishes_replayed_running_entries() { restore_summary: None, restore_degree: None, running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); @@ -273,6 +275,7 @@ fn session_loaded_purges_replay_transient() { restore_summary: None, restore_degree: None, running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); @@ -297,6 +300,7 @@ fn session_loaded_during_open_reload_window_defers_to_window() { restore_summary: None, restore_degree: None, running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); @@ -406,6 +410,7 @@ fn session_loaded_with_restore_failure_shows_warning_banner() { ), restore_degree: None, running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); @@ -447,6 +452,7 @@ fn session_loaded_without_restore_no_summary() { restore_summary: None, restore_degree: None, running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); @@ -489,6 +495,7 @@ fn session_loaded_without_restore_resets_restore_degree() { restore_summary: Some("checked out abc".into()), restore_degree: Some(xai_grok_workspace::session::git::RestoreDegree::Full), running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); @@ -505,6 +512,7 @@ fn session_loaded_without_restore_resets_restore_degree() { restore_summary: None, restore_degree: None, running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); @@ -533,6 +541,7 @@ fn session_loaded_with_flag_emits_five_fetches_and_clears_flag() { restore_summary: None, restore_degree: None, running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); @@ -725,6 +734,7 @@ fn session_loaded_drains_pending_first_prompt_to_front() { restore_summary: None, restore_degree: None, running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); @@ -754,6 +764,7 @@ fn session_loaded_with_no_pending_first_prompt_does_not_enqueue() { restore_summary: None, restore_degree: None, running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); @@ -859,6 +870,7 @@ fn session_loaded_clears_stale_running_entries() { restore_summary: None, restore_degree: None, running_prompt_id: None, + scheduler_background_loops: None, }), &mut app, ); @@ -898,6 +910,7 @@ fn resume_focuses_existing_agent_for_open_session() { agent_id: agent_0, session_id: "wt-sess-1".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -915,6 +928,7 @@ fn resume_focuses_existing_agent_for_open_session() { agent_id: agent_1, session_id: "new-sess-2".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -945,6 +959,7 @@ fn resume_unknown_session_still_creates_new_agent() { agent_id: AgentId(0), session_id: "sess-aaa".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -975,6 +990,7 @@ fn resume_open_session_does_not_rearm_stale_overlay() { agent_id: agent_0, session_id: "sess-a".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -985,6 +1001,7 @@ fn resume_open_session_does_not_rearm_stale_overlay() { agent_id: agent_1, session_id: "sess-b".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -1010,6 +1027,7 @@ fn resume_conversation_does_not_focus_build_id_collision() { agent_id: agent_0, session_id: "shared-id".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -1041,6 +1059,7 @@ fn duplicate_load_unbind_invalidates_old_minimal_btw_response() { agent_id: old_owner, session_id: "shared-id".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -1084,6 +1103,7 @@ fn resume_under_chat_mode_focuses_despite_entry_false() { agent_id: agent_0, session_id: "chat-mode-sess".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -1095,6 +1115,7 @@ fn resume_under_chat_mode_focuses_despite_entry_false() { agent_id: agent_1, session_id: "other".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -1120,6 +1141,7 @@ fn resume_stale_attached_target_focuses_dashboard_row() { agent_id: agent_0, session_id: "sess-a".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -1130,6 +1152,7 @@ fn resume_stale_attached_target_focuses_dashboard_row() { agent_id: agent_1, session_id: "sess-b".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); @@ -1203,6 +1226,7 @@ fn session_restored_clears_stale_session_id() { agent_id: AgentId(0), session_id: "remote-sess".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/modal.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/modal.rs index aed9242..1519230 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/modal.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/session/modal.rs @@ -69,6 +69,7 @@ fn session_created_with_flag_but_modal_closed_clears_flag_no_fetches() { agent_id: id, session_id: acp::SessionId::new("s"), models: None, + scheduler_background_loops: None, }), &mut app, ); diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/task_result.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/task_result.rs index 4300587..5c0c756 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/task_result.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/task_result.rs @@ -1489,6 +1489,7 @@ fn no_deferred_switch_means_no_extra_effect() { agent_id: id, session_id: "new-session".into(), models: None, + scheduler_background_loops: None, }), &mut app, ); diff --git a/crates/codegen/xai-grok-pager/src/app/effects/helpers.rs b/crates/codegen/xai-grok-pager/src/app/effects/helpers.rs index fbc7ba2..363af63 100644 --- a/crates/codegen/xai-grok-pager/src/app/effects/helpers.rs +++ b/crates/codegen/xai-grok-pager/src/app/effects/helpers.rs @@ -168,6 +168,25 @@ pub(crate) fn parse_session_load_running_prompt_id( .and_then(|v| v.as_str()) .map(String::from) } +/// CANONICAL wire parser for the `session/new` / `session/load` response +/// `_meta[SCHEDULER_BACKGROUND_LOOPS_META_KEY]`. +/// +/// Carries whether THIS session's scheduled fires run as detached background +/// subagents, as the shell resolved it when the session's actor spawned. The +/// pager stores it per session and must not re-resolve the setting: a +/// mid-session flip would then make `/loop`'s wording describe a runtime the +/// already-spawned session will never use. `None` when the shell predates the +/// key (or for gateway chat sessions, which have no local fires), leaving the +/// reader on the startup seed. +pub(crate) fn parse_session_scheduler_background_loops( + resp_meta: Option<&acp::Meta>, +) -> Option { + resp_meta + .and_then(|m| { + m.get(xai_grok_shell::session::SCHEDULER_BACKGROUND_LOOPS_META_KEY) + }) + .and_then(|v| v.as_bool()) +} /// Whether `raw` is (or wraps) a disk-full / ENOSPC failure. fn is_disk_full_error(raw: &str) -> bool { raw.contains(xai_fast_worktree::OUT_OF_DISK_CONTEXT) diff --git a/crates/codegen/xai-grok-pager/src/app/effects/mod.rs b/crates/codegen/xai-grok-pager/src/app/effects/mod.rs index 33ce532..5770147 100644 --- a/crates/codegen/xai-grok-pager/src/app/effects/mod.rs +++ b/crates/codegen/xai-grok-pager/src/app/effects/mod.rs @@ -11,7 +11,9 @@ use super::session_title_resolve::worktree_resume_failure_message; #[allow(unused_imports)] use super::{agent, dispatch}; pub use helpers::ConversationsPartial; -pub(super) use helpers::parse_session_load_running_prompt_id; +pub(super) use helpers::{ + parse_session_load_running_prompt_id, parse_session_scheduler_background_loops, +}; pub(crate) use helpers::{ EffectMeta, RestoreProgressMsg, SessionFlags, persist_permission_mode_and_notify, persist_setting, sanitize_user_error, @@ -199,6 +201,9 @@ pub(crate) fn execute( agent_id, session_id: resp.session_id, models: resp.models, + scheduler_background_loops: parse_session_scheduler_background_loops( + resp.meta.as_ref(), + ), } } Err(e) => { @@ -493,6 +498,9 @@ pub(crate) fn execute( worktree_path: worktree_root, session_cwd, models: resp.models, + scheduler_background_loops: parse_session_scheduler_background_loops( + resp.meta.as_ref(), + ), } } Err(e) => { @@ -574,6 +582,9 @@ pub(crate) fn execute( restore_summary, restore_degree, running_prompt_id, + scheduler_background_loops: parse_session_scheduler_background_loops( + resp.meta.as_ref(), + ), } } Err(e) => { diff --git a/crates/codegen/xai-grok-pager/src/app/event_loop.rs b/crates/codegen/xai-grok-pager/src/app/event_loop.rs index f9e858a..4b5c7c3 100644 --- a/crates/codegen/xai-grok-pager/src/app/event_loop.rs +++ b/crates/codegen/xai-grok-pager/src/app/event_loop.rs @@ -94,6 +94,11 @@ struct AgentLoadOutcome { /// client is driving mid-reconnect, adopted at finalize (mirrors the /// `SessionLoaded` adoption in `dispatch.rs`). running_prompt_id: Option, + /// `x.ai/schedulerBackgroundLoops` from the reload response. A reconnect + /// re-spawns the session actor, which re-pins the fire mode, so the + /// pre-reconnect value can be stale — adopt the reloaded one or `/loop` + /// describes a runtime the new actor will not use. + scheduler_background_loops: Option, } /// Fields of the reconnect `session/load`, derived from the agent being @@ -165,10 +170,11 @@ fn plan_reconnect_load( fn reconnect_restore_outcome( init_ok: bool, pending_agent_ids: &[super::agent::AgentId], - loads: &std::collections::HashMap)>, + loads: &std::collections::HashMap, Option)>, active_agent_id: Option, ) -> (bool, bool) { - let load_ok = |id: &super::agent::AgentId| -> bool { loads.get(id).is_some_and(|(ok, _)| *ok) }; + let load_ok = + |id: &super::agent::AgentId| -> bool { loads.get(id).is_some_and(|(ok, ..)| *ok) }; let all_restored = init_ok && pending_agent_ids.iter().all(load_ok); let active_restored = init_ok && active_agent_id.is_some_and(|aid| pending_agent_ids.contains(&aid) && load_ok(&aid)); @@ -1123,6 +1129,18 @@ pub(crate) async fn run( .value, ); + // Pre-arrival seed only. The authoritative per-session value rides the + // `session/new` / `session/load` response, but `/loop` can be reached from + // the session-less dashboard and from a session whose response has not + // landed yet; both need an answer now, and this is the same resolver the + // shell runs at spawn, so the seed agrees with the flag as it stands today. + app.scheduler_background_loops_seed = + xai_grok_shell::util::config::resolve_scheduler_background_loops( + remote_settings + .as_ref() + .and_then(|s| s.scheduler_background_loops), + ); + app.usage_billing_redirect_url = remote_settings .as_ref() .and_then(|s| s.usage_billing_redirect_url.clone()); @@ -2571,6 +2589,10 @@ pub(crate) async fn run( effects::parse_session_load_running_prompt_id( resp.meta.as_ref(), ), + scheduler_background_loops: + effects::parse_session_scheduler_background_loops( + resp.meta.as_ref(), + ), }); } Err(e) => { @@ -2581,6 +2603,7 @@ pub(crate) async fn run( agent_id, success: false, running_prompt_id: None, + scheduler_background_loops: None, }); } } @@ -2656,7 +2679,12 @@ pub(crate) async fn run( let mut loads: std::collections::HashMap<_, _> = outcome .loads .into_iter() - .map(|l| (l.agent_id, (l.success, l.running_prompt_id))) + .map(|l| { + ( + l.agent_id, + (l.success, l.running_prompt_id, l.scheduler_background_loops), + ) + }) .collect(); // Resolved BEFORE the finalize loop drains `loads` via `remove` // (see `reconnect_restore_outcome`). @@ -2672,8 +2700,14 @@ pub(crate) async fn run( ); restore_dashboard_peek_before_reload(&mut app.dashboard, &mut app.agents); for id in &pending.agent_ids { - let (ok, running_prompt_id) = loads.remove(id).unwrap_or((false, None)); + let (ok, running_prompt_id, scheduler_background_loops) = + loads.remove(id).unwrap_or((false, None, None)); if let Some(agent) = app.agents.get_mut(id) { + // The reloaded actor re-pinned the fire mode; a failed + // load leaves the previous value rather than guessing. + if let Some(mode) = scheduler_background_loops { + agent.scheduler_background_loops = Some(mode); + } agent.finalize_reload_and_maybe_adopt( pending.generation, ok, @@ -3908,8 +3942,8 @@ mod tests { let active = AgentId(0); let background = AgentId(1); let mut loads = std::collections::HashMap::new(); - loads.insert(active, (true, None)); - loads.insert(background, (false, None)); + loads.insert(active, (true, None, None)); + loads.insert(background, (false, None, None)); let pending = vec![active, background]; let (all_restored, active_restored) = @@ -3932,8 +3966,8 @@ mod tests { let active = AgentId(0); let background = AgentId(1); let mut loads = std::collections::HashMap::new(); - loads.insert(active, (false, None)); - loads.insert(background, (true, None)); + loads.insert(active, (false, None, None)); + loads.insert(background, (true, None, None)); let pending = vec![active, background]; let (all_restored, active_restored) = @@ -3952,7 +3986,7 @@ mod tests { use super::super::agent::AgentId; let active = AgentId(0); let mut loads = std::collections::HashMap::new(); - loads.insert(active, (true, None)); + loads.insert(active, (true, None, None)); let pending = vec![active]; let (all_restored, active_restored) = @@ -3982,7 +4016,7 @@ mod tests { use super::super::agent::AgentId; let background = AgentId(1); let mut loads = std::collections::HashMap::new(); - loads.insert(background, (true, None)); + loads.insert(background, (true, None, None)); let pending = vec![background]; let (all_restored, active_restored) = diff --git a/crates/codegen/xai-grok-pager/src/app/mod.rs b/crates/codegen/xai-grok-pager/src/app/mod.rs index 59c88f1..9a5b75a 100644 --- a/crates/codegen/xai-grok-pager/src/app/mod.rs +++ b/crates/codegen/xai-grok-pager/src/app/mod.rs @@ -389,37 +389,117 @@ pub(crate) struct ExitSummary { /// `None` when the newest prompt is still unanswered. pub last_response: Option, } -/// Resolve leader mode → `(use_leader, policy_disable_reason)`. +/// Resolve leader mode, reporting both why it is off and what turned it off. +/// +/// Precedence (highest first): `--no-leader` → `--leader` → eligibility → local +/// config `use_leader` → remote `leader_mode` (release-dist) → default off. +/// `requested_confinement` then vetoes leader use when `Some` (in-process tools +/// stay under the OS sandbox) without reclaiming a shared leader on its own. /// /// `policy_disable_reason` is `Some("config"|"remote")` only when leader mode is /// *definitively* off by policy (local `use_leader = false`, or remote /// `leader_mode` fetched as `false`). Unknown remote state (`None` / prefetch /// timeout), the default, `--no-leader`, and ineligibility are `None` — never /// reclaim a leader on an unknown signal. -pub fn resolve_use_leader( +pub fn resolve_leader_mode<'p>( leader_flag: bool, no_leader_flag: bool, raw_config: &toml::Value, _remote_settings: Option<&xai_grok_shell::util::config::RemoteSettings>, eligible: bool, + requested_confinement: Option<&'p str>, +) -> LeaderMode<'p> { + let (use_leader, policy_disable_reason) = 'policy: { + if no_leader_flag { + break 'policy (false, None); + } + if leader_flag { + break 'policy (true, None); + } + if !eligible { + break 'policy (false, None); + } + if let Some(v) = config::use_leader_from_toml_opt(raw_config) { + break 'policy (v, (!v).then_some("config")); + } + #[cfg(feature = "release-dist")] + if let Some(remote_val) = _remote_settings.and_then(|s| s.leader_mode) { + break 'policy (remote_val, (!remote_val).then_some("remote")); + } + (false, None) + }; + if let Some(profile) = requested_confinement { + return LeaderMode { + use_leader: false, + policy_disable_reason, + disabled_by_confinement: use_leader.then_some(profile), + }; + } + LeaderMode { + use_leader, + policy_disable_reason, + disabled_by_confinement: None, + } +} +/// Leader mode as resolved, plus the sandbox profile that overrode it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct LeaderMode<'p> { + pub use_leader: bool, + /// `Some` only when leader mode is *definitively* off by policy, which is + /// what licenses reclaiming a leftover leader. + pub policy_disable_reason: Option<&'static str>, + /// The profile that turned leader mode off, set only when leader mode was + /// otherwise on — the case worth telling the user about. + pub disabled_by_confinement: Option<&'p str>, +} +/// The leader-mode decision alone, for callers with nothing to report. +/// +/// See [`resolve_leader_mode`] for the precedence chain and the +/// `policy_disable_reason` contract. +pub fn resolve_use_leader( + leader_flag: bool, + no_leader_flag: bool, + raw_config: &toml::Value, + remote_settings: Option<&xai_grok_shell::util::config::RemoteSettings>, + eligible: bool, + requested_confinement: Option<&str>, ) -> (bool, Option<&'static str>) { - if no_leader_flag { - return (false, None); - } - if leader_flag { - return (true, None); - } - if !eligible { - return (false, None); - } - if let Some(v) = config::use_leader_from_toml_opt(raw_config) { - return (v, (!v).then_some("config")); - } - #[cfg(feature = "release-dist")] - if let Some(remote_val) = _remote_settings.and_then(|s| s.leader_mode) { - return (remote_val, (!remote_val).then_some("remote")); - } - (false, None) + let resolved = resolve_leader_mode( + leader_flag, + no_leader_flag, + raw_config, + remote_settings, + eligible, + requested_confinement, + ); + (resolved.use_leader, resolved.policy_disable_reason) +} +/// How long the sandbox note stays uncovered before a fullscreen TUI opens over +/// it. Paid only when the note was printed and the screen is about to hide it. +const SANDBOX_NOTICE_LINGER: std::time::Duration = std::time::Duration::from_millis(1_200); +/// Tell the user at startup that the sandbox turned leader mode off. +/// +/// Writes to the dup'd terminal stderr, which survives the TUI's fd-2 redirect +/// (`redirect_native_stderr`). A fullscreen TUI still paints over it, leaving +/// the line to be read on exit; `leader_disabled_by_sandbox` on the +/// leader-mode decision log is the durable record. +pub fn warn_leader_disabled_by_sandbox(profile: &str) { + xai_grok_shell::util::with_locked_stderr(|stderr| { + print_leader_disabled_by_sandbox(profile, stderr) + }); +} +/// Says only that the profile was *requested*: enforcement can still fail +/// (`apply_sandbox` warns and continues) while the leader is refused either way. +/// +/// Write errors are dropped — `eprintln!` would panic on a closed stderr. +fn print_leader_disabled_by_sandbox(profile: &str, w: &mut impl Write) { + let _ = writeln!( + w, + "note: sandbox profile '{profile}' was requested, so leader mode is off for this \ + session and tool calls stay in this process instead of the shared leader. \ + Disable the profile at the source that selected it (CLI, env, config, or a \ + managed requirement) to use the leader." + ); } /// Join early prefetch to get remote settings (with timeout). /// @@ -527,19 +607,32 @@ pub async fn run( let raw_config = xai_grok_shell::config::load_effective_config() .map_err(|e| anyhow::anyhow!("Failed to load config: {e}"))?; let prefetch_elapsed = startup_start.elapsed(); - let (use_leader, policy_disable_reason) = resolve_use_leader( + let requested_confinement = xai_grok_sandbox::requested_confinement_profile(); + let LeaderMode { + use_leader, + policy_disable_reason, + disabled_by_confinement, + } = resolve_leader_mode( args.leader, args.no_leader, &raw_config, remote_settings.as_ref(), true, + requested_confinement, ); tracing::info!( use_leader, ?policy_disable_reason, + sandbox_profile = ?requested_confinement, + // The other fields cannot distinguish this from leader mode being off + // already while a sandbox is on. + leader_disabled_by_sandbox = disabled_by_confinement.is_some(), prefetch_ms = prefetch_elapsed.as_millis() as u64, "pager TUI leader mode resolved" ); + if let Some(profile) = disabled_by_confinement { + warn_leader_disabled_by_sandbox(profile); + } if session_startup::chat_mode_conflicts_with_leader(args.chat(), use_leader) { anyhow::bail!("{}", session_startup::CHAT_MODE_LEADER_CONFLICT); } @@ -713,6 +806,9 @@ pub async fn run( multiplexer = ?term_ctx.multiplexer, "resolved fullscreen policy" ); + if disabled_by_confinement.is_some() && screen_mode.is_fullscreen() { + tokio::time::sleep(SANDBOX_NOTICE_LINGER).await; + } engage_startup_theme(screen_mode); let minimal_live_rows = config_watcher.current().minimal_live_rows; let (frame_tx, writer_sync, writer_event_rx, writer_thread) = @@ -1614,50 +1710,128 @@ mod tests { #[test] fn no_leader_flag_wins_over_leader_flag_and_config() { let cfg = config_with_leader(true); - let (use_leader, reason) = resolve_use_leader(true, true, &cfg, None, true); + let (use_leader, reason) = resolve_use_leader(true, true, &cfg, None, true, None); assert!(!use_leader); assert_eq!(reason, None); } #[test] fn leader_flag_enables() { - let (use_leader, reason) = resolve_use_leader(true, false, &empty_config(), None, true); + let (use_leader, reason) = + resolve_use_leader(true, false, &empty_config(), None, true, None); assert!(use_leader); assert_eq!(reason, None); } #[test] fn not_eligible_returns_false() { let cfg = config_with_leader(true); - let (use_leader, reason) = resolve_use_leader(false, false, &cfg, None, false); + let (use_leader, reason) = resolve_use_leader(false, false, &cfg, None, false, None); assert!(!use_leader); assert_eq!(reason, None); } #[test] fn config_toml_enables() { let cfg = config_with_leader(true); - let (use_leader, reason) = resolve_use_leader(false, false, &cfg, None, true); + let (use_leader, reason) = resolve_use_leader(false, false, &cfg, None, true, None); assert!(use_leader); assert_eq!(reason, None); } #[test] fn config_toml_disables() { let cfg = config_with_leader(false); - let (use_leader, reason) = resolve_use_leader(false, false, &cfg, None, true); + let (use_leader, reason) = resolve_use_leader(false, false, &cfg, None, true, None); assert!(!use_leader); assert_eq!(reason, Some("config")); } #[test] fn default_is_false() { - let (use_leader, reason) = resolve_use_leader(false, false, &empty_config(), None, true); + let (use_leader, reason) = + resolve_use_leader(false, false, &empty_config(), None, true, None); assert!(!use_leader); assert_eq!(reason, None); } #[test] fn cli_flag_overrides_config() { let cfg = config_with_leader(false); - let (use_leader, reason) = resolve_use_leader(true, false, &cfg, None, true); + let (use_leader, reason) = resolve_use_leader(true, false, &cfg, None, true, None); assert!(use_leader); assert_eq!(reason, None); } + #[test] + fn sandbox_confinement_refuses_leader_even_with_leader_flag_and_config_on() { + let cfg = config_with_leader(true); + let (use_leader, reason) = + resolve_use_leader(true, false, &cfg, None, true, Some("strict")); + assert!(!use_leader); + assert_eq!(reason, None); + } + /// `disabled_by_confinement` for the four leader × sandbox cells, driven by + /// every input that can decide leader mode — not just `[cli] use_leader`. + #[test] + fn matrix_reports_the_profile_only_when_the_sandbox_takes_leader_mode_away() { + let on = config_with_leader(true); + let off = config_with_leader(false); + let sandbox = Some("strict"); + for (label, leader_flag, cfg) in [ + ("config on", false, &on), + ("--leader", true, &empty_config()), + ("--leader over config off", true, &off), + ] { + let resolved = resolve_leader_mode(leader_flag, false, cfg, None, true, sandbox); + assert!(!resolved.use_leader, "{label}: leader must be vetoed"); + assert_eq!( + resolved.disabled_by_confinement, + Some("strict"), + "{label}: the profile that took leader mode away must be named" + ); + } + for (label, cfg, expect_leader) in [("leader on", &on, true), ("leader off", &off, false)] { + let resolved = resolve_leader_mode(false, false, cfg, None, true, None); + assert_eq!(resolved.use_leader, expect_leader, "{label}"); + assert_eq!(resolved.disabled_by_confinement, None, "{label}"); + } + for (label, leader_flag, no_leader_flag, cfg, eligible) in [ + ("config off", false, false, &off, true), + ("--no-leader over config on", false, true, &on, true), + ("default", false, false, &empty_config(), true), + ("ineligible mode with config on", false, false, &on, false), + ] { + let resolved = + resolve_leader_mode(leader_flag, no_leader_flag, cfg, None, eligible, sandbox); + assert!(!resolved.use_leader, "{label}"); + assert_eq!( + resolved.disabled_by_confinement, None, + "{label}: the sandbox took nothing away, so it must stay silent" + ); + } + } + #[test] + fn sandbox_notice_names_the_profile_without_promising_enforcement() { + let mut out = Vec::new(); + print_leader_disabled_by_sandbox("strict", &mut out); + let msg = String::from_utf8(out).expect("utf-8"); + assert!(msg.contains("'strict'"), "must name the profile: {msg}"); + assert!( + msg.contains("was requested"), + "must describe the request, not enforcement: {msg}" + ); + assert!( + !msg.contains("is active"), + "must not claim the profile is enforced: {msg}" + ); + assert!( + msg.contains("Disable the profile at the source"), + "must say how to get leader mode back: {msg}" + ); + assert_eq!(msg.lines().count(), 1, "single line: {msg}"); + } + #[test] + fn sandbox_confinement_preserves_config_off_reclaim_reason() { + let cfg = config_with_leader(false); + let (use_leader, reason) = + resolve_use_leader(false, false, &cfg, None, true, Some("strict")); + assert!(!use_leader); + assert_eq!(reason, Some("config")); + } fn try_parse_pager(args: &[&str]) -> Result { use clap::Parser; PagerArgs::try_parse_from(args) @@ -1688,7 +1862,7 @@ mod tests { #[test] fn no_leader_flag_overrides_config_for_tui_fallback() { let cfg = config_with_leader(true); - let (use_leader, reason) = resolve_use_leader(false, true, &cfg, None, true); + let (use_leader, reason) = resolve_use_leader(false, true, &cfg, None, true, None); assert!(!use_leader); assert_eq!(reason, None); } @@ -1708,7 +1882,8 @@ mod tests { } #[test] fn remote_settings_none_falls_through_to_default() { - let (use_leader, reason) = resolve_use_leader(false, false, &empty_config(), None, true); + let (use_leader, reason) = + resolve_use_leader(false, false, &empty_config(), None, true, None); assert!(!use_leader); assert_eq!(reason, None); } @@ -1720,7 +1895,7 @@ mod tests { ..Default::default() }; let (use_leader, reason) = - resolve_use_leader(false, false, &empty_config(), Some(&rs), true); + resolve_use_leader(false, false, &empty_config(), Some(&rs), true, None); assert!(use_leader); assert_eq!(reason, None); } @@ -1732,7 +1907,7 @@ mod tests { ..Default::default() }; let (use_leader, reason) = - resolve_use_leader(false, false, &empty_config(), Some(&rs), true); + resolve_use_leader(false, false, &empty_config(), Some(&rs), true, None); assert!(!use_leader); assert_eq!(reason, Some("remote")); } @@ -1744,7 +1919,7 @@ mod tests { ..Default::default() }; let (use_leader, reason) = - resolve_use_leader(false, false, &empty_config(), Some(&rs), true); + resolve_use_leader(false, false, &empty_config(), Some(&rs), true, None); assert!(!use_leader); assert_eq!(reason, None); } @@ -1756,7 +1931,7 @@ mod tests { ..Default::default() }; let cfg = config_with_leader(false); - let (use_leader, reason) = resolve_use_leader(false, false, &cfg, Some(&rs), true); + let (use_leader, reason) = resolve_use_leader(false, false, &cfg, Some(&rs), true, None); assert!(!use_leader); assert_eq!(reason, Some("config")); } @@ -2089,6 +2264,7 @@ mod tests { print_exit_resume_hint(&bare_exit_info("sess-abc", true), 80, &mut w); print_exit_resume_hint(&full_exit_info("sess-abc"), 80, &mut w); print_relaunch_failure_hint(&"exec failed", "sess-xyz", true, &mut w); + print_leader_disabled_by_sandbox("strict", &mut w); } /// Close the *read* end so writes on the write end get EPIPE /// (SIGPIPE is SIG_IGN → BrokenPipe, not process death). @@ -2107,5 +2283,6 @@ mod tests { print_exit_resume_hint(&bare_exit_info("pipe-sid", true), 80, &mut writer); print_exit_resume_hint(&full_exit_info("pipe-sid"), 80, &mut writer); print_relaunch_failure_hint(&"exec failed", "pipe-sid", false, &mut writer); + print_leader_disabled_by_sandbox("strict", &mut writer); } } diff --git a/crates/codegen/xai-grok-pager/src/settings/registry.rs b/crates/codegen/xai-grok-pager/src/settings/registry.rs index 6db1e1a..6690724 100644 --- a/crates/codegen/xai-grok-pager/src/settings/registry.rs +++ b/crates/codegen/xai-grok-pager/src/settings/registry.rs @@ -299,6 +299,11 @@ pub struct PagerLocalSnapshot { /// language actually in effect when `[ui].voice_stt_language` is unset but /// an explicit `[voice].language` applies. pub voice_stt_language: String, + /// Mirrors `AgentView::scheduler_background_loops` — the value the shell + /// pinned for THIS session — falling back to + /// `AppView::scheduler_background_loops_seed` before the session response + /// lands. `/loop` reads it to describe where a scheduled fire runs. + pub scheduler_background_loops: bool, } impl Default for PagerLocalSnapshot { @@ -323,6 +328,8 @@ impl Default for PagerLocalSnapshot { auto_mode_gate: false, ask_user_question_timeout_enabled: None, voice_stt_language: xai_grok_voice::STT_LANGUAGE_DEFAULT.to_string(), + // Matches `resolve_scheduler_background_loops`'s default. + scheduler_background_loops: true, } } } diff --git a/crates/codegen/xai-grok-pager/src/slash/commands/loop_cmd.rs b/crates/codegen/xai-grok-pager/src/slash/commands/loop_cmd.rs index 0b69813..e641415 100644 --- a/crates/codegen/xai-grok-pager/src/slash/commands/loop_cmd.rs +++ b/crates/codegen/xai-grok-pager/src/slash/commands/loop_cmd.rs @@ -1,6 +1,6 @@ use agent_client_protocol as acp; use xai_grok_tools::implementations::grok_build::{ - SCHEDULER_CREATE_TOOL_NAME, loop_schedule_instruction, loop_usage_message, + LoopFireMode, SCHEDULER_CREATE_TOOL_NAME, loop_schedule_instruction, loop_usage_message, }; use crate::slash::command::{CommandExecCtx, CommandResult, ScheduledTaskPreview, SlashCommand}; @@ -108,12 +108,17 @@ impl SlashCommand for LoopCommand { LOOP_REQUIRED_TOOLS } - fn run(&self, _ctx: &mut CommandExecCtx, args: &str) -> CommandResult { + fn run(&self, ctx: &mut CommandExecCtx, args: &str) -> CommandResult { if args.trim().is_empty() { return CommandResult::Message(loop_usage_message().to_string()); } let (interval_token, prompt) = parse_loop_args(args); + let fire_mode = if ctx.pager_state.scheduler_background_loops { + LoopFireMode::Detached + } else { + LoopFireMode::InSession + }; // Show a concrete cadence only for an unambiguous leading token; // otherwise a neutral placeholder, since the authoritative schedule @@ -127,7 +132,7 @@ impl SlashCommand for LoopCommand { CommandResult::InjectSkill { display_text: format!("/loop {args}"), prompt_blocks: vec![acp::ContentBlock::Text(acp::TextContent::new( - loop_schedule_instruction(args), + loop_schedule_instruction(args, fire_mode), ))], display_as_skill: false, scheduled_task_preview: Some(ScheduledTaskPreview { @@ -254,6 +259,10 @@ mod tests { } fn run_loop(args: &str) -> CommandResult { + run_loop_with_background_loops(args, true) + } + + fn run_loop_with_background_loops(args: &str, background_loops: bool) -> CommandResult { let models = ModelState::default(); let bundle = BundleState::default(); let mut ctx = CommandExecCtx { @@ -262,7 +271,10 @@ mod tests { bundle_state: &bundle, screen_mode: crate::app::ScreenMode::Inline, billing_surface_visible: true, - pager_state: crate::settings::PagerLocalSnapshot::default(), + pager_state: crate::settings::PagerLocalSnapshot { + scheduler_background_loops: background_loops, + ..Default::default() + }, }; LoopCommand.run(&mut ctx, args) } @@ -353,14 +365,19 @@ mod tests { #[test] fn run_instruction_matches_shared_helper() { let args = "2h run tests"; - match run_loop(args) { - CommandResult::InjectSkill { prompt_blocks, .. } => { - let acp::ContentBlock::Text(text) = &prompt_blocks[0] else { - panic!("expected a text prompt block"); - }; - assert_eq!(text.text, loop_schedule_instruction(args)); + for (background_loops, mode) in [ + (true, LoopFireMode::Detached), + (false, LoopFireMode::InSession), + ] { + match run_loop_with_background_loops(args, background_loops) { + CommandResult::InjectSkill { prompt_blocks, .. } => { + let acp::ContentBlock::Text(text) = &prompt_blocks[0] else { + panic!("expected a text prompt block"); + }; + assert_eq!(text.text, loop_schedule_instruction(args, mode)); + } + other => panic!("expected InjectSkill, got {other:?}"), } - other => panic!("expected InjectSkill, got {other:?}"), } } diff --git a/crates/codegen/xai-grok-sandbox/src/lib.rs b/crates/codegen/xai-grok-sandbox/src/lib.rs index 7f85d93..b0ee2c2 100644 --- a/crates/codegen/xai-grok-sandbox/src/lib.rs +++ b/crates/codegen/xai-grok-sandbox/src/lib.rs @@ -104,6 +104,19 @@ pub fn set_configured_profile(name: impl Into) { pub fn configured_profile_name() -> Option<&'static str> { CONFIGURED_PROFILE.get().map(|s| s.as_str()) } +/// The non-`off` sandbox profile this process was **requested** with, if any. +/// +/// This is the configured request, not a report that enforcement succeeded — +/// `is_active()` can be false while the process is still confined (e.g. some +/// Linux bwrap paths), and a requested-but-unapplied profile already warns the +/// user. Keying on the request is the fail-closed choice. +pub fn requested_confinement_profile() -> Option<&'static str> { + configured_profile_name().filter(|name| profile_confines(name)) +} +fn profile_confines(name: &str) -> bool { + name.parse::() + .is_ok_and(|profile| profile != ProfileName::Off) +} /// Whether the sandbox was successfully applied to this process. pub fn is_active() -> bool { SANDBOX.get().is_some_and(|s| s.applied) @@ -772,6 +785,15 @@ mod tests { assert_eq!(configured_profile_name(), Some("read-only")); } #[test] + fn profile_confines_only_for_non_off_profiles() { + assert!(!super::profile_confines("off")); + assert!(!super::profile_confines("none")); + assert!(super::profile_confines("strict")); + assert!(super::profile_confines("read-only")); + assert!(super::profile_confines("readonly")); + assert!(super::profile_confines("my-custom-profile")); + } + #[test] fn known_launch_guard_is_linux_only() { assert_eq!( restrict_network_at_known_linux_launches(true, true), diff --git a/crates/codegen/xai-grok-shell/Cargo.toml b/crates/codegen/xai-grok-shell/Cargo.toml index a10700d..ab3dcc9 100644 --- a/crates/codegen/xai-grok-shell/Cargo.toml +++ b/crates/codegen/xai-grok-shell/Cargo.toml @@ -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"] } diff --git a/crates/codegen/xai-grok-shell/src/agent/config.rs b/crates/codegen/xai-grok-shell/src/agent/config.rs index ff33fda..d92cc00 100644 --- a/crates/codegen/xai-grok-shell/src/agent/config.rs +++ b/crates/codegen/xai-grok-shell/src/agent/config.rs @@ -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.]` 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 { 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#" diff --git a/crates/codegen/xai-grok-shell/src/agent/config_model_override_parse.rs b/crates/codegen/xai-grok-shell/src/agent/config_model_override_parse.rs index 926f548..126696c 100644 --- a/crates/codegen/xai-grok-shell/src/agent/config_model_override_parse.rs +++ b/crates/codegen/xai-grok-shell/src/agent/config_model_override_parse.rs @@ -69,6 +69,9 @@ pub enum WarningTarget { #[serde(skip_serializing_if = "Option::is_none")] field: Option, }, + 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" ); } } diff --git a/crates/codegen/xai-grok-shell/src/agent/mvp_agent/acp_agent.rs b/crates/codegen/xai-grok-shell/src/agent/mvp_agent/acp_agent.rs index 8f04884..b00006a 100644 --- a/crates/codegen/xai-grok-shell/src/agent/mvp_agent/acp_agent.rs +++ b/crates/codegen/xai-grok-shell/src/agent/mvp_agent/acp_agent.rs @@ -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 diff --git a/crates/codegen/xai-grok-shell/src/agent/mvp_agent/agent_ops.rs b/crates/codegen/xai-grok-shell/src/agent/mvp_agent/agent_ops.rs index 27cebba..7cbb8e6 100644 --- a/crates/codegen/xai-grok-shell/src/agent/mvp_agent/agent_ops.rs +++ b/crates/codegen/xai-grok-shell/src/agent/mvp_agent/agent_ops.rs @@ -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, +} +/// 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 = self @@ -1465,9 +1483,11 @@ impl MvpAgent { origin_client: Option, ) -> 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, session_id: &acp::SessionId, cwd: String, title: Option, 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, diff --git a/crates/codegen/xai-grok-shell/src/agent/mvp_agent/mod.rs b/crates/codegen/xai-grok-shell/src/agent/mvp_agent/mod.rs index 6754170..05d7bbc 100644 --- a/crates/codegen/xai-grok-shell/src/agent/mvp_agent/mod.rs +++ b/crates/codegen/xai-grok-shell/src/agent/mvp_agent/mod.rs @@ -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(), diff --git a/crates/codegen/xai-grok-shell/src/agent/mvp_agent/subagent_coordinator.rs b/crates/codegen/xai-grok-shell/src/agent/mvp_agent/subagent_coordinator.rs index 3762d70..6da9a1a 100644 --- a/crates/codegen/xai-grok-shell/src/agent/mvp_agent/subagent_coordinator.rs +++ b/crates/codegen/xai-grok-shell/src/agent/mvp_agent/subagent_coordinator.rs @@ -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()), diff --git a/crates/codegen/xai-grok-shell/src/agent/mvp_agent/tests.rs b/crates/codegen/xai-grok-shell/src/agent/mvp_agent/tests.rs index e4ce229..2db5e86 100644 --- a/crates/codegen/xai-grok-shell/src/agent/mvp_agent/tests.rs +++ b/crates/codegen/xai-grok-shell/src/agent/mvp_agent/tests.rs @@ -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; diff --git a/crates/codegen/xai-grok-shell/src/agent/subagent/handle_request.rs b/crates/codegen/xai-grok-shell/src/agent/subagent/handle_request.rs index 2feda57..b248800 100644 --- a/crates/codegen/xai-grok-shell/src/agent/subagent/handle_request.rs +++ b/crates/codegen/xai-grok-shell/src/agent/subagent/handle_request.rs @@ -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, diff --git a/crates/codegen/xai-grok-shell/src/agent/subagent/mod.rs b/crates/codegen/xai-grok-shell/src/agent/subagent/mod.rs index bfc0e55..43c3f72 100644 --- a/crates/codegen/xai-grok-shell/src/agent/subagent/mod.rs +++ b/crates/codegen/xai-grok-shell/src/agent/subagent/mod.rs @@ -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, 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 diff --git a/crates/codegen/xai-grok-shell/src/config/mod.rs b/crates/codegen/xai-grok-shell/src/config/mod.rs index 098cc78..2d424dd 100644 --- a/crates/codegen/xai-grok-shell/src/config/mod.rs +++ b/crates/codegen/xai-grok-shell/src/config/mod.rs @@ -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, /// 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, remote: Option) -> u32 { + if let Some(raw) = env { + match raw.trim().parse::() { + 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. /// diff --git a/crates/codegen/xai-grok-shell/src/config/tests.rs b/crates/codegen/xai-grok-shell/src/config/tests.rs index 40d015d..a9a13af 100644 --- a/crates/codegen/xai-grok-shell/src/config/tests.rs +++ b/crates/codegen/xai-grok-shell/src/config/tests.rs @@ -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()); diff --git a/crates/codegen/xai-grok-shell/src/leader/mod.rs b/crates/codegen/xai-grok-shell/src/leader/mod.rs index 57dc299..c647595 100644 --- a/crates/codegen/xai-grok-shell/src/leader/mod.rs +++ b/crates/codegen/xai-grok-shell/src/leader/mod.rs @@ -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 { + 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. diff --git a/crates/codegen/xai-grok-shell/src/session/acp_session.rs b/crates/codegen/xai-grok-shell/src/session/acp_session.rs index 9cc8300..245418f 100644 --- a/crates/codegen/xai-grok-shell/src/session/acp_session.rs +++ b/crates/codegen/xai-grok-shell/src/session/acp_session.rs @@ -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"] diff --git a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/hook_dispatch.rs b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/hook_dispatch.rs index 0f0cff9..901367d 100644 --- a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/hook_dispatch.rs +++ b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/hook_dispatch.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, }; diff --git a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/notification_drain.rs b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/notification_drain.rs index 2126798..28dfebc 100644 --- a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/notification_drain.rs +++ b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/notification_drain.rs @@ -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 { - 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() diff --git a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/run_loop.rs b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/run_loop.rs index d761eec..884aec3 100644 --- a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/run_loop.rs +++ b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/run_loop.rs @@ -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 diff --git a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/spawn.rs b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/spawn.rs index 29b1b8e..c37cb5b 100644 --- a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/spawn.rs +++ b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/spawn.rs @@ -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, @@ -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, @@ -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, diff --git a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/stop_gate.rs b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/stop_gate.rs index 435a5e9..a7fbdbf 100644 --- a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/stop_gate.rs +++ b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/stop_gate.rs @@ -388,6 +388,7 @@ mod stop_gate_snapshot_tests { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, } } diff --git a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/tool_calls.rs b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/tool_calls.rs index 806e4f0..5577069 100644 --- a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/tool_calls.rs +++ b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/tool_calls.rs @@ -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, +) -> 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, + kind_of: impl Fn(&str) -> Option, +) -> ( + Vec, + Vec, +) { + 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 = None; let mut deferred_followups: Vec = 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, + deferred_followups: &mut Vec, + final_result: &mut Option, + ) -> Result<(), acp::Error> { let mut approved: Vec = 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 = 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 { + 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) -> 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] diff --git a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/turn.rs b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/turn.rs index 6c02f44..3eba52a 100644 --- a/crates/codegen/xai-grok-shell/src/session/acp_session_impl/turn.rs +++ b/crates/codegen/xai-grok-shell/src/session/acp_session_impl/turn.rs @@ -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()), ); diff --git a/crates/codegen/xai-grok-shell/src/session/acp_session_tests/auto_wake_suppression_tests.rs b/crates/codegen/xai-grok-shell/src/session/acp_session_tests/auto_wake_suppression_tests.rs index 6a0a07b..c1b502c 100644 --- a/crates/codegen/xai-grok-shell/src/session/acp_session_tests/auto_wake_suppression_tests.rs +++ b/crates/codegen/xai-grok-shell/src/session/acp_session_tests/auto_wake_suppression_tests.rs @@ -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 diff --git a/crates/codegen/xai-grok-shell/src/session/acp_session_tests/plan_exit_batch_barrier_tests.rs b/crates/codegen/xai-grok-shell/src/session/acp_session_tests/plan_exit_batch_barrier_tests.rs new file mode 100644 index 0000000..254f0e5 --- /dev/null +++ b/crates/codegen/xai-grok-shell/src/session/acp_session_tests/plan_exit_batch_barrier_tests.rs @@ -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::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, + 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::(); + let (persistence_tx, _persistence_rx) = + tokio::sync::mpsc::unbounded_channel::(); + 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::(), + ToolConfig::for_tool::(), + ]) + .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, +) -> ( + tokio::task::JoinHandle<()>, + std::sync::Arc>>, +) { + let captured = std::sync::Arc::new(std::sync::Mutex::new(None::)); + 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::(); + let (persistence_tx, _persistence_rx) = + tokio::sync::mpsc::unbounded_channel::(); + 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::(), + ToolConfig::for_tool::(), + ]) + .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; +} diff --git a/crates/codegen/xai-grok-shell/src/session/agent_rebuild.rs b/crates/codegen/xai-grok-shell/src/session/agent_rebuild.rs index 706463e..29d9df9 100644 --- a/crates/codegen/xai-grok-shell/src/session/agent_rebuild.rs +++ b/crates/codegen/xai-grok-shell/src/session/agent_rebuild.rs @@ -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, pub user_question_tx: UnboundedSender, pub subagent_depth: u32, + pub subagents_max_depth: u32, pub session_id_str: String, pub blocking_wait_depth: Arc, 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>, 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 { 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, diff --git a/crates/codegen/xai-grok-shell/src/session/handle.rs b/crates/codegen/xai-grok-shell/src/session/handle.rs index aaf2583..908a932 100644 --- a/crates/codegen/xai-grok-shell/src/session/handle.rs +++ b/crates/codegen/xai-grok-shell/src/session/handle.rs @@ -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, /// YOLO (auto-approve) mode for this session. /// Per-session tracking prevents cross-client contamination in leader mode diff --git a/crates/codegen/xai-grok-shell/src/session/slash_commands.rs b/crates/codegen/xai-grok-shell/src/session/slash_commands.rs index 1ac7516..e65e049 100644 --- a/crates/codegen/xai-grok-shell/src/session/slash_commands.rs +++ b/crates/codegen/xai-grok-shell/src/session/slash_commands.rs @@ -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, 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 { +fn build_loop_prompt_blocks(args: &str, mode: LoopFireMode) -> Vec { 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 { 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, + skills: &[SkillInfo], + availability: CommandAvailability, + skill_rewrite: SkillSlashRewrite, + workflows: &[crate::session::workflow::registry::WorkflowListing], + ) -> Result, 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] diff --git a/crates/codegen/xai-grok-shell/src/session/storage/mod.rs b/crates/codegen/xai-grok-shell/src/session/storage/mod.rs index 6445399..83cf61d 100644 --- a/crates/codegen/xai-grok-shell/src/session/storage/mod.rs +++ b/crates/codegen/xai-grok-shell/src/session/storage/mod.rs @@ -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( diff --git a/crates/codegen/xai-grok-shell/src/session/storage/search.rs b/crates/codegen/xai-grok-shell/src/session/storage/search.rs index 60b2247..b8f2e98 100644 --- a/crates/codegen/xai-grok-shell/src/session/storage/search.rs +++ b/crates/codegen/xai-grok-shell/src/session/storage/search.rs @@ -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, diff --git a/crates/codegen/xai-grok-shell/src/session/testkit/synth/mod.rs b/crates/codegen/xai-grok-shell/src/session/testkit/synth/mod.rs index f945eef..6df3e7b 100644 --- a/crates/codegen/xai-grok-shell/src/session/testkit/synth/mod.rs +++ b/crates/codegen/xai-grok-shell/src/session/testkit/synth/mod.rs @@ -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, +}; diff --git a/crates/codegen/xai-grok-shell/src/session/testkit/synth/replay.rs b/crates/codegen/xai-grok-shell/src/session/testkit/synth/replay.rs index ab958db..1767832 100644 --- a/crates/codegen/xai-grok-shell/src/session/testkit/synth/replay.rs +++ b/crates/codegen/xai-grok-shell/src/session/testkit/synth/replay.rs @@ -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 { diff --git a/crates/codegen/xai-grok-shell/src/session/wire_tags.rs b/crates/codegen/xai-grok-shell/src/session/wire_tags.rs index ad605ee..6c8fd12 100644 --- a/crates/codegen/xai-grok-shell/src/session/wire_tags.rs +++ b/crates/codegen/xai-grok-shell/src/session/wire_tags.rs @@ -82,6 +82,7 @@ pub(crate) static TASK_COMPLETED: LazyLock = LazyLock::new(|| { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, }, will_wake: false, }) diff --git a/crates/codegen/xai-grok-shell/src/terminal/adapter.rs b/crates/codegen/xai-grok-shell/src/terminal/adapter.rs index 67ca57f..0549937 100644 --- a/crates/codegen/xai-grok-shell/src/terminal/adapter.rs +++ b/crates/codegen/xai-grok-shell/src/terminal/adapter.rs @@ -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, } } } diff --git a/crates/codegen/xai-grok-shell/src/test_support/lsp_runtime.rs b/crates/codegen/xai-grok-shell/src/test_support/lsp_runtime.rs index b23ecba..e0906de 100644 --- a/crates/codegen/xai-grok-shell/src/test_support/lsp_runtime.rs +++ b/crates/codegen/xai-grok-shell/src/test_support/lsp_runtime.rs @@ -104,6 +104,7 @@ pub(crate) fn ctx_with_toggle(toggle: HashMap) -> 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, diff --git a/crates/codegen/xai-grok-shell/src/tools/notification_bridge.rs b/crates/codegen/xai-grok-shell/src/tools/notification_bridge.rs index 147666c..9e9a389 100644 --- a/crates/codegen/xai-grok-shell/src/tools/notification_bridge.rs +++ b/crates/codegen/xai-grok-shell/src/tools/notification_bridge.rs @@ -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. diff --git a/crates/codegen/xai-grok-shell/src/tools/tool_context.rs b/crates/codegen/xai-grok-shell/src/tools/tool_context.rs index 241024a..5850bd6 100644 --- a/crates/codegen/xai-grok-shell/src/tools/tool_context.rs +++ b/crates/codegen/xai-grok-shell/src/tools/tool_context.rs @@ -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, + Option, pub task_completion_reservations: Option, pub task_wake_suppressed: diff --git a/crates/codegen/xai-grok-shell/src/util/config/resolve/features.rs b/crates/codegen/xai-grok-shell/src/util/config/resolve/features.rs index a2a896e..eb19b9a 100644 --- a/crates/codegen/xai-grok-shell/src/util/config/resolve/features.rs +++ b/crates/codegen/xai-grok-shell/src/util/config/resolve/features.rs @@ -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 { v.get("features")?.get("remote_fetch")?.as_bool() } diff --git a/crates/codegen/xai-grok-shell/src/util/config/tips.rs b/crates/codegen/xai-grok-shell/src/util/config/tips.rs index ce74d95..4b260b8 100644 --- a/crates/codegen/xai-grok-shell/src/util/config/tips.rs +++ b/crates/codegen/xai-grok-shell/src/util/config/tips.rs @@ -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 { 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()); diff --git a/crates/codegen/xai-grok-shell/tests/test_leader_sandbox_confinement.rs b/crates/codegen/xai-grok-shell/tests/test_leader_sandbox_confinement.rs new file mode 100644 index 0000000..0b92912 --- /dev/null +++ b/crates/codegen/xai-grok-shell/tests/test_leader_sandbox_confinement.rs @@ -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:?}" + ); +} diff --git a/crates/codegen/xai-grok-shell/tests/test_session_load_memory.rs b/crates/codegen/xai-grok-shell/tests/test_session_load_memory.rs index 163d28b..4126784 100644 --- a/crates/codegen/xai-grok-shell/tests/test_session_load_memory.rs +++ b/crates/codegen/xai-grok-shell/tests/test_session_load_memory.rs @@ -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(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()); diff --git a/crates/codegen/xai-grok-tools-api/src/slash_commands.rs b/crates/codegen/xai-grok-tools-api/src/slash_commands.rs index da5e146..b1f0d3a 100644 --- a/crates/codegen/xai-grok-tools-api/src/slash_commands.rs +++ b/crates/codegen/xai-grok-tools-api/src/slash_commands.rs @@ -15,37 +15,81 @@ pub fn loop_usage_message() -> &'static str { Tell me how often it should run (e.g. 30m, 1 hour, every 2 days)." } +/// Where a scheduled fire runs, which decides what the stored prompt can rely on. +/// +/// Resolved from `[scheduler] background_loops` (env, config, managed policy and +/// remote settings all feed it), so `/loop` describes the runtime the user +/// actually has rather than hedging across both. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum LoopFireMode { + /// Each fire runs in a detached background subagent that cannot see this + /// conversation. The default. + Detached, + /// Each fire runs as a turn in this conversation, where earlier results from + /// the same task may still be visible. + InSession, +} + /// Build the model instruction that `/loop` expands into for `args`. /// /// The model, not brittle host parsing, turns the request into the /// `scheduler_create` interval, accepting every natural phrasing and erroring /// on bad input rather than silently defaulting. See [`loop_usage_message`]. -pub fn loop_schedule_instruction(args: &str) -> String { +/// +/// Only the framing differs by `mode`; the stop condition and length guidance +/// are identical, because both hold wherever the fire runs. +pub fn loop_schedule_instruction(args: &str, mode: LoopFireMode) -> String { + let fire_context = match mode { + LoopFireMode::Detached => { + "Each fire runs in a detached background subagent, not in this conversation,\n\ + so the prompt you store must stand on its own.\n\n\ + ## Writing a prompt that survives a fresh fire\n\ + - Inline the state a fire needs: paths, job/PR/branch ids, the command that checks\n\ + status, and what \"healthy\" looks like. A fire cannot see this conversation, and\n\ + a long-running task restarts from a short summary every few iterations.\n\ + - Only a short status comes back here, so say what that status must contain." + } + LoopFireMode::InSession => { + "Each fire arrives as a new turn in this conversation, and earlier results from\n\ + the same task may still be above it. The stored prompt is re-sent verbatim every\n\ + time, so write a standing order rather than a one-off request.\n\n\ + ## Writing a prompt that reads well on every fire\n\ + - Name the state that must not be guessed: paths, job/PR/branch ids, the command\n\ + that checks status, and what \"healthy\" looks like. This conversation is\n\ + compacted as it grows, so do not rely on details staying visible.\n\ + - Earlier fires may be above you: continue from them instead of restarting." + } + }; format!( "# /loop -- schedule a recurring prompt\n\n\ - Parse the input below into an interval and a prompt, then schedule it with scheduler_create.\n\n\ + Turn the input below into a scheduler_create call. {fire_context}\n\ + - Say what one fire does and when it bails: \"if still pending, report one line and\n\ + stop.\" A fire must not poll inline.\n\ + - Give it a stop condition and an exit: \"when holds, report it and call\n\ + scheduler_delete .\" Without that the loop runs until it expires.\n\ + - Keep it short and concrete -- the stored prompt is re-sent on every fire.\n\n\ ## Deriving the interval\n\ - Read how often to run from the user's request — however they phrase it — and convert it\n\ - to a compact `` string, where unit is one of `s` (seconds), `m` (minutes),\n\ - `h` (hours), or `d` (days). The interval may appear at the start or end of the request;\n\ - extract it and use the remaining text as the prompt.\n\n\ - The minimum interval is 60 seconds; shorter values are raised to 60s, so tell the user if that applies.\n\n\ - If the request contains no interval at all, ask the user how often it should run before\n\ - scheduling. Do NOT invent or assume a default interval.\n\n\ + Convert the user's cadence -- however phrased, at either end of the request -- into a\n\ + compact `` string (`s`/`m`/`h`/`d`); the remaining text is the prompt.\n\ + The minimum is 60 seconds and shorter values are raised, so say so when it applies.\n\ + If no cadence is given, ask the user how often it should run -- never invent one.\n\n\ ## Action\n\ - 1. Call scheduler_create with: interval (the compact string you derived), prompt,\n\ - fire_immediately: true. If the interval is unparseable, the tool\n\ - returns an error — fix the interval string rather than guessing.\n\ - 2. Confirm: what's scheduled, the cadence, that it auto-expires after 7 days,\n\ - and that they can cancel with scheduler_delete (include the job ID).\n\ - 3. Do NOT execute the prompt inline. The scheduler will fire it immediately.\n\n\ + Schedule from what the user already gave you \u{2014} do not explore the workspace or run\n\ + checks before scheduling; the first fire does that.\n\ + 1. Call scheduler_create with the interval, the prompt, and fire_immediately: true.\n\ + If the interval is rejected, fix the string rather than guessing.\n\ + 2. Confirm what's scheduled, the cadence, its stop condition, that it auto-expires\n\ + after 7 days, and the task_id to cancel with scheduler_delete.\n\ + 3. Do NOT execute the prompt inline. The scheduler fires it immediately.\n\n\ + ## Wrong tool for the job\n\ + - \"Tell me when X finishes\" -> a background command or watch tool that wakes you on\n\ + the event, not a recurring loop that re-checks on a timer.\n\ + - \"Do X once in N minutes\" -> background `sleep && `; scheduling is\n\ + recurring-only.\n\n\ ## Changing an existing loop\n\ - Call scheduler_create with its task_id and the fields that change; do not\n\ + Call scheduler_create with its task_id and only the changed fields; do not\n\ delete and recreate. If later work changes what a loop should do, update its\n\ prompt the same way.\n\n\ - ## One-time delayed work\n\ - Scheduling is recurring-only. For \"do X once in N minutes\", run a background\n\ - terminal command (`sleep && `); its completion notifies you.\n\n\ ## Input\n\ {args}" ) @@ -197,23 +241,50 @@ mod tests { #[test] fn instruction_carries_args_and_contract_tokens() { - let text = loop_schedule_instruction("every 30 minutes do x"); - assert!(text.contains("every 30 minutes do x")); - assert!(text.contains("")); - assert!(text.contains("ask the user how often")); - assert!(!text.contains("10m"), "no host-side default interval"); - assert!( - !text.contains("recurring:"), - "the retired one-shot flag must not be referenced" - ); - assert!( - text.contains("task_id"), - "must teach in-place updates via task_id" - ); - assert!( - text.contains("delete and recreate"), - "must steer away from delete+recreate" - ); + for mode in [LoopFireMode::Detached, LoopFireMode::InSession] { + let text = loop_schedule_instruction("every 30 minutes do x", mode); + assert!(text.contains("every 30 minutes do x"), "{mode:?}"); + assert!(text.contains(""), "{mode:?}"); + assert!(text.contains("ask the user how often"), "{mode:?}"); + assert!( + !text.contains("10m"), + "no host-side default interval: {mode:?}" + ); + assert!( + !text.contains("recurring:"), + "the retired one-shot flag must not be referenced: {mode:?}" + ); + assert!( + text.contains("task_id"), + "must teach in-place updates via task_id: {mode:?}" + ); + assert!( + text.contains("delete and recreate"), + "must steer away from delete+recreate: {mode:?}" + ); + assert!( + text.contains("scheduler_delete "), + "every mode must authorize the fire to end the task: {mode:?}" + ); + } + } + + #[test] + fn each_fire_mode_describes_its_own_runtime() { + let detached = loop_schedule_instruction("5m check ci", LoopFireMode::Detached); + let in_session = loop_schedule_instruction("5m check ci", LoopFireMode::InSession); + + assert!(detached.contains("cannot see this conversation")); + assert!(!detached.contains("arrives as a new turn in this conversation")); + + assert!(in_session.contains("arrives as a new turn in this conversation")); + assert!(!in_session.contains("cannot see this conversation")); + + // The two levers the A/B showed carry the behavior are mode-independent. + for text in [&detached, &in_session] { + assert!(text.contains("report it and call")); + assert!(text.contains("Keep it short and concrete")); + } } #[test] diff --git a/crates/codegen/xai-grok-tools/Cargo.toml b/crates/codegen/xai-grok-tools/Cargo.toml index 27ee2bb..6581aae 100644 --- a/crates/codegen/xai-grok-tools/Cargo.toml +++ b/crates/codegen/xai-grok-tools/Cargo.toml @@ -85,6 +85,7 @@ chrono = { workspace = true } futures-util = { workspace = true } xai-grok-sandbox = { path = "../xai-grok-sandbox", default-features = false } xai-file-utils = { path = "../xai-file-utils" } +dhat = { workspace = true, optional = true } [target.'cfg(unix)'.dependencies] command-fds = { version = "0.3", features = ["tokio"] } @@ -101,9 +102,11 @@ windows = { workspace = true, features = ["Win32_Storage_FileSystem"] } [dev-dependencies] dirs = { workspace = true } +pretty_assertions = { workspace = true } tokio = { workspace = true, features = ["test-util"] } tracing-subscriber = { workspace = true, features = ["fmt"] } wiremock = { workspace = true } +xai-grok-test-support = { workspace = true } xai-test-utils = { workspace = true } [build-dependencies] @@ -118,3 +121,4 @@ workspace = true default-bazel = ["serde"] default = ["serde"] serde = [] +dhat-heap = ["dep:dhat"] diff --git a/crates/codegen/xai-grok-tools/src/bridge.rs b/crates/codegen/xai-grok-tools/src/bridge.rs index a882a29..09d9e75 100644 --- a/crates/codegen/xai-grok-tools/src/bridge.rs +++ b/crates/codegen/xai-grok-tools/src/bridge.rs @@ -835,6 +835,7 @@ mod tests { explicitly_killed: false, owner_session_id: owner.map(|s| s.to_string()), description: None, + is_backgrounded: false, } } diff --git a/crates/codegen/xai-grok-tools/src/computer/local/terminal.rs b/crates/codegen/xai-grok-tools/src/computer/local/terminal.rs index e6b950f..b456767 100644 --- a/crates/codegen/xai-grok-tools/src/computer/local/terminal.rs +++ b/crates/codegen/xai-grok-tools/src/computer/local/terminal.rs @@ -444,6 +444,7 @@ impl ProcessState { kind: self.kind, owner_session_id: self.owner_session_id.clone(), description: self.description.clone(), + is_backgrounded: self.bg_status.is_backgrounded(), } } } @@ -1617,6 +1618,7 @@ impl LocalTerminalActor { explicitly_killed: p.explicitly_killed, owner_session_id: p.owner_session_id.clone(), description: p.description.clone(), + is_backgrounded: true, }; self.completed_task_snapshots.insert(id.clone(), snapshot); } diff --git a/crates/codegen/xai-grok-tools/src/computer/types.rs b/crates/codegen/xai-grok-tools/src/computer/types.rs index 75cb955..3511320 100644 --- a/crates/codegen/xai-grok-tools/src/computer/types.rs +++ b/crates/codegen/xai-grok-tools/src/computer/types.rs @@ -219,6 +219,9 @@ pub struct TaskSnapshot { /// Model-supplied label for task UI / snapshots. #[serde(default, skip_serializing_if = "Option::is_none")] pub description: Option, + /// True after explicit/user/auto backgrounding; false for pure foreground runs. + #[serde(default)] + pub is_backgrounded: bool, } impl TaskSnapshot { @@ -238,6 +241,11 @@ impl TaskSnapshot { pub fn is_outstanding(&self) -> bool { !self.completed } + + /// Incomplete and backgrounded — tray/`tasks_snapshot` predicate (not FG in-flight). + pub fn is_outstanding_background(&self) -> bool { + !self.completed && self.is_backgrounded + } } /// Result of killing a terminal task. diff --git a/crates/codegen/xai-grok-tools/src/implementations/grok_build/mod.rs b/crates/codegen/xai-grok-tools/src/implementations/grok_build/mod.rs index 1ff57d2..b5eaed2 100644 --- a/crates/codegen/xai-grok-tools/src/implementations/grok_build/mod.rs +++ b/crates/codegen/xai-grok-tools/src/implementations/grok_build/mod.rs @@ -50,7 +50,8 @@ pub use lsp::LspTool; pub use monitor::tool::MonitorTool; pub use read_file::ReadFileTool; pub use scheduler::create::{ - SCHEDULER_CREATE_TOOL_NAME, SchedulerCreateTool, loop_schedule_instruction, loop_usage_message, + LoopFireMode, SCHEDULER_CREATE_TOOL_NAME, SchedulerCreateTool, loop_schedule_instruction, + loop_usage_message, }; pub use scheduler::delete::{SCHEDULER_DELETE_TOOL_NAME, SchedulerDeleteTool}; pub use scheduler::list::SchedulerListTool; diff --git a/crates/codegen/xai-grok-tools/src/implementations/grok_build/monitor/types.rs b/crates/codegen/xai-grok-tools/src/implementations/grok_build/monitor/types.rs index 7720fcf..8907484 100644 --- a/crates/codegen/xai-grok-tools/src/implementations/grok_build/monitor/types.rs +++ b/crates/codegen/xai-grok-tools/src/implementations/grok_build/monitor/types.rs @@ -110,6 +110,53 @@ impl MonitorInput { } } +// Mid-turn monitor event buffer + +/// A monitor event notification to be surfaced as a `` mid-turn. +#[derive(Debug, Clone)] +pub struct MonitorEventNotification { + pub task_id: String, + pub event_text: String, + /// Session that owns the monitor which produced this event. + /// + /// In leader mode every session shares one [`MonitorEventBuffer`], so the + /// drain sites filter on this to avoid surfacing one session's monitor + /// events inside another session's turn. `None` for legacy / non-grok-build + /// backends, which any session drains for backwards compatibility. + pub owner_session_id: Option, +} + +impl MonitorEventNotification { + /// Whether this buffered event should surface in the session whose owner id + /// is `my_owner`. Mirrors `task_owned_by_session`: an event surfaces only + /// when it has no recorded owner (legacy) or its owner matches the draining + /// session. Foreign events stay buffered for their own session to drain. + pub fn owned_by_session(&self, my_owner: Option<&str>) -> bool { + match (my_owner, self.owner_session_id.as_deref()) { + (Some(me), Some(owner)) => me == owner, + _ => true, + } + } +} + +/// Shared buffer for mid-turn monitor event notifications: an [`EventQueue`] +/// of [`MonitorEventNotification`]. Producers `push_capped`; the turn loop +/// drains its session's events via [`drain_owned`]. +/// +/// [`EventQueue`]: xai_interjection_core::EventQueue +pub type MonitorEventBuffer = xai_interjection_core::EventQueue; + +crate::register_resource!("grok_build", "MonitorEventBuffer", MonitorEventBuffer); + +/// Drain only `my_owner`'s events (the buffer is shared across sessions in +/// leader mode); owner-less legacy events drain anywhere. +pub fn drain_owned( + buffer: &MonitorEventBuffer, + my_owner: Option<&str>, +) -> Vec { + buffer.drain_matching(|e| e.owned_by_session(my_owner)) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/codegen/xai-grok-tools/src/implementations/grok_build/scheduler/create.rs b/crates/codegen/xai-grok-tools/src/implementations/grok_build/scheduler/create.rs index 9285532..af34be2 100644 --- a/crates/codegen/xai-grok-tools/src/implementations/grok_build/scheduler/create.rs +++ b/crates/codegen/xai-grok-tools/src/implementations/grok_build/scheduler/create.rs @@ -8,7 +8,7 @@ use super::types::{ScheduledTask, SchedulerCommand, SchedulerHandle, scheduler_t // Canonical /loop wording lives in the light API crate so other consumers can // link it without the tools implementation crate; re-exported to keep paths stable. pub use xai_grok_tools_api::slash_commands::{ - SCHEDULER_CREATE_TOOL_NAME, loop_schedule_instruction, loop_usage_message, + LoopFireMode, SCHEDULER_CREATE_TOOL_NAME, loop_schedule_instruction, loop_usage_message, }; #[derive(Debug, Clone, serde::Serialize, serde::Deserialize, schemars::JsonSchema)] @@ -475,7 +475,7 @@ mod tests { #[test] fn loop_schedule_instruction_holds_invariants() { let args = "every 30 minutes do x"; - let instr = loop_schedule_instruction(args); + let instr = loop_schedule_instruction(args, LoopFireMode::Detached); assert!( !instr.contains("10m"), "instruction must not default: {instr}" diff --git a/crates/codegen/xai-grok-tools/src/implementations/grok_build/task/coordinator_state.rs b/crates/codegen/xai-grok-tools/src/implementations/grok_build/task/coordinator_state.rs index d7a83a5..ae49b64 100644 --- a/crates/codegen/xai-grok-tools/src/implementations/grok_build/task/coordinator_state.rs +++ b/crates/codegen/xai-grok-tools/src/implementations/grok_build/task/coordinator_state.rs @@ -14,7 +14,6 @@ use super::types::{ }; /// Cap on retained completed-subagent entries before the oldest are evicted. -/// Public so the subagent soak test can assert the coordinator stays bounded. pub const MAX_COMPLETED_ENTRIES: usize = 1024; pub(super) const OUTPUT_UNAVAILABLE_PLACEHOLDER: &str = "[subagent output no longer available]"; diff --git a/crates/codegen/xai-grok-tools/src/implementations/grok_build/task/mod.rs b/crates/codegen/xai-grok-tools/src/implementations/grok_build/task/mod.rs index f152d54..b538ae8 100644 --- a/crates/codegen/xai-grok-tools/src/implementations/grok_build/task/mod.rs +++ b/crates/codegen/xai-grok-tools/src/implementations/grok_build/task/mod.rs @@ -9,6 +9,7 @@ //! //! - `SubagentBackendResource` — backend for spawn/query/cancel (required) //! - `SubagentDepthCounter` — current nesting depth (optional, defaults to 0) +//! - `MaxSubagentDepth` — max nesting (optional, defaults to [`MAX_SUBAGENT_DEPTH`]) //! - `SessionIdResource` — current session ID for parent scoping (optional) //! - `SubagentForegroundWait` — host wait-window guard factory (optional) //! - `TaskModelValidator` — validates explicit model slugs before spawn @@ -30,10 +31,16 @@ use crate::types::resources::SharedResources; use crate::types::tool::{ToolKind, ToolNamespace}; use xai_tool_types::{SubagentCompletedOutput, SubagentIsolationMode, TaskToolInput}; -/// Maximum nesting depth for subagents. A top-level session is depth 0; -/// the first subagent is depth 1. Subagents cannot spawn further subagents. +/// Default max nesting depth when [`MaxSubagentDepth`] is not injected. pub const MAX_SUBAGENT_DEPTH: u32 = 1; +pub fn effective_max_subagent_depth(resources: &crate::types::resources::Resources) -> u32 { + resources + .get::() + .map(|d| d.0) + .unwrap_or(MAX_SUBAGENT_DEPTH) +} + // ─────────────────────────────────────────────────────────────────────────── // Tool implementation // ─────────────────────────────────────────────────────────────────────────── @@ -125,10 +132,19 @@ impl xai_tool_runtime::Tool for TaskTool { .map(|cancellation| cancellation.0.clone()); // 1. Depth check - let (depth, backend, model_validator, parent_session_id, parent_prompt_id, foreground_wait) = { + let ( + depth, + max_depth, + backend, + model_validator, + parent_session_id, + parent_prompt_id, + foreground_wait, + ) = { let res = resources.lock().await; let depth = res.get::().map(|d| d.0).unwrap_or(0); + let max_depth = effective_max_subagent_depth(&res); let backend = res .get::() @@ -155,6 +171,7 @@ impl xai_tool_runtime::Tool for TaskTool { ( depth, + max_depth, backend, model_validator, parent_session_id, @@ -163,9 +180,9 @@ impl xai_tool_runtime::Tool for TaskTool { ) }; - if depth >= MAX_SUBAGENT_DEPTH { + if depth >= max_depth { return Err(xai_tool_runtime::ToolError::invalid_arguments(format!( - "Subagent depth limit exceeded (current depth: {depth}, max: {MAX_SUBAGENT_DEPTH}). \ + "Subagent depth limit exceeded (current depth: {depth}, max: {max_depth}). \ Cannot spawn further nested subagents." ))); } @@ -558,6 +575,41 @@ mod tests { assert!(err.contains("depth limit exceeded"), "error: {err}"); } + #[tokio::test] + async fn raised_max_depth_allows_nested_spawn() { + let (backend, mut rx) = make_backend(); + let mut resources = Resources::new(); + resources.insert(backend); + resources.insert(SubagentDepthCounter(1)); + resources.insert(MaxSubagentDepth(2)); + resources.insert(SessionIdResource("child-session".to_string())); + resources.insert(CurrentPromptIdResource("prompt-nested".to_string())); + + let result = xai_tool_runtime::Tool::run( + &TaskTool, + test_ctx(resources.into_shared()), + TaskToolInput { + description: "nested ok".into(), + prompt: "should be allowed at max_depth=2".into(), + subagent_type: "explore".into(), + run_in_background: true, + capability_mode: None, + isolation: None, + resume_from: None, + cwd: None, + model: None, + task_id: None, + }, + ) + .await; + + assert!( + result.is_ok(), + "expected Ok at depth 1 with max 2: {result:?}" + ); + let _ = rx.try_recv(); + } + #[tokio::test] async fn subagent_cannot_spawn_nested_subagent() { let (backend, _rx) = make_backend(); diff --git a/crates/codegen/xai-grok-tools/src/implementations/grok_build/task/types.rs b/crates/codegen/xai-grok-tools/src/implementations/grok_build/task/types.rs index eccb2b4..18d9fe1 100644 --- a/crates/codegen/xai-grok-tools/src/implementations/grok_build/task/types.rs +++ b/crates/codegen/xai-grok-tools/src/implementations/grok_build/task/types.rs @@ -10,7 +10,8 @@ //! //! - `SubagentBackendResource` — wraps an `Arc` that //! abstracts spawn/query/cancel (see [`super::backend`]) -//! - `SubagentDepthCounter` — tracks nesting depth (max 1, no recursive spawning) +//! - `SubagentDepthCounter` — current nesting depth +//! - `MaxSubagentDepth` — configured max nesting depth //! - `SessionIdResource` — carries the current session ID for parent scoping //! - `TaskModelValidator` — validates explicit model slugs before background spawn //! @@ -877,51 +878,6 @@ pub struct SubagentEventSender(#[educe(Debug(ignore))] pub mpsc::UnboundedSender register_resource!("grok_build", "SubagentEventSender", SubagentEventSender); -// Mid-turn monitor event buffer - -/// A monitor event notification to be surfaced as a `` mid-turn. -#[derive(Debug, Clone)] -pub struct MonitorEventNotification { - pub task_id: String, - pub event_text: String, - /// Session that owns the monitor which produced this event. - /// - /// In leader mode every session shares one [`MonitorEventBuffer`], so the - /// drain sites filter on this to avoid surfacing one session's monitor - /// events inside another session's turn. `None` for legacy / non-grok-build - /// backends, which any session drains for backwards compatibility. - pub owner_session_id: Option, -} - -impl MonitorEventNotification { - /// Whether this buffered event should surface in the session whose owner id - /// is `my_owner`. Mirrors `task_owned_by_session`: an event surfaces only - /// when it has no recorded owner (legacy) or its owner matches the draining - /// session. Foreign events stay buffered for their own session to drain. - pub fn owned_by_session(&self, my_owner: Option<&str>) -> bool { - match (my_owner, self.owner_session_id.as_deref()) { - (Some(me), Some(owner)) => me == owner, - _ => true, - } - } -} - -/// Shared buffer for mid-turn monitor event notifications: an [`EventQueue`] -/// of [`MonitorEventNotification`]. Producers `push_capped`; the turn loop -/// drains its session's events via [`drain_owned`]. -pub type MonitorEventBuffer = xai_interjection_core::EventQueue; - -register_resource!("grok_build", "MonitorEventBuffer", MonitorEventBuffer); - -/// Drain only `my_owner`'s events (the buffer is shared across sessions in -/// leader mode); owner-less legacy events drain anywhere. -pub fn drain_owned( - buffer: &MonitorEventBuffer, - my_owner: Option<&str>, -) -> Vec { - buffer.drain_matching(|e| e.owned_by_session(my_owner)) -} - // Active subagent listing (compaction) /// Lightweight summary of a running subagent. @@ -952,15 +908,18 @@ pub struct SubagentListActiveRequest { pub respond_to: oneshot::Sender>, } -/// Tracks nesting depth. Injected into child's Resources with depth+1. -/// -/// Top-level sessions start at depth 0. Each child increments by 1. -/// `TaskTool` rejects spawns when `depth >= MAX_SUBAGENT_DEPTH`. +/// Current nesting depth (top-level = 0; child = parent + 1). #[derive(Debug, Clone)] pub struct SubagentDepthCounter(pub u32); register_resource!("grok_build", "SubagentDepthCounter", SubagentDepthCounter); +/// Host-injected max nesting depth; absent → [`super::MAX_SUBAGENT_DEPTH`]. +#[derive(Debug, Clone, Copy)] +pub struct MaxSubagentDepth(pub u32); + +register_resource!("grok_build", "MaxSubagentDepth", MaxSubagentDepth); + /// Session-scoped validator for model-facing `Task.model` arguments. /// /// Returns an error message for an invalid slug and `None` for a valid slug. diff --git a/crates/codegen/xai-grok-tools/src/implementations/grok_build/task_output/mod.rs b/crates/codegen/xai-grok-tools/src/implementations/grok_build/task_output/mod.rs index 6f9b25a..fbc1ef1 100644 --- a/crates/codegen/xai-grok-tools/src/implementations/grok_build/task_output/mod.rs +++ b/crates/codegen/xai-grok-tools/src/implementations/grok_build/task_output/mod.rs @@ -917,6 +917,7 @@ pub(crate) mod test_helpers { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, } } diff --git a/crates/codegen/xai-grok-tools/src/implementations/grok_build/workflow/mod.rs b/crates/codegen/xai-grok-tools/src/implementations/grok_build/workflow/mod.rs index d5387cc..766c375 100644 --- a/crates/codegen/xai-grok-tools/src/implementations/grok_build/workflow/mod.rs +++ b/crates/codegen/xai-grok-tools/src/implementations/grok_build/workflow/mod.rs @@ -1,7 +1,6 @@ use crate::types::requirements::{Expr, ToolRequirement}; use crate::types::tool::{ToolKind, ToolNamespace}; -use super::task::MAX_SUBAGENT_DEPTH; use super::task::types::SubagentDepthCounter; pub use xai_grok_tools_api::slash_commands::WORKFLOW_TOOL_NAME; @@ -237,7 +236,8 @@ impl xai_tool_runtime::Tool for WorkflowTool { (depth, sender) }; - if depth >= MAX_SUBAGENT_DEPTH { + // Workflows stay top-level-only regardless of configurable subagent depth. + if depth > 0 { return Err(xai_tool_runtime::ToolError::custom( "workflow_depth_exceeded", "Workflows can only be launched from a top-level session (subagents and \ diff --git a/crates/codegen/xai-grok-tools/src/implementations/task_output/tool.rs b/crates/codegen/xai-grok-tools/src/implementations/task_output/tool.rs index 58e6668..0aa032b 100644 --- a/crates/codegen/xai-grok-tools/src/implementations/task_output/tool.rs +++ b/crates/codegen/xai-grok-tools/src/implementations/task_output/tool.rs @@ -108,6 +108,7 @@ mod tests { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, } } diff --git a/crates/codegen/xai-grok-tools/src/reminders/task_completion.rs b/crates/codegen/xai-grok-tools/src/reminders/task_completion.rs index a8b777b..53bd9f9 100644 --- a/crates/codegen/xai-grok-tools/src/reminders/task_completion.rs +++ b/crates/codegen/xai-grok-tools/src/reminders/task_completion.rs @@ -255,7 +255,7 @@ fn split_wrapped_monitor_event(event_text: &str) -> Option<(&str, &str)> { /// Buffered `event_text` arrives pre-wrapped (`wrap_monitor_event`); it is /// unwrapped via [`split_wrapped_monitor_event`] with verbatim fallback. pub fn format_monitor_events( - events: &[crate::implementations::grok_build::task::types::MonitorEventNotification], + events: &[crate::implementations::grok_build::monitor::types::MonitorEventNotification], task_output_name: Option<&str>, ) -> Option { use std::fmt::Write as _; @@ -278,7 +278,8 @@ pub fn format_monitor_events( )) } _ => { - type Event = crate::implementations::grok_build::task::types::MonitorEventNotification; + type Event = + crate::implementations::grok_build::monitor::types::MonitorEventNotification; let mut groups: Vec<(&str, Vec<&Event>)> = Vec::new(); for event in events { match groups.iter_mut().find(|(id, _)| *id == event.task_id) { @@ -802,6 +803,7 @@ mod tests { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, }; let msg = format_bash_completion(&task, Some("get_command_or_subagent_output"), None); assert!(msg.contains("abc-123")); @@ -829,6 +831,7 @@ mod tests { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, }; let msg = format_monitor_completion(&task, Some("get_command_or_subagent_output")); assert!( @@ -862,6 +865,7 @@ mod tests { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, }; let msg = format_monitor_completion(&task, None); assert!( @@ -890,6 +894,7 @@ mod tests { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, }; let msg = format_bash_completion(&task, Some("get_command_or_subagent_output"), None); assert!(msg.contains("cargo test")); @@ -915,6 +920,7 @@ mod tests { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, }; let msg = format_bash_completion(&task, Some("get_command_or_subagent_output"), None); assert!(msg.contains("exit code: unknown")); @@ -943,6 +949,7 @@ mod tests { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, }; let msg = format_bash_completion(&task, Some("get_command_or_subagent_output"), None); assert!( @@ -982,6 +989,7 @@ mod tests { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, }; let msg = format_bash_completion(&task, Some("get_command_or_subagent_output"), None); assert!( @@ -1020,6 +1028,7 @@ mod tests { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, }; let msg = format_bash_completion(&task, Some("get_command_or_subagent_output"), None); assert!(msg.contains("exit code: 0")); @@ -1181,6 +1190,7 @@ mod tests { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, } } fn make_running(id: &str) -> TaskSnapshot { @@ -1202,6 +1212,7 @@ mod tests { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, } } fn make_bg_started(id: &str) -> crate::types::output::BackgroundTaskStarted { @@ -1811,7 +1822,7 @@ mod tests { /// reintroduced. #[tokio::test] async fn reminder_pipeline_ignores_monitor_event_buffer() { - use crate::implementations::grok_build::task::types::{ + use crate::implementations::grok_build::monitor::types::{ MonitorEventBuffer, MonitorEventNotification, }; use crate::types::resources::Resources; @@ -1845,7 +1856,7 @@ mod tests { /// own + owner-less legacy events; foreign events stay buffered. #[test] fn drain_owned_partitions_by_session_owner() { - use crate::implementations::grok_build::task::types::{ + use crate::implementations::grok_build::monitor::types::{ MonitorEventBuffer, MonitorEventNotification, drain_owned, }; let shared_buffer = MonitorEventBuffer::default(); @@ -1882,7 +1893,7 @@ mod tests { /// empty => `None`. #[test] fn format_monitor_events_single_vs_batched() { - use crate::implementations::grok_build::task::types::MonitorEventNotification; + use crate::implementations::grok_build::monitor::types::MonitorEventNotification; let event = |task: &str, desc: &str, text: &str| MonitorEventNotification { task_id: task.to_string(), event_text: format!( @@ -1900,7 +1911,7 @@ mod tests { single, "\n[alpha] line 0\n", "single event must use the lean monitor-event form" ); - let bare = crate::implementations::grok_build::task::types::MonitorEventNotification { + let bare = crate::implementations::grok_build::monitor::types::MonitorEventNotification { task_id: "task-9".into(), event_text: "bare text, no wrapper".into(), owner_session_id: None, @@ -1988,7 +1999,7 @@ mod tests { /// End-to-end multibyte safety through the formatter (single + batch). #[test] fn format_monitor_events_handles_multibyte_content() { - use crate::implementations::grok_build::task::types::MonitorEventNotification; + use crate::implementations::grok_build::monitor::types::MonitorEventNotification; let event = |task: &str, desc: &str, text: &str| MonitorEventNotification { task_id: task.to_string(), event_text: format!( diff --git a/crates/codegen/xai-grok-shell/tests/test_subagent_soak.rs b/crates/codegen/xai-grok-tools/tests/test_subagent_soak.rs similarity index 80% rename from crates/codegen/xai-grok-shell/tests/test_subagent_soak.rs rename to crates/codegen/xai-grok-tools/tests/test_subagent_soak.rs index 57d5523..3f0aebf 100644 --- a/crates/codegen/xai-grok-shell/tests/test_subagent_soak.rs +++ b/crates/codegen/xai-grok-tools/tests/test_subagent_soak.rs @@ -2,7 +2,7 @@ //! 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 \ +//! SUBAGENT_SOAK_CYCLES=20000 cargo test -p xai-grok-tools \ //! [--features dhat-heap] --test test_subagent_soak -- --ignored --nocapture #![cfg(unix)] @@ -80,6 +80,14 @@ impl Metric { Metric::Threads | Metric::Fds => raw as f64, } } + + /// RSS is sampled on every unix; thread and fd counts are Linux-only. + fn expected_on_this_platform(self) -> bool { + match self { + Metric::Rss => true, + Metric::Threads | Metric::Fds => cfg!(target_os = "linux"), + } + } } /// Reads a metric's field from a snapshot or a growth delta so serialization and @@ -162,6 +170,7 @@ struct Bounds { warmup: u64, #[serde(rename = "measured_cycles")] measure: u64, + concurrency: u64, max_thread_growth: u64, max_fd_growth: u64, max_rss_growth_mib: u64, @@ -177,8 +186,10 @@ impl Bounds { // 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), + concurrency: env_parse("SUBAGENT_SOAK_CONCURRENCY", 16u64), + // RSS is looser than threads and fds to absorb allocator noise. + max_thread_growth: env_parse("SUBAGENT_SOAK_MAX_THREAD_GROWTH", 8u64), + max_fd_growth: env_parse("SUBAGENT_SOAK_MAX_FD_GROWTH", 16u64), 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), @@ -211,10 +222,15 @@ fn serialize_counts( 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)?; + let entries = [ + ("pending", pending), + ("active", active), + ("completed", completed), + ]; + let mut map = serializer.serialize_map(Some(entries.len()))?; + for (key, value) in entries { + map.serialize_entry(key, value)?; + } map.end() } @@ -276,7 +292,9 @@ impl ChildControl for SoakControl { } } -struct SoakRunner; +struct SoakRunner { + gate: Arc, +} impl ChildRunner for SoakRunner { type Control = SoakControl; @@ -286,6 +304,7 @@ impl ChildRunner for SoakRunner { type DescribeFuture = LocalBoxFuture; fn run(&self, run: ChildRunRequest) -> Self::RunFuture { + let gate = self.gate.clone(); Box::pin(async move { let ChildRunRequest { request, @@ -320,6 +339,10 @@ impl ChildRunner for SoakRunner { snapshot_ref: None, }; } + if request.id.starts_with("conc-") { + // Hold in `active` until the concurrent phase releases the gate. + let _ = gate.acquire().await; + } ChildRunOutput { result: SubagentResult { success: true, @@ -395,6 +418,42 @@ async fn run_cycle(backend: &ChannelBackend, i: u64) { ); } +async fn await_concurrency(backend: &ChannelBackend, n: u64) -> bool { + const MAX_POLLS: usize = 400; + const SLEEP: Duration = Duration::from_millis(5); + for _ in 0..MAX_POLLS { + if backend.registry_counts().await.active as u64 >= n { + return true; + } + tokio::time::sleep(SLEEP).await; + } + false +} + +async fn concurrent_phase(backend: &ChannelBackend, gate: &tokio::sync::Semaphore, n: u64) { + let handles: Vec<_> = (0..n) + .map(|k| { + let backend = backend.clone(); + tokio::task::spawn_local(async move { + backend.spawn(soak_request(format!("conc-{k}"), true)).await + }) + }) + .collect(); + + let reached = await_concurrency(backend, n).await; + + // Release then join before asserting, so no child is left blocked on failure. + gate.add_permits(n as usize); + for h in handles { + let result = h.await.expect("concurrent spawn task"); + assert!( + result.expect("concurrent spawn round-trips").success, + "concurrent child must complete" + ); + } + assert!(reached, "expected {n} concurrently active children"); +} + async fn warmup(backend: &ChannelBackend, cycles: u64) -> bool { for i in 0..cycles { run_cycle(backend, i).await; @@ -402,7 +461,11 @@ async fn warmup(backend: &ChannelBackend, cycles: u64) -> bool { quiesce(backend).await } -async fn measure(backend: &ChannelBackend, bounds: &Bounds, warmup_quiesced: bool) -> Measurement { +async fn measure( + backend: &ChannelBackend, + bounds: &Bounds, + baseline_quiesced: bool, +) -> Measurement { let heap_before = heap_capture(); let before = ResourceSnapshot::capture(); @@ -411,9 +474,9 @@ async fn measure(backend: &ChannelBackend, bounds: &Bounds, warmup_quiesced: boo 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; + // A baseline that never drained already poisons `before`, so skip the + // measured-window drain and report the window as not quiesced. + let quiesced = baseline_quiesced && quiesce(backend).await; let heap_after = heap_capture(); let after = ResourceSnapshot::capture(); @@ -431,6 +494,32 @@ async fn measure(backend: &ChannelBackend, bounds: &Bounds, warmup_quiesced: boo } } +/// Takes `expected` as a parameter so the skip arm is testable on any platform. +fn metric_failure( + metric: Metric, + value: Option, + expected: bool, + bounds: &Bounds, +) -> Option { + let Some(raw) = value else { + return expected.then(|| { + format!( + "{}: growth sample unavailable; the soak cannot bound it", + metric.label() + ) + }); + }; + let growth = metric.growth_in_budget_unit(raw); + let budget = metric.budget(bounds); + (growth > budget).then(|| { + let unit = metric.unit().map(|u| format!(" {u}")).unwrap_or_default(); + format!( + "{}: grew {growth:.1}{unit} over the soak (bound {budget:.1}{unit})", + metric.label() + ) + }) +} + fn check_bounds(bounds: &Bounds, m: &Measurement) -> Vec { // 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 @@ -463,17 +552,9 @@ fn check_bounds(bounds: &Bounds, m: &Measurement) -> Vec { } 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() - )); + let expected = metric.expected_on_this_platform(); + if let Some(f) = metric_failure(metric, m.growth.value_of(metric), expected, bounds) { + failures.push(f); } } @@ -528,13 +609,19 @@ async fn subagent_lifecycle_soak_bounds_threads_fds_and_heap() { foreground_budget: Duration::from_secs(600), ..CoordinatorConfig::default() }; + let gate = Arc::new(tokio::sync::Semaphore::new(0)); tokio::task::spawn_local( - SubagentCoordinator::new(command_rx, SoakRunner, config).run(), + SubagentCoordinator::new(command_rx, SoakRunner { gate: gate.clone() }, config) + .run(), ); let backend = ChannelBackend::new(command_tx); let warmup_quiesced = warmup(&backend, bounds.warmup).await; - let measurement = measure(&backend, &bounds, warmup_quiesced).await; + // Drain the concurrent phase into the baseline; a failed drain marks + // the window unreliable. + concurrent_phase(&backend, &gate, bounds.concurrency).await; + let baseline_quiesced = warmup_quiesced && quiesce(&backend).await; + let measurement = measure(&backend, &bounds, baseline_quiesced).await; let summary = Summary { bounds: &bounds, @@ -552,6 +639,7 @@ async fn subagent_lifecycle_soak_bounds_threads_fds_and_heap() { mod tests { use super::*; + use pretty_assertions::assert_eq; #[test] fn value_of_reads_the_matching_slot_of_snapshot_and_growth() { @@ -606,6 +694,7 @@ mod tests { let bounds = Bounds { warmup: 0, measure: 0, + concurrency: 0, max_thread_growth: 3, max_fd_growth: 5, max_rss_growth_mib: 7, @@ -638,6 +727,7 @@ mod tests { Bounds { warmup: 0, measure: 4, + concurrency: 4, max_thread_growth: 100, max_fd_growth: 100, max_rss_growth_mib: 100, @@ -646,6 +736,15 @@ mod tests { } } + /// Zero growth that reads as measured, unlike `ResourceGrowth::default()`. + fn zero_growth() -> ResourceGrowth { + ResourceGrowth { + rss: Some(0), + threads: Some(0), + fds: Some(0), + } + } + fn drained(growth: ResourceGrowth, heap: Option) -> Measurement { Measurement { before: ResourceSnapshot::default(), @@ -663,13 +762,46 @@ mod tests { #[test] fn check_bounds_passes_a_clean_drained_window() { - let m = drained(ResourceGrowth::default(), None); + let m = drained(zero_growth(), None); assert!(check_bounds(&generous_bounds(), &m).is_empty()); } + #[test] + fn check_bounds_fails_when_an_expected_metric_is_unavailable() { + let growth = ResourceGrowth { + rss: None, + threads: Some(0), + fds: Some(0), + }; + let failures = check_bounds(&generous_bounds(), &drained(growth, None)); + assert!( + failures + .iter() + .any(|f| f.starts_with("rss:") && f.contains("unavailable")), + "{failures:?}" + ); + } + + #[test] + fn metric_failure_covers_expected_missing_unexpected_missing_and_budget() { + let b = generous_bounds(); + assert!(metric_failure(Metric::Threads, None, false, &b).is_none()); + assert!( + metric_failure(Metric::Rss, None, true, &b) + .unwrap() + .contains("unavailable") + ); + assert!(metric_failure(Metric::Fds, Some(0), true, &b).is_none()); + assert!( + metric_failure(Metric::Rss, Some(500 * 1024 * 1024), true, &b) + .unwrap() + .starts_with("rss:") + ); + } + #[test] fn check_bounds_reports_non_quiesce_first_and_alone() { - let mut m = drained(ResourceGrowth::default(), None); + let mut m = drained(zero_growth(), None); m.quiesced = false; m.counts.pending = 3; let failures = check_bounds(&generous_bounds(), &m); @@ -704,7 +836,7 @@ mod tests { #[test] fn check_bounds_flags_nonzero_counts_and_heap_leak() { let mut m = drained( - ResourceGrowth::default(), + zero_growth(), Some(HeapMetrics { before: HeapSample { blocks: 0, @@ -732,7 +864,7 @@ mod tests { #[test] fn check_bounds_flags_pending_while_quiesced() { - let mut m = drained(ResourceGrowth::default(), None); + let mut m = drained(zero_growth(), None); m.counts.pending = 3; let failures = check_bounds(&generous_bounds(), &m); assert!( @@ -743,7 +875,7 @@ mod tests { #[test] fn check_bounds_flags_completed_over_cap() { - let mut m = drained(ResourceGrowth::default(), None); + let mut m = drained(zero_growth(), None); m.counts.completed = MAX_COMPLETED_ENTRIES + 1; let failures = check_bounds(&generous_bounds(), &m); assert!( @@ -773,7 +905,7 @@ mod tests { #[test] fn check_bounds_flags_block_count_leak() { let m = drained( - ResourceGrowth::default(), + zero_growth(), Some(HeapMetrics { before: HeapSample { blocks: 0, diff --git a/crates/codegen/xai-grok-workspace-types/src/rpc/workspace.rs b/crates/codegen/xai-grok-workspace-types/src/rpc/workspace.rs index 968d3ce..6d65fcf 100644 --- a/crates/codegen/xai-grok-workspace-types/src/rpc/workspace.rs +++ b/crates/codegen/xai-grok-workspace-types/src/rpc/workspace.rs @@ -211,17 +211,15 @@ pub struct ScheduledTaskSnapshotWire { pub created_at: String, } -/// Response of `workspace.tasks_snapshot` — outstanding background tasks and -/// live scheduled tasks. +/// Response of `workspace.tasks_snapshot`. #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct TasksSnapshotResponse { + /// Incomplete and backgrounded only (not in-flight foreground runs). pub background_tasks: Vec, pub scheduled_tasks: Vec, } -/// `workspace.tasks_snapshot` — point-in-time snapshot of the session's -/// outstanding background tasks and scheduled tasks, for task -/// UI rebuild on client attach/reconnect. +/// Point-in-time task UI rebuild on attach/reconnect. #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct TasksSnapshotReq { pub session_id: String, diff --git a/crates/codegen/xai-grok-workspace/src/hub.rs b/crates/codegen/xai-grok-workspace/src/hub.rs index 35c1d66..57e1371 100644 --- a/crates/codegen/xai-grok-workspace/src/hub.rs +++ b/crates/codegen/xai-grok-workspace/src/hub.rs @@ -1078,6 +1078,7 @@ mod tests { explicitly_killed: false, owner_session_id: None, description: None, + is_backgrounded: false, }) } fn started_id(n: &ToolNotification) -> &str { diff --git a/crates/codegen/xai-grok-workspace/src/hub_server.rs b/crates/codegen/xai-grok-workspace/src/hub_server.rs index 156089c..7674342 100644 --- a/crates/codegen/xai-grok-workspace/src/hub_server.rs +++ b/crates/codegen/xai-grok-workspace/src/hub_server.rs @@ -247,8 +247,7 @@ async fn list_outstanding_background_tasks( }) .collect() } -/// Point-in-time snapshot of the session's outstanding background terminal -/// tasks and live scheduled tasks. +/// Incomplete backgrounded terminal tasks + live scheduled tasks (client tray rebuild). async fn tasks_snapshot(toolset: &FinalizedToolset) -> TasksSnapshotResponse { let (terminal, scheduler) = { let res = toolset.resources.lock().await; @@ -262,7 +261,7 @@ async fn tasks_snapshot(toolset: &FinalizedToolset) -> TasksSnapshotResponse { .list_tasks() .await .into_iter() - .filter(|t| !t.completed) + .filter(|t| t.is_outstanding_background()) .map(|t| { let command = t .display_command @@ -1524,6 +1523,149 @@ mod tests { loop_task.next_fire_at ); } + /// FG in-flight out of snapshot; after backgrounding in; completed BG out. + /// Preconditions ensure a bare `!completed` filter would fail. + #[tokio::test] + async fn tasks_snapshot_excludes_foreground_and_completed_processes() { + use crate::handle::tests::terminal_run_request; + use std::time::{Duration, Instant}; + let handle = make_handle(); + let cfg = background_capable_cfg(); + let session = handle + .create_session_with_config( + "snap-fg-rpc", + None, + Some(cfg.clone()), + CapabilityMode::All, + None, + false, + ) + .expect("create background-capable session"); + session.set_bind_tool_config_fingerprint(serde_json::to_value(&cfg).ok()); + let out_dir = tempfile::tempdir().expect("temp dir"); + let handler = WorkspaceRpcHandler::new(handle.clone()); + async fn snapshot(handler: &WorkspaceRpcHandler) -> TasksSnapshotResponse { + let value = handler + .dispatch( + "workspace.tasks_snapshot", + serde_json::json!({"session_id": "snap-fg-rpc"}), + Some("snap-fg-rpc"), + ) + .await + .expect("tasks_snapshot rpc"); + serde_json::from_value(value).expect("decode response") + } + let backend = session.terminal_backend().clone(); + let fg_req = terminal_run_request("sleep 30", out_dir.path(), "snap-fg-task"); + let fg_join = tokio::spawn(async move { backend.run(fg_req).await }); + let poll_deadline = Instant::now() + Duration::from_secs(5); + loop { + let listed = session.terminal_backend().list_tasks().await; + if listed.iter().any(|t| !t.completed && !t.is_backgrounded) { + break; + } + assert!( + Instant::now() < poll_deadline, + "timeout waiting for incomplete FG in list_tasks: {listed:?}" + ); + tokio::time::sleep(Duration::from_millis(20)).await; + } + let snap = snapshot(&handler).await; + assert!( + snap.background_tasks.is_empty(), + "in-flight FG must not appear in tasks_snapshot: {:?}", + snap.background_tasks + ); + assert!( + session + .terminal_backend() + .background_foreground_command("snap-fg-task") + .await, + "expected FG process snap-fg-task to background" + ); + let snap = snapshot(&handler).await; + assert!( + snap.background_tasks + .iter() + .any(|t| t.task_id == "snap-fg-task"), + "backgrounded former FG must appear: {:?}", + snap.background_tasks + ); + assert_eq!( + snap.background_tasks.len(), + 1, + "only the transitioned FG so far: {:?}", + snap.background_tasks + ); + let bg = start_background_sleep(&session, out_dir.path(), "snap-bg-task").await; + let snap = snapshot(&handler).await; + assert_eq!( + snap.background_tasks.len(), + 2, + "transitioned FG + incomplete BG must appear: {:?}", + snap.background_tasks + ); + assert!( + snap.background_tasks + .iter() + .any(|t| t.task_id == bg.task_id), + "run_background task missing: {:?}", + snap.background_tasks + ); + let short = session + .terminal_backend() + .run_background(terminal_run_request( + "true", + out_dir.path(), + "snap-done-task", + )) + .await + .expect("start short background task"); + let done = session + .terminal_backend() + .wait_for_completion(&short.task_id, Some(Duration::from_secs(5))) + .await + .expect("short background task should complete"); + assert!(done.completed, "short task must complete: {done:?}"); + let listed = session.terminal_backend().list_tasks().await; + assert!( + listed + .iter() + .any(|t| t.task_id == short.task_id && t.completed && t.is_backgrounded), + "precondition: completed BG must still be in list_tasks: {listed:?}" + ); + let snap = snapshot(&handler).await; + assert!( + snap.background_tasks + .iter() + .all(|t| t.task_id != short.task_id), + "completed BG must not appear: {:?}", + snap.background_tasks + ); + assert_eq!( + snap.background_tasks.len(), + 2, + "still-running BG tasks remain: {:?}", + snap.background_tasks + ); + assert!( + snap.background_tasks + .iter() + .any(|t| t.task_id == bg.task_id), + "run_background task should still be present: {:?}", + snap.background_tasks + ); + assert!( + snap.background_tasks + .iter() + .any(|t| t.task_id == "snap-fg-task"), + "transitioned FG should still be present: {:?}", + snap.background_tasks + ); + session.terminal_backend().kill_task(&bg.task_id).await; + session.terminal_backend().kill_task("snap-fg-task").await; + let _ = fg_join.await; + } /// Evicting one session while another is live must NOT global-drain (which /// would close the shared queue for the survivor) — even when the evicted /// id is no longer in the session map. diff --git a/crates/common/xai-computer-hub-sdk/src/connection.rs b/crates/common/xai-computer-hub-sdk/src/connection.rs index fe6ae0d..a6d311a 100644 --- a/crates/common/xai-computer-hub-sdk/src/connection.rs +++ b/crates/common/xai-computer-hub-sdk/src/connection.rs @@ -198,6 +198,33 @@ impl DisconnectCause { _ => None, } } + /// Bounded classification of transport error detail for metrics. Collapses + /// free-form OS/tungstenite messages into a small allowlist so reconnect + /// storms can be attributed without high-cardinality labels. + fn detail_class(&self) -> Option<&'static str> { + let detail = self.detail()?; + Some(classify_transport_detail(detail)) + } +} +/// Map a transport error detail string to a bounded class label. +fn classify_transport_detail(detail: &str) -> &'static str { + let d = detail.to_ascii_lowercase(); + if d.contains("connection reset") || d.contains("econnreset") || d.contains("reset by peer") { + "connection_reset" + } else if d.contains("broken pipe") || d.contains("epipe") { + "broken_pipe" + } else if d.contains("unexpected eof") + || d.contains("connection closed") + || d.contains("connection aborted without closing") + { + "unexpected_eof" + } else if d.contains("timed out") || d.contains("timeout") || d.contains("etimedout") { + "timeout" + } else if d.contains("connection aborted") || d.contains("econnaborted") { + "connection_aborted" + } else { + "other" + } } struct OutageInfo { cause: DisconnectCause, @@ -1458,6 +1485,9 @@ async fn reconnect_and_replay( "server reconnect succeeded" ); crate::metrics::reconnect_cause(outage.cause.label()); + if let Some(detail_class) = outage.cause.detail_class() { + crate::metrics::disconnect_detail_class(outage.cause.label(), detail_class); + } crate::metrics::reconnect_gap_observe(silent_gap_ms as f64 / 1_000.0); *inner.connection_id.lock().await = Some(ack.connection_id.clone()); *inner.hello_capabilities.write() = std::mem::take(&mut ack.capabilities); @@ -1682,6 +1712,29 @@ mod tests { assert_eq!(DisconnectCause::Forced.label(), "forced"); } #[test] + fn classify_transport_detail_is_bounded() { + assert_eq!( + classify_transport_detail("Connection reset by peer (os error 104)"), + "connection_reset" + ); + assert_eq!(classify_transport_detail("Broken pipe"), "broken_pipe"); + assert_eq!( + classify_transport_detail("Unexpected EOF"), + "unexpected_eof" + ); + assert_eq!(classify_transport_detail("operation timed out"), "timeout"); + assert_eq!( + classify_transport_detail("Connection aborted"), + "connection_aborted" + ); + assert_eq!(classify_transport_detail("something novel"), "other"); + assert_eq!( + DisconnectCause::ReadError("ECONNRESET".to_owned()).detail_class(), + Some("connection_reset") + ); + assert!(DisconnectCause::Eof.detail_class().is_none()); + } + #[test] fn conn_health_snapshot_without_clock_skew_reports_zero_jump() { let health = ConnHealth::new(); health.record_inbound(); diff --git a/crates/common/xai-computer-hub-sdk/src/metrics.rs b/crates/common/xai-computer-hub-sdk/src/metrics.rs index bbf05cc..f837f70 100644 --- a/crates/common/xai-computer-hub-sdk/src/metrics.rs +++ b/crates/common/xai-computer-hub-sdk/src/metrics.rs @@ -68,6 +68,17 @@ mod inner { .expect("computer_hub_client_reconnects_by_cause_total must register once") }); + static DISCONNECT_DETAIL_CLASS_TOTAL: LazyLock = LazyLock::new(|| { + register_int_counter_vec!( + "computer_hub_client_disconnect_detail_class_total", + "Disconnects with a transport error detail, by cause (transport_read_error |\ + transport_write_error) and bounded detail_class (connection_reset | \ + broken_pipe | unexpected_eof | timeout | connection_aborted | other).", + &["cause", "detail_class"] + ) + .expect("computer_hub_client_disconnect_detail_class_total must register once") + }); + static RECONNECT_GAP_SECONDS: LazyLock = LazyLock::new(|| { register_histogram!( "computer_hub_client_reconnect_gap_seconds", @@ -340,6 +351,12 @@ mod inner { RECONNECTS_BY_CAUSE_TOTAL.with_label_values(&[cause]).inc(); } + pub(crate) fn disconnect_detail_class(cause: &str, detail_class: &str) { + DISCONNECT_DETAIL_CLASS_TOTAL + .with_label_values(&[cause, detail_class]) + .inc(); + } + pub(crate) fn reconnect_gap_observe(secs: f64) { RECONNECT_GAP_SECONDS.observe(secs); } @@ -551,6 +568,7 @@ mod inner { pub(crate) fn reconnect_failed(_reason: &str) {} pub(crate) fn reconnect_duration_observe(_secs: f64) {} pub(crate) fn reconnect_cause(_cause: &str) {} + pub(crate) fn disconnect_detail_class(_cause: &str, _detail_class: &str) {} pub(crate) fn reconnect_gap_observe(_secs: f64) {} pub(crate) fn call_dispatch_observe(_secs: f64) {} pub(crate) fn demux_inbox_depth_set(_depth: i64) {} @@ -598,6 +616,7 @@ pub(crate) use inner::cancel_hook_received; pub(crate) use inner::cancel_no_target; pub(crate) use inner::cancel_pending_tombstoned; pub(crate) use inner::demux_inbox_depth_set; +pub(crate) use inner::disconnect_detail_class; pub(crate) use inner::early_notif_buffered; pub(crate) use inner::heartbeat_pong_dropped; pub(crate) use inner::hook_send;