fix(jobs): retry resets started_at/attempts/stalled_counter (#2783) (#2974)

* fix(jobs): retry resets started_at/attempts_made/attempts_started (#2783)

`gbrain jobs retry` re-queued a dead job by resetting status/error_text/
locks/delay/finished_at, but left started_at, attempts_made, and
attempts_started untouched. On re-claim, claim()'s
`started_at = COALESCE(started_at, now())` preserved the ORIGINAL
first-claim timestamp instead of re-stamping it. handleWallClockTimeouts()
anchors on `now() - started_at`: a retry issued more than timeout_ms * 2
after the original claim was immediately dead-lettered again in under a
second, with attempts_made already past max_attempts — making retry
useless for exactly the case it exists for (recovering work after an
outage that outlasted the job's timeout).

An explicit `jobs retry` is an operator asserting "run this fresh", so
retryJob now also clears started_at (NULL, re-stamped on next claim) and
resets attempts_made/attempts_started to 0.

Two new tests: direct assertion that retry resets all three columns, and
a full repro of the reported bug (wall-clock-killed job retried long
after the original claim now survives re-claim instead of being
immediately re-killed).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS

* fix(jobs): also reset stalled_counter on retry (#2783)

Codex review round 1 found the fix was incomplete: a job dead-lettered by
stall exhaustion (handleStalled() at stalled_counter + 1 >= max_stalled)
retained its exhausted stalled_counter across retry. The retried job's
very first lock expiry after re-claim would immediately re-satisfy the
dead-letter threshold, contradicting the same "run this fresh" intent the
started_at/attempts reset already established.

New test mirrors the existing wall-clock repro: exhaust the stall budget
via two real handleStalled() calls, retry, confirm one more stall now
requeues instead of dead-lettering again.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

src/core/minions/queue.ts

@@ -471,12 +471,34 @@ export class MinionQueue {
     });
   }
 
-  /** Re-queue a failed or dead job for retry. */
+  /**
+   * Re-queue a failed or dead job for retry.
+   *
+   * #2783: an explicit `jobs retry` is an operator asserting "run this
+   * fresh" — so it clears `started_at` (re-stamped on re-claim via
+   * `claim()`'s `COALESCE(started_at, now())`, `queue.ts:620`) and resets
+   * `attempts_made`/`attempts_started` to 0. Without this, `started_at`
+   * kept the ORIGINAL first-claim time, so `handleWallClockTimeouts()`
+   * (anchored on `now() - started_at`, `queue.ts:729-749`) could measure
+   * from long before the retry — a retry issued more than `timeout_ms * 2`
+   * after the original claim was dead-lettered again in under a second,
+   * with `attempts_made` already past `max_attempts`. This made retry
+   * useless for exactly the case it exists for: recovering work after an
+   * outage that outlasted the job's timeout.
+   *
+   * Also resets `stalled_counter` (Codex review): `handleStalled()`
+   * dead-letters once `stalled_counter + 1 >= max_stalled` (`queue.ts:1190`).
+   * A job dead-lettered BY stall exhaustion, left un-reset, would hit that
+   * same threshold on its very first lock expiry after retry — a job
+   * killed by 3 stalls doesn't get a fresh stall budget, contradicting
+   * "run this fresh" the same way the unreset attempt counters did.
+   */
   async retryJob(id: number): Promise<MinionJob | null> {
     const rows = await this.engine.executeRaw<Record<string, unknown>>(
       `UPDATE minion_jobs SET status = 'waiting', error_text = NULL,
         lock_token = NULL, lock_until = NULL, delay_until = NULL,
-        finished_at = NULL, updated_at = now()
+        finished_at = NULL, started_at = NULL, attempts_made = 0,
+        attempts_started = 0, stalled_counter = 0, updated_at = now()
        WHERE id = $1 AND status IN ('failed', 'dead')
        RETURNING *`,
       [id]

test/minions.test.ts

@@ -735,6 +735,99 @@ describe('MinionQueue: Cancel & Retry', () => {
     expect(retried!.status).toBe('waiting');
     expect(retried!.error_text).toBeNull();
   });
+
+  // #2783: retry must reset started_at/attempts_made/attempts_started/
+  // stalled_counter — an explicit `jobs retry` is an operator asserting
+  // "run this fresh".
+  test('retry resets started_at/attempts_made/attempts_started/stalled_counter', async () => {
+    const job = await queue.add('sync', {}, { max_attempts: 3, max_stalled: 3 });
+    await queue.claim('tok1', 30000, 'default', ['sync']);
+    await queue.failJob(job.id, 'tok1', 'error', 'dead');
+    // Simulate the original claim having stamped started_at long ago,
+    // attempts already elevated, and a near-exhausted stall budget —
+    // matching what a real dead job (killed by wall-clock OR by stall
+    // exhaustion) looks like.
+    await engine.executeRaw(
+      "UPDATE minion_jobs SET started_at = now() - interval '1 hour', stalled_counter = 2 WHERE id = $1",
+      [job.id],
+    );
+    const retried = await queue.retryJob(job.id);
+    expect(retried!.status).toBe('waiting');
+    expect(retried!.started_at).toBeNull();
+    expect(retried!.attempts_made).toBe(0);
+    expect(retried!.attempts_started).toBe(0);
+    expect(retried!.stalled_counter).toBe(0);
+  });
+
+  // #2783 repro: retry issued long after the original claim must NOT be
+  // immediately dead-lettered by the wall-clock sweep on re-claim.
+  test('retry survives handleWallClockTimeouts after re-claim, even long after the original attempt', async () => {
+    const job = await queue.add('sync', {}, { max_attempts: 3 });
+    await engine.executeRaw('UPDATE minion_jobs SET timeout_ms = 1000 WHERE id = $1', [job.id]);
+    await queue.claim('tok1', 30000, 'default', ['sync']);
+    // Original attempt dies from a wall-clock timeout — matches the issue's
+    // repro (an outage that outlasts timeout_ms).
+    await engine.executeRaw(
+      "UPDATE minion_jobs SET started_at = now() - interval '10 seconds' WHERE id = $1",
+      [job.id],
+    );
+    const firstDead = await queue.handleWallClockTimeouts(30000);
+    expect(firstDead.length).toBe(1);
+    expect(firstDead[0].status).toBe('dead');
+
+    // Outage clears; operator retries — LONG after the original claim time
+    // (this is the exact scenario that used to dead-letter in <1s: without
+    // the fix, started_at would still be the original claim's timestamp).
+    await queue.retryJob(job.id);
+    const reclaimed = await queue.claim('tok2', 30000, 'default', ['sync']);
+    expect(reclaimed).not.toBeNull();
+    expect(reclaimed!.attempts_made).toBe(0);
+
+    // The sweep must NOT kill it immediately this time — started_at was
+    // re-stamped fresh on re-claim (claim()'s COALESCE(started_at, now())).
+    const stillAlive = await queue.handleWallClockTimeouts(30000);
+    expect(stillAlive.length).toBe(0);
+    expect((await queue.getJob(job.id))!.status).toBe('active');
+  });
+
+  // #2783 repro (stall side): a job dead-lettered by stall exhaustion must
+  // get a fresh stall budget on retry, not immediately re-die on its first
+  // stall after being re-claimed.
+  test('retry survives one stall after re-claim, even after the original stall budget was exhausted', async () => {
+    const job = await queue.add('sync', {}, { max_attempts: 3, max_stalled: 2 });
+
+    // Exhaust the stall budget the same way the existing stall test does:
+    // one requeue stall, then one dead-lettering stall.
+    await queue.claim('tok1', 30000, 'default', ['sync']);
+    await engine.executeRaw(
+      "UPDATE minion_jobs SET lock_until = now() - interval '1 second' WHERE id = $1",
+      [job.id],
+    );
+    await queue.handleStalled();
+    await queue.claim('tok2', 30000, 'default', ['sync']);
+    await engine.executeRaw(
+      "UPDATE minion_jobs SET lock_until = now() - interval '1 second' WHERE id = $1",
+      [job.id],
+    );
+    const r2 = await queue.handleStalled();
+    expect(r2.dead.length).toBe(1);
+    expect(r2.dead[0].status).toBe('dead');
+    expect(r2.dead[0].stalled_counter).toBe(2); // == max_stalled — exhausted
+
+    // Operator retries. Without the stalled_counter reset, the very next
+    // stall would immediately satisfy `stalled_counter + 1 >= max_stalled`
+    // and dead-letter again despite "run this fresh".
+    const retried = await queue.retryJob(job.id);
+    expect(retried!.stalled_counter).toBe(0);
+    await queue.claim('tok3', 30000, 'default', ['sync']);
+    await engine.executeRaw(
+      "UPDATE minion_jobs SET lock_until = now() - interval '1 second' WHERE id = $1",
+      [job.id],
+    );
+    const r3 = await queue.handleStalled();
+    expect(r3.requeued.length).toBe(1); // fresh budget — requeued, not dead
+    expect(r3.dead.length).toBe(0);
+  });
 });
 
 // --- Pause / Resume (5 tests) ---