mirror of
https://github.com/codewhale-hq/Codewhale.git
synced 2026-11-11 10:01:30 -10:00
feat(tui): add a file-scoped restore endpoint and gate the whole-tree rollback (#6111)
* feat(tui): restore per-file revert behind a file-scoped snapshot endpoint
`POST /v1/snapshots/{id}/restore` restores the whole workspace, so it cannot
back a per-file action: reverting one file would silently roll back every
other working-tree change. The engine had no narrower primitive — the only
restore path is `SnapshotRepo::restore()`, whose `git checkout <sha> -- :/`
covers every path in the snapshot tree, and it is what `patch_undo()`,
the `revert_turn` tool and the patch-undo endpoint all call.
Add that primitive and expose it:
- `SnapshotRepo::restore_paths()` restores a pathspec rather than the tree,
and `path_differs_from_snapshot()` answers "would restoring just this file
change anything?" without consulting the whole-tree diff.
- `workspace_relative_path()` is the single gate every caller-supplied path
passes through, so a request cannot name a path outside the work tree.
- `POST /v1/threads/{id}/file-revert` wires those to a thread, reusing the
ownership rule `/undo` already applies: only `tool:`/`pre-turn:` snapshots
tagged with the thread's own session are candidates.
"Nothing to revert" is a 409 rather than a silent success, and a scoped
restore takes the same `pre-restore:` safety snapshot as the whole-tree path.
The response reports the action it took (`modified` / `recreated` /
`removed`) so a client can update its change record without guessing.
Signed-off-by: Ben Gao <[email protected]>
* fix(tui): gate /undo's file rollback on thread trust and report what it changed
Three defects in the file-rollback step of `/undo`, which the patch-undo
endpoint shares with `patch_undo()`:
1. No trust gate. `POST /v1/threads/{id}/patch-undo` restores the whole
workspace and had no gate of its own, so a client could drive a
workspace-wide rollback from a thread in Ask or Auto-Review posture.
`patch_undo()` refuses that (`yolo || trust_mode`); the endpoint now
applies the same rule, read from the thread's own record so a caller
cannot assert it.
2. Half-undo on failure. The step reported failure inside `patch_result` and
the turn was forked anyway, dropping the turn while its file changes stayed
on disk — a workspace the transcript can no longer account for, which is
worse than refusing outright. The helper now returns `Result`: `Ok` means
the fork may proceed (files restored, or provably nothing to restore) and
`Err` aborts the whole undo.
The gate is evaluated *after* a real target is found, so "there was
nothing to revert" still undoes the conversation, while "there is
something to revert but you are not trusted" refuses.
3. A summary that described the wrong files. The "Files affected" list came
from `git diff --stat` run in the user's workspace against the user's
`.git`, which reports their own uncommitted work: it could list files the
restore never touched, and reported nothing when that work happened to be
committed. `SnapshotRepo::snapshot_diff_stat()` diffs the target snapshot
against the working tree inside the side repo instead, captured before the
restore runs (afterwards the two sides agree by construction).
Signed-off-by: Ben Gao <[email protected]>
* fix(tui): make file-scoped restore exact, trust-gated and race-free
Finish the file-scoped restore endpoint from #6111 on the confirmed review
findings. The request now names the exact `tool:`/`pre-turn:` snapshot from
the change the user selected plus the SHA-256 of the bytes they reviewed
(`absent` for a deleted file); the server never guesses the newest differing
snapshot, so an unrelated later snapshot cannot erase a user's edit while
keeping the tool's change. File names are literal Git pathspecs
(`--literal-pathspecs`), so `file[12].txt` no longer restores `file1.txt`.
Restores validate a regular file on both sides (no directories, symlinks in
any component, or `.git` paths), require a durable `pre-restore:` backup,
refuse files the backup excludes (for example gitignored ones), re-check the
reviewed hash immediately before mutation, and propagate removal errors
instead of reporting a false `removed`. Snapshot open, listing and
comparison failures abort patch-undo and preserve the conversation; an
unborn side repository is distinguished from broken history; a missing
workspace directory is the one provable "nothing to restore" case.
All three restore routes reserve the runtime's config/checkpoint admission
through an owned guard, reject active turns in the same, nested or parent
workspace with 409, and move the reservation into the worker so a client
disconnect cannot release it while Git runs. The thread record is read under
that admission, so trust and session binding are the values in force during
the mutation. File restore mirrors the thread trust/Full Access rule that
patch-undo applies. Fork preparation is split from publication so invalid
depth or history is rejected before any file changes; a fork persistence
failure after a restore names the restored snapshot and the safety backup.
Document the body, responses (200/400/404/409/422/500), capability probe,
exact snapshot identity, hash conflicts, trust, backup and file-versus-
whole-workspace semantics in docs/RUNTIME_API.md, including the direct
operator contract of `POST /v1/snapshots/{id}/restore`.
Tests: literal-pathspec sibling isolation, stale-hash and `absent` refusal,
deleted/created file outcomes, directory/`.git`/gitignored/symlink refusal,
unborn-versus-broken history, exact snapshot identity across unrelated and
foreign snapshots, overlapping active-turn admission on same/nested/parent
workspaces, and route-level body/trust/session gates. The Windows fixture
uses a real absolute temp path (it failed CI at repo.rs:1732).
Validation: focused codewhale-tui tests (snapshot repo, runtime API undo,
patch-undo and file-revert, restore guard, fork, revert_turn) 75 passed,
0 failed; `cargo check -p codewhale-tui --tests --locked`, CI-equivalent
Clippy for the crate and `cargo fmt --check` passed. Hosted CI on the PR
head and the full workspace gate on the integrated main are separate.
Refs #6111. Engine half of HengQuWorld/CodeWhale-VSCode#3; the GUI consumer
must supply `snapshot_id` and `expected_hash` from its change record.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Signed-off-by: Hunter Bown <[email protected]>
* fix(tui): keep restore admission on the request and refuse unavailable workspaces
Independent review of the previous commit found three gaps. Patch-undo
waited for admission inside the detached task, so a client that gave up
while queued left its undo behind the next one and the workspace walked
back twice; the admission wait now stays on the request and only the
mutation and fork publication outlive a disconnect. A fork whose records
were already durable reported "could not be saved" when only its event
append failed; the event failure is now logged and the saved fork is
returned. A workspace directory that is not readable (unmounted volume,
disconnected share) was treated as "nothing to restore" and the
conversation was forked away from files that still exist; both patch-undo
and file-revert now refuse with 409.
Also: the server-wide snapshot restore validates its id (400), the
active-turn scan does store and filesystem I/O outside the `active` lock,
a file-scoped removal no longer prunes the emptied parent directory, and
Git or removal failures after the safety snapshot name that snapshot.
Documentation covers the runtime-wide admission stall, the unavailable
workspace 409 and the recovery hint.
Tests: route-level 409 for file-revert and patch-undo under an active turn
plus admission again after it settles, 400 for a malformed server-wide
snapshot id, 409 then 201 for an unavailable then present workspace, and
parent-directory retention on removal.
Refs #6111.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Signed-off-by: Hunter Bown <[email protected]>
---------
Signed-off-by: Ben Gao <[email protected]>
Signed-off-by: Hunter Bown <[email protected]>
Co-authored-by: Hunter B <[email protected]>
Co-authored-by: Claude Fable 5.1 <[email protected]>
This commit is contained in:
8 files changed
+2036
-116
No files matched your search
@@ -203,10 +203,19 @@ pub fn patch_undo(app: &mut App) -> CommandResult {
|
||||
// Pick the newest current-session candidate whose tree differs from the
|
||||
// workspace. Skipping identical snapshots makes repeated `/undo` walk
|
||||
// backward only inside the proven session boundary.
|
||||
let differs = |s: &&crate::snapshot::Snapshot| {
|
||||
matches!(repo.work_tree_matches_snapshot(&s.id), Ok(false))
|
||||
};
|
||||
let target = candidates.iter().find(differs);
|
||||
let mut target = None;
|
||||
for snapshot in &candidates {
|
||||
match repo.work_tree_matches_snapshot(&snapshot.id) {
|
||||
Ok(false) => {
|
||||
target = Some(snapshot);
|
||||
break;
|
||||
}
|
||||
Ok(true) => {}
|
||||
Err(error) => {
|
||||
return CommandResult::error(format!("Failed to compare snapshot: {error}"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let Some(target) = target else {
|
||||
return CommandResult::message(
|
||||
@@ -224,6 +233,19 @@ pub fn patch_undo(app: &mut App) -> CommandResult {
|
||||
);
|
||||
}
|
||||
|
||||
// Capture what this restore is about to change *before* it runs: after the
|
||||
// checkout the work tree matches the snapshot and the diff is empty by
|
||||
// construction. Computed in the side repo, not the user's — the user's
|
||||
// `git diff --stat` reports their own uncommitted work, which is not what
|
||||
// the undo changed.
|
||||
let diff_stat = match repo.snapshot_diff_stat(&target.id) {
|
||||
Ok(stat) => stat,
|
||||
Err(e) => {
|
||||
tracing::warn!(target: "snapshot", "diff stat for the undo summary failed: {e}");
|
||||
None
|
||||
}
|
||||
};
|
||||
|
||||
if let Err(e) = repo.restore(&target.id) {
|
||||
return CommandResult::error(format!("Restore failed: {e}"));
|
||||
}
|
||||
@@ -234,20 +256,6 @@ pub fn patch_undo(app: &mut App) -> CommandResult {
|
||||
prune_undone_turn_context(app);
|
||||
}
|
||||
|
||||
// Show diff stat so the user knows what changed.
|
||||
let diff_stat = Git::command()
|
||||
.map(|mut git| {
|
||||
git.args(["diff", "--stat"])
|
||||
.current_dir(&workspace)
|
||||
.output()
|
||||
.ok()
|
||||
.and_then(|o| {
|
||||
let s = String::from_utf8_lossy(&o.stdout).trim().to_string();
|
||||
if s.is_empty() { None } else { Some(s) }
|
||||
})
|
||||
})
|
||||
.unwrap_or(None);
|
||||
|
||||
let short = &target.id.as_str()[..target.id.as_str().len().min(8)];
|
||||
let summary = match diff_stat {
|
||||
Some(ref stat) => {
|
||||
|
||||
+338
-69
@@ -1146,6 +1146,7 @@ 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}/patch-undo", 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(
|
||||
"/v1/threads/{id}/turn-operations/{operation_key}",
|
||||
@@ -4488,104 +4489,179 @@ async fn patch_undo_thread_turn(
|
||||
Json(req): Json<UndoTurnRequest>,
|
||||
) -> Result<(StatusCode, Json<PatchUndoResponse>), ApiError> {
|
||||
let depth = req.depth.unwrap_or(0);
|
||||
|
||||
// Step 1: Try snapshot-based file rollback (patch_undo).
|
||||
let thread = state
|
||||
// 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
|
||||
// for admission stays on the request so a client that gives up while
|
||||
// queued cancels its undo instead of leaving it queued behind the next
|
||||
// one and walking the workspace back twice.
|
||||
let (reservation, thread) = state
|
||||
.runtime_threads
|
||||
.get_thread(&id)
|
||||
.thread_restore_guard(&id)
|
||||
.await
|
||||
.map_err(map_thread_err)?;
|
||||
let patch_result = patch_undo_workspace_files(&thread.workspace, thread.session_id.as_deref());
|
||||
|
||||
// Step 2: Remove the last conversation turn (undo_conversation).
|
||||
let (forked_thread, original_user_text, original_user_images, _) = state
|
||||
.runtime_threads
|
||||
.fork_at_user_message(&id, depth)
|
||||
// Once admitted, own the operation even when the HTTP caller disconnects:
|
||||
// the reservation must outlive both the file mutation and the fork
|
||||
// publication, so a dropped connection cannot release it mid-Git.
|
||||
tokio::spawn(async move {
|
||||
let reservation = reservation;
|
||||
// Validate depth/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(&id, depth)
|
||||
.await
|
||||
.map_err(map_thread_err)?;
|
||||
// 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.
|
||||
let trusted = thread.trust_mode || thread.auto_approve;
|
||||
let workspace = thread.workspace.clone();
|
||||
let session_id = thread.session_id.clone();
|
||||
// Step 1: snapshot-based file rollback. The `?` is deliberate: a
|
||||
// refusal or a failed restore aborts *before* the conversation is
|
||||
// forked, so the turn never disappears while its file changes stay.
|
||||
let patch_result = tokio::task::spawn_blocking(move || {
|
||||
patch_undo_workspace_files(&workspace, session_id.as_deref(), trusted)
|
||||
})
|
||||
.await
|
||||
.map_err(map_thread_err)?;
|
||||
|
||||
Ok((
|
||||
StatusCode::CREATED,
|
||||
Json(PatchUndoResponse {
|
||||
patch_result,
|
||||
thread: forked_thread,
|
||||
original_user_text,
|
||||
original_user_images,
|
||||
}),
|
||||
))
|
||||
.map_err(|e| ApiError::internal(format!("Patch undo task failed: {e}")))??;
|
||||
// Step 2: publish the already-validated fork.
|
||||
let (forked_thread, original_user_text, original_user_images, _) = state
|
||||
.runtime_threads
|
||||
.publish_prepared_fork(prepared)
|
||||
.await
|
||||
.map_err(|error| {
|
||||
if patch_result.files_restored {
|
||||
ApiError::internal(format!(
|
||||
"Workspace files were restored from snapshot {}, but the conversation fork could not be saved: {error}. The original thread still holds the undone turn; the `pre-restore:` safety snapshot holds the files as they were before this undo.",
|
||||
patch_result
|
||||
.snapshot_label
|
||||
.as_deref()
|
||||
.unwrap_or("(unknown)")
|
||||
))
|
||||
} else {
|
||||
map_thread_err(error)
|
||||
}
|
||||
})?;
|
||||
drop(reservation);
|
||||
Ok((
|
||||
StatusCode::CREATED,
|
||||
Json(PatchUndoResponse {
|
||||
patch_result,
|
||||
thread: forked_thread,
|
||||
original_user_text,
|
||||
original_user_images,
|
||||
}),
|
||||
))
|
||||
})
|
||||
.await
|
||||
.map_err(|e| ApiError::internal(format!("Patch undo task failed: {e}")))?
|
||||
}
|
||||
|
||||
/// Restore the newest `tool:` or `pre-turn:` snapshot that differs from the
|
||||
/// current workspace — same target selection as the TUI's `patch_undo`.
|
||||
///
|
||||
/// # The rollback contract
|
||||
///
|
||||
/// `Ok` is a decision the conversation fork may proceed on: either the files
|
||||
/// were restored, or there was *provably* nothing to restore. `Err` aborts the
|
||||
/// whole undo, and the caller must not fork either — dropping the turn while
|
||||
/// leaving its file changes on disk hands the user a workspace the transcript
|
||||
/// can no longer account for, which is worse than refusing outright.
|
||||
///
|
||||
/// `trusted` mirrors the gate the TUI's `patch_undo()` applies
|
||||
/// (`yolo || trust_mode`). It is evaluated *after* a real target is found, so
|
||||
/// that "there was nothing to revert" still undoes the conversation, while
|
||||
/// "there is something to revert but you are not trusted" aborts.
|
||||
fn patch_undo_workspace_files(
|
||||
workspace: &FsPath,
|
||||
current_session_id: Option<&str>,
|
||||
) -> PatchUndoResult {
|
||||
let repo = match crate::snapshot::SnapshotRepo::open_or_init(workspace) {
|
||||
Ok(repo) => repo,
|
||||
Err(e) => {
|
||||
return PatchUndoResult {
|
||||
files_restored: false,
|
||||
summary: Some(format!("Snapshot repo unavailable: {e}")),
|
||||
snapshot_label: None,
|
||||
};
|
||||
}
|
||||
};
|
||||
trusted: bool,
|
||||
) -> Result<PatchUndoResult, ApiError> {
|
||||
// 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. Every repository
|
||||
// failure is operational and aborts for the same reason: "no snapshots"
|
||||
// cannot be proven while Git is unavailable.
|
||||
if !workspace.is_dir() {
|
||||
return Err(ApiError::conflict(format!(
|
||||
"Workspace directory {} is not available; mount or restore it before undoing files, or use /undo for a conversation-only undo.",
|
||||
workspace.display()
|
||||
)));
|
||||
}
|
||||
let repo = crate::snapshot::SnapshotRepo::open_or_init(workspace).map_err(|e| {
|
||||
ApiError::internal(format!(
|
||||
"Snapshot repo unavailable; conversation preserved: {e}"
|
||||
))
|
||||
})?;
|
||||
let Some(current_session_id) = current_session_id else {
|
||||
return PatchUndoResult {
|
||||
return Ok(PatchUndoResult {
|
||||
files_restored: false,
|
||||
summary: Some(
|
||||
"No current session is bound to this thread; workspace files were not changed."
|
||||
.to_string(),
|
||||
),
|
||||
snapshot_label: None,
|
||||
};
|
||||
});
|
||||
};
|
||||
let snapshots = match repo.list(100) {
|
||||
Ok(snapshots) => snapshots,
|
||||
Err(e) => {
|
||||
return PatchUndoResult {
|
||||
files_restored: false,
|
||||
summary: Some(format!("Failed to list snapshots: {e}")),
|
||||
snapshot_label: None,
|
||||
};
|
||||
}
|
||||
};
|
||||
let target = snapshots
|
||||
let snapshots = repo
|
||||
.list(100)
|
||||
.map_err(|e| ApiError::internal(format!("Failed to list snapshots: {e}")))?;
|
||||
let mut target = None;
|
||||
for snapshot in snapshots
|
||||
.iter()
|
||||
.filter(|s| s.label.starts_with("tool:") || s.label.starts_with("pre-turn:"))
|
||||
.filter(|s| s.session_id.as_deref() == Some(current_session_id))
|
||||
.find(|s| matches!(repo.work_tree_matches_snapshot(&s.id), Ok(false)));
|
||||
{
|
||||
if !repo.work_tree_matches_snapshot(&snapshot.id).map_err(|e| {
|
||||
ApiError::internal(format!(
|
||||
"Failed to compare snapshot; conversation preserved: {e}"
|
||||
))
|
||||
})? {
|
||||
target = Some(snapshot);
|
||||
break;
|
||||
}
|
||||
}
|
||||
let Some(target) = target else {
|
||||
return PatchUndoResult {
|
||||
return Ok(PatchUndoResult {
|
||||
files_restored: false,
|
||||
summary: Some(
|
||||
"No current-session tool or pre-turn snapshots differ from the current workspace."
|
||||
.to_string(),
|
||||
),
|
||||
snapshot_label: None,
|
||||
};
|
||||
});
|
||||
};
|
||||
if let Err(e) = repo.restore(&target.id) {
|
||||
return PatchUndoResult {
|
||||
files_restored: false,
|
||||
summary: Some(format!("Restore failed: {e}")),
|
||||
snapshot_label: None,
|
||||
};
|
||||
|
||||
// Restoring is a workspace mutation. Gate it exactly where the TUI gates
|
||||
// it — after a real, current-session target is known — so the two surfaces
|
||||
// cannot drift into "one refuses, the other half-undoes".
|
||||
if !trusted {
|
||||
return Err(ApiError::conflict(
|
||||
"Refusing to undo workspace files outside trusted mode. \
|
||||
Turn on /trust or switch this thread to Full Access, then undo again.",
|
||||
));
|
||||
}
|
||||
|
||||
// Compute a diff stat for the summary.
|
||||
use crate::dependencies::{ExternalTool as _, Git};
|
||||
let diff_stat = Git::command().and_then(|mut git| {
|
||||
git.args(["diff", "--stat"])
|
||||
.current_dir(workspace)
|
||||
.output()
|
||||
.ok()
|
||||
.and_then(|o| {
|
||||
let s = String::from_utf8_lossy(&o.stdout).trim().to_string();
|
||||
if s.is_empty() { None } else { Some(s) }
|
||||
})
|
||||
});
|
||||
// Capture what this restore is about to change *before* it runs: after the
|
||||
// checkout the work tree matches the snapshot, so a post-restore stat would
|
||||
// always be empty. Runs against the side repo, not the user's — the user's
|
||||
// `git diff --stat` reports their own uncommitted work, which is not what
|
||||
// the undo changed.
|
||||
let diff_stat = match repo.snapshot_diff_stat(&target.id) {
|
||||
Ok(stat) => stat,
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
target: "snapshot",
|
||||
"diff stat for the patch-undo summary failed: {e}"
|
||||
);
|
||||
None
|
||||
}
|
||||
};
|
||||
|
||||
repo.restore(&target.id)
|
||||
.map_err(|e| ApiError::internal(format!("Restore failed: {e}")))?;
|
||||
|
||||
let short = &target.id.as_str()[..target.id.as_str().len().min(8)];
|
||||
let summary = match diff_stat {
|
||||
@@ -4594,14 +4670,191 @@ fn patch_undo_workspace_files(
|
||||
target.label, short
|
||||
),
|
||||
None => format!(
|
||||
"Restored snapshot '{}' ({}). No diff changes detected.",
|
||||
"Restored snapshot '{}' ({}). No diff stat available.",
|
||||
target.label, short
|
||||
),
|
||||
};
|
||||
PatchUndoResult {
|
||||
Ok(PatchUndoResult {
|
||||
files_restored: true,
|
||||
summary: Some(summary),
|
||||
snapshot_label: Some(target.label.clone()),
|
||||
})
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct RevertThreadFileRequest {
|
||||
/// The single file to restore, relative to the thread's workspace.
|
||||
/// Absolute paths inside the workspace are accepted and normalized.
|
||||
path: String,
|
||||
/// Exact pre-tool/pre-turn snapshot from the change the user selected.
|
||||
snapshot_id: String,
|
||||
/// SHA-256 of the bytes reviewed by the client, or `absent` for deletion.
|
||||
expected_hash: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize)]
|
||||
struct RevertThreadFileResponse {
|
||||
/// Workspace-relative path that was restored.
|
||||
path: String,
|
||||
/// What the restore did to the working tree: `modified`, `recreated`, or
|
||||
/// `removed`.
|
||||
action: String,
|
||||
/// Snapshot the file came from.
|
||||
snapshot_id: String,
|
||||
snapshot_label: String,
|
||||
}
|
||||
|
||||
/// Restore one deliberately selected file revision.
|
||||
///
|
||||
/// The file-scoped counterpart of `patch-undo`. Where `patch-undo` checks out
|
||||
/// a whole snapshot tree, this restores exactly one regular file, so unrelated
|
||||
/// working-tree changes are never rolled back. The client names the exact
|
||||
/// `tool:`/`pre-turn:` snapshot from the change record it displayed and the
|
||||
/// hash of the bytes it reviewed; the server never guesses a "newest differing"
|
||||
/// snapshot, because an unrelated newer snapshot can erase later user edits.
|
||||
///
|
||||
/// Ownership follows the rule the TUI's `/undo` applies: only snapshots tagged
|
||||
/// with this thread's own session are candidates, and the thread must be in
|
||||
/// trusted mode or Full Access. Nothing to revert is a `409`, not a silent
|
||||
/// success, so the GUI can tell the user why the button did nothing.
|
||||
async fn revert_thread_file(
|
||||
State(state): State<RuntimeApiState>,
|
||||
Path(id): Path<String>,
|
||||
Json(req): Json<RevertThreadFileRequest>,
|
||||
) -> Result<Json<RevertThreadFileResponse>, ApiError> {
|
||||
if !snapshot_id_is_well_formed(&req.snapshot_id) {
|
||||
return Err(ApiError::bad_request(
|
||||
"snapshot_id must be the exact hexadecimal id reported by GET /v1/snapshots",
|
||||
));
|
||||
}
|
||||
if !expected_hash_is_well_formed(&req.expected_hash) {
|
||||
return Err(ApiError::bad_request(
|
||||
"expected_hash must be `sha256:<64 lowercase hex digits>` of the reviewed file bytes, or `absent` for a file the client saw as deleted",
|
||||
));
|
||||
}
|
||||
// Admission first, then the thread record under it. Active turns in an
|
||||
// overlapping workspace are rejected instead of raced.
|
||||
let (reservation, thread) = state
|
||||
.runtime_threads
|
||||
.thread_restore_guard(&id)
|
||||
.await
|
||||
.map_err(map_thread_err)?;
|
||||
if !(thread.trust_mode || thread.auto_approve) {
|
||||
return Err(ApiError::conflict(
|
||||
"Refusing to restore workspace files outside trusted mode. Turn on /trust or switch this thread to Full Access, then retry.",
|
||||
));
|
||||
}
|
||||
let Some(session_id) = thread.session_id else {
|
||||
return Err(ApiError::conflict(
|
||||
"Thread has no bound session, so no snapshot can be proven to own this file.",
|
||||
));
|
||||
};
|
||||
let workspace = thread.workspace;
|
||||
// The worker owns the reservation: a client disconnect cannot release it
|
||||
// while Git is still changing files. Snapshot listing, diffing and
|
||||
// checkout all shell out to git; keep that off the async workers.
|
||||
let response = tokio::task::spawn_blocking(move || {
|
||||
let _reservation = reservation;
|
||||
revert_file_from_snapshot(&workspace, &session_id, &req)
|
||||
})
|
||||
.await
|
||||
.map_err(|e| ApiError::internal(format!("file restore task failed: {e}")))??;
|
||||
Ok(Json(response))
|
||||
}
|
||||
|
||||
fn snapshot_id_is_well_formed(id: &str) -> bool {
|
||||
matches!(id.len(), 40 | 64) && id.bytes().all(|b| b.is_ascii_hexdigit())
|
||||
}
|
||||
|
||||
fn expected_hash_is_well_formed(hash: &str) -> bool {
|
||||
hash == "absent"
|
||||
|| hash.strip_prefix("sha256:").is_some_and(|digest| {
|
||||
digest.len() == 64
|
||||
&& digest
|
||||
.bytes()
|
||||
.all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b))
|
||||
})
|
||||
}
|
||||
|
||||
fn revert_file_from_snapshot(
|
||||
workspace: &FsPath,
|
||||
session_id: &str,
|
||||
req: &RevertThreadFileRequest,
|
||||
) -> Result<RevertThreadFileResponse, ApiError> {
|
||||
// Every caller-supplied path passes through this one gate. It accepts a
|
||||
// workspace-relative path or an absolute path inside the workspace and
|
||||
// rejects everything else (`..`, empty, or outside the work tree). The
|
||||
// name is used literally: brackets, spaces and glob characters are part
|
||||
// of the filename, never a pattern.
|
||||
let rel = crate::snapshot::workspace_relative_path(workspace, &req.path).ok_or_else(|| {
|
||||
ApiError::bad_request(format!(
|
||||
"path must name a regular file inside the thread workspace {}; got '{}'",
|
||||
workspace.display(),
|
||||
req.path
|
||||
))
|
||||
})?;
|
||||
if !workspace.is_dir() {
|
||||
return Err(ApiError::conflict(format!(
|
||||
"Workspace directory {} is not available; mount or restore it before restoring files.",
|
||||
workspace.display()
|
||||
)));
|
||||
}
|
||||
let repo = crate::snapshot::SnapshotRepo::open_or_init(workspace)
|
||||
.map_err(|e| ApiError::internal(format!("Snapshot repo unavailable: {e}")))?;
|
||||
repo.validate_restore_file(&rel)
|
||||
.map_err(map_file_restore_err)?;
|
||||
let snapshots = repo
|
||||
.list(usize::MAX)
|
||||
.map_err(|e| ApiError::internal(format!("Failed to list snapshots: {e}")))?;
|
||||
// Exact identity only: the snapshot must exist, be owned by this thread's
|
||||
// session and be a tool/pre-turn restore point. A stale or foreign id is
|
||||
// a conflict the client resolves by refreshing its change record.
|
||||
let target = snapshots
|
||||
.iter()
|
||||
.find(|snapshot| {
|
||||
snapshot.id.as_str() == req.snapshot_id
|
||||
&& snapshot.session_id.as_deref() == Some(session_id)
|
||||
&& (snapshot.label.starts_with("tool:")
|
||||
|| snapshot.label.starts_with("pre-turn:"))
|
||||
})
|
||||
.ok_or_else(|| {
|
||||
ApiError::conflict(
|
||||
"Selected restore point is unavailable or belongs to another session; refresh the change record and select the change again.",
|
||||
)
|
||||
})?;
|
||||
|
||||
if !repo
|
||||
.path_differs_from_snapshot(&target.id, &rel)
|
||||
.map_err(map_file_restore_err)?
|
||||
{
|
||||
return Err(ApiError::conflict(format!(
|
||||
"'{}' already matches snapshot '{}'; nothing to revert.",
|
||||
rel.display(),
|
||||
target.label
|
||||
)));
|
||||
}
|
||||
let outcomes = repo
|
||||
.restore_file_if_unchanged(&target.id, &rel, &req.expected_hash)
|
||||
.map_err(map_file_restore_err)?;
|
||||
let outcome = outcomes
|
||||
.into_iter()
|
||||
.next()
|
||||
.ok_or_else(|| ApiError::conflict("Nothing was restored."))?;
|
||||
Ok(RevertThreadFileResponse {
|
||||
path: outcome.path.to_string_lossy().into_owned(),
|
||||
action: outcome.action.as_str().to_string(),
|
||||
snapshot_id: target.id.as_str().to_string(),
|
||||
snapshot_label: target.label.clone(),
|
||||
})
|
||||
}
|
||||
|
||||
fn map_file_restore_err(error: std::io::Error) -> ApiError {
|
||||
if error.kind() == std::io::ErrorKind::InvalidInput {
|
||||
ApiError::bad_request(error.to_string())
|
||||
} else if error.kind() == std::io::ErrorKind::WouldBlock {
|
||||
ApiError::conflict(error.to_string())
|
||||
} else {
|
||||
ApiError::internal(format!("File restore failed: {error}"))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5987,7 +6240,23 @@ async fn restore_snapshot(
|
||||
State(state): State<RuntimeApiState>,
|
||||
Path(id): Path<String>,
|
||||
) -> Result<Json<Value>, ApiError> {
|
||||
restore_snapshot_for_workspace(&state.workspace, &id)?;
|
||||
if !snapshot_id_is_well_formed(&id) {
|
||||
return Err(ApiError::bad_request(
|
||||
"snapshot id must be the exact hexadecimal id reported by GET /v1/snapshots",
|
||||
));
|
||||
}
|
||||
let reservation = state
|
||||
.runtime_threads
|
||||
.workspace_restore_guard(&state.workspace)
|
||||
.await
|
||||
.map_err(map_thread_err)?;
|
||||
let restored_id = id.clone();
|
||||
tokio::task::spawn_blocking(move || {
|
||||
let _reservation = reservation;
|
||||
restore_snapshot_for_workspace(&state.workspace, &restored_id)
|
||||
})
|
||||
.await
|
||||
.map_err(|e| ApiError::internal(format!("Restore task failed: {e}")))??;
|
||||
Ok(Json(json!({
|
||||
"restored": id,
|
||||
})))
|
||||
|
||||
@@ -5215,6 +5215,18 @@ async fn patch_undo_endpoint_forks_and_reports_file_rollback_state() -> Result<(
|
||||
create_seeded_thread(&addr, &runtime_threads, &root, "Roll back the patch").await?;
|
||||
let client = crate::tls::reqwest_client();
|
||||
|
||||
// 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.
|
||||
let resp = client
|
||||
.post(format!("http://{addr}/v1/threads/{thread_id}/patch-undo"))
|
||||
.json(&json!({}))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), StatusCode::CONFLICT);
|
||||
assert!(resp.text().await?.contains("not available"));
|
||||
|
||||
fs::create_dir_all(root.join("workspace"))?;
|
||||
let resp = client
|
||||
.post(format!("http://{addr}/v1/threads/{thread_id}/patch-undo"))
|
||||
.json(&json!({}))
|
||||
@@ -5223,8 +5235,8 @@ async fn patch_undo_endpoint_forks_and_reports_file_rollback_state() -> Result<(
|
||||
assert_eq!(resp.status(), StatusCode::CREATED);
|
||||
let undone: serde_json::Value = resp.json().await?;
|
||||
// The fresh workspace has no tool/pre-turn snapshots to roll back to,
|
||||
// so the file-restore step reports failure while the conversation
|
||||
// undo still forks the thread.
|
||||
// so the file-restore step reports nothing restored while the
|
||||
// conversation undo still forks the thread.
|
||||
assert_eq!(undone["patch_result"]["files_restored"], false);
|
||||
assert!(undone["patch_result"]["summary"].is_string());
|
||||
assert_eq!(undone["original_user_text"], "Roll back the patch");
|
||||
@@ -5255,17 +5267,527 @@ fn patch_undo_helper_restores_only_the_bound_session() -> Result<()> {
|
||||
repo.snapshot_with_session("pre-turn:foreign", Some("session-foreign"))?;
|
||||
fs::write(&file, "current-after")?;
|
||||
|
||||
let restored = patch_undo_workspace_files(&workspace, Some("session-current"));
|
||||
let restored = patch_undo_workspace_files(&workspace, Some("session-current"), true)
|
||||
.expect("a trusted rollback should succeed");
|
||||
assert!(restored.files_restored, "{:?}", restored.summary);
|
||||
assert_eq!(fs::read_to_string(&file)?, "current-before");
|
||||
|
||||
fs::write(&file, "must-stay")?;
|
||||
let unbound = patch_undo_workspace_files(&workspace, None);
|
||||
let unbound = patch_undo_workspace_files(&workspace, None, true)
|
||||
.expect("an unbound thread has nothing to roll back, which is not a refusal");
|
||||
assert!(!unbound.files_restored);
|
||||
assert_eq!(fs::read_to_string(&file)?, "must-stay");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The gate the TUI's `/undo` applies, stated as a contract: an untrusted
|
||||
/// thread may not roll files back. When there *is* something to roll back the
|
||||
/// whole undo aborts — nothing is changed, and the caller must not fork the
|
||||
/// conversation either.
|
||||
#[test]
|
||||
fn patch_undo_helper_refuses_an_untrusted_rollback_and_touches_nothing() -> Result<()> {
|
||||
let _lock = lock_test_env();
|
||||
let root = tempfile::tempdir()?;
|
||||
let home = root.path().join("home");
|
||||
fs::create_dir_all(&home)?;
|
||||
let _home = EnvVarGuard::set("HOME", &home);
|
||||
|
||||
let workspace = root.path().join("workspace");
|
||||
fs::create_dir_all(&workspace)?;
|
||||
let repo = crate::snapshot::SnapshotRepo::open_or_init(&workspace)?;
|
||||
let file = workspace.join("a.txt");
|
||||
|
||||
fs::write(&file, "before")?;
|
||||
repo.snapshot_with_session("pre-turn:1", Some("session-a"))?;
|
||||
fs::write(&file, "after")?;
|
||||
|
||||
let err = patch_undo_workspace_files(&workspace, Some("session-a"), false)
|
||||
.expect_err("an untrusted rollback must be refused");
|
||||
assert_eq!(err.status, StatusCode::CONFLICT);
|
||||
assert!(
|
||||
err.message.contains("outside trusted mode"),
|
||||
"got: {}",
|
||||
err.message
|
||||
);
|
||||
assert_eq!(
|
||||
fs::read_to_string(&file)?,
|
||||
"after",
|
||||
"a refusal must leave the workspace exactly as it was"
|
||||
);
|
||||
|
||||
// The identical call once trusted performs the rollback.
|
||||
let restored = patch_undo_workspace_files(&workspace, Some("session-a"), true)
|
||||
.expect("a trusted rollback should succeed");
|
||||
assert!(restored.files_restored, "{:?}", restored.summary);
|
||||
assert_eq!(fs::read_to_string(&file)?, "before");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// ...but with provably nothing to roll back, an untrusted undo is still
|
||||
/// allowed: the conversation half needs no trust, and skipping it would make
|
||||
/// Undo useless in Ask / Auto-Review for no safety gain.
|
||||
#[test]
|
||||
fn patch_undo_helper_allows_an_untrusted_undo_with_nothing_to_roll_back() -> Result<()> {
|
||||
let _lock = lock_test_env();
|
||||
let root = tempfile::tempdir()?;
|
||||
let home = root.path().join("home");
|
||||
fs::create_dir_all(&home)?;
|
||||
let _home = EnvVarGuard::set("HOME", &home);
|
||||
|
||||
let workspace = root.path().join("workspace");
|
||||
fs::create_dir_all(&workspace)?;
|
||||
let repo = crate::snapshot::SnapshotRepo::open_or_init(&workspace)?;
|
||||
let file = workspace.join("a.txt");
|
||||
|
||||
fs::write(&file, "only-state")?;
|
||||
repo.snapshot_with_session("pre-turn:1", Some("session-a"))?;
|
||||
// Nothing changed after the snapshot, so no target exists.
|
||||
|
||||
let result = patch_undo_workspace_files(&workspace, Some("session-a"), false)
|
||||
.expect("nothing to roll back is not a refusal");
|
||||
assert!(!result.files_restored);
|
||||
assert!(result.summary.is_some());
|
||||
assert_eq!(fs::read_to_string(&file)?, "only-state");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn sha256_of(path: &FsPath) -> String {
|
||||
format!(
|
||||
"sha256:{}",
|
||||
crate::hashing::sha256_hex(fs::read(path).unwrap())
|
||||
)
|
||||
}
|
||||
|
||||
fn revert_request(path: &str, snapshot_id: &str, expected_hash: &str) -> RevertThreadFileRequest {
|
||||
RevertThreadFileRequest {
|
||||
path: path.to_string(),
|
||||
snapshot_id: snapshot_id.to_string(),
|
||||
expected_hash: expected_hash.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
/// The acceptance criterion for per-file revert: restoring one file must not
|
||||
/// roll back any other file's turn changes.
|
||||
#[test]
|
||||
fn revert_file_helper_restores_only_the_named_file() -> Result<()> {
|
||||
let _lock = lock_test_env();
|
||||
let root = tempfile::tempdir()?;
|
||||
let home = root.path().join("home");
|
||||
fs::create_dir_all(&home)?;
|
||||
let _home = EnvVarGuard::set("HOME", &home);
|
||||
|
||||
let workspace = root.path().join("workspace");
|
||||
fs::create_dir_all(&workspace)?;
|
||||
let repo = crate::snapshot::SnapshotRepo::open_or_init(&workspace)?;
|
||||
let wanted = workspace.join("wanted.txt");
|
||||
let other = workspace.join("other.txt");
|
||||
|
||||
fs::write(&wanted, "wanted-before")?;
|
||||
fs::write(&other, "other-before")?;
|
||||
repo.snapshot_with_session("pre-turn:1", Some("session-a"))?;
|
||||
|
||||
fs::write(&wanted, "wanted-after")?;
|
||||
fs::write(&other, "other-after")?;
|
||||
|
||||
let snapshot = repo.list(10)?.remove(0);
|
||||
let reverted = revert_file_from_snapshot(
|
||||
&workspace,
|
||||
"session-a",
|
||||
&revert_request("wanted.txt", snapshot.id.as_str(), &sha256_of(&wanted)),
|
||||
)
|
||||
.expect("scoped revert should succeed");
|
||||
assert_eq!(reverted.path, "wanted.txt");
|
||||
assert_eq!(reverted.action, "modified");
|
||||
assert_eq!(reverted.snapshot_label, "pre-turn:1");
|
||||
assert_eq!(reverted.snapshot_id, snapshot.id.as_str());
|
||||
assert_eq!(fs::read_to_string(&wanted)?, "wanted-before");
|
||||
assert_eq!(
|
||||
fs::read_to_string(&other)?,
|
||||
"other-after",
|
||||
"reverting one file must leave every other file's changes in place"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn revert_file_helper_accepts_an_absolute_path_inside_the_workspace() -> Result<()> {
|
||||
let _lock = lock_test_env();
|
||||
let root = tempfile::tempdir()?;
|
||||
let home = root.path().join("home");
|
||||
fs::create_dir_all(&home)?;
|
||||
let _home = EnvVarGuard::set("HOME", &home);
|
||||
|
||||
let workspace = root.path().join("workspace");
|
||||
fs::create_dir_all(&workspace)?;
|
||||
let repo = crate::snapshot::SnapshotRepo::open_or_init(&workspace)?;
|
||||
let file = workspace.join("a.txt");
|
||||
|
||||
fs::write(&file, "v1")?;
|
||||
repo.snapshot_with_session("pre-turn:1", Some("session-a"))?;
|
||||
fs::write(&file, "v2")?;
|
||||
|
||||
// A recorded path may arrive absolute; normalization reports it back
|
||||
// workspace-relative so the GUI's change record stays consistent.
|
||||
let snapshot = repo.list(10)?.remove(0);
|
||||
let reverted = revert_file_from_snapshot(
|
||||
&workspace,
|
||||
"session-a",
|
||||
&revert_request(
|
||||
&file.to_string_lossy(),
|
||||
snapshot.id.as_str(),
|
||||
&sha256_of(&file),
|
||||
),
|
||||
)
|
||||
.expect("scoped revert should accept an in-workspace absolute path");
|
||||
assert_eq!(reverted.path, "a.txt");
|
||||
assert_eq!(fs::read_to_string(&file)?, "v1");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The client names the exact snapshot from its change record. A newer
|
||||
/// unrelated snapshot must not be selected for it, and a stale hash means
|
||||
/// the user edited the file after the record was captured: refuse and leave
|
||||
/// the workspace untouched.
|
||||
#[test]
|
||||
fn revert_file_helper_requires_exact_snapshot_identity_and_reviewed_hash() -> Result<()> {
|
||||
let _lock = lock_test_env();
|
||||
let root = tempfile::tempdir()?;
|
||||
let home = root.path().join("home");
|
||||
fs::create_dir_all(&home)?;
|
||||
let _home = EnvVarGuard::set("HOME", &home);
|
||||
|
||||
let workspace = root.path().join("workspace");
|
||||
fs::create_dir_all(&workspace)?;
|
||||
let repo = crate::snapshot::SnapshotRepo::open_or_init(&workspace)?;
|
||||
let a = workspace.join("a.txt");
|
||||
let b = workspace.join("b.txt");
|
||||
|
||||
// Tool A edits a.txt; tool B snapshots before editing only b.txt; the user
|
||||
// then edits a.txt by hand.
|
||||
fs::write(&a, "a-before")?;
|
||||
fs::write(&b, "b-before")?;
|
||||
let tool_a = repo.snapshot_with_session("tool:call-a", Some("session-a"))?;
|
||||
fs::write(&a, "a-after-tool")?;
|
||||
let tool_b = repo.snapshot_with_session("tool:call-b", Some("session-a"))?;
|
||||
fs::write(&b, "b-after-tool")?;
|
||||
fs::write(&a, "a-user-edit")?;
|
||||
|
||||
// Restoring a.txt from tool B's snapshot would erase only the user edit;
|
||||
// the client must ask for tool A's snapshot and gets back a-before.
|
||||
let err = revert_file_from_snapshot(
|
||||
&workspace,
|
||||
"session-a",
|
||||
&revert_request("a.txt", tool_a.as_str(), &sha256_of(&b)),
|
||||
)
|
||||
.expect_err("a hash from different bytes must refuse");
|
||||
assert_eq!(err.status, StatusCode::CONFLICT);
|
||||
assert!(
|
||||
err.message.contains("changed after"),
|
||||
"got: {}",
|
||||
err.message
|
||||
);
|
||||
assert_eq!(fs::read_to_string(&a)?, "a-user-edit");
|
||||
|
||||
let reverted = revert_file_from_snapshot(
|
||||
&workspace,
|
||||
"session-a",
|
||||
&revert_request("a.txt", tool_a.as_str(), &sha256_of(&a)),
|
||||
)
|
||||
.expect("exact identity restores");
|
||||
assert_eq!(reverted.snapshot_label, "tool:call-a");
|
||||
assert_eq!(fs::read_to_string(&a)?, "a-before");
|
||||
assert_eq!(
|
||||
fs::read_to_string(&b)?,
|
||||
"b-after-tool",
|
||||
"b.txt is untouched"
|
||||
);
|
||||
|
||||
// Already matching: 409, nothing changed.
|
||||
let err = revert_file_from_snapshot(
|
||||
&workspace,
|
||||
"session-a",
|
||||
&revert_request("a.txt", tool_a.as_str(), &sha256_of(&a)),
|
||||
)
|
||||
.expect_err("nothing to revert");
|
||||
assert_eq!(err.status, StatusCode::CONFLICT);
|
||||
assert!(
|
||||
err.message.contains("nothing to revert"),
|
||||
"got: {}",
|
||||
err.message
|
||||
);
|
||||
|
||||
// A snapshot owned by another session, an unknown id, or a non-restore
|
||||
// label are all refused with a refresh hint.
|
||||
let foreign = repo.snapshot_with_session("tool:foreign", Some("session-b"))?;
|
||||
let post = repo.snapshot_with_session("post-turn:1", Some("session-a"))?;
|
||||
for id in [
|
||||
foreign.as_str(),
|
||||
post.as_str(),
|
||||
"0123456789abcdef0123456789abcdef01234567",
|
||||
] {
|
||||
let err = revert_file_from_snapshot(
|
||||
&workspace,
|
||||
"session-a",
|
||||
&revert_request("b.txt", id, &sha256_of(&b)),
|
||||
)
|
||||
.expect_err(id);
|
||||
assert_eq!(err.status, StatusCode::CONFLICT, "{id}");
|
||||
assert!(
|
||||
err.message.contains("refresh the change record"),
|
||||
"{id}: {}",
|
||||
err.message
|
||||
);
|
||||
}
|
||||
assert_eq!(fs::read_to_string(&b)?, "b-after-tool");
|
||||
let _ = tool_b;
|
||||
|
||||
// Directories and Git metadata are 400s before anything is compared.
|
||||
for path in ["", "src", ".git/config", "../escape.txt"] {
|
||||
fs::create_dir_all(workspace.join("src"))?;
|
||||
let err = revert_file_from_snapshot(
|
||||
&workspace,
|
||||
"session-a",
|
||||
&revert_request(path, tool_a.as_str(), "absent"),
|
||||
)
|
||||
.expect_err(path);
|
||||
assert_eq!(err.status, StatusCode::BAD_REQUEST, "{path}");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn revert_file_helper_refuses_foreign_sessions_and_paths_outside_the_workspace() -> Result<()> {
|
||||
let _lock = lock_test_env();
|
||||
let root = tempfile::tempdir()?;
|
||||
let home = root.path().join("home");
|
||||
fs::create_dir_all(&home)?;
|
||||
let _home = EnvVarGuard::set("HOME", &home);
|
||||
|
||||
let workspace = root.path().join("workspace");
|
||||
fs::create_dir_all(&workspace)?;
|
||||
let repo = crate::snapshot::SnapshotRepo::open_or_init(&workspace)?;
|
||||
let file = workspace.join("a.txt");
|
||||
|
||||
// Only another session owns a snapshot, so this session has nothing it is
|
||||
// allowed to revert.
|
||||
fs::write(&file, "foreign-before")?;
|
||||
repo.snapshot_with_session("pre-turn:1", Some("session-b"))?;
|
||||
fs::write(&file, "foreign-after")?;
|
||||
|
||||
let foreign = repo.list(10)?.remove(0);
|
||||
let err = revert_file_from_snapshot(
|
||||
&workspace,
|
||||
"session-a",
|
||||
&revert_request("a.txt", foreign.id.as_str(), &sha256_of(&file)),
|
||||
)
|
||||
.expect_err("a foreign session's snapshot must not be restorable");
|
||||
assert_eq!(err.status, StatusCode::CONFLICT);
|
||||
assert!(
|
||||
err.message.contains("another session"),
|
||||
"got: {}",
|
||||
err.message
|
||||
);
|
||||
assert_eq!(fs::read_to_string(&file)?, "foreign-after");
|
||||
|
||||
let traversal = revert_file_from_snapshot(
|
||||
&workspace,
|
||||
"session-a",
|
||||
&revert_request("../escape.txt", foreign.id.as_str(), "absent"),
|
||||
)
|
||||
.expect_err("traversal must be refused");
|
||||
assert_eq!(traversal.status, StatusCode::BAD_REQUEST);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The GUI decides whether to show the per-file Revert button from a `GET`
|
||||
/// probe of this route: 405 ("route exists, wrong method") means available,
|
||||
/// 404 means an older engine that must degrade with an explanation. Assert the
|
||||
/// wire contract, not just the Rust helper.
|
||||
#[tokio::test]
|
||||
async fn file_revert_route_probes_as_available_and_404s_unknown_threads() -> Result<()> {
|
||||
let root = std::env::temp_dir().join(format!("deepseek-file-revert-route-{}", Uuid::new_v4()));
|
||||
let sessions_dir = root.join("sessions");
|
||||
let Some((addr, _runtime_threads, handle)) =
|
||||
spawn_test_server_with_root(root.clone(), sessions_dir).await?
|
||||
else {
|
||||
return Ok(());
|
||||
};
|
||||
let client = crate::tls::reqwest_client();
|
||||
|
||||
let probe = client
|
||||
.get(format!("http://{addr}/v1/threads/__probe__/file-revert"))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(
|
||||
probe.status(),
|
||||
StatusCode::METHOD_NOT_ALLOWED,
|
||||
"the capability probe reads any non-404 as 'endpoint available'"
|
||||
);
|
||||
|
||||
let body = json!({
|
||||
"path": "a.txt",
|
||||
"snapshot_id": "0123456789abcdef0123456789abcdef01234567",
|
||||
"expected_hash": "absent"
|
||||
});
|
||||
let missing = client
|
||||
.post(format!("http://{addr}/v1/threads/thr_missing/file-revert"))
|
||||
.json(&body)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(missing.status(), StatusCode::NOT_FOUND);
|
||||
|
||||
handle.abort();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Route-level contract for the new destructive endpoint: malformed bodies
|
||||
/// are 422/400 before any thread state is read, an untrusted thread is a 409
|
||||
/// even with a well-formed request, and a trusted thread without a bound
|
||||
/// session is a 409 that names the reason. No file is touched in any case.
|
||||
#[tokio::test]
|
||||
async fn file_revert_route_validates_body_then_trust_then_session_binding() -> Result<()> {
|
||||
let root = std::env::temp_dir().join(format!("deepseek-file-revert-gates-{}", Uuid::new_v4()));
|
||||
let sessions_dir = root.join("sessions");
|
||||
let Some((addr, runtime_threads, handle)) =
|
||||
spawn_test_server_with_root(root.clone(), sessions_dir).await?
|
||||
else {
|
||||
return Ok(());
|
||||
};
|
||||
let thread_id = create_seeded_thread(&addr, &runtime_threads, &root, "Edit a.txt").await?;
|
||||
let workspace = root.join("workspace");
|
||||
fs::create_dir_all(&workspace)?;
|
||||
let file = workspace.join("a.txt");
|
||||
fs::write(&file, "keep")?;
|
||||
let client = crate::tls::reqwest_client();
|
||||
let url = format!("http://{addr}/v1/threads/{thread_id}/file-revert");
|
||||
let well_formed = json!({
|
||||
"path": "a.txt",
|
||||
"snapshot_id": "0123456789abcdef0123456789abcdef01234567",
|
||||
"expected_hash": format!("sha256:{}", "a".repeat(64))
|
||||
});
|
||||
|
||||
// Missing required fields: rejected by the JSON extractor.
|
||||
let resp = client
|
||||
.post(&url)
|
||||
.json(&json!({ "path": "a.txt" }))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), StatusCode::UNPROCESSABLE_ENTITY);
|
||||
// Malformed identity/hash: 400 with the expected format.
|
||||
for bad in [
|
||||
json!({ "path": "a.txt", "snapshot_id": "not-hex", "expected_hash": "absent" }),
|
||||
json!({ "path": "a.txt", "snapshot_id": "0123456789abcdef0123456789abcdef01234567", "expected_hash": "md5:abc" }),
|
||||
json!({ "path": "a.txt", "snapshot_id": "0123456789abcdef0123456789abcdef01234567", "expected_hash": format!("sha256:{}", "A".repeat(64)) }),
|
||||
] {
|
||||
let resp = client.post(&url).json(&bad).send().await?;
|
||||
assert_eq!(resp.status(), StatusCode::BAD_REQUEST, "{bad}");
|
||||
}
|
||||
// Untrusted thread: 409 that says how to proceed.
|
||||
let resp = client.post(&url).json(&well_formed).send().await?;
|
||||
assert_eq!(resp.status(), StatusCode::CONFLICT);
|
||||
let text = resp.text().await?;
|
||||
assert!(text.contains("trusted mode"), "got: {text}");
|
||||
|
||||
// Trusted, but no bound session: still a 409 with the reason.
|
||||
client
|
||||
.patch(format!("http://{addr}/v1/threads/{thread_id}"))
|
||||
.json(&json!({ "trust_mode": true }))
|
||||
.send()
|
||||
.await?
|
||||
.error_for_status()?;
|
||||
let resp = client.post(&url).json(&well_formed).send().await?;
|
||||
assert_eq!(resp.status(), StatusCode::CONFLICT);
|
||||
let text = resp.text().await?;
|
||||
assert!(text.contains("no bound session"), "got: {text}");
|
||||
assert_eq!(fs::read_to_string(&file)?, "keep");
|
||||
|
||||
handle.abort();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// All three restore routes refuse while a turn is active in an overlapping
|
||||
/// workspace, and admit again once it settles. Nothing is changed on refusal.
|
||||
#[tokio::test]
|
||||
async fn restore_routes_refuse_an_active_turn_in_the_workspace() -> Result<()> {
|
||||
let root = std::env::temp_dir().join(format!("deepseek-restore-active-{}", Uuid::new_v4()));
|
||||
let sessions_dir = root.join("sessions");
|
||||
let Some((addr, runtime_threads, handle)) =
|
||||
spawn_test_server_with_root(root.clone(), sessions_dir).await?
|
||||
else {
|
||||
return Ok(());
|
||||
};
|
||||
let thread_id = create_seeded_thread(&addr, &runtime_threads, &root, "Keep working").await?;
|
||||
let workspace = root.join("workspace");
|
||||
fs::create_dir_all(&workspace)?;
|
||||
let file = workspace.join("a.txt");
|
||||
fs::write(&file, "live")?;
|
||||
let client = crate::tls::reqwest_client();
|
||||
client
|
||||
.patch(format!("http://{addr}/v1/threads/{thread_id}"))
|
||||
.json(&json!({ "trust_mode": true }))
|
||||
.send()
|
||||
.await?
|
||||
.error_for_status()?;
|
||||
let harness = crate::core::engine::mock_engine_handle();
|
||||
runtime_threads
|
||||
.install_test_engine(&thread_id, harness.handle.clone())
|
||||
.await?;
|
||||
runtime_threads
|
||||
.set_active_turn_for_test(&thread_id, Some("turn_live"))
|
||||
.await?;
|
||||
|
||||
let revert_body = json!({
|
||||
"path": "a.txt",
|
||||
"snapshot_id": "0123456789abcdef0123456789abcdef01234567",
|
||||
"expected_hash": "absent"
|
||||
});
|
||||
let refusals = [
|
||||
client
|
||||
.post(format!("http://{addr}/v1/threads/{thread_id}/file-revert"))
|
||||
.json(&revert_body)
|
||||
.send()
|
||||
.await?,
|
||||
client
|
||||
.post(format!("http://{addr}/v1/threads/{thread_id}/patch-undo"))
|
||||
.json(&json!({}))
|
||||
.send()
|
||||
.await?,
|
||||
];
|
||||
for resp in refusals {
|
||||
assert_eq!(resp.status(), StatusCode::CONFLICT);
|
||||
let text = resp.text().await?;
|
||||
assert!(text.contains("already has an active turn"), "got: {text}");
|
||||
}
|
||||
assert_eq!(fs::read_to_string(&file)?, "live");
|
||||
// The server-wide snapshot route validates its id first, then admission.
|
||||
let resp = client
|
||||
.post(format!("http://{addr}/v1/snapshots/not-hex/restore"))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
|
||||
|
||||
runtime_threads
|
||||
.set_active_turn_for_test(&thread_id, None)
|
||||
.await?;
|
||||
// Admitted again: the revert now fails on the unknown snapshot id, past
|
||||
// the admission and trust gates, without touching the file.
|
||||
let resp = client
|
||||
.post(format!("http://{addr}/v1/threads/{thread_id}/file-revert"))
|
||||
.json(&revert_body)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), StatusCode::CONFLICT);
|
||||
let text = resp.text().await?;
|
||||
assert!(
|
||||
text.contains("no bound session") || text.contains("refresh the change record"),
|
||||
"got: {text}"
|
||||
);
|
||||
assert_eq!(fs::read_to_string(&file)?, "live");
|
||||
|
||||
handle.abort();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn retry_endpoint_reuses_dropped_user_text_to_start_a_turn() -> Result<()> {
|
||||
let root = std::env::temp_dir().join(format!("deepseek-retry-endpoint-{}", Uuid::new_v4()));
|
||||
|
||||
@@ -4036,6 +4036,17 @@ struct ActiveThreads {
|
||||
lru: VecDeque<String>,
|
||||
}
|
||||
|
||||
pub(crate) struct PreparedThreadFork {
|
||||
source_id: String,
|
||||
target_turn_id: String,
|
||||
depth_from_tail: usize,
|
||||
thread: ThreadRecord,
|
||||
records: Vec<(TurnRecord, Vec<TurnItemRecord>)>,
|
||||
original_user_text: Option<String>,
|
||||
original_images: Vec<codewhale_protocol::runtime::RuntimeImageInput>,
|
||||
max_output_tokens: Option<std::num::NonZeroU32>,
|
||||
}
|
||||
|
||||
/// Shared ownership of an existing task's join. A canceled drain drops only
|
||||
/// its await/lock guard; the unfinished join stays available to the next drain.
|
||||
type RuntimeCompletion = Arc<Mutex<Option<tokio::task::JoinHandle<()>>>>;
|
||||
@@ -7399,6 +7410,87 @@ impl RuntimeThreadManager {
|
||||
Arc::clone(&self.config_admission).write_owned().await
|
||||
}
|
||||
|
||||
/// Exclude new turns and saved-history/config changes while a restore
|
||||
/// mutates files. Active turns in any overlapping workspace (the same
|
||||
/// tree, a parent or a nested checkout) are rejected instead of raced.
|
||||
/// Callers move the owned guard into the worker that performs the Git
|
||||
/// mutation so client cancellation cannot release it early.
|
||||
pub(crate) async fn workspace_restore_guard(
|
||||
&self,
|
||||
workspace: &Path,
|
||||
) -> Result<tokio::sync::OwnedRwLockWriteGuard<()>> {
|
||||
let admission = self.session_checkpoint_guard().await;
|
||||
self.reject_active_turns_in_workspace(workspace).await?;
|
||||
Ok(admission)
|
||||
}
|
||||
|
||||
/// Same reservation as [`Self::workspace_restore_guard`], but the thread
|
||||
/// record is read *under* admission so trust, session binding and
|
||||
/// workspace are the values that hold while the restore runs.
|
||||
pub(crate) async fn thread_restore_guard(
|
||||
&self,
|
||||
id: &str,
|
||||
) -> Result<(tokio::sync::OwnedRwLockWriteGuard<()>, ThreadRecord)> {
|
||||
let admission = self.session_checkpoint_guard().await;
|
||||
let thread = self.get_thread(id).await?;
|
||||
self.reject_active_turns_in_workspace(&thread.workspace)
|
||||
.await?;
|
||||
Ok((admission, thread))
|
||||
}
|
||||
|
||||
/// Lock order: `config_admission` (held by the caller) before `active`,
|
||||
/// matching turn admission and config reload.
|
||||
async fn reject_active_turns_in_workspace(&self, workspace: &Path) -> Result<()> {
|
||||
// A workspace directory that no longer exists cannot host a running
|
||||
// tool; compare the recorded path as-is in that case.
|
||||
let workspace = workspace
|
||||
.canonicalize()
|
||||
.unwrap_or_else(|_| workspace.to_path_buf());
|
||||
// Collect ids under the async lock, then do filesystem and store
|
||||
// I/O without holding it.
|
||||
let busy: Vec<String> = {
|
||||
let active = self.active.lock().await;
|
||||
active
|
||||
.engines
|
||||
.iter()
|
||||
.filter(|(_, state)| state.active_turn.is_some())
|
||||
.map(|(id, _)| id.clone())
|
||||
.collect()
|
||||
};
|
||||
for id in busy {
|
||||
let other = self.store.load_thread(&id)?.workspace;
|
||||
let other = other.canonicalize().unwrap_or(other);
|
||||
if workspace.starts_with(&other) || other.starts_with(&workspace) {
|
||||
bail!(
|
||||
"Workspace already has an active turn (thread {id}); wait for it to finish before restoring files"
|
||||
);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Test seam: mark or clear an active turn on an installed test engine so
|
||||
/// route-level tests can exercise restore admission.
|
||||
#[cfg(test)]
|
||||
pub(crate) async fn set_active_turn_for_test(
|
||||
&self,
|
||||
thread_id: &str,
|
||||
turn_id: Option<&str>,
|
||||
) -> Result<()> {
|
||||
let mut active = self.active.lock().await;
|
||||
let state = active
|
||||
.engines
|
||||
.get_mut(thread_id)
|
||||
.ok_or_else(|| anyhow!("no engine installed for {thread_id}"))?;
|
||||
state.active_turn = turn_id.map(|turn_id| ActiveTurnState {
|
||||
turn_id: turn_id.to_string(),
|
||||
goal_id: None,
|
||||
interrupt_requested: false,
|
||||
compaction_id: None,
|
||||
});
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Bind full-fidelity saved history to the exact prefix it covers. Callers
|
||||
/// hold session_checkpoint_guard across the snapshot, save and this write.
|
||||
pub(crate) async fn set_thread_session_checkpoint(
|
||||
@@ -7592,6 +7684,17 @@ impl RuntimeThreadManager {
|
||||
Vec<codewhale_protocol::runtime::RuntimeImageInput>,
|
||||
Option<std::num::NonZeroU32>,
|
||||
)> {
|
||||
let prepared = self
|
||||
.prepare_fork_at_user_message(id, depth_from_tail)
|
||||
.await?;
|
||||
self.publish_prepared_fork(prepared).await
|
||||
}
|
||||
|
||||
pub(crate) async fn prepare_fork_at_user_message(
|
||||
&self,
|
||||
id: &str,
|
||||
depth_from_tail: usize,
|
||||
) -> Result<PreparedThreadFork> {
|
||||
let source = self.get_thread(id).await?;
|
||||
let source_turns = self.store.list_turns_for_thread(&source.id)?;
|
||||
|
||||
@@ -7700,26 +7803,57 @@ impl RuntimeThreadManager {
|
||||
forked.updated_at = now;
|
||||
cloned_records.push((cloned_turn, cloned_items));
|
||||
}
|
||||
self.publish_fork(&forked, &cloned_records)?;
|
||||
|
||||
self.emit_event(
|
||||
&forked.id,
|
||||
None,
|
||||
None,
|
||||
"thread.forked",
|
||||
json!({
|
||||
"thread": forked,
|
||||
"source_thread_id": source.id,
|
||||
"backtrack_depth_from_tail": depth_from_tail,
|
||||
"dropped_turn_id": target_turn_id,
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
Ok((
|
||||
forked,
|
||||
Ok(PreparedThreadFork {
|
||||
source_id: source.id,
|
||||
target_turn_id,
|
||||
depth_from_tail,
|
||||
thread: forked,
|
||||
records: cloned_records,
|
||||
original_user_text,
|
||||
original_images,
|
||||
source_turns[target_turn_idx].max_output_tokens,
|
||||
max_output_tokens: source_turns[target_turn_idx].max_output_tokens,
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) async fn publish_prepared_fork(
|
||||
&self,
|
||||
prepared: PreparedThreadFork,
|
||||
) -> Result<(
|
||||
ThreadRecord,
|
||||
Option<String>,
|
||||
Vec<codewhale_protocol::runtime::RuntimeImageInput>,
|
||||
Option<std::num::NonZeroU32>,
|
||||
)> {
|
||||
self.publish_fork(&prepared.thread, &prepared.records)?;
|
||||
// The fork is durable once publish_fork returns. A failed event
|
||||
// append must not report the fork as unsaved: the caller already
|
||||
// holds the forked record and clients can reload it.
|
||||
if let Err(error) = self
|
||||
.emit_event(
|
||||
&prepared.thread.id,
|
||||
None,
|
||||
None,
|
||||
"thread.forked",
|
||||
json!({
|
||||
"thread": prepared.thread,
|
||||
"source_thread_id": prepared.source_id,
|
||||
"backtrack_depth_from_tail": prepared.depth_from_tail,
|
||||
"dropped_turn_id": prepared.target_turn_id,
|
||||
}),
|
||||
)
|
||||
.await
|
||||
{
|
||||
tracing::warn!(
|
||||
thread_id = %prepared.thread.id,
|
||||
"fork {} was saved but its thread.forked event failed: {error:#}",
|
||||
prepared.thread.id
|
||||
);
|
||||
}
|
||||
Ok((
|
||||
prepared.thread,
|
||||
prepared.original_user_text,
|
||||
prepared.original_images,
|
||||
prepared.max_output_tokens,
|
||||
))
|
||||
}
|
||||
|
||||
|
||||
@@ -8415,6 +8415,80 @@ async fn update_thread_workspace_rejects_empty_path() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Restore admission rejects any active turn whose workspace overlaps the
|
||||
/// one about to be mutated: the same tree, a nested checkout, or a parent.
|
||||
/// Unrelated workspaces are unaffected, and the thread-scoped guard reads the
|
||||
/// record under admission.
|
||||
#[tokio::test]
|
||||
async fn workspace_restore_guard_rejects_overlapping_active_turns() -> Result<()> {
|
||||
let manager = test_manager(test_runtime_dir())?;
|
||||
let root = std::env::temp_dir().join(format!("codewhale-restore-guard-{}", Uuid::new_v4()));
|
||||
let busy = root.join("busy");
|
||||
let nested = busy.join("nested");
|
||||
let idle = root.join("idle");
|
||||
std::fs::create_dir_all(&nested)?;
|
||||
std::fs::create_dir_all(&idle)?;
|
||||
let thread = manager
|
||||
.create_thread(CreateThreadRequest {
|
||||
workspace: Some(busy.clone()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
|
||||
// Nothing is running: both guards are granted and released.
|
||||
drop(manager.workspace_restore_guard(&busy).await?);
|
||||
let (guard, record) = manager.thread_restore_guard(&thread.id).await?;
|
||||
assert_eq!(record.id, thread.id);
|
||||
drop(guard);
|
||||
|
||||
let _harness = install_mock_engine(&manager, &thread.id).await;
|
||||
{
|
||||
let mut active = manager.active.lock().await;
|
||||
let state = active.engines.get_mut(&thread.id).expect("mock engine");
|
||||
state.active_turn = Some(ActiveTurnState {
|
||||
goal_id: None,
|
||||
turn_id: "turn_live".to_string(),
|
||||
interrupt_requested: false,
|
||||
compaction_id: None,
|
||||
});
|
||||
}
|
||||
for workspace in [&busy, &nested, &root] {
|
||||
let err = manager
|
||||
.workspace_restore_guard(workspace)
|
||||
.await
|
||||
.expect_err("overlapping active turn must be rejected");
|
||||
assert!(
|
||||
format!("{err:#}").contains("already has an active turn"),
|
||||
"{}: {err:#}",
|
||||
workspace.display()
|
||||
);
|
||||
}
|
||||
let err = manager
|
||||
.thread_restore_guard(&thread.id)
|
||||
.await
|
||||
.expect_err("the busy thread itself must be rejected");
|
||||
assert!(format!("{err:#}").contains("already has an active turn"));
|
||||
// An unrelated workspace is not blocked by someone else's turn.
|
||||
drop(manager.workspace_restore_guard(&idle).await?);
|
||||
let err = manager
|
||||
.thread_restore_guard("thr_missing")
|
||||
.await
|
||||
.expect_err("unknown thread");
|
||||
assert!(format!("{err:#}").contains("Thread not found"));
|
||||
|
||||
{
|
||||
let mut active = manager.active.lock().await;
|
||||
active
|
||||
.engines
|
||||
.get_mut(&thread.id)
|
||||
.expect("mock engine")
|
||||
.active_turn = None;
|
||||
}
|
||||
drop(manager.workspace_restore_guard(&busy).await?);
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_thread_workspace_rejects_active_turn() -> Result<()> {
|
||||
let manager = test_manager(test_runtime_dir())?;
|
||||
|
||||
@@ -54,6 +54,7 @@ pub const DEFAULT_MAX_SNAPSHOTS: usize = 50;
|
||||
#[allow(unused_imports)]
|
||||
pub use repo::{
|
||||
DEFAULT_MAX_WORKSPACE_BYTES_FOR_SNAPSHOT, GATE_TOO_LARGE_MARKER, GATE_TOO_MANY_ENTRIES_MARKER,
|
||||
GATE_UNSAFE_LOCATION_MARKER, SIZE_WALK_MAX_ENTRIES, Snapshot, SnapshotId, SnapshotRepo,
|
||||
WorkspaceGate, estimate_workspace_size_bounded,
|
||||
GATE_UNSAFE_LOCATION_MARKER, PathRestoreAction, PathRestoreOutcome, SIZE_WALK_MAX_ENTRIES,
|
||||
Snapshot, SnapshotId, SnapshotRepo, WorkspaceGate, estimate_workspace_size_bounded,
|
||||
workspace_relative_path,
|
||||
};
|
||||
@@ -48,6 +48,41 @@ pub struct Snapshot {
|
||||
pub session_id: Option<String>,
|
||||
}
|
||||
|
||||
/// What a file-scoped restore did to one path, relative to the working tree
|
||||
/// it was applied to.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum PathRestoreAction {
|
||||
/// Both the snapshot and the working tree had the path; its content came
|
||||
/// back from the snapshot.
|
||||
Modified,
|
||||
/// The snapshot had the path and the working tree no longer did, so the
|
||||
/// restore recreated the file.
|
||||
Recreated,
|
||||
/// The working tree had the path and the snapshot did not, so the restore
|
||||
/// removed the file.
|
||||
Removed,
|
||||
}
|
||||
|
||||
impl PathRestoreAction {
|
||||
/// Stable wire name, also used by the runtime API response.
|
||||
pub fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::Modified => "modified",
|
||||
Self::Recreated => "recreated",
|
||||
Self::Removed => "removed",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Report of what [`SnapshotRepo::restore_paths`] did to one path.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct PathRestoreOutcome {
|
||||
/// Workspace-relative path that was restored.
|
||||
pub path: PathBuf,
|
||||
/// How the working tree changed.
|
||||
pub action: PathRestoreAction,
|
||||
}
|
||||
|
||||
/// Wrapper around the per-workspace side-git repo.
|
||||
pub struct SnapshotRepo {
|
||||
git_dir: PathBuf,
|
||||
@@ -591,6 +626,285 @@ impl SnapshotRepo {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// File restore never traverses symlinks, directories, or Git metadata.
|
||||
/// Validate every existing component before reading, backing up or writing.
|
||||
pub fn validate_restore_file(&self, rel: &Path) -> io::Result<bool> {
|
||||
if !is_safe_relative_path(rel)
|
||||
|| rel.components().any(|part| {
|
||||
part.as_os_str()
|
||||
.as_encoded_bytes()
|
||||
.eq_ignore_ascii_case(b".git")
|
||||
})
|
||||
{
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
format!(
|
||||
"refusing to restore unsafe path '{}': restore requires a regular workspace file",
|
||||
rel.display()
|
||||
),
|
||||
));
|
||||
}
|
||||
let mut path = self.work_tree.clone();
|
||||
for part in rel.components() {
|
||||
path.push(part);
|
||||
match std::fs::symlink_metadata(&path) {
|
||||
Ok(meta)
|
||||
if meta.file_type().is_symlink()
|
||||
|| (path == self.work_tree.join(rel) && !meta.is_file())
|
||||
|| (path != self.work_tree.join(rel) && !meta.is_dir()) =>
|
||||
{
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
"restore refuses directories, symlinks and non-regular files",
|
||||
));
|
||||
}
|
||||
Ok(_) => {}
|
||||
Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(false),
|
||||
Err(error) => return Err(error),
|
||||
}
|
||||
}
|
||||
Ok(true)
|
||||
}
|
||||
|
||||
fn snapshot_contains_regular_file(&self, id: &SnapshotId, rel: &Path) -> io::Result<bool> {
|
||||
let entry = run_git(
|
||||
&self.git_dir,
|
||||
&self.work_tree,
|
||||
&[
|
||||
"--literal-pathspecs",
|
||||
"ls-tree",
|
||||
"-z",
|
||||
id.as_str(),
|
||||
"--",
|
||||
rel.to_str()
|
||||
.ok_or_else(|| io_other("restore path must be UTF-8"))?,
|
||||
],
|
||||
)?;
|
||||
if !entry.status.success() {
|
||||
return Err(io_other(format!(
|
||||
"Failed to inspect snapshot file: {}",
|
||||
String::from_utf8_lossy(&entry.stderr).trim()
|
||||
)));
|
||||
}
|
||||
if entry.stdout.is_empty() {
|
||||
return Ok(false);
|
||||
}
|
||||
if !(entry.stdout.starts_with(b"100644 blob ") || entry.stdout.starts_with(b"100755 blob "))
|
||||
{
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
"snapshot path is not a regular file",
|
||||
));
|
||||
}
|
||||
Ok(true)
|
||||
}
|
||||
|
||||
/// Return whether `rel` differs between snapshot `id` and the current
|
||||
/// working tree.
|
||||
///
|
||||
/// This is the single-path counterpart of
|
||||
/// [`Self::work_tree_matches_snapshot`]: it answers "would restoring just
|
||||
/// this file change anything?", which is what file-scoped revert
|
||||
/// cursoring needs. A path that exists in neither the snapshot nor the
|
||||
/// working tree does not differ.
|
||||
pub fn path_differs_from_snapshot(&self, id: &SnapshotId, rel: &Path) -> io::Result<bool> {
|
||||
let in_work = self.validate_restore_file(rel)?;
|
||||
let in_target = self.snapshot_contains_regular_file(id, rel)?;
|
||||
match (in_target, in_work) {
|
||||
// Neither side has it: nothing to restore and nothing to remove.
|
||||
(false, false) => Ok(false),
|
||||
// The snapshot has it and the working tree lost it.
|
||||
(true, false) => Ok(true),
|
||||
// The path was created after the snapshot.
|
||||
(false, true) => Ok(true),
|
||||
(true, true) => {
|
||||
let rel = rel.to_string_lossy().into_owned();
|
||||
let diff = run_git(
|
||||
&self.git_dir,
|
||||
&self.work_tree,
|
||||
&[
|
||||
"--literal-pathspecs",
|
||||
"diff",
|
||||
"--quiet",
|
||||
id.as_str(),
|
||||
"--",
|
||||
rel.as_str(),
|
||||
],
|
||||
)?;
|
||||
git_diff_matches(diff).map(|matches| !matches)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Restore only `rel_paths` from snapshot `id`.
|
||||
///
|
||||
/// This is the file-scoped counterpart of [`Self::restore`]. The
|
||||
/// difference that matters: the whole-tree `git checkout <sha> -- :/` is
|
||||
/// replaced by a pathspec-limited checkout, so a working-tree path outside
|
||||
/// `rel_paths` is never written or deleted. The safety backup reads the workspace.
|
||||
///
|
||||
/// A path the snapshot does not track is removed from the working tree
|
||||
/// (that is how a file created after the snapshot is reverted), and a path
|
||||
/// the snapshot tracks but the working tree lost is recreated. A path that
|
||||
/// exists in neither side produces no outcome at all, rather than a
|
||||
/// report claiming a change that did not happen.
|
||||
#[cfg(test)]
|
||||
pub fn restore_paths(
|
||||
&self,
|
||||
id: &SnapshotId,
|
||||
rel_paths: &[PathBuf],
|
||||
) -> io::Result<Vec<PathRestoreOutcome>> {
|
||||
self.restore_paths_checked(id, rel_paths, || Ok(()))
|
||||
}
|
||||
|
||||
pub fn restore_file_if_unchanged(
|
||||
&self,
|
||||
id: &SnapshotId,
|
||||
rel: &Path,
|
||||
expected_hash: &str,
|
||||
) -> io::Result<Vec<PathRestoreOutcome>> {
|
||||
let verify = || {
|
||||
let actual = if self.validate_restore_file(rel)? {
|
||||
let bytes = std::fs::read(self.work_tree.join(rel))?;
|
||||
format!("sha256:{}", crate::hashing::sha256_hex(bytes))
|
||||
} else {
|
||||
"absent".to_string()
|
||||
};
|
||||
if actual != expected_hash {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::WouldBlock,
|
||||
"The file changed after the selected change record. Refresh and review it before restoring; nothing was changed.",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
};
|
||||
verify()?;
|
||||
self.restore_paths_checked(id, &[rel.to_path_buf()], verify)
|
||||
}
|
||||
|
||||
fn restore_paths_checked(
|
||||
&self,
|
||||
id: &SnapshotId,
|
||||
rel_paths: &[PathBuf],
|
||||
preflight: impl FnOnce() -> io::Result<()>,
|
||||
) -> io::Result<Vec<PathRestoreOutcome>> {
|
||||
if rel_paths.is_empty() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
// Validate the entire request before any mutation or backup. A snapshot
|
||||
// directory entry must not turn a file action into recursive checkout.
|
||||
let mut pre_state = Vec::with_capacity(rel_paths.len());
|
||||
for rel in rel_paths {
|
||||
let in_work = self.validate_restore_file(rel)?;
|
||||
let in_target = self.snapshot_contains_regular_file(id, rel)?;
|
||||
pre_state.push((rel.clone(), in_target, in_work));
|
||||
}
|
||||
|
||||
// A durable backup is required for this new destructive API. Ignored
|
||||
// files cannot be removed/overwritten if the snapshot cannot retain them.
|
||||
let target_short = &id.as_str()[..id.as_str().len().min(12)];
|
||||
let backup = self.snapshot_with_session(&format!("pre-restore:{target_short}"), None)?;
|
||||
for (rel, _, in_work) in &pre_state {
|
||||
if *in_work && !self.snapshot_contains_regular_file(&backup, rel)? {
|
||||
return Err(io_other(
|
||||
"File was excluded from the safety snapshot; nothing was restored",
|
||||
));
|
||||
}
|
||||
self.validate_restore_file(rel)?;
|
||||
}
|
||||
|
||||
// Recheck after the potentially slow safety snapshot, immediately
|
||||
// before checkout/removal. New editor work is retained in the backup.
|
||||
preflight()?;
|
||||
|
||||
let tracked: Vec<String> = pre_state
|
||||
.iter()
|
||||
.filter(|(_, in_target, _)| *in_target)
|
||||
.map(|(rel, _, _)| rel.to_string_lossy().into_owned())
|
||||
.collect();
|
||||
if !tracked.is_empty() {
|
||||
let mut args: Vec<String> = vec![
|
||||
"--literal-pathspecs".to_string(),
|
||||
"checkout".to_string(),
|
||||
id.as_str().to_string(),
|
||||
"--".to_string(),
|
||||
];
|
||||
args.extend(tracked);
|
||||
let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect();
|
||||
let checkout = run_git(&self.git_dir, &self.work_tree, &arg_refs)?;
|
||||
if !checkout.status.success() {
|
||||
return Err(io_other(format!(
|
||||
"git checkout failed: {} (safety snapshot {} holds the previous files)",
|
||||
String::from_utf8_lossy(&checkout.stderr).trim(),
|
||||
backup.as_str()
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
let mut outcomes = Vec::new();
|
||||
for (rel, in_target, was_in_work) in pre_state {
|
||||
match (in_target, was_in_work) {
|
||||
(true, true) => outcomes.push(PathRestoreOutcome {
|
||||
path: rel,
|
||||
action: PathRestoreAction::Modified,
|
||||
}),
|
||||
(true, false) => outcomes.push(PathRestoreOutcome {
|
||||
path: rel,
|
||||
action: PathRestoreAction::Recreated,
|
||||
}),
|
||||
(false, true) => {
|
||||
let path = self.work_tree.join(&rel);
|
||||
self.validate_restore_file(&rel)?;
|
||||
// Only the requested file goes; its parent directories
|
||||
// stay even when emptied, because the request named a
|
||||
// file, not a tree.
|
||||
std::fs::remove_file(&path).map_err(|error| {
|
||||
io_other(format!(
|
||||
"removing '{}' failed: {error} (safety snapshot {} holds the previous files)",
|
||||
rel.display(),
|
||||
backup.as_str()
|
||||
))
|
||||
})?;
|
||||
outcomes.push(PathRestoreOutcome {
|
||||
path: rel,
|
||||
action: PathRestoreAction::Removed,
|
||||
});
|
||||
}
|
||||
// Already in the snapshot's state.
|
||||
(false, false) => {}
|
||||
}
|
||||
}
|
||||
Ok(outcomes)
|
||||
}
|
||||
|
||||
/// `git diff --stat` between snapshot `id` and the current working tree,
|
||||
/// computed inside the side repo.
|
||||
///
|
||||
/// This is what restoring `id` *would* change, so it must be captured
|
||||
/// before the restore runs — afterwards the work tree matches the snapshot
|
||||
/// and the diff is empty by construction.
|
||||
///
|
||||
/// It deliberately runs against the side repo rather than the user's. The
|
||||
/// previous summary ran `git diff --stat` in the workspace with the user's
|
||||
/// `.git`, which reports the user's own uncommitted work: it listed files
|
||||
/// the restore had not touched, and reported nothing when that work
|
||||
/// happened to be committed. Returns `None` when nothing differs.
|
||||
pub fn snapshot_diff_stat(&self, id: &SnapshotId) -> io::Result<Option<String>> {
|
||||
let diff = run_git(
|
||||
&self.git_dir,
|
||||
&self.work_tree,
|
||||
&["diff", "--stat", id.as_str(), "--", ":/"],
|
||||
)?;
|
||||
if !diff.status.success() {
|
||||
return Err(io_other(format!(
|
||||
"git diff --stat failed: {}",
|
||||
String::from_utf8_lossy(&diff.stderr).trim()
|
||||
)));
|
||||
}
|
||||
let stat = String::from_utf8_lossy(&diff.stdout).trim().to_string();
|
||||
Ok((!stat.is_empty()).then_some(stat))
|
||||
}
|
||||
|
||||
/// Return whether the current workspace matches the given snapshot's
|
||||
/// tracked file content.
|
||||
///
|
||||
@@ -606,7 +920,7 @@ impl SnapshotRepo {
|
||||
&self.work_tree,
|
||||
&["diff", "--quiet", id.as_str(), "--", ":/"],
|
||||
)?;
|
||||
Ok(diff.status.success())
|
||||
git_diff_matches(diff)
|
||||
}
|
||||
|
||||
fn tree_paths(&self, treeish: &str) -> io::Result<HashSet<PathBuf>> {
|
||||
@@ -673,8 +987,26 @@ impl SnapshotRepo {
|
||||
let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect();
|
||||
let log = run_git(&self.git_dir, &self.work_tree, &arg_refs)?;
|
||||
if !log.status.success() {
|
||||
// No commits yet → empty list.
|
||||
return Ok(Vec::new());
|
||||
let head = run_git(
|
||||
&self.git_dir,
|
||||
&self.work_tree,
|
||||
&["symbolic-ref", "-q", "HEAD"],
|
||||
)?;
|
||||
if head.status.success() {
|
||||
let reference = String::from_utf8_lossy(&head.stdout);
|
||||
let exists = run_git(
|
||||
&self.git_dir,
|
||||
&self.work_tree,
|
||||
&["show-ref", "--verify", "--quiet", reference.trim()],
|
||||
)?;
|
||||
if exists.status.code() == Some(1) {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
}
|
||||
return Err(io_other(format!(
|
||||
"git log failed: {}",
|
||||
String::from_utf8_lossy(&log.stderr).trim()
|
||||
)));
|
||||
}
|
||||
let stdout = String::from_utf8_lossy(&log.stdout);
|
||||
let mut out = Vec::new();
|
||||
@@ -1042,6 +1374,17 @@ fn run_git(git_dir: &Path, work_tree: &Path, args: &[&str]) -> io::Result<Output
|
||||
.output()
|
||||
}
|
||||
|
||||
fn git_diff_matches(output: Output) -> io::Result<bool> {
|
||||
match output.status.code() {
|
||||
Some(0) => Ok(true),
|
||||
Some(1) => Ok(false),
|
||||
_ => Err(io_other(format!(
|
||||
"git diff failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr).trim()
|
||||
))),
|
||||
}
|
||||
}
|
||||
|
||||
fn io_other(msg: impl Into<String>) -> io::Error {
|
||||
io::Error::other(msg.into())
|
||||
}
|
||||
@@ -1169,6 +1512,30 @@ fn is_safe_relative_path(path: &Path) -> bool {
|
||||
.all(|component| matches!(component, Component::Normal(_)))
|
||||
}
|
||||
|
||||
/// Normalize a caller-supplied path into a safe workspace-relative path.
|
||||
///
|
||||
/// Accepts either a workspace-relative path or an absolute path inside
|
||||
/// `workspace`. Returns `None` when the result is not a plain relative path —
|
||||
/// absolute, empty, containing `..`, or pointing outside the workspace. Every
|
||||
/// file-scoped restore path passes through here, so a caller never gets to
|
||||
/// name a path the snapshot repo would resolve outside the work tree.
|
||||
///
|
||||
/// The name is literal: leading or trailing spaces, brackets and glob
|
||||
/// characters are filename bytes, never trimmed and never patterns. Git is
|
||||
/// invoked with `--literal-pathspecs` for every file-scoped operation.
|
||||
pub fn workspace_relative_path(workspace: &Path, raw: &str) -> Option<PathBuf> {
|
||||
if raw.is_empty() {
|
||||
return None;
|
||||
}
|
||||
let candidate = Path::new(raw);
|
||||
let rel = if candidate.is_absolute() {
|
||||
candidate.strip_prefix(workspace).ok()?.to_path_buf()
|
||||
} else {
|
||||
candidate.to_path_buf()
|
||||
};
|
||||
is_safe_relative_path(&rel).then_some(rel)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -1302,6 +1669,450 @@ mod tests {
|
||||
assert!(!added.exists(), "restore must remove tracked added files");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restore_paths_leaves_unrelated_files_alone() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let (repo, _home) = make_repo(tmp.path());
|
||||
let wanted = repo.work_tree().join("wanted.txt");
|
||||
let unrelated = repo.work_tree().join("unrelated.txt");
|
||||
|
||||
std::fs::write(&wanted, b"original").unwrap();
|
||||
std::fs::write(&unrelated, b"original").unwrap();
|
||||
let id = repo.snapshot("pre-turn:1").expect("snapshot");
|
||||
|
||||
std::fs::write(&wanted, b"clobbered").unwrap();
|
||||
std::fs::write(&unrelated, b"also clobbered").unwrap();
|
||||
repo.snapshot("post-turn:1").expect("snapshot 2");
|
||||
|
||||
let outcomes = repo
|
||||
.restore_paths(&id, &[PathBuf::from("wanted.txt")])
|
||||
.expect("scoped restore");
|
||||
|
||||
assert_eq!(std::fs::read_to_string(&wanted).unwrap(), "original");
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(&unrelated).unwrap(),
|
||||
"also clobbered",
|
||||
"a file-scoped restore must not touch a path it was not given"
|
||||
);
|
||||
assert_eq!(outcomes.len(), 1);
|
||||
assert_eq!(outcomes[0].path, PathBuf::from("wanted.txt"));
|
||||
assert_eq!(outcomes[0].action, PathRestoreAction::Modified);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn only_restore_paths_is_safe_for_a_single_file_action() {
|
||||
// Characterizes the difference the per-file Revert control depends on.
|
||||
// `restore()` is what the TUI's `patch_undo()` and the runtime's
|
||||
// `patch-undo` endpoint both call. It is scoped in *snapshot selection*
|
||||
// (it picks a recent `tool:` snapshot) but not in *effect*: it checks
|
||||
// out the whole tree, so it also rolls back a working-tree path that no
|
||||
// tool touched. That is the data loss #2 removed the control over, and
|
||||
// it is why a per-file action cannot be built on top of it.
|
||||
let tmp = tempdir().unwrap();
|
||||
let (repo, _home) = make_repo(tmp.path());
|
||||
let touched = repo.work_tree().join("touched.txt");
|
||||
let unrelated = repo.work_tree().join("unrelated.txt");
|
||||
|
||||
std::fs::write(&touched, b"v1").unwrap();
|
||||
std::fs::write(&unrelated, b"snapshot-time").unwrap();
|
||||
let id = repo.snapshot("tool:call-1").expect("snapshot");
|
||||
|
||||
// The tool edits one file; something else — the user, another editor —
|
||||
// changes the other one after the snapshot was taken.
|
||||
std::fs::write(&touched, b"v2").unwrap();
|
||||
std::fs::write(&unrelated, b"user-work-in-progress").unwrap();
|
||||
|
||||
repo.restore(&id).expect("whole-tree restore");
|
||||
assert_eq!(std::fs::read_to_string(&touched).unwrap(), "v1");
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(&unrelated).unwrap(),
|
||||
"snapshot-time",
|
||||
"whole-tree restore rolls back a file no tool touched"
|
||||
);
|
||||
|
||||
// Same situation again, but through the file-scoped path the
|
||||
// `file-revert` endpoint uses.
|
||||
std::fs::write(&touched, b"v1").unwrap();
|
||||
std::fs::write(&unrelated, b"snapshot-time").unwrap();
|
||||
let id2 = repo.snapshot("tool:call-2").expect("snapshot 2");
|
||||
std::fs::write(&touched, b"v2").unwrap();
|
||||
std::fs::write(&unrelated, b"user-work-in-progress").unwrap();
|
||||
|
||||
repo.restore_paths(&id2, &[PathBuf::from("touched.txt")])
|
||||
.expect("scoped restore");
|
||||
assert_eq!(std::fs::read_to_string(&touched).unwrap(), "v1");
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(&unrelated).unwrap(),
|
||||
"user-work-in-progress",
|
||||
"the scoped restore must leave the unrelated edit alone"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn snapshot_diff_stat_describes_what_a_restore_would_change() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let (repo, _home) = make_repo(tmp.path());
|
||||
let changed = repo.work_tree().join("changed.txt");
|
||||
let untouched = repo.work_tree().join("untouched.txt");
|
||||
|
||||
std::fs::write(&changed, b"v1").unwrap();
|
||||
std::fs::write(&untouched, b"stable").unwrap();
|
||||
let id = repo.snapshot("pre-turn:1").expect("snapshot");
|
||||
|
||||
std::fs::write(&changed, b"v2").unwrap();
|
||||
|
||||
let stat = repo
|
||||
.snapshot_diff_stat(&id)
|
||||
.expect("diff stat")
|
||||
.expect("the snapshot differs, so something must be reported");
|
||||
assert!(stat.contains("changed.txt"), "got: {stat}");
|
||||
// The stat describes the restore's effect, not the workspace's whole
|
||||
// uncommitted state — a file the restore will not touch must not appear.
|
||||
assert!(!stat.contains("untouched.txt"), "got: {stat}");
|
||||
|
||||
// After restoring, the two sides agree: nothing left to report. (Which
|
||||
// is why the caller must capture this *before* the restore runs.)
|
||||
repo.restore(&id).expect("restore");
|
||||
assert_eq!(repo.snapshot_diff_stat(&id).expect("diff stat"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restore_paths_removes_a_file_created_after_the_snapshot() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let (repo, _home) = make_repo(tmp.path());
|
||||
let kept = repo.work_tree().join("kept.txt");
|
||||
let created = repo.work_tree().join("created.txt");
|
||||
|
||||
std::fs::write(&kept, b"kept").unwrap();
|
||||
let id = repo.snapshot("pre-turn:1").expect("snapshot");
|
||||
|
||||
std::fs::write(&created, b"new file").unwrap();
|
||||
repo.snapshot("post-turn:1").expect("snapshot 2");
|
||||
|
||||
let outcomes = repo
|
||||
.restore_paths(&id, &[PathBuf::from("created.txt")])
|
||||
.expect("scoped restore");
|
||||
|
||||
assert!(
|
||||
!created.exists(),
|
||||
"a created file must be removed by revert"
|
||||
);
|
||||
assert!(kept.exists(), "the untouched file must survive");
|
||||
assert_eq!(outcomes[0].action, PathRestoreAction::Removed);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restore_paths_recreates_a_file_deleted_after_the_snapshot() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let (repo, _home) = make_repo(tmp.path());
|
||||
let deleted = repo.work_tree().join("deleted.txt");
|
||||
|
||||
std::fs::write(&deleted, b"content").unwrap();
|
||||
let id = repo.snapshot("pre-turn:1").expect("snapshot");
|
||||
|
||||
std::fs::remove_file(&deleted).unwrap();
|
||||
repo.snapshot("post-turn:1").expect("snapshot 2");
|
||||
|
||||
let outcomes = repo
|
||||
.restore_paths(&id, &[PathBuf::from("deleted.txt")])
|
||||
.expect("scoped restore");
|
||||
|
||||
assert_eq!(std::fs::read_to_string(&deleted).unwrap(), "content");
|
||||
assert_eq!(outcomes[0].action, PathRestoreAction::Recreated);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restore_paths_rejects_parent_traversal() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let (repo, _home) = make_repo(tmp.path());
|
||||
std::fs::write(repo.work_tree().join("a.txt"), b"a").unwrap();
|
||||
let id = repo.snapshot("pre-turn:1").expect("snapshot");
|
||||
|
||||
let err = repo
|
||||
.restore_paths(&id, &[PathBuf::from("../escape.txt")])
|
||||
.expect_err("traversal must be refused");
|
||||
assert!(err.to_string().contains("unsafe path"), "got: {err}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn path_differs_from_snapshot_is_scoped_to_the_named_path() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let (repo, _home) = make_repo(tmp.path());
|
||||
let touched = repo.work_tree().join("touched.txt");
|
||||
let untouched = repo.work_tree().join("untouched.txt");
|
||||
|
||||
std::fs::write(&touched, b"v1").unwrap();
|
||||
std::fs::write(&untouched, b"v1").unwrap();
|
||||
let id = repo.snapshot("pre-turn:1").expect("snapshot");
|
||||
|
||||
std::fs::write(&touched, b"v2").unwrap();
|
||||
|
||||
assert!(
|
||||
repo.path_differs_from_snapshot(&id, Path::new("touched.txt"))
|
||||
.expect("differs")
|
||||
);
|
||||
assert!(
|
||||
!repo
|
||||
.path_differs_from_snapshot(&id, Path::new("untouched.txt"))
|
||||
.expect("differs")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workspace_relative_path_accepts_inside_paths_and_refuses_outside_ones() {
|
||||
// A real absolute temp path so the fixture is absolute on Windows too
|
||||
// (`/tmp/ws` is a relative path with a root-dir component there).
|
||||
let temp = std::env::temp_dir();
|
||||
let workspace = temp.join("ws");
|
||||
let other = temp.join("other");
|
||||
|
||||
assert_eq!(
|
||||
workspace_relative_path(&workspace, "src/lib.rs"),
|
||||
Some(PathBuf::from("src/lib.rs"))
|
||||
);
|
||||
let inside = workspace.join("src").join("lib.rs");
|
||||
assert_eq!(
|
||||
workspace_relative_path(&workspace, &inside.to_string_lossy()),
|
||||
Some(PathBuf::from("src").join("lib.rs"))
|
||||
);
|
||||
let outside = other.join("lib.rs");
|
||||
assert_eq!(
|
||||
workspace_relative_path(&workspace, &outside.to_string_lossy()),
|
||||
None
|
||||
);
|
||||
assert_eq!(workspace_relative_path(&workspace, "../escape"), None);
|
||||
assert_eq!(
|
||||
workspace_relative_path(&workspace, "src/../../escape"),
|
||||
None
|
||||
);
|
||||
assert_eq!(workspace_relative_path(&workspace, ""), None);
|
||||
// Whitespace and glob characters are literal filename bytes.
|
||||
assert_eq!(
|
||||
workspace_relative_path(&workspace, " padded.txt "),
|
||||
Some(PathBuf::from(" padded.txt "))
|
||||
);
|
||||
assert_eq!(
|
||||
workspace_relative_path(&workspace, "file[12].txt"),
|
||||
Some(PathBuf::from("file[12].txt"))
|
||||
);
|
||||
}
|
||||
|
||||
fn sha256_hash(path: &Path) -> String {
|
||||
format!(
|
||||
"sha256:{}",
|
||||
crate::hashing::sha256_hex(std::fs::read(path).unwrap())
|
||||
)
|
||||
}
|
||||
|
||||
/// The Git primitive treats `[12]` as a pattern even after `--`; the
|
||||
/// file-scoped restore must not. `file[12].txt` and `file1.txt` both exist
|
||||
/// in the snapshot, so only literal pathspecs keep the second one intact.
|
||||
#[test]
|
||||
fn restore_file_if_unchanged_treats_glob_characters_literally() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let (repo, _home) = make_repo(tmp.path());
|
||||
let literal = repo.work_tree().join("file[12].txt");
|
||||
let sibling = repo.work_tree().join("file1.txt");
|
||||
std::fs::write(&literal, b"literal-before").unwrap();
|
||||
std::fs::write(&sibling, b"sibling-before").unwrap();
|
||||
let id = repo.snapshot("tool:call-1").expect("snapshot");
|
||||
std::fs::write(&literal, b"literal-after").unwrap();
|
||||
std::fs::write(&sibling, b"sibling-after").unwrap();
|
||||
|
||||
assert!(
|
||||
repo.path_differs_from_snapshot(&id, Path::new("file[12].txt"))
|
||||
.unwrap()
|
||||
);
|
||||
let outcomes = repo
|
||||
.restore_file_if_unchanged(&id, Path::new("file[12].txt"), &sha256_hash(&literal))
|
||||
.expect("literal restore");
|
||||
assert_eq!(outcomes.len(), 1);
|
||||
assert_eq!(outcomes[0].action, PathRestoreAction::Modified);
|
||||
assert_eq!(std::fs::read_to_string(&literal).unwrap(), "literal-before");
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(&sibling).unwrap(),
|
||||
"sibling-after",
|
||||
"a bracketed filename must never restore its glob siblings"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restore_file_if_unchanged_refuses_when_the_reviewed_bytes_changed() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let (repo, _home) = make_repo(tmp.path());
|
||||
let file = repo.work_tree().join("a.txt");
|
||||
std::fs::write(&file, b"v1").unwrap();
|
||||
let id = repo.snapshot("pre-turn:1").expect("snapshot");
|
||||
std::fs::write(&file, b"v2").unwrap();
|
||||
let reviewed = sha256_hash(&file);
|
||||
// The user edits again after the client captured its change record.
|
||||
std::fs::write(&file, b"v3-user-edit").unwrap();
|
||||
|
||||
let err = repo
|
||||
.restore_file_if_unchanged(&id, Path::new("a.txt"), &reviewed)
|
||||
.expect_err("stale hash must refuse");
|
||||
assert_eq!(err.kind(), io::ErrorKind::WouldBlock);
|
||||
assert_eq!(std::fs::read_to_string(&file).unwrap(), "v3-user-edit");
|
||||
// `absent` is only valid for a file the client saw as deleted.
|
||||
let err = repo
|
||||
.restore_file_if_unchanged(&id, Path::new("a.txt"), "absent")
|
||||
.expect_err("absent must not match an existing file");
|
||||
assert_eq!(err.kind(), io::ErrorKind::WouldBlock);
|
||||
// The exact current bytes restore.
|
||||
let outcomes = repo
|
||||
.restore_file_if_unchanged(&id, Path::new("a.txt"), &sha256_hash(&file))
|
||||
.expect("current hash restores");
|
||||
assert_eq!(outcomes[0].action, PathRestoreAction::Modified);
|
||||
assert_eq!(std::fs::read_to_string(&file).unwrap(), "v1");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restore_file_if_unchanged_handles_deleted_and_created_files() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let (repo, _home) = make_repo(tmp.path());
|
||||
let deleted = repo.work_tree().join("deleted.txt");
|
||||
std::fs::write(&deleted, b"content").unwrap();
|
||||
let id = repo.snapshot("pre-turn:1").expect("snapshot");
|
||||
std::fs::remove_file(&deleted).unwrap();
|
||||
let created = repo.work_tree().join("created.txt");
|
||||
std::fs::write(&created, b"new").unwrap();
|
||||
|
||||
let outcomes = repo
|
||||
.restore_file_if_unchanged(&id, Path::new("deleted.txt"), "absent")
|
||||
.expect("recreate");
|
||||
assert_eq!(outcomes[0].action, PathRestoreAction::Recreated);
|
||||
assert_eq!(std::fs::read_to_string(&deleted).unwrap(), "content");
|
||||
|
||||
let outcomes = repo
|
||||
.restore_file_if_unchanged(&id, Path::new("created.txt"), &sha256_hash(&created))
|
||||
.expect("remove");
|
||||
assert_eq!(outcomes[0].action, PathRestoreAction::Removed);
|
||||
assert!(!created.exists());
|
||||
// A created file inside a new directory is removed alone; the
|
||||
// directory the user made stays.
|
||||
let nested_dir = repo.work_tree().join("newdir");
|
||||
std::fs::create_dir_all(&nested_dir).unwrap();
|
||||
let nested = nested_dir.join("only.txt");
|
||||
std::fs::write(&nested, b"n").unwrap();
|
||||
let outcomes = repo
|
||||
.restore_file_if_unchanged(&id, Path::new("newdir/only.txt"), &sha256_hash(&nested))
|
||||
.expect("remove nested");
|
||||
assert_eq!(outcomes[0].action, PathRestoreAction::Removed);
|
||||
assert!(!nested.exists());
|
||||
assert!(nested_dir.is_dir(), "the parent directory is not pruned");
|
||||
// A path missing on both sides is not a change and reports nothing.
|
||||
assert!(
|
||||
!repo
|
||||
.path_differs_from_snapshot(&id, Path::new("never.txt"))
|
||||
.unwrap()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restore_file_if_unchanged_refuses_directories_git_metadata_and_ignored_files() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let (repo, _home) = make_repo(tmp.path());
|
||||
let dir = repo.work_tree().join("src");
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
std::fs::write(dir.join("lib.rs"), b"fn a() {}").unwrap();
|
||||
std::fs::write(
|
||||
repo.work_tree().join(".gitignore"),
|
||||
"ignored.txt
|
||||
",
|
||||
)
|
||||
.unwrap();
|
||||
std::fs::write(repo.work_tree().join("ignored.txt"), b"secret").unwrap();
|
||||
let id = repo.snapshot("pre-turn:1").expect("snapshot");
|
||||
std::fs::write(dir.join("lib.rs"), b"fn b() {}").unwrap();
|
||||
|
||||
for rel in ["src", ".git/config", "src/.GIT/x", ".git"] {
|
||||
let err = repo.validate_restore_file(Path::new(rel)).expect_err(rel);
|
||||
assert_eq!(err.kind(), io::ErrorKind::InvalidInput, "{rel}");
|
||||
}
|
||||
let err = repo
|
||||
.restore_file_if_unchanged(&id, Path::new("src"), "absent")
|
||||
.expect_err("directories are refused");
|
||||
assert_eq!(err.kind(), io::ErrorKind::InvalidInput);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(dir.join("lib.rs")).unwrap(),
|
||||
"fn b() {}"
|
||||
);
|
||||
|
||||
// A gitignored file is excluded from the safety backup, so removing
|
||||
// it would be unrecoverable: refuse and leave it in place.
|
||||
let ignored = repo.work_tree().join("ignored.txt");
|
||||
let err = repo
|
||||
.restore_file_if_unchanged(&id, Path::new("ignored.txt"), &sha256_hash(&ignored))
|
||||
.expect_err("ignored files are refused");
|
||||
assert!(err.to_string().contains("safety snapshot"), "got: {err}");
|
||||
assert_eq!(std::fs::read_to_string(&ignored).unwrap(), "secret");
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn restore_file_if_unchanged_refuses_symlinks_anywhere_in_the_path() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let (repo, _home) = make_repo(tmp.path());
|
||||
let outside = tmp.path().join("outside");
|
||||
std::fs::create_dir_all(&outside).unwrap();
|
||||
std::fs::write(outside.join("target.txt"), b"outside").unwrap();
|
||||
std::fs::write(repo.work_tree().join("real.txt"), b"real").unwrap();
|
||||
std::os::unix::fs::symlink(&outside, repo.work_tree().join("linkdir")).unwrap();
|
||||
std::os::unix::fs::symlink(
|
||||
outside.join("target.txt"),
|
||||
repo.work_tree().join("link.txt"),
|
||||
)
|
||||
.unwrap();
|
||||
let id = repo.snapshot("pre-turn:1").expect("snapshot");
|
||||
|
||||
for rel in ["link.txt", "linkdir/target.txt"] {
|
||||
let err = repo
|
||||
.restore_file_if_unchanged(&id, Path::new(rel), "absent")
|
||||
.expect_err(rel);
|
||||
assert_eq!(err.kind(), io::ErrorKind::InvalidInput, "{rel}");
|
||||
}
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(outside.join("target.txt")).unwrap(),
|
||||
"outside"
|
||||
);
|
||||
// The snapshot side is checked too: a symlink entry in the tree is
|
||||
// not a regular file even when the work tree copy is gone.
|
||||
std::fs::remove_file(repo.work_tree().join("link.txt")).unwrap();
|
||||
let err = repo
|
||||
.restore_file_if_unchanged(&id, Path::new("link.txt"), "absent")
|
||||
.expect_err("snapshot symlink entry");
|
||||
assert_eq!(err.kind(), io::ErrorKind::InvalidInput);
|
||||
assert!(!repo.work_tree().join("link.txt").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn list_distinguishes_an_unborn_head_from_broken_history() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let (repo, _home) = make_repo(tmp.path());
|
||||
assert!(repo.list(10).expect("unborn HEAD lists nothing").is_empty());
|
||||
|
||||
std::fs::write(repo.work_tree().join("a.txt"), b"a").unwrap();
|
||||
repo.snapshot("pre-turn:1").expect("snapshot");
|
||||
assert_eq!(repo.list(10).unwrap().len(), 1);
|
||||
|
||||
// Point the branch at an object that does not exist: the history is
|
||||
// now broken, which must surface as an error rather than "no
|
||||
// snapshots" (an empty list would let patch-undo drop a turn).
|
||||
let head = String::from_utf8(
|
||||
run_git(repo.git_dir(), repo.work_tree(), &["symbolic-ref", "HEAD"])
|
||||
.unwrap()
|
||||
.stdout,
|
||||
)
|
||||
.unwrap();
|
||||
std::fs::write(
|
||||
repo.git_dir().join(head.trim()),
|
||||
"0123456789abcdef0123456789abcdef01234567\n",
|
||||
)
|
||||
.unwrap();
|
||||
let err = repo.list(10).expect_err("broken history must error");
|
||||
assert!(err.to_string().contains("git log failed"), "got: {err}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restore_takes_a_pre_restore_safety_snapshot_that_round_trips() {
|
||||
let tmp = tempdir().unwrap();
|
||||
|
||||
+103
-2
@@ -708,7 +708,8 @@ accept an empty string to clear a previously-set value. Added in v0.8.10 (#562):
|
||||
- `POST /v1/threads/{id}/turns/{turn_id}/interrupt`
|
||||
- `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
|
||||
- `POST /v1/threads/{id}/patch-undo` - snapshot-based file rollback followed by the same fork (`{"depth": N}`); returns `patch_result` (`files_restored`, `summary`, `snapshot_label`) alongside the forked thread
|
||||
- `POST /v1/threads/{id}/patch-undo` - snapshot-based whole-workspace rollback followed by 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 the trust, admission and abort rules.
|
||||
- `POST /v1/threads/{id}/file-revert` - restore exactly one file from one named snapshot (`{"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)
|
||||
|
||||
`POST /v1/threads/{id}/turns` accepts the same optional
|
||||
@@ -895,7 +896,11 @@ the first returned event advances past exactly the omitted history.
|
||||
`/v1/snapshots` lists recent side-git restore points for the runtime workspace.
|
||||
`limit` defaults to `20` and must be between `1` and `100`. `POST
|
||||
/v1/snapshots/{id}/restore` restores workspace files from the snapshot and
|
||||
returns `{"restored": "<snapshot-id>"}`.
|
||||
returns `{"restored": "<snapshot-id>"}`. It is the direct operator surface for
|
||||
the server's own workspace (the same action as the TUI's `/restore <N>`): it is
|
||||
gated by the Runtime API bearer token, not by any thread's trust flag, and it
|
||||
is refused with `409` while a turn is active in an overlapping workspace (see
|
||||
below). A `pre-restore:` safety snapshot is taken first.
|
||||
|
||||
```json
|
||||
[
|
||||
@@ -907,6 +912,102 @@ returns `{"restored": "<snapshot-id>"}`.
|
||||
]
|
||||
```
|
||||
|
||||
### Workspace restore endpoints
|
||||
|
||||
Three routes mutate workspace files from side-git snapshots. They share one
|
||||
admission rule and one safety net, and they differ in scope and trust.
|
||||
|
||||
| Route | Scope | Trust | Forks the thread |
|
||||
| --- | --- | --- | --- |
|
||||
| `POST /v1/snapshots/{id}/restore` | whole server workspace | bearer token only (operator action) | no |
|
||||
| `POST /v1/threads/{id}/patch-undo` | whole thread workspace | thread `trust_mode` or `auto_approve` when files would change | yes |
|
||||
| `POST /v1/threads/{id}/file-revert` | exactly one regular file | thread `trust_mode` or `auto_approve`, always | no |
|
||||
|
||||
**Admission.** A restore reserves the same admission the Runtime uses for
|
||||
config reloads and session checkpoints, so no new turn starts and no saved
|
||||
history changes while files are being rewritten. If any thread already has an
|
||||
active turn in the same workspace, a nested checkout of it, or a parent of it,
|
||||
the request is refused with `409` and the message `already has an active turn`.
|
||||
The reservation is owned by the worker performing the Git mutation, so a client
|
||||
that disconnects mid-request cannot release it early; the operation completes
|
||||
or fails as a whole. Concurrent restores serialize. The reservation is
|
||||
runtime-wide: while a restore's safety snapshot and checkout run, new turns,
|
||||
steering, compaction and user-input delivery on every thread wait for it to
|
||||
finish, so a large workspace can add seconds of latency elsewhere during a
|
||||
restore. A thread whose workspace directory is not available (unmounted
|
||||
volume, disconnected share, missing directory) is refused with `409` rather
|
||||
than treated as having nothing to restore.
|
||||
|
||||
**Safety net.** Every restore first records a `pre-restore:<target>` snapshot
|
||||
of the current workspace. That label is never a `/undo`, `patch-undo` or
|
||||
`file-revert` candidate, so the net does not change what later undos select.
|
||||
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`.** Selects the newest `tool:`/`pre-turn:` snapshot owned by the
|
||||
thread's own session whose tree differs from the workspace, restores the whole
|
||||
tree from it, then forks the conversation exactly as `/undo` does. `Ok` means
|
||||
either files were restored or there was provably nothing to restore (no bound
|
||||
session, or no differing session-owned snapshot); `files_restored` says which.
|
||||
When there is something to restore and the thread is not trusted, the whole
|
||||
undo aborts with `409` and neither files nor conversation change. Snapshot
|
||||
repository, listing or comparison failures abort with `500`, and an unavailable
|
||||
workspace directory aborts with `409`; both preserve the conversation, so a
|
||||
turn is never dropped while its file changes stay on disk.
|
||||
Depth and history are validated before any file changes. If the fork cannot be
|
||||
persisted after files were restored, the response is a `500` that names the
|
||||
restored snapshot; the original thread still holds the turn and the
|
||||
`pre-restore:` snapshot holds the previous files.
|
||||
|
||||
**`file-revert`.** Request body:
|
||||
|
||||
```json
|
||||
{
|
||||
"path": "src/lib.rs",
|
||||
"snapshot_id": "3f2a…40-or-64 hex…",
|
||||
"expected_hash": "sha256:<64 lowercase hex digits>"
|
||||
}
|
||||
```
|
||||
|
||||
- `path`: workspace-relative, or absolute inside the thread workspace. The
|
||||
name is literal (brackets, spaces and glob characters are filename bytes;
|
||||
Git runs with `--literal-pathspecs`). It must name a regular file: directories,
|
||||
symlinks anywhere in the path, and `.git` components are `400`.
|
||||
- `snapshot_id`: the exact `tool:<call_id>` or `pre-turn:<n>` snapshot from the
|
||||
change the user selected. Clients obtain ids from `GET /v1/snapshots` (labels
|
||||
carry the tool call id) and must keep the selected change's identity; the
|
||||
server never picks "the newest snapshot that differs", because an unrelated
|
||||
newer snapshot can erase later user edits while leaving the tool's change.
|
||||
- `expected_hash`: `sha256:` of the current file bytes the client displayed,
|
||||
or `absent` when the client saw the file as deleted. It is checked before the
|
||||
safety backup and again immediately before the mutation.
|
||||
|
||||
Responses:
|
||||
|
||||
- `200 {"path", "action", "snapshot_id", "snapshot_label"}` — `action` is
|
||||
`modified`, `recreated` (file was missing) or `removed` (the snapshot does
|
||||
not contain the file, so the file the tool created is deleted; its parent
|
||||
directories are left in place).
|
||||
- `400`: malformed `snapshot_id`/`expected_hash`, path outside the workspace,
|
||||
or a path that is not a regular file on either side.
|
||||
- `404`: unknown thread.
|
||||
- `409`: thread not in trusted mode or Full Access; no bound session; active
|
||||
turn in an overlapping workspace; workspace directory not available;
|
||||
snapshot unknown, owned by another session
|
||||
or not a restore point (refresh the change record); file already matches the
|
||||
snapshot (nothing to revert); or the file changed after the reviewed
|
||||
`expected_hash` (refresh and review again). Nothing is changed in any of
|
||||
these cases.
|
||||
- `422`: missing or mistyped body fields.
|
||||
- `500`: Git or filesystem failure; a failure after the safety snapshot names
|
||||
that snapshot so the previous bytes can be recovered with
|
||||
`POST /v1/snapshots/{id}/restore` or `/restore`.
|
||||
|
||||
Capability probe: `GET` on the route returns `405` where the endpoint exists
|
||||
and `404` on an older engine; clients treat any non-`404` as available and
|
||||
degrade with an explanation otherwise.
|
||||
|
||||
**Receipts** (future read-only audit export)
|
||||
- Proposed only: `GET /v1/threads/{thread_id}/turns/{turn_id}/receipt`
|
||||
|
||||
|
||||
Reference in new issue
Block a user