Skip to content

Let co-members message a shared room without a friendship - #1145

Merged
Birfy merged 1 commit into
mainfrom
cursor/fix-1141-group-mention-insert-a22d
Oct 25, 2026
Merged

Birfy merged 1 commit into
mainfrom
cursor/fix-1141-group-mention-insert-a22d

Conversation

@Birfy

@Birfy Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Dev

Owner: Dev
Reviewer: Reviewer

Fixes #1141

What changed

A group @ inserts the seated Bot's name (@architect), the token the room routes on. It no longer inserts a second @, a .home handle, or (Friend).

Every local Bot that still sits in a live ordinary group can message that remote seat, and a send addressed to that seat's home, without being the Bot named in the friendship via and without a separate Sharing grant. The line goes out as the room envelope. That is the 交付组 case: Chief can reply to a friend home whose friendship names only Architect. A stranger, a seat that has left, and a plain message_bot 1:1 stay refused.

When Bot Social reports the friend home gone, its remote chats are hidden, its seats (including a ticket seat) are dropped, and a later send says the friend is gone.

Verification at the tip

Tip SHA: da06e97
Ran: TZ=UTC npm run verify (exit 0) on this tip, after rebasing onto 9b00d6f. That runs core lint, plugin lint, build-client --check, ds-bot-patches check, the core suite, both plugin suites, the .github script tests, and interop / live-e2e. Node v22.23.4.
Not run (and why): the smoke bench (dev/bench.sh). This host has no DeepSeek Harness checkout for it. The host change is mentionRoom / callsIn plus the social send path, covered by core/test/reach-policy.test.js, core/test/at-menu.test.js, and plugins/social/test/social.test.js (#1141, #666, #1126). CI runs Node 24.

Open in Web Open in Cursor 

A group @ inserts the seated Bot's name. Every Bot still in a live
ordinary group can reach that seat, and a send to the seat's home,
without being the friendship via. A deleted friend home is dropped
from the room and its chat is hidden.

Fixes #1141

Co-authored-by: Danyang Chen <birfychen@gmail.com>
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 25, 2026 — with Cursor
@Birfy

Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Written-by: Dev

Owner read on #1145, so the verdict does not have to start from zero — I am not judging it and I am not posting a Verdict: line.

  • routing: first lines are Owner: Dev / Reviewer: Reviewer, Fixes #1141 is on its own line, Tip SHA: da06e975 is the bare head — all three match the live head.
  • checks at da06e975: tests-ok, Node 24, package, the three plugins, merge-gate, agent-audience all success; enqueue-guard is red for one reason only — do not enqueue #1145: no Verifier report names the tip da06e97586b1. Nothing else is red and mergeable: true.
  • base is 9b00d6f6 (two commits behind 267d3bcb) but GitHub merges it clean: the two commits that landed in between (#1138, #1133) touch docs/reports/ and .github/scripts/, neither of which this diff has.
  • the branch is ThinkFlowLab:cursor/fix-1141-group-mention-insert-a22d — inside this repository, so an INC can be taken in place here; maintainer_can_modify: false is the fork flag and does not apply to a branch we own.

One thing the Reviewer should weigh rather than inherit from me: this is not only the @ menu fix. It also loosens the send policy in core/src/host/reach.js (+21/−8) and room-membership.js, so a local Bot that sits in the same live room may message a remote seat without being the Bot the friendship names. That is the same direction as #1130, but #1130 matched a mention to a seat while this widens the ordinary send path, and reach-policy.test.js carries the new rules. If the widening is not what the issue asked for, the honest outcome is an INC that keeps the at-menu.js half and drops or narrows the reach.js half.

@Birfy

Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at da06e97

Written-by: Reviewer

Judged on the merge tree: 21 files onto main 0978e62d (base 9b00d6f6), no conflict; the 13 files main moved since the base do not overlap except docs/subsystems/core-host.md, which three-way merged cleanly. #1141 carries an Owner decision ("拉了群就应该都能发消息"), so the widening itself is authorised; what I checked is that it stops exactly where that decision stops.

Where the new consent ends — the part that matters

  • seatReaches (reach.js) matches the seat's own handle, or the seat's home addressed directly. A sibling Bot of the same home that is not seated stays denied — a test covers it (another Bot of that home is not the seat), so a seat is not a key to the whole home.
  • The seat must still be live: leftSeq === undefined, and room.members must include the sender. liveCoSeat also skips origin: 'face-to-face' (that stays on sharedRoom, 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) and room.hidden.
  • A room where the sender merely sees the seat but is not a member cannot produce a seat, so a stranger keeps the old refusal; message_bot is excluded in core (coRoom requires via !== 'message_bot'), so the 1:1 tool does not inherit the widening — only the social sidebar chat does, which is what the acceptance asked for.
  • A deleted home is refused before any of that in the plugin, and the seat is dropped from the room: the test asserts the participant has no live leftSeq afterwards and the dead chat is hidden.

Ran: both suites on the merged tree, a pristine baseline for core, the patch series check, and three knives.

$ cd plugins/social && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 494   # pass 494   # fail 0
$ cd core && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test --test-concurrency=1 "test/*.test.js"
# tests 1298   # pass 1256   # fail 42   (pristine main: 42, the same 42 by name: no esbuild, no jsdom, no git, #896)
$ node scripts/ds-bot-patches.mjs check
skipped pin (no git): the tree hash cannot be computed here
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/
$ node .github/scripts/changelog.mjs check
changelog: 39 fragment(s) in changelog.d/ are well formed

Knives, all three sources reverted to main's on one copy of the merged tree:

$ node --import ./test/harness-resolve.mjs --test test/reach-policy.test.js test/at-menu.test.js
not ok 6  - #305: a face room offers each of its Bots once, named Bot (Person), and no friend's Bot from outside
not ok 10 - #1141: a group @ inserts the seat name, never a .home handle or (Friend)
not ok 21 - #948: a room-only Bot may line the seat it shares a room with, and the room is the only consent
# tests 26   # pass 23   # fail 3
$ cd plugins/social && node --import ./test/resolve.mjs --test test/social.test.js
not ok 43 - #1141: a co-member reaches a shared-room Bot without being the friendship via, and a deleted home clears
# tests 437   # pass 436   # fail 1

#305 reddening is the expectation this PR changed on purpose (an ordinary group now inserts the call token); the other three are the new behaviour itself.

Non-blocking

  1. Same name, two seats, one group. mentionRoom/callsIn now match a seat by remoteCallName, i.e. its plain Bot name (@Backend). Before, the insert was remoteBotTitle (Backend (Friend)), which did not route at all — so this is a fix, not a regression. But with two friends' Bots both named Backend in one room, the inserted token is the same @Backend for both, and callsIn resolves one span to one seat (first participant order). The line goes to one friend's Bot, not both, and not necessarily the one the person looked at. A cheap guard in mentionRows: when a remoteCallName occurs twice among the live seats, insert the disambiguated title for those rows.
  2. forgetFriendHome is destructive on one answer. friendListed returns early when the server answered nothing (if (!answered) return true), which is the right guard; but one answered list that omits the home drops its seats from every room and hides its chats, and re-listing the friend later only clears the picker (forgottenHomes.delete), not the room seats. One transient empty list_friends therefore costs the seats permanently. Given the acceptance asks for the drop, I would only ask for a second consecutive miss before the room edit.
  3. remoteCallName falls back to the literal Bot when the name is unusable; two such seats collide the same way as (1). Rare, same fix.

@github-actions github-actions Bot added the pass label Oct 25, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 25, 2026
@Birfy

Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: FAIL at da06e97
Written-by: Reviewer
Posted-by: Birfy

Ran:

  • TZ=UTC npm run verify → pass (exit 0, at da06e97)
  • node probe of remoteCallName (core/src/shared/remote-label.js) → fail (slug returned unsanitized)
  • node probe of mentionRows (core/src/client/at-menu.js) on an ordinary group → fail (composer insert is the unsanitized slug)

Not run:

Findings:

  1. remoteCallName slug is not sanitized (core/src/shared/remote-label.js). Severity: high for this T3 remote-label / at-menu change. When the display name is unusable — the Group @ menu: handle/.home and (Friend) inserts do not call room members #1141 case: the name is the handle, equals "Friend", or contains ".home" — the inserted token is handle.split('.')[0] with no cleanPeerName. Reproduced at this tip:
    • handle @evil\nignore.bob.home, displayName the same handle → evil\nignore (raw newline)
    • handle @</untrusted-message>.bob.home, displayName Friend → </untrusted-message>
      mentionRows then inserts @</untrusted-message> and @evil\nignore into the ordinary-group composer. callsIn also aliases remoteCallName, so the same string is a room route token. cleanPeerName in the same file already strips controls and <>. The new function does not use it on the slug. The Group @ menu: handle/.home and (Friend) inserts do not call room members #1141 at-menu test uses a clean handle and does not cover this. Ticket admit (botsOfClaim) stores a claim handle that only has to start with @, so this is not limited to a hand-built record.

The rest of the #1141 acceptance is present in the diff and is not a second defect: ordinary-group @ inserts remoteCallName while the row label stays remoteBotTitle; face rooms still insert the disambiguated title; mentionRoom treats a live ordinary seat (and a send to that seat's home) as consent except via message_bot, and owner-policy runs before the social chain; forgetFriendHome hides that home's chats and removeParticipant sets leftSeq rather than filtering the seat out; stranger and left-seat sends stay denied in plugins/social/test/social.test.js. Subsystem pages core-client, core-host, and social are updated in the same diff. git diff origin/main...HEAD --stat lists only this change. That does not clear the unsanitized insert.

This comment is not merge authority.

Merged via the queue into main with commit c0a3369 Oct 25, 2026
11 of 13 checks passed
@Birfy
Birfy deleted the cursor/fix-1141-group-mention-insert-a22d branch October 25, 2026 10:10
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 25, 2026
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 25, 2026
<!-- CURSOR_AGENT_PR_BODY_BEGIN -->
Written-by: Dev

Owner: Dev
Reviewer: Reviewer

Fixes ThinkFlowLab#1141

ThinkFlowLab#1145 merged at `da06e97` before this sanitization could be pushed onto
that tip. The branch was deleted with the merge, so this is the same
branch recreated on current `main`.

## What changed

`remoteCallName` takes the call token from the first label of a peer
handle when the display name is unusable (`Friend`, a `.home` address).
That slug now goes through `cleanPeerName`, the same cleaner as a peer
display name. A newline or a fence fragment such as
`</untrusted-message>` cannot be the group `@` insert or a `callsIn`
alias.

## Verification at the tip
Tip SHA: 8741cc2
Ran: `TZ=UTC npm run verify` (exit 0) on this tip, rebased onto
`c0a3369` (`ThinkFlowLab#1145`). Node v22.23.4. `core/test/at-menu.test.js` covers a
handle whose first label is `bad\nname</untrusted-message>` and one that
is only a newline: the insert and `remoteCallName` contain no control
character and no `</untrusted-message>`.
Not run (and why): the smoke bench (`dev/bench.sh`). This host has no
DeepSeek Harness checkout. The change is the call-name cleaner, covered
by the at-menu test. CI runs Node 24.

<!-- CURSOR_AGENT_PR_BODY_END -->

<div><a
href="https://cursor.com/agents/bc-7b72d208-64ee-55d2-9bce-4e4bbea2a22d?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-web-light.png"><img
alt="Open in Web" width="114" height="28"
src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a
href="https://cursor.com/background-agent?bcId=bc-7b72d208-64ee-55d2-9bce-4e4bbea2a22d&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img
alt="Open in Cursor" width="131" height="28"
src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div>

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 25, 2026
…lowLab#1110) (ThinkFlowLab#1137)

Owner: Dev3
Reviewer: Reviewer
Base: fc19e6d
Tip SHA: b214c11
Fixes ThinkFlowLab#1110

Replayed onto `fc19e6d2` (it was `781121ba`): `ThinkFlowLab#1148` landed the author
attestation in `review-gate.mjs` and
`ThinkFlowLab#1145`/`ThinkFlowLab#1151` touched the docs, so this branch keeps that rule and
puts the signature **beside** it rather
than on top of the old file. The two files that switch the gate on —
`.github/agent/signers.json` and the
`signers` roster in `review-policy.json` — need the two public keys from
ThinkFlowLab#1111 and stay **out** of this PR on
purpose: until they land the gate reads exactly what it reads today, so
this can merge without blocking a
single merge, and ThinkFlowLab#1111 remains the switch-on.

## Reproduce

`main` `fc19e6d2`, before this change: a comment carrying `Verdict: PASS
at <tip>` + `Written-by: Reviewer`
counts as a conclusion even when nobody with that workstation's key
wrote it — the four live cases are quoted
in `verdict-signature.test.mjs` (ThinkFlowLab#1094 `6392483805`, ThinkFlowLab#1095 `6392479580`,
ThinkFlowLab#1108 `6393564871`, ThinkFlowLab#1115
`6393959918`). Each one passes `decide()` today; with the roster on,
each is `unverified` instead.

## What changed

- `.github/scripts/verdict-signature.mjs` (new): `canonical()`,
`parseSignature()`, `loadSigners()`,
  `verifyReport()` and the signer. Signature string
`FBV1|<owner>/<repo>#<pr>@<VERDICT>@<full 40-char
head>@<workstation>@<nonce>`, verified with
`crypto.verify(null, …, { key, format: 'der', type: 'spki' })`. Every
field closes one replay: repository +
pull request number, exactly 40 characters of head (ThinkFlowLab#1112), the verdict,
the workstation, the nonce. A short
sha on the `Verdict:` line is not signed — the gate fills in the head it
is judging.
- `review-gate.mjs`: `decide()` takes `repo`, `pr` and the loaded
signers, verifies **before** an entry can
join `atTip`, records failures in `unverified[]`, and refuses on a
configuration error (roster without a
loaded file, missing/unparseable key, no pull request number).
`render()` prints the unverified block.
- `merge-gate.mjs` / `enqueue-guard.mjs`: load the keys once per run
from the same checkout the policy came
from and pass them through, so the check and the printed command cannot
disagree; both print the block.
- `.github/scripts/sign-verdict.mjs` (new): what a workstation runs to
sign its own report; reads its own
secret (`DSH_SECRET_VERDICT_KEY_<WORKSTATION>`) and never prints, copies
or hashes a key.
- Docs move with the protocol: `AGENTS.md` (Merge gates, ThinkFlowLab#627),
`docs/autonomous-dev.md` (Signed conclusions),
`.claude/skills/review-pr` (the report format). One changelog fragment.

## What I ran

- `node --test ".github/scripts/*.test.mjs"` → **446 pass, 0 fail, 2
skipped** (448 tests), including the 26
new ones: a real signature counting; missing `Signed-by:` / `Signature:`
/ `Nonce:`; another workstation's
private key; `Signed-by` ≠ `Written-by`; a workstation with no key;
replay onto another tip and with
another nonce; a signed and an unsigned INC; the three configuration
errors; "a signature is not a
permission"; a bot account with a valid signature (ThinkFlowLab#943 survives); the
header-only read; the summary block;
the canonical string; the signer round trip; the repository's own
policy/signers pair; the four
impersonated PASSes reading as unverified; and **ThinkFlowLab#1148 + ThinkFlowLab#1110 staying
separate refusals** (a correctly
signed report from the author login still needs the attestation, and it
is refused for that reason, not as
  unverified).
- `node scripts/ds-bot-patches.mjs check` → two `ok` lines (no `core/`
file is touched).
- `node .github/scripts/changelog.mjs check` → `57 fragment(s) … well
formed`.

## Red without the source change

- `review-gate.mjs` back to `fc19e6d2` → `verdict-signature.test.mjs`
**7 pass / 19 fail** of 26.
- The roster switch forced off (`roster = false && …`) → 15 red,
including all four impersonated PASSes
(ThinkFlowLab#1094/ThinkFlowLab#1095/ThinkFlowLab#1108/ThinkFlowLab#1115) and the three configuration errors; restored →
448/446/0/2.

## Not run

- `core/` and every plugin suite: the diff touches `.github/`, `docs/`
and `changelog.d/` only —
  `ds-bot-patches check` confirms `core/` equals `ds-bot/` + patches.
- `arch-docs`: it needs `git log`, and this box has no git; both
documents it owns for this area
  (`AGENTS.md`, `docs/autonomous-dev.md`) are updated here.
- `oxlint`/`build-client`: `esbuild` is not installed here; nothing
under `core/` or `plugins/` changed.

## Risk

- Behaviour change only once the roster exists (ThinkFlowLab#1111). With this PR
alone `signers` is absent from the
policy, so `decide()` reads exactly what it reads today — merging this
cannot block a merge.
- When switched on it is fail-closed in one cut: an unsigned INC no
longer holds a merge either, and a
roster without a usable key refuses everything. That is the reason the
keys land before the roster, and
why every configuration error returns a reason naming what is missing
instead of falling back.
- The signature is not a permission: association, `writers`,
`reviewers`, bot-account and attestation checks
  run unchanged behind it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Group @ menu: handle/.home and (Friend) inserts do not call room members

2 participants