// SPDX-License-Identifier: GPL-3.0-or-later package engine import ( "context" "errors" "fmt" "io" "os" "path/filepath" "sort" "strings" "syscall" "time" "krino/internal/apply" "krino/internal/journal" "krino/internal/plan" "krino/internal/scan" "krino/internal/trash" "krino/internal/xdg" ) // ApplyResult is what one Apply or ApplyUndo call did. type ApplyResult struct { // Dir is the directory the plan came from. ApplyUndo leaves it nil: one // run's undo can span several directories (each UndoFile carries its // own Dir name), so there is no single *Dir to attach here the way // Apply's caller already holds one via its DirPlan. Dir *Dir Files []FileResult Applied int // files with at least one step that ran Failed int // files with at least one failed step Declined int } // FileResult is one file's outcome within an ApplyResult. type FileResult struct { File scan.File Steps []apply.StepResult } // Apply carries out dp's plan and logs every event: run-start before the // first file, run-end after the last, and one entry per step (spec §9). // approved names the files to act on by Chain.File.Rel; a chain that is not // named is left alone but still logged, one entry per step, status // "declined" — spec §9 says declined files are logged even though nothing // happens to them. // // ctx is checked between files, never within one: apply.Chain has no ctx // parameter and always runs a whole file's chain synchronously, so a file // already underway always finishes and is logged before Apply looks at ctx // again (spec §11 — Ctrl-C finishes the current step, logs it, and stops). // On cancellation, Apply returns what it did so far together with ctx.Err() // and never writes run-end: the log is left exactly like the crashed-run // shape journal.Entries already knows how to read back (run-start, no // run-end), which is what makes an interrupted run still undoable. func (e *Engine) Apply(ctx context.Context, dp *DirPlan, approved map[string]bool, j *journal.Writer, run string) (*ApplyResult, error) { result := &ApplyResult{Dir: dp.Dir} var actionable []plan.Chain for _, c := range dp.Chains { if len(c.Steps) > 0 { actionable = append(actionable, c) } } if len(actionable) == 0 { return result, nil } if err := ctx.Err(); err != nil { return result, err } if err := j.Append(journal.Entry{Time: e.Now(), Run: run, Dir: dp.Dir.Name, Action: "run-start", Status: "ok"}); err != nil { return result, fmt.Errorf("engine: apply: %w", err) } for _, c := range actionable { if err := ctx.Err(); err != nil { return result, err } fr, err := e.applyFile(ctx, dp.Dir.Name, c, approved[c.File.Rel], j, run) if err != nil { return result, fmt.Errorf("engine: apply: %w", err) } result.Files = append(result.Files, fr) tallyFile(result, fr.Steps, nil) // every forward action is file-affecting } if err := j.Append(journal.Entry{Time: e.Now(), Run: run, Dir: dp.Dir.Name, Action: "run-end", Status: "ok"}); err != nil { return result, fmt.Errorf("engine: apply: %w", err) } return result, nil } // applyFile carries out (or declines) one file's chain and logs it. func (e *Engine) applyFile(ctx context.Context, dirName string, c plan.Chain, approved bool, j *journal.Writer, run string) (FileResult, error) { rel := c.File.Rel if !approved { steps := make([]apply.StepResult, len(c.Steps)) for i, step := range c.Steps { sr := apply.StepResult{Step: step, Status: "declined"} steps[i] = sr if err := e.logStep(j, run, dirName, rel, i+1, step, sr); err != nil { return FileResult{}, err } } return FileResult{File: c.File, Steps: steps}, nil } // Each step is logged the moment it has run (review M9), not after the // whole chain: a run killed mid-chain must leave what it did undoable. results, err := apply.ChainLogged(ctx, c, func(i int, sr apply.StepResult) error { if err := e.logStep(j, run, dirName, rel, i+1, c.Steps[i], sr); err != nil { return unloggedStep(rel, c.Steps[i], sr, err) } return nil }) if err != nil { return FileResult{}, err } return FileResult{File: c.File, Steps: results}, nil } // unloggedStep is the error for a step whose log entry could not be written // (re-review N1). A step that ran is named with where its file is now: // undo cannot see it, so the user must be told where to look. func unloggedStep(rel string, step plan.Step, sr apply.StepResult, err error) error { if sr.Status != "ok" { return fmt.Errorf("%s: step %s (%s) could not be logged: %w", rel, actionName(step.Kind), sr.Status, err) } var where string switch { case step.Kind == plan.Copy: where = "a copy is at " + xdg.Abbrev(sr.Dst) case sr.Dst != "": where = "the file is now at " + xdg.Abbrev(sr.Dst) default: where = "the file is deleted for good" } return fmt.Errorf("%s: %s ran but could not be logged, so undo cannot see it (%s): %w", rel, actionName(step.Kind), where, err) } // tallyFile updates result's Applied/Failed/Declined counters from one // file's step outcomes. The three are not mutually exclusive: a chain that // ran one step ok and then failed on the next counts toward both Applied // and Failed, matching each field's own "at least one step" definition. // // failureCounts, when non-nil, is asked before letting a "failed" status at // index i count toward Failed. This is undo-mkdir's exemption (fix round 2, // item 3): Task 7 maps ApplyResult to krino undo's exit code, and a file // whose only failure is an undo-mkdir it correctly declined to remove (a // shared directory not yet empty - not a hazard, see planUndoFile's and // undoFile's comments) must not make the whole run look failed. The forward // path passes nil: every one of its actions is file-affecting, so every // failure counts. func tallyFile(result *ApplyResult, steps []apply.StepResult, failureCounts func(i int) bool) { var ok, failed, declined bool for i, sr := range steps { switch sr.Status { case "ok": ok = true case "failed": if failureCounts == nil || failureCounts(i) { failed = true } case "declined": declined = true } } // Fix wave item 4 / Minor 6: a file every one of whose steps came back // "skipped" - the shape an approved all-skipped chain used to take, one // step for each rule action but every step's own Skip already set - // left none of ok/failed/declined true above, so it fell out of the // outcome tally entirely: "0 applied · 0 failed · 0 declined" for a // file the user was asked about and approved. The converged // actionableChains/countActing definition (cmd/krino, same fix wave // item) keeps such a chain from ever reaching here approved in the // first place, but tallyFile is the shared invariant, not a guarantee // upheld only by that one caller: every file it is given must land in // exactly one of the three buckets. Nothing ran and nothing failed, // which is what "declined" already means to this tally, so an // otherwise-uncounted file lands there. if !ok && !failed && !declined && len(steps) > 0 { declined = true } if ok { result.Applied++ } if failed { result.Failed++ } if declined { result.Declined++ } } // actionName is the log's action vocabulary (spec §9) for a plan.Kind. // Trash and DeletePermanent share one Go type (plan.Kind) but two different // words in the log: "trash" is recoverable (goes to the Trash), "delete" is // not. func actionName(k plan.Kind) string { switch k { case plan.Copy: return "copy" case plan.Move: return "move" case plan.Rename: return "rename" case plan.Trash: return "trash" case plan.DeletePermanent: return "delete" } panic(fmt.Sprintf("engine: unknown plan.Kind %d", int(k))) } // logStep writes every journal line one step produces: a "displace" entry // when the step trashed a file that was in its way (StepResult.DisplacedEntry // is the only place that trash entry name exists — Task 3's ruling), a // "mkdir" entry per directory the step actually created (outermost first, so // undo can remove them innermost first), and finally the step's own entry. // All three share stepNum, the step's 1-based position in the chain, so a // reader can see which step of the plan a mkdir or displace line belongs to; // PlanUndo does not rely on that number, only on log order and File. // // Size and ModTime on the primary entry describe the file at Dst after the // step (spec §9) only when the step actually ran (Status "ok"): sr.Dst is // where the file really ended up (conflict resolution can rename it at // execution time), and sr.Size/sr.ModTime are read from there. For every // other status nothing happened at a destination, so Dst falls back to the // step's planned destination (informational only — PlanUndo never reverses // a non-"ok" entry) and Size/ModTime stay zero. func (e *Engine) logStep(j *journal.Writer, run, dirName, rel string, stepNum int, step plan.Step, sr apply.StepResult) error { if sr.DisplacedEntry != "" { dst := filepath.Join(trash.Dir(), "files", sr.DisplacedEntry) size, mtime := statSizeModTime(dst) if err := j.Append(journal.Entry{ Time: e.Now(), Run: run, Dir: dirName, File: rel, Step: stepNum, Action: "displace", Status: "ok", Rule: step.Rule, // Detail carries the trash entry name explicitly (fix round 1, // item 3): Dst's shape (trash.Dir()/files/) is // internal/apply's and this file's own convention, not a // contract undo may quietly depend on. PlanUndo/ApplyUndo read // the name from here, never by taking Dst's basename. Src: step.Displaces, Dst: dst, Size: size, ModTime: mtime, Detail: sr.DisplacedEntry, }); err != nil { return err } } for _, dir := range sr.Made { size, mtime := statSizeModTime(dir) if err := j.Append(journal.Entry{ Time: e.Now(), Run: run, Dir: dirName, File: rel, Step: stepNum, Action: "mkdir", Status: "ok", Rule: step.Rule, Dst: dir, Size: size, ModTime: mtime, }); err != nil { return err } } dst := step.Dst var size int64 var mtime time.Time detail := sr.Detail if sr.Status == "ok" { dst, size, mtime = sr.Dst, sr.Size, sr.ModTime if step.Kind == plan.Trash { // Same reasoning as the displace entry above: sr.Detail is // always empty on a successful trash (apply.runTrashStep sets // it only on failure), so this costs nothing and gives undo an // explicit entry name instead of one derived from Dst. detail = sr.Entry } } return j.Append(journal.Entry{ Time: e.Now(), Run: run, Dir: dirName, File: rel, Step: stepNum, Action: actionName(step.Kind), Status: sr.Status, Rule: step.Rule, Src: step.Src, Dst: dst, Size: size, ModTime: mtime, Detail: detail, }) } // statSizeModTime best-effort stats path, returning zero values rather than // an error: it is used only to make a log line more informative (mkdir and // displace entries), never to decide anything. func statSizeModTime(path string) (int64, time.Time) { fi, err := os.Stat(path) if err != nil { return 0, time.Time{} } return fi.Size(), fi.ModTime() } // Runs lists recent runs, newest first; n <= 0 means all. func (e *Engine) Runs(n int) ([]journal.Run, error) { runs, err := journal.Runs(e.Config.LogFile(), n) if err != nil { return nil, fmt.Errorf("engine: runs: %w", err) } return runs, nil } // UndoPlan is the reversal of one run, one UndoFile per file the run // touched, in the order the run first mentioned them. type UndoPlan struct { Run string Files []UndoFile // Cleanup holds files with nothing left to reverse but directories the // run made that something else occupied when the plan was built (re-review // undo F3). They are not offered - that would repeat on every undo - but // ApplyUndo removes any of those directories the other reversals leave // empty, and logs it (plan 10 re-check R1). A front end that rebuilds the // plan must carry Cleanup over. Cleanup []UndoFile } // UndoFile is the reversal of one file's chain, last original step first. // Refused set means none of Steps is reversed by ApplyUndo — spec §10: "no // file is left half undone" — even though individual steps may carry their // own, informational Refused (see UndoStep). type UndoFile struct { File string // the Rel the original run logged Dir string Steps []UndoStep // last original step first Refused string // non-empty: nothing in this file is reversed, and why // Declined is never set by PlanUndo - it carries the front end's own // review decision back into ApplyUndo without widening ApplyUndo's // signature (fix round 2026-09-12, item 2 of Task 8's review): true // means the caller chose not to reverse an otherwise-reversible file // (Refused empty), and ApplyUndo logs it exactly as a declined forward // chain is logged (spec §9: "declined files are logged even though // nothing happens to them") - one entry per step, status "declined" - // rather than silently omitting it the way a Refused file still is. // Setting this on a file that is also Refused has no effect: Refused's // own silent-decline path is checked first and wins. Declined bool } // UndoStep is the reversal of one logged step. type UndoStep struct { Original journal.Entry // the step being reversed Action string // undo-move, undo-rename, undo-copy, undo-trash, undo-displace, undo-mkdir Src, Dst string // what the reversal will do Refused string // non-empty: this step cannot be reversed } // PlanUndo builds the reversal of runID, per spec §10's table. It reads the // log only — no file is touched — so it can be shown and approved before // anything happens (spec §10: undo is planned and approved like any other // plan). // // journal.Entries returning a nil error is the only signal that runID's // chain is intact (Task 1's ruling); a non-nil error, meaning a line inside // the run's window failed to parse or the run has no readable run-start, // refuses the whole run rather than build a reversal from a chain that might // be missing steps. A run with no run-end (a crash) is not this case: // Entries extends the window to end of file and still returns cleanly, so // PlanUndo treats a crashed run exactly like an intact one. func (e *Engine) PlanUndo(runID string) (*UndoPlan, error) { entries, err := journal.Entries(e.Config.LogFile(), runID) if err != nil { return nil, fmt.Errorf("engine: plan undo: %w", err) } if len(entries) == 0 { return nil, fmt.Errorf("engine: plan undo: run %s not found", runID) } if isUndoRun(entries) { return nil, fmt.Errorf("engine: plan undo: run %s is itself an undo and cannot be undone", runID) } // Reversals an earlier undo of this same run already completed are not // offered again (review M10): an undo that stopped part way can be // finished by undoing the run once more. reversed, err := journal.ReversedSteps(e.Config.LogFile(), runID) if err != nil { return nil, fmt.Errorf("engine: plan undo: %w", err) } // Entries are grouped by directory and file together (review M7): one run // spans every directory, and two directories can each hold a file of the // same name. type fileKey struct{ dir, file string } var order []fileKey byFile := map[fileKey][]journal.Entry{} for _, en := range entries { if en.File == "" { // run-start / run-end continue } k := fileKey{en.Dir, en.File} if _, seen := byFile[k]; !seen { order = append(order, k) } byFile[k] = append(byFile[k], en) } up := &UndoPlan{Run: runID} for _, k := range order { uf := planUndoFile(k.dir, k.file, byFile[k], reversed) // Critical finding, Task 8's review: a file every one of whose // entries has Status != "ok" (declined by the ORIGINAL run's own // review, or skipped, or failed before anything happened) yields // an UndoFile with no Steps and no Refused - not a reversible // file, and not a refused one either, just a file this run never // touched. Appending it anyway lied about the plan: it counted // toward "to reverse" (anything with Refused == "" does) while // rendering no row and reversing nothing, so the final tally came // up one short with no message. The condition is deliberately // "no steps AND no refusal", never "no steps" alone - a // permanently deleted file also has zero Steps, but planUndoFile // sets Refused for it (spec §10: it must stay visible, with its // reason, as not undoable), and that file must still be appended. if len(uf.Steps) == 0 && uf.Refused == "" { continue } 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 // would repeat on every undo (re-review undo F3). An empty one is // still offered, and removed. up.Cleanup = append(up.Cleanup, uf) continue } up.Files = append(up.Files, uf) } return up, nil } // onlyOccupiedDirectoryRemovals reports whether every step is an undo-mkdir // of a directory that is not empty now, so none of them could run. func onlyOccupiedDirectoryRemovals(steps []UndoStep) bool { for _, s := range steps { if s.Action != "undo-mkdir" { return false } if entries, err := os.ReadDir(s.Src); err == nil && len(entries) == 0 { return false } } return true } // isUndoRun reports whether every one of entries' file-scoped actions is // already an "undo-" action, i.e. entries belongs to a run ApplyUndo itself // produced. Runs cannot themselves be undone (spec §10). This does not read // journal's own undo-of-run bookkeeping (Detail on the undo run's own // run-start, unexported to this package): a run ApplyUndo writes never logs // a plain action, only "undo-" ones, so checking the action vocabulary // directly is a self-contained equivalent. func isUndoRun(entries []journal.Entry) bool { any := false for _, en := range entries { // A damaged line says nothing about which kind of run this is (plan // 10 re-check R3). if en.Action == "run-start" || en.Action == "run-end" || en.Action == "damaged" { continue } any = true if !strings.HasPrefix(en.Action, "undo-") { return false } } return any } // isFileAffecting reports whether an undo action's own success or failure // bears on the FILE's data - as opposed to "undo-mkdir", whose refusal // gates neither planning nor execution the way every other action's does. // See the comment on planUndoFile for why the two are treated differently. func isFileAffecting(action string) bool { return action != "undo-mkdir" } // planUndoFile builds one file's UndoFile from its log entries (file's own // order, chronological). Only "ok" entries ever happened and so are // candidates for reversal; declined, skipped and failed ones are ignored. // Entries are walked from last to first, per spec §10 ("last step first // within each file"), which also puts a step's own mkdir/displace // sub-entries in the right place relative to it: logStep always writes them // before the step's own primary entry, so reversed order visits the primary // entry first (undo it) and its mkdir/displace satellites after (clean up, // innermost mkdir first; restore what it displaced last) — exactly the // order a real reversal needs. // // Fix round 1, ruling on item 1: "undo-mkdir" is the one action excluded // from the whole-file refusal gate, and the distinction is deliberate, not // an inconsistency. Every other reversal's refusal condition - dst // missing/changed, src now occupied, the trash entry gone - means the same // thing: the world changed under us since the run, and reversing anyway // could lose data. That is what spec §10's "no file is left half undone" // exists to prevent, so it correctly gates the whole file. "Directory not // empty" is not that kind of condition: it means a SIBLING file still lives // there, which is not a hazard to anything, and a directory two files share // is only actually empty once every file that used it has been reversed - // checking it once at planning time, before any of those reversals have // run, would refuse it (and, by the whole-file rule, the entire owning // file, including its otherwise-safe undo-move) essentially every time two // files share a destination directory, which is the common case. So an // undo-mkdir reversal is never refused at planning time, and a failed one // at execution time (ApplyUndo/undoFile) leaves the rest of that file's // steps to run rather than aborting the file — the same "the substantive // act's result is what is reported, cleanup is best-effort" shape as // Task 2's .trashinfo ruling. isFileAffecting is the one predicate both // this function and undoFile's stop-on-failure check share, so the two // places this distinction matters cannot drift apart. // // Fix wave item 1 (Critical): this loop owns an undoProjection, built up as // it appends steps in the order they will actually execute. resolveConflict // (internal/plan/conflict.go) can make one contested path both a step's own // Dst and its Displaces - deliberately, and correct for the forward run - // which means the reversal that puts the incoming file back where it came // from (freeing the contested path) and the reversal that restores the // displaced original to that same path are two steps of ONE file's chain // that genuinely contend for it. reverseStep alone cannot see that: it is a // pure function of one journal entry. So the src-exists occupancy check // (refuseIfSrcExists) is no longer decided there; it is decided here, after // reverseStep returns, with the projection recording what every // earlier-executing step (already appended) will do to the filesystem once // it runs. A path a predecessor will vacate does not count as occupied for // a step that runs after it - the previous ordering assumption ("the world // exactly as it is now, before ANY reversal has run") was simply false for // two steps of one file that touch the same path, and every un-contended // check keeps behaving exactly as before, since the projection only ever // overrides a real occupant that this same chain is itself about to clear. func planUndoFile(dir, file string, ents []journal.Entry, reversed map[journal.ReversedKey]int) UndoFile { uf := UndoFile{File: file, Dir: dir} for _, en := range ents { if en.Action == "damaged" { // A log line of this file is cut or damaged (journal.Entries): a // step may be missing from its chain, so none of it is reversed. uf.Refused = fmt.Sprintf("its log is damaged (%s); a step may not be recorded", en.Detail) return uf } } proj := newUndoProjection() for i := len(ents) - 1; i >= 0; i-- { en := ents[i] if en.Status != "ok" { continue } if en.Action == "delete" { // Permanent delete is terminal: nothing can follow it for this // file, and it is never reversible (spec §10). No UndoStep is // built for it - there is no undo- action for a permanent // delete - the file is simply refused outright. if uf.Refused == "" { uf.Refused = "permanent delete cannot be undone" } break } step := reverseStep(en) if k := (journal.ReversedKey{Dir: dir, File: file, Action: step.Action, Src: step.Src}); reversed[k] > 0 { // An earlier undo of this run already reversed this step: the // disk already shows it, and it is not offered again. It is not // recorded in the projection either (re-review undo F1): what it // put back is on disk now and is checked there, so a file changed // since is refused rather than vouched for by the old reversal. reversed[k]-- continue } if (en.Action == "move" || en.Action == "rename") && proj.occupied[en.Dst] { // A reversal already queued for this same file puts it back at // en.Dst before this one runs, and that reversal was checked // against the disk itself. Judged against the disk as it is now, // en.Dst is empty - a later step of the chain moved the file on - // and every rename-then-move, move-then-move or move-then-trash // chain would be refused (plan 8, found by the property test). step.Refused = "" } if step.Refused == "" { step.Refused = refuseIfSrcExists(step, proj) } proj.record(step) uf.Steps = append(uf.Steps, step) if step.Refused != "" && isFileAffecting(step.Action) && uf.Refused == "" { uf.Refused = step.Refused } } return uf } // 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 // path one of this same file's own earlier-executing steps is about to // vacate. It never touches the filesystem itself - it is a bookkeeping // overlay purely for the offer planUndoFile builds; the execution-time // guards (trash.Restore's own occupancy refusal, renameOrCopy's Lstat) are // what actually protects a file once ApplyUndo runs, regardless of whether // this projection turns out right. type undoProjection struct { vacated map[string]bool // paths a queued step will free once it runs occupied map[string]bool // paths a queued step will place a file at once it runs } func newUndoProjection() *undoProjection { return &undoProjection{vacated: map[string]bool{}, occupied: map[string]bool{}} } // record updates the projection with one step's effect, once it has already // been queued: its own Src becomes free (every reversal action vacates the // path it reads from), and, for the actions that put a file back at a fixed // path (undo-move, undo-rename, undo-trash, undo-displace - never // undo-copy, whose destination is chosen by trash.Put at execution time, and // never undo-mkdir, which only ever frees a path), its Dst becomes occupied. // A path cannot be both at once, so whichever happens second here wins. func (p *undoProjection) record(us UndoStep) { delete(p.occupied, us.Src) p.vacated[us.Src] = true if needsOccupancyCheck(us.Action) { delete(p.vacated, us.Dst) p.occupied[us.Dst] = true } } // occupiedNow reports whether path is spoken for, from the projection's // point of view: really on disk and not about to be vacated by an // earlier-queued step, or not on disk yet but about to be occupied by one // anyway (two of this file's own steps landing on the same path, which // would be a real, if so-far unseen, contention). func (p *undoProjection) occupiedNow(path string) bool { if p.occupied[path] { return true } if p.vacated[path] { return false } _, err := os.Lstat(path) return err == nil } // needsOccupancyCheck reports whether action restores a file to a fixed // path - the only actions refuseIfSrcExists ever needs to check, and // therefore the only ones record above tracks as occupying their Dst. func needsOccupancyCheck(action string) bool { switch action { case "undo-move", "undo-rename", "undo-trash", "undo-displace": return true } return false } // reverseStep computes the UndoStep for one logged "ok" entry, per spec // §10's reversal table. It only stats the filesystem to decide Refused (the // "changed since" and "trash entry is gone" checks); it never mutates // anything, so PlanUndo stays read-only. // // Fix wave item 1: it deliberately does NOT decide the "src now exists" // occupancy refusal any more - that is refuseIfSrcExists, called by // planUndoFile's loop instead of from here. reverseStep is a pure function // of one journal entry: it has no way to see the rest of the file's chain, // so it cannot tell a real occupant from a path an earlier-executing step // of this same chain is about to vacate. planUndoFile owns the projection // that can. func reverseStep(en journal.Entry) UndoStep { us := UndoStep{Original: en, Action: "undo-" + en.Action} switch en.Action { case "move", "rename": us.Src, us.Dst = en.Dst, en.Src us.Refused = refuseIfChanged(en) case "copy": // The reversal moves the copy to the Trash; where it lands there is // decided at execution time (trash.Put chooses the entry name), the // same reason plan.Step.Dst is always "" for a Trash-kind step. us.Src = en.Dst us.Refused = refuseIfChanged(en) case "trash", "displace": us.Src, us.Dst = en.Dst, en.Src us.Refused = refuseIfTrashChanged(en) case "mkdir": us.Src = en.Dst } return us } // refuseIfChanged is the move/rename/copy refusal check: the file at // en.Dst, as it is now, must still match the size and mtime the run logged // for it (spec §9: those columns describe the file at Dst after the step, // precisely so undo can tell whether it has been touched since). // // The mtime comparison is exact (time.Time.Equal), not truncated to whole // seconds: journal entries now round-trip through RFC3339Nano // (journal.Writer.Append, fix round 1 item 4), which preserves the // sub-second precision a fresh os.Stat also has. A whole-second comparison // would let a file rewritten within the same second as the recorded mtime // read as untouched, and undo would move it back believing it had not // changed - the one guard that decides whether to overwrite the user's // file, so it must not have that gap. // Both refusal messages below go through xdg.Abbrev (Minor 4 / fix wave // item 5): every step cell in the printed plan already abbreviates its path // against $HOME (cmd/krino/undo.go's undoActionCell, via xdg.Abbrev), and a // refusal reason sitting two lines under a "→ ~/dl/a.pdf" row in raw // "/tmp/.../sbx/home/dl/a.pdf" form was the one cell that did not match. func refuseIfChanged(en journal.Entry) string { fi, err := os.Stat(en.Dst) if err != nil { return fmt.Sprintf("%s is missing", xdg.Abbrev(en.Dst)) } if fi.Size() != en.Size || !fi.ModTime().Equal(en.ModTime) { return fmt.Sprintf("%s changed since the run", xdg.Abbrev(en.Dst)) } return "" } // refuseIfTrashChanged is the trash reversal's identity check (review M2): // the entry must still be the file this run put there - the size and mtime // the run logged for it - and its trashinfo must still record the path it // came from. Emptying the Trash and trashing another file of the same name // would otherwise have undo restore that file instead. func refuseIfTrashChanged(en journal.Entry) string { fi, err := os.Lstat(en.Dst) if err != nil { return "the trash entry is gone" } if fi.Size() != en.Size || !fi.ModTime().Equal(en.ModTime) { return fmt.Sprintf("the trash entry %s is not the file this run put there", en.Detail) } if p, err := trash.InfoPath(en.Detail); err != nil || p != en.Src { return fmt.Sprintf("the trash entry %s now belongs to another file", en.Detail) } return "" } // recheck repeats, at execution time, the identity check PlanUndo made // (review undo F8): an undo plan is shown and approved first, and a file // changed in that window must not be moved back or trashed. func recheck(step UndoStep) string { switch step.Action { case "undo-move", "undo-rename", "undo-copy": return refuseIfChanged(step.Original) case "undo-trash", "undo-displace": return refuseIfTrashChanged(step.Original) } return "" } // refuseIfSrcExists is the second half of every "reversal puts a file back // at a fixed path" refusal condition: reversing would silently clobber // whatever is there now. Fix wave item 1: it is no longer reverseStep's own // call (see reverseStep's comment) - planUndoFile calls it after reverseStep // returns, passing the projection built from every reversal step already // queued ahead of us in this same file's chain, so a path a predecessor is // about to vacate does not read as occupied. needsOccupancyCheck excludes // undo-copy (destination chosen by trash.Put at execution time) and // undo-mkdir (its own, execution-time-only refusal), the two actions whose // UndoStep.Dst is not a fixed path this check would even make sense against. func refuseIfSrcExists(us UndoStep, proj *undoProjection) string { if !needsOccupancyCheck(us.Action) { return "" } if proj.occupiedNow(us.Dst) { return fmt.Sprintf("%s already exists", xdg.Abbrev(us.Dst)) } return "" } // ApplyUndo reverses up, skipping every file whose Refused is set and // logging a declined file's steps without reversing them (see // declineUndoFile), and logs the reversal as a run of its own: a run-start // whose Detail records which run this undoes (journal's own convention - // Task 1 - so Runs can mark the original run Undone), one entry per undo // step, and a run-end. // // Dir is left blank on the run-start/run-end entries: unlike Apply, which is // always scoped to one directory's DirPlan, one undo run can span several // directories, so there is no single name to put there; each step's own // entry still carries its own file's real directory name from UndoFile.Dir. // // actionable preserves up.Files' own order (the order the original run first // mentioned them), whether a file is actually reversed or only logged as // declined - a single pass, not two, so the two kinds of file interleave in // the log exactly as the run touched them, the same as Apply's own // approved-and-declined chains do. // // Task 1 (plan 5): after every file's reversal has been attempted, a second, // run-wide pass retries the directory removals that were refused as // non-empty. planUndoFile puts the undo-mkdir step for a shared destination // on whichever file's chain first created it (spec §9: only the step that // actually created a directory logs a "mkdir" entry, so only that file's // reversal carries the matching undo-mkdir); when that file reverses first, // its siblings are usually still inside, the removal is correctly refused as // non-empty (spec §10), and - without this pass - nothing ever retries it, // leaving empty directories behind even though every file came back. This // mirrors planUndoFile's own undoProjection insight (see its comment) one // level up: a removal judged too early is judging the wrong world, whether // that "too early" is mid-file (what the projection fixes) or mid-run (what // this retry fixes). // // The retry is a run-level tidy-up, never a re-run of a step: it does not // touch what the first undo-mkdir attempt already logged (that entry, ok or // failed, stands exactly as it was written), and a directory the retry does // manage to remove gets an ADDITIONAL journal entry - never a rewrite - so // the log never disagrees with reality (my ruling on the point the brief // left open: spec §9 logs every step, and a directory removed while the log // still says its removal was refused would be a false record). Because // journal.ranAnyUndoStep already excludes "undo-mkdir" from what marks a run // "(undone)", this extra "ok" entry cannot change that marking either - // TestApplyUndoRetryLogsBothMkdirEntriesAndStillMarksOriginalRunUndone pins // it rather than assuming it. A retried removal is likewise never folded // into ApplyResult: it is // collected from candidates whose first attempt already went through // tallyFile once (via isFileAffecting's exemption), and counting it again // here would double-count a directory that failed once and then quietly // tidied itself away. // // Candidates are collected only from directories this run's own reversal // created - by construction, since every candidate comes from an undo-mkdir // step, and an undo-mkdir step exists only for a directory the forward run's // Made recorded - never a directory the retry merely happens to find empty. // They are retried deepest path first (retryDirRemovals), so a nested // directory - e.g. Work/Sub under Work - is removed before its // now-possibly-empty parent, the same outermost-created/innermost-removed // discipline logStep and undoFile already keep within one file's own chain, // applied here across files. A directory still non-empty at retry time // genuinely holds something else (or the retry runs before every sibling // happens to have reversed, on a later undo of a different run) and simply // stays, with its original refusal the only record of it. func (e *Engine) ApplyUndo(ctx context.Context, up *UndoPlan, j *journal.Writer, run string) (*ApplyResult, error) { result := &ApplyResult{} var actionable []UndoFile for _, f := range up.Files { if f.Refused != "" { result.Declined++ continue } // Fix wave item 4 / final-wave item 24: the same invariant // PlanUndo's own "no steps AND no refusal" guard states explicitly // (see its comment) - a file with no steps and no Refused is one // this run never touched, not a reversible one - given the same // two-condition form here, rather than relying on the Refused // branch above to have already made f.Refused == "" true by the // time this runs. Written this way, the guard is correct on its // own, independent of that branch's order or presence, rather than // unreachable-by-construction the way the parked ruling on this // line described it before Minor 6 showed the same error live. if len(f.Steps) == 0 && f.Refused == "" { continue } actionable = append(actionable, f) } if len(actionable) == 0 { return result, nil } if err := ctx.Err(); err != nil { return result, err } if err := j.Append(journal.Entry{Time: e.Now(), Run: run, Action: "run-start", Status: "ok", Detail: journal.UndoOf(up.Run)}); err != nil { return result, fmt.Errorf("engine: apply undo: %w", err) } var retries []dirRetry for _, f := range actionable { if err := ctx.Err(); err != nil { return result, err } if f.Declined { fr, err := e.declineUndoFile(f, j, run) if err != nil { return result, fmt.Errorf("engine: apply undo: %w", err) } result.Files = append(result.Files, fr) result.Declined++ continue } fr, err := e.undoFile(f, j, run) if err != nil { return result, fmt.Errorf("engine: apply undo: %w", err) } result.Files = append(result.Files, fr) tallyFile(result, fr.Steps, func(i int) bool { return isFileAffecting(f.Steps[i].Action) }) for i, us := range f.Steps { if us.Action == "undo-mkdir" && fr.Steps[i].Status == "failed" { retries = append(retries, dirRetry{dir: us.Src, dirName: f.Dir, file: f.File, step: i + 1, log: true}) } for _, made := range fr.Steps[i].Made { retries = append(retries, dirRetry{dir: made}) } } } for _, f := range up.Cleanup { for i, us := range f.Steps { retries = append(retries, dirRetry{dir: us.Src, dirName: f.Dir, file: f.File, step: i + 1, log: true}) } } if err := e.retryDirRemovals(j, run, retries); err != nil { return result, fmt.Errorf("engine: apply undo: %w", err) } if err := j.Append(journal.Entry{Time: e.Now(), Run: run, Action: "run-end", Status: "ok"}); err != nil { return result, fmt.Errorf("engine: apply undo: %w", err) } return result, nil } // dirRetry names one directory whose undo-mkdir was refused (as non-empty) // during ApplyUndo's main pass, kept for the run-wide retry once every // file's reversal has been attempted. file and dirName are the file and // config directory name that owned the original undo-mkdir step, carried // forward so retryDirRemovals's journal entry - if the retry succeeds - // names the same file and directory the original refusal did, not an // arbitrary one; step is that same step's 1-based index, so the two entries // (the original "failed" and, if the retry succeeds, this "ok") read // together under the same File/Step in the log. type dirRetry struct { dir string dirName string file string step int // log is false for a directory this undo run itself created on the way // (runUndoStep's Made): the original run's log already says it was // removed, so removing it again needs no entry of its own. log bool } // retryDirRemovals is ApplyUndo's run-wide second pass (Task 1, plan 5): once // every file's reversal has run, some directories an undo-mkdir step could // not remove earlier may now be empty, because a sibling file that shared // the directory has since reversed too. candidates is sorted deepest path // first (by descending path-segment count) so a nested directory is removed // before its parent, exactly the order a real cleanup needs; a directory // still non-empty at its turn genuinely holds something else and is left // exactly as its first attempt recorded it - no second entry, no error. // // This never rewrites or removes the original undo-mkdir entry (ok or // failed, whichever the first attempt logged): a directory the retry does // manage to remove gets one ADDITIONAL entry instead (my ruling on the point // the brief left open - see ApplyUndo's comment), so the log always agrees // with what is actually on disk. The new entry's own Action is still // "undo-mkdir", so journal.ranAnyUndoStep - which excludes that action on // principle, not by accident (see its own comment) - continues to treat this // exactly like any other undo-mkdir for the purpose of marking a run // "(undone)": tidying up an empty directory, on the first attempt or the // retry, is still not a restoration. func (e *Engine) retryDirRemovals(j *journal.Writer, run string, candidates []dirRetry) error { sort.SliceStable(candidates, func(i, j int) bool { return pathDepth(candidates[i].dir) > pathDepth(candidates[j].dir) }) for _, c := range candidates { if err := os.Remove(c.dir); err != nil { // Still not empty (or gone, or otherwise unremovable): the // original refusal already recorded this, and it stands. continue } if !c.log { continue } if err := j.Append(journal.Entry{ Time: e.Now(), Run: run, Dir: c.dirName, File: c.file, Step: c.step, Action: "undo-mkdir", Status: "ok", Src: c.dir, }); err != nil { return err } } return nil } // pathDepth counts path's separators after cleaning it, so retryDirRemovals // can sort deepest first: a nested directory (more separators) is always // removed before the parent it sits under, whatever the two paths' common // root. func pathDepth(path string) int { return strings.Count(filepath.Clean(path), string(filepath.Separator)) } // declineUndoFile logs f's reversal as declined without carrying out any of // it - spec §9's "declined files are logged even though nothing happens to // them", extended to undo (fix round 2026-09-12, item 2 of Task 8's review): // a file the front end's own review chose not to reverse still gets one // entry per step, status "declined", the same shape applyFile already gives // a declined forward chain. Every step is declined, not just the first: an // undo file whose reversal was never started needs the same per-step record // a partially-run one would have, so a reader scanning the log by step // number sees a complete, if inert, chain rather than a gap. func (e *Engine) declineUndoFile(f UndoFile, j *journal.Writer, run string) (FileResult, error) { steps := make([]apply.StepResult, len(f.Steps)) for i, step := range f.Steps { sr := apply.StepResult{Status: "declined"} steps[i] = sr if err := j.Append(journal.Entry{ Time: e.Now(), Run: run, Dir: f.Dir, File: f.File, Step: i + 1, Action: step.Action, Status: "declined", Src: step.Src, Dst: step.Dst, }); err != nil { return FileResult{}, err } } return FileResult{Steps: steps}, nil } // undoFile executes every step of f in order (already last-original-step // first from PlanUndo) and logs each. // // Fix round 1, ruling on item 2: a failed FILE-AFFECTING step (everything // but undo-mkdir - see isFileAffecting) stops the rest of the file's steps, // matching apply.Chain's forward model, exactly because continuing past it // is the half-undone state spec §10 forbids: if undo-move fails, reversing // this file's still-earlier steps anyway would leave it in a state that was // never real. A failed undo-mkdir does not stop anything: a sibling file // still occupying that directory is not a hazard (see planUndoFile's // comment), so the remaining steps - which may include another file's // still-untouched undo-copy or undo-trash - keep running. func (e *Engine) undoFile(f UndoFile, j *journal.Writer, run string) (FileResult, error) { steps := make([]apply.StepResult, len(f.Steps)) stopped := false for i, step := range f.Steps { var sr apply.StepResult if stopped { sr = apply.StepResult{Status: "skipped", Detail: "an earlier step in this file's reversal failed"} } else { sr = runUndoStep(step) if sr.Status == "failed" && isFileAffecting(step.Action) { stopped = true } } steps[i] = sr // dst falls back to the reversal's planned destination when nothing // actually happened (failed or skipped), the same informational // convention logStep uses for a forward step that did not run. dst := step.Dst if sr.Status == "ok" { dst = sr.Dst } if err := j.Append(journal.Entry{ Time: e.Now(), Run: run, Dir: f.Dir, File: f.File, Step: i + 1, Action: step.Action, Status: sr.Status, Src: step.Src, Dst: dst, Size: sr.Size, ModTime: sr.ModTime, Detail: sr.Detail, }); err != nil { return FileResult{}, err } } return FileResult{Steps: steps}, nil } // runUndoStep actually carries out one reversal. It reuses apply.StepResult // as a convenient result shape (Status, Detail, Dst, Size, ModTime); its // Step field does not apply here (there is no plan.Kind for an undo) and is // left zero. func runUndoStep(step UndoStep) apply.StepResult { if why := recheck(step); why != "" { return apply.StepResult{Status: "failed", Detail: why} } switch step.Action { case "undo-move", "undo-rename": // The directories created here are returned in Made: ApplyUndo removes // them again once empty, since an earlier file's undo-mkdir may already // have removed the directory this file passes back through (review // undo F6). made, err := apply.MkdirAllTracked(filepath.Dir(step.Dst)) if err != nil { return apply.StepResult{Status: "failed", Detail: err.Error(), Made: made} } if err := renameOrCopy(step.Src, step.Dst); err != nil { return apply.StepResult{Status: "failed", Detail: err.Error(), Made: made} } size, mtime := statSizeModTime(step.Dst) return apply.StepResult{Status: "ok", Dst: step.Dst, Size: size, ModTime: mtime, Made: made} case "undo-copy": entry, err := trash.Put(step.Src) if err != nil { return apply.StepResult{Status: "failed", Detail: err.Error()} } dst := filepath.Join(trash.Dir(), "files", entry) size, mtime := statSizeModTime(dst) return apply.StepResult{Status: "ok", Dst: dst, Size: size, ModTime: mtime} case "undo-trash", "undo-displace": // The trash entry name is read from Original.Detail, where logStep // put it explicitly (fix round 1, item 3) - never re-derived from // Src or Dst's shape, which belong to internal/apply's and this // file's own conventions and must stay free to change independently. // The directory the file goes back into is created here, tracked, // rather than silently by trash.Restore, so ApplyUndo removes it again // when it ends up empty (review undo F6). made, err := apply.MkdirAllTracked(filepath.Dir(step.Dst)) if err != nil { return apply.StepResult{Status: "failed", Detail: err.Error(), Made: made} } restored, err := trash.Restore(step.Original.Detail) if err != nil { return apply.StepResult{Status: "failed", Detail: err.Error(), Made: made} } size, mtime := statSizeModTime(restored) return apply.StepResult{Status: "ok", Dst: restored, Size: size, ModTime: mtime, Made: made} case "undo-mkdir": if err := os.Remove(step.Src); err != nil { return apply.StepResult{Status: "failed", Detail: err.Error()} } return apply.StepResult{Status: "ok"} } return apply.StepResult{Status: "failed", Detail: "engine: unknown undo action " + step.Action} } // renameOrCopy moves src to dst, falling back to a copy-then-remove when // they are on different filesystems. It is engine's own minimal equivalent // of internal/apply's unexported moveFile: that package exports only // Chain, so undo cannot reach its careful temp-file machinery and carries a // small, independent implementation instead. // // The Lstat guard below is not optional (fix round 2, item 1, Critical): // POSIX rename(2) replaces an existing regular file at dst without error, // and PlanUndo's own "src now exists" check ran at planning time, not now - // spec §10 has an undo plan "shown and approved the same way" as any other, // a real human-length window in which something can create a file at dst // before ApplyUndo gets here. Every other reversal path in this file // already re-checks at execution time (the forward executor re-Lstats its // destination, trash.Restore refuses "already exists" at call time, // copyThenRemove below does its own check for the EXDEV fallback); only // this, the common same-filesystem path, was missing it. func renameOrCopy(src, dst string) error { if _, err := os.Lstat(dst); err == nil { return fmt.Errorf("engine: undo: %s already exists", dst) } else if !os.IsNotExist(err) { return err } if err := os.Rename(src, dst); err == nil { return nil } else if !errors.Is(err, syscall.EXDEV) { return err } return copyThenRemove(src, dst) } // copyThenRemove copies src to dst (which must not exist) and, only once // that copy has landed, removes src - the same failure direction as // internal/apply's moveFile: a failure before the copy lands leaves src // untouched, and dst is never partially written where something might read // it (the temporary is removed on any failure before rename). func copyThenRemove(src, dst string) error { in, err := os.Open(src) if err != nil { return err } defer in.Close() fi, err := in.Stat() if err != nil { return err } dir := filepath.Dir(dst) tmp, err := os.CreateTemp(dir, ".krino-undo-*") if err != nil { return err } tmpName := tmp.Name() done := false defer func() { if !done { os.Remove(tmpName) } }() if _, err := io.Copy(tmp, in); err != nil { tmp.Close() return err } if err := tmp.Chmod(fi.Mode().Perm()); err != nil { tmp.Close() return err } // Fix round 2, item 2 (Important): sync before close, matching // internal/apply's copyFile (fs.go), which this was modelled on - same // durability requirement, same reason. if err := tmp.Sync(); err != nil { tmp.Close() return err } if err := tmp.Close(); err != nil { return err } if err := os.Chtimes(tmpName, fi.ModTime(), fi.ModTime()); err != nil { return err } if _, err := os.Lstat(dst); err == nil { return fmt.Errorf("engine: undo: destination already exists: %s", dst) } else if !os.IsNotExist(err) { return err } if err := os.Rename(tmpName, dst); err != nil { return err } done = true return os.Remove(src) }