fix(session): refuse file rollback under a running turn, recover the crash checkpoint on resume, keep imports on the window's route

Three defects found by native acceptance of the previous head, plus the
platform fixes its hosted run asked for.

- `/undo` and `/restore <N>` rewrote workspace files while a turn was running
  in that workspace, discarding in-flight work and leaving the model's view of
  the files wrong. Both now refuse while a turn is active or compacting, like
  the Runtime's restore routes; nothing is changed on refusal.
- `--resume <id>` after a crash opened the older saved document and the next
  save cleared the session's crash checkpoint, losing the interrupted turn
  while its file edits stayed on disk. An explicit resume now promotes that
  session's own newer checkpoint first, in the same order `--continue` uses;
  a session live elsewhere or a newer saved document is left alone.
- `/resume <file>` imported a session with the record's default provider
  instead of the window's route, so opening it sent the conversation to
  another provider's endpoint. The import now binds to the window's route.

Hosted failures on e86d67711 (Linux, Windows, release parity):
- four Computer Use tests start the bundled plugin, which applies only to
  macOS hosts; they return early elsewhere;
- the structured-copy key test used a fixture key ending in `key`, which the
  shared vocabulary redacts by design; the fixture key no longer looks like a
  credential and all five values are asserted again;
- the read-guard file-tool test redirects the home directory through `HOME`,
  which Windows does not use; it is Unix-only.

Local macOS, Rust 1.99: focused codewhale-tui lib selections 302 passed /
0 failed (undo, restore, checkpoint, resume, continue) and 354 passed /
0 failed (import, control, resume); 102 passed / 0 failed for the platform
test fixes. CI-policy Clippy, whole workspace, all targets: exit 0.
Native evidence for the three defects is in the acceptance receipts; the
fixes themselves have not yet been re-run natively.

Signed-off-by: CodeWhale Bot <[email protected]>
This commit is contained in:
CodeWhale Bot committed 2026-10-01 09:32:39 -07:00
1 parent 8cef5e7ee9
commit df755a38af
8 files changed
+186 -2

No files matched your search

+22 -1
View File
@@ -1439,11 +1439,21 @@ fn import_session_container(
};
let model = app.model.clone();
let workspace = app.workspace.clone();
let imported =
let mut imported =
match crate::session_manager::SavedSession::import_foreign(container, workspace, model) {
Ok(s) => s,
Err(e) => return Err(format!("foreign import failed: {e}")),
};
// The import takes this window's model, so it takes this window's route
// too. Left at the record default it named a different provider, and
// opening the imported session sent its whole history there.
let (provider, provider_id) = (
app.provider_identity_for_persistence().to_string(),
app.provider_id_for_persistence().map(str::to_string),
);
imported
.metadata
.set_model_provider_route(&provider, provider_id.as_deref());
let new_id = imported.metadata.id.clone();
let queue_transition = crate::tui::ui::prepare_offline_queue_transition(app, &new_id)?;
if let Err(e) = manager.save_session(&imported) {
@@ -3138,6 +3148,11 @@ impl CommandSkillGroupContext for SkillGroupAdapter<'_> {
}
fn restore_snapshot(&mut self, id: &str) -> Result<(), String> {
if let Some(refusal) =
debug_operations::active_turn_restore_refusal(&self.host.app.borrow())
{
return Err(refusal);
}
let workspace = self.host.app.borrow().workspace.clone();
let id = id.to_owned();
let restore = move || {
@@ -7244,6 +7259,8 @@ mod tests {
let import_file = tmpdir.path().join("foreign-export.json");
std::fs::write(&import_file, &json).unwrap();
// This window is on a non-default route; the import must bind to it.
app.api_provider = crate::config::ApiProvider::Openai;
let receipt = {
let mut bundle = app.command_contexts();
let mut parts = bundle.parts();
@@ -7261,6 +7278,10 @@ mod tests {
// (fresh id/title), matching the baseline import path exactly.
assert_eq!(saved.metadata.title, "New Session");
assert_ne!(saved.metadata.id, "foreign-source");
assert_eq!(
saved.metadata.model_provider, "openai",
"an imported session runs on this window's route, not a default one"
);
assert!(
manager
.sessions_dir()
@@ -497,7 +497,26 @@ const POST_TURN_SNAPSHOT_WAIT: std::time::Duration = std::time::Duration::from_s
///
/// Posts a `HistoryCell::System` entry so the user can see what was
/// reverted in the transcript.
/// Why workspace files may not be rolled back right now, if they may not.
///
/// A running turn is reading and writing this workspace: restoring files
/// under it discards the turn's in-flight work and leaves the model's view of
/// the files wrong. `/undo` and `/restore` refuse while one is active, like
/// the Runtime's restore routes.
pub(in crate::commands) fn active_turn_restore_refusal(app: &App) -> Option<String> {
let turn_active = app.is_loading
|| app.is_compacting
|| matches!(app.runtime_turn_status.as_deref(), Some("in_progress"));
turn_active.then(|| {
"A turn is still running in this workspace, so files were not restored and nothing was changed. Wait for it to finish, or press Esc to stop it, then run the command again."
.to_string()
})
}
pub(in crate::commands) fn undo_files(app: &mut App) -> DebugUndoOutcome {
if let Some(refusal) = active_turn_restore_refusal(app) {
return DebugUndoOutcome::RestoreBlocked(refusal);
}
let workspace = app.workspace.clone();
let repo = match crate::snapshot::SnapshotRepo::open_or_init(&workspace) {
@@ -309,7 +309,27 @@ fn test_patch_undo_requests_session_resync_after_restore() {
}],
});
// A running turn owns the workspace: nothing is restored under it.
app.is_loading = true;
let refused = patch_undo(&mut app);
assert!(
refused
.message
.as_deref()
.is_some_and(|message| message.contains("still running")),
"{:?}",
refused.message
);
assert!(refused.action.is_none());
assert_eq!(
std::fs::read(workspace.join("a.txt")).unwrap(),
b"modified",
"a refused undo changes no file"
);
app.is_loading = false;
let result = patch_undo(&mut app);
assert_eq!(std::fs::read(workspace.join("a.txt")).unwrap(), b"original");
assert!(!result.is_error);
assert!(matches!(
@@ -587,7 +587,9 @@ fn hostile_object_keys_are_scrubbed_bounded_and_deduped_deterministically() {
long_a.clone(): 1,
long_b.clone(): 2,
long_c.clone(): 3,
"\u{1b}[31mansi\u{1b}[0m\nkey": 4,
// Not a credential-shaped name: a key ending in `key` is redacted by
// the shared vocabulary, which other tests cover.
"\u{1b}[31mansi\u{1b}[0m\nlabel": 4,
format!("at {workspace}/src"): 5,
});
app.api_messages = std::sync::Arc::new(vec![Message {
+15
View File
@@ -30266,6 +30266,11 @@ async fn run_computer_use_live_card_case(
#[tokio::test]
async fn computer_use_human_card_allows_the_exact_live_call() {
// The bundled Computer Use plugin applies only to macOS hosts; elsewhere
// there is no live plugin to start.
if !cfg!(target_os = "macos") {
return;
}
let _env = lock_test_env();
run_computer_use_live_card_case(
ApprovalMode::Suggest,
@@ -30278,6 +30283,11 @@ async fn computer_use_human_card_allows_the_exact_live_call() {
#[tokio::test]
async fn computer_use_session_rule_and_posture_cannot_mint_a_human_decision() {
// The bundled Computer Use plugin applies only to macOS hosts; elsewhere
// there is no live plugin to start.
if !cfg!(target_os = "macos") {
return;
}
let _env = lock_test_env();
for decider in [
crate::approval_log::ApprovalDecider::SessionRule,
@@ -30289,6 +30299,11 @@ async fn computer_use_session_rule_and_posture_cannot_mint_a_human_decision() {
#[tokio::test]
async fn computer_use_autonomous_postures_block_before_the_plugin() {
// The bundled Computer Use plugin applies only to macOS hosts; elsewhere
// there is no live plugin to start.
if !cfg!(target_os = "macos") {
return;
}
let _env = lock_test_env();
for posture in [
ApprovalMode::Bypass,
+99
View File
@@ -2895,6 +2895,9 @@ async fn run_async_main_dispatch(
io::stdin().is_terminal() && io::stdout().is_terminal(),
)
} else if let Some(id) = cli.resume.clone() {
if io::stdin().is_terminal() && io::stdout().is_terminal() {
promote_interrupted_checkpoint_for_session(&id);
}
Some(id)
} else if !cli.fresh {
let workspace = resolve_workspace(&cli);
@@ -11746,6 +11749,36 @@ fn recover_interrupted_checkpoint_for_resume(launch_workspace: &Path) -> Option<
Some(session_id)
}
/// `--resume <id>` of a session that was interrupted mid-turn opens the
/// interrupted state, as `--continue` does. Resuming the older saved document
/// instead dropped the in-flight turn, and the next save then cleared the
/// only record of it while the turn's file edits stayed on disk.
///
/// Only an exact session id finds its checkpoint; a session open in another
/// terminal, or a saved document at least as new, is left alone.
fn promote_interrupted_checkpoint_for_session(session_id: &str) {
let Ok(manager) = session_manager::SessionManager::default_location() else {
return;
};
if manager.is_session_live_anywhere(session_id) {
return;
}
let Ok(Some(checkpoint)) = manager.load_session_checkpoint(session_id) else {
return;
};
// Same order as `--continue`: hold the session before touching its files.
match manager.reserve_session_for_attach(session_id) {
Ok(lease) => lease.commit(),
Err(_) => return,
}
if !saved_session_is_newer(&manager, &checkpoint) && manager.save_session(&checkpoint).is_err()
{
return;
}
let _ = manager.clear_session_checkpoint(session_id);
eprintln!("Recovered this session's interrupted turn from its crash checkpoint.");
}
/// Whether a regular session file for the checkpoint's id already exists and
/// is at least as recent as the checkpoint. When it is, persisting the
/// checkpoint over it would replace newer durable state with older in-flight
@@ -21730,6 +21763,72 @@ mod setup_helper_tests {
});
}
/// `--resume <id>` after a crash opens the interrupted turn, not the
/// older saved document, and never replaces a newer document.
#[test]
fn explicit_resume_promotes_that_sessions_newer_crash_checkpoint() {
let _guard = crate::test_support::lock_test_env();
let tmp = TempDir::new().unwrap();
let workspace = tmp.path().join("workspace");
std::fs::create_dir_all(&workspace).unwrap();
with_home(tmp.path(), || {
let manager = SessionManager::default_location().expect("manager");
let user = |text: &str| Message {
role: Role::User,
content: vec![ContentBlock::Text {
text: text.to_string(),
cache_control: None,
}],
};
let saved = create_saved_session(&[user("first")], "test-model", &workspace, 0, None);
let session_id = saved.metadata.id.clone();
manager.save_session(&saved).expect("save session");
let mut interrupted = saved.clone();
interrupted.messages.push(user("in-flight turn"));
interrupted.metadata.updated_at =
saved.metadata.updated_at + chrono::Duration::seconds(5);
manager
.save_checkpoint(&interrupted)
.expect("save checkpoint");
promote_interrupted_checkpoint_for_session(&session_id);
assert_eq!(
manager
.load_session(&session_id)
.expect("session")
.messages
.len(),
2,
"the interrupted turn is part of the resumed session"
);
assert!(
manager
.load_session_checkpoint(&session_id)
.expect("load checkpoint")
.is_none(),
"the recovered checkpoint is consumed"
);
// A stale checkpoint never replaces a newer saved document.
let mut stale = saved.clone();
stale.metadata.updated_at = saved.metadata.updated_at - chrono::Duration::seconds(5);
manager
.save_checkpoint(&stale)
.expect("save stale checkpoint");
promote_interrupted_checkpoint_for_session(&session_id);
assert_eq!(
manager
.load_session(&session_id)
.expect("session")
.messages
.len(),
2
);
});
}
/// Write a legacy single-slot checkpoint file the way pre-cutover
/// binaries did. The current binary only reads this slot.
fn write_legacy_checkpoint(manager: &SessionManager, session: &session_manager::SavedSession) {
+5
View File
@@ -10330,6 +10330,11 @@ pub(crate) fn computer_use_test_fixture() -> (
#[tokio::test]
async fn computer_use_real_plugin_host_handshake_rejects_tamper_replay_and_late_keys() {
// The bundled Computer Use plugin applies only to macOS hosts; elsewhere
// there is no live plugin to start.
if !cfg!(target_os = "macos") {
return;
}
let _env = crate::test_support::lock_test_env();
let (root, _registry, mut pool, server) = computer_use_test_fixture();
let _home = crate::test_support::EnvVarGuard::set("CODEWHALE_HOME", root.path());
+3
View File
@@ -771,6 +771,9 @@ mod tests {
.unwrap_or_else(|poisoned| poisoned.into_inner())
}
// Unix only: the fixture redirects the home directory through `HOME`,
// and Windows resolves the profile through the known-folder API instead.
#[cfg(unix)]
#[test]
fn sandbox_read_guard_denies_active_and_known_agent_credentials_in_real_file_tool() {
use crate::tools::spec::{ToolContext, ToolSpec};