diff --git a/CHANGELOG.md b/CHANGELOG.md index fe5981f5..4c1e9a52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,25 @@ 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. +- **Event ingest hydrates the routed constitution.** `POST /v1/events` + attaches the selected agent's `agent_def` on the synthetic orchestrate + body. File-backed matches and tenant agents off the newest-200 catalog + page no longer 404 `agent not found`. When a file-backed id collides + with a tenant row, the file constitution is the one that runs (file + routing won). +- **Nested artifact routes honor `:cid`.** `GET`/`DELETE` + `/v1/conversations/:id/artifacts/:aid` (and `/raw`) 404 when the + conversation is missing or TUI-origin, or when the artifact belongs + to a different conversation. Matches the documented tenant+conversation + pair and the list/create prefix. Tenant-wide `/v1/artifacts/:aid` + is unchanged. - **Delegation pipeline memory is sibling-only.** The orchestrator's `pipeline-entries` probe now lists rows pinned to the active conversation (`conversation_id = ?`). Default `list_entries` still ORs in unscoped 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 83fd22d2..dc5d4ee6 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 bd84b8ff..bb29b6e9 100644 --- a/src/api_server.cpp +++ b/src/api_server.cpp @@ -5971,19 +5971,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 62fed9aa..bcae147b 100644 --- a/src/schedule_parser.cpp +++ b/src/schedule_parser.cpp @@ -549,4 +549,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 22f716f8..ddc850b7 100644 --- a/tests/test_schedule_parser.cpp +++ b/tests/test_schedule_parser.cpp @@ -268,3 +268,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); +}