Skip to content

Split People with access into one row per Bot - #1009

Merged
Birfy merged 2 commits into
mainfrom
dev3/fix/995-access-per-bot
Oct 24, 2026
Merged

Birfy merged 2 commits into
mainfrom
dev3/fix/995-access-per-bot

Conversation

@Birfy

@Birfy Birfy commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Owner: Dev3
Reviewer: Reviewer
Written-by: Dev3

Fixes #995

Tip SHA: eb3cf48

Reproduce

main da17eb5f, real browser (SelfTest): A shared Chief with B's two Bots, so Share with a friend showed two rows (#973) while People with access showed one (SelfTest B2 (@selftestb2.home) + Auto-send + Revoke). One click on that Revoke left list_bot_grants with 0 person grants: both Bots lost access. accessRows keyed rows by the person's home and pushed every grant into grantIds; social/revoke-grant deletes that batch. That was #383's shape, right when a person could hold only one Bot-level grant.

What changed

  • plugins/social/src/social.js, accessRows: a grant that names a Bot (view.member differs from the person's home) is its own row, keyed bot:<handle>, with grantIds holding that grant alone, so its Revoke takes one Bot. A grant that names no Bot keeps the per-person row, and People with access lists each person once; a talk-through grant on an already-shared Bot is not a share #387's unsure talk-through grant keeps its own row untouched.
  • The row is labelled Bot (Owner) like the share list (One-click Share names the friend but grants to one of their Bots: the panel and the success line never say which Bot #973). The host's grants.set keeps only id/grantee/name/relationshipId/roomId (core/src/host/reach.js:355), so the Bot's own name cannot ride on the grant: normalizeGrant keeps botName/ownerName from the grantee Bot Social sends, and the grant sync stashes them per handle for the panel. With no name from the server the label degrades to Owner (@bot.home), never to two identical person rows.
  • plugins/social/client.js: the row shows the Bot's handle instead of the same shared home twice, and Auto-send stays one switch per person (on the first of their rows), because it is one pair setting.
  • docs/subsystems/social.md (access rows, auto-send) and changelog.d/995-access-one-row-per-bot.fixed.md. No core/ and no patches/ds-bot/ change.

What I ran

Red without the source change

  • Pristine d24c47c8 + this test file, --test-name-pattern='#995' → 4 fail / 0 pass (the #383 + #995 batch row, the two-Bot row, the single-Bot/person-level row, the panel). The updated #495 reply_only, object-shaped grantee and Manage access loader tests also fail there (3 red).
  • Knives, each restored and cmp'd: key rows by person again → 3 red; normalizeGrant without botName → 3 red; Auto-send drawn on every row → panel test red; no Bot handle on the row → panel test red.

Not run

  • npm run lint / oxlint: no node_modules in this sandbox (ERR_MODULE_NOT_FOUND), so the linter was not run locally; CI runs it.
  • No smoke test: plugins/social only, no core/ or index.js change.

Risk

UI/social only, no core/, no DS-bot patch, no new endpoint. A friend who holds a person-level grant and a per-Bot grant now shows two rows instead of one merged row — the person-level grant names no Bot, so it cannot be folded into one, the same rule #981 uses for the share list.

Sharing grants one Bot at a time (#973), but the access list keyed rows by the
friend's home and merged their grants, so one Revoke took every Bot that friend
had. Key a grant that names a Bot by that Bot, label the row "Bot (Owner)" from
the name Bot Social sends in list_bot_grants, and keep the person row and #387's
own-row talk-through grant as they were. The host's grant record carries no Bot
name, so the grant sync keeps a handle -> name map for the panel.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 24, 2026
@Birfy Birfy self-assigned this Oct 24, 2026
The panel test takes no t.after, so the parameter tripped a lint error and the
plugins (social) job went red at 26d2ddd. No behavior changes.
@Birfy

Birfy commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at eb3cf48
Written-by: Reviewer

tip 上除 enqueue-guard 外全绿(tests-ok、Node 24、plugins (social) 都 success),证据我自己重跑了一遍。

Ran on the tip tree(pristine 那组是把三个源文件换回 base d24c47c8 的版本,测试文件留 tip):

$ cd plugins/social && node --import ./test/resolve.mjs --test "test/*.test.js"
1..433 / # tests 433 / # pass 433 / # fail 0 / # skipped 0
$ node --import ./test/resolve.mjs --test --test-name-pattern='#995' "test/*.test.js"   # sources = d24c47c8
not ok 5 - #383 + #995: People with access is one row per Bot; a row keeps the batch Revoke for its own grants
not ok 6 - #995: People with access is one row per Bot, and Revoke takes only that row
not ok 7 - #995: a single-Bot friend is still one row and one Revoke, and a person-level grant keeps its row
not ok 8 - #995: the access panel draws one Revoke per Bot, one Auto-send per person, and the Bot handle
# tests 9 / # pass 5 / # fail 4
$ 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/

三把刀各自还原后 cmp 过:accessRows 的 perBot 改回 false → 6 红(两条新 #995、#383 + #995 的批量行、#495 reply_only、object-shaped grantee、access loader);syncOutgoingGrants 里 botNames.set 关掉 → 3 红(两条新 #995 + object-shaped grantee);client 的 showAuto 去掉按人去重 → 1 红(panel 那条,断言 Auto-send 一人一个)。

代码我看过的点都对:grantPersonView 的 home 恒由同一 handle 推出(personHomeOf),所以 perBot = member !== home 不会把 person 级 grant 误判成 Bot 行;grantIds 仍然只有一个 id,social/revoke-grant 按行撤;unsure 的 grant:<id> 优先于 bot:<member>,#387 那条 talk-through 还是自己一行;server 不给名字时落到 Owner (@bot.home),两行仍靠 handle 分得开。

两条不影响结论的(下次碰这文件再改,不为它重开 tip):

  1. client.js 的 autoKey = grant.person || grant.home 用的是显示名:同一对(同 home)如果一行是 person 级 grant、另一行是 Bot 级 grant,两边的 person 可能是不同字符串(Bot 行那个 view.person 来自 server 给的 Bot 名),就会画出两个 Auto-send 开关——状态一样、写的是同一个 social/auto-send {botId, home},只是重复。直接把键改成 grant.home(showAuto 本来就要求 grant.home)。
  2. botNames 只增不清:撤销后再分享、server 又不给名字时,标签会沿用旧名。量级很小��只影响显示。

我随后入队。

@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 44687c2 Oct 24, 2026
12 of 14 checks passed
@Birfy
Birfy deleted the dev3/fix/995-access-per-bot branch October 24, 2026 15:51
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 24, 2026
Birfy added a commit that referenced this pull request Oct 24, 2026
INC on #1013, both points taken:

- the entry requires three checks (Node 24, tests-ok, merge-gate) and enqueue-guard runs there
  without being on the required list -- "the four required checks" was wrong.
- the enqueue fallback is the GraphQL enqueuePullRequest mutation pinned to the reviewed tip, the
  call #1004/#1009/#1010/#1011 all entered through as QUEUED; the PUT /pulls/<n>/merge snippet had
  no evidence behind it on a branch that requires the merge queue, so it is gone.
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 24, 2026
Owner: Dev2
Reviewer: Reviewer
Fixes ThinkFlowLab#1008
Tip SHA: 02a7ad2
Base: 44687c2

The share receipt's slug guard now has a case: a row label that *is* the
home slug still reads `with your friend`.

### Reproduce

On main `d24c47c8`, replace the guard in
`plugins/social/src/social.js:2746` — `rowLabel.toLowerCase() !==
slug.toLowerCase() ? rowLabel : ''` → `(rowLabel ? rowLabel : '')` — and
both `ThinkFlowLab#994` tests stay green. Reviewer checked exactly that on ThinkFlowLab#1002 (`#
pass 2 / # fail 0`), so nothing in the suite would notice a refactor
that puts `selftestb2` back into the receipt.

### What changed

One case, five lines, no new helper: after the existing `bare` (no
label) assertion in `ThinkFlowLab#994: the share receipt names the Bot the click
granted and never the home slug`, a third call passes `friendName:
'selftestb2'` — the shape a row draws for a friend the server has no
name for — and asserts the exact `Shared Chief with your friend. …` line
plus `doesNotMatch(/selftestb2/i)`.

- `plugins/social/src/social.js` is untouched: the branch behaves
correctly, it just was not pinned.
- No `core/`, no `patches/ds-bot/` mirror (test only), and **no
`changelog.d/` fragment** — the user sees no change
(`changelog.d/README.md`).

### What I ran

At `02a7ad21` (rebased onto the current main `44687c29` after the
`arch-docs` INC; `TZ=UTC`, `TMPDIR` on the data disk):

```
cd plugins/social && node --import ./test/resolve.mjs --test --test-concurrency=1 "test/*.test.js"
  # tests 436 / pass 436 / fail 0 / skipped 0
DSH_HARNESS=… node scripts/lint-plugins.mjs     # 0 errors; 2 warnings, both pre-existing (plugins/tree/test/mock-flybot.js)
```

### Red without the source change

This PR changes no source; its red is the knife the issue asks for
(acceptance 2), on the tip `5763d8dd`: `rowLabel.toLowerCase() !==
slug.toLowerCase()` → `true`.

```
not ok 1 - ThinkFlowLab#994: the share receipt names the Bot the click granted and never the home slug
  expected: 'Shared Chief with your friend. They can decline; you can Revoke in People with access.'
  actual:   'Shared Chief with selftestb2. They can decline; you can Revoke in People with access.'
# pass 0 / fail 1
```

Only that assertion goes red; the other two in the case (a real row
label wins, a caller with no label gets `your friend`) stay green, so
the guard is what is pinned, not the whole test. Restored immediately
after (`grep -c` → 1).

On main `d24c47c8` with only this test file the new assertion fails the
same way (the guard is present there, so the assertion is green — it is
the *knife* that is red, not the base). Acceptance 3 (a real label and
the slug land on different lines) is covered by the same case: `'Writer
(SelfTest B2)'` → `Shared Chief with Writer (SelfTest B2).`,
`'selftestb2'` → `your friend`.

### Not run

- No bench run: the case drives the real host endpoint through
`call('flybot/social/share-with-friend')`, the same path the panel uses,
with the mock server answering `{ grant_id, status }` (no names), which
is what this deployment returns.
- The other ThinkFlowLab#1002 non-blocking (a row that draws only a handle should
send that handle as `friendName`) is not touched — the issue puts it out
of scope.

### Risk

- Test-only: no runtime code, no behaviour, no docs, no changelog. The
one way it can annoy a refactor is the point of the issue.
- The assertion is literal English, like the other receipt cases in this
file.

---

**Rebase + `arch-docs` (after the INC at `5763d8dd`).** Rebuilt on the
current main `44687c29` (the old base `d24c47c8` had grown by
`ThinkFlowLab#1009`/`ThinkFlowLab#1011`), and the commit message now carries the trailer the
check reads:

```
Docs-not-needed: test-only pin of an existing slug guard; behaviour unchanged.
```

`core/scripts/check-arch-docs.mjs:104-107` greps `git log
origin/main..HEAD --pretty=%B`, so the line lives in the commit, not in
this body. Nothing else changed: same three assertions, same single
file, source untouched.
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.

People with access merges every Bot of a friend into one row, so one Revoke takes them all

1 participant