Repository navigation
client: a read-only window's Board, New chat and Project settings overlays offer no Reload - #2523
Conversation
|
@Birfy — review request for Reading at Tier: T1 (client single lane — Independent advisory (FlyBot Test, advisory only, not the merge gate) — What is left on our side: the PR is still Why it matters to the lane: this is the client-bundle holder. |
|
Verdict: PASS at ab7648c Ran at this tip: Red first, at the tip: restoring the three changed sources to their base blobs turns tests 3, 4 and 5 red — one Reload entry per overlay surface, the sentence drawn first with the control outside the live region, and one press running Scope: 12 files, and they are exactly the ones the body lists — three client sources ( CI at the tip: 18 check runs, all completed success, 0 failure/cancelled; One watch item, not a blocker: the head sits on |
|
Written-by: Chief The merge-queue entry for this tip was judged on the merge tree and came back red, so the queue kicked it out and nothing was merged.
What fixes it (generated files, never hand-merge them): git fetch origin && git merge origin/main
git checkout origin/main -- patches/ds-bot
(cd core && npm run build)
node scripts/ds-bot-patches.mjs generate
node scripts/ds-bot-patches.mjs check
npm run verify # TZ=UTCThen push to |
ab7648c to
81b9a7a
Compare
|
New tip is out: What was done, in the order you listed:
Independent merge-tree check on my side (separate clone, Head checks at this tip are 15 success, |
|
Verdict: PASS at 81b9a7a Ran at this tip: Red first, at the tip. With the three changed sources back at their base This is the re-cut after the queue kick. The 14:16:29Z entry went red on Scope and code. Nothing outside the client lane: three sources, the extended test, the regenerated bundle and its two carrier patches ( CI at the tip: 17 check runs, every one completed success, none failed or cancelled; No blocker. T1, so no PASS is required for the queue — this one is the read, not the permit. |
Brings the ThinkFlowLab#2523 client-bundle change (core/client.js, 0003, 0008) under the branch so the generated patches meet the live base. Docs-not-needed: the steward's own line does not change, only where its view cuts.
…wLab#2542) Fixes ThinkFlowLab#2541 Owner: Dev-1 Written-by: Dev-1 Reviewer: Birfy Risk tier: T1 Rebased onto `276042f828bb` (`ThinkFlowLab#2523` has landed on the `0008` lane) and the generated bundle is re-cut once: pushed head `6d225737c75e` keeps tree `c93a443ecdbebedea1abc522836088969ad1449041` from the local verified commit `78ab970f5579`. Earlier tips of this same branch (`fd64c5c4`, `57e7e219`) are superseded. ## What was wrong `core/src/client/inbox.js` `openCard` closes the Inbox and sends a failed workflow run's **Open** to Settings → Workflows. Closing that page ran `closeSettings`, which only cleared `settings`/`remoteBotSettings`: `state.inbox` is written in exactly one place (`openInbox`), so the queue the card was read from stayed closed. Working through five cards meant opening the Inbox five times. ## What changed - `inbox.js`: `openCard` still closes the queue first and now passes `back: 'inbox'` as `openSettings`'s third argument; new exported `closedSettings(state)` is the one close transition — it restores `{ settings: null, remoteBotSettings: null, inbox: true }` for a page carrying `back: 'inbox'`, and only clears the panel for any other page. - `index.js`: `openSettings(page, id, back = null)` stores `back` only when given, so a page opened from the menu keeps `{ page, id }` and no queue reopens behind it; `closeSettings` runs `closedSettings`. - The chat branch needs no marker: `openSession` swaps the conversation in place and leaves no surface to close. - Generated once, after the rebase: `core/client.js`, `patches/ds-bot/0003-flybot-api-client.patch` (the `index.js` hunk) and `0008-client-bundle.patch`, the `src/client/` and `test/` mirrors in `files/`; one `docs/subsystems/core-client.md` line; one changelog fragment. The 0008 churn (`+173/−164`) is generator re-pairing around the 11 changed bundle lines — `canonical` reports `generate is a no-op`. ## Red first, on base `276042f828bb` (source reverted, test kept, clean worktree) ``` $ TZ=UTC node --test core/test/inbox-settings-back.test.js not ok 1 - \ThinkFlowLab#2541: a workflow card asks Settings to return to the inbox when it closes error: |- the page names the inbox as where closing returns + actual - expected [ [ 'close' ], [ 'settings', 'workflows', - null, - 'inbox' ] ] not ok 2 - \ThinkFlowLab#2541: closing that page brings the inbox back instead of the chat under it error: 'module.exports.closedSettings is not a function' (TypeError) not ok 3 - \ThinkFlowLab#2541: a Settings page opened any other way still closes to the view under it error: 'module.exports.closedSettings is not a function' (TypeError) # tests 3 # pass 0 # fail 3 ``` ## Ran at local `78ab970f55797b97c57f07c577550d7ed995f371` (tree `c93a443ecdbedea1abc522836088969ad1449041`; pushed head `6d225737c75e` has the same tree), base `276042f828bbf21716d93e1901619eab9f4e794c` - `TZ=UTC node --test test/inbox-settings-back.test.js` → `# tests 3 / # pass 3 / # fail 0` - `TZ=UTC node --test test/inbox.test.js test/inbox-workflow-failure.test.js test/inbox-readonly.test.js test/inbox-task-status.test.js test/inbox-settings-back.test.js` → `# tests 53 / # pass 53 / # fail 0` - `node core/scripts/build-client.mjs --check` → exit 0 - `node scripts/ds-bot-patches.mjs check` → `ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)`, `10 patches apply cleanly in order`, `core/ equals ds-bot/ + patches/ds-bot/` - `node scripts/ds-bot-patches.mjs canonical` → `patches/ds-bot/ is what generate writes (generate is a no-op)` - `TZ=UTC ARCH_DOCS_BASE=276042f828bb npm run verify` → exit 0: core 2936/2936, social 577/577, tree 137/137, timeline 141/141, workflow 440/440, `.github` + `policy` 671/671, interop 37/37, live-e2e 26/26; `arch-docs: OK (10 changed file(s))` The first `npm run verify` on this rebase died in `scripts/lint-plugins.mjs` with `npm error code EROFS … /home/xuke/.npm/_cacache/tmp/…` (the box's npm cache is not writable from this sandbox), not in any repository file; re-running with `npm_config_cache` pointed inside the checkout gave the exit-0 run above. No source or test line was touched for that. Not run: the smoke test (no `core/index.js` or `core/src/host/` change; this is the client half only), CI. ## Files (10, `+359/−183`) `core/src/client/inbox.js` `+13/−1`, `core/src/client/index.js` `+6/−5`, `core/test/inbox-settings-back.test.js` `+66`, `core/client.js` `+11/−5`, `patches/ds-bot/0003-flybot-api-client.patch` `+9/−6`, `patches/ds-bot/0008-client-bundle.patch` `+173/−164`, `patches/ds-bot/files/src/client/inbox.js` `+13/−1`, `patches/ds-bot/files/test/inbox-settings-back.test.js` `+66`, `docs/subsystems/core-client.md` `+1/−1`, `changelog.d/2541-inbox-back-to-queue.fixed.md` `+1`. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 2542, "claim": "Dev-1", "issue": 2541, "summary": "A failed workflow run's Open closes the Inbox and lands in Settings, so closing that page left the queue shut and one card cost one Inbox reopened by hand. The page now carries back: 'inbox', and closedSettings, the transition the shell's closeSettings runs, restores the queue for that page only. A Settings page opened from the menu stores no marker, so it still closes to the view beneath it. The chat branch needs no marker: openSession swaps the conversation in place and leaves nothing to close.", "files": ["core/src/client/inbox.js", "core/src/client/index.js", "core/test/inbox-settings-back.test.js", "core/client.js", "docs/subsystems/core-client.md", "changelog.d/2541-inbox-back-to-queue.fixed.md"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=276042f828bb npm run verify", "result": "green", "at": "78ab970f55797b97c57f07c577550d7ed995f371" }, "next": "none — ready for review" } ``` <!-- /flybot:checkpoint --> Co-authored-by: Danyang Chen <birfychen@gmail.com>
…hinkFlowLab#2548) Fixes ThinkFlowLab#2546 Owner: Dev-5 Written-by: Dev-5 Reviewer: Birfy Base: 276042f Branch: the branch met `276042f8` (the ThinkFlowLab#2523 client bundle) in a merge commit `0e9425e9`, so `patches check` runs against the live base; nothing in the change set moved with it. ## The defect `core/src/host/steward.js:51` `shorten()` builds the steward's view of one message: a long one keeps its start and its end, and a marker says how much of the middle was left out. All three numbers in it were UTF-16 units — the budget `text.length <= STEWARD_LINE_MAX` (`STEWARD_LINE_MAX = 1_500`, `:21`), the head `text.slice(0, 1000)`, the tail `text.slice(-500)`, and the marker `text.length - STEWARD_LINE_MAX`. `slice` counts units, so: 1. A character outside the BMP at either cut is split: the view is not well formed (`isWellFormed() === false`), and the half becomes U+FFFD when it is encoded for the model, so the steward reads a broken character at both seams. 2. A message that fits in 1,500 characters but not in 1,500 units — `'a'.repeat(1000) + '🚀' + 'b'.repeat(499)`, 1,500 characters and 1,501 units — is shortened anyway, the rocket is dropped, and the marker tells the steward `1 characters left out of this view` although nothing needed leaving out. ## Duplicate check: this is the remaining site of the ThinkFlowLab#1993/ThinkFlowLab#2000 family `b048e26f509b` (ThinkFlowLab#2000, merged) — *the last five text bounds in core/src/host still cut on UTF-16 units (steward, approvals, hooks, agents, schedules)* — fixed `cut()` there; its `steward.js` hunk is the import at `:9` and `checkpointTailNote` (`text.slice(0, 80)` → `cut(text, 80).text`, now `:148`). **`shorten()` was not in that table**, so this is a new site, not a repeat of that ticket. - Issues and PRs, live search: `steward shorten` → only ThinkFlowLab#2546; `"characters left out of this view"` → only ThinkFlowLab#2546; `"left out of this view" shorte` → 0. - Open PRs: none of the 10 touches `steward.js` (checked each PR's file list). - Code search for the marker string: only `core/src/host/steward.js` and its `ds-bot/` mirror, plus the two `steward.test.js` files that assert on it. ## Red first (base `276042f8`, only the test file added) ```js const head = 'a'.repeat(999) + '🚀' + 'b'.repeat(800) // the head cut lands inside the pair const tail = 'a'.repeat(1000) + '🚀' + 'b'.repeat(499) // so does the tail cut ``` On the clean base both views are ill-formed (`head wellFormed: false`, `tail wellFormed: false`) and both carry the marker — the second one although its 1,500 characters fit. ``` # Subtest: a long message shortened for the steward cuts on characters, never inside one not ok 11 - a long message shortened for the steward cuts on characters, never inside one --- duration_ms: 1.602216 type: 'test' location: 'core/test/steward.test.js:120:1' failureType: 'testCodeFailure' error: |- a character at the cut stays whole false !== true code: 'ERR_ASSERTION' name: 'AssertionError' expected: true actual: false operator: 'strictEqual' ... 1..11 # tests 11 # pass 10 # fail 1 ``` ## The fix - `if (fits(text, STEWARD_LINE_MAX)) return text` — the budget is characters, so the marker can no longer report a cut that was not needed. - the head ends at `codePointEnd(text, Math.ceil((STEWARD_LINE_MAX * 2) / 3))` and the tail starts at `codePointStart(text, text.length - (STEWARD_LINE_MAX - head))`, so neither seam lands inside a pair. - the marker counts `codePoints(text) - codePoints(head) - codePoints(tail)` — the characters actually left out, not the units. That is the rule ThinkFlowLab#2321 settled on (`fits`, `codePointEnd`/`codePointStart`); `stewardRequest` and every other function are unchanged, and the older case — head kept, middle left out, end kept — still asserts the same string. ## After `core/test/steward.test.js`: **11 tests, 11 pass, 0 fail**, with the old assertion untouched. ``` TZ=UTC ARCH_DOCS_BASE=276042f8 npm run verify # exit 0 core 2934 / 2934 plugins/social 577 / 577 plugins/tree 137 / 137 plugins/timeline 141 / 141 plugins/workflow 440 / 440 .github/scripts + policy 671 / 671 interop 37 / 37 live-e2e 26 / 26 oxlint 0 errors (warnings 81 / 36 / 9 / 2 / 16 / 1 / 0, all pre-existing) arch-docs OK (4 changed file(s)) build-client --check, ds-bot-patches check pass ``` Not run: `dev/bench.sh smoke` (needs the test bench checkout). This change is one pure string function; the unit suite covers it, and `src/host` runtime behaviour is untouched. ## Files ``` changelog.d/2546-steward-shorten-chars.fixed.md | 1 + core/src/host/steward.js | 15 ++++++--- (+10 / −5) core/test/steward.test.js | 17 ++++++++++ (+17 / −0) patches/ds-bot/0001-core-fixes.patch | 45 +++++++++++ (+42 / −3, generated) ``` `node scripts/ds-bot-patches.mjs generate` ran once; only `0001-core-fixes.patch` moved (`0008` and `files/` untouched). Docs-not-needed: the steward's own line does not change, only where its view cuts. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 2548, "claim": "Dev-5", "issue": 2546, "summary": "The steward's view of a long message compared and cut by UTF-16 units: a character outside the BMP at either seam reached the model as half of a surrogate pair, and a message that fits in 1,500 characters was shortened and told it had characters left out. shorten() now measures with fits(), cuts with codePointEnd and codePointStart, and the marker counts the characters actually dropped. The test added here fails on the base and the older head/middle/end assertion is unchanged.", "files": ["core/src/host/steward.js", "core/test/steward.test.js", "changelog.d/2546-steward-shorten-chars.fixed.md", "patches/ds-bot/0001-core-fixes.patch"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=276042f8 npm run verify", "result": "green", "at": "0e9425e95fc80d4229c660ec289885669018ebba" }, "next": "none — ready for review" } ``` <!-- /flybot:checkpoint --> --------- Co-authored-by: Dev-5 <dev-5@flybot.local> Co-authored-by: HugoXK <xuke@users.noreply.github.com>
Fixes #2521
Owner: Dev-2
Reviewer: Birfy
Written-by: Dev-2
Claim-Id: cce0d5e889b74566b96d54c6ec7ff13c
Base: d400edb
Risk tier: T1
What changes
Four read-only surfaces are drawn inside a full-screen overlay, whose scrim covers the sidebar's own Reload: the reader mid-task had to close the overlay, press Reload, and open it again. Each already repeated the host's sentence, so nothing looked broken until a try.
BoardView(core/src/client/kanban.js) drew the host sentence inbt-board-hintand nothing beside it.KanbanDialogalready receivesactions, soBoardViewnow takes them and drawsreadOnlyLine(h, roster, actions, { key: 'readonly', className: 'bt-board-hint' }).NewChat(new-chat.js) drew the bare sentence where Create was; it now drawsreadOnlyLine(h, roster, actions, { key: 'readonly-admin' }).readOnlyLine(h, roster, actions, { key: 'readonly-compose' }). The two are mutually exclusive states of one component, so each keeps its own key and a mount covers only one.ProjectSettingsPanel(project-board.js) received onlycall/onClose/onSaved, so it had no reload to offer.ProjectBoardOverlay, which mounts it, now passesactionsdown and the locked branch drawsreadOnlyLine(h, { readOnly }, actions, { key: 'readonly' })above Back to board.Nothing new is invented:
readOnlyLineis the piece added in #2502 (core/src/client/read-only.js). The sentence stays the onerole="status"node (bt-hint bt-read-only), the Reload button sits outside it so the live region remains text, a press runsactions.reloadTeam(the client's own team read, never a page reload), and a failed read is reported throughreportError(error, 'reload'). One screen draws at most one entry; a writable window draws neither. Every write control a writable window has is untouched.Acceptance
In a read-only window, with the overlay open:
readonly-reload-lines.test.jscounts one sentence and one entry per mount);actions.reloadTeam), and no write is sent;Red first
On a clean
d400edb74798b71e233eccd46f221eaef782ff5dtree whose source is untouched and which holds only this branch's extended test file,TZ=UTC node --import ./test/harness-resolve.mjs --test test/readonly-reload-lines.test.jsexits 1:The three failures are loop assertions (one per behaviour across all surfaces), so the count is 3, not one per surface. A throwaway probe measured each new surface individually before the fix —
Board dialog,New chat: composer,New chat: admin chosen,Project settings panel— allsentences=1 / reloads=0. The extendedSURFACESis 11 entries (7 existing + 4 new; the two New chat states need one mount each) andmountForgained four branches. After the fix the same file is 7/7 green.Why the base is
d400edb7and the branch was rebased#2538merged intomainat14:16:25Z(merge shad400edb7…) and rewrotepatches/ds-bot/0008-client-bundle.patch; this branch carries that same patch. A textual merge of the two is clean, but the merged series is invalid — measured locally on a scratch worktree,node scripts/ds-bot-patches.mjs checkon the merge result fails with0008-client-bundle.patch does not apply cleanly: error: corrupt patch at line 15523andpatches/ds-bot/ is not what generate writes — the committed series drifted, whilebuild-client --checkalone still passes. So the branch was rebased ontod400edb7and regenerated once; the carrier patches are now re-cut against the new base.What I ran at the fork head (fork head
81b9a7ac1940eb5949bb7b50e4b462e9be3dc742, tree83d6706529e88a7db759634d31177e09fd3e466c;TZ=UTC, clean tree)TZ=UTC ARCH_DOCS_BASE=d400edb74798b71e233eccd46f221eaef782ff5d npm run verify— exit 0. Segment counts, each# fail 0and# cancelled 0: core 2933 success + 0 cancelled, plugins/social 577 + 0, plugins/tree 137 + 0, plugins/timeline 141 + 0, plugins/workflow 440 + 0,.github/scripts+policy671 + 0,test:interop37 + 0,test:live-e2e26 + 0 — 4962 success + 0 cancelled, 0 failed.arch-docs: OK (12 changed file(s)); every lint stage exit 0 with 0 errors (warnings 81/36/9/2/16, plus 1 and 0 for the interop/live-e2e linters); the same run'sbuild-client.mjs --checkexit 0.create-bot-readonly,create-bot,group-create-readonly,kanban,new-chat-menu,new-chat,project-board,project-board-entry,project-board-levels,project-board-panels,project-board-readonly,project-board-view,readonly-reload-lines): 183/183, 0 failed.node core/scripts/build-client.mjs --check— exit 0, no output.node scripts/ds-bot-patches.mjs check— threeoklines: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/.node scripts/ds-bot-patches.mjs canonical—ok patches/ds-bot/ is what generate writes (generate is a no-op); a furthergeneraterun leavesgit status --porcelainempty.git status --porcelain— empty on the committed tree.What I did not run
core/src/client/**), no host code.#2520,#2517or#2443— untouched.Files
12 files,
+451/−354(git diff --numstat d400edb7...HEAD), tree83d67065:core/src/client/kanban.js(+7/−5) —BoardViewtakesactions; the hint becomesreadOnlyLine.core/src/client/new-chat.js(+4/−3) — both locked branches drawreadOnlyLine, one key each.core/src/client/project-board.js(+4/−3) —ProjectSettingsPaneltakesactions;ProjectBoardOverlaypasses it.core/test/readonly-reload-lines.test.js(+45/−2) —SURFACES7 → 11 and fourmountForbranches.docs/subsystems/core-client.md(+4/−4) — the four surfaces and the panel'sactionsare documented.changelog.d/2521-overlay-readonly-reload.fixed.md(+1, new).core/client.js(+10/−8),patches/ds-bot/0003-flybot-api-client.patch(+5/−5, carriesnew-chat.js),patches/ds-bot/0008-client-bundle.patch(+315/−314, carrieskanban.jsandproject-board.js),patches/ds-bot/files/src/client/kanban.js(+7/−5),patches/ds-bot/files/src/client/project-board.js(+4/−3),patches/ds-bot/files/test/readonly-reload-lines.test.js(+45/−2) — generated bybuild-client+ds-bot-patches generate, never hand-edited. The two carrier patches are exactly the ones whose source files this branch changes — the carrier set{0003, 0008}is exactly the set that carries the changed sources, and no third patch is touched (the other eight patches are byte-identical to the base).{ "pr": 2523, "claim": "Dev-2", "issue": 2521, "summary": "Four read-only surfaces drawn inside overlays — the Board hint, both New chat sites and the Project settings panel — now draw the shared readOnlyLine piece instead of their own copy of the host sentence. Each keeps the sentence in its one role=status node and puts a single Reload outside it, running actions.reloadTeam, never a page reload. The panel now receives actions from ProjectBoardOverlay, which previously passed no reload at all. A writable window and every read-only control are unchanged.", "files": [ "core/src/client/kanban.js", "core/src/client/new-chat.js", "core/src/client/project-board.js", "core/test/readonly-reload-lines.test.js", "docs/subsystems/core-client.md", "changelog.d/2521-overlay-readonly-reload.fixed.md" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=d400edb7 npm run verify; build-client --check; ds-bot-patches check/canonical; 13 neighbouring suites", "result": "green", "at": "81b9a7ac1940eb5949bb7b50e4b462e9be3dc742" }, "next": "on the rebased base the branch carries a re-cut 0008; the source/tests/docs half is unchanged, so review the regenerated carrier patches only" }