From e2200a5858818334e518f24af97ab0c7234eced1 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 13 Sep 2026 06:19:44 +0000 Subject: [PATCH] Keep /schedule resume paused-only so completed one-shots stay terminal. A successful one-shot leaves next_fire_at in the past. Resume used to set status=active and next=now+1 for any non-running row, so an agent could re-queue finished work. Gate pause/resume on status so terminal rows cannot take the pause-then-resume path either. Operators still PATCH failed one-shots to active. Co-authored-by: Tyler Reckart --- CHANGELOG.md | 9 +++++++++ docs/concepts/scheduler.md | 2 ++ include/commands.h | 4 ++-- include/schedule_parser.h | 10 ++++++++++ src/api_server.cpp | 20 ++++++++++++-------- src/schedule_parser.cpp | 27 +++++++++++++++++++++++++++ tests/test_schedule_parser.cpp | 28 ++++++++++++++++++++++++++++ 7 files changed, 90 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dae12b43..67948761 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ loosely while pre-1.0 (breaking changes can land on minor bumps). ## [Unreleased] +### Fixed +- **`/schedule resume` is paused-only.** The writ no longer flips + `completed`, `failed`, `canceled`, `running`, or already-`active` rows + back to `active`, and `/schedule pause` refuses those terminal + statuses so pause-then-resume cannot re-queue a finished one-shot + (`next_fire_at` is still in the past after a successful fire; the old + resume path set `next=now+1`). Recreate a finished one-shot to run it + again; operators still PATCH failed one-shots to `active` to retry. + ## [0.13.4] — 2026-09-10 ### Fixed diff --git a/docs/concepts/scheduler.md b/docs/concepts/scheduler.md index 20128e3c..5f907ed2 100644 --- a/docs/concepts/scheduler.md +++ b/docs/concepts/scheduler.md @@ -57,6 +57,8 @@ Agents emit: /schedule resume 42 ``` +`/schedule resume` only reactivates a `paused` row, and `/schedule pause` refuses `completed` / `failed` / `canceled` so pause-then-resume cannot re-queue a finished one-shot. Recreate it to run again (operators can still `PATCH` a failed one-shot back to `active`). + The `:` separates the schedule phrase from the message that the agent will see when the task fires. The scheduled task targets the calling agent by default — a `scout` agent emitting `/schedule …` queues work for `scout`. Other agents are addressable via the HTTP `POST /v1/schedules` endpoint with an `agent` field. A successful create returns: diff --git a/include/commands.h b/include/commands.h index e15a0f1c..0c88ea82 100644 --- a/include/commands.h +++ b/include/commands.h @@ -414,8 +414,8 @@ using TodoInvoker = std::function: — create a scheduled task // /schedule list — render the active schedules // /schedule cancel — delete a scheduled task -// /schedule pause — set status='paused' -// /schedule resume — set status='active' and recompute next_fire_at +// /schedule pause — active/running → paused (not terminal) +// /schedule resume — paused → active; recompute next_fire_at if due // The callback receives (kind, rest-of-line, caller_agent_id) where kind // is the leading subcommand keyword and rest-of-line is everything after // it. For the implicit "create" form (no recognised subcommand), kind diff --git a/include/schedule_parser.h b/include/schedule_parser.h index 640f6670..acd39f5e 100644 --- a/include/schedule_parser.h +++ b/include/schedule_parser.h @@ -66,4 +66,14 @@ int64_t next_fire_for_recur(const std::string& recur_json, int64_t after); // trip to the human. std::string schedule_parser_help(); +// /schedule pause / resume status gates. Empty return = allowed. +// Terminal one-shots (completed / failed / canceled) stay terminal so +// pause-then-resume cannot re-queue them: next_fire_at is still in the +// past after a successful fire, and the old resume writ set next=now+1. +// Failed one-shots stay on HTTP PATCH-to-active, which scheduler.h +// documents as the operator retry. Pause of running is allowed (the +// in-flight finalize CAS will not clobber it). +std::string schedule_pause_block_reason(const std::string& status); +std::string schedule_resume_block_reason(const std::string& status); + } // namespace arbiter diff --git a/src/api_server.cpp b/src/api_server.cpp index 019b9b40..d76670a6 100644 --- a/src/api_server.cpp +++ b/src/api_server.cpp @@ -5708,19 +5708,23 @@ SchedulerInvoker make_scheduler_invoker_callback( return "ERR: schedule #" + std::to_string(id) + " not found"; } - // Pause or resume: PATCH status. Resume also recomputes - // next_fire_at for a recurring task whose previous fire is - // now in the past. + // Pause or resume: PATCH status. Terminal one-shots stay + // terminal so pause-then-resume cannot re-queue them + // (next_fire_at is still in the past after a successful + // fire). Recurring resume still recomputes next_fire_at + // when the previous fire is now in the past. std::string new_status = (kind == "pause") ? "paused" : "active"; std::optional next; - if (kind == "resume") { + if (kind == "pause" || kind == "resume") { auto row = tenants.get_scheduled_task(tenant_id, id); if (!row) return "ERR: schedule #" + std::to_string(id) + " not found"; - if (row->status == "running") { - return "ERR: schedule #" + std::to_string(id) + - " is running; wait for completion before resuming"; + const std::string reason = (kind == "pause") + ? schedule_pause_block_reason(row->status) + : schedule_resume_block_reason(row->status); + if (!reason.empty()) { + return "ERR: schedule #" + std::to_string(id) + " " + reason; } - if (row->next_fire_at <= now) { + if (kind == "resume" && row->next_fire_at <= now) { if (row->schedule_kind == "recurring") { int64_t n = next_fire_for_recur(row->recur_json, now); if (n > 0) next = n; diff --git a/src/schedule_parser.cpp b/src/schedule_parser.cpp index d1cc9a96..341b6aab 100644 --- a/src/schedule_parser.cpp +++ b/src/schedule_parser.cpp @@ -497,4 +497,31 @@ std::string schedule_parser_help() { " every (Mon|Tue|...) [at HH:MM]"; } +std::string schedule_pause_block_reason(const std::string& status) { + if (status == "active" || status == "running" || status == "paused") + return {}; + if (status == "completed") + return "is completed and cannot be paused"; + if (status == "failed") + return "is failed and cannot be paused"; + if (status == "canceled") + return "is canceled"; + return "has status '" + status + "' and cannot be paused"; +} + +std::string schedule_resume_block_reason(const std::string& status) { + if (status == "paused") return {}; + if (status == "running") + return "is running; wait for completion before resuming"; + if (status == "completed") + return "is completed; recreate the schedule to run it again"; + if (status == "failed") + return "is failed; recreate the schedule to retry"; + if (status == "canceled") + return "is canceled"; + if (status == "active") + return "is already active"; + return "has status '" + status + "' and cannot be resumed"; +} + } // namespace arbiter diff --git a/tests/test_schedule_parser.cpp b/tests/test_schedule_parser.cpp index 4f3795a3..bf0f45e3 100644 --- a/tests/test_schedule_parser.cpp +++ b/tests/test_schedule_parser.cpp @@ -223,3 +223,31 @@ TEST_CASE("parse: huge intervals fail closed without throwing") { CHECK(r.spec.fire_at == now + 2 * 3600); } } + +TEST_CASE("/schedule pause/resume refuse terminal statuses") { + CHECK(schedule_resume_block_reason("paused").empty()); + CHECK(schedule_pause_block_reason("paused").empty()); + CHECK(schedule_pause_block_reason("active").empty()); + CHECK(schedule_pause_block_reason("running").empty()); + + CHECK(schedule_resume_block_reason("completed").find("completed") + != std::string::npos); + CHECK(schedule_pause_block_reason("completed").find("completed") + != std::string::npos); + CHECK(schedule_resume_block_reason("failed").find("failed") + != std::string::npos); + CHECK(schedule_pause_block_reason("failed").find("failed") + != std::string::npos); + CHECK(schedule_resume_block_reason("canceled").find("canceled") + != std::string::npos); + CHECK(schedule_pause_block_reason("canceled").find("canceled") + != std::string::npos); + CHECK(schedule_resume_block_reason("active").find("already active") + != std::string::npos); + CHECK(schedule_resume_block_reason("running").find("running") + != std::string::npos); + CHECK(schedule_resume_block_reason("bogus").find("cannot be resumed") + != std::string::npos); + CHECK(schedule_pause_block_reason("bogus").find("cannot be paused") + != std::string::npos); +}