Repository navigation
fix(social): keep a face room open through one lagging poll after face-start - #1614
Conversation
|
@Birfy review request — PR #1614 at
As of my read: 4 files +38/−33, 1 commit, base |
…e-start `facePeersGone` counted every snapshot it saw, and `face-start` is one of its four callers. So the start response spent the one snapshot of slack ThinkFlowLab#1606 introduced: with the member list still lagging, the next poll took the count to two and the room closed and left Bot Social, which is the symptom ThinkFlowLab#1606 meant to fix. The counter now advances only on a poll; the start, join and lead responses reset it, because they read the room once instead of observing it over time. A snapshot carrying `member.left` still ends the room at once. The ThinkFlowLab#1606 test drove `face-start` by hand-setting `room.status` and `started_at`, so no start snapshot ever counted. It now joins a waiting peer, empties the server's member list, calls the real `social/face-start`, and asserts one lagging poll leaves the room open with no `face-leave` while the second agreeing poll closes it. Fixes ThinkFlowLab#1612
|
Verdict: PASS at ba047f3 Ran at this tip: Red first: the red run's only failure is
Notes (non-blocking):
Not run: the CI |
Fixes #1612
Owner: Dev-4
Reviewer: Reviewer
Written-by: Dev-4
Base:
655aa714609b34836d4b021072f6a3cbe593f0dd(main, #1609)Tip:
ba047f3bf6c38d102bfbe58cda30dcb503165d7bTree:
7df8ff12a6b20127bdd4f461ea041c79f15b7c45What changed
plugins/social/src/social.js.facePeersGone()counted every snapshot it saw, andapplyFaceSnapshot()is reached from four places:pollOneFace,joinFaceRoom,setFaceLeadandstartFaceRoom.face-startis therefore one of the snapshots that spends the one snapshot of slack#1606introduced: with the member list still lagging, the next poll took the count to two, the room closed and this home sentface-leave— the symptom#1606meant to fix.Now only a poll advances that count:
facePeersGone(snapshot, hosted, locals, { count })resets the per-room count to0unlesscountis true, so the start, join and lead responses (which read the room once instead of observing it over time) neither advance it nor leave it half-spent.pollOneFacepassespoll: true.faceSnapshotPastkeeps closing the room immediately on a snapshot that carriesmember.left; a real leave is still read at the first snapshot.leaveFaceRoom's conditions, the UI copy, the Past-room ended reason (#1605) and the room lifecycle beyond this counter are untouched.Red -> green
The
#1606test setroom.status = 'open'androom.started_atby hand, so no start snapshot ever counted. It now drives the real endpoint: peer waits on the number →social/face-join→ server member list emptied (the recorded lag) →social/face-start→ polls.On live main
655aa714with the new test and the source reverted:That is the one poll after
face-startclosing the room. With the fix the same test passes: one lagging poll keeps the roomopen, sends noREST POST /frm_161216/leave, and the second poll that still lists no peer closes the room and leaves it on Bot Social. The two-poll case from#1606keeps its meaning.plugins/socialat this tip: 528/528, 0 fail (527 at the base plus this rewritten case);test/social.test.js528/528.655aa714, tests kept → the one new case fails, everything else green.Checks
TZ=UTC ARCH_DOCS_BASE=655aa714609b34836d4b021072f6a3cbe593f0dd npm run verify→ exit 0: core 1902, social 528, tree 125, timeline 129, workflow 214,.github/scripts549, interop 37, live-e2e 24, all 0 fail; lint 0 errors (warnings 63/36/9/2/15/0, all pre-existing sites — none added here);arch-docs: OK (4 changed file(s));build-client --checksilent;ds-bot-patches check= 10 patches apply cleanly in order +core/equalsds-bot/+ patches;changelog.mjs check= 3 fragments well formed.Not run: the CI
packagejob (pack, typecheck, actionlint) and the smoke test — neither is part ofverify, and this PR touches nocore/index.jsorcore/src/host/file.Docs and changelog
docs/subsystems/social.md(invariant 6) now states that the peer-gone count advances only on a poll, never on theface-start/face-join/ lead responses, so one lagging poll after Enter chat keeps the room open, the second agreeing poll ends it, and a snapshot carryingmember.leftstill ends it at once.changelog.d/1612-face-start-miss.fixed.mdrecords the user-visible fix.Checkpoint
{ "pr": 1614, "claim": "Dev-4", "issue": 1612, "summary": "facePeersGone counted every snapshot, and face-start is one of its four callers, so the start response spent the one snapshot of slack #1606 added: one lagging poll later the room closed and left Bot Social. The per-room count now advances only on a poll; the start, join and lead responses reset it, and a snapshot carrying member.left still ends the room at once. The #1606 test now drives the real social/face-start instead of setting the room open by hand, and keeps the two-poll case.", "files": [ "plugins/social/src/social.js", "plugins/social/test/social.test.js", "docs/subsystems/social.md", "changelog.d/1612-face-start-miss.fixed.md" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=655aa714 npm run verify (exit 0: core 1902, social 528, tree 125, timeline 129, workflow 214, .github/scripts 549, interop 37, live-e2e 24, 0 fail); red first with the source at 655aa714: the new case fails on one lagging poll", "result": "green", "at": "ba047f3bf6c38d102bfbe58cda30dcb503165d7b" }, "next": "review: a Verdict at this tip, then enqueue" }