573800eb25d3

close the sixth review's two findings on the 3.8.6 Doctor fixes

time
2026-10-07 17:34 UTC · 5 days ago
machine
Junes-MacBook-Pro.local
agent
claude-opus-5-5
session
resolve doctor safety findings
parent
35a69dc83f0d
5 files src/cli/doctor_repair.v+4 −9src/exchange/exchange.v+14 −3src/exchange/exchange_test.v+44 −1src/exchange/repair.v+11 −4src/exchange/strip.v+2 −1
src/cli/doctor_repair.v+4 −9
@@ -125,7 +125,7 @@125125 	mut lost := []string{}126126 	for index, hash in hashes {127127 		restored := restore_object(store, hash, source, remote_repo) or {128-			return exchange.partway(done_text('restored', index - lost.len, 'damaged or missing object'),128+			return exchange.partway(exchange.done_text('restored', index - lost.len, 'damaged or missing object'),129129 				err)130130 		}131131 		if !restored {@@ -134,7 +134,7 @@134134 	}135135 	if lost.len > 0 {136136 		failure := error('no verified copy here or on the remote of ${lost.join(', ')}')137-		return exchange.partway(done_text('restored', hashes.len - lost.len, 'damaged or missing object'),137+		return exchange.partway(exchange.done_text('restored', hashes.len - lost.len, 'damaged or missing object'),138138 			failure)139139 	}140140 }@@ -144,18 +144,13 @@144144 	for index, path in paths {145145 		if requeue {146146 			exchange.requeue_strip(store, path) or {147-				return exchange.partway(done_text('sent', index, 'strip'), err)147+				return exchange.partway(exchange.done_text('sent', index, 'strip'), err)148148 			}149149 		}150150 		exchange.send_strip(store, mut backend, path) or {151-			return exchange.partway(done_text('sent', index, 'strip'), err)151+			return exchange.partway(exchange.done_text('sent', index, 'strip'), err)152152 		}153153 	}154-}155-156-// done_text is what a repair finished before it failed, if anything.157-fn done_text(verb string, done int, noun string) []string {158-	return if done == 0 { []string{} } else { ['${verb} ${fmt.plural(done, noun)}'] }159154 }160155 161156 fn restore_object(store &storage.Store, hash string, source string, remote_repo string) !bool {

view file

src/exchange/exchange.v+14 −3
@@ -294,6 +294,11 @@294294 	return e.reason295295 }296296 297+// done_text is what a step finished before it failed, if anything.298+pub fn done_text(verb string, done int, noun string) []string {299+	return if done == 0 { []string{} } else { ['${verb} ${fmt.plural(done, noun)}'] }300+}301+297302 // partway adds what was `done` before `failure` to it, including what a298303 // failing step of its own had done.299304 pub fn partway(done []string, failure IError) IError {@@ -315,10 +320,14 @@315320 // another machine may hold the same bytes there.316321 fn drop_stripped_objects(store &storage.Store, mut backend remote.Backend, repo string, uploaded Uploaded, strips []events.Event) ! {317322 	mut reached := map[string]bool{}323+	// A local save that still names it under another path may have no other copy.324+	mut kept := map[string]bool{}318325 	for save in history.all_saves(store)! {319-		for file in history.files_of(store, save.id)! {320-			if file.hash in uploaded.objects && strips.any(reaches(it, save.id) && covers(it.path, file.path)) {326+		for file in history.files_of(store, save.id)!.filter(it.hash in uploaded.objects) {327+			if strips.any(reaches(it, save.id) && covers(it.path, file.path)) {321328 				reached[file.hash] = true329+			} else {330+				kept[file.hash] = true322331 			}323332 		}324333 	}@@ -337,7 +346,9 @@337346 		}338347 	}339348 	for hash, _ in reached {340-		backend.delete(object_key(repo, hash))!349+		if hash !in kept {350+			backend.delete(object_key(repo, hash))!351+		}341352 	}342353 }343354 

view file

src/exchange/exchange_test.v+44 −1
@@ -4,6 +4,7 @@44 55 module exchange66 7+import json278 import os89 import src.events910 import src.history@@ -735,6 +736,9 @@735736 	raced       bool736737 	// fail listing keys under a prefix ending in this737738 	fail_list string739+	// fail every delete after this many succeed, 0 never740+	fail_delete_after int741+	deletes           int738742 }739743 740744 fn (mut f FaultyBackend) get(key string) !remote.Object {@@ -774,9 +778,10 @@774778 }775779 776780 fn (mut f FaultyBackend) delete(key string) ! {777-	if f.fail_delete {781+	if f.fail_delete || (f.fail_delete_after > 0 && f.deletes >= f.fail_delete_after) {778782 		return error('injected delete failure')779783 	}784+	f.deletes++780785 	f.inner.delete(key)!781786 }782787 @@ -1485,6 +1490,44 @@14851490 	settle_unsettled(a, mut backend, 'proj')!14861491 	assert !backend.get(object_key('proj', hash))!.found14871492 	assert unsettled(a)!.len == 01493+}1494+1495+// Settling never deletes what a local save still names, even when the local1496+// copy is damaged, and reports what it deleted before a failure.1497+fn test_settling_an_upload_keeps_named_objects_and_reports_deletions() {1498+	base := scratch('settle_named')!1499+	defer {1500+		os.rmdir_all(base) or {}1501+	}1502+	remote_dir := os.join_path(base, 'remote')1503+	mut backend := remote.new_folder(remote_dir)!1504+	mut a := open_repo(machine('settle_named_a')!)!1505+	defer {1506+		a.close()1507+	}1508+	put(a.root, 'app', 'app\n')!1509+	save := make_save(a, 'app')!1510+	offer_now(a, mut backend)!1511+	app := storage.hash_hex('app\n'.bytes())1512+	gone := ['gone one', 'gone two'].map(storage.hash_hex(it.bytes()))1513+	for hash in gone {1514+		backend.put_if_absent(object_key('proj', hash), 'stripped'.bytes())!1515+	}1516+	backend.delete(save_key('proj', save.id))!1517+	a.db.exec('PRAGMA foreign_keys=OFF')!1518+	a.db.exec("DELETE FROM objects WHERE hash = '${app}'")!1519+	a.set_state(unsettled_key, json2.encode([app, gone[0], gone[1]]))!1520+	mut flaky := FaultyBackend{1521+		inner:             remote.new_folder(remote_dir)!1522+		fail_delete_after: 11523+	}1524+	if _ := settle_unsettled(a, mut flaky, 'proj') {1525+		assert false, 'the injected delete failure should have surfaced'1526+	} else {1527+		assert (err as Partway).done == 'deleted 1 remote object', (err as Partway).done1528+	}1529+	assert backend.get(object_key('proj', app))!.found1530+	assert unsettled(a)!.len == 314881531 }14891532 14901533 // A strip that sent its event before failing says so.

view file

src/exchange/repair.v+11 −4
@@ -133,7 +133,7 @@133133 134134 /// Delete what an upload sent before a late strip it could not settle: each135135 /// object no remote record lists that this repo, with every remote strip136-/// applied, no longer holds.136+/// applied, neither holds nor names in a save.137137 pub fn settle_unsettled(store &storage.Store, mut backend remote.Backend, repo string) ! {138138 	if unapplied_strips(store, mut backend, repo)!.len > 0 {139139 		return error(strip_not_applied)@@ -147,12 +147,19 @@147147 			}148148 		}149149 	}150+	mut deleted := 0150151 	for hash in unsettled(store)! {151-		if hash !in needed && storage.valid_hash(hash) && !store.has_object(hash)! {152-			backend.delete(object_key(repo, hash))!152+		if hash !in needed && storage.valid_hash(hash) && !store.has_object(hash)!153+			&& store.paths_of_object(hash)!.len == 0 {154+			backend.delete(object_key(repo, hash)) or {155+				return partway(done_text('deleted', deleted, 'remote object'), err)156+			}157+			deleted++153158 		}154159 	}155-	store.remove_state(unsettled_key)!160+	store.remove_state(unsettled_key) or {161+		return partway(done_text('deleted', deleted, 'remote object'), err)162+	}156163 }157164 158165 /// Remove what recorded strips cover but a save still holds, without

view file

src/exchange/strip.v+2 −1
@@ -209,7 +209,8 @@209209 		report.records++210210 	}211211 	for hash, _ in freed {212-		if hash in needed || !storage.valid_hash(hash) || store.has_object(hash)! {212+		if hash in needed || !storage.valid_hash(hash) || store.has_object(hash)!213+			|| store.paths_of_object(hash)!.len > 0 {213214 			continue214215 		}215216 		backend.delete(object_key(name, hash))!

view file