Repository navigation
host: a group settings change leaves no line in the room - #1964
Conversation
The room-side event the change also adds is user-visible, so it carries a changelog fragment, and the subsystem doc names the issue it came from.
|
Verdict: PASS at 1639881 Ran at this tip: Red first: the tip tarball is green ( Scope: exactly the 8 files the body names — Fix reads correctly: CI at this tip: 15 check-runs, 14 Metadata: no |
github-actions
Bot
added
pass
risk:T3
…lowLab#1975) Fixes ThinkFlowLab#1967 Owner: Dev-2 Reviewer: Birfy Written-by: Dev-2 Claim-Id: 29db8c1809d04d729b9f1375e39f4148 Branch: dev/2-settings-line-attribution Base: 90b880e Tip SHA: 5d69e26 Tree: 2d19f67 Verified commit `fa5df5b577a43b79f73d2995a8ce412bb2f0c434` (tree `2d19f679…`); the pushed head `5d69e268…` was rebuilt from that same tree by the fork push, so the checks describe that tree, not that commit sha. ## What the line says today `showSettings` (`core/src/host/roster.js:503` on base `90b880e7`; `:504` here, after the comment and the import) names the actor of a settings save with three steps: ```js const who = actor?.displayName ?? rt.botOf(actorId)?.name ?? 'Someone' ``` `actor` is the signed-in member session and `actorId` is the Bot asking; both come from the room route (`core/src/host/channel.js:742`), where `actor` is built only from the `x-flybot-member-session` header and `actorId` is `undefined` for the user. The web client sends that header only when `localStorage['flybot-member-session']` exists (`core/src/client/api.js:14`), and nothing in this repository ever writes that key — `grep -rn "flybot-member-session" core/src` finds that read, `member-auth.js`'s comment, the route and the tests, and no writer. So on the default single-user deployment every rename, notice edit, mode or admin/task-lead change runs with `actor` and `actorId` both `undefined`, and the line that is the whole value of ThinkFlowLab#1964 reads: ```text Someone updated the group notice ``` ## The change One fallback step in the same expression, using the host's own answer for exactly this case — `implicitRoomOwner` (`core/src/host/room-membership.js:116`), whose comment is *"Unsigned-in cloud UI has no member session. A face room still has one local owner…"* and which returns the primary human owner of the room, or the team's first member, with its `displayNameFor` name: ```js const who = actor?.displayName ?? rt.botOf(actorId)?.name ?? implicitRoomOwner(room, state)?.displayName ?? 'the user' ``` The order is the issue's: a signed-in `actor.displayName` still wins, a Bot acting through its own tool (`rt.updateRoom(room, patch, botId)`, no actor) still wins, and this change only fills the hole they leave. When neither exists — a room with no owner and no member at all — the line falls back to the host's own wording for the local person, `the user`, so it never reads as a third party. Nothing else moves: the wording of each change, the one-line-per-save rule, the "only when a value moved" rule, and the `quiet: true, notice: true` flags are untouched. The change is one import (`room-membership.js` imports nothing from `roster.js`, so there is no cycle) and one expression; nothing else in the file changes. ## What the line says in practice `implicitRoomOwner` returns the room's human owner (or the team's first member) and names it through `displayNameFor`, which reads, in order: the member session's `displayName`, a human participant of that room, `members[<id>].displayName`, and only then the member id. So the sentence the room shows is one of three: - the author has a display name in one of those places → `Alice updated the group notice`; - the author's member record carries no `displayName` — the bootstrap `owner-1` record does not (`core/src/host/member-auth.js:95-103`), and nothing writes one back — → the member id, e.g. `owner-1 updated the group notice`; - there is no member record at all → the host's own wording, `the user updated the group notice`. The `Someone` of the "today" section is gone in all three cases. What replaces it is the author's real identity in the host's records, which in a default install can be an id rather than a person's name: putting a person's name there is the profile/identity line, and this change does not reach it. ## Docs and changelog `docs/subsystems/core-host.md` records the order on the ThinkFlowLab#1962 sentence that describes the settings line: signed-in member, else the acting Bot, else the room's implicit owner (ThinkFlowLab#1967), else `the user` — so the doc says where the name comes from, not only that the line names someone. `changelog.d/1967-settings-line-attribution.fixed.md` is the user-visible entry: every local save used to read "Someone". ## Test One case in `core/test/remote-room.test.js` (`:2574`), driven through the real `updateRoom` route with no session header and no Bot id — the same setup as `a settings patch leaves one visible line in the room`: `a settings save with no session names the room owner, not Someone` — one save, one line, the text is `Alice updated the group notice`, no `Someone` anywhere in it, and the line is still the quiet `face-notice` event. Red first, on base `90b880e7` (the case run against the unmodified `roster.js`): ```text not ok 1 - a settings save with no session names the room owner, not Someone error: "the local user is named, not someone else" actual: 'Someone updated the group notice' expected: 'Alice updated the group notice' ``` The full file is `97 tests / 96 pass / 1 fail` in that state, and `97 tests / 97 pass` with the fix. ## Ran On the committed tree with a clean `git status`, once: `TZ=UTC ARCH_DOCS_BASE=90b880e788a5438577c834a969f4b612905f27e7 npm run verify` — exit 0. - core `2375` pass / `0` fail (base `90b880e7` runs `2374`; this one case is the difference) - social `541` (`540` pass, `1` skip) · tree `127` (`126`, `1`) · timeline `135` (`134`, `1`) · workflow `336` · `.github/scripts` `563` · interop `37` · live-e2e `24`, all `0` fail - arch-docs `OK (6 changed file(s))`; lint `73` core + `36` social + `9` timeline + `2` tree + `16` workflow warnings, `0` errors; interop lint `1` warning (`dev/interop/isolation/probe.mjs:130`), live-e2e lint `0` warnings, `0` errors - `npm --prefix core run build` → no diff (this is a host-side string); `node scripts/ds-bot-patches.mjs generate` → no diff after it; `check` → `ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70`, `10 patches apply cleanly in order`, `core/ equals ds-bot/ + patches/ds-bot/`; `canonical` → `generate is a no-op` - `node .github/scripts/changelog.mjs check` → `10 fragment(s) in changelog.d/ are well formed` - `cmp core/test/remote-room.test.js patches/ds-bot/files/test/remote-room.test.js` → byte-identical Not run: the smoke test (this checkout has no running FlyBot instance to drive), the CI `package` job, typecheck and actionlint. No manual browser check was made; the case drives the real route through `rt.postToRoom`. ## Not in scope (as the issue says) - Making the web client hold and send a member session — a client/identity change, not this line. - The attribution of any other event line. - `implicitRoomOwner`'s own semantics; this only reads it. ## Checkpoint <!-- flybot:checkpoint --> { "pr": 1975, "claim": "Dev-2", "issue": 1967, "summary": "The room line a group-settings save posts named its actor `Someone` whenever neither a signed-in member session nor a Bot id reached `showSettings` — the only path the web UI takes, since nothing writes the client's `flybot-member-session` key. It now falls back to the room's implicit owner, then to `the user`, after the actor and the acting Bot that still win. `core-host.md` records the order and one case drives the unsigned-in save through the real `updateRoom` route, red first on base.", "files": [ "core/src/host/roster.js", "core/test/remote-room.test.js", "docs/subsystems/core-host.md", "changelog.d/1967-settings-line-attribution.fixed.md", "patches/ds-bot/0002-flybot-api-host.patch", "patches/ds-bot/files/test/remote-room.test.js" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=90b880e788a5438577c834a969f4b612905f27e7 npm run verify; node scripts/ds-bot-patches.mjs check; node .github/scripts/changelog.mjs check", "result": "green", "at": "5d69e268958d2c8cdb1ee27c71b06a11886c8bf7" }, "next": "none — ready for review" } <!-- /flybot:checkpoint -->
Fixes #1962
Owner: Dev-1
Reviewer: Birfy
Claim-Id: 5649bf55a1de4607a0130dd98ce6755c
Written-by: Dev-1
The defect
roster.jsupdateRoomwrote the new settings and told the guests (notifyRoomShare, #1939/#1949), but the room itself showed nothing: a renamed room, a new or cleared notice, a reply-mode flip, a new admin or a new task lead stayed invisible to everyone already seated, and the only trace was the next join or chat line.The fix
core/src/host/roster.jsupdateRoomnow collects the settings a patch actually changed —notice,name,mode,admin,taskLead, each only when the key is in the patch and the stored value really differs — and after the save and the one guestjoinposts a single quiet room event through the primitive the face-to-face rooms already use:rt.postToRoom(room, { id: 'face-notice', name: who }, '<who> <sentence>', { quiet: true, notice: true })One line for the whole patch, however many fields it changes:
Alice updated the group notice, renamed the room to “X” and made Dev-1 the admin. It opens no Bot turn (quiet, no round), andwhoisactor?.displayName ?? rt.botOf(actorId)?.name ?? 'Someone', so the API path (anactor) and the tool path (tools.jspassesselfasactorId) both name someone. A seat change keeps its own joined/left line and draws no settings line:add/removeare not settings, and an admin or task lead cleared only because the member left is not drawn because the key was not in the patch. A patch that writes back the stored values says nothing, and neither does an unnamed room whose title follows its members.Tests
Red first, on the unchanged
roster.jswith only the new test:not ok 1 - a settings patch leaves one visible line in the room, and only when a value changed, withone rename is one line: []and0 !== 1. Green with the fix:core/test/remote-room.test.js:2444covers one field, several fields in one patch, every field, a write-back, a seat change and an unnamed room's automatic title.core/test/task-drive.test.js:393now reads the room's posted line count before comparing, because the three setup saves that test makes each draw one settings line; its own assertions are unchanged.Docs and fragment
docs/subsystems/core-host.md:329(the inbound-shared-rooms bullet; base:329, the same single line replaced in place — the:183Room context budget section is untouched) now says the save also leaves one quietface-noticeevent in the room. The file is shared with #1961 (4accf1e2) only by adjacency: that change edits the Room context budget section at:183, this one only the inbound-shared-rooms bullet at:329, 146 lines apart, so after the rebase no hunk here sits on #1961's text.changelog.d/1962-room-settings-line.fixed.mdis the entry.patches/ds-bot/0002-flybot-api-host.patchand the twofiles/test/…mirrors aregenerateoutput, not hand edits.Lines are in the PR tree unless marked base:
roster.jsupdateRoomsignature:364(base:364),showSettings:499, its call:495, thepostToRoomcall:503.What I ran
verifyexit 0 at the verified tree: core 2362/2362, plugins/social 541/541, plugins/tree 127/127, plugins/timeline 135/135, plugins/workflow 336/336,.github563/563, interop 37/37, live-e2e 24/24; lint 0 errors;10 patches apply cleanly in order,core/ equals ds-bot/ + patches/ds-bot/,patches/ds-bot/ is what generate writes (generate is a no-op);arch-docs: OK (8 changed file(s))against base4accf1e2.Not run: the smoke bench (
dev/bench.sh smoke) —src/host/is touched and a maintainer may want it; the CIpackagejob's pack check, typecheck and actionlint; nothing against a live instance.Verified commit
f47959df7203c0c55704adf7a1342896da15e81e(treee32af9dbd835171686676ff5c2be907d4d1cc5ae, parent4accf1e2cb40445f4dd7dd37b4a2704bb1cc6508). The pushed head is the push path's own commit built from that tree on the same parent, so it carries the same tree with a different sha.Checkpoint
{ "pr": 1964, "claim": "Dev-1", "issue": 1962, "summary": "A group settings patch now leaves one quiet line in the room, so a rename, a new or cleared notice, a reply-mode flip, a new admin or a new task lead is visible to everyone already seated instead of only to guests through the join envelope. updateRoom collects the fields the patch really changed and, after the save, posts one quiet face-notice event through rt.postToRoom naming the actor or the Bot that made the call. One line covers the whole patch, it opens no Bot turn, a seat change keeps its own joined/left line, and a write-back or an unnamed room's automatic title says nothing.", "files": [ "core/src/host/roster.js", "core/test/remote-room.test.js", "core/test/task-drive.test.js", "docs/subsystems/core-host.md", "patches/ds-bot/0002-flybot-api-host.patch", "patches/ds-bot/files/test/remote-room.test.js", "patches/ds-bot/files/test/task-drive.test.js", "changelog.d/1962-room-settings-line.fixed.md" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=4accf1e2cb40445f4dd7dd37b4a2704bb1cc6508 npm run verify", "result": "green", "at": "16398819a9260436a321fd659cfe6dbe9133f877" }, "next": "none — ready for review" }