Repository navigation
fix(social): Codex review follow-up for #141 (canonical grants, inbound backfill, scoped unhide) - #142
Conversation
…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>
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: dfda61a3cf
ℹ️ 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 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) |
There was a problem hiding this comment.
When join_invite returns a normal MCP isError payload, createMcpClient rebuilds it at plugins/social/src/mcp-client.js:137 as a BotSocialError containing only message and code, so both mapped.bot and mapped.peer are absent here. Consequently focus and handles stay empty and reusing an invite can no longer unhide the requested Bot; the new test misses this because its mock throws the enriched JavaScript error before MCP serialization. Preserve the structured error fields when wrapping the response, or resolve the relationship through another call.
Useful? React with 👍 / 👎.
| })() | ||
| 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 |
There was a problem hiding this comment.
When a face room contains @writer.bob.home while @coder.bob.home is already a normal friend, listedRemoteBots() contains an entry whose home is @bob.home, so this same-home check suppresses Writer's room-only marker even though Writer itself was never granted. After leaving, dropRoomRemotes has no record for Writer, allowing the server-leak path that roomBots is designed to filter to leave that sibling in Friends' Bots; only an exact member match (or an explicitly home-level identity) should count as already listed.
AGENTS.md reference: AGENTS.md:L59-L59
Useful? React with 👍 / 👎.
Summary
Follow-up to the already-merged #141. Addresses all Codex review findings on that PR before they ship further.
P1
list_bot_grantsdecorated values likeChief@chief.bob.homeare stored as@chief.bob.homein core grants (normalizeGrant+syncOutgoingGrants), sonormGrantee/ reach can match.social/respondresolves the peer home from the pending request (or result / newly added bots for that relationship), not the first nonempty roster home.inboundBackfilledseedsinboundBotsfrom the currentlist_remote_botsroster before the outgoing-only filter, so upgrade does not permanently drop reciprocal friends.P2
homeHandleOf's 32-char sanitized limit.Test plan
ARCH_DOCS_BASE=upstream/main npm run verify(local)