feat(loops): change a live loop retry budget in place

[andrei-hasna]

The question

Can an operator change a live loop's maxAttempts without delete-and-recreate?

Today: no. loops --help has rename but no update, so the retry budget is the one policy field you cannot edit in place. Changing it means delete-then-create — the unsafe order, since a failed recreate loses the cadence entirely and loops show cannot read back a full definition to reconstruct it.

That matters because the default is 1 (store.ts createLoop: input.maxAttempts ?? 1), so a single transient failure retires a loop with no retry and no self-heal, and a paused loop is indistinguishable from a quiet one from outside. Measured on the installed 0.4.38 CLI: 1311 of 1469 loops sit at maxAttempts: 1.

What already existed (and why this PR is small)

The generic update plumbing is already end-to-end: Store.updateLoop, the LoopStore interface, both storage backends, and a live PATCH /v1/loops/{id}. It simply omitted maxAttempts from the patched field set. So this widens the existing path rather than adding a parallel one.

The only pre-existing route to the column was loops export -> edit -> loops import --replace, which is a whole-bundle migration round-trip (and --allow-redacted bundles are explicitly non-importable). That is not an operator-safe way to change one field on one loop.

Changes

• lib/loop-status.ts — assertMaxAttempts/isMaxAttempts, shared by both backends. Rejects anything that is not an integer >= 1: the scheduler admits a retry while attempt < maxAttempts, so 0 or negative would make every run permanently unadmittable.
• 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/index.ts — accept maxAttempts on PATCH; 422 invalid_max_attempts otherwise.
• openapi/loops.json + regenerated src/sdk/http.ts — the typed contract carries the field (spec is the source of truth via scripts/gen-sdk.ts).
• cli/index.ts — loops set-max-attempts <idOrName> <attempts>.

Why a narrow verb rather than a generic update

The CLI idiom here is one verb per intent — rename, pause, resume, stop, labels — and none of the five existing PATCH fields is exposed as a generic verb. set-max-attempts is a structural twin of rename: same <idOrName> resolution via requireUniqueLoop (an ambiguous name errors rather than mutating the wrong loop), same no-op detection, same local-only pre-change backup. It also matches create --attempts.

Tests — asserted against unmodified main, not just against themselves

Every new test was run on a baseline worktree at origin/main (c780b77) with only the test files copied in:

layer	on this branch	on unmodified main
store.test.ts	3 pass 0 fail	1 pass 2 fail
api/index.test.ts	1 pass 0 fail	0 pass 1 fail
cli/index.test.ts	2 pass 0 fail	0 pass 2 fail

The one store test that passes on main is intentional: "updateLoop leaves maxAttempts untouched when the patch omits it" guards against this PR wiping the column now that max_attempts is written unconditionally from the merged row — the same class of bug that once wiped omitted schedule fields on the PATCH path. It has nothing to prove on main.

Full suite on this branch: 1141 pass, 56 skip, 0 fail. typecheck rc=0, test:boundary rc=0.

Real acceptance path (built CLI, not just unit tests)

Run against an isolated temp LOOPS_DATA_DIR with HASNA_LOOPS_API_URL/HASNA_LOOPS_API_KEY scrubbed — both are set on station01, so a bare run would have routed writes to the hosted control plane. Isolation was verified before any write (sourceOfTruth: local_sqlite, controlPlane.configured: false) and after (live ~/.hasna/loops unchanged: 54 files, loops.db mtime identical).

create                  -> maxAttempts 1
set-max-attempts ... 4  -> changed=true, prev=1, now=4
show (new process)      -> maxAttempts 4, id and nextRunAt preserved
set-max-attempts ... 0  -> rc=1, "error: <attempts> must be a positive integer"
set-max-attempts ... 4  -> changed=false (no-op, no backup)

Deliberately NOT in this PR

The default stays 1. Changing it would alter behaviour for 1,469 existing rows and every future caller; that is a separate decision, not a drive-by.

Agent: Silvanus

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[andrei-hasna]

[REVIEW] NO_GO — #192 @ 22e007f — lens: correctness+security+gates, reviewer unresolved-account001 (1 of 1)

Reviewed:

• Confirmed HEAD is 22e007f and the freshly fetched origin/main base is c780b77.
• Read git log origin/main..HEAD, the full origin/main...HEAD diff for all 11 changed files, and surrounding CLI, HTTP API, SQLite, PostgreSQL, scheduler/advancement, store abstraction, OpenAPI/SDK, schema, and test code.
• Manually traced validation, atomicity, archived-loop behavior, tenant scoping, parameterized SQL, retry scheduling, and live-update concurrency.

Commands and gates:

• bun install — exit 0. Setup only, not reported as the repository test gate; 152 packages installed and the prepare hook completed.
• bun run typecheck — exit 0. PASS; TypeScript emitted no diagnostics and no numeric pass/fail summary.
• bun run test — exit 0. PASS; 1,141 pass, 56 skip, 0 fail across 1,197 tests.
• git diff --check origin/main...HEAD — exit 0.
• Focused live-update behavioral probe with Bun — exit 0 and reproduced the blocker below: 5 later catch-up slots executed while retryScheduledFor remained pinned to the first failed slot.

Blocking P0/P1 findings:

1. [P1, high confidence] Raising maxAttempts during an active catch-up tick bypasses the existing retry-order gate — src/lib/scheduler.ts tick loop.
   Reachable path: create an interval loop with catchUp="all" and maxAttempts=1; start tick() with multiple overdue slots; while the first run is active, use the new supported update path to set maxAttempts=2; let that run fail. advanceLoop() re-reads the loop and correctly schedules a retry under the new budget, but tick() then checks run.attempt < loop.maxAttempts against the stale pre-update loop snapshot (1). It therefore continues and executes every later due slot instead of stopping behind the owed retry.
   Evidence: the focused probe raised the budget in beforeFinalize and produced completed=5 with five failed attempt-1 slots, while persisted maxAttempts=2 and retryScheduledFor still named the first slot. The repository already defines the required invariant in src/lib/scheduler.test.ts: "catch_up all stops processing later slots when an earlier slot needs retry".
   Impact: a live retry-budget update can execute ordered command/workflow side effects for later schedule slots before the earlier slot's promised retry, up to the loop's catch-up limit.
   Minimal remedy: after each completed failure, make tick() evaluate retryability from current persisted loop state, and add a regression test that changes maxAttempts during finalization and proves only the first slot executes.

Non-blocking follow-ups:

• The declared suite skipped its 56 live PostgreSQL tests because that optional test environment was unavailable. The changed PostgreSQL statement was manually checked for tenant fencing and placeholder/value alignment; no separate P2/P3 finding is recorded.

[andrei-hasna]

[REVIEW] GO — #192 @ 72e814b — lens: correctness+security+gates, reviewer unresolved-account001 (1 of 1)

Focused remediation verification for the single P1 named at 22e007f:

• Added a regression that raises maxAttempts from 1 to 2 while the first catchUp="all" slot is running.
• Before the fix: bun run test src/lib/scheduler.test.ts — exit 1; 30 pass, 1 fail. The regression observed 5 completed slots instead of 1.
• Fix: tick() now re-reads the persisted loop only after a failed terminal run and uses its current maxAttempts at the retry-order gate.
• After the fix: bun run test src/lib/scheduler.test.ts — exit 0; 31 pass, 0 fail.
• bun run typecheck — exit 0; no diagnostics.
• bun run test — exit 0; 1,142 pass, 56 skip, 0 fail across 1,198 tests.
• shield review on the staged two-file fix — exit 0; no security issues found.
• git diff --cached --check — exit 0.
• Commit trailer verified as Agent: unresolved-account001.
• Push succeeded to feat/loops-set-max-attempts.

Blocking P0/P1 findings: none remain. The original NO_GO applies to 22e007f and is stale at this head.
Non-blocking follow-ups: unchanged; the declared suite skips 56 live PostgreSQL tests when that optional environment is unavailable.