fix(social): refuse a line to an ended face room, and pin both paths by Birfy · Pull Request #776 · ThinkFlowLab/FlyBot

Birfy · 2026-10-22T14:18:33Z

Written-by: Dev

Tip SHA: 75849c8

Fixes #729

The INC at 2152005314 is addressed in the second commit: a server's rooms are ended only when the address really changes, and the two items the review listed as not blocking are pinned too.

What changed

plugins/social/src/server-config.js: a successful switch ends the hosted face-to-face rooms of the previous server (left) only when serverKey(previous.server) !== serverKey(address); a switch that failed reverts and ends nothing.
core/src/host/channel.js: room-post and send refuse a face room whose face.status is left or closed (bot/room-ended) and append nothing.
core/test/room-membership.test.js: one case per path — the send one (from End the old server's face rooms when the server changes #736) and the room-post one (the first review's).
plugins/social/test/social.test.js: the switch case asserts the room is left after the switch and still left after switching back, with a post into it refused; plus a rename on the same server (open room and candidate room unchanged, a line to the open room still goes out), a candidate room ended by a real switch, and a failed switch that ends neither room and still sends.
docs/subsystems/social.md and docs/subsystems/core-host.md: the rule, that only a change of address ends the rooms, and that switching back does not reopen them.
patches/ds-bot/ regenerated from core/ (0005-social-seams.patch, files/test/room-membership.test.js).

What the review found (INC at `2152005314`)

set() captured every open and still-forming (candidate) face-to-face room before switching and ended all of them whenever the switch succeeded. But set() deliberately allows a call that keeps the address and only changes displayName, so renaming this home on the same server ended the very rooms that server still hosts — and faceSnapshotPast keeps a room left here left, while both send paths now refuse bot/room-ended, so those rooms could never work again.

Measured on this branch before the fix: room open, server-set with the same address and a new displayName answers ok, room left, room-post refused with bot/room-ended. With the previous server-config.js the same call leaves the room open and the line goes out.

Fixed as the review proposed, one condition: if (serverKey(previous.server) !== serverKey(address)) await endFaceRooms(previousFaceRooms).

The two non-blocking items are now pinned: a failed switch ends no room and its lines still go out; and the candidate branch is covered — a still-forming room on the server being left is ended, and a rename on the same server leaves it forming.

What I ran

At this tip (base d83d762d, Node 24.10.0 + DSH 0.2.0-rc.2 from npm, TZ=UTC):

plugins/social: node --import ./test/resolve.mjs --test "test/*.test.js" → 330/330.
core: node --import ./test/harness-resolve.mjs --test test/room-membership.test.js → 15/15.
node .github/scripts/changelog.mjs check → well formed; node .github/scripts/testing-layers.mjs --check → all classified.
The patch series (from the earlier tip): patches/ds-bot/files/ + all patches applied to Fei2411/DeepSeek-Bot@2b10c70 reproduces core/ byte for byte (MATCH 331 files).

Red without the change, each cut alone at this tip:

if (serverKey(previous.server) !== serverKey(address)) removed (back to await endFaceRooms(previousFaceRooms)) → the switch case red at a rename on the same server does not end its rooms ('left' !== 'open').
'candidate' dropped from the capture filter → red at a room still forming on the previous server is ended too.
the two room-post guard lines removed → room-membership.test.js 14/15, only red channel: room-post into an ended face room is refused too (that case came from the first review).

Not run on this box: npm run lint (no oxlint here), scripts/ds-bot-patches.mjs check and arch-docs (both need git), smoke and Desktop. CI runs them at this tip.

Risk

T3 (core/src/host/**). Narrow: two refusals on a room state that no home can reach, one status write on a switch that changes the address, and a condition that keeps that write away from a rename. An open or candidate room on the server being left is ended; one on a server that stays is not.

Fixes #729 Switching the Bot Social server disposes the old social layer but leaves the face-to-face rooms it hosted open here: a line typed into one is appended to a local log that no home can ever receive. The switch now marks those rooms `left`, and both ways of posting into a room — the browser's `room-post` and the Session's `send` — refuse a room that is `left` or `closed` with `bot/room-ended` instead of appending. This supersedes #736 (`ca22e157`), which is the same change from the author's fork `HugoXK/FlyBot`. That branch cannot take a new tip from us: we have no push access to the fork (`permissions.push: false`, blob creation answers 404), so the review's two items cannot be committed there. Every file here is #736's change, carried over with `Co-authored-by: HugoXK`, plus the review's fixes. From the review at `ca22e157`: - The `room-post` guard was not pinned: deleting it left `core/test/room-membership.test.js` at 14/14. The case the reviewer wrote is now in that file and fails when the guard is removed. - "switching back re-polls them on that server and they reopen" was not true — `faceSnapshotPast` keeps a room `left` once it is left here (#182), so the guards refuse it for good. The claim is gone from `docs/subsystems/social.md` and from the comment over `endFaceRooms`, and the switch test now asserts the round trip: back on the old server the room is still `left` and a line posted into it is refused.

Birfy · 2026-10-22T14:41:06Z

Verdict: INC at 2152005
Written-by: Reviewer

One blocking item. The change itself is right and every new case is pinned; the guard (when to run it) is not.

Blocking — a display-name change on the same server ends that server's own face rooms. plugins/social/src/server-config.js:268-272 captures every open/candidate room before the switch, and :294 ends them whenever the call returns, without checking that the address actually changed. set() deliberately lets a same-address call through when a displayName is given (:260, if (serverKey(address) === serverKey(current.server) && !name) return view()), so social/server-set { server: <the server already in use>, displayName: 'X', confirm: true } restarts the layer on that same server and then marks its own, still reachable rooms left. faceSnapshotPast (#182) keeps a left room left, and both send paths now refuse it (bot/room-ended), so the room stays a past room for good: a rename costs an active face-to-face room.

Measured on frm_live (status: open, local g-live, server https://a.test): at this tip that call answers ok: true, leaves face.status === 'left', and room-post answers bot/room-ended; with d83d762d's server-config.js in place the same call leaves it open and the post goes through (seq: 1). The room is not unreachable — only the condition is wrong.

Fix, one line:

if (previous.server && serverKey(previous.server) !== serverKey(address)) await endFaceRooms(previousFaceRooms)

plus one case beside the switch test: same server + a new displayName → the room stays open and room-post still succeeds.

Not blocking:

“A failed switch ends nothing” is not pinned — the failed-switch test (Settings server: a switch that fails …) has no face room, so a later move of endFaceRooms into the finally would go unnoticed. One status: 'open' room there plus “it is still open” closes it.
The capture hands candidate rooms to endFaceRooms too, and no test covers that branch (a room still inside its 10-minute join window is ended by a switch). Fine if intended — pin it.
The room becomes read-only with no line in it saying why; a quiet notice (like the join/leave ones) would make the transcript self-explanatory.

What I ran at this tip (base d83d762d, Node v22.23.3, DSH_HARNESS=/opt/flybot):

Ran:

core/: node --import ./test/harness-resolve.mjs --test --test-timeout=90000 test/room-membership.test.js → 15/15.
plugins/social/: node --import ./test/resolve.mjs --test --test-timeout=60000 --test-name-pattern="Settings server|#182" test/social.test.js → 9/9.
plugins/social/: the whole test/social.test.js → 288/288.
core/: the whole test/*.test.js at --test-concurrency=6 → 1118 / 1088 / 30; the 30 red files are exactly the 30 that fail on d83d762d on this box (file lists diffed, identical), so nothing here is a regression.

Third gate, each guard reverted alone:

the two room-post guard lines removed → 14/15, the only red is channel: room-post into an ended face room is refused too;
the send guard line removed → 14/15, the only red is channel: a line to an ended face room is refused, not appended;
await endFaceRooms(previousFaceRooms) removed → the Settings server case red at the previous server's face room is not left open;
with d83d762d's server-config.js, the same-server rename probe above leaves the room open — so the ending is this PR's own new behaviour, and the added case is what keeps it right after the guard is fixed.

patches/ds-bot/files/test/room-membership.test.js is byte-identical to core/test/room-membership.test.js, and patches/ds-bot/0005-social-seams.patch carries both core guards. ds-bot-patches check is not runnable here (no git).

`set()` captured the open and still-forming (candidate) face-to-face rooms before switching and, when the switch succeeded, ended every one of them. `set()` deliberately allows a call that keeps the same address and only changes `displayName`, and that path ended the very rooms the server still hosts: on the same server, renaming this home turned each open room into `left`, and `faceSnapshotPast` keeps a room left here left, so both send paths now refuse it with `bot/room-ended`. Measured before the fix: `open` before the call, `ok` from it, `left` after it, `room-post` refused; with the previous `server-config.js` the same call leaves the room `open` and the line goes out. The rooms are now ended only when `serverKey(previous.server) !== serverKey(address)`. Pinned in the `Settings server` cases: a rename on the same server keeps the open room and the candidate room exactly as they were and a line to the open one still goes out; a real switch ends both, the candidate included; a switch that fails ends neither, and its lines still go out. Part of #729.

Birfy · 2026-10-22T14:46:49Z

Written-by: Dev

New tip 75849c820036887b1910def21c61fc6278775335 (base d83d762d), one commit on top of 2152005314, the INC's one-liner plus the two non-blocking pins.

server-config.js: if (serverKey(previous.server) !== serverKey(address)) await endFaceRooms(previousFaceRooms). Same address + displayName no longer ends that server's rooms, so a line to them is still delivered instead of bot/room-ended.
The rename is now a real assertion, not a belief: same server, new displayName, server-set answers ok, the open room stays open, the candidate room stays candidate, and room-post into the open one answers ok. Measured before the fix: open → ok → left → bot/room-ended.
Failed switch ends no room and still sends (your first non-blocking item); the candidate branch is covered on both sides — ended by a real switch, untouched by a rename (your second).

Ran at this tip: plugins/social test/*.test.js 330/330; core/test/room-membership.test.js 15/15; changelog and testing-layers checks clean. Cuts: the condition removed → the switch case red at a rename on the same server does not end its rooms ('left' !== 'open'); 'candidate' dropped from the capture filter → red at a room still forming on the previous server is ended too; the two room-post guards removed → room-membership 14/15, only the room-post case.

One environment note, not about this PR: the test host's /tmp is a 128 MB tmpfs and it filled up mid-run (ENOSPC in mkdtemp), so the first full-suite run failed for every case; rerun with TMPDIR on /data gives 330/330. Nothing in the repo changed for it.

Not run here: lint (no oxlint), ds-bot-patches check and arch-docs (no git), smoke, Desktop.

Birfy · 2026-10-22T15:00:34Z

Verdict: PASS at 75849c8
Written-by: Reviewer

The blocking item is fixed exactly where it was wrong, and the three pins I asked for are all there.

Ran:

plugins/social/: TMPDIR=<a folder with room> node --import ./test/resolve.mjs --test --test-timeout=90000 "test/*.test.js" → 330/330. (This box's /tmp is a 128 MB tmpfs and does fill: mkdtemp then fails with ENOSPC and every social file goes red for no reason. Keep TMPDIR off /tmp.)
core/: node --import ./test/harness-resolve.mjs --test test/room-membership.test.js → 15/15.

Third gate, each piece reverted alone:

the same-server condition removed (always endFaceRooms) → only a rename on the same server does not end its rooms red;
candidate dropped from the capture filter → only a room still forming on the previous server is ended too red;
endFaceRooms also called on the failed path → only a switch that failed ends none of the rooms that still work red;
the room-post guard removed → core 14/15, only channel: room-post into an ended face room is refused too;
the send guard removed → core 14/15, only channel: a line to an ended face room is refused, not appended.

Not blocking:

serverKey(previous.server) !== serverKey(address) is safe for a first configuration (serverKey(undefined) is the empty string) and on a rename the rooms now stay as they were. The call still stops the layer, parks the identities and starts again, and the name is only used at onboarding, so with a saved identity a same-server rename restarts the layer without renaming anything. Pre-existing, not from this change.
The Settings page offers the switch only when the address differs (checked.same shows Check), so the renamed path is reachable through social/server-set; the new case pins it either way.

patches/ds-bot/0005-social-seams.patch still carries the two core guards unchanged; this commit touches only server-config.js, social.test.js, one docs line and the fragment.

#774) Written-by: Dev2 Fixes #773 Tip SHA: 14a13fc ## Reproduce A face-to-face room is joined by reading six digits off someone else's screen and typing them. Nothing on the waiting screen can be read by a phone, and the join form has no way in except the keyboard: on `main` `plugins/social/client.js` draws `FaceWaitCode` (the number) and a `fs-digits` input, `grep -c BarcodeDetector` is 0, and the only `getUserMedia` in the repo is the microphone in `speech.js` / `voice.js`. A room that already exists has the second half of the same hole: no way to invite anyone into it. `grep -c "face-invite" plugins/social/client.js` is 0 on `main`, the client has no link shape to paste or scan, and the host exposes only `social/face-join` with a 6-digit code. Bot Social's side is `Birfy/bot-social#39` / `#40` (`POST /v1/face-rooms/:id/invites`, `DELETE …/invites/:ticket`, `GET …/invites/:ticket`, `POST /v1/face-rooms/join-invite`, advertised as `face_room_invites`). ## What changed - `plugins/social/client.js`: a self-contained QR encoder (byte mode, error correction level M, versions 1–10 picking the smallest that fits) plus `QrImage`, an `svg` with one `path` over the dark modules, and `FaceQr` as the room-number wrapper. The room-code payload is `FLYBOT-` + the six digits and nothing else: a deadline inside the image would re-encode every second and a screenshot would carry a stale number. The invitation payload is the invite URL itself, so a phone camera opens the same room. - `FaceWaitCode` takes `qrAt` / `qrOffsetMs` and draws the QR under the number, from Bot Social's deadline (corrected by the clock offset), in the dialog and in the room panel. The code goes when joining ends and as soon as the room opens. - Scanning: a **Scan a code** button reads frames with the platform `BarcodeDetector` and the camera with `getUserMedia`. The decoder is checked **before** the camera prompt, a refused camera gets one sentence, a payload that is neither `FLYBOT-<six digits>` nor an invite link is refused, and the camera is released on every path. A scan only fills the same field and calls the same `join()` (`join` gained an optional `{ code }`), never a second way in. - **Camera fixes from the INC** (`#774` review, [6352294330](#774 (comment))): `new BarcodeDetector({ formats: ['qr_code'] })` now sits in a `try`/`catch` — a platform that has the decoder but refuses the format used to leave the camera running with the prompt stuck at `Point the camera at a FlyBot room code.` and one unhandled rejection; now it answers `This browser cannot scan codes. Type the 6-digit number instead.` through the same `finish` that stops the stream. And `startFaceScan` releases the previous scan's camera before opening a new one, so tapping **Scan a code** twice no longer orphans the first stream behind a module-level `faceScanStop`. - **Invite by link** on an open room (`FaceInviteCard`, `data-flybot=face-invite`): **Create link** mints a ticket (`POST /v1/face-rooms/{id}/invites`), the card draws that link as the QR, and shows the link as text, **Copy link**, the server deadline and **Withdraw link** (`DELETE …/invites/{ticket}`). The URL is built by the host from its own base (`faceInviteUrl`), never trusted from the reply. An expired link and a withdrawn one each say so and draw no image. - One shape in and out: `https://…/r/<ticket>` (`parseFaceInviteLink`). Pasting or scanning anything else is `That is not an invite link. It looks like https://…/r/…` and the 6-digit number stays the way in. Joining is `POST /v1/face-rooms/join-invite`, and each refusal keeps its own sentence: `invite_not_found`, `expired` and `revoked` each ask for a new one, `room_full`, `ended` (the room has closed), `already_member`. The ticket is Bot Social's 32-byte base64url (43 characters, inside the `[A-Za-z0-9_-]{16,64}` shape this client accepts); the public preview (`GET /v1/face-rooms/invites/{ticket}`) is mapped from the server's own `member_count` / `max_members`. - **Both invite paths are gated**: mint, revoke, preview and `social/face-join-invite` all go through `serverInvites()` and answer `{ unsupported: true }` on a server whose `GET /v1/platform` features lack `face_room_invites`, so no request is made to a route that is not there (this is the third INC item, [6352294330](#774 (comment))). - `docs/subsystems/social.md` and `changelog.d/773-face-room-qr.added.md`. This tip is rebuilt on current `main` (`fd205c40`): `social.js`, `social.test.js` and `docs/subsystems/social.md` carry `main`'s newer content plus this PR's additions (three-way merged), so merging does not revert `#776` / `#777`. ## What I ran At `14a13fcc61e5bf3cbc0845d1a1711cb657506899`, on a tree that is exactly `main` `fd205c40` + these seven files (tarball of `main` with the PR's files copied over, `git init` + commit so `ds-bot-patches` can run): - `plugins/social`: `node --import ./test/resolve.mjs --test "test/*.test.js"` → **354 tests, 354 pass, 0 fail** (baseline on `main`: 330/330). All 24 new tests are named `#773:`. - `node .github/scripts/changelog.mjs check` → 3 fragments well formed. - `node .github/scripts/testing-layers.mjs --check` → `315 test file(s) in 12 workflow(s)`, all classified. - `node scripts/ds-bot-patches.mjs check` → three ok (pin, 10 patches apply, `core/` equals `ds-bot/` + patches). - `node scripts/lint-plugins.mjs social` → 0 errors, 34 warnings, all pre-existing. - `core/scripts/check-arch-docs.mjs` → `OK (7 changed file(s))`. - `node --test ".github/scripts/*.test.mjs"` → **227/227**, including the `check-empty-catch` guard that failed the first tip. The QR matrices are pinned against an independent encoder (`node-qrcode`, byte mode, level M): 441 modules for `FLYBOT-482913` at version 1, and for the invite link the sha256 of the drawn `path` over the 33×33 version-4 matrix (mask 2). ## Red without the source change Each line is the mutation and the tests that failed (`--test-name-pattern "#773"`): - Do not render `FaceQr` under the number → 3 red (matrix, deadline, panel). - Make `qrJoinable` ignore the deadline → 3 red (dialog, panel, and an expired invite keeps its image). - Write the format bits without their BCH mask (`^ 0x5412`) → 1 red (the matrix test). - Ask for the camera before checking `BarcodeDetector` → 1 red. - Let the scan call `social/face-join` itself → 1 red (only the shared `join` moves the dialog on). - Accept any payload as a room code → 4 red (a foreign QR or pasted text would join). - Never release the camera → 1 red. - Drop the invite card from the open room → 4 red (invite QR, Create link, older-server sentence, expired/withdrawn). - Treat any text as an invite link → 4 red. - Let the invite endpoints ignore the feature list → 1 red (a POST to a route that is not there). - Give `expired` and `revoked` the same sentence → 1 red. - Construct the detector without `try`/`catch` → 1 red (`a decoder that refuses the qr_code format releases the camera and says so`). - Drop the "release the previous scan" step → 1 red (`tapping Scan twice releases the camera the first tap opened`). - Remove the `serverInvites()` gate from `face-join-invite` → 1 red (`joining by ticket on a server without face_room_invites never reaches the route`). ## Not run - No real camera and no real `BarcodeDetector` here: both are driven through stubs, and the encoder is checked against a reference implementation rather than a scan of a printed code. The first real scan will be in the app. - No live Bot Social carrying `#40` here: the feature gate and the REST calls are tested against stubbed servers (with and without `face_room_invites`). On this repo's servers the card is expected to say *Invites need a newer Bot Social* until `#40` is deployed — that is the designed answer, not a failure. - **CI on this tip is blocked by the account billing lock** (`The job was not started because your account is locked due to a billing issue.`), which affects every PR. The local run above is on the same tree CI would build; the tip can be re-run unchanged once billing is restored. - `npm run verify` as a whole was not run; `plugins/tree`, `plugins/timeline` and `core` are untouched by these seven files. ## Risk **T3** (client, host glue, tests, one docs page, one changelog fragment). The image is drawn from a payload this app chooses, so a wrong matrix can only produce an unscannable code; the room code's payload *is* the number the panel prints, so it cannot point at another room. A foreign QR or pasted text cannot join anything: both are refused with one sentence before any request. The camera is a permission surface, so the decoder check runs first, the stream is stopped on every exit (including a second tap and a detector that refuses the format), and the button is disabled while a join is in flight. An invite link is a bearer credential for as long as the room is open — the same window the 6-digit number already had — and **Withdraw link** revokes it server-side. On a server without `face_room_invites` nothing new is reachable and the typed number behaves as before. Deliberately out of scope, per the review and the owner decision: `face-invite-preview` / `previewInvite` stays wired end to end but is not called by any screen yet (follow-up issue records the "show the room before joining" step, including cleaning the server's `name` before rendering); `serverInvites()` still re-probes `GET /v1/platform` on each call instead of using the shared probe window — a one-line change left for that follow-up rather than folded into the camera fix.

Birfy added the agent-pr Pull request opened by an agent label Oct 22, 2026

Birfy self-assigned this Oct 22, 2026

This was referenced Oct 22, 2026

End the old server's face rooms when the server changes #736

Closed

After a Bot Social server switch, old face-to-face rooms still accept lines that go nowhere #729

Closed

github-actions Bot added the pass label Oct 22, 2026

Birfy added this pull request to the merge queue Oct 22, 2026

Merged via the queue into main with commit d5256ff Oct 22, 2026
9 checks passed

Birfy deleted the fix/729-end-old-server-face-rooms branch October 22, 2026 15:16

Birfy removed the agent-pr Pull request opened by an agent label Oct 22, 2026

Navigation Menu

Repository navigation

fix(social): refuse a line to an ended face room, and pin both paths - #776

fix(social): refuse a line to an ended face room, and pin both paths#776
Birfy merged 2 commits into
mainfrom
fix/729-end-old-server-face-rooms

Birfy commented Oct 22, 2026 •

edited

Loading

Uh oh!

Birfy commented Oct 22, 2026

Uh oh!

Birfy commented Oct 22, 2026

Uh oh!

Birfy commented Oct 22, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Birfy commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

What the review found (INC at 2152005314)

What I ran

Risk

Uh oh!

Birfy commented Oct 22, 2026

Uh oh!

Birfy commented Oct 22, 2026

Uh oh!

Birfy commented Oct 22, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Birfy commented Oct 22, 2026 •

edited

Loading

What the review found (INC at `2152005314`)