mark a restore before its save is recorded and name it when status cannot walk, closing 3.0.31's codex review (3.0.39)

@@ -1 +1 @@1-3.0.381+3.0.39

@@ -154,6 +154,21 @@154154 // working tree and does not ask anything: the history view has already asked,155155 // and the order still has to be save-first, files-second.156156 pub fn apply_restore(store &storage.Store, target history.Save, verb string, plan RestorePlan, discard_first bool) !history.Save {157+	// A revert may be finishing an earlier restore; that one's mark stays158+	// until the files are right.159+	earlier := snapshot.restore_pending(store)160+	snapshot.mark_restore(store)!161+	return record_restore(store, target, verb, plan, discard_first) or {162+		// Nothing moved, so this restore has nothing to finish. A mark that163+		// cannot be lifted only means revert is suggested once needlessly.164+		if !earlier {165+			snapshot.clear_restore_mark(store) or {}166+		}167+		return err168+	}169+}170+171+fn record_restore(store &storage.Store, target history.Save, verb string, plan RestorePlan, discard_first bool) !history.Save {157172 	if discard_first {158173 		history.commit_worktree(store, store.root, snapshot.load(store.root), history.SaveInput{159174 			message:       'revert: discarded changes'

@@ -42,3 +42,63 @@4242 	assert !peek_is_old(now - peek_age, now)4343 	assert peek_is_old(now - peek_age - 1, now)4444 }45+46+// The restore's save is recorded before its files are written. A kill between47+// the two must still leave the mark, so the mark goes down first.48+fn test_a_restore_is_marked_before_its_save_is_recorded() {49+	dir := os.join_path(os.temp_dir(), 'koh3_cli_mark_first_${os.getpid()}')50+	os.rmdir_all(dir) or {}51+	os.mkdir_all(dir)!52+	defer {53+		os.rmdir_all(dir) or {}54+	}55+	store := storage.create(dir)!56+	defer {57+		store.close()58+	}59+	os.write_file(os.join_path(dir, 'a.txt'), 'one\n')!60+	first := history.commit_worktree(store, dir, snapshot.load(dir), history.SaveInput{61+		message: 'one'62+	})!63+	os.write_file(os.join_path(dir, 'a.txt'), 'two\n')!64+	history.commit_worktree(store, dir, snapshot.load(dir), history.SaveInput{65+		message: 'two'66+	})!67+	// Any save recorded while the mark is missing aborts the restore.68+	store.db.exec("CREATE TEMP TRIGGER mark_first BEFORE INSERT ON saves69+		WHEN NOT EXISTS (SELECT 1 FROM state WHERE key = 'restoring')70+		BEGIN SELECT RAISE(ABORT, 'save recorded before the mark'); END")!71+	apply_restore(store, first, 'load', plan_restore(store, first)!, false)!72+	assert snapshot.restore_pending(store)73+}74+75+// A revert finishing an earlier restore must not lift that restore's mark when76+// it fails to record anything itself.77+fn test_a_failed_restore_keeps_an_earlier_mark() {78+	dir := os.join_path(os.temp_dir(), 'koh3_cli_keep_mark_${os.getpid()}')79+	os.rmdir_all(dir) or {}80+	os.mkdir_all(dir)!81+	defer {82+		os.rmdir_all(dir) or {}83+	}84+	store := storage.create(dir)!85+	defer {86+		store.close()87+	}88+	os.write_file(os.join_path(dir, 'a.txt'), 'one\n')!89+	first := history.commit_worktree(store, dir, snapshot.load(dir), history.SaveInput{90+		message: 'one'91+	})!92+	snapshot.mark_restore(store)!93+	// An object the store does not have makes recording the save fail.94+	broken := RestorePlan{95+		files: [snapshot.FileEntry{96+			path: 'a.txt'97+			hash: 'not-an-object'98+		}]99+	}100+	if _ := apply_restore(store, first, 'revert', broken, false) {101+		assert false, 'recording a save with a missing object should fail'102+	}103+	assert snapshot.restore_pending(store)104+}

@@ -86,9 +86,13 @@8686 	}8787 	// The resulting tree is the working tree with the change applied, so its8888 	// object bytes come from disk.89+	snapshot.mark_restore(store) or { return refuse(args.json(), err.msg()) }8990 	save := history.commit_files(store, store.root, fresh.files, history.SaveInput{9091 		message: '${verb}: ${source.id}'91-	}) or { return refuse(args.json(), err.msg()) }92+	}) or {93+		snapshot.clear_restore_mark(store) or {}94+		return refuse(args.json(), err.msg())95+	}9296 	remove := not_in(current, fresh.files)9397 	report := snapshot.restore_files(store, store.root, snapshot.load(store.root), fresh.files,9498 		remove) or { return refuse(args.json(), err.msg()) }

@@ -19,8 +19,16 @@1919 	head := history.head(store) or {2020 		return refuse(args.json(), 'this repo has no saves yet — run "koh save \\"first save\\""')2121 	}22-	changes := compare_with_head(store, head) or { return refuse(args.json(), err.msg()) }2322 	unfinished := snapshot.restore_pending(store)23+	changes := compare_with_head(store, head) or {24+		// Whatever stopped the restore may also stop this walk.25+		reason := if unfinished {26+			'${err.msg()} — ${snapshot.restore_unfinished}'27+		} else {28+			err.msg()29+		}30+		return refuse(args.json(), reason)31+	}2432 2533 	if args.json() {2634 		emit_json(json.encode(StatusJson{

@@ -164,6 +164,10 @@164164 		store.rollback()165165 		return refuse(args.json(), err.msg())166166 	}167+	snapshot.mark_restore(store) or {168+		store.rollback()169+		return refuse(args.json(), err.msg())170+	}167171 	store.commit() or { return refuse(args.json(), err.msg()) }168172 	files := history.files_of(store, fetched.head) or { return refuse(args.json(), err.msg()) }169173 	snapshot.restore_files(store, target, snapshot.Ignore{}, files, []) or {

@@ -21,6 +21,12 @@2121 	return store.state(restoring_key) != none2222 }2323 24+// mark_restore goes down before a restore records its save, so a kill between25+// the save and the files still leaves it; restore_files lifts it at the end.26+pub fn mark_restore(store &storage.Store) ! {27+	store.set_state(restoring_key, 'yes')!28+}29+2430 pub fn clear_restore_mark(store &storage.Store) ! {2531 	store.remove_state(restoring_key)!2632 }@@ -38,11 +44,8 @@3844 // temp file and a rename, so an interrupted restore leaves a readable tree3945 // rather than half-written files.4046 pub fn restore_files(store &storage.Store, root string, ig Ignore, target []FileEntry, remove []string) !RestoreReport {41-	// Peek writes elsewhere; only the repo's own tree is marked.47+	// Peek writes elsewhere; only the repo's own tree carries the mark.4248 	marks := root == store.root43-	if marks {44-		store.set_state(restoring_key, 'yes')!45-	}4649 	mut wanted := map[string]bool{}4750 	for entry in target {4851 		wanted[entry.path] = true

@@ -130,6 +130,13 @@130130 		store.rollback()131131 		return err132132 	}133+	// The files follow the commit; the mark lands with it, in one step.134+	if head_before == none || final.id != head_before or { history.Save{} }.id {135+		snapshot.mark_restore(store) or {136+			store.rollback()137+			return err138+		}139+	}133140 	store.commit() or { return err }134141 	report.head = final.id135142 	flags := events.latest_flags(store)!

@@ -157,6 +157,8 @@157157 	report := steal_now(a, mut backend)!158158 	assert report.replayed == 0159159 	assert history.head(a)!.id == ahead.id160+	// Nothing was restored, so nothing is left marked as half-restored.161+	assert !snapshot.restore_pending(a)160162 	assert history.all_saves(a)!.len == 2161163 	offer_now(a, mut backend)!162164 	assert backend.get(head_key('proj'))!.body.bytestr() == ahead.id

@@ -221,21 +221,28 @@221221 fn test_a_restore_cut_short_points_at_revert() {222222 	dir := temp_repo('cut-short')223223 	defer {224-		os.chmod(os.join_path(dir, 'ro'), 0o755) or {}224+		os.chmod(os.join_path(dir, 'zz'), 0o755) or {}225225 		os.rmdir_all(dir) or {}226226 	}227227 	init_repo(dir)228228 	write(dir, 'zero', '0\n')229229 	assert run(dir, 'save', 'zero').code == 0230-	write(dir, 'ro/f', 'v1\n')231-	write(dir, 'top', 'v1\n')230+	write(dir, 'a.txt', 'v1\n')231+	write(dir, 'zz/f', 'v1\n')232232 	assert run(dir, 'save', 'first').code == 0233-	write(dir, 'ro/f', 'v2\n')234-	write(dir, 'top', 'v2\n')233+	write(dir, 'a.txt', 'v2\n')234+	write(dir, 'zz/f', 'v2\n')235235 	assert run(dir, 'save', 'second').code == 0236-	os.chmod(os.join_path(dir, 'ro'), 0o555)!236+	// a.txt is written, then the read-only folder stops the restore: the237+	// tree is genuinely half one save and half the other.238+	os.chmod(os.join_path(dir, 'zz'), 0o555)!237239 	assert run(dir, 'undo', '--yes').code == 1238-	os.chmod(os.join_path(dir, 'ro'), 0o755)!240+	assert os.read_file(os.join_path(dir, 'a.txt'))! == 'v1\n'241+	assert os.read_file(os.join_path(dir, 'zz/f'))! == 'v2\n'242+	// Even a status that cannot read the tree says what happened.243+	os.chmod(os.join_path(dir, 'zz'), 0o000)!244+	assert run(dir, 'status').err.contains('koh revert')245+	os.chmod(os.join_path(dir, 'zz'), 0o755)!239246 240247 	assert run(dir, 'status', '--json').out.contains('"restore_unfinished":true')241248 	for args in [['save', 'half'], ['checkpoint', 'half'], ['undo', '--yes']] {@@ -244,10 +251,10 @@244251 		assert r.err.contains('koh revert')245252 	}246253 	assert run(dir, 'revert', '--yes').code == 0247-	assert os.read_file(os.join_path(dir, 'ro/f'))! == 'v1\n'248-	assert os.read_file(os.join_path(dir, 'top'))! == 'v1\n'254+	assert os.read_file(os.join_path(dir, 'a.txt'))! == 'v1\n'255+	assert os.read_file(os.join_path(dir, 'zz/f'))! == 'v1\n'249256 	assert run(dir, 'status', '--json').out.contains('"restore_unfinished":false')250-	write(dir, 'top', 'v3\n')257+	write(dir, 'a.txt', 'v3\n')251258 	assert run(dir, 'save', 'carry on').code == 0252259 }253260 

@@ -1,6 +1,6 @@11 Module {22 	name: 'koh3'3-	version: '3.0.38'3+	version: '3.0.39'44 	description: 'One SQLite file per repo, one line of history.'55 	license: 'MIT'66 }