Skip to content

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

Closed
HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:fix/social-switch-ends-face-rooms
Closed

HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:fix/social-switch-ends-face-rooms

Conversation

@HugoXK

@HugoXK HugoXK commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

End the old server's face rooms when the server changes
Fixes #729
Switching the Bot Social server disposes the old social layer but leaves the face-to-face rooms it hosted in core state with face.status: 'open'. The new layer has none of them in its settings.faceRooms, so nothing polls them, and its faceApi belongs to the new server: a line typed into such a room was appended locally while the fanout failed (face_room_not_found / no speaker identity), which room-membership.js only warns about. The room still showed an open composer, so every line the person typed was silently lost.
When the switch succeeds, server-config.js now marks the previous server's open/candidate face-to-face rooms left (a read-only past room; switching back re-polls them on that server and they reopen). channel.js also refuses send and room-post for a face room whose status is left or closed (bot/room-ended) and appends nothing, so a stale client cannot write a line no home can receive. A failed switch never reaches this step.
plugins/social/test/social.test.js hosts a face room on server A, switches to B and asserts it is left (fails without the change: it stays open). core/test/room-membership.test.js sends to a left face room and asserts bot/room-ended with the room log unchanged (fails without the change: the line is appended). docs/subsystems/social.md and docs/subsystems/core-host.md record both.
Ran at the tip: npm run verify (green, TZ=UTC), ARCH_DOCS_BASE=upstream/main node core/scripts/check-arch-docs.mjs, ds-bot-patches generate + check. Not run: smoke (this macOS box has no setsid, so dev/bench.sh cannot start dsh web), Desktop, real-model steward.

Switching the Bot Social server disposed the old layer but left its
hosted face-to-face rooms open in core state. Nothing polled them from
the new server, and a line typed into one was appended locally while the
fanout failed with only a warning: every line silently lost. When the
switch succeeds, mark the previous server's open/candidate face rooms
`left` (a read-only past room; switching back re-polls and reopens
them), and refuse `send` / `room-post` for a face room whose status is
`left` or `closed` instead of appending a line no home can receive.

Fixes ThinkFlowLab#729
@Birfy

Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Verdict: INC at ca22e15
Written-by: Reviewer

Two items. The core of the change is right — both ends are refused, the switch marks the old server's rooms left, and the switch test does pin that — and every check is green at this tip. Neither item is a redesign.

  1. The room-post guard is not pinned. channel.js gains the same refusal on two paths, room-post (the browser posting into a room) and send (the Session path). The new test only covers send: I deleted the two room-post lines (const posted = rt.roomOf(…) and its fail('This room has ended', 'bot/room-ended')) and core/test/room-membership.test.js was still 14/14. So the UI path can silently go back to appending a line into a room no home can reach.

    This test passes at this tip as written and fails with the guard removed (posted.ok === true), so it can be added as is:

    test('channel: room-post into an ended face room is refused too', async (t) => {
      const { call, rt } = startChannel(t, TEAM)
      await rt.load()
      const room = await rt.upsertFaceRoom({ roomId: 'frm_left', localId: 'g-left', status: 'left', name: 'Ended room' })
      const before = room.log.length
      const posted = await call('room-post', { roomId: 'g-left', text: 'hello' })
      assert.equal(posted.ok, false)
      assert.equal(posted.error?.code, 'bot/room-ended')
      assert.equal(rt.roomOf('g-left').log.length, before, 'nothing was appended')
    })
  2. "switching back re-polls them on that server and they reopen" is not true. That sentence is in docs/subsystems/social.md and again as the comment over endFaceRooms in plugins/social/src/server-config.js, but social.js faceSnapshotPast holds the state it was left in: if (held === 'left' || held === 'closed') return { status: held }, with its own comment "Once the room is left or closed here, a later poll must not reopen it (Face-to-face: avatars and room title differ per home (regressions of #137/#172); Leave is abrupt and not seen by the peer #182)". applyFaceSnapshot passes that through as the status, so after A → B → A the room still reads left here and the new guards refuse every line typed into it: it is a past room for good, not "reopened". Nothing tests the round trip, so nothing tells the reader which of the two is intended.

    Pick one and make it true: either (a) drop the claim and say in social.md that switching is one-way for the old server's face rooms (you start a new room when you come back), or (b) actually reopen a room when its own server is selected again — the room already carries face.roomId and there is a place to clear the hold — and assert it in social.test.js by switching away and back, checking the status returns to open and a typed line is accepted. Either way, the comment and the doc must match the code.

Ran:

  • cd plugins/social && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/resolve.mjs --test --test-name-pattern="Settings server" test/social.test.js → 4/4 pass at this tip
  • red, as the PR body claims: await endFaceRooms(previousFaceRooms) commented out in server-config.js → 3 pass / 1 fail, Settings server: status, check and switch …
  • cd core && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/room-membership.test.js → 14/14 pass at this tip
  • red, the gap in item 1: the two room-post guard lines deleted → 14/14 still pass; with the test in item 1 added → 15/15 with the guard and 14/15 (the new case) without it

Not run: the full core suite and the full social suite (I ran the two files this PR touches and the switch cases); node scripts/ds-bot-patches.mjs check — it needs git, which this box does not have, so the generated patches/ds-bot/… files are covered only by CI's green tests-ok/package at this tip; no browser check (no client file changed).

@github-actions github-actions Bot added the risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff label Oct 22, 2026
@Birfy

Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Dev

Superseded by #776, which carries this change from ca22e157 (with Co-authored-by: HugoXK) plus both items from the INC at ca22e157:

  • the room-post guard now has its own case in core/test/room-membership.test.js — remove the guard and it goes 14/15, the new case red;
  • the "switching back re-polls them and they reopen" claim is gone from docs/subsystems/social.md and from the comment over endFaceRooms, and the switch test asserts the round trip: back on the old server the room is still left and a line posted into it is refused (bot/room-ended).

Why a new pull request rather than a new tip here: this branch lives in HugoXK/FlyBot and we cannot push to it (permissions.push: false; blob creation answers 404). maintainer_can_modify is on, but the API has no path for another owner's account to commit into that fork, so the fixes cannot land on this branch.

@Birfy Birfy closed this Oct 22, 2026
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 23, 2026
…hinkFlowLab#776)

Written-by: Dev

Tip SHA: 75849c8

Fixes ThinkFlowLab#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 ThinkFlowLab#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants