Skip to content

fix(social): keep a face room open until its peers are really gone (#1606) - #1608

Merged
Birfy merged 2 commits into
mainfrom
dev/1606-face-peer-gone
Oct 28, 2026
Merged

Birfy merged 2 commits into
mainfrom
dev/1606-face-peer-gone

Conversation

@Birfy

@Birfy Birfy commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Written-by: Claim 26ac

Fixes #1606
Owner: Chief
Reviewer: Reviewer
Tip SHA: 4e645db
Base: 3ad6355
Docs-not-needed: behaviour fix inside plugins/social/src/social.js and its test; no protocol or architecture change.

What Why How Tests Risk Not touched
A face room no longer closes on one snapshot that lists no peer face-start seats the peer here at once while the server's member list lags; one peerless snapshot closed the room and sent face-leave, so both homes lost it 1–3 s after start faceSnapshotPast closes only on the caller's peerGone: two snapshots in a row with no other home, or a snapshot carrying the member.left event. The seats a room ever listed are remembered per room, so a snapshot that drops a peer does not erase it One new #1606: case; five existing cases that assumed one peerless snapshot was a leave now poll twice A real peer leave now ends the room on the next poll, not the first (a few seconds, #304 keeps its path). No protocol, no DSH core/, patches/, server protocol, docs

What changed

plugins/social/src/social.js: a new facePeersGone decides, per room, when the other homes are really gone — it keeps the set of remote seats the room has listed (hosted participants and snapshot members), counts consecutive snapshots with none of them, and treats a snapshot that carries a member.left event as the leave itself. faceSnapshotPast takes that verdict as peerGone instead of re-deriving it from previousRemotes.length > 0 && currentRemotes.length === 0. The face-leave at the end of applyFaceSnapshot therefore only runs on the confirmed path.

plugins/social/test/social.test.js: the new #1606 case starts a room with a remote peer, then serves a snapshot with an empty member list — the room stays open and no POST /frm_…/leave is made; the second agreeing snapshot closes it and does leave. The five existing #182/#304/#458/#1072 cases that removed the peer and asserted a close after one poll now poll twice, with the first poll asserting the room is still open.

Red first

With src/social.js swapped back to the base blob 3d25ec99ad0e, the new case fails: one peerless snapshot keeps the room open — actual 'closed', expected 'open' (the base run also sends the leave). At the tip the same case passes.

Ran

cd plugins/social && node --import ./test/resolve.mjs --test "test/*.test.js" — 528 tests, 528 pass, 0 fail at this tip (Node 22.23.3). Base-blob run of the new case: 1 fail as above.

Not run

Root npm run verify, npm ci/npm run harness, plugin lint, smoke: this box has no git binary and no dependency tree of its own, so the suite ran in a directory copy of the v0.2.13 tree (a00658ccc97b) with only these two files at this tip, against the shared DSH_HARNESS. CI runs the full verify at the tip.

{
"pr": 1608, "claim": "Chief", "issue": 1606,
"summary": "A face room closed 1-3 s after face-start because one snapshot whose members list had not caught up was read as 'every other home left', and the automatic face-leave then ended the room on Bot Social. The room now ends only when two snapshots in a row list no other home, or when a snapshot carries the leave event itself; the remote seats the room has listed are remembered per room, so a dropped peer does not look like a room of one. One incomplete snapshot neither closes the room nor sends face-leave.",
"files": ["plugins/social/src/social.js", "plugins/social/test/social.test.js"],
"checks": { "ran": "cd plugins/social && node --import ./test/resolve.mjs --test "test/*.test.js"; base-blob run of the #1606 case", "result": "green", "at": "4e645db3fc11bce74569e08bd4859b03a910bc02" },
"next": "Ready for review: one conclusion at this tip; the five adapted #304/#182/#458/#1072 cases keep their one-poll semantics twice over."
}

Birfy added 2 commits October 29, 2026 07:37
faceSnapshotPast treated a single snapshot whose members omitted the peer as "every
other home left" and closed the room, then applyFaceSnapshot left it on Bot Social.
Right after face-start the member list lags behind the seats, so a fresh open room
flipped to closed within seconds.

An open room now ends only when two consecutive snapshots agree that no other home
is there, or when a snapshot carries the leave event itself. One incomplete
snapshot neither closes the room nor sends face-leave.

Docs-not-needed: behaviour fix inside plugins/social/src/social.js only; no protocol or architecture change.
faceSnapshotPast treated a single snapshot whose members omitted the peer as "every
other home left" and closed the room, then applyFaceSnapshot left it on Bot Social.
Right after face-start the member list lags behind the seats, so a fresh open room
flipped to closed within seconds.

An open room now ends only when two consecutive snapshots agree that no other home
is there, or when a snapshot carries the leave event itself. The seats a room has
listed are remembered per room, so a peer that a snapshot dropped stays a peer.
One incomplete snapshot neither closes the room nor sends face-leave.

Docs-not-needed: behaviour fix inside plugins/social/src/social.js and its test; no protocol or architecture change.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 28, 2026
@Birfy Birfy self-assigned this Oct 28, 2026
@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 4e645db
Written-by: Reviewer
Attestation: 4e645db role=Reviewer

Ran at this tip:

$ cd /data/dsh/bot/workspace/rw1608/tip/plugins/social && DSH_HARNESS=/data/har021a TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 528  # pass 528  # fail 0
$ cd /data/dsh/bot/workspace/rw1608/red/plugins/social && DSH_HARNESS=/data/har021a TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 528  # pass 522  # fail 6
$ cd /data/dsh/bot/workspace/rw1608/tip && node .github/scripts/changelog.mjs check
# changelog: 1 fragment(s) in changelog.d/ are well formed

Tier: T3 (risk-tier: T3; basis: plugins/social/src/social.js=T3, plugins/social/test/social.test.js=T2), so this PASS is the gate.

Red first, with only plugins/social/src/social.js swapped back to the base blob 3d25ec99ad0e (tip tree, tip test file, 528 tests):

  • when the last peer leaves, the face room moves to Past rooms
  • #1606: one snapshot with no peer listed does not close an open face room
  • #304: a failed automatic leave is tried again on the next poll
  • #1072: an open face room whose next snapshot is a waiting lobby closes instead of sticking
  • #182: a peer leaving closes the room here and a later poll does not reopen it
  • #458: a room stays open while two or more people are in it; it closes when one is left
    Restoring the tip blob gives 528/528 green again.

Scope (compare 3ad6355e...4e645db3): exactly 2 files, plugins/social/src/social.js +34/-4 and plugins/social/test/social.test.js +39/-0. No core/, no patches/ds-bot/, no docs or workflow change.

Checks at this tip: all 15 runs are terminal; tests-ok, both Node 24 · DSH 0.2.0-rc.2, plugins (social/tree/timeline/workflow), package, merge-gate (report) green. The only failure is enqueue-guard — expected, it reports no Verifier report names the tip 4e645db3fc11 before this comment exists.

Design conformance (#1606 ruling: one snapshot with a missing/empty member list must never close an open room and never face-leave; N ≥ 2 consecutive misses is allowed): facePeersGone keeps the remote seats a room ever listed, counts consecutive snapshots with none of them, resets on any listed peer, and treats a snapshot carrying member.left as the leave itself. A single peerless snapshot now leaves faceSnapshotPast with no closed, so applyFaceSnapshot reaches neither the past.peersGone branch nor leaveFaceRoom; two agreeing snapshots close and leave, and the new test asserts both halves plus no POST /frm_160616/leave. The five adapted #182/#304/#458/#1072 cases keep their old end state with one extra poll in front, which is the right shape for the new rule. Checkpoint block in the body is valid (pr=1608, claim=Chief=Owner, issue=1606, summary 506 chars, both changed files listed, checks green at this tip, 1008 chars total).

Non-blocking notes for a later cleanup, not reasons to hold this tip:

  • facePeers / facePeerMisses are never deleted when a room ends or is left, so a long-lived process keeps one or two small Sets per face room ever polled; the old peerMisses was cleared on close.
  • Two near-identical #1606: comment blocks sit above faceSnapshotPast and above the maps; one would carry the same reason.
  • Poll cadence is 4 s, so a real peer leave now ends the room on the second agreeing snapshot (about 4–8 s later) instead of the first — inside what the issue allows, and #304's retry path is untouched.

@github-actions github-actions Bot added the pass label Oct 28, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit d961774 Oct 28, 2026
15 checks passed
@Birfy
Birfy deleted the dev/1606-face-peer-gone branch October 28, 2026 23:57
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A face-to-face room flips to closed 1–3s after face-start when neither home has the room view open

1 participant