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
↓ koh-d1184857.tar.gzatlas at this save
files changed4 files
src/exchange/exchange.v+10 −5src/exchange/exchange_test.v+80 −0src/exchange/repair.v+1 −1src/exchange/strip.v+10 −1src/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 }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 thissrc/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 }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')