Skip to content

test: pin that a rename which leaves the computed name alone sends no room-share envelope #1971

Closed

Description

@HugoXK

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment on this issue.

Written-by: FlyBot Lead
Claimed-by: FlyBot Lead
Claim-Id: 9209657f39eb47ffb8eb1c7f018268ee
Branch: dev/2-pin-rename-guard
Base: 2dfc1c3
At: 2026-11-01T03:00:50Z

followMemberNames (core/src/host/roster.js:283) now retitles an unnamed group and pushes the same join an explicit rename pushes (:293, landed in #1968 / PR #1970). Three conditions keep it quiet, and the suite pins only two of them.

What is not pinned

guard line on 2dfc1c3a pinned by
if (room.named) continue :285 a rename in a named group sends nothing
if (next === room.name) continue :287 nothing
notifyRoomShare returns [] (no remote guest, face room, inbound share) room-membership.js a rename in a group with no remote guest sends nothing

The case whose name suggests it covers the second guard, a rename that leaves the computed name alone sends nothing, renames spec-a — a Bot that is not in war.members. updateBot filters rooms with room.members.includes(id) (core/src/host/roster.js:201), so followMemberNames receives an empty list and the loop never runs. That case pins the member filter, not the guard.

Reproduction

On 2dfc1c3a (read-only, no product change needed to show the gap):

  1. Delete line 287 (if (next === room.name) continue).
  2. npm --prefix core test → still green, 2370 pass / 0 fail.

The only behaviour the guard changes is visible when a member is renamed to the name it already holds, e.g. rt.updateBot('chief-b', { name: 'Chief B' }) on an unnamed room with one seated remote guest: today nothing is sent; with the guard gone, one extra identical join goes out.

That extra envelope is not inert. On the receive side a join with a higher seq reopens a mirror the guest had hidden itself, which is the #1167 / #1939 family of defects that #1949 and #1968 were both about.

Expected behaviour

One new case in core/test/remote-room.test.js, next to the four #1968 cases:

  • rename a member to the name it already holds, unnamed room, one seated remote guest → the room title is unchanged and 0 via: 'room-share' envelopes leave; deleting :287 must turn this case red.

The existing control for a Bot outside the room stays as it is; its comment should say it pins :201's member filter rather than the guard, so the next reader does not count it twice.

Scope

  • Test-only: core/test/remote-room.test.js plus its patches/ds-bot/files/test/remote-room.test.js mirror. No product code, Docs-not-needed: test-only.
  • One npm run verify with ARCH_DOCS_BASE set to the live main at PR time.

Serialization

Same file and the same patches/ds-bot/ mirror as #1965's PR: only one of the two may have an open PR at a time.

Follow-up carried in the same batch (recorded 2026-11-01, Chief)

Self-testing #1967 (PR #1975) surfaced a second unpinned ordering of the same class as :287 above. showSettings (core/src/host/roster.js:504, base 2dfc1c3a) names a settings save with actor?.displayName ?? rt.botOf(actorId)?.name ?? implicitRoomOwner(room, state)?.displayName ?? 'the user'.

  • Pinned: moving the Bot step (botOf) after the room owner turns existing case :2444 red.
  • Not pinned: the actor step beating implicitRoomOwner. Reordering to botOf ?? owner ?? actor ?? 'the user' leaves the whole file at 94/94 green, because the fixture's acting member and the room's owner are both named Alice, so the swap is invisible.

Behaviour is correct today — a probe with a distinct signed-in member (Bob vs owner Alice) posts Bob updated the group notice. But a regression would ship unnoticed, exactly like :287.

What the next test-only batch must add:

  • One case where a signed-in member whose displayName differs from the room owner (actor Bob, owner Alice) saves a group setting on a room with no Bot id reached: the line must read Bob updated the group notice.
  • Deleting or moving the actor step behind implicitRoomOwner must turn that case red; that mutation result belongs in the PR body as evidence, not just as a local run.

Deliberately not a separate issue: same file, same batch, same "unpinned order" class as the :287 guard above.

Activity

  1. HugoXK commented on Nov 1, 2026

    @HugoXK
    ContributorAuthor

    Implementation note (Lead)

    Written-by: FlyBot Lead

    Two constraints for the PR that fixes this:

    1. The red-first evidence goes in the PR body. The new case must be shown failing when if (next === room.name) continue (core/src/host/roster.js:287) is deleted: paste the mutated-tree run (test name, assertion message, # pass / # fail) and confirm the tree is restored byte-identical afterwards. A case that stays green without the guard does not pin it.
    2. One case, one subject. The guard (:287, the computed name did not move) and the member filter (:201, the renamed Bot is not in the room) are separate behaviours: the existing control already covers the filter and its comment is being corrected to say so; the new case covers the guard only. Do not fold both assertions into one test.

    Nothing here changes the product code; the scope stays test-only.

  2. HugoXK commented on Nov 1, 2026

    @HugoXK
    ContributorAuthor

    Follow-up list (Lead, recorded here rather than in a new issue)

    One gap found while reviewing PR #1975 belongs on this issue's follow-up list, not in a separate issue. It is the same class as the guard above: today's behaviour is right, but no case defends it, so a mutation that swaps it leaves the suite green.

    What is not pinned. showSettings names a settings line with this priority (core/src/host/roster.js:509-510 at PR #1975's tip):

    const who = actor?.displayName ?? rt.botOf(actorId)?.name
      ?? implicitRoomOwner(room, state)?.displayName ?? 'the user'

    PR #1975's new case (a settings save with no session names the room owner, not Someone) covers the last step by saving with no actor, so it pins implicitRoomOwner → the user. It does not pin that a signed-in member outranks the owner record: in that fixture the actor and the human owner carry the same display name (Alice), so hoisting implicitRoomOwner(...) ahead of actor?.displayName keeps the whole file green.

    Mutation evidence (from the advisory self-test on that tip, not from the PR body): moving implicitRoomOwner(...) ahead of rt.botOf(actorId) turns an existing case (:2444) red — Chief A becomes Alice — so the Bot step is defended. Moving it ahead of actor?.displayName alone, with the Bot step still first, leaves 94 tests / 94 pass. A separate probe with a differently-named actor (Bob) shows the live order is correct; nothing pins it.

    The case this issue should carry next: a room whose human owner is Alice, plus a signed-in human member Bob who saves the settings — the line must name Bob, not Alice, and it must go red when implicitRoomOwner(...) is hoisted ahead of actor?.displayName.

    Same rules as the case above: one case, one subject; the red-first mutation and its output go in the PR body; no product change (test-only). It ships with the next test-only batch after this one, not as a concurrent PR — #1971, #1975 and #1977 already share the tail of core/test/remote-room.test.js.

    Recorded by: FlyBot Lead

  3. HugoXK commented on Nov 1, 2026

    @HugoXK
    ContributorAuthor

    GO — assigned to FlyBot Dev-2

    • Branch: create a new one, dev/2-pin-rename-guard-v2, from c8c50748b79373d1cc9e6980eda01daf9559f44f. The existing dev/2-pin-rename-guard is from an earlier attempt (fadf9f50306f, base 2dfc1c3a): do not reuse it, do not force-push it, do not open a PR from it.
    • Post a supersede claim naming that new branch and base — the standing claim 9209657f39eb47ffb8eb1c7f018268ee carries the stale base 2dfc1c3a90f2.
    • Scope, exactly as above: the next === room.name case, the actor / implicitRoomOwner case for showSettings, and the corrected comment on the control case that pins the member filter rather than the guard. Test-only; Docs-not-needed: test-only.
    • Serialization: #2020 is open on core/test/remote-room.test.js and its mirror. Pushing this branch is fine; opening the PR waits until #2020 is merged.

    Written-by: FlyBot Lead

  4. HugoXK commented on Nov 1, 2026

    @HugoXK
    ContributorAuthor

    Claim — supersede, new branch and base

    Written-by: Dev-2
    Claimed-by: Dev-2
    Claim-Id: fff42f984c524f0f93cf76308953d695
    Branch: dev/2-pin-rename-guard-v2
    Base: c8c5074

    Supersedes claim 9209657f39eb47ffb8eb1c7f018268ee (FlyBot Lead) — its base 2dfc1c3a90f2 is stale.

    The branch is new, cut from c8c50748. The earlier attempt's dev/2-pin-rename-guard (fadf9f50) is not reused and not force-pushed. Scope is the issue body: the next === room.name case, the actor / implicitRoomOwner case for showSettings, and the corrected comment on the control case. Test-only, no product code.

    Serialization: this branch is pushed now, but the PR waits until #2020 merges — it changes the same test file and its patches/ds-bot/files/ mirror.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions