AXI-92: Add scheduled task automation surface#35
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7995983be9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (result?.status === "FAILED") toast.error("Run failed. Error details were saved."); | ||
| else toast.success("Scheduled task completed."); |
There was a problem hiding this comment.
When this manual run loses the atomic claim after the preflight check—for example, the scheduled sweep starts it or another admin pauses/deletes it before executeScheduledTask runs—the mutation can resolve to null. This optional check treats that null as success, so the user sees “Scheduled task completed” even though no run or issue was created; handle a null result as a conflict/error or make the mutation throw.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e930b0d. The mutation now awaits the atomic execution claim and converts a lost claim into a CONFLICT response, with focused regression coverage for the guard.
| CREATE INDEX "ScheduledTask_workspaceId_enabled_nextRunAt_idx" | ||
| ON "ScheduledTask"("workspaceId", "enabled", "nextRunAt"); |
There was a problem hiding this comment.
The worker sweep is global across workspaces (enabled/nextRunAt plus ORDER BY nextRunAt in sweepScheduledTasks), but this index leads with workspaceId, which is not constrained by that query. As scheduled tasks accumulate across tenants, the once-per-minute worker cannot read due rows in next-run order from this index and will have to scan/sort; add a worker-oriented index starting with enabled, nextRunAt (or an equivalent partial index).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e930b0d. Added Prisma index [enabled, nextRunAt] plus migration 0107; the migration applied cleanly and the live dev schema reports ScheduledTask_enabled_nextRunAt_idx with the expected btree columns.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e930b0d539
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const nextRunAt = scheduledForAt | ||
| ? nextScheduledRunAt(scheduleFromTask(task), now, scheduledForAt) | ||
| : task.nextRunAt; |
There was a problem hiding this comment.
When the worker is down or behind and an admin clicks Run now on a task whose nextRunAt is already in the past, this manual branch preserves that past timestamp. After the manual run succeeds, the next maintenance sweep still sees the task as due and creates another issue immediately, duplicating the work the admin just ran. Consume/advance an overdue nextRunAt for manual runs while leaving truly future occurrences unchanged.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e9fc056. Manual claims now advance nextRunAt only when the stored occurrence is already overdue; future occurrences remain unchanged. The integration lifecycle forces nextRunAt into the past, runs manually, and verifies the persisted next run is future. Focused 18/18 and full 1,304/1,304 tests pass.
|
@codex review |
Codename-11
force-pushed
the
feat/axi-92-scheduled-tasks
branch
from
e9fc056 to
d2bde75
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2bde75c87
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| where: { | ||
| id: task.id, | ||
| enabled: true, | ||
| status: { not: ScheduledTaskStatus.RUNNING }, | ||
| ...(args.trigger === ScheduledTaskRunTrigger.SCHEDULE |
There was a problem hiding this comment.
When the maintenance sweep reads due ids it filters out archived workspaces, but the actual transactional claim here only checks the task row. If a workspace is archived after the sweep's due query has returned an id and before executeScheduledTask() claims it, this update still flips the task to RUNNING and the action creates an issue in an archived workspace. Include workspace: { deletedAt: null } or an equivalent recheck in the claim so archive state is enforced at the point of execution.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 57dc807. The atomic task updateMany claim now includes workspace.deletedAt = null, so both scheduled and manual execution lose the claim if archival wins the race. Integration coverage directly invokes execution for an archived workspace and verifies no run is created.
| const task = await tx.scheduledTask.update({ | ||
| where: { id: current.id }, |
There was a problem hiding this comment.
When the worker claims a due task between the preflight current.status read and this unconditional update, the edit still succeeds against a RUNNING task. That bypasses the conflict branch above and can overwrite nextRunAt/schedule while the in-flight run is using the old task snapshot, leaving the persisted schedule inconsistent with the run. Make the update conditional on status != RUNNING inside the transaction and return CONFLICT if no row is updated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 57dc807. Edits now use an updateMany predicate scoped by task/workspace with status != RUNNING, return CONFLICT when the atomic update loses, and read the updated task back only after a successful claim. Running-task edit coverage was added.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57dc807f66
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const task = await tx.scheduledTask.update({ | ||
| where: { id: current.id }, | ||
| data: { enabled: true, status: ScheduledTaskStatus.ACTIVE, nextRunAt }, |
There was a problem hiding this comment.
If two admins have a paused task open, the first resume makes it active with a near-term nextRunAt; a stale second Resume call later still reaches this update and recomputes nextRunAt from the later time. For schedules whose first occurrence has become due (for example daily 09:00 resumed at 08:59, then resumed again at 09:01), this overwrites the due run with tomorrow's time and the worker never claims today's occurrence. Make the update conditional on the task still being disabled/not RUNNING and return a conflict when it is not.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 24bdc0e. Resume now rejects already-active tasks at preflight and uses an atomic updateMany predicate requiring enabled=false and status != RUNNING. Losing that predicate returns CONFLICT without changing nextRunAt; repeat-resume regression coverage is green.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24bdc0e30f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| deliveryType: input.deliveryType, | ||
| projectId: input.projectId, | ||
| ...flattenedSchedule(input.schedule as ScheduledTaskSchedule), | ||
| nextRunAt, |
There was a problem hiding this comment.
When an edit begins while the task is paused, nextRunAt is precomputed as null. If another admin resumes the same task before this update commits, this update still matches because the predicate only excludes RUNNING rows, and it writes the resumed ACTIVE row back with nextRunAt: null. The worker only claims rows with nextRunAt <= now, so that active task will never fire until manually touched again; include the enabled state/version in the update predicate or recompute nextRunAt inside the transaction.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5dbbd71. The edit updateMany predicate now includes enabled: current.enabled, so any concurrent pause/resume causes a CONFLICT instead of writing a stale nextRunAt.
| type RouterOutputs = inferRouterOutputs<AppRouter>; | ||
| export type ScheduledTaskListItem = RouterOutputs["scheduledTask"]["list"][number]; | ||
|
|
||
| const TIMEZONES = [ |
There was a problem hiding this comment.
This hard-coded list is the only set the combobox can choose from (its search box filters options, it does not accept free text). On create, an admin whose browser is e.g. New York cannot schedule for valid zones such as America/Phoenix or America/Sao_Paulo, even though the server validation and docs accept arbitrary IANA timezones. Populate from a full timezone list or allow validated free-form input so daily/weekly tasks can target remote locales.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5dbbd71. The combobox now uses the runtime full Intl.supportedValuesOf("timeZone") set plus UTC, with a compatibility fallback for older engines and preservation of existing saved zones. A pure helper test verifies the full native set (including America/Phoenix) is exposed.
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
ScheduledTask/ScheduledTaskRunpersistence with interval, daily, and weekly timezone-aware schedulesVerification
pnpm exec prisma validate— passedpnpm exec prisma generate— passed0106_scheduled_tasks— applied successfully on the development stackpnpm lint— passed with existing native-select/type-import warnings onlypnpm typecheck— passedscheduled-taskrouter/service/schedule + sidebar) — 17/17 passedpnpm build:app— passed;/w/[slug]/scheduled-tasksemittedpnpm exec playwright test tests/e2e/scheduled-tasks.spec.ts— 1/1 passed after rebase/final fixesgit diff --check— passedReview hardening
An independent review found and the patch addresses: worker-crash recovery for abandoned RUNNING tasks, exclusion of archived workspaces, rejection of soft-deleted delivery projects, and pause/resume races around active runs.
Browser-suite note
The earlier full parallel Playwright run reached 35/40. The Scheduled Tasks strict-locator issue was fixed and reran green; three unrelated failures reran green individually. One pre-existing dispatch-default assertion remained affected by dirty idempotent E2E seed state. The new Scheduled Tasks lifecycle itself is green in a fresh isolated run.
Closes AXI-92.