From 243ad00810238b426acc913cf9b1779f2ff7f4f6 Mon Sep 17 00:00:00 2001
From: Josh Salway <josh.salway@gmail.com>
Date: Wed, 22 Apr 2026 14:55:01 +1000
Subject: [PATCH] Bound retry and unique-lock lifetimes on UpdateApps and
 ProcessApps

Both jobs implement ShouldBeUnique without a $uniqueFor value, which on
Redis and database drivers produces a lock that never expires. If the
worker is killed mid-fire (OOM, SIGKILL, server crash), the lock
persists and blocks all future dispatches of UpdateApps or ProcessApps
until the cache entry is manually cleared.

Neither job sets $tries, $backoff, or $timeout, so they inherit the
worker command's defaults (1 for queue:work, 0 for vapor:work), which
varies by platform and is brittle.

This change adds:
 - $uniqueFor = 3600   lock expires after 1 hour
 - $tries = 3          hard cap across worker restarts
 - $backoff = [30, 60, 120]  pace retries to reduce GitHub API load
 - $timeout = 60       bound per-attempt wall-clock time
 - failed(Throwable)    log permanent failures (and preserve the
                        existing Cache::lock('updateApps')->forceRelease
                        on UpdateApps)

A new test (tests/Feature/QueueSafetyTest.php) asserts the retry
properties are present.

All existing tests still pass.
---
 app/Jobs/ProcessApps.php          | 25 ++++++++++++++++++++
 app/Jobs/UpdateApps.php           | 28 +++++++++++++++++++++-
 tests/Feature/QueueSafetyTest.php | 39 +++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100644 tests/Feature/QueueSafetyTest.php

diff --git a/app/Jobs/ProcessApps.php b/app/Jobs/ProcessApps.php
index 682761277..862a4bdb3 100644
--- a/app/Jobs/ProcessApps.php
+++ b/app/Jobs/ProcessApps.php
@@ -14,11 +14,29 @@ use Illuminate\Queue\InteractsWithQueue;
 use Illuminate\Queue\SerializesModels;
 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Storage;
+use Throwable;
 
 class ProcessApps implements ShouldQueue, ShouldBeUnique
 {
     use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
 
+    /**
+     * Bound total attempts so a failed job stops after three retries across
+     * worker restarts, independent of the broker's reserved-job recovery path.
+     */
+    public int $tries = 3;
+
+    /** @var array<int, int> seconds between retries */
+    public array $backoff = [30, 60, 120];
+
+    public int $timeout = 60;
+
+    /**
+     * Expire the ShouldBeUnique lock after 1 hour so a crashed worker does
+     * not permanently block future ProcessApps dispatches.
+     */
+    public int $uniqueFor = 3600;
+
     /**
      * Create a new job instance.
      *
@@ -57,4 +75,11 @@ class ProcessApps implements ShouldQueue, ShouldBeUnique
             }
         }
     }
+
+    public function failed(Throwable $exception): void
+    {
+        Log::error(static::class . ' permanently failed', [
+            'exception' => $exception->getMessage(),
+        ]);
+    }
 }
diff --git a/app/Jobs/UpdateApps.php b/app/Jobs/UpdateApps.php
index 636792b47..7911cc824 100644
--- a/app/Jobs/UpdateApps.php
+++ b/app/Jobs/UpdateApps.php
@@ -12,11 +12,33 @@ use Illuminate\Queue\InteractsWithQueue;
 use Illuminate\Queue\SerializesModels;
 use Illuminate\Support\Facades\Cache;
 use Illuminate\Support\Facades\Log;
+use Throwable;
 
 class UpdateApps implements ShouldQueue, ShouldBeUnique
 {
     use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
 
+    /**
+     * Bound total attempts so a failed job stops after three retries across
+     * worker restarts, independent of the broker's reserved-job recovery path.
+     */
+    public int $tries = 3;
+
+    /** @var array<int, int> seconds between retries */
+    public array $backoff = [30, 60, 120];
+
+    /**
+     * Per-attempt wall-clock ceiling. Heavy users with many apps may need
+     * a larger value; 60s covers the typical Heimdall deployment.
+     */
+    public int $timeout = 60;
+
+    /**
+     * Expire the ShouldBeUnique lock after 1 hour so a crashed worker does
+     * not permanently block future UpdateApps dispatches.
+     */
+    public int $uniqueFor = 3600;
+
     /**
      * Create a new job instance.
      *
@@ -49,8 +71,12 @@ class UpdateApps implements ShouldQueue, ShouldBeUnique
         Cache::lock('updateApps')->forceRelease();
     }
 
-    public function failed($exception): void
+    public function failed(Throwable $exception): void
     {
         Cache::lock('updateApps')->forceRelease();
+
+        Log::error(static::class . ' permanently failed', [
+            'exception' => $exception->getMessage(),
+        ]);
     }
 }
diff --git a/tests/Feature/QueueSafetyTest.php b/tests/Feature/QueueSafetyTest.php
new file mode 100644
index 000000000..c4fc32db9
--- /dev/null
+++ b/tests/Feature/QueueSafetyTest.php
@@ -0,0 +1,39 @@
+<?php
+
+namespace Tests\Feature;
+
+use App\Jobs\ProcessApps;
+use App\Jobs\UpdateApps;
+use Tests\TestCase;
+
+class QueueSafetyTest extends TestCase
+{
+    /** @test */
+    public function updateApps_has_bounded_retry_properties(): void
+    {
+        $job = new UpdateApps();
+
+        $this->assertSame(3, $job->tries);
+        $this->assertSame([30, 60, 120], $job->backoff);
+        $this->assertSame(60, $job->timeout);
+        $this->assertSame(3600, $job->uniqueFor);
+    }
+
+    /** @test */
+    public function processApps_has_bounded_retry_properties(): void
+    {
+        $job = new ProcessApps();
+
+        $this->assertSame(3, $job->tries);
+        $this->assertSame([30, 60, 120], $job->backoff);
+        $this->assertSame(60, $job->timeout);
+        $this->assertSame(3600, $job->uniqueFor);
+    }
+
+    /** @test */
+    public function both_jobs_expose_a_failed_method(): void
+    {
+        $this->assertTrue(method_exists(UpdateApps::class, 'failed'));
+        $this->assertTrue(method_exists(ProcessApps::class, 'failed'));
+    }
+}