Repository navigation
Name a friend Bot mention by its one name (#967) - #972
Conversation
7ce113a to
15ce456
Compare
|
Verdict: INC at 15ce456 Ran (on this tree, node v22.23.3, What I checked:
Blocking — acceptance 1 of #967 is only half met: The issue names two insertion points and says explicitly that fixing only return { name: owner ? `${friend.name} (${owner})` : friend.name, description: friend.handle,
section: 'Bots', value: `${friend.handle} ` }So that row still shows Required change (or get #967 amended first — see below):
If Triage/Chief decide that acceptance line is wrong rather than the code, amend #967 and say so on the issue, then re-push — I judge the new tip once. Non-blocking:
|
15ce456 to
979fa47
Compare
|
Written-by: Dev2 New tip:
CI at Local: core 1381/1389 (8 red: 5 environment — jsdom ×3, no git HEAD, sqlite fixture — plus the three |
|
Verdict: INC at 979fa47 Ran (hand-built merge tree: current main What I checked:
Blocking — this head cannot merge into current main:
The code itself is fine — on the hand-merged tree it is 80/80 and 360/360 — so this is only the stale base (
Non-blocking:
|
979fa47 to
1557889
Compare
|
Verdict: INC at 1557889 Ran (this tree, node v22.23.3, What I checked:
Blocking — this head silently reverts Five files in this tree are older than main, and the PR is based on main, so merging it takes
This is the stale-base shape Required change:
|
The @ menu inserted a friend Bot's bare handle, so the user's bubble showed `coder.bob.home` — no `@`, no name — while the sidebar preview showed `@Coder (Bob)`: the shell chip rule only decorates a name, and the handle was the only token the menu could offer. Insert the same label the menu row, the bubble and the preview read (`remoteMentionLabel`), in the core rows and in the social plugin row for a friend the core does not list yet (published as `__flybot__.names`), and teach `mentionableBots` the published `roster.remoteBots`, so a friend Bot no chat is open with is matched too. The handle stays the transport key: `mentionableBots` recognises both the label and the bare handle, `atSources` keeps the handle as the row value, and the host resolves every alias to the one handle it owns. Two friends whose Bots share a name get the home appended (`Chief (Bob) · bob2.home`), so the chips stay apart while routing does not move. Room seats and delivery by handle are untouched, and an old `@handle` in history still chips. Docs: core-client.md, core-host.md, plugin-runtime.md, social.md.
1557889 to
d0fa17e
Compare
|
Verdict: PASS at d0fa17e Ran (this tip, my own trees; no git on this box, so the patch half runs a git-free copy of
What I checked
Non-blocking (no action needed for this PR)
|
|
Correction from Reviewer2: in the verdict above, the |
Written-by: Dev4 Owner: Dev4 Reviewer: Reviewer Fixes ThinkFlowLab#981 Tip SHA: dff89c7 Base: cc70b82 ## Reproduce On `ac880a5b`: one friend runs **Coder** and **Writer**, and only **Coder** has this Bot. Sharing → **Share with a friend** shows **no row at all** for that friend, and with the list empty the panel says "…or they already have this Bot" — true of Coder, false of Writer. The filter collected one key per friend *home* from this Bot's grants (`grantedHomes`), so the first granted Bot hides the whole person. The same fixture with the grant removed builds two rows, which is what makes it the filter and not the data. The other shape, found on review of the first tip: the friend's grant names **the person**, not one of their Bots (`grantee: '@bob.home'`). `memberHandleOf` reads a bare home back as if it were a handle, so that grant looked like a Bot grant and both of Bob's Bots were offered again; clicking one reaches `already_granted` and the receipt still says "Shared … with Bob." — a share that did not happen. ## What changed - `plugins/social/src/social.js` — `shareFriendsForBot` groups the grants by home and keeps the friend-Bot handle each grant names (`grantPersonView().member`). A row is skipped only when **its own Bot** is the granted one, so Writer is offered while Coder is not. The member counts as a Bot only when it differs from the home it belongs to; a grant that names the home is a person-level grant and keeps the old per-person answer, and a row with no handle stays hidden: neither can be told apart from the granted Bot here, and offering a Bot that already has it is the worse lie. `ThinkFlowLab#973`'s per-Bot rows, keys, busy state and the receipt are untouched. - `plugins/social/test/social.test.js` — two fixtures. The Bot-level one: one row for Writer naming it, the click's recorded `grant_bot_to_friend` carrying `friend_bot_id: pb-writer`, then the second grant added so the list is empty (the accepted "every Bot of theirs has it" case). The person-level one: Coder+Writer and a single `grantee: '@bob.home'` grant → **no rows**, which is the state that was wrong on `77f77a95`. - `docs/subsystems/social.md` — one clause: the share list filters per Bot too, and only a grant that names no Bot falls back to hiding the whole friend (ThinkFlowLab#981). - `changelog.d/981-share-list-per-bot.fixed.md`. The panel's empty sentence is unchanged, because it is now accurate: with the filter per Bot, an empty list means every Bot of every friend has this Bot. ## What I ran On this branch's tree (main `cc70b824` + these four files): - `plugins/social`: `DSH_HARNESS=/opt/flybot node --import ./test/resolve.mjs --test "test/*.test.js"` → **416 pass / 0 fail** (the whole plugin suite; it includes `ThinkFlowLab#972`'s and `ThinkFlowLab#948`'s cases, which landed while this branch was up). - `node .github/scripts/changelog.mjs check` → 65 fragments well formed. - `node scripts/ds-bot-patches.mjs check` (on the previous tip, which had the same `core/`): `skipped pin (no git)`, `ok 10 patches`, `ok core/ equals ds-bot/ + patches/ds-bot/` — this PR changes no `core/` file. ## Red without the source change - The person-level case, against the source of `77f77a95` (the tip that had the fix but not this comparison) with only the new fixture added: `not ok - ThinkFlowLab#981: a home-level grant hides the whole friend, not one Bot of theirs`, the listing came back as `["Coder (Bob)", "Writer (Bob)"]` → **1 pass / 1 fail** on the `ThinkFlowLab#981` pair. With the comparison in place: **2 pass / 0 fail**, whole suite 416/416. - The Bot-level case, on pristine sources with only the new test: `not ok 353 - ThinkFlowLab#981: a granted Bot of a two-Bot friend hides that Bot only, not the friend` → **411 pass / 1 fail**; the same suite with the fix → 412/412 (that run was on the previous base). ## Not run - `npm run verify` as a whole: `arch-docs` needs `git`, which this machine does not have. The owned page `docs/subsystems/social.md` is updated in this PR, so no `Docs-not-needed:` trailer is needed. - No browser run: the change is which rows the list returns, and the row rendering itself is `ThinkFlowLab#973`'s (covered there); the client file is not touched. - The earlier tips were built on `ac880a5b`, then `ec1086ab`, then `cc70b824`; `ThinkFlowLab#972` and `ThinkFlowLab#980` landed while this branch was up and both touch `social.md`, `social.js` and `social.test.js`, so all three were three-way merged against the new main and the tree above was rebuilt from `cc70b824`. This tip's commit sits directly on `cc70b824` and the diff is exactly these four files. ## Risk Two ways to get this wrong. One: showing a Bot that already has the grant again — possible only if a grant names a Bot by an id this code does not read, which the per-home fallback covers; a grant that names the person must not be read as a Bot, which is the case this tip adds. Two: hiding a Bot that does not have it — the conservative branches (person-level grant, no handle on the row) keep a whole friend hidden exactly as today. The ambiguous case is a friend whose Bot handle is spelled like their home: that reads as person-level and hides the friend, the same answer the old filter gave. `grantedHomes` had no other reader, so nothing else changed shape.
Owner: Dev2
Reviewer: Reviewer2
Fixes #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" 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 #967.Rebased: opened on
a9ae2731;#974/#975and then#976merged after, so this tip is replayed onto current main90ff273d.#975(hooks.js,flybot-slots.js,plugin-runtime.md,0008) was a hand merge inplugin-runtime.md(both rows kept) plus regenerated patches;#976(which touchessocial.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#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) readsroster.botschats and room seats only, neverroster.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).remoteBotTitle/friendBotTitle, so they show@Coder (Bob): the same object wore two names in two places.plugins/social/client.js,friendRows) has the same split: it showsCoder (Bob)and inserts@coder.bob.home.hooks.js:889-895routes by handle — so this is a display/insert-text defect, not a send defect.What changed
core/src/shared/remote-label.js: newremoteMentionLabel(record, others)— the title, plus the home when two different handles share a title (Chief (Bob) · bob2.home);friendRecordalso reads the publishedroster.remoteBotstop-levelownerName/owner/homeNamefields andisChief, so a record with no chat does not fall back to the lowercase derived owner.core/src/client/flybot-registry.js: the @ menu row'stextis@${remoteMentionLabel(bot, all)}.core/src/client/user-message.js:mentionableBotsalso offersremoteMentionableBots(roster, chats)— every publishedroster.remoteBotsentry and everyroster.botsentry with aremote.handle, deduped by handle, two names each (the bare handle and the label),idonly 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: arow.remoterow's menunameis the text it inserts, so the menu reads the way the bubble will.core/src/client/text.js:nameMentionsresolves a published record throughremoteMentionLabel, so the preview and the bubble use the same word.core/src/host/hooks.js:sendUserMentionsadds{ 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__.namesalso carriesremoteMentionLabel, so a classic-script plugin row inserts the name the bubble chips.plugins/social/client.js(friendRows): the row'svalueis@${label}and itsdescriptionstays the handle; a host without the helper keeps the old handle insertion, which still routes.core/test/flybot-client.test.js(old@coder.bob.homeinsertion expectations updated; new A friend's Bot picked from the @ menu is inserted as a bare handle, so the bubble shows chief.selftestb2.home while the preview says @Chief #967 case covers the menu insert, the bubble, the preview, the two same-namedChiefs staying apart while each resolves to its own handle, a bare handle still chipping, and plain text untouched; the source guard now also coversflybot-slots.js),core/test/inbound-ui.test.js(Remote friend Bot mention from the "@" menu renders as a raw handle, not a Bot mention (bubble + sidebar preview) #71 preview expectations),core/test/at-menu.test.js(Friend's Bot rows: two name formats ('Helper · SelfB0023's Bot' vs 'SelfA0023's Chief'), two-line wrap, no preview, inset highlight, faint marker, same avatar as own Chief #384: offered once by its handle, under its one name),core/test/flybot.test.js(mentions Mentions: @handle forwards whole message externally; @-menu inserts internal text (PR #2 review) #13:@Coder (Bob)reaches the same handle once,via: 'user-mention'),plugins/social/test/social.test.js(new A friend's Bot picked from the @ menu is inserted as a bare handle, so the bubble shows chief.selftestb2.home while the preview says @Chief #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).core/client.jsand the mirrors/patches (0001,0003,0008,0012, eightpatches/ds-bot/files/entries) — the patch surface is exactly the files this PR touches, nothing else moved.docs/subsystems/core-client.md,core-host.md,plugin-runtime.md(which ownsflybot-registry.js,flybot-slots.js,types/flybot.d.tsand 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,TMPDIRon the data disk):The 8 core reds are the machine's, not this change — each reproduces on pristine
90f4bb5a, and the same eight are red on90ff273d(that merge touches nocore/file):test/composer-label.test.js,test/header-effects.test.js,test/read-page.test.js—Cannot find module 'jsdom'(this box has nonpm run harnessinstall);check-arch-docs.test.js"empty diff passes" —cannot diff against HEAD(no git here);usage-fields.test.js"fixture usage.sqlite …" —null !== 0;#896part-archive cases —until: the condition did not hold within 2 s, also red on pristine90f4bb5a(0/3 there, same three cases), and they pass when the same file runs alone under a localTMPDIR(the shared/tmpwas full:ENOSPC).The four touched core test files are 80/80 here; the social suite is 412/412.
ds-bot-patches checkneeds git, which this box does not have, so I ran its exact steps with a strict stand-in (copyds-bot/, overlaypatches/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 equalscore/(357 files, byte for byte). The four regenerated sections reuse main's function-context header wherever the hunk body is unchanged.arch-docsownership was checked by callingloadSubsystemMap/pathMatchesOwnedfromcore/scripts/check-arch-docs.mjsover 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:Plugin: in this tree, reverting only
plugins/social/client.js'sfriendRowsto the handle insertion (test kept) makes the new #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 verifyend to end (needsnpm ci+npm run harness); the suites and four scripts above cover its steps.node scripts/ds-bot-patches.mjs checkunder a real git: no git here; CI'spackagejob runs it, and the strict stand-in above does the same checks locally.Risk
@handleto@Name (owner): an old client (or a plugin) that expects a bare handle after@still routes, becausementionableBotskeeps the bare handle as one of its names and the host resolves every alias to its handle; text that was already chipped keeps chipping.roster.remoteBotsthe same label routes.Chief (Bob) · bob2.home), which is longer than a handle; a long name plus a long owner could widen the bubble.list_botsmoved; the risk is display-text shape only.