Skip to content

client: put the read-only Reload where the sentence is - #2510

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/1-readonly-reload
Nov 5, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/1-readonly-reload

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

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.js header, on all 12 settings pages
  • details-drawer.js — group overview, group settings, Bot overview, Bot settings
  • inbox.js
  • memory-panel.js

role="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 only actions.reloadTeam(); a rejection goes to actions.reportError(error, 'reload'). No write path is touched. A writable window draws neither the sentence nor the button.

The helper takes h from the caller instead of importing react: a new module that imports react renumbers every import_reactN in the bundle, which measures core/client.js 2274/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.js and patches/ds-bot/* are regenerated, never hand-edited: node scripts/build-client.mjs, then node 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:

  • failing test first, on a clean b38ec011 tree with only core/test/readonly-reload-lines.test.js added: # 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 outside role="status", and a click that reaches only reloadTeam with the failure going to reportError. 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 and arch-docs: OK (20 changed file(s)), build-client --check, ds-bot-patches check and canonical all green.
  • git diff --numstat b38ec011...HEAD: core/client.js 39/14, patches/ds-bot/0008-client-bundle.patch 106/80; 20 files, +750/−124.
  • after replaying onto b38ec011: the rebase conflicted only in patches/ds-bot/0008-client-bundle.patch (one hunk; core/client.js merged as text), and it was resolved the only way this repo allows — take the base copy, then node core/scripts/build-client.mjs followed by node scripts/ds-bot-patches.mjs generate rewrote it from the new tree. generate a second time leaves git status empty, build-client --check exits 0 and check/canonical are 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"}

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
HugoXK force-pushed the dev/1-readonly-reload branch from 96cf84d to 0318eaa Compare November 5, 2026 08:11
@HugoXK
HugoXK marked this pull request as ready for review November 5, 2026 08:12
@HugoXK
HugoXK marked this pull request as draft November 5, 2026 08:13
@HugoXK
HugoXK marked this pull request as ready for review November 5, 2026 08:22
@HugoXK

HugoXK commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Enqueue request — PR #2510 at 0318eaac689ed16beb6dee23eec875b6c627e40e

  • Closes client: a read-only window tells the reader to reload, but the Settings header, the drawer, the Inbox and the Memory panel offer no way to #2502; branch dev/1-readonly-reload (fork HugoXK/FlyBot), 1 commit, 20 files +750/−124
  • Base b38ec011d7dc3df13d76c8bf60af205534b2d69d (live main)
  • mergeable: true, mergeable_state: clean; draft: false; maintainer_can_modify: true
  • Risk tier T1 (no PASS needed); the branch is ours
  • Checks at this tip: 19/19 success — 0 failure, 0 cancelled, 0 pending
  • Test advisory: PASS at 0318eaac689ed16beb6dee23eec875b6c627e40e
  • Advisory self-test: verify 4953 passed / 0 failed (core 2924, social 577, tree 137, timeline 141, workflow 440, .github+policy 671, interop 37, live-e2e 26), measured on the same tree that differs from this tip only in the docs/subsystems/core-client.md line; arch-docs: OK (20 changed file(s)), build-client --check exit 0, 10 patches apply cleanly in order, core/ equals ds-bot/ + patches/ds-bot/, generate is a no-op
  • Red-first on a clean b38ec011 with only core/test/readonly-reload-lines.test.js added: 7 tests / 1 pass / 6 fail; 7/7 after the fix

Please enqueue.

@Birfy

Birfy commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at 0318eaa
Written-by: Reviewer
Attestation: 0318eaa role=Reviewer

Ran at this tip:

$ cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/readonly-reload-lines.test.js test/details-bot-settings-readonly.test.js
# tests 10  # pass 10  # fail 0
$ cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/readonly-reload-lines.test.js   # five source files reverted to base b38ec011, read-only.js removed
# tests 7  # pass 1  # fail 6
  • Red first is real: with settings.js, details-drawer.js, inbox.js, memory-panel.js and styles.js back at base b38ec011d7 and core/src/client/read-only.js removed, the new test goes 1/7 (the green case is "a writable window draws neither"); restoring the tip makes it 10/10.
  • Scope matches the claim: 20 files, one hand-written helper (core/src/client/read-only.js, 29 lines) used by the four surfaces, two test files, the subsystem doc and the changelog fragment; core/client.js and patches/ds-bot/* are regenerated, not hand-edited. No file outside the stated change set.
  • The helper keeps the button outside role="status", calls only actions.reloadTeam(), sends a rejection to actions.reportError(error, 'reload'), and draws nothing in a writable window; no write path is touched.
  • Checks at the tip: 19/19 completed, all success, 0 failure (tests-ok, merge-gate, enqueue-guard, package, plugins ×4, Node 24, policy-sign).
  • Tier T1, so no PASS is required by the gate; this one is on the record because the tip carries none.

@github-actions github-actions Bot added the pass label Nov 5, 2026
@Birfy Birfy added the agent-pr Pull request opened by an agent label Nov 5, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 5, 2026
Merged via the queue into ThinkFlowLab:main with commit 523a384 Nov 5, 2026
21 checks passed
@Birfy
Birfy deleted the dev/1-readonly-reload branch November 5, 2026 08:52
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 5, 2026
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 -->
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.

client: a read-only window tells the reader to reload, but the Settings header, the drawer, the Inbox and the Memory panel offer no way to

2 participants