ask steal's clash questions before the write lock and refuse a steal that would overwrite unsaved files (3.0.9)

@@ -1 +1 @@1-3.0.81+3.0.9

@@ -72,20 +72,34 @@7272 		if has_after {7373 			to_bytes = bytes_of(store, to, to_map[path])!7474 		}75-		mut delta := Delta{76-			path: path77-			kind: kind78-		}79-		if looks_binary(from_bytes) || looks_binary(to_bytes) {80-			delta.binary = true81-			delta.patch = binary_notice82-		} else {83-			delta.patch = run_diff(work, path, from_bytes, to_bytes)!84-			delta.added, delta.removed = count_lines(delta.patch)85-		}86-		deltas << delta75+		deltas << diff_in(work, path, kind, from_bytes, to_bytes)!8776 	}8877 	return deltas78+}79+80+// diff_bytes compares two versions of one file held in memory, for callers81+// whose bytes are not in the store yet.82+pub fn diff_bytes(path string, kind string, from_bytes []u8, to_bytes []u8) !Delta {83+	work := temp_workspace()!84+	defer {85+		os.rmdir_all(work) or {}86+	}87+	return diff_in(work, path, kind, from_bytes, to_bytes)!88+}89+90+fn diff_in(work string, path string, kind string, from_bytes []u8, to_bytes []u8) !Delta {91+	mut delta := Delta{92+		path: path93+		kind: kind94+	}95+	if looks_binary(from_bytes) || looks_binary(to_bytes) {96+		delta.binary = true97+		delta.patch = binary_notice98+	} else {99+		delta.patch = run_diff(work, path, from_bytes, to_bytes)!100+		delta.added, delta.removed = count_lines(delta.patch)101+	}102+	return delta89103 }90104 91105 pub fn index_files(files []FileEntry) map[string]FileEntry {

@@ -8,8 +8,10 @@88 99 module sync1010 11+import compress.zlib1112 import os1213 import src.apply14+import src.events1315 import src.history1416 import src.remote1517 import src.snapshot@@ -54,18 +56,19 @@5456 }5557 5658 pub const dirty_tree = 'save or checkpoint first — steal needs a clean tree'59+pub const no_saves_here = 'this folder has files but no saves — steal a repo into an empty folder with "koh steal <name>"'5760 5861 // steal brings the remote's new saves in. Inside a repo with its own work this5962 // may have to combine; a repo with nothing of its own simply fast-forwards.6063 pub fn steal(store &storage.Store, mut backend remote.Backend, opts StealOptions) !StealReport {6164 	name := repo_name(store, mut backend)!6265 	head_before := history.head(store)63-	if head := head_before {64-		// Combining rewrites the working tree, so unsaved work would be lost.65-		changed := dirty(store, head)!66-		if changed {67-			return error(dirty_tree)66+	// Combining rewrites the working tree, so unsaved work would be lost.67+	if dirty(store, head_before)! {68+		if head_before == none {69+			return error(no_saves_here)6870 		}71+		return error(dirty_tree)6972 	}7073 	fetched := fetch_back_to_known(store, mut backend, name)!7174 	mut report := StealReport{@@ -88,10 +91,32 @@8891 		return error('the remote keeps a different history — it shares no save with this repo')8992 	}9093 94+	strips_before := events.strip_events(store)!.len95+	mut clashes := []Clash{}96+	mut side_of := map[string]Side{}97+	if fetched.saves.len > 0 && local_only.len > 0 {98+		clashes = combine_clashes(store, fetched)!99+		report.clashes = clashes.map(it.path)100+		// Asked before the write lock: a person thinking must not stall every101+		// agent's save on this repo.102+		side_of = choose_sides(clashes, opts)!103+	}104+91105 	store.begin() or { return err }92-	final := (write_and_combine(store, fetched, local_only, opts, mut report) or {106+	// The questions ran without the lock. If a save or an edit landed107+	// meanwhile, the answers were about a line that no longer exists.108+	if moved := changed_since(store, head_before, strips_before) {109+		if moved {110+			store.rollback()111+			return error('this repo changed while steal was asking — run koh steal again')112+		}113+	} else {93114 		store.rollback()94115 		return err116+	}117+	final := (write_and_combine(store, fetched, local_only, clashes, side_of, mut report) or {118+		store.rollback()119+		return err95120 	})96121 	store.commit() or { return err }97122 	report.head = final.id@@ -126,7 +151,7 @@126151 // write_and_combine lands a fetch and, when the local line has saves the remote127152 // lacks, replays them on top of the remote head. It runs inside the caller's128153 // transaction and returns the new head.129-fn write_and_combine(store &storage.Store, fetched Fetched, local_only []history.Save, opts StealOptions, mut report StealReport) !history.Save {154+fn write_and_combine(store &storage.Store, fetched Fetched, local_only []history.Save, clashes []Clash, side_of map[string]Side, mut report StealReport) !history.Save {130155 	write_fetched(store, fetched)!131156 	if fetched.saves.len == 0 {132157 		// The remote head is already on this line: level, or this machine is@@ -140,18 +165,10 @@140165 		return history.head(store) or { return error('the remote head did not land') }141166 	}142167 	remote_files := history.files_of(store, fetched.head)!143-	base_files := if fetched.base != '' {144-		history.files_of(store, fetched.base)!145-	} else {146-		[]snapshot.FileEntry{}147-	}148168 	mut mine_files := []snapshot.FileEntry{}149169 	if our_head := history.head(store) {150170 		mine_files = history.files_of(store, our_head.id)!151171 	}152-	clashes := describe_clashes(store, find_clashes(base_files, mine_files, remote_files))153-	report.clashes = clashes.map(it.path)154-	side_of := choose_sides(clashes, opts)!155172 156173 	// Take the remote's line first, so the replayed saves chain onto it.157174 	history.set_head(store, fetched.head)!@@ -253,36 +270,54 @@253270 	return clashes254271 }255272 256-// describe_clashes adds the diff stat each prompt needs.257-fn describe_clashes(store &storage.Store, clashes []Clash) []Clash {258-	mut out := []Clash{}259-	for clash in clashes {260-		mut decorated := clash261-		decorated.stat = clash_stat(store, clash)262-		out << decorated273+// combine_clashes finds the files both lines changed, with the diff stat each274+// prompt shows. It reads the remote head from the fetch, not the store, so it275+// can run before anything is written.276+fn combine_clashes(store &storage.Store, fetched Fetched) ![]Clash {277+	base_files := if fetched.base != '' {278+		history.files_of(store, fetched.base)!279+	} else {280+		[]snapshot.FileEntry{}263281 	}264-	return out282+	head := history.head(store) or { return []Clash{} }283+	// The transaction applies the incoming strips before it replays, so the284+	// clashes are found on the trees as they will be then, or the answers285+	// would be about files that are no longer there.286+	strips := fetched.events_out.filter(it.kind == events.kind_strip)287+	mut clashes := find_clashes(after_strips(base_files, fetched.base, strips), after_strips(history.files_of(store,288+		head.id)!, head.id, strips), after_strips(to_files(fetched.saves.last()), fetched.head,289+		strips))290+	for mut clash in clashes {291+		mine := fetched_bytes(store, fetched, clash.mine)!292+		theirs := fetched_bytes(store, fetched, clash.theirs)!293+		delta := snapshot.diff_bytes(clash.path, 'modified', mine, theirs)!294+		clash.stat = if delta.binary { 'binary' } else { '+${delta.added} -${delta.removed}' }295+	}296+	return clashes265297 }266298 267-fn clash_stat(store &storage.Store, clash Clash) string {268-	deltas := snapshot.diff_trees(store, snapshot.Tree{269-		files: present(clash.mine)270-	}, snapshot.Tree{271-		files: present(clash.theirs)272-	}) or { return '' }273-	if deltas.len == 0 {274-		return 'no textual difference'275-	}276-	return '+${deltas[0].added} -${deltas[0].removed}'299+// after_strips is a save's file list once the given strip events are applied.300+fn after_strips(files []snapshot.FileEntry, save_id string, strips []events.Event) []snapshot.FileEntry {301+	return files.filter(fn [strips, save_id] (file snapshot.FileEntry) bool {302+		for strip in strips {303+			if (strip.save_id == '' || strip.save_id == save_id) && covers(strip.path, file.path) {304+				return false305+			}306+		}307+		return true308+	})277309 }278310 279-// present drops an absent side, so a comparison never asks the object store for280-// an empty hash.281-fn present(entry snapshot.FileEntry) []snapshot.FileEntry {311+// fetched_bytes reads one side of a clash: from the fetch when it has just312+// arrived, else from the store. An absent side is empty.313+fn fetched_bytes(store &storage.Store, fetched Fetched, entry snapshot.FileEntry) ![]u8 {282314 	if entry.hash == '' {283-		return []snapshot.FileEntry{}315+		return []u8{}284316 	}285-	return [entry]317+	if blob := fetched.repo_objects[entry.hash] {318+		return zlib.decompress(blob)!319+	}320+	return store.get_object(entry.hash)!286321 }287322 288323 fn choose_sides(clashes []Clash, opts StealOptions) !map[string]Side {@@ -315,11 +350,27 @@315350 	return a.hash == b.hash && a.executable == b.executable316351 }317352 318-// dirty reports whether the working tree differs from `head` in a way a steal319-// would overwrite.320-fn dirty(store &storage.Store, head history.Save) !bool {321-	return !snapshot.same_tree(history.files_of(store, head.id)!, snapshot.walk(store, store.root,322-		snapshot.load(store.root))!)353+fn changed_since(store &storage.Store, before ?history.Save, strips_before int) !bool {354+	if events.strip_events(store)!.len != strips_before {355+		return true356+	}357+	now := history.head(store)358+	if now or { history.Save{} }.id != before or { history.Save{} }.id {359+		return true360+	}361+	return dirty(store, now)!362+}363+364+// dirty reports whether the working tree holds anything a steal would365+// overwrite. With no saves yet, that is every file but the untouched ignore366+// list `koh init` wrote.367+fn dirty(store &storage.Store, head ?history.Save) !bool {368+	files := snapshot.walk(store, store.root, snapshot.load(store.root))!369+	if saved := head {370+		return !snapshot.same_tree(history.files_of(store, saved.id)!, files)371+	}372+	seed := storage.hash_hex(snapshot.default_ignore.bytes())373+	return files.any(it.path != snapshot.ignore_file || it.hash != seed)323374 }324375 325376 // fetch_repo pulls a whole repo — head, saves, objects and events — for

@@ -264,6 +264,61 @@264264 	assert line_head == history.head(b)!.id265265 }266266 267+// A person deciding a clash may take minutes; the question must come before268+// the write lock, or every agent's save on this repo waits and then fails.269+fn test_clash_questions_are_asked_before_the_write_lock() {270+	base := scratch('ask_unlocked')!271+	defer {272+		os.rmdir_all(base) or {}273+	}274+	mut backend := remote.new_folder(os.join_path(base, 'remote'))!275+	mut a := open_repo(machine('ask_a')!)!276+	defer {277+		a.close()278+	}279+	mut b := open_repo(machine('ask_b')!)!280+	defer {281+		b.close()282+	}283+	put(a.root, 'same.txt', 'base\n')!284+	commit(a, 'base')!285+	offer_now(a, mut backend)!286+	join(b, mut backend)!287+	put(a.root, 'same.txt', 'A version\n')!288+	commit(a, 'A edits same')!289+	offer_now(a, mut backend)!290+	put(b.root, 'same.txt', 'B version\n')!291+	commit(b, 'B edits same')!292+	root := b.root293+	// An agent saves while the person is still choosing.294+	if _ := steal(b, mut backend, StealOptions{295+		ask: fn [root] (clash Clash) Side {296+			other := storage.open(root) or { panic(err) }297+			defer {298+				other.close()299+			}300+			os.write_file(os.join_path(root, 'agent.txt'), 'saved meanwhile\n') or { panic(err) }301+			history.commit_worktree(other, root, snapshot.load(root), history.SaveInput{302+				message: 'agent saves during the question'303+			}) or { panic('steal held the write lock while asking: ${err}') }304+			return .mine305+		}306+	})307+	{308+		assert false, 'a steal whose line moved during the question must refuse'309+	} else {310+		assert err.msg().contains('changed while')311+	}312+	// The agent's save is still head, and a second steal combines it too.313+	assert history.head(b)!.message == 'agent saves during the question'314+	report := steal(b, mut backend, StealOptions{315+		mine: true316+	})!317+	assert report.kept_mine == 1318+	assert os.read_file(os.join_path(b.root, 'agent.txt'))! == 'saved meanwhile\n'319+	assert os.read_file(os.join_path(b.root, 'same.txt'))! == 'B version\n'320+}321+267322 fn test_clashing_files_are_chosen_one_at_a_time() {268323 	base := scratch('clash')!269324 	defer {@@ -828,4 +883,114 @@828883 	push_strip(a, mut backend, 'secrets.env')!829884 	steal_now(b, mut backend)!830885 	assert 'secrets.env' !in history.files_of(b, second.id)!.map(it.path)886+}887+888+// A clash on a path the other machine stripped is no clash: the strip removes889+// the path from every save, so there is nothing to choose.890+fn test_a_stripped_path_is_not_asked_about() {891+	base := scratch('clash_strip')!892+	defer {893+		os.rmdir_all(base) or {}894+	}895+	mut backend := remote.new_folder(os.join_path(base, 'remote'))!896+	mut a := open_repo(machine('clash_strip_a')!)!897+	defer {898+		a.close()899+	}900+	mut b := open_repo(machine('clash_strip_b')!)!901+	defer {902+		b.close()903+	}904+	put(a.root, 'secrets.env', 'key one\n')!905+	commit(a, 'base')!906+	offer_now(a, mut backend)!907+	join(b, mut backend)!908+	put(a.root, 'secrets.env', 'key two\n')!909+	commit(a, 'A rotates')!910+	offer_now(a, mut backend)!911+	strip_local(a, 'secrets.env', '')!912+	push_strip(a, mut backend, 'secrets.env')!913+	put(b.root, 'secrets.env', 'key three\n')!914+	mine := commit(b, 'B rotates')!915+	report := steal(b, mut backend, StealOptions{})!916+	assert report.clashes.len == 0917+	for save in history.all_saves(b)! {918+		assert 'secrets.env' !in history.files_of(b, save.id)!.map(it.path)919+	}920+	assert history.by_id(b, mine.id) != none921+}922+923+// A strip scoped to the shared base save changes what the replay sees; the924+// clashes must be found on the same trees, or the replay meets a conflict.925+fn test_a_strip_of_the_shared_base_is_seen_before_asking() {926+	base := scratch('clash_base_strip')!927+	defer {928+		os.rmdir_all(base) or {}929+	}930+	mut backend := remote.new_folder(os.join_path(base, 'remote'))!931+	mut a := open_repo(machine('base_strip_a')!)!932+	defer {933+		a.close()934+	}935+	mut b := open_repo(machine('base_strip_b')!)!936+	defer {937+		b.close()938+	}939+	put(a.root, 'notes.txt', 'shared\n')!940+	common := commit(a, 'base')!941+	offer_now(a, mut backend)!942+	join(b, mut backend)!943+	put(a.root, 'other.txt', 'A moves on\n')!944+	commit(a, 'A moves on')!945+	offer_now(a, mut backend)!946+	strip_local(a, 'notes.txt', common.id)!947+	push_strip(a, mut backend, 'notes.txt')!948+	put(b.root, 'notes.txt', 'B edits\n')!949+	commit(b, 'B edits notes')!950+	if _ := steal(b, mut backend, StealOptions{}) {951+		assert false, 'with the base stripped, notes.txt differs on both sides'952+	} else {953+		assert err.msg().contains('changed on both machines')954+	}955+	report := steal(b, mut backend, StealOptions{956+		mine: true957+	})!958+	assert report.clashes == ['notes.txt']959+	assert os.read_file(os.join_path(b.root, 'notes.txt'))! == 'B edits\n'960+}961+962+// A folder with files but no saves has nothing to combine with; a steal that963+// fast-forwarded into it would overwrite whatever shares a name.964+fn test_steal_into_a_folder_with_unsaved_files_refuses() {965+	base := scratch('steal_unsaved')!966+	defer {967+		os.rmdir_all(base) or {}968+	}969+	mut backend := remote.new_folder(os.join_path(base, 'remote'))!970+	mut a := open_repo(machine('unsaved_a')!)!971+	defer {972+		a.close()973+	}974+	put(a.root, 'notes.txt', 'from A\n')!975+	commit(a, 'A writes')!976+	offer_now(a, mut backend)!977+	b_root := machine('unsaved_b')!978+	mut b := storage.create(b_root)!979+	defer {980+		b.close()981+	}982+	put(b_root, 'notes.txt', 'never saved\n')!983+	// Only a repo that already knows its remote name gets this far.984+	b.set_state('remote_repo', 'proj')!985+	if _ := steal(b, mut backend, StealOptions{}) {986+		assert false, 'steal overwrote a folder with unsaved files'987+	} else {988+		assert err.msg() == no_saves_here989+	}990+	assert os.read_file(os.join_path(b_root, 'notes.txt'))! == 'never saved\n'991+	// A fresh `koh init` folder, holding only the default ignore list, is fine.992+	os.rm(os.join_path(b_root, 'notes.txt'))!993+	put(b_root, snapshot.ignore_file, snapshot.default_ignore)!994+	steal(b, mut backend, StealOptions{})!995+	assert os.read_file(os.join_path(b_root, 'notes.txt'))! == 'from A\n'831996 }

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