Skip to content

Commit fed2140

Browse files
author
CodeWhale Bot
committed
fix(tui): preserve and surface interrupted sessions
Retain crash checkpoints, discover recoverable sessions in the picker, and promote only under the session lease with workspace and saved-state checks. Preserve the shared legacy slot and fail closed when damaged recovery could otherwise be overwritten. Verified on macOS with the locked hermetic TUI checks: checkpoint 78 passed, continue 12 passed, picker rename 1 passed, picker archive 1 passed. The linker reported a compact-unwind size warning. Legacy-slot cleanup and old-writer coordination remain limited.
1 parent 3690f9c commit fed2140

7 files changed

Lines changed: 636 additions & 250 deletions

File tree

‎crates/tui/src/lib.rs‎

Lines changed: 59 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -10064,14 +10064,14 @@ async fn prepare_mounted_session(
1006410064
.ok_or_else(|| anyhow!("No saved sessions found for this workspace"))?
1006510065
.id
1006610066
} else {
10067-
manager.resolve_session_id_prefix(&selector)?
10067+
manager.resolve_recoverable_session_id_prefix(&selector)?
1006810068
};
1006910069
// Under the existing inactive-session guard, promote any crash
1007010070
// checkpoint first: the mount below stamps a newer document, and the
1007110071
// later attach would then discard the checkpoint as stale. Then
1007210072
// capture the complete document and separate local goal.
1007310073
let _source_lease = manager.reserve_session_for_external_write(&id)?;
10074-
let stale_checkpoint = manager.promote_interrupted_checkpoint(&id);
10074+
let stale_checkpoint = manager.promote_interrupted_checkpoint(&id, Some(&selection_workspace))?;
1007510075
let saved = manager.load_session_snapshot_bounded(&id, MAX_CANONICAL_HISTORY_BYTES)?;
1007610076
if stale_checkpoint {
1007710077
let _ = manager.clear_session_checkpoint(&id);
@@ -12960,23 +12960,17 @@ fn default_mouse_capture_enabled(
1296012960
true
1296112961
}
1296212962

12963-
/// A loadable crash-recovery checkpoint candidate: session content, file
12964-
/// age, and which slot it came from (per-session file or the legacy single
12965-
/// slot).
12963+
/// A crash-recovery checkpoint candidate: session metadata and file age.
1296612964
struct RecentCheckpoint {
12967-
session: session_manager::SavedSession,
12965+
metadata: session_manager::SessionMetadata,
1296812966
age: std::time::Duration,
12969-
source: session_manager::CheckpointSource,
1297012967
}
1297112968

12972-
const CHECKPOINT_MAX_AGE: std::time::Duration = std::time::Duration::from_secs(24 * 3600);
12973-
12974-
/// Load all recent crash-recovery checkpoints, pruning stale ones first.
12969+
/// Load crash-recovery checkpoints.
1297512970
///
1297612971
/// Candidates are the per-session checkpoint files plus the legacy
12977-
/// single-slot `checkpoints/latest.json` (compatibility read). Files older
12978-
/// than 24 hours are removed; unreadable files are skipped. The result is
12979-
/// sorted most recent first.
12972+
/// single-slot `checkpoints/latest.json` (compatibility read). Unreadable
12973+
/// files are skipped. The result is sorted most recent first.
1298012974
fn load_recent_checkpoints(manager: &session_manager::SessionManager) -> Vec<RecentCheckpoint> {
1298112975
let refs = manager.list_checkpoints().unwrap_or_default();
1298212976
let mut recent = Vec::new();
@@ -12986,39 +12980,16 @@ fn load_recent_checkpoints(manager: &session_manager::SessionManager) -> Vec<Rec
1298612980
// take that session's only crash-recovery record while it runs. The
1298712981
// legacy slot is checked by the session it names, before anything
1298812982
// prunes, promotes or migrates it over that session's live state.
12989-
let owner = match &checkpoint_ref.source {
12990-
session_manager::CheckpointSource::Session(id) => Some(id.clone()),
12991-
session_manager::CheckpointSource::Legacy => {
12992-
manager.legacy_checkpoint_origin().ok().flatten()
12993-
}
12994-
};
12995-
if owner.is_some_and(|id| manager.is_session_live_anywhere(&id)) {
12996-
continue;
12997-
}
12998-
let Ok(age) = std::time::SystemTime::now().duration_since(checkpoint_ref.modified) else {
12983+
let Ok(Some(metadata)) = manager.checkpoint_metadata(&checkpoint_ref) else {
1299912984
continue;
1300012985
};
13001-
if age > CHECKPOINT_MAX_AGE {
13002-
let _ = match &checkpoint_ref.source {
13003-
session_manager::CheckpointSource::Session(id) => {
13004-
manager.clear_session_checkpoint(id)
13005-
}
13006-
session_manager::CheckpointSource::Legacy => manager.clear_legacy_checkpoint(),
13007-
};
12986+
if manager.is_session_live_anywhere(&metadata.id) {
1300812987
continue;
1300912988
}
13010-
let loaded = match &checkpoint_ref.source {
13011-
session_manager::CheckpointSource::Session(id) => manager.load_session_checkpoint(id),
13012-
session_manager::CheckpointSource::Legacy => manager.load_legacy_checkpoint(),
13013-
};
13014-
let Ok(Some(session)) = loaded else {
12989+
let Ok(age) = std::time::SystemTime::now().duration_since(checkpoint_ref.modified) else {
1301512990
continue;
1301612991
};
13017-
recent.push(RecentCheckpoint {
13018-
session,
13019-
age,
13020-
source: checkpoint_ref.source,
13021-
});
12992+
recent.push(RecentCheckpoint { metadata, age });
1302212993
}
1302312994
// `list_checkpoints` sorts newest-first already; keep it explicit here so
1302412995
// selection does not silently depend on the manager's ordering.
@@ -13041,9 +13012,9 @@ fn checkpoint_age_label(age: std::time::Duration) -> String {
1304113012
/// workspace.
1304213013
///
1304313014
/// Candidates are all per-session checkpoint files plus the legacy
13044-
/// single-slot `checkpoints/latest.json`; each must be younger than 24 hours
13045-
/// **and its workspace must match the resolved launch workspace after
13046-
/// canonicalisation** — the newest matching candidate wins. If no candidate
13015+
/// single-slot `checkpoints/latest.json`; its workspace must match the
13016+
/// resolved launch workspace after canonicalisation — the newest matching
13017+
/// candidate wins. If no candidate
1304713018
/// matches, a one-line notice points at `codewhale sessions`, and nothing is
1304813019
/// auto-loaded: another workspace's checkpoint file is never touched (it may
1304913020
/// belong to a live session there).
@@ -13081,10 +13052,7 @@ fn recover_interrupted_checkpoint_for_resume(launch_workspace: &Path) -> Option<
1308113052
// using `--workspace` cannot accidentally recover a checkpoint from the
1308213053
// directory their shell happened to be in.
1308313054
let (matching, mismatched): (Vec<_>, Vec<_>) = candidates.into_iter().partition(|candidate| {
13084-
session_manager::workspace_scope_matches(
13085-
&candidate.session.metadata.workspace,
13086-
launch_workspace,
13087-
)
13055+
session_manager::workspace_scope_matches(&candidate.metadata.workspace, launch_workspace)
1308813056
});
1308913057

1309013058
let Some(best) = matching.into_iter().next() else {
@@ -13093,46 +13061,49 @@ fn recover_interrupted_checkpoint_for_resume(launch_workspace: &Path) -> Option<
1309313061
"Note: an interrupted session from another workspace ({}) is \
1309413062
available. Run `codewhale sessions` to list saved sessions. Starting \
1309513063
fresh in {}.",
13096-
newest.session.metadata.workspace.display(),
13064+
newest.metadata.workspace.display(),
1309713065
launch_workspace.display(),
1309813066
);
1309913067
}
1310013068
return None;
1310113069
};
1310213070

13103-
let session_id = best.session.metadata.id.clone();
13071+
let session_id = best.metadata.id.clone();
1310413072

1310513073
// Hold the session's live lease while promoting or clearing anything:
1310613074
// the liveness filter above is a check, and another terminal can attach
1310713075
// between it and these writes. Losing the race leaves every file alone.
1310813076
// The lease is released on return, as `--resume <id>` does, so the resume
1310913077
// capture that follows can take it.
13110-
let Ok(_lease) = manager.reserve_session_for_attach(&session_id) else {
13111-
return None;
13078+
let _lease = match manager.reserve_session_for_attach(&session_id) {
13079+
Ok(lease) => lease,
13080+
Err(error) => {
13081+
eprintln!("Interrupted session was preserved but could not be resumed: {error}");
13082+
return None;
13083+
}
1311213084
};
1311313085

13114-
// Persist the checkpoint as a regular session so the TUI can load it by
13115-
// id — unless a newer regular session file for the same id already
13116-
// exists (e.g. `--continue` ran before and the session advanced since).
13117-
// A stale checkpoint must never overwrite newer durable session state.
13118-
if !saved_session_is_newer(&manager, &best.session)
13119-
&& manager.save_session(&best.session).is_err()
13120-
{
13086+
let stale_checkpoint =
13087+
match manager.promote_interrupted_checkpoint(&session_id, Some(launch_workspace)) {
13088+
Ok(stale) => stale,
13089+
Err(error) => {
13090+
eprintln!("Interrupted session was preserved but could not be resumed: {error}");
13091+
return None;
13092+
}
13093+
};
13094+
let recovered = match manager.load_session(&session_id) {
13095+
Ok(recovered) => recovered,
13096+
Err(error) => {
13097+
eprintln!("Interrupted session was preserved but could not be resumed: {error}");
13098+
return None;
13099+
}
13100+
};
13101+
if !session_manager::workspace_scope_matches(&recovered.metadata.workspace, launch_workspace) {
13102+
eprintln!("Interrupted session was preserved but belongs to another workspace.");
1312113103
return None;
1312213104
}
13123-
13124-
match &best.source {
13125-
session_manager::CheckpointSource::Session(id) => {
13126-
// Consume the per-session checkpoint now that it is recovered.
13127-
let _ = manager.clear_session_checkpoint(id);
13128-
}
13129-
session_manager::CheckpointSource::Legacy => {
13130-
// Migrate the legacy slot to a per-session file (never
13131-
// overwriting an existing one) and leave `latest.json` in place
13132-
// so an older binary can still find it; its writer is already
13133-
// gone and the file ages out within 24 hours.
13134-
let _ = manager.write_session_checkpoint_if_absent(&best.session);
13135-
}
13105+
if stale_checkpoint {
13106+
let _ = manager.clear_session_checkpoint(&session_id);
1313613107
}
1313713108

1313813109
let age_str = checkpoint_age_label(best.age);
@@ -13141,43 +13112,20 @@ fn recover_interrupted_checkpoint_for_resume(launch_workspace: &Path) -> Option<
1314113112
Some(session_id)
1314213113
}
1314313114

13144-
/// Whether a regular session file for the checkpoint's id already exists and
13145-
/// is at least as recent as the checkpoint. When it is, persisting the
13146-
/// checkpoint over it would replace newer durable state with older in-flight
13147-
/// state.
13148-
fn saved_session_is_newer(
13149-
manager: &session_manager::SessionManager,
13150-
checkpoint: &session_manager::SavedSession,
13151-
) -> bool {
13152-
manager
13153-
.load_session(&checkpoint.metadata.id)
13154-
.is_ok_and(|existing| existing.metadata.updated_at >= checkpoint.metadata.updated_at)
13155-
}
13156-
1315713115
/// Preserve an interrupted checkpoint on a normal fresh launch without
1315813116
/// attaching it to the new TUI instance. This keeps "open another codewhale in
1315913117
/// the same folder" from re-entering the previous in-flight session while still
1316013118
/// leaving an explicit resume path.
1316113119
///
1316213120
/// Only the newest recent checkpoint drives the notice, which is returned for
13163-
/// the TUI to show. The legacy single-slot file is migrated to a per-session
13164-
/// checkpoint and consumed; per-session checkpoint files are left in place —
13121+
/// the TUI to show. Checkpoint files are left in place —
1316513122
/// they may belong to a live session in another terminal, and `--continue`
1316613123
/// reads them directly.
1316713124
fn preserve_interrupted_checkpoint_for_explicit_resume(launch_workspace: &Path) -> Option<String> {
1316813125
let manager = session_manager::SessionManager::default_location().ok()?;
1316913126
let newest = load_recent_checkpoints(&manager).into_iter().next()?;
1317013127

13171-
let session_workspace = newest.session.metadata.workspace.clone();
13172-
// #4479: removed save_session call — checkpoint should not be auto-promoted to session
13173-
if newest.source == session_manager::CheckpointSource::Legacy {
13174-
// Migrate legacy single-slot checkpoint to per-session format
13175-
// before clearing the legacy file, or the data is unrecoverable.
13176-
if manager.save_checkpoint(&newest.session).is_ok() {
13177-
let _ = manager.clear_legacy_checkpoint();
13178-
}
13179-
}
13180-
13128+
let session_workspace = newest.metadata.workspace.clone();
1318113129
let age_str = checkpoint_age_label(newest.age);
1318213130
Some(
1318313131
if session_manager::workspace_scope_matches(&session_workspace, launch_workspace) {
@@ -20078,7 +20026,7 @@ mod setup_helper_tests {
2007820026
}
2007920027

2008020028
#[test]
20081-
fn plain_launch_consumes_legacy_checkpoint_after_preserving_it() {
20029+
fn plain_launch_preserves_legacy_checkpoint() {
2008220030
let _guard = crate::test_support::lock_test_env();
2008320031
let tmp = TempDir::new().unwrap();
2008420032
let workspace = tmp.path().join("workspace");
@@ -20108,16 +20056,16 @@ mod setup_helper_tests {
2010820056
manager
2010920057
.load_legacy_checkpoint()
2011020058
.expect("load legacy checkpoint")
20111-
.is_none(),
20112-
"normal launch should consume the legacy single-slot checkpoint"
20059+
.is_some(),
20060+
"normal launch must preserve the legacy single-slot checkpoint"
2011320061
);
2011420062
// #4479: checkpoint is no longer promoted to session file.
2011520063
assert!(
2011620064
manager
2011720065
.load_session_checkpoint(&session_id)
2011820066
.expect("load checkpoint")
20119-
.is_some(),
20120-
"checkpoint stays in checkpoints/ for --continue"
20067+
.is_none(),
20068+
"fresh launch need not copy the legacy checkpoint"
2012120069
);
2012220070
});
2012320071
}
@@ -20140,7 +20088,15 @@ mod setup_helper_tests {
2014020088
}];
2014120089
let session = create_saved_session(&messages, "test-model", &workspace, 0, None);
2014220090
let session_id = session.metadata.id.clone();
20143-
manager.save_checkpoint(&session).expect("save checkpoint");
20091+
let checkpoint = manager.save_checkpoint(&session).expect("save checkpoint");
20092+
std::fs::File::options()
20093+
.write(true)
20094+
.open(&checkpoint)
20095+
.expect("open checkpoint")
20096+
.set_times(std::fs::FileTimes::new().set_modified(
20097+
std::time::SystemTime::now() - std::time::Duration::from_secs(48 * 3600),
20098+
))
20099+
.expect("age checkpoint");
2014420100

2014520101
let recovered = recover_interrupted_checkpoint_for_resume(&workspace);
2014620102

@@ -20360,7 +20316,7 @@ mod setup_helper_tests {
2036020316
}
2036120317

2036220318
#[test]
20363-
fn continue_recovers_legacy_checkpoint_and_migrates_it() {
20319+
fn continue_recovers_legacy_checkpoint_without_removing_it() {
2036420320
let _guard = crate::test_support::lock_test_env();
2036520321
let tmp = TempDir::new().unwrap();
2036620322
let workspace = tmp.path().join("workspace");
@@ -20386,13 +20342,6 @@ mod setup_helper_tests {
2038620342
manager.load_session(&session_id).is_ok(),
2038720343
"recovered legacy checkpoint must be loadable as a session"
2038820344
);
20389-
assert!(
20390-
manager
20391-
.load_session_checkpoint(&session_id)
20392-
.expect("load per-session checkpoint")
20393-
.is_some(),
20394-
"legacy recovery must migrate to a per-session checkpoint file"
20395-
);
2039620345
assert!(
2039720346
manager
2039820347
.load_legacy_checkpoint()

0 commit comments

Comments
 (0)