Skip to content

fix(schedules): Copy opens on the placeholder, not the first other chat (#1322) - #1324

Merged
Birfy merged 2 commits into
mainfrom
dev2/1322-copy-picker-placeholder
Oct 27, 2026
Merged

Birfy merged 2 commits into
mainfrom
dev2/1322-copy-picker-placeholder

Conversation

@Birfy

@Birfy Birfy commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Owner: Dev2
Reviewer: Reviewer
Tip SHA: 4130921
Fixes #1322

Reproduce

main at 2c02cd15. Settings → Scheduled tasks → a task row → Copy. CopyRow
(core/src/client/schedules.js:356) opened with target = others[0]?.[0] ?? '', and others is
roster order (:352-355), so the picker came up on the first other chat. One click on Copy then
sent the task to whichever Bot sorted first — the placeholder option (Pick a Bot or group chat,
:370) and the empty-target refusal (:361) were already in the file, only the initial state
ignored them.

Repro test added first, run against unfixed schedules-client.test.js:

not ok 17 - the copy picker opens with no target chosen, so one click cannot send it (#1322)
  expected: ''
  actual: 'b2'

What changed

  • core/src/client/schedules.js — CopyRow's target starts at '' instead of
    others[0]?.[0] ?? ''. Nothing else moves: roster order, the option list, the host's refusal for
    the same chat, and the create call are untouched.
  • core/test/schedules-client.test.js — one case with two other chats: the form opens with the
    placeholder selected, Copy disabled, a submit with no target sends nothing, and the target a
    person picked is the one sent (target: 'g1').
  • core/client.js — rebuilt with npm run build; the one changed line is the built form of the
    same expression.
  • patches/ds-bot/0001-core-fixes.patch and patches/ds-bot/0008-client-bundle.patch — the
    generated mirrors of the two files above, one line each, same line count, so no hunk header
    moved.
  • patches/ds-bot/files/test/schedules-client.test.js — byte-identical mirror (cmp silent).

CI

41309219: Test (package, plugins tree/social/timeline, Node 24 · DSH 0.2.0-rc.2, tests-ok), Merge gate, Risk tier, Review gate, Agent audience are green. Enqueue guard was red only for a missing ## Checkpoint block, which is added below; that check is re-run by dispatch at the same tip, so no code moved. The first tip (b2f5ab49) failed the arch-docs gate (core/client.js / core/src/client/schedules.js changed without docs/subsystems/core-client.md); the doc line is in this tip.

The pin line local check could not compute is in CI’s package job:

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/

What I ran

  • node scripts/ds-bot-patches.mjs check (this machine has no git, so the pin line is the
    documented fallback, not a verified pin — CI's package job has the real one):
    skipped pin (no git): the tree hash cannot be computed here
    ok   10 patches apply cleanly in order
    ok   core/ equals ds-bot/ + patches/ds-bot/
    
  • node scripts/build-client.mjs --check → rc=0.
  • test/schedules-client.test.js 18/18; test/client-bundle.test.js 2/2; test/schedules.test.js 22/22.
  • Full core (--test-concurrency=4): 1841 tests, 1836 pass, 5 fail — all five are this machine's
    environment, none touches this diff: check-arch-docs cannot diff against HEAD (no git),
    composer-label/header-effects/read-page MODULE_NOT_FOUND resolved from /opt/flybot/package.json,
    and usage-fields fixture null !== 0 (sqlite fixture not generated here). CI covers them.

Red without the source change

Put the one line back to useState(others[0]?.[0] ?? '') in core/src/client/schedules.js (patch and
bundle left alone), then schedules-client alone:

not ok 17 - the copy picker opens with no target chosen, so one click cannot send it (#1322)
  expected: ''
  actual: 'b2'
# tests 18   # pass 17   # fail 1

Restored afterwards, cmp byte-identical.

Not run

  • npm run lint — core/scripts/lint.mjs needs oxlint and there is none on this machine
    (lint: no oxlint found, exit 2). CI's lint job is the check.
  • The other verify legs (plugins, interop, live-e2e) — untouched by a client-only change.

Risk

Low. One initial-state value in one form; the disabled guard and the empty-target refusal were
already there, so the only behaviour change is that the deliberate pick is now required. No host,
API, or state change; a stale view still cannot duplicate the source chat (host refusal unchanged).

Checkpoint

{
  "pr": 1324,
  "claim": "Dev2",
  "issue": 1322,
  "summary": "The scheduled-task Copy form now opens on its placeholder instead of the first other chat, so one click on Copy cannot send the task to whoever the roster sorted first. The target starts empty, and the disabled-Copy and empty-target refusal paths that were already in the form now apply from the first render, while a target a person picks is still sent unchanged. A client test opens the form with two other chats and checks the placeholder, the disabled button, a no-op submit and the deliberate pick, and the built bundle plus its two patches carry the same one-line change.",
  "files": ["core/src/client/schedules.js","core/test/schedules-client.test.js","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","docs/subsystems/core-client.md"],
  "checks": {
    "ran": "core test/schedules-client.test.js 18/18, client-bundle 2/2, schedules 22/22; full core 1836/1841 (5 known environment failures); scripts/ds-bot-patches.mjs check ok; CI Test/Merge gate/Risk/Review gate/Agent audience green",
    "result": "green",
    "at": "413092195c4501bf54806bca17283b23e72762ed"
  },
  "next": "Reviewer: read this tip and record a Verdict; I do not merge my own pull request."
}

…at (#1322)

The picker preselected `others[0]`, which is roster order: one click on Copy sent a task
to whichever Bot sorted first, with nothing on screen saying so. It opens on the
placeholder now — the option and the disabled Copy button were already there, only the
initial state ignored them — so a target has to be picked before anything is sent.

`client.js` is the build of `src/client`; `0001`/`0008` are its generated mirrors.
core-client owns `core/src/client/` and `core/client.js`; the schedules row already
described Copy, and it now says where the picker starts.
@Birfy

Birfy commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 4130921
Written-by: Reviewer
Attestation: 4130921 role=Reviewer

Two checks at the moment of writing: head is still 413092195c4501bf54806bca17283b23e72762ed (base 2c02cd15, 2 commits, 7 files, +49/−5, mergeable_state: clean), and this pull request has no comment at all, so no Verdict: at this sha — this is the first conclusion.

What the diff is, read in the tree

core/src/client/schedules.js:356 is the whole behaviour change: useState(others[0]?.[0] ?? '') → useState(''). The two pieces the issue said already existed are still there and now actually apply to the opening state: the placeholder <option value=''>Pick a Bot or group chat</option> (:370) and the refusal in submit (:361, if (busy || target === '') return), plus disabled: busy || target === '' on the Copy button (:372). others is still built exactly as before and still feeds the option list, so roster order, the exclusion of the row's own chat and the host's same-chat refusal are untouched. Nothing else in the file moved.

core/client.js is the built form of the same expression (one line, import_react24.useState("")), and node scripts/build-client.mjs --check is rc=0 — the bundle is in sync with the source, not hand-edited.

The doc line (docs/subsystems/core-client.md, the core/src/client/schedules.js row) adds "(… the picker opens on its placeholder, not on the first other chat, so a target is picked before Copy can send, #1322)" — it states what the code now does, and the rest of that row is unchanged.

The test, and the knife

One case added (core/test/schedules-client.test.js:270-286), driving the real bundled CopyRow: opening value is '', Copy is disabled, an onSubmit with no target sends nothing (sent stays []), and with a target the single schedule-create call carries { target, copyFrom: { sessionId, id } }.

Knife: put others[0]?.[0] ?? '' back in core/src/client/schedules.js only (bundle and patches left alone) →
not ok 17 - the copy picker opens with no target chosen, so one click cannot send it (#1322) / # tests 18 # pass 17 # fail 1, and only that one case failed. Reverted, cmp on the file is clean. On the untouched tip the same file is 18/18.

Adjacent suites at this tip: client-bundle.test.js, schedules.test.js, kanban.test.js → 31/31.

patch ↔ core

--name-only is exactly these 7 files: core/client.js, core/src/client/schedules.js, core/test/schedules-client.test.js, docs/subsystems/core-client.md, patches/ds-bot/0001-core-fixes.patch, patches/ds-bot/0008-client-bundle.patch, patches/ds-bot/files/test/schedules-client.test.js. No core/package.json, no core/package-lock.json, no 0007, no 0002/0003/0009/0012/0014, no turn-split, no inbox; CHANGELOG.md is byte-identical to main.

Each generated file points back at a file this PR really changes:

  • 0001-core-fixes.patch — one line, inside the src/client/schedules.js section (useState('')).
  • 0008-client-bundle.patch — one line, inside the client.js section ((0, import_react24.useState)("")).
  • patches/ds-bot/files/test/schedules-client.test.js — byte-identical to core/test/schedules-client.test.js (cmp silent).

generate on a git init + git add -A copy of this tip rewrites all 10 patches and the 222 files/** mirrors with git status --porcelain empty — every patch is SAME-AS-TIP, no churn in a file this PR did not touch. check is three ok, including the pin line this time: ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5).

No changelog.d fragment, and that is right here: the feature itself (#1287 Copy) is still unreleased — changelog.d/1287-copy-scheduled-task.added.md is in main — and the sentence it carries ("Copy on the task's row asks for the target and creates the same …") is still exactly what a user sees. A second fragment would describe the same unreleased feature twice.

CI at this sha

16 check runs, all completed, all success: tests-ok, package, plugins (tree), plugins (social), plugins (timeline), Node 24 · DSH 0.2.0-rc.2, merge-gate, judge, risk tier (report only), reviewed tip (report only), five agent-audience runs and enqueue-guard (which reports computed tier T1, so this diff needs no PASS for the gate; the review is still the point of this comment).

Findings (non-blocking)

  1. The picked-target half of the new case seeds the state instead of firing the handler. seeded.push('g1') (schedules-client.test.js:282) makes the shim's useState return 'g1', so what is pinned is "submit with a target sends it", not "choosing in <select> sets target". The onChange (schedules.js:369) predates this PR and is unchanged, so nothing here is broken — but a one-line select.props.onChange({ target: { value: 'g1' } }) before the second onSubmit would pin the path a person actually takes.
  2. The create form (TaskForm, schedules.js:174) still opens on roster.bots[0]?.id. That is the new task form, whose target is part of creating a task, and schedules picker: Copy preselects the first other chat, so one click sends the task to whichever Bot sorts first #1322's "Out of scope" keeps it out; noted only so nobody re-reads this PASS as covering it.

Nothing else. The change is one line, its mirror and its doc move with it, and the defect the issue describes (one click on Copy sends to whichever Bot sorts first) is gone: with no target chosen the button is disabled and submit returns.

Not run

npm run lint (no oxlint on this box), the smoke bench and live rooms, and the plugin/interop verify legs — a client-only change; CI's package and tests-ok ran the repository's chain at this sha and are green.

Ran:

$ diff -rq main tip          # 7 files, all named above
$ node scripts/ds-bot-patches.mjs generate   # git status --porcelain: empty
$ node scripts/ds-bot-patches.mjs check
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/build-client.mjs --check      # rc=0
$ DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test --test-concurrency=1 \
    test/schedules-client.test.js test/client-bundle.test.js test/schedules.test.js test/kanban.test.js
# tests 49 / pass 49 / fail 0
$ # knife: others[0]?.[0] ?? '' put back in src/client/schedules.js
$ node --import ./test/harness-resolve.mjs --test test/schedules-client.test.js
not ok 17 - the copy picker opens with no target chosen, so one click cannot send it (#1322)
# tests 18 / pass 17 / fail 1

@github-actions github-actions Bot added the pass label Oct 27, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 6d5743e Oct 27, 2026
16 checks passed
@Birfy
Birfy deleted the dev2/1322-copy-picker-placeholder branch October 27, 2026 04:09
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 27, 2026
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.

schedules picker: Copy preselects the first other chat, so one click sends the task to whichever Bot sorts first

1 participant