fix: address deep review — indexes, redis metadata, capacity, crash-retry, cascade (14 findings)

P1:
- F1: explicitly build Schedule/ScheduleRun indexes at engine start (autoIndex
  is off by default, so the unique idempotency + TTL indexes never existed)
- F2: persist scheduleId/scheduledFor through RedisJobStore deserialization
- F3: re-enter owner tenant context before getLimits in reconciliation
- F4: a scheduled HITL pause no longer decrements an interactive slot it never took
- F5: reserve-then-verify fireConcurrency cap (covers engine + run-now)
- F6: cascade schedule/run deletion on account delete

P2:
- F7: classify ambiguous loopback failures as interrupted (no false auto-disable)
- F8: crash-retryable bookkeeping (lastCountedFor idempotency + bookkept marker,
  reconciler catches un-bookkept terminal runs)
- F9: reconciliation fetches started + requires_action in separate budgeted,
  ordered buckets; add {status,firedAt} index
- F10: abort route finalizes scheduled runs (interrupted)
- F11: resolve attachments before inserting the started run (no phantom active run)
- F12: PATCH re-validates effective cadence against the current interval floor
- F13: dialog preserves stored weekly daysOfWeek when cadence untouched
- F14: schedules query refetches on focus/interval so panel status stays fresh

Plus e2e mock spec for the schedule CRUD round-trip. +4 method tests.
This commit is contained in:
Danny Avila 2026-07-21 20:13:15 -04:00
parent 51011e644b
commit 431547757f
19 changed files with 507 additions and 61 deletions

View file

@ -1485,7 +1485,12 @@ class AgentClient extends BaseClient {
// teardown (request.js pause branch / resume.js finally) that would otherwise
// release it, and `/resume` 429s under LIMIT_CONCURRENT_MESSAGES. Idempotent via
// the flag; if it fails here, the teardown still releases (it checks the flag).
if (!this.pendingRequestReleased) {
// A scheduled fire never acquired an interactive concurrency slot, so it must
// not release one on pause (that would clear a real user's counter). Mark it
// released so downstream teardown skips the decrement too.
if (this.options.req?._isScheduledFire) {
this.pendingRequestReleased = true;
} else if (!this.pendingRequestReleased) {
try {
await decrementPendingRequest(this.options.req?.user?.id);
this.pendingRequestReleased = true;