4149f626ab4a
refuse undo, load, pluck and view load on unsaved changes and re-plan after the prompt (3.0.2)
- time
- 2026-09-23 21:14 UTC · 2 weeks ago
- machine
- koh2
- agent
- claude-opus-5-5
- session
- koh 3 review fixes
- parent
- 8aa3154c04f0
↓ koh-4149f626.tar.gzatlas at this save
files changed6 files
VERSION+1 −1src/cli/cmd_back.v+32 −22src/cli/cmd_back_test.v+37 −0src/cli/cmd_pluck.v+40 −18tests/cli_test.v+19 −1v.mod+1 −1src/cli/cmd_back.v+32 −22
@@ -107,19 +107,28 @@107107 if as_json && !args.flag('yes') {108108 return refuse(true, 'needs --yes when not interactive')109109 }110- plan := plan_restore(store, target, discard_first) or { return refuse(as_json, err.msg()) }110+ if !discard_first {111+ require_clean(store) or { return refuse(as_json, err.msg()) }112+ }113+ preview := plan_restore(store, target) or { return refuse(as_json, err.msg()) }111114 if !as_json {112115 ok('${verb}: [${target.id}] ${target.message}')113- for delta in plan.deltas {116+ for delta in preview.deltas {114117 println(' ${change_mark(delta.kind)} ${delta.path}')115118 }116- note('${fmt.plural(plan.deltas.len, 'file')} change · ${fmt.plural(plan.remove.len,119+ note('${fmt.plural(preview.deltas.len, 'file')} change · ${fmt.plural(preview.remove.len,117120 'file')} removed')118121 }119122 approved := confirm(args.flag('yes')) or { return refuse(as_json, err.msg()) }120123 if !approved {121124 return 1122125 }126+ // The tree may have changed while the prompt waited, so the restore acts on127+ // a fresh plan; the one above only drew the preview.128+ if !discard_first {129+ require_clean(store) or { return refuse(as_json, err.msg()) }130+ }131+ plan := plan_restore(store, target) or { return refuse(as_json, err.msg()) }123132 save := apply_restore(store, target, verb, plan, discard_first) or {124133 return refuse(as_json, err.msg())125134 }@@ -158,7 +167,8 @@158167 // restore_now performs a whole restore without prompting: the plan, the save159168 // and the files. The history view uses it for its `l` key.160169 pub fn restore_now(store &storage.Store, target history.Save, verb string) !history.Save {161- plan := plan_restore(store, target, false)!170+ require_clean(store)!171+ plan := plan_restore(store, target)!162172 save := apply_restore(store, target, verb, plan, false)!163173 snapshot.restore_files(store, store.root, snapshot.load(store.root), plan.files, plan.remove)!164174 return save@@ -170,13 +180,21 @@170180 deltas []snapshot.Delta171181 }172182 173-// plan_restore works out what writing `target` back onto disk involves.174-//175-// `prune_untracked` is revert's rule: revert promises to discard unsaved176-// changes, so a file that was never saved goes too. undo and load are more177-// conservative and only remove paths history already knows about; a file the178-// user never saved is not theirs to delete.179-fn plan_restore(store &storage.Store, target history.Save, prune_untracked bool) !RestorePlan {183+const unsaved_changes = 'unsaved changes — save or checkpoint first, or discard them with koh revert'184+185+// require_clean refuses to restore over work only the disk has, as Koh 2 did.186+// `revert` is the one going-back that may discard it, and it records it first.187+fn require_clean(store &storage.Store) ! {188+ head := history.head(store) or { return }189+ if compare_with_head(store, head)!.dirty {190+ return error(unsaved_changes)191+ }192+}193+194+// plan_restore works out what writing `target` back onto disk involves. Every195+// file on disk the target lacks goes: undo, load and pluck only run on a clean196+// tree, so those are tracked files, and revert has recorded the rest.197+fn plan_restore(store &storage.Store, target history.Save) !RestorePlan {180198 files := history.files_of(store, target.id)!181199 mut wanted := map[string]bool{}182200 for file in files {@@ -184,17 +202,9 @@184202 }185203 from := working_tree(store)!186204 mut remove := []string{}187- if prune_untracked {188- for file in from.files {189- if file.path !in wanted {190- remove << file.path191- }192- }193- } else if head := history.head(store) {194- for file in history.files_of(store, head.id) or { []snapshot.FileEntry{} } {195- if file.path !in wanted {196- remove << file.path197- }205+ for file in from.files {206+ if file.path !in wanted {207+ remove << file.path198208 }199209 }200210 deltas := snapshot.diff_trees(store, from, snapshot.Tree{src/cli/cmd_back_test.v+37 −0
@@ -0,0 +1,37 @@1+module cli2+3+import os4+import src.history5+import src.snapshot6+import src.storage7+8+// The history view's `l` key goes through restore_now, which a CLI test cannot9+// reach without a terminal, so it is checked here directly.10+fn test_restore_now_refuses_to_overwrite_unsaved_work() {11+ dir := os.join_path(os.temp_dir(), 'koh3_cli_restore_now_${os.getpid()}')12+ os.rmdir_all(dir) or {}13+ os.mkdir_all(dir)!14+ defer {15+ os.rmdir_all(dir) or {}16+ }17+ store := storage.create(dir)!18+ defer {19+ store.close()20+ }21+ path := os.join_path(dir, 'a.txt')22+ os.write_file(path, 'one\n')!23+ first := history.commit_worktree(store, dir, snapshot.load(dir), history.SaveInput{24+ message: 'one'25+ })!26+ os.write_file(path, 'two\n')!27+ history.commit_worktree(store, dir, snapshot.load(dir), history.SaveInput{28+ message: 'two'29+ })!30+ os.write_file(path, 'unsaved\n')!31+ if _ := restore_now(store, first, 'load') {32+ assert false, 'restore_now overwrote unsaved work'33+ } else {34+ assert err.msg() == unsaved_changes35+ }36+ assert os.read_file(path)! == 'unsaved\n'37+}src/cli/cmd_pluck.v+40 −18
@@ -12,6 +12,7 @@1212 import src.apply1313 import src.history1414 import src.snapshot15+import src.storage1516 1617 pub fn cmd_pluck(raw []string) int {1718 args := parse('pluck', raw, Options{@@ -30,25 +31,18 @@3031 if args.positional.len > 1 {3132 filters = args.positional[1..]3233 }34+ // A dry run only reads, so it may look at a dirty tree; a real pluck35+ // rewrites files and must not overwrite unsaved work.36+ if !args.flag('dry-run') {37+ require_clean(store) or { return refuse(args.json(), err.msg()) }38+ }3339 reverse := args.flag('reverse')3440 verb := if reverse { 'pluck reverse' } else { 'pluck' }3541 36- source_tree := tree_of(store, source.id) or { return refuse(args.json(), err.msg()) }37- mut parent_tree := snapshot.Tree{}38- if source.parent != '' {39- parent_tree = tree_of(store, source.parent) or { return refuse(args.json(), err.msg()) }42+ plan, _ := plan_pluck(store, source, reverse, filters) or {43+ return refuse(args.json(), err.msg())4044 }41- current := working_tree(store) or { return refuse(args.json(), err.msg()) }42- // Forward pluck applies the save's change; --reverse applies its inverse.43- base := if reverse { source_tree } else { parent_tree }44- target := if reverse { parent_tree } else { source_tree }4545 46- plan := apply.three_way(current.files, base.files, target.files, filters, [])47- if apply.conflicts(plan) {48- return refuse(args.json(),49- '${plan.conflict} has changed since [${source.id}]; koh does not merge lines — resolve it by hand, or load the save and re-edit')50- }51-5246 if args.flag('dry-run') || plan.applied.len == 0 {5347 if args.json() {5448 emit_json(json.encode(PluckJson{@@ -80,20 +74,30 @@8074 if !approved {8175 return 18276 }77+ // The tree may have changed while the prompt waited, so the pluck acts on a78+ // fresh plan; the one above only drew the preview.79+ require_clean(store) or { return refuse(args.json(), err.msg()) }80+ fresh, current := plan_pluck(store, source, reverse, filters) or {81+ return refuse(args.json(), err.msg())82+ }83+ if fresh.applied.len == 0 {84+ return refuse(args.json(),85+ 'nothing to apply from [${source.id}] any more — the tree changed while waiting')86+ }8387 // The resulting tree is the working tree with the change applied, so its8488 // object bytes come from disk.85- save := history.commit_files(store, store.root, plan.files, history.SaveInput{89+ save := history.commit_files(store, store.root, fresh.files, history.SaveInput{8690 message: '${verb}: ${source.id}'8791 }) or { return refuse(args.json(), err.msg()) }88- remove := not_in(current.files, plan.files)89- report := snapshot.restore_files(store, store.root, snapshot.load(store.root), plan.files,92+ remove := not_in(current, fresh.files)93+ report := snapshot.restore_files(store, store.root, snapshot.load(store.root), fresh.files,9094 remove) or { return refuse(args.json(), err.msg()) }9195 if args.json() {9296 emit_json(json.encode(PluckedJson{9397 id: save.id9498 source: source.id9599 reverse: reverse96- applied: plan.applied100+ applied: fresh.applied97101 written: report.written98102 removed: report.removed99103 }))@@ -101,6 +105,24 @@101105 }102106 ok('plucked [${source.id}] into [${save.id}]')103107 return 0108+}109+110+// plan_pluck applies the source save's change (or, reversed, its inverse) to111+// the working tree, and returns the plan with the tree it was made from.112+fn plan_pluck(store &storage.Store, source history.Save, reverse bool, filters []string) !(apply.Plan, []snapshot.FileEntry) {113+ source_tree := tree_of(store, source.id)!114+ mut parent_tree := snapshot.Tree{}115+ if source.parent != '' {116+ parent_tree = tree_of(store, source.parent)!117+ }118+ current := working_tree(store)!119+ base := if reverse { source_tree } else { parent_tree }120+ target := if reverse { parent_tree } else { source_tree }121+ plan := apply.three_way(current.files, base.files, target.files, filters, [])122+ if apply.conflicts(plan) {123+ return error('${plan.conflict} has changed since [${source.id}]; koh does not merge lines — resolve it by hand, or load the save and re-edit')124+ }125+ return plan, current.files104126 }105127 106128 // not_in lists the paths of `before` that `after` no longer has.tests/cli_test.v+19 −1
@@ -228,7 +228,24 @@228228 write(dir, 'a.txt', 'three\n')229229 third := run(dir, 'save', 'three', '--json').out230230 id3 := third.all_after('"id":"')[..12]231+ // Unsaved work — here a file never saved — blocks every going-back but232+ // revert, as in Koh 2: a restore must never overwrite what only the disk has.231233 write(dir, 'never.txt', 'keep me\n')234+ log_before := run(dir, 'log').out235+ for args in [['undo', '--yes'], ['load', id1, '--yes'], ['pluck', id2, '--yes']] {236+ blocked := run(dir, ...args)237+ assert blocked.code == 1238+ assert blocked.err.contains('unsaved changes')239+ }240+ assert run(dir, 'log').out == log_before241+ os.rm(os.join_path(dir, 'never.txt')) or { panic(err) }242+ write(dir, 'a.txt', 'edited, never saved\n')243+ assert run(dir, 'undo', '--yes').err.contains('unsaved changes')244+ assert os.read_file(os.join_path(dir, 'a.txt'))! == 'edited, never saved\n'245+ write(dir, 'a.txt', 'three\n')246+ os.rm(os.join_path(dir, 'gone.txt')) or { panic(err) }247+ assert run(dir, 'load', id1, '--yes').err.contains('unsaved changes')248+ write(dir, 'gone.txt', 'gone\n')232249 undo := run(dir, 'undo', '--yes', '--json')233250 assert undo.code == 0234251 if !undo.out.contains(id2) {@@ -236,7 +253,6 @@236253 }237254 assert os.read_file(os.join_path(dir, 'a.txt'))! == 'two\n'238255 assert os.exists(os.join_path(dir, 'gone.txt'))239- assert os.exists(os.join_path(dir, 'never.txt'))240256 assert run(dir, 'load', id3, '--yes').code == 0241257 assert os.read_file(os.join_path(dir, 'a.txt'))! == 'three\n'242258 assert run(dir, 'undo', '2', '--yes').code == 0@@ -304,6 +320,8 @@304320 assert run(dir, 'pluck', '--reverse', id2, '--yes').code == 0305321 assert os.read_file(os.join_path(dir, 'a.txt'))! == 'one\n'306322 write(dir, 'a.txt', 'different\n')323+ assert run(dir, 'pluck', id2, '--dry-run').code == 1324+ assert run(dir, 'save', 'different').code == 0307325 conflict := run(dir, 'pluck', id2, '--yes')308326 assert conflict.code == 1309327 assert conflict.err.contains('a.txt')v.mod+1 −1
@@ -1,6 +1,6 @@11 Module {22 name: 'koh3'3- version: '3.0.1'3+ version: '3.0.2'44 description: 'One SQLite file per repo, one line of history.'55 license: 'MIT'66 }