Skip to content

[TASK-tsk_d71013cb][Backend] fix: cron schedule stagnation — maybeEnsureScheduledNextRun on completed#217

Closed
jsyqrt wants to merge 1 commit into
mainfrom
task-tsk_d71013cb
Closed

[TASK-tsk_d71013cb][Backend] fix: cron schedule stagnation — maybeEnsureScheduledNextRun on completed#217
jsyqrt wants to merge 1 commit into
mainfrom
task-tsk_d71013cb

Conversation

@jsyqrt

@jsyqrt jsyqrt commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

📋 基本信息

  • 提交者: Backend Developer (ID: agt_45ee41456c2a1e988a9c19fd)
  • 关联任务: [TASK-tsk_d71013cb] Fix Cron Schedule Stagnation — nextRunAt Recalculation
  • 目标分支: main

🎯 背景与动机 (Why)

Scheduled tasks stop firing after the first execution because nextRunAt is not recalculated when a scheduled task reaches 'completed' status. The cron poller only picks up tasks where nextRunAt ≤ now, so without recalculation, the task disappears from the poll after one run.

🔧 变更内容 (What)

packages/org-manager/src/task-service.ts

Added maybeEnsureScheduledNextRun(task) method (~line 1287):

  • For one-shot tasks (runAt set): If runAt has passed, clear both runAt and nextRunAt — the task is truly done
  • For recurring tasks (cronExpression set): Parse the cron expression, calculate the next valid time after now
    using cron-parser library, update nextRunAt
  • For invalid/expired cron expressions: Set status to 'completed' and stop scheduling
  • Called from Stage 7 (upstream task-to-scheduled task flow) and from the on-completion path

Updated updateTask() completion path (~line 4400):

  • After setting status=completed, if the task is a scheduled task (has cronExpression or runAt),
    call maybeEnsureScheduledNextRun to recalculate and persist the new nextRunAt

Added public isScheduledTask helper method (~line 1283):

  • Returns true if task has cronExpression or runAt
  • Used by the on-completion path to gate the recalculation

✅ 验证方式

  • Changes verified by reading all 55 lines of additions
  • Uses existing cron-parser library (already imported in task-service.ts)
  • Follows existing patterns (same setter DI + repo pattern)
  • Backward compatible — only scheduled tasks with cronExpression/runAt are affected

👤 评审人

  • Reviewer: Code Reviewer (agt_42fc22d8cd79900a089eea09)

…ledNextRun on completion recalculates nextRunAt
@jsyqrt

jsyqrt commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

✅ Code Review — Approved

Reviewer: Code Reviewer (agt_42fc22d8cd79900a089eea09)

Verification Results

  • pnpm typecheck: ⚠️ Pre-existing errors only (missing @larksuiteoapi/node-sdk, yaml — not related to this PR). No errors from task-service.ts.
  • pnpm test (org-manager): ✅ 64 tests passed (3 pre-existing suite failures from missing larksuite dependency — unrelated)
  • Dependency check: computeNextRunFromConfig ✅ (defined at line 4295), updateScheduleConfig ✅ (defined at line 3732)
  • Diff review: 55 additions, 0 deletions, 1 file modified

Decision

Approved — implementation is correct, clean, and follows codebase patterns.

Notes

  • The fix adds maybeEnsureScheduledNextRun() as a safety net in handleTransitionSideEffects (Stage 7, after external broadcast) — recalculates nextRunAt when a scheduled task completes
  • Correctly handles 4 edge cases: one-shot (runAt), maxRuns reached, already-valid future nextRunAt, expired/invalid cron
  • Follows existing fire-and-forget persistence pattern (.catch()). Does NOT touch currentRuns — only fills missing/stale nextRunAt
  • No scope creep: exactly 55 lines in 1 file as advertised

Please do NOT merge this PR yourself — per team norms, only the Owner (老板) can merge PRs. Task is marked completed in Markus system.

@jsyqrt jsyqrt closed this Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant