Skip to content

Commit dda4151

Browse files
tonyfettesclaude
andcommitted
fix(desktop): keep the atomic-save retry off non-file targets
The retry after a failed rename exists for Windows, which may refuse to replace an existing file. It removed the target unconditionally through `remove_if_exists`, which deletes a directory recursively — so a path occupied by a directory (`.openseek/worktrees.json/`, say) was erased along with everything under it, and the write then reported success. This was inherited from `write_settings_file`, but promoting it into a shared helper spread it to the workspace and worktree registries. Restrict the retry to a regular file at the target and re-raise the original rename failure otherwise, so a wrong path is reported rather than cleared. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent de080ae commit dda4151

1 file changed

Lines changed: 30 additions & 2 deletions

File tree

desktop/internal/fsx/lock.mbt

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,18 @@ pub async fn write_text_atomic(
9797
)
9898
@fs.rename(temp.to_string(), target) catch {
9999
error if @async.is_being_cancelled() => raise error
100-
_ => {
101-
remove_if_exists(path)
100+
rename_failure => {
101+
// The retry exists only for the Windows refusal above, which is a
102+
// regular file standing in the way. Anything else at the target is a
103+
// different failure and must surface as one: a directory there means
104+
// the path is wrong, and removing it would recursively destroy whatever
105+
// it holds rather than report that.
106+
let replaceable = (@fs.kind(target) is Regular) catch {
107+
error if @async.is_being_cancelled() => raise error
108+
_ => false
109+
}
110+
guard replaceable else { raise rename_failure }
111+
@fs.remove(target)
102112
@fs.rename(temp.to_string(), target)
103113
}
104114
}
@@ -166,6 +176,24 @@ async test "shared locks admit each other" {
166176
@fs.rmdir(root.to_string(), recursive=true)
167177
}
168178

179+
///|
180+
/// The Windows retry must not turn "something else already occupies this
181+
/// path" into a recursive delete of whatever that something holds.
182+
async test "an atomic save refuses a target that is not a regular file" {
183+
let root : @pathx.Path = @fs.tmpdir(prefix="openseek-fsx-kind-")
184+
let occupied = root.join("registry.json")
185+
ensure_dir(occupied)
186+
let kept = occupied.join("keep.txt")
187+
write_text_atomic(kept, "precious")
188+
let mut refused = false
189+
write_text_atomic(occupied, "clobber") catch {
190+
_ => refused = true
191+
}
192+
assert_true(refused)
193+
assert_eq(read_text(kept), "precious")
194+
@fs.rmdir(root.to_string(), recursive=true)
195+
}
196+
169197
///|
170198
/// A released lock must leave nothing behind that keeps the next acquire out
171199
/// — the failure this catches is `release` closing the descriptor without

0 commit comments

Comments
 (0)