Synced from monorepo
Synced from monorepo Changes: - Refresh tool search when the managed MCP catalog is re-fetched - Prevent duplicate leader process spawn and startup hang from stale leaders - Document marketplaces, plugins, and organization controls - Stamp session ID on image generation direct-to-API requests - Fix auto mode blocked documentation - Auto mode considers recent user intent - Expose deploy archive, taken-down, limit, and in-progress reasons on the chat API - Fail-closed auth refresh contract for shell clients - Emit a chat-supplied per-session turn index in turn hooks - Show bash mode chrome in minimal mode - Add metrics for true-noop and stationarity stops - Include voice interim text on prompt submit - Silently end turn on true-noop thrash - Quiet copy toast when clipboard delivery is confirmed - Fix session fork truncating at the wrong prompt in rewound sessions - Make the idle "still running" watcher cue clickable to open the tasks pane - Default web search model to grok-4.5 - Let plugin subagents inherit parent MCP servers - Gate no-op end-turn reminder on system reminders - Add gateway bridge lifecycle telemetry - Allow editing finalized text while voice is open - Relocate token carrier to turn-commit events and plumb per-turn origin context - Raise workflow scratch quotas and make failed runs resumable - Workflows overlay: auto-progress phases, live agent status, and drop budget meter Source-Revision: 9b8d35b46d959c042ea9aa31cbbebbd1f0c5c527
This commit is contained in:
parent
69f0ba880a
commit
6e38642082
103 changed files with 4964 additions and 1261 deletions
|
|
@ -1321,8 +1321,8 @@ Each feature section below documents its own config. This section covers the gen
|
|||
auto_update = true # check for updates on launch
|
||||
|
||||
[models]
|
||||
default = "grok-build" # model used for new sessions
|
||||
web_search = "grok-4.20-multi-agent" # model used by the web_search tool
|
||||
default = "grok-4.5" # model used for new sessions
|
||||
web_search = "grok-4.5" # model used by the web_search tool
|
||||
|
||||
[ui]
|
||||
max_thoughts_width = 120 # max column width for reasoning display
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@
|
|||
|
||||
## Bug Fixes
|
||||
|
||||
- **Plugin subagents** now inherit the parent session’s connected MCP servers (default `mcpInheritance: all`), so `search_tool` / `use_tool` work the same as for local agents. Plugin agents still cannot declare their own MCP servers, hooks, or elevated permission modes.
|
||||
- **`!cmd` commands** now allow up to one hour before timing out.
|
||||
- **npm package** now installs the native binary under `$GROK_HOME/bin` (honoring the same override as the Rust CLI).
|
||||
- **Startup warnings** now point to `/doctor` for details and fixes.
|
||||
|
|
|
|||
|
|
@ -71,6 +71,12 @@ const AUTO_UPDATE_FLUSH_GRACE: Duration = Duration::from_secs(10);
|
|||
/// the bounded-grace semantics of the `RelaunchForUpdate` drain.
|
||||
const MAX_AUTO_UPDATE_BUSY_DEFERRALS: u32 = 24;
|
||||
|
||||
/// Bounded wait for the leader flock when it is held but no socket is bound yet
|
||||
/// (a spawner mid-handoff, an old-flow client holding the flock across its ~10s
|
||||
/// spawn window, or a same-version sibling briefly holding it). Exceeds that
|
||||
/// old-flow window so a legitimately-spawning peer wins the race.
|
||||
const LEADER_ACQUIRE_TIMEOUT: Duration = Duration::from_secs(15);
|
||||
|
||||
/// Run the auto-update checker loop.
|
||||
///
|
||||
/// Periodically calls `check_fn` to check for, download, and install updates.
|
||||
|
|
@ -906,17 +912,16 @@ fn spawn_leader_relay(
|
|||
/// serve clients over IPC only. See [`spawn_leader_relay`] for when the relay
|
||||
/// connection is opened (eager by default, demand-gated with `relay_on_demand`).
|
||||
///
|
||||
/// Startup sequence:
|
||||
/// 1. Lock acquisition check — bail if another leader is already running.
|
||||
/// Startup sequence (lock-then-socket):
|
||||
/// 1. Acquire the leader flock FIRST — bail if another process holds it.
|
||||
/// 2. Socket cleanup, channel + readiness-watch creation.
|
||||
/// 3. IPC server started (`tokio::spawn`) — socket bound HERE, before auth.
|
||||
/// 4. Wait for socket to appear (fast: < 100 ms).
|
||||
/// 5. Lock handoff with spawner (if launched via connect_or_spawn).
|
||||
/// 6. Auth + model prefetch (slow path, but socket already available to clients).
|
||||
/// 5. Auth + model prefetch (slow path, but socket already available to clients).
|
||||
/// - Auth resolves non-interactively; `None` (BYOK / no session) is not an
|
||||
/// error — the relay is gated off and login is deferred to ACP.
|
||||
/// 7. `ready_tx.send(true)` — unblocks ACP forwarding in the IPC server.
|
||||
/// 8. LocalSet: agent, IPC↔agent bridges, WS↔agent bridges, relay, config watcher.
|
||||
/// 6. `ready_tx.send(true)` — unblocks ACP forwarding in the IPC server.
|
||||
/// 7. LocalSet: agent, IPC↔agent bridges, WS↔agent bridges, relay, config watcher.
|
||||
///
|
||||
/// # Arguments
|
||||
///
|
||||
|
|
@ -934,7 +939,7 @@ pub async fn run_leader(
|
|||
) -> anyhow::Result<()> {
|
||||
use crate::agent::relay::RelayConfig;
|
||||
use crate::leader::{
|
||||
LeaderLock, LeaderServerControlState, LeaderServerMetadata, ShutdownReason,
|
||||
LeaderLock, LeaderServerControlState, LeaderServerMetadata, LockError, ShutdownReason,
|
||||
compute_ws_url_suffix, run_leader_server,
|
||||
};
|
||||
use tokio::sync::watch;
|
||||
|
|
@ -964,41 +969,62 @@ pub async fn run_leader(
|
|||
let mut lock = LeaderLock::new(ws_url);
|
||||
let socket_path = lock.socket_path().clone();
|
||||
|
||||
// Early bail-out: lock held + socket exists → another leader is running.
|
||||
// ── Phase 1: Acquire the leader flock FIRST (lock-then-socket) ────────────
|
||||
//
|
||||
// Three cases:
|
||||
// - Lock free → we ARE the leader; hold lock through setup.
|
||||
// - Lock held + socket → another leader running → bail out immediately.
|
||||
// - Lock held + no socket → spawner (connect_or_spawn) holds lock and is
|
||||
// waiting for our socket → proceed normally.
|
||||
let lock_already_held = match lock.try_acquire() {
|
||||
// SINGLE-LEADER INVARIANT: only the flock holder may create/remove the socket
|
||||
// and it holds the flock for its whole lifetime, so a racing leader can never
|
||||
// clobber a live socket.
|
||||
match lock.try_acquire() {
|
||||
Ok(true) => {
|
||||
lock.write_pid()?;
|
||||
debug!("Lock acquired immediately, proceeding as leader");
|
||||
true
|
||||
debug!("Acquired leader lock, proceeding as leader");
|
||||
}
|
||||
Ok(false) => {
|
||||
// Fast path: a fully-running leader (flock held AND socket bound) →
|
||||
// exit so the client adopts it.
|
||||
if crate::leader::listener_is_ready(&socket_path) {
|
||||
info!(
|
||||
"Another leader is already running (lock held, socket exists at {}). Exiting.",
|
||||
"Another process holds the leader lock with a bound socket ({}). \
|
||||
Exiting so the client adopts it.",
|
||||
socket_path.display()
|
||||
);
|
||||
return Err(anyhow::anyhow!(
|
||||
"Another leader is already running at {}",
|
||||
"Another leader already holds the lock at {}",
|
||||
socket_path.display()
|
||||
));
|
||||
}
|
||||
debug!("Lock held by spawner (no socket yet), proceeding with socket-then-lock flow");
|
||||
false
|
||||
}
|
||||
Err(e) => return Err(anyhow::anyhow!("Failed to check leader lock: {}", e)),
|
||||
};
|
||||
|
||||
// ── Phase 1: Clean up stale socket ────────────────────────────────────────
|
||||
// Held but no socket yet: a spawner is mid-handoff, or an old-flow
|
||||
// client holds the flock across its spawn window. Wait (re-opening the
|
||||
// path each poll to tolerate the old client's Drop unlinking the inode)
|
||||
// before conceding.
|
||||
match lock.acquire_reopen_timeout(LEADER_ACQUIRE_TIMEOUT).await {
|
||||
Ok(()) => {
|
||||
lock.write_pid()?;
|
||||
debug!("Acquired leader lock after bounded wait, proceeding as leader");
|
||||
}
|
||||
Err(LockError::Timeout(_)) => {
|
||||
info!(
|
||||
"Timed out waiting for the leader lock ({}). Exiting so the \
|
||||
client adopts whoever won it.",
|
||||
socket_path.display()
|
||||
);
|
||||
return Err(anyhow::anyhow!(
|
||||
"Timed out acquiring leader lock at {}",
|
||||
socket_path.display()
|
||||
));
|
||||
}
|
||||
Err(e) => return Err(anyhow::anyhow!("Failed to acquire leader lock: {}", e)),
|
||||
}
|
||||
}
|
||||
Err(e) => return Err(anyhow::anyhow!("Failed to acquire leader lock: {}", e)),
|
||||
}
|
||||
|
||||
// ── Phase 2: Clean up stale socket (we hold the flock, so this is safe) ────
|
||||
lock.cleanup_socket()?;
|
||||
info!("Leader server starting");
|
||||
|
||||
// ── Phase 2: Create all channels + readiness watch ────────────────────────
|
||||
// ── Phase 3: Create all channels + readiness watch ────────────────────────
|
||||
//
|
||||
// All channels are created here so the IPC server can start receiving
|
||||
// client connections immediately, before auth/prefetch begin.
|
||||
|
|
@ -1058,7 +1084,7 @@ pub async fn run_leader(
|
|||
// Cloned before control_state moves into the IPC server; auth wired below.
|
||||
let workspace_control = control_state.workspace.clone();
|
||||
|
||||
// ── Phase 3: Bind socket and start IPC server (BEFORE auth/prefetch) ──────
|
||||
// ── Phase 4: Bind socket and start IPC server (BEFORE auth/prefetch) ──────
|
||||
//
|
||||
// Starting the server here means connect_or_spawn sees the socket in < 100 ms
|
||||
// regardless of how long auth + model prefetch take. The `ready_rx` gate inside
|
||||
|
|
@ -1092,7 +1118,7 @@ pub async fn run_leader(
|
|||
}
|
||||
});
|
||||
|
||||
// ── Phase 4: Wait for socket to appear (fast: < 100 ms now) ──────────────
|
||||
// ── Phase 5: Wait for socket to appear (fast: < 100 ms now) ──────────────
|
||||
let socket_ready_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(5);
|
||||
while !crate::leader::listener_is_ready(&socket_path) {
|
||||
if tokio::time::Instant::now() >= socket_ready_deadline {
|
||||
|
|
@ -1105,42 +1131,8 @@ pub async fn run_leader(
|
|||
}
|
||||
debug!("IPC socket created");
|
||||
|
||||
// ── Phase 5: Lock handoff ─────────────────────────────────────────────────
|
||||
//
|
||||
// (a) lock_already_held=true: We acquired the lock at startup. Keep it.
|
||||
// (b) lock_already_held=false: spawner holds lock, waiting for our socket.
|
||||
// Now that socket is up, the spawner will see it, connect, and release
|
||||
// the lock. We acquire it here (30 s timeout).
|
||||
let _lock = if lock_already_held {
|
||||
info!("Leader lock already held from startup, PID already written");
|
||||
lock
|
||||
} else {
|
||||
const LEADER_LOCK_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
// spawn_blocking so we don't stall the async runtime while waiting.
|
||||
let lock_result = tokio::task::spawn_blocking(move || {
|
||||
lock.try_acquire_timeout(LEADER_LOCK_TIMEOUT)?;
|
||||
lock.write_pid()?;
|
||||
Ok::<_, anyhow::Error>(lock)
|
||||
})
|
||||
.await;
|
||||
|
||||
match lock_result {
|
||||
Ok(Ok(lock)) => {
|
||||
info!("Leader lock acquired, PID written");
|
||||
lock
|
||||
}
|
||||
Ok(Err(e)) => {
|
||||
warn!(error = ?e, "Failed to acquire leader lock");
|
||||
cancel.cancel();
|
||||
return Err(anyhow::anyhow!("Failed to acquire leader lock: {}", e));
|
||||
}
|
||||
Err(e) => {
|
||||
warn!(error = ?e, "Lock task panicked");
|
||||
cancel.cancel();
|
||||
return Err(anyhow::anyhow!("Lock task failed: {}", e));
|
||||
}
|
||||
}
|
||||
};
|
||||
// Keep `lock` alive so its `Drop` removes the lock + socket on exit.
|
||||
let _lock = lock;
|
||||
|
||||
// ── Phase 6: Auth + model prefetch ───────────────────────────────────────
|
||||
//
|
||||
|
|
|
|||
|
|
@ -3073,6 +3073,9 @@ impl acp::Agent for MvpAgent {
|
|||
crate::session::commands::PromptCompletionKind::MaxTurnsReached {
|
||||
..
|
||||
} => Some("max_turns_reached".to_string()),
|
||||
crate::session::commands::PromptCompletionKind::StationarityEnded => {
|
||||
Some("action_stationarity".to_string())
|
||||
}
|
||||
_ => None,
|
||||
};
|
||||
Ok(
|
||||
|
|
|
|||
|
|
@ -272,6 +272,81 @@ impl MvpAgent {
|
|||
}
|
||||
});
|
||||
}
|
||||
/// Push a fresh legacy managed-MCP catalog into live sessions' per-session
|
||||
/// `McpServers` (called after `mcp/list` with `cache=false`).
|
||||
///
|
||||
/// The per-session `merge_managed_mcp_servers` re-reads disk, so the whole
|
||||
/// broadcast is deferred off the `mcp/list` response-latency path via
|
||||
/// `spawn_local`. This ONLY re-merges/pushes connectors; rebuilding the
|
||||
/// agent-level gateway catalog's `search_tool` index is a separate,
|
||||
/// independently-gated broadcast (see `refresh_mcp_search_index_in_sessions`),
|
||||
/// because the two run in mutually-exclusive modes (legacy fetch only when
|
||||
/// gateway tools are OFF, gateway fetch only when ON).
|
||||
/// Caller must confirm the managed fetch succeeded (cache `Ready`) first: a
|
||||
/// failed fetch returns an empty vec and syncing it tears down live servers.
|
||||
pub(crate) fn sync_fresh_managed_mcp_to_sessions(
|
||||
&self,
|
||||
managed: &[crate::session::managed_mcp::ManagedMcpConfig],
|
||||
) {
|
||||
let sessions: Vec<_> = self
|
||||
.sessions
|
||||
.borrow()
|
||||
.values()
|
||||
.map(|handle| (
|
||||
handle.cmd_tx.clone(),
|
||||
handle.info.cwd.clone(),
|
||||
handle.initial_client_mcp_servers.clone(),
|
||||
))
|
||||
.collect();
|
||||
if sessions.is_empty() {
|
||||
return;
|
||||
}
|
||||
let compat = self.cfg.borrow().compat_resolved;
|
||||
let plugin_snapshot = self.plugin_registry_handle.snapshot();
|
||||
let managed = managed.to_vec();
|
||||
tokio::task::spawn_local(async move {
|
||||
let mut updated = 0u32;
|
||||
for (cmd_tx, cwd, initial_client_mcp_servers) in sessions {
|
||||
let cwd = std::path::PathBuf::from(cwd);
|
||||
if crate::session::managed_mcp::merge_and_send_managed_mcp_update(
|
||||
&cmd_tx,
|
||||
&cwd,
|
||||
initial_client_mcp_servers,
|
||||
&managed,
|
||||
plugin_snapshot.as_deref(),
|
||||
&compat,
|
||||
) {
|
||||
updated += 1;
|
||||
}
|
||||
}
|
||||
if updated > 0 {
|
||||
tracing::info!(
|
||||
updated,
|
||||
managed_count = managed.len(),
|
||||
"synced fresh managed MCP catalog into live sessions"
|
||||
);
|
||||
}
|
||||
});
|
||||
}
|
||||
/// Rebuild `search_tool` in every live session after a fresh gateway tool
|
||||
/// catalog committed.
|
||||
///
|
||||
/// Gateway tools live in the agent-level catalog (not per-session
|
||||
/// `McpServers`), so a fresh gateway catalog needs a session-side
|
||||
/// `search_tool` rebuild even though the legacy managed cache stays
|
||||
/// `NotFetched` in gateway mode. Callers gate on a successful refetch and
|
||||
/// skip on failure to keep the last-good index.
|
||||
pub(crate) fn refresh_mcp_search_index_in_sessions(&self) {
|
||||
let session_txs: Vec<_> = self
|
||||
.sessions
|
||||
.borrow()
|
||||
.values()
|
||||
.map(|handle| handle.cmd_tx.clone())
|
||||
.collect();
|
||||
for tx in session_txs {
|
||||
let _ = tx.send(SessionCommand::RefreshMcpSearchIndex);
|
||||
}
|
||||
}
|
||||
/// Resolve the launch dir's project-scope trust verdict ONCE and return it
|
||||
/// with its path.
|
||||
///
|
||||
|
|
|
|||
|
|
@ -333,20 +333,14 @@ async fn reload_project_servers_after_grant(ctx: ReloadAfterGrant<'_>) {
|
|||
// MCP: `merge_managed_mcp_servers` re-reads disk + runs
|
||||
// `filter_untrusted_project_mcp`, which now KEEPS project servers because
|
||||
// the cached verdict was flipped to trusted (same workspace key).
|
||||
let merged = crate::session::managed_mcp::merge_managed_mcp_servers(
|
||||
target.initial_client_mcp_servers,
|
||||
let _ = crate::session::managed_mcp::merge_and_send_managed_mcp_update(
|
||||
&target.cmd_tx,
|
||||
session_cwd,
|
||||
target.initial_client_mcp_servers,
|
||||
&managed,
|
||||
plugin_snapshot.as_deref(),
|
||||
ctx.compat,
|
||||
);
|
||||
let (tx, _rx) = tokio::sync::oneshot::channel();
|
||||
let _ = target
|
||||
.cmd_tx
|
||||
.send(crate::session::SessionCommand::UpdateMcpServers {
|
||||
mcp_servers: merged,
|
||||
respond_to: tx,
|
||||
});
|
||||
// Plugins (+ plugin-contributed hooks) built for this session's own cwd
|
||||
// on the folder-trust verdict (mirrors `broadcast_plugin_registry_to_sessions`);
|
||||
// the grant + resolve_and_record above flipped the cached verdict to trusted.
|
||||
|
|
|
|||
|
|
@ -1606,6 +1606,77 @@ async fn ext_method_routes_auth_cleared_and_refreshes_resident_sessions() {
|
|||
})
|
||||
.await;
|
||||
}
|
||||
/// Fresh managed catalog sync must push UpdateMcpServers with the injected
|
||||
/// managed connector. The `search_tool` rebuild is a SEPARATE broadcast
|
||||
/// (`refresh_mcp_search_index_in_sessions`), so it is not asserted here.
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn sync_fresh_managed_mcp_pushes_update() {
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local
|
||||
.run_until(async {
|
||||
let agent = build_agent_with_auth(crate::auth::GrokAuth {
|
||||
key: "eligible".into(),
|
||||
auth_mode: crate::auth::AuthMode::WebLogin,
|
||||
..crate::auth::GrokAuth::test_default()
|
||||
});
|
||||
let sid = acp::SessionId::new("sess-managed-sync");
|
||||
let (handle, _tx, mut cmd_rx) = make_live_session_handle(&sid, None);
|
||||
agent.sessions.borrow_mut().insert(sid, handle);
|
||||
let managed = vec![crate::session::managed_mcp::ManagedMcpConfig {
|
||||
name: "Linear".into(),
|
||||
endpoint: "https://mcp.example.com/linear".into(),
|
||||
headers: std::collections::HashMap::from([(
|
||||
"Authorization".into(),
|
||||
"Bearer tok".into(),
|
||||
)]),
|
||||
token_expires_at: None,
|
||||
scope: None,
|
||||
scope_id: None,
|
||||
scope_name: None,
|
||||
}];
|
||||
agent.sync_fresh_managed_mcp_to_sessions(&managed);
|
||||
let first = tokio::time::timeout(std::time::Duration::from_secs(1), cmd_rx.recv())
|
||||
.await
|
||||
.expect("UpdateMcpServers should be sent")
|
||||
.expect("channel should stay open");
|
||||
let SessionCommand::UpdateMcpServers { mcp_servers, .. } = first else {
|
||||
panic!("expected UpdateMcpServers as the first synced command");
|
||||
};
|
||||
let managed_name = crate::session::managed_mcp::to_managed_name("Linear");
|
||||
let linear = mcp_servers
|
||||
.iter()
|
||||
.find_map(|s| match s {
|
||||
acp::McpServer::Http(http) if http.name == managed_name => Some(http),
|
||||
_ => None,
|
||||
})
|
||||
.unwrap_or_else(|| {
|
||||
panic!("merged catalog must contain managed HTTP server {managed_name}")
|
||||
});
|
||||
assert!(
|
||||
linear
|
||||
.headers
|
||||
.iter()
|
||||
.any(|h| h.name == "Authorization" && h.value == "Bearer tok"),
|
||||
"managed server must carry the injected Authorization header"
|
||||
);
|
||||
})
|
||||
.await;
|
||||
}
|
||||
/// The gateway-catalog refresh broadcast pushes `RefreshMcpSearchIndex` to every
|
||||
/// live session (independent of the legacy managed-connector sync).
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn refresh_mcp_search_index_broadcasts_to_sessions() {
|
||||
let agent = build_minimal_agent_for_tests();
|
||||
let sid = acp::SessionId::new("sess-search-index");
|
||||
let (handle, _tx, mut cmd_rx) = make_live_session_handle(&sid, None);
|
||||
agent.sessions.borrow_mut().insert(sid, handle);
|
||||
agent.refresh_mcp_search_index_in_sessions();
|
||||
let cmd = tokio::time::timeout(std::time::Duration::from_secs(1), cmd_rx.recv())
|
||||
.await
|
||||
.expect("RefreshMcpSearchIndex should be sent")
|
||||
.expect("channel should stay open");
|
||||
assert!(matches!(cmd, SessionCommand::RefreshMcpSearchIndex));
|
||||
}
|
||||
/// Build a minimal MvpAgent suitable for testing extension methods.
|
||||
fn build_minimal_agent_for_tests() -> MvpAgent {
|
||||
use crate::agent::config::Config as AgentConfig;
|
||||
|
|
|
|||
|
|
@ -861,7 +861,7 @@ pub(crate) async fn run_shell_child(
|
|||
}
|
||||
}
|
||||
}
|
||||
let agent_mcp_servers: Vec<_> = if is_plugin_agent {
|
||||
let agent_mcp_servers: Vec<_> = if !agent_owned_mcp_servers_allowed(is_plugin_agent) {
|
||||
if !definition.mcp_servers.is_empty() {
|
||||
tracing::warn!(
|
||||
agent = %definition.name,
|
||||
|
|
@ -921,19 +921,8 @@ pub(crate) async fn run_shell_child(
|
|||
})
|
||||
.collect()
|
||||
};
|
||||
let parent_mcp_pool = if is_plugin_agent {
|
||||
if ctx.parent_mcp_pool.is_some() {
|
||||
tracing::debug!(
|
||||
agent = %definition.name,
|
||||
"skipping MCP pool inheritance for plugin agent"
|
||||
);
|
||||
}
|
||||
None
|
||||
} else {
|
||||
ctx.parent_mcp_pool
|
||||
.take()
|
||||
.and_then(|pool| filter_pool_by_inheritance(pool, &definition.mcp_inheritance))
|
||||
};
|
||||
let parent_mcp_pool =
|
||||
resolve_inherited_mcp_pool(ctx.parent_mcp_pool.take(), &definition.mcp_inheritance);
|
||||
let mcp_inherited_count = parent_mcp_pool
|
||||
.as_ref()
|
||||
.map(|p| p.len() as u32)
|
||||
|
|
|
|||
|
|
@ -1338,6 +1338,21 @@ fn durable_resume_source_for(
|
|||
model_id: meta.effective_model_id,
|
||||
})
|
||||
}
|
||||
/// Resolve the MCP pool a child subagent should import from its parent.
|
||||
///
|
||||
/// Inheritance applies to **every** agent source (built-in, user, project,
|
||||
/// and plugin). Plugin agents are not excluded: the parent already connected
|
||||
/// these servers for the session. Agent-owned `mcpServers` (spawned by the
|
||||
/// child itself) are handled separately and remain blocked for plugins.
|
||||
///
|
||||
/// Returns `None` when there is no parent pool or `inheritance` is
|
||||
/// [`McpInheritance::None`] (avoids an empty import call downstream).
|
||||
fn resolve_inherited_mcp_pool(
|
||||
parent_pool: Option<crate::session::mcp_servers::SharedMcpPool>,
|
||||
inheritance: &xai_grok_agent::config::McpInheritance,
|
||||
) -> Option<crate::session::mcp_servers::SharedMcpPool> {
|
||||
parent_pool.and_then(|pool| filter_pool_by_inheritance(pool, inheritance))
|
||||
}
|
||||
/// Apply `McpInheritance` filtering to a parent MCP pool snapshot.
|
||||
///
|
||||
/// Returns `None` for `McpInheritance::None` (no pool at all — avoids
|
||||
|
|
@ -1374,6 +1389,14 @@ fn filter_pool_by_inheritance(
|
|||
}
|
||||
}
|
||||
}
|
||||
/// Whether a subagent may declare its own agent-owned `mcpServers`.
|
||||
///
|
||||
/// Plugin agents cannot: untrusted packages must not spawn MCP processes or
|
||||
/// open network MCP endpoints. Parent-pool inheritance is independent and
|
||||
/// always available subject to [`McpInheritance`].
|
||||
fn agent_owned_mcp_servers_allowed(is_plugin_agent: bool) -> bool {
|
||||
!is_plugin_agent
|
||||
}
|
||||
/// Resolve a subagent type name to its `AgentDefinition`, with the parent
|
||||
/// session's CLI tool/permission overrides already applied (so the spawn path
|
||||
/// can never obtain a definition that skips them).
|
||||
|
|
|
|||
|
|
@ -2552,6 +2552,76 @@ fn filter_inheritance_except_all_servers_gives_empty() {
|
|||
let result = result.expect("Except should return Some");
|
||||
assert_eq!(result.server_names().count(), 0);
|
||||
}
|
||||
#[test]
|
||||
fn resolve_inherited_pool_all_passes_parent_pool() {
|
||||
let pool = make_pool(&["github", "atlassian"]);
|
||||
let result = super::resolve_inherited_mcp_pool(
|
||||
Some(pool),
|
||||
&xai_grok_agent::config::McpInheritance::All,
|
||||
)
|
||||
.expect("All should return Some");
|
||||
assert_eq!(pool_names(&result), vec!["atlassian", "github"]);
|
||||
}
|
||||
#[test]
|
||||
fn resolve_inherited_pool_none_returns_none() {
|
||||
let pool = make_pool(&["github", "atlassian"]);
|
||||
let result = super::resolve_inherited_mcp_pool(
|
||||
Some(pool),
|
||||
&xai_grok_agent::config::McpInheritance::None,
|
||||
);
|
||||
assert!(result.is_none());
|
||||
}
|
||||
#[test]
|
||||
fn resolve_inherited_pool_named_filters() {
|
||||
let pool = make_pool(&["github", "atlassian", "slack"]);
|
||||
let result = super::resolve_inherited_mcp_pool(
|
||||
Some(pool),
|
||||
&xai_grok_agent::config::McpInheritance::Named(vec!["atlassian".into()]),
|
||||
)
|
||||
.expect("Named should return Some");
|
||||
assert_eq!(pool_names(&result), vec!["atlassian"]);
|
||||
}
|
||||
#[test]
|
||||
fn resolve_inherited_pool_missing_parent_returns_none() {
|
||||
let result = super::resolve_inherited_mcp_pool(
|
||||
None,
|
||||
&xai_grok_agent::config::McpInheritance::All,
|
||||
);
|
||||
assert!(result.is_none());
|
||||
}
|
||||
/// Plugin agents must still inherit the parent pool under default
|
||||
/// `mcpInheritance: all`. The product rule is: plugins cannot *declare*
|
||||
/// mcpServers, but they do inherit already-connected parent servers.
|
||||
#[test]
|
||||
fn plugin_agents_inherit_parent_mcp_pool_by_default() {
|
||||
assert!(
|
||||
!super::agent_owned_mcp_servers_allowed(true),
|
||||
"plugin agents must not declare agent-owned mcpServers"
|
||||
);
|
||||
assert!(
|
||||
super::agent_owned_mcp_servers_allowed(false),
|
||||
"non-plugin agents may declare agent-owned mcpServers"
|
||||
);
|
||||
let pool = make_pool(&["atlassian", "github"]);
|
||||
let inherited = super::resolve_inherited_mcp_pool(
|
||||
Some(pool),
|
||||
&xai_grok_agent::config::McpInheritance::All,
|
||||
)
|
||||
.expect("plugin children inherit parent pool with mcpInheritance=all");
|
||||
assert_eq!(pool_names(&inherited), vec!["atlassian", "github"]);
|
||||
}
|
||||
#[test]
|
||||
fn plugin_agents_can_opt_out_via_mcp_inheritance_none() {
|
||||
let pool = make_pool(&["atlassian"]);
|
||||
let inherited = super::resolve_inherited_mcp_pool(
|
||||
Some(pool),
|
||||
&xai_grok_agent::config::McpInheritance::None,
|
||||
);
|
||||
assert!(
|
||||
inherited.is_none(),
|
||||
"mcpInheritance: none must drop the parent pool for every source"
|
||||
);
|
||||
}
|
||||
fn make_test_skill(
|
||||
name: &str,
|
||||
plugin: Option<&str>,
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@ impl HttpAuth for ShellAuthCredentialProvider {
|
|||
fn apply(&self, builder: RequestBuilder, base_url: &str) -> RequestBuilder {
|
||||
let mut creds = self.static_credentials.clone();
|
||||
if creds.deployment_key.is_none()
|
||||
&& let Some(auth) = self.auth_manager.current_or_expired()
|
||||
&& let Some(auth) = self.auth_manager.current_wire_valid()
|
||||
{
|
||||
creds.user_token = Some(auth.key);
|
||||
}
|
||||
|
|
@ -60,12 +60,12 @@ impl AuthCredentialProvider for ShellAuthCredentialProvider {
|
|||
..Default::default()
|
||||
};
|
||||
}
|
||||
let auth = self.auth_manager.current_or_expired();
|
||||
let user_id = auth.as_ref().map(|a| a.user_id.clone());
|
||||
let team_id = auth.as_ref().and_then(|a| a.team_id.clone());
|
||||
let organization_id = auth.as_ref().and_then(|a| a.organization_id.clone());
|
||||
let api_key_id = api_key_id_for(auth.as_ref());
|
||||
let token = auth.map(|a| a.key);
|
||||
let identity = self.auth_manager.current_or_expired();
|
||||
let user_id = identity.as_ref().map(|a| a.user_id.clone());
|
||||
let team_id = identity.as_ref().and_then(|a| a.team_id.clone());
|
||||
let organization_id = identity.as_ref().and_then(|a| a.organization_id.clone());
|
||||
let api_key_id = api_key_id_for(identity.as_ref());
|
||||
let token = self.auth_manager.current_wire_valid().map(|a| a.key);
|
||||
CredentialSnapshot {
|
||||
token,
|
||||
user_id,
|
||||
|
|
|
|||
|
|
@ -141,4 +141,10 @@ impl AuthError {
|
|||
pub(crate) fn permanent(reason: RefreshTokenFailedReason) -> Self {
|
||||
AuthError::Refresh(RefreshTokenError::Permanent(reason.into()))
|
||||
}
|
||||
|
||||
/// Retryable refresh failure (network, 5xx, sleep/dark-wake defer, etc.).
|
||||
/// Permanent failures, NotLoggedIn, and policy rejects are not transient.
|
||||
pub(crate) fn is_transient(&self) -> bool {
|
||||
matches!(self, AuthError::Refresh(RefreshTokenError::Transient(_)))
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -701,9 +701,8 @@ pub(crate) async fn try_ensure_session_noninteractive(
|
|||
let grok_home = grok_home::grok_home();
|
||||
let auth_manager = Arc::new(AuthManager::new(&grok_home, grok_com_config.clone()));
|
||||
|
||||
// A refresh failure leaves the session on disk (credentials are retained;
|
||||
// the verdict gates re-attempts). Return it so consumers self-recover on
|
||||
// 401, rather than disabling the relay for the leader's lifetime.
|
||||
// Transient refresh failure: credentials remain (usable on 401 recovery).
|
||||
// Permanent failure already discarded them.
|
||||
if let Some(expired) = expired_refreshable_session(&auth_manager) {
|
||||
return Some(expired);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -478,6 +478,9 @@ impl AuthManager {
|
|||
);
|
||||
if scope == self.scope {
|
||||
self.clear_inner();
|
||||
// Intentional logout/scope removal: drop sticky permanent so the
|
||||
// next state is NotLoggedIn, not a retained invalid_grant verdict.
|
||||
*self.permanent_failure.write() = None;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -499,9 +502,9 @@ impl AuthManager {
|
|||
}
|
||||
}
|
||||
|
||||
/// Drop the in-memory auth. The sticky permanent-failure verdict is scoped
|
||||
/// to a credential key, so an empty cache reads through as "no failure"
|
||||
/// without explicit clearing.
|
||||
/// Drop the in-memory auth. Sticky `RefreshTokenRejected` still
|
||||
/// short-circuits with no live credential until a wire-valid login;
|
||||
/// non-sticky verdicts read absent once their scoped key is gone.
|
||||
fn clear_inner(&self) {
|
||||
*self.inner.write() = None;
|
||||
}
|
||||
|
|
@ -588,8 +591,11 @@ impl AuthManager {
|
|||
}
|
||||
|
||||
/// Drop the in-memory credentials, loudly. Logs the discard (with `reason`)
|
||||
/// before routing through [`clear_inner`] so the cached permanent_failure
|
||||
/// (if any) goes with them. Centralizes the "credentials gone" telemetry.
|
||||
/// before routing through [`clear_inner`]. Also clears a sticky permanent
|
||||
/// verdict so force-reload / disk-anomaly paths surface `NotLoggedIn`
|
||||
/// rather than a retained `invalid_grant`. Permanent discard after a live
|
||||
/// IdP rejection uses [`clear_inner`] alone so the sticky short-circuit
|
||||
/// survives until login.
|
||||
fn drop_in_memory_credentials(&self, reason: &str) {
|
||||
if let Some(d) = self.current_or_expired() {
|
||||
xai_grok_telemetry::unified_log::warn(
|
||||
|
|
@ -605,6 +611,7 @@ impl AuthManager {
|
|||
);
|
||||
}
|
||||
self.clear_inner();
|
||||
*self.permanent_failure.write() = None;
|
||||
}
|
||||
|
||||
// ── Read methods ─────────────────────────────────────────────────
|
||||
|
|
@ -851,6 +858,7 @@ impl AuthManager {
|
|||
// current session work with fresh credentials while the user fixes the
|
||||
// filesystem (e.g. read-only disk). Without this, a disk failure leaves
|
||||
// the stale/dead token in memory and the user is completely stuck.
|
||||
*self.permanent_failure.write() = None;
|
||||
self.with_inner_write(|inner| *inner = Some(auth.clone()));
|
||||
|
||||
// Fire-and-forget enrichment. Off the critical path -- a slow
|
||||
|
|
@ -908,6 +916,7 @@ impl AuthManager {
|
|||
),
|
||||
}
|
||||
// Always update in-memory, even if disk write failed (see update()).
|
||||
*self.permanent_failure.write() = None;
|
||||
self.with_inner_write(|inner| *inner = Some(auth.clone()));
|
||||
write_result?;
|
||||
Ok(auth)
|
||||
|
|
@ -962,13 +971,19 @@ impl AuthManager {
|
|||
}
|
||||
|
||||
/// Hot-swap credentials (called by config watcher). Does NOT write to disk.
|
||||
/// Clears a sticky permanent verdict only when the new bearer is wire-valid
|
||||
/// (login / sibling adopt). Hard-expired swaps keep the sticky short-circuit
|
||||
/// so a dead RT is not re-tried until a real login.
|
||||
pub(crate) fn hot_swap(&self, new_auth: GrokAuth) {
|
||||
if !self.is_token_hard_expired(&new_auth) {
|
||||
*self.permanent_failure.write() = None;
|
||||
}
|
||||
self.with_inner_write(|inner| *inner = Some(new_auth));
|
||||
}
|
||||
|
||||
/// Clear in-memory credentials. Does NOT touch disk, and does NOT clear the
|
||||
/// permanent-failure verdict: that is credential-scoped and self-invalidates
|
||||
/// on the next lookup once the credential it targets is gone.
|
||||
/// Clear in-memory credentials. Does NOT touch disk. Sticky
|
||||
/// `RefreshTokenRejected` remains until wire-valid login; other verdicts
|
||||
/// are key-scoped and drop out once their credential is gone.
|
||||
pub(crate) fn clear_in_memory(&self) {
|
||||
self.clear_inner();
|
||||
}
|
||||
|
|
@ -1053,23 +1068,23 @@ impl AuthManager {
|
|||
/// 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.
|
||||
///
|
||||
/// 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;
|
||||
};
|
||||
// Expired AT = dead sibling, not a live one. Disk may have
|
||||
// diverged from memory due to failed writes (e.g. disk full)
|
||||
// while both RTs are revoked.
|
||||
if self.is_token_expired(disk) {
|
||||
return false;
|
||||
}
|
||||
let disk_rt = disk.refresh_token.as_deref();
|
||||
let Some(disk_rt) = disk_rt else {
|
||||
let Some(disk_rt) = disk.refresh_token.as_deref() else {
|
||||
return false;
|
||||
};
|
||||
let mem_rt = self.expired_auth().and_then(|a| a.refresh_token);
|
||||
mem_rt.as_deref() != Some(disk_rt)
|
||||
let Some(mem_rt) = self.current_or_expired().and_then(|a| a.refresh_token) else {
|
||||
return false;
|
||||
};
|
||||
mem_rt.as_str() != disk_rt
|
||||
}
|
||||
|
||||
/// Re-read `auth.json` from disk without updating in-memory state.
|
||||
|
|
@ -1348,13 +1363,18 @@ impl AuthManager {
|
|||
{
|
||||
Ok(auth) => Ok(auth),
|
||||
Err(e) => {
|
||||
// Grace: the early-invalidation buffer is OUR
|
||||
// conservative estimate, not the IdP's actual
|
||||
// expiry. If the cached token is still wire-valid
|
||||
// ([`Self::is_token_hard_expired`]), return it so a
|
||||
// transient IdP blip during the buffer window
|
||||
// is invisible to the user.
|
||||
if let Some(auth) = snapshot
|
||||
// Grace for still wire-valid ATs on transient failures
|
||||
// and retain-path permanents (ClientRejected / Other).
|
||||
// RefreshTokenRejected discards AT+RT — never re-serve
|
||||
// that snapshot even when it is only soft-expired.
|
||||
let deny_grace = matches!(
|
||||
&e,
|
||||
AuthError::Refresh(crate::auth::RefreshTokenError::Permanent(pe))
|
||||
if pe.reason
|
||||
== crate::auth::error::RefreshTokenFailedReason::RefreshTokenRejected
|
||||
);
|
||||
if !deny_grace
|
||||
&& let Some(auth) = snapshot
|
||||
&& !self.is_token_hard_expired(&auth)
|
||||
{
|
||||
tracing::debug!(
|
||||
|
|
@ -1804,16 +1824,67 @@ impl AuthManager {
|
|||
) {
|
||||
return Ok(refreshed);
|
||||
}
|
||||
if self.sibling_has_different_refresh_token() {
|
||||
tracing::info!("auth: sibling-rotation detected; demoting to transient");
|
||||
return Err(AuthError::transient(format!("sibling-rotation: {error}")));
|
||||
}
|
||||
// No clear: the verdict (+ TTL) gates re-attempts; the dead
|
||||
// bearer is dropped only on explicit logout. Key on the
|
||||
// credential the refresher actually sent (`tried_key`), falling
|
||||
// back to our own resolution when the authority has no key.
|
||||
// Client contract: only genuine IdP RT rejection discards.
|
||||
// Escalated `Other` / `ClientRejected` retain credentials.
|
||||
// When mem and disk RTs diverge, clear only the side that was
|
||||
// actually tried so an untried successor RT (e.g. mem after a
|
||||
// disk-persist failure) is not wiped by a disk-first invalid_grant.
|
||||
let failed_reason = error.reason;
|
||||
if let Some(key) = tried_key.or(attempted_key) {
|
||||
let is_rtr = failed_reason
|
||||
== crate::auth::error::RefreshTokenFailedReason::RefreshTokenRejected;
|
||||
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() {
|
||||
tracing::info!("auth: sibling-rotation detected; demoting to transient");
|
||||
return Err(AuthError::transient(format!(
|
||||
"sibling-rotation: {failed_reason:?}"
|
||||
)));
|
||||
}
|
||||
let (clear_mem, clear_disk) = match (tried_key.as_ref(), &mem, &disk) {
|
||||
(Some(tk), m, d) => {
|
||||
let mem_match = m.as_ref().is_some_and(|a| a.key == *tk);
|
||||
let disk_match = d.as_ref().is_some_and(|a| a.key == *tk);
|
||||
if mem_match || disk_match {
|
||||
(mem_match, disk_match)
|
||||
} else {
|
||||
(true, true)
|
||||
}
|
||||
}
|
||||
(None, _, _) => (true, true),
|
||||
};
|
||||
if let Some(key) = tried_key.or(attempted_key) {
|
||||
self.record_permanent_failure(key, error);
|
||||
}
|
||||
let mut disk_mutation = "unchanged";
|
||||
if clear_disk {
|
||||
disk_mutation = match self.write_scope_removal(&self.scope) {
|
||||
Ok(m) => m.label(),
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
error = %e,
|
||||
"auth: failed to clear credentials after permanent refresh failure"
|
||||
);
|
||||
"write_failed"
|
||||
}
|
||||
};
|
||||
}
|
||||
if clear_mem {
|
||||
self.clear_inner();
|
||||
}
|
||||
xai_grok_telemetry::unified_log::warn(
|
||||
"auth: cleared credentials after permanent refresh failure",
|
||||
None,
|
||||
Some(serde_json::json!({
|
||||
"reason": format!("{failed_reason:?}"),
|
||||
"disk_mutation": disk_mutation,
|
||||
"cleared_mem": clear_mem,
|
||||
"cleared_disk": clear_disk,
|
||||
})),
|
||||
);
|
||||
} else if let Some(key) = tried_key.or(attempted_key) {
|
||||
self.record_permanent_failure(key, error);
|
||||
}
|
||||
Err(AuthError::permanent(failed_reason))
|
||||
|
|
@ -1905,6 +1976,16 @@ impl AuthManager {
|
|||
/// the common no-verdict case returns before any disk I/O; only a stored
|
||||
/// verdict triggers [`Self::attempted_verdict_key`]'s disk read.
|
||||
///
|
||||
/// After a permanent failure **discards** credentials, sticky reasons
|
||||
/// (`RefreshTokenRejected`) still short-circuit with no live credential so
|
||||
/// concurrent callers cannot re-hit the IdP with a dead RT. Login
|
||||
/// (`hot_swap` / `update`) and logout clear the verdict.
|
||||
///
|
||||
/// Sticky applies only to the **same** rejected key or to **no** live
|
||||
/// credential (post-discard). A different attempted key (sibling RT/AT
|
||||
/// on disk) must be allowed to refresh — otherwise a hard-expired sibling
|
||||
/// AT strands a process that could still refresh a live RT.
|
||||
///
|
||||
/// TTL expiry is judged on *both* clocks (see [`GateRaise`]): the monotonic
|
||||
/// clock pauses during a system suspend, so a wall-clock arm is required
|
||||
/// for the TTL to elapse across sleep. Without it, a recoverable failure
|
||||
|
|
@ -1929,8 +2010,14 @@ impl AuthManager {
|
|||
// would attempt. Guard dropped above so `inner` isn't co-held.
|
||||
// Deliberately `ServerRejected` (the widest resolution) regardless of
|
||||
// the caller's reason, so the read never misses a stored verdict.
|
||||
(self.attempted_verdict_key(RefreshReason::ServerRejected)? == token_key)
|
||||
.then(|| AuthError::permanent(reason))
|
||||
match self.attempted_verdict_key(RefreshReason::ServerRejected) {
|
||||
Some(k) if k == token_key => Some(AuthError::permanent(reason)),
|
||||
// Different credential key: never sticky-block a sibling RT.
|
||||
Some(_) => None,
|
||||
// No live credential after discard: sticky short-circuit until login.
|
||||
None if reason.is_sticky() => Some(AuthError::permanent(reason)),
|
||||
None => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// `true` iff [`Self::permanent_failure`] has a non-expired entry. Lets
|
||||
|
|
@ -1990,17 +2077,51 @@ impl AuthManager {
|
|||
crate::auth::recovery::UnauthorizedRecovery::new(self.clone(), rejected, source)
|
||||
}
|
||||
|
||||
/// One-shot 401 recovery off the live bearer, snapshotted once so the
|
||||
/// rejected key and KPI attribution describe one credential.
|
||||
/// 401 recovery off the live bearer. Snapshots the rejected credential once
|
||||
/// for KPI attribution. On **transient** refresh failure (network, 5xx,
|
||||
/// sleep/dark-wake defer, lock timeout) retries with backoff before giving
|
||||
/// up. Permanent failures and NotLoggedIn stop immediately.
|
||||
///
|
||||
/// After a successful recovery the **caller** retries the original request
|
||||
/// (turn-level may resubmit more than once; API resubmit is separate from
|
||||
/// refresh retries).
|
||||
pub(crate) async fn try_recover_unauthorized(
|
||||
self: &Arc<Self>,
|
||||
source: crate::auth::recovery::RecoverySource,
|
||||
) -> bool {
|
||||
/// Bounded refresh attempts for non-permanent failures. Kept strictly
|
||||
/// below OidcRefresher's consecutive-transient escalation threshold so
|
||||
/// one 401 recovery cannot alone escalate a network blip to permanent
|
||||
/// `Other`.
|
||||
const MAX_TRANSIENT_ATTEMPTS: u32 = 2;
|
||||
|
||||
let cached = self.with_inner_read(|inner| inner.cloned());
|
||||
self.unauthorized_recovery(cached, source)
|
||||
.next()
|
||||
.await
|
||||
.is_ok()
|
||||
let mut delay = StdDuration::from_millis(500);
|
||||
for attempt in 0..MAX_TRANSIENT_ATTEMPTS {
|
||||
match self
|
||||
.unauthorized_recovery(cached.clone(), source)
|
||||
.next()
|
||||
.await
|
||||
{
|
||||
Ok(_) => return true,
|
||||
Err(e) if e.is_transient() && attempt + 1 < MAX_TRANSIENT_ATTEMPTS => {
|
||||
xai_grok_telemetry::unified_log::warn(
|
||||
"auth recovery: transient failure, retrying",
|
||||
None,
|
||||
Some(serde_json::json!({
|
||||
"attempt": attempt + 1,
|
||||
"max_attempts": MAX_TRANSIENT_ATTEMPTS,
|
||||
"delay_ms": delay.as_millis() as u64,
|
||||
"error": format!("{e}"),
|
||||
})),
|
||||
);
|
||||
tokio::time::sleep(delay).await;
|
||||
delay = (delay.saturating_mul(2)).min(StdDuration::from_secs(4));
|
||||
}
|
||||
Err(_) => return false,
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
pub(crate) fn record_manual_auth(
|
||||
|
|
@ -2335,6 +2456,12 @@ impl AuthManager {
|
|||
let key = key.map(|k| k.trim().to_string()).filter(|k| !k.is_empty());
|
||||
*self.process_static_api_key.write() = key;
|
||||
}
|
||||
|
||||
/// Static/BYOK key for export paths (e.g. desktop `getBearerToken`). Never a
|
||||
/// session JWT; respects kill-switch and preferred-method pin.
|
||||
pub(crate) fn static_api_key_for_export(&self) -> Option<String> {
|
||||
resolve_static_api_key(self)
|
||||
}
|
||||
}
|
||||
|
||||
fn non_empty_key(key: Option<String>) -> Option<String> {
|
||||
|
|
|
|||
|
|
@ -729,25 +729,6 @@ fn record_permanent_failure(
|
|||
auth_manager.record_permanent_failure(key, reason.into());
|
||||
}
|
||||
|
||||
/// Permanent-failure refresher that reports a specific `tried_key` (the
|
||||
/// credential it claims to have sent to the IdP), letting tests assert the
|
||||
/// verdict is keyed on the actually-tried credential.
|
||||
struct TriedKeyFailRefresher {
|
||||
tried_key: String,
|
||||
call_count: Arc<AtomicU32>,
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl TokenRefresher for TriedKeyFailRefresher {
|
||||
async fn refresh(&self, _reason: RefreshReason) -> crate::auth::refresh::RefreshOutcome {
|
||||
self.call_count.fetch_add(1, Ordering::SeqCst);
|
||||
crate::auth::refresh::RefreshOutcome::permanent(
|
||||
crate::auth::error::RefreshTokenFailedReason::RefreshTokenRejected,
|
||||
Some(self.tried_key.clone()),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/// With `inner == None` but a dead refresh-token on disk, the refresher still
|
||||
/// exchanges that disk RT. The verdict must be keyed on the
|
||||
/// credential actually tried (the disk RT), so repeated reactive refreshes
|
||||
|
|
@ -791,10 +772,10 @@ async fn storm_cap_engages_with_empty_inner_and_dead_disk_refresh_token() {
|
|||
}
|
||||
|
||||
/// Record/check consistency: in-mem and disk are DIFFERENT stale credentials.
|
||||
/// The refresher resolves & sends the DISK refresh token, so the verdict must be
|
||||
/// keyed on THAT — proven by swapping the in-mem bearer afterward and confirming
|
||||
/// the verdict still caps the storm (a verdict mis-keyed to the in-mem bearer
|
||||
/// would read absent after the swap and re-hit the IdP). The `tried_key == None`
|
||||
/// The refresher reports `tried_key = disk`; with a retain-path permanent
|
||||
/// (`ClientRejected`) credentials stay, so the verdict stays scoped to disk.
|
||||
/// Swapping the in-mem bearer must not re-open the IdP (a verdict mis-keyed to
|
||||
/// the in-mem bearer would read absent after the swap). The `tried_key == None`
|
||||
/// fallback (external-binary flow → `attempted_verdict_key`) is covered by
|
||||
/// `storm_cap_engages_with_empty_inner_and_dead_disk_refresh_token`.
|
||||
#[tokio::test]
|
||||
|
|
@ -813,7 +794,7 @@ async fn verdict_not_keyed_on_in_mem_bearer() {
|
|||
..GrokAuth::test_default()
|
||||
});
|
||||
// disk: a DIFFERENT stale credential K_disk (expired, with RT) — what the
|
||||
// refresher resolves first.
|
||||
// refresher claims to have tried.
|
||||
let disk = GrokAuth {
|
||||
key: "disk-stale".into(),
|
||||
auth_mode: AuthMode::Oidc,
|
||||
|
|
@ -826,7 +807,23 @@ async fn verdict_not_keyed_on_in_mem_bearer() {
|
|||
write_auth_json(&dir.path().join("auth.json"), &store).unwrap();
|
||||
|
||||
let calls = Arc::new(AtomicU32::new(0));
|
||||
mgr.set_refresher(Arc::new(TriedKeyFailRefresher {
|
||||
struct TriedKeyClientRejected {
|
||||
tried_key: String,
|
||||
call_count: Arc<AtomicU32>,
|
||||
}
|
||||
#[async_trait::async_trait]
|
||||
impl TokenRefresher for TriedKeyClientRejected {
|
||||
async fn refresh(&self, _reason: RefreshReason) -> crate::auth::refresh::RefreshOutcome {
|
||||
self.call_count.fetch_add(1, Ordering::SeqCst);
|
||||
// ClientRejected retains credentials (unlike RefreshTokenRejected),
|
||||
// so the disk-scoped verdict remains the storm cap after mem swap.
|
||||
crate::auth::refresh::RefreshOutcome::permanent(
|
||||
crate::auth::error::RefreshTokenFailedReason::ClientRejected,
|
||||
Some(self.tried_key.clone()),
|
||||
)
|
||||
}
|
||||
}
|
||||
mgr.set_refresher(Arc::new(TriedKeyClientRejected {
|
||||
tried_key: "disk-stale".into(),
|
||||
call_count: calls.clone(),
|
||||
}));
|
||||
|
|
@ -839,6 +836,10 @@ async fn verdict_not_keyed_on_in_mem_bearer() {
|
|||
1,
|
||||
"first call hits the IdP once"
|
||||
);
|
||||
assert!(
|
||||
mgr.read_disk_auth().is_some(),
|
||||
"ClientRejected must retain the disk credential the verdict is keyed on",
|
||||
);
|
||||
|
||||
// Swap the in-mem bearer to yet another stale key: a verdict mis-keyed to
|
||||
// the old in-mem bearer would now read absent.
|
||||
|
|
@ -1197,8 +1198,8 @@ async fn proactive_refresh_backs_off_on_permanent_failure() {
|
|||
failure is recorded, got {after_failure} calls"
|
||||
);
|
||||
assert!(
|
||||
mgr.permanent_failure().is_some(),
|
||||
"permanent failure must be cached after invalid_grant",
|
||||
mgr.current_or_expired().is_none(),
|
||||
"permanent refresh failure must clear credentials",
|
||||
);
|
||||
// The proactive (background) loop must never emit the manual_auth KPI:
|
||||
// a background failure is not a user-facing forced re-login.
|
||||
|
|
@ -1322,10 +1323,10 @@ async fn reactive_401_recovery_produces_fresh_token_end_to_end() {
|
|||
// refresh_chain permanent-failure short-circuit via recovery is tested
|
||||
// in recovery::tests::refresh_authority_short_circuits_on_cached_permanent_failure.
|
||||
|
||||
/// Different disk RT with expired AT: PermanentFailure is recorded
|
||||
/// (not demoted to transient), stopping the retry loop.
|
||||
/// Different disk RT with expired AT: demote to transient so a sibling's
|
||||
/// still-usable RT is not wiped by permanent clear.
|
||||
#[tokio::test]
|
||||
async fn refresh_chain_records_permanent_failure_when_disk_rt_differs_but_at_expired() {
|
||||
async fn refresh_chain_demotes_when_disk_rt_differs_even_if_at_expired() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let cfg = GrokComConfig::default();
|
||||
let scope = cfg.auth_scope();
|
||||
|
|
@ -1355,7 +1356,7 @@ async fn refresh_chain_records_permanent_failure_when_disk_rt_differs_but_at_exp
|
|||
..GrokAuth::test_default()
|
||||
};
|
||||
let mut store = AuthStore::new();
|
||||
store.insert(scope, sibling);
|
||||
store.insert(scope, sibling.clone());
|
||||
write_auth_json(&dir.path().join("auth.json"), &store).unwrap();
|
||||
|
||||
struct FailingRefresher;
|
||||
|
|
@ -1374,28 +1375,254 @@ async fn refresh_chain_records_permanent_failure_when_disk_rt_differs_but_at_exp
|
|||
mgr.set_refresher(Arc::new(FailingRefresher));
|
||||
|
||||
let err = mgr.auth().await.unwrap_err();
|
||||
// An expired disk AT means the sibling is dead too — the failure is
|
||||
// permanent (not demoted to transient). Credentials are retained; the
|
||||
// scoped verdict is cached and stops the retry storm.
|
||||
assert!(
|
||||
matches!(err, AuthError::Refresh(RefreshTokenError::Transient(_))),
|
||||
"disk RT mismatch must demote even when sibling AT is expired, got: {err:?}",
|
||||
);
|
||||
assert_eq!(
|
||||
mgr.read_disk_auth().and_then(|a| a.refresh_token),
|
||||
Some("rt-new".into()),
|
||||
"sibling RT on disk must not be wiped when AT is only expired",
|
||||
);
|
||||
assert!(
|
||||
mgr.permanent_failure().is_none(),
|
||||
"demotion must not record a sticky permanent 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]
|
||||
async fn permanent_rtr_clears_only_the_tried_side_when_rts_diverge() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let cfg = GrokComConfig::default();
|
||||
let scope = cfg.auth_scope();
|
||||
let mgr = Arc::new(AuthManager::new(dir.path(), cfg));
|
||||
|
||||
// Mem: successor RT after a successful refresh whose disk write failed.
|
||||
mgr.hot_swap(GrokAuth {
|
||||
key: "mem-successor".into(),
|
||||
auth_mode: AuthMode::Oidc,
|
||||
refresh_token: Some("rt-new".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()
|
||||
});
|
||||
// Disk: revoked predecessor RT (disk-first resolve will try this).
|
||||
let disk = GrokAuth {
|
||||
key: "disk-predecessor".into(),
|
||||
auth_mode: AuthMode::Oidc,
|
||||
refresh_token: Some("rt-old".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()
|
||||
};
|
||||
let mut store = AuthStore::new();
|
||||
store.insert(scope, disk);
|
||||
write_auth_json(&dir.path().join("auth.json"), &store).unwrap();
|
||||
|
||||
let calls = Arc::new(AtomicU32::new(0));
|
||||
struct TriedDiskRtr(Arc<AtomicU32>);
|
||||
#[async_trait::async_trait]
|
||||
impl TokenRefresher for TriedDiskRtr {
|
||||
async fn refresh(
|
||||
&self,
|
||||
_reason: crate::auth::manager::RefreshReason,
|
||||
) -> crate::auth::refresh::RefreshOutcome {
|
||||
self.0.fetch_add(1, Ordering::SeqCst);
|
||||
crate::auth::refresh::RefreshOutcome::permanent(
|
||||
crate::auth::error::RefreshTokenFailedReason::RefreshTokenRejected,
|
||||
Some("disk-predecessor".into()),
|
||||
)
|
||||
}
|
||||
}
|
||||
mgr.set_refresher(Arc::new(TriedDiskRtr(calls.clone())));
|
||||
|
||||
let err = mgr
|
||||
.refresh_chain(TokenType::OidcSession, RefreshReason::ServerRejected)
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
matches!(err, AuthError::Refresh(RefreshTokenError::Permanent(_))),
|
||||
"must surface a permanent failure when disk AT is expired, got: {err:?}",
|
||||
"must surface permanent for the tried disk RT, got: {err:?}",
|
||||
);
|
||||
assert!(
|
||||
mgr.permanent_failure().is_some(),
|
||||
"verdict must be cached (scoped to the retained credential)",
|
||||
mgr.read_disk_auth().is_none(),
|
||||
"rejected disk predecessor must be cleared",
|
||||
);
|
||||
assert_eq!(
|
||||
mgr.current_or_expired().and_then(|a| a.refresh_token),
|
||||
Some("rt-new".into()),
|
||||
"untried in-memory successor RT must not be wiped",
|
||||
);
|
||||
assert_eq!(calls.load(Ordering::SeqCst), 1);
|
||||
}
|
||||
|
||||
/// Retain-path permanent (ClientRejected) still graces a soft-expired wire-valid AT.
|
||||
#[tokio::test]
|
||||
async fn client_rejected_graces_soft_expired_access_token() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mgr = Arc::new(AuthManager::new(dir.path(), GrokComConfig::default()));
|
||||
|
||||
// Inside the early-invalidation buffer but still hard-valid.
|
||||
mgr.hot_swap(GrokAuth {
|
||||
key: "buffered-at".into(),
|
||||
auth_mode: AuthMode::Oidc,
|
||||
refresh_token: Some("rt".into()),
|
||||
expires_at: Some(Utc::now() + Duration::seconds(30)),
|
||||
oidc_issuer: Some("https://issuer.example".into()),
|
||||
oidc_client_id: Some("client-1".into()),
|
||||
..GrokAuth::test_default()
|
||||
});
|
||||
|
||||
struct AlwaysClientRejected;
|
||||
#[async_trait::async_trait]
|
||||
impl TokenRefresher for AlwaysClientRejected {
|
||||
async fn refresh(
|
||||
&self,
|
||||
_reason: crate::auth::manager::RefreshReason,
|
||||
) -> crate::auth::refresh::RefreshOutcome {
|
||||
crate::auth::refresh::RefreshOutcome::permanent(
|
||||
crate::auth::error::RefreshTokenFailedReason::ClientRejected,
|
||||
Some("buffered-at".into()),
|
||||
)
|
||||
}
|
||||
}
|
||||
mgr.set_refresher(Arc::new(AlwaysClientRejected));
|
||||
|
||||
let auth = mgr
|
||||
.auth()
|
||||
.await
|
||||
.expect("retain-path permanent must grace wire-valid AT");
|
||||
assert_eq!(auth.key, "buffered-at");
|
||||
assert!(
|
||||
mgr.current_or_expired().is_some(),
|
||||
"ClientRejected must retain credentials",
|
||||
);
|
||||
}
|
||||
|
||||
/// Escalated permanent `Other` retains AT+RT (only RefreshTokenRejected discards).
|
||||
#[tokio::test]
|
||||
async fn permanent_other_retains_credentials() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mgr = Arc::new(AuthManager::new(dir.path(), GrokComConfig::default()));
|
||||
|
||||
let session = GrokAuth {
|
||||
key: "live-key".into(),
|
||||
auth_mode: AuthMode::Oidc,
|
||||
refresh_token: Some("rt-still-valid".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(session.clone());
|
||||
// Persist so disk clear would be observable.
|
||||
let mut store = AuthStore::new();
|
||||
store.insert(GrokComConfig::default().auth_scope(), session);
|
||||
write_auth_json(&dir.path().join("auth.json"), &store).unwrap();
|
||||
|
||||
struct OtherPermanent;
|
||||
#[async_trait::async_trait]
|
||||
impl crate::auth::refresh::TokenRefresher for OtherPermanent {
|
||||
async fn refresh(
|
||||
&self,
|
||||
_reason: crate::auth::manager::RefreshReason,
|
||||
) -> crate::auth::refresh::RefreshOutcome {
|
||||
crate::auth::refresh::RefreshOutcome::permanent(
|
||||
crate::auth::error::RefreshTokenFailedReason::Other,
|
||||
Some("live-key".into()),
|
||||
)
|
||||
}
|
||||
}
|
||||
mgr.set_refresher(Arc::new(OtherPermanent));
|
||||
|
||||
let err = mgr.auth().await.unwrap_err();
|
||||
assert!(
|
||||
matches!(err, AuthError::Refresh(RefreshTokenError::Permanent(_))),
|
||||
"escalated Other must still surface permanent, got: {err:?}",
|
||||
);
|
||||
// No-clear invariant: a refresh failure must NOT delete auth.json (a future
|
||||
// regression that re-adds disk-clear-on-invalid_grant would fail here).
|
||||
assert!(
|
||||
mgr.read_disk_auth().is_some(),
|
||||
"invalid_grant must not delete auth.json (no auto-clear)",
|
||||
"Other must not clear disk credentials",
|
||||
);
|
||||
// Second attempt short-circuits on the cached verdict — no extra IdP call.
|
||||
assert!(matches!(
|
||||
mgr.auth().await.unwrap_err(),
|
||||
AuthError::Refresh(RefreshTokenError::Permanent(_))
|
||||
));
|
||||
assert_eq!(
|
||||
mgr.current_or_expired().and_then(|a| a.refresh_token),
|
||||
Some("rt-still-valid".into()),
|
||||
"Other must retain in-memory RT",
|
||||
);
|
||||
}
|
||||
|
||||
/// Sticky permanent must not block a different credential key (sibling RT).
|
||||
#[tokio::test]
|
||||
async fn sticky_permanent_allows_refresh_when_attempted_key_differs() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let cfg = GrokComConfig::default();
|
||||
let scope = cfg.auth_scope();
|
||||
let mgr = Arc::new(AuthManager::new(dir.path(), cfg));
|
||||
|
||||
mgr.hot_swap(GrokAuth {
|
||||
key: "dead-key".into(),
|
||||
auth_mode: AuthMode::Oidc,
|
||||
refresh_token: Some("rt-dead".into()),
|
||||
expires_at: Some(Utc::now() - Duration::hours(1)),
|
||||
..GrokAuth::test_default()
|
||||
});
|
||||
record_permanent_failure(
|
||||
&mgr,
|
||||
crate::auth::error::RefreshTokenFailedReason::RefreshTokenRejected,
|
||||
);
|
||||
assert!(mgr.permanent_failure().is_some());
|
||||
|
||||
// Sibling writes a different key + RT (AT hard-expired, RT may still work).
|
||||
let sibling = GrokAuth {
|
||||
key: "sibling-key".into(),
|
||||
auth_mode: AuthMode::Oidc,
|
||||
refresh_token: Some("rt-sibling".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.clone());
|
||||
write_auth_json(&dir.path().join("auth.json"), &store).unwrap();
|
||||
// Load sibling into memory without clearing sticky via wire-valid hot_swap.
|
||||
mgr.with_inner_write(|inner| *inner = Some(sibling));
|
||||
|
||||
assert!(
|
||||
mgr.permanent_failure().is_none(),
|
||||
"sticky verdict must not apply to a different credential key",
|
||||
);
|
||||
|
||||
let calls = Arc::new(AtomicU32::new(0));
|
||||
struct CountingOk(Arc<AtomicU32>);
|
||||
#[async_trait::async_trait]
|
||||
impl crate::auth::refresh::TokenRefresher for CountingOk {
|
||||
async fn refresh(
|
||||
&self,
|
||||
_reason: crate::auth::manager::RefreshReason,
|
||||
) -> crate::auth::refresh::RefreshOutcome {
|
||||
self.0.fetch_add(1, Ordering::SeqCst);
|
||||
crate::auth::refresh::RefreshOutcome::Success(Box::new(GrokAuth {
|
||||
key: "fresh-from-sibling-rt".into(),
|
||||
auth_mode: AuthMode::Oidc,
|
||||
refresh_token: Some("rt-sibling".into()),
|
||||
expires_at: Some(Utc::now() + Duration::hours(1)),
|
||||
..GrokAuth::test_default()
|
||||
}))
|
||||
}
|
||||
}
|
||||
mgr.set_refresher(Arc::new(CountingOk(calls.clone())));
|
||||
|
||||
let auth = mgr
|
||||
.auth()
|
||||
.await
|
||||
.expect("sibling key must reach refresh_chain");
|
||||
assert_eq!(auth.key, "fresh-from-sibling-rt");
|
||||
assert_eq!(calls.load(Ordering::SeqCst), 1);
|
||||
}
|
||||
|
||||
/// Different disk RT with valid AT: adopt the sibling's token directly.
|
||||
|
|
@ -1514,10 +1741,15 @@ async fn permanent_failure_reads_absent_after_clear_so_auth_reports_not_logged_i
|
|||
crate::auth::error::RefreshTokenFailedReason::RefreshTokenRejected,
|
||||
);
|
||||
mgr.clear_in_memory();
|
||||
// clear_in_memory drops the credential but keeps a sticky permanent
|
||||
// verdict so a just-revoked RT is not re-tried until login.
|
||||
let err = mgr.auth().await.unwrap_err();
|
||||
assert!(
|
||||
matches!(err, AuthError::NotLoggedIn),
|
||||
"auth() after hot_swap_clear() must report NotLoggedIn, got: {err:?}",
|
||||
matches!(
|
||||
err,
|
||||
AuthError::Refresh(RefreshTokenError::Permanent(_)) | AuthError::NotLoggedIn
|
||||
),
|
||||
"auth() after clear_in_memory must not re-hit a dead RT, got: {err:?}",
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -2629,9 +2861,10 @@ async fn update_recovers_from_whitespace_only_auth_json() {
|
|||
|
||||
// -- sibling_has_different_refresh_token ----------------------------------
|
||||
|
||||
/// Expired disk AT with different RT is not a live sibling.
|
||||
/// Expired disk AT with different RT is still treated as a sibling RT
|
||||
/// (may still be refreshable; must not be wiped by permanent clear).
|
||||
#[tokio::test]
|
||||
async fn sibling_different_rt_with_expired_at_is_not_treated_as_live() {
|
||||
async fn sibling_different_rt_with_expired_at_is_still_sibling() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let cfg = GrokComConfig::default();
|
||||
let mgr = Arc::new(AuthManager::new(dir.path(), cfg.clone()));
|
||||
|
|
@ -2659,8 +2892,8 @@ async fn sibling_different_rt_with_expired_at_is_not_treated_as_live() {
|
|||
write_auth_json(&dir.path().join("auth.json"), &store).unwrap();
|
||||
|
||||
assert!(
|
||||
!mgr.sibling_has_different_refresh_token(),
|
||||
"expired disk token must not be treated as a live sibling"
|
||||
mgr.sibling_has_different_refresh_token(),
|
||||
"different disk RT must demote even when the sibling AT is expired"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -319,9 +319,11 @@ async fn auth_backend_contract_transient_failures_escalate_to_non_sticky_permane
|
|||
auth_manager.hot_swap(expired_oidc(&base_url));
|
||||
|
||||
// One refresher instance: it owns the consecutive-failure counter.
|
||||
// Budget is above try_recover_unauthorized's per-recovery attempts so a
|
||||
// single 401 recovery cannot alone escalate; exhaust the full budget here.
|
||||
let refresher = OidcRefresher::new(auth_manager.clone());
|
||||
let mut outcomes = Vec::new();
|
||||
for _ in 0..3 {
|
||||
for _ in 0..5 {
|
||||
outcomes.push(refresher.refresh(RefreshReason::ServerRejected).await);
|
||||
}
|
||||
|
||||
|
|
@ -330,7 +332,12 @@ async fn auth_backend_contract_transient_failures_escalate_to_non_sticky_permane
|
|||
"first blip is transient, not a lockout: {:?}",
|
||||
outcomes[0],
|
||||
);
|
||||
match &outcomes[2] {
|
||||
assert!(
|
||||
matches!(outcomes[3], RefreshOutcome::TransientFailure { .. }),
|
||||
"4th blip still under escalation budget: {:?}",
|
||||
outcomes[3],
|
||||
);
|
||||
match &outcomes[4] {
|
||||
RefreshOutcome::PermanentFailure { error, .. } => {
|
||||
assert_eq!(
|
||||
error.reason,
|
||||
|
|
|
|||
|
|
@ -94,8 +94,9 @@ pub(crate) enum RefreshOutcome {
|
|||
Success(Box<GrokAuth>),
|
||||
/// Terminal failure (e.g. invalid_grant), or a transient escalated to
|
||||
/// `Other` after repeated blips. Caller records a verdict scoped to the
|
||||
/// rejected credential and retains it (`RefreshTokenRejected` is sticky,
|
||||
/// the rest age out past the TTL).
|
||||
/// rejected credential. `refresh_chain` discards AT+RT only for
|
||||
/// `RefreshTokenRejected` (sticky until login); `ClientRejected` / `Other`
|
||||
/// retain credentials and age out past the TTL.
|
||||
PermanentFailure {
|
||||
error: crate::auth::error::RefreshTokenFailedError,
|
||||
/// Key of the credential the refresher actually sent to the IdP, so
|
||||
|
|
|
|||
|
|
@ -13,8 +13,9 @@ use crate::auth::manager::AuthManager;
|
|||
/// Escalate to `PermanentFailure` after this many consecutive transient
|
||||
/// failures (then `PERMANENT_FAILURE_TTL` allows recovery). OIDC tolerates more
|
||||
/// blips than `ExternalBinaryRefresher` (1) since network refreshes flake more
|
||||
/// than a local binary.
|
||||
const MAX_CONSECUTIVE_TRANSIENT_FAILURES: u32 = 3;
|
||||
/// than a local binary. Kept above `try_recover_unauthorized`'s per-recovery
|
||||
/// attempt budget so one 401 recovery cannot alone escalate.
|
||||
const MAX_CONSECUTIVE_TRANSIENT_FAILURES: u32 = 5;
|
||||
|
||||
/// Consecutive transient-failure budget, scoped to the credential it accrued
|
||||
/// against. Held under one lock so the credential check, reset, and increment
|
||||
|
|
|
|||
|
|
@ -301,26 +301,29 @@ async fn oidc_refresher_e2e_near_expiry_idp_rejects_refresh() {
|
|||
};
|
||||
mgr.hot_swap(near_expiry);
|
||||
|
||||
// auth() dispatches to refresh_chain -> OidcRefresher -> invalid_grant.
|
||||
// Because the token is still within real expires_at (3 min from now),
|
||||
// the grace path returns the cached token as a fallback.
|
||||
// Permanent invalid_grant discards AT+RT (no grace re-serve of pre-refresh
|
||||
// snapshot). Grace remains for *transient* refresh failures only.
|
||||
mgr.set_refresher(std::sync::Arc::new(OidcRefresher::new(mgr.clone())));
|
||||
let refreshed = mgr.auth().await;
|
||||
let err = mgr.auth().await.unwrap_err();
|
||||
assert!(
|
||||
refreshed.is_ok(),
|
||||
"grace path should return the cached token while within real expires_at"
|
||||
matches!(
|
||||
err,
|
||||
crate::auth::AuthError::Refresh(crate::auth::RefreshTokenError::Permanent(_))
|
||||
),
|
||||
"permanent invalid_grant must not grace-serve the pre-refresh AT, got: {err:?}",
|
||||
);
|
||||
assert!(
|
||||
mgr.current_or_expired().is_none(),
|
||||
"permanent invalid_grant must clear credentials",
|
||||
);
|
||||
assert_eq!(refreshed.unwrap().key, "about-to-expire-token");
|
||||
|
||||
server.abort();
|
||||
}
|
||||
|
||||
/// On `invalid_client` (client_id rotated, soft-deleted, or disabled), the
|
||||
/// credential is retained and a permanent-failure verdict cached. Verdict + TTL
|
||||
/// stop the retry loop; the bearer drops only on explicit logout, so a
|
||||
/// transient client-rotation blip self-heals without a fleet re-login.
|
||||
/// On `invalid_client` (client_id rotated, soft-deleted, or disabled) with a
|
||||
/// hard-expired AT, permanent failure retains AT+RT (only invalid_grant discards).
|
||||
#[tokio::test]
|
||||
async fn oidc_refresher_e2e_invalid_client_caches_verdict_and_retains_credentials() {
|
||||
async fn oidc_refresher_e2e_invalid_client_retains_credentials() {
|
||||
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();
|
||||
|
|
@ -371,34 +374,25 @@ async fn oidc_refresher_e2e_invalid_client_caches_verdict_and_retains_credential
|
|||
mgr.hot_swap(expired);
|
||||
|
||||
mgr.set_refresher(std::sync::Arc::new(OidcRefresher::new(mgr.clone())));
|
||||
let refreshed = mgr.auth().await.ok();
|
||||
let err = mgr.auth().await.unwrap_err();
|
||||
assert!(
|
||||
refreshed.is_none(),
|
||||
"refresh should fail when client is unknown"
|
||||
matches!(
|
||||
err,
|
||||
crate::auth::AuthError::Refresh(crate::auth::RefreshTokenError::Permanent(_))
|
||||
),
|
||||
"refresh should fail permanently when client is unknown, got: {err:?}",
|
||||
);
|
||||
assert_eq!(
|
||||
mgr.current_or_expired()
|
||||
.and_then(|a| a.refresh_token)
|
||||
.as_deref(),
|
||||
Some("rt-valid"),
|
||||
"invalid_client must retain RT for TTL-gated retry after client rotation",
|
||||
);
|
||||
|
||||
// Credential retained (not cleared) — the bearer may be fine; the client
|
||||
// credential isn't.
|
||||
assert!(
|
||||
mgr.expired_auth().is_some(),
|
||||
"credentials must be retained after invalid_client",
|
||||
mgr.read_disk_auth().is_some() || mgr.current_or_expired().is_some(),
|
||||
"invalid_client must not clear credentials",
|
||||
);
|
||||
// The verdict is cached, scoped to the retained credential, and carries
|
||||
// the non-sticky `ClientRejected` reason (so it ages out, not stuck-forever).
|
||||
match mgr.permanent_failure() {
|
||||
Some(crate::auth::AuthError::Refresh(crate::auth::RefreshTokenError::Permanent(e))) => {
|
||||
assert_eq!(
|
||||
e.reason,
|
||||
crate::auth::RefreshTokenFailedReason::ClientRejected,
|
||||
"invalid_client must map to ClientRejected",
|
||||
);
|
||||
assert!(
|
||||
!e.reason.is_sticky(),
|
||||
"ClientRejected must age out past the TTL, not stick forever",
|
||||
);
|
||||
}
|
||||
other => panic!("invalid_client must cache a permanent-failure verdict, got {other:?}"),
|
||||
}
|
||||
|
||||
server.abort();
|
||||
}
|
||||
|
|
@ -990,22 +984,18 @@ async fn refresher_disk_retry_invalid_client_with_different_client_id_preserves_
|
|||
other => panic!("expected PermanentFailure, got: {other:?}"),
|
||||
}
|
||||
|
||||
// Credential retained; the cached verdict (scoped to it) stops the storm.
|
||||
// Disk-retry already tried the sibling RT and got invalid_client —
|
||||
// permanent is recorded, but ClientRejected retains credentials.
|
||||
assert!(
|
||||
mgr.current_or_expired().is_some(),
|
||||
"credential must be retained on permanent failure"
|
||||
);
|
||||
assert!(
|
||||
mgr.permanent_failure().is_some(),
|
||||
"verdict must be cached to stop the retry storm"
|
||||
mgr.current_or_expired().is_some() || mgr.read_disk_auth().is_some(),
|
||||
"invalid_client permanent must retain credentials (only invalid_grant discards)"
|
||||
);
|
||||
assert_eq!(attempts.load(Ordering::SeqCst), 2, "no recursion");
|
||||
|
||||
server.abort();
|
||||
}
|
||||
|
||||
/// Both RTs revoked: retry is strictly one-shot (no third call);
|
||||
/// refresh_chain's disk-RT-differs guard preserves disk creds.
|
||||
/// Both RTs revoked: retry is strictly one-shot (no third call).
|
||||
#[tokio::test]
|
||||
async fn refresher_disk_retry_is_one_shot() {
|
||||
use std::sync::atomic::{AtomicU32, Ordering};
|
||||
|
|
@ -1060,7 +1050,8 @@ async fn refresher_disk_retry_is_one_shot() {
|
|||
"exactly two IdP calls — disk-token retry must NOT recurse"
|
||||
);
|
||||
|
||||
// Disk auth must still be present (the refresher never clears).
|
||||
// This test calls the refresher directly (not refresh_chain); disk is
|
||||
// unchanged here — refresh_chain is responsible for permanent clear.
|
||||
assert!(
|
||||
mgr.read_disk_auth().is_some(),
|
||||
"refresher must not touch disk; clearing is refresh_chain's responsibility"
|
||||
|
|
|
|||
|
|
@ -48,14 +48,17 @@ fn handle_cancel(agent: &MvpAgent, args: &acp::ExtRequest) -> ExtResult {
|
|||
}
|
||||
|
||||
async fn handle_get_bearer_token(agent: &MvpAgent) -> ExtResult {
|
||||
// Fail closed for session tokens: desktop resume treats non-null as success.
|
||||
// Never return a hard-expired AT. Still surface wire-valid session ATs and
|
||||
// static/BYOK keys (process model key / env / disk api_key) so non-session
|
||||
// sessions keep working when AuthManager has no OIDC entry.
|
||||
let token = match agent.auth_manager.get_valid_token().await {
|
||||
Ok(token) => Some(token),
|
||||
Err(_) => agent
|
||||
.sampling_config
|
||||
.borrow()
|
||||
.api_key
|
||||
.clone()
|
||||
.or_else(|| agent.auth_manager.current().map(|a| a.key)),
|
||||
.auth_manager
|
||||
.current_wire_valid()
|
||||
.map(|a| a.key)
|
||||
.or_else(|| agent.auth_manager.static_api_key_for_export()),
|
||||
};
|
||||
ExtMethodResult::success(serde_json::json!({ "token": token }))
|
||||
.to_ext_response()
|
||||
|
|
|
|||
|
|
@ -59,8 +59,9 @@ use crate::session::mcp_servers::{MCP_TOOL_NAME_DELIMITER, McpClient, McpServerN
|
|||
pub struct McpListRequest {
|
||||
#[serde(default)]
|
||||
pub session_id: Option<String>,
|
||||
/// When false, bypasses the managed MCP config cache and fetches fresh
|
||||
/// from cli-chat-proxy. Set this after OAuth enrollment or disconnect.
|
||||
/// When false, bypass cache and refetch from cli-chat-proxy, then sync
|
||||
/// into live sessions so `search_tool` sees new tools. Use after OAuth
|
||||
/// enrollment or disconnect.
|
||||
#[serde(default = "default_true")]
|
||||
pub cache: bool,
|
||||
}
|
||||
|
|
@ -960,6 +961,34 @@ async fn handle_list(agent: &MvpAgent, args: &acp::ExtRequest) -> ExtResult {
|
|||
session_state_fut
|
||||
);
|
||||
|
||||
// Post-enrollment / explicit refresh: sync fresh state into live sessions.
|
||||
// The two broadcasts are INDEPENDENT concerns, gated separately (and in
|
||||
// practice mutually exclusive — legacy managed fetch runs only when gateway
|
||||
// tools are OFF, gateway fetch only when ON):
|
||||
if !cache {
|
||||
// 1. Legacy managed connectors -> per-session `McpServers`. Only when
|
||||
// the managed fetch actually succeeded (cache `Ready`). A failed
|
||||
// proxy fetch returns an empty vec AND rolls the cache back to
|
||||
// `NotFetched`; syncing that would tear down working servers. A
|
||||
// genuinely-empty `Ready(vec![])` still syncs so disconnect-all works.
|
||||
let managed_ready = matches!(
|
||||
agent.managed_mcp_cache().lock().await.cache,
|
||||
crate::session::managed_mcp::ManagedMcpCache::Ready(_)
|
||||
);
|
||||
if managed_ready {
|
||||
agent.sync_fresh_managed_mcp_to_sessions(&managed_configs);
|
||||
}
|
||||
// 2. Agent-level gateway catalog -> session `search_tool` index. Only
|
||||
// when a fresh gateway catalog committed (`Some`); a failed refetch is
|
||||
// `None` and must not wipe the last-good index. This must fire even
|
||||
// when `managed_ready` is false: in gateway mode the legacy managed
|
||||
// cache stays `NotFetched`, yet the fresh gateway catalog still needs
|
||||
// a session-side rebuild.
|
||||
if gateway_catalog.is_some() {
|
||||
agent.refresh_mcp_search_index_in_sessions();
|
||||
}
|
||||
}
|
||||
|
||||
let compat = agent.cfg.borrow().compat_resolved;
|
||||
let plugin_registry_snapshot = agent.plugin_registry_snapshot();
|
||||
let local_servers = crate::util::config::load_mcp_servers(&cwd, &compat);
|
||||
|
|
|
|||
|
|
@ -417,23 +417,14 @@ async fn handle_reload_all_mcp_servers(agent: &MvpAgent) -> ExtResult {
|
|||
// `load_mcp_servers()` output here was redundant — and silently
|
||||
// dropped client servers that exist in no on-disk config, tearing
|
||||
// them down on every config hot-reload.
|
||||
let merged = crate::session::managed_mcp::merge_managed_mcp_servers(
|
||||
handle.initial_client_mcp_servers.clone(),
|
||||
if crate::session::managed_mcp::merge_and_send_managed_mcp_update(
|
||||
&handle.cmd_tx,
|
||||
&cwd,
|
||||
handle.initial_client_mcp_servers.clone(),
|
||||
&managed,
|
||||
agent.plugin_registry_handle().snapshot().as_deref(),
|
||||
&compat,
|
||||
);
|
||||
|
||||
let (tx, _rx) = tokio::sync::oneshot::channel();
|
||||
if handle
|
||||
.cmd_tx
|
||||
.send(SessionCommand::UpdateMcpServers {
|
||||
mcp_servers: merged,
|
||||
respond_to: tx,
|
||||
})
|
||||
.is_ok()
|
||||
{
|
||||
) {
|
||||
updated += 1;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -213,33 +213,35 @@ impl LeaderLock {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
/// Try to acquire exclusive lock with a timeout.
|
||||
/// Acquire exclusive lock with a bounded wait, re-opening the lock-file path
|
||||
/// on every attempt.
|
||||
///
|
||||
/// Polls `try_lock_exclusive()` every 200ms until the lock is acquired or the
|
||||
/// timeout elapses. Returns `LockError::Timeout` if the deadline is exceeded.
|
||||
/// Polls `try_lock_exclusive()` every 200ms until acquired or the timeout
|
||||
/// elapses (`LockError::Timeout`). The re-open is load-bearing on the leader
|
||||
/// path: an old-flow client's `Drop` unlinks the lock file on its timeout, so
|
||||
/// the winner must acquire on the freshly re-created inode — a single held fd
|
||||
/// would keep polling the stale, unlinked inode forever.
|
||||
///
|
||||
/// Used by the leader subprocess in the socket-then-lock startup flow: the
|
||||
/// spawning client holds the lock while the leader binds its IPC socket, then
|
||||
/// releases it. This method waits for that handoff, but gives up after `timeout`
|
||||
/// so a duplicate leader (started while another is already running) exits
|
||||
/// cleanly instead of blocking forever.
|
||||
pub fn try_acquire_timeout(&mut self, timeout: Duration) -> Result<(), LockError> {
|
||||
let file = self.open_lock_file()?;
|
||||
|
||||
/// Async so the 200ms poll yields to the Tokio runtime instead of blocking a
|
||||
/// worker thread — `run_leader` calls this on the multi-thread runtime.
|
||||
pub async fn acquire_reopen_timeout(&mut self, timeout: Duration) -> Result<(), LockError> {
|
||||
let deadline = Instant::now() + timeout;
|
||||
let poll_interval = Duration::from_millis(200);
|
||||
|
||||
loop {
|
||||
// Re-open each attempt: the inode may have been replaced since the last poll.
|
||||
let file = self.open_lock_file()?;
|
||||
match file.try_lock_exclusive() {
|
||||
Ok(()) => {
|
||||
self.mark_acquired(file);
|
||||
return Ok(());
|
||||
}
|
||||
Err(e) if is_lock_contended(&e) => {
|
||||
drop(file); // release the fd before sleeping; re-open next poll
|
||||
if Instant::now() >= deadline {
|
||||
return Err(LockError::Timeout(timeout));
|
||||
}
|
||||
std::thread::sleep(poll_interval);
|
||||
tokio::time::sleep(poll_interval).await;
|
||||
}
|
||||
Err(e) => return Err(LockError::Io(e)),
|
||||
}
|
||||
|
|
@ -278,18 +280,14 @@ impl LeaderLock {
|
|||
}
|
||||
}
|
||||
|
||||
/// Release the lock explicitly.
|
||||
///
|
||||
/// This is used by the spawner to release the lock after the leader has bound
|
||||
/// its socket. After calling this, the `Drop` impl will NOT clean up files,
|
||||
/// since we're intentionally handing off to the leader process.
|
||||
/// Release the lock explicitly. `Drop` will NOT clean up files afterward.
|
||||
pub fn release(&mut self) -> io::Result<()> {
|
||||
// Clear FIRST: even if `unlock()` errors, `Drop` must not delete the live
|
||||
// child leader's socket.
|
||||
self.was_leader = false;
|
||||
if let Some(file) = self.lock_file.take() {
|
||||
file.unlock()?;
|
||||
}
|
||||
// Clear was_leader so Drop doesn't delete files.
|
||||
// The actual leader process will clean up when it exits.
|
||||
self.was_leader = false;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
|
@ -550,24 +548,28 @@ mod tests {
|
|||
assert!(lock.read_pid().is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn try_acquire_timeout_succeeds_when_unlocked() {
|
||||
#[tokio::test]
|
||||
async fn acquire_reopen_timeout_succeeds_when_unlocked() {
|
||||
let temp = TempDir::new().unwrap();
|
||||
let mut lock = test_lock(&temp);
|
||||
|
||||
lock.try_acquire_timeout(Duration::from_secs(1)).unwrap();
|
||||
lock.acquire_reopen_timeout(Duration::from_secs(1))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(lock.is_held());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn try_acquire_timeout_returns_timeout_when_held() {
|
||||
#[tokio::test]
|
||||
async fn acquire_reopen_timeout_returns_timeout_when_held() {
|
||||
let temp = TempDir::new().unwrap();
|
||||
let mut lock1 = test_lock(&temp);
|
||||
let mut lock2 = test_lock(&temp);
|
||||
|
||||
assert!(lock1.try_acquire().unwrap());
|
||||
|
||||
let result = lock2.try_acquire_timeout(Duration::from_millis(500));
|
||||
let result = lock2
|
||||
.acquire_reopen_timeout(Duration::from_millis(500))
|
||||
.await;
|
||||
assert!(
|
||||
matches!(result, Err(LockError::Timeout(_))),
|
||||
"Expected Timeout error, got {:?}",
|
||||
|
|
@ -576,8 +578,89 @@ mod tests {
|
|||
assert!(!lock2.is_held());
|
||||
}
|
||||
|
||||
/// The re-open is load-bearing: while `lock1` holds the flock on the ORIGINAL
|
||||
/// (now-unlinked) inode for the whole test, re-opening the path each poll lets
|
||||
/// the waiter acquire on a fresh inode. A single-fd waiter would time out here.
|
||||
#[tokio::test]
|
||||
async fn acquire_reopen_timeout_tolerates_unlinked_recreated_lock_file() {
|
||||
let temp = TempDir::new().unwrap();
|
||||
let mut lock1 = test_lock(&temp);
|
||||
let mut lock2 = test_lock(&temp);
|
||||
|
||||
assert!(lock1.try_acquire().unwrap()); // inode A, held for the whole test
|
||||
let lock_path = lock1.lock_path().clone();
|
||||
|
||||
let handle = std::thread::spawn(move || {
|
||||
std::thread::sleep(Duration::from_millis(200));
|
||||
// Simulate the old-flow client's Drop unlinking the lock file while it
|
||||
// still holds the (now-anonymous) inode.
|
||||
fs::remove_file(&lock_path).unwrap();
|
||||
lock1 // return to keep inode A flock-held until the waiter has acquired
|
||||
});
|
||||
|
||||
lock2
|
||||
.acquire_reopen_timeout(Duration::from_secs(5))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(lock2.is_held());
|
||||
|
||||
let _lock1 = handle.join().unwrap();
|
||||
}
|
||||
|
||||
/// Mirrors `run_leader`'s lock-then-socket guard: only the flock winner
|
||||
/// binds the socket; a loser returns `false` without touching it.
|
||||
fn try_start_leader(lock: &mut LeaderLock, socket_contents: &str) -> bool {
|
||||
match lock.try_acquire() {
|
||||
Ok(true) => {
|
||||
lock.cleanup_socket().unwrap();
|
||||
fs::write(lock.socket_path(), socket_contents).unwrap();
|
||||
true
|
||||
}
|
||||
Ok(false) | Err(_) => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Single-leader invariant: a racing would-be leader that loses the flock
|
||||
/// must not touch the socket.
|
||||
#[test]
|
||||
fn try_acquire_timeout_succeeds_after_release() {
|
||||
fn racing_leader_without_flock_cannot_clobber_socket() {
|
||||
let temp = TempDir::new().unwrap();
|
||||
let mut leader1 = test_lock(&temp);
|
||||
let mut leader2 = test_lock(&temp);
|
||||
|
||||
assert!(try_start_leader(&mut leader1, "leader1-socket"));
|
||||
assert!(!try_start_leader(&mut leader2, "leader2-socket"));
|
||||
|
||||
// Leader 1's socket survives untouched.
|
||||
assert!(leader1.socket_path().exists());
|
||||
assert_eq!(
|
||||
fs::read_to_string(leader1.socket_path()).unwrap(),
|
||||
"leader1-socket"
|
||||
);
|
||||
}
|
||||
|
||||
/// The leader holds the flock continuously for its lifetime (released only on
|
||||
/// `Drop`), so no second leader can acquire it while the leader is alive.
|
||||
#[test]
|
||||
fn flock_held_continuously_blocks_second_leader_until_drop() {
|
||||
let temp = TempDir::new().unwrap();
|
||||
let mut contender = test_lock(&temp);
|
||||
|
||||
{
|
||||
let mut leader = test_lock(&temp);
|
||||
assert!(leader.try_acquire().unwrap());
|
||||
leader.write_pid().unwrap();
|
||||
|
||||
assert!(!contender.try_acquire().unwrap());
|
||||
assert!(!contender.try_acquire().unwrap());
|
||||
// leader dropped here (simulating exit) → flock released, files cleaned
|
||||
}
|
||||
|
||||
assert!(contender.try_acquire().unwrap());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn acquire_reopen_timeout_succeeds_after_release() {
|
||||
let temp = TempDir::new().unwrap();
|
||||
let mut lock1 = test_lock(&temp);
|
||||
let mut lock2 = test_lock(&temp);
|
||||
|
|
@ -593,7 +676,10 @@ mod tests {
|
|||
});
|
||||
|
||||
// lock2 should acquire within the timeout because lock1 is released after 200ms
|
||||
lock2.try_acquire_timeout(Duration::from_secs(5)).unwrap();
|
||||
lock2
|
||||
.acquire_reopen_timeout(Duration::from_secs(5))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(lock2.is_held());
|
||||
|
||||
handle.join().unwrap();
|
||||
|
|
|
|||
|
|
@ -76,7 +76,7 @@ pub use server::{
|
|||
use std::fs;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::process::Command;
|
||||
use std::time::Duration;
|
||||
use std::time::{Duration, Instant};
|
||||
use tokio::sync::{mpsc, watch};
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tracing::{debug, info, warn};
|
||||
|
|
@ -87,6 +87,9 @@ const SPAWN_POLL_INTERVAL: Duration = Duration::from_millis(100);
|
|||
const CLIENT_LEADER_VERSION: &str = xai_grok_version::VERSION;
|
||||
/// Max wait for an evicted leader to exit before force-killing (relaunch drain ~5s).
|
||||
const EVICT_WAIT_TIMEOUT: Duration = Duration::from_secs(8);
|
||||
/// How long the SAME live grok flock-holder may stay unconnectable before
|
||||
/// `connect_or_spawn` treats it as a "zombie leader" and evicts it.
|
||||
const ZOMBIE_EVICT_DEADLINE: Duration = Duration::from_secs(30);
|
||||
/// Whether `leader_version` is a strictly-older parseable semver than `baseline`.
|
||||
/// Unparseable versions (e.g. dev `"unknown"`) return `false` — leave them alone.
|
||||
pub fn leader_is_older_than(leader_version: &str, baseline: &str) -> bool {
|
||||
|
|
@ -1158,7 +1161,7 @@ async fn request_leader_vacate(conn: &LeaderConnection, pid: Option<u32>) {
|
|||
}
|
||||
/// Evict a below-floor leader that holds the socket but NOT the flock (the caller
|
||||
/// MUST hold the flock, so this teardown is serialized against other clients).
|
||||
/// Signals it to vacate, waits for the pid to exit, then force-kills if it
|
||||
/// Signals it to vacate, waits for the pid to exit, then re-sends SIGTERM if it
|
||||
/// overran the grace window, so the caller can reclaim the socket and respawn.
|
||||
async fn evict_leader(conn: LeaderConnection, lock: &LeaderLock) {
|
||||
let pid = lock.read_pid();
|
||||
|
|
@ -1171,14 +1174,14 @@ async fn evict_leader(conn: LeaderConnection, lock: &LeaderLock) {
|
|||
if !crate::util::is_process_alive(pid) {
|
||||
"exited"
|
||||
} else if let Err(e) = crate::util::kill_process_by_pid(pid) {
|
||||
warn!(error = %e, pid, "Failed to force-kill stale leader");
|
||||
warn!(error = %e, pid, "Failed to re-signal (SIGTERM) stale leader");
|
||||
"timed_out"
|
||||
} else {
|
||||
wait_for_pid_exit(pid, EVICT_WAIT_TIMEOUT).await;
|
||||
if crate::util::is_process_alive(pid) {
|
||||
"timed_out"
|
||||
} else {
|
||||
"force_killed"
|
||||
"resignaled_sigterm"
|
||||
}
|
||||
}
|
||||
} else {
|
||||
|
|
@ -1196,6 +1199,209 @@ async fn evict_leader(conn: LeaderConnection, lock: &LeaderLock) {
|
|||
})),
|
||||
);
|
||||
}
|
||||
/// PID-keyed timer state: the holder PID being timed and when we first saw it
|
||||
/// live-but-unconnectable.
|
||||
type ZombieTimer = Option<(u32, Instant)>;
|
||||
/// Decision produced by [`zombie_evict_decision`].
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum ZombieAction {
|
||||
/// Not a zombie candidate this round; timer cleared.
|
||||
Clear,
|
||||
/// A live grok holder is still unconnectable; timer (re)armed, keep waiting.
|
||||
Wait,
|
||||
/// The SAME holder PID has been unconnectable for the full deadline — evict.
|
||||
Evict { pid: u32, waited: Duration },
|
||||
}
|
||||
/// Pure decision for the zombie-eviction net. The timer is keyed to the PID so a
|
||||
/// timer accrued against an old zombie can never evict a freshly-spawned leader.
|
||||
fn zombie_evict_decision(
|
||||
holder: Option<u32>,
|
||||
now: Instant,
|
||||
deadline: Duration,
|
||||
timer: &mut ZombieTimer,
|
||||
) -> ZombieAction {
|
||||
let Some(pid) = holder else {
|
||||
*timer = None;
|
||||
return ZombieAction::Clear;
|
||||
};
|
||||
match *timer {
|
||||
Some((tracked_pid, since)) if tracked_pid == pid => {
|
||||
let waited = now.saturating_duration_since(since);
|
||||
if waited >= deadline {
|
||||
*timer = None;
|
||||
ZombieAction::Evict { pid, waited }
|
||||
} else {
|
||||
ZombieAction::Wait
|
||||
}
|
||||
}
|
||||
_ => {
|
||||
*timer = Some((pid, now));
|
||||
ZombieAction::Wait
|
||||
}
|
||||
}
|
||||
}
|
||||
/// The live *grok* PID that ACTUALLY holds the flock on the lock file, if any.
|
||||
/// `None` for a dead / non-grok PID, OR when the file PID can't be confirmed to be
|
||||
/// the real flock holder — so the auto-kill zombie net never SIGKILLs a process
|
||||
/// that does not hold the flock (a stale-but-live PID left in `leader.lock`, or a
|
||||
/// brief spawner that held the flock without rewriting the file). Uses the
|
||||
/// stricter (name-matching) grok check since this drives the auto-kill path.
|
||||
///
|
||||
/// Linux confirms the holder via `/proc/locks`. macOS/BSD have no `/proc/locks`,
|
||||
/// so the holder is unconfirmable and this returns `None` (eviction skipped),
|
||||
/// accepting that a genuine zombie there is not auto-killed.
|
||||
fn live_grok_lock_holder(lock: &LeaderLock) -> Option<u32> {
|
||||
let file_pid = lock.read_pid()?;
|
||||
let pid = evictable_holder(file_pid, confirmed_flock_holder(lock.lock_path()))?;
|
||||
(crate::util::is_process_alive(pid) && crate::util::is_grok_process_strict(pid)).then_some(pid)
|
||||
}
|
||||
/// Safety gate: a file PID is evictable only when the confirmed flock `holder` is
|
||||
/// known AND equals it. An unknown holder or a mismatch (file PID ≠ real holder)
|
||||
/// is NOT evictable. Pure so the "do not evict" invariant is unit-testable.
|
||||
fn evictable_holder(file_pid: u32, holder: Option<u32>) -> Option<u32> {
|
||||
match holder {
|
||||
Some(h) if h == file_pid => Some(file_pid),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
/// PID that actually holds the exclusive flock on the lock file, or `None` when it
|
||||
/// can't be determined. Linux reads `/proc/locks`; other platforms lack that
|
||||
/// interface, so the holder is unknowable there and we return `None` (callers must
|
||||
/// not auto-kill a PID they can't confirm holds the flock).
|
||||
fn confirmed_flock_holder(lock_path: &Path) -> Option<u32> {
|
||||
#[cfg(target_os = "linux")]
|
||||
{
|
||||
flock_holder_pid(lock_path)
|
||||
}
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
{
|
||||
let _ = lock_path;
|
||||
None
|
||||
}
|
||||
}
|
||||
/// The flock-holder PID for `lock_path` per `/proc/locks`: stat the path for its
|
||||
/// device:inode, then find the matching `FLOCK`/`WRITE` (fs2's exclusive lock)
|
||||
/// entry. Linux-only.
|
||||
#[cfg(target_os = "linux")]
|
||||
fn flock_holder_pid(lock_path: &Path) -> Option<u32> {
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
let meta = std::fs::metadata(lock_path).ok()?;
|
||||
let proc_locks = std::fs::read_to_string("/proc/locks").ok()?;
|
||||
let (major, minor) = glibc_dev_major_minor(meta.dev());
|
||||
parse_flock_holder(&proc_locks, major, minor, meta.ino())
|
||||
}
|
||||
/// Decode a glibc 64-bit `dev_t` into (major, minor) — the same bit layout glibc's
|
||||
/// `gnu_dev_major`/`gnu_dev_minor` use, matching the numbers the kernel prints in
|
||||
/// `/proc/locks`. (libc 0.2 dropped `major`/`minor` for the gnu target.) Pure, so
|
||||
/// it (and the parser below) compile+test on all hosts even though only Linux
|
||||
/// consumes them.
|
||||
#[cfg(any(target_os = "linux", test))]
|
||||
fn glibc_dev_major_minor(dev: u64) -> (u64, u64) {
|
||||
let major = ((dev & 0x0000_0000_000f_ff00) >> 8) | ((dev & 0xffff_f000_0000_0000) >> 32);
|
||||
let minor = (dev & 0x0000_0000_0000_00ff) | ((dev & 0x0000_0fff_fff0_0000) >> 12);
|
||||
(major, minor)
|
||||
}
|
||||
/// Parse `/proc/locks` for the PID holding an exclusive `flock` on the file
|
||||
/// identified by `major:minor:inode`. Skips blocked waiters (lines whose second
|
||||
/// field is `->`, which does not hold the lock and shifts the field layout).
|
||||
/// Returns `None` if no matching `FLOCK`/`WRITE` holder is present. Pure (parses a
|
||||
/// string) so it is unit-testable without real kernel locks.
|
||||
#[cfg(any(target_os = "linux", test))]
|
||||
fn parse_flock_holder(proc_locks: &str, major: u64, minor: u64, inode: u64) -> Option<u32> {
|
||||
for line in proc_locks.lines() {
|
||||
let f: Vec<&str> = line.split_whitespace().collect();
|
||||
if f.get(1) == Some(&"->") {
|
||||
continue;
|
||||
}
|
||||
if f.len() < 6 || f[1] != "FLOCK" || f[3] != "WRITE" {
|
||||
continue;
|
||||
}
|
||||
let mut dev_inode = f[5].split(':');
|
||||
let (Some(maj), Some(min), Some(ino)) =
|
||||
(dev_inode.next(), dev_inode.next(), dev_inode.next())
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
let (Ok(maj), Ok(min), Ok(ino)) = (
|
||||
u64::from_str_radix(maj, 16),
|
||||
u64::from_str_radix(min, 16),
|
||||
ino.parse::<u64>(),
|
||||
) else {
|
||||
continue;
|
||||
};
|
||||
if maj == major && min == minor && ino == inode {
|
||||
return f[4].parse::<u32>().ok();
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
/// Max zombie-eviction attempts against the SAME PID before `connect_or_spawn`
|
||||
/// surfaces an error instead of looping forever.
|
||||
const MAX_ZOMBIE_EVICT_ATTEMPTS: u32 = 3;
|
||||
/// Max times `connect_or_spawn` will self-spawn a leader that fails to become
|
||||
/// connectable before surfacing an error. Bounds a persistent spawn/bind failure
|
||||
/// (bad socket-dir perms, exec fault in `run_leader`) that would otherwise
|
||||
/// re-fork every `SPAWN_WAIT_TIMEOUT` forever; still allows the intended
|
||||
/// single-retry after a transient same-version sibling race.
|
||||
const MAX_SELF_SPAWN_ATTEMPTS: u32 = 3;
|
||||
/// Records an eviction attempt against `pid`; returns `false` once the per-PID
|
||||
/// budget is exhausted. Attempts reset when the target PID changes.
|
||||
fn register_evict_attempt(state: &mut Option<(u32, u32)>, pid: u32, max_attempts: u32) -> bool {
|
||||
let count = match *state {
|
||||
Some((tracked, n)) if tracked == pid => n + 1,
|
||||
_ => 1,
|
||||
};
|
||||
*state = Some((pid, count));
|
||||
count <= max_attempts
|
||||
}
|
||||
/// A connect-level failure: never became connectable (`Timeout`) or the socket
|
||||
/// file exists but refuses connections (`Connect`, e.g. ECONNREFUSED against a
|
||||
/// stale socket / dead IPC task). Both drive the zombie net. Registration- and
|
||||
/// protocol-level errors mean the socket ANSWERED and must surface instead.
|
||||
fn is_connect_level_failure(error: &ConnectionError) -> bool {
|
||||
matches!(
|
||||
error,
|
||||
ConnectionError::Timeout | ConnectionError::Client(ClientError::Connect(_, _))
|
||||
)
|
||||
}
|
||||
/// Evict a suspected zombie leader (holds the flock but is not connectable).
|
||||
/// SIGTERM, wait, then escalate to SIGKILL if it overran the grace window.
|
||||
async fn evict_zombie_leader(pid: u32, sock_path: &Path, waited: Duration) {
|
||||
use crate::util::KillSignal;
|
||||
warn!(
|
||||
pid,
|
||||
socket = %sock_path.display(),
|
||||
"Suspected zombie leader (holds lock, not connectable past deadline); evicting"
|
||||
);
|
||||
if let Err(e) = crate::util::kill_process_with_signal(pid, KillSignal::Term) {
|
||||
warn!(error = %e, pid, "Failed to SIGTERM suspected zombie leader");
|
||||
}
|
||||
wait_for_pid_exit(pid, EVICT_WAIT_TIMEOUT).await;
|
||||
let outcome = if !crate::util::is_process_alive(pid) {
|
||||
"exited"
|
||||
} else if let Err(e) = crate::util::kill_process_with_signal(pid, KillSignal::Kill) {
|
||||
warn!(error = %e, pid, "Failed to SIGKILL suspected zombie leader");
|
||||
"sigkill_failed"
|
||||
} else {
|
||||
wait_for_pid_exit(pid, EVICT_WAIT_TIMEOUT).await;
|
||||
if crate::util::is_process_alive(pid) {
|
||||
"survived_sigkill"
|
||||
} else {
|
||||
"sigkilled"
|
||||
}
|
||||
};
|
||||
xai_grok_telemetry::unified_log::warn(
|
||||
"leader.zombie.evicted",
|
||||
None,
|
||||
Some(serde_json::json!({
|
||||
"zombie_pid": pid,
|
||||
"socket_path": sock_path.display().to_string(),
|
||||
"outcome": outcome,
|
||||
"client_version": CLIENT_LEADER_VERSION,
|
||||
"waited_ms": waited.as_millis() as u64,
|
||||
})),
|
||||
);
|
||||
}
|
||||
/// Connect to existing leader or spawn a new one.
|
||||
///
|
||||
/// Uses OS-level file locking (flock) to coordinate:
|
||||
|
|
@ -1255,6 +1461,9 @@ pub async fn connect_or_spawn(
|
|||
}
|
||||
}
|
||||
}
|
||||
let mut zombie_timer: ZombieTimer = None;
|
||||
let mut evict_attempts: Option<(u32, u32)> = None;
|
||||
let mut self_spawn_attempts: u32 = 0;
|
||||
loop {
|
||||
match lock.try_acquire() {
|
||||
Ok(true) => {
|
||||
|
|
@ -1291,15 +1500,35 @@ pub async fn connect_or_spawn(
|
|||
replacing_stale = true;
|
||||
}
|
||||
info!("Acquired lock, spawning leader subprocess");
|
||||
if let Err(e) = lock.cleanup_socket() {
|
||||
warn!(error = %e, "Failed to clean up stale socket");
|
||||
if let Err(e) = lock.release() {
|
||||
warn!(error = %e, "Failed to release lock before spawning leader");
|
||||
}
|
||||
spawn_leader_subprocess(env_urls)?;
|
||||
wait_for_listener_ready(&sock_path).await?;
|
||||
if let Err(e) = lock.release() {
|
||||
warn!(error = %e, "Failed to release lock");
|
||||
}
|
||||
let conn = connect_to_leader(&sock_path, client_type, mode, capabilities).await?;
|
||||
let conn = match wait_for_socket_connectable(
|
||||
&sock_path,
|
||||
client_type,
|
||||
mode,
|
||||
capabilities.clone(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(conn) => conn,
|
||||
Err(ConnectionError::Timeout) => {
|
||||
self_spawn_attempts += 1;
|
||||
if self_spawn_attempts >= MAX_SELF_SPAWN_ATTEMPTS {
|
||||
return Err(ConnectionError::SpawnFailed(format!(
|
||||
"spawned leader did not become connectable after \
|
||||
{MAX_SELF_SPAWN_ATTEMPTS} attempts"
|
||||
)));
|
||||
}
|
||||
debug!(
|
||||
attempt = self_spawn_attempts,
|
||||
"Spawned leader not connectable yet, retrying"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
Err(e) => return Err(e),
|
||||
};
|
||||
let elapsed_ms = start.elapsed().as_millis() as u64;
|
||||
info!(elapsed_ms, "Spawned and connected to leader");
|
||||
if replacing_stale {
|
||||
|
|
@ -1316,7 +1545,7 @@ pub async fn connect_or_spawn(
|
|||
return Ok(conn);
|
||||
}
|
||||
Ok(false) => {
|
||||
debug!("Lock held by another process, waiting for socket");
|
||||
debug!("Lock held by another process, probing socket connectability");
|
||||
}
|
||||
Err(e) => {
|
||||
return Err(e.into());
|
||||
|
|
@ -1325,6 +1554,7 @@ pub async fn connect_or_spawn(
|
|||
match wait_for_socket_connectable(&sock_path, client_type, mode, capabilities.clone()).await
|
||||
{
|
||||
Ok(conn) => {
|
||||
zombie_timer = None;
|
||||
if !should_evict_conn(&conn) {
|
||||
info!(
|
||||
elapsed_ms = start.elapsed().as_millis() as u64,
|
||||
|
|
@ -1338,9 +1568,37 @@ pub async fn connect_or_spawn(
|
|||
tokio::time::sleep(SPAWN_POLL_INTERVAL).await;
|
||||
continue;
|
||||
}
|
||||
Err(ConnectionError::Timeout) => {
|
||||
debug!("Timeout waiting for socket, retrying lock acquisition");
|
||||
continue;
|
||||
Err(e) if is_connect_level_failure(&e) => {
|
||||
let holder = live_grok_lock_holder(&lock);
|
||||
match zombie_evict_decision(
|
||||
holder,
|
||||
Instant::now(),
|
||||
ZOMBIE_EVICT_DEADLINE,
|
||||
&mut zombie_timer,
|
||||
) {
|
||||
ZombieAction::Evict { pid, waited } => {
|
||||
if !register_evict_attempt(
|
||||
&mut evict_attempts,
|
||||
pid,
|
||||
MAX_ZOMBIE_EVICT_ATTEMPTS,
|
||||
) {
|
||||
return Err(ConnectionError::SpawnFailed(format!(
|
||||
"zombie leader pid {pid} could not be evicted after \
|
||||
{MAX_ZOMBIE_EVICT_ATTEMPTS} attempts"
|
||||
)));
|
||||
}
|
||||
evict_zombie_leader(pid, &sock_path, waited).await;
|
||||
continue;
|
||||
}
|
||||
ZombieAction::Wait => {
|
||||
debug!("Flock-holder not connectable yet, waiting");
|
||||
continue;
|
||||
}
|
||||
ZombieAction::Clear => {
|
||||
debug!("Timeout waiting for socket, retrying lock acquisition");
|
||||
continue;
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(e) => return Err(e),
|
||||
}
|
||||
|
|
@ -1469,19 +1727,6 @@ async fn connect_to_leader(
|
|||
LeaderClient::connect(sock_path.to_path_buf(), client_type, mode, capabilities).await?;
|
||||
Ok(LeaderConnection { client })
|
||||
}
|
||||
/// Poll until the IPC listener at `sock_path` is reachable. A full
|
||||
/// connect would deadlock (see inline comment at the call site).
|
||||
async fn wait_for_listener_ready(sock_path: &Path) -> Result<(), ConnectionError> {
|
||||
let deadline = tokio::time::Instant::now() + SPAWN_WAIT_TIMEOUT;
|
||||
while tokio::time::Instant::now() < deadline {
|
||||
if crate::leader::transport::listener_is_ready(sock_path) {
|
||||
debug!("Leader listener is ready");
|
||||
return Ok(());
|
||||
}
|
||||
tokio::time::sleep(SPAWN_POLL_INTERVAL).await;
|
||||
}
|
||||
Err(ConnectionError::Timeout)
|
||||
}
|
||||
/// Wait for socket to appear and successfully connect.
|
||||
///
|
||||
/// Polls the socket path until it becomes connectable or timeout is reached.
|
||||
|
|
@ -1519,6 +1764,163 @@ mod tests {
|
|||
};
|
||||
use std::fs;
|
||||
use tempfile::TempDir;
|
||||
const TEST_DEADLINE: Duration = Duration::from_secs(30);
|
||||
/// No live grok holder → `Clear`, and any pending timer is reset.
|
||||
#[test]
|
||||
fn zombie_decision_clears_when_no_holder() {
|
||||
let mut timer: ZombieTimer = Some((100, Instant::now()));
|
||||
assert_eq!(
|
||||
zombie_evict_decision(None, Instant::now(), TEST_DEADLINE, &mut timer),
|
||||
ZombieAction::Clear
|
||||
);
|
||||
assert_eq!(timer, None, "timer must be cleared when there is no holder");
|
||||
}
|
||||
/// First sighting of a holder arms the timer and waits (never evicts).
|
||||
#[test]
|
||||
fn zombie_decision_arms_timer_on_first_sighting() {
|
||||
let mut timer: ZombieTimer = None;
|
||||
let t0 = Instant::now();
|
||||
assert_eq!(
|
||||
zombie_evict_decision(Some(100), t0, TEST_DEADLINE, &mut timer),
|
||||
ZombieAction::Wait
|
||||
);
|
||||
assert_eq!(timer, Some((100, t0)));
|
||||
}
|
||||
/// The SAME holder is evicted only after staying unconnectable for the deadline.
|
||||
#[test]
|
||||
fn zombie_decision_evicts_same_pid_after_deadline() {
|
||||
let mut timer: ZombieTimer = None;
|
||||
let t0 = Instant::now();
|
||||
assert_eq!(
|
||||
zombie_evict_decision(Some(100), t0, TEST_DEADLINE, &mut timer),
|
||||
ZombieAction::Wait
|
||||
);
|
||||
let t_mid = t0 + Duration::from_secs(29);
|
||||
assert_eq!(
|
||||
zombie_evict_decision(Some(100), t_mid, TEST_DEADLINE, &mut timer),
|
||||
ZombieAction::Wait
|
||||
);
|
||||
let t_end = t0 + Duration::from_secs(30);
|
||||
assert_eq!(
|
||||
zombie_evict_decision(Some(100), t_end, TEST_DEADLINE, &mut timer),
|
||||
ZombieAction::Evict {
|
||||
pid: 100,
|
||||
waited: Duration::from_secs(30),
|
||||
}
|
||||
);
|
||||
assert_eq!(timer, None);
|
||||
}
|
||||
/// A holder PID change re-keys the timer, so time accrued against an old zombie
|
||||
/// can never evict a fresh leader.
|
||||
#[test]
|
||||
fn zombie_decision_resets_timer_when_pid_changes() {
|
||||
let mut timer: ZombieTimer = None;
|
||||
let t0 = Instant::now();
|
||||
assert_eq!(
|
||||
zombie_evict_decision(Some(100), t0, TEST_DEADLINE, &mut timer),
|
||||
ZombieAction::Wait
|
||||
);
|
||||
let t1 = t0 + Duration::from_secs(40);
|
||||
assert_eq!(
|
||||
zombie_evict_decision(Some(200), t1, TEST_DEADLINE, &mut timer),
|
||||
ZombieAction::Wait
|
||||
);
|
||||
assert_eq!(timer, Some((200, t1)), "timer must re-key to the new PID");
|
||||
let t2 = t1 + Duration::from_secs(1);
|
||||
assert_eq!(
|
||||
zombie_evict_decision(None, t2, TEST_DEADLINE, &mut timer),
|
||||
ZombieAction::Clear
|
||||
);
|
||||
assert_eq!(timer, None);
|
||||
}
|
||||
/// Eviction safety gate: a file PID is evictable only when the confirmed flock
|
||||
/// holder is known AND equals it. Unknown holder or a mismatch → do not evict.
|
||||
#[test]
|
||||
fn evictable_holder_requires_confirmed_matching_holder() {
|
||||
assert_eq!(evictable_holder(100, Some(100)), Some(100));
|
||||
assert_eq!(evictable_holder(100, Some(200)), None);
|
||||
assert_eq!(evictable_holder(100, None), None);
|
||||
}
|
||||
/// glibc `dev_t` decode matches the logical major:minor the kernel prints.
|
||||
/// makedev(253, 1) == 0xfd01 → (253, 1).
|
||||
#[test]
|
||||
fn glibc_dev_major_minor_decodes_makedev() {
|
||||
assert_eq!(glibc_dev_major_minor(0xfd01), (253, 1));
|
||||
assert_eq!(glibc_dev_major_minor(0), (0, 0));
|
||||
}
|
||||
/// `/proc/locks` parsing: match an exclusive FLOCK holder by device:inode and
|
||||
/// return its PID; skip waiters, POSIX locks, and non-matching dev/inode.
|
||||
#[test]
|
||||
fn parse_flock_holder_matches_dev_inode_and_pid() {
|
||||
let sample = "\
|
||||
1: POSIX ADVISORY WRITE 111 fd:01:2000 0 EOF
|
||||
2: FLOCK ADVISORY WRITE 592 fd:01:1000 0 EOF
|
||||
3: FLOCK ADVISORY WRITE 700 fd:01:3000 0 EOF
|
||||
";
|
||||
assert_eq!(parse_flock_holder(sample, 253, 1, 1000), Some(592));
|
||||
assert_eq!(parse_flock_holder(sample, 253, 1, 9999), None);
|
||||
assert_eq!(parse_flock_holder(sample, 8, 1, 1000), None);
|
||||
}
|
||||
/// Blocked waiters (`->`) do not hold the lock and shift the field layout, so
|
||||
/// they must be skipped even when their dev:inode matches.
|
||||
#[test]
|
||||
fn parse_flock_holder_skips_waiters() {
|
||||
let sample = "\
|
||||
1: FLOCK ADVISORY WRITE 592 fd:01:1000 0 EOF
|
||||
1: -> FLOCK ADVISORY WRITE 800 fd:01:1000 0 EOF
|
||||
";
|
||||
assert_eq!(parse_flock_holder(sample, 253, 1, 1000), Some(592));
|
||||
}
|
||||
/// A stale-but-live PID in the lock file (file PID ≠ real flock holder) is
|
||||
/// classified "do not evict" end-to-end through the parse + gate helpers.
|
||||
#[test]
|
||||
fn stale_file_pid_not_matching_holder_is_not_evictable() {
|
||||
let sample = "1: FLOCK ADVISORY WRITE 592 fd:01:1000 0 EOF\n";
|
||||
let holder = parse_flock_holder(sample, 253, 1, 1000);
|
||||
assert_eq!(holder, Some(592));
|
||||
assert_eq!(evictable_holder(12345, holder), None);
|
||||
}
|
||||
/// Connect-level failures (timeout / connection-refused) drive the zombie
|
||||
/// net; registration/protocol errors (socket answered) surface instead.
|
||||
#[test]
|
||||
fn connect_level_failure_classification() {
|
||||
use std::io::{Error, ErrorKind};
|
||||
assert!(is_connect_level_failure(&ConnectionError::Timeout));
|
||||
assert!(is_connect_level_failure(&ConnectionError::Client(
|
||||
ClientError::Connect(3, Error::from(ErrorKind::ConnectionRefused))
|
||||
)));
|
||||
assert!(!is_connect_level_failure(&ConnectionError::Client(
|
||||
ClientError::Registration("rejected".into())
|
||||
)));
|
||||
assert!(!is_connect_level_failure(&ConnectionError::Client(
|
||||
ClientError::ConnectionClosed
|
||||
)));
|
||||
}
|
||||
/// Per-PID eviction budget: allows `max` attempts, then denies; a PID change
|
||||
/// resets the counter so a fresh zombie gets its own budget.
|
||||
#[test]
|
||||
fn register_evict_attempt_bounds_per_pid() {
|
||||
let mut state: Option<(u32, u32)> = None;
|
||||
assert!(register_evict_attempt(&mut state, 100, 3));
|
||||
assert!(register_evict_attempt(&mut state, 100, 3));
|
||||
assert!(register_evict_attempt(&mut state, 100, 3));
|
||||
assert!(!register_evict_attempt(&mut state, 100, 3));
|
||||
assert!(register_evict_attempt(&mut state, 200, 3));
|
||||
assert_eq!(state, Some((200, 1)));
|
||||
}
|
||||
/// `live_grok_lock_holder` returns `None` for a missing or dead PID, so the
|
||||
/// zombie net never times/kills a recycled or unrelated PID.
|
||||
#[test]
|
||||
fn live_grok_lock_holder_none_for_missing_or_dead_pid() {
|
||||
let temp = TempDir::new().unwrap();
|
||||
let lock = LeaderLock::from_paths(
|
||||
temp.path().join("leader.lock"),
|
||||
temp.path().join("leader.sock"),
|
||||
);
|
||||
assert_eq!(live_grok_lock_holder(&lock), None);
|
||||
fs::write(lock.lock_path(), "4000000000").unwrap();
|
||||
assert_eq!(live_grok_lock_holder(&lock), None);
|
||||
}
|
||||
#[test]
|
||||
fn reachable_leader_pids_skips_stale_locks() {
|
||||
let reachable = LeaderDescriptor {
|
||||
|
|
|
|||
|
|
@ -9,7 +9,9 @@ pub(super) fn turn_result_to_hook_outcome(
|
|||
) -> xai_tool_protocol::turn_hook::TurnHookOutcome {
|
||||
use xai_tool_protocol::turn_hook::TurnHookOutcome;
|
||||
match result {
|
||||
Ok(TurnOutcome::Completed { .. }) => TurnHookOutcome::Completed,
|
||||
Ok(TurnOutcome::Completed { .. }) | Ok(TurnOutcome::StationarityEnded { .. }) => {
|
||||
TurnHookOutcome::Completed
|
||||
}
|
||||
Ok(TurnOutcome::Cancelled { .. }) | Ok(TurnOutcome::MaxTurnsReached { .. }) => {
|
||||
TurnHookOutcome::Cancelled
|
||||
}
|
||||
|
|
|
|||
|
|
@ -449,6 +449,14 @@ fn format_workflow_completion_reminder(
|
|||
);
|
||||
}
|
||||
}
|
||||
if run.status == crate::session::workflow::tracker::WorkflowRunStatus::Failed {
|
||||
let _ = writeln!(
|
||||
buf,
|
||||
" Resumable: call the workflow tool with resume_from_run_id: \"{}\" — \
|
||||
completed agents replay from the journal and the failed step re-executes.",
|
||||
run.run_id
|
||||
);
|
||||
}
|
||||
let report_path = session_dir
|
||||
.join("workflows")
|
||||
.join(&run.run_id)
|
||||
|
|
@ -850,7 +858,15 @@ mod workflow_reminder_tests {
|
|||
let run = failed_run(detail);
|
||||
let session_dir = tempfile::tempdir().unwrap();
|
||||
let reminder = format_workflow_completion_reminder(&[run], session_dir.path(), false, None);
|
||||
let rendered_detail = reminder.split_once(" Detail: ").unwrap().1.trim_end();
|
||||
let rendered_detail = reminder
|
||||
.split_once(" Detail: ")
|
||||
.unwrap()
|
||||
.1
|
||||
.lines()
|
||||
.next()
|
||||
.unwrap()
|
||||
.trim_end();
|
||||
assert!(reminder.contains("resume_from_run_id: \"wf_1\""));
|
||||
assert!(rendered_detail.starts_with("first second "));
|
||||
assert!(rendered_detail.ends_with('…'));
|
||||
assert!(rendered_detail.len() <= WORKFLOW_RESULT_SUMMARY_REMINDER_CAP);
|
||||
|
|
|
|||
|
|
@ -410,7 +410,7 @@ pub(super) async fn run_session(
|
|||
if let Some(notification) = replay_buffer.flush() {
|
||||
session.emit_buffered(notification).await;
|
||||
}
|
||||
let (turn_succeeded, infra_pause_message) =
|
||||
let (turn_succeeded, suppress_goal_continuation, infra_pause_message) =
|
||||
SessionActor::post_turn_goal_degradation_plan(&result);
|
||||
session.handle_completion(prompt_id, result).await;
|
||||
// Drain any monitor events that were routed to the mid-turn buffer
|
||||
|
|
@ -421,7 +421,9 @@ pub(super) async fn run_session(
|
|||
}
|
||||
// Goal continuation (success) or back-off (non-success).
|
||||
// Owns the streak-tracking and reminder-injection path.
|
||||
session.handle_turn_end(turn_succeeded).await;
|
||||
session
|
||||
.handle_turn_end(turn_succeeded, suppress_goal_continuation)
|
||||
.await;
|
||||
// Interjections that raced past the turn's final drain
|
||||
// (arrived during turn-end bookkeeping) have no turn left
|
||||
// to merge into — convert them to front-of-queue prompt
|
||||
|
|
|
|||
|
|
@ -417,7 +417,7 @@ impl SessionActor {
|
|||
}
|
||||
impl xai_grok_sampler::BearerResolver for AuthManagerBearerResolver {
|
||||
fn current_bearer(&self) -> Option<String> {
|
||||
self.0.current_or_expired().map(|a| a.key)
|
||||
self.0.current_wire_valid().map(|a| a.key)
|
||||
}
|
||||
}
|
||||
let cfg = self
|
||||
|
|
@ -448,6 +448,13 @@ impl SessionActor {
|
|||
if use_bearer_resolver && let Some(am) = self.auth_manager.as_ref() {
|
||||
let _ = am.auth().await;
|
||||
}
|
||||
let api_key = if use_bearer_resolver {
|
||||
self.auth_manager
|
||||
.as_ref()
|
||||
.and_then(|am| am.current_wire_valid().map(|a| a.key))
|
||||
} else {
|
||||
creds.api_key
|
||||
};
|
||||
let auth_scheme = model_facts.auth_scheme;
|
||||
let mut extra_headers = cfg.extra_headers;
|
||||
crate::agent::config::inject_url_derived_headers(
|
||||
|
|
@ -480,7 +487,7 @@ impl SessionActor {
|
|||
}
|
||||
}
|
||||
SamplingConfig {
|
||||
api_key: creds.api_key,
|
||||
api_key,
|
||||
base_url: cfg.base_url,
|
||||
model: cfg.model,
|
||||
max_completion_tokens: cfg.max_completion_tokens,
|
||||
|
|
@ -1194,6 +1201,11 @@ impl SessionActor {
|
|||
}
|
||||
Err(e) => {
|
||||
let hard_expired = !am.has_usable_token();
|
||||
if hard_expired && creds.api_key.is_some() {
|
||||
let mut cleared = creds;
|
||||
cleared.api_key = None;
|
||||
self.chat_state_handle.update_credentials(cleared);
|
||||
}
|
||||
tracing::warn!(
|
||||
error = %e,
|
||||
hard_expired,
|
||||
|
|
|
|||
|
|
@ -964,6 +964,34 @@ impl SessionActor {
|
|||
},
|
||||
);
|
||||
}
|
||||
Ok(TurnOutcome::StationarityEnded { .. }) => {
|
||||
self.emit_turn_ended(
|
||||
crate::session::events::TurnOutcomeLabel::Completed,
|
||||
None,
|
||||
None,
|
||||
);
|
||||
self.send_after_turn_event(xai_tool_protocol::turn_hook::AfterTurnPayload {
|
||||
turn_number: current_prompt_index as u64,
|
||||
outcome: xai_tool_protocol::turn_hook::TurnHookOutcome::Completed,
|
||||
duration_ms: turn_duration_ms,
|
||||
tool_call_count: turn_tool_count,
|
||||
model_id: turn_model_id.clone(),
|
||||
written_repo_paths: Vec::new(),
|
||||
cancellation_category: Some("action_stationarity".to_string()),
|
||||
cancellation_context: None,
|
||||
})
|
||||
.await;
|
||||
xai_grok_telemetry::session_ctx::log_event(
|
||||
xai_grok_telemetry::events::TurnCompleted {
|
||||
outcome: xai_grok_telemetry::events::Outcome::Completed,
|
||||
duration_ms: turn_duration_ms,
|
||||
tool_call_count: turn_tool_count,
|
||||
model_id: turn_model_id,
|
||||
cancellation_category: Some("action_stationarity".to_string()),
|
||||
error_category: None,
|
||||
},
|
||||
);
|
||||
}
|
||||
Ok(TurnOutcome::Cancelled { category, context }) => {
|
||||
self.emit_turn_ended(
|
||||
crate::session::events::TurnOutcomeLabel::Cancelled,
|
||||
|
|
@ -1095,7 +1123,7 @@ impl SessionActor {
|
|||
);
|
||||
}
|
||||
match &result {
|
||||
Ok(TurnOutcome::Completed { .. }) => {
|
||||
Ok(TurnOutcome::Completed { .. }) | Ok(TurnOutcome::StationarityEnded { .. }) => {
|
||||
for contributor in self.extension_registry.turn_lifecycle_contributors() {
|
||||
contributor
|
||||
.on_turn_done(&xai_agent_lifecycle::TurnDoneInput)
|
||||
|
|
@ -1162,6 +1190,12 @@ impl SessionActor {
|
|||
PromptCompletionKind::Completed,
|
||||
structured_output,
|
||||
),
|
||||
TurnOutcome::StationarityEnded { snapshot, .. } => (
|
||||
acp::StopReason::EndTurn,
|
||||
*snapshot,
|
||||
PromptCompletionKind::StationarityEnded,
|
||||
None,
|
||||
),
|
||||
TurnOutcome::Cancelled { category, context } => {
|
||||
let cancellation_ctx = context.and_then(|v| serde_json::from_value(v).ok());
|
||||
(
|
||||
|
|
@ -1395,11 +1429,12 @@ impl SessionActor {
|
|||
/// goal is `Active` (`goal_active_now == true`):
|
||||
///
|
||||
/// 1. **Success.** Reset `goal_continuation_streak` to 0, then call
|
||||
/// `maybe_queue_goal_continuation`. That helper verifies any
|
||||
/// pending completion via its turn-end drain, queues the
|
||||
/// continuation reminder if the goal is still `Active`, and runs
|
||||
/// the stop-detector to select the nudge flavor (generic vs.
|
||||
/// bail-specific) and emit `Event::GoalPrematureStopDetected`.
|
||||
/// `maybe_queue_goal_continuation` unless `suppress_goal_continuation`
|
||||
/// (stationarity silent EndTurn). That helper verifies any pending
|
||||
/// completion via its turn-end drain, queues the continuation reminder
|
||||
/// if the goal is still `Active`, and runs the stop-detector to select
|
||||
/// the nudge flavor (generic vs. bail-specific) and emit
|
||||
/// `Event::GoalPrematureStopDetected`.
|
||||
/// 2. **Non-success.** Increment `goal_continuation_streak`. At
|
||||
/// [`GOAL_CONTINUATION_BACKOFF_THRESHOLD`] consecutive hits,
|
||||
/// reset the streak and auto-pause with
|
||||
|
|
@ -1413,7 +1448,11 @@ impl SessionActor {
|
|||
/// before this method and already transitioned the goal out of
|
||||
/// Active), both branches are skipped: neither streak moves and the
|
||||
/// existing pause cause is preserved.
|
||||
pub(crate) async fn handle_turn_end(&self, turn_succeeded: bool) {
|
||||
pub(crate) async fn handle_turn_end(
|
||||
&self,
|
||||
turn_succeeded: bool,
|
||||
suppress_goal_continuation: bool,
|
||||
) {
|
||||
let goal_active_now = laziness_injection_active(
|
||||
self.goal_harness_enabled(),
|
||||
self.goal_tracker.lock().status(),
|
||||
|
|
@ -1421,7 +1460,9 @@ impl SessionActor {
|
|||
if turn_succeeded && goal_active_now {
|
||||
self.goal_continuation_streak
|
||||
.store(0, std::sync::atomic::Ordering::Relaxed);
|
||||
self.maybe_queue_goal_continuation().await;
|
||||
if !suppress_goal_continuation {
|
||||
self.maybe_queue_goal_continuation().await;
|
||||
}
|
||||
return;
|
||||
}
|
||||
if !turn_succeeded && goal_active_now {
|
||||
|
|
@ -1513,7 +1554,10 @@ impl SessionActor {
|
|||
json_schema.clone(),
|
||||
)
|
||||
.await;
|
||||
if matches!(result, Ok(TurnOutcome::MaxTurnsReached { .. })) {
|
||||
if matches!(
|
||||
result,
|
||||
Ok(TurnOutcome::MaxTurnsReached { .. }) | Ok(TurnOutcome::StationarityEnded { .. })
|
||||
) {
|
||||
return result;
|
||||
}
|
||||
if let Ok(TurnOutcome::Completed {
|
||||
|
|
@ -1577,7 +1621,10 @@ impl SessionActor {
|
|||
None,
|
||||
)
|
||||
.await;
|
||||
if matches!(result, Ok(TurnOutcome::MaxTurnsReached { .. })) {
|
||||
if matches!(
|
||||
result,
|
||||
Ok(TurnOutcome::MaxTurnsReached { .. }) | Ok(TurnOutcome::StationarityEnded { .. })
|
||||
) {
|
||||
return result;
|
||||
}
|
||||
if let Ok(TurnOutcome::Completed {
|
||||
|
|
@ -1735,6 +1782,25 @@ impl SessionActor {
|
|||
));
|
||||
StructuredOutputStep::Complete(validated)
|
||||
}
|
||||
/// Single shell tool call whose parsed command is `true` (via ToolBridge).
|
||||
async fn is_run_true_step(
|
||||
&self,
|
||||
tool_calls: &[xai_grok_sampling_types::conversation::ToolCall],
|
||||
) -> bool {
|
||||
let [tc] = tool_calls else {
|
||||
return false;
|
||||
};
|
||||
let Ok(args) = serde_json::from_str::<serde_json::Value>(tc.arguments.as_ref()) else {
|
||||
return false;
|
||||
};
|
||||
let Ok(input) = self.tool_bridge_handle().try_parse(&tc.name, args).await else {
|
||||
return false;
|
||||
};
|
||||
match input {
|
||||
ToolInput::Bash(b) => command_is_true(&b.command),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
/// Shared turn-completion bookkeeping (plan cleanup, signals snapshot +
|
||||
/// persistence, BigQuery turn delta, feedback prompt). Runs identically for
|
||||
/// the native and StructuredOutput-tool completion paths. Returns the
|
||||
|
|
@ -1922,14 +1988,16 @@ impl SessionActor {
|
|||
loop {
|
||||
self.emit_event(crate::session::events::Event::LoopStarted { loop_index });
|
||||
loop_index += 1;
|
||||
if identical_tool_calls.run_len >= MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS {
|
||||
if identical_tool_calls.run_len >= identical_tool_calls.hard_stop_threshold() {
|
||||
let run_len = identical_tool_calls.run_len;
|
||||
let tool_name = identical_tool_calls.tool_name.clone();
|
||||
let true_noop = identical_tool_calls.is_true_noop_run;
|
||||
tracing::warn!(
|
||||
session_id = %self.session_info.id,
|
||||
tool_name = %tool_name,
|
||||
run_len,
|
||||
"action stationarity: stopping turn after repeated identical tool calls"
|
||||
true_noop,
|
||||
"action stationarity: ending turn after repeated identical tool calls"
|
||||
);
|
||||
xai_grok_telemetry::unified_log::warn(
|
||||
"shell.turn.action_stationarity_stop",
|
||||
|
|
@ -1938,29 +2006,26 @@ impl SessionActor {
|
|||
"loop_index": loop_index,
|
||||
"tool_name": tool_name,
|
||||
"run_len": run_len,
|
||||
"true_noop": true_noop,
|
||||
})),
|
||||
);
|
||||
let notice = format!(
|
||||
"Stopped: the agent ran the same command (`{tool_name}`) {run_len} times in \
|
||||
a row with no change in the result. If it's waiting on a long-running job, \
|
||||
use a background task or the `monitor` tool (or a single `sleep` then check) \
|
||||
instead of polling; otherwise send a new instruction."
|
||||
xai_grok_telemetry::session_ctx::log_event(
|
||||
xai_grok_telemetry::events::ActionStationarityStop {
|
||||
true_noop,
|
||||
run_len,
|
||||
tool_name: tool_name.clone(),
|
||||
},
|
||||
);
|
||||
self.send_update(
|
||||
acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new(
|
||||
acp::ContentBlock::Text(acp::TextContent::new(notice)),
|
||||
)),
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
return Ok(TurnOutcome::Cancelled {
|
||||
category: Some(
|
||||
crate::session::events::CancellationCategory::ActionStationarity,
|
||||
),
|
||||
context: Some(serde_json::json!({
|
||||
"tool_name": tool_name,
|
||||
"run_len": run_len,
|
||||
})),
|
||||
let snapshot = self
|
||||
.finalize_turn_bookkeeping(
|
||||
req_id,
|
||||
conv_turn_start,
|
||||
&turn_span_totals,
|
||||
model_fingerprint.clone(),
|
||||
)
|
||||
.await;
|
||||
return Ok(TurnOutcome::StationarityEnded {
|
||||
snapshot: Box::new(snapshot),
|
||||
});
|
||||
}
|
||||
self.drain_pending_interjections().await;
|
||||
|
|
@ -2433,7 +2498,16 @@ impl SessionActor {
|
|||
.first()
|
||||
.map(|tc| tc.name.clone())
|
||||
.unwrap_or_default();
|
||||
let identical_run_len = identical_tool_calls.observe(&step_signature, &step_tool_name);
|
||||
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);
|
||||
if is_true_noop {
|
||||
xai_grok_telemetry::session_ctx::log_event(
|
||||
xai_grok_telemetry::events::ShellTrueNoop {
|
||||
tool_name: step_tool_name.clone(),
|
||||
},
|
||||
);
|
||||
}
|
||||
if identical_run_len == NUDGE_AFTER_IDENTICAL_TOOL_CALLS {
|
||||
tracing::warn!(
|
||||
session_id = %self.session_info.id,
|
||||
|
|
@ -2542,7 +2616,9 @@ impl SessionActor {
|
|||
}
|
||||
const MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS: u32 = 16;
|
||||
const NUDGE_AFTER_IDENTICAL_TOOL_CALLS: u32 = 8;
|
||||
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 \
|
||||
|
|
@ -2557,58 +2633,80 @@ fn hash_step_signature(signature: &str) -> u64 {
|
|||
signature.hash(&mut hasher);
|
||||
hasher.finish()
|
||||
}
|
||||
fn command_is_true(cmd: &str) -> bool {
|
||||
cmd.trim().eq_ignore_ascii_case("true")
|
||||
}
|
||||
#[derive(Default)]
|
||||
struct IdenticalToolCallRun {
|
||||
last_signature_hash: Option<u64>,
|
||||
tool_name: String,
|
||||
run_len: u32,
|
||||
is_true_noop_run: bool,
|
||||
}
|
||||
impl IdenticalToolCallRun {
|
||||
fn observe(&mut self, signature: &str, tool_name: &str) -> u32 {
|
||||
let hash = hash_step_signature(signature);
|
||||
fn observe(&mut self, signature: &str, tool_name: &str, is_true_noop: bool) -> u32 {
|
||||
let hash = hash_step_signature(if is_true_noop {
|
||||
"\0true_noop"
|
||||
} else {
|
||||
signature
|
||||
});
|
||||
if self.last_signature_hash == Some(hash) {
|
||||
self.run_len += 1;
|
||||
} else {
|
||||
self.run_len = 1;
|
||||
self.last_signature_hash = Some(hash);
|
||||
self.is_true_noop_run = is_true_noop;
|
||||
}
|
||||
self.tool_name = tool_name.to_string();
|
||||
self.run_len
|
||||
}
|
||||
fn hard_stop_threshold(&self) -> u32 {
|
||||
if self.is_true_noop_run {
|
||||
MAX_CONSECUTIVE_TRUE_NOOPS
|
||||
} else {
|
||||
MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS
|
||||
}
|
||||
}
|
||||
}
|
||||
#[cfg(test)]
|
||||
mod identical_tool_call_run_tests {
|
||||
use super::{IdenticalToolCallRun, MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS};
|
||||
use super::{
|
||||
IdenticalToolCallRun, MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS, MAX_CONSECUTIVE_TRUE_NOOPS,
|
||||
command_is_true,
|
||||
};
|
||||
#[test]
|
||||
fn counts_consecutive_identical_calls() {
|
||||
fn identical_non_true_resets_and_caps_at_16() {
|
||||
let mut run = IdenticalToolCallRun::default();
|
||||
let sig = "run_terminal_cmd\u{1f}{\"command\":\"squeue\"}";
|
||||
assert_eq!(run.observe(sig, "run_terminal_cmd"), 1);
|
||||
assert_eq!(run.observe(sig, "run_terminal_cmd"), 2);
|
||||
assert_eq!(run.observe(sig, "run_terminal_cmd"), 3);
|
||||
}
|
||||
#[test]
|
||||
fn a_different_call_resets_the_run() {
|
||||
let mut run = IdenticalToolCallRun::default();
|
||||
run.observe("a", "a");
|
||||
run.observe("a", "a");
|
||||
assert_eq!(run.observe("b", "b"), 1, "a different signature resets");
|
||||
assert_eq!(run.observe("b", "b"), 2);
|
||||
assert_eq!(run.tool_name, "b");
|
||||
assert_eq!(run.observe("a", "a", false), 1);
|
||||
assert_eq!(run.observe("a", "a", false), 2);
|
||||
assert_eq!(run.observe("b", "b", false), 1);
|
||||
let mut last = 0;
|
||||
for _ in 0..MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS {
|
||||
last = run.observe("same", "same", false);
|
||||
}
|
||||
assert_eq!(last, MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS);
|
||||
assert_eq!(
|
||||
run.observe("a", "a"),
|
||||
1,
|
||||
"not consecutive with the first run"
|
||||
run.hard_stop_threshold(),
|
||||
MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS
|
||||
);
|
||||
}
|
||||
#[test]
|
||||
fn run_reaches_the_bound_after_n_identical_calls() {
|
||||
fn true_noops_chain_across_args_and_stop_at_4() {
|
||||
let mut run = IdenticalToolCallRun::default();
|
||||
let mut last = 0;
|
||||
for _ in 0..MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS {
|
||||
last = run.observe("same", "same");
|
||||
for i in 1..=4 {
|
||||
assert_eq!(run.observe(&format!("sig{i}"), "bash", true), i);
|
||||
}
|
||||
assert_eq!(last, MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS);
|
||||
assert!(run.is_true_noop_run);
|
||||
assert_eq!(run.hard_stop_threshold(), MAX_CONSECUTIVE_TRUE_NOOPS);
|
||||
assert_eq!(run.observe("squeue", "bash", false), 1);
|
||||
assert!(!run.is_true_noop_run);
|
||||
}
|
||||
#[test]
|
||||
fn command_is_true_trim_and_case() {
|
||||
assert!(command_is_true("true"));
|
||||
assert!(command_is_true(" TRUE "));
|
||||
assert!(!command_is_true("true && echo hi"));
|
||||
assert!(!command_is_true("lisa status"));
|
||||
}
|
||||
}
|
||||
/// Backoff schedule for resubmits after a *successful* 401 auth recovery
|
||||
|
|
|
|||
|
|
@ -400,12 +400,18 @@ impl SessionActor {
|
|||
)
|
||||
}
|
||||
|
||||
/// `(turn_succeeded, infra_pause_message)` for the completion handler.
|
||||
/// `infra_pause_message` is extracted before `handle_completion` consumes
|
||||
/// `result`.
|
||||
/// `(turn_succeeded, suppress_goal_continuation, infra_pause_message)`.
|
||||
/// StationarityEnded is success for the streak but skips GoalSummary re-queue.
|
||||
/// `infra_pause_message` is extracted before `handle_completion` consumes `result`.
|
||||
pub(super) fn post_turn_goal_degradation_plan(
|
||||
result: &PromptTurnResult,
|
||||
) -> (bool, Option<String>) {
|
||||
) -> (bool, bool, Option<String>) {
|
||||
let suppress_goal_continuation = result.as_ref().ok().is_some_and(|ok| {
|
||||
matches!(
|
||||
ok.completion_kind,
|
||||
crate::session::commands::PromptCompletionKind::StationarityEnded
|
||||
)
|
||||
});
|
||||
let turn_cancelled = result.as_ref().ok().is_some_and(|ok| {
|
||||
matches!(
|
||||
ok.completion_kind,
|
||||
|
|
@ -422,7 +428,11 @@ impl SessionActor {
|
|||
.err()
|
||||
.filter(|err| Self::is_infra_turn_error(err))
|
||||
.map(Self::format_turn_error_message);
|
||||
(turn_succeeded, infra_pause_message)
|
||||
(
|
||||
turn_succeeded,
|
||||
suppress_goal_continuation,
|
||||
infra_pause_message,
|
||||
)
|
||||
}
|
||||
|
||||
pub(super) async fn apply_infra_pause_after_turn_err(&self, message: String) -> bool {
|
||||
|
|
|
|||
|
|
@ -63,6 +63,11 @@ pub(crate) enum TurnOutcome {
|
|||
},
|
||||
/// The `--max-turns` limit was reached after a tool-execution cycle.
|
||||
MaxTurnsReached { limit: usize },
|
||||
/// Silent EndTurn after stationarity/true-noop thrash. Distinct from
|
||||
/// Completed so recovery/goal/stop-hook cannot re-open the sampling loop.
|
||||
StationarityEnded {
|
||||
snapshot: Box<Option<TurnDeltaSnapshot>>,
|
||||
},
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
|
|
|
|||
|
|
@ -129,7 +129,7 @@ impl SessionActor {
|
|||
if status == WorkflowRunStatus::Active {
|
||||
return format!("Run '{name}' is already running.");
|
||||
}
|
||||
if !status.is_paused() {
|
||||
if !status.is_resumable() {
|
||||
return format!(
|
||||
"Run '{name}' cannot be resumed (status: {}). Start a new run instead.",
|
||||
status.as_str()
|
||||
|
|
@ -305,7 +305,7 @@ fn narrow_run_matches(mut all: Vec<RunMatch>, selector: &str, op: &str) -> Vec<R
|
|||
.iter()
|
||||
.filter(|(_, status, ..)| match op {
|
||||
"pause" => *status == WorkflowRunStatus::Active,
|
||||
"resume" => status.is_paused(),
|
||||
"resume" => status.is_resumable(),
|
||||
"stop" => !status.is_terminal(),
|
||||
_ => true,
|
||||
})
|
||||
|
|
@ -360,6 +360,17 @@ mod run_match_tests {
|
|||
assert_eq!(picked[0].2, "b");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn failed_run_is_applicable_for_resume_narrowing() {
|
||||
let all = vec![
|
||||
run("wf_1", "a", WorkflowRunStatus::Complete),
|
||||
run("wf_2", "b", WorkflowRunStatus::Failed),
|
||||
];
|
||||
let picked = narrow_run_matches(all, "", "resume");
|
||||
assert_eq!(picked.len(), 1);
|
||||
assert_eq!(picked[0].2, "b");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ambiguous_stays_ambiguous() {
|
||||
let all = vec![
|
||||
|
|
|
|||
|
|
@ -325,7 +325,7 @@ async fn pre_flight_refreshes_hard_expired_session_token() {
|
|||
}
|
||||
|
||||
/// Hard-expired + failed refresh: do not fall through to JWT/config.toml;
|
||||
/// leave credentials unchanged so 401 recovery remains the safety net.
|
||||
/// strip the chat-state seed so default headers cannot carry a dead AT.
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[serial_test::serial(attribution_emit_count)]
|
||||
async fn pre_flight_hard_expired_refresh_failure_skips_jwt_fallthrough() {
|
||||
|
|
@ -363,8 +363,8 @@ async fn pre_flight_hard_expired_refresh_failure_skips_jwt_fallthrough() {
|
|||
.await
|
||||
.api_key
|
||||
.as_deref(),
|
||||
Some("initial-test-key"),
|
||||
"failed hard-expired pre-flight must not invent a JWT/config bearer"
|
||||
None,
|
||||
"hard-expired pre-flight failure must strip the chat-state seed"
|
||||
);
|
||||
assert!(
|
||||
!am.has_usable_token(),
|
||||
|
|
@ -378,6 +378,71 @@ async fn pre_flight_hard_expired_refresh_failure_skips_jwt_fallthrough() {
|
|||
.await;
|
||||
}
|
||||
|
||||
/// Soft-expired (early-invalidation buffer) + transient fail: retain the seed
|
||||
/// so a still-accepted wire AT can continue until 401 recovery.
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[serial_test::serial(attribution_emit_count)]
|
||||
async fn pre_flight_soft_expired_transient_fail_retains_seed() {
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local
|
||||
.run_until(async {
|
||||
let call_count = Arc::new(std::sync::atomic::AtomicU32::new(0));
|
||||
let refresher: Arc<dyn crate::auth::refresh::TokenRefresher> = Arc::new({
|
||||
struct AlwaysFail(Arc<std::sync::atomic::AtomicU32>);
|
||||
#[async_trait::async_trait]
|
||||
impl crate::auth::refresh::TokenRefresher for AlwaysFail {
|
||||
async fn refresh(
|
||||
&self,
|
||||
_: crate::auth::refresh::RefreshReason,
|
||||
) -> crate::auth::refresh::RefreshOutcome {
|
||||
self.0.fetch_add(1, Ordering::SeqCst);
|
||||
crate::auth::refresh::RefreshOutcome::transient("refresh failed")
|
||||
}
|
||||
}
|
||||
AlwaysFail(call_count.clone())
|
||||
});
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let am = Arc::new(AuthManager::new(dir.path(), GrokComConfig::default()));
|
||||
// Inside the early-invalidation buffer but still hard-valid.
|
||||
am.hot_swap(GrokAuth {
|
||||
key: "buffered-test-key".into(),
|
||||
auth_mode: AuthMode::Oidc,
|
||||
refresh_token: Some("rt".into()),
|
||||
expires_at: Some(chrono::Utc::now() + chrono::Duration::seconds(30)),
|
||||
..GrokAuth::test_default()
|
||||
});
|
||||
am.set_refresher(refresher);
|
||||
let (actor, _rx) = make_actor_with_auth_and_credentials(
|
||||
Some(am.clone()),
|
||||
xai_chat_state::AuthType::SessionToken,
|
||||
"buffered-test-key".to_string(),
|
||||
)
|
||||
.await;
|
||||
|
||||
actor.refresh_token_if_expired().await;
|
||||
|
||||
assert!(
|
||||
call_count.load(Ordering::SeqCst) >= 1,
|
||||
"soft-expired pre-flight must still attempt refresh"
|
||||
);
|
||||
assert_eq!(
|
||||
actor
|
||||
.chat_state_handle
|
||||
.get_credentials()
|
||||
.await
|
||||
.api_key
|
||||
.as_deref(),
|
||||
Some("buffered-test-key"),
|
||||
"buffer-window soft-expired + transient fail must retain seed"
|
||||
);
|
||||
assert!(
|
||||
am.has_usable_token(),
|
||||
"token inside hard-expiry buffer remains usable"
|
||||
);
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
/// Proactive refresh keeps the cache hot so `refresh_token_if_expired`
|
||||
/// (per-turn pre-flight) is a cache hit — the refresher fires once
|
||||
/// (proactive), then the per-turn call sees the fresh token without
|
||||
|
|
|
|||
|
|
@ -58,6 +58,16 @@ fn turn_result_cancelled() {
|
|||
);
|
||||
}
|
||||
#[test]
|
||||
fn turn_result_stationarity_ended_is_completed() {
|
||||
let result: Result<TurnOutcome, acp::Error> = Ok(TurnOutcome::StationarityEnded {
|
||||
snapshot: Box::new(None),
|
||||
});
|
||||
assert_eq!(
|
||||
turn_result_to_hook_outcome(&result),
|
||||
TurnHookOutcome::Completed
|
||||
);
|
||||
}
|
||||
#[test]
|
||||
fn turn_result_error() {
|
||||
let result: Result<TurnOutcome, acp::Error> = Err(acp::Error::internal_error());
|
||||
assert_eq!(turn_result_to_hook_outcome(&result), TurnHookOutcome::Error);
|
||||
|
|
|
|||
|
|
@ -24,6 +24,9 @@ pub struct CancellationContext {
|
|||
#[derive(Debug, Clone)]
|
||||
pub enum PromptCompletionKind {
|
||||
Completed,
|
||||
/// Silent EndTurn after stationarity/true-noop thrash. Distinct from
|
||||
/// Completed so goal continuation is not re-queued under an active goal.
|
||||
StationarityEnded,
|
||||
Cancelled {
|
||||
category: Option<xai_file_utils::events::types::CancellationCategory>,
|
||||
context: Option<CancellationContext>,
|
||||
|
|
|
|||
|
|
@ -209,7 +209,6 @@ pub(crate) fn prior_turn_interrupt_from_cancellation(
|
|||
CancellationCategory::PermissionRejected => Some(PriorTurnInterrupt::PermissionRejected),
|
||||
CancellationCategory::PermissionCancelled => Some(PriorTurnInterrupt::PermissionCancelled),
|
||||
CancellationCategory::HookDenied => None,
|
||||
CancellationCategory::ActionStationarity => None,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -105,6 +105,37 @@ pub fn merge_managed_mcp_servers(
|
|||
.collect()
|
||||
}
|
||||
|
||||
/// Merge the managed catalog into ONE live session's MCP set and push the
|
||||
/// result via [`crate::session::SessionCommand::UpdateMcpServers`]; returns
|
||||
/// `true` if the command was enqueued (session still alive).
|
||||
///
|
||||
/// Shared core for every "re-merge managed configs into a live session" path
|
||||
/// (`mcp/list cache=false`, config hot-reload, post-grant reload) so the merge
|
||||
/// inputs and the dropped-oneshot-response contract can't drift between them.
|
||||
pub(crate) fn merge_and_send_managed_mcp_update(
|
||||
cmd_tx: &tokio::sync::mpsc::UnboundedSender<crate::session::SessionCommand>,
|
||||
cwd: &std::path::Path,
|
||||
initial_client_mcp_servers: Vec<acp::McpServer>,
|
||||
managed: &[ManagedMcpConfig],
|
||||
plugin_registry: Option<&xai_grok_agent::plugins::PluginRegistry>,
|
||||
compat: &xai_grok_tools::types::compat::CompatConfig,
|
||||
) -> bool {
|
||||
let merged = merge_managed_mcp_servers(
|
||||
initial_client_mcp_servers,
|
||||
cwd,
|
||||
managed,
|
||||
plugin_registry,
|
||||
compat,
|
||||
);
|
||||
let (tx, _rx) = tokio::sync::oneshot::channel();
|
||||
cmd_tx
|
||||
.send(crate::session::SessionCommand::UpdateMcpServers {
|
||||
mcp_servers: merged,
|
||||
respond_to: tx,
|
||||
})
|
||||
.is_ok()
|
||||
}
|
||||
|
||||
pub fn merge_managed_mcp_servers_with_policy(
|
||||
client_mcp_servers: Vec<acp::McpServer>,
|
||||
cwd: &std::path::Path,
|
||||
|
|
|
|||
|
|
@ -133,7 +133,7 @@ pub const HTTP_RECOVERY_BACKOFF: [Duration; 7] = [
|
|||
/// `Disabled` vs `NotConfigured` both come from
|
||||
/// [`RestartActions::is_stdio_server_configured`] returning `false`;
|
||||
/// the split is temporal (schedule time vs inside the backoff loop) so
|
||||
/// on-call can tell "flipped off mid-restart" from "stale event".
|
||||
/// operators can tell "flipped off mid-restart" from "stale event".
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum SkipReason {
|
||||
/// Server is in the dispatcher's `shutting_down` set
|
||||
|
|
@ -666,7 +666,7 @@ fn record_skipped(server: &str, reason: SkipReason) {
|
|||
}
|
||||
|
||||
// ── in-place HTTP recovery metrics (kept separate from auto_restart.* so
|
||||
// on-call can distinguish stdio respawn from HTTP transport reset) ──
|
||||
// operators can distinguish stdio respawn from HTTP transport reset) ──
|
||||
|
||||
fn record_http_recovery_attempted(server: &str) {
|
||||
tracing::info!(target: "metrics.mcp.http_recovery.attempted", server = %server);
|
||||
|
|
|
|||
|
|
@ -1039,6 +1039,12 @@ pub(crate) fn fork_filter_chat(items: &mut Vec<ConversationItem>) {
|
|||
}
|
||||
items.truncate(last_complete_end);
|
||||
}
|
||||
fn conversation_truncate_after_prompt(
|
||||
conversation: &[ConversationItem],
|
||||
target_prompt_index: usize,
|
||||
) -> usize {
|
||||
conversation_truncate_for_prompt(conversation, target_prompt_index + 1)
|
||||
}
|
||||
impl JsonlStorageAdapter {
|
||||
/// Fully synchronous version of `copy_session_data` for use inside
|
||||
/// `spawn_blocking`. Identical logic but uses `std::fs::write` instead
|
||||
|
|
@ -1059,8 +1065,12 @@ impl JsonlStorageAdapter {
|
|||
let mut updates_to_copy: Vec<super::SessionUpdate> =
|
||||
self.read_updates_jsonl(self.updates_file(source_info))?;
|
||||
if let Some(target_idx) = options.target_prompt_index {
|
||||
chat_to_copy.truncate(conversation_truncate_for_prompt(&chat_to_copy, target_idx));
|
||||
updates_to_copy = super::filter_rewind_updates(updates_to_copy);
|
||||
updates_to_copy.truncate(updates_truncate_for_prompt(&updates_to_copy, target_idx));
|
||||
chat_to_copy.truncate(conversation_truncate_after_prompt(
|
||||
&chat_to_copy,
|
||||
target_idx,
|
||||
));
|
||||
}
|
||||
if options.fork_filter {
|
||||
fork_filter_chat(&mut chat_to_copy);
|
||||
|
|
|
|||
|
|
@ -1226,6 +1226,128 @@ async fn test_copy_session_data_transforms_xai_updates() {
|
|||
_ => panic!("Expected xAI update"),
|
||||
}
|
||||
}
|
||||
fn fork_user_chunk(session_id: &str, text: &str, prompt_index: usize) -> SessionUpdate {
|
||||
let chunk = acp::ContentChunk::new(
|
||||
acp::ContentBlock::Text(acp::TextContent::new(text.to_string())),
|
||||
)
|
||||
.meta(serde_json::json!({ "promptIndex": prompt_index }).as_object().cloned());
|
||||
SessionUpdate::Acp(
|
||||
Box::new(
|
||||
acp::SessionNotification::new(
|
||||
acp::SessionId::new(session_id),
|
||||
acp::SessionUpdate::UserMessageChunk(chunk),
|
||||
),
|
||||
),
|
||||
)
|
||||
}
|
||||
fn fork_agent_chunk(session_id: &str, text: &str) -> SessionUpdate {
|
||||
SessionUpdate::Acp(
|
||||
Box::new(
|
||||
acp::SessionNotification::new(
|
||||
acp::SessionId::new(session_id),
|
||||
acp::SessionUpdate::AgentMessageChunk(
|
||||
acp::ContentChunk::new(
|
||||
acp::ContentBlock::Text(acp::TextContent::new(text.to_string())),
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
)
|
||||
}
|
||||
fn fork_rewind_marker(session_id: &str, target_prompt_index: usize) -> SessionUpdate {
|
||||
use crate::extensions::notification::{
|
||||
SessionNotification as XaiSessionNotification,
|
||||
SessionUpdate as XaiSessionUpdateType,
|
||||
};
|
||||
SessionUpdate::Xai(
|
||||
Box::new(XaiSessionNotification {
|
||||
session_id: acp::SessionId::new(session_id),
|
||||
update: XaiSessionUpdateType::RewindMarker {
|
||||
target_prompt_index,
|
||||
created_at: "2026-01-01T00:00:00Z".to_string(),
|
||||
},
|
||||
meta: None,
|
||||
}),
|
||||
)
|
||||
}
|
||||
fn chat_user(text: &str, prompt_index: usize) -> ConversationItem {
|
||||
let mut item = ConversationItem::user(text);
|
||||
item.set_prompt_index(prompt_index);
|
||||
item
|
||||
}
|
||||
/// Fork truncation targets the live branch — dead-branch runs from a
|
||||
/// prior rewind overlap its stamps (indices are branch-local) — and keeps
|
||||
/// prompt N inclusive in both the updates and chat (model-context) files.
|
||||
#[tokio::test]
|
||||
async fn copy_session_data_fork_truncates_live_branch_inclusive() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let adapter = JsonlStorageAdapter::with_root(temp_dir.path().to_path_buf());
|
||||
let sid = "src-rewound";
|
||||
let source_info = Info {
|
||||
id: acp::SessionId::new(sid),
|
||||
cwd: "/src".to_string(),
|
||||
};
|
||||
adapter.init_session(&source_info, default_model_id()).await.unwrap();
|
||||
for update in [
|
||||
fork_user_chunk(sid, "P0", 0),
|
||||
fork_agent_chunk(sid, "A0"),
|
||||
fork_user_chunk(sid, "P1-dead", 1),
|
||||
fork_agent_chunk(sid, "A1-dead"),
|
||||
fork_rewind_marker(sid, 1),
|
||||
fork_user_chunk(sid, "P1b", 1),
|
||||
fork_agent_chunk(sid, "A1b"),
|
||||
fork_user_chunk(sid, "P2", 2),
|
||||
] {
|
||||
adapter.append_update(&source_info, &update).await.unwrap();
|
||||
}
|
||||
for item in [
|
||||
chat_user("P0", 0),
|
||||
ConversationItem::assistant("A0"),
|
||||
chat_user("P1b", 1),
|
||||
ConversationItem::assistant("A1b"),
|
||||
chat_user("P2", 2),
|
||||
] {
|
||||
adapter.append_chat_message(&source_info, &item).await.unwrap();
|
||||
}
|
||||
let fork_at = |target: usize, fork_id: &str| {
|
||||
let target_info = Info {
|
||||
id: acp::SessionId::new(fork_id),
|
||||
cwd: "/src".to_string(),
|
||||
};
|
||||
let options = CopySessionOptions {
|
||||
target_prompt_index: Some(target),
|
||||
..Default::default()
|
||||
};
|
||||
(target_info, options)
|
||||
};
|
||||
let (target_info, options) = fork_at(1, "fork-at-1");
|
||||
let result = adapter
|
||||
.copy_session_data(&source_info, &target_info, options)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(result.updates_copied, 4);
|
||||
assert_eq!(result.chat_messages_copied, 4);
|
||||
let loaded = adapter.load_session(&target_info).await.unwrap();
|
||||
let last = loaded.updates.last().unwrap();
|
||||
assert!(
|
||||
matches!(
|
||||
last,
|
||||
SessionUpdate::Acp(n) if matches!(
|
||||
&n.update,
|
||||
acp::SessionUpdate::AgentMessageChunk(c)
|
||||
if matches!(&c.content, acp::ContentBlock::Text(t) if t.text == "A1b")
|
||||
)
|
||||
),
|
||||
"fork must end at the live branch's A1b, got {last:?}"
|
||||
);
|
||||
let (target_info, options) = fork_at(0, "fork-at-0");
|
||||
let result = adapter
|
||||
.copy_session_data(&source_info, &target_info, options)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(result.updates_copied, 2, "P0 + A0");
|
||||
assert_eq!(result.chat_messages_copied, 2, "P0 + A0 in model context");
|
||||
}
|
||||
#[tokio::test]
|
||||
async fn test_copy_session_data_source_not_found() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
|
|
|
|||
|
|
@ -22,8 +22,8 @@ pub(crate) const WORKFLOW_MAX_AGENT_RUNS: u32 =
|
|||
(xai_workflow::MAX_AGENT_BUDGET as u32) * (SCHEMA_CONTRACT_RETRIES + 1);
|
||||
pub(crate) const WORKFLOW_MAX_SCRIPT_TELEMETRY_EVENTS: u32 = 64;
|
||||
pub(crate) const WORKFLOW_MAX_SCRATCH_FILES: usize = 64;
|
||||
pub(crate) const WORKFLOW_MAX_SCRATCH_FILE_BYTES: usize = 1024 * 1024;
|
||||
pub(crate) const WORKFLOW_MAX_SCRATCH_TOTAL_BYTES: u64 = 8 * 1024 * 1024;
|
||||
pub(crate) const WORKFLOW_MAX_SCRATCH_FILE_BYTES: usize = 10 * 1024 * 1024;
|
||||
pub(crate) const WORKFLOW_MAX_SCRATCH_TOTAL_BYTES: u64 = 64 * 1024 * 1024;
|
||||
const WORKFLOW_MAX_AGENT_PROMPT_BYTES: usize = 1024 * 1024;
|
||||
const WORKFLOW_MAX_TEMPLATE_OUTPUT_BYTES: usize = 1024 * 1024;
|
||||
const WORKFLOW_MAX_PHASE_BYTES: usize = 256;
|
||||
|
|
@ -126,7 +126,7 @@ struct HostService {
|
|||
|
||||
struct FinishOnce<'a> {
|
||||
host: &'a HostService,
|
||||
agent_id: &'a str,
|
||||
agent_id: String,
|
||||
finished: bool,
|
||||
}
|
||||
|
||||
|
|
@ -138,12 +138,21 @@ impl FinishOnce<'_> {
|
|||
}
|
||||
self.host.params.tracker.lock().agent_finished(
|
||||
&self.host.params.run_id,
|
||||
self.agent_id,
|
||||
&self.agent_id,
|
||||
state,
|
||||
total_tokens,
|
||||
total_duration,
|
||||
);
|
||||
}
|
||||
|
||||
fn rebind(&mut self, new_agent_id: &str) {
|
||||
self.host.params.tracker.lock().rebind_agent_id(
|
||||
&self.host.params.run_id,
|
||||
&self.agent_id,
|
||||
new_agent_id,
|
||||
);
|
||||
self.agent_id = new_agent_id.to_string();
|
||||
}
|
||||
}
|
||||
|
||||
impl HostService {
|
||||
|
|
@ -397,7 +406,7 @@ impl HostService {
|
|||
);
|
||||
let mut row = FinishOnce {
|
||||
host: self,
|
||||
agent_id: &id,
|
||||
agent_id: id.clone(),
|
||||
finished: false,
|
||||
};
|
||||
let cancel_token = CancellationToken::new();
|
||||
|
|
@ -451,7 +460,9 @@ impl HostService {
|
|||
let child_id = if attempts == 1 {
|
||||
id.clone()
|
||||
} else {
|
||||
uuid::Uuid::now_v7().to_string()
|
||||
let retry_id = uuid::Uuid::now_v7().to_string();
|
||||
row.rebind(&retry_id);
|
||||
retry_id
|
||||
};
|
||||
let request = spawn_once(
|
||||
child_id.clone(),
|
||||
|
|
|
|||
|
|
@ -126,7 +126,7 @@ impl WorkflowManager {
|
|||
.lock()
|
||||
.get(run_id)
|
||||
.ok_or_else(|| LaunchError::UnknownRun(run_id.clone()))?;
|
||||
if !existing.status.is_paused() {
|
||||
if !existing.status.is_resumable() {
|
||||
return Err(LaunchError::NotResumable(
|
||||
existing.status.as_str().to_string(),
|
||||
));
|
||||
|
|
@ -150,7 +150,7 @@ impl WorkflowManager {
|
|||
execution_script = self.store.script_for(run_id).ok_or_else(|| {
|
||||
LaunchError::Store("immutable workflow script is missing".into())
|
||||
})?;
|
||||
let journal = match existing
|
||||
let mut journal = match existing
|
||||
.journal_path
|
||||
.as_ref()
|
||||
.and_then(|p| self.session_dir.as_ref().map(|d| (d, p)))
|
||||
|
|
@ -167,6 +167,11 @@ impl WorkflowManager {
|
|||
}
|
||||
None => Journal::new(None),
|
||||
};
|
||||
if existing.status == crate::session::workflow::tracker::WorkflowRunStatus::Failed {
|
||||
journal
|
||||
.prune_trailing_host_error(existing.pause_message.as_deref().unwrap_or(""))
|
||||
.map_err(|e| LaunchError::Journal(e.to_string()))?;
|
||||
}
|
||||
let state = {
|
||||
let mut tracker = self.tracker.lock();
|
||||
tracker.reconcile_agents_used(run_id, journal.agent_reservation_count());
|
||||
|
|
@ -963,7 +968,74 @@ mod tests {
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn failed_cancelled_and_completed_runs_are_not_resumable() {
|
||||
async fn failed_run_resumes_and_reexecutes_failed_host_call_live() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let (mut manager, _rx) = test_manager(Some(dir.path().to_path_buf()));
|
||||
let script = "let meta = #{ name: \"t\", description: \"d\" };\n\
|
||||
let content = read_scratch_file(\"data.txt\");\n\
|
||||
complete(content);";
|
||||
let (run_id, outcome_rx) = manager
|
||||
.launch(resolve_inline(script.into()).unwrap(), spec())
|
||||
.unwrap();
|
||||
match outcome_rx.await.unwrap() {
|
||||
WorkflowOutcome::Failed { error } => {
|
||||
assert!(error.contains("scratch"), "{error}");
|
||||
}
|
||||
other => panic!("expected Failed, got {other:?}"),
|
||||
}
|
||||
assert_eq!(
|
||||
manager.tracker.lock().get(&run_id).unwrap().status,
|
||||
crate::session::workflow::tracker::WorkflowRunStatus::Failed
|
||||
);
|
||||
let journal_path = dir
|
||||
.path()
|
||||
.join("workflows")
|
||||
.join(&run_id)
|
||||
.join("journal.jsonl");
|
||||
assert!(
|
||||
std::fs::read_to_string(&journal_path)
|
||||
.unwrap()
|
||||
.contains("__xai_workflow_host_error"),
|
||||
"the uncaught host error must be journaled as a trailing sentinel"
|
||||
);
|
||||
|
||||
let scratch = dir.path().join("workflows").join(&run_id).join("scratch");
|
||||
std::fs::create_dir_all(&scratch).unwrap();
|
||||
std::fs::write(scratch.join("data.txt"), "hello").unwrap();
|
||||
|
||||
let (_same_id, outcome_rx) = manager
|
||||
.launch(
|
||||
resolve_inline(script.into()).unwrap(),
|
||||
LaunchSpec {
|
||||
resume_run_id: Some(run_id.clone()),
|
||||
..spec()
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
match outcome_rx.await.unwrap() {
|
||||
WorkflowOutcome::Completed { result } => {
|
||||
assert_eq!(
|
||||
result,
|
||||
serde_json::json!("hello"),
|
||||
"the failed host call must go live instead of replaying the sentinel"
|
||||
);
|
||||
}
|
||||
other => panic!("expected Completed, got {other:?}"),
|
||||
}
|
||||
assert_eq!(
|
||||
manager.tracker.lock().get(&run_id).unwrap().status,
|
||||
crate::session::workflow::tracker::WorkflowRunStatus::Complete
|
||||
);
|
||||
assert!(
|
||||
!std::fs::read_to_string(&journal_path)
|
||||
.unwrap()
|
||||
.contains("__xai_workflow_host_error"),
|
||||
"the trailing sentinel must be pruned and replaced by the live result"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn completed_cancelled_and_interrupted_runs_are_not_resumable() {
|
||||
use xai_grok_tools::implementations::grok_build::task::types::{
|
||||
SubagentEvent, SubagentResult,
|
||||
};
|
||||
|
|
@ -992,7 +1064,6 @@ mod tests {
|
|||
let state = manager.tracker.lock().get(&run_id).unwrap();
|
||||
for status in [
|
||||
crate::session::workflow::tracker::WorkflowRunStatus::Complete,
|
||||
crate::session::workflow::tracker::WorkflowRunStatus::Failed,
|
||||
crate::session::workflow::tracker::WorkflowRunStatus::Cancelled,
|
||||
crate::session::workflow::tracker::WorkflowRunStatus::Interrupted,
|
||||
] {
|
||||
|
|
|
|||
|
|
@ -59,6 +59,10 @@ impl WorkflowRunStatus {
|
|||
)
|
||||
}
|
||||
|
||||
pub fn is_resumable(self) -> bool {
|
||||
self.is_paused() || self == Self::Failed
|
||||
}
|
||||
|
||||
fn from_pause(kind: PauseKind) -> Self {
|
||||
match kind {
|
||||
PauseKind::User => Self::UserPaused,
|
||||
|
|
@ -263,7 +267,7 @@ impl WorkflowTracker {
|
|||
new_agent_budget: Option<u64>,
|
||||
) -> Option<WorkflowRunState> {
|
||||
let run = self.run_mut(run_id)?;
|
||||
if !run.state.status.is_paused() {
|
||||
if !run.state.status.is_resumable() {
|
||||
return None;
|
||||
}
|
||||
let candidate_budget = match new_agent_budget {
|
||||
|
|
@ -416,6 +420,19 @@ impl WorkflowTracker {
|
|||
label
|
||||
}
|
||||
|
||||
/// Point a roster row at a fresh child session id. Contract retries
|
||||
/// spawn a new child session per attempt; the row must follow so live
|
||||
/// progress lookups and transcript clicks resolve to the current child.
|
||||
pub fn rebind_agent_id(&mut self, run_id: &str, agent_id: &str, new_agent_id: &str) {
|
||||
let Some(run) = self.run_mut(run_id) else {
|
||||
return;
|
||||
};
|
||||
if let Some(row) = run.state.agents.iter_mut().find(|a| a.agent_id == agent_id) {
|
||||
row.agent_id = new_agent_id.to_string();
|
||||
run.state.advance_revision();
|
||||
}
|
||||
}
|
||||
|
||||
pub fn agent_finished(
|
||||
&mut self,
|
||||
run_id: &str,
|
||||
|
|
@ -793,6 +810,34 @@ mod tests {
|
|||
assert_eq!(s.result_summary.as_deref(), Some("shipped"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rebind_agent_id_points_row_at_retry_child_and_bumps_revision() {
|
||||
let (mut t, id) = tracker_with_run();
|
||||
t.agent_started(
|
||||
&id,
|
||||
WorkflowAgentRow {
|
||||
agent_id: "child-attempt-1".into(),
|
||||
label: "worker".into(),
|
||||
phase: None,
|
||||
model: None,
|
||||
state: "running".into(),
|
||||
tokens_used: 0,
|
||||
duration_ms: 0,
|
||||
},
|
||||
);
|
||||
let before = t.get(&id).unwrap().revision;
|
||||
t.rebind_agent_id(&id, "child-attempt-1", "child-attempt-2");
|
||||
let run = t.get(&id).unwrap();
|
||||
assert_eq!(run.agents.len(), 1);
|
||||
assert_eq!(run.agents[0].agent_id, "child-attempt-2");
|
||||
assert_eq!(run.agents[0].label, "worker");
|
||||
assert_eq!(run.agents[0].state, "running");
|
||||
assert!(run.revision > before);
|
||||
|
||||
t.agent_finished(&id, "child-attempt-2", "done", 42, 1_000);
|
||||
assert_eq!(t.get(&id).unwrap().agents[0].state, "done");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn snapshot_restore_marks_active_non_resumable_interrupted() {
|
||||
let (t, _) = tracker_with_run();
|
||||
|
|
@ -825,11 +870,64 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn resume_rejects_nonpaused_states() {
|
||||
fn resume_rejects_nonresumable_states() {
|
||||
let (mut t, id) = tracker_with_run();
|
||||
t.interrupt(&id, "lost executor").unwrap();
|
||||
assert!(t.resume_run(&id, None).is_none());
|
||||
assert_eq!(t.get(&id).unwrap().status, WorkflowRunStatus::Interrupted);
|
||||
|
||||
let (mut t, id) = tracker_with_run();
|
||||
t.apply_outcome(&id, &WorkflowOutcome::Cancelled);
|
||||
assert!(t.resume_run(&id, None).is_none());
|
||||
assert_eq!(t.get(&id).unwrap().status, WorkflowRunStatus::Cancelled);
|
||||
|
||||
let (mut t, id) = tracker_with_run();
|
||||
t.apply_outcome(
|
||||
&id,
|
||||
&WorkflowOutcome::Completed {
|
||||
result: serde_json::json!("done"),
|
||||
},
|
||||
);
|
||||
assert!(t.resume_run(&id, None).is_none());
|
||||
assert_eq!(t.get(&id).unwrap().status, WorkflowRunStatus::Complete);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn failed_run_resumes_to_active_bumps_epoch_and_cancels_ghost_agents() {
|
||||
let (mut t, id) = tracker_with_run();
|
||||
t.agent_started(
|
||||
&id,
|
||||
WorkflowAgentRow {
|
||||
agent_id: "child".into(),
|
||||
label: "worker".into(),
|
||||
phase: None,
|
||||
model: None,
|
||||
state: "running".into(),
|
||||
tokens_used: 0,
|
||||
duration_ms: 0,
|
||||
},
|
||||
);
|
||||
t.apply_outcome(
|
||||
&id,
|
||||
&WorkflowOutcome::Failed {
|
||||
error: "scratch byte quota exceeded".into(),
|
||||
},
|
||||
);
|
||||
let failed = t.get(&id).unwrap();
|
||||
assert_eq!(failed.status, WorkflowRunStatus::Failed);
|
||||
assert!(failed.status.is_resumable());
|
||||
assert!(failed.status.is_terminal());
|
||||
assert!(!failed.status.is_paused());
|
||||
assert_eq!(t.execution_epoch(&id), Some(0));
|
||||
|
||||
let resumed = t.resume_run(&id, None).unwrap();
|
||||
assert_eq!(resumed.status, WorkflowRunStatus::Active);
|
||||
assert!(resumed.pause_message.is_none());
|
||||
assert_eq!(
|
||||
resumed.agents[0].state, "cancelled",
|
||||
"ghost running agent rows must be cancelled on resume"
|
||||
);
|
||||
assert_eq!(t.execution_epoch(&id), Some(1));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
Loading…
Reference in a new issue