Repository navigation
fix(social): name collisions, per-Bot grant leaks, Bot removal and approval defaults (#136 #138 #139 #140) - #141
Conversation
…FlowLab#139, ThinkFlowLab#140 Retry a taken home name at onboard, show the connected handle, keep per-Bot invites from leaking Chief, confirm Remove, and default Auto-send off. Owner-chosen avatars (ThinkFlowLab#137) stay deferred: Bot Social has no profile look fields. Co-authored-by: Danyang Chen <birfychen@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 041923e911
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const view = grantPersonView(grant) | ||
| return { | ||
| id: grant.id || grant.relationshipId || `${bot.id}:${grant.grantee}`, | ||
| grantee: grant.grantee, |
There was a problem hiding this comment.
When list_bot_grants returns the decorated format covered by this change (for example Chief@chief.bob.home), this keeps that value as grantee and puts the usable handles only in home/member. flybot.grants.set ultimately goes through core/src/host/reach.js:212-221, which discards those two side fields, while normGrantee at lines 43-53 does not parse a decorated non-@ value. The core reach gate therefore cannot match the grant to @chief.bob.home or @bob.home, so an accepted per-Bot grant is denied; persist the canonical member/home handle as grantee.
Useful? React with 👍 / 👎.
| const peerHome = roster?.homes?.find(entry => entry.bots?.length)?.home | ||
| if (autoAcceptOf(peerHome)) await serverAutoApprove(String(relationshipId ?? ''), true) |
There was a problem hiding this comment.
When another friendship already appears first in the roster, accepting a request from a new person uses that unrelated first home's Auto-send setting. For example, if Bob is first and has Auto-send enabled, accepting Carol calls set_auto_approve for Carol's relationship even though Carol should start off; the inverse also suppresses an existing opt-in. Resolve the peer home for relationshipId from the pending request/result instead of selecting the first nonempty roster entry.
Useful? React with 👍 / 👎.
| const key = norm(bot.handle) | ||
| if (!key) return false | ||
| if (settings.inboundBots?.[key] || settings.inboundBots?.[bot.home]) return true | ||
| if (outgoingPeerHandles().has(key)) return false |
There was a problem hiding this comment.
On the first start after this upgrade, inboundBots is empty, so an existing reciprocal share is misclassified: if @chief.bob.home both accepted an invite to one local Bot and was separately shared back, it is in outgoingPeerHandles() and this branch drops it even though list_remote_bots legitimately returned it. Only new join/respond calls populate the marker, so existing users permanently lose the valid Bot and leaveMissingRemotes() also evicts it from groups; migrate or derive inbound direction before applying this filter.
Useful? React with 👍 / 👎.
| const localHandles = new Set(locals.map(bot => bot.handle).filter(Boolean)) | ||
| for (const member of snapshot.members ?? []) { | ||
| const handle = norm(member.handle) | ||
| if (handle && !localHandles.has(handle)) markRoomRemote(handle, snapshot.roomId) |
There was a problem hiding this comment.
If a face-room participant is already a normally shared friend Bot but the friendship predates the new inboundBots map, this unconditional call records it as room-only. The next roster refresh rejects every roomBots handle, and dropRoomRemotes() deliberately retains the record after leaving, so merely joining a face room makes that existing friend's Bot disappear permanently. Mark only handles whose room-only provenance is established, such as remotes absent from the pre-room friend roster.
Useful? React with 👍 / 👎.
| try { | ||
| const result = await client.callTool('list_remote_bots', { home: home.id }) | ||
| const homes = mapRemoteHomes(result?.homes) | ||
| const next = revealHiddenIfPresent(settings, presentRemoteHandles(homes)) |
There was a problem hiding this comment.
When multiple locally removed Bots are still present in the home-wide list_remote_bots response, retrying one invite that returns relationship_exists passes every returned handle here and clears every matching hiddenRemotes entry. Re-adding Bob therefore silently restores unrelated Eve/other Bots the user explicitly removed; scope the reveal to the Bot or relationship associated with the retried invite.
Useful? React with 👍 / 👎.
|
|
||
| export function nextOnboardDisplayName(base, attempt) { | ||
| const name = String(base ?? 'FlyBot').trim() || 'FlyBot' | ||
| return attempt <= 1 ? name : `${name}${attempt}` |
There was a problem hiding this comment.
For a display name whose sanitized handle part already occupies the 32-character limit modeled by homeHandleOf, appending 2, 3, and so on after the full name leaves the first 32 sanitized characters unchanged. All 12 retries request the same taken home (or the suffixed display name is rejected for length), so collision recovery still fails; truncate/reserve space for the suffix inside the handle-bearing portion.
Useful? React with 👍 / 👎.
…ound backfill, scoped unhide - Persist canonical @Handles in core grants (not decorated Chief@…). - Seed inboundBots once from the existing remote roster so upgrade does not drop reciprocal friends before the outgoing-only filter. - Resolve Auto-send on respond from the pending relationship, not the first roster home. - Scope relationship_exists unhide to the invite's Bot/peer home. - Do not mark existing friend remotes as room-only; truncate onboard suffixes within the 32-char homeHandleOf limit. Co-authored-by: Danyang Chen <birfychen@gmail.com>
Fixes #136, #138, #140 and partly #139, from the automated self-test on main 1ac7104. Social plugin only; no core/pin changes.
FlyBot2,FlyBot3, ... so start never fails. Friends' Bots shows your own@handlewith Copy.{person} shared {Bot} with you.Manage access listsName (@home)vialist_bot_grants.relationship_exists/invite is used. Still needs Bot Social: hide is local, so the other side's Manage access keeps the grant until they revoke (no incoming un-grant tool).Tool removed: ask_user_questionis a DSH core tool cell.Deferred (needs Bot Social server work): #137 Bot avatar sync (
update_bot_profilehas onlydisplay_name+bot_id; face join sends{ code, bots: [handle] }) and the other-side half of #139. Documented indocs/subsystems/social.md.Verified:
ARCH_DOCS_BASE=upstream/main npm run verifypasses.