fix(queue): raise retry_after above the longest job timeout (PHP-LARAVEL-2D) (#645)

## What & why

Fixes **PHP-LARAVEL-2D** — `MaxAttemptsExceededException:
CategorizeUncategorizedTransactionsJob has been attempted too many
times.` (recurring, still firing as of yesterday, ~18 events).

### Root cause
The `database` queue connection used `retry_after = 90s`, but several
jobs declare a `$timeout` far above it (up to **600s**). `retry_after`
is **per-connection**. When a job outlives its 90s reservation, the DB
queue driver hands it to a second worker. For a `tries = 1` job that
immediately raises `MaxAttemptsExceededException` — and, worse, the body
can execute twice. `CategorizeUncategorizedTransactionsJob` runs on the
`ai` queue, which the deploy runs with **two concurrent workers**
(`docker/supervisor/supervisord.conf`), so the double-run re-bills
Gemini and resets progress — the exact harm its `tries = 1` was meant to
prevent.

At `retry_after = 90` **8 jobs** violated the invariant (600s rule jobs,
300s AI jobs, 120s sync jobs).

## Changes (one concern per commit)
1. **fix(queue): raise `retry_after` above the longest job timeout** —
default `90 → 900s` (env-overridable via `DB_QUEUE_RETRY_AFTER`),
clearing the 600s longest job with margin. Adds `QueueConfigTest`
asserting every `app/Jobs` `$timeout` stays `< retry_after` so a new
long job can't silently re-break it.
2. **fix(ai): de-duplicate the backfill per user** — `retry_after` stops
one dispatch being re-reserved, but not a *duplicate* dispatch (double
"Enable AI" click / re-enable mid-run) on the 2-worker `ai` queue.
Implement `ShouldBeUnique` keyed on user id, mirroring the sibling
`RetryTransientAiCategorizationJob`.
3. **test(queue): widen the guard** to also scan queued Mailables
(`app/Mail`) and Notifications (`app/Notifications`), which share the
connection and honor `$timeout`.
4. **docs(queue): correct the `retry_after` comment** — per-job
`$timeout` takes precedence over the worker `--timeout` (which is
currently unset), so the earlier "must sit between" phrasing was
inaccurate.

## Verification
- `QueueConfigTest` fails at the old `retry_after=90` (8 offenders) and
passes at `900`.
- `vendor/bin/pint --test` clean; 131 tests across `tests/Feature/Ai`,
AI-consent flow, and queue config pass.
- Confirmed against the real deploy: 2 workers on the `ai` queue,
`pcntl` installed (SIGALRM enforced), workers set no `--timeout` so each
job's own `$timeout` bounds execution below 900s.

## Reviewed by two independent agents (architecture + product)
Both concluded **ship it**. The double-dispatch gap they flagged is
closed by change #2. Neither found a correctness regression.

## Operational notes (no code change needed, flagged for awareness)
- **Reclaim window:** `retry_after` is per-connection, so
`emails`/`default` also inherit the 90→900s window. A job orphaned by a
*hard* worker crash (OOM/SIGKILL) is now reclaimed after up to ~15 min
instead of ~90s. Graceful deploys (SIGTERM + `stopwaitsecs=3600`) don't
orphan jobs, so this only bites on hard crashes — an acceptable trade. A
dedicated connection for the long AI queue would remove it; left as a
possible follow-up.
- **Worker `--timeout`:** the fix relies on `pcntl` staying installed
and long jobs keeping a `$timeout`. Setting an explicit `--timeout` (<
900) on the supervisord `queue:work` commands would make it
self-enforcing; not done here to avoid changing short-job hang
detection.
- If prod sets `DB_QUEUE_RETRY_AFTER` explicitly, ensure it's above the
longest job timeout (≥ 600, ideally 900).

app/Jobs/CategorizeUncategorizedTransactionsJob.php

@@ -5,6 +5,7 @@ namespace App\Jobs;
 use App\Models\User;
 use App\Services\Ai\AiCategorizationGate;
 use App\Services\Ai\AiCategorizer;
+use Illuminate\Contracts\Queue\ShouldBeUnique;
 use Illuminate\Contracts\Queue\ShouldQueue;
 use Illuminate\Foundation\Queue\Queueable;
 use Illuminate\Support\Facades\Cache;
@@ -15,11 +16,17 @@ use Throwable;
  * consent outside of onboarding. Progress is written to the cache so the
  * transactions page can poll it and surface live progress while the batch runs.
  *
+ * De-duplicated per user: a second dispatch (double "Enable AI" click, or
+ * re-enabling consent while a run is still in flight) would read the same
+ * pending-transaction snapshot on a concurrent worker and re-bill the model for
+ * work already underway — the exact harm tries=1 targets but cannot prevent,
+ * since tries only bounds re-attempts of one dispatch, not duplicate dispatches.
+ *
  * ponytail: mirrors CategorizeOnboardingTransactionsJob's selection + chunking;
  * kept separate so the onboarding pass stays progress-free. Fold the two
  * together if a third caller ever needs the same loop.
  */
-class CategorizeUncategorizedTransactionsJob implements ShouldQueue
+class CategorizeUncategorizedTransactionsJob implements ShouldBeUnique, ShouldQueue
 {
     use Queueable;
 
@@ -34,8 +41,19 @@ class CategorizeUncategorizedTransactionsJob implements ShouldQueue
      */
     public int $tries = 1;
 
+    /**
+     * Safety TTL for the unique lock in case a worker dies mid-run; comfortably
+     * longer than a full run.
+     */
+    public int $uniqueFor = 1800;
+
     public function __construct(public User $user, public string $jobId) {}
 
+    public function uniqueId(): string
+    {
+        return $this->user->id;
+    }
+
     public function viaQueue(): string
     {
         return (string) config('ai_categorization.queue');

config/queue.php

@@ -40,7 +40,14 @@ return [
             'connection' => env('DB_QUEUE_CONNECTION'),
             'table' => env('DB_QUEUE_TABLE', 'jobs'),
             'queue' => env('DB_QUEUE', 'default'),
-            'retry_after' => (int) env('DB_QUEUE_RETRY_AFTER', 90),
+            // retry_after must exceed the longest job $timeout on this connection
+            // (currently 600s). Otherwise a long job outlives its reservation, the
+            // queue hands it to a second worker, and a tries=1 job dies with
+            // MaxAttemptsExceededException (and re-runs its side effects). Each
+            // job's execution ceiling — its own $timeout, or the worker --timeout
+            // when a job declares none — must stay below this value. QueueConfigTest
+            // guards the $timeout side.
+            'retry_after' => (int) env('DB_QUEUE_RETRY_AFTER', 900),
             'after_commit' => true,
         ],
 

tests/Feature/Ai/CategorizeUncategorizedTransactionsTest.php

@@ -9,6 +9,7 @@ use App\Models\Transaction;
 use App\Models\User;
 use App\Services\Ai\CategorizeTransactions;
 use App\Services\Ai\CategoryCatalog;
+use Illuminate\Contracts\Queue\ShouldBeUnique;
 use Illuminate\Support\Facades\Bus;
 use Illuminate\Support\Facades\Cache;
 
@@ -218,3 +219,11 @@ it('categorizes the most recent transactions first', function () {
 
     expect($order)->toBe([$newest->id, $middle->id, $oldest->id]);
 });
+
+it('de-duplicates the backfill per user so a concurrent dispatch cannot double-bill', function () {
+    $user = User::factory()->create();
+    $job = new CategorizeUncategorizedTransactionsJob($user, 'job-x');
+
+    expect($job)->toBeInstanceOf(ShouldBeUnique::class)
+        ->and($job->uniqueId())->toBe($user->id);
+});

tests/Feature/QueueConfigTest.php

@@ -0,0 +1,41 @@
+<?php
+
+use Illuminate\Support\Facades\File;
+use Illuminate\Support\Str;
+
+/**
+ * The database queue driver releases a job back to the queue once its
+ * reservation is older than retry_after. If a job runs longer than that, a
+ * second worker picks it up: a tries=1 job then dies with
+ * MaxAttemptsExceededException and re-runs its side effects. Guard the
+ * invariant so a new long-running job can never silently reintroduce it.
+ * Queued Mailables and Notifications honor $timeout too, so scan them as well.
+ */
+test('every queued job timeout stays below the database queue retry_after', function () {
+    $retryAfter = (int) config('queue.connections.database.retry_after');
+
+    $queueableDirs = ['Jobs', 'Mail', 'Notifications'];
+
+    $offenders = collect($queueableDirs)
+        ->flatMap(fn (string $dir): array => File::allFiles(app_path($dir)))
+        ->map(function ($file): string {
+            $relative = Str::of($file->getPathname())
+                ->after(app_path().DIRECTORY_SEPARATOR)
+                ->replace(DIRECTORY_SEPARATOR, '\\')
+                ->replaceLast('.php', '');
+
+            return 'App\\'.$relative;
+        })
+        ->filter(fn (string $class): bool => class_exists($class))
+        ->mapWithKeys(function (string $class): array {
+            $timeout = (new ReflectionClass($class))->getDefaultProperties()['timeout'] ?? null;
+
+            return [$class => $timeout];
+        })
+        ->filter(fn ($timeout): bool => is_int($timeout) && $timeout >= $retryAfter);
+
+    expect($offenders->all())->toBe(
+        [],
+        'These jobs have a $timeout >= retry_after ('.$retryAfter.'s); raise DB_QUEUE_RETRY_AFTER above the longest job timeout.'
+    );
+});