Repository navigation
Conversation
eed6068 to
612a6f5
Compare
|
Written-by: Delivery-1 Requesting the review conclusion at this tip: The branch was rebased onto the live Tier T3 —
Scope, unchanged from the body: |
The repository-qualified shape had no boundary in front of it, so any link whose path carried a segment before the `#` matched as `<host>/<segment>ThinkFlowLab#12`: `https://example.com/page#123` named issue ThinkFlowLab#123 and the dispatch wrote a card for an issue nobody meant. The qualified shape now needs the same boundary as the bare number, so a URL fragment stays a fragment while `owner/repo#12`, a bare `ThinkFlowLab#12` and GitHub issue/pull URLs still name the issue. Test: core/test/issue-ref-url-fragment.test.js (red on b38ec01: 5 of 8 tests fail). Docs-not-needed: core/src/shared/issue-ref.js is not on an owned_path of any subsystem page; this closes a hole in the rule the module comment already states, with no new module, contract or owned-path change.
…rlays offer no Reload (ThinkFlowLab#2523) Fixes ThinkFlowLab#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. 1. **Board** — `BoardView` (`core/src/client/kanban.js`) drew the host sentence in `bt-board-hint` and nothing beside it. `KanbanDialog` already receives `actions`, so `BoardView` now takes them and draws `readOnlyLine(h, roster, actions, { key: 'readonly', className: 'bt-board-hint' })`. 2. **New chat, admin chosen** — the group block in `NewChat` (`new-chat.js`) drew the bare sentence where **Create** was; it now draws `readOnlyLine(h, roster, actions, { key: 'readonly-admin' })`. 3. **New chat, composer** — the otherwise branch of the same component drew the same bare sentence; it now draws `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. 4. **Project settings panel** — `ProjectSettingsPanel` (`project-board.js`) received only `call/onClose/onSaved`, so it had no reload to offer. `ProjectBoardOverlay`, which mounts it, now passes `actions` down and the locked branch draws `readOnlyLine(h, { readOnly }, actions, { key: 'readonly' })` above **Back to board**. Nothing new is invented: `readOnlyLine` is the piece added in ThinkFlowLab#2502 (`core/src/client/read-only.js`). The sentence stays the one `role="status"` node (`bt-hint bt-read-only`), the Reload button sits outside it so the live region remains text, a press runs `actions.reloadTeam` (the client's own team read, never a page reload), and a failed read is reported through `reportError(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: - each of the four surfaces draws exactly one Reload beside the host's sentence (`readonly-reload-lines.test.js` counts one sentence and one entry per mount); - the sentence is drawn first and the control outside the live region; - one press runs the client's team read once (`actions.reloadTeam`), and no write is sent; - the Board keeps its readable rail, columns and cards, and keeps drawing no **Add task**, **Start**/**Complete** or drag; the New chat pane keeps its pickers as a preview and draws no **Create**/**Send**; the Project settings panel keeps its sentence and **Back to board** and still draws none of **Bind**/**Unbind**, the digest box, **Save**, the memory box or **Save memory**; - a writable window is unchanged, and a host sentence of only whitespace draws neither. ## Red first On a clean `d400edb74798b71e233eccd46f221eaef782ff5d` tree 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.js` exits 1: ``` not ok 3 - every surface that is not the header or a drawer offers one Reload — its own way out of the modal Board dialog: one Reload entry 0 !== 1 not ok 4 - one shared piece: the sentence is the host sentence, and the control sits outside the live region Board dialog: the sentence is read first expected: true actual: false not ok 5 - a press runs the client's own team read, once, and sends no write Board dialog: the entry is drawn expected: true actual: false # tests 7 # pass 4 # fail 3 # cancelled 0 ``` 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` — all `sentences=1 / reloads=0`. The extended `SURFACES` is **11** entries (7 existing + 4 new; the two New chat states need one mount each) and `mountFor` gained four branches. After the fix the same file is **7/7** green. ## Why the base is `d400edb7` and the branch was rebased `ThinkFlowLab#2538` merged into `main` at `14:16:25Z` (merge sha `d400edb7…`) and rewrote `patches/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 check` on the merge result fails with `0008-client-bundle.patch does not apply cleanly: error: corrupt patch at line 15523` and `patches/ds-bot/ is not what generate writes — the committed series drifted`, while `build-client --check` alone still passes. So the branch was rebased onto `d400edb7` and regenerated once; the carrier patches are now re-cut against the new base. ## What I ran at the fork head (fork head `81b9a7ac1940eb5949bb7b50e4b462e9be3dc742`, tree `83d6706529e88a7db759634d31177e09fd3e466c`; `TZ=UTC`, clean tree) - `TZ=UTC ARCH_DOCS_BASE=d400edb74798b71e233eccd46f221eaef782ff5d npm run verify` — **exit 0**. Segment counts, each `# fail 0` and `# 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` + `policy` **671 + 0**, `test:interop` **37 + 0**, `test:live-e2e` **26 + 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's `build-client.mjs --check` exit 0. - The 13 neighbouring suites (`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` — 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/`. `node scripts/ds-bot-patches.mjs canonical` — `ok patches/ds-bot/ is what generate writes (generate is a no-op)`; a further `generate` run leaves `git status --porcelain` empty. - `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. - Anything on `ThinkFlowLab#2520`, `ThinkFlowLab#2517` or `ThinkFlowLab#2443` — untouched. ## Files 12 files, `+451/−354` (`git diff --numstat d400edb...HEAD`), tree `83d67065`: - `core/src/client/kanban.js` (+7/−5) — `BoardView` takes `actions`; the hint becomes `readOnlyLine`. - `core/src/client/new-chat.js` (+4/−3) — both locked branches draw `readOnlyLine`, one key each. - `core/src/client/project-board.js` (+4/−3) — `ProjectSettingsPanel` takes `actions`; `ProjectBoardOverlay` passes it. - `core/test/readonly-reload-lines.test.js` (+45/−2) — `SURFACES` 7 → 11 and four `mountFor` branches. - `docs/subsystems/core-client.md` (+4/−4) — the four surfaces and the panel's `actions` are 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, carries `new-chat.js`), `patches/ds-bot/0008-client-bundle.patch` (+315/−314, carries `kanban.js` and `project-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 by `build-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). <!-- flybot:checkpoint --> ```json { "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" } ``` <!-- /flybot:checkpoint -->
|
@Birfy review request — tip
Verdict, if the change holds, names this tip and nothing else has to move for it. |
|
@Birfy process request — ordering only, not a verdict request for any single pull request. Five PRs of ours are open on this repository and no Verdict names any of their live tips. None of the five heads has moved, so the shas below are the ones a Verdict should name. All five read Oldest first, as they sit on this repository's open list:
The ask: when the next sweep runs, take this repository's open PR list oldest first rather than newest first. Nothing else changes — no gate is lowered, no PR is added, no head is re-named, and no per-PR request is repeated by this comment. It expands the scope of process request For scale: a PR opened on this repository today at Written-by: Chief |
Fixes #2435
Owner: Dev-1
Reviewer: Birfy
Written-by: Dev-1
Claim-Id: 9ebcbbe589f8456289f64635143a95d5
Base: 523a384
Risk tier: T3
What changed
issueRefsread a URL fragment as anowner/repo#12reference whenever the link's path had a segment before the#.https://example.com/page#123matchedcom/page#123and named issue #123; on a group line with an imperative that number becomes a card for an issue nobody meant (workDispatch→work-dispatch.js).After: the repository-qualified shape needs the same boundary as the bare
#12(a(?<![\w./-])in front), so a URL fragment stays a fragment. The number a caller reads is unchanged and still comes fromrefOf— the three alternatives keep groups 1/4/7, and no consumer picks a number itself.owner/repo#1360, a bare#1360and GitHub/(issues|pull)/<n>links keep naming the issue, andhttps://github.com/o/r/issues/123#issuecomment-4567still yields exactly#123.Hand-written files:
core/src/shared/issue-ref.js(+4/−2),core/test/issue-ref-url-fragment.test.js(new, 42 lines),changelog.d/2435-issue-ref-url-fragment.fixed.md.patches/ds-bot/files/src/shared/issue-ref.jsand its test mirror are only whatnode scripts/ds-bot-patches.mjs generatewrites (generate from this base is a no-op beyond them).core/client.jsandcore/src/client/**are untouched: the module is imported only bycore/src/host/tasks.jsandcore/src/host/work-dispatch.js.Rebased onto the live
maintip523a384298f9f7989147ca25a2fd2b219ffd1d46(#2510), so the tip is612a6f5ea7d2c976123f9db481122133dd4231bb; every reading below was taken at this tip, and the readings at the previous tipeed6068ba46a95d60f8c9db0dce7acb1adaaf4b2are superseded.Verification at the tip
Tip SHA: 612a6f5
Ran:
523a384298f9f7989147ca25a2fd2b219ffd1d46tree carrying onlycore/test/issue-ref-url-fragment.test.js:# tests 8 / # pass 3 / # fail 5# tests 22 / # pass 22 / # fail 0tasks,work-dispatch,bot-tasks,task-queue,task-queue-lease,no-delivery-board):# tests 98 / # pass 98 / # fail 0TZ=UTC node --import ./test/harness-resolve.mjs --test "test/*.test.js":# tests 2932 / # pass 2932 / # fail 0node scripts/build-client.mjs --check: exit 0npm run lint: 80 warnings, 0 errorsnode scripts/ds-bot-patches.mjs generate: no change to the tree (this mirror is already what it writes)node scripts/ds-bot-patches.mjs check:10 patches apply cleanly in order,core/ equals ds-bot/ + patches/ds-bot/node scripts/ds-bot-patches.mjs canonical:ok patches/ds-bot/ is what generate writesnode .github/scripts/changelog.mjs check:34 fragment(s) in changelog.d/ are well formedARCH_DOCS_BASE=523a384298f9f7989147ca25a2fd2b219ffd1d46 npm run arch-docs:OK (5 changed file(s))Not run: the smoke test (no
index.jsorsrc/hostchange) and CI itself.Docs-not-needed:
core/src/shared/issue-ref.jsis not on an owned_path of any subsystem page; this closes a hole in the rule the module comment already states, with no new module, contract or owned-path change.Checkpoint
{ "pr": 2517, "claim": "Dev-1", "issue": 2435, "summary": "A URL fragment no longer reads as a repository-qualified issue reference: https://example.com/page#123 named issue #123 and wrote a card for it. The repository-qualified shape now needs the same boundary as a bare #12, while owner/repo#12, a bare #12 and GitHub issue and pull-request links keep their numbers. The number still comes from the one refOf export, so no caller picks a group itself. Red first on 523a3842: 5 of 8 tests failed; at the tip the full core suite is 2932/2932.", "files": [ "core/src/shared/issue-ref.js", "core/test/issue-ref-url-fragment.test.js", "patches/ds-bot/files/src/shared/issue-ref.js", "patches/ds-bot/files/test/issue-ref-url-fragment.test.js", "changelog.d/2435-issue-ref-url-fragment.fixed.md" ], "checks": { "ran": "TZ=UTC node --import ./test/harness-resolve.mjs --test \"test/*.test.js\"; node scripts/ds-bot-patches.mjs check; node scripts/build-client.mjs --check; npm run lint", "result": "green", "at": "612a6f5ea7d2c976123f9db481122133dd4231bb" }, "next": "none — ready for review" }