codewhale-hq/Codewhale
1
0 Fork 0
mirror of https://github.com/codewhale-hq/Codewhale.git synced 2026-11-08 08:04:25 -10:00
Code Issues Packages Projects Releases 1 Wiki Activity

fix(runtime): bind file rewind to the selected reviewed range [skip ci]

Browse Source 
Refs CW-7670, CW-6966. patch-undo ignored turn_id and undid the latest turn. Reuse the selected-user-turn preparer already used by conversation undo, retain restore admission through file changes and fork publication, and advertise thread_patch_undo_turn_id. GET previews share the physical restore planner; POST can require the exact dropped turn IDs and affected paths so changed selections fail before writes. Existing trust, owned-snapshot attribution, safety backup and later-edit refusal stay authoritative. Default depth and public restore helper behavior remain compatible. Generated protocol/SDK contracts and feature docs are updated.

Verified 22 distinct focused existing checks, 0 failed: 14 AppServer/protocol after the file-plan amendment; 8 Engine fork/restore cases reused against the exact same built binary. All 4,249 inputs matched the guarded native26 build and frozen source. Actual native macOS development CLI: five tool-write turns plus stale/foreign/range/path/later-edit/active-work refusals, named selection overriding depth, exact restoration/deletion, retained source and restart passed. Mounted Electron paired with native26 passed preview/cancel, separately confirmed conversation-only fallback, double-click guard, restore, composer and restart; no restore-triggered model request. Codewhale DeepSeek Flash supplied a read-only design review; root checked its suggestions against source and adopted exact path consent.

Scoped evidence only: no optimized package or Windows acceptance, no release/deployment. Restore and fork publication are not a single crash-atomic cross-store transaction; partial operational failures preserve the safety snapshot/source and are not reported as successful rewinds. This endpoint does not promise operation-key idempotency.
This commit is contained in:
CodeWhale Bot committed 2026-11-03 04:49:25 -08:00
1 parent d29abf3124
commit eb17ae0ec1
9 files changed
+257 -42
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/app-server-protocol/schema/json/runtime.schema.json
+5
View File
@@ -281,6 +281,10 @@
"type": "boolean",
"default": false
},
"thread_patch_undo_turn_id": {
"type": "boolean",
"default": false
},
"thread_shell_consent": {
"type": "boolean",
"description": "Explicit per-thread shell opt-in is checked against loaded policy and\ncannot broaden a conversation while it has an active turn.",
@@ -444,6 +448,7 @@
"threads",
"thread_create_idempotency",
"thread_idle_precondition",
"thread_patch_undo_turn_id",
"thread_shell_consent",
"turns",
"turn_operation_idempotency",
crates/app-server-protocol/schema/typescript/runtime.d.ts
Vendored
+1
View File
@@ -47,6 +47,7 @@ export type RuntimeCapabilities = {
threads: boolean
thread_create_idempotency: boolean
thread_idle_precondition: boolean
thread_patch_undo_turn_id: boolean
thread_shell_consent: boolean
turns: boolean
turn_operation_idempotency: boolean
crates/app-server-protocol/src/runtime.rs
+14
View File
@@ -86,6 +86,8 @@ pub struct RuntimeCapabilities {
pub thread_create_idempotency: bool,
#[serde(default)]
pub thread_idle_precondition: bool,
#[serde(default)]
pub thread_patch_undo_turn_id: bool,
/// Explicit per-thread shell opt-in is checked against loaded policy and
/// cannot broaden a conversation while it has an active turn.
#[serde(default)]
@@ -495,6 +497,7 @@ mod tests {
threads: true,
thread_create_idempotency: true,
thread_idle_precondition: true,
thread_patch_undo_turn_id: true,
thread_shell_consent: true,
turns: true,
turn_operation_idempotency: true,
@@ -543,6 +546,17 @@ mod tests {
.client_token_intents
);
assert_eq!(obj.get("threads").unwrap(), &json!(true));
assert_eq!(obj.get("thread_patch_undo_turn_id"), Some(&json!(true)));
let mut legacy_patch_undo = value.clone();
legacy_patch_undo
.as_object_mut()
.unwrap()
.remove("thread_patch_undo_turn_id");
assert!(
!serde_json::from_value::<RuntimeCapabilities>(legacy_patch_undo)
.unwrap()
.thread_patch_undo_turn_id
);
assert_eq!(obj.get("thread_shell_consent"), Some(&json!(true)));
assert!(
serde_json::from_value::<RuntimeCapabilities>(value.clone())
crates/app-server/src/runtime_api.rs
+163 -34
View File
@@ -1569,6 +1569,7 @@ fn default_runtime_capabilities() -> RuntimeCapabilities {
threads: true,
thread_create_idempotency: true,
thread_idle_precondition: true,
thread_patch_undo_turn_id: true,
thread_shell_consent: true,
turns: true,
turn_operation_idempotency: true,
@@ -3081,7 +3082,10 @@ pub fn build_router(state: RuntimeApiState) -> Router {
.route("/v1/threads/{id}/fork", post(fork_thread))
.route("/v1/threads/{id}/undo", post(undo_thread_turn))
.route("/v1/threads/{id}/fork-at-turn", post(fork_thread_at_turn))
.route("/v1/threads/{id}/patch-undo", post(patch_undo_thread_turn))
.route(
"/v1/threads/{id}/patch-undo",
get(preview_patch_undo_thread_turn).post(patch_undo_thread_turn),
)
.route("/v1/threads/{id}/file-revert", post(revert_thread_file))
.route("/v1/threads/{id}/retry", post(retry_thread_turn))
.route(
@@ -8703,6 +8707,10 @@ struct UndoTurnRequest {
/// The user turn to undo, with every turn after it. Wins over `depth`.
#[serde(default)]
turn_id: Option<String>,
#[serde(default)]
expected_turn_ids: Option<Vec<String>>,
#[serde(default)]
expected_paths: Option<Vec<String>>,
}
#[derive(Debug, Serialize)]
@@ -8814,12 +8822,92 @@ struct PatchUndoResponse {
original_user_images: Vec<codewhale_app_server_protocol::runtime::RuntimeImageInput>,
}
async fn prepare_patch_undo(
state: &RuntimeApiState,
id: &str,
req: &UndoTurnRequest,
) -> Result<codewhale_engine::runtime_threads::manager::PreparedThreadFork, ApiError> {
let prepared = match req.turn_id.as_deref() {
Some(turn_id) => {
state
.runtime_threads
.prepare_fork_dropping_user_turn_in_sessions_dir(id, turn_id, &state.sessions_dir)
.await
}
None => {
state
.runtime_threads
.prepare_fork_at_user_message_in_sessions_dir(
id,
req.depth.unwrap_or(0),
&state.sessions_dir,
)
.await
}
}
.map_err(map_thread_err)?;
if let Some(expected) = &req.expected_turn_ids {
let actual: Vec<_> = prepared
.dropped_turns()
.iter()
.map(|turn| &turn.turn_id)
.collect();
if actual != expected.iter().collect::<Vec<_>>() {
return Err(ApiError::conflict(
"The selected turn range changed; review it again before undoing files.",
)
.with_code("undo_selection_changed"));
}
}
Ok(prepared)
}
#[derive(Debug, Serialize)]
struct PatchUndoPreview {
turn_ids: Vec<String>,
paths: Vec<String>,
summary: Option<String>,
}
async fn preview_patch_undo_thread_turn(
State(state): State<RuntimeApiState>,
Path(id): Path<String>,
Query(req): Query<UndoTurnRequest>,
) -> Result<Json<PatchUndoPreview>, ApiError> {
let (_reservation, thread) = state
.runtime_threads
.thread_restore_guard(&id)
.await
.map_err(map_thread_err)?;
let prepared = prepare_patch_undo(&state, &id, &req).await?;
let dropped_turns = prepared.dropped_turns().to_vec();
let turn_ids = dropped_turns
.iter()
.map(|turn| turn.turn_id.clone())
.collect();
let trusted = thread.trust_mode || thread.auto_approve;
#[cfg(any(test, feature = "test-support"))]
let env_ticket = codewhale_engine::test_support::env_scope_ticket();
let (result, paths) = tokio::task::spawn_blocking(move || {
let _reservation = _reservation;
#[cfg(any(test, feature = "test-support"))]
let _membership = codewhale_engine::test_support::join_env_scope(env_ticket);
patch_undo_workspace_files_impl(&thread.workspace, &dropped_turns, trusted, true, None)
})
.await
.map_err(|error| ApiError::internal(format!("Patch undo preview failed: {error}")))??;
Ok(Json(PatchUndoPreview {
turn_ids,
paths,
summary: result.summary,
}))
}
async fn patch_undo_thread_turn(
State(state): State<RuntimeApiState>,
Path(id): Path<String>,
Json(req): Json<UndoTurnRequest>,
) -> Result<(StatusCode, Json<PatchUndoResponse>), ApiError> {
let depth = req.depth.unwrap_or(0);
// Admission first, then the thread record under it: trust, session
// binding and workspace are the values that hold while files change.
// Active turns in an overlapping workspace are rejected (409). The wait
@@ -8838,13 +8926,9 @@ async fn patch_undo_thread_turn(
let env_ticket = codewhale_engine::test_support::env_scope_ticket();
tokio::spawn(async move {
let reservation = reservation;
// Validate depth/history before touching any file, so an invalid
// Validate the selected turn/history before touching any file, so an invalid
// undo request cannot leave a half-applied workspace.
let prepared = state
.runtime_threads
.prepare_fork_at_user_message_in_sessions_dir(&id, depth, &state.sessions_dir)
.await
.map_err(map_thread_err)?;
let prepared = prepare_patch_undo(&state, &id, &req).await?;
// File rollback is a workspace mutation, so it needs the trust the
// TUI's `/undo` requires. Read from the thread's own record: the
// client does not get to assert it.
@@ -8860,7 +8944,7 @@ async fn patch_undo_thread_turn(
let patch_result = tokio::task::spawn_blocking(move || {
#[cfg(any(test, feature = "test-support"))]
let _membership = codewhale_engine::test_support::join_env_scope(env_ticket);
patch_undo_workspace_files(&workspace, &dropped_turns, trusted)
patch_undo_workspace_files_impl(&workspace, &dropped_turns, trusted, false, req.expected_paths.as_deref()).map(|(result, _)| result)
})
.await
.map_err(|e| ApiError::internal(format!("Patch undo task failed: {e}")))??;
@@ -9142,6 +9226,26 @@ pub fn patch_undo_workspace_files(
dropped_turns: &[codewhale_engine::runtime_threads::manager::DroppedTurnSnapshots],
trusted: bool,
) -> Result<PatchUndoResult, ApiError> {
patch_undo_workspace_files_impl(workspace, dropped_turns, trusted, false, None)
.map(|(result, _)| result)
}
fn patch_undo_workspace_files_impl(
workspace: &FsPath,
dropped_turns: &[codewhale_engine::runtime_threads::manager::DroppedTurnSnapshots],
trusted: bool,
preview: bool,
expected_paths: Option<&[String]>,
) -> Result<(PatchUndoResult, Vec<String>), ApiError> {
let check_paths = |paths: &[String]| {
if expected_paths.is_some_and(|expected| expected != paths) {
return Err(ApiError::conflict(
"The files to restore changed; review the selection again before undoing files.",
)
.with_code("undo_selection_changed"));
}
Ok(())
};
// An unreadable workspace directory (unmounted volume, disconnected
// share, permissions) proves nothing about the files a turn changed, so
// the conversation is not forked away from them.
@@ -9179,14 +9283,15 @@ pub fn patch_undo_workspace_files(
);
}
if windows.is_empty() {
return Ok(PatchUndoResult {
check_paths(&[])?;
return Ok((PatchUndoResult {
files_restored: false,
summary: Some(
"The undone turn(s) ran no tools, so they changed no workspace files; nothing to restore."
.to_string(),
),
snapshot_label: None,
});
}, Vec::new()));
}
// Every repository failure is operational and aborts: "nothing to
@@ -9366,18 +9471,23 @@ pub fn patch_undo_workspace_files(
}
let first = &segments[0];
if to_restore.is_empty() {
return Ok(PatchUndoResult {
files_restored: false,
summary: Some(if plan.is_empty() {
"The undone turn(s) left workspace files unchanged; nothing to restore.".to_string()
} else {
format!(
"The files the undone turn(s) changed are already at their state before turn {}; nothing to restore.",
first.turn_id
)
}),
snapshot_label: None,
});
check_paths(&[])?;
return Ok((
PatchUndoResult {
files_restored: false,
summary: Some(if plan.is_empty() {
"The undone turn(s) left workspace files unchanged; nothing to restore."
.to_string()
} else {
format!(
"The files the undone turn(s) changed are already at their state before turn {}; nothing to restore.",
first.turn_id
)
}),
snapshot_label: None,
},
Vec::new(),
));
}
// Restoring is a workspace mutation. Gate it exactly where the TUI gates
@@ -9391,6 +9501,22 @@ pub fn patch_undo_workspace_files(
.with_code(PATCH_UNDO_UNTRUSTED));
}
let paths: Vec<String> = to_restore
.iter()
.map(|(path, _, _)| path.display().to_string())
.collect();
check_paths(&paths)?;
if preview {
return Ok((
PatchUndoResult {
files_restored: false,
summary: None,
snapshot_label: Some(first.pre_label.clone()),
},
paths,
));
}
let restore_plan: Vec<(PathBuf, codewhale_snapshot::SnapshotId)> = to_restore
.iter()
.map(|(path, before, _)| (path.clone(), before.clone()))
@@ -9425,17 +9551,20 @@ pub fn patch_undo_workspace_files(
.iter()
.map(|outcome| format!("{} {}", outcome.action.as_str(), outcome.path.display()))
.collect();
Ok(PatchUndoResult {
files_restored: true,
summary: Some(format!(
"Restored {} file(s) to their state before turn {} (snapshot '{}'):\n{}",
outcomes.len(),
first.turn_id,
first.pre_label,
lines.join("\n")
)),
snapshot_label: Some(first.pre_label.clone()),
})
Ok((
PatchUndoResult {
files_restored: true,
summary: Some(format!(
"Restored {} file(s) to their state before turn {} (snapshot '{}'):\n{}",
outcomes.len(),
first.turn_id,
first.pre_label,
lines.join("\n")
)),
snapshot_label: Some(first.pre_label.clone()),
},
paths,
))
}
#[derive(Debug, Deserialize)]
crates/app-server/src/runtime_api/tests.rs
+52
View File
@@ -7917,6 +7917,36 @@ async fn patch_undo_endpoint_refuses_turns_it_cannot_prove() -> Result<()> {
create_seeded_thread(&addr, &runtime_threads, &root, "Roll back the patch").await?;
let client = codewhale_release::tls::reqwest_client();
let info: serde_json::Value = client
.get(format!("http://{addr}/v1/runtime/info"))
.send()
.await?
.error_for_status()?
.json()
.await?;
assert_eq!(info["capabilities"]["thread_patch_undo_turn_id"], true);
let resp = client
.get(format!(
"http://{addr}/v1/threads/{thread_id}/patch-undo?turn_id=foreign_turn"
))
.send()
.await?;
assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
let resp = client
.post(format!("http://{addr}/v1/threads/{thread_id}/patch-undo"))
.json(&json!({"turn_id": "foreign_turn", "depth": 0}))
.send()
.await?;
assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
let resp = client
.post(format!("http://{addr}/v1/threads/{thread_id}/patch-undo"))
.json(&json!({"expected_turn_ids": []}))
.send()
.await?;
assert_eq!(resp.status(), StatusCode::CONFLICT);
let refused: serde_json::Value = resp.json().await?;
assert_eq!(refused["error"]["code"], "undo_selection_changed");
// A workspace directory that is not available proves nothing about the
// files a turn changed: the undo is refused instead of forking the
// conversation away from them.
@@ -8055,6 +8085,28 @@ fn patch_undo_helper_restores_only_the_dropped_turns_changes() -> Result<()> {
true,
shell_turn_receipts(&repo, &pre, &post, "thr_a"),
);
let snapshot_count = repo.list(usize::MAX)?.len();
let (preview, paths) =
patch_undo_workspace_files_impl(&workspace, std::slice::from_ref(&turn), true, true, None)
.expect("a trusted preview should succeed");
assert!(!preview.files_restored);
assert_eq!(paths, vec!["a.txt", "made/new.txt"]);
let refused = patch_undo_workspace_files_impl(
&workspace,
std::slice::from_ref(&turn),
true,
false,
Some(&[]),
)
.expect_err("an unreviewed file plan must refuse before restoring");
assert_eq!(refused.status, StatusCode::CONFLICT);
assert_eq!(refused.code, Some("undo_selection_changed"));
assert_eq!(fs::read_to_string(&a)?, "second write");
assert!(workspace.join("made/new.txt").is_file());
assert_eq!(fs::read_to_string(&other)?, "later work");
assert_eq!(repo.list(usize::MAX)?.len(), snapshot_count);
let restored = patch_undo_workspace_files(&workspace, std::slice::from_ref(&turn), true)
.expect("a trusted rollback should succeed");
assert!(restored.files_restored, "{:?}", restored.summary);
crates/engine/src/runtime_threads/manager.rs
+15 -5
View File
@@ -9984,12 +9984,8 @@ impl RuntimeThreadManager {
let dir = sessions_dir.to_path_buf();
tokio::spawn(async move {
let _admission = admission;
let turns = runtime.store.list_turns_for_thread(&id)?;
let items_by_turn = runtime.prepared_items_for_turns(&turns)?;
let (_, _, depth_from_tail) =
runtime.fork_cut_for_user_turn(&turns, &items_by_turn, &turn_id)?;
let prepared = runtime
.prepare_fork_at_user_message_in_sessions_dir(&id, depth_from_tail, &dir)
.prepare_fork_dropping_user_turn_in_sessions_dir(&id, &turn_id, &dir)
.await?;
runtime.publish_prepared_fork(prepared).await
})
@@ -10126,6 +10122,20 @@ impl RuntimeThreadManager {
)
}
pub async fn prepare_fork_dropping_user_turn_in_sessions_dir(
&self,
id: &str,
turn_id: &str,
sessions_dir: &Path,
) -> Result<PreparedThreadFork> {
let turns = self.store.list_turns_for_thread(id)?;
let items_by_turn = self.prepared_items_for_turns(&turns)?;
let (_, _, depth_from_tail) =
self.fork_cut_for_user_turn(&turns, &items_by_turn, turn_id)?;
self.prepare_fork_at_user_message_in_sessions_dir(id, depth_from_tail, sessions_dir)
.await
}
pub async fn prepare_fork_at_user_turn_in_sessions_dir(
&self,
id: &str,
docs/RUNTIME_API.md
+5 -2
View File
@@ -1451,7 +1451,8 @@ latest turn and publishes a `thread.permissions_changed` event (`from`, `to`,
- `POST /v1/threads/{id}/compact` (manual compaction)
- `POST /v1/threads/{id}/undo` - fork the thread with the last N turns removed (`{"depth": N}`, default 0 = last turn only); returns the forked thread plus `original_user_text` so a GUI can pre-populate the input box. `{"turn_id": "turn_…"}` instead names the user turn to remove, together with every turn after it, and wins over `depth`; the Runtime resolves it under the thread's restore guard, so a turn another client appends meanwhile cannot move the cut. `400` when the turn is not a user turn of that thread
- `POST /v1/threads/{id}/fork-at-turn` - fork at one named user turn (`{"turn_id": "turn_…"}`, as `GET /v1/threads/{id}` reports it). The fork *keeps* that turn and every turn before it, and drops the turns after it; naming the last turn therefore keeps the whole conversation. The receipt is `/undo`'s (`thread`, `original_user_text`, `original_user_images`), carrying the *first dropped* user turn's prompt — what was asked next, even when a prompt-less turn such as a manual `/compact` sits between — so a client can put it back in the composer for editing. The source thread, its session document and the workspace are untouched, and there is no file rollback: a fork is a sibling conversation, and rewinding the workspace would rewind the branch left behind with it. Clients should name the turn instead of computing a `depth` — the transcript they render and the turn list this cuts are not the same list (steers, image-only prompts and injected handoffs each sit on one side only), and a client-side count that is off by one forks the wrong prefix while answering `201`. `400` when the turn is not a user turn of that thread.
- `POST /v1/threads/{id}/patch-undo` - rolls back the files the dropped turns changed, then the same fork (`{"depth": N}`); returns `patch_result` (`files_restored`, `summary`, `snapshot_label`) alongside the forked thread. See [Workspace restore endpoints](#workspace-restore-endpoints) for ownership, the trust, admission and abort rules, and the `error.code` values of a refusal.
- `GET /v1/threads/{id}/patch-undo?turn_id=turn_…` - validates the selected restore without changing files or publishing a fork; returns `turn_ids`, the exact dropped range, `paths` needing restoration and an optional no-change `summary`. Requires the `thread_patch_undo_turn_id` capability.
- `POST /v1/threads/{id}/patch-undo` - rolls back the files the dropped turns changed, then the same fork (`{"depth": N}` or `{"turn_id": "turn_…"}`, with `turn_id` taking precedence); returns `patch_result` (`files_restored`, `summary`, `snapshot_label`) alongside the forked thread. See [Workspace restore endpoints](#workspace-restore-endpoints) for ownership, the trust, admission and abort rules, and the `error.code` values of a refusal.
- `POST /v1/threads/{id}/file-revert` - restore exactly one file from one restore point the thread owns (`{"path", "snapshot_id", "expected_hash"}`); never forks the conversation. See [Workspace restore endpoints](#workspace-restore-endpoints).
- `POST /v1/threads/{id}/retry` - fork with the last N turns removed and immediately start a new turn (`{"depth": N, "prompt": "..."}`; `prompt` overrides the original user text, which is re-used when omitted)
@@ -1845,7 +1846,9 @@ For `file-revert` the backup is mandatory: if it cannot be written, or the
requested file is excluded from it (for example by `.gitignore`), the request
fails and nothing is changed.
**`patch-undo`.** Undoes whole turns. For each dropped turn's `pre_turn` →
**`patch-undo`.** Undoes whole turns. A named `turn_id` removes that user turn and every turn after it, under the same restore admission as `/undo`. Clients must require `thread_patch_undo_turn_id` before sending selected-turn requests: older Engines may ignore that field. Preview with `GET`, then send its exact `turn_ids` as `expected_turn_ids` and `paths` as `expected_paths` on `POST`; `409 undo_selection_changed` refuses an altered range or file plan before file changes. A preview does not reserve future execution or promise idempotency. The mutation is not automatically retryable after a lost response.
For each dropped turn's `pre_turn` →
`post_turn` window, the paths that differ between the two snapshots must all be
the turn's own: changed only in the span of one of the turn's tool calls (a
`tool` → `post_tool` span, or a shell turn's whole window), and, inside a file
docs/features.toml
+1 -1
View File
@@ -39,7 +39,7 @@
[[feature]]
id = "undo-range"
name = "Request range undo"
summary = "Select a request with /undo select to restore its file changes and later requests with visible progress."
summary = "Select a request with /undo select to restore its file changes and later requests with visible progress; Runtime clients can preview and confirm an exact named-turn range."
status = "experimental"
surfaces = { tui = "preview" }
since = "unreleased"
npm/runtime-sdk/generated/runtime.d.ts
Vendored
+1
View File
@@ -47,6 +47,7 @@ export type RuntimeCapabilities = {
threads: boolean
thread_create_idempotency: boolean
thread_idle_precondition: boolean
thread_patch_undo_turn_id: boolean
thread_shell_consent: boolean
turns: boolean
turn_operation_idempotency: boolean