Repository navigation
feat(face-rooms): Bots answer a plain line without @; cross-home @ labels resolve (#287) - #288
Conversation
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: 69036c0c6f
ℹ️ 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".
| // The friend's own typed line @-calls my Bots by the labels they see ("<owner>'s Chief", handle): | ||
| // only those local Bots answer (#287). A line that calls nobody here stays a bubble. | ||
| const callIds = isFaceRoom(room) && typeof rt.botOf === 'function' ? faceMentions(room, String(text ?? ''), rt.botOf, { fromFriend: true }).bots.map(bot => bot.id) : [] | ||
| return rt.postToRoom(room, { id: 'face-notice', name: who }, String(text ?? ''), { quiet: true, notice: true, human: { authorId: String(authorId || who), name: who }, ...(callIds.length > 0 ? { callIds } : {}) }) |
There was a problem hiding this comment.
On deployments whose /v1/platform does not advertise typed_by_owner (or while that probe is unavailable), renderFaceEvent deliberately treats any parseable legacy FLYBOT-FACE-HUMAN envelope as human and applyFaceSnapshot routes it to postHuman without an attestation flag. This spread therefore lets peer-controlled envelope text containing @<local label> produce callIds; openingSpeakers checks those IDs before quiet, so an unverified remote Bot message can start a local Bot turn. Pass and require attested provenance before adding callIds, while leaving legacy envelopes as quiet bubbles.
Useful? React with 👍 / 👎.
| return rt.postToRoom(room, { id: 'face-notice', name: who }, String(text ?? ''), { quiet: true, notice: true, human: { authorId: String(authorId || who), name: who } }) | ||
| // The friend's own typed line @-calls my Bots by the labels they see ("<owner>'s Chief", handle): | ||
| // only those local Bots answer (#287). A line that calls nobody here stays a bubble. | ||
| const callIds = isFaceRoom(room) && typeof rt.botOf === 'function' ? faceMentions(room, String(text ?? ''), rt.botOf, { fromFriend: true }).bots.map(bot => bot.id) : [] |
There was a problem hiding this comment.
When an attested friend sends @all, faceMentions returns all: true with an empty bots array, but this expression maps only bots, so the receiving home posts the line as quiet with no callIds. Its openingSpeakers consequently returns no speakers, and only the sender's home's Bots answer even though the new room contract and hint say @all asks every Bot; include every local room member when said.all is true.
Useful? React with 👍 / 👎.
| const { ctx } = rt | ||
| const participant = createDshRoomParticipant(rt) | ||
| fanoutRoomLine = (room, line) => rt.fanoutRoomLine?.(room, line) | ||
| rt.notePostedLine = (roomId, text) => postedLines.set(roomId, [...(postedLines.get(roomId) ?? []), text].slice(-20)) |
There was a problem hiding this comment.
If a face-room Bot turn runs long enough for more than 20 owner sends to queue, every postRoomMessage has already logged and fanned out its line, but this slice(-20) discards markers for the oldest sends before drainRoomInbox reads them. takePosted then returns false for those messages and appendRoom logs and fans them out a second time, reintroducing the duplicate-line bug this mechanism is meant to fix; retain markers until their corresponding Session messages are consumed, preferably by message ID.
Useful? React with 👍 / 👎.
| - Another member's own face-room line arrives through `flybot.rooms.postHuman` (#180): a quiet `postToRoom` post carrying `source.human { authorId, name }`. `group_relay` turns it into a round reply `{ kind: 'user', other: true, name, authorId }` (not the grey `notice`), so it draws as a left-hand bubble from that person; it asks no Bot to answer. `postNotice` stays for system events (joined, left, started). Covered by `core/test/rounds.test.js`. | ||
| - `flybot.chatApprovals.changed()` saves the team (moving the revision) so `bot.pendingCount` reaches the browser at once when a hold arrives or is decided (#226). Covered by `core/test/flybot.test.js`. | ||
| - The DSH composer still prompts the group Session (`user/message` → `group_relay`). For a face-to-face room, `openingSpeakers` returns no Bots unless the line `@Name`s a member or `@all`, so a plain human line is logged and does not start a Chief/Bot model turn (#167). Covered by `core/test/rounds.test.js`. | ||
| - The DSH composer still prompts the group Session (`user/message` → `group_relay`). Face-to-face turn rule (#167, changed in #287), `openingSpeakers`: the owner's own line on this home starts **one** local Bot, never every Bot at once. `@Name`, the label the picker shows (`Friend's Chief`, `Writer · Friend's Bot`) or a handle picks that local Bot (`face-mentions.js`); `@all` is everyone; a line that only @s a friend's Bot starts none here; a plain line goes to the room's admin, else its first member. A friend's line (`flybot.rooms.postHuman`, only when the server attests `typed_by_owner`) stays quiet unless it names my Bot by label or handle (`callIds` in the post's source, read by `openingSpeakers` before the quiet rule); a bare `@Chief` from a friend whose own Chief is in the room calls nobody here. A Bot's post in a face room still starts only the Bots it @s, so Bots in two homes cannot set each other off. The reply is held / sent under the usual approval and Auto-send rules. An `@` that names nobody in the room adds one quiet notice line (`No one in this room is called @X …`). A face-room line the send path already posted (`rt.notePostedLine`) is not appended or sent again when the round reads it from the Session. Covered by `core/test/rounds.test.js`, `core/test/face-mentions.test.js` and `plugins/social/test/social.test.js`. |
There was a problem hiding this comment.
The face-room channel branch now always falls through to sessionController.prompt, and postHuman can start a Bot when it carries callIds, but this same section still says on line 109 that channel send prompts only for a local mention and on line 111 that postHuman asks no Bot to answer. Those statements directly contradict the new invariant documented here, so update the older bullets and the stale channel comment before this subsystem contract guides a future change in the opposite direction.
Useful? React with 👍 / 👎.
…bels resolve; hint when an @ names nobody (ThinkFlowLab#287)
69036c0 to
1e7f129
Compare
…dles, receiver resolves by handle first (ThinkFlowLab#293) (ThinkFlowLab#294) Closes ThinkFlowLab#293. Follow-up to ThinkFlowLab#287/ThinkFlowLab#288 (the sender-side explicit mention list left "not done"). Companion: Birfy/bot-social#9/#10 (merged, `1870786`; **not deployed**). ## What - **Sender:** the owner's typed face-room line carries the server handle of every Bot its `@` names (`faceMentions().handles`, read against the room; core `onFaceLine(fn)` now passes `{ mentions }`). The social plugin sends it as `mentions` next to `typed_by_owner`, only when `GET /v1/platform` `features` lists `mentions` (same shared probe as typed_by_owner). Never on the legacy envelope, Bot lines or an older server. - **Receiver:** `mentions` is read only from a line the server attests `typed_by_owner` (`cleanMentions`, <= 8 handles). `postHuman` resolves the handles against the room's participants first (`mentionedByHandle`): only my own Bots in the list answer. A list naming nobody the room knows, or no list (older server/client), falls back to the existing label/name reading (ThinkFlowLab#287). - The composer's `@` picker only inserts the label text (the DSH shell's trigger menu returns `{text}`), so the handles are derived from the picked labels at send time instead of being tracked in the composer. - Old servers/clients keep working unchanged. No instance upgrades. ## Tests core: `face-mentions` (handles, resolver), `flybot` (onFaceLine mentions, postHuman by handle vs text); social: send gated by feature/typed/Bot lines/legacy, receive honoured only on attested lines, fallback, malformed list. `TZ=UTC npm run verify` passes. Co-authored-by: Birfy <15378933+Birfy@users.noreply.github.com>
… pass, capped (ThinkFlowLab#295) (ThinkFlowLab#296) Closes ThinkFlowLab#295. Follow-up to ThinkFlowLab#288/ThinkFlowLab#287 (user feedback: only the first Bot answered a plain line). ## Rule - **Plain line** (no `@` naming a Bot) in a face room: one round, every local Bot in the room may answer, **admin first, then the room's order**, **capped at 6 speakers**, one at a time, each seeing the earlier replies (the existing sequential round). The round is `single`: the replies' own `@`s start no follow-up turns inside it. - **Both homes:** the owner's line starts my Bots; a friend's line starts my Bots too, but **only when the server attests it `typed_by_owner`** (the plugin passes `attested` to `postHuman`; a plain or `@all` attested line carries `plain: true` in the post source). An attested line naming Bots calls only those; one naming only a friend's Bot calls none here; a server that cannot attest never starts Bots. - `@Name` / label / handle still targets only that Bot; `@all` unchanged (everyone, uncapped as before). A Bot's own post still starts only the Bots it `@`s and a remote Bot's post starts none, so two homes cannot set each other off. Approval / Auto-send gating of every outbound Bot line is unchanged. - Cross-home replies are not sequenced across homes: each home runs its own pass; replies from the other home are in the log once they arrive. ## Tests core `rounds` (plain pass over two local Bots with a friend's Bot present, sees earlier reply, no follow-ups, `@Writer` only one, Bot post none, unattested none, attested plain starts both; large room capped at 6), `flybot` (`postHuman` plain/@all/named/unattested/list), social (only stamped lines are `attested`). `TZ=UTC npm run verify` passes. --------- Co-authored-by: Birfy <15378933+Birfy@users.noreply.github.com>
Fixes #287. Changes the rule of #167.
Turn rule (face-to-face room),
openingSpeakers@Name, the picker label (Friend's Chief,Writer · Friend's Bot) or a handle picks it;@allis everyone; a line that only @s a friend's Bot starts none here; a plain line goes to the room's admin, else its first member. Replies stay under the usual approval / Auto-send holds.postHuman, only when Bot Social atteststyped_by_owner) stays quiet unless it names one of my Bots by label or handle (callIds): only those answer. A bare@Chieffrom a friend whose own Chief is in the room calls nobody here (ambiguous).@that names nobody in the room adds one quiet line: "No one in this room is called @x. …".Cross-home labels: new
core/src/host/face-mentions.jsbuilds every alias of a participant (bare name,<owner>'s Chief,<name> · <owner>'s Bot,<name> · <owner>, handle) from the room's participants, local and friend's.Also fixed on the way: a face-room line posted by the send path was appended and sent a second time when the round read it from the Session (so every plain line would now go out twice);
rt.notePostedLineskips that second append.Not done: sender-side explicit mention list on the wire. Bot Social messages carry text only, so that needs a server field; the receiver reading the labels (the one both homes already show) covers it without a server change.
Tests:
core/test/face-mentions.test.js,core/test/rounds.test.js(plain line, friend's @, label, hint, no double send), a two-home case inplugins/social/test(attested line by label and handle calls my Chief; bare name, plain line and unattested line call none). Docs and changelog updated.