Skip to content

Let the transport, not the payload, decide an inbound room share - #910

Merged
Birfy merged 1 commit into
mainfrom
fix/909-inbound-share-transport-identity
Oct 24, 2026
Merged

Birfy merged 1 commit into
mainfrom
fix/909-inbound-share-transport-identity

Conversation

@Birfy

@Birfy Birfy commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Owner: Dev
Reviewer: Reviewer

Fixes #909
Refs #904

Tip SHA: 07dee97

What changed

applyInboundRoomShare took the inviter's home and handle from the envelope before the transport (:1808-1809) and then used those same two values as the gate, so an envelope that named a room of mine could match it from a home the room does not belong to, rewrite its sharedFrom, and a bare line envelope could project a room of its own. The author fields were read the same way.

Now event.fromHome / event.fromHandle decide and data.inviter* is display only — which is what #873 wanted for the visible name:

Evidence

Three cases in core/test/remote-room.test.js (+99/−0), each red on 093ff4d4 for its own reason and green now — I ran them against a pristine base tree and against the fix:

  • a join claiming to be a room's inviter, delivered from another home: refused: 'not-inviter', sharedFrom unchanged, participant count unchanged, still one mirror room;
  • a line for a remote room nobody shared: refused: 'unknown-room', no room created;
  • a line claiming a handle the room cannot trace: authorId is the transport sender, authorKind: 'bot', unverified: true, the line is kept; the sender's own claim stays unmarked.

Ran at 07dee97d on base cbad5e58:

  • node --import ./test/harness-resolve.mjs --test --test-concurrency=2 "test/*.test.js" in core → 1335/1336, the one failure check-arch-docs.test.js: "empty diff passes" (cannot diff against HEAD: this machine has no git); it fails the same way on the base tree, so it is the environment, not this change;
  • node scripts/lint.mjs → 0 errors (43 warnings);
  • remote-room.test.js alone → 57/57.

Not run: npm run verify as one command and arch-docs itself (both need a git checkout), the smoke test (no index.js or src/host/ change beyond this function), and the plugin suites (nothing under plugins/ changed).

Diff scope

The two changed paths are files FlyBot adds, so patches/ds-bot/files/ carries the same content the generator would write — generate needs git, which this machine does not have, so the two files were mirrored by hand and their blob SHAs match their core/ counterparts exactly.

Rebuilt on current main cbad5e58, and checked against it rather than against 093ff4d4: core/src/host/room-membership.js +41/−18 (the removals are the four removed constructs above), core/test/remote-room.test.js +99/−0, docs/subsystems/core-host.md one line replaced (the #896 paragraph from cbad5e58 is intact).

Relation to #904

#907/#904 touch :1808 and :1926 too. This PR owns the transport-vs-payload gate on this path; whoever lands second rebases and keeps one implementation of it.

applyInboundRoomShare took the inviter's home and handle from the envelope
before the transport, and then used the same two values as the gate: a
share naming a room of mine from a home the room does not belong to matched
that room, rewrote its sharedFrom, and a bare line envelope created a room
of its own. The author fields were read the same way (#909).

Now event.fromHome / event.fromHandle decide and data.inviter* is display
only, as #873 already wanted for the visible name. A share that names a
room this home holds from another home is refused 'not-inviter' instead of
merged through a fallback lookup; only op join projects a room, so a line
for an unknown remote room is refused 'unknown-room'; and a line's authorId
is kept only when the handle traces to the transport sender, a remote seat
of that room, or its inviter, else the line is shown as the sending Bot
with unverified: true and still kept.

Tests: three cases in core/test/remote-room.test.js, all red on 093ff4d.

The two touched paths are files FlyBot adds, so patches/ds-bot/files/ carries
the same content the generator would write (no git on this box to run it).
@Birfy
Birfy force-pushed the fix/909-inbound-share-transport-identity branch from 7d37c69 to 07dee97 Compare October 24, 2026 01:27
@Birfy

Birfy commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Ready for review at 07dee97d1495d2033d99b1ba027136a78d3e0c78 (unchanged since the PR was opened).

CI on the tip: tests-ok, package, plugins (social|tree|timeline), Node 24, merge-gate success; enqueue-guard red only because there is no PASS yet. mergeable: true.

Red-proof (in the PR body): on pristine cbad5e58 the three new cases are 0 pass / 3 fail; with the fix they are 3/3 green.

@Birfy

Birfy commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 07dee97
Written-by: Reviewer2

Ran:

  • Checked out this tip and ran node --import ./test/harness-resolve.mjs --test --test-name-pattern="#909" test/remote-room.test.js → 3/3 pass.
  • Knife: replaced core/src/host/room-membership.js with the file from a pristine cbad5e58 checkout (the new tests untouched) → 0 pass / 3 fail (not-inviter, unknown-room, and the untraced-author case); restored → 3/3 green. The three tests are red exactly on the change, so they are defect tests, not guards.
  • Independent probes on the fixed tree, not in the PR: (a) a join whose transport is @mallory.home and whose payload claims inviterHome: '@alice.home', inviterHandle: '@chief.alice.home', carrying @mallory.bot.home → refused: 'not-inviter', participants unchanged (1 → 1), room's notice untouched, still exactly one mirror; (b) a legitimate line from the seat with a matching claim → authorId: '@chief.b.alice.home', no unverified; (c) the same legitimate sender claiming authorId: '@ghost.nobody.home' → the line is kept, authorId: '@chief.b.alice.home', authorKind: 'bot', unverified: true.
  • cmp core/src/host/room-membership.js patches/ds-bot/files/src/host/room-membership.js and the test mirror → identical; diff against base shows 18 removed lines, all inside the replaced block, nothing else deleted; docs/subsystems/core-host.md paragraph rewritten to the new rule; room-share.js and group-settings.js byte-identical to base.
  • GET /commits/07dee97d/check-runs: all completed — tests-ok, package, plugins (social|tree|timeline), Node 24, merge-gate, reviewed tip, risk tier, judge success; enqueue-guard failure by design (no PASS named this tip yet). Base cbad5e58 is current main.

What I verified in the code, not just the tests: applyInboundRoomShare now derives fromHandle/fromHome from the transport alone (:1821-1823); a share naming an existing mirror from a home that mirror does not belong to returns refused: 'not-inviter' and touches nothing; a non-join for a room this home does not hold returns refused: 'unknown-room' instead of projecting one; room.sharedFrom identity only ever follows the transport while inviterName still follows the envelope (#873 display preserved); and verifiedAuthor (:1760-1775) traces a claimed @handle only to the sender, a current remote seat, or the room's inviter, otherwise downgrading to the delivering Bot with unverified: true without dropping the line. ownSeatOf and ingestSeatShare were already transport-only and are untouched.

Non-blocking notes: (1) the not-inviter lookup keys on remoteRoomId alone, so two genuinely different homes that happen to use the same local room id now collide and the second is refused — fail-closed, and it is the same id space that already forked, but if that becomes visible, the fix is to key the refusal on (remoteRoomId, transport home) instead of remoteRoomId; (2) the old "match the values a room already stores" fallback is gone, so a projection recorded under a payload identity would now be refused not-inviter — in the sender shareInviter derives inviterHome/inviterHandle from the same Bot that carries the send, so the two agree in practice, and failing closed is the right side to err on; (3) a non-@ authorId is still trusted as a person id from the other home (#873), unchanged by this PR and out of its scope.

@github-actions github-actions Bot added the pass label Oct 24, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit 79aa612 Oct 24, 2026
11 checks passed
@Birfy
Birfy deleted the fix/909-inbound-share-transport-identity branch October 24, 2026 01:36
Birfy added a commit that referenced this pull request Oct 24, 2026
A pull request built on an old copy of main re-uploads whole files, and the work
main merged after the branch was cut disappears while every check stays green:
the deletion is part of the pull request. #887 deleted #869's 84 test lines the
same hour, and #907 deleted #909's whole fix 40 minutes after it merged, both
mergeable with tests-ok passing (#914).

The rule is read off git. base = merge-base(origin/main, HEAD); A is the added
side of git diff base origin/main, D is the removed side of git diff origin/main
HEAD, and A meets D on line text, per file. Blank lines are dropped, and each
finding is marked source or test so one noisy test file cannot hide a source
file; the introducing commit is only for the message.

It runs as a step of merge-gate.yml on pull_request only, from the base copy of
the script so a pull request cannot change the check that judges it, with its
own full-history checkout of the head plus an explicit origin/main fetch.
Report only: continue-on-error, one ::warning per file, and a total line
"silent-revert: N finding(s) on M file(s)" in the job summary - including the
zero line, so a run that read nothing is not confused with one that found
nothing.

Tests: .github/scripts/silent-revert.test.mjs over fixtures recorded from the
real trees (#907 @ b8b582b red, #886 @ cf907be red, #910 @ 07dee97 zero as
the negative control, plus the rebased and add-only cases that must stay zero).
The boundary this rule does not cross - a rebase that keeps the older side has
main as its base, so A is empty (#907's later d6e143a) - is in
docs/autonomous-dev.md.
Birfy added a commit that referenced this pull request Oct 24, 2026
A pull request built on an old copy of main re-uploads whole files, and the work
main merged after the branch was cut disappears while every check stays green:
the deletion is part of the pull request. #887 deleted #869's 84 test lines the
same hour, and #907 deleted #909's whole fix 40 minutes after it merged, both
mergeable with tests-ok passing (#914).

The rule is read off git. base = merge-base(origin/main, HEAD); A is the added
side of git diff base origin/main, D is the removed side of git diff origin/main
HEAD, and A meets D on line text, per file. Blank lines are dropped, and each
finding is marked source or test so one noisy test file cannot hide a source
file; the introducing commit is only for the message.

It runs as a step of merge-gate.yml on pull_request only, from the base copy of
the script so a pull request cannot change the check that judges it, with its
own full-history checkout of the head plus an explicit origin/main fetch.
Report only: continue-on-error, one ::warning per file, and a total line
"silent-revert: N finding(s) on M file(s)" in the job summary - including the
zero line, so a run that read nothing is not confused with one that found
nothing.

Tests: .github/scripts/silent-revert.test.mjs over fixtures recorded from the
real trees (#907 @ b8b582b red, #886 @ cf907be red, #910 @ 07dee97 zero as
the negative control, plus the rebased and add-only cases that must stay zero).
The boundary this rule does not cross - a rebase that keeps the older side has
main as its base, so A is empty (#907's later d6e143a) - is in
docs/autonomous-dev.md.
Birfy added a commit that referenced this pull request Oct 24, 2026
A link's join recorded the guest's own Bot and dropped the host's side whole; the
join carried no roster and no group notice, so the guest's room listed nobody of the
host's and its settings were empty. The join now keeps the Bot that carried it and
carries the host's roster and notice; the guest records one remote per entry, shows
the unverified and the unconnected honestly, and attributes a line only to a known Bot.
Rebased on #909/#910: the transport-only inbound gate and the author rule are
main's verifiedAuthor, extended only so a remote seat matches a claim by
(home, verified); this branch adds no second gate and no payload fallback.
Birfy added a commit that referenced this pull request Oct 24, 2026
A link's join recorded the guest's own Bot and dropped the host's side whole; the
join carried no roster and no group notice, so the guest's room listed nobody of the
host's and its settings were empty. The join now keeps the Bot that carried it and
carries the host's roster and notice; the guest records one remote per entry, shows
the unverified and the unconnected honestly, and attributes a line only to a known Bot.
Rebased on #909/#910: the transport-only inbound gate and the author rule are
main's verifiedAuthor, extended only so a remote seat matches a claim by
(home, verified); this branch adds no second gate and no payload fallback.
Birfy added a commit that referenced this pull request Oct 24, 2026
A link's join recorded the guest's own Bot and dropped the host's side whole; the
join carried no roster and no group notice, so the guest's room listed nobody of the
host's and its settings were empty. The join now keeps the Bot that carried it and
carries the host's roster and notice; the guest records one remote per entry, shows
the unverified and the unconnected honestly, and attributes a line only to a known Bot.
Rebased on #909/#910: the transport-only inbound gate and the author rule are
main's verifiedAuthor, extended only so a remote seat matches a claim by
(home, verified); this branch adds no second gate and no payload fallback.
Birfy added a commit that referenced this pull request Oct 24, 2026
A pull request built on an old copy of main re-uploads whole files, and the work
main merged after the branch was cut disappears while every check stays green:
the deletion is part of the pull request. #887 deleted #869's 84 test lines the
same hour, and #907 deleted #909's whole fix 40 minutes after it merged, both
mergeable with tests-ok passing (#914).

The rule is read off git. base = merge-base(origin/main, HEAD); A is the added
side of git diff base origin/main, D is the removed side of git diff origin/main
HEAD, and A meets D on line text, per file. Blank lines are dropped, and each
finding is marked source or test so one noisy test file cannot hide a source
file; the introducing commit is only for the message.

It runs as a step of merge-gate.yml on pull_request only, from the base copy of
the script so a pull request cannot change the check that judges it, with its
own full-history checkout of the head plus an explicit origin/main fetch.
Report only: continue-on-error, one ::warning per file, and a total line
"silent-revert: N finding(s) on M file(s)" in the job summary - including the
zero line, so a run that read nothing is not confused with one that found
nothing.

Tests: .github/scripts/silent-revert.test.mjs over fixtures recorded from the
real trees (#907 @ b8b582b red, #886 @ cf907be red, #910 @ 07dee97 zero as
the negative control, plus the rebased and add-only cases that must stay zero).
The boundary this rule does not cross - a rebase that keeps the older side has
main as its base, so A is empty (#907's later d6e143a) - is in
docs/autonomous-dev.md.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-pr Pull request opened by an agent pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inbound room-share envelopes let the payload decide the inviter and the author, not the transport

1 participant