From 22e007f1edc437a0bc7f6f1c5aee3fc8d397b470 Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Mon, 3 Aug 2026 19:23:54 +0300 Subject: [PATCH 1/2] feat(loops): change a live loop's retry budget in place Changing maxAttempts previously meant delete-and-recreate: the unsafe order, since a failed recreate loses the cadence entirely and `loops show` cannot read back a full definition to reconstruct it. With the default of 1 (store.ts createLoop), one transient failure pauses a loop forever, and a paused loop is indistinguishable from a quiet one from outside. The generic update plumbing already existed end to end -- Store.updateLoop, the LoopStore interface, both storage backends, and PATCH /v1/loops/{id}. It simply omitted maxAttempts from the patched field set. This widens that existing path rather than adding a parallel one, and exposes it as one CLI verb modelled on `rename`. - lib/loop-status.ts: assertMaxAttempts/isMaxAttempts, shared by both backends, rejecting anything that is not an integer >= 1 (0 or negative would make `attempt < maxAttempts` false forever, so a run could never be admitted or retried). - lib/store.ts + storage/postgres-loop-storage.ts: write max_attempts from the merged row, so an omitted key falls through to the current value. - api: accept maxAttempts on PATCH, 422 invalid_max_attempts otherwise. - openapi + regenerated src/sdk/http.ts so the typed contract carries it. - cli: `loops set-max-attempts `, using requireUniqueLoop so an ambiguous name errors rather than mutating the wrong loop, with no-op detection and a local-only pre-change backup. The default of 1 is deliberately NOT changed here: it would alter behaviour for every existing loop and every future caller. Tests assert against unmodified main: the in-place-change and validation cases fail without this commit at every layer; the omitted-key case is a guard against this commit wiping the column and passes on main by design. Agent: Silvanus --- openapi/loops.json | 5 ++ src/api/index.test.ts | 50 +++++++++++++ src/api/index.ts | 9 ++- src/cli/index.test.ts | 52 +++++++++++++ src/cli/index.ts | 36 +++++++++ src/lib/loop-status.ts | 14 ++++ src/lib/storage/postgres-loop-storage.ts | 10 ++- src/lib/store.test.ts | 93 ++++++++++++++++++++++++ src/lib/store.ts | 8 +- src/lib/store/index.ts | 6 +- src/sdk/http.ts | 2 +- 11 files changed, 272 insertions(+), 13 deletions(-) diff --git a/openapi/loops.json b/openapi/loops.json index 44a25bd..19aac0d 100644 --- a/openapi/loops.json +++ b/openapi/loops.json @@ -3801,6 +3801,11 @@ "type": "string", "nullable": true }, + "maxAttempts": { + "type": "integer", + "minimum": 1, + "description": "Retry budget for this loop: the scheduler admits a retry while attempt < maxAttempts, so 1 means a single transient failure retires the loop with no retry. Omit to leave the current budget unchanged." + }, "labels": { "type": "array", "items": { diff --git a/src/api/index.test.ts b/src/api/index.test.ts index fb248cb..40011d0 100644 --- a/src/api/index.test.ts +++ b/src/api/index.test.ts @@ -1579,6 +1579,56 @@ describe("loops-api foundation", () => { } }); + test("PATCH updates maxAttempts in place and rejects a non-integer budget with a stable 422", async () => { + const mod = await import("./index.js"); + const storage = createSqliteLoopStorage(":memory:"); + const server = createTestServer(mod, { host: "127.0.0.1", port: 0, storage }); + + try { + const loop = await storage.createLoop({ + name: "api-retry-budget", + schedule: { type: "once", at: "2027-01-01T00:00:00Z" }, + target: { type: "command", command: "true" }, + }, new Date("2026-01-01T00:00:00Z")); + expect(loop.maxAttempts).toBe(1); + + const ok = await fetch(apiUrl(server, `/v1/loops/${loop.id}`), { + method: "PATCH", + headers: jsonHeaders, + body: JSON.stringify({ maxAttempts: 3 }), + }); + expect(ok.status).toBe(200); + expect((await ok.json()).loop.maxAttempts).toBe(3); + expect((await storage.getLoop(loop.id))?.maxAttempts).toBe(3); + // The schedule must survive a retry-budget-only PATCH. + expect((await storage.getLoop(loop.id))?.nextRunAt).toBe(loop.nextRunAt); + + const before = await storage.getLoop(loop.id); + for (const maxAttempts of [0, -1, 1.5, "2", null, {}]) { + const response = await fetch(apiUrl(server, `/v1/loops/${loop.id}`), { + method: "PATCH", + headers: jsonHeaders, + body: JSON.stringify({ maxAttempts, labels: ["mutated"] }), + }); + expect(response.status).toBe(422); + expect(await response.json()).toEqual({ ok: false, error: "invalid_max_attempts" }); + expect(await storage.getLoop(loop.id)).toEqual(before); + } + + // A PATCH that omits maxAttempts must not reset the budget. + const other = await fetch(apiUrl(server, `/v1/loops/${loop.id}`), { + method: "PATCH", + headers: jsonHeaders, + body: JSON.stringify({ status: "paused" }), + }); + expect(other.status).toBe(200); + expect((await storage.getLoop(loop.id))?.maxAttempts).toBe(3); + } finally { + server.stop(true); + await storage.close(); + } + }); + test("PATCH rejects every invalid loop status atomically with a stable 422", async () => { const mod = await import("./index.js"); const storage = createSqliteLoopStorage(":memory:"); diff --git a/src/api/index.ts b/src/api/index.ts index 6e9ffb0..2e55718 100644 --- a/src/api/index.ts +++ b/src/api/index.ts @@ -63,7 +63,7 @@ import { type CircuitBreakerThreshold, } from "../lib/advancement.js"; import { normalizeLoopLabels } from "../lib/labels.js"; -import { isLoopStatus, LOOP_STATUSES } from "../lib/loop-status.js"; +import { isLoopStatus, isMaxAttempts, LOOP_STATUSES } from "../lib/loop-status.js"; import { normalizeRunCompletion } from "../lib/run-completion.js"; import { scrubSecretsDeep } from "../lib/redact.js"; import type { LoopStorageContract } from "../lib/storage/contract.js"; @@ -486,17 +486,22 @@ async function handleLoopsRequest(ctx: V1RequestContext, segments: string[]): Pr nextRunAt: string | null; retryScheduledFor: string | null; expiresAt: string | null; + maxAttempts: unknown; }>; // Only forward keys the caller actually sent. Store.updateLoop merges // {...current, ...patch}, so a present-but-undefined key overrides the // current value: emitting all four keys unconditionally wiped omitted // schedule fields (and set status=NULL -> NOT NULL 500). A key set to // JSON null is an explicit clear (mapped to undefined -> merged to null). - const patch: Partial<{ status: LoopStatus; labels: string[]; nextRunAt: string; retryScheduledFor: string; expiresAt: string }> = {}; + const patch: Partial<{ status: LoopStatus; labels: string[]; nextRunAt: string; retryScheduledFor: string; expiresAt: string; maxAttempts: number }> = {}; if ("status" in body) { if (!isLoopStatus(body.status)) throw apiError("invalid_loop_status", 422); patch.status = body.status; } + if ("maxAttempts" in body) { + if (!isMaxAttempts(body.maxAttempts)) throw apiError("invalid_max_attempts", 422); + patch.maxAttempts = body.maxAttempts; + } if ("labels" in body) patch.labels = normalizedLabels(body.labels); if ("nextRunAt" in body) patch.nextRunAt = body.nextRunAt === null ? undefined : body.nextRunAt; if ("retryScheduledFor" in body) patch.retryScheduledFor = body.retryScheduledFor === null ? undefined : body.retryScheduledFor; diff --git a/src/cli/index.test.ts b/src/cli/index.test.ts index bd27544..d316acb 100644 --- a/src/cli/index.test.ts +++ b/src/cli/index.test.ts @@ -1140,6 +1140,58 @@ describe("loops CLI", () => { expect(oldName.status).not.toBe(0); }); + test("set-max-attempts changes only the retry budget and writes a backup", () => { + const dataDir = freshDataDir("loops-cli-max-attempts-"); + const create = runCli(dataDir, ["--json", "create", "command", "retry-budget-loop", "--at", futureAt(), "--cmd", "true"]); + expect(create.status).toBe(0); + const created = JSON.parse(create.stdout); + expect(created.maxAttempts).toBe(1); + + const set = runCli(dataDir, ["--json", "set-max-attempts", created.id, "3"]); + + expect(set.status).toBe(0); + const value = JSON.parse(set.stdout); + expect(value).toMatchObject({ + changed: true, + id: created.id, + previousMaxAttempts: 1, + maxAttempts: 3, + }); + expect(value.backupPath).toContain(join(dataDir, "backups")); + expect(existsSync(value.backupPath)).toBe(true); + + // Read it back through a separate process: the loop keeps its id, name, + // and schedule, which delete-and-recreate would not have. + const after = runCli(dataDir, ["--json", "show", created.id]); + expect(after.status).toBe(0); + const loop = JSON.parse(after.stdout); + expect(loop.id).toBe(created.id); + expect(loop.name).toBe("retry-budget-loop"); + expect(loop.maxAttempts).toBe(3); + expect(loop.schedule).toEqual(created.schedule); + }); + + test("set-max-attempts reports a no-op and rejects a budget below 1", () => { + const dataDir = freshDataDir("loops-cli-max-attempts-invalid-"); + const create = runCli(dataDir, ["--json", "create", "command", "budget-guard", "--at", futureAt(), "--cmd", "true", "--attempts", "2"]); + expect(create.status).toBe(0); + const created = JSON.parse(create.stdout); + expect(created.maxAttempts).toBe(2); + + const noop = runCli(dataDir, ["--json", "set-max-attempts", "budget-guard", "2"]); + expect(noop.status).toBe(0); + const noopValue = JSON.parse(noop.stdout); + expect(noopValue.changed).toBe(false); + expect(noopValue.backupPath).toBeUndefined(); + + for (const bad of ["0", "-1", "1.5", "abc"]) { + const rejected = runCli(dataDir, ["--json", "set-max-attempts", "budget-guard", bad]); + expect(rejected.status).not.toBe(0); + const still = JSON.parse(runCli(dataDir, ["--json", "show", "budget-guard"]).stdout); + expect(still.maxAttempts).toBe(2); + } + }); + test("rename reports no-op without writing a backup", () => { const dataDir = freshDataDir("loops-cli-rename-noop-"); const create = runCli(dataDir, ["create", "command", "stable-name", "--at", futureAt(), "--cmd", "true"]); diff --git a/src/cli/index.ts b/src/cli/index.ts index 0d6d929..ecf3b78 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -2653,6 +2653,42 @@ program ); }))); +program + .command("set-max-attempts ") + .description("change a loop's retry budget in place, without losing its id, schedule, runs, or history") + .action(runAction((idOrName, attempts) => withStore(async (store) => { + // requireUniqueLoop so an ambiguous name errors instead of mutating the + // newest same-named loop -- the same guard rename and pause/resume use. + const loop = await store.requireUniqueLoop(idOrName); + const previous = loop.maxAttempts; + const next = positiveInteger(String(attempts), ""); + if (next === undefined) throw new ValidationError("attempts must be an integer >= 1"); + + if (next === previous) { + print( + { changed: false, id: loop.id, maxAttempts: previous, loop: publicLoop(loop) }, + `${loop.id} unchanged (maxAttempts=${previous})`, + ); + return; + } + + // Backups protect the on-box sqlite file; there is nothing local to snapshot + // when the update is routed to the hosted API. + const backupPath = store.transport === "local" ? backupLoopsDatabase("set-max-attempts") : undefined; + const updated = await store.updateLoop(loop.id, { maxAttempts: next }); + print( + { + changed: true, + id: updated.id, + previousMaxAttempts: previous, + maxAttempts: updated.maxAttempts, + backupPath, + loop: publicLoop(updated), + }, + `${updated.id} maxAttempts ${previous} -> ${updated.maxAttempts}\nbackup=${backupPath ?? "skipped (recent backup exists)"}`, + ); + }))); + function updateStatus(idOrName: string, status: "paused" | "active" | "stopped"): Promise { return withStore(async (store) => { // requireUniqueLoop so an ambiguous name errors instead of mutating the diff --git a/src/lib/loop-status.ts b/src/lib/loop-status.ts index 815478e..b64ade9 100644 --- a/src/lib/loop-status.ts +++ b/src/lib/loop-status.ts @@ -12,3 +12,17 @@ export function assertLoopStatus(value: unknown): asserts value is LoopStatus { throw new ValidationError("loop status must be one of active, paused, stopped, expired"); } } + +export function isMaxAttempts(value: unknown): value is number { + return typeof value === "number" && Number.isSafeInteger(value) && value >= 1; +} + +// maxAttempts is the retry budget the scheduler compares an attempt number +// against (`attempt < loop.maxAttempts`), so 1 means "no retry" and 0 or a +// negative value would make every run unretryable AND unadmittable. Validate it +// in one place because both the sqlite and postgres backends write the column. +export function assertMaxAttempts(value: unknown): asserts value is number { + if (!isMaxAttempts(value)) { + throw new ValidationError("loop maxAttempts must be an integer >= 1"); + } +} diff --git a/src/lib/storage/postgres-loop-storage.ts b/src/lib/storage/postgres-loop-storage.ts index 7196e1a..091d97c 100644 --- a/src/lib/storage/postgres-loop-storage.ts +++ b/src/lib/storage/postgres-loop-storage.ts @@ -108,7 +108,7 @@ import type { } from "../../types.js"; import { normalizeRunReceipt } from "../run-receipts.js"; import { normalizeLoopLabels } from "../labels.js"; -import { assertLoopStatus } from "../loop-status.js"; +import { assertLoopStatus, assertMaxAttempts } from "../loop-status.js"; import { normalizeRunCompletion } from "../run-completion.js"; import type { PoolQueryClient, TypedQueryClient } from "../../generated/storage-kit/query.js"; import type { LoopStorageContract, LoopStorageMethodName } from "./contract.js"; @@ -618,6 +618,7 @@ export class PostgresLoopStorage implements LoopStorageContract { async updateLoop(...args: M<"updateLoop">["args"]): Promise["result"]> { const [id, patch, opts = {}] = args; if ("status" in patch && patch.status !== undefined) assertLoopStatus(patch.status); + if ("maxAttempts" in patch && patch.maxAttempts !== undefined) assertMaxAttempts(patch.maxAttempts); const updated = (opts.now ?? new Date()).toISOString(); return this.client.transaction(async (c) => { const current = await this.loadLoop(c, id); @@ -630,15 +631,16 @@ export class PostgresLoopStorage implements LoopStorageContract { updatedAt: updated, }; const res = await c.query( - `UPDATE loops SET status=$1, labels_json=$2::jsonb, next_run_at=$3, retry_scheduled_for=$4, expires_at=$5, updated_at=$6 - WHERE tenant_id = open_loops_current_tenant_id() AND id=$7 - AND ($8::text IS NULL OR EXISTS (SELECT 1 FROM daemon_lease WHERE tenant_id = open_loops_current_tenant_id() AND id=$8 AND expires_at > $9))`, + `UPDATE loops SET status=$1, labels_json=$2::jsonb, next_run_at=$3, retry_scheduled_for=$4, expires_at=$5, max_attempts=$6, updated_at=$7 + WHERE tenant_id = open_loops_current_tenant_id() AND id=$8 + AND ($9::text IS NULL OR EXISTS (SELECT 1 FROM daemon_lease WHERE tenant_id = open_loops_current_tenant_id() AND id=$9 AND expires_at > $10))`, [ merged.status, JSON.stringify(merged.labels), merged.nextRunAt ?? null, merged.retryScheduledFor ?? null, merged.expiresAt ?? null, + merged.maxAttempts, merged.updatedAt, id, opts.daemonLeaseId ?? null, diff --git a/src/lib/store.test.ts b/src/lib/store.test.ts index 5315a6a..a9ad86b 100644 --- a/src/lib/store.test.ts +++ b/src/lib/store.test.ts @@ -69,6 +69,99 @@ describe("Store", () => { } }); + test("updateLoop changes maxAttempts in place, keeping the loop's id, schedule and run history", () => { + const store = new Store(":memory:"); + try { + const loop = store.createLoop( + { + name: "retry-budget", + schedule: { type: "once", at: "2027-01-01T00:00:00Z" }, + target: { type: "command", command: "true" }, + }, + new Date("2026-01-01T00:00:00Z"), + ); + // The default is 1: one transient failure retires the loop with no retry. + expect(loop.maxAttempts).toBe(1); + store.claimRun(loop, "2027-01-01T00:00:00.000Z", "test"); + const runsBefore = store.listRuns({ loopId: loop.id }).length; + expect(runsBefore).toBe(1); + + const updated = store.updateLoop(loop.id, { maxAttempts: 3 }); + + expect(updated.maxAttempts).toBe(3); + expect(store.getLoop(loop.id)?.maxAttempts).toBe(3); + // In place: nothing that a delete-and-recreate would have destroyed moved. + expect(updated.id).toBe(loop.id); + expect(updated.name).toBe(loop.name); + expect(updated.schedule).toEqual(loop.schedule); + expect(updated.nextRunAt).toBe(loop.nextRunAt); + expect(updated.createdAt).toBe(loop.createdAt); + expect(store.listRuns({ loopId: loop.id }).length).toBe(runsBefore); + } finally { + store.close(); + } + }); + + test("updateLoop leaves maxAttempts untouched when the patch omits it", () => { + // Regression: updateLoop writes max_attempts unconditionally from the merged + // row, so an omitted key must fall through to the current value rather than + // resetting the retry budget. This is the same class of bug that once wiped + // omitted schedule fields on the /v1 PATCH path. + const store = new Store(":memory:"); + try { + const loop = store.createLoop( + { + name: "retry-budget-preserved", + maxAttempts: 5, + schedule: { type: "once", at: "2027-01-01T00:00:00Z" }, + target: { type: "command", command: "true" }, + }, + new Date("2026-01-01T00:00:00Z"), + ); + expect(loop.maxAttempts).toBe(5); + + store.updateLoop(loop.id, { labels: ["unrelated"] }); + expect(store.getLoop(loop.id)?.maxAttempts).toBe(5); + + store.updateLoop(loop.id, { status: "paused" }); + expect(store.getLoop(loop.id)?.maxAttempts).toBe(5); + } finally { + store.close(); + } + }); + + test("updateLoop rejects a maxAttempts that is not an integer >= 1, atomically", () => { + const store = new Store(":memory:"); + try { + const loop = store.createLoop( + { + name: "retry-budget-invalid", + maxAttempts: 4, + schedule: { type: "once", at: "2027-01-01T00:00:00Z" }, + target: { type: "command", command: "true" }, + }, + new Date("2026-01-01T00:00:00Z"), + ); + const before = store.getLoop(loop.id); + // 0 and negatives would make `attempt < maxAttempts` false forever, so a + // run could never be admitted or retried. + for (const maxAttempts of [0, -1, 1.5, "2", null, {}, Number.NaN]) { + expect(() => + store.updateLoop(loop.id, { + maxAttempts, + labels: ["mutated"], + } as unknown as Parameters[1]) + ).toThrow(ValidationError); + expect(store.getLoop(loop.id)).toEqual(before); + } + + expect(store.updateLoop(loop.id, { maxAttempts: 1 }).maxAttempts).toBe(1); + expect(store.updateLoop(loop.id, { maxAttempts: 10 }).maxAttempts).toBe(10); + } finally { + store.close(); + } + }); + test("updateLoop rejects erased invalid statuses atomically and accepts every canonical status", () => { const store = new Store(":memory:"); try { diff --git a/src/lib/store.ts b/src/lib/store.ts index 16080dd..a947abc 100644 --- a/src/lib/store.ts +++ b/src/lib/store.ts @@ -67,7 +67,7 @@ import { } from "./run-artifacts.js"; import { normalizeRunReceipt } from "./run-receipts.js"; import { normalizeLoopLabels } from "./labels.js"; -import { assertLoopStatus } from "./loop-status.js"; +import { assertLoopStatus, assertMaxAttempts } from "./loop-status.js"; import { normalizeRunCompletion } from "./run-completion.js"; import { runLocalCommand, todosCliArgs, todosMutationSummary } from "./route/todos-cli.js"; @@ -1838,11 +1838,12 @@ export class Store { updateLoop( id: string, - patch: Partial>, + patch: Partial>, opts: DaemonLeaseFence = {}, ): Loop { const updated = (opts.now ?? new Date()).toISOString(); if ("status" in patch && patch.status !== undefined) assertLoopStatus(patch.status); + if ("maxAttempts" in patch && patch.maxAttempts !== undefined) assertMaxAttempts(patch.maxAttempts); this.db.exec("BEGIN IMMEDIATE"); try { const current = this.getLoop(id); @@ -1859,7 +1860,7 @@ export class Store { const res = this.db .query( `UPDATE loops SET status=$status, labels_json=$labels, next_run_at=$nextRun, retry_scheduled_for=$retrySlot, - expires_at=$expiresAt, updated_at=$updated + expires_at=$expiresAt, max_attempts=$maxAttempts, updated_at=$updated WHERE id=$id AND ($daemonLeaseId IS NULL OR EXISTS ( SELECT 1 FROM daemon_lease WHERE id=$daemonLeaseId AND expires_at > $now @@ -1872,6 +1873,7 @@ export class Store { $nextRun: merged.nextRunAt ?? null, $retrySlot: merged.retryScheduledFor ?? null, $expiresAt: merged.expiresAt ?? null, + $maxAttempts: merged.maxAttempts, $updated: merged.updatedAt, $daemonLeaseId: opts.daemonLeaseId ?? null, $now: updated, diff --git a/src/lib/store/index.ts b/src/lib/store/index.ts index 6f59548..138d6d3 100644 --- a/src/lib/store/index.ts +++ b/src/lib/store/index.ts @@ -95,7 +95,7 @@ export interface LoopStore { name?: string; }): Promise; countLoops(status?: LoopStatus, opts?: { archived?: boolean; includeArchived?: boolean }): Promise; - updateLoop(id: string, patch: Partial>): Promise; + updateLoop(id: string, patch: Partial>): Promise; renameLoop(id: string, name: string): Promise; archiveLoop(idOrName: string): Promise; unarchiveLoop(idOrName: string): Promise; @@ -205,7 +205,7 @@ export class LocalStore implements LoopStore { } async updateLoop( id: string, - patch: Partial>, + patch: Partial>, ): Promise { return this.store.updateLoop(id, patch); } @@ -426,7 +426,7 @@ export class ApiStore implements LoopStore { } async updateLoop( id: string, - patch: Partial>, + patch: Partial>, ): Promise { // The `/v1` PATCH contract distinguishes a present-null nullable field (an // explicit clear) from an absent one (leave unchanged). Callers signal a diff --git a/src/sdk/http.ts b/src/sdk/http.ts index c65f2f7..a493c8a 100644 --- a/src/sdk/http.ts +++ b/src/sdk/http.ts @@ -27,7 +27,7 @@ export interface Loop { "id": string; "name": string; "description"?: string | n export interface CreateLoopInput { "name": string; "description"?: string; "labels"?: Array; "schedule": Record; "target": Record } -export interface UpdateLoopInput { "status"?: "active" | "paused" | "stopped" | "expired"; "nextRunAt"?: string | null; "retryScheduledFor"?: string | null; "expiresAt"?: string | null; "labels"?: Array } +export interface UpdateLoopInput { "status"?: "active" | "paused" | "stopped" | "expired"; "nextRunAt"?: string | null; "retryScheduledFor"?: string | null; "expiresAt"?: string | null; "maxAttempts"?: number; "labels"?: Array } export interface Run { "id": string; "loopId": string; "status": string; "attempt"?: number; "scheduledFor"?: string; "startedAt"?: string | null; "finishedAt"?: string | null } From 72e814b78640ab4bd3238644d8b8fb6ff9e4027c Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Mon, 3 Aug 2026 19:37:49 +0300 Subject: [PATCH 2/2] fix(scheduler): honor live retry budget updates Re-read the persisted loop after a failed inline tick so a retry budget raised during execution still stops later catch-up slots. Agent: unresolved-account001 --- src/lib/scheduler.test.ts | 42 +++++++++++++++++++++++++++++++++++++++ src/lib/scheduler.ts | 7 ++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/lib/scheduler.test.ts b/src/lib/scheduler.test.ts index 6dc1744..360712a 100644 --- a/src/lib/scheduler.test.ts +++ b/src/lib/scheduler.test.ts @@ -444,6 +444,48 @@ describe("scheduler", () => { } }); + test("catch_up all honors a retry budget raised while the first slot is running", async () => { + const store = new Store(":memory:"); + try { + const loop = store.createLoop( + { + name: "catch-up-live-retry-budget", + schedule: { type: "interval", everyMs: 1_000 }, + target: { type: "command", command: "true" }, + catchUp: "all", + catchUpLimit: 10, + maxAttempts: 1, + retryDelayMs: 5_000, + }, + new Date("2026-01-01T00:00:00Z"), + ); + const firstSlot = loop.nextRunAt!; + let raisedBudget = false; + + const out = await tick({ + store, + runnerId: "test", + now: () => new Date("2026-01-01T00:00:05Z"), + random: noJitter, + beforeFinalize: () => { + if (raisedBudget) return; + store.updateLoop(loop.id, { maxAttempts: 2 }); + raisedBudget = true; + }, + execute: async () => result("failed", "2026-01-01T00:00:05.000Z"), + }); + + expect(out.completed).toHaveLength(1); + expect(store.listRuns({ loopId: loop.id })).toHaveLength(1); + const updated = store.getLoop(loop.id); + expect(updated?.maxAttempts).toBe(2); + expect(updated?.retryScheduledFor).toBe(firstSlot); + expect(updated?.nextRunAt).toBe("2026-01-01T00:00:10.000Z"); + } finally { + store.close(); + } + }); + test("preserves retry intent for recovered abandoned runs before advancing later recovered slots", async () => { const store = new Store(":memory:"); try { diff --git a/src/lib/scheduler.ts b/src/lib/scheduler.ts index 3130187..080280c 100644 --- a/src/lib/scheduler.ts +++ b/src/lib/scheduler.ts @@ -601,7 +601,12 @@ export async function tick(deps: SchedulerDeps): Promise { loopSkips += 1; } else completed.push(run); // tick-only retry gate: see recoverAndExpire() doc comment. - if (["failed", "timed_out", "abandoned"].includes(run.status) && run.attempt < loop.maxAttempts) break; + // The retry budget can change while a run is executing, so decide from + // the persisted loop instead of the pre-run snapshot used by dueSlots(). + if (["failed", "timed_out", "abandoned"].includes(run.status)) { + const current = deps.store.getLoop(loop.id); + if (current && run.attempt < current.maxAttempts) break; + } } }