[fix] Backport: lift a restore hold on blocks no holder has

src/shared/core/restore-hold-rules.js

@@ -1,18 +1,20 @@
 // When a restored core may be written again. A core restored from a key or a backup is at most as
 // long as the copies peers hold, and appending before catching up signs a second history at a length
 // they already have, which every peer that sees both refuses for good. So it stays read-only until
-// every connected holder has been matched and the copy is complete, and for a dwell after the first
-// holder answered, so a stale holder that happens to connect first does not decide alone. A profile
-// this device already holds data of, with no co-member in any space it knows, has no other holder. A
-// catalog (`emptyMayBeSolo`) is known with its space, so it has no other holder even when empty — a
-// profile restored from a key alone starts empty and knows no spaces yet, so it must wait.
+// every connected holder has been matched and holds no block the copy still lacks, and for a dwell
+// after the first holder answered, so a stale holder that happens to connect first does not decide
+// alone. A block no holder has does not hold the core: holders keep sparse copies, so it may never
+// arrive, and appending needs only the matched length. A profile this device already holds data of,
+// with no co-member in any space it knows, has no other holder. A catalog (`emptyMayBeSolo`) is known
+// with its space, so it has no other holder even when empty — a profile restored from a key alone
+// starts empty and knows no spaces yet, so it must wait.
 import { RESTORE_VERDICT } from '../contract/restore-verdict.js'
 
-export function releaseVerdict({ localLength, contiguousLength, holderLengths, firstHolderAt, now, dwellMs, coMembers, emptyMayBeSolo = false }) {
+export function releaseVerdict({ localLength, contiguousLength, holderLengths, firstHolderAt, now, dwellMs, coMembers, missingServable, emptyMayBeSolo = false }) {
   if (coMembers === 0 && (localLength > 0 || emptyMayBeSolo)) return RESTORE_VERDICT.CAUGHT_UP
   if (holderLengths.length === 0 || firstHolderAt === null) return RESTORE_VERDICT.NO_HOLDER
   if (localLength < Math.max(...holderLengths)) return RESTORE_VERDICT.BEHIND
-  if (contiguousLength < localLength) return RESTORE_VERDICT.DOWNLOADING
+  if (contiguousLength < localLength && missingServable) return RESTORE_VERDICT.DOWNLOADING
   if (now - firstHolderAt < dwellMs) return RESTORE_VERDICT.DWELL
   return RESTORE_VERDICT.CAUGHT_UP
 }

src/worker/restore-catch-up.js

@@ -26,6 +26,15 @@ function coMembers(spaces, self, spaceId) {
   return keys.size
 }
 
+// Whether a connected holder has a block this copy still lacks; only such a block is worth waiting for.
+// Peers are asked first, as their bitfields are in memory and the local check reads storage.
+async function missingServable(core) {
+  for (let i = core.contiguousLength; i < core.length; i++) {
+    if (core.peers.some((peer) => peer.remoteBitfield?.get(i)) && !(await core.has(i))) return true
+  }
+  return false
+}
+
 export class RestoreCatchUp extends Subsystem {
   constructor(name, deps) { super(name, deps); this.require('ipc', 'profile') }
 
@@ -112,6 +121,7 @@ export class RestoreCatchUp extends Subsystem {
       now: Date.now(),
       dwellMs: getRestoreReleaseDwellMs(),
       coMembers: members,
+      missingServable: await missingServable(core),
       emptyMayBeSolo: !isProfile,
     })
     if (isProfile) this.report({ verdict, length: core.contiguousLength, target: Math.max(core.length, ...holderLengths), released: false })

test/flow/backup-restore.test.js

@@ -67,7 +67,8 @@ test('a device restored from a backup catches up from a co-member, then writes w
   const status = await restoring.request('identity:status')
   t.ok(status.restore, 'the restored device starts in restore mode')
   t.is((await restoring.request('profile:get')).displayName, 'Alice', 'with the profile as it was backed up')
-  await restoring.until('identity:status', {}, (s) => s.restore?.profile?.released === true, { ms: 180000, every: 500 })
+  // The profile and the space's catalog are released apart; a hold still on at stop carries into the next boot.
+  await restoring.until('identity:status', {}, (s) => s.restore?.profile?.released === true && s.restore.heldSpaceIds.length === 0, { ms: 180000, every: 500 })
   await stop(restoring)
 
   const back = await launch()

test/unit/restore-hold-rules.test.js

@@ -2,7 +2,7 @@ import test from 'brittle'
 import { releaseVerdict } from '../../src/shared/core/restore-hold-rules.js'
 import { RESTORE_VERDICT } from '../../src/shared/contract/restore-verdict.js'
 
-const state = (over = {}) => ({ localLength: 10, contiguousLength: 10, holderLengths: [10], firstHolderAt: 0, now: 30_000, dwellMs: 20_000, coMembers: 1, ...over })
+const state = (over = {}) => ({ localLength: 10, contiguousLength: 10, holderLengths: [10], firstHolderAt: 0, now: 30_000, dwellMs: 20_000, coMembers: 1, missingServable: true, ...over })
 
 test('a core no connected peer holds stays held', (t) => {
   t.is(releaseVerdict(state({ holderLengths: [], firstHolderAt: null })), RESTORE_VERDICT.NO_HOLDER)
@@ -23,6 +23,13 @@ test('a matched length with blocks still missing is not caught up', (t) => {
   t.is(releaseVerdict(state({ contiguousLength: 7 })), RESTORE_VERDICT.DOWNLOADING)
 })
 
+test('REGRESSION (FIX: restore hold on a sparse holder) — a missing block no holder has does not hold the core', (t) => {
+  t.is(releaseVerdict(state({ contiguousLength: 7, missingServable: false, now: 19_999 })), RESTORE_VERDICT.DWELL)
+  t.is(releaseVerdict(state({ contiguousLength: 7, missingServable: false })), RESTORE_VERDICT.CAUGHT_UP)
+  t.is(releaseVerdict(state({ contiguousLength: 7, holderLengths: [12], missingServable: false })), RESTORE_VERDICT.BEHIND,
+    'a holder with a longer copy still keeps it held')
+})
+
 test('a caught-up core waits out the dwell after the first holder answered', (t) => {
   t.is(releaseVerdict(state({ now: 19_999 })), RESTORE_VERDICT.DWELL)
   t.is(releaseVerdict(state({ now: 20_000 })), RESTORE_VERDICT.CAUGHT_UP, 'the boundary releases')