d118485757e6

close the third review's three findings on the 3.8.6 Doctor fixes

time
2026-10-07 17:01 UTC · 5 days ago
machine
Junes-MacBook-Pro.local
agent
claude-opus-5-5
session
resolve doctor safety findings
parent
b0f012ef7fe0
4 files src/exchange/exchange.v+10 −5src/exchange/exchange_test.v+80 −0src/exchange/repair.v+1 −1src/exchange/strip.v+10 −1
src/exchange/exchange.v+10 −5
@@ -227,10 +227,12 @@227227 	if strips.len == 0 {228228 		return false229229 	}230+	// Objects go first: a record that still lists the path is what lets231+	// Doctor's remote audit find the strip again if this stops part way.232+	drop_stripped_objects(store, mut backend, repo, uploaded, strips)!230233 	for id in uploaded.saves {231234 		strip_record(mut backend, save_key(repo, id), id, strips)!232235 	}233-	drop_stripped_objects(store, mut backend, repo, uploaded, strips)!234236 	return true235237 }236238 @@ -285,8 +287,8 @@285287 }286288 287289 // drop_stripped_objects deletes what an upload put that a strip reaches in a288-// save here, unless a remote record still lists it: another machine may hold289-// the same bytes under a path no strip names.290+// save here, unless a remote record lists it under a path no strip covers:291+// another machine may hold the same bytes there.290292 fn drop_stripped_objects(store &storage.Store, mut backend remote.Backend, repo string, uploaded Uploaded, strips []events.Event) ! {291293 	mut reached := map[string]bool{}292294 	for save in history.all_saves(store)! {@@ -302,8 +304,11 @@302304 	for key in backend.list('${repo}/saves')! {303305 		object := backend.get(key)!304306 		if object.found {305-			for file in decode_save(object.body)!.files {306-				reached.delete(file.hash)307+			record := decode_save(object.body)!308+			for file in record.files {309+				if !strips.any(reaches(it, record.id) && covers(it.path, file.path)) {310+					reached.delete(file.hash)311+				}307312 			}308313 		}309314 	}

view file

src/exchange/exchange_test.v+80 −0
@@ -1123,6 +1123,7 @@11231123 	// fail every put after the one the strip raced11241124 	fail_after_race bool11251125 	raced_put       bool1126+	fail_delete     bool11261127 }11271128 11281129 fn (mut r StripRace) get(key string) !remote.Object {@@ -1151,6 +1152,9 @@11511152 }11521153 11531154 fn (mut r StripRace) delete(key string) ! {1155+	if r.fail_delete {1156+		return error('injected delete failure')1157+	}11541158 	r.inner.delete(key)!11551159 }11561160 @@ -1228,6 +1232,8 @@12281232 		assert false, 'a resend that raced a strip must refuse'12291233 	} else {12301234 		assert err.msg().contains('koh steal'), err.msg()1235+		// The restored record is still a repair Doctor made.1236+		assert (err as Partway).done == 'uploaded 0 objects and 1 save record'12311237 	}12321238 	// Head names the restored record, so it stays, less the stripped path.12331239 	record := decode_save(backend.get(save_key('proj', first.id))!.body)!@@ -1316,6 +1322,80 @@13161322 		assert false, 'a note naming a path outside the objects must be refused'13171323 	}13181324 	assert os.read_file(victim)! == 'another repo'1325+}1326+1327+// A settlement that stops part way leaves the record naming the stripped1328+// path, which is what lets Doctor send the strip again once this repo has it.1329+fn test_a_failed_settlement_leaves_residue_doctor_can_finish() {1330+	base := scratch('settle_fail')!1331+	defer {1332+		os.rmdir_all(base) or {}1333+	}1334+	mut backend := remote.new_folder(os.join_path(base, 'remote'))!1335+	mut a := open_repo(machine('settle_fail_a')!)!1336+	defer {1337+		a.close()1338+	}1339+	mut b := open_repo(machine('settle_fail_b')!)!1340+	defer {1341+		b.close()1342+	}1343+	put(a.root, 'secrets.env', 'api_key=1\n')!1344+	first := make_save(a, 'leak the key')!1345+	offer_now(a, mut backend)!1346+	join(b, mut backend)!1347+	secret := object_key('proj', storage.hash_hex('api_key=1\n'.bytes()))1348+	backend.delete(save_key('proj', first.id))!1349+	backend.delete(secret)!1350+	mut race := StripRace{1351+		inner:       backend1352+		other:       &b1353+		path:        'secrets.env'1354+		fail_delete: true1355+	}1356+	if _ := resend(a, mut race, 'proj', [], [first.id]) {1357+		assert false, 'the injected delete failure should have surfaced'1358+	}1359+	record := decode_save(backend.get(save_key('proj', first.id))!.body)!1360+	assert 'secrets.env' in record.files.map(it.path)1361+	steal_now(a, mut backend)!1362+	requeue_strip(a, 'secrets.env')!1363+	send_strip(a, mut backend, 'secrets.env')!1364+	cleaned := decode_save(backend.get(save_key('proj', first.id))!.body)!1365+	assert 'secrets.env' !in cleaned.files.map(it.path)1366+	assert !backend.get(secret)!.found1367+}1368+1369+// A strip whose deletes fail after its rewrite keeps, in its note, the objects1370+// the rewritten record no longer names, so the retry still deletes them.1371+fn test_a_strip_retry_deletes_objects_its_rewrite_found() {1372+	base := scratch('strip_found')!1373+	defer {1374+		os.rmdir_all(base) or {}1375+	}1376+	remote_dir := os.join_path(base, 'remote')1377+	mut backend := remote.new_folder(remote_dir)!1378+	mut a := open_repo(machine('strip_found_a')!)!1379+	defer {1380+		a.close()1381+	}1382+	put(a.root, 'secrets.env', 'api_key=1\n')!1383+	make_save(a, 'leak the key')!1384+	offer_now(a, mut backend)!1385+	strip_local(a, 'secrets.env', '')!1386+	// A note rebuilt from the event alone, as Doctor's residue repair writes it.1387+	a.remove_state(pending_key('secrets.env'))!1388+	requeue_strip(a, 'secrets.env')!1389+	mut flaky := FaultyBackend{1390+		inner:       remote.new_folder(remote_dir)!1391+		fail_delete: true1392+	}1393+	if _ := send_strip(a, mut flaky, 'secrets.env') {1394+		assert false, 'the injected delete failure should have surfaced'1395+	}1396+	send_strip(a, mut backend, 'secrets.env')!1397+	assert !backend.get(object_key('proj', storage.hash_hex('api_key=1\n'.bytes())))!.found1398+	assert !strip_pending(a, 'secrets.env')13191399 }13201400 13211401 // MovingRecord stands in for another machine rewriting a record between this

view file

src/exchange/repair.v+1 −1
@@ -91,7 +91,7 @@9191 	if undo_late_strips(store, mut backend, repo, uploaded) or {9292 		return partway(uploaded.done(), err)9393 	} {94-		return error(strip_not_applied)94+		return partway(uploaded.done(), error(strip_not_applied))9595 	}9696 	return uploaded.objects.len + uploaded.saves.len9797 }

view file

src/exchange/strip.v+10 −1
@@ -170,7 +170,7 @@170170 }171171 172172 fn send_steps(store &storage.Store, mut backend remote.Backend, path string, mut report SendReport) ! {173-	note := read_pending(store, path)!173+	mut note := read_pending(store, path)!174174 	name := repo_name(store, mut backend)!175175 	mut upload := OfferReport{176176 		repo: name@@ -190,9 +190,11 @@190190 		}191191 		record := decode_save(remote_object.body)!192192 		mut kept := []FileRecord{}193+		mut dropped := []string{}193194 		for file in record.files {194195 			if (every_save || record.id in note.saves) && covers(path, file.path) {195196 				freed[file.hash] = true197+				dropped << file.hash196198 				continue197199 			}198200 			kept << file@@ -200,6 +202,13 @@200202 		}201203 		if kept.len == record.files.len {202204 			continue205+		}206+		// Once rewritten, the record no longer names these objects; the note207+		// keeps them so a retry can still delete them.208+		unnoted := dropped.filter(it !in note.hashes)209+		if unnoted.len > 0 {210+			note.hashes << unnoted211+			store.set_state(pending_key(path), json2.encode(note))!203212 		}204213 		if !backend.put_if_match(key, encode_save(rewrite(record, kept)).bytes(), remote_object.tag)! {205214 			return error('the remote copy of save ${record.id} moved while stripping — run koh steal, then strip again')

view file