fix(social): Codex review follow-up for #141 (canonical grants, inbound backfill, scoped unhide)

docs/subsystems/social.md

@@ -68,9 +68,10 @@ Everything in `plugins/social/` (entry `plugins/social/index.js` and any helpers
 
 - **#136:** A taken `displayName` at onboard retries `Name2` … rather than refusing to start. Friends' Bots shows `@your.home` with Copy.
 - **#137 (deferred):** The owner's chosen Bot avatar/color does not cross homes. Live Bot Social `update_bot_profile` only has `display_name` / `bot_id`; face-room join POST is `{ code, bots: [handle] }`. Peers keep the handle-seeded look until the server grows profile look fields.
-- **#138:** Accepting a Helper invite grants Helper only. The joiner picks a talk-through Bot (not shared). Success copy is `{person} shared {Bot} with you.` Manage access lists `SelfB1935 (@selfb1935.home)`. Face-room remotes leave Friends' Bots when the room is left.
-- **#139:** **Remove this Bot** asks once (`Remove Helper? Bob can share it again.`). Hide stays local (A's Manage access still shows the grant until A revokes — an incoming un-grant needs Bot Social). Re-joining an existing friendship unhides instead of a raw `relationship_exists`; used invites say “This invite was already used.”
-- **#140:** Auto-send defaults off. The owner sees Approve / Reject in the chat, not “Waiting for the owner's approval.” Composer drafts are labelled You.
+- **#138:** Accepting a Helper invite grants Helper only. The joiner picks a talk-through Bot (not shared). Success copy is `{person} shared {Bot} with you.` Manage access lists `SelfB1935 (@selfb1935.home)`. Core grants store a canonical `@handle` (never decorated `Chief@…`). On upgrade, remotes already on `list_remote_bots` are seeded into `inboundBots` once so reciprocal friends are not dropped. Face-room remotes leave Friends' Bots when the room is left; existing friend handles are not reclassified as room-only.
+- **#139:** **Remove this Bot** asks once (`Remove Helper? Bob can share it again.`). Hide stays local (A's Manage access still shows the grant until A revokes — an incoming un-grant needs Bot Social). Re-joining an existing friendship unhides only that invite's Bot/peer home (not every hidden remote); used invites say “This invite was already used.”
+- **#140:** Auto-send defaults off. Accepting a friend request uses that relationship's peer home for Auto-send, not the first roster entry. The owner sees Approve / Reject in the chat, not “Waiting for the owner's approval.” Composer drafts are labelled You.
+- **#136 (suffix):** Onboard retries truncate the sanitized handle body so `Name2` / `Name3` still change within the 32-character `homeHandleOf` limit.
 
 ### Past rooms, unique titles, poll, and + (#129–#133)
 

plugins/social/src/mcp-client.js

@@ -41,7 +41,7 @@
   const lib = parsed.protocol === 'http:' ? http : https
   const agent = init.agent ?? agentFor(url)
   const method = String(init.method ?? 'GET').toUpperCase()
   const headers = { ...(init.headers ?? {}) }
   const signal = init.signal
   return new Promise((resolve, reject) => {
     const req = lib.request(parsed, { method, headers, agent }, (res) => {
@@ -76,7 +76,7 @@
   const list = [...items]
   if (list.length === 0) return []
   const cap = Math.max(1, Math.min(limit || GRANT_CONCURRENCY, list.length))
   const out = new Array(list.length)
   let next = 0
   async function worker() {
     for (;;) {
@@ -212,7 +212,8 @@
   })
   return {
     ...(id ? { id: String(id) } : {}),
-    grantee: String(grantee),
+    // Canonical @handle for core grants.set / normGrantee (not decorated Chief@…).
+    grantee: view.member || view.home || memberHandleOf(grantee) || String(grantee),
     ...(view.label ? { name: view.label } : {}),
     ...(view.home ? { home: view.home } : {}),
     ...(view.member ? { member: view.member } : {}),

plugins/social/src/social.js

@@ -76,9 +76,9 @@
 const TEXTUAL = new Set(['text/plain', 'text/markdown', 'text/csv', 'text/tab-separated-values', 'application/json'])
 const sizeLabel = n => (n < 1024 ? `${n} B` : n < 1048576 ? `${(n / 1024).toFixed(1)} KB` : `${(n / 1048576).toFixed(1)} MB`)
 // Remote strings that reach model-visible text: one line, no brackets or markup, capped.
 export const cleanText = (value, fallback = '') => String(value ?? '').replace(/[\u0000-\u001f\u007f-\u009f\u2028\u2029]/g, ' ').replace(/[[\]"<>`]/g, '').replace(/\s+/g, ' ').trim().slice(0, 60) || fallback
 // Owner-only draft body: keep punctuation the friend will see; strip controls and cap.
 export const draftText = (value) => String(value ?? '').replace(/[\u0000-\u001f\u007f-\u009f\u2028\u2029]/g, ' ').replace(/\s+/g, ' ').trim().slice(0, 800)
 // Bot Social list_approvals card → the Friends' Bots queue (outbound drafts only).
 const looksLikeBotId = (value) => /^bot_[a-z0-9]+$/i.test(String(value ?? '').trim())
 export function approvalView(item, { localName } = {}) {
@@ -166,7 +166,13 @@
 
 export function nextOnboardDisplayName(base, attempt) {
   const name = String(base ?? 'FlyBot').trim() || 'FlyBot'
-  return attempt <= 1 ? name : `${name}${attempt}`
+  if (attempt <= 1) return name
+  const suffix = String(attempt)
+  // homeHandleOf keeps 32 alphanumeric chars; reserve space so the suffix changes the handle.
+  const sanitized = name.toLowerCase().replace(/[^a-z0-9]+/g, '')
+  const maxBase = Math.max(1, 32 - suffix.length)
+  if (sanitized.length <= maxBase) return `${name}${suffix}`
+  return `${sanitized.slice(0, maxBase)}${suffix}`
 }
 
 async function onboardError(response, displayName) {
@@ -220,7 +226,14 @@
   const code = String(error?.code ?? '')
   const blob = `${code} ${msg}`
   if (/relationship_exists/i.test(blob)) {
-    return Object.assign(new Error("You're already friends. That Bot is in Friends' Bots again."), { code: 'relationship_exists' })
+    return Object.assign(new Error("You're already friends. That Bot is in Friends' Bots again."), {
+      code: 'relationship_exists',
+      ...(error?.peer ? { peer: error.peer } : {}),
+      ...(error?.bot ? { bot: error.bot } : {}),
+      ...(error?.relationshipId || error?.relationship_id
+        ? { relationshipId: String(error.relationshipId ?? error.relationship_id) }
+        : {}),
+    })
   }
   if (/invite is used|invite_used|already used|max_uses|used_up/i.test(blob)) {
     return Object.assign(new Error('This invite was already used. Ask your friend for a new one.'), { code: 'invite_used' })
@@ -548,7 +561,13 @@
     const key = norm(handle)
     const room = String(faceId ?? '')
     if (!key || key === '@' || !room) return
-    if (settings.inboundBots?.[key]) return
+    const peerHome = (() => {
+      const parts = key.slice(1).split('.')
+      return parts.length > 2 ? `@${parts.slice(1).join('.')}` : key
+    })()
+    if (settings.inboundBots?.[key] || settings.inboundBots?.[peerHome]) return
+    // Already a normal friend in the roster — do not reclassify as room-only.
+    if (listedRemoteBots().some(bot => bot.handle === key || bot.home === key || bot.home === peerHome)) return
     settings.roomBots ??= {}
     const rec = settings.roomBots[key] ?? { rooms: {} }
     if (rec.rooms?.[room]) return
@@ -582,6 +601,20 @@
     }
     return keys
   }
+  // One-time upgrade: remotes already on the server roster are treated as inbound so
+  // reciprocal friends are not dropped when inboundBots was empty (Codex P1).
+  function backfillInboundFromRoster(mappedHomes) {
+    if (settings.inboundBackfilled) return
+    settings.inboundBots ??= {}
+    for (const bot of (mappedHomes ?? []).flatMap(entry => entry.bots ?? [])) {
+      const key = norm(bot.handle)
+      if (!key || key === '@') continue
+      if (settings.roomBots?.[key]) continue
+      settings.inboundBots[key] = true
+    }
+    settings.inboundBackfilled = true
+    saveSettings()
+  }
   function keepGrantedRemote(bot) {
     const key = norm(bot.handle)
     if (!key) return false
@@ -629,6 +662,7 @@
       if (home) {
         result = await client.callTool('list_remote_bots', { home: home.id })
         const mapped = mapRemoteHomes(result?.homes)
+        backfillInboundFromRoster(mapped)
         unfiltered = { homes: mapped.map(entry => ({ ...entry, bots: entry.bots.filter(bot => keepGrantedRemote(bot)) })) }
       } else {
         // Per-bot identity (legacy): each active friend is its own "home" of one Bot.
@@ -787,9 +821,11 @@
         const roomScoped = (flybot.grants.of?.(bot.id) ?? []).filter(grant => grant.roomId)
         const people = listed.filter(grant => !grant.roomId).map(grant => {
           const view = grantPersonView(grant)
+          // Core normGrantee cannot match decorated non-@ values; persist a canonical @handle.
+          const canonical = view.member || view.home || memberHandleOf(grant.grantee) || grant.grantee
           return {
-            id: grant.id || grant.relationshipId || `${bot.id}:${grant.grantee}`,
-            grantee: grant.grantee,
+            id: grant.id || grant.relationshipId || `${bot.id}:${canonical}`,
+            grantee: canonical,
             ...(view.label ? { name: view.label } : grant.name ? { name: grant.name } : {}),
             ...(view.home ? { home: view.home } : {}),
             ...(view.member ? { member: view.member } : {}),
@@ -1211,7 +1247,16 @@
         try {
           const result = await client.callTool('list_remote_bots', { home: home.id })
           const homes = mapRemoteHomes(result?.homes)
-          const next = revealHiddenIfPresent(settings, presentRemoteHandles(homes))
+          // Scope unhide to the invite's Bot/peer home — never every remote on the home.
+          const focus = norm(mapped.bot?.handle || mapped.peer?.handle)
+          const present = presentRemoteHandles(homes)
+          const handles = focus
+            ? present.filter(h => {
+              const homeKey = isHomeLevelHandle(focus) ? focus : homeOfPeer(focus)
+              return h === focus || h === homeKey || homeOfPeer(h) === homeKey
+            })
+            : []
+          const next = handles.length ? revealHiddenIfPresent(settings, handles) : settings
           if (next !== settings) { settings = next; saveSettings() }
           fetchedAt = 0
           await remoteBots({ fresh: true }).catch(() => {})
@@ -1248,17 +1293,33 @@
   }))
   disposers.push(flybot.api.handle('social/respond', async ({ relationshipId, decision } = {}) => {
     if (decision !== 'accept' && decision !== 'decline') throw new Error('decision is accept or decline')
-    const result = await confirmedCall('respond_friend_request', { relationship_id: String(relationshipId ?? ''), decision, ...(decision === 'accept' ? { grant_capabilities: ['chat.public'] } : {}) })
+    const rid = String(relationshipId ?? '')
+    let pendingPeer
     if (decision === 'accept') {
-      const before = new Set(listedRemoteBots().map(bot => bot.handle))
-      const roster = await remoteBots({ fresh: true }).catch(() => remote)
-      for (const bot of (roster?.homes ?? []).flatMap(entry => entry.bots)) {
-        if (!before.has(bot.handle)) rememberInbound(bot.handle)
+      try {
+        const overview = await friendsOverview()
+        pendingPeer = (overview.incoming ?? []).find(item => String(item.relationshipId) === rid)
+      } catch (error) {
+        warn('could not load pending request for respond', error)
       }
-      const peerHome = roster?.homes?.find(entry => entry.bots?.length)?.home
-      if (autoAcceptOf(peerHome)) await serverAutoApprove(String(relationshipId ?? ''), true)
     }
-    return { relationshipId, status: result?.status ?? (decision === 'accept' ? 'active' : 'declined') }
+    const result = await confirmedCall('respond_friend_request', { relationship_id: rid, decision, ...(decision === 'accept' ? { grant_capabilities: ['chat.public'] } : {}) })
+    if (decision === 'accept') {
+      const before = new Set(listedRemoteBots().map(bot => bot.handle))
+      const roster = await remoteBots({ fresh: true }).catch(() => remote)
+      const added = (roster?.homes ?? []).flatMap(entry => entry.bots).filter(bot => !before.has(bot.handle))
+      for (const bot of added) rememberInbound(bot.handle)
+      const resultPeer = result?.peer?.handle || result?.friend?.handle || result?.inviter?.handle
+      const peerHome = pendingPeer?.handle
+        ? (isHomeLevelHandle(pendingPeer.handle) ? pendingPeer.handle : homeOfPeer(pendingPeer.handle))
+        : resultPeer
+          ? (isHomeLevelHandle(norm(resultPeer)) ? norm(resultPeer) : homeOfPeer(resultPeer))
+          : added.length
+            ? added[0].home
+            : undefined
+      if (peerHome && autoAcceptOf(peerHome)) await serverAutoApprove(rid, true)
+    }
+    return { relationshipId: rid, status: result?.status ?? (decision === 'accept' ? 'active' : 'declined') }
   }))
 
   // The owner's composer for a remote member: text plus files, through bot.send (approval applies).