Repository navigation
Offer Run now on a scheduled task, not only Retry after a failure - #2509
Merged
Merged
Conversation
…inkFlowLab#2507) The row drew its retry control only when `task.lastRun.result === 'failed'`, while the host's `retrySchedule` accepts any task and refuses only a paused one: a healthy task or one that never ran had no way to be run once now from this page. Draw the same control for a task the host will run on demand and label it `Run now`; a failed run still reads `Retry`. A paused task, and one whose run is in flight, offers neither, and a read-only window still draws no write. The control calls the same `actions.scheduleRetry` with just the session and task id, so DSH's own next run is untouched. Docs-not-needed: no owned path changed outside docs/subsystems/core-client.md, which is updated.
HugoXK
force-pushed
the
dev/3-schedules-run-now
branch
from
November 5, 2026 06:53
0e0a222 to
5932b9a
Compare
HugoXK
marked this pull request as ready for review
November 5, 2026 07:16
Contributor
Author
|
Enqueue request — PR #2509 at
Please enqueue. |
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 #2507
Owner: Dev-3
Reviewer: Birfy
Claim-Id: 31c8ce45f8584df08e3993fa5ebeb798
Risk tier: T1
Written-by: FlyBot Dev-3
The defect
core/src/client/schedules.js:194drew its one-shot control only whenlastRun.result === 'failed':!locked && failed && task.status !== 'paused' && actions.scheduleRetry ? … t('Retry') : null. A task that is healthy, or one that has never run, therefore had no way to be run once from its own row, while the host has had that capability all along —retryScheduleincore/src/host/schedules.js:567-594refuses only a paused or unknown task, callsnoteRun(record.id, { at, result: 'running' })and thenagent.followup(retryText(...)), leaving DSH's own next run untouched. The row already carried the right title (Run the task once now); only its label and its condition were tied to failure.The change
!locked && actions.scheduleRetry && task.status !== 'paused' && task.lastRun?.result !== 'running', and the label becomest(failed ? 'Retry' : 'Run now'). A failed run still reads Retry; an active task, and one that never ran, reads Run now; a paused task and one whose run is in flight draw neither. The call is stillactions.scheduleRetry(sessionId, id)with only those two ids, so nothing about DSH's own schedule changes.core/src/client/i18n.jsgains one key,'Run now': '现在运行';Run the task once nowstays as the title.core/test/schedules-client.test.jsadds the two P-590 cases (five tasks: a delivered run, no last run, a failed run, a run in flight, a paused one) and addsRun nowtoWRITE_LABELS, so the read-only assertion in#2232covers the new control too.docs/subsystems/core-client.mdstates the new control and lists it among the writes a read-only window does not draw;changelog.d/2507-run-now.added.mdis the fragment.core/client.js,patches/ds-bot/0001-core-fixes.patch(i18n + schedules),patches/ds-bot/0008-client-bundle.patch(the bundle) andpatches/ds-bot/files/test/schedules-client.test.jsare generated fromcore/.No host file and no endpoint changed.
Red first
On a clean tree at
e5fc62de, with onlycore/test/schedules-client.test.jsplaced on it,node --test test/schedules-client.test.jsexits 1:Case 63 also presses the control once and asserts exactly one
schedule-retry([['retry', 'chat-b1', 's1']]) and that the row'sNext:line is byte-identical before and after; case 64 asserts the read-only window draws neither label and no other scheduled-task write, with the writable page as the positive control. With the fix, the same file is 64 of 64, and the ten adjacent client suites (schedules,details-drawer,details-drawer-refusal-line,details-bot-settings-readonly,details-group-settings-readonly,details-overview-readonly,details-group-overview-readonly,readonly-window-reads,settings-readonly-pages,groups) are 73 of 73.Verified at the tip (
5932b9a181760cf94d760e2373b079a6ba94a2c3)One commit over
e5fc62de, treec37e5681. On a clean checkout at that head,TZ=UTC ARCH_DOCS_BASE=e5fc62dea0c067a308fbf8f122d691a35e455fb1 npm run verifyexited 0: 4946 tests, 4946 pass, 0 fail — core 2917, social 577, tree 137, timeline 141, workflow 440,.github+policy 671, interop 37, live-e2e 26;arch-docs: OK (9 changed file(s));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);client.js is the build of src/client.The branch was rebased onto
e5fc62deafter#2504merged; that rebase needed no hand resolution incore/client.jsor the patch series, and the bundle and patches were rebuilt rather than merged by hand.npm run buildran incore/beforenode scripts/ds-bot-patches.mjs generate; generate ran three times and every run left the tree unchanged.git status --porcelainreports zero tracked changes. Any later commit on this branch carries its own measurements and is reported with them; the numbers above are this tip's.Files
Hand-written, 5 files, +56/−7:
core/src/client/schedules.js(+9/−4)core/test/schedules-client.test.js(+44/−2)core/src/client/i18n.js(+1/−0)docs/subsystems/core-client.md(+1/−1)changelog.d/2507-run-now.added.md(+1/−0)Generated from
core/, 4 files, +67/−13:core/client.js(+4/−1)patches/ds-bot/0001-core-fixes.patch(+13/−7)patches/ds-bot/0008-client-bundle.patch(+6/−3)patches/ds-bot/files/test/schedules-client.test.js(+44/−2)9 files, +123/−20.
Not run
The smoke test, because no
index.jsorsrc/host/file changed, and the pack check, typecheck and actionlint of CI'spackagejob; CI runs those on this tip.Checkpoint
{
"pr": 2509,
"claim": "Dev-3",
"issue": 2507,
"summary": "A scheduled task's row offered a one-shot run only after a failed run, while the host's retrySchedule accepts any task and refuses only a paused one, so a healthy task or one that never ran had no way to run once now. The row draws the same control for any task the host will run on demand and labels it Run now; a failed run still reads Retry, and a paused task or one with a run in flight offers neither. A read-only window still draws no write, the call is the same scheduleRetry with just the session and task id, and DSH's own next run does not move.",
"files": ["core/src/client/schedules.js", "core/test/schedules-client.test.js", "core/src/client/i18n.js", "docs/subsystems/core-client.md", "changelog.d/2507-run-now.added.md", "core/client.js", "patches/ds-bot/0001-core-fixes.patch", "patches/ds-bot/0008-client-bundle.patch", "patches/ds-bot/files/test/schedules-client.test.js"],
"checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=e5fc62dea0c067a308fbf8f122d691a35e455fb1 npm run verify", "result": "green", "at": "5932b9a181760cf94d760e2373b079a6ba94a2c3" },
"next": "none — ready for review"
}