Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion crates/ironclaw_engine/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
57 changes: 54 additions & 3 deletions crates/ironclaw_engine/src/runtime/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vec<ThreadId>, 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();
Expand Down Expand Up @@ -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,
Expand All @@ -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";
Comment on lines +719 to +727

/// 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)
Expand Down Expand Up @@ -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]
Expand Down
18 changes: 15 additions & 3 deletions crates/ironclaw_gateway/static/js/core/history.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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() {
Expand Down
31 changes: 31 additions & 0 deletions crates/ironclaw_gateway/static/js/core/init-auth.js
Original file line number Diff line number Diff line change
@@ -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;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Severity

The hydration retry can be consumed before any loader has failed.

runInitialHydrationRetry() marks _hydrationRetryDone = true and later clears _initialHydrationPending even when all pending flags are still false. In initApp(), connectSSE() is called before the initial loadThreads() call, so a fast SSE onopen can run this function first, consume the one retry, and clear the tracker. If the first loadThreads / loadHistory / loadMissions request then rejects, the catch block sees _initialHydrationPending === null and cannot flag itself for retry. That leaves the same blank initial UI state this change is trying to recover from.

Consider only marking the retry as done when at least one pending flag is true, and have loader catches trigger the retry immediately when SSE is already open (or track an explicit initialHydrationSseReady flag).

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
Expand Down
6 changes: 6 additions & 0 deletions crates/ironclaw_gateway/static/js/core/sse.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
10 changes: 9 additions & 1 deletion crates/ironclaw_gateway/static/js/surfaces/projects.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
127 changes: 118 additions & 9 deletions src/bridge/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<chrono::Utc>,
) -> 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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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<chrono::Utc>) -> 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.
Expand Down
Loading