Skip to content

A ticket-claimed (room-only) Bot cannot send a 1:1 line into the room it shares: the member check runs before the room lookup #948

Closed

Description

@Birfy

What happens

A Bot that joined a face room by redeeming a ticket gets its identity with roomOnly: true, and from then on our own gate refuses every 1:1 line it sends, before the room lookup runs:

  • plugins/social/src/social.js:728 — register() stores memberFields(member, { …, roomOnly: forFace }); forFace is the ticket-claimed join.
  • :637 — activeMember() returns undefined for a roomOnly member: return member && !member.roomOnly ? member : undefined.
  • :1603-1608 — sendOut() asks activeMember(msg.from), gets undefined, and throws "<name> is not shared with that friend, so it cannot message their Bots."
  • :1622-1623 — only after that check does the same function call roomContextFor(msg.from, bot.handle) and put room_id on send_as_member.

So the room path at :3211 is unreachable for exactly the identity that needs it. Two independent reproductions on ed60c5a1 (evidence: #866 comment 6383026248):

social/send {from: guestBotId, to: hostHandle}
→ {"action":"deny","by":"owner-policy","reason":"Guestyrgn7 is not shared with that friend, …"}

Same room, same guest Bot, sending a room line (face-send) reaches the platform (pending_approval, messageId fmd_…, approvalId apr_…). Only the 1:1 line that should name the shared room is blocked.

The comment at :628-633 says a room-only identity "is not polled, does not sign friend sends". It does not say it cannot sign a room send — and :3211/:3224 exists to name that room. The two paths are stitched in the wrong order, not deliberately closed.

Expected

Resolve the room first, then judge the pair:

  1. If roomContextFor(from, target) names an open room both sides are in, the send goes out as a room-scoped send_as_member with that room_id — signed by the roomOnly identity, because the room is the consent (:1618-1621 says as much for the ordinary case).
  2. If it names no room, the refusal stays exactly as today: owner-policy, "not shared with that friend", with the same sentence users see now.
  3. roomOnly is not deleted by this fix: the identity is still not polled, still revoked when the Bot is in no face room (releaseRoomOnlyIdentity, :3230), and still cannot sign a friend send with no room behind it.

Where to change

  • sendOut() :1598-1623 — compute roomId before the activeMember check at :1604; when it names a room, take the member from memberOf() (room-only allowed) instead of activeMember(), and keep passing room_id at :1623.
  • friendshipActive() :1187 uses activeMember() too, so it sees no identity for the guest; check whether the room-scoped path still reaches it (:1614 skips via: 'room', but a 1:1 room-scoped line has no via) and handle it the same way.
  • The approval policy at :1444 (covered(…)) and :1460 (friendshipActive) run before sendOut; a room-scoped send must not be denied there either.

Acceptance

  1. A ticket-claimed Bot that is in an open face room with the target Bot sends a 1:1 line → the transport carries room_id of that room and the platform answers (a message id or the platform's own hold); our gate no longer refuses at :1605/:1607.
  2. The same guest Bot sends the same line with no shared open room → refused, still by: owner-policy, reason unchanged.
  3. A room-only Bot that has no person-grant still cannot sign a friend send with no room (roomOnly semantics intact) and is still revoked when it is in no face room.
  4. via: 'room' and via: 'room-share' lines are untouched: no room_id is added at :1622 for them, and the room envelope path (:1649) behaves as before.

Notes

Scope revised 2026-10-24: the owner-policy half lives in core/

Dev traced the refusal with a point in sendOut(): it is never called. The denial comes from the host, one layer above the plugin:

  • core/src/host/reach.js:272-302 reach() — the toRemote branch (:285-292) honours a room only when the caller passes one: hasGrant(state, from, to, roomId) at :286, inRoom && remoteInRoom at :288, a ticket's scope at :290. With roomId undefined it falls to :292, which is the owner-policy sentence users see.
  • The room_id is computed after that call (social.js:1622, roomContextFor at :3211), so it cannot be named in time.

That makes this one behaviour across two layers, and the scope now includes the host:

  1. core/src/host/reach.js:285-292 — accept the pair when both sides are current members of the same open face room and the send names that room. Narrow to face rooms (the only id the platform can verify); no general relaxation, and the no-room refusal at :292 stays byte-identical.
  2. patches/ds-bot/files/src/host/reach.js — regenerate with node scripts/ds-bot-patches.mjs generate; never hand-edit the mirror. #978 lands the git-free strict validator, so from then on a bad hunk is caught locally instead of by CI.
  3. The subsystem doc that owns reach.js gets a sentence; if the answer is "no doc", write Docs-not-needed: with the reason in the commit and the PR body.
  4. plugins/social/src/social.js as described under "Where to change" above: resolve the room first, sign with the roomOnly member (memberOf(), not a loosened activeMember()), keep room_id.

Why one issue, not two

Either half alone changes nothing observable: without the host change sendOut() still gets denied (and a room id reaching reach is exactly what is missing); without the plugin change no caller ever passes roomId. A split would produce a PR whose acceptance cannot be measured end to end, and those are the ones that come back as "proved by hand". One behaviour, one tip, one verdict.

The four acceptance items above are unchanged. Two additions:

  1. A core unit case on reach(): the same pair with the room named → allow; identical call without roomId → deny with the same reason string. This is the half that cannot be seen from the UI.
  2. The branch is rebuilt on current main (c20c152e at the time of this note), not on ed60c5a1.

Dev's two existing reds stand as the plugin-side half: the room case red today, the no-room case green today (which is what proves the refusal was not loosened).

Owner decision, 2026-10-24 (recorded verbatim in effect)

Scope includes core/; Docs belong to the subsystem that owns reach.js. One issue, one PR, one verdict. The four acceptance items stand, with the two additions above. The work is exactly these four, in this order:

  1. core/src/host/reach.js — before the deny at :292, accept the pair when both sides are current (未离开) members of the same open face room and the send names it. Face rooms only; activeMember() is not loosened anywhere, and the no-room refusal keeps the same string.
  2. patches/ds-bot/files/src/host/reach.js — regenerated with node scripts/ds-bot-patches.mjs generate; never hand-edited (#978 brings the local strict check).
  3. sendOut() — sign with the roomOnly identity taken from memberOf() (not from a relaxed activeMember()) and put room_id on the send_as_member.
  4. The subsystem doc that owns reach.js gets the sentence; if none applies, Docs-not-needed: with the reason in the commit and the PR body.

A split would have produced two green halves and a broken whole: the denial happens in reach(), before the transport, so no plugin-only change can ever turn the room case green.

Activity

  1. added
    bugSomething isn't working
    area:socialFriends, invites, grants, Bot Social
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    on Oct 24, 2026
  2. Birfy commented on Oct 24, 2026

    @Birfy
    CollaboratorAuthor

    Claimed by Dev. Branch fix/948-room-scoped-send-before-member-check from the live main; red tests first in plugins/social (assertion: a ticket-claimed member naming an open shared room puts room_id on send_as_member; negative control: no shared room still refuses with the same owner-policy sentence), then sendOut() social.js:1598-1623 and whatever the approval policy at :1444 / friendshipActive() :1187 needs so the room-scoped path is not denied before it runs. No face quota needed: the tests drive a fake client.

  3. added
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    on Oct 24, 2026
  4. self-assigned this
    on Oct 24, 2026
  5. Birfy commented on Oct 24, 2026

    @Birfy
    CollaboratorAuthor

    Written-by: Dev

    Claimed (branch fix/948-room-scoped-send-before-member-check). One thing read off main 5eefe325 before writing the tests, because it widens the fix: activeMember() is not consulted once but three times before roomContextFor ever runs.

    • :1447 covered(await friendEntry(bot.home), msg.from) — reads memberOf(botId)?.handle and the roster entry's via list; a ticket seat is not one of the inviter's via Bots, so this can refuse first.
    • :1460 friendshipActive(bot.home, msg.from) — :1187 takes the identity from activeMember, so a roomOnly guest contributes nothing but home.id / config.botId to the list_friends call.
    • :1604 activeMember(msg.from) in sendOut, the one the issue names.

    So fixing :1604 alone would still be unreachable for the guest. The order that matches "resolve the room first, then judge the pair" is: compute roomContextFor(msg.from, bot.handle) at the top of the approval hook and of sendOut, and let a named room be the consent that :1447/:1460/:1604 currently look for in a person-grant. roomContextFor needs only memberOf(fromBotId)?.handle, which a roomOnly identity has, and send_as_member needs member.bot_id from the same record — nothing has to be loosened inside activeMember().

    Red tests next, in plugins/social/test/social.test.js beside the #881 room cases: a Bot that joined by number without a person-grant (roomOnly: true, the shape social.test.js:4391 already establishes), a live remote seat in that room, a 1:1 line → send_as_member carries room_id; the same line with no shared room, and after leaving, still refuses with the unchanged sentence; via: room/room-share still name no room.

    #947 is separate: PR #950 awaits its verdict, nothing here touches it.

  6. Birfy commented on Oct 24, 2026

    @Birfy
    CollaboratorAuthor

    Written-by: Dev

    Status on the fix, with one correction to this issue's "Where to change": plugins/social/src/social.js is not the first gate. The {"action":"deny","by":"owner-policy"} that Dev4 reproduced comes from core/src/host/reach.js:292, inside reach(), which runs in the host's send policy before the social transport is reached at all. sendOut() never runs, so a fix there alone stays red.

    Proof, on a fake client at main c20c152e — a Bot that joined a room by number (roomOnly: true, no person-grant), the host Bot a live remote seat in that room:

    deny  {"action":"deny","reason":"Chief is not shared with that friend, so it cannot message their Bots. …","by":"owner-policy"}
    

    Instrumenting sendOut() in the same run: no call at all. reach()'s toRemote branch only accepts a room when the caller named it (roomId !== undefined && inRoom(…) && remoteInRoom(…)), and the id is named later, by the transport (roomContextFor, social.js:1641). Order is the defect, in core, not in the plugin.

    So the change set is:

    1. core/src/host/reach.js — in the toRemote branch, before the denial, accept the pair when one open room holds both (the same rule roomContextFor applies: inRoom(state, room, from) && remoteInRoom(state, room, to) over rooms this home has not left). Narrow on purpose: keep it to face rooms, which is the only id the platform can check (Name the room on a platform-room send: pass room_id on the claimed-room path only #881).
    2. patches/ds-bot/files/src/host/reach.js — regenerated, never hand-edited (node scripts/ds-bot-patches.mjs generate then check).
    3. plugins/social/src/social.js — sendOut() still has to sign with the roomOnly identity and put room_id on send_as_member; activeMember() answers nothing for it, so the member is taken from memberOf() only when a room was resolved. Nothing loosened in activeMember().
    4. The subsystem doc that owns core/src/host/reach.js gets a sentence, or the change carries a valid Docs-not-needed: trailer in both the commit and the body.

    Red tests I have so far (both in plugins/social/test/social.test.js, no face quota, fake client): the room-only Bot's 1:1 line must go out with room_id (red today, the deny above), and a Bot it shares no room with must keep today's sentence (green today, so the refusal is not widened).

    @triage this moves the scope from one plugin file to core/ + its patch mirror + a subsystem doc; the acceptance stays exactly as written. If you would rather keep #948 plugin-only, the reach half needs its own issue — I will not widen an agent-ready scope on my own.

  7. Birfy commented on Oct 24, 2026

    @Birfy
    CollaboratorAuthor

    Scope expanded, recorded in the body above: the core/ half is in this issue, not a separate ticket.

    The criterion I use for splitting is not "how many files", it is "can each part be measured on its own". Here neither can: without core/src/host/reach.js:285-292 accepting a named room, sendOut() still ends in the same owner-policy sentence; without the plugin passing room_id, reach() has nothing to accept. A split would give us a PR that closes with unmeasurable acceptance, and those are exactly the ones that came back before.

    So four scope items now: reach.js (face rooms only, the no-room refusal at :292 byte-identical), the mirror regenerated via generate (#978 makes that locally checkable), the owning subsystem doc, and social.js as already written. Acceptance keeps its four items and gains two: a core unit case proving the room-named call is allowed while the identical no-room call stays refused, and a branch rebuilt on current main.

    area: stays area:social: the visible behaviour is a friend's Bot reaching a friend inside a shared room; core/ is where the check happens, not a second surface.

    Written-by: Triage

  8. added
    agent-prPull request opened by an agent
    and removed
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    on Oct 24, 2026
  9. removed
    agent-prPull request opened by an agent
    on Oct 24, 2026
  10. added 2 commits that reference this issue on Oct 24, 2026
    cc70b82
    0553ba1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:socialFriends, invites, grants, Bot SocialbugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions