fix(scheduled-task): preserve snoozed occurrence on edits - #5226
Conversation
Generated-by: Codex Signed-off-by: Dante <duanjl.china@gmail.com>
Astro-Han
left a comment
There was a problem hiding this comment.
Review result: NO-GO (1 P2)
Reviewed commit 2b912f3d4199f0476fa00597217f86b8d7b7d529.
P2 — disabling and re-enabling still drops the snoozed occurrence
Issue #5221 explicitly includes this acceptance path: create a daily task at 09:00, snooze it to 09:10, edit only the title, then disable and re-enable it; the original 09:00 recurrence anchor should remain and the pending 09:10 occurrence should still be reflected.
This commit correctly fixes the title-only edit itself. packages/ui/src/scheduled-task-helpers.ts:295-315,372-387 detects unchanged schedule fields and retains the original schedule, packages/ui/src/scheduled-task-form-dialog.tsx:179-211 omits the schedule from the update patch, and packages/storage/src/scheduled-task-store.ts:287-297 preserves a future nextFireAt when no schedule patch is present. The new storage/UI tests cover that edit path at packages/storage/src/__tests__/scheduled-task-row-operations.test.ts:275-317 and packages/ui/src/__tests__/scheduled-task-form-schedule.test.ts:57-102.
The subsequent toggle path is unchanged: apps/desktop/src/preload/preload.ts:3279-3283 maps Disable/Enable to pause/resume; packages/core/src/scheduled-task.ts:319-326 clears nextFireAt during pause; and packages/core/src/scheduled-task.ts:329-357 recomputes the next occurrence from the persisted recurrence anchor during resume. Consequently, the 09:10 snoozed occurrence is lost after re-enabling, and the stated reproduction is still not closed. Please either persist the pending snooze across pause/resume or explicitly change the product contract and add a regression test for the chosen behavior.
Validation
The exact-head hosted test check is red because an existing, unchanged Storybook story (apps/desktop/stories/app-shell.stories.tsx:2714-2767) timed out; the label check passed. The merge tree and whitespace checks are clean. I could not run local TypeScript/build/test suites or a real Electron toggle smoke because this review worktree has no workspace dependencies.
Automated review notice: This review was generated by an AI agent and does not replace independent human review.
Generated-by: Codex Signed-off-by: Dante <duanjl.china@gmail.com>
|
Addressed the pause/resume gap from the review in commit
Validation passed on this commit: |
me2seeks
left a comment
There was a problem hiding this comment.
I found no blocking correctness issue at 1d1595ccadbaf7066eaa41a0dd54bcc1daebe255.
The follow-up commit closes the previously reported pause/resume gap. Pausing retains the pending occurrence without making it eligible to fire because due discovery still requires status === 'active'. Resuming reuses that occurrence only while it remains in the future; after it elapses, the next occurrence is recomputed from the persisted schedule. Metadata-only edits preserve the pending occurrence, while an explicit schedule edit replaces it and causes the next occurrence to be derived from the new schedule.
The UI also stops converting the displayed snoozed time into a new recurrence anchor when the schedule fields were left unchanged. Create, duplicate, interval, calendar, and cron paths remain distinguishable through the seed and submit contract.
Validation on the exact head: clean install; successful @maka/core, @maka/storage, and @maka/ui builds; 33/33 focused scheduled-task tests; Biome checks on all seven changed files; and git diff --check.
中文说明
我在精确提交 1d1595ccadbaf7066eaa41a0dd54bcc1daebe255 上没有发现阻塞问题。
此前指出的“停用再启用会丢失 snooze”已经修复：暂停任务会保留尚未到期的执行时间，但调度器仍只会执行 active 任务；恢复时只复用未来时间，若该时间已经过去，则按原始重复规则计算下一次。只改标题等元数据不会改 recurrence anchor；明确修改时间或重复规则时，才会使用新 schedule。
本地干净安装后，core、storage、UI 构建通过，相关测试 33/33 通过，七个改动文件的 Biome 与 diff 检查通过。
) Thirty-one upstream commits. The one that reaches the new renderer is apache#5170, which gives the Renderer the transcript window: Main keeps a tail cache and answers page requests pass-through, `loadBefore` / `loadAfter` return a page, `loadAround` / `loadLatest` a reset, `acknowledgeTail` is new, and a batch carries `extends` / `coversFrom` / `navigation` instead of `evictedDurableSequences` / `completedOverlayMessageIds`. Also in: apache#5217's observation contract (`subscribeEvents` loses `onSeeded`; readiness follows seed consumption as the `ready` phase, and the execution projection it offers is not consumed here yet), the memory work across composer and stream (apache#5153), interactions cleared per Turn on abort/complete (apache#4562), Session bundles and external agents in main/preload (apache#5197, apache#5164), Code Mode (apache#3615, apache#5219), and the scheduled-task snooze fix (apache#5226). Resolution per the sync policy: conflicts under the old renderer's trees, packages/ui's deleted components, their stories, e2e specs and the main tests that import them stay deleted; upstream's new files in those trees are dropped (`application/contracts/settings-presentation`, `features/external-agent-settings`, `features/session-bundle`, `workhub/ui/return-button`, `model-wheel-picker`, the prompt-rail and live-turn-buffer tests, `workhub-return-rail.spec.ts`). The renderer side of apache#5217 (one live Turn per Session → a buffer keyed by Turn, `liveTurn` → `liveTurns`, `phase` gone) stays out: `packages/ui` `live-turn-projection.ts`, `transcript-projection.ts` and their tests keep ours and `live-turn-buffer.ts` is dropped; `session-event-handlers.ts` keeps ours plus upstream's display-frame scheduler. `packages/ui` `conversation-copy.ts` keeps `transcriptGap` (our gap rows use it), `transcript-row-projection.ts` is restored, `use-pending-selection.ts` goes. Astryx stays out of package.json and the lockfile; `@ai-sdk/provider-utils` moves to 5.0.40 and the `@ai-sdk/code-mode` override lands. Re-implemented for the new contract: - `lib/ported/desktop-transcript-range-store.ts` and `transcript-reading-position.ts` are re-ported from upstream head (the previous copies were format-only ports of the old versions); `TranscriptReadSupersededError` lives in the latter, and `display-frame-scheduler.ts` joins `lib/ported`. - `store/active-session-store.ts`: the window is the store's — the display follows a store subscription rather than `accept`'s return; the paging gate and `loadTranscriptHistory` are gone (the controller refuses a read against an edge it already read), `loadHistory` keeps only the gap-row indicator; `prefetchHistory` and `retainWindow` serve `useChatScroll`'s geometry-driven filling and trimming; `setReadingAnchor` only moves the bookmark; the bookmark re-anchors after a replica generation change, by sequence within a Host epoch and by Turn through the landmark index across one; a read superseded by an epoch change is not an error. - `SessionView` passes `onPrefetchHistory` / `onRetainWindow`; the gap rows and the return-to-latest button keep their explicit commands. - `bridge/sessions.ts` drops `onSeeded`. - Main tests for the range store, navigation race, overlay settlement and the two new probes are upstream's with paths under `lib/ported`; the reading-position test keeps upstream's pure-module cases (send pinning, overlay-only bookmark, superseded read) — the shell-shaped cases live with the store's tests. - `settings-sections.ts` and the copy files name core's new `external-agents` section id as a deferred page. - Ported apache#5226: an edit that leaves the schedule fields alone omits `schedule` from its patch, so the Host keeps a snoozed fire (`scheduled-task-form-payload.ts`, `ScheduleFormDialog.tsx`, a test in `scheduled-module.test.tsx`). Also in this tree, found while verifying the sync and not caused by it: a live Turn's finished steps vanished after switching to another task and back, leaving only "Working on it…". The Host re-seeds only what is still incomplete (the streaming text, pending interactions) and Main's transcript overlay is bootstrapped once per replica, so the steps that finished while the Session was on screen existed only in the renderer's live projection — which the store wiped on every selection and reseed. The projection now survives the switch (a reseed drops only the incomplete text and thinking it replays; a Turn that ended meanwhile is retired by the transcript it left behind). `test:streaming-switch` drives the real app through it with a new fake-backend scenario that settles a text step and a tool call, then holds the Turn open. The compatible-change declaration `base64-length-allocation.json` is re-pinned from 143 to the epoch this branch carries (147, upstream's own): upstream left it at the epoch of its commit and its per-commit hook never re-judged it, while our merge stages it next to the epoch bump. Its reason (Base64 byte counting in `artifact.ts` / `session-transcript.ts` without observable change) still holds against the protocol as merged. Gates: build:test + build:renderer, typecheck, biome lint and format, locale hygiene, ASF headers, renderer architecture ledger (rewritten with `--write`; the new range store's `window` local reads as environment capabilities to the checker), e2e budget, third-party notices, knip (same findings as before the merge), desktop dist tests (1599), renderer state (282), Electron smoke (44 checks, no renderer errors), core-dialogue smoke, streaming-switch smoke. `packages/storage` `workspace-identity` (git worktree ENOTEMPTY) is a parallel-run flake that passes in isolation, as is `packages/eval` `lifecycle-boundaries` (relay cancellation timing); `packages/runtime` `model-adapter-onerror` fails on this machine before and after the merge (asynchronous activity after the test ended; the file is unchanged this round). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Summary
Keep schedule edits distinct from metadata-only edits so renaming a snoozed recurring task preserves both its recurrence anchor and its pending snoozed occurrence.
The edit form now omits the schedule patch when its schedule fields are unchanged. Storage preserves a future pending occurrence for metadata-only updates, while explicit schedule changes and overdue occurrences continue to recompute normally.
Fixes #5221
Verification
npm run lintnpm run format:checknpm --workspace @maka/storage run typechecknpm --workspace @maka/ui run typechecknpm --workspace @maka/storage run buildnpm --workspace @maka/ui run buildAI use
Select exactly one:
Tool(s) and scope: Codex implemented the schedule-update fix and added the targeted regression coverage.
Checklist
Does this PR entail a change in behavior?