read the tree again under the lock when head moved during a save, so a save can never undo a steal or undo that landed meanwhile (3.0.46)

@@ -1 +1 @@1-3.0.451+3.0.46

@@ -170,7 +170,10 @@170170 171171 fn record_restore(store &storage.Store, target history.Save, verb string, plan RestorePlan, discard_first bool) !history.Save {172172 	if discard_first {173-		history.commit_worktree(store, store.root, snapshot.load(store.root), history.SaveInput{173+		// Recorded even while a restore is unfinished: finishing it is what174+		// revert is for. The mark keeps every other writer off head meanwhile.175+		history.commit_files(store, store.root, snapshot.walk(store, store.root,176+			snapshot.load(store.root))!, history.SaveInput{174177 			message:       'revert: discarded changes'175178 			is_checkpoint: true176179 		})!

@@ -26,9 +26,6 @@2626 	defer {2727 		store.close()2828 	}29-	if snapshot.restore_pending(store) {30-		return refuse(args.json(), snapshot.restore_unfinished)31-	}3229 	save := history.commit_worktree(store, store.root, snapshot.load(store.root), history.SaveInput{3330 		message:       message3431 		agent:         args.value('agent')

@@ -29,17 +29,49 @@2929 pub const nothing_changed = 'nothing has changed since the last save'3030 3131 // commit_worktree walks the working tree and records it. A plain save refuses32-// when the tree matches head; a checkpoint always records.32+// when the tree matches head; a checkpoint always records. The walk runs33+// before the lock, so other commands are not held up by the hashing.3334 pub fn commit_worktree(store &storage.Store, root string, ig snapshot.Ignore, input SaveInput) !Save {35+	walked_head := head_id(store)3436 	files := snapshot.walk(store, root, ig)!37+	store.begin()!38+	save := commit_walk(store, root, input, walked_head, files) or {39+		store.rollback()40+		return err41+	}42+	store.commit()!43+	refresh_log_md(store)44+	return save45+}46+47+// commit_walk records a walk inside the caller's transaction. A restore still48+// writing its files is refused: the tree is half one save and half another.49+// When another command moved head after the walk, the walk is stale, and50+// recording it on the new head would quietly undo that command, so the tree51+// (and its ignore rules, which may have changed too) is read again under the52+// lock.53+fn commit_walk(store &storage.Store, root string, input SaveInput, walked_head string, walked []snapshot.FileEntry) !Save {54+	if snapshot.restore_pending(store) {55+		return error(snapshot.restore_unfinished)56+	}57+	mut files := walked.clone()58+	moved := head_id(store) != walked_head59+	if moved {60+		files = snapshot.walk(store, root, snapshot.load(root))!61+	}3562 	if input.restores == '' && !input.is_checkpoint {3663 		if previous := head(store) {3764 			if snapshot.same_tree(files, files_of(store, previous.id)!) {65+				if moved {66+					// Another agent's save, made while this one waited for the67+					// lock, already holds these changes.68+					return error('${nothing_changed} — [${previous.id}] "${previous.message}" was saved a moment ago and already holds this tree')69+				}3870 				return error(nothing_changed)3971 			}4072 		}4173 	}42-	return commit_files(store, root, files, input)!74+	return record_save(store, root, files, input)!4375 }4476 4577 // commit_files records an already-known tree. `root` is '' when the objects are

@@ -147,3 +147,144 @@147147 	assert latest_checkpoint(store)?.id == kept.id148148 	assert session_saves(store, 'work')!.map(it.id) == [kept.id]149149 }150+151+// A save walks the tree before it takes the lock. If another command moved152+// head meanwhile — a steal or an undo whose files are still being written — the153+// walk is stale, and recording it on top of the new head would quietly undo154+// that command. The tree is read again under the lock instead, and a restore155+// still writing files is refused.156+fn test_a_save_whose_walk_went_stale_reads_the_tree_again() {157+	mut store, dir := temp_repo('stale_walk')!158+	defer {159+		store.close()160+		os.rmdir_all(dir) or {}161+	}162+	write_file(dir, 'a.txt', 'one\n')!163+	first := commit_worktree(store, dir, snapshot.Ignore{}, SaveInput{164+		message: 'first'165+	})!166+	write_file(dir, 'b.txt', 'mine\n')!167+	stale := snapshot.walk(store, dir, snapshot.Ignore{})!168+	// Another command lands a save and its files between that walk and the lock.169+	write_file(dir, 'a.txt', 'two\n')!170+	other := commit_files(store, dir,171+		snapshot.walk(store, dir, snapshot.Ignore{})!.filter(it.path == 'a.txt'), SaveInput{172+		message: 'somebody else'173+	})!174+	store.begin()!175+	saved := commit_walk(store, dir, SaveInput{176+		message: 'mine'177+	}, first.id, stale)!178+	store.commit()!179+	assert saved.parent == other.id180+	a := files_of(store, saved.id)!.filter(it.path == 'a.txt')181+	assert a.len == 1 && a[0].hash == storage.hash_hex('two\n'.bytes())182+183+	// A restore that moved head and is still writing files: no save on top.184+	stale_again := snapshot.walk(store, dir, snapshot.Ignore{})!185+	snapshot.mark_restore(store)!186+	moved := commit_files(store, '', files_of(store, first.id)!, SaveInput{187+		message:  'undo: ${first.id}'188+		restores: first.id189+	})!190+	store.begin()!191+	if _ := commit_walk(store, dir, SaveInput{192+		message: 'on a half tree'193+	}, saved.id, stale_again)194+	{195+		assert false, 'a save must not land on a restore still writing files'196+	} else {197+		assert err.msg() == snapshot.restore_unfinished198+	}199+	store.rollback()200+	assert head_id(store) == moved.id201+}202+203+// Two agents saving the same shared tree: the one that waited is told whose204+// save already holds its changes, not just that nothing changed.205+fn test_a_save_beaten_to_the_same_tree_names_the_save_that_holds_it() {206+	mut store, dir := temp_repo('beaten')!207+	defer {208+		store.close()209+		os.rmdir_all(dir) or {}210+	}211+	write_file(dir, 'a.txt', 'one\n')!212+	first := commit_worktree(store, dir, snapshot.Ignore{}, SaveInput{213+		message: 'first'214+	})!215+	write_file(dir, 'b.txt', 'both agents\n')!216+	walked := snapshot.walk(store, dir, snapshot.Ignore{})!217+	winner := commit_worktree(store, dir, snapshot.Ignore{}, SaveInput{218+		message: 'agent one'219+	})!220+	store.begin()!221+	if _ := commit_walk(store, dir, SaveInput{222+		message: 'agent two'223+	}, first.id, walked)224+	{225+		assert false, 'the same tree must not be saved twice'226+	} else {227+		assert err.msg().contains('[${winner.id}] "agent one"')228+	}229+	store.rollback()230+}231+232+// The save may read head just after a restore moved it, so its walk is not233+// stale but the files are still landing: the mark alone decides.234+fn test_a_save_after_a_restore_moved_head_waits_for_its_files() {235+	mut store, dir := temp_repo('after_restore')!236+	defer {237+		store.close()238+		os.rmdir_all(dir) or {}239+	}240+	write_file(dir, 'a.txt', 'one\n')!241+	first := commit_worktree(store, dir, snapshot.Ignore{}, SaveInput{242+		message: 'first'243+	})!244+	write_file(dir, 'a.txt', 'two\n')!245+	commit_worktree(store, dir, snapshot.Ignore{}, SaveInput{246+		message: 'second'247+	})!248+	snapshot.mark_restore(store)!249+	moved := commit_files(store, '', files_of(store, first.id)!, SaveInput{250+		message:  'undo: ${first.id}'251+		restores: first.id252+	})!253+	// Head read after the move, tree still the old one: not stale, but half.254+	if _ := commit_worktree(store, dir, snapshot.Ignore{}, SaveInput{255+		message: 'on a half tree'256+	})257+	{258+		assert false, 'a save must not land while a restore writes its files'259+	} else {260+		assert err.msg() == snapshot.restore_unfinished261+	}262+	assert head_id(store) == moved.id263+}264+265+// A .kohignore change that lands with the other command must apply to the266+// tree read again under the lock, or a newly ignored file would be saved.267+fn test_a_stale_walk_is_read_again_with_the_current_ignore_rules() {268+	mut store, dir := temp_repo('stale_rules')!269+	defer {270+		store.close()271+		os.rmdir_all(dir) or {}272+	}273+	write_file(dir, 'a.txt', 'one\n')!274+	first := commit_worktree(store, dir, snapshot.Ignore{}, SaveInput{275+		message: 'first'276+	})!277+	write_file(dir, 'secret.env', 'key\n')!278+	stale := snapshot.walk(store, dir, snapshot.Ignore{})!279+	write_file(dir, '.kohignore', 'secret.env\n')!280+	commit_files(store, dir, snapshot.walk(store, dir, snapshot.load(dir))!, SaveInput{281+		message: 'ignore the secret'282+	})!283+	write_file(dir, 'a.txt', 'two\n')!284+	store.begin()!285+	saved := commit_walk(store, dir, SaveInput{286+		message: 'mine'287+	}, first.id, stale)!288+	store.commit()!289+	assert !files_of(store, saved.id)!.any(it.path == 'secret.env')290+}

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