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

@@ -430,6 +430,7 @@430430 	strip_residue        bool431431 	strip_residue_paths  []string432432 	remote_strip_residue []string433+	upload_unsettled     bool433434 	remote_bad_objects   []string434435 	remote_missing_saves []string435436 }

@@ -542,7 +542,9 @@542542 	report := DoctorJson{543543 		remote_repo:          'repo'544544 		remote_strip_residue: ['app', 'bad']545+		upload_unsettled:     true545546 	}547+	assert plan_repairs(store, report).any(it.action.contains('interrupted upload'))546548 	repairs := plan_repairs(store, report).filter(it.action.contains('remote still holds'))547549 	assert repairs.len == 1548550 	if _ := repairs[0].run() {

@@ -144,6 +144,7 @@144144 	report.head_id = stats.state[storage.head_key]145145 	report.restore_unfinished = 'restoring' in stats.state146146 	report.pending_strips = stats.pending147+	report.upload_unsettled = exchange.unsettled_key in stats.state147148 	report.check('references and metadata', stats.problems)148149 	saves := history.all_saves(store)!149150 	report.check('history links and cycles', doctor_history(saves, report.head_id))@@ -176,6 +177,7 @@176177 	for path in stats.pending {177178 		exchange.read_pending(store, path) or { report.problems << err.msg() }178179 	}180+	exchange.unsettled(store) or { report.problems << err.msg() }179181 	report.check('saved paths and events', report.problems[before..].clone())180182 	if report.restore_unfinished {181183 		report.check('restore state', [snapshot.restore_unfinished])@@ -184,6 +186,9 @@184186 	}185187 	for path in report.pending_strips {186188 		report.warnings << 'remote strip unfinished for ${snapshot.escape_path(path)}; retry koh strip ${shell_quote(path)}'189+	}190+	if report.upload_unsettled {191+		report.warnings << 'an upload stopped before it could remove what a remote strip covers; after koh steal, koh doctor can finish it'187192 	}188193 	if stats.unused > 0 {189194 		report.warnings << '${fmt.plural(int(stats.unused), 'unreferenced object')} retained; no save uses them and koh doctor can remove them'

@@ -89,6 +89,12 @@8989 			send_strips(store, remote_repo, paths, true)!9090 		}}9191 	}92+	if report.upload_unsettled && writable_remote {93+		repairs << DoctorRepair{'delete what an interrupted upload sent that a strip removed', fn [store, remote_repo] () ! {94+			mut backend, repo := doctor_backend(store, '', remote_repo)!95+			exchange.settle_unsettled(store, mut backend, repo)!96+		}}97+	}9298 	// With damaged rows an unused object may be the only copy of saved content.9399 	if report.unreferenced_objects > 0 && report.problems.len == 0 {94100 		repairs << DoctorRepair{'delete ${fmt.plural(int(report.unreferenced_objects), 'object')} no save uses', fn [store] () ! {

@@ -10,6 +10,7 @@1010 1111 module exchange1212 13+import json21314 import os1415 import src.events1516 import src.fmt@@ -217,13 +218,34 @@217218 	return failure218219 }219220 220-// undo_late_strips applies strips that reached the remote during an upload to221-// what the upload put, and reports whether there were any. Records stay, less222-// the stripped paths: another offer, or head itself, may already name them.223-// Each strip's note is written first, so if this stops part way, Doctor sends224-// the strip again once a steal brings it here. Objects go last, so no record225-// is left naming one that is gone.221+// unsettled_key keeps what an upload sent when settling a late strip failed,222+// for Doctor to finish once a steal brings the strip here.223+pub const unsettled_key = 'upload_unsettled'224+225+pub fn unsettled(store &storage.Store) ![]string {226+	text := store.state(unsettled_key) or { return []string{} }227+	return json2.decode[[]string](text) or {228+		return error('the note of an unsettled upload is unreadable; it is kept for recovery')229+	}230+}231+226232 fn undo_late_strips(store &storage.Store, mut backend remote.Backend, repo string, uploaded Uploaded) !bool {233+	return settle_late_strips(store, mut backend, repo, uploaded) or {234+		failure := err235+		mut kept := unsettled(store) or { return error('${failure.msg()}; ${err.msg()}') }236+		kept << uploaded.objects.keys().filter(it !in kept)237+		store.set_state(unsettled_key, json2.encode(kept)) or {238+			return error('${failure.msg()}; ${err.msg()}')239+		}240+		return failure241+	}242+}243+244+// settle_late_strips applies strips that reached the remote during an upload245+// to what it put, and reports whether there were any. Records stay, less the246+// stripped paths, as another offer or head may name them. Notes go first and247+// objects last, so a stop part way leaves a note and no record without its object.248+fn settle_late_strips(store &storage.Store, mut backend remote.Backend, repo string, uploaded Uploaded) !bool {227249 	strips := unapplied_strips(store, mut backend, repo)!228250 	if strips.len == 0 {229251 		return false

@@ -733,6 +733,8 @@733733 	// the first `format` write loses, to a value this koh does not speak.734734 	race_format bool735735 	raced       bool736+	// fail listing keys under a prefix ending in this737+	fail_list string736738 }737739 738740 fn (mut f FaultyBackend) get(key string) !remote.Object {@@ -744,6 +746,9 @@744746 }745747 746748 fn (mut f FaultyBackend) list(prefix string) ![]string {749+	if f.fail_list != '' && prefix.ends_with(f.fail_list) {750+		return error('injected list failure')751+	}747752 	return f.inner.list(prefix)!748753 }749754 @@ -1127,6 +1132,8 @@11271132 	fail_match      bool11281133 	// race at the first object instead of the first save record11291134 	on_objects bool1135+	// fail listing events once the strip has raced1136+	fail_events_after_race bool11301137 }11311138 11321139 fn (mut r StripRace) get(key string) !remote.Object {@@ -1134,6 +1141,9 @@11341141 }11351142 11361143 fn (mut r StripRace) list(prefix string) ![]string {1144+	if r.fail_events_after_race && r.raced_put && prefix.ends_with('/events') {1145+		return error('injected list failure')1146+	}11371147 	return r.inner.list(prefix)!11381148 }11391149 @@ -1433,6 +1443,75 @@14331443 	send_strip(a, mut backend, 'secrets.env')!14341444 	assert !backend.get(object_key('proj', hash))!.found14351445 	assert !strip_pending(a, 'secrets.env')1446+}1447+1448+// When the late check itself fails, what the upload sent is kept, and Doctor1449+// deletes what the strip removed once a steal brings the strip here.1450+fn test_an_unsettled_upload_is_finished_after_steal() {1451+	base := scratch('unsettled')!1452+	defer {1453+		os.rmdir_all(base) or {}1454+	}1455+	mut backend := remote.new_folder(os.join_path(base, 'remote'))!1456+	mut a := open_repo(machine('unsettled_a')!)!1457+	defer {1458+		a.close()1459+	}1460+	mut b := open_repo(machine('unsettled_b')!)!1461+	defer {1462+		b.close()1463+	}1464+	put(a.root, 'secrets.env', 'api_key=1\n')!1465+	make_save(a, 'leak the key')!1466+	offer_now(a, mut backend)!1467+	join(b, mut backend)!1468+	hash := storage.hash_hex('api_key=1\n'.bytes())1469+	backend.delete(object_key('proj', hash))!1470+	mut race := StripRace{1471+		inner:                  backend1472+		other:                  &b1473+		path:                   'secrets.env'1474+		on_objects:             true1475+		fail_events_after_race: true1476+	}1477+	if _ := resend(a, mut race, 'proj', [hash], []) {1478+		assert false, 'the injected list failure should have surfaced'1479+	}1480+	assert unsettled(a)! == [hash]1481+	if _ := settle_unsettled(a, mut backend, 'proj') {1482+		assert false, 'settling before the strip is here would keep its bytes'1483+	}1484+	steal_now(a, mut backend)!1485+	settle_unsettled(a, mut backend, 'proj')!1486+	assert !backend.get(object_key('proj', hash))!.found1487+	assert unsettled(a)!.len == 01488+}1489+1490+// A strip that sent its event before failing says so.1491+fn test_a_strip_that_fails_after_its_event_reports_it() {1492+	base := scratch('strip_event_partway')!1493+	defer {1494+		os.rmdir_all(base) or {}1495+	}1496+	remote_dir := os.join_path(base, 'remote')1497+	mut backend := remote.new_folder(remote_dir)!1498+	mut a := open_repo(machine('strip_event_partway_a')!)!1499+	defer {1500+		a.close()1501+	}1502+	put(a.root, 'secrets.env', 'api_key=1\n')!1503+	make_save(a, 'leak the key')!1504+	offer_now(a, mut backend)!1505+	strip_local(a, 'secrets.env', '')!1506+	mut flaky := FaultyBackend{1507+		inner:     remote.new_folder(remote_dir)!1508+		fail_list: '/saves'1509+	}1510+	if _ := send_strip(a, mut flaky, 'secrets.env') {1511+		assert false, 'the injected list failure should have surfaced'1512+	} else {1513+		assert (err as Partway).done.starts_with('sent its event'), (err as Partway).done1514+	}14361515 }14371516 14381517 // A strip whose deletes fail after its rewrite keeps, in its note, the objects

@@ -131,6 +131,30 @@131131 	}132132 }133133 134+/// Delete what an upload sent before a late strip it could not settle: each135+/// object no remote record lists that this repo, with every remote strip136+/// applied, no longer holds.137+pub fn settle_unsettled(store &storage.Store, mut backend remote.Backend, repo string) ! {138+	if unapplied_strips(store, mut backend, repo)!.len > 0 {139+		return error(strip_not_applied)140+	}141+	mut needed := map[string]bool{}142+	for key in backend.list('${repo}/saves')! {143+		object := backend.get(key)!144+		if object.found {145+			for file in decode_save(object.body)!.files {146+				needed[file.hash] = true147+			}148+		}149+	}150+	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))!153+		}154+	}155+	store.remove_state(unsettled_key)!156+}157+134158 /// Remove what recorded strips cover but a save still holds, without135159 /// recording a new strip.136160 pub fn reapply_strips(store &storage.Store) !int {

@@ -156,8 +156,8 @@156156 	mut report := SendReport{}157157 	send_steps(store, mut backend, path, mut report) or {158158 		mut done := []string{}159-		if report.records + report.objects > 0 {160-			done << 'rewrote ${fmt.plural(report.records, 'remote record')} and deleted ${fmt.plural(report.objects, 'remote object')} for ${path}'159+		if report.event_sent || report.records + report.objects > 0 {160+			done << '${if report.event_sent { 'sent its event, ' } else { '' }}rewrote ${fmt.plural(report.records, 'remote record')} and deleted ${fmt.plural(report.objects, 'remote object')} for ${path}'161161 		}162162 		return partway(done, err)163163 	}@@ -170,8 +170,10 @@170170 	mut upload := OfferReport{171171 		repo: name172172 	}173+	defer {174+		report.event_sent = upload.events > 0175+	}173176 	upload_events(store, mut backend, name, mut upload)!174-	report.event_sent = upload.events > 0175177 	mut freed := map[string]bool{}176178 	for hash in note.hashes {177179 		freed[hash] = true