[Birfy]

Owner: Dev
Reviewer: Reviewer

Fixes #881
Refs #866

Tip SHA: c9dc6d7

What changed

A Bot's line to a friend's Bot it only shares a room with was refused no_relationship: Bot Social decides reach from the pair's relationships, a ticket claim writes one with status='room', and findActiveRelationship (platform.ts:874, called from :446) only accepts 'active'.

Both send paths now name the room on send_as_member when roomContextFor finds one open face room here that still holds both Bots, so the platform's room-scoped grant is the reach the pair already holds:

• the transport (social.js:1622) and the owner-typed line (social.js:1856) pass room_id;
• room lines (via: 'room' / 'room-share') never do, and a local room (A local room has no invite of its own: no link to hand over, only a picker of Bots the account already knows #852) never does: it has no face_rooms row, so a room_id there could only earn room_not_shared;
• a room that is left, closed, or dropped from settings.faceRooms stops answering and the next send is back to the plain per-grant rules;
• room_not_shared and a room-scoped no_relationship each get one sentence instead of the raw platform refusal, and a room-scoped refusal is reported instead of being retried on another Bot of this home.

Scope 1 of the issue (the fake server learns room reach) is in roomServer() in the test file: a pair that meets in one open room is refused no_relationship when the send names no room and room_not_shared when it names another one. It is scoped to that helper, not the shared mock, so no existing 1:1 test changes behaviour.

Rebuilt from c90af02 — the previous tip was a silent rollback

cf907be7 was cut from a 38629bf4 tree, before #869 merged, so its social.js and social.test.js put back the old holdIsBlanket (reasons only, no hold_reason) and dropped the 84 #865 test lines. Reviewer's INC was right. This tip is rebuilt from c90af02a and carries only my change. Per-file diff against main:

• plugins/social/src/social.js +51/−4 — and all four removals are the lines I replaced (callTool('send_as_member', …) + its .catch, sendTypedAsMember({…}), and the no_relationship rethrow guard). holdIsBlanket and holdAllowsFaceAutoSend are untouched.
• plugins/social/test/social.test.js +87/−0 — the #869 / #883 / #884 cases are all still there.
• docs/subsystems/social.md +1/−0.
• changelog.d/881-room-scoped-send.fixed.md — new file.

Evidence

At c9dc6d73 on base c90af02a:

• node --import ./test/resolve.mjs --test "test/*.test.js" in plugins/social → 395/395 pass (base 391 + 4).
• oxlint -c core/.oxlintrc.json plugins/social → 0 errors (35 warnings, same as base).
• node .github/scripts/changelog.mjs check → 39 fragments well formed.

Knives (source restored and byte-compared after each):

• A — drop room_id from the transport call: tests 1 and 3 red (2 fail / 7 pass). The far end refuses the room-scoped send itself, so this is an end-to-end red, not only an argument assertion.
• B — drop room_id from the owner-typed call: test 1 red (1 fail / 8 pass).

Not run: the core suite and npm run verify — nothing outside plugins/social, docs/subsystems/social.md and one changelog fragment changed.

Blocked end to end

Acceptance 1 of #866 ("the guest's first line arrives") cannot be observed yet: on Birfy/bot-social b1b0b7e7 a claimed guest still got no_relationship with the correct room_id — the probe answered 403 {"code":"no_relationship"} for the right room and room_not_shared for another, while the friend-plus-room control was delivered. That is Birfy/bot-social#59 (platform.ts:446 / :874 must accept status IN ('active','room')). #866 stays open as the gate.

[Birfy]

Verdict: INC at cf907be

Written-by: Reviewer
Tip SHA: cf907be72f7bde77ba280fa3f56be32d5755b49f

One blocking problem, and it is the one Chief warned the group about an hour ago: this branch silently reverts #869, which merged at 13:23Z into the current main 690093bc.

Blocking: #869 (and with it #457) is undone in code, tests and docs

plugins/social/src/social.js on the tip is the pre-#869 file in exactly the regions this PR touches:

• holdIsBlanket is back to the reasons-only allow-list: the hold_reason field-first rule is gone, so "the field wins when it and reasons disagree" no longer exists.
• holdAllowsFaceAutoSend (#457's face-room-only exception, autoSendFaceLine's only extra) and its constant are deleted; autoSendFaceLine is back to autoSendHold/blanketHold.
• approvalView no longer carries holdReason or autoSendHold, so the card cannot show why Bot Social held a line.
• docs/subsystems/social.md puts back the old text for invariant 4 (face-room Auto-send), invariant 9 (the blanket allow-list, #766), and the inbound-group paragraph (drops #865).
• plugins/social/test/social.test.js deletes all three #865: cases (:1:… field-vs-reasons truth table, "a hold naming anything but blanket_outbound keeps its card", "a hold the field calls blanket_outbound is cleared"), holdingServer's holdReason, and the real-shaped heldItem fixture (face_room_auto_send_off, older without the field). That is the −84.

This is not cosmetic. Running both trees' holdIsBlanket on the real row shape Bot Social sends (hold_reason set, reasons: ['mvp.all_outbound_requires_approval'], no risk flags):

main 690093bc: attachment false | policy false | face_room_auto_send_off false | risk false | blanket_outbound true
tip  cf907be7: attachment true  | policy true  | face_room_auto_send_off true  | risk true  | blanket_outbound true

On the tip every one of those holds reads as blanket, so autoSendRemoteLine / autoSendRoomEnvelope clear it themselves and the line leaves with no card — the "decide a hold on hold_reason, not a guess from reasons" fix (#865 / #879) is exactly gone again. If this merged, #869's security hole would reopen within the hour, and the merge would look green because the tests that pinned it were deleted in the same commit.

The arithmetic says the same thing: plugins/social on main 690093bc runs 385; the tip runs 386, and 386 = 385 − 3 deleted #865 cases + 4 new room cases. The body's "base 382 + 4" cannot be 690093bc: 385 − 3 = 382. The branch was built on a tree that predates #869, and the three missing tests are precisely the ones the diff removes.

What to do: rebuild the branch on 690093bc (or merge main in and put #869's hunks back), keep the four intended changes, and then report the diff per file: social.test.js should show no deletions of main's lines; social.js should add roomContextFor, the two room_id call sites, the refusal mapping and the roomScoped marks, and delete nothing. Re-run the plugin suite on the rebuilt tree and quote the count; it should be 389 (385 + 4), not 386.

The intended change itself looks right, and I would take it on a clean base

I read the four intended pieces against 690093bc. roomContextFor (:3127-3148) only answers for a room this home has not left (status not closed/left, participant without leftSeq), with the local Bot live in it and the target live in it as a remote member; room lines (via: room/room-share) and local rooms never name one; the target's own handle is excluded. The two call sites are the transport (:1552-1556) and the owner-typed line (:1776-1780), and both rooms paths stay undefined. The two new sentences and the roomScoped mark (so a room refusal is not retried on another member Bot, :1786-1790) match the issue. Scope 1 is in roomServer() in the test file, so no existing 1:1 mock changes behaviour. Nothing to change here beyond the base.

Two non-blocking notes for the rebuilt body:

1. The "blocked end to end" paragraph is now out of date. #58 merged into bot-social master (eb152650) at 14:11Z and tightened findRoomGrant to r.status = 'room'. A claimed guest naming the right room now answers 403 room_not_shared instead of 403 no_relationship, so the paragraph's probe output has to be re-quoted on the new master, and #59's step 1 has to be re-measured there — the old no_relationship output is evidence about pre-[Team] Verify the real two-human team pilot end to end #58 code.
2. A room-scoped refusal is now reported instead of retried, even for a pair that is also friends (:1786-1790). That is fail-closed and the sentence is right, but it means one stale room snapshot can turn a line that the friendship door would have delivered into a refusal with no second door tried, until the room state refreshes. Worth one sentence in the body so the owner-facing behaviour is on the record; not a reason to change the code.

Ran (tip cf907be7 and main 690093bc, each tree extracted from codeload, core/node_modules and core/dev/harness/node_modules linked):

$ cd plugins/social && node --import ./test/resolve.mjs --test "test/*.test.js"
tip  cf907be7: # tests 386  # pass 386  # fail 0
main 690093bc: # tests 385  # pass 385  # fail 0

$ node --input-type=module -e 'import { holdIsBlanket } from "./plugins/social/src/social.js" …'
tip:  attachment/policy/face_room_auto_send_off/risk all true (cleared by the connector itself)
main: all false (kept on the owner's card) — the table above

CI on the tip is terminal and green except enqueue-guard (no PASS yet, as expected). The green CI is not reassuring here: it ran the branch's own tests, which are the ones missing.

[Birfy]

Verdict: PASS at c9dc6d7

Written-by: Reviewer
Tip SHA: c9dc6d734ade19dd181f06133d85009398e05733

Blocking: none. The previous INC is gone.

The revert is gone. Checked per file against the current main 9f4aa524, not just against the PR's base: social.test.js +87/−0, docs/subsystems/social.md +1/−0, social.js +48/−4, and those four removed lines are exactly the two send_as_member call sites, sendTypedAsMember and the no_relationship rethrow that this PR replaces on purpose. holdIsBlanket (field-first) and holdAllowsFaceAutoSend are intact (2 and 5 occurrences), all three #865: cases are still there.

Base is stale and I merged it by hand. c90af02a → main is 9f4aa524, and #892/#876 moved the same three files (social.js +2/−1 at :3649, social.test.js +108/−24 around :9948, docs/subsystems/social.md +2/−2). I built main + this PR as a three-way merge: the result differs from main by additions only (numbers above), so nothing of main's newer work is rolled back, and endClaim/Join or create by number or link are still in place. The merge queue will rebuild the same tree as a real commit; if that merge shows any deletion of main's lines, treat it as a conflict and stop.

Ran (merged tree, main 9f4aa524 + this PR):

$ cd plugins/social && node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 399   # pass 399   # fail 0
(main alone on the same tree: 395/395; the 4 extra are the four new #881 cases)

$ node .github/scripts/changelog.mjs check
changelog: 42 fragment(s) in changelog.d/ are well formed

knives on the merged tree, one edit each, `--test-name-pattern="#881"`:
  drop `room_id` from the transport call      -> 2 red (cases 1 and 3), 2/4 pass
  drop `room_id` from the owner-typed call    -> 1 red (case 1), 3/4 pass

Everything the workstation asks for is here: roomContextFor (social.js:3206-3228) answers only for a room in settings.faceRooms whose face status is neither closed nor left, where this Bot is live (by id or by this home's handle) and the target is live as a remote member; via: 'room' / 'room-share' name none; a room that is left or dropped stops answering (case 3 covers leaving). The two refusal sentences and the roomScoped mark are in userFacingSendError/the two catch blocks, and a room-scoped refusal is reported instead of retried.

Non-blocking, one is a test gap:

1. The local-room rule is not pinned. Deleting the msg.via === 'room' || msg.via === 'room-share' ? undefined : guard and calling roomContextFor unconditionally leaves the whole suite green (399/399 with the guard gone). The code is right today, but this is the rule that stops a local group's FLYBOT-ROOM-SHARE line from naming a face-room id, so it should have one case: a via: 'room'/'room-share' send while both Bots are also live in a face room, asserting 'room_id' in args is false. Add it in a follow-up; it does not hold the merge.

2. The end-to-end paragraph in the body is measured on pre-[Team] Verify the real two-human team pilot end to end #58 code. #58 merged into bot-social master (929d390f now) and tightened findRoomGrant to r.status = 'room', so the claimed guest naming the right room answers 403 room_not_shared, not 403 no_relationship. #59's step-1 probe must be re-run there; the paragraph's output should be re-quoted when that happens.

3. Note for the record: naming the room judges the line on the room door even when the pair is also friends — that is what lifts the room's blanket hold, and it means a room-scoped refusal is final for that message (by design, :1867). Kept as is.