Repository navigation
Word a revoked share as one line, with no retry - #974
Merged
Merged
Conversation
When a friend's owner took a share back, Bot Social answered the send with
403 capability_not_granted ("peer has not granted chat.public"). The social
plugin had no branch for it, so it fell to the generic tail and the transcript
read "Not sent: peer has not granted chat.public. Try again." — an internal
capability name, and advice that cannot come true until that owner shares again.
`capability_not_granted` is the same fact as `bot_not_shared` seen from the
other side, so one predicate, one sentence and one no-retry mark now cover
both: the existing "is not shared with your Bots" line, which the transport
prefixes with "Not delivered:", and `noRetry: true` so a card offers no Retry.
`notSentLine` passes that sentence through whole instead of appending
"Try again.", and still words a raw capability name if a path reports it
before the plugin maps it. Fixes #971.
Collaborator
Author
|
Verdict: PASS at 2f7f46d One fact, one sentence, one no-retry mark: both send paths ( Not a code problem, but it blocks the merge: this pull request carries no labels. Ran at this tip (tarball of
Two non-blocking:
Not run: the whole |
HugoXK
pushed a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 24, 2026
Owner: Dev4 Reviewer: Reviewer Fixes ThinkFlowLab#973 Written-by: Dev4 Tip SHA: a754927 Base: 90f4bb5 ## Reproduce On `a9ae2731`: one friend runs two Bots (say **Coder** and **Writer** under one home). Sharing → **Share with a friend** shows a single row reading `SelfTest B2 (@selftestb2.home)` — the person — and clicking it grants whichever of their Bots `list_friends` listed first. The panel's own text says "share with SelfTest B2", the receipt says "Shared Chief with SelfTest B2", and the friend can end up holding the wrong Bot. Reproduced in the self-test's third reading at `2ab9e3928279` (`share-friends` returned `peerBotId: bot_01M5PV1G7…`, the second Bot). ## What changed - `plugins/social/src/social.js` — `shareFriendsForBot` keys on the friend's Bot (`peer_bot_id`) instead of the home, so a friend with two Bots is two rows; each row is labelled `Bot (Owner)` and now carries the Bot's `handle` as well as its `peerBotId`. A friend without a Bot id still keys on the home, so nothing is dropped or duplicated. `share-with-friend`'s receipt names the Bot as well (`Shared <my Bot> with Writer (Bob).`), not only the owner. - `plugins/social/client.js` — one key per row (`friendKey`: Bot id, else handle, else home), the busy flag and the "Sharing…" label are per row instead of per person, the disabled state is only on the row being shared, the fallback refresh removes only that row, and the row's second line shows the Bot's handle. - `docs/subsystems/social.md` (both places that describe the share list) and `changelog.d/973-share-names-the-bot.fixed.md`. ## What I ran - `plugins/social`: `DSH_HARNESS=/opt/flybot node --import ./test/resolve.mjs --test "test/*.test.js"` → **411 pass / 0 fail** (the whole plugin suite on the rebased tree, not only `social.test.js`; the two extra cases over the 409 at `a9ae2731` are `ThinkFlowLab#968`'s, already in main). - `changelog.mjs check` → 62 fragments well formed; no `core/` or `patches/` file is touched by this PR, so the patch series cannot move. - Rebased onto `90f4bb5a` (`ThinkFlowLab#974` + `ThinkFlowLab#975`). They touched `docs/subsystems/social.md`, `plugins/social/client.js`, `plugins/social/src/social.js` and `plugins/social/test/social.test.js` too, so those four were three-way merged against `a9ae2731` rather than replaced: the rebuilt files differ from `90f4bb5a` only by this PR's hunks, and the suite above runs on that tree. ## Red without the source change With only the new tests on pristine sources: `not ok 303` (the share list still returns one row and the grant goes to `pb-coder`) and `not ok 308` (both rows carry the same React key) → **357 pass / 2 fail**. Restored: 409/409. The panel case asserts the two rows have two distinct keys, one reason per row is what lets the click reach the right Bot. ## Not run - `npm run verify` as a whole (the shared `node_modules` on this box makes the full core run abort with esbuild `EAGAIN`); CI's `tests-ok` is the judge. This PR changes no `core/` file, so the core suite is not affected by it. - `arch-docs`: needs `git`; both owned paths changed here (`plugins/social/src/`, `plugins/social/client.js`) have their subsystem doc updated in this PR. - No browser run: the row is asserted through the panel's own render (two rows, two keys, per-row button) and through the endpoint that feeds it. The self-test routine is the one that sees it in the UI. ## Risk The one way to make this worse: listing a friend twice for the *same* Bot. The key is the Bot id, so a repeated `peer_bot_id` collapses exactly as before; a friend whose server sends no Bot id still collapses by home. `grantedHomes` is unchanged — if any grant exists for that home the whole friend is still omitted, which is the older behaviour and narrower than this PR (per-Bot grant filtering would need the grant's Bot id, which `list_bot_grants` does not give us today); it cannot show a Bot the friend already has, only hide one they do not.
HugoXK
pushed a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 24, 2026
…owLab#972) Owner: Dev2 Reviewer: Reviewer2 Fixes ThinkFlowLab#967 Tip SHA: d0fa17e A friend's Bot picked from the @ menu is inserted under its one name — the name its bubble, the menu row and the sidebar preview all read — while the handle stays the routing key. **One claim note:** [HugoXK said "Claiming for Dev-1"](ThinkFlowLab#967 (comment)) before the Chief assigned this issue to Dev2; there is no branch and no PR from that claim (404 on `dev1/fix/967-mention-handle`), so this is the one PR for ThinkFlowLab#967. **Rebased:** opened on `a9ae2731`; `ThinkFlowLab#974`/`ThinkFlowLab#975` and then `ThinkFlowLab#976` merged after, so this tip is replayed onto current main `90ff273d`. `ThinkFlowLab#975` (`hooks.js`, `flybot-slots.js`, `plugin-runtime.md`, `0008`) was a hand merge in `plugin-runtime.md` (both rows kept) plus regenerated patches; `ThinkFlowLab#976` (which touches `social.md`, `plugins/social/client.js`, `plugins/social/test/social.test.js` — three files this PR also edits) is a clean three-way merge against the real merge base, so `ThinkFlowLab#973`’s rows and this PR’s label insertion both survive. Inspected both sides after the merge, not just the exit status. ### Reproduce On `a9ae2731`, publish a friend's Bot in the roster and pick it from the @ menu (no 1:1 chat needed): - `remoteMentionRows` (`core/src/client/flybot-registry.js:137-142`) inserts `@<handle>`. - `mentionableBots` (`core/src/client/user-message.js:36-47`) reads `roster.bots` chats and room seats only, never `roster.remoteBots`, so a Bot with no chat open matches nothing; the `@` stays literal text and the shell draws a file chip from the bare handle (`coder.bob.home`, no `@`, no name). - The menu row and the sidebar preview already read `remoteBotTitle` / `friendBotTitle`, so they show `@Coder (Bob)`: the same object wore two names in two places. - The social plugin's own @ row (`plugins/social/client.js`, `friendRows`) has the same split: it shows `Coder (Bob)` and inserts `@coder.bob.home`. - Delivery was never broken — `hooks.js:889-895` routes by handle — so this is a display/insert-text defect, not a send defect. ### What changed - `core/src/shared/remote-label.js`: new `remoteMentionLabel(record, others)` — the title, plus the home when two different handles share a title (`Chief (Bob) · bob2.home`); `friendRecord` also reads the published `roster.remoteBots` top-level `ownerName` / `owner` / `homeName` fields and `isChief`, so a record with no chat does not fall back to the lowercase derived owner. - `core/src/client/flybot-registry.js`: the @ menu row's `text` is `@${remoteMentionLabel(bot, all)} `. - `core/src/client/user-message.js`: `mentionableBots` also offers `remoteMentionableBots(roster, chats)` — every published `roster.remoteBots` entry and every `roster.bots` entry with a `remote.handle`, deduped by handle, two names each (the bare handle and the label), `id` only where a chat exists; a mention with no chat renders as text, not as a button that cannot open. Room seats and face-to-face routing are untouched. - `core/src/client/at-menu.js`: a `row.remote` row's menu `name` is the text it inserts, so the menu reads the way the bubble will. - `core/src/client/text.js`: `nameMentions` resolves a published record through `remoteMentionLabel`, so the preview and the bubble use the same word. - `core/src/host/hooks.js`: `sendUserMentions` adds `{ name: remoteMentionLabel(bot, remoteList), handle }` beside the bare handle and the chat-name aliases — one more name for the same handle, the transport key does not move. - `core/src/client/flybot-slots.js` + `core/types/flybot.d.ts`: `__flybot__.names` also carries `remoteMentionLabel`, so a classic-script plugin row inserts the name the bubble chips. - `plugins/social/client.js` (`friendRows`): the row's `value` is `@${label} ` and its `description` stays the handle; a host without the helper keeps the old handle insertion, which still routes. - Tests: `core/test/flybot-client.test.js` (old `@coder.bob.home` insertion expectations updated; new ThinkFlowLab#967 case covers the menu insert, the bubble, the preview, the two same-named `Chief`s staying apart while each resolves to its own handle, a bare handle still chipping, and plain text untouched; the source guard now also covers `flybot-slots.js`), `core/test/inbound-ui.test.js` (ThinkFlowLab#71 preview expectations), `core/test/at-menu.test.js` (ThinkFlowLab#384: offered once by its handle, under its one name), `core/test/flybot.test.js` (mentions ThinkFlowLab#13: `@Coder (Bob) ` reaches the same handle once, `via: 'user-mention'`), `plugins/social/test/social.test.js` (new ThinkFlowLab#967 case: the plugin row inserts the label, two same-named friends' Bots carry the home, and a host without the helper gets the handle). - Regenerated `core/client.js` and the mirrors/patches (`0001`, `0003`, `0008`, `0012`, eight `patches/ds-bot/files/` entries) — the patch surface is exactly the files this PR touches, nothing else moved. - Docs: `docs/subsystems/core-client.md`, `core-host.md`, `plugin-runtime.md` (which owns `flybot-registry.js`, `flybot-slots.js`, `types/flybot.d.ts` and these test files), `social.md` (which owns the plugin row); `changelog.d/967-friend-bot-mention.fixed.md`. ### What I ran At `d0fa17e` (this tree, `TZ=UTC`, `TMPDIR` on the data disk): ``` cd core && node --import ./test/harness-resolve.mjs --test --test-concurrency=1 "test/*.test.js" # tests 1394 / pass 1386 / fail 8 cd plugins/social && node --import ./test/resolve.mjs --test --test-concurrency=1 "test/*.test.js" # tests 412 / pass 412 / fail 0 node scripts/build-client.mjs --check # client.js matches src/client node .github/scripts/changelog.mjs check # 62 fragment(s) well formed oxlint -c .oxlintrc.json index.js web-search.js src dev scripts test # 0 errors, 48 warnings node scripts/ds-bot-patches.mjs check # needs git; exact steps run via the strict stand-in below ``` The 8 core reds are the machine's, not this change — each reproduces on pristine `90f4bb5a`, and the same eight are red on `90ff273d` (that merge touches no `core/` file): - `test/composer-label.test.js`, `test/header-effects.test.js`, `test/read-page.test.js` — `Cannot find module 'jsdom'` (this box has no `npm run harness` install); - `check-arch-docs.test.js` "empty diff passes" — `cannot diff against HEAD` (no git here); - `usage-fields.test.js` "fixture usage.sqlite …" — `null !== 0`; - the three `ThinkFlowLab#896` part-archive cases — `until: the condition did not hold within 2 s`, also red on pristine `90f4bb5a` (0/3 there, same three cases), and they pass when the same file runs alone under a local `TMPDIR` (the shared `/tmp` was full: `ENOSPC`). The four touched core test files are **80/80** here; the social suite is **412/412**. `ds-bot-patches check` needs git, which this box does not have, so I ran its exact steps with a strict stand-in (copy `ds-bot/`, overlay `patches/ds-bot/files/`, apply the 10 patches in order **without search** — every hunk must land on the line its header declares, its counts must be used up exactly, and no hunk may run past EOF): **10 patches, 114 file sections, 1021 hunks, all on declared lines**, and the rebuilt tree equals `core/` (357 files, byte for byte). The four regenerated sections reuse main's function-context header wherever the hunk body is unchanged. `arch-docs` ownership was checked by calling `loadSubsystemMap`/`pathMatchesOwned` from `core/scripts/check-arch-docs.mjs` over the exact changed-file list: every owned path resolves to one of the four updated docs (or is unowned). ### Red without the source change Core: pristine `90ff273d`, only the four test files dropped in (sources untouched) — **75 pass / 5 fail**: ``` not ok 7 ThinkFlowLab#384: … offered once … under its one name not ok 14 remote members: @ rows insert the one name, … not ok 23 a friend's Bot picked from the @ menu reads as its one name (… ThinkFlowLab#967) not ok 46 mentions (ThinkFlowLab#13): a friend's Bot named with @ gets the message once not ok 71 a friend Bot's handle in a preview reads as its one name (ThinkFlowLab#71, ThinkFlowLab#967) ``` Plugin: in this tree, reverting only `plugins/social/client.js`'s `friendRows` to the handle insertion (test kept) makes the new ThinkFlowLab#967 case fail — `not ok 1` — and restoring it is green. So the plugin half has its own red, not just the core half. ### Not run - `npm run verify` end to end (needs `npm ci` + `npm run harness`); the suites and four scripts above cover its steps. - `node scripts/ds-bot-patches.mjs check` under a real git: no git here; CI's `package` job runs it, and the strict stand-in above does the same checks locally. - No browser pass: the @ menu and the bubble are covered by the unit cases above, but I did not click through the UI. ### Risk - The inserted token changes from `@handle` to `@Name (owner)`: an old client (or a plugin) that expects a bare handle after `@` still routes, because `mentionableBots` keeps the bare handle as one of its names and the host resolves every alias to its handle; text that was already chipped keeps chipping. - The social plugin row only renders for friends the core does not list yet, and for those the core cannot route a mention either way — the row now reads as a name instead of a handle, and the moment the friend reaches `roster.remoteBots` the same label routes. - Two friends' Bots with the same label disambiguate with the home (`Chief (Bob) · bob2.home`), which is longer than a handle; a long name plus a long owner could widen the bubble. - Nothing about delivery, room seats or `list_bots` moved; the risk is display-text shape only.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Owner: Dev4
Reviewer: Reviewer
Fixes #971
Written-by: Dev4
Tip SHA: 2f7f46d
Base: a9ae273
Reproduce
On
a9ae2731: two friends, A shares a Bot, B sends a line to it (it arrives), A revokes in People with access → Revoke, B sends again. The transcript getsNot sent: peer has not granted chat.public. Try again.— an internal capability name and advice nothing can satisfy. The card next to it already says the human sentence; the same situation is drawn two ways on one screen. The self-test routine reproduced it in two runs at2ab9e3928279.What changed
plugins/social/src/social.js—isNotSharednow also matchescapability_not_granted(the code Bot Social answers with once a share is taken back) and the "has not granted " wording, so one predicate covers the two spellings of one fact.notSharedErroris the one sentence both take, it now carriesnoRetry: true(nothing changes until that owner shares again), and its "…not shared with your Bots yet…" lost the "yet" so it reads true whether the share never existed or was withdrawn. Both send paths (userFacingSendError,mapSendErrorwith itsNot delivered:lead) go through it.core/src/host/remote-chat.js—notSentLinepasses that "not shared with your Bots" sentence through whole instead of falling to the tail that appends Try again., and words a rawcapability_not_granted/ "has not granted …" error as one sentence if some path reports it before the plugin maps it.docs/subsystems/social.md(both codes, one sentence,noRetry) anddocs/subsystems/core-host.md(notSentLinepass-through);changelog.d/971-capability-not-granted.fixed.md.What I ran
plugins/social:DSH_HARNESS=/opt/flybot node --import ./test/resolve.mjs --test test/social.test.js→ 358 pass / 0 fail.core:DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/remote-chat.test.js test/new-chat.test.js test/flybot-client.test.js test/host-approval-status.test.js→ 48 pass / 0 fail.changelog.mjs check→ 60 fragments well formed;oxlinton the two changed sources → 0 errors, 15 pre-existing warnings.githere): every stored diff applied in order and checked the waygit applyreads it — counts, each hunk at the line its header names, nothing past EOF — 10 patches / 114 sections / 0 bad, ordered rebuild 358 files / 0 differences; the two mirrored files (src/host/remote-chat.js,test/remote-chat.test.js) are byte-identical tocore/.Red without the source change
With only the new tests (pristine sources): social
not ok 277→ 357/1 (the new#971case), corenot ok 13→ 15/1 (the newnotSentLinecase). Restored: 358/358 and 16/16. DroppingnoRetry: truealone also reds the old#336case, which now asserts it forbot_not_sharedas well.Not run
npm run verifyas a whole: the fullcoresuite aborts on this box (esbuildEAGAINfrom the sharednode_modules), so CI'stests-okis the judge; the subsets above are every file that reads this code.ds-bot-patches check/arch-docs: both needgit. CI'spackagecovers the first; for the second, both owned paths changed here (plugins/social/,core/src/host/) have their subsystem doc updated in this PR.notSentLineand the plugin's own error mapping, which is where it is produced. The self-test routine will see it on the next pass.Risk
The one way to make it worse: swallowing a different 403 into the "not shared" sentence.
capability_not_grantedand "has not granted " are narrow,isNotSharedis checked afterisBotGoneand the pause branch and before the generic tail, and every other 403 keeps its own line. Nothing about routing, delivery or grants changed — a message that cannot be delivered still is not delivered; only its sentence and its retry mark did.