diff --git a/crates/ironclaw_engine/src/lib.rs b/crates/ironclaw_engine/src/lib.rs index 25a6ebeffd9..312464faf34 100644 --- a/crates/ironclaw_engine/src/lib.rs +++ b/crates/ironclaw_engine/src/lib.rs @@ -83,7 +83,10 @@ pub use gate::{ pub use executor::prompt::PlatformInfo; pub use runtime::conversation::ConversationManager; -pub use runtime::manager::ThreadManager; +pub use runtime::manager::{ + ENGINE_RESTART_RECOVERY_METADATA_KEY, PENDING_APPROVAL_METADATA_KEY, + RUNTIME_CHECKPOINT_METADATA_KEY, ThreadManager, +}; pub use runtime::messaging::ThreadOutcome; pub use runtime::mission::{ BudgetGate, FireRateLimit, MissionGateInfo, MissionManager, MissionNotification, MissionUpdate, diff --git a/crates/ironclaw_engine/src/runtime/manager.rs b/crates/ironclaw_engine/src/runtime/manager.rs index 2d154037771..2af17413967 100644 --- a/crates/ironclaw_engine/src/runtime/manager.rs +++ b/crates/ironclaw_engine/src/runtime/manager.rs @@ -648,13 +648,14 @@ impl ThreadManager { /// Reconcile persisted non-terminal threads after process startup. /// /// The current engine does not support mid-thread replay/resume, so any - /// thread left in a non-terminal state is marked failed-safe. + /// thread left in a non-terminal state is marked failed-safe. Threads + /// transitioned to `Failed` here carry the + /// [`ENGINE_RESTART_RECOVERY_METADATA_KEY`] flag so callers can + /// distinguish them from real, user-actionable failures. pub async fn recover_project_threads( &self, project_id: ProjectId, ) -> Result, EngineError> { - const PENDING_APPROVAL_METADATA_KEY: &str = "pending_approval"; - const RUNTIME_CHECKPOINT_METADATA_KEY: &str = "runtime_checkpoint"; // System operation: recover all non-terminal threads regardless of user. let threads = self.store.list_all_threads(project_id).await?; let mut recovered = Vec::new(); @@ -688,6 +689,16 @@ impl ThreadManager { continue; } + // Tag the thread before transitioning so downstream consumers + // (projects "needs attention" feed, health rollup) can skip + // restart-recovery noise and only surface real failures. + if let Some(obj) = thread.metadata.as_object_mut() { + obj.insert( + ENGINE_RESTART_RECOVERY_METADATA_KEY.to_string(), + serde_json::Value::Bool(true), + ); + } + if thread .transition_to( ThreadState::Failed, @@ -705,6 +716,24 @@ impl ThreadManager { } } +/// Metadata key set on a thread that has an in-flight pending-approval +/// gate. Persisted threads carrying this key skip restart-recovery so the +/// gate survives a process restart. +pub const PENDING_APPROVAL_METADATA_KEY: &str = "pending_approval"; + +/// Metadata key set on a thread that has a serialized runtime checkpoint +/// (CodeAct VM state, nudge counters, compaction count). Threads carrying +/// this key are suspended on restart instead of failed. +pub const RUNTIME_CHECKPOINT_METADATA_KEY: &str = "runtime_checkpoint"; + +/// Metadata key set on threads that were forced into `Failed` by +/// [`ThreadManager::recover_project_threads`] because the process +/// restarted before they could complete. The thread did not fail for +/// user-visible reasons; the projects "needs attention" surface filters +/// these out so an upgrade does not cascade into a wall of phantom +/// failure warnings. +pub const ENGINE_RESTART_RECOVERY_METADATA_KEY: &str = "engine_restart_recovery"; + fn is_resolved_call_message(message: &ThreadMessage, call_id: &str) -> bool { if message.role == MessageRole::ActionResult && message.action_call_id.as_deref() == Some(call_id) @@ -1563,6 +1592,28 @@ mod tests { assert_eq!(saved.state, ThreadState::Failed); let events = store.load_events(running.id).await.unwrap(); assert!(!events.is_empty()); + + // Restart-recovery flag must be set so the projects "needs + // attention" feed can filter these out (#3274). + assert_eq!( + saved + .metadata + .get(ENGINE_RESTART_RECOVERY_METADATA_KEY) + .and_then(|v| v.as_bool()), + Some(true), + "recovered thread should carry the engine_restart_recovery flag" + ); + + // Threads that were already terminal before recovery must NOT + // gain the flag — they failed for real reasons. + let saved_completed = store.load_thread(completed.id).await.unwrap().unwrap(); + assert!( + saved_completed + .metadata + .get(ENGINE_RESTART_RECOVERY_METADATA_KEY) + .is_none(), + "pre-existing failed thread must not be flagged as restart-recovery" + ); } #[tokio::test] diff --git a/crates/ironclaw_gateway/static/js/core/history.js b/crates/ironclaw_gateway/static/js/core/history.js index a3b2c91d329..e6257bea9aa 100644 --- a/crates/ironclaw_gateway/static/js/core/history.js +++ b/crates/ironclaw_gateway/static/js/core/history.js @@ -239,8 +239,15 @@ function loadHistory(before) { hasMore = data.has_more || false; oldestTimestamp = data.oldest_timestamp || null; - }).catch(() => { - // No history or no active thread + }).catch((err) => { + // Surface the error in DevTools and flag for SSE-open retry (#3274). + // The previous silent swallow left the user staring at an empty chat + // when the very first request after auth raced engine initialization + // and only a manual refresh recovered. + console.error('[chat] loadHistory failed:', err); + if (window._initialHydrationPending) { + window._initialHydrationPending.history = true; + } }).finally(() => { loadingOlder = false; removeScrollSpinner(); @@ -520,7 +527,12 @@ function loadThreads() { enableChatInput(); } } - }).catch(() => {}); + }).catch((err) => { + console.error('[chat] loadThreads failed:', err); + if (window._initialHydrationPending) { + window._initialHydrationPending.threads = true; + } + }); } function disableChatInputReadOnly() { diff --git a/crates/ironclaw_gateway/static/js/core/init-auth.js b/crates/ironclaw_gateway/static/js/core/init-auth.js index 45085efc2df..7b1061e370e 100644 --- a/crates/ironclaw_gateway/static/js/core/init-auth.js +++ b/crates/ironclaw_gateway/static/js/core/init-auth.js @@ -1,4 +1,35 @@ +// Tracks loaders that failed on the very first call after `initApp()`. The +// SSE `onopen` handler in `core/sse.js` retries each flagged loader exactly +// once — see `runInitialHydrationRetry` below. Defensive net for the upgrade +// race in #3274 where the first hydration request loses to in-flight engine +// state initialization or DB migration; a manual refresh used to be the only +// recovery path. See `.claude/rules/error-handling.md` (silent-failure rule). +function runInitialHydrationRetry() { + var pending = window._initialHydrationPending; + if (!pending || window._hydrationRetryDone) return; + window._hydrationRetryDone = true; + if (pending.threads && typeof loadThreads === 'function') { + console.info('[hydration] retrying loadThreads after SSE connect'); + loadThreads(); + } + if (pending.history && typeof loadHistory === 'function') { + console.info('[hydration] retrying loadHistory after SSE connect'); + loadHistory(); + } + if (pending.missions + && currentTab === 'missions' + && typeof loadMissions === 'function') { + console.info('[hydration] retrying loadMissions after SSE connect'); + loadMissions(); + } + window._initialHydrationPending = null; +} + function initApp() { + // Reset hydration tracker each time we (re-)initialize the app — token + // re-auth and OIDC auto-auth both flow through here. + window._initialHydrationPending = { threads: false, history: false, missions: false }; + window._hydrationRetryDone = false; var authScreen = document.getElementById('auth-screen'); var app = document.getElementById('app'); // Cross-fade: fade out auth screen, then show app diff --git a/crates/ironclaw_gateway/static/js/core/sse.js b/crates/ironclaw_gateway/static/js/core/sse.js index 2c72d1a23e1..419ce038de6 100644 --- a/crates/ironclaw_gateway/static/js/core/sse.js +++ b/crates/ironclaw_gateway/static/js/core/sse.js @@ -88,6 +88,12 @@ function connectSSE(lastEventIdOverride) { // Refresh sidebar so stale spinners are removed immediately. processingThreads.clear(); debouncedLoadThreads(); + // Retry any first-load loader (chat history, threads, missions) that + // raced engine init and failed silently. SSE-accept implies the + // backend has stabilized — see init-auth.js for the rationale (#3274). + if (typeof runInitialHydrationRetry === 'function') { + runInitialHydrationRetry(); + } sseHasConnectedBefore = true; }; diff --git a/crates/ironclaw_gateway/static/js/surfaces/projects.js b/crates/ironclaw_gateway/static/js/surfaces/projects.js index b6455f14e89..303ea1a607f 100644 --- a/crates/ironclaw_gateway/static/js/surfaces/projects.js +++ b/crates/ironclaw_gateway/static/js/surfaces/projects.js @@ -719,7 +719,15 @@ function loadMissions() { renderMissionsList(currentMissionList); renderMissionsActivity(threadData.threads || []); enrichMissionProgress(currentMissionList); - }).catch(function() {}); + }).catch(function(err) { + // See #3274: a silent catch here left the Missions tab blank when the + // first request raced engine init. Log + flag so the SSE-open retry + // in init-auth.js refetches once the engine is fully ready. + console.error('[missions] loadMissions failed:', err); + if (window._initialHydrationPending) { + window._initialHydrationPending.missions = true; + } + }); } function renderMissionsSummary(s) { diff --git a/src/bridge/router.rs b/src/bridge/router.rs index 53824063490..dfa01bbec4e 100644 --- a/src/bridge/router.rs +++ b/src/bridge/router.rs @@ -5760,6 +5760,35 @@ pub async fn get_engine_project( })) } +/// Whether `thread` should be surfaced as a user-actionable failure. +/// +/// A thread counts as a "real" failure for the projects "needs attention" +/// feed when: +/// - its state is `Failed`, AND +/// - it failed within the last 24 hours, AND +/// - it was NOT force-failed by `recover_project_threads` on engine +/// restart (those carry the +/// [`ironclaw_engine::ENGINE_RESTART_RECOVERY_METADATA_KEY`] flag and +/// are crash-recovery artifacts, not user errors). +/// +/// Filtering on the metadata flag fixes #3274: an upgrade transitioned +/// every still-running thread to `Failed`, which then flooded the +/// Projects tab with phantom "Thread failed" warnings. +fn is_real_thread_failure( + thread: &ironclaw_engine::types::thread::Thread, + h24_ago: chrono::DateTime, +) -> bool { + matches!( + thread.state, + ironclaw_engine::types::thread::ThreadState::Failed + ) && thread.updated_at >= h24_ago + && !thread + .metadata + .get(ironclaw_engine::ENGINE_RESTART_RECOVERY_METADATA_KEY) + .and_then(|v| v.as_bool()) + .unwrap_or(false) +} + /// Projects overview — health, stats, attention items for all projects. /// /// Iterates all projects, computes per-project stats from missions and threads, @@ -5855,12 +5884,14 @@ pub async fn get_engine_projects_overview( .map(|t| t.total_cost_usd) .sum(); + // Filter restart-recovery noise: `recover_project_threads` + // force-fails non-terminal threads on engine restart and tags + // them with `engine_restart_recovery`. They aren't actionable + // failures, so we exclude them from both the count and the + // attention feed (#3274). let failures_24h = threads .iter() - .filter(|t| { - matches!(t.state, ironclaw_engine::types::thread::ThreadState::Failed) - && t.updated_at >= h24_ago - }) + .filter(|t| is_real_thread_failure(t, h24_ago)) .count() as u64; let last_activity = threads @@ -5889,11 +5920,7 @@ pub async fn get_engine_projects_overview( }); } for thread in &threads { - if matches!( - thread.state, - ironclaw_engine::types::thread::ThreadState::Failed - ) && thread.updated_at >= h24_ago - { + if is_real_thread_failure(thread, h24_ago) { attention.push(AttentionItem { kind: "failure".to_string(), project_id: pid.to_string(), @@ -10294,6 +10321,88 @@ mod tests { ); } + // ── is_real_thread_failure (#3274) ────────────────────────────────── + + /// Helper: build a Failed thread with a configurable updated_at. + fn make_failed_thread(updated_at: chrono::DateTime) -> ironclaw_engine::Thread { + let mut t = ironclaw_engine::Thread::new( + "test goal", + ironclaw_engine::ThreadType::Foreground, + ironclaw_engine::ProjectId::new(), + "alice", + ironclaw_engine::ThreadConfig::default(), + ); + t.transition_to(ironclaw_engine::ThreadState::Running, None) + .unwrap(); + t.transition_to( + ironclaw_engine::ThreadState::Failed, + Some("LLM error".into()), + ) + .unwrap(); + t.updated_at = updated_at; + t + } + + #[test] + fn real_failure_recent_within_window() { + let now = chrono::Utc::now(); + let h24_ago = now - chrono::Duration::hours(24); + let t = make_failed_thread(now); + assert!( + super::is_real_thread_failure(&t, h24_ago), + "recent failed thread should surface as a real failure" + ); + } + + #[test] + fn real_failure_excluded_when_older_than_24h() { + let now = chrono::Utc::now(); + let h24_ago = now - chrono::Duration::hours(24); + let t = make_failed_thread(now - chrono::Duration::hours(25)); + assert!( + !super::is_real_thread_failure(&t, h24_ago), + "stale failure outside the 24h window must not be surfaced" + ); + } + + #[test] + fn real_failure_excludes_engine_restart_recovery() { + let now = chrono::Utc::now(); + let h24_ago = now - chrono::Duration::hours(24); + let mut t = make_failed_thread(now); + // Simulate `recover_project_threads` having tagged the thread. + if let Some(obj) = t.metadata.as_object_mut() { + obj.insert( + ironclaw_engine::ENGINE_RESTART_RECOVERY_METADATA_KEY.to_string(), + serde_json::Value::Bool(true), + ); + } + assert!( + !super::is_real_thread_failure(&t, h24_ago), + "restart-recovery threads must not surface as user-actionable failures" + ); + } + + #[test] + fn real_failure_ignores_non_failed_states() { + let now = chrono::Utc::now(); + let h24_ago = now - chrono::Duration::hours(24); + let mut t = ironclaw_engine::Thread::new( + "still running", + ironclaw_engine::ThreadType::Foreground, + ironclaw_engine::ProjectId::new(), + "alice", + ironclaw_engine::ThreadConfig::default(), + ); + t.transition_to(ironclaw_engine::ThreadState::Running, None) + .unwrap(); + t.updated_at = now; + assert!( + !super::is_real_thread_failure(&t, h24_ago), + "Running thread must not be classified as a failure" + ); + } + // ── persist_always_allow / revert_always_allow ───────────────────── /// Minimal in-memory SettingsStore for persistence tests.