host: repair a null saved record instead of failing every load

changelog.d/2755-state-null-record.fixed.md

@@ -0,0 +1 @@
+A `state.json` whose saved room or Bot record is `null` no longer hides the whole team: the file as it was is kept as a `state.json.broken-<stamp>` copy, one warning names `state.json`, the room is folded to an empty member list (an unusable Bot record is dropped), and the repaired team loads and saves. A version-1 file still keeps its `.v1` backup, and a read-only window records the evidence but never writes `state.json`. See [#2755](https://github.com/ThinkFlowLab/FlyBot/issues/2755).

core/src/host/store.js

@@ -84,6 +84,8 @@ export function install(rt) {
   let stopped = false
   // mtime and size of the state.json this host last read or wrote.
   let seen = ''
+  // A broken saved record was folded on the last read, so that read owes the disk a write.
+  let brokenRepaired = false
   const stamp = () => {
     try {
       const { mtimeMs, size } = statSync(statePath)
@@ -221,6 +223,40 @@ export function install(rt) {
       }
       return undefined
     }
+    // A saved record that is null (or a scalar) is a broken record, not a crash: a truncated write,
+    // a hand edit, a restore from another machine or an older build leaves `rooms: { r1: null }` or
+    // `bots: { chief: null }`. The record used to throw inside adopt (`room.members` on null,
+    // `'fields' in bot`), `loaded ??=` then cached the rejection and the team never appeared (#2755).
+    // Keep the file exactly as it was, then fold the record: a room stays with an empty member list,
+    // a Bot record is dropped because no data is left to keep. Both are named in one warning that
+    // names state.json, so "the team is gone" has a line to find.
+    if (parsed?.version === 1 || parsed?.version === STATE_VERSION) {
+      const broken = []
+      for (const key of ['bots', 'rooms']) {
+        if (!isRecord(parsed[key])) continue
+        for (const [id, record] of Object.entries(parsed[key])) if (!isRecord(record)) broken.push(`${key}.${id}`)
+      }
+      if (broken.length) {
+        const backup = `${statePath}.broken-${Date.now()}`
+        try {
+          await copyFile(statePath, backup)
+        } catch (copyError) {
+          stuck = `a broken record (${broken.join(', ')}) could not be preserved before repairing it (${copyError?.code ?? copyError?.message ?? copyError})`
+          warn('ds-bot: state %s has a broken record (%s) and the copy to %s failed: %s; the file is left untouched', statePath, broken.join(', '), backup, copyError)
+          return undefined
+        }
+        warn('ds-bot: state %s has a broken record (%s); the file as it was is kept as %s and the record is repaired', statePath, broken.join(', '), backup)
+        for (const key of ['bots', 'rooms']) {
+          if (!isRecord(parsed[key])) continue
+          for (const [id, record] of Object.entries(parsed[key])) {
+            if (isRecord(record)) continue
+            if (key === 'rooms') parsed[key][id] = {}
+            else delete parsed[key][id]
+          }
+        }
+        brokenRepaired = true
+      }
+    }
     if (parsed?.version === 1) {
       const problem = legacyOwnershipProblem(parsed)
       if (problem !== undefined) {
@@ -266,7 +302,8 @@ export function install(rt) {
     state.prefs ??= {}
     // Teams saved before the DSH Agent existed carry no `agents`; corrupt values reset.
     if (state.agents === null || typeof state.agents !== 'object' || Array.isArray(state.agents)) state.agents = {}
-    let migrated = !hadCapabilities
+    let migrated = !hadCapabilities || brokenRepaired
+    brokenRepaired = false
     // Everything above is the file's value, not EMPTY's, and a hand edit, a restore from another
     // machine or a hand-merged file can bring `null` or a scalar where the schema holds a table.
     // Those are repaired to their empty shape before anything reads them, because the alternative

core/test/store.test.js

@@ -701,3 +701,69 @@ test('a zero-byte or truncated team file is backed up and loads fresh, never ref
     assert.equal(fs.readdirSync(home).filter(name => name.startsWith('state.json.broken-')).length, 1, label)
   }
 })
+
+// #2755: a saved record that is null (a truncated write, a hand edit, an older build) used to throw
+// out of adopt(), and `loaded ??=` cached the rejection: the team never appeared, `readOnly()` had no
+// answer and nothing named state.json. The record is evidence-backed up and folded instead.
+test('a room record that is null is backed up and folded, and the team still appears (#2755)', async (t) => {
+  const { rt, home, statePath, warnings } = runtime(t, team({ r1: null }))
+  await rt.load()
+  assert.equal(rt.readOnly(), undefined, 'the file loads as a writable team instead of a null answer')
+  assert.deepEqual(rt.state.rooms.r1.members, [], 'the room is kept and folded to an empty member list')
+
+  const backups = fs.readdirSync(home).filter(name => name.startsWith('state.json.broken-'))
+  assert.equal(backups.length, 1, 'the pre-repair file is kept as one .broken copy')
+  const kept = JSON.parse(fs.readFileSync(join(home, backups[0]), 'utf8'))
+  assert.equal(kept.rooms.r1, null, 'the dropped value is preserved verbatim in the evidence copy')
+
+  const written = JSON.parse(fs.readFileSync(statePath, 'utf8'))
+  assert.equal(written.version, 2, 'the repair is written back as version 2')
+  assert.deepEqual(written.rooms.r1.members, [], 'the saved room carries the repaired list')
+  assert.equal(warnings.some(line => line.join(' ').includes('state.json')), true, 'a warning names state.json')
+})
+
+test('a version-1 Bot record that is null migrates, keeps .v1 and drops only that record (#2755)', async (t) => {
+  const { rt, home, statePath, warnings } = runtime(t, {
+    version: 1,
+    revision: 1,
+    bots: { chief: null, kept: { id: 'kept', name: 'Keeper', createdAt: 1 } },
+  })
+  await rt.load()
+  assert.equal(rt.readOnly(), undefined, 'the team is not refused read-only')
+  assert.equal(rt.state.version, 2, 'the version-1 team is migrated')
+  assert.deepEqual(Object.keys(rt.state.bots), ['kept'], 'only the broken record is dropped')
+
+  const v1 = JSON.parse(fs.readFileSync(`${statePath}.v1`, 'utf8'))
+  assert.equal(v1.version, 1, 'the version-1 backup is kept')
+  assert.equal(v1.bots.chief, null, 'the backup is the file as it was, broken record included')
+  const backups = fs.readdirSync(home).filter(name => name.startsWith('state.json.broken-'))
+  assert.equal(backups.length, 1)
+  assert.equal(JSON.parse(fs.readFileSync(join(home, backups[0]), 'utf8')).bots.chief, null)
+
+  const written = JSON.parse(fs.readFileSync(statePath, 'utf8'))
+  assert.equal(written.version, 2)
+  assert.equal(Object.hasOwn(written.bots, 'chief'), false, 'the repaired file carries no null Bot record')
+  assert.equal(warnings.some(line => line.join(' ').includes('state.json')), true, 'a warning names state.json')
+})
+
+test('a version-1 team with a null record is not written back in a read-only pass (#2755)', async (t) => {
+  const home = fs.mkdtempSync(join(tmpdir(), 'ds-bot-store-'))
+  t.after(() => fs.rmSync(home, { recursive: true, force: true, maxRetries: 5 }))
+  const original = JSON.stringify({ version: 1, revision: 1, bots: { chief: null, kept: { id: 'kept', name: 'Keeper', createdAt: 1 } } })
+  fs.writeFileSync(join(home, 'state.lock'), JSON.stringify({ pid: process.ppid, host: hostname(), token: 'other' }))
+  fs.writeFileSync(join(home, 'state.json'), original)
+
+  const { rt } = runtime(t, undefined, { home })
+  await rt.load()
+  assert.match(rt.readOnly(), /another dsh/, 'the window is told why it may not write')
+  assert.equal(fs.readFileSync(join(home, 'state.json'), 'utf8'), original, 'a read-only pass writes no state.json')
+  assert.equal(JSON.parse(fs.readFileSync(join(home, 'state.json.v1'), 'utf8')).version, 1, '.v1 is still kept')
+})
+
+test('a healthy team loads with no broken-record warning and no .broken copy (#2755)', async (t) => {
+  const { rt, home, warnings } = runtime(t, team({ r1: { id: 'r1', name: 'Ops', members: ['a'], mode: 'admin', admin: 'a', named: true, sessions: {}, log: [], seq: 0, seen: {} } }))
+  await rt.load()
+  assert.equal(rt.readOnly(), undefined)
+  assert.deepEqual(fs.readdirSync(home).filter(name => name.startsWith('state.json.broken-')), [], 'no evidence copy for a healthy file')
+  assert.equal(warnings.some(line => line.join(' ').includes('broken record')), false, 'no warning for a healthy file')
+})