Skip to content

host: one settings-page seat change sends the guest already seated two identical joins - #2013

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/3-settings-seat-join-dedupe
Nov 1, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/3-settings-seat-join-dedupe

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2011

Owner: Dev-3
Written-by: Dev-3
Reviewer: Birfy
Claim-Id: 5fbe53113e8a43969c8f22f513253910

Tip: 4af0644
Tree: c8360dcece5d9a9b71f93fb60363c88cc36821de
Base: 1932812

What changed

#2004 made a seat change that moved an unnamed group's name broadcast the new name to the guests
already seated. The settings page then got two of those joins for one seat change: updateRoom runs the
seat path, saves, and sends one join of its own with the final name — and the seat path had already
sent one. A guest already seated received two identical joins (same seq, same name, same roster) on an
add and on a remove. Receivers are idempotent, so nothing visible changed, but the broadcast was doubled
and the claim that an arriving guest gets exactly one join was false on this path.

addRoomParticipant and removeRoomParticipant now take opts.broadcast, defaulting to on; updateRoom
passes { broadcast: false } for the seats it changes, because it sends that one join itself after the
save. Everything else keeps today's single broadcast from the seat path, so a guest added or removed by
hand or by a claim still tells the seated guests. The arriving seat's own targeted join is not a
broadcast and is still sent. An explicit name (or notice) in the settings patch is still told, once, by
updateRoom — the change does not touch that path.

Docs: the room-share paragraph of docs/subsystems/core-host.md now says the settings page sends one
join per change and the seat path it drives adds none.

Verification at the tip

Tip SHA: 4af0644dc2e0770d8112e0abf5e552728bcb6a76
Ran: TZ=UTC ARCH_DOCS_BASE=193281239a4a8d3c06b74afa10555fc1057696e8 npm run verify → exit 0, on a clean
checkout of this tip.
core 2415/2415 (the same core suite measures 2414/2414 on 19328123, so the tip adds exactly the one
new test); social 541, tree 127, timeline 135, workflow 336, .github/scripts 563, interop 37,
live-e2e 24, all 0 fail; core lint 73 warnings / 0 errors, plugin lint 36/9/2/16 warnings / 0 errors,
interop lint 1 warning / 0 errors, live-e2e lint 0 warnings / 0 errors; arch-docs: OK (7 changed file(s)) measured against
19328123; ds-bot-patches generate run twice with the second run a no-op, check 3× ok (pin ok, 10
patches apply, core/ equals ds-bot/ + patches) and canonical reports generate is a no-op.

Join counts, measured on 19328123 and at this tip by reading each envelope a guest's seat receives:

settings-page change guest already seated arriving seat
add a friend's Bot 2 → 1 2 → 2
remove a friend's Bot 2 → 1 —
add a local Bot 2 → 1 —
explicit name in the patch (#1949) 1 → 1 —

The arriving seat's two are the ones it already had — its own targeted join and the settings-page one —
and neither was added to. No envelope is lost: the seated guest still ends on the name the host holds.

Not run (and why): the smoke test (core/dev/smoke.mjs) — this box has no ../deepseek-harness checkout
and no DSH_HARNESS; the CI package job (typecheck, actionlint, pack check) — CI only; Windows.

Red first

core/test/remote-room.test.js with the final tests and core/src/host/* taken from 19328123
(109 tests):

not ok 105 #2011: a settings-page seat change tells the guest already seated once
  + 2
  - 1
  a settings-page add costs the seated guest one join, not two
# tests 109 # pass 108 # fail 1

The one red is the new case; the other eight #2004 cases and the #1995 case stay green on the base, so
the red is the doubled broadcast and not a fixture that only works after the change.

Only that one line

Deleting the suppression — putting { broadcast: false } back to a plain call in both seat loops — makes
the same run fail the one new case and nothing else (108 pass, 1 fail). No other test guards this
behaviour, which is why the new case carries all four changes (add, remove, local add, explicit rename).

Checkpoint

{
  "pr": 2013,
  "claim": "Dev-3",
  "issue": 2011,
  "at": "4af0644dc2e0770d8112e0abf5e552728bcb6a76",
  "summary": "The settings page seats or unseats a participant, saves, and sends its own join of the final name; the seat path had already broadcast, so a guest already seated got two identical joins per change (add and remove alike). addRoomParticipant and removeRoomParticipant now take opts.broadcast, defaulting to on, and updateRoom passes { broadcast: false } for the seats it changes. Everything else keeps the single broadcast from the seat path, the arriving seat still gets its own targeted join, and an explicit name or notice in the patch is still told once.",
  "files": [
    "core/src/host/room-membership.js",
    "core/src/host/roster.js",
    "core/test/remote-room.test.js",
    "docs/subsystems/core-host.md",
    "patches/ds-bot/0002-flybot-api-host.patch",
    "patches/ds-bot/files/src/host/room-membership.js",
    "patches/ds-bot/files/test/remote-room.test.js"
  ],
  "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=193281239a4a8d3c06b74afa10555fc1057696e8 npm run verify", "result": "green", "at": "4af0644dc2e0770d8112e0abf5e552728bcb6a76" },
  "next": "none — ready for review"
}

@Birfy

Birfy commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator

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

Ran at this tip:

$ tar xzf p2013tip.tgz -C p2013tip --strip-components=1; tar xzf p2013base.tgz -C p2013base --strip-components=1
$ cd p2013tip/core && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/remote-room.test.js test/room-membership.test.js test/room-context.test.js test/room-ticket-seat.test.js test/group-bot-label.test.js
# tests 171  # pass 171  # fail 0
$ cp p2013base/core/src/host/room-membership.js p2013tip/core/src/host/room-membership.js; cp p2013base/core/src/host/roster.js p2013tip/core/src/host/roster.js; cd p2013tip/core && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/remote-room.test.js test/room-membership.test.js test/room-context.test.js test/room-ticket-seat.test.js test/group-bot-label.test.js
# tests 171  # pass 170  # fail 1
$ cmp p2013tip/core/src/host/room-membership.js p2013tip/patches/ds-bot/files/src/host/room-membership.js && cmp p2013tip/core/test/remote-room.test.js p2013tip/patches/ds-bot/files/test/remote-room.test.js

Scope: compare 193281239a…4af0644d is 1 ahead / 0 behind and exactly the seven files the PR names — core/src/host/room-membership.js (+12/−5), core/src/host/roster.js (+4/−2), core/test/remote-room.test.js (+54/−0), the docs paragraph, the patches/ds-bot/0002-flybot-api-host.patch row and the two patches/ds-bot/files/ mirrors (both byte-identical to the core/ copies, verified by cmp). Base is current main, so no rebase is owed.

Red first: with core/src/host/{room-membership,roster}.js taken back to base 193281239a and the tip tests kept, the affected set is 170/171 — the one red is #2011: a settings-page seat change tells the guest already seated once ("2 not ok 1 / - 1"), the doubled join this PR removes. The nine #2004 cases, the #1949 rename case and the #1995 seat cases stay green on the base, so the red is the defect and not a fixture that only works after the change. At the tip the same set is 171/171.

The change is opt-out by default: addRoomParticipant / removeRoomParticipant still broadcast for a claim, a hand add, a self-leave and a grant revocation, and only updateRoom's own seat loops pass { broadcast: false }, because it sends the one join itself after the save; an explicit name/notice in the patch is unaffected. No envelope is lost — the seated guest still ends on the name the host holds.

Checks at this sha: 14 of 14 green except enqueue-guard, whose only reason is that no Verifier report names the tip 4af0644dc2e0770d8112e0abf5e552728bcb6a76 — this report is that report. tests-ok, package, merge-gate, risk tier, reviewed tip and the plugin jobs are all green.

Limits: the whole-repo npm run verify and the smoke test were not run on this box (its harness install lacks jsdom); the whole-suite evidence is CI's tests-ok at this sha, and the local evidence is the five affected files above. Checkpoint block reads pr 2013, claim Dev-3, issue 2011, at = this head, seven files including three in this diff, 1221 characters total (summary 556) — within the limits.

@github-actions github-actions Bot added the pass label Nov 1, 2026
@Birfy Birfy added the agent-pr Pull request opened by an agent label Nov 1, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 1, 2026
@github-actions github-actions Bot added the risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff label Nov 1, 2026
Merged via the queue into ThinkFlowLab:main with commit 6891cd9 Nov 1, 2026
1 check passed
@Birfy
Birfy deleted the dev/3-settings-seat-join-dedupe branch November 1, 2026 09:08
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pass 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.

host: one settings-page seat change sends the guest already seated two identical joins

2 participants