Repository navigation
client: put the read-only Reload where the sentence is - #2510
Merged
Merged
Conversation
HugoXK
marked this pull request as draft
November 5, 2026 06:29
HugoXK
force-pushed
the
dev/1-readonly-reload
branch
from
November 5, 2026 07:56
b483b93 to
96cf84d
Compare
The host's sentence tells the reader to close the other window and reload, but only the sidebar and the refusal toast offered that entry: the Settings header, the drawer's three hint sites, the Inbox dialog and the Memory panel kept the sentence with nothing to answer it, and inside the Inbox modal the sidebar's own button cannot be reached at all. One shared helper (client/read-only.js, readOnlyLine) draws the host's own words in a role="status" line with one Reload outside it, running the same actions.reloadTeam read the sidebar runs and reporting a failure through reportError(error, 'reload'). The caller passes h rather than the module importing react, so the generated bundle does not renumber every module's react import: that alone turns this into a two-thousand-line client.js diff. Fixes ThinkFlowLab#2502
HugoXK
force-pushed
the
dev/1-readonly-reload
branch
from
November 5, 2026 08:11
96cf84d to
0318eaa
Compare
HugoXK
marked this pull request as ready for review
November 5, 2026 08:12
HugoXK
marked this pull request as draft
November 5, 2026 08:13
HugoXK
marked this pull request as ready for review
November 5, 2026 08:22
Contributor
Author
|
Enqueue request — PR #2510 at
Please enqueue. |
Collaborator
|
Verdict: PASS at 0318eaa Ran at this tip:
|
HugoXK
added a commit
to HugoXK/FlyBot
that referenced
this pull request
Nov 5, 2026
…lead's Change, and a hint that is false there (ThinkFlowLab#2513) Fixes ThinkFlowLab#2512 Owner: Dev-2 Reviewer: Birfy Written-by: FlyBot Dev-2 Claim-Id: 660434858f894844b50bfa7688ee56ae Risk tier: T1 ## What changes A read-only (locked) window's Drawer overview of a face-to-face room still drew the Room lead card's **Change**, and, for anyone the host reports as not able to pick, the sentence `Only the person who created this room can change it.`. Neither belongs there: - `social/face-lead` is registered without `{ read: true }` (`plugins/social/src/social.js:4684`), so the host's channel layer answers it `bot/read-only`. Pressing **Change** is therefore a live-looking control whose only possible result is a refusal. - the "only the person who created this room" sentence is false for the creator in a locked window: the creator *can* change the lead, just not from this window, so the line explains the wrong reason. - `FaceLead` (`core/src/client/group-settings.js`) decided with `lead.canPick === true && typeof pick === 'function'`, and the host reports `canPick: true` for the room's creator regardless of the window (`plugins/social/src/face-rooms.js:519`, `core/src/host/face-lead.js:29` passes it through). The card already receives the roster, so it can tell a locked window from a writable one itself; nothing has to be threaded down from `details-drawer.js`. The fix is one guard in the card: `FaceLead` reads `lockedWindow(roster)` (the same helper every other read-only surface uses, `core/src/client/schedules.js:141`), folds it into `canPick` (`!locked && lead.canPick === true && typeof pick === 'function'`), and skips the only-the-person hint when `locked`. A locked card keeps the lead's name, its `Bot (person)` label, its *Lead* tag and the sentence about what the lead does; it draws no **Change**, no picker, no **Make lead**, no **Use the default lead** and no refusal hint. A writable window is byte-identical to the base: **Change** → picker → one press sends exactly one `social/face-lead`, and a non-creator still reads the only-the-person sentence. Rebased onto `523a384298f9f7989147ca25a2fd2b219ffd1d46` (`ThinkFlowLab#2510`, merged after this branch was first pushed). The rebase again merged `patches/ds-bot/0008-client-bundle.patch` and `patches/ds-bot/0012-room-membership.patch` without a textual conflict; `build-client` + `ds-bot-patches generate` then rewrote the same bytes, so `git status --porcelain` is empty and no patch file was hand-edited. Scope: `core/src/client/group-settings.js`, one new test file, one doc sentence, one changelog fragment, and the regenerated `core/client.js` plus the `patches/ds-bot/` copies the repo's build requires. `details-drawer.js`, the host's `social/face-lead` registration, `face-rooms.js` and every other read-only surface are untouched. ## Acceptance In a read-only window, opening a face room's Drawer overview: - the Room lead card is still drawn and still names the lead (`Room lead`, `Chief (Ann)`) with the sentence about what the lead does; - the card offers **no** button at all — the case asserts `buttonLabels(card)` is `[]`, not merely that a drawn button is disabled — so **Change**, **Make lead** and **Use the default lead** are all absent; - the `Only the person who created this room can change it.` hint is not drawn (it is false for the creator here), for the creator and for a non-creator alike; - drawing the card reaches the host with nothing (`hostCalls` is empty), so nothing is attempted and no `bot/read-only` can come back. The neighbouring writable control, in the same test file, is unchanged: **Change** is drawn, pressing it lists the other seat with **Make lead**, one press is exactly `[['social/face-lead', { roomId: 'face', bot: '@writer.bob.home' }]]`, someone who is not the creator still sees no **Change** and does read the only-the-person sentence. ## Red first On the clean base `523a384298f9f7989147ca25a2fd2b219ffd1d46` (the rebased base), in a throwaway detached worktree at that commit with the base source untouched and only this branch's test file added, `node --import ./test/harness-resolve.mjs --test test/details-face-lead-readonly.test.js` under `TZ=UTC`: ``` not ok 1 - a read-only window draws the room lead as text: no Change, no picker, no refusal hint the card itself offers no control + [ 'Change' ] − [] not ok 2 - a read-only window draws no Change for the creator and no refusal hint for anyone else the creator cannot change the lead in a read-only window either + { type: 'button', props: { className: 'bt-mini', children: [ 'Change' ], onClick: [Function: onClick] } } − undefined ok 3 - a writable window keeps Change, the picker and exactly one social/face-lead write # tests 3 # pass 1 # fail 2 ``` These are the same two `not ok` lines the pre-rebase run produced on `e5fc62dea0c067a308fbf8f122d691a35e455fb1`: `3` tests, `1` pass, `2` fail. With the fix in place the same file is 3/3 green, and the writable positive control is unchanged from the base. ## What I ran at `b8e4b8fa66f7b00afacc8a5c9480c140d5c479b6` (`TZ=UTC`, clean tree, tree `b50e5031`) - `node core/scripts/build-client.mjs` then `node scripts/ds-bot-patches.mjs generate` on the committed tree: both wrote the same bytes back (`git status --porcelain` empty afterwards), and `npm run verify`'s own `canonical` gate reports `ok patches/ds-bot/ is what generate writes (generate is a no-op)`. - `node scripts/ds-bot-patches.mjs check` (inside `verify`) — three `ok` lines: `ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)`, `ok 10 patches apply cleanly in order`, `ok core/ equals ds-bot/ + patches/ds-bot/`; `canonical` — `ok patches/ds-bot/ is what generate writes (generate is a no-op)`; a second `generate` + `build-client` run afterwards leaves `git status --porcelain` empty (idempotent). - `TZ=UTC ARCH_DOCS_BASE=523a384298f9f7989147ca25a2fd2b219ffd1d46 npm run verify` — **exit 0**. Segment counts, each `# fail 0` and `# cancelled 0`: core **2927 success + 0 cancelled**, plugins/social **577 + 0**, plugins/tree **137 + 0**, plugins/timeline **141 + 0**, plugins/workflow **440 + 0**, `.github/scripts` + `policy` **671 + 0**, `test:interop` **37 + 0**, `test:live-e2e` **26 + 0** — **4956 success + 0 cancelled, 0 failed**. `arch-docs: OK (8 changed file(s))`; every lint stage exit 0 with **0 errors** (warning counts 81/36/9/2/16, unchanged from the base commit); `build-client.mjs --check` exit 0 in the same run. - The 13 neighbouring read-only suites (`face-lead`, `settings-readonly-pages`, `settings-readonly-head`, `details-bot-settings-readonly`, `details-face-lead-readonly`, `details-group-overview-readonly`, `details-group-settings-readonly`, `details-overview-readonly`, `room-approvals-readonly`, `group-create-readonly`, `composer-send-readonly`, `readonly-window-reads`, `approvals-readonly`): 52/52, 0 failed. - `git status --porcelain` — empty on the committed tree. ## What I did not run - The host smoke test and a live service on a bench port — this change is client-only (`core/src/client/**`), no host code. - `ThinkFlowLab#2509` / `ThinkFlowLab#2510`'s surfaces and any other read-only card — out of scope for this issue and untouched here. ## Files 8 files, +400/−11 (`git diff --numstat 523a384...b8e4b8f`), tree `b50e5031`: - `core/src/client/group-settings.js` (+8/−2) — `FaceLead` reads `lockedWindow(roster)`, folds it into `canPick`, and skips the only-the-person hint when locked; hand-written. - `core/test/details-face-lead-readonly.test.js` (+187, new) — the two read-only cases plus the writable positive control; hand-written. - `docs/subsystems/core-client.md` (+1/−1) — the face-room lead paragraph gains the locked-window sentence and points at the test. - `changelog.d/2512-face-lead-readonly.fixed.md` (+1, new). - `core/client.js` (+3/−2), `patches/ds-bot/0008-client-bundle.patch` (+4/−3), `patches/ds-bot/0012-room-membership.patch` (+9/−3), `patches/ds-bot/files/test/details-face-lead-readonly.test.js` (+187, new) — generated by build + `generate`, never hand-edited. <!-- flybot:checkpoint --> ```json { "pr": 2513, "claim": "Dev-2", "issue": 2512, "summary": "A read-only window's face-room overview no longer draws the Room lead card's Change: social/face-lead is a write the host answers bot/read-only, and the host still reports canPick true for the room's creator. FaceLead now reads lockedWindow(roster) itself, so canPick is false even for the creator and the 'only the person who created this room' hint is skipped as false there. A locked card keeps the lead's name and its sentence only; a writable window is unchanged, one press still sends exactly one social/face-lead.", "files": [ "core/src/client/group-settings.js", "core/test/details-face-lead-readonly.test.js", "docs/subsystems/core-client.md", "changelog.d/2512-face-lead-readonly.fixed.md" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=523a3842 npm run verify (plus build-client and ds-bot-patches check/generate)", "result": "green", "at": "b8e4b8fa66f7b00afacc8a5c9480c140d5c479b6" }, "next": "none — ready for review" } ``` <!-- /flybot:checkpoint -->
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.
Fixes #2502
Owner: Dev-1
Reviewer: Birfy
Written-by: FlyBot Dev-1
Claim-Id: 41446521c1724173bfcc99f75889b209
Risk tier: T1
Base: b38ec01
What changed
In a read-only window the room tells the reader to reload to pick up new permissions, and the sidebar carries that Reload — but the Settings header, the details drawer, the Inbox and the Memory panel only print the read-only sentence. A reader looking at one of those four faces has to close and reopen the window.
After: one helper,
core/src/client/read-only.js, renders the sentence and its Reload at every place the sentence appears:settings.jsheader, on all 12 settings pagesdetails-drawer.js— group overview, group settings, Bot overview, Bot settingsinbox.jsmemory-panel.jsrole="status"wraps only the sentence and the button sits outside it, so a screen reader does not re-announce the whole line when the sentence changes. A click calls onlyactions.reloadTeam(); a rejection goes toactions.reportError(error, 'reload'). No write path is touched. A writable window draws neither the sentence nor the button.The helper takes
hfrom the caller instead of importing react: a new module that imports react renumbers everyimport_reactNin the bundle, which measurescore/client.js2274/2248 for the same source instead of 39/14. The module header records that reason. Because the sentence and the button come from one place, this adds no further hand-written copy of the read-only reason.Hand-written files:
core/src/client/read-only.js(new),core/src/client/settings.js,core/src/client/details-drawer.js,core/src/client/inbox.js,core/src/client/memory-panel.js,core/src/client/styles.js,docs/subsystems/core-client.md,changelog.d/2502-readonly-reload.added.md,core/test/readonly-reload-lines.test.js(new),core/test/details-bot-settings-readonly.test.js.core/client.jsandpatches/ds-bot/*are regenerated, never hand-edited:node scripts/build-client.mjs, thennode scripts/ds-bot-patches.mjs generate.Verification at the tip
Tip SHA: 0318eaa (tree 21657b43e3cf8c0e0c194473caac84a018dd2fa1), parent b38ec01 (live main), rebuilt from local a8969814… — same tree, the push path re-commits.
Ran:
b38ec011tree with onlycore/test/readonly-reload-lines.test.jsadded:# tests 7 / # pass 1 / # fail 6. The six red are the Settings header page by page, the four drawer faces, Inbox, Memory, one shared sentence with the button outsiderole="status", and a click that reaches onlyreloadTeamwith the failure going toreportError. The one green is a writable or blank window drawing no control.TZ=UTC npm run verify→ exit 0: core 2924/2924, social 577, tree 137, timeline 141, workflow 440,.github+ policy 671, interop 37, live-e2e 26; oxlint andarch-docs: OK (20 changed file(s)),build-client --check,ds-bot-patches checkandcanonicalall green.git diff --numstat b38ec011...HEAD:core/client.js39/14,patches/ds-bot/0008-client-bundle.patch106/80; 20 files, +750/−124.b38ec011: the rebase conflicted only inpatches/ds-bot/0008-client-bundle.patch(one hunk;core/client.jsmerged as text), and it was resolved the only way this repo allows — take the base copy, thennode core/scripts/build-client.mjsfollowed bynode scripts/ds-bot-patches.mjs generaterewrote it from the new tree.generatea second time leavesgit statusempty,build-client --checkexits 0 andcheck/canonicalare green, so the committed bundle and patches are exactly what the generators produce from this source.Not run (and why): the two-Bot live smoke, which needs a running instance; the window behaviour here is covered by the component tests above.
This is a draft, and later commits on the branch add their own measurements.
Checkpoint
{"pr":2510,"claim":"Dev-1","issue":2502,"summary":"In a read-only window the Settings header, the details drawer, the Inbox and the Memory panel printed the read-only sentence but offered no way to reload, so the reader had to close and reopen the window. One helper, core/src/client/read-only.js, now renders the sentence and its Reload at all six call sites and keeps the button outside role=\"status\". A click calls only actions.reloadTeam() and a rejection goes to actions.reportError(error, 'reload'); a writable window draws neither. core/client.js and the patches are regenerated from that source, not hand-edited.","files":["core/src/client/read-only.js","core/src/client/settings.js","core/src/client/details-drawer.js","core/src/client/inbox.js","core/src/client/memory-panel.js","core/src/client/styles.js","core/test/readonly-reload-lines.test.js","core/test/details-bot-settings-readonly.test.js","docs/subsystems/core-client.md","changelog.d/2502-readonly-reload.added.md","core/client.js","patches/ds-bot/0008-client-bundle.patch"],"checks":{"ran":"failing test first on clean b38ec011 (7 tests, 6 red); TZ=UTC ARCH_DOCS_BASE=b38ec011 npm run verify exit 0, core 2924/2924; build-client --check; ds-bot-patches check + canonical","result":"green","at":"0318eaac689ed16beb6dee23eec875b6c627e40e"},"next":"none — lead holds this draft until the core client lane reaches #2502; no review requested yet"}