social: refuse a damaged Settings or identity file instead of reading it as absent

changelog.d/2717-social-readjson.fixed.md

@@ -0,0 +1 @@
+A Bot Social settings or identity file that exists but cannot be read or parsed is no longer treated as "nothing was ever saved". A torn `flybot-social-bots.<tag>.json` used to make a server switch silently restart the Bot as a stranger — no member identity, no grants — and the next switch rewrote the only copy; the switch now refuses before anything moves, names the file and leaves its bytes alone. A torn `flybot-social-server.json` refuses at start instead of quietly connecting to the default server as if no server had been chosen, and a damaged credentials file of another server is warned about and left out of the known-server list. A file that is absent still means a first run. See [#2717](https://github.com/ThinkFlowLab/FlyBot/issues/2717).

docs/subsystems/social.md

@@ -34,7 +34,7 @@ At phone width (`<= 640 px`, the last block in `client.js's stylesheet`) this pa
 | --- | --- |
 | `plugins/social/index.js` | Plugin entry. |
 | `plugins/social/src/social.js` | Host: roster, grants, inbox, `social/status` pending-approval queue, Approve / Reject, hide-remote, Manage access, shared-room approvals provider, face-to-face join/poll/leave/start/send and `social/face-auto`. |
-| `plugins/social/src/server-config.js` | Which server this FlyBot uses (#337): address validation (`parseServerUrl`), the `GET /v1/platform` probe (`probeServer`), and `createServerController`, which starts the social layer for the effective server and switches it live. `index.js` wires it and registers `social/server`, `social/server-check`, `social/server-set`. |
+| `plugins/social/src/server-config.js` | Which server this FlyBot uses (#337): address validation (`parseServerUrl`), the `GET /v1/platform` probe (`probeServer`), and `createServerController`, which starts the social layer for the effective server and switches it live. `index.js` wires it and registers `social/server`, `social/server-check`, `social/server-set`. The Settings choice and the parked Bot identities are read with the same `ENOENT`-vs-damaged rule as the credentials: absent means "nothing saved yet", a file that exists and does not parse refuses and names its path (#2717). |
 | `plugins/social/src/mcp-client.js` | Owner MCP client plus isolated grant/invite REST fallbacks (`listGrants`, `revokeGrant`, `updateBotProfile`, `declineGrant`, invites). |
 | `plugins/social/src/look.js` | Map Bot Social profile look fields to FlyBot `{ color, avatar }` (optional on older servers). A look's image is pasted into the page-wide mention stylesheet as `url("…")`, so `lookImage` keeps only a plain `https?://` URL or a `data:image/(png|jpeg|webp|gif);base64,` string and drops one carrying a quote, a paren, a backslash, whitespace or a control character (#2646); gif is accepted because the host stores one in the same field (#2649); `core/src/shared/look-image.js` is the same test at the core entries and sink, and `test/look.test.js` keeps the two in step. |
 | `plugins/social/src/face-rooms.js` | Face-to-face REST client (join/list/get/start/leave/messages, mint/revoke/preview/join invites, `parseInviteLink` / `parseFaceInviteLink`), error mapping, event rendering, peer-string sanitization. Auto-send is MCP in `social.js`. |
@@ -164,7 +164,7 @@ Creating a group opens its Session with zero turns, and the host draws no sessio
 - **What may be written where.** `flybot-social.json`, the `.bak` copy `startSocial` keeps for another server (`flybot-social.json.<tag>`) and `flybot-social-settings.json` are written by one function, and it asks the leaf with `lstat` before the first byte: a missing file is created (mode 600), a plain file of its own (`nlink === 1`) is rewritten through a same-directory `<path>.<pid>.tmp` that is fsynced and renamed over the file (the temp name is a leaf of its own too, so a link left there refuses the save as well), and a symbolic link, a hard link, a directory or any other kind refuses the write — nothing is written there, the file behind a link keeps its bytes, one line names the path and carries no value from the file, and `startSocial` fails with that refusal instead of running on a link. The temp plus rename is what keeps a write that stops half way (ENOSPC, EFBIG, a kill) from taking the previous file with it: the old bytes stay, and a failed write removes its temp (#2631). The refusal is asked **before** onboarding: the onboarding is a real home on the server and a round-trip, so a start whose credentials path cannot be written refuses first and never spends one on a token it would drop. A refused `.bak` leaves the credentials in use untouched, and its name is asked in that same pre-check: the backup name follows from the server in the credentials file already on disk (`flybot-social.json.<tag>`), a local read, so a link left there refuses the start before the onboarding rather than after it, and a free name still keeps the old credentials under their server. The read side tells "nothing saved yet" from "damaged": `ENOENT` is a first start, while a file that exists and does not parse refuses with a message naming the path and is left where it is — a torn credentials file therefore never reads as "not configured", and `startSocial` never onboards a fresh home over an identity it could not read (#2631); `flybot-social-settings.json` is read with that same rule (a link at that name is followed, like any read) and written only when a setting changes, where a write refusal is caught and warned as `settings not saved` — no onboarding depends on that write, but a damaged settings file refuses the start instead of reading as empty. A read-only window (`flybot.readOnly?.()`) writes it not at all (#2231): its read endpoints (`social/remote-bots`, `social/friends`) still derive the one-shot inbound backfill, the hidden-remote aging and the accept-notice cleanup in memory and answer the same, but the save path returns without writing, so it cannot create the file, cannot age a hide on it, and cannot flush an older copy over what the writable window saved.
 - **Validation.** `parseServerUrl`: `https://` only, `http://` just for `localhost`, `127.0.0.1` and `[::1]`; no user name or password, query or fragment; at most 200 characters; an address without a scheme gets `https://` (`http://` for those local hosts). `probeServer` GETs `<server>/v1/platform` without credentials, 8 s deadline, `redirect: 'manual'` (a redirect is refused, so the saved address is the checked one); the answer must look like Bot Social (a `name`, a `platform_key` or a `features` list) and its `features` are shown (cleaned, at most 40) in words (`featureLine` in `client.js`: `typed_by_owner` → typing as you, `mentions` → @mentions, `presence` → online status; an unknown id is only counted, and every raw id is in the line's tooltip, #421). `social/server-set` repeats both before it touches anything, so a UI that skipped **Check** gains nothing.
 - **A server is an identity.** The default server keeps its files. Any other server gets `flybot-social.json.<tag>` (credentials; the same suffix `startSocial` already used for backups, `tag` = first 8 hex of the sha-256 of the normalized address) and `flybot-social-settings.<tag>.json`; a different server never reads another's file, and no credentials file is ever deleted or rewritten by a switch. The per-Bot parts that live in host state (the `member` identity, the `autoApprove` marks and the person/room grants, which are Bot Social ids and handles) are parked in `flybot-social-bots.<tag>.json` (no token) and cleared before the new server starts, so a stale grant to `@bob.home` can never apply to a different Bob on the new server; switching back restores them and the saved credentials, without a second onboarding. The old server's friends' Bots leave the list when the new server's first roster fetch replaces it (`remote.publish`); face-to-face rooms already hosted stay as local transcripts, but nothing polls them.
-- **Applied live, with a way back.** `social/server-set` (needs `confirm: true`; a write, so a read-only host and a multi-member team refuse it) stops the social layer, parks the old server's Bot identities, saves the choice, restores what the new server had, and starts again (first start onboards). If that start fails or takes over 45 s, the previous server's files, identities and choice are put back, it is started again, and the error says so; a start that finishes late is disposed. A disposed instance never re-arms its poll: a face poll already in flight when `dispose()` runs cannot install a new interval on the discarded layer, so it never keeps polling (or delivering from) the server it was built for. When the switch succeeds and the address really changed, the previous server's hosted face-to-face rooms — open or still forming (candidate) — are marked `left` here — a read-only past room whose composer is gone, since the new server cannot reach them. Renaming this home on the same server is not a switch: that server still reaches those rooms, so only a new address ends them (the room and its composer stay as they were). Switching back does not reopen them: a room this home left stays a past room (`faceSnapshotPast`, #182), and a line to it is refused (`bot/room-ended`). The Settings page says plainly that this is a new identity, that friends and sharing stay on the old server, and that the old sign-in is kept as a backup and never deleted. Tests: `plugins/social/test/social.test.js` (`Settings server`, `server controller`, `parseServerUrl`, `probeServer`).
+- **Applied live, with a way back.** `social/server-set` (needs `confirm: true`; a write, so a read-only host and a multi-member team refuse it) stops the social layer, parks the old server's Bot identities, saves the choice, restores what the new server had, and starts again (first start onboards). If that start fails or takes over 45 s, the previous server's files, identities and choice are put back, it is started again, and the error says so; a start that finishes late is disposed. A disposed instance never re-arms its poll: a face poll already in flight when `dispose()` runs cannot install a new interval on the discarded layer, so it never keeps polling (or delivering from) the server it was built for. When the switch succeeds and the address really changed, the previous server's hosted face-to-face rooms — open or still forming (candidate) — are marked `left` here — a read-only past room whose composer is gone, since the new server cannot reach them. Renaming this home on the same server is not a switch: that server still reaches those rooms, so only a new address ends them (the room and its composer stay as they were). A damaged file refuses before anything moves (#2717): a torn `flybot-social-bots.<tag>.json` of the server being switched to refuses the switch with the live layer still running and the file byte for byte where it was, instead of being read as "nothing parked" and then replaced by the next `park()`; a torn `flybot-social-server.json` refuses the controller at start with a message naming the path, instead of falling back to `config.server` as if the owner had never chosen; and a torn credentials file of another server is warned about and left out of `known[]` rather than taking the Settings page down with it. `ENOENT` stays the one answer that means "nothing saved yet", so first runs and servers never visited keep working. Switching back does not reopen them: a room this home left stays a past room (`faceSnapshotPast`, #182), and a line to it is refused (`bot/room-ended`). The Settings page says plainly that this is a new identity, that friends and sharing stay on the old server, and that the old sign-in is kept as a backup and never deleted. Tests: `plugins/social/test/social.test.js` (`Settings server`, `server controller`, `parseServerUrl`, `probeServer`).
 
 ### Friends and sharing on the Bot Social page (#1074)
 

plugins/social/src/server-config.js

@@ -41,7 +41,7 @@
   return `${url.protocol}//${url.host}${url.pathname.replace(/\/+$/, '')}`
 }
 
 const cleanLine = (value, max) => String(value ?? '').replace(/[\u0000-\u001f\u007f<>]+/g, ' ').replace(/\s+/g, ' ').trim().slice(0, max)
 
 // GET <server>/v1/platform: what the server is and which optional features it lists. A redirect is a
 // refusal (the address saved is the address checked), and so is anything that does not look like Bot Social.
@@ -87,7 +87,21 @@
   renameSync(temp, path)
   try { chmodSync(path, 0o600) } catch { /* file systems that ignore the write mode */ }
 }
-const readJson = (path) => { try { return JSON.parse(readFileSync(path, 'utf8')) } catch { return undefined } }
+// Absent is the one answer that means "nothing saved yet" (`ENOENT`): a first run and a server
+// never visited keep working. A file that exists but cannot be read or parsed is not that answer —
+// reading it as absent lost a parked Bot identity and let the next `park()` replace the only copy,
+// and a torn server choice fell back to the config server (#2717). It refuses, names the path, and
+// leaves the file where a person can move it aside on purpose.
+const readJson = (path, what = 'A saved Bot Social file') => {
+  let text
+  try { text = readFileSync(path, 'utf8') } catch (failure) {
+    if (failure?.code === 'ENOENT') return undefined
+    throw refused(`${what} at ${path} cannot be read: ${cleanLine(failure?.message ?? failure, 120)}. Nothing there was changed.`, 'unreadable')
+  }
+  try { return JSON.parse(text) } catch {
+    throw refused(`${what} at ${path} exists but does not parse as JSON. Move it aside on purpose to discard it; nothing here changed it.`, 'damaged')
+  }
+}
 
 /**
  * @param base    the plugin config (yaml) with the plugin defaults; `server` is the default server
@@ -109,7 +123,7 @@
   const botsPath = tag => join(dir, `flybot-social-bots.${tag}.json`)
 
   const readOverride = () => {
-    const saved = readJson(overridePath)
+    const saved = readJson(overridePath, 'The Bot Social server chosen in Settings')
     if (typeof saved?.server !== 'string') return undefined
     try { return { server: parseServerUrl(saved.server), displayName: cleanLine(saved.displayName, 40) || undefined } } catch { return undefined }
   }
@@ -219,8 +233,12 @@
       try { await flybot.grants?.set?.(id, []) } catch (failure) { warn('flybot-social: grants not cleared:', failure) }
     }))
   }
-  async function restore(key) {
-    const saved = readJson(botsPath(serverTag(key)))
+  // Read before a switch touches any file: a torn file for the server being switched to then
+  // refuses with every identity and every saved file where they were, instead of parking the
+  // previous server first and finding out here. The default argument keeps callers that read at
+  // restore time (a same-server rename) working.
+  const readParked = key => readJson(botsPath(serverTag(key)), 'The Bot identities saved for this server')
+  async function restore(key, saved = readParked(key)) {
     if (!saved || serverKey(saved.server) !== serverKey(key)) return
     for (const [id, entry] of Object.entries(saved.bots ?? {})) {
       if (!flybot.bot.get(id)) continue
@@ -237,9 +255,23 @@
   }
 
   // Servers this profile has saved credentials for (the address only, never the credentials).
+  const warnedKnown = new Set()
   function knownServers() {
     const found = new Set()
-    const add = path => { const saved = readJson(path); if (typeof saved?.server === 'string' && saved.token) found.add(saved.server) }
+    // Listing the saved servers is a convenience, not an identity restore: one damaged credentials
+    // file is warned about and left out instead of taking the whole Settings page down with it.
+    // Warned once per path — `status` is polled, and one broken file is one fact for the operator.
+    const add = path => {
+      let saved
+      try { saved = readJson(path, 'Saved Bot Social credentials') } catch (failure) {
+        if (!warnedKnown.has(path)) {
+          warnedKnown.add(path)
+          warn('flybot-social: saved credentials not listed:', failure)
+        }
+        return
+      }
+      if (typeof saved?.server === 'string' && saved.token) found.add(saved.server)
+    }
     add(credentialsPath)
     try {
       const stem = `${basename(credentialsPath)}.`
@@ -270,7 +302,7 @@
       if (!address) throw refused('No server is set yet.', 'no_server')
       const info = await probe(address, { fetch: base.fetch })
       if (serverKey(address) === serverKey(current.server)) lastProbe = { server: address, info }
-      return { server: address, ...info, same: serverKey(address) === serverKey(current.server), known: knownServers().some(item => serverKey(item) === serverKey(address)) || serverKey(address) === defaultKey && Boolean(readJson(credentialsPath)?.token) }
+      return { server: address, ...info, same: serverKey(address) === serverKey(current.server), known: knownServers().some(item => serverKey(item) === serverKey(address)) || serverKey(address) === defaultKey && Boolean(readJson(credentialsPath, 'The Bot Social credentials')?.token) }
     },
     async set({ server, displayName, confirm } = {}) {
       if (flybot.readOnly?.() === true) throw refused('Another FlyBot window holds this team. Change the server there.', 'read_only')
@@ -288,10 +320,14 @@
         const previousFaceRooms = (flybot.rooms?.list?.() ?? [])
           .filter(room => room.origin === 'face-to-face' && room.face?.roomId && (room.face.status === 'open' || room.face.status === 'candidate'))
           .map(room => room.face.roomId)
+        // A same-server rename is no switch (#729): park() rewrites this server's file and restore()
+        // reads it back, so it is read at restore time. Any other target is read before anything
+        // moves — a torn identity file refuses the switch with the live server still connected.
+        const savedIdentities = serverKey(address) === serverKey(previous.server) ? undefined : readParked(address)
         stop()
         if (previous.server) await park(previous.server)
         writeOverride(target)
-        await restore(address)
+        await restore(address, savedIdentities)
         current = target
         lastProbe = { server: address, info }
         try {
@@ -302,7 +338,11 @@
           // A failed start leaves nothing of the new server worth parking: a failed park changes nothing here.
           if (target.server) await park(target.server).catch(() => undefined)
           writeOverride(previous)
-          if (previous.server) await restore(previous.server)
+          if (previous.server) {
+            // The rollback must not mask the switch failure; park() just wrote this file, so a
+            // failure here is reported and the file left where it is.
+            try { await restore(previous.server) } catch (failure) { warn('flybot-social: previous server identities not restored:', failure) }
+          }
           current = previous
           lastProbe = undefined
           // launch() has already recorded the failure (state, error, the unavailable answers); the message below reports it.