Repository navigation
feat(server): archive stale scheduled tasks and delete them 30 days later - #222
Conversation
…ater Scheduled tasks that stop doing anything piled up forever. An hourly sweep on the existing scheduler tick now archives a task (turns it off and records why) after 10 days as a fired one-time task, paused task, failing task (3+ consecutive failures over 10 days) or task whose thread was archived, and a never-triggered webhook after 30 days. Archived tasks are deleted 30 days after archiving, with their webhook deliveries. Re-enabling or saving restores one, and a restored webhook keeps its URL. Fork migration 23 adds the archive and failure-counter columns and sets agent tasks that legacy imports left on new_thread to continue. Thread auto-archive now skips threads with an enabled bound schedule, which would otherwise fail every later run. Web and mobile Settings get an Active/Archived filter with the reason, the deletion date and Restore. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
jayleaton
left a comment
There was a problem hiding this comment.
VERDICT #222: CHANGES
Confirmed the inline finding with a deterministic service test. Reviewed head 539ab4c.
Repository checks against main passed apart from a missing local Chromium executable; after installing it, the 2 browser files passed (9 tests). The other 17 test files passed (263 tests). Format, lint, knip, typecheck and React compiler checks passed.
… started The sweep reads task rows and active runs once. A manual run that started while the sweep waited on an earlier row's thread lookup left archived_at unchanged, so the delete removed the running task. The delete now requires the row's read updated_at and a non-running status, and holds the run reservation so a run cannot start mid-delete. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Fixed in e1e6d06: the cleanup sweep no longer deletes a task whose manual run starts after its snapshot. |
jayleaton
left a comment
There was a problem hiding this comment.
VERDICT #222: CHANGES
The original regression now fails before the fix and passes after. check:changed against main passes, including lint, typecheck and all 273 focused tests. The new run reservation introduces a separate restored-webhook delivery race, reproduced before/after and described inline.
…'s delivery Cleanup took the run reservation while deleting, so a webhook restored and triggered in that window was refused as already running and its accepted delivery was recorded as dispatch_failed. Cleanup now only skips tasks that are running; the updated_at and status guards on the delete already drop it when a run, restore or edit lands first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Fixed in 278f960: cleanup no longer reserves the task, so a webhook restored while the sweep is about to delete it still runs its accepted delivery. |
jayleaton
left a comment
There was a problem hiding this comment.
VERDICT #222: CHANGES
The restored-webhook issue is fixed: Doug’s regression fails on e1e6d06 and passes on this head, and Randy’s previous webhook reproduction passes too. The original manual-run regression passes. Both basic orders after the in-memory running check are safe: a persisted start prevents deletion, and deletion before the start UPDATE aborts dispatch if the ID stays absent. A same-ID recreation in that last gap exposes the additional race described inline.
check:changed against main passes, including formatting/lint, knip, GraphQL, 41-package typecheck, all focused tests and React compiler checks. The isolated row-generation-guard experiment passes all five targeted checks; the proposed guard was reverted locally afterward. No commits, pushes or merges.
RANGE-DIFF #224: PURE — e1e6d06..e7d0f17 versus 278f960..b4cce2f has two '=' entries.
RANGE-DIFF #226: PURE — e7d0f17..6d40a19 versus b4cce2f..8a47c72 has three '=' entries.
A run reads the task, then marks it running by id alone. If the task was deleted and recreated under the same id in between, as cleanup plus an idempotent create replay can do, the old run started on the replacement and stamped its history. The start now also matches the created_at it read and aborts when the row was replaced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Fixed in 3be595a: a run's start now matches the task's created_at, so it can't start on a task deleted and recreated under the same id. |
jayleaton
left a comment
There was a problem hiding this comment.
VERDICT #222: MERGE
Reviewed 3be595a. The start UPDATE now matches the ID and created_at read by runTask, so deletion/recreation returns “Schedule task not found” before dispatch and cannot stamp the replacement. Doug’s recreation regression fails against 278f960 and passes here. All eight targeted checks pass: the original manual-run race; both write orderings after cleanup’s running check; same-ID recreation; the earlier restored-webhook regression and Randy reproduction; and the failure-path extension.
The stuck-run argument holds under the service’s run reservation: the replacement cannot start while the old run is exiting, so it remains “never” and fails releaseStuckRun’s running-status predicate. I verified a replacement start is rejected during that window, its history remains unchanged after the old run fails, and a subsequent replacement start succeeds normally. No remaining race or new finding identified in this change.
check:changed against main passes: formatting, lint, knip, GraphQL, 41-package typecheck, 19 focused files (275 tests), and React compiler checks. All commands ran through heavy.sh. No commits, pushes or merges.
RANGE-DIFF #224: PURE — e1e6d06..e7d0f17 versus 3be595a..e3fbc4e has two '=' entries.
RANGE-DIFF #226: PURE — e7d0f17..6d40a19 versus e3fbc4e..d3303ea has three '=' entries.
Summary
Scheduled tasks never went away: fired one-shots, paused tasks and tasks failing every run stayed in every list. This adds a reversible cleanup rule on the existing scheduler tick.
enabled = 0+archived_at+archive_reason, guarded onupdated_atso a concurrent edit or run wins. Tokens are untouched, so a restored webhook keeps its URL.setEnabled(true)and any save clear the archive (and the old failure count).consecutive_failures,failing_since) are written bymarkCompleted,releaseStuckRunand restart recovery; a success resets both.thread_mode = 'continue'for profile rows. The legacy V1 import now writescontinuetoo (it left them onnew_thread).getAutoArchiveCandidatesskips threads with an enabled bound task, which would otherwise fail every later run.archivedAt/archiveReasononScheduledTask(optional, so older clients just see a paused task). Web and mobile Settings get an Active/Archived filter with the reason, "Deletes on " and Restore (a past one-shot opens the editor). Thread panel and Agents board hide archived tasks. Shared helpers live inpackages/client-runtime/src/scheduledTaskArchive.ts.docs/user/project-settings.md(new "Stale tasks are archived"),docs/user/thread-sidebar.md.Windows are fixed (10 days, 30 for unused webhooks, 30-day retention) per JJ's decisions; there is no setting.
Evidence
The 11 cases from the design, written first:
Cannot find module './cleanup.ts'(decision table),no such column: archived_at(service, webhook, migration cases),expected 'new_thread' to equal 'continue'(legacy import), auto-archive candidatesexpected [ …(6) ] to deeply equal [ …(5) ].vp test runon cleanup, ScheduledTaskService (base/webhook/profile), fork 023 migration and ProjectionSettlement: 6 files, 105 tests passed. Wider:src/scheduledTasks, ForkMigrations, Migrations, ThreadSettlementService, runtimeLayer, ProjectionSettlement: 17 files, 411 passed.TestClock.setTime/adjust; webhook cases use the mocked scheduler so no ticks run. No sleeps.vp run check:changed: format, lint, knip, typecheck (41 packages), tests beside changes (19 files) and react compiler all pass.scheduledTaskArchive.test.ts(9), web logic +ScheduledTasksSettings.browser.test.tsx(34) pass.Real-client captures for the Archived filter are added below after the web pass.
Merge Danger
Door: one-way for deleted rows. Archiving is reversible; deletion 30 days after archive is not. The migration only adds columns and updates
thread_modeon profile rows (which the service already forced tocontinueon save).Blast Radius: service
The first sweep after deploy archives existing stale tasks (on JJ's live data, the 11-day-old fired one-shot immediately). Nothing is deleted for at least 30 days after that.
Stack: base of three. PR 2 (MCP) and PR 3 (UI unification) sit on this.
Implemented by Claude Opus 5.5 in Claude Code (T3 Code).
Captures (web dev client, seeded data)
These show the Settings list with this PR's Active/Archived toggle. #226 replaces the toggle with the full filter and fixes how it overlaps the heading at 390px.
🤖 Generated with Claude Code