Repository navigation
Rooms: show a group to the owner of a friend's Bot added to it (inbound-share) - #112
Conversation
Adding a granted remote Bot to a group never notified Bot Social or the other FlyBot, and Bot Social has no group record — only 1:1 inbox after a message. Fan out a FLYBOT-ROOM-SHARE envelope on join, leave, and room lines so the owner's inbound-share projection appears as soon as the Bot is added, and hide it when the Bot leaves or the grant is revoked. 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: fab015a5d6
ℹ️ 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".
| if (share?.handled) { | ||
| delivered += 1 | ||
| done() | ||
| continue |
There was a problem hiding this comment.
For an op: "line" envelope, applyInbound returns handled and this branch advances the cursor and continues before the only flybot.bot.send call below. The sender transmits no second copy of the group line, so the invited local Bot never gets an agent turn and can never produce the held reply surfaced by the new shared-room approval UI; only the passive projection updates. Join/leave packets can be consumed here, but decoded line packets must also be routed to local.id.
AGENTS.md reference: AGENTS.md:L56-L57
Useful? React with 👍 / 👎.
| localBotId, | ||
| ownerId, | ||
| remoteRoomId: data.roomId, | ||
| joinedSeq: data.seq, |
There was a problem hiding this comment.
When a remote Bot is added to an existing room, the source has already appended joined and invited entries, so this value is at least 2 and can be much larger. The inbound room nevertheless starts its own log at sequence 0 and numbers received lines from 1, while roomHistory filters those local lines against this remote sequence; consequently the first messages after a later invite are hidden, potentially for hundreds of lines in an active room. Store a local join boundary or preserve the remote sequence on imported lines.
Useful? React with 👍 / 👎.
| delete room.hidden | ||
| for (const p of room.participants ?? []) { | ||
| if (p.leftSeq !== undefined) { | ||
| delete p.leftSeq | ||
| delete p.leftAt |
There was a problem hiding this comment.
After leaveInboundShares marks a room hidden and closes its participants, any delayed op: "line" packet finds that existing room and unconditionally clears both the hidden flag and the closed interval, without checking that the grant is active or that this is a fresh join. Thus a line already queued when the owner revokes a grant resurrects the shared room and its old history. Only an authorized join should reopen it, with a new local membership boundary.
Useful? React with 👍 / 👎.
| const participants = remotes.length === 0 ? undefined : [ | ||
| ...members.map(id => ({ kind: 'bot', id, role: id === admin ? 'admin' : 'member' })), | ||
| ...remotes, | ||
| ] | ||
| const room = await actions.createRoom(members, { admin, ...(participants ? { participants } : {}), ...(text ? {} : { greet: true }) }) |
There was a problem hiding this comment.
This new picker path commonly creates a valid group with one local admin and one remote participant, leaving room.members with just the local Bot. Every later updateRoom call still runs checkMembers([...room.members]) before applying participant changes, so renaming the group, changing settings, or removing the friend's Bot all fail with “needs at least two Bots.” The update validation needs to count live remote participants (and allow the inbound leave case) just as creation now does.
Useful? React with 👍 / 👎.
| if (ch === '{') depth += 1 | ||
| else if (ch === '}') { | ||
| depth -= 1 | ||
| if (depth === 0) { end = i; break } | ||
| } |
There was a problem hiding this comment.
This depth counter treats braces inside JSON string values as structural braces. A valid room name or chat line containing an unmatched { or } therefore makes parseRoomShare return undefined even though it was produced by encodeRoomShare; the social poll then drops the event from the projection and may deliver the protocol blob as an ordinary 1:1 message instead. The JSON boundary scan must be string/escape-aware, or parse the protocol's known JSON suffix directly.
Useful? React with 👍 / 👎.
| async function sendShareTo(room, remote, payload, via) { | ||
| const from = shareSpeaker(room) | ||
| if (!from || !rt.flybotSend) return | ||
| try { | ||
| await rt.flybotSend({ | ||
| from, | ||
| to: remote.handle || remote.id, |
There was a problem hiding this comment.
shareSpeaker chooses one generally shareable room member and this function reuses it for every remote recipient. With per-Bot friendships, that member may have a grant to another friend while the target home's via list covers a different local Bot; Bot Social's covered() check then denies the join/line even when a covered room member exists, and this catch reduces the failure to a warning after group creation already succeeded. Select the sending identity per remote recipient and require its Bot Social coverage.
Useful? React with 👍 / 👎.
| inviterName: fromName, | ||
| inviterHome: fromHome || room.sharedFrom?.inviterHome, | ||
| inviterHandle: fromHandle || room.sharedFrom?.inviterHandle, | ||
| invitedHandle: data.invitedHandle || room.sharedFrom?.invitedHandle, |
There was a problem hiding this comment.
If the same friend's two Bots are invited to one source room, both inbox packets resolve to the same inbound room by remote room/home, but the existing-room path retains only the first localBotId and repeatedly overwrites this single invitedHandle. Each source line is then delivered once to each invited Bot and appended twice to the same projection, while a leave for either Bot hides the entire room although the other remains. Aggregate the invited local participants and deduplicate imported lines by source sequence.
Useful? React with 👍 / 👎.
Bug: after creating a group and adding another person's Bot, that Bot's owner never saw the group.
Cause: (a) FlyBot never told the other side a room existed (New group passed the remote chat session as a local member, and add/create did not notify Bot Social); (b) Bot Social only has 1:1 conversations, so an empty room is invisible; (c) the inbound-share projection (#50) was never wired in production.
Fix (FlyBot only): a FLYBOT-ROOM-SHARE envelope over 1:1 send_as_member carries join/leave and chat lines. The owner's pollOnce applies it before 1:1 delivery and creates an inbound-share room (Shared room from ); leave, hide or grant revoke marks it left. New group splits remote vs local members. The owner's Bot replies still go through in-chat approvals. Local verify passed.