From 9ab6686b98499c745024a474a71e3d99b6e14973 Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Thu, 17 Sep 2026 13:58:25 +0200 Subject: a chain that deletes itself still gives back the file it displaced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (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 --- internal/journal/read.go | 19 +++++++++++++++-- internal/journal/read_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) (limited to 'internal/journal') diff --git a/internal/journal/read.go b/internal/journal/read.go index aa79118..a41cd8a 100644 --- a/internal/journal/read.go +++ b/internal/journal/read.go @@ -236,6 +236,7 @@ func Runs(path string, n int) ([]Run, error) { type fileOf struct{ run, dir, file string } reversibleOf := map[fileOf]int{} deletedFile := map[fileOf]bool{} + displacedOf := map[fileOf]int{} for _, line := range lines { e, ok := parseLine(line) @@ -259,6 +260,13 @@ func Runs(path string, n int) ([]Run, error) { if e.Action == "delete" { deletedFile[fileOf{e.Run, e.Dir, e.File}] = true } + if e.Action == "displace" { + // Counted apart from the rest of its file: a permanent + // delete ends the file it deleted, but the file that chain + // trashed to make room is a different file and can still + // come back. + displacedOf[fileOf{e.Run, e.Dir, e.File}]++ + } } if e.Action == "run-start" { if orig, ok := strings.CutPrefix(e.Detail, undoOfPrefix); ok && orig != "" { @@ -298,9 +306,16 @@ func Runs(path string, n int) ([]Run, error) { if r.Undone { took := 0 for f, n := range reversibleOf { - if f.run == r.ID && !deletedFile[f] { - took += n + if f.run != r.ID { + continue + } + if deletedFile[f] { + // Nothing of a permanently deleted file comes back + // except what it displaced. + took += displacedOf[f] + continue } + took += n } r.PartlyUndone = reversed[r.ID] < took } diff --git a/internal/journal/read_test.go b/internal/journal/read_test.go index d2bbd3a..dd202a4 100644 --- a/internal/journal/read_test.go +++ b/internal/journal/read_test.go @@ -808,3 +808,52 @@ func TestRunsIgnoresAPermanentlyDeletedFilesSteps(t *testing.T) { } } } + +// TestPermanentDeleteDoesNotExcuseItsDisplace: a chain that overwrote and +// then permanently deleted trashed a file the user owned. That file can +// come back, so until it does the run is only partly undone - subtracting +// every reversible step of a permanently deleted file, its displace +// included, made krino report the run fully undone while the user's file +// was still in the Trash. +func TestPermanentDeleteDoesNotExcuseItsDisplace(t *testing.T) { + path := filepath.Join(t.TempDir(), "krino.log") + w, _ := Open(path) + now := time.Now() + w.Append(Entry{Time: now, Run: "A", Action: "run-start", Status: "ok"}) + w.Append(Entry{Time: now, Run: "A", Dir: "dl", File: "a.pdf", Step: 1, + Action: "displace", Status: "ok", Src: "/archive/a.pdf", Dst: "/trash/a.pdf", Detail: "a.pdf"}) + w.Append(Entry{Time: now, Run: "A", Dir: "dl", File: "a.pdf", Step: 1, + Action: "move", Status: "ok", Src: "/dl/a.pdf", Dst: "/archive/a.pdf"}) + w.Append(Entry{Time: now, Run: "A", Dir: "dl", File: "a.pdf", Step: 2, + Action: "delete", Status: "ok", Src: "/archive/a.pdf"}) + // A second file the same run moved, which undo does put back. + w.Append(Entry{Time: now, Run: "A", Dir: "dl", File: "b.txt", Step: 1, + Action: "move", Status: "ok", Src: "/dl/b.txt", Dst: "/dl/Text/b.txt"}) + w.Append(Entry{Time: now, Run: "A", Action: "run-end", Status: "ok"}) + w.Append(Entry{Time: now.Add(time.Hour), Run: "B", Action: "run-start", Status: "ok", + Detail: undoOfPrefix + "A"}) + w.Append(Entry{Time: now.Add(time.Hour), Run: "B", Dir: "dl", File: "b.txt", Step: 1, + Action: "undo-move", Status: "ok", Src: "/dl/Text/b.txt", Dst: "/dl/b.txt"}) + w.Append(Entry{Time: now.Add(time.Hour), Run: "B", Action: "run-end", Status: "ok"}) + w.Close() + + runs, err := Runs(path, 0) + if err != nil { + t.Fatal(err) + } + var orig *Run + for i := range runs { + if runs[i].ID == "A" { + orig = &runs[i] + } + } + if orig == nil { + t.Fatal("the original run is not in the log") + } + if !orig.Undone { + t.Fatal("the run is not marked undone at all") + } + if !orig.PartlyUndone { + t.Error("a run whose displaced file is still in the Trash reads as fully undone") + } +} -- cgit v1.3