fix(editors): stage external edits per-remote-file to avoid clobbering (#76)#77
Merged
Merged
Conversation
#76) Edit-in-editor downloaded every remote file to a flat `temp/anyscp-edit/<filename>`. Two remote files sharing a basename (e.g. `a/compose.yml` and `b/compose.yml`) therefore mapped to the same local path, so with both open in an editor, saving one re-uploaded its contents to the other's remote path. Add `editors::edit_temp_path(key, file_name)`, which namespaces each download under `anyscp-edit/<hash>/<filename>` where `<hash>` is a stable hash of the remote identity (session id + remote path / bucket + key). Wire it into the SFTP, SCP, and S3 edit_external commands, and remove the now-empty staging dir alongside the temp file on cleanup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The per-file hash dir from #76 left two gaps: two distinct keys could collide in the 64-bit group hash and share a file, and a second edit of the *same* remote file mapped to one local path, so one save-watcher's cleanup could delete the file out from under another live editor. Add a per-invocation UUID layer, staging edits at `anyscp-edit/<group>/<unique>/<file>`. The UUID isolates every edit session, closing both gaps; the group hash is kept only so edits of one remote file share a readable dir. Centralize teardown in `edit_temp_cleanup`, which prunes empty per-edit dirs but stops at (and never removes) the `anyscp-edit` root, so a concurrent edit's still-live staging is left intact.
a70ce10 to
60f5f16
Compare
The ModalShell refactor (785b76f) moved HostEditModal's root into the shared shell, which exposes only data-testid — so the data-host-modal-mode attribute the Cmd+T e2e test reads disappeared and the test has been red on main since. Add an optional dataAttributes pass-through to ModalShell and set data-host-modal-mode={new|edit} from HostEditModal.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Refs #76.
"Edit in editor" downloaded every remote file to a flat
temp/anyscp-edit/<filename>. Two remote files sharing a basename —a/compose.ymlandb/compose.yml— therefore mapped to the same local path. With both open in an editor, saving one re-uploaded its contents to the other's remote path, silently overwriting it. The flaw was duplicated across all three transports:sftp_edit_external,scp_edit_external, ands3_edit_external.Fix
Namespace every edit session under a unique staging directory:
<group>— a stableDefaultHasherdigest ofkey(session_id + "\0" + remote_path/key), so all edits of one remote file share a readable group dir. The same path on different servers also stays separate.<unique>— a fresh UUID per call. This isolates every edit session, which closes two gaps a per-file hash alone couldn't:Teardown is centralized in a shared
edit_temp_cleanup(local_path)(replacing the three duplicated cleanup blocks). It removes the file and prunes empty per-edit dirs, walking up but stopping at — and never removing — theanyscp-editroot. A still-live concurrent edit's staging dir is non-empty, so the walk halts there and leaves it intact.Implementation
editors/mod.rs—edit_temp_path(key, file_name)now appends the per-invocation UUID; newedit_temp_cleanup(path)helper.key, create the per-edit parent dir, and calledit_temp_cleanupon teardown.Trade-off
This intentionally drops the determinism the first cut had (same key → same path) — that property was the cause of the concurrent-edit race, and nothing relied on it (each caller captures the returned
local_pathdirectly and never reconstructs it).Testing
cargo test --lib editors::— 10 passed, including:edit_temp_path_separates_same_basename— distinct dirs even for the same key, while sharing a group dir.edit_temp_cleanup_prunes_dirs_but_keeps_root— pruning stops at the root.edit_temp_cleanup_halts_at_nonempty_group— cleaning up edit A doesn't touch concurrent edit B's live file.cargo check --lib— clean.