Repository navigation
feat(rooms): support human and Chief participants with attributed messages (#50) - #82
Conversation
…sages Typed room membership (humans and designated Chiefs), room roles, and persistent author ids so Alice, Bob and both Chiefs stay distinct. History follows a membership window: current members see seq >= joinedSeq; leave withdraws access; rejoin starts a new interval. Specialists stay owner-private. Team mode stays disabled. Closes ThinkFlowLab#50 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0dc6b59034
ℹ️ 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".
| createdAt: now, | ||
| revokedAt: null, | ||
| // Session-scoped only: the member record stays the #47 identity shape. | ||
| ...(displayName ? { displayName } : {}), |
There was a problem hiding this comment.
Sanitize peer-controlled display names before persistence
When an invitee redeems with a display name containing newlines or control characters, this stores it verbatim; displayNameFor later copies it to line.who, and groups.js interpolates that value directly into model-visible transcript prefixes. A peer can therefore forge trusted-looking headers such as System: in another owner's Chief prompt, so display names need control stripping and a length cap before storage or use.
AGENTS.md reference: AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
|
|
||
| // Shared rooms (#50): a current human participant may read and speak. Private | ||
| // owner rooms still match roomOwnedBy (legacy rooms with no participants). | ||
| const roomMemberOf = (actor, roomId) => roomVisibleTo(state, actor, roomId) |
There was a problem hiding this comment.
Keep multi-human room authorization disabled
When a second redeemed owner is listed as a room participant, this helper immediately makes that owner authorized for state, history, and send operations on every runtime. That ships cross-owner multi-human operation even though the repository contract requires team mode to remain disabled until its authorization gates pass; the room-member authorization paths need to remain unreachable until that gate is satisfied.
AGENTS.md reference: AGENTS.md:L37-L37
Useful? React with 👍 / 👎.
| const sessions = room.sessions ?? {} | ||
| if (Object.values(sessions).includes(id) || Object.values(room.segments ?? {}).some(list => (list ?? []).some(s => s?.sessionId === id))) return roomOwnedBy(actor, room.id) | ||
| if (Object.values(sessions).includes(id) || Object.values(room.segments ?? {}).some(list => (list ?? []).some(s => s?.sessionId === id))) { | ||
| return roomMemberOf(actor, room.id) || roomOwnedBy(actor, room.id) |
There was a problem hiding this comment.
Keep hidden Bot sessions owner-scoped
When Bob shares a room whose Alice-owned Chief already has a hidden group Session, the state response exposes that Session ID in room.parts, and this branch treats it as Bob-owned. A send to that ID then bypasses the room branch in channel.js and calls sessionController.prompt directly, allowing Bob to run Alice's Chief and its tools outside the room relay and audit log; membership should authorize the room Session only, while per-Bot group Sessions remain owner-scoped.
AGENTS.md reference: AGENTS.md:L37-L37
Useful? React with 👍 / 👎.
| if (Array.isArray(body?.humans)) { | ||
| for (const id of body.humans) if (typeof id === 'string') ids.add(id) | ||
| } |
There was a problem hiding this comment.
Reject human IDs that the mutation never applies
When an admin of an A/B shared room submits add: ['chief-c'] together with humans: ['owner-c'], these lines count owner C as a participant for authorization even though update-room discards the humans property. addRoomParticipant subsequently accepts the designated Chief after adding its owner only to its local validation set, so Chief C joins and executes in the room although owner C never joined; derive this set only from current participants or participant mutations that are actually applied.
AGENTS.md reference: AGENTS.md:L37-L37
Useful? React with 👍 / 👎.
| const bots = [...(Array.isArray(body?.add) ? body.add : []), ...(Array.isArray(body?.members) ? body.members : [])] | ||
| if (bots.every(id => permittedSharedBot(actor, id, humans) || botOwnedBy(actor, id))) return ALLOW | ||
| if (bots.length === 0) return ALLOW |
There was a problem hiding this comment.
Require room-admin role before updating shared settings
When a current participant with role member sends update-room containing only a name, notice, mode, admin, or work directory, bots is empty and this every check grants the request. roster.updateRoom also receives no actorId and consequently treats the caller as byMain, so an ordinary member can reconfigure the shared room and other owners' Chiefs; these settings need the same human-admin check used for participant management.
AGENTS.md reference: AGENTS.md:L37-L37
Useful? React with 👍 / 👎.
| if (participantOf(room, 'human', actor.memberId)) return true | ||
| const ids = room.members ?? [] | ||
| if (ids.length === 0) return false | ||
| return ids.every(id => state.bots?.[id]?.ownerId === actor.memberId) |
There was a problem hiding this comment.
Stop the legacy fallback after a participant leaves
When the human in a typed single-owner room leaves while their owned Bots remain, participantOf no longer finds them but the final ownership fallback still returns true. roomHistory then uses a default join sequence of zero, restoring all history even though leaving is supposed to withdraw access; the ownership fallback must be restricted to rooms that do not have a participants array.
Useful? React with 👍 / 👎.
| const last = (log ?? []).filter(line => line.kind === undefined).at(-1) | ||
| return { ...room, parts, ...(last ? { preview: last.who === 'User' ? last.text : `${last.who}:${last.text}` } : {}) } | ||
| const previewWho = last && (last.authorKind === 'human' && last.who && last.who !== 'User' ? `${last.who}: ${last.text}` : last.who === 'User' ? last.text : `${last.who}:${last.text}`) | ||
| return { ...room, parts, ...(last ? { preview: previewWho } : {}) } |
There was a problem hiding this comment.
Filter room previews through the join sequence
When a human joins after a room already has messages and calls state before a new ordinary message is posted, this preview is still selected from the entire log. The join event is excluded by kind, so the response leaks the latest pre-join message in the sidebar despite the participant's joinedSeq; compute the preview from linesVisibleTo for the requesting actor.
Useful? React with 👍 / 👎.
| const botPart = rt.participantsOf?.(room)?.find(p => p.kind === 'bot' && p.id === self) | ||
| const from = botPart?.joinedSeq ?? 0 | ||
| const lines = room.log.filter(line => (line.seq ?? 0) >= from).slice(-ROOM_READ_MAX) |
There was a problem hiding this comment.
Filter model prompts through the Bot join sequence
When a Chief is added after a room contains messages, this filter protects only the explicit read_group_chat tool. On the Chief's first normal turn, groupSessionFor clears its seen entry and roomPrompt calls unseenLines, which returns the last 12 log lines without consulting joinedSeq, so the model receives pre-join history; apply the membership window in the prompt path as well.
Useful? React with 👍 / 👎.
| return h('span', { className: 'bt-composer-as', title: t('Messages in this room are attributed to you') }, | ||
| t('Sending as {name}', { name: self.displayName || self.memberId })) |
There was a problem hiding this comment.
Attribute the shell composer submit path
When two humans use the ordinary text composer, the shell's send button remains the submit path and this change only adds a visual identity label. The new source.authorId is added only by actions.send (whose in-repo callers are voice and new-chat flows), so a normal user/message reaches turns.js without author metadata and is still logged and rendered as User; wire the standard submit path to the authenticated actor before claiming attribution.
Useful? React with 👍 / 👎.
| await rt.postRoomMessage(room.id, { text, displayName: actor.displayName }, actor) | ||
| const agent = await rt.resolveAgent(sessionId) | ||
| agent.followup(createUserMessage({ |
There was a problem hiding this comment.
Avoid appending attributed room sends twice
When an authenticated actions.send call targets a room, postRoomMessage appends the text immediately and the following followup emits the normal user event; turns.js queues that event and group_relay or drainRoomInbox appends it again. Voice and new-chat sends therefore produce duplicate log entries and duplicate model context, so only one of these paths should own the room-log append.
Useful? React with 👍 / 👎.
A room can take a friend's Chief or any Bot they published to friends (social/remote-bots). Unpublished specialists stay private on the host. Inviters see owner badges; the invited owner sees Shared room from the inviter, only the Bot's membership window, and can leave. Replies still go through Bot Social's outbound hold. Team mode stays disabled. Follow-up to ThinkFlowLab#50 Co-authored-by: Danyang Chen <birfychen@gmail.com>
A Bot is reachable from other owners and from friends only if its owner published it. The Chief is published by default; every other Bot (including tree children) is not. Published Bots are connected and pulled into rooms directly, never through the Chief; a Bot that joined a room by its room code (#83) is reachable in that room only. Core (patch 0013-published-reach, with the touched files staying in their patches): - reach.js: `published` flag, `reach()` owner policy, rt.setPublished, rt.isPublished, `bot.published` event, `set-published` endpoint. - hooks.js runs reach() outside the plugin chain: before bot.send, on hold release and on every deliver with a sender or remote origin (resumed, retried, deferred, re-sent), so allow/bypass verdicts cannot skip it. - room-membership.js: a room with a friend's Bot (or several owners) takes published local Bots only; unpublishing or replacing the Chief evicts the Bot at once; fanoutRoomLine checks author and recipient. - authz.js: another owner's Bot joins only if published. Plugins: @flybot/social registers, polls and signs only for published Bots; @flybot/tree denies a friend's Bot toward an unpublished tree Bot, reaches a published one directly (no redirect through the root). Docs: team, ds-bot-core, plugin-runtime, social, tree subsystem pages, READMEs, CHANGELOG. Refs #51, #82.
Friends' Bots gets Join with a number: type six digits, pick which Bot to bring (default first), and join with no preview. The matching room is hosted as a normal native room (origin face-to-face, reusing ThinkFlowLab#82 participants, owner badges, and the membership window). Leave is one click; Start chat voids the number. Member events and peer messages poll Bot Social with the event cursor; peer text is fenced. Bot replies POST /v1/face-rooms/{id}/messages and pending_approval uses the existing Approve / Reject queue. FlyBot members join unpublished (ISSUE ThinkFlowLab#51 hook). Closes ThinkFlowLab#83 Co-authored-by: Danyang Chen <birfychen@gmail.com>
Closes #50.
Typed room membership (human and bot/Chief participants), roles, participant management, persistent author IDs with visible attribution. Additive state, no STATE_VERSION bump; legacy rooms stay implicit-owner until a #50 mutation. Team mode stays disabled; ds-bot/ pin unchanged.
Model
History visibility: only a current participant may read, from their open joinedSeq. Leaving closes the interval and withdraws access; rejoining starts a new interval at the then-current seq. Non-members cannot read or inject. Specialists stay owner-private. Cross-owner rooms: humans + each human's designated Chief only. Actor wins over forged authorId/displayName.
API
Helpers: currentParticipants, roomVisibleTo, linesVisibleTo, authorOf, humanOwnersOf, isCrossOwnerRoom, isDesignatedChief, canManageParticipants, publicHumans, authoredRoomLine. Mutations: addRoomParticipant, removeRoomParticipant, postRoomMessage, applyParticipants. Reads: roomHistory. Channel: room-history, room-post; create-room/update-room accept participants/addParticipants/removeParticipants.
Checks (head 0dc6b59)
Run: core npm test (632), build-client --check, lint + arch-docs, ds-bot-patches generate+check, tree tests (49), social tests (38), lint-plugins.
Not run: smoke/Desktop/real-model bench, live deployment, package job (typecheck/pack/actionlint). No independent review yet.