diff --git a/Cargo.lock b/Cargo.lock index 94f6ddf..c8eb2db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13419,7 +13419,7 @@ dependencies = [ [[package]] name = "xai-grok-pager" -version = "0.2.112" +version = "0.2.114" dependencies = [ "agent-client-protocol", "ansi-to-tui", @@ -13509,7 +13509,7 @@ dependencies = [ [[package]] name = "xai-grok-pager-bin" -version = "0.2.112" +version = "0.2.114" dependencies = [ "anyhow", "clap", @@ -13773,7 +13773,7 @@ dependencies = [ [[package]] name = "xai-grok-shell" -version = "0.2.112" +version = "0.2.114" dependencies = [ "agent-client-protocol", "anyhow", @@ -14177,7 +14177,7 @@ dependencies = [ [[package]] name = "xai-grok-version" -version = "0.2.112" +version = "0.2.114" dependencies = [ "semver", ] diff --git a/SOURCE_REV b/SOURCE_REV index 8553d0a..bef75a4 100644 --- a/SOURCE_REV +++ b/SOURCE_REV @@ -1 +1 @@ -2a818575225183d8ca915f5632a09b8067b5156a +6372e41d828b8a6ee82c29e01a69e27ec895cca9 diff --git a/crates/codegen/xai-fsnotify/src/watcher.rs b/crates/codegen/xai-fsnotify/src/watcher.rs index 7820e71..5d4e5b8 100644 --- a/crates/codegen/xai-fsnotify/src/watcher.rs +++ b/crates/codegen/xai-fsnotify/src/watcher.rs @@ -963,7 +963,7 @@ pub(crate) fn start_with_timeout( let progress_for_thread = progress.clone(); - let thread = std::thread::spawn(move || { + let watcher_loop = move || { let update_stage = |stage: &'static str| { if let Ok(mut p) = progress_for_thread.lock() { p.set_stage(stage); @@ -1342,7 +1342,11 @@ pub(crate) fn start_with_timeout( let _ = ready_tx.send(Err(Box::new(e))); } } - }); + }; + let thread = std::thread::Builder::new() + .name("fsnotify-watcher".into()) + .spawn(watcher_loop) + .map_err(|e| crate::FsNotifyError::WatcherStart(Box::new(e)))?; // Wait for watcher to be ready (with timeout) if let Ok(mut p) = progress.lock() { diff --git a/crates/codegen/xai-grok-config-types/src/lib.rs b/crates/codegen/xai-grok-config-types/src/lib.rs index 66ebbe1..9ab5ac1 100644 --- a/crates/codegen/xai-grok-config-types/src/lib.rs +++ b/crates/codegen/xai-grok-config-types/src/lib.rs @@ -36,8 +36,9 @@ pub struct CampaignOverride { #[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq, Eq)] #[serde(default)] pub struct DoomLoopRecoverySettings { - /// Send the `x-grok-doom-loop-check` header and parse the reported - /// triggers. `Some(false)` is a kill-switch; absent ⇒ client default (off). + /// Send the `x-grok-doom-loop-check` header, parse the reported + /// triggers, and resample confident loops. `Some(false)` is a + /// kill-switch; absent ⇒ client default (ON). #[serde(skip_serializing_if = "Option::is_none")] pub enabled: Option, /// Highest `tail_repetition` threshold considered confident (clamped to diff --git a/crates/codegen/xai-grok-hooks/src/dispatcher.rs b/crates/codegen/xai-grok-hooks/src/dispatcher.rs index 1d4963d..ce620a8 100644 --- a/crates/codegen/xai-grok-hooks/src/dispatcher.rs +++ b/crates/codegen/xai-grok-hooks/src/dispatcher.rs @@ -526,6 +526,7 @@ mod tests { RunContext { session_id: "test-session", workspace_root: "/tmp", + process_scope: None, } } diff --git a/crates/codegen/xai-grok-hooks/src/runner/command.rs b/crates/codegen/xai-grok-hooks/src/runner/command.rs index 1ff5ac9..492307a 100644 --- a/crates/codegen/xai-grok-hooks/src/runner/command.rs +++ b/crates/codegen/xai-grok-hooks/src/runner/command.rs @@ -1,6 +1,8 @@ +use std::sync::Arc; use std::time::{Duration, Instant}; use tokio::io::AsyncWriteExt; +use xai_grok_tools::util::ProcessGroup; use crate::config::HookSpec; use crate::event::HookEventEnvelope; @@ -18,6 +20,23 @@ const MAX_OUTPUT_BYTES: usize = 64 * 1024; /// or block (Stop/SubagentStop, with stderr as the feedback). const GATE_EXIT_CODE: i32 = 2; +/// `None` when the group cannot be built, which only costs session reaping, so +/// the hook still runs. +fn hook_process_group(child: &tokio::process::Child) -> Option> { + let mut group = ProcessGroup::new() + .inspect_err( + |e| tracing::warn!(pid = child.id(), error = %e, "hook: no process group; not reaped on session close"), + ) + .ok()?; + group + .attach(child) + .inspect_err( + |e| tracing::warn!(pid = child.id(), error = %e, "hook: process group attach failed; not reaped on session close"), + ) + .ok()?; + Some(Arc::new(group)) +} + /// Run a single hook command. /// /// Spawns the command as a child process, writes the envelope JSON on stdin, @@ -166,6 +185,21 @@ pub async fn run_command_hook( } }; + let mut hook_group = None; + if let Some(scope) = ctx.process_scope.as_ref() + && let Some(group) = hook_process_group(&child) + { + // A closed scope means the session is gone and `register` already killed + // the child, so stop rather than write stdin to a corpse. + if !scope.register(&group) { + return ( + HookRunnerResult::Failed("session closed before the hook ran".to_string()), + start.elapsed(), + ); + } + hook_group = Some(group); + } + // Write stdin concurrently with draining output, under the timeout: a hook // that never reads stdin would otherwise block `write_all` on a full pipe // buffer, outside the deadline. @@ -184,14 +218,18 @@ pub async fn run_command_hook( let elapsed = start.elapsed(); + // killpg takes grandchildren that kill_on_drop would miss. + if !matches!(result, Ok(Ok(_))) + && let Some(group) = &hook_group + { + let _ = group.kill(); + } + match result { - Err(_) => { - // Timeout: kill_on_drop handles cleanup. - ( - HookRunnerResult::Failed(format!("timed out after {}ms", spec.timeout_ms)), - elapsed, - ) - } + Err(_) => ( + HookRunnerResult::Failed(format!("timed out after {}ms", spec.timeout_ms)), + elapsed, + ), Ok(Err(e)) => ( HookRunnerResult::Failed(format!("command execution failed: {e}")), elapsed, @@ -907,6 +945,14 @@ mod tests { RunContext { session_id: "test-session", workspace_root: "/tmp", + process_scope: None, + } + } + + fn make_scoped_ctx(scope: xai_grok_tools::util::ProcessScope) -> RunContext<'static> { + RunContext { + process_scope: Some(scope), + ..make_ctx() } } @@ -1106,6 +1152,7 @@ mod tests { let ctx = RunContext { session_id: "test-session", workspace_root: &workspace, + process_scope: None, }; let (result, _) = run_command_hook(&spec, &envelope, &ctx, GateKind::Observe).await; @@ -1382,4 +1429,70 @@ mod tests { result ); } + + #[cfg(unix)] + #[tokio::test] + async fn command_hook_session_close_reaps_whole_group() { + let tmp = tempfile::tempdir().unwrap(); + let marker = tmp.path().join("grandchild_alive"); + // `& wait` keeps the leader alive while the grandchild outlives it, so + // only a group kill stops the marker being written. + let mut spec = make_shell_spec(&format!( + "sh -c 'sleep 2 && echo alive > {}' & wait", + marker.display() + )); + spec.timeout_ms = 60_000; + let envelope = make_envelope(); + let scope = xai_grok_tools::util::ProcessScope::new(); + let hook_scope = scope.clone(); + let hook = tokio::spawn(async move { + run_command_hook( + &spec, + &envelope, + &make_scoped_ctx(hook_scope), + GateKind::Observe, + ) + .await + }); + + tokio::time::sleep(Duration::from_millis(800)).await; + scope.kill_all(); + + tokio::time::timeout(Duration::from_secs(15), hook) + .await + .expect("kill_all must reap the enrolled hook, not leave it on its 60s timeout") + .expect("hook task join"); + + tokio::time::sleep(Duration::from_secs(3)).await; + assert!( + !marker.exists(), + "grandchild outlived session close, so the group was not killpg'd" + ); + } + + #[cfg(unix)] + #[tokio::test] + async fn command_hook_fails_fast_when_scope_already_closed() { + let scope = xai_grok_tools::util::ProcessScope::new(); + scope.kill_all(); + let mut spec = make_shell_spec("sleep 600"); + spec.timeout_ms = 60_000; + + let (result, _) = tokio::time::timeout( + Duration::from_secs(15), + run_command_hook( + &spec, + &make_envelope(), + &make_scoped_ctx(scope), + GateKind::Observe, + ), + ) + .await + .expect("a closed scope must fail the hook immediately, not run to its 60s timeout"); + + assert!( + matches!(result, HookRunnerResult::Failed(_)), + "got {result:?}" + ); + } } diff --git a/crates/codegen/xai-grok-hooks/src/runner/http.rs b/crates/codegen/xai-grok-hooks/src/runner/http.rs index 7fb7b8c..3612115 100644 --- a/crates/codegen/xai-grok-hooks/src/runner/http.rs +++ b/crates/codegen/xai-grok-hooks/src/runner/http.rs @@ -673,6 +673,7 @@ mod tests { let ctx = crate::runner::RunContext { session_id: "test", workspace_root: "/tmp", + process_scope: None, }; let (result, _, info) = run_http_hook(&spec, &envelope, &ctx, GateKind::Tool).await; @@ -751,6 +752,7 @@ mod tests { let ctx = crate::runner::RunContext { session_id: "test", workspace_root: "/tmp", + process_scope: None, }; let (result, _, info) = run_http_hook(&spec, &envelope, &ctx, GateKind::Tool).await; diff --git a/crates/codegen/xai-grok-hooks/src/runner/mod.rs b/crates/codegen/xai-grok-hooks/src/runner/mod.rs index 006e33c..1149d34 100644 --- a/crates/codegen/xai-grok-hooks/src/runner/mod.rs +++ b/crates/codegen/xai-grok-hooks/src/runner/mod.rs @@ -17,6 +17,7 @@ pub use crate::event::GateKind; pub struct RunContext<'a> { pub session_id: &'a str, pub workspace_root: &'a str, + pub process_scope: Option, } /// Result of running a single hook (any handler type). diff --git a/crates/codegen/xai-grok-hooks/tests/integration.rs b/crates/codegen/xai-grok-hooks/tests/integration.rs index 278ebdc..4957eba 100644 --- a/crates/codegen/xai-grok-hooks/tests/integration.rs +++ b/crates/codegen/xai-grok-hooks/tests/integration.rs @@ -72,6 +72,7 @@ async fn hook_deny_via_exit_code_only() { let ctx = RunContext { session_id: "test", workspace_root: dir.path().to_str().unwrap(), + process_scope: None, }; let pre_result = @@ -101,6 +102,7 @@ async fn hook_fail_open_on_crash() { let ctx = RunContext { session_id: "test", workspace_root: dir.path().to_str().unwrap(), + process_scope: None, }; let pre_result = @@ -134,6 +136,7 @@ async fn hook_fail_open_on_timeout() { let ctx = RunContext { session_id: "test", workspace_root: dir.path().to_str().unwrap(), + process_scope: None, }; let pre_result = @@ -162,6 +165,7 @@ async fn matcher_filters_tool_name() { let ctx = RunContext { session_id: "test", workspace_root: dir.path().to_str().unwrap(), + process_scope: None, }; let pre_result = dispatcher::dispatch_pre_tool_use( @@ -194,6 +198,7 @@ async fn non_blocking_dispatch() { let ctx = RunContext { session_id: "test", workspace_root: dir.path().to_str().unwrap(), + process_scope: None, }; let results = dispatcher::dispatch_non_blocking( @@ -232,6 +237,7 @@ async fn first_deny_stops_chain() { let ctx = RunContext { session_id: "test", workspace_root: dir.path().to_str().unwrap(), + process_scope: None, }; let pre_result = dispatcher::dispatch_pre_tool_use( @@ -264,6 +270,7 @@ async fn hook_receives_stdin_envelope() { let ctx = RunContext { session_id: "test-sess-123", workspace_root: dir.path().to_str().unwrap(), + process_scope: None, }; let pre_result = @@ -288,6 +295,7 @@ async fn shell_pipe_command_works() { let ctx = RunContext { session_id: "test", workspace_root: dir.path().to_str().unwrap(), + process_scope: None, }; let pre_result = @@ -419,6 +427,7 @@ async fn new_event_types_fire_and_receive_correct_envelope() { let ctx = RunContext { session_id: "test", workspace_root: dir.path().to_str().unwrap(), + process_scope: None, }; let results = @@ -508,6 +517,7 @@ async fn runner_injected_vars_override_extra_env_at_spawn() { let ctx = RunContext { session_id: real_session, workspace_root: real_workspace, + process_scope: None, }; let result = @@ -628,6 +638,7 @@ async fn direct_exec_command_with_env_var_resolves_at_load_time() { let ctx = RunContext { session_id: "test", workspace_root: dir.path().to_str().unwrap(), + process_scope: None, }; let result = dispatcher::dispatch_pre_tool_use(®istry, &pre_tool_use_envelope("read_file"), &ctx) @@ -697,6 +708,7 @@ async fn http_hook_url_env_expansion_end_to_end() { let ctx = RunContext { session_id: "test", workspace_root: dir.path().to_str().unwrap(), + process_scope: None, }; let pre_result = dispatcher::dispatch_pre_tool_use(®istry, &pre_tool_use_envelope("read_file"), &ctx) @@ -779,6 +791,7 @@ async fn lenient_parsing_with_mixed_claude_events() { let ctx = RunContext { session_id: "test", workspace_root: dir.path().to_str().unwrap(), + process_scope: None, }; let result = dispatcher::dispatch_pre_tool_use( ®istry, diff --git a/crates/codegen/xai-grok-pager-bin/Cargo.toml b/crates/codegen/xai-grok-pager-bin/Cargo.toml index 39f2d86..35a9d6a 100644 --- a/crates/codegen/xai-grok-pager-bin/Cargo.toml +++ b/crates/codegen/xai-grok-pager-bin/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "xai-grok-pager-bin" -version = "0.2.112" +version = "0.2.114" edition.workspace = true license = "Apache-2.0" authors = ["xAI"] diff --git a/crates/codegen/xai-grok-pager-bin/src/main.rs b/crates/codegen/xai-grok-pager-bin/src/main.rs index eba0623..f3485aa 100644 --- a/crates/codegen/xai-grok-pager-bin/src/main.rs +++ b/crates/codegen/xai-grok-pager-bin/src/main.rs @@ -1229,6 +1229,13 @@ async fn run_agent_command( let cancel = CancellationToken::new(); match mode { ClientMode::Stdio => { + if let Err(error) = xai_tty_utils::kill_current_process_on_parent_death() { + tracing::warn!( + %error, + "failed to bind to parent death; stdio bridge will not die \ + with its parent — stdin EOF remains the only cleanup" + ); + } let replay_state = Arc::new(std::sync::Mutex::new(StdioReplayState::default())); let leader_tx = Arc::new(TokioMutex::new(tx)); let leader_tx_stdin = leader_tx.clone(); diff --git a/crates/codegen/xai-grok-pager/Cargo.toml b/crates/codegen/xai-grok-pager/Cargo.toml index 8ea39a7..758af4c 100644 --- a/crates/codegen/xai-grok-pager/Cargo.toml +++ b/crates/codegen/xai-grok-pager/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "xai-grok-pager" -version = "0.2.112" +version = "0.2.114" edition.workspace = true license = "Apache-2.0" authors = ["xAI"] @@ -161,8 +161,8 @@ core-foundation = { workspace = true } windows-sys = { version = "0.59", features = ["Win32_System_Console"] } [dev-dependencies] -# Enable the render crate's test-only helpers for the pager's test build. xai-grok-pager-render = { path = "../xai-grok-pager-render", features = ["test-support"] } +xai-grok-shell = { workspace = true, features = ["test-support"] } pretty_assertions = { workspace = true } insta = { workspace = true } criterion = { workspace = true } diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/03-keyboard-shortcuts.md b/crates/codegen/xai-grok-pager/docs/user-guide/03-keyboard-shortcuts.md index a376c82..92afaf2 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/03-keyboard-shortcuts.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/03-keyboard-shortcuts.md @@ -212,6 +212,7 @@ Actions available from any screen. | Key | Alt Key | Action | Confirmation | |-----|---------|--------|-------------| | `Ctrl+N` | | Create a new session (optionally in a git worktree) | Yes (double-press within 1000ms) | +| `Ctrl+\` | | Open or toggle the [Agent Dashboard](23-dashboard.md) | No | | `Ctrl+Q` | `Ctrl+D` | Quit the application | Yes (double-press within 1000ms) | **VS Code family terminal** (VS Code, Cursor, Windsurf, Zed integrated terminals): `Ctrl+Q` is captured by the host, so Grok makes **`Ctrl+D` the sole quit key** (`Ctrl+Q` is not bound). Half-page-down is rebound to bare **`Shift+D`**. Mid-turn interject uses **`Ctrl+L`** (no alternates) because `Ctrl+Enter` / `Ctrl+I` do not reliably reach the PTY; extensions are opened via `/plugins` instead of `Ctrl+L`. @@ -239,6 +240,30 @@ Bindings that only fire on the welcome screen (before any agent session is open) --- +## Agent Dashboard + +Bindings while the [Agent Dashboard](23-dashboard.md) is focused (`Ctrl+\` or `/dashboard`). + +| Key | Action | +|-----|--------| +| `↑` / `↓`, `j` / `k` | Navigate agent rows (selecting a row opens peek) | +| `Enter` | Open the selected agent, or send a typed peek reply / dispatch prompt | +| `Ctrl+S` | Reply or dispatch **and** attach to that agent | +| `Ctrl+/` | Toggle search / filter mode | +| `Ctrl+R` | Rename the selected agent | +| `Ctrl+T` | Pin / unpin | +| `Ctrl+G` | Toggle grouping (state ↔ working directory) | +| `Ctrl+X` | Stop a running turn, or press twice within 2s to close the session | +| `Ctrl+O` | Toggle always-approve on the selected agent | +| `Tab` | Toggle focus between the list and the dispatch / peek input | +| `Esc` | Step back (cancel search → close peek → clear filter → unfocus → unselect → exit) | +| `Ctrl+\` | Exit the dashboard (or return from an attached agent) | +| `Ctrl+.` (alt: `?`) | Shortcuts cheatsheet | + +Details (peek vs dispatch, search prefixes, persistence): [Agent Dashboard](23-dashboard.md). + +--- + ## Command Palette Press `Ctrl+P` or `?` to open the command palette -- a searchable list of actions. The palette shows: diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/04-slash-commands.md b/crates/codegen/xai-grok-pager/docs/user-guide/04-slash-commands.md index 8602330..05947f0 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/04-slash-commands.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/04-slash-commands.md @@ -18,6 +18,12 @@ Start a fresh session and clear the current conversation. Alias: `/clear`. Open the session picker to reload a previous session from disk. +### `/dashboard` + +Open the [Agent Dashboard](23-dashboard.md): live roster of top-level sessions in this pager (peek, reply, dispatch, pin, rename, stop, attach). Aliases: `/agents-dashboard`, `/sessions`. + +Not `/config-agents` (alias `/agents`), which manages agent *definitions* and personas. Hidden in minimal mode; disable with `GROK_AGENT_DASHBOARD=0` or `[dashboard].enabled = false`. + ### `/compact [context]` Compress conversation history to reclaim context-window space. Pass a note to tell Grok what to keep: @@ -80,6 +86,12 @@ Quit the application. Alias: `/exit`. Leave the current session and return to the welcome screen. Alias: `/welcome`. +### `/delete` + +Delete the current session's history and return to the welcome screen. Confirms first. + +To delete a session you are not in, open `/resume` and press `d` then `y`. + ### `/rename` Rename the current session. Alias: `/title`. @@ -369,6 +381,8 @@ Open the Claude import modal to bring over `~/.claude` settings: permissions, en Open the agents modal to view and manage agent definitions, set the default, and switch the active one. Alias: `/agents`. +Not the live multi-session [Agent Dashboard](23-dashboard.md) (`/dashboard` / `Ctrl+\`). + ### `/personas` Create, edit, and delete personas. A subagent can apply a persona to shape how it behaves. diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/17-sessions.md b/crates/codegen/xai-grok-pager/docs/user-guide/17-sessions.md index 330d12c..00a9e73 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/17-sessions.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/17-sessions.md @@ -62,6 +62,14 @@ End the session and quit Grok: Alias: `/exit`. To leave the current session but stay in Grok, use `/home` to return to the welcome screen. +### Delete the current session + +``` +/delete +``` + +Confirms, then permanently removes the session history and returns to the welcome screen. From `/resume`, press `d` then `y` on a row to delete a session you are not currently in. + --- ## Resuming Sessions @@ -78,7 +86,7 @@ This opens a session picker that lists recent sessions for the current workspace Typing in the picker filters the list by title and also searches your conversation content as you type; content matches appear under an "Extended search results" heading. Press `Ctrl+/` to search immediately without the brief pause. -To switch between, rename, or close the sessions that are currently active (the parent session and any forks), use `/dashboard` (or its alias `/sessions`) instead. +For the live top-level sessions in this pager (parent and forks) — switch, rename, peek, dispatch, or close — use the [Agent Dashboard](23-dashboard.md): `/dashboard` (aliases `/sessions`, `/agents-dashboard`) or `Ctrl+\`. ### From the Command Line diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/23-dashboard.md b/crates/codegen/xai-grok-pager/docs/user-guide/23-dashboard.md index 8b24641..0245f86 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/23-dashboard.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/23-dashboard.md @@ -1,22 +1,26 @@ # Agent Dashboard -The Agent Dashboard is a centralised, agent-native overview of every -top-level session you have in flight — your local sessions and forks -— grouped by state, with peek, attach, and dispatch from one screen. -Subagents are not listed here: they run under their parent session, -which already shows when work is in flight. +The Agent Dashboard lists every top-level session in this pager process — +local sessions and forks — grouped by state. From one screen you can peek, +reply, attach, pin, rename, stop, or dispatch a new agent. Subagents are not +listed; they run under their parent, which already shows when work is in +flight. + +Not the agents modal (`/config-agents` / `/agents` — definitions and +personas), the session picker (`/resume` / `Ctrl+S` — past conversations on +disk), or the workflows run UI (`/workflows`). --- ## Opening the dashboard -Three entry points, all opening the same view: - -- **`grok dashboard`** — launches the TUI directly into the dashboard. +- **`grok dashboard`** — launch the TUI into the dashboard. - **`/dashboard`** (aliases **`/agents-dashboard`**, **`/sessions`**) — open - from inside an active session. -- **Ctrl+\\** — same as the slash command, two keystrokes. Configurable - in `~/.grok/config.toml` under `[keybindings]` like every other shortcut. + from inside a session. +- **`Ctrl+\`** — same view as the slash command. + +Hidden in minimal mode. Set `GROK_AGENT_DASHBOARD=0` or +`[dashboard].enabled = false` to disable. --- @@ -35,46 +39,36 @@ Three entry points, all opening the same view: ↑/↓ select (peek) · Enter open · Ctrl+R rename · Ctrl+T pin · Ctrl+X stop · ? help · Esc new ``` -Each row is a top-level agent (subagents aren't shown — they run under -their parent). Rows are sorted by state (Needs input → Working → Idle → -Inactive → Completed → Failed) so same-state rows sit adjacent, or by -working directory (toggle with `Ctrl+G`). **Inactive** holds roster-only -sessions — idle/dormant sessions owned by other pager processes that -haven't been loaded in this one — so **Idle** stays focused on the -sessions you're actively cycling between. Because it's background noise, -**Inactive starts collapsed** (expand it with `→` / click — see below). +Each row is a top-level agent. Sort by state (Needs input → Working → Idle → +Inactive → Completed → Failed) so same-state rows sit together, or by working +directory (`Ctrl+G` toggles). **Inactive** is roster-only sessions owned by +other pager processes that this process has not loaded — background noise, so +the section **starts collapsed** (expand with `→` / click). -To keep the **Idle** group scannable, only the most recent idle agents -stay visible — the 8 freshest, plus any active within the last hour. The -rest fold into a **"N more"** row (marked with a `+` / `-` toggle) at the -bottom of the group; select it and press `Enter` / `→` (or click it) to -reveal them all, and `←` to re-fold. The Idle header always shows the true total. Folding is -suspended while a filter or search is active (so every match shows). +To keep **Idle** scannable, only the most recent idle agents stay visible — +the 8 freshest, plus any active within the last hour. The rest fold into a +**"N more"** row at the bottom of the group; select it and press `Enter` / +`→` (or click) to expand, `←` to re-fold. The Idle header always shows the +true total. Folding is suspended while a filter or search is active. -The state icon matches Grok Build's sibling views ( -`tasks_pane`): +State icons match other session lists in Grok Build: -- `⋅`/`:`/`⸬`/`⁙` — animated spinner for **Working** rows. +- `⋅`/`:`/`⸬`/`⁙` — animated spinner for **Working** - `●` — filled circle for **Needs input**, **Completed**, **Failed**, - **Blocked**. Colour communicates the state (yellow / green / red / - amber). -- `○` — hollow circle for **Idle** and **Inactive** rows. + **Blocked** (color: yellow / green / red / amber) +- `○` — hollow circle for **Idle** and **Inactive** -A row stays in **Working** while it has live background work even if its -turn has finished — a running background task, a `monitor`, or an active -scheduled `/loop`. The activity line says what's running (e.g. -`1 monitor · 2 loops still running`), since each can wake the agent for a -new turn. +A row stays **Working** while it has live background work even if its turn +has finished — a background task, a `monitor`, or an active scheduled +`/loop`. The activity line says what is still running (for example +`1 monitor · 2 loops still running`). -There are no inline group headers — the sort order keeps same-state -rows adjacent and the per-row dot+colour communicates which group -each row belongs to (matching other session lists). +There are no inline group headers; sort order keeps same-state rows adjacent, +and the per-row dot + color shows the group. -The dispatch input shares the same `PromptWidget` chrome as the -agent view's prompt (rounded box, `❯` prefix, accent border, info -line). Pressing `Ctrl+/` flips it into **search mode**: the `❯` -prefix becomes a yellow `Search:` and whatever you type live-filters -the row list instead of being dispatched. +The dispatch input uses the same prompt chrome as the agent view. Press +`Ctrl+/` to flip it into **search mode**: the `❯` prefix becomes a yellow +`Search:` and typing live-filters the list instead of dispatching. --- @@ -82,248 +76,185 @@ the row list instead of being dispatched. | Key | Action | | --- | --- | -| `↑` / `↓`, `j` / `k` | Navigate rows AND section titles (selecting a row opens its peek panel) | -| `→` / `←` (on a section title) | Expand / collapse the section (shows / hides its rows); `l` / `h` in vim mode | +| `↑` / `↓`, `j` / `k` | Navigate rows and section titles (selecting a row opens peek) | +| `→` / `←` (on a section title) | Expand / collapse the section (`l` / `h` in vim mode) | | `Enter` (on a section title) | Toggle the section collapsed / expanded | -| `Enter` (empty reply) | Open the selected agent's conversation full-screen (details view) | -| `Ctrl+S` | Send the peek reply AND open the agent (or dispatch + attach a new session) | -| `Shift+Enter` / `Alt+Enter` | Insert a newline in the reply / dispatch input (multiline compose) | -| `1`–`9` | Answer a pending permission / ask question (when the peek shows options) | +| `Enter` (empty reply) | Open the selected agent full-screen (details view) | +| `Ctrl+S` | Send the peek reply and open the agent (or dispatch and attach a new session) | +| `Shift+Enter` / `Alt+Enter` | Newline in the reply / dispatch input | +| `1`–`9` | Answer a pending permission / ask question when peek shows options | | `Enter` (typed reply) | Send / queue the reply to the selected agent | -| `/` | Types a literal `/` into the prompt | -| `Ctrl+/` | Toggle search mode (live-filter the rows) | +| `/` | Literal `/` into the prompt | +| `Ctrl+/` | Toggle search mode (live-filter rows) | | `Ctrl+R` | Rename selected row | | `Ctrl+T` | Pin / unpin | | `Ctrl+G` | Toggle grouping (state ↔ directory) | | `Ctrl+X` | Stop / kill (two presses within 2s to close a session) | | `Shift+↑` / `Shift+↓` | Reorder pinned rows | -| `Esc` | Step back one level: cancel search → close peek (clear reply draft, then unselect) → clear filter → **unfocus the dispatch input** (so `↑`/`↓`, `j`/`k` navigate the list) → unselect row (→ `[+ New Agent]`) → exit dashboard. Esc never clears your typed dispatch draft — use `Ctrl+U` / `Ctrl+C` for that | -| `Ctrl+\` | Return to the dashboard from the details view, or exit dashboard | -| `Ctrl+.` (alt: `?`) | Open the keyboard shortcuts cheatsheet. Footer advertises `?` when `Ctrl+.` cannot be delivered. Bare `?` opens help when list-focused or the draft is empty (otherwise types); `Ctrl+X` remains stop | +| `Esc` | Step back: cancel search → close peek → clear filter → unfocus dispatch → unselect row → exit. Never clears a typed dispatch draft (`Ctrl+U` / `Ctrl+C` for that) | +| `Ctrl+\` | Return from details view, or exit dashboard | +| `Ctrl+.` (alt: `?`) | Keyboard shortcuts cheatsheet. Footer shows `?` when `Ctrl+.` cannot be delivered. Bare `?` opens help when list-focused or the draft is empty | -When grouping by state, each group has a **section title** (e.g. `Working`, -`Idle`) with a `▸`/`▾` disclosure marker. Section titles are part of the -up/down navigation: select one and press `→` to expand it (showing its rows) -or `←` to collapse it — `l` / `h` do the same when vim mode is on. -**Clicking** a section title toggles it, and **hovering** -brightens its text. Collapse state is remembered while the dashboard stays open. -The **Inactive** section starts collapsed by default each time the pager -starts; expanding it sticks until you quit. +When grouping by state, each group has a **section title** (for example +`Working`, `Idle`) with a `▸`/`▾` marker. Select a title and press `→` / +`←` to expand or collapse (`l` / `h` in vim mode). Click toggles; hover +brightens. Collapse state is remembered while the dashboard stays open. +**Inactive** starts collapsed each time the pager starts; expanding it sticks +until you quit. -Opening a row shows the agent's conversation in the **details view**: -a single top header row (the agent name on the left, `{i}/{n} [‹][›] -[Dashboard]` cycle/close affordances on the right) sits above the conversation, -which renders **full-width** — no bordered modal frame — so the prompt -position and overall padding match the dashboard list view. All key -presses route to the attached agent; `Esc` / `Ctrl+\\` (or the `[Dashboard]` -affordance) return to the dashboard, the `[‹]` / `[›]` chips cycle to -the previous / next agent, and the agent's shortcuts bar shows a -`Ctrl+\\: back to dashboard` hint. Quick gotcha — `Esc` only returns to -the dashboard; typing `/exit` inside the agent actually closes the -underlying session (returning to the dashboard with a "Session closed" -toast). +Opening a row shows the agent's conversation in the **details view**: a top +header (agent name; `{i}/{n}` cycle chips and `[Dashboard]` on the right) +above a full-width conversation — no bordered modal — so padding matches the +list view. Keys go to the attached agent; `Esc` / `Ctrl+\` (or `[Dashboard]`) +return to the dashboard; `[‹]` / `[›]` cycle agents. The shortcuts bar shows +`Ctrl+\: back to dashboard`. Gotcha: `Esc` only returns; `/exit` inside the +agent closes the session (dashboard toast: "Session closed"). -`Ctrl+X` in the details view is state-dependent. While a **turn is -running** it cancels the turn — the same behaviour as `Ctrl+C`, -including the keep-subagents prompt — and never touches the session -itself, so mashing it to stop a turn can't close anything. In any -other state — **idle**, a slash command in flight (commands can't -be cancelled yet), or a cancel still pending — `Ctrl+X` arms a -confirmation: the shortcuts bar flips to "press Ctrl+x again to -close this session", and a second press within 2 seconds closes the -session and returns you to the dashboard. Pressing any other key -cancels the confirmation, and a turn that starts inside the window -downgrades the confirmed press to a cancel instead of closing. -(On terminals where `Ctrl+X` doubles as the shortcuts-cheatsheet -binding, the cheatsheet stays reachable via `Ctrl+.` inside the -details view.) +`Ctrl+X` in the details view is state-dependent. While a **turn is running** +it cancels the turn (same as `Ctrl+C`, including the keep-subagents prompt) +and never closes the session. Otherwise — **idle**, a slash command in +flight, or a cancel still pending — `Ctrl+X` arms a confirmation: press again +within 2 seconds to close the session and return to the dashboard. Any other +key cancels the confirmation; a turn that starts inside the window turns the +confirmed press into a cancel instead. (If `Ctrl+X` is also the cheatsheet +binding on your terminal, use `Ctrl+.` inside the details view.) -For the full behavioural specification (including the registry-lookup -rules and the mouse-event intercept matrix) see plan -[§3.10](../../plan/agent-dashboard.md) "Keybindings (v1)" — the user -guide here is intentionally short and cross-references the plan as -the source of truth. +See [Keyboard Shortcuts](03-keyboard-shortcuts.md#agent-dashboard). -All shortcuts are registered under `When::DashboardFocused` and can be -rebound via `~/.grok/config.toml`. +--- + +## Completing or closing a session + +There is **no** “mark completed” command. Row state is derived from the agent: + +- **Completed** / **Failed** when work ends on its own (turn finished and no + background task / monitor / `/loop` still running). +- **`Ctrl+X` once** while a turn is running cancels the turn. +- **`Ctrl+X` twice** (within 2s) on an idle / stopped row **closes** the + session and removes it from the live roster. +- In the details view, `/exit` also closes the session (Esc only returns). + +Use close/stop when you want a row gone; there is no manual complete flag. --- ## Dispatch input -The bottom textarea **always spawns a NEW session** — it is never a -reply target. A selected row is the overview's navigation cursor, not a -reply destination; to talk to an existing agent, open it (navigate + -`Enter`, or click) and reply inside its own view. +The bottom textarea **always spawns a new session**. A selected row is the +navigation cursor, not a reply target — open an agent to talk to it. -Enter handler: +- Free text → new top-level session seeded with the prompt. Text is never + treated as a filter (even if it starts with `/`, `s:`, `a:`, or `#`); + filtering is `Ctrl+/` search mode. A leading `/` runs a pager-global slash + command. +- Empty input → open the selected row, or create a new agent when + `[+ New Agent]` is focused. -- Free text → creates a new top-level session, seeded with the prompt. - Text is **never** reinterpreted as a filter — a prompt may start with - `/`, `s:`, `a:`, or `#` and still dispatches verbatim (filtering is - the explicit `Ctrl+/` search mode). A leading `/` runs a pager-global - slash command. -- Empty input → opens the selected row (`Attach`), or creates a new - agent when the `[+ New Agent]` button is focused. +`Ctrl+S` after typing dispatches **and** attaches; plain `Enter` stays on the +dashboard so you can dispatch several sessions. `Shift+Enter` / `Alt+Enter` +insert a newline; the box grows with the draft (up to a cap, then scrolls). -Press `Ctrl+S` after typing a prompt to dispatch AND attach -(jump into the new session); plain `Enter` stays on the dashboard so -you can dispatch several sessions in a row. `Shift+Enter` / `Alt+Enter` -insert a newline for a multi-line prompt — the box **grows in height** -as you add lines (up to a cap, after which it scrolls), so the whole -draft stays visible. - -The dispatch input accepts any non-empty prompt; an empty / -whitespace-only prompt is ignored. Prompts above 64 KiB are rejected -with a toast. +Empty or whitespace-only prompts are ignored. Prompts above 64 KiB are +rejected with a toast. ### Focus: input bar ↔ overview list (`Tab`) -The dashboard has two focus areas — the **dispatch input bar** (typing) -and the **overview list** (navigating). `Tab` toggles between them; the -inactive input dims its border and hides its caret. +Two focus areas: the **dispatch input** and the **overview list**. `Tab` +toggles between them; the inactive input dims its border and hides its caret. On open, focus defaults to the **overview list** when at least one agent -exists (so `↑`/`↓` / vim `j`/`k` navigate immediately). With **no** -agents, focus stays on the **dispatch input** so you can type a first -prompt right away. Either way, the `[+ New Agent]` button is the cursor -target (no agent row is pre-selected). +exists (so `↑`/`↓` / vim `j`/`k` navigate immediately). With **no** agents, +focus stays on the **dispatch input**. Either way, the cursor starts on +`[+ New Agent]` (no agent row pre-selected). -- **Input focused**: type to compose a new-session prompt. `↑`/`↓` - navigate the row list when the prompt is empty (a convenience), - otherwise move the caret. `Esc` unfocuses the input → overview list - (your typed draft is kept) so you can navigate straight away. -- **Overview focused**: `↑`/`↓` — and, in **vim mode**, `j`/`k` — move - between agent rows. `Enter` opens the highlighted agent (on - `[+ New Agent]`, it sends a typed draft, else creates a new session). - `Esc` **stays on the list** and steps back — clearing an active filter, - then unselecting the row (→ `[+ New Agent]`), then exiting the - dashboard. `Tab` or `i` (vim) — or any other printable key — return to - the input. +- **Input focused**: type a new-session prompt. Empty prompt: `↑`/`↓` + navigate rows; non-empty: move the caret. `Esc` unfocuses to the list + (draft kept). +- **Overview focused**: `↑`/`↓` (and vim `j`/`k`) move between rows. `Enter` + opens the highlighted agent (on `[+ New Agent]`, sends a typed draft or + creates a new session). `Esc` stays on the list and steps back — clear + filter, then unselect (→ `[+ New Agent]`), then exit. `Tab`, `i` (vim), or + any printable key returns to the input. --- ## Peek panel -The peek panel is shown **by default whenever an agent row is -selected** — it **replaces** the new-session dispatch box. With no row -selected (the `[+ New Agent]` button focused, or after `Esc`), the -dispatch box returns for starting a new session. So selecting a row is -how you talk to an existing agent; deselecting is how you start a new +Selecting an agent row shows the **peek panel** in place of the dispatch box. +With no row selected (`[+ New Agent]`, or after `Esc`), the dispatch box +returns. Select a row to talk to an existing agent; deselect to start a new one. -The panel shows, top to bottom, a header (the **last response type** — -`Thinking` / `Thought` / `Response` / `Edit` / `Read` / `Bash` / … — on -the left, **time** on the far right), the most recent response -(**word-wrapped** to fit, up to ~3 rows), and a live `❯ reply` input. A -`…` marker appears on the last row only when there's more than fits. +Top to bottom: header (**last response type** — `Thinking` / `Thought` / +`Response` / `Edit` / `Read` / `Bash` / … — and **time**), the most recent +response (word-wrapped, up to ~3 rows; `…` when truncated), and a live +`❯ reply` input. -The selected agent's **model** and, when it's in always-approve (yolo) -mode, an **`always-approve`** flag are shown on the panel's **bottom -border** (bottom-right) — the same config-badge slot the new-session -dispatch box uses. This holds in the question / approval modes too, so -the model and approval mode are always in view while you answer. (The -dashboard list rows no longer repeat the model or an always-approve badge, -keeping the list compact.) +The selected agent's **model** and, in always-approve (yolo) mode, an +**`always-approve`** flag sit on the panel's bottom border (same badge slot as +the dispatch box), including while answering questions. List rows no longer +repeat model or always-approve badges. **`Shift+Tab` cycles the peeked agent's mode** (Normal → Plan → -Always-approve → Normal) — the same cycle as Shift+Tab inside that agent's -chat view, applied to the **live** agent (the badge updates to match). -This differs from the new-session dispatch box, where Shift+Tab only -stages the mode for the *next* agent. +Always-approve → Normal) on the **live** agent. On the dispatch box, +Shift+Tab only stages mode for the *next* agent. -Unlike the dispatch box (which only ever spawns new sessions), the -peek's reply **talks to the selected agent**: +Unlike dispatch (new sessions only), peek reply **talks to the selected +agent**: -- **Type into `❯ reply`, then `Enter`** to send. An **idle** agent - starts the turn immediately; a **busy** agent **queues** the message - so it sends after the current turn finishes (the same queue/drain - behaviour as the agent view's own prompt). `Ctrl+S` replies AND - opens the agent's detail view; `Shift+Enter` / `Alt+Enter` insert a - newline (multiline compose) and the reply **grows in height** to fit - the draft (up to a cap, then it scrolls). -- With an **empty** reply, `Enter` opens the agent. -- **`↑`/`↓` move the caret within the reply** once it has content (so you - can edit a multi-line draft). While the reply is **empty** (or - unfocused via `Tab`), `↑`/`↓` instead **switch the selected agent** — - the panel follows the selection cursor and refreshes live, and the - switch clears any half-typed draft so a reply can't land on the wrong - agent. (`Tab` to the row list to navigate agents while a draft is in - the reply.) -- **`Esc` unselects**: it first clears a typed reply, then deselects the - row and focuses the `[+ New Agent]` button (bringing back the - new-session input). -- **`Tab`** toggles focus between the reply input and the row list: an - unfocused reply dims its border and hides the caret; a printable key - re-focuses it and starts composing. -- The reply is a **full prompt editor** (the same component as the - dispatch box and the agent prompt): pasting multi-line text folds - into a `[Pasted: N lines]` chip with the same preview overlay and - expand affordances as the agent prompt (`Enter` / double-click / - paste-again), mouse click / drag place the caret and select text, - and the usual editing chords work (word navigation, `Ctrl+A`/`Ctrl+E`, - `Alt+Backspace`, `Ctrl+W`/`Ctrl+U`/`Ctrl+K`, undo, Shift+arrow - selection, `Ctrl+Shift+V` inline paste). - Typing **`@`** opens the file-context picker rooted at the **peeked - agent's** working directory (so `@path` resolves against the agent - you're replying to); its dropdown floats **above** the panel and - `↑`/`↓`/`Tab`/`Enter`/`Esc` drive it while it's open. - Dashboard chords (`Ctrl+X` stop, `Ctrl+T` pin, `Shift+↑/↓` reorder, - …) still win over the editor while the panel is open. -- When a **permission / ask-tool question** is pending, the `❯ reply` - row is hidden and the options are listed instead: **`↑`/`↓` move the - highlighted option** (marked with `▸`) and **`Enter` answers** it. - **`1`–`9`** still answer an option directly. (While answering, the - arrows pick options rather than switching agents.) -- The **free-text row** accepts an inline typed answer (just like the - chat panel): the permission **"No" / reject** option ("No, reject - (type to add feedback)") and the ask-tool **"Other"** row ("Other - (type your own answer)"). Type on it and `Enter` sends the rejection + - message / the free-text answer. -- This also covers the agent's **Ask tool** (`AskUserQuestion`): its - options + the "Other" row show in the peek, answered the same way. - **Multi-question** forms are walked one question at a time — a `(i/N)` - marker shows progress and each answer advances to the next, submitting - on the last. (Forms with a **multi-select** question are left to the - agent's own view — open the agent to answer those.) +- **Type into `❯ reply`, then `Enter`** to send. Idle agents start immediately; + busy agents **queue** the message (same as the agent view prompt). `Ctrl+S` + replies and opens the detail view; `Shift+Enter` / `Alt+Enter` insert a + newline (reply grows with the draft). +- Empty reply + `Enter` opens the agent. +- **`↑`/`↓` move the caret** once the reply has content. While empty (or + unfocused via `Tab`), `↑`/`↓` **switch the selected agent** — the panel + follows, and a half-typed draft is cleared so it cannot land on the wrong + agent. (`Tab` to the list to navigate while a draft is in the reply.) +- **`Esc` unselects**: clear a typed reply first, then deselect and focus + `[+ New Agent]`. +- **`Tab`** toggles focus between reply and row list; a printable key + re-focuses the reply. +- Full prompt editor (same as dispatch / agent prompt): multi-line paste + chips, mouse select, word navigation, `Ctrl+A`/`Ctrl+E`, `Alt+Backspace`, + `Ctrl+W`/`Ctrl+U`/`Ctrl+K`, undo, Shift+arrow selection, `Ctrl+Shift+V` + inline paste. **`@`** opens the file picker rooted at the **peeked agent's** + working directory; the dropdown floats above the panel. Dashboard chords + (`Ctrl+X` stop, `Ctrl+T` pin, `Shift+↑/↓` reorder, …) still win while the + panel is open. +- Pending **permission / ask-tool** question: `❯ reply` hides; options list + instead. **`↑`/`↓` highlight**, **`Enter` answers**, **`1`–`9`** answer + directly. Free-text **No / reject** and ask-tool **Other** accept a typed + answer on the free-text row. Multi-question Ask forms walk one at a time + (`(i/N)`); multi-select forms need the agent's own view. -The panel only renders when the terminal is tall enough; on very short -terminals the dispatch box shows even with a row selected. +On very short terminals the panel may not fit; the dispatch box stays even +with a row selected. --- ## Search / filter (`Ctrl+/`) -Filtering lives behind an explicit **search mode** so normal typing -always dispatches. Press `Ctrl+/` to toggle it: the prompt prefix -flips from `❯` to a yellow `Search:` and every keystroke live-filters -the row list. +`Ctrl+/` toggles search mode so normal typing always dispatches. Prefix +flips from `❯` to yellow `Search:`; every keystroke live-filters the list. -Inside search mode: +- `Enter` — confirm: keep the filter and return to the dispatch prompt. +- `Esc` or `Ctrl+/` — cancel: clear the filter and exit search. +- `↑` / `↓` — navigate filtered rows. -- `Enter` — **confirm**: keep the filter applied and return to the - dispatch prompt (rows stay filtered; `Esc` later clears them). -- `Esc` or `Ctrl+/` — **cancel**: clear the filter and exit search. -- `↑` / `↓` — navigate the filtered rows. +Prefixes (only inside search mode): -The query supports the same prefixes as before (they are only honoured -*inside* search mode now): - -- `a:` — filter by agent label (case-insensitive substring, - matches persona / role). -- `s:` — filter by row state. Accepts `working`, `idle`, - `completed`, `failed`, `needs-input`, `blocked` and synonyms - (`busy`/`running`/`done`/etc.). -- `#` — substring match on `#` (matches the literal - `#` in labels; reserved for future PR filtering). -- anything else — plain substring match over label + working dir. +- `a:` — agent label (case-insensitive substring; persona / role). +- `s:` — row state: `working`, `idle`, `completed`, `failed`, + `needs-input`, `blocked` and synonyms (`busy`/`running`/`done`/etc.). +- `#` — substring match on `#` (literal `#` in labels). +- anything else — substring over label + working dir. --- ## Persistence -Per-user dashboard preferences live under `[dashboard]` in -`~/.grok/config.toml`: +Per-user preferences under `[dashboard]` in `~/.grok/config.toml`: ```toml [dashboard] @@ -333,19 +264,5 @@ pinned = ["top:", "sub::"] reorder = ["top:"] ``` -Pinned/reorder entries are keyed by **session id**, not by the -per-process `AgentId(usize)`, so they survive restarts and don't -attach to whatever agent happens to share the old slot number. - -Set `GROK_AGENT_DASHBOARD=0` to force-disable the feature for a single -pager invocation; the slash command and CLI subcommand will print a -friendly toast. - ---- - -## Phase 4 (out of scope for v1) - -The current dashboard lists only agents owned by **this** pager -process. The plan's Phase 4 ("supervisor / `grok --bg`") would list -sessions that survive pager exit — that's a separate roadmap and not -shipped yet. +Pinned/reorder entries use **session id** (not a per-process agent slot), so +they survive restarts. diff --git a/crates/codegen/xai-grok-pager/src/app/acp_handler/interactions.rs b/crates/codegen/xai-grok-pager/src/app/acp_handler/interactions.rs index 4c366f9..c3eddda 100644 --- a/crates/codegen/xai-grok-pager/src/app/acp_handler/interactions.rs +++ b/crates/codegen/xai-grok-pager/src/app/acp_handler/interactions.rs @@ -85,6 +85,7 @@ pub(crate) fn handle_ask_user_question( LocalQuestionKind::AgentTypeMismatch { .. } => "model switch", LocalQuestionKind::ProjectSelect { .. } => "project select", LocalQuestionKind::DoctorFix { .. } => "/doctor fix", + LocalQuestionKind::DeleteCurrentSession => "/delete", }; let message = if matches!(kind, LocalQuestionKind::DoctorFix { .. }) { "/doctor fix was cancelled because another question opened.".to_owned() diff --git a/crates/codegen/xai-grok-pager/src/app/actions.rs b/crates/codegen/xai-grok-pager/src/app/actions.rs index 8c9b9bb..b888d3f 100644 --- a/crates/codegen/xai-grok-pager/src/app/actions.rs +++ b/crates/codegen/xai-grok-pager/src/app/actions.rs @@ -56,6 +56,11 @@ pub enum Action { ExitSession, /// Exit session without double-press confirmation (e.g., from command palette). ExitSessionConfirmed, + /// `/delete`: confirm, then delete history and return home. + DeleteCurrentSession, + DeleteCurrentSessionAnswered { + confirmed: bool, + }, /// Open grok.com in the browser for SuperGrok subscription upsell. OpenSupergrokUrl, /// Re-check subscription status via the shell's `x.ai/auth/check_subscription`. @@ -1368,6 +1373,14 @@ pub struct DoctorFixTarget { pub session_binding_epoch: u32, pub cwd: std::path::PathBuf, } +/// Aftermath of a successful session delete. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AfterSessionDelete { + /// Picker delete — stay put. + Stay, + /// `/delete` — return to welcome. + Welcome, +} #[derive(Debug)] pub enum Effect { /// Create a new ACP session. @@ -2012,6 +2025,7 @@ pub enum Effect { source: String, session_id: String, cwd: String, + after: AfterSessionDelete, }, /// Deep-search sessions by content (FTS via ACP). DeepSearchSessions { query: String, seq: u64 }, @@ -2587,6 +2601,7 @@ pub enum TaskResult { DeleteSessionComplete { source: String, session_id: String, + after: AfterSessionDelete, }, /// Session delete failed. DeleteSessionFailed { 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 19be97d..5307c3a 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 @@ -1733,6 +1733,11 @@ fn translate_local_submit( InputOutcome::Action(Action::DoctorFixCancelled(target)) } } + LocalQuestionKind::DeleteCurrentSession => { + InputOutcome::Action(Action::DeleteCurrentSessionAnswered { + confirmed: *idx == 0, + }) + } LocalQuestionKind::ProjectSelect { .. } => unreachable!(), } } diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/prompt.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/prompt.rs index ca1c1d8..8ba3f4e 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent_view/prompt.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/prompt.rs @@ -448,7 +448,9 @@ impl AgentView { { let history = self.combined_prompt_history(); let current_text = self.prompt.text().to_string(); - if !history.is_empty() { + // Without a matcher thread the panel can never populate, and filling + // the composer would only be undone by the next Down/Enter. + if !history.is_empty() && self.prompt.history_search.is_available() { self.prompt .history_search .activate_browse(&history, ¤t_text); 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 ee0726e..d532c4c 100644 --- a/crates/codegen/xai-grok-pager/src/app/app_view.rs +++ b/crates/codegen/xai-grok-pager/src/app/app_view.rs @@ -265,7 +265,7 @@ pub enum TickDemand { pub const SLOW_TICK_INTERVAL: Duration = Duration::from_millis(83); /// Welcome toast lifetime (wall clock, so the duration holds whether the /// event loop is ticking Slow or Fast). -const WELCOME_TOAST_DURATION: Duration = Duration::from_secs(4); +const WELCOME_TOAST_DURATION: Duration = Duration::from_secs(2); /// Which prompt box in-flight voice dictation appends its finalized text to. /// Captured when recording **starts** so a trailing STT final still lands where /// the user was dictating, even if they navigate away — or toggle a dashboard @@ -1197,8 +1197,16 @@ fn privacy_banner_reshow_elapsed(acked_at: &str, reshow_days: Option) -> bo }; chrono::Utc::now() >= next } -/// Bottom-right toast overlay on the welcome screen (mirrors agent toast style). -fn paint_welcome_toast(buf: &mut ratatui::buffer::Buffer, area: ratatui::layout::Rect, msg: &str) { +/// Welcome-screen toast overlay (mirrors agent toast style). +/// +/// Prefer one row above the prompt, right-aligned to it. Fall back to +/// the view bottom-right when no prompt rect is available (login / gate). +fn paint_welcome_toast( + buf: &mut ratatui::buffer::Buffer, + area: ratatui::layout::Rect, + msg: &str, + prompt_rect: Option, +) { let theme = crate::theme::Theme::current(); let max_msg = (area.width as usize).saturating_sub(4); if max_msg == 0 || area.height == 0 { @@ -1211,8 +1219,16 @@ fn paint_welcome_toast(buf: &mut ratatui::buffer::Buffer, area: ratatui::layout: format!(" {}… ", truncated.trim_end()) }; let w = toast.chars().count() as u16; - let x = area.right().saturating_sub(w + 1); - let y = area.bottom().saturating_sub(1); + let (x, y) = if let Some(prompt) = prompt_rect.filter(|r| r.width > 0 && r.y > area.y) { + let max_x = area.right().saturating_sub(w).max(area.x); + let x = prompt.right().saturating_sub(w + 1).clamp(area.x, max_x); + (x, prompt.y.saturating_sub(1)) + } else { + ( + area.right().saturating_sub(w + 1), + area.bottom().saturating_sub(1), + ) + }; for (i, ch) in toast.chars().enumerate() { if let Some(cell) = buf.cell_mut((x + i as u16, y)) { cell.set_char(ch); @@ -1964,7 +1980,7 @@ impl AppView { /// /// From the dashboard, toasts route into the dispatch input's inline /// error slot. From an agent view the existing per-agent toast machinery - /// fires. On welcome, a bottom-right overlay for + /// fires. On welcome, an overlay above the prompt for /// [`WELCOME_TOAST_DURATION`]. pub fn show_toast(&mut self, msg: &str) { match self.active_view { @@ -4373,7 +4389,12 @@ impl AppView { self.welcome_privacy_banner_policy_rect = result.privacy_banner_policy_rect; self.welcome_changelog_cta_rect = result.changelog_cta_rect; if let Some((ref msg, _)) = self.welcome_toast { - paint_welcome_toast(f.buffer_mut(), view_area, msg); + paint_welcome_toast( + f.buffer_mut(), + view_area, + msg, + self.welcome_prompt_rect, + ); } self.welcome_announcement.truncated = result.announcement_truncated; self.welcome_announcement.rect = result.announcement_rect; diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/router.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/router.rs index 6989457..65e7c6d 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/router.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/router.rs @@ -60,9 +60,10 @@ use super::session::fork::{ dispatch_startup_fork_session, }; use super::session::lifecycle::{ - clear_startup_actions, dispatch_agent_type_mismatch_answered, dispatch_exit_session, - dispatch_new_session, dispatch_new_session_inner, dispatch_new_session_with_id, - dispatch_new_worktree_session, dispatch_trust_folder, open_new_session_question, + clear_startup_actions, dispatch_agent_type_mismatch_answered, + dispatch_delete_current_session_answered, dispatch_exit_session, dispatch_new_session, + dispatch_new_session_inner, dispatch_new_session_with_id, dispatch_new_worktree_session, + dispatch_trust_folder, open_delete_current_session_question, open_new_session_question, }; use super::session::load::{ dispatch_cycle_session_source_filter, dispatch_load_session, dispatch_pick_content_session, @@ -194,6 +195,10 @@ pub(crate) fn dispatch(action: Action, app: &mut AppView) -> Vec { Action::NewSession => dispatch_new_session(app), Action::ChooseNewSessionMode => open_new_session_question(app), Action::ExitSession | Action::ExitSessionConfirmed => dispatch_exit_session(app), + Action::DeleteCurrentSession => open_delete_current_session_question(app), + Action::DeleteCurrentSessionAnswered { confirmed } => { + dispatch_delete_current_session_answered(app, confirmed) + } Action::NewWorktreeSession { load_session_id, label, @@ -949,7 +954,11 @@ pub(crate) fn dispatch(action: Action, app: &mut AppView) -> Vec { Action::SaveRememberNoteFromModal => dispatch_save_remember_note_from_modal(app), Action::SendBtw(question) => dispatch_send_btw(app, question), Action::SendRecap { auto } => dispatch_send_recap(app, auto), - Action::SetCodingDataSharing { opted_in } => set_coding_data_sharing(app, opted_in), + Action::SetCodingDataSharing { opted_in } => set_coding_data_sharing( + app, + opted_in, + xai_grok_telemetry::events::CodingDataConsentSource::Settings, + ), Action::ToggleYolo => dispatch_toggle_yolo(app), Action::ToggleMultiline => dispatch_toggle_multiline(app), Action::ToggleCompactMode => dispatch_toggle_compact_mode(app), @@ -1119,6 +1128,7 @@ pub(crate) fn dispatch(action: Action, app: &mut AppView) -> Vec { source, session_id, cwd, + after: crate::app::actions::AfterSessionDelete::Stay, }] } Action::Fork(args) => dispatch_fork(app, args), 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 ae9fadf..e65cbb4 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 @@ -394,6 +394,108 @@ pub(in crate::app::dispatch) fn dispatch_exit_session(app: &mut AppView) -> Vec< app.exit_session_pending = None; effects } +/// Confirm deleting the parent session (not a subagent view). +pub(in crate::app::dispatch) fn open_delete_current_session_question( + app: &mut AppView, +) -> Vec { + use crate::views::question_view::{LocalQuestionKind, QuestionViewState}; + use xai_grok_tools::implementations::grok_build::ask_user_question::{ + Question, QuestionOption, + }; + let ActiveView::Agent(id) = app.active_view else { + return vec![]; + }; + let Some(agent) = app.agents.get_mut(&id) else { + return vec![]; + }; + if agent.session.session_id.is_none() { + app.show_toast("No active session to delete"); + return vec![]; + } + if agent.question_view.is_some() { + app.show_toast("Finish answering the current question first"); + return vec![]; + } + let question = Question { + question: "Delete this session permanently?".into(), + id: None, + options: vec![ + QuestionOption { + label: "Delete".into(), + description: "Remove history and return home".into(), + preview: None, + id: None, + }, + QuestionOption { + label: "Cancel".into(), + description: "Keep the session".into(), + preview: None, + id: None, + }, + ], + multi_select: Some(false), + }; + let stashed = agent.prompt.stash(); + agent.question_view = Some( + QuestionViewState::new( + format!("delete-session-{}", uuid::Uuid::new_v4()), + vec![question], + stashed, + ) + .with_local_kind(LocalQuestionKind::DeleteCurrentSession) + .with_no_freeform(), + ); + agent.prompt.set_text(""); + vec![] +} +pub(in crate::app::dispatch) fn dispatch_delete_current_session_answered( + app: &mut AppView, + confirmed: bool, +) -> Vec { + if !confirmed { + return vec![]; + } + let ActiveView::Agent(id) = app.active_view else { + return vec![]; + }; + let Some((session_id, cwd, running_bg_tasks)) = app.agents.get(&id).and_then(|agent| { + let session_id = agent.session.session_id.clone()?; + let cwd = agent.session.cwd.display().to_string(); + let running_bg_tasks: Vec = agent + .session + .bg_tasks + .values() + .filter(|t| t.status == crate::app::agent::BgTaskStatus::Running) + .map(|t| t.task_id.clone()) + .collect(); + Some((session_id, cwd, running_bg_tasks)) + }) else { + app.show_toast("No active session to delete"); + return vec![]; + }; + let mut effects = vec![Effect::CancelTurn { + session_id: session_id.clone(), + cancel_subagents: true, + trigger: None, + rewind_if_pristine: false, + }]; + effects.extend( + running_bg_tasks + .into_iter() + .map(|task_id| Effect::KillBgTask { + session_id: session_id.clone(), + task_id, + }), + ); + app.show_toast("Deleting session\u{2026}"); + effects.push(Effect::DeleteSession { + source: "current".into(), + session_id: session_id.to_string(), + cwd, + after: crate::app::actions::AfterSessionDelete::Welcome, + }); + effects +} /// Handle the user accepting the folder-trust question: persist the grant for /// the workspace (writes `~/.grok/trusted_folders.toml`), mark trust resolved, /// then replay any deferred session startup (only if auth is also done). 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 5939ba7..3a36b40 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 @@ -413,6 +413,18 @@ pub(in crate::app::dispatch) fn dispatch_pick_session_in_worktree( } dispatch_new_worktree_session(app, Some(session_id), None, None, None, None, None) } +fn keep_picker_entry( + entry: &crate::app::app_view::SessionPickerEntry, + source: &str, + session_id: &str, + match_id_only: bool, +) -> bool { + if match_id_only { + entry.id != session_id + } else { + entry.source != source || entry.id != session_id + } +} /// Remove a deleted session identity from the modal session picker and the /// welcome-screen picker, then re-anchor the selection on a real row. /// @@ -422,6 +434,7 @@ pub(in crate::app::dispatch) fn remove_session_from_pickers( app: &mut AppView, source: &str, session_id: &str, + match_id_only: bool, ) { use crate::views::modal::ActiveModal; use crate::views::session_picker::build_entry_map; @@ -447,7 +460,7 @@ pub(in crate::app::dispatch) fn remove_session_from_pickers( *pending_delete = None; } if let Some(list) = entries.as_mut() { - list.retain(|entry| entry.source != source || entry.id != session_id); + list.retain(|entry| keep_picker_entry(entry, source, session_id, match_id_only)); } if let Some(hits) = content_results.as_mut() { hits.retain(|h| h.session_id != session_id); @@ -469,7 +482,7 @@ pub(in crate::app::dispatch) fn remove_session_from_pickers( reanchor_grouped_selection(state, &map); } if let Some(list) = app.session_picker_entries.as_mut() { - list.retain(|entry| entry.source != source || entry.id != session_id); + list.retain(|entry| keep_picker_entry(entry, source, session_id, match_id_only)); } if let Some(hits) = app.session_picker_content_results.as_mut() { hits.retain(|h| h.session_id != session_id); diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/status.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/status.rs index 7ddd5c3..dd35c68 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/status.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/status.rs @@ -110,9 +110,27 @@ fn is_current_coding_data_write(app: &AppView, seq: u64, agent_id: AgentId) -> b false } +fn log_coding_data_consent_selected( + source: xai_grok_telemetry::events::CodingDataConsentSource, + opted_in: bool, + previous_opted_in: bool, +) { + use xai_grok_telemetry::events::{CodingDataConsentChoice, CodingDataConsentSelected}; + xai_grok_telemetry::session_ctx::log_event(CodingDataConsentSelected { + source, + choice: CodingDataConsentChoice::from_opted_in(opted_in), + previous_choice: CodingDataConsentChoice::from_opted_in(previous_opted_in), + changed: opted_in != previous_opted_in, + }); +} + /// Set coding-data-sharing preference. SHELL-owned, auth-metadata-backed /// (persists via ACP ext-request, NOT `~/.grok/config.toml`). -pub(super) fn set_coding_data_sharing(app: &mut AppView, opted_in: bool) -> Vec { +pub(super) fn set_coding_data_sharing( + app: &mut AppView, + opted_in: bool, + source: xai_grok_telemetry::events::CodingDataConsentSource, +) -> Vec { // ── Guard 1: Enterprise ZDR ────────────────────────────────────── if app.is_zdr { app.show_toast("\u{2717} Cannot change: Zero Data Retention enabled"); @@ -131,6 +149,7 @@ pub(super) fn set_coding_data_sharing(app: &mut AppView, opted_in: bool) -> Vec< } let agent_id = coding_data_sharing_agent_id(app); let prev = !app.coding_data_retention_opt_out; + log_coding_data_consent_selected(source, opted_in, prev); // ── Idempotent path: skip the ACP round-trip. ──────────────────── if prev == opted_in { @@ -455,7 +474,11 @@ pub(in crate::app::dispatch) fn dispatch_privacy_banner_opt_in(app: &mut AppView if app.privacy_banner_opt_in_inflight || !app.privacy_banner_should_show() { return vec![]; } - let effects = set_coding_data_sharing(app, true); + let effects = set_coding_data_sharing( + app, + true, + xai_grok_telemetry::events::CodingDataConsentSource::PrivacyBanner, + ); // should_show guarantees opted-out + unguarded, so effects is only empty // if a guard regresses; leaving inflight false keeps [Opt in] clickable. app.privacy_banner_opt_in_inflight = !effects.is_empty(); @@ -476,6 +499,12 @@ pub(in crate::app::dispatch) fn dispatch_privacy_banner_opt_out(app: &mut AppVie if app.privacy_banner_opt_in_inflight || !app.privacy_banner_should_show() { return vec![]; } + let previous_opted_in = !app.coding_data_retention_opt_out; + log_coding_data_consent_selected( + xai_grok_telemetry::events::CodingDataConsentSource::PrivacyBanner, + false, + previous_opted_in, + ); let mut effects = ack_privacy_banner(app); effects.push(Effect::SetCodingDataSharing { agent_id: coding_data_sharing_agent_id(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 784c8cc..239c96d 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 @@ -39,6 +39,7 @@ use super::session::load::{ handle_session_loaded, handle_session_restore_failed, handle_session_restored, handle_session_search_debounce_expired, remove_session_from_pickers, }; +use super::session::modal::remove_agent_and_cleanup; use super::settings::ui::apply_setting_rollback; use super::status::{ commit_session_usage_block, handle_coding_data_sharing_failed, @@ -896,10 +897,40 @@ pub(super) fn dispatch_task_result(result: TaskResult, app: &mut AppView) -> Vec } vec![] } - TaskResult::DeleteSessionComplete { source, session_id } => { - remove_session_from_pickers(app, &source, &session_id); + TaskResult::DeleteSessionComplete { + source, + session_id, + after, + } => { + use crate::app::actions::AfterSessionDelete; + remove_session_from_pickers( + app, + &source, + &session_id, + after != AfterSessionDelete::Stay, + ); + if after == AfterSessionDelete::Stay { + app.show_toast("Session deleted"); + return vec![]; + } + let sid = acp::SessionId::new(session_id.clone()); + let to_remove: Vec<_> = app + .agents + .iter() + .filter(|(_, agent)| agent.session.session_id.as_ref() == Some(&sid)) + .map(|(id, _)| *id) + .collect(); + let foreground = + matches!(app.active_view, ActiveView::Agent(id) if to_remove.contains(&id)); + for id in to_remove { + remove_agent_and_cleanup(app, id); + } + let mut effects = unregister_session_effect(Some(sid)); + if foreground && after == AfterSessionDelete::Welcome { + effects.extend(dispatch_exit_session(app)); + } app.show_toast("Session deleted"); - vec![] + effects } TaskResult::DeleteSessionFailed { source, 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 f6b8139..0d762bf 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 @@ -1681,6 +1681,7 @@ fn translate_local_submit_never_returns_persist_never_for_new_session() { } #[test] fn delete_session_action_emits_delete_effect() { + use crate::app::actions::AfterSessionDelete; let mut app = test_app_with_agent(); open_session_picker_with(&mut app, vec![make_picker_entry("s1", "/repo")]); let effects = dispatch( @@ -1691,17 +1692,108 @@ fn delete_session_action_emits_delete_effect() { }, &mut app, ); + assert!(matches!( + effects.as_slice(), + [Effect::DeleteSession { + source, + session_id, + cwd, + after: AfterSessionDelete::Stay, + }] if source == "local" && session_id == "s1" && cwd == "/repo" + )); +} +#[test] +fn delete_current_session_confirm_emits_effect() { + use crate::app::actions::AfterSessionDelete; + let mut app = test_app_with_agent(); + { + let a = app.agents.get_mut(&AgentId(0)).unwrap(); + a.session.session_id = Some(acp::SessionId::new("sess-current")); + a.session.cwd = std::path::PathBuf::from("/repo"); + } + assert!(dispatch(Action::DeleteCurrentSession, &mut app).is_empty()); + assert!(matches!( + app.agents[&AgentId(0)] + .question_view + .as_ref() + .unwrap() + .local_kind, + Some(crate::views::question_view::LocalQuestionKind::DeleteCurrentSession) + )); + assert!( + dispatch( + Action::DeleteCurrentSessionAnswered { confirmed: false }, + &mut app, + ) + .is_empty() + ); + let effects = dispatch( + Action::DeleteCurrentSessionAnswered { confirmed: true }, + &mut app, + ); assert!( matches!( - effects.as_slice(), - [Effect::DeleteSession { - source, - session_id, - cwd, - }] if source == "local" && session_id == "s1" && cwd == "/repo" + effects.first(), + Some(Effect::CancelTurn { + cancel_subagents: true, + .. + }) ), - "DeleteSession action must emit exactly one matching DeleteSession effect" + "must cancel the turn/subagents before delete, got {effects:?}" ); + assert!( + matches!( + effects.last(), + Some(Effect::DeleteSession { + session_id, + after: AfterSessionDelete::Welcome, + .. + }) if session_id == "sess-current" + ), + "got {effects:?}" + ); +} +#[test] +fn delete_current_session_complete_welcome_and_guard() { + use crate::app::actions::{AfterSessionDelete, TaskResult}; + let mut app = test_app_with_agent(); + app.agents.get_mut(&AgentId(0)).unwrap().session.session_id = + Some(acp::SessionId::new("sess-a")); + let effects = dispatch_task_result( + TaskResult::DeleteSessionComplete { + source: "current".into(), + session_id: "sess-a".into(), + after: AfterSessionDelete::Welcome, + }, + &mut app, + ); + assert!(matches!(app.active_view, ActiveView::Welcome)); + assert!(app.agents.is_empty()); + assert!( + effects + .iter() + .any(|e| matches!(e, Effect::UnregisterActiveSession { .. })) + ); + let mut app = test_app_with_agent(); + app.agents.get_mut(&AgentId(0)).unwrap().session.session_id = + Some(acp::SessionId::new("sess-a")); + let other = AgentId(1); + let session = make_test_agent_session(&app, other, "unused"); + app.agents + .insert(other, AgentView::new(session, ScrollbackState::new())); + app.agents.get_mut(&other).unwrap().session.session_id = Some(acp::SessionId::new("sess-b")); + app.active_view = ActiveView::Agent(other); + let effects = dispatch_task_result( + TaskResult::DeleteSessionComplete { + source: "current".into(), + session_id: "sess-a".into(), + after: AfterSessionDelete::Welcome, + }, + &mut app, + ); + assert!(matches!(app.active_view, ActiveView::Agent(id) if id == other)); + assert!(!app.agents.contains_key(&AgentId(0))); + assert!(!effects.iter().any(|e| matches!(e, Effect::Quit))); } #[test] fn entry_title_falls_back_to_short_session_id_when_no_prompt() { 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 5c0c756..941371a 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 @@ -1701,6 +1701,7 @@ fn delete_session_complete_removes_only_matching_source_and_id() { TaskResult::DeleteSessionComplete { source: "local".into(), session_id: "s1".into(), + after: crate::app::actions::AfterSessionDelete::Stay, }, &mut app, ); @@ -1781,6 +1782,7 @@ fn delete_both_session_clears_modal_and_welcome_content_hits() { TaskResult::DeleteSessionComplete { source: "both".into(), session_id: "shared".into(), + after: crate::app::actions::AfterSessionDelete::Stay, }, &mut app, ); @@ -1878,6 +1880,7 @@ fn delete_remote_session_clears_modal_and_welcome_content_hits() { TaskResult::DeleteSessionComplete { source: "remote".into(), session_id: "remote-only".into(), + after: crate::app::actions::AfterSessionDelete::Stay, }, &mut app, ); 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 631b25b..70bb99b 100644 --- a/crates/codegen/xai-grok-pager/src/app/effects/mod.rs +++ b/crates/codegen/xai-grok-pager/src/app/effects/mod.rs @@ -3247,7 +3247,7 @@ pub(crate) fn execute( } }); } - Effect::DeleteSession { source, session_id, cwd } => { + Effect::DeleteSession { source, session_id, cwd, after } => { let tx = acp_tx.clone(); tasks .spawn(async move { @@ -3291,6 +3291,7 @@ pub(crate) fn execute( TaskResult::DeleteSessionComplete { source, session_id, + after, } } Err(e) => { diff --git a/crates/codegen/xai-grok-pager/src/app/leader_cluster/mod.rs b/crates/codegen/xai-grok-pager/src/app/leader_cluster/mod.rs index 8ca720b..f02e22d 100644 --- a/crates/codegen/xai-grok-pager/src/app/leader_cluster/mod.rs +++ b/crates/codegen/xai-grok-pager/src/app/leader_cluster/mod.rs @@ -37,16 +37,9 @@ use std::time::Duration; use agent_client_protocol as acp; use tempfile::TempDir; -use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::task::JoinSet; -use tokio_util::compat::{TokioAsyncReadCompatExt, TokioAsyncWriteCompatExt}; use tokio_util::sync::CancellationToken; -use xai_acp_lib::{ - AcpAgentGatewayReceiver as GatewayReceiver, AcpAgentGatewaySender as GatewaySender, - AcpClientRx, LineBufferedRead, acp_send, -}; -use xai_grok_shell::agent::config::Config as AgentConfig; -use xai_grok_shell::agent::mvp_agent::MvpAgent; +use xai_acp_lib::{AcpClientRx, acp_send}; use xai_grok_shell::leader::{ ClientCapabilities as LeaderClientCapabilities, ClientMode, ConnectionStatus, LEADER_SOCKET_ENV, LeaderClient, LeaderEnvUrls, LeaderLock, LeaderReconnector, @@ -63,7 +56,6 @@ use crate::acp::leader_bridge::bridge_channels; use crate::acp::model_state::ModelState; use crate::scrollback::block::RenderBlock; -const SIMPLEX_BUF: usize = 8 * 1024 * 1024; const PUMP_TICK: Duration = Duration::from_millis(10); const TURN_BUDGET: Duration = Duration::from_secs(60); @@ -358,7 +350,7 @@ impl PagerLeaderCluster { /// wire a fresh REAL agent behind it. async fn spawn_leader_generation(&mut self) { let _ = std::fs::remove_file(&self.sock_path); - let (acp_tx, mut acp_rx) = tokio::sync::mpsc::unbounded_channel::(); + let (acp_tx, acp_rx) = tokio::sync::mpsc::unbounded_channel::(); let (response_tx, response_rx) = tokio::sync::mpsc::unbounded_channel::(); let cancel = CancellationToken::new(); self.server_cancel = cancel.clone(); @@ -396,66 +388,10 @@ impl PagerLeaderCluster { .await; })); - // Real agent behind the server. Copied from `run_leader`'s - // agent-spawn + IPC/stdout bridge blocks in - // xai-grok-shell/src/agent/app.rs (inside its LocalSet body) — a - // deliberate copy so production stays untouched. Second copy of the - // same wiring: xai-grok-shell/tests/test_leader_soak.rs ("Real agent - // behind it" block) — keep the two copies behaviorally identical. - let (agent_in_read, agent_in_write) = tokio::io::simplex(SIMPLEX_BUF); - let (agent_out_read, agent_out_write) = tokio::io::simplex(SIMPLEX_BUF); - - generation_tasks.push(tokio::task::spawn_local(async move { - let agent_config = AgentConfig::default(); - let auth_manager = Arc::new(agent_config.create_auth_manager()); - let (gw_tx, gw_rx) = tokio::sync::mpsc::unbounded_channel(); - let gateway = GatewaySender::new(gw_tx); - let agent = MvpAgent::new(gateway, &agent_config, auth_manager, None) - .expect("valid agent config"); - let incoming = LineBufferedRead::spawn_local(agent_in_read.compat()); - let (conn, handle_io) = acp::AgentSideConnection::new( - agent, - agent_out_write.compat_write(), - incoming, - |fut| { - tokio::task::spawn_local(fut); - }, - ); - tokio::task::spawn_local( - GatewayReceiver::new(gw_rx, conn) - .with_on_meta(xai_file_utils::trace_context::span_from_meta_traceparent) - .run(), - ); - let _ = handle_io.await; - })); - - generation_tasks.push(tokio::task::spawn_local(async move { - let mut agent_in_write = agent_in_write; - while let Some(msg) = acp_rx.recv().await { - if agent_in_write.write_all(msg.as_bytes()).await.is_err() - || agent_in_write.write_all(b"\n").await.is_err() - { - break; - } - } - })); - generation_tasks.push(tokio::task::spawn_local(async move { - let mut reader = BufReader::new(agent_out_read); - let mut line = String::new(); - loop { - line.clear(); - match reader.read_line(&mut line).await { - Ok(0) => break, - Ok(_) => { - let msg = line.trim_end_matches(['\r', '\n']).to_string(); - if !msg.is_empty() && response_tx.send(msg).is_err() { - break; - } - } - Err(_) => break, - } - } - })); + generation_tasks.extend(xai_grok_shell::leader::in_process::spawn_agent( + acp_rx, + response_tx, + )); self.generation_tasks = generation_tasks; let deadline = tokio::time::Instant::now() + Duration::from_secs(10); diff --git a/crates/codegen/xai-grok-pager/src/docs.rs b/crates/codegen/xai-grok-pager/src/docs.rs index f0f6d77..b54f93b 100644 --- a/crates/codegen/xai-grok-pager/src/docs.rs +++ b/crates/codegen/xai-grok-pager/src/docs.rs @@ -156,6 +156,16 @@ pub static USER_GUIDE: &[Doc] = &[ "Permissions and Safety", "Modes, authorization order, allow/ask/deny rules, matching, and hooks" ), + guide!( + "23-dashboard.md", + "Agent Dashboard", + "Live multi-session roster: peek, dispatch, pin, stop, and search" + ), + guide!( + "24-monitoring-usage.md", + "Monitoring Usage (External OpenTelemetry)", + "Export usage metrics to a customer OpenTelemetry collector" + ), ]; /// Non-user-guide reference docs. Separate from USER_GUIDE because they diff --git a/crates/codegen/xai-grok-pager/src/notifications/sleep.rs b/crates/codegen/xai-grok-pager/src/notifications/sleep.rs index 6f75bf5..c075e62 100644 --- a/crates/codegen/xai-grok-pager/src/notifications/sleep.rs +++ b/crates/codegen/xai-grok-pager/src/notifications/sleep.rs @@ -111,6 +111,13 @@ impl SleepInhibitor { .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()); xai_tty_utils::detach_std_command(&mut cmd); + // The spawned process is the lock holder: `systemd-inhibit` keeps + // the idle-inhibit fd itself and runs `sleep infinity` as its child + // — it is the same pid `release()` SIGTERMs on a clean turn end. + // Bind that pid to us so a crashed/killed grok (SIGKILL, + // `panic=abort` SIGABRT — no Drop runs) can't leave an immortal + // inhibitor holding the lock and pid slots on shared hosts. + xai_tty_utils::kill_on_parent_death_std(&mut cmd); let result = cmd.spawn(); match result { diff --git a/crates/codegen/xai-grok-pager/src/slash/commands/delete.rs b/crates/codegen/xai-grok-pager/src/slash/commands/delete.rs new file mode 100644 index 0000000..5a2c884 --- /dev/null +++ b/crates/codegen/xai-grok-pager/src/slash/commands/delete.rs @@ -0,0 +1,31 @@ +//! `/delete` — delete this session's history and return home. + +use crate::app::actions::Action; +use crate::slash::command::{CommandExecCtx, CommandResult, SlashCommand}; + +pub struct DeleteCommand; + +impl SlashCommand for DeleteCommand { + fn name(&self) -> &str { + "delete" + } + + fn description(&self) -> &str { + "Delete this session and return home" + } + + fn session_scoped(&self) -> bool { + true + } + + fn usage(&self) -> &str { + "/delete" + } + + fn run(&self, ctx: &mut CommandExecCtx, _args: &str) -> CommandResult { + if ctx.session_id.is_none() { + return CommandResult::Error("No active session to delete".into()); + } + CommandResult::Action(Action::DeleteCurrentSession) + } +} diff --git a/crates/codegen/xai-grok-pager/src/slash/commands/mod.rs b/crates/codegen/xai-grok-pager/src/slash/commands/mod.rs index c3f3e0e..82fe20a 100644 --- a/crates/codegen/xai-grok-pager/src/slash/commands/mod.rs +++ b/crates/codegen/xai-grok-pager/src/slash/commands/mod.rs @@ -15,6 +15,7 @@ pub mod context; pub mod copy; pub mod dashboard; pub mod debug; +pub mod delete; pub mod docs; pub mod doctor; pub mod edit_prompt; @@ -81,6 +82,7 @@ pub fn builtin_commands() -> Vec> { Arc::new(help::HelpCommand), Arc::new(docs::DocsCommand), Arc::new(home::HomeCommand), + Arc::new(delete::DeleteCommand), Arc::new(new::NewCommand), Arc::new(fork::ForkCommand), Arc::new(compact::CompactCommand), @@ -263,6 +265,7 @@ mod tests { "cost", "dashboard", "debug", + "delete", "docs", "doctor", "edit-prompt", @@ -397,6 +400,19 @@ mod tests { assert!(matches!(result, CommandResult::Action(Action::ExitSession))); } #[test] + fn delete_requires_session_and_dispatches() { + let models = ModelState::default(); + let cmd = delete::DeleteCommand; + let mut ctx = make_ctx(&models); + assert!(matches!(cmd.run(&mut ctx, ""), CommandResult::Error(_))); + let session_id = acp::SessionId::new("sess-delete"); + ctx.session_id = Some(&session_id); + assert!(matches!( + cmd.run(&mut ctx, ""), + CommandResult::Action(Action::DeleteCurrentSession) + )); + } + #[test] fn view_plan_returns_show_plan_action() { let models = ModelState::default(); let mut ctx = make_ctx(&models); diff --git a/crates/codegen/xai-grok-pager/src/views/history_search.rs b/crates/codegen/xai-grok-pager/src/views/history_search.rs index 9ab46b6..6475438 100644 --- a/crates/codegen/xai-grok-pager/src/views/history_search.rs +++ b/crates/codegen/xai-grok-pager/src/views/history_search.rs @@ -63,7 +63,7 @@ enum Msg { struct Daemon { shared: Arc>, tx: SyncSender, - _handle: JoinHandle<()>, + handle: Option>, } const MAX_RESULTS: usize = 100; @@ -74,7 +74,7 @@ impl Daemon { let (tx, rx) = sync_channel::(256); let out = shared.clone(); - let handle = thread::spawn(move || { + let worker = move || { let mut pattern = MultiPattern::new(1); let mut matcher = Matcher::new(Config::DEFAULT); let mut items: Vec<(String, Utf32String)> = Vec::new(); @@ -82,7 +82,6 @@ impl Daemon { let mut prev_q = String::new(); while let Ok(msg) = rx.recv() { - // Drain to latest — skip intermediate queries. let msg = drain_to_latest(msg, &rx); match msg { @@ -144,13 +143,23 @@ impl Daemon { Msg::Stop => break, } } - }); + }; + let handle = thread::Builder::new() + .name("history-search".into()) + .spawn(worker); - Self { - shared, - tx, - _handle: handle, - } + let handle = match handle { + Ok(h) => Some(h), + Err(e) => { + tracing::error!( + error = %e, + "history search daemon thread spawn failed; history search disabled" + ); + None + } + }; + + Self { shared, tx, handle } } } @@ -355,6 +364,9 @@ impl HistorySearchState { } fn activate_inner(&mut self, history: &[HistoryEntry], current_text: &str, browse: bool) { + if !self.is_available() { + return; + } self.active = true; self.browse = browse; self.saved_text = current_text.to_string(); @@ -369,6 +381,12 @@ impl HistorySearchState { self.selected = self.snapshot.items.len().saturating_sub(1); } + /// False when the matcher thread never started, so the overlay cannot open + /// and callers must leave the composer alone. + pub fn is_available(&self) -> bool { + self.daemon.handle.is_some() + } + /// True while the overlay is in browse mode (see [`Self::activate_browse`]). pub fn is_browse(&self) -> bool { self.active && self.browse diff --git a/crates/codegen/xai-grok-pager/src/views/modal.rs b/crates/codegen/xai-grok-pager/src/views/modal.rs index bb0f52f..c0d5191 100644 --- a/crates/codegen/xai-grok-pager/src/views/modal.rs +++ b/crates/codegen/xai-grok-pager/src/views/modal.rs @@ -399,6 +399,11 @@ pub(crate) fn default_palette_entries( shortcut: "/home".into(), command: PaletteCommand::Home, }, + PaletteEntry { + label: "Delete This Session".into(), + shortcut: "/delete".into(), + command: PaletteCommand::SlashCommand("/delete".into()), + }, PaletteEntry { label: "Resume Session".into(), shortcut: "/resume".into(), diff --git a/crates/codegen/xai-grok-pager/src/views/question_view.rs b/crates/codegen/xai-grok-pager/src/views/question_view.rs index 23a9333..7e4f3cc 100644 --- a/crates/codegen/xai-grok-pager/src/views/question_view.rs +++ b/crates/codegen/xai-grok-pager/src/views/question_view.rs @@ -131,6 +131,7 @@ pub enum LocalQuestionKind { target: crate::app::actions::DoctorFixTarget, plan: Box, }, + DeleteCurrentSession, } // ── State ────────────────────────────────────────────────────────────── diff --git a/crates/codegen/xai-grok-pager/tests/pty_e2e/subscription_watch_and_gate_verify_pty.rs b/crates/codegen/xai-grok-pager/tests/pty_e2e/subscription_watch_and_gate_verify_pty.rs index 2e947cc..a5f1b2c 100644 --- a/crates/codegen/xai-grok-pager/tests/pty_e2e/subscription_watch_and_gate_verify_pty.rs +++ b/crates/codegen/xai-grok-pager/tests/pty_e2e/subscription_watch_and_gate_verify_pty.rs @@ -327,8 +327,9 @@ async fn subscription_watch_polls_free_tier_then_goes_dormant_after_upgrade() { #[ignore = "PTY e2e; run the owning pty_e2e_* Cargo test with --ignored (see Cargo.toml)"] async fn startup_gate_shows_paywall_for_free_user_after_live_check() { let content = ContentController::start().await.expect("start content"); - // Gated settings (no allow_access), free user (no subscriptionTier). + // Explicit deny + gate copy. Absent allow_access now fails open. content.server().set_settings(json!({ + "allow_access": false, "gate_message": GATE_MSG, "gate_url": "https://grok.com/supergrok?referrer=grok-build", "gate_label": "Subscribe", @@ -398,7 +399,10 @@ async fn stale_gate_push_never_flashes_paywall_for_subscribed_user() { // One stale gated snapshot: the "remote settings stale moment". content.enqueue_response( "/v1/settings", - ScriptedResponse::json(200, json!({ "gate_message": GATE_MSG })), + ScriptedResponse::json( + 200, + json!({ "allow_access": false, "gate_message": GATE_MSG }), + ), ); harness.inject_keys(b"/new\r").expect("run /new"); diff --git a/crates/codegen/xai-grok-sampler/src/stream/messages.rs b/crates/codegen/xai-grok-sampler/src/stream/messages.rs index f153eac..f8347f4 100644 --- a/crates/codegen/xai-grok-sampler/src/stream/messages.rs +++ b/crates/codegen/xai-grok-sampler/src/stream/messages.rs @@ -205,11 +205,7 @@ pub fn stream_messages<'a>( }; } } - ContentBlock::ToolUse { - id, - name, - input: _, - } => { + ContentBlock::ToolUse { id, name, .. } => { let tool_index = next_tool_index; next_tool_index += 1; block_to_tool_index.insert(index, tool_index); diff --git a/crates/codegen/xai-grok-sampler/src/stream/messages_tests.rs b/crates/codegen/xai-grok-sampler/src/stream/messages_tests.rs index b70eb72..60a7616 100644 --- a/crates/codegen/xai-grok-sampler/src/stream/messages_tests.rs +++ b/crates/codegen/xai-grok-sampler/src/stream/messages_tests.rs @@ -216,6 +216,8 @@ async fn tool_use_block_assembles_into_tool_call() { id: "call_xyz".into(), name: "do_thing".into(), input: serde_json::json!({}), + // Set: a parser matching only the absent case must fail here. + cache_control: Some(xai_grok_sampling_types::messages::CacheControl::ephemeral()), }, }; let arg_delta_1 = MessageStreamEvent::ContentBlockDelta { @@ -416,6 +418,7 @@ async fn refusal_after_tool_use_blocks_keeps_tool_calls_stop_reason() { id: "call_refused".into(), name: "do_thing".into(), input: serde_json::json!({}), + cache_control: None, }, }; let arg_delta = MessageStreamEvent::ContentBlockDelta { diff --git a/crates/codegen/xai-grok-sampling-types/src/conversation.rs b/crates/codegen/xai-grok-sampling-types/src/conversation.rs index ff06349..d6389fb 100644 --- a/crates/codegen/xai-grok-sampling-types/src/conversation.rs +++ b/crates/codegen/xai-grok-sampling-types/src/conversation.rs @@ -3064,12 +3064,77 @@ pub fn dedup_duplicate_tool_results(conversation: &mut Vec) -> // Anthropic Messages API Conversion // ============================================================================ +/// Marks the last block that can carry one, scanning back past `Thinking`, +/// which the API rejects a breakpoint on. +fn mark_message_cache_breakpoint(msg: &mut crate::messages::Message) -> bool { + use crate::messages::{CacheControl, ContentBlock, MessageContent}; + + match &mut msg.content { + MessageContent::Blocks(blocks) => { + for block in blocks.iter_mut().rev() { + let cache_control = match block { + ContentBlock::Text { cache_control, .. } + | ContentBlock::ToolResult { cache_control, .. } + | ContentBlock::Image { cache_control, .. } + | ContentBlock::ToolUse { cache_control, .. } => cache_control, + ContentBlock::Thinking { .. } => continue, + }; + *cache_control = Some(CacheControl::ephemeral()); + return true; + } + false + } + // Plain text cannot carry a breakpoint, so promote it to block form. + MessageContent::Text(text) => { + let text = std::mem::take(text); + msg.content = MessageContent::Blocks(vec![ContentBlock::Text { + text, + cache_control: Some(CacheControl::ephemeral()), + }]); + true + } + } +} + +/// An entry is written only at a breakpoint, so marking the system prompt alone +/// leaves the transcript uncached. The third covers a turn that appends more +/// than the API's 20 block lookback. The fourth slot stays free: a gateway that +/// turns on automatic caching takes it, and five is rejected outright. +fn apply_cache_breakpoints( + system_blocks: &mut [crate::messages::TextBlock], + messages: &mut [crate::messages::Message], +) { + use crate::messages::{CacheControl, MessageRole}; + + if let Some(last) = system_blocks.last_mut() { + last.cache_control = Some(CacheControl::ephemeral()); + } + + let tip = (0..messages.len()) + .rev() + .find(|&i| mark_message_cache_breakpoint(&mut messages[i])); + + // Where the previous request ended. A turn can append several user messages + // in a row, so skip the whole trailing run rather than a neighbour of the tip. + if let Some(tip) = tip + && let Some(prev) = messages[..tip] + .iter() + .rposition(|m| matches!(m.role, MessageRole::Assistant)) + .and_then(|assistant| { + messages[..assistant] + .iter() + .rposition(|m| matches!(m.role, MessageRole::User)) + }) + { + mark_message_cache_breakpoint(&mut messages[prev]); + } +} + /// Convert a ConversationRequest to Anthropic MessagesRequest. pub fn build_messages_request(req: &ConversationRequest) -> crate::messages::MessagesRequest { use crate::messages::{ - CacheControl, ContentBlock, ImageSource, Message, MessageContent, MessageRole, - MessagesRequest, OutputConfig, SystemParam, TextBlock, ToolChoiceParam, ToolParam, - ToolResultContent, + ContentBlock, ImageSource, Message, MessageContent, MessageRole, MessagesRequest, + OutputConfig, SystemParam, TextBlock, ToolChoiceParam, ToolParam, ToolResultContent, }; let mut system_blocks: Vec = Vec::new(); @@ -3115,6 +3180,7 @@ pub fn build_messages_request(req: &ConversationRequest) -> crate::messages::Mes media_type, data: data.to_string(), }, + cache_control: None, } } else { // Malformed data URI, treat as text @@ -3128,6 +3194,7 @@ pub fn build_messages_request(req: &ConversationRequest) -> crate::messages::Mes source: ImageSource::Url { url: url.as_ref().to_owned(), }, + cache_control: None, } } else { // Unknown format, treat as text @@ -3207,6 +3274,7 @@ pub fn build_messages_request(req: &ConversationRequest) -> crate::messages::Mes id: sanitize_tool_call_id(&tc.id), name: tc.name.clone(), input, + cache_control: None, }); } } @@ -3237,7 +3305,10 @@ pub fn build_messages_request(req: &ConversationRequest) -> crate::messages::Mes url: url.as_ref().to_owned(), } }; - blocks.push(ContentBlock::Image { source }); + blocks.push(ContentBlock::Image { + source, + cache_control: None, + }); } } ToolResultContent::Blocks(blocks) @@ -3283,12 +3354,7 @@ pub fn build_messages_request(req: &ConversationRequest) -> crate::messages::Mes flush_assistant(&mut pending_assistant, &mut messages); flush_tool_results(&mut pending_tool_results, &mut messages); - // Attach cache_control: {type: "ephemeral"} to last system block - if let Some(last) = system_blocks.last_mut() { - last.cache_control = Some(CacheControl { - r#type: "ephemeral".to_string(), - }); - } + apply_cache_breakpoints(&mut system_blocks, &mut messages); // Build system param let system: Option = if system_blocks.is_empty() { @@ -3394,7 +3460,9 @@ impl From for ConversationItem { } content.push_str(&text); } - ContentBlock::ToolUse { id, name, input } => { + ContentBlock::ToolUse { + id, name, input, .. + } => { tool_calls.push(ToolCall { id: Arc::::from(id), name, @@ -5493,6 +5561,176 @@ mod tests { ); } + fn count_cache_control(value: &serde_json::Value) -> usize { + match value { + serde_json::Value::Object(map) => { + usize::from(map.contains_key("cache_control")) + + map.values().map(count_cache_control).sum::() + } + serde_json::Value::Array(items) => items.iter().map(count_cache_control).sum(), + _ => 0, + } + } + + fn marker_on_last_block(message: &serde_json::Value) -> Option<&str> { + message + .get("content")? + .as_array()? + .last()? + .pointer("/cache_control/type")? + .as_str() + } + + fn agent_turn(n: usize) -> Vec { + let id = format!("call_{n}"); + vec![ + ConversationItem::assistant_tool_calls(vec![ToolCall { + id: id.as_str().into(), + name: "read_file".to_string(), + arguments: r#"{"path": "src/main.rs"}"#.into(), + }]), + ConversationItem::tool_result(id, "fn main() {}"), + ] + } + + fn agent_request(turns: usize) -> serde_json::Value { + let mut items = vec![ + ConversationItem::system("You are a helpful assistant."), + ConversationItem::user("Fix the bug"), + ]; + for n in 0..turns { + items.extend(agent_turn(n)); + } + serde_json::to_value(build_messages_request( + &ConversationRequest::from_items(items).with_model("messages-compatible-model"), + )) + .unwrap() + } + + #[test] + fn test_messages_request_cache_breakpoint_placement() { + let json = agent_request(2); + let messages = json["messages"].as_array().unwrap(); + + assert_eq!( + json.pointer("/system/0/cache_control/type") + .and_then(|v| v.as_str()), + Some("ephemeral"), + "{json:#}", + ); + assert_eq!( + marker_on_last_block(messages.last().unwrap()), + Some("ephemeral"), + "tip: {json:#}" + ); + assert_eq!( + messages.last().unwrap()["content"] + .as_array() + .and_then(|b| b.last()) + .and_then(|b| b["type"].as_str()), + Some("tool_result"), + ); + + let previous_user = messages[..messages.len() - 1] + .iter() + .rposition(|m| m["role"] == "user") + .unwrap(); + assert_eq!( + marker_on_last_block(&messages[previous_user]), + Some("ephemeral"), + "previous turn's tip: {json:#}", + ); + assert_eq!(count_cache_control(&json), 3, "{json:#}"); + } + + #[test] + fn test_messages_request_previous_tip_skips_a_trailing_user_run() { + let mut items = vec![ + ConversationItem::system("You are a helpful assistant."), + ConversationItem::user("Fix the bug"), + ]; + items.extend(agent_turn(0)); + items.extend(agent_turn(1)); + // The shape after a parallel batch: tool results, then followups. + items.push(ConversationItem::user("[Image content]")); + items.push(ConversationItem::user("")); + + let json = serde_json::to_value(build_messages_request( + &ConversationRequest::from_items(items).with_model("messages-compatible-model"), + )) + .unwrap(); + let messages = json["messages"].as_array().unwrap(); + + let marked: Vec = (0..messages.len()) + .filter(|&i| marker_on_last_block(&messages[i]).is_some()) + .collect(); + let last_assistant = messages + .iter() + .rposition(|m| m["role"] == "assistant") + .unwrap(); + assert_eq!(marked.len(), 2, "tip and previous tip only: {json:#}"); + assert_eq!(marked[1], messages.len() - 1, "tip: {json:#}"); + assert!( + marked[0] < last_assistant, + "the previous tip must sit before the last assistant turn, not inside \ + the trailing user run; got {marked:?} in {json:#}", + ); + } + + #[test] + fn test_messages_request_cache_breakpoint_marks_an_image_tip() { + let req = ConversationRequest::from_items(vec![ + ConversationItem::system("You are a helpful assistant."), + ConversationItem::User(UserItem { + content: vec![ + ContentPart::Text { + text: "what is in this screenshot".into(), + }, + ContentPart::Image { + url: "data:image/png;base64,iVBOR".into(), + }, + ], + ..Default::default() + }), + ]) + .with_model("messages-compatible-model"); + + let json = serde_json::to_value(build_messages_request(&req)).unwrap(); + let blocks = json["messages"][0]["content"].as_array().unwrap(); + + assert_eq!(blocks.last().unwrap()["type"].as_str(), Some("image")); + assert_eq!( + marker_on_last_block(&json["messages"][0]), + Some("ephemeral"), + "{json:#}", + ); + assert!(blocks[0].get("cache_control").is_none(), "{json:#}"); + } + + #[test] + fn test_messages_request_cache_breakpoint_skips_thinking() { + let req = ConversationRequest::from_items(vec![ + ConversationItem::user("Fix the bug"), + ConversationItem::Reasoning(synthesized_reasoning_item("weighing options")), + ConversationItem::assistant("Fixed it."), + ]) + .with_model("messages-compatible-model"); + + let json = serde_json::to_value(build_messages_request(&req)).unwrap(); + let blocks = json["messages"][1]["content"].as_array().unwrap(); + + let thinking = blocks + .iter() + .find(|b| b["type"] == "thinking") + .expect("reasoning should emit a thinking block"); + assert!(thinking.get("cache_control").is_none(), "{json:#}"); + assert_eq!( + marker_on_last_block(&json["messages"][1]), + Some("ephemeral"), + "{json:#}", + ); + } + /// Regression: /btw side questions snapshot the conversation (which may /// include thinking blocks from prior turns) but fire without /// reasoning_effort. If the caller forgets to strip reasoning from items, @@ -7954,7 +8192,7 @@ mod tests { matches!(&inner[0], crate::messages::ContentBlock::Text { text, .. } if text == "Read image file: photo.png") ); assert!( - matches!(&inner[1], crate::messages::ContentBlock::Image { source: crate::messages::ImageSource::Base64 { media_type, data } } if media_type == "image/png" && data == "iVBOR") + matches!(&inner[1], crate::messages::ContentBlock::Image { source: crate::messages::ImageSource::Base64 { media_type, data }, .. } if media_type == "image/png" && data == "iVBOR") ); } diff --git a/crates/codegen/xai-grok-sampling-types/src/messages.rs b/crates/codegen/xai-grok-sampling-types/src/messages.rs index e4b5bce..92b5d06 100644 --- a/crates/codegen/xai-grok-sampling-types/src/messages.rs +++ b/crates/codegen/xai-grok-sampling-types/src/messages.rs @@ -94,6 +94,14 @@ pub struct CacheControl { pub r#type: String, // "ephemeral" } +impl CacheControl { + pub fn ephemeral() -> Self { + Self { + r#type: "ephemeral".to_owned(), + } + } +} + /// Content blocks used in both requests and responses #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "snake_case")] @@ -105,11 +113,15 @@ pub enum ContentBlock { }, Image { source: ImageSource, + #[serde(skip_serializing_if = "Option::is_none")] + cache_control: Option, }, ToolUse { id: String, name: String, input: serde_json::Value, + #[serde(skip_serializing_if = "Option::is_none")] + cache_control: Option, }, ToolResult { tool_use_id: String, diff --git a/crates/codegen/xai-grok-shell/CHANGELOG.md b/crates/codegen/xai-grok-shell/CHANGELOG.md index 966ffc4..aca920f 100644 --- a/crates/codegen/xai-grok-shell/CHANGELOG.md +++ b/crates/codegen/xai-grok-shell/CHANGELOG.md @@ -1,5 +1,51 @@ # Changelog +# 0.2.114 — 2026-07-29 + +## Features + +- **New `/delete` slash command** removes the current session's history after confirmation. + +## Bug Fixes + +- **Grok** no longer crashes on startup when the host machine has no free threads. + + +# 0.2.113 — 2026-07-28 + +## Features + +- **MCP servers** can now be enabled or disabled directly from the CLI with `grok mcp enable ` and `grok mcp disable `. +- **Full plan markdown** can now be copied to the clipboard with `y` during plan approval or preview. +- **Added support for the new SuperGrok Plus subscription tier** in authentication and feature gating. +- **Enabled automatic recovery** from repetitive loops in model output by default. + +## Bug Fixes + +- **Terminal command output** is no longer lost or duplicated when the gateway is unreachable. +- **Invalid MCP server entries** in config.toml no longer prevent Grok from starting; problems are shown in `grok inspect`. +- **SessionEnd hooks** now run on exit in non-leader TUI and headless sessions. +- **Paste chips** now display with the correct background in inline prompts and question inputs. +- **Pasted content chips** now behave consistently when editing answers in the question view. +- **Background task status** now shows only elapsed duration instead of absolute timestamps. +- **Session lists** no longer drop real sessions when the remote registry reports an outdated turn count of zero. +- **/loop** now stores prompts that include stop conditions so recurring tasks can terminate themselves when done. +- **Reduced spurious warning messages** for common auth and config scenarios. +- **Fixed conda activation** (and other sourced scripts that read $@) when using persistent or login-capture shells. +- **Fixed stuck background-task tray rows** after long foreground shell commands complete. +- **Agent subprocesses and idle inhibitors** are now cleaned up when the parent CLI process dies unexpectedly. +- **Fixed truncated plans** in minimal mode and improved visual separation between reasoning and output (including NO_COLOR). +- **Fixed credential loss** across multiple grok processes sharing the same auth file. +- **Fixed doubled Enter** and other keys on older Alacritty terminals. +- **Fixed false paywall** messages for free-tier and unmatched users. + +## Performance + +- **Cold start** shows the UI instantly while models and settings load in the background. +- **Large session forks and resumes** now use far less memory and avoid spikes. +- **Prevented thread exhaustion** on high-core shared machines by limiting the workspace daemon's worker threads. + + # 0.2.112 — 2026-07-24 ## Breaking Changes diff --git a/crates/codegen/xai-grok-shell/Cargo.toml b/crates/codegen/xai-grok-shell/Cargo.toml index ab3dcc9..b6fa97f 100644 --- a/crates/codegen/xai-grok-shell/Cargo.toml +++ b/crates/codegen/xai-grok-shell/Cargo.toml @@ -1,7 +1,7 @@ [package] license = "Apache-2.0" name = "xai-grok-shell" -version = "0.2.112" +version = "0.2.114" edition.workspace = true [features] @@ -222,7 +222,10 @@ name = "fork_copy" harness = false required-features = ["test-support"] -# Consume `session::testkit`, so they need the gate (on by default under Bazel). +[[test]] +name = "test_leader_soak" +required-features = ["test-support"] + [[test]] name = "test_session_load_memory" required-features = ["test-support"] diff --git a/crates/codegen/xai-grok-shell/README.md b/crates/codegen/xai-grok-shell/README.md index 0147365..00f22b8 100644 --- a/crates/codegen/xai-grok-shell/README.md +++ b/crates/codegen/xai-grok-shell/README.md @@ -232,7 +232,9 @@ export GROK_AUTH_TOKEN_TTL=3600 # optional If your binary outputs a bare token string (not JSON with `expires_in`), set `auth_token_ttl` to the token's expected lifetime in seconds. Without it, Grok cannot detect expiry proactively and will only refresh after a 401. -The command is run via `sh -c`, so it can be a binary path, a shell script, or a pipeline. +The command runs through the platform shell — `sh -c` on macOS/Linux, `cmd /C` on Windows — so it can be a binary path, a script, or a pipeline. + +> **Windows:** write the path as a TOML *literal* string (single quotes) so backslashes survive: `auth_provider_command = 'C:\corp\grok-auth.exe'`. Inside a double-quoted TOML string `\t`, `\n`, `\r`, `\b` and `\f` are escape sequences, so `"C:\temp\auth.exe"` parses into a path containing a tab character and the provider fails to start — after which Grok falls back to browser login as if the setting were ignored. When `auth_provider_label` is set, the TUI welcome screen shows **"Login with Acme Corp"** instead of "Login with grok.com". In headless mode (`grok -p`), the label has no effect — stderr from your binary is printed directly to the terminal. diff --git a/crates/codegen/xai-grok-shell/changelogs/0.2.113.json b/crates/codegen/xai-grok-shell/changelogs/0.2.113.json new file mode 100644 index 0000000..7c39d37 --- /dev/null +++ b/crates/codegen/xai-grok-shell/changelogs/0.2.113.json @@ -0,0 +1,117 @@ +[ + { + "category": "performance", + "description": "**Cold start** shows the UI instantly while models and settings load in the background.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Terminal command output** is no longer lost or duplicated when the gateway is unreachable.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Invalid MCP server entries** in config.toml no longer prevent Grok from starting; problems are shown in `grok inspect`.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**SessionEnd hooks** now run on exit in non-leader TUI and headless sessions.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Paste chips** now display with the correct background in inline prompts and question inputs.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Pasted content chips** now behave consistently when editing answers in the question view.", + "breaking_change": false + }, + { + "category": "performance", + "description": "**Large session forks and resumes** now use far less memory and avoid spikes.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Background task status** now shows only elapsed duration instead of absolute timestamps.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Session lists** no longer drop real sessions when the remote registry reports an outdated turn count of zero.", + "breaking_change": false + }, + { + "category": "features", + "description": "**MCP servers** can now be enabled or disabled directly from the CLI with `grok mcp enable ` and `grok mcp disable `.", + "breaking_change": false + }, + { + "category": "features", + "description": "**Full plan markdown** can now be copied to the clipboard with `y` during plan approval or preview.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**/loop** now stores prompts that include stop conditions so recurring tasks can terminate themselves when done.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Reduced spurious warning messages** for common auth and config scenarios.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Fixed conda activation** (and other sourced scripts that read $@) when using persistent or login-capture shells.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Fixed stuck background-task tray rows** after long foreground shell commands complete.", + "breaking_change": false + }, + { + "category": "performance", + "description": "**Prevented thread exhaustion** on high-core shared machines by limiting the workspace daemon's worker threads.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Agent subprocesses and idle inhibitors** are now cleaned up when the parent CLI process dies unexpectedly.", + "breaking_change": false + }, + { + "category": "features", + "description": "**Added support for the new SuperGrok Plus subscription tier** in authentication and feature gating.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Fixed truncated plans** in minimal mode and improved visual separation between reasoning and output (including NO_COLOR).", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Fixed credential loss** across multiple grok processes sharing the same auth file.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Fixed doubled Enter** and other keys on older Alacritty terminals.", + "breaking_change": false + }, + { + "category": "features", + "description": "**Enabled automatic recovery** from repetitive loops in model output by default.", + "breaking_change": false + }, + { + "category": "fixes", + "description": "**Fixed false paywall** messages for free-tier and unmatched users.", + "breaking_change": false + } +] diff --git a/crates/codegen/xai-grok-shell/changelogs/0.2.113.md b/crates/codegen/xai-grok-shell/changelogs/0.2.113.md new file mode 100644 index 0000000..255bf40 --- /dev/null +++ b/crates/codegen/xai-grok-shell/changelogs/0.2.113.md @@ -0,0 +1,34 @@ +# 0.2.113 — 2026-07-28 + +## Features + +- **MCP servers** can now be enabled or disabled directly from the CLI with `grok mcp enable ` and `grok mcp disable `. +- **Full plan markdown** can now be copied to the clipboard with `y` during plan approval or preview. +- **Added support for the new SuperGrok Plus subscription tier** in authentication and feature gating. +- **Enabled automatic recovery** from repetitive loops in model output by default. + +## Bug Fixes + +- **Terminal command output** is no longer lost or duplicated when the gateway is unreachable. +- **Invalid MCP server entries** in config.toml no longer prevent Grok from starting; problems are shown in `grok inspect`. +- **SessionEnd hooks** now run on exit in non-leader TUI and headless sessions. +- **Paste chips** now display with the correct background in inline prompts and question inputs. +- **Pasted content chips** now behave consistently when editing answers in the question view. +- **Background task status** now shows only elapsed duration instead of absolute timestamps. +- **Session lists** no longer drop real sessions when the remote registry reports an outdated turn count of zero. +- **/loop** now stores prompts that include stop conditions so recurring tasks can terminate themselves when done. +- **Reduced spurious warning messages** for common auth and config scenarios. +- **Fixed conda activation** (and other sourced scripts that read $@) when using persistent or login-capture shells. +- **Fixed stuck background-task tray rows** after long foreground shell commands complete. +- **Agent subprocesses and idle inhibitors** are now cleaned up when the parent CLI process dies unexpectedly. +- **Fixed truncated plans** in minimal mode and improved visual separation between reasoning and output (including NO_COLOR). +- **Fixed credential loss** across multiple grok processes sharing the same auth file. +- **Fixed doubled Enter** and other keys on older Alacritty terminals. +- **Fixed false paywall** messages for free-tier and unmatched users. + +## Performance + +- **Cold start** shows the UI instantly while models and settings load in the background. +- **Large session forks and resumes** now use far less memory and avoid spikes. +- **Prevented thread exhaustion** on high-core shared machines by limiting the workspace daemon's worker threads. + diff --git a/crates/codegen/xai-grok-shell/changelogs/0.2.114.json b/crates/codegen/xai-grok-shell/changelogs/0.2.114.json new file mode 100644 index 0000000..aba581d --- /dev/null +++ b/crates/codegen/xai-grok-shell/changelogs/0.2.114.json @@ -0,0 +1,12 @@ +[ + { + "category": "fixes", + "description": "**Grok** no longer crashes on startup when the host machine has no free threads.", + "breaking_change": false + }, + { + "category": "features", + "description": "**New `/delete` slash command** removes the current session's history after confirmation.", + "breaking_change": false + } +] diff --git a/crates/codegen/xai-grok-shell/changelogs/0.2.114.md b/crates/codegen/xai-grok-shell/changelogs/0.2.114.md new file mode 100644 index 0000000..9b5c65e --- /dev/null +++ b/crates/codegen/xai-grok-shell/changelogs/0.2.114.md @@ -0,0 +1,10 @@ +# 0.2.114 — 2026-07-29 + +## Features + +- **New `/delete` slash command** removes the current session's history after confirmation. + +## Bug Fixes + +- **Grok** no longer crashes on startup when the host machine has no free threads. + diff --git a/crates/codegen/xai-grok-shell/src/agent/app.rs b/crates/codegen/xai-grok-shell/src/agent/app.rs index 24e4ee7..8ffa84c 100644 --- a/crates/codegen/xai-grok-shell/src/agent/app.rs +++ b/crates/codegen/xai-grok-shell/src/agent/app.rs @@ -294,6 +294,22 @@ pub async fn run_stdio_agent( memory_config: Option, ) -> anyhow::Result<()> { register_fs_watch_runtime(); + // A stdio agent is a protocol child speaking over pipes inherited from + // whoever spawned it (grok-desktop, IDE clients, the agent SDKs, a parent + // agent's subagent harness) — it is useless without that parent. stdin + // EOF already triggers shutdown below, but an agent wedged mid-turn (or + // under thread exhaustion) may never read stdin again; bind to parent + // death (Linux `PR_SET_PDEATHSIG(SIGTERM)`, no-op elsewhere) so the + // kernel reaps it instead of leaving an orphan accumulating pid slots on + // shared hosts. The leader entrypoint intentionally does NOT do this — + // it is designed to outlive its clients. + if let Err(error) = xai_tty_utils::kill_current_process_on_parent_death() { + tracing::warn!( + %error, + "failed to bind to parent death; agent will not die with its \ + parent — stdin EOF remains the only cleanup" + ); + } // Stamp binary version into unified log entries so zombie processes // are identifiable by version in diagnostic logs. xai_grok_telemetry::unified_log::set_version(xai_grok_version::VERSION); diff --git a/crates/codegen/xai-grok-shell/src/agent/config.rs b/crates/codegen/xai-grok-shell/src/agent/config.rs index d92cc00..6e92cee 100644 --- a/crates/codegen/xai-grok-shell/src/agent/config.rs +++ b/crates/codegen/xai-grok-shell/src/agent/config.rs @@ -2464,10 +2464,11 @@ impl Config { /// remote settings `doom_loop_recovery` object (a partial remote object only /// overrides the fields it sets). Gate precedence: env /// `GROK_DOOM_LOOP_RECOVERY` > TOML `enabled` > remote `enabled` > - /// default off — `None` IS the off state, so disabled has exactly one - /// spelling. Tunables have no env layer (TOML > remote > default) and - /// are clamped to their documented ranges. Returns the composite runtime - /// policy rather than `Resolved` because each knob resolves from its own + /// default ON — each layer's `false` is an independent kill switch, and + /// `None` IS the off state, so disabled has exactly one spelling. + /// Tunables have no env layer (TOML > remote > default) and are clamped + /// to their documented ranges. Returns the composite runtime policy + /// rather than `Resolved` because each knob resolves from its own /// source (the `resolve_reminder_policy` pattern). pub(crate) fn resolve_doom_loop_recovery( &self, @@ -2480,7 +2481,7 @@ impl Config { let enabled = BoolFlag::env("GROK_DOOM_LOOP_RECOVERY") .config(self.doom_loop_recovery.enabled) .feature_flag(remote.and_then(|s| s.enabled)) - .default(false) + .default(true) .resolve() .value; enabled.then(|| Policy { @@ -9052,8 +9053,9 @@ reasoning_effort = "low" unsafe { std::env::remove_var("GROK_TWO_PASS_COMPACTION") }; } /// Gate precedence: env > `[doom_loop_recovery]` > remote settings > - /// default(off), with the remote layer merged PER-FIELD from the nested - /// `doom_loop_recovery` object. One test covers the full ladder (the + /// default(ON), with the remote layer merged PER-FIELD from the nested + /// `doom_loop_recovery` object and each layer's `false` an independent + /// kill switch. One test covers the full ladder (the /// `resolve_two_pass_compaction_precedence` pattern). #[test] #[serial] @@ -9061,10 +9063,42 @@ reasoning_effort = "low" use crate::util::config::DoomLoopRecoverySettings; unsafe { std::env::remove_var("GROK_DOOM_LOOP_RECOVERY") }; let default_cfg = Config::default(); + let p = default_cfg + .resolve_doom_loop_recovery() + .expect("default is ON"); + assert_eq!(p.max_threshold, 8, "default tunables unchanged"); + assert_eq!(p.max_retries, 2, "default tunables unchanged"); + let toml_off = Config { + doom_loop_recovery: DoomLoopRecoverySettings { + enabled: Some(false), + ..Default::default() + }, + ..Default::default() + }; + assert!( + toml_off.resolve_doom_loop_recovery().is_none(), + "TOML kill switch" + ); + let remote_off = Config { + remote_settings: Some(crate::util::config::RemoteSettings { + doom_loop_recovery: Some(DoomLoopRecoverySettings { + enabled: Some(false), + ..Default::default() + }), + ..Default::default() + }), + ..Default::default() + }; + assert!( + remote_off.resolve_doom_loop_recovery().is_none(), + "remote settings kill switch" + ); + unsafe { std::env::set_var("GROK_DOOM_LOOP_RECOVERY", "0") }; assert!( default_cfg.resolve_doom_loop_recovery().is_none(), - "default is opt-in off" + "env kill switch" ); + unsafe { std::env::remove_var("GROK_DOOM_LOOP_RECOVERY") }; let remote_on = Config { remote_settings: Some(crate::util::config::RemoteSettings { doom_loop_recovery: Some(DoomLoopRecoverySettings { @@ -9080,10 +9114,6 @@ reasoning_effort = "low" assert_eq!(p.max_threshold, 16); assert_eq!(p.max_retries, 1); let partial_remote = Config { - doom_loop_recovery: DoomLoopRecoverySettings { - enabled: Some(true), - ..Default::default() - }, remote_settings: Some(crate::util::config::RemoteSettings { doom_loop_recovery: Some(DoomLoopRecoverySettings { max_threshold: Some(16), @@ -9095,7 +9125,7 @@ reasoning_effort = "low" }; let p = partial_remote .resolve_doom_loop_recovery() - .expect("gate from TOML despite remote object omitting enabled"); + .expect("default-on gate despite remote object omitting enabled"); assert_eq!(p.max_threshold, 16, "remote tunable applies"); assert_eq!(p.max_retries, 2, "unset field falls to the default"); let config_over_remote = Config { 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 8bde520..f2e7779 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 @@ -134,6 +134,7 @@ pub(crate) fn jwt_tier_claim(jwt: &str) -> Option { 4 => "x_premium_plus", 5 => "supergrok_heavy", 6 => "supergrok_lite", + 7 => "supergrok_plus", 0 => "free", _ => return Some(tier.to_string()), } @@ -178,6 +179,7 @@ pub(crate) fn jwt_claim_matches_user_subscription_tier( "XPremiumPlus" => jwt_claim == "x_premium_plus", "SuperGrokPro" => jwt_claim == "supergrok_heavy", "SuperGrokLite" => jwt_claim == "supergrok_lite", + "SuperGrokPlus" => jwt_claim == "supergrok_plus", _ => jwt_claim.parse::().is_ok_and(|n| n != 0), } } @@ -1777,10 +1779,9 @@ impl MvpAgent { /// Check whether the user has access via remote settings `allow_access`. /// /// Non-xAI auth (API keys, enterprise) always passes. For xAI OAuth2 - /// users, reads `allow_access` from remote settings. When settings exist - /// but the field is absent/false, defaults to `false` (blocked); when - /// settings have not arrived yet (background fetch pending) the gate is - /// provisionally open and re-resolved on arrival. + /// users, reads `allow_access` from remote settings (explicit `false` + /// blocks; absent field fails open). When settings have not arrived yet + /// the gate is provisionally open and re-resolved on arrival. pub(super) async fn enforce_grok_code_access(&self, auth: &crate::auth::GrokAuth) { if !auth.is_xai_auth() { self.tier_allowed.set(true); @@ -2005,18 +2006,7 @@ impl MvpAgent { .auth_manager .current() .map(|auth| { - let gate = if !self.tier_allowed.get() && gate.is_none() { - let message = "A subscription is required.".to_string(); - Some(crate::auth::GateInfo { - message, - url: Some( - "https://grok.com/supergrok?referrer=grok-build".to_string(), - ), - label: Some("Subscribe".to_string()), - }) - } else { - gate - }; + let gate = if self.tier_allowed.get() { None } else { gate }; let auth_meta = crate::auth::AuthMeta { email: auth.email.clone(), auth_mode: Some(format!("{:?}", auth.auth_mode)), @@ -2636,19 +2626,11 @@ fn spawn_post_unblock_jwt_and_catalog_retry( } }); } -/// Resolve `allow_access` from remote settings. -/// -/// Returns `true` only when remote settings explicitly set `allow_access: true`. -/// Defaults to `false` (blocked) when settings are `None` or the field is -/// absent — matching the `grok_build_access_gate` flag's server-side default. -/// -/// Used by both `enforce_grok_code_access` (initial login gate) and -/// `retry_subscription_check` (poller gate lift) to keep the decision in -/// one place. +/// `allow_access` from remote settings. Fail-open unless explicitly `false`. pub(crate) fn settings_allow_access( rs: Option<&crate::util::config::RemoteSettings>, ) -> bool { - rs.and_then(|s| s.allow_access).unwrap_or(false) + !matches!(rs.and_then(|s| s.allow_access), Some(false)) } #[cfg(test)] mod tests; diff --git a/crates/codegen/xai-grok-shell/src/agent/mvp_agent/session_lifecycle.rs b/crates/codegen/xai-grok-shell/src/agent/mvp_agent/session_lifecycle.rs index e141374..3e58a74 100644 --- a/crates/codegen/xai-grok-shell/src/agent/mvp_agent/session_lifecycle.rs +++ b/crates/codegen/xai-grok-shell/src/agent/mvp_agent/session_lifecycle.rs @@ -9,6 +9,30 @@ impl MvpAgent { let _ = handle.cmd_tx.send(SessionCommand::Shutdown); } } + /// Hard-stop a live session before wiping its history. + /// + /// Cancels the turn (subagents + background tasks), shuts the actor down, + /// reaps process scope, then waits briefly for flush so delete can remove + /// the session directory without the actor rewriting it. + pub(crate) async fn teardown_live_session_before_delete(&self, id: &acp::SessionId) { + let Some(handle) = self.sessions.borrow().get(id).cloned() else { + return; + }; + let _ = handle.cmd_tx.send(SessionCommand::Cancel { + cancel_subagents: true, + kill_background_tasks: true, + rewind_if_pristine: false, + trigger: Some("session_delete".into()), + }); + let _ = handle.cmd_tx.send(SessionCommand::Shutdown); + drop(handle); + let thread = self.session_threads.borrow_mut().remove(id); + self.remove_session_terminal(id, SessionLiveState::Completed); + if let Some(thread) = thread { + self.session_threads.borrow_mut().insert(id.clone(), thread); + self.drain_old_session_thread(id).await; + } + } /// Finalize the cloud session replica (fire-and-forget, "Hook 4"). /// /// Marks the session **done** upstream, so this MUST only run on a genuine @@ -438,9 +462,10 @@ impl MvpAgent { ) .registry_counts() .await; - let (session_index_claims, require_gateway_sessions) = { + let (resident_resources, session_index_claims, require_gateway_sessions) = { let resident = self.resident_resources.borrow(); ( + resident.len(), resident .values() .filter(|r| r.codebase_index.is_some()) @@ -449,6 +474,7 @@ impl MvpAgent { ) }; let retained = self.retained_resources.borrow(); + let retained_resources = retained.len(); let dispatch_locks = retained .values() .filter(|d| d.dispatch_lock.is_some()) @@ -465,6 +491,8 @@ impl MvpAgent { RegistrySnapshot { sessions: self.sessions.borrow().len(), session_threads: self.session_threads.borrow().len(), + resident_resources, + retained_resources, dispatch_locks, session_turn_numbers, permission_event_receivers, @@ -489,6 +517,8 @@ impl MvpAgent { pub struct RegistrySnapshot { pub sessions: usize, pub session_threads: usize, + pub resident_resources: usize, + pub retained_resources: usize, pub dispatch_locks: usize, pub session_turn_numbers: usize, pub permission_event_receivers: usize, 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 c3a7083..78da8d8 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 @@ -34,6 +34,10 @@ fn jwt_tier_claim_maps_free_and_paid() { jwt_tier_claim(&jwt_with_tier(6)).as_deref(), Some("supergrok_lite") ); + assert_eq!( + jwt_tier_claim(&jwt_with_tier(7)).as_deref(), + Some("supergrok_plus") + ); assert_eq!(jwt_tier_claim(&jwt_with_tier(9)).as_deref(), Some("9")); assert_eq!(jwt_tier_claim(&jwt_with_tier(99)).as_deref(), Some("99")); } @@ -102,6 +106,7 @@ fn jwt_claim_matches_user_subscription_tier_known_pairs() { ("supergrok_heavy", "SuperGrokPro"), ("9", "EnterpriseMystery"), ("supergrok_lite", "SuperGrokLite"), + ("supergrok_plus", "SuperGrokPlus"), ]; for (claim, user_tier) in cases { assert!( @@ -120,6 +125,14 @@ fn jwt_claim_matches_user_subscription_tier_rejects_stale_and_unknown() { "supergrok", "SuperGrokPro" )); + assert!(!jwt_claim_matches_user_subscription_tier( + "supergrok", + "SuperGrokPlus" + )); + assert!(!jwt_claim_matches_user_subscription_tier( + "supergrok_heavy", + "SuperGrokPlus" + )); assert!(!jwt_claim_matches_user_subscription_tier("free", "GrokPro")); assert!(!jwt_claim_matches_user_subscription_tier("", "XPremium")); assert!(!jwt_claim_matches_user_subscription_tier( @@ -360,12 +373,10 @@ fn trace_turn_to_i32_saturates_at_max() { let result = i32::try_from(boundary).unwrap_or(i32::MAX); assert_eq!(result, i32::MAX); } -/// When remote settings are absent (`None`), default to blocked. #[test] -fn settings_allow_access_none_settings_is_blocked() { - assert!(!settings_allow_access(None)); +fn settings_allow_access_none_settings_is_allowed() { + assert!(settings_allow_access(None)); } -/// When `allow_access` is `Some(true)`, user is allowed. #[test] fn settings_allow_access_true_is_allowed() { let rs = crate::util::config::RemoteSettings { @@ -374,10 +385,6 @@ fn settings_allow_access_true_is_allowed() { }; assert!(settings_allow_access(Some(&rs))); } -/// When `allow_access` is `Some(false)` (remote settings default / rule -/// disabled), user stays blocked — even if they hold a qualifying -/// subscription. This is the regression guard for the bug where -/// `retry_subscription_check` unconditionally lifted the gate. #[test] fn settings_allow_access_false_is_blocked() { let rs = crate::util::config::RemoteSettings { @@ -386,15 +393,13 @@ fn settings_allow_access_false_is_blocked() { }; assert!(!settings_allow_access(Some(&rs))); } -/// When `/settings` returned successfully but the field is absent -/// (`None`), default to blocked (conservative). #[test] -fn settings_allow_access_field_absent_is_blocked() { +fn settings_allow_access_field_absent_is_allowed() { let rs = crate::util::config::RemoteSettings { allow_access: None, ..Default::default() }; - assert!(!settings_allow_access(Some(&rs))); + assert!(settings_allow_access(Some(&rs))); } /// After allocating a turn number, the retained (in-memory) turn counter holds /// the next value (current + 1). This is the value that must be persisted via diff --git a/crates/codegen/xai-grok-shell/src/agent/subscription_check.rs b/crates/codegen/xai-grok-shell/src/agent/subscription_check.rs index 2938cbd..5210b75 100644 --- a/crates/codegen/xai-grok-shell/src/agent/subscription_check.rs +++ b/crates/codegen/xai-grok-shell/src/agent/subscription_check.rs @@ -147,6 +147,7 @@ mod tests { fn all_paid_tiers_qualify() { for tier in &[ "SuperGrokPro", + "SuperGrokPlus", "GrokPro", "SuperGrokLite", "XPremiumPlus", diff --git a/crates/codegen/xai-grok-shell/src/auth/auth_provider.rs b/crates/codegen/xai-grok-shell/src/auth/auth_provider.rs index c1b2302..24d0e3a 100644 --- a/crates/codegen/xai-grok-shell/src/auth/auth_provider.rs +++ b/crates/codegen/xai-grok-shell/src/auth/auth_provider.rs @@ -383,15 +383,7 @@ async fn mint_provider_token( cmd.args(args); cmd } - None => { - #[cfg(windows)] - let (shell, flag) = ("cmd", "/C"); - #[cfg(not(windows))] - let (shell, flag) = ("sh", "-c"); - let mut cmd = tokio::process::Command::new(shell); - cmd.args([flag, config.command.as_str()]); - cmd - } + None => crate::util::subprocess::shell_c(config.command.as_str()), }; if let Some(ref dir) = cwd { cmd.current_dir(dir); diff --git a/crates/codegen/xai-grok-shell/src/auth/external_auth.rs b/crates/codegen/xai-grok-shell/src/auth/external_auth.rs index 8f37424..02b7db6 100644 --- a/crates/codegen/xai-grok-shell/src/auth/external_auth.rs +++ b/crates/codegen/xai-grok-shell/src/auth/external_auth.rs @@ -5,7 +5,7 @@ use crate::util::subprocess::CommandLog; use crate::util::subprocess::RunError; use crate::util::subprocess::RunOptions; use crate::util::subprocess::run_detached_with_timeout; -use crate::util::subprocess::sh_c; +use crate::util::subprocess::shell_c; use std::time::Duration; /// Parse stdout into a session-credential `GrokAuth`. @@ -48,7 +48,7 @@ const EXTERNAL_AUTH_REFRESH_TIMEOUT: Duration = Duration::from_secs(5); pub(crate) async fn run_external_refresh(command: &str) -> Option { tracing::info!(cmd = %command, timeout_secs = EXTERNAL_AUTH_REFRESH_TIMEOUT.as_secs(), "auth: running external auth provider (headless refresh)"); - let mut cmd = sh_c(command); + let mut cmd = shell_c(command); cmd.env("GROK_AUTH_EXPIRED", "1"); // Route through the group-killing runner so a provider that spawns helpers // is torn down as a unit on timeout. diff --git a/crates/codegen/xai-grok-shell/src/auth/flow.rs b/crates/codegen/xai-grok-shell/src/auth/flow.rs index 38c75cb..b59c13c 100644 --- a/crates/codegen/xai-grok-shell/src/auth/flow.rs +++ b/crates/codegen/xai-grok-shell/src/auth/flow.rs @@ -215,12 +215,14 @@ async fn run_external_auth_provider( "auth: running external auth provider" ); - let mut cmd = tokio::process::Command::new("sh"); - cmd.args(["-c", command]) - .stdin(std::process::Stdio::null()) + // `sh -c` on unix, `cmd /C` on Windows — a hardcoded `sh` cannot spawn on a + // default Windows install, and the spawn failure fell through to the + // built-in browser login instead of honoring `auth_provider_command`. + let mut cmd = crate::util::subprocess::shell_c(command); + cmd.stdin(std::process::Stdio::null()) .stdout(std::process::Stdio::piped()) .kill_on_drop(true); - // TODO: `kill_on_drop` SIGKILLs only the direct `sh` child; a provider that + // TODO: `kill_on_drop` SIGKILLs only the direct shell child; a provider that // backgrounds work (setsid / `&`) leaks the grandchild on shutdown-cancel. // Proper fix: pgid-kill via xai-tty-utils. diff --git a/crates/codegen/xai-grok-shell/src/auth/manager.rs b/crates/codegen/xai-grok-shell/src/auth/manager.rs index cd6d782..4dfeb4c 100644 --- a/crates/codegen/xai-grok-shell/src/auth/manager.rs +++ b/crates/codegen/xai-grok-shell/src/auth/manager.rs @@ -1072,27 +1072,49 @@ impl AuthManager { Some(auth) } - /// `true` when a sibling process has rotated the refresh token on - /// disk (disk RT differs from in-memory RT). Used by `refresh_chain` - /// to demote a `PermanentFailure` to transient so the sibling's - /// fresher token can be tried on the next attempt. + /// `true` when the refresh token on disk is present and differs from the + /// one we actually spent — i.e. a sibling process rotated the RT while our + /// exchange was in flight, so the rejection we just got is a lost race + /// rather than a revoked session. + /// + /// The single definition of "disk moved past the token we spent". Two + /// hand-rolled copies of this comparison is how the wrong one survived + /// long enough to log a dozen processes out at once. + /// + /// Takes an already-observed `disk_rt` rather than reading `auth.json` + /// itself, so one observation drives the decision, the unattributed + /// fallback, and the telemetry that explains them. A second read can catch + /// a *later* sibling write and produce a record that contradicts the + /// branch it documents — in the log whose whole purpose is post-incident + /// truth. Callers read under the auth file lock, so the observation + /// includes the sibling's committed write. + /// + /// Disk holding no RT is *not* divergence: there is no successor to fall + /// back to, so the rejection must be honored. + fn refresh_token_superseded(disk_rt: Option<&str>, spent_rt: &str) -> bool { + disk_rt.is_some_and(|disk_rt| disk_rt != spent_rt) + } + + /// `true` when a sibling process has rotated the refresh token on disk + /// past the one in memory. Used by `refresh_chain` to demote a + /// `PermanentFailure` to transient so the sibling's fresher token can be + /// tried on the next attempt. /// /// Requires an in-memory RT: empty `inner` means the disk credential is /// the only candidate (not a multi-process rotation). Does **not** /// require a non-expired disk AT — a sibling may still hold a usable RT /// while its AT is buffer/hard-expired. - fn sibling_has_different_refresh_token(&self) -> bool { - let disk_auth = self.read_disk_auth(); - let Some(ref disk) = disk_auth else { - return false; - }; - let Some(disk_rt) = disk.refresh_token.as_deref() else { - return false; - }; - let Some(mem_rt) = self.current_or_expired().and_then(|a| a.refresh_token) else { - return false; - }; - mem_rt.as_str() != disk_rt + /// + /// Only a fallback for authorities that cannot report which RT they spent. + /// Attributed refreshers pass the token they actually sent to + /// [`Self::refresh_token_superseded`] directly; because + /// `resolve_refresh_credential` is disk-first, the RT actually sent is + /// usually the disk one, and comparing disk against *memory* then answers + /// `false` in precisely the case that needs the demotion. + fn sibling_has_different_refresh_token(&self, disk_rt: Option<&str>) -> bool { + self.current_or_expired() + .and_then(|a| a.refresh_token) + .is_some_and(|mem_rt| Self::refresh_token_superseded(disk_rt, &mem_rt)) } /// Re-read `auth.json` from disk without updating in-memory state. @@ -1815,7 +1837,11 @@ impl AuthManager { Err(AuthError::transient_source(e)) } }, - RefreshOutcome::PermanentFailure { error, tried_key } => { + RefreshOutcome::PermanentFailure { + error, + tried_key, + tried_refresh_token, + } => { tracing::warn!(reason = ?error.reason, "auth.refresh.permanent_failure"); xai_grok_telemetry::unified_log::warn( "auth.refresh.permanent_failure", @@ -1843,10 +1869,42 @@ impl AuthManager { if is_rtr { let mem = self.current_or_expired(); let disk = self.read_disk_auth(); - // Unattributed + diverging RTs: demote without recording so - // the next attempt can try the other side (no sticky lockout). - if tried_key.is_none() && self.sibling_has_different_refresh_token() { + // Diverging RTs mean a sibling rotated while we were in + // flight: our RT was superseded, not revoked. Demote + // without recording so the next attempt picks up the + // sibling's token (no sticky lockout, no credential loss). + // + // When the refresher told us which RT it spent (every + // in-tree OIDC path), compare disk against *that*. The + // legacy disk-vs-memory heuristic is only a fallback for + // unattributed authorities: it asks the wrong question, + // because `resolve_refresh_credential` is disk-first, so + // the RT actually spent is usually the disk one and the + // comparison collapses to "false" exactly when it matters. + // + // Both arms and the log below read one `disk` observation. + // Re-reading per use lets the decision and the line that + // explains it disagree about what disk held. + let disk_rt = disk.as_ref().and_then(|d| d.refresh_token.as_deref()); + let sibling_rotated = match tried_refresh_token.as_deref() { + Some(tried_rt) => Self::refresh_token_superseded(disk_rt, tried_rt), + None => { + tried_key.is_none() && self.sibling_has_different_refresh_token(disk_rt) + } + }; + if sibling_rotated { tracing::info!("auth: sibling-rotation detected; demoting to transient"); + xai_grok_telemetry::unified_log::info( + "auth.refresh.sibling_rotation_demoted", + None, + Some(serde_json::json!({ + "reason": format!("{failed_reason:?}"), + "tried_rt_prefix": tried_refresh_token + .as_deref() + .map(token_suffix), + "disk_rt_prefix": disk_rt.map(token_suffix), + })), + ); return Err(AuthError::transient(format!( "sibling-rotation: {failed_reason:?}" ))); @@ -1912,7 +1970,13 @@ impl AuthManager { /// Re-read auth.json from disk and update the in-memory cache (used by the /// refresh chains). Non-destructive: only updates in-memory if disk has a /// different valid token (a sibling process wrote a fresher one). - pub(crate) fn pick_up_sibling_token(&self) { + /// + /// Returns `true` only when in-memory state was actually replaced, so + /// callers can log adoption truthfully instead of inferring it from + /// "we have a token now" — which is also true when our own token was fine + /// all along, and made the proactive-refresh log actively misleading when + /// reconstructing a rotation chain after an incident. + pub(crate) fn pick_up_sibling_token(&self) -> bool { let auth = match read_auth_json(&self.path) { Ok(map) => lookup_auth(&map, &self.scope), _ => None, @@ -1932,7 +1996,9 @@ impl AuthManager { })), ); self.with_inner_write(|inner| *inner = Some(a.clone())); + return true; } + false } /// Check if a candidate auth has a different token than what's in memory. @@ -2227,7 +2293,7 @@ impl AuthManager { // already refreshed and wrote a valid token to disk. // Combined with jitter, the first process to wake // refreshes; later processes adopt the result here. - this.pick_up_sibling_token(); + let adopted_from_sibling = this.pick_up_sibling_token(); if this.current().is_some() { let adopted = this.current().map(|a| token_suffix(&a.key).to_owned()); let expires_at = this @@ -2235,14 +2301,25 @@ impl AuthManager { .read() .as_ref() .and_then(|a| a.expires_at.map(|e| e.to_rfc3339())); - tracing::info!( - "auth: proactive refresh skipped, adopted sibling token from disk" - ); + // Distinguish "a sibling's token replaced ours" from "our + // own token is still valid". Both skip the refresh, but + // conflating them makes the log actively misleading when + // reconstructing a rotation chain after an incident. + if adopted_from_sibling { + tracing::info!( + "auth: proactive refresh skipped, adopted sibling token from disk" + ); + } else { + tracing::info!( + "auth: proactive refresh skipped, in-memory token still valid" + ); + } xai_grok_telemetry::unified_log::info( - "auth: proactive refresh adopted sibling token", + "auth: proactive refresh skipped", None, Some(serde_json::json!({ - "adopted_key_prefix": adopted, + "adopted_from_sibling": adopted_from_sibling, + "key_prefix": adopted, "expires_at": expires_at, })), ); diff --git a/crates/codegen/xai-grok-shell/src/auth/manager_tests.rs b/crates/codegen/xai-grok-shell/src/auth/manager_tests.rs index 840b429..d04f768 100644 --- a/crates/codegen/xai-grok-shell/src/auth/manager_tests.rs +++ b/crates/codegen/xai-grok-shell/src/auth/manager_tests.rs @@ -1396,6 +1396,145 @@ async fn refresh_chain_demotes_when_disk_rt_differs_even_if_at_expired() { ); } +/// Regression test for the multi-process logout incident. +/// +/// This is the shape `OidcRefresher` actually emits in production: the tried +/// credential is **fully attributed** (`tried_key` *and* `tried_refresh_token` +/// are `Some`). The pre-existing demotion tests all built the outcome with +/// `tried_key = None` — the external-binary shape — so they passed while the +/// OIDC path was gated behind `tried_key.is_none()` and could never demote. +/// +/// Scenario: a sibling rotated the RT while our token exchange was in flight, +/// so the IdP rejected the RT we spent. That is a lost race, not a revoked +/// session: it must demote to transient and leave the sibling's credential on +/// disk untouched. +#[tokio::test] +async fn refresh_chain_demotes_when_attributed_tried_rt_differs_from_disk() { + let dir = tempfile::tempdir().unwrap(); + let cfg = GrokComConfig::default(); + let scope = cfg.auth_scope(); + let mgr = Arc::new(AuthManager::new(dir.path(), cfg)); + + // We hold, and spend, the predecessor RT. + let tried = GrokAuth { + key: "tried-key".into(), + auth_mode: AuthMode::Oidc, + refresh_token: Some("rt-spent".into()), + expires_at: Some(Utc::now() - Duration::hours(1)), + oidc_issuer: Some("https://issuer.example".into()), + oidc_client_id: Some("client-1".into()), + ..GrokAuth::test_default() + }; + mgr.hot_swap(tried.clone()); + + // A sibling already rotated: disk carries the successor RT. Its AT is + // expired too, so disk adoption cannot short-circuit the failure path — + // the demotion is the only thing standing between us and a wipe. + let sibling = GrokAuth { + key: "sibling-key".into(), + auth_mode: AuthMode::Oidc, + refresh_token: Some("rt-successor".into()), + expires_at: Some(Utc::now() - Duration::minutes(30)), + oidc_issuer: Some("https://issuer.example".into()), + oidc_client_id: Some("client-1".into()), + ..GrokAuth::test_default() + }; + let mut store = AuthStore::new(); + store.insert(scope, sibling); + write_auth_json(&dir.path().join("auth.json"), &store).unwrap(); + + struct AttributedRejection(GrokAuth); + #[async_trait::async_trait] + impl TokenRefresher for AttributedRejection { + async fn refresh( + &self, + _reason: crate::auth::manager::RefreshReason, + ) -> crate::auth::refresh::RefreshOutcome { + // Exactly what OidcRefresher builds on a 400 invalid_grant. + crate::auth::refresh::RefreshOutcome::permanent_for( + crate::auth::error::RefreshTokenFailedReason::RefreshTokenRejected, + &self.0, + ) + } + } + mgr.set_refresher(Arc::new(AttributedRejection(tried))); + + let err = mgr + .refresh_chain(TokenType::OidcSession, RefreshReason::PreRequest) + .await + .unwrap_err(); + + assert!( + matches!(err, AuthError::Refresh(RefreshTokenError::Transient(_))), + "a rejected RT that disk has already rotated past is a lost race, \ + not a revoked session; must demote to transient, got: {err:?}", + ); + assert_eq!( + mgr.read_disk_auth().and_then(|a| a.refresh_token), + Some("rt-successor".into()), + "the sibling's successor RT must survive our rejection", + ); + assert!( + mgr.permanent_failure().is_none(), + "demotion must not record a sticky verdict that locks out every \ + sibling process until the user re-runs `grok login`", + ); +} + +/// The demotion must *not* fire when disk still holds the very RT that was +/// just rejected: nobody rotated, the session really is dead, and holding on +/// to a known-revoked credential would loop forever. +#[tokio::test] +async fn refresh_chain_still_discards_when_attributed_tried_rt_matches_disk() { + let dir = tempfile::tempdir().unwrap(); + let cfg = GrokComConfig::default(); + let scope = cfg.auth_scope(); + let mgr = Arc::new(AuthManager::new(dir.path(), cfg)); + + let tried = GrokAuth { + key: "only-key".into(), + auth_mode: AuthMode::Oidc, + refresh_token: Some("rt-revoked".into()), + expires_at: Some(Utc::now() - Duration::hours(1)), + oidc_issuer: Some("https://issuer.example".into()), + oidc_client_id: Some("client-1".into()), + ..GrokAuth::test_default() + }; + mgr.hot_swap(tried.clone()); + let mut store = AuthStore::new(); + store.insert(scope, tried.clone()); + write_auth_json(&dir.path().join("auth.json"), &store).unwrap(); + + struct AttributedRejection(GrokAuth); + #[async_trait::async_trait] + impl TokenRefresher for AttributedRejection { + async fn refresh( + &self, + _reason: crate::auth::manager::RefreshReason, + ) -> crate::auth::refresh::RefreshOutcome { + crate::auth::refresh::RefreshOutcome::permanent_for( + crate::auth::error::RefreshTokenFailedReason::RefreshTokenRejected, + &self.0, + ) + } + } + mgr.set_refresher(Arc::new(AttributedRejection(tried))); + + let err = mgr + .refresh_chain(TokenType::OidcSession, RefreshReason::PreRequest) + .await + .unwrap_err(); + + assert!( + matches!(err, AuthError::Refresh(RefreshTokenError::Permanent(_))), + "an un-rotated rejected RT is a genuinely dead session, got: {err:?}", + ); + assert!( + mgr.permanent_failure().is_some(), + "a genuine revocation must still record a verdict", + ); +} + /// Disk-first invalid_grant must not wipe an untried in-memory successor RT /// (mem-ahead-of-disk after a failed persist of a successful rotation). #[tokio::test] @@ -2865,6 +3004,28 @@ async fn update_recovers_from_whitespace_only_auth_json() { assert!(on_disk.contains("ws-token"), "credential must be persisted"); } +// -- sibling-rotation comparison ------------------------------------------ + +/// The demotion — and therefore whether a dozen processes keep their +/// credentials — rests entirely on this comparison, so pin its three cases +/// directly rather than only through the refresh chain. +#[test] +fn refresh_token_superseded_needs_a_successor_on_disk() { + assert!( + AuthManager::refresh_token_superseded(Some("rt-successor"), "rt-spent"), + "a different RT on disk is a sibling's successor: demote" + ); + assert!( + !AuthManager::refresh_token_superseded(Some("rt-spent"), "rt-spent"), + "disk still holding the RT the IdP just rejected is a real revocation" + ); + assert!( + !AuthManager::refresh_token_superseded(None, "rt-spent"), + "no RT on disk means there is no successor to fall back to, so the \ + rejection must be honored rather than demoted into a retry loop" + ); +} + // -- sibling_has_different_refresh_token ---------------------------------- /// Expired disk AT with different RT is still treated as a sibling RT @@ -2897,8 +3058,9 @@ async fn sibling_different_rt_with_expired_at_is_still_sibling() { store.insert(cfg.auth_scope(), successor); write_auth_json(&dir.path().join("auth.json"), &store).unwrap(); + let disk_rt = mgr.read_disk_auth().and_then(|a| a.refresh_token); assert!( - mgr.sibling_has_different_refresh_token(), + mgr.sibling_has_different_refresh_token(disk_rt.as_deref()), "different disk RT must demote even when the sibling AT is expired" ); } @@ -2931,8 +3093,9 @@ async fn sibling_different_rt_with_valid_at_is_treated_as_live() { store.insert(cfg.auth_scope(), sibling); write_auth_json(&dir.path().join("auth.json"), &store).unwrap(); + let disk_rt = mgr.read_disk_auth().and_then(|a| a.refresh_token); assert!( - mgr.sibling_has_different_refresh_token(), + mgr.sibling_has_different_refresh_token(disk_rt.as_deref()), "valid disk token with different RT must be treated as live sibling" ); } diff --git a/crates/codegen/xai-grok-shell/src/auth/refresh/mod.rs b/crates/codegen/xai-grok-shell/src/auth/refresh/mod.rs index a735cba..3cfd03b 100644 --- a/crates/codegen/xai-grok-shell/src/auth/refresh/mod.rs +++ b/crates/codegen/xai-grok-shell/src/auth/refresh/mod.rs @@ -104,6 +104,16 @@ pub(crate) enum RefreshOutcome { /// has no token key (external binary flow); the caller falls back to /// its own resolution. tried_key: Option, + /// The **refresh token** actually spent at the IdP. `refresh_chain` + /// compares it against disk to tell "this session is revoked" apart + /// from "a sibling process rotated the RT out from under us" — the + /// latter must never discard credentials. + /// + /// `tried_key` cannot answer that question: it is the *access* token, + /// and a sibling's rotation changes the RT while the AT the loser + /// holds may be untouched. `None` when the authority does not expose + /// which RT it sent (external binary flow). + tried_refresh_token: Option, }, /// Transient / unknown failure. Caller may retry later. Message-only: the /// underlying cause is logged structurally at the refresher, then flattened @@ -119,6 +129,12 @@ impl RefreshOutcome { /// Terminal failure for an already-classified reason against the credential /// `tried_key` (the one actually sent to the IdP). + /// + /// Leaves the tried **refresh token** unattributed, which disables the + /// sibling-rotation check in `refresh_chain`. Only correct for authorities + /// that genuinely cannot report which RT they spent (the external-binary + /// flow). Any refresher holding the [`GrokAuth`] it sent must use + /// [`Self::permanent_for`] instead. pub(crate) fn permanent( reason: crate::auth::error::RefreshTokenFailedReason, tried_key: Option, @@ -126,6 +142,23 @@ impl RefreshOutcome { Self::PermanentFailure { error: reason.into(), tried_key, + tried_refresh_token: None, + } + } + + /// Terminal failure attributed to the exact credential sent to the IdP. + /// + /// Prefer this wherever the attempted [`GrokAuth`] is in hand: it captures + /// both the AT key (verdict scope) and the RT (sibling-rotation check), so + /// a lost rotation race cannot be mistaken for a revoked session. + pub(crate) fn permanent_for( + reason: crate::auth::error::RefreshTokenFailedReason, + tried: &GrokAuth, + ) -> Self { + Self::PermanentFailure { + error: reason.into(), + tried_key: Some(tried.key.clone()), + tried_refresh_token: tried.refresh_token.clone(), } } diff --git a/crates/codegen/xai-grok-shell/src/auth/refresh/oidc_refresher.rs b/crates/codegen/xai-grok-shell/src/auth/refresh/oidc_refresher.rs index 6f3463c..d3a4ca4 100644 --- a/crates/codegen/xai-grok-shell/src/auth/refresh/oidc_refresher.rs +++ b/crates/codegen/xai-grok-shell/src/auth/refresh/oidc_refresher.rs @@ -147,10 +147,7 @@ impl OidcRefresher { None, Some(serde_json::json!({ "reason": format!("{reason:?}") })), ); - Some(RefreshOutcome::permanent( - reason, - Some(disk_now.key.clone()), - )) + Some(RefreshOutcome::permanent_for(reason, &disk_now)) } OidcRefreshResult::Failed => { Some(RefreshOutcome::transient("OIDC disk-retry refresh failed")) @@ -245,7 +242,7 @@ impl TokenRefresher for OidcRefresher { &self.upload_in_flight, ); } - RefreshOutcome::permanent(reason, Some(auth.key.clone())) + RefreshOutcome::permanent_for(reason, &auth) } OidcRefreshResult::Failed => { tracing::warn!( diff --git a/crates/codegen/xai-grok-shell/src/auth/refresh/oidc_refresher_tests.rs b/crates/codegen/xai-grok-shell/src/auth/refresh/oidc_refresher_tests.rs index 0424ace..62380e1 100644 --- a/crates/codegen/xai-grok-shell/src/auth/refresh/oidc_refresher_tests.rs +++ b/crates/codegen/xai-grok-shell/src/auth/refresh/oidc_refresher_tests.rs @@ -251,6 +251,83 @@ async fn oidc_refresher_e2e_near_expiry_within_buffer_refreshes() { server.abort(); } +/// Contract: on `invalid_grant`, `OidcRefresher` must report **which refresh +/// token it spent**, not just the access-token key. +/// +/// `refresh_chain` uses `tried_refresh_token` to tell a lost rotation race +/// apart from a revoked session; an unattributed outcome silently disables +/// that check and turns any concurrent-refresh race into a machine-wide +/// logout. This is the shape assertion that the previous demotion tests +/// missed — they hand-built outcomes with `tried_key: None`, a shape this +/// refresher never emits, so they passed while production was unprotected. +#[tokio::test] +async fn oidc_refresher_attributes_the_refresh_token_it_spent_on_invalid_grant() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let base_url = format!("http://127.0.0.1:{}", listener.local_addr().unwrap().port()); + let base_for_discovery = base_url.clone(); + + let app = axum::Router::new() + .route( + "/.well-known/openid-configuration", + axum::routing::get(move || { + let b = base_for_discovery.clone(); + async move { + axum::Json(serde_json::json!({ + "authorization_endpoint": format!("{b}/authorize"), + "token_endpoint": format!("{b}/token"), + })) + } + }), + ) + .route( + "/token", + axum::routing::post(|| async { + ( + axum::http::StatusCode::BAD_REQUEST, + axum::Json(serde_json::json!({"error": "invalid_grant"})), + ) + }), + ); + let server = tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + + let dir = tempfile::tempdir().unwrap(); + let mgr = Arc::new( + AuthManager::new(dir.path(), GrokComConfig::default()).with_proxy_base_url(&base_url), + ); + mgr.hot_swap(GrokAuth { + key: "spent-access-token".into(), + user_id: "user-42".into(), + refresh_token: Some("rt-spent".into()), + expires_at: Some(Utc::now() - Duration::minutes(1)), + oidc_issuer: Some(base_url.clone()), + oidc_client_id: Some("test-client".into()), + ..GrokAuth::test_default() + }); + + let outcome = OidcRefresher::new(mgr.clone()) + .refresh(crate::auth::manager::RefreshReason::PreRequest) + .await; + + match outcome { + RefreshOutcome::PermanentFailure { + tried_key, + tried_refresh_token, + .. + } => { + assert_eq!( + tried_refresh_token.as_deref(), + Some("rt-spent"), + "the RT actually sent to the IdP must be reported so \ + refresh_chain can detect a sibling rotation", + ); + assert_eq!(tried_key.as_deref(), Some("spent-access-token")); + } + other => panic!("expected PermanentFailure, got: {other:?}"), + } + + server.abort(); +} + /// When the near-expiry token has a refresh_token but the IdP rejects /// the refresh (e.g. refresh_token revoked), silent refresh must fail. #[tokio::test] diff --git a/crates/codegen/xai-grok-shell/src/extensions/session_admin.rs b/crates/codegen/xai-grok-shell/src/extensions/session_admin.rs index 30bde0f..2ae1695 100644 --- a/crates/codegen/xai-grok-shell/src/extensions/session_admin.rs +++ b/crates/codegen/xai-grok-shell/src/extensions/session_admin.rs @@ -253,6 +253,13 @@ async fn handle_session_delete(agent: &MvpAgent, args: &acp::ExtRequest) -> ExtR let needs_remote = agent.is_writeback_storage() && agent.current_auth().is_some_and(|a| !a.is_zdr_team()); + // Tear down any live actor first (cancel turn/subagents/bg tasks, + // process-scope kill, flush). Then wipe history so shutdown cannot + // rewrite the session directory after delete. + if agent.sessions.borrow().contains_key(&session_id) { + agent.teardown_live_session_before_delete(&session_id).await; + } + // Shared delete: remote-first, then local disk + FTS eviction. // Mirrored by the `grok sessions delete ` CLI path. crate::session::persistence::delete_session_history( @@ -269,15 +276,6 @@ async fn handle_session_delete(agent: &MvpAgent, args: &acp::ExtRequest) -> ExtR acp::Error::internal_error().data(e.to_string()) })?; - // If an in-memory live session exists for this id (e.g. the user - // deleted history for a session that is still open in another agent - // or the current one), shut it down and drop the MvpAgent bookkeeping - // so we don't leave a live actor whose on-disk/FTS state is gone. - if agent.sessions.borrow().contains_key(&session_id) { - agent.request_session_shutdown(&session_id); - agent.remove_session(&session_id); - } - tracing::info!(session_id = %req.session_id, "Session deleted"); to_raw_response(&serde_json::json!({ "success": true })) diff --git a/crates/codegen/xai-grok-shell/src/leader/in_process.rs b/crates/codegen/xai-grok-shell/src/leader/in_process.rs new file mode 100644 index 0000000..ad64a44 --- /dev/null +++ b/crates/codegen/xai-grok-shell/src/leader/in_process.rs @@ -0,0 +1,77 @@ +//! A real agent behind a leader server, in this process rather than a child. + +use std::sync::Arc; + +use agent_client_protocol as acp; +use tokio::io::{AsyncBufReadExt as _, AsyncWriteExt as _, BufReader, simplex}; +use tokio::sync::mpsc::{UnboundedReceiver, UnboundedSender}; +use tokio::task::JoinHandle; +use tokio_util::compat::{TokioAsyncReadCompatExt as _, TokioAsyncWriteCompatExt as _}; +use xai_acp_lib::{ + AcpAgentGatewayReceiver as GatewayReceiver, AcpAgentGatewaySender as GatewaySender, + LineBufferedRead, +}; + +use crate::agent::config::Config as AgentConfig; +use crate::agent::mvp_agent::MvpAgent; + +const SIMPLEX_BUF: usize = 8 * 1024 * 1024; + +/// Spawns an agent on the current `LocalSet`, reading requests from `to_agent` +/// and writing responses to `from_agent`. Returns the task handles so a caller +/// can end the agent. Panics if the ambient configuration cannot build one. +pub fn spawn_agent( + mut to_agent: UnboundedReceiver, + from_agent: UnboundedSender, +) -> Vec> { + let (agent_in_read, mut agent_in_write) = simplex(SIMPLEX_BUF); + let (agent_out_read, agent_out_write) = simplex(SIMPLEX_BUF); + + let agent = tokio::task::spawn_local(async move { + let config = AgentConfig::default(); + let auth_manager = Arc::new(config.create_auth_manager()); + let (gateway_tx, gateway_rx) = tokio::sync::mpsc::unbounded_channel(); + let agent = MvpAgent::new(GatewaySender::new(gateway_tx), &config, auth_manager, None) + .expect("valid agent config"); + let incoming = LineBufferedRead::spawn_local(agent_in_read.compat()); + let (conn, handle_io) = + acp::AgentSideConnection::new(agent, agent_out_write.compat_write(), incoming, |fut| { + tokio::task::spawn_local(fut); + }); + tokio::task::spawn_local( + GatewayReceiver::new(gateway_rx, conn) + .with_on_meta(xai_file_utils::trace_context::span_from_meta_traceparent) + .run(), + ); + let _ = handle_io.await; + }); + + let requests = tokio::task::spawn_local(async move { + while let Some(msg) = to_agent.recv().await { + if agent_in_write.write_all(msg.as_bytes()).await.is_err() + || agent_in_write.write_all(b"\n").await.is_err() + { + break; + } + } + }); + + let responses = tokio::task::spawn_local(async move { + let mut reader = BufReader::new(agent_out_read); + let mut line = String::new(); + loop { + line.clear(); + match reader.read_line(&mut line).await { + Ok(0) | Err(_) => break, + Ok(_) => { + let msg = line.trim_end_matches(['\r', '\n']).to_string(); + if !msg.is_empty() { + let _ = from_agent.send(msg); + } + } + } + } + }); + + vec![agent, requests, responses] +} diff --git a/crates/codegen/xai-grok-shell/src/leader/mod.rs b/crates/codegen/xai-grok-shell/src/leader/mod.rs index c647595..f656584 100644 --- a/crates/codegen/xai-grok-shell/src/leader/mod.rs +++ b/crates/codegen/xai-grok-shell/src/leader/mod.rs @@ -51,6 +51,8 @@ //! } //! ``` mod client; +#[cfg(feature = "test-support")] +pub mod in_process; mod lock; pub mod protocol; mod server; 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 245418f..0d50ce2 100644 --- a/crates/codegen/xai-grok-shell/src/session/acp_session.rs +++ b/crates/codegen/xai-grok-shell/src/session/acp_session.rs @@ -1822,6 +1822,9 @@ mod build_tool_parse_error_message_tests; #[path = "acp_session_tests/cancel_running_task_tests.rs"] mod cancel_running_task_tests; #[cfg(test)] +#[path = "acp_session_tests/turn/chat_history_integrity_tests.rs"] +mod chat_history_integrity_tests; +#[cfg(test)] #[path = "acp_session_tests/feedback_turn_lookup_tests.rs"] mod feedback_turn_lookup_tests; #[cfg(test)] 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 901367d..17bf4b7 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 @@ -98,6 +98,7 @@ impl SessionActor { xai_grok_hooks::runner::RunContext { session_id: &self.session_info.id.0, workspace_root: &self.hook_resolved_workspace_root, + process_scope: self.tool_context.process_scope.clone(), } } 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 af15f99..fca3b06 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 @@ -698,6 +698,7 @@ pub(crate) async fn spawn_session_actor( std::sync::Arc::new(LocalTerminalBackend::new_local_with_persistent_shell( resolve_search_shadows(), resolve_policy(), + tool_context.process_scope.clone(), )) } TerminalBackendKind::LocalNonPersistent => { @@ -708,6 +709,7 @@ pub(crate) async fn spawn_session_actor( resolve_search_shadows(), login_shell_capture, resolve_policy(), + tool_context.process_scope.clone(), )) } }; 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 3eba52a..7e0621d 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 @@ -2035,6 +2035,37 @@ impl SessionActor { snapshot: Box::new(snapshot), }); } + if identical_tool_calls.take_nudge() { + let run_len = identical_tool_calls.run_len; + let tool_name = identical_tool_calls.tool_name.clone(); + tracing::warn!( + session_id = %self.session_info.id, + tool_name = %tool_name, + run_len, + "action stationarity: nudging model to break repeated identical tool calls" + ); + xai_grok_telemetry::unified_log::warn( + "shell.turn.action_stationarity_nudge", + Some(self.session_info.id.0.as_ref()), + Some(serde_json::json!({ + "loop_index": loop_index, + "tool_name": tool_name, + "run_len": run_len, + })), + ); + let reminder = self + .tool_bridge_handle() + .render_prompt( + ACTION_STATIONARITY_NUDGE_TEMPLATE, + &serde_json::json!({ + "tool_name": tool_name, + "run_len": run_len, + }), + ) + .await + .unwrap_or_else(|| ACTION_STATIONARITY_NUDGE_TEMPLATE.to_string()); + self.push_system_reminder(&reminder); + } self.drain_pending_interjections().await; self.flush_pending_skill_reminders().await; self.inject_pending_monitor_events().await; @@ -2506,8 +2537,7 @@ impl SessionActor { .map(|tc| tc.name.clone()) .unwrap_or_default(); let is_true_noop = self.is_run_true_step(&tool_calls).await; - let identical_run_len = - identical_tool_calls.observe(&step_signature, &step_tool_name, is_true_noop); + identical_tool_calls.observe(&step_signature, &step_tool_name, is_true_noop); if is_true_noop { xai_grok_telemetry::session_ctx::log_event( xai_grok_telemetry::events::ShellTrueNoop { @@ -2515,35 +2545,6 @@ impl SessionActor { }, ); } - if identical_run_len == NUDGE_AFTER_IDENTICAL_TOOL_CALLS { - tracing::warn!( - session_id = %self.session_info.id, - tool_name = %step_tool_name, - run_len = identical_run_len, - "action stationarity: nudging model to break repeated identical tool calls" - ); - xai_grok_telemetry::unified_log::warn( - "shell.turn.action_stationarity_nudge", - Some(self.session_info.id.0.as_ref()), - Some(serde_json::json!({ - "loop_index": loop_index, - "tool_name": step_tool_name, - "run_len": identical_run_len, - })), - ); - let reminder = self - .tool_bridge_handle() - .render_prompt( - ACTION_STATIONARITY_NUDGE_TEMPLATE, - &serde_json::json!({ - "tool_name": step_tool_name, - "run_len": identical_run_len, - }), - ) - .await - .unwrap_or_else(|| ACTION_STATIONARITY_NUDGE_TEMPLATE.to_string()); - self.push_system_reminder(&reminder); - } let tool_call_responses: Vec = tool_calls .into_iter() .map(|tc| ToolCallResponse { @@ -2627,13 +2628,13 @@ const MAX_CONSECUTIVE_TRUE_NOOPS: u32 = 4; const _: () = assert!(NUDGE_AFTER_IDENTICAL_TOOL_CALLS < MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS); const _: () = assert!(MAX_CONSECUTIVE_TRUE_NOOPS < NUDGE_AFTER_IDENTICAL_TOOL_CALLS); const ACTION_STATIONARITY_NUDGE_TEMPLATE: &str = "You have called the same tool \ - (`${{ tool_name }}`) with the exact same arguments ${{ run_len }} times in a row, \ - getting the same result each time — you appear to be stuck in a polling loop. Stop \ - repeating this call. If you are waiting on a long-running job or command, use a \ - background task${%- if tools.by_kind.monitor %} or the `${{ tools.by_kind.monitor }}` \ - tool${%- endif %}, or run a single `sleep` and then check once — do not poll in a tight \ - loop. If you cannot make progress, stop and tell the user what you are waiting for. This \ - turn will be halted automatically if the identical call keeps repeating."; + (`${{ tool_name }}`) with the exact same arguments ${{ run_len }} times in a row — \ + you appear to be stuck in a polling loop. Stop repeating this call. If you are \ + waiting on a long-running job or command, use a background task${%- if tools.by_kind.monitor %} \ + or the `${{ tools.by_kind.monitor }}` tool${%- endif %}, or run a single `sleep` and \ + then check once — do not poll in a tight loop. If you cannot make progress, stop and \ + tell the user what you are waiting for. This turn will be halted automatically if the \ + identical call keeps repeating."; fn hash_step_signature(signature: &str) -> u64 { use std::hash::{Hash, Hasher}; let mut hasher = std::collections::hash_map::DefaultHasher::new(); @@ -2649,6 +2650,7 @@ struct IdenticalToolCallRun { tool_name: String, run_len: u32, is_true_noop_run: bool, + nudged: bool, } impl IdenticalToolCallRun { fn observe(&mut self, signature: &str, tool_name: &str, is_true_noop: bool) -> u32 { @@ -2663,10 +2665,17 @@ impl IdenticalToolCallRun { self.run_len = 1; self.last_signature_hash = Some(hash); self.is_true_noop_run = is_true_noop; + self.nudged = false; } self.tool_name = tool_name.to_string(); self.run_len } + /// Once per identical run at/after the nudge threshold. Call only after results are committed. + fn take_nudge(&mut self) -> bool { + let fire = self.run_len >= NUDGE_AFTER_IDENTICAL_TOOL_CALLS && !self.nudged; + self.nudged |= fire; + fire + } fn hard_stop_threshold(&self) -> u32 { if self.is_true_noop_run { MAX_CONSECUTIVE_TRUE_NOOPS @@ -2679,7 +2688,7 @@ impl IdenticalToolCallRun { mod identical_tool_call_run_tests { use super::{ IdenticalToolCallRun, MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS, MAX_CONSECUTIVE_TRUE_NOOPS, - command_is_true, + NUDGE_AFTER_IDENTICAL_TOOL_CALLS, command_is_true, }; #[test] fn identical_non_true_resets_and_caps_at_16() { @@ -2715,6 +2724,31 @@ mod identical_tool_call_run_tests { assert!(!command_is_true("true && echo hi")); assert!(!command_is_true("lisa status")); } + #[test] + fn nudge_latch_fires_once_per_run_after_threshold() { + let mut run = IdenticalToolCallRun::default(); + for i in 1..NUDGE_AFTER_IDENTICAL_TOOL_CALLS { + assert_eq!(run.observe("poll", "get_task_output", false), i); + assert!( + !run.take_nudge(), + "must not nudge before threshold; run_len={i}" + ); + } + assert_eq!( + run.observe("poll", "get_task_output", false), + NUDGE_AFTER_IDENTICAL_TOOL_CALLS + ); + assert!(run.take_nudge()); + assert!(!run.take_nudge()); + assert_eq!( + run.observe("poll", "get_task_output", false), + NUDGE_AFTER_IDENTICAL_TOOL_CALLS + 1 + ); + assert!(!run.take_nudge()); + assert_eq!(run.observe("other", "bash", false), 1); + assert!(!run.nudged); + assert!(!run.take_nudge()); + } } /// Backoff schedule for resubmits after a *successful* 401 auth recovery /// (fresh token minted, request to be re-sent). diff --git a/crates/codegen/xai-grok-shell/src/session/acp_session_tests/turn/chat_history_integrity_tests.rs b/crates/codegen/xai-grok-shell/src/session/acp_session_tests/turn/chat_history_integrity_tests.rs new file mode 100644 index 0000000..e43abbc --- /dev/null +++ b/crates/codegen/xai-grok-shell/src/session/acp_session_tests/turn/chat_history_integrity_tests.rs @@ -0,0 +1,252 @@ +//! Chat-history integrity across mid-turn user-message injection. +//! +//! A `push_user_message` (system reminder, interjection, etc.) while an +//! assistant `tool_use` is committed but unanswered makes integrity repair +//! fabricate a `"cancelled by the user"` `tool_result`. If the tool then +//! runs and appends its real result, the conversation has **two** +//! `tool_result`s for one `tool_use_id`. Providers reject that shape with +//! HTTP 400 on every subsequent request — a permanently bricked session +//! with no in-band recovery. +//! +//! The concrete injector that first hit this was the action-stationarity +//! nudge (8 consecutive identical tool calls). The invariant is broader: +//! **no mid-turn user injection may leave duplicate results for one id.** +//! The nudge is only the driver that reaches the vulnerable window. +//! +//! This is intentionally *not* a unit test of `IdenticalToolCallRun`'s latch +//! (see `identical_tool_call_run_tests` in `turn.rs`). It drives a real +//! turn loop against a scripted model so a reordering that pushes the +//! reminder between `record_assistant_response` and `execute_tool_calls` +//! fails here. + +use super::support::*; +use super::*; +use std::collections::HashMap; +use std::sync::Arc; +use std::time::Duration; +use xai_grok_test_support::sse::{ + responses_api_reasoning_then_tool_call_events, responses_api_script_exact, +}; +use xai_grok_test_support::{MockInferenceServer, ScriptedResponse}; + +/// Product threshold at which the stationarity nudge fires. Kept as a local +/// literal so this suite does not couple to the private latch constants; +/// changing the threshold still trips the same history invariant as long as +/// a nudge is delivered mid-turn after identical calls. +const IDENTICAL_CALLS_TO_TRIP_NUDGE: usize = 8; + +const TODO_ARGS: &str = r#"{"todos":[{"id":"t1","content":"poll","status":"completed"}]}"#; + +const CANCEL_MARKER: &str = "cancelled by the user"; + +/// Distinctive fragment of the action-stationarity reminder. Present in both +/// the rendered template and the unrendered fallback string. +const STATIONARITY_NUDGE_MARKER: &str = "stuck in a polling loop"; + +fn tool_call_sse(call_id: &str) -> ScriptedResponse { + ScriptedResponse::sse(responses_api_reasoning_then_tool_call_events( + "poll", + call_id, + "todo_write", + TODO_ARGS, + "test", + )) +} + +fn drain_gateway(mut rx: tokio::sync::mpsc::UnboundedReceiver) { + tokio::task::spawn_local(async move { + while let Some(msg) = rx.recv().await { + if let xai_acp_lib::AcpClientMessage::SessionNotification(args) = msg { + let _ = args.response_tx.send(Ok(())); + } + } + }); +} + +fn drain_persistence(mut rx: tokio::sync::mpsc::UnboundedReceiver) { + tokio::task::spawn_local(async move { + while let Some(msg) = rx.recv().await { + if let PersistenceMsg::FlushAndAck { respond_to } = msg { + let _ = respond_to.send(()); + } + } + }); +} + +/// Group `ToolResult` bodies by `tool_call_id` across the whole conversation +/// (not just the contiguous run after an assistant message). Provider validation +/// is global; a user row between two results for the same id still 400s. +fn tool_results_by_call_id(conv: &[ConversationItem]) -> HashMap> { + let mut by_id: HashMap> = HashMap::new(); + for item in conv { + if let ConversationItem::ToolResult(tr) = item { + by_id + .entry(tr.tool_call_id.clone()) + .or_default() + .push(item.text_content()); + } + } + by_id +} + +/// Mid-turn system reminder (stationarity nudge after 8 identical tool calls) +/// must not fabricate a phantom cancel that duplicates a live tool's result. +/// +/// Pre-fix this failed: the nudge was pushed after the assistant `tool_use` +/// was committed and before `execute_tool_calls`, so integrity repair wrote +/// a cancel result and the real result landed beside it under the same id. +#[tokio::test(flavor = "current_thread")] +async fn mid_turn_user_injection_must_not_duplicate_tool_results_for_one_tool_use_id() { + let local = tokio::task::LocalSet::new(); + local + .run_until(async { + let server = MockInferenceServer::start().await.expect("mock inference server"); + for i in 1..=IDENTICAL_CALLS_TO_TRIP_NUDGE { + server.enqueue_response( + "/v1/responses", + tool_call_sse(&format!("stat-call-{i}")), + ); + } + server.enqueue_response( + "/v1/responses", + ScriptedResponse::sse(responses_api_script_exact("done", "test")), + ); + + let sampling_cfg = xai_grok_sampler::SamplerConfig { + api_key: Some("test-key".to_string()), + base_url: server.url(), + model: "test".to_string(), + api_backend: xai_grok_sampler::ApiBackend::Responses, + context_window: 256_000, + max_retries: Some(0), + idle_timeout_secs: Some(30), + ..Default::default() + }; + + let (sampler_event_tx, sampler_event_rx) = + tokio::sync::mpsc::unbounded_channel::(); + let sampler_handle = xai_grok_sampler::SamplerActor::spawn( + sampling_cfg, + xai_grok_sampler::RetryPolicy { + max_retries: 0, + rate_limit_retry_threshold: 0, + ..Default::default() + }, + sampler_event_tx, + ); + + let (gateway_tx, gateway_rx) = + tokio::sync::mpsc::unbounded_channel::(); + drain_gateway(gateway_rx); + let (persistence_tx, persistence_rx) = + tokio::sync::mpsc::unbounded_channel::(); + drain_persistence(persistence_rx); + + let mut actor = create_test_actor(0, 256_000, 85, gateway_tx, persistence_tx).await; + actor.sampler_handle = sampler_handle; + *actor.agent.borrow_mut() = test_grok_build_agent_with_todo().await; + + let mut cfg = actor + .chat_state_handle + .get_sampling_config() + .await + .expect("test actor has sampling config"); + cfg.base_url = server.url(); + cfg.api_backend = xai_grok_sampling_types::ApiBackend::Responses; + cfg.model = "test".to_string(); + actor.chat_state_handle.update_sampling_config(cfg); + let mut creds = actor.chat_state_handle.get_credentials().await; + creds.api_key = Some("test-key".to_string()); + actor.chat_state_handle.update_credentials(creds); + + 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"); + + let actor = Arc::new(actor); + { + let drainer = actor.clone(); + let mut sampler_event_rx = sampler_event_rx; + tokio::task::spawn_local(async move { + while let Some(event) = sampler_event_rx.recv().await { + drainer.handle_sampling_event(event).await; + } + }); + } + + let prompt_blocks = vec![acp::ContentBlock::Text(acp::TextContent::new( + "keep polling the same todo".to_string(), + ))]; + let outcome = tokio::time::timeout( + Duration::from_secs(60), + actor.handle_prompt( + "chat-history-integrity", + prompt_blocks, + PromptMode::Agent, + None, + None, + None, + None, + true, + None, + None, + None, + ), + ) + .await + .expect("turn must finish within timeout"); + assert!( + outcome.is_ok(), + "turn must not error: {outcome:?}" + ); + + let conv = actor.chat_state_handle.get_conversation().await; + let by_id = tool_results_by_call_id(&conv); + + assert!( + by_id.len() >= IDENTICAL_CALLS_TO_TRIP_NUDGE, + "expected at least {IDENTICAL_CALLS_TO_TRIP_NUDGE} executed tool calls to trip the nudge; got {} distinct tool_call_ids. conversation={conv:#?}", + by_id.len() + ); + + for (tool_call_id, results) in &by_id { + let has_cancel = results.iter().any(|r| r.contains(CANCEL_MARKER)); + let has_real = results.iter().any(|r| !r.contains(CANCEL_MARKER)); + assert!( + !(has_cancel && has_real), + "tool_use_id `{tool_call_id}` has both a fabricated `{CANCEL_MARKER}` \ + tool_result and a real execution result. The tool ran; integrity repair \ + must not claim it was cancelled. A mid-turn user message was pushed while \ + the tool_use was unanswered. results={results:?}" + ); + assert_eq!( + results.len(), + 1, + "tool_use_id `{tool_call_id}` has {} tool_results. \ + Duplicate tool_results for one tool_use_id brick the session: the \ + Anthropic Messages API (and siblings) reject the history with HTTP 400 \ + on every subsequent request, and there is no in-band recovery. \ + Mid-turn user messages (nudges, reminders, interjections) must not be \ + pushed while a tool_use is unanswered — integrity repair fabricates a \ + cancel result, then the live tool appends a second result. results={results:?}", + results.len() + ); + } + + assert!( + conv.iter() + .any(|item| item.text_content().contains(STATIONARITY_NUDGE_MARKER)), + "action-stationarity nudge must still be delivered after the identical-call \ + run; deleting the nudge is not a valid fix for chat-history corruption. \ + conversation={conv:#?}" + ); + }) + .await; +} diff --git a/crates/codegen/xai-grok-shell/src/util/subprocess.rs b/crates/codegen/xai-grok-shell/src/util/subprocess.rs index 8d2fac5..e54967f 100644 --- a/crates/codegen/xai-grok-shell/src/util/subprocess.rs +++ b/crates/codegen/xai-grok-shell/src/util/subprocess.rs @@ -51,11 +51,24 @@ pub(crate) fn git_bin() -> OsString { } } -/// A `sh -c