diff --git a/src/core/minions/queue.ts b/src/core/minions/queue.ts index 70234d276..e21eb8a41 100644 --- a/src/core/minions/queue.ts +++ b/src/core/minions/queue.ts @@ -647,9 +647,18 @@ export class MinionQueue { async handleTimeouts(): Promise { return this.engine.transaction(async (tx) => { const rows = await tx.executeRaw>( + // #1737: count the timed-out run as a spent attempt (terminal, no retry), + // mirroring handleWallClockTimeouts + handleStalled. handleTimeouts is the + // FIRST killer to fire for the long-lane handlers (timeout_ms stamped at + // submit), so without this the job reads `attempts: 0/N (started: N)`. + // Safe against double-count: the worker sweep runs handleStalled -> + // handleTimeouts -> handleWallClockTimeouts sequentially and awaited, and + // each guards on `status = 'active'`, so the first to set status='dead' + // excludes the row from the later sweeps. `UPDATE minion_jobs SET status = 'dead', error_text = 'timeout exceeded', + attempts_made = attempts_made + 1, lock_token = NULL, lock_until = NULL, finished_at = now(), diff --git a/test/e2e/minions-resilience.test.ts b/test/e2e/minions-resilience.test.ts index c10dcfbc9..21614fdd5 100644 --- a/test/e2e/minions-resilience.test.ts +++ b/test/e2e/minions-resilience.test.ts @@ -138,6 +138,10 @@ describeE2E('E2E: Minions resilience (OpenClaw real-world patterns)', () => { const final = await queue.getJob(job.id); expect(final?.error_text).toMatch(/timeout exceeded/i); + // #1737 regression: the runaway run is counted as a spent attempt by + // handleTimeouts (it's the first killer to fire), so accounting reads + // honestly instead of `attempts: 0/1 (started: 1)`. + expect(final?.attempts_made).toBe(1); } finally { await a.disconnect(); await b.disconnect(); diff --git a/test/minions.test.ts b/test/minions.test.ts index ec990406a..012e91bdd 100644 --- a/test/minions.test.ts +++ b/test/minions.test.ts @@ -1300,6 +1300,12 @@ describe('MinionQueue: handleTimeouts', () => { const dead = await queue.getJob(job.id); expect(dead!.status).toBe('dead'); expect(dead!.error_text).toBe('timeout exceeded'); + // #1737 regression: the timed-out run counts as a spent attempt, mirroring + // the wall-clock + stall dead-letter paths. Without this the job reads + // `attempts: 0/N (started: N)`. Asserted on both the RETURNING row and the + // persisted row so a future refactor can't silently drop the increment. + expect(timedOut[0].attempts_made).toBe(1); + expect(dead!.attempts_made).toBe(1); }); test('handleTimeouts ignores stalled jobs (lock_until > now guard)', async () => {