aboutsummaryrefslogtreecommitdiff
path: root/internal/engine
diff options
context:
space:
mode:
authorLukasz Kasprzak <lukas@labunix.xyz>2026-09-17 13:58:25 +0200
committerLukasz Kasprzak <lukas@labunix.xyz>2026-09-17 13:58:25 +0200
commit9ab6686b98499c745024a474a71e3d99b6e14973 (patch)
tree1debf57412d72c6289dd06bd17f0c024a490a3af /internal/engine
parenta80d470cbad7335fd5e534f32d935e5a49aadb24 (diff)
downloadkrino-9ab6686b98499c745024a474a71e3d99b6e14973.tar.gz
krino-9ab6686b98499c745024a474a71e3d99b6e14973.zip
a chain that deletes itself still gives back the file it displaced
(on-conflict overwrite) trashes the file in the way; §7.4 promises undo restores it. Walking a file's log entries stopped dead at a permanent delete, so the displace written earlier in the same chain was never reached: the user's file stayed in the Trash, the refusal named only the file they did not care about, and krino log called the run undone. The displaced file is a different file, so it is offered as its own entry in the undo plan, keyed by its own path - the deleted file stays refused, since nothing of it can come back, and the copy or move that preceded the delete stays unreversed too (undoing a copy whose original was then deleted would destroy the last remaining copy). The accounting matched: every reversible step of a deleted file was subtracted, its displace included, so the run read (undone). Only what genuinely cannot come back is subtracted now. End to end, the scenario from the review: the only copy of a file is displaced by an incoming one that is then permanently deleted. before: archive/ empty, "(undone)", nothing offered after: archive/a.pdf restored, run reads partly undone
Diffstat (limited to 'internal/engine')
-rw-r--r--internal/engine/apply.go61
-rw-r--r--internal/engine/apply_test.go79
2 files changed, 140 insertions, 0 deletions
diff --git a/internal/engine/apply.go b/internal/engine/apply.go
index e2d4db8..f54169c 100644
--- a/internal/engine/apply.go
+++ b/internal/engine/apply.go
@@ -344,6 +344,18 @@ type UndoFile struct {
Declined bool
}
+// Label names the file for display: the directory it belongs to and the
+// path within it, except for a file that lies outside any sorted directory
+// - one displaced into the Trash by a chain that then deleted itself - whose
+// path is absolute and stands alone. Dir stays set on those either way: it
+// is what the undo locks.
+func (uf UndoFile) Label() string {
+ if filepath.IsAbs(uf.File) {
+ return xdg.Abbrev(uf.File)
+ }
+ return uf.Dir + "/" + uf.File
+}
+
// UndoStep is the reversal of one logged step.
type UndoStep struct {
Original journal.Entry // the step being reversed
@@ -420,6 +432,13 @@ func (e *Engine) PlanUndo(runID string) (*UndoPlan, error) {
if len(uf.Steps) == 0 && uf.Refused == "" {
continue
}
+ // A file refused outright still displaced someone else's file into
+ // the Trash, and that file is a different file - §7.4 promises undo
+ // restores it. Offered on its own, since the refusal is about the
+ // file that cannot come back, not about this one.
+ if rest, ok := displacedUndoFile(k.dir, byFile[k], reversed); ok {
+ up.Files = append(up.Files, rest)
+ }
if uf.Refused == "" && onlyOccupiedDirectoryRemovals(uf.Steps) {
// Nothing of the file itself is left to reverse, only directories
// the run made that something else still occupies: offering them
@@ -585,6 +604,48 @@ func planUndoFile(dir, file string, ents []journal.Entry, reversed map[journal.R
return uf
}
+// displacedUndoFile offers the file a refused chain trashed to make room.
+// It applies only to a chain planUndoFile refuses as a whole - a reversible
+// chain reverses its own displace as one of its steps - and only to a
+// refusal the displaced file is not itself the cause of: a permanent
+// delete, which ends the deleted file for good while leaving the file it
+// displaced sitting in the Trash, recoverable, with its original path now
+// free.
+//
+// It is keyed by the displaced path rather than the chain's file, because
+// that is what it puts back; a plan that named the chain's file twice would
+// read as one file being reversed in two places.
+func displacedUndoFile(dir string, ents []journal.Entry, reversed map[journal.ReversedKey]int) (UndoFile, bool) {
+ deleted := false
+ for _, en := range ents {
+ if en.Status == "ok" && en.Action == "delete" {
+ deleted = true
+ }
+ }
+ if !deleted {
+ return UndoFile{}, false
+ }
+ var out UndoFile
+ for i := len(ents) - 1; i >= 0; i-- {
+ en := ents[i]
+ if en.Status != "ok" || en.Action != "displace" {
+ continue
+ }
+ step := reverseStep(en)
+ if k := (journal.ReversedKey{Dir: dir, File: en.File, Action: step.Action, Src: step.Src}); reversed[k] > 0 {
+ reversed[k]--
+ continue
+ }
+ out.Dir = dir
+ out.File = en.Src
+ out.Steps = append(out.Steps, step)
+ if step.Refused != "" && out.Refused == "" {
+ out.Refused = step.Refused
+ }
+ }
+ return out, len(out.Steps) > 0
+}
+
// undoProjection tracks what the reversal steps planUndoFile has already
// queued (in the order they will execute) will do to the filesystem, so a
// later step's occupancy check can tell a real, external occupant from a
diff --git a/internal/engine/apply_test.go b/internal/engine/apply_test.go
index a6b3185..c832e3a 100644
--- a/internal/engine/apply_test.go
+++ b/internal/engine/apply_test.go
@@ -1463,3 +1463,82 @@ func TestApplyUndoRetryLogsBothMkdirEntriesAndStillMarksOriginalRunUndone(t *tes
t.Errorf("the undo run %q itself must never read as Undone", undoRun)
}
}
+
+// TestPermanentDeleteStillRestoresWhatItDisplaced: a chain that overwrites
+// and then permanently deletes trashes a file the user owned to make room.
+// That file is a different file, and §7.4 promises of it: "move the
+// existing target to Trash first (logged, so undo restores it)". Walking
+// the file's entries used to stop dead at the permanent delete, so the
+// displace was never reached and the user's file stayed in the Trash with
+// krino reporting the run fully undone.
+func TestPermanentDeleteStillRestoresWhatItDisplaced(t *testing.T) {
+ // A real trash entry, so the reversal is offered rather than refused
+ // for a reason that has nothing to do with this test.
+ h := sandbox(t)
+ entry := filepath.Join(trash.Dir(), "files", "a.pdf")
+ if err := os.MkdirAll(filepath.Dir(entry), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(entry, []byte("the file that was in the way"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ fi, err := os.Lstat(entry)
+ if err != nil {
+ t.Fatal(err)
+ }
+ displaced := filepath.Join(h, "archive", "a.pdf")
+ info := filepath.Join(trash.Dir(), "info", "a.pdf.trashinfo")
+ if err := os.MkdirAll(filepath.Dir(info), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(info, []byte("[Trash Info]\nPath="+displaced+"\nDeletionDate=2026-09-17T00:00:00\n"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ ents := []journal.Entry{
+ // Chronological, as the log has them: the displace first, then the
+ // move that needed the name, then the permanent delete.
+ {Action: "displace", Status: "ok", File: "a.pdf", Dir: "dl", Step: 1,
+ Src: displaced, Dst: entry, Detail: "a.pdf",
+ Size: fi.Size(), ModTime: fi.ModTime()},
+ {Action: "move", Status: "ok", File: "a.pdf", Dir: "dl", Step: 1,
+ Src: filepath.Join(h, "dl", "a.pdf"), Dst: displaced},
+ {Action: "delete", Status: "ok", File: "a.pdf", Dir: "dl", Step: 2,
+ Src: displaced},
+ }
+ uf := planUndoFile("dl", "a.pdf", ents, map[journal.ReversedKey]int{})
+ if uf.Refused == "" {
+ t.Error("the permanently deleted file is no longer refused")
+ }
+
+ rest, ok := displacedUndoFile("dl", ents, map[journal.ReversedKey]int{})
+ if !ok {
+ t.Fatal("the displaced file was not offered for reversal at all")
+ }
+ if rest.Refused != "" {
+ t.Errorf("the displaced file is refused: %q", rest.Refused)
+ }
+ if len(rest.Steps) != 1 || rest.Steps[0].Action != "undo-displace" {
+ t.Fatalf("steps = %+v; want one undo-displace", rest.Steps)
+ }
+ if rest.Steps[0].Dst != displaced {
+ t.Errorf("the reversal puts the file at %q, want %q", rest.Steps[0].Dst, displaced)
+ }
+ if rest.File != displaced {
+ t.Errorf("the offered file is %q, want the displaced file %q", rest.File, displaced)
+ }
+}
+
+// TestDisplacedFileIsNotOfferedTwice: when the chain's own file is
+// reversible, the displace is reversed as one of its steps, as before -
+// the separate offer exists only for the file that cannot be reversed.
+func TestDisplacedFileIsNotOfferedTwice(t *testing.T) {
+ ents := []journal.Entry{
+ {Action: "displace", Status: "ok", File: "a.pdf", Dir: "dl", Step: 1,
+ Src: "/archive/a.pdf", Dst: "/trash/files/a.pdf", Detail: "a.pdf"},
+ {Action: "move", Status: "ok", File: "a.pdf", Dir: "dl", Step: 1,
+ Src: "/dl/a.pdf", Dst: "/archive/a.pdf"},
+ }
+ if _, ok := displacedUndoFile("dl", ents, map[journal.ReversedKey]int{}); ok {
+ t.Error("a reversible chain's displace was offered a second time on its own")
+ }
+}