From 2b7e2ebafb3266c47f3f222ed7cc983fdbd2df58 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 26 Feb 2026 16:27:02 +0100 Subject: [PATCH 1/7] chore: prepare for PR --- app/Models/PrivateKey.php | 2 +- scripts/upgrade.sh | 9 +++++++++ tests/Unit/PrivateKeyStorageTest.php | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/app/Models/PrivateKey.php b/app/Models/PrivateKey.php index bb76d5ed6..7163ae7b5 100644 --- a/app/Models/PrivateKey.php +++ b/app/Models/PrivateKey.php @@ -237,7 +237,7 @@ protected function ensureStorageDirectoryExists() $testSuccess = $disk->put($testFilename, 'test'); if (! $testSuccess) { - throw new \Exception('SSH keys storage directory is not writable'); + throw new \Exception('SSH keys storage directory is not writable. Run on the host: sudo chown -R 9999 /data/coolify/ssh && sudo chmod -R 700 /data/coolify/ssh && docker restart coolify'); } // Clean up test file diff --git a/scripts/upgrade.sh b/scripts/upgrade.sh index 648849d5c..f32db9b8d 100644 --- a/scripts/upgrade.sh +++ b/scripts/upgrade.sh @@ -141,6 +141,15 @@ else log "Network 'coolify' already exists" fi +# Fix SSH directory ownership if not owned by container user UID 9999 (fixes #6621) +# Only changes owner — preserves existing group to respect custom setups +SSH_OWNER=$(stat -c '%u' /data/coolify/ssh 2>/dev/null || echo "unknown") +if [ "$SSH_OWNER" != "9999" ]; then + log "Fixing SSH directory ownership (was owned by UID $SSH_OWNER)" + chown -R 9999 /data/coolify/ssh + chmod -R 700 /data/coolify/ssh +fi + # Check if Docker config file exists DOCKER_CONFIG_MOUNT="" if [ -f /root/.docker/config.json ]; then diff --git a/tests/Unit/PrivateKeyStorageTest.php b/tests/Unit/PrivateKeyStorageTest.php index 00f39e3df..09472604b 100644 --- a/tests/Unit/PrivateKeyStorageTest.php +++ b/tests/Unit/PrivateKeyStorageTest.php @@ -112,7 +112,7 @@ public function it_throws_exception_when_storage_directory_is_not_writable() ); $this->expectException(\Exception::class); - $this->expectExceptionMessage('SSH keys storage directory is not writable'); + $this->expectExceptionMessage('SSH keys storage directory is not writable. Run on the host: sudo chown -R 9999 /data/coolify/ssh && sudo chmod -R 700 /data/coolify/ssh && docker restart coolify'); PrivateKey::createAndStore([ 'name' => 'Test Key', From 6b2a669cb988d4a7788c0411572cec87b95395f1 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Fri, 27 Feb 2026 22:03:54 +0100 Subject: [PATCH 2/7] docs(sponsors): add huge sponsors section and reorganize list - Create new "Huge Sponsors" section with SerpAPI - Move SerpAPI from Small Sponsors to Huge Sponsors - Replace Dade2 with Darweb - Add Greptile and MVPS as new sponsors --- README.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 276ef07b5..c78e47997 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,10 @@ ## Donations Thank you so much! +### Huge Sponsors + +* [SerpAPI](https://serpapi.com?ref=coolify.io) - Google Search API — Scrape Google and other search engines from our fast, easy, and complete API + ### Big Sponsors * [23M](https://23m.com?ref=coolify.io) - Your experts for high-availability hosting solutions! @@ -70,9 +74,10 @@ ### Big Sponsors * [CompAI](https://www.trycomp.ai?ref=coolify.io) - Open source compliance automation platform * [Convex](https://convex.link/coolify.io) - Open-source reactive database for web app developers * [CubePath](https://cubepath.com/?ref=coolify.io) - Dedicated Servers & Instant Deploy -* [Dade2](https://dade2.net/?ref=coolify.io) - IT Consulting, Cloud Solutions & System Integration +* [Darweb](https://darweb.nl/?ref=coolify.io) - 3D CPQ solutions for ecommerce design * [Formbricks](https://formbricks.com?ref=coolify.io) - The open source feedback platform * [GoldenVM](https://billing.goldenvm.com?ref=coolify.io) - Premium virtual machine hosting solutions +* [Greptile](https://www.greptile.com?ref=coolify.io) - The AI Code Reviewer * [Hetzner](http://htznr.li/CoolifyXHetzner) - Server, cloud, hosting, and data center solutions * [Hostinger](https://www.hostinger.com/vps/coolify-hosting?ref=coolify.io) - Web hosting and VPS solutions * [JobsCollider](https://jobscollider.com/remote-jobs?ref=coolify.io) - 30,000+ remote jobs for developers @@ -80,6 +85,7 @@ ### Big Sponsors * [LiquidWeb](https://liquidweb.com?ref=coolify.io) - Premium managed hosting solutions * [Logto](https://logto.io?ref=coolify.io) - The better identity infrastructure for developers * [Macarne](https://macarne.com?ref=coolify.io) - Best IP Transit & Carrier Ethernet Solutions for Simplified Network Connectivity +* [MVPS](https://www.mvps.net?ref=coolify.io) - Cheap VPS servers at the highest possible quality * [Mobb](https://vibe.mobb.ai/?ref=coolify.io) - Secure Your AI-Generated Code to Unlock Dev Productivity * [PFGLabs](https://pfglabs.com?ref=coolify.io) - Build Real Projects with Golang * [Ramnode](https://ramnode.com/?ref=coolify.io) - High Performance Cloud VPS Hosting @@ -126,7 +132,6 @@ ### Small Sponsors RunPod DartNode Tyler Whitesides -SerpAPI Aquarela Crypto Jobs List Alfred Nutile From f68793ed69de6167926fa0f34e3574034b71e46c Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sat, 28 Feb 2026 13:18:44 +0100 Subject: [PATCH 3/7] feat(jobs): optimize async job dispatches and enhance Stripe subscription sync Reduce unnecessary job queue pressure and improve subscription sync reliability: - Cache ServerStorageCheckJob dispatch to only trigger on disk percentage changes - Rate-limit ConnectProxyToNetworksJob to maximum once per 10 minutes - Add progress callback support to SyncStripeSubscriptionsJob for UI feedback - Implement bulk fetching of valid Stripe subscription IDs for efficiency - Detect and report resubscribed users (same email, different customer ID) - Fix CleanupUnreachableServers query operator (>= 3 instead of = 3) - Improve empty subId validation in PushServerUpdateJob - Optimize relationship access by using properties instead of query methods - Add comprehensive test coverage for all optimizations --- .../Commands/CleanupUnreachableServers.php | 2 +- .../Cloud/SyncStripeSubscriptions.php | 22 ++- app/Jobs/PushServerUpdateJob.php | 33 +++- app/Jobs/SyncStripeSubscriptionsJob.php | 159 +++++++++++++++--- .../Feature/CleanupUnreachableServersTest.php | 73 ++++++++ .../PushServerUpdateJobOptimizationTest.php | 150 +++++++++++++++++ 6 files changed, 402 insertions(+), 37 deletions(-) create mode 100644 tests/Feature/CleanupUnreachableServersTest.php create mode 100644 tests/Feature/PushServerUpdateJobOptimizationTest.php diff --git a/app/Console/Commands/CleanupUnreachableServers.php b/app/Console/Commands/CleanupUnreachableServers.php index def01b265..09563a2c3 100644 --- a/app/Console/Commands/CleanupUnreachableServers.php +++ b/app/Console/Commands/CleanupUnreachableServers.php @@ -14,7 +14,7 @@ class CleanupUnreachableServers extends Command public function handle() { echo "Running unreachable server cleanup...\n"; - $servers = Server::where('unreachable_count', 3)->where('unreachable_notification_sent', true)->where('updated_at', '<', now()->subDays(7))->get(); + $servers = Server::where('unreachable_count', '>=', 3)->where('unreachable_notification_sent', true)->where('updated_at', '<', now()->subDays(7))->get(); if ($servers->count() > 0) { foreach ($servers as $server) { echo "Cleanup unreachable server ($server->id) with name $server->name"; diff --git a/app/Console/Commands/Cloud/SyncStripeSubscriptions.php b/app/Console/Commands/Cloud/SyncStripeSubscriptions.php index e64f86926..46f6b4edd 100644 --- a/app/Console/Commands/Cloud/SyncStripeSubscriptions.php +++ b/app/Console/Commands/Cloud/SyncStripeSubscriptions.php @@ -36,7 +36,14 @@ public function handle(): int $this->newLine(); $job = new SyncStripeSubscriptionsJob($fix); - $result = $job->handle(); + $fetched = 0; + $result = $job->handle(function (int $count) use (&$fetched): void { + $fetched = $count; + $this->output->write("\r Fetching subscriptions from Stripe... {$fetched}"); + }); + if ($fetched > 0) { + $this->output->write("\r".str_repeat(' ', 60)."\r"); + } if (isset($result['error'])) { $this->error($result['error']); @@ -68,6 +75,19 @@ public function handle(): int $this->info('No discrepancies found. All subscriptions are in sync.'); } + if (count($result['resubscribed']) > 0) { + $this->newLine(); + $this->warn('Resubscribed users (same email, different customer): '.count($result['resubscribed'])); + $this->newLine(); + + foreach ($result['resubscribed'] as $resub) { + $this->line(" - Team ID: {$resub['team_id']} | Email: {$resub['email']}"); + $this->line(" Old: {$resub['old_stripe_subscription_id']} (cus: {$resub['old_stripe_customer_id']})"); + $this->line(" New: {$resub['new_stripe_subscription_id']} (cus: {$resub['new_stripe_customer_id']}) [{$resub['new_status']}]"); + $this->newLine(); + } + } + if (count($result['errors']) > 0) { $this->newLine(); $this->error('Errors encountered: '.count($result['errors'])); diff --git a/app/Jobs/PushServerUpdateJob.php b/app/Jobs/PushServerUpdateJob.php index 5d018cf19..85684ff19 100644 --- a/app/Jobs/PushServerUpdateJob.php +++ b/app/Jobs/PushServerUpdateJob.php @@ -24,6 +24,7 @@ use Illuminate\Queue\Middleware\WithoutOverlapping; use Illuminate\Queue\SerializesModels; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Cache; use Laravel\Horizon\Contracts\Silenced; class PushServerUpdateJob implements ShouldBeEncrypted, ShouldQueue, Silenced @@ -130,7 +131,14 @@ public function handle() $this->containers = collect(data_get($data, 'containers')); $filesystemUsageRoot = data_get($data, 'filesystem_usage_root.used_percentage'); - ServerStorageCheckJob::dispatch($this->server, $filesystemUsageRoot); + + // Only dispatch storage check when disk percentage actually changes + $storageCacheKey = 'storage-check:'.$this->server->id; + $lastPercentage = Cache::get($storageCacheKey); + if ($lastPercentage === null || (string) $lastPercentage !== (string) $filesystemUsageRoot) { + Cache::put($storageCacheKey, $filesystemUsageRoot, 600); + ServerStorageCheckJob::dispatch($this->server, $filesystemUsageRoot); + } if ($this->containers->isEmpty()) { return; @@ -207,7 +215,7 @@ public function handle() $serviceId = $labels->get('coolify.serviceId'); $subType = $labels->get('coolify.service.subType'); $subId = $labels->get('coolify.service.subId'); - if (empty($subId)) { + if (empty(trim((string) $subId))) { continue; } if ($subType === 'application') { @@ -327,6 +335,10 @@ private function aggregateServiceContainerStatuses() // Parse key: serviceId:subType:subId [$serviceId, $subType, $subId] = explode(':', $key); + if (empty($subId)) { + continue; + } + $service = $this->services->where('id', $serviceId)->first(); if (! $service) { continue; @@ -335,9 +347,9 @@ private function aggregateServiceContainerStatuses() // Get the service sub-resource (ServiceApplication or ServiceDatabase) $subResource = null; if ($subType === 'application') { - $subResource = $service->applications()->where('id', $subId)->first(); + $subResource = $service->applications->where('id', $subId)->first(); } elseif ($subType === 'database') { - $subResource = $service->databases()->where('id', $subId)->first(); + $subResource = $service->databases->where('id', $subId)->first(); } if (! $subResource) { @@ -476,8 +488,13 @@ private function updateProxyStatus() } catch (\Throwable $e) { } } else { - // Connect proxy to networks asynchronously to avoid blocking the status update - ConnectProxyToNetworksJob::dispatch($this->server); + // Connect proxy to networks periodically (every 10 min) to avoid excessive job dispatches. + // On-demand triggers (new network, service deploy) use dispatchSync() and bypass this. + $proxyCacheKey = 'connect-proxy:'.$this->server->id; + if (! Cache::has($proxyCacheKey)) { + Cache::put($proxyCacheKey, true, 600); + ConnectProxyToNetworksJob::dispatch($this->server); + } } } } @@ -545,7 +562,7 @@ private function updateServiceSubStatus(string $serviceId, string $subType, stri return; } if ($subType === 'application') { - $application = $service->applications()->where('id', $subId)->first(); + $application = $service->applications->where('id', $subId)->first(); if ($application) { if ($application->status !== $containerStatus) { $application->status = $containerStatus; @@ -553,7 +570,7 @@ private function updateServiceSubStatus(string $serviceId, string $subType, stri } } } elseif ($subType === 'database') { - $database = $service->databases()->where('id', $subId)->first(); + $database = $service->databases->where('id', $subId)->first(); if ($database) { if ($database->status !== $containerStatus) { $database->status = $containerStatus; diff --git a/app/Jobs/SyncStripeSubscriptionsJob.php b/app/Jobs/SyncStripeSubscriptionsJob.php index 9eb946e4d..4301a80d1 100644 --- a/app/Jobs/SyncStripeSubscriptionsJob.php +++ b/app/Jobs/SyncStripeSubscriptionsJob.php @@ -22,7 +22,7 @@ public function __construct(public bool $fix = false) $this->onQueue('high'); } - public function handle(): array + public function handle(?\Closure $onProgress = null): array { if (! isCloud() || ! isStripe()) { return ['error' => 'Not running on Cloud or Stripe not configured']; @@ -33,48 +33,73 @@ public function handle(): array ->get(); $stripe = new \Stripe\StripeClient(config('subscription.stripe_api_key')); + + // Bulk fetch all valid subscription IDs from Stripe (active + past_due) + $validStripeIds = $this->fetchValidStripeSubscriptionIds($stripe, $onProgress); + + // Find DB subscriptions not in the valid set + $staleSubscriptions = $subscriptions->filter( + fn (Subscription $sub) => ! in_array($sub->stripe_subscription_id, $validStripeIds) + ); + + // For each stale subscription, get the exact Stripe status and check for resubscriptions $discrepancies = []; + $resubscribed = []; $errors = []; - foreach ($subscriptions as $subscription) { + foreach ($staleSubscriptions as $subscription) { try { $stripeSubscription = $stripe->subscriptions->retrieve( $subscription->stripe_subscription_id ); + $stripeStatus = $stripeSubscription->status; - // Check if Stripe says cancelled but we think it's active - if (in_array($stripeSubscription->status, ['canceled', 'incomplete_expired', 'unpaid'])) { - $discrepancies[] = [ - 'subscription_id' => $subscription->id, - 'team_id' => $subscription->team_id, - 'stripe_subscription_id' => $subscription->stripe_subscription_id, - 'stripe_status' => $stripeSubscription->status, - ]; - - // Only fix if --fix flag is passed - if ($this->fix) { - $subscription->update([ - 'stripe_invoice_paid' => false, - 'stripe_past_due' => false, - ]); - - if ($stripeSubscription->status === 'canceled') { - $subscription->team?->subscriptionEnded(); - } - } - } - - // Small delay to avoid Stripe rate limits - usleep(100000); // 100ms + usleep(100000); // 100ms rate limit delay } catch (\Exception $e) { $errors[] = [ 'subscription_id' => $subscription->id, 'error' => $e->getMessage(), ]; + + continue; + } + + // Check if this user resubscribed under a different customer/subscription + $activeSub = $this->findActiveSubscriptionByEmail($stripe, $stripeSubscription->customer); + if ($activeSub) { + $resubscribed[] = [ + 'subscription_id' => $subscription->id, + 'team_id' => $subscription->team_id, + 'email' => $activeSub['email'], + 'old_stripe_subscription_id' => $subscription->stripe_subscription_id, + 'old_stripe_customer_id' => $stripeSubscription->customer, + 'new_stripe_subscription_id' => $activeSub['subscription_id'], + 'new_stripe_customer_id' => $activeSub['customer_id'], + 'new_status' => $activeSub['status'], + ]; + + continue; + } + + $discrepancies[] = [ + 'subscription_id' => $subscription->id, + 'team_id' => $subscription->team_id, + 'stripe_subscription_id' => $subscription->stripe_subscription_id, + 'stripe_status' => $stripeStatus, + ]; + + if ($this->fix) { + $subscription->update([ + 'stripe_invoice_paid' => false, + 'stripe_past_due' => false, + ]); + + if ($stripeStatus === 'canceled') { + $subscription->team?->subscriptionEnded(); + } } } - // Only notify if discrepancies found and fixed if ($this->fix && count($discrepancies) > 0) { send_internal_notification( 'SyncStripeSubscriptionsJob: Fixed '.count($discrepancies)." discrepancies:\n". @@ -85,8 +110,88 @@ public function handle(): array return [ 'total_checked' => $subscriptions->count(), 'discrepancies' => $discrepancies, + 'resubscribed' => $resubscribed, 'errors' => $errors, 'fixed' => $this->fix, ]; } + + /** + * Given a Stripe customer ID, get their email and search for other customers + * with the same email that have an active subscription. + * + * @return array{email: string, customer_id: string, subscription_id: string, status: string}|null + */ + private function findActiveSubscriptionByEmail(\Stripe\StripeClient $stripe, string $customerId): ?array + { + try { + $customer = $stripe->customers->retrieve($customerId); + $email = $customer->email; + + if (! $email) { + return null; + } + + usleep(100000); + + $customers = $stripe->customers->all([ + 'email' => $email, + 'limit' => 10, + ]); + + usleep(100000); + + foreach ($customers->data as $matchingCustomer) { + if ($matchingCustomer->id === $customerId) { + continue; + } + + $subs = $stripe->subscriptions->all([ + 'customer' => $matchingCustomer->id, + 'limit' => 10, + ]); + + usleep(100000); + + foreach ($subs->data as $sub) { + if (in_array($sub->status, ['active', 'past_due'])) { + return [ + 'email' => $email, + 'customer_id' => $matchingCustomer->id, + 'subscription_id' => $sub->id, + 'status' => $sub->status, + ]; + } + } + } + } catch (\Exception $e) { + // Silently skip — will fall through to normal discrepancy + } + + return null; + } + + /** + * Bulk fetch all active and past_due subscription IDs from Stripe. + * + * @return array + */ + private function fetchValidStripeSubscriptionIds(\Stripe\StripeClient $stripe, ?\Closure $onProgress = null): array + { + $validIds = []; + $fetched = 0; + + foreach (['active', 'past_due'] as $status) { + foreach ($stripe->subscriptions->all(['status' => $status, 'limit' => 100])->autoPagingIterator() as $sub) { + $validIds[] = $sub->id; + $fetched++; + + if ($onProgress) { + $onProgress($fetched); + } + } + } + + return $validIds; + } } diff --git a/tests/Feature/CleanupUnreachableServersTest.php b/tests/Feature/CleanupUnreachableServersTest.php new file mode 100644 index 000000000..edfd0511c --- /dev/null +++ b/tests/Feature/CleanupUnreachableServersTest.php @@ -0,0 +1,73 @@ += 3 after 7 days', function () { + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'unreachable_count' => 50, + 'unreachable_notification_sent' => true, + 'updated_at' => now()->subDays(8), + ]); + + $this->artisan('cleanup:unreachable-servers')->assertSuccessful(); + + $server->refresh(); + expect($server->ip)->toBe('1.2.3.4'); +}); + +it('does not clean up servers with unreachable_count less than 3', function () { + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'unreachable_count' => 2, + 'unreachable_notification_sent' => true, + 'updated_at' => now()->subDays(8), + ]); + + $originalIp = $server->ip; + + $this->artisan('cleanup:unreachable-servers')->assertSuccessful(); + + $server->refresh(); + expect($server->ip)->toBe($originalIp); +}); + +it('does not clean up servers updated within 7 days', function () { + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'unreachable_count' => 10, + 'unreachable_notification_sent' => true, + 'updated_at' => now()->subDays(3), + ]); + + $originalIp = $server->ip; + + $this->artisan('cleanup:unreachable-servers')->assertSuccessful(); + + $server->refresh(); + expect($server->ip)->toBe($originalIp); +}); + +it('does not clean up servers without notification sent', function () { + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'unreachable_count' => 10, + 'unreachable_notification_sent' => false, + 'updated_at' => now()->subDays(8), + ]); + + $originalIp = $server->ip; + + $this->artisan('cleanup:unreachable-servers')->assertSuccessful(); + + $server->refresh(); + expect($server->ip)->toBe($originalIp); +}); diff --git a/tests/Feature/PushServerUpdateJobOptimizationTest.php b/tests/Feature/PushServerUpdateJobOptimizationTest.php new file mode 100644 index 000000000..eb51059db --- /dev/null +++ b/tests/Feature/PushServerUpdateJobOptimizationTest.php @@ -0,0 +1,150 @@ +create(); + $server = Server::factory()->create(['team_id' => $team->id]); + + $data = [ + 'containers' => [], + 'filesystem_usage_root' => ['used_percentage' => 45], + ]; + + $job = new PushServerUpdateJob($server, $data); + $job->handle(); + + Queue::assertPushed(ServerStorageCheckJob::class, function ($job) use ($server) { + return $job->server->id === $server->id && $job->percentage === 45; + }); +}); + +it('does not dispatch storage check when disk percentage is unchanged', function () { + $team = Team::factory()->create(); + $server = Server::factory()->create(['team_id' => $team->id]); + + // Simulate a previous push that cached the percentage + Cache::put('storage-check:'.$server->id, 45, 600); + + $data = [ + 'containers' => [], + 'filesystem_usage_root' => ['used_percentage' => 45], + ]; + + $job = new PushServerUpdateJob($server, $data); + $job->handle(); + + Queue::assertNotPushed(ServerStorageCheckJob::class); +}); + +it('dispatches storage check when disk percentage changes from cached value', function () { + $team = Team::factory()->create(); + $server = Server::factory()->create(['team_id' => $team->id]); + + // Simulate a previous push that cached 45% + Cache::put('storage-check:'.$server->id, 45, 600); + + $data = [ + 'containers' => [], + 'filesystem_usage_root' => ['used_percentage' => 50], + ]; + + $job = new PushServerUpdateJob($server, $data); + $job->handle(); + + Queue::assertPushed(ServerStorageCheckJob::class, function ($job) use ($server) { + return $job->server->id === $server->id && $job->percentage === 50; + }); +}); + +it('rate-limits ConnectProxyToNetworksJob dispatch to every 10 minutes', function () { + $team = Team::factory()->create(); + $server = Server::factory()->create(['team_id' => $team->id]); + $server->settings->update(['is_reachable' => true, 'is_usable' => true]); + + // First push: should dispatch ConnectProxyToNetworksJob + $containersWithProxy = [ + [ + 'name' => 'coolify-proxy', + 'state' => 'running', + 'health_status' => 'healthy', + 'labels' => ['coolify.managed' => true], + ], + ]; + + $data = [ + 'containers' => $containersWithProxy, + 'filesystem_usage_root' => ['used_percentage' => 10], + ]; + + $job = new PushServerUpdateJob($server, $data); + $job->handle(); + + Queue::assertPushed(ConnectProxyToNetworksJob::class, 1); + + // Second push: should NOT dispatch ConnectProxyToNetworksJob (rate-limited) + Queue::fake(); + $job2 = new PushServerUpdateJob($server, $data); + $job2->handle(); + + Queue::assertNotPushed(ConnectProxyToNetworksJob::class); +}); + +it('dispatches ConnectProxyToNetworksJob again after cache expires', function () { + $team = Team::factory()->create(); + $server = Server::factory()->create(['team_id' => $team->id]); + $server->settings->update(['is_reachable' => true, 'is_usable' => true]); + + $containersWithProxy = [ + [ + 'name' => 'coolify-proxy', + 'state' => 'running', + 'health_status' => 'healthy', + 'labels' => ['coolify.managed' => true], + ], + ]; + + $data = [ + 'containers' => $containersWithProxy, + 'filesystem_usage_root' => ['used_percentage' => 10], + ]; + + // First push + $job = new PushServerUpdateJob($server, $data); + $job->handle(); + + Queue::assertPushed(ConnectProxyToNetworksJob::class, 1); + + // Clear cache to simulate expiration + Cache::forget('connect-proxy:'.$server->id); + + // Next push: should dispatch again + Queue::fake(); + $job2 = new PushServerUpdateJob($server, $data); + $job2->handle(); + + Queue::assertPushed(ConnectProxyToNetworksJob::class, 1); +}); + +it('uses default queue for PushServerUpdateJob', function () { + $team = Team::factory()->create(); + $server = Server::factory()->create(['team_id' => $team->id]); + + $job = new PushServerUpdateJob($server, ['containers' => []]); + + expect($job->queue)->toBeNull(); +}); From a0c177f6f24f1f75bfaa84d894b020f9cd60f774 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sat, 28 Feb 2026 15:06:25 +0100 Subject: [PATCH 4/7] feat(jobs): add queue delay resilience to scheduled job execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement dedup key-based cron tracking to make scheduled jobs resilient to queue delays. Even if a job is delayed by minutes, it will catch the missed cron window by tracking previousRunDate in cache instead of relying on isDue() alone. - Add dedupKey parameter to shouldRunNow() in ScheduledJobManager - When provided, uses getPreviousRunDate() + cache tracking for resilience - Falls back to isDue() for docker cleanups without dedup key - Prevents double-dispatch within same cron window - Optimize ServerConnectionCheckJob dispatch - Skip SSH checks if Sentinel is healthy (enabled and live) - Reduces redundant checks when Sentinel heartbeat proves connectivity - Remove hourly Sentinel update checks - Consolidate to daily CheckAndStartSentinelJob dispatch - Crash recovery handled by sentinelOutOfSync → ServerCheckJob flow - Add logging for skipped database backups with context (backup_id, database_id, status) - Refactor skip reason methods to accept server parameter, avoiding redundant queries - Add comprehensive test suite for scheduling with various delay scenarios and timezones --- app/Jobs/DatabaseBackupJob.php | 6 + app/Jobs/ScheduledJobManager.php | 52 +++-- app/Jobs/ServerManagerJob.php | 19 +- .../ScheduledJobManagerShouldRunNowTest.php | 208 ++++++++++++++++++ .../ServerManagerJobSentinelCheckTest.php | 143 ++++++------ 5 files changed, 322 insertions(+), 106 deletions(-) create mode 100644 tests/Feature/ScheduledJobManagerShouldRunNowTest.php diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index a585baa69..f2f454f87 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -111,6 +111,12 @@ public function handle(): void $status = str(data_get($this->database, 'status')); if (! $status->startsWith('running') && $this->database->id !== 0) { + Log::info('DatabaseBackupJob skipped: database not running', [ + 'backup_id' => $this->backup->id, + 'database_id' => $this->database->id, + 'status' => (string) $status, + ]); + return; } if (data_get($this->backup, 'database_type') === \App\Models\ServiceDatabase::class) { diff --git a/app/Jobs/ScheduledJobManager.php b/app/Jobs/ScheduledJobManager.php index de782b96d..fd641abb0 100644 --- a/app/Jobs/ScheduledJobManager.php +++ b/app/Jobs/ScheduledJobManager.php @@ -160,7 +160,8 @@ private function processScheduledBackups(): void foreach ($backups as $backup) { try { - $skipReason = $this->getBackupSkipReason($backup); + $server = $backup->server(); + $skipReason = $this->getBackupSkipReason($backup, $server); if ($skipReason !== null) { $this->skippedCount++; $this->logSkip('backup', $skipReason, [ @@ -173,7 +174,6 @@ private function processScheduledBackups(): void continue; } - $server = $backup->server(); $serverTimezone = data_get($server->settings, 'server_timezone', config('app.timezone')); if (validate_timezone($serverTimezone) === false) { @@ -185,7 +185,7 @@ private function processScheduledBackups(): void $frequency = VALID_CRON_STRINGS[$frequency]; } - if ($this->shouldRunNow($frequency, $serverTimezone)) { + if ($this->shouldRunNow($frequency, $serverTimezone, "scheduled-backup:{$backup->id}")) { DatabaseBackupJob::dispatch($backup); $this->dispatchedCount++; Log::channel('scheduled')->info('Backup dispatched', [ @@ -213,19 +213,19 @@ private function processScheduledTasks(): void foreach ($tasks as $task) { try { - $skipReason = $this->getTaskSkipReason($task); + $server = $task->server(); + $skipReason = $this->getTaskSkipReason($task, $server); if ($skipReason !== null) { $this->skippedCount++; $this->logSkip('task', $skipReason, [ 'task_id' => $task->id, 'task_name' => $task->name, - 'team_id' => $task->server()?->team_id, + 'team_id' => $server?->team_id, ]); continue; } - $server = $task->server(); $serverTimezone = data_get($server->settings, 'server_timezone', config('app.timezone')); if (validate_timezone($serverTimezone) === false) { @@ -237,7 +237,7 @@ private function processScheduledTasks(): void $frequency = VALID_CRON_STRINGS[$frequency]; } - if ($this->shouldRunNow($frequency, $serverTimezone)) { + if ($this->shouldRunNow($frequency, $serverTimezone, "scheduled-task:{$task->id}")) { ScheduledTaskJob::dispatch($task); $this->dispatchedCount++; Log::channel('scheduled')->info('Task dispatched', [ @@ -256,7 +256,7 @@ private function processScheduledTasks(): void } } - private function getBackupSkipReason(ScheduledDatabaseBackup $backup): ?string + private function getBackupSkipReason(ScheduledDatabaseBackup $backup, ?Server $server): ?string { if (blank(data_get($backup, 'database'))) { $backup->delete(); @@ -264,7 +264,6 @@ private function getBackupSkipReason(ScheduledDatabaseBackup $backup): ?string return 'database_deleted'; } - $server = $backup->server(); if (blank($server)) { $backup->delete(); @@ -282,12 +281,11 @@ private function getBackupSkipReason(ScheduledDatabaseBackup $backup): ?string return null; } - private function getTaskSkipReason(ScheduledTask $task): ?string + private function getTaskSkipReason(ScheduledTask $task, ?Server $server): ?string { $service = $task->service; $application = $task->application; - $server = $task->server(); if (blank($server)) { $task->delete(); @@ -319,16 +317,38 @@ private function getTaskSkipReason(ScheduledTask $task): ?string return null; } - private function shouldRunNow(string $frequency, string $timezone): bool + /** + * Determine if a cron schedule should run now. + * + * When a dedupKey is provided, uses getPreviousRunDate() + last-dispatch tracking + * instead of isDue(). This is resilient to queue delays — even if the job is delayed + * by minutes, it still catches the missed cron window. Without dedupKey, falls back + * to simple isDue() check. + */ + private function shouldRunNow(string $frequency, string $timezone, ?string $dedupKey = null): bool { $cron = new CronExpression($frequency); - - // Use the frozen execution time, not the current time - // Fallback to current time if execution time is not set (shouldn't happen) $baseTime = $this->executionTime ?? Carbon::now(); $executionTime = $baseTime->copy()->setTimezone($timezone); - return $cron->isDue($executionTime); + // No dedup key → simple isDue check (used by docker cleanups) + if ($dedupKey === null) { + return $cron->isDue($executionTime); + } + + // Get the most recent time this cron was due (including current minute) + $previousDue = Carbon::instance($cron->getPreviousRunDate($executionTime, allowCurrentDate: true)); + + $lastDispatched = Cache::get($dedupKey); + + // Run if: never dispatched before, OR there's been a due time since last dispatch + if ($lastDispatched === null || $previousDue->gt(Carbon::parse($lastDispatched))) { + Cache::put($dedupKey, $executionTime->toIso8601String(), 86400); + + return true; + } + + return false; } private function processDockerCleanups(): void diff --git a/app/Jobs/ServerManagerJob.php b/app/Jobs/ServerManagerJob.php index a4619354d..c8219a2ea 100644 --- a/app/Jobs/ServerManagerJob.php +++ b/app/Jobs/ServerManagerJob.php @@ -64,11 +64,11 @@ public function handle(): void private function getServers(): Collection { - $allServers = Server::where('ip', '!=', '1.2.3.4'); + $allServers = Server::with('settings')->where('ip', '!=', '1.2.3.4'); if (isCloud()) { $servers = $allServers->whereRelation('team.subscription', 'stripe_invoice_paid', true)->get(); - $own = Team::find(0)->servers; + $own = Team::find(0)->servers()->with('settings')->get(); return $servers->merge($own); } else { @@ -82,6 +82,10 @@ private function dispatchConnectionChecks(Collection $servers): void if ($this->shouldRunNow($this->checkFrequency)) { $servers->each(function (Server $server) { try { + // Skip SSH connection check if Sentinel is healthy — its heartbeat already proves connectivity + if ($server->isSentinelEnabled() && $server->isSentinelLive()) { + return; + } ServerConnectionCheckJob::dispatch($server); } catch (\Exception $e) { Log::channel('scheduled-errors')->error('Failed to dispatch ServerConnectionCheck', [ @@ -134,9 +138,7 @@ private function processServerTasks(Server $server): void // Dispatch Sentinel restart if due (daily for Sentinel-enabled servers) if ($shouldRestartSentinel) { - dispatch(function () use ($server) { - $server->restartContainer('coolify-sentinel'); - }); + CheckAndStartSentinelJob::dispatch($server); } // Dispatch ServerStorageCheckJob if due (only when Sentinel is out of sync or disabled) @@ -160,11 +162,8 @@ private function processServerTasks(Server $server): void ServerPatchCheckJob::dispatch($server); } - // Sentinel update checks (hourly) - check for updates to Sentinel version - // No timezone needed for hourly - runs at top of every hour - if ($isSentinelEnabled && $this->shouldRunNow('0 * * * *')) { - CheckAndStartSentinelJob::dispatch($server); - } + // Note: CheckAndStartSentinelJob is only dispatched daily (line above) for version updates. + // Crash recovery is handled by sentinelOutOfSync → ServerCheckJob → CheckAndStartSentinelJob. } private function shouldRunNow(string $frequency, ?string $timezone = null): bool diff --git a/tests/Feature/ScheduledJobManagerShouldRunNowTest.php b/tests/Feature/ScheduledJobManagerShouldRunNowTest.php new file mode 100644 index 000000000..8862cc71e --- /dev/null +++ b/tests/Feature/ScheduledJobManagerShouldRunNowTest.php @@ -0,0 +1,208 @@ +getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + $result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:1'); + + expect($result)->toBeTrue(); +}); + +it('dispatches backup when job is delayed past the cron minute', function () { + // Freeze time at 02:07 — job was delayed 7 minutes past the 02:00 cron + Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 7, 0, 'UTC')); + + $job = new ScheduledJobManager; + + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + // isDue() would return false at 02:07, but getPreviousRunDate() = 02:00 + // No lastDispatched in cache → should run + $result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:1'); + + expect($result)->toBeTrue(); +}); + +it('does not double-dispatch on subsequent runs within same cron window', function () { + // First run at 02:00 — dispatches and sets cache + Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 0, 0, 'UTC')); + + $job = new ScheduledJobManager; + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + $first = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:2'); + expect($first)->toBeTrue(); + + // Second run at 02:01 — should NOT dispatch (previousDue=02:00, lastDispatched=02:00) + Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 1, 0, 'UTC')); + $executionTimeProp->setValue($job, Carbon::now()); + + $second = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:2'); + expect($second)->toBeFalse(); +}); + +it('fires every_minute cron correctly on consecutive minutes', function () { + $job = new ScheduledJobManager; + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + // Minute 1 + Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 0, 0, 'UTC')); + $executionTimeProp->setValue($job, Carbon::now()); + $result1 = $method->invoke($job, '* * * * *', 'UTC', 'test-backup:3'); + expect($result1)->toBeTrue(); + + // Minute 2 + Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 1, 0, 'UTC')); + $executionTimeProp->setValue($job, Carbon::now()); + $result2 = $method->invoke($job, '* * * * *', 'UTC', 'test-backup:3'); + expect($result2)->toBeTrue(); + + // Minute 3 + Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 2, 0, 'UTC')); + $executionTimeProp->setValue($job, Carbon::now()); + $result3 = $method->invoke($job, '* * * * *', 'UTC', 'test-backup:3'); + expect($result3)->toBeTrue(); +}); + +it('re-dispatches after cache loss instead of missing', function () { + // First run at 02:00 — dispatches + Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 0, 0, 'UTC')); + + $job = new ScheduledJobManager; + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:4'); + + // Simulate Redis restart — cache lost + Cache::forget('test-backup:4'); + + // Run again at 02:01 — should re-dispatch (lastDispatched = null after cache loss) + Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 1, 0, 'UTC')); + $executionTimeProp->setValue($job, Carbon::now()); + + $result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:4'); + expect($result)->toBeTrue(); +}); + +it('does not dispatch when cron is not due and was not recently due', function () { + // Time is 10:00, cron is daily at 02:00 — last due was 8 hours ago + Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 0, 0, 'UTC')); + + $job = new ScheduledJobManager; + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + // previousDue = 02:00, but lastDispatched was set at 02:00 (simulate) + Cache::put('test-backup:5', Carbon::create(2026, 2, 28, 2, 0, 0, 'UTC')->toIso8601String(), 86400); + + $result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:5'); + expect($result)->toBeFalse(); +}); + +it('falls back to isDue when no dedup key is provided', function () { + // Time is exactly 02:00, cron is "0 2 * * *" — should be due + Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 0, 0, 'UTC')); + + $job = new ScheduledJobManager; + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + // No dedup key → simple isDue check + $result = $method->invoke($job, '0 2 * * *', 'UTC'); + expect($result)->toBeTrue(); + + // At 02:01 without dedup key → isDue returns false + Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 1, 0, 'UTC')); + $executionTimeProp->setValue($job, Carbon::now()); + + $result2 = $method->invoke($job, '0 2 * * *', 'UTC'); + expect($result2)->toBeFalse(); +}); + +it('respects server timezone for cron evaluation', function () { + // UTC time is 22:00 Feb 28, which is 06:00 Mar 1 in Asia/Singapore (+8) + Carbon::setTestNow(Carbon::create(2026, 2, 28, 22, 0, 0, 'UTC')); + + $job = new ScheduledJobManager; + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + // Simulate that today's 06:00 UTC run was already dispatched (at 06:00 UTC) + Cache::put('test-backup:7', Carbon::create(2026, 2, 28, 6, 0, 0, 'UTC')->toIso8601String(), 86400); + + // Cron "0 6 * * *" in Asia/Singapore: local time is 06:00 Mar 1 → previousDue = 06:00 Mar 1 SGT + // That's a NEW cron window (Mar 1) that hasn't been dispatched → should fire + $resultSingapore = $method->invoke($job, '0 6 * * *', 'Asia/Singapore', 'test-backup:6'); + expect($resultSingapore)->toBeTrue(); + + // Cron "0 6 * * *" in UTC: previousDue = 06:00 Feb 28 UTC, already dispatched at 06:00 → should NOT fire + $resultUtc = $method->invoke($job, '0 6 * * *', 'UTC', 'test-backup:7'); + expect($resultUtc)->toBeFalse(); +}); diff --git a/tests/Unit/ServerManagerJobSentinelCheckTest.php b/tests/Unit/ServerManagerJobSentinelCheckTest.php index 0f2613f11..d8449adc3 100644 --- a/tests/Unit/ServerManagerJobSentinelCheckTest.php +++ b/tests/Unit/ServerManagerJobSentinelCheckTest.php @@ -1,6 +1,7 @@ instance_timezone = 'UTC'; $this->app->instance(InstanceSettings::class, $settings); - // Create a mock server with sentinel enabled $server = Mockery::mock(Server::class)->makePartial(); $server->shouldReceive('isSentinelEnabled')->andReturn(true); + $server->shouldReceive('isSentinelLive')->andReturn(true); $server->id = 1; $server->name = 'test-server'; $server->ip = '192.168.1.100'; @@ -34,29 +34,76 @@ $server->shouldReceive('getAttribute')->with('settings')->andReturn((object) ['server_timezone' => 'UTC']); $server->shouldReceive('waitBeforeDoingSshCheck')->andReturn(120); - // Mock the Server query Server::shouldReceive('where')->with('ip', '!=', '1.2.3.4')->andReturnSelf(); Server::shouldReceive('get')->andReturn(collect([$server])); - // Execute the job $job = new ServerManagerJob; $job->handle(); - // Assert CheckAndStartSentinelJob was dispatched for the sentinel-enabled server - Queue::assertPushed(CheckAndStartSentinelJob::class, function ($job) use ($server) { - return $job->server->id === $server->id; - }); + // Hourly CheckAndStartSentinelJob dispatch was removed — ServerCheckJob handles it when Sentinel is out of sync + Queue::assertNotPushed(CheckAndStartSentinelJob::class); }); -it('does not dispatch CheckAndStartSentinelJob for servers without sentinel enabled', function () { - // Mock InstanceSettings +it('skips ServerConnectionCheckJob when sentinel is live', function () { + $settings = Mockery::mock(InstanceSettings::class); + $settings->instance_timezone = 'UTC'; + $this->app->instance(InstanceSettings::class, $settings); + + $server = Mockery::mock(Server::class)->makePartial(); + $server->shouldReceive('isSentinelEnabled')->andReturn(true); + $server->shouldReceive('isSentinelLive')->andReturn(true); + $server->id = 1; + $server->name = 'test-server'; + $server->ip = '192.168.1.100'; + $server->sentinel_updated_at = Carbon::now(); + $server->shouldReceive('getAttribute')->with('settings')->andReturn((object) ['server_timezone' => 'UTC']); + $server->shouldReceive('waitBeforeDoingSshCheck')->andReturn(120); + + Server::shouldReceive('where')->with('ip', '!=', '1.2.3.4')->andReturnSelf(); + Server::shouldReceive('get')->andReturn(collect([$server])); + + $job = new ServerManagerJob; + $job->handle(); + + // Sentinel is healthy so SSH connection check is skipped + Queue::assertNotPushed(ServerConnectionCheckJob::class); +}); + +it('dispatches ServerConnectionCheckJob when sentinel is not live', function () { + $settings = Mockery::mock(InstanceSettings::class); + $settings->instance_timezone = 'UTC'; + $this->app->instance(InstanceSettings::class, $settings); + + $server = Mockery::mock(Server::class)->makePartial(); + $server->shouldReceive('isSentinelEnabled')->andReturn(true); + $server->shouldReceive('isSentinelLive')->andReturn(false); + $server->id = 1; + $server->name = 'test-server'; + $server->ip = '192.168.1.100'; + $server->sentinel_updated_at = Carbon::now()->subMinutes(10); + $server->shouldReceive('getAttribute')->with('settings')->andReturn((object) ['server_timezone' => 'UTC']); + $server->shouldReceive('waitBeforeDoingSshCheck')->andReturn(120); + + Server::shouldReceive('where')->with('ip', '!=', '1.2.3.4')->andReturnSelf(); + Server::shouldReceive('get')->andReturn(collect([$server])); + + $job = new ServerManagerJob; + $job->handle(); + + // Sentinel is out of sync so SSH connection check is needed + Queue::assertPushed(ServerConnectionCheckJob::class, function ($job) use ($server) { + return $job->server->id === $server->id; + }); +}); + +it('dispatches ServerConnectionCheckJob when sentinel is not enabled', function () { $settings = Mockery::mock(InstanceSettings::class); $settings->instance_timezone = 'UTC'; $this->app->instance(InstanceSettings::class, $settings); - // Create a mock server with sentinel disabled $server = Mockery::mock(Server::class)->makePartial(); $server->shouldReceive('isSentinelEnabled')->andReturn(false); + $server->shouldReceive('isSentinelLive')->never(); $server->id = 2; $server->name = 'test-server-no-sentinel'; $server->ip = '192.168.1.101'; @@ -64,78 +111,14 @@ $server->shouldReceive('getAttribute')->with('settings')->andReturn((object) ['server_timezone' => 'UTC']); $server->shouldReceive('waitBeforeDoingSshCheck')->andReturn(120); - // Mock the Server query Server::shouldReceive('where')->with('ip', '!=', '1.2.3.4')->andReturnSelf(); Server::shouldReceive('get')->andReturn(collect([$server])); - // Execute the job $job = new ServerManagerJob; $job->handle(); - // Assert CheckAndStartSentinelJob was NOT dispatched - Queue::assertNotPushed(CheckAndStartSentinelJob::class); -}); - -it('respects server timezone when scheduling sentinel checks', function () { - // Mock InstanceSettings - $settings = Mockery::mock(InstanceSettings::class); - $settings->instance_timezone = 'UTC'; - $this->app->instance(InstanceSettings::class, $settings); - - // Set test time to top of hour in America/New_York (which is 17:00 UTC) - Carbon::setTestNow('2025-01-15 17:00:00'); // 12:00 PM EST (top of hour in EST) - - // Create a mock server with sentinel enabled and America/New_York timezone - $server = Mockery::mock(Server::class)->makePartial(); - $server->shouldReceive('isSentinelEnabled')->andReturn(true); - $server->id = 3; - $server->name = 'test-server-est'; - $server->ip = '192.168.1.102'; - $server->sentinel_updated_at = Carbon::now(); - $server->shouldReceive('getAttribute')->with('settings')->andReturn((object) ['server_timezone' => 'America/New_York']); - $server->shouldReceive('waitBeforeDoingSshCheck')->andReturn(120); - - // Mock the Server query - Server::shouldReceive('where')->with('ip', '!=', '1.2.3.4')->andReturnSelf(); - Server::shouldReceive('get')->andReturn(collect([$server])); - - // Execute the job - $job = new ServerManagerJob; - $job->handle(); - - // Assert CheckAndStartSentinelJob was dispatched (should run at top of hour in server's timezone) - Queue::assertPushed(CheckAndStartSentinelJob::class, function ($job) use ($server) { + // Sentinel is not enabled so SSH connection check must run + Queue::assertPushed(ServerConnectionCheckJob::class, function ($job) use ($server) { return $job->server->id === $server->id; }); }); - -it('does not dispatch sentinel check when not at top of hour', function () { - // Mock InstanceSettings - $settings = Mockery::mock(InstanceSettings::class); - $settings->instance_timezone = 'UTC'; - $this->app->instance(InstanceSettings::class, $settings); - - // Set test time to middle of the hour (not top of hour) - Carbon::setTestNow('2025-01-15 12:30:00'); - - // Create a mock server with sentinel enabled - $server = Mockery::mock(Server::class)->makePartial(); - $server->shouldReceive('isSentinelEnabled')->andReturn(true); - $server->id = 4; - $server->name = 'test-server-mid-hour'; - $server->ip = '192.168.1.103'; - $server->sentinel_updated_at = Carbon::now(); - $server->shouldReceive('getAttribute')->with('settings')->andReturn((object) ['server_timezone' => 'UTC']); - $server->shouldReceive('waitBeforeDoingSshCheck')->andReturn(120); - - // Mock the Server query - Server::shouldReceive('where')->with('ip', '!=', '1.2.3.4')->andReturnSelf(); - Server::shouldReceive('get')->andReturn(collect([$server])); - - // Execute the job - $job = new ServerManagerJob; - $job->handle(); - - // Assert CheckAndStartSentinelJob was NOT dispatched (not top of hour) - Queue::assertNotPushed(CheckAndStartSentinelJob::class); -}); From 63be5928ab109c0df51dd000642b35fba3961bfb Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sat, 28 Feb 2026 16:23:58 +0100 Subject: [PATCH 5/7] feat(scheduler): add pagination to skipped jobs and filter manager start events - Implement pagination for skipped jobs display with 20 items per page - Add pagination controls (previous/next buttons) to the scheduled jobs view - Exclude ScheduledJobManager "started" events from run logs, keeping only "completed" events - Add ShouldBeEncrypted interface to ScheduledTaskJob for secure queue handling - Update log filtering to fetch 500 recent skips and slice for pagination - Use Log facade instead of fully qualified class name --- app/Jobs/DatabaseBackupJob.php | 2 +- app/Jobs/DockerCleanupJob.php | 2 + app/Jobs/ScheduledTaskJob.php | 3 +- app/Livewire/Settings/ScheduledJobs.php | 38 ++++++++++++++++++- app/Services/SchedulerLogParser.php | 2 +- .../settings/scheduled-jobs.blade.php | 23 ++++++++++- tests/Feature/ScheduledJobMonitoringTest.php | 36 ++++++++++++++++++ 7 files changed, 101 insertions(+), 5 deletions(-) diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index f2f454f87..5fc9f6cd8 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -478,7 +478,7 @@ private function backup_standalone_mongodb(string $databaseWithCollections): voi throw new \Exception('MongoDB credentials not found. Ensure MONGO_INITDB_ROOT_USERNAME and MONGO_INITDB_ROOT_PASSWORD environment variables are available in the container.'); } } - \Log::info('MongoDB backup URL configured', ['has_url' => filled($url), 'using_env_vars' => blank($this->database->internal_db_url)]); + Log::info('MongoDB backup URL configured', ['has_url' => filled($url), 'using_env_vars' => blank($this->database->internal_db_url)]); if ($databaseWithCollections === 'all') { $commands[] = 'mkdir -p '.$this->backup_dir; if (str($this->database->image)->startsWith('mongo:4')) { diff --git a/app/Jobs/DockerCleanupJob.php b/app/Jobs/DockerCleanupJob.php index f3f3a2ae4..78ef7f3a2 100644 --- a/app/Jobs/DockerCleanupJob.php +++ b/app/Jobs/DockerCleanupJob.php @@ -91,6 +91,8 @@ public function handle(): void $this->server->team?->notify(new DockerCleanupSuccess($this->server, $message)); event(new DockerCleanupDone($this->execution_log)); + + return; } if ($this->usageBefore >= $this->server->settings->docker_cleanup_threshold) { diff --git a/app/Jobs/ScheduledTaskJob.php b/app/Jobs/ScheduledTaskJob.php index b21bc11a1..49b9b9702 100644 --- a/app/Jobs/ScheduledTaskJob.php +++ b/app/Jobs/ScheduledTaskJob.php @@ -14,13 +14,14 @@ use App\Notifications\ScheduledTask\TaskSuccess; use Carbon\Carbon; use Illuminate\Bus\Queueable; +use Illuminate\Contracts\Queue\ShouldBeEncrypted; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; use Illuminate\Support\Facades\Log; -class ScheduledTaskJob implements ShouldQueue +class ScheduledTaskJob implements ShouldBeEncrypted, ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; diff --git a/app/Livewire/Settings/ScheduledJobs.php b/app/Livewire/Settings/ScheduledJobs.php index 66480cd8d..4947dd19b 100644 --- a/app/Livewire/Settings/ScheduledJobs.php +++ b/app/Livewire/Settings/ScheduledJobs.php @@ -16,6 +16,18 @@ class ScheduledJobs extends Component public string $filterDate = 'last_24h'; + public int $skipPage = 0; + + public int $skipDefaultTake = 20; + + public bool $showSkipNext = false; + + public bool $showSkipPrev = false; + + public int $skipCurrentPage = 1; + + public int $skipTotalCount = 0; + protected Collection $executions; protected Collection $skipLogs; @@ -42,11 +54,30 @@ public function mount(): void public function updatedFilterType(): void { + $this->skipPage = 0; $this->loadData(); } public function updatedFilterDate(): void { + $this->skipPage = 0; + $this->loadData(); + } + + public function skipNextPage(): void + { + $this->skipPage += $this->skipDefaultTake; + $this->showSkipPrev = true; + $this->loadData(); + } + + public function skipPreviousPage(): void + { + $this->skipPage -= $this->skipDefaultTake; + if ($this->skipPage < 0) { + $this->skipPage = 0; + } + $this->showSkipPrev = $this->skipPage > 0; $this->loadData(); } @@ -69,7 +100,12 @@ private function loadData(?int $teamId = null): void $this->executions = $this->getExecutions($teamId); $parser = new SchedulerLogParser; - $this->skipLogs = $parser->getRecentSkips(50, $teamId); + $allSkips = $parser->getRecentSkips(500, $teamId); + $this->skipTotalCount = $allSkips->count(); + $this->skipLogs = $allSkips->slice($this->skipPage, $this->skipDefaultTake)->values(); + $this->showSkipPrev = $this->skipPage > 0; + $this->showSkipNext = ($this->skipPage + $this->skipDefaultTake) < $this->skipTotalCount; + $this->skipCurrentPage = intval($this->skipPage / $this->skipDefaultTake) + 1; $this->managerRuns = $parser->getRecentRuns(30, $teamId); } diff --git a/app/Services/SchedulerLogParser.php b/app/Services/SchedulerLogParser.php index a735a11c3..6e29851df 100644 --- a/app/Services/SchedulerLogParser.php +++ b/app/Services/SchedulerLogParser.php @@ -64,7 +64,7 @@ public function getRecentRuns(int $limit = 60, ?int $teamId = null): Collection continue; } - if (! str_contains($entry['message'], 'ScheduledJobManager')) { + if (! str_contains($entry['message'], 'ScheduledJobManager') || str_contains($entry['message'], 'started')) { continue; } diff --git a/resources/views/livewire/settings/scheduled-jobs.blade.php b/resources/views/livewire/settings/scheduled-jobs.blade.php index d22aca911..60acc9062 100644 --- a/resources/views/livewire/settings/scheduled-jobs.blade.php +++ b/resources/views/livewire/settings/scheduled-jobs.blade.php @@ -34,7 +34,7 @@ class="flex flex-col gap-8"> ]) :class="activeTab === 'skipped-jobs' && 'dark:bg-coollabs bg-coollabs text-white'" @click="activeTab = 'skipped-jobs'; window.location.hash = 'skipped-jobs'"> - Skipped Jobs ({{ $skipLogs->count() }}) + Skipped Jobs ({{ $skipTotalCount }}) @@ -186,6 +186,27 @@ class="border-b border-gray-200 dark:border-coolgray-400"> {{-- Skipped Jobs Tab --}}
Jobs that were not dispatched because conditions were not met.
+ @if($skipTotalCount > $skipDefaultTake) +
+ + + + + + + Page {{ $skipCurrentPage }} of {{ ceil($skipTotalCount / $skipDefaultTake) }} + + + + + + +
+ @endif
diff --git a/tests/Feature/ScheduledJobMonitoringTest.php b/tests/Feature/ScheduledJobMonitoringTest.php index 1348375d4..90a3f57a4 100644 --- a/tests/Feature/ScheduledJobMonitoringTest.php +++ b/tests/Feature/ScheduledJobMonitoringTest.php @@ -173,6 +173,42 @@ @unlink($logPath); }); +test('scheduler log parser excludes started events from runs', function () { + $logPath = storage_path('logs/scheduled-test-started-filter.log'); + $logDir = dirname($logPath); + if (! is_dir($logDir)) { + mkdir($logDir, 0755, true); + } + + // Temporarily rename existing logs so they don't interfere + $existingLogs = glob(storage_path('logs/scheduled-*.log')); + $renamed = []; + foreach ($existingLogs as $log) { + $tmp = $log.'.bak'; + rename($log, $tmp); + $renamed[$tmp] = $log; + } + + $logPath = storage_path('logs/scheduled-'.now()->format('Y-m-d').'.log'); + $lines = [ + '['.now()->format('Y-m-d H:i:s').'] production.INFO: ScheduledJobManager started {}', + '['.now()->format('Y-m-d H:i:s').'] production.INFO: ScheduledJobManager completed {"duration_ms":74,"dispatched":1,"skipped":13}', + ]; + file_put_contents($logPath, implode("\n", $lines)."\n"); + + $parser = new SchedulerLogParser; + $runs = $parser->getRecentRuns(); + + expect($runs)->toHaveCount(1); + expect($runs->first()['message'])->toContain('completed'); + + // Cleanup + @unlink($logPath); + foreach ($renamed as $tmp => $original) { + rename($tmp, $original); + } +}); + test('scheduler log parser filters by team id', function () { $logPath = storage_path('logs/scheduled-'.now()->format('Y-m-d').'.log'); $logDir = dirname($logPath); From 31555f9e8a3a589e309eefc6a9267da66ecfe12a Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sat, 28 Feb 2026 18:03:29 +0100 Subject: [PATCH 6/7] fix(jobs): prevent non-due jobs firing on restart and enrich skip logs with resource links - Refactor shouldRunNow() to only fire on first run (empty cache) if actually due by cron schedule, preventing spurious executions after cache loss or service restart - Add enrichSkipLogsWithLinks() method to fetch and populate resource names and links for tasks, backups, and docker cleanup jobs in skip logs - Update skip logs UI to display resource column with links to related resources, improving navigation and context - Add fallback display when linked resources are deleted - Expand tests to cover both restart scenarios: non-due jobs (should not fire) and due jobs (should fire) --- app/Jobs/ScheduledJobManager.php | 15 +++- app/Livewire/Settings/ScheduledJobs.php | 76 ++++++++++++++++++- .../settings/scheduled-jobs.blade.php | 22 +++--- .../ScheduledJobManagerShouldRunNowTest.php | 46 +++++++---- tests/Feature/ScheduledJobMonitoringTest.php | 36 +++++++++ 5 files changed, 166 insertions(+), 29 deletions(-) diff --git a/app/Jobs/ScheduledJobManager.php b/app/Jobs/ScheduledJobManager.php index fd641abb0..4195a1946 100644 --- a/app/Jobs/ScheduledJobManager.php +++ b/app/Jobs/ScheduledJobManager.php @@ -341,8 +341,19 @@ private function shouldRunNow(string $frequency, string $timezone, ?string $dedu $lastDispatched = Cache::get($dedupKey); - // Run if: never dispatched before, OR there's been a due time since last dispatch - if ($lastDispatched === null || $previousDue->gt(Carbon::parse($lastDispatched))) { + if ($lastDispatched === null) { + // First run after restart or cache loss: only fire if actually due right now. + // Seed the cache so subsequent runs can use tolerance/catch-up logic. + $isDue = $cron->isDue($executionTime); + if ($isDue) { + Cache::put($dedupKey, $executionTime->toIso8601String(), 86400); + } + + return $isDue; + } + + // Subsequent runs: fire if there's been a due time since last dispatch + if ($previousDue->gt(Carbon::parse($lastDispatched))) { Cache::put($dedupKey, $executionTime->toIso8601String(), 86400); return true; diff --git a/app/Livewire/Settings/ScheduledJobs.php b/app/Livewire/Settings/ScheduledJobs.php index 4947dd19b..1e54f1483 100644 --- a/app/Livewire/Settings/ScheduledJobs.php +++ b/app/Livewire/Settings/ScheduledJobs.php @@ -3,8 +3,11 @@ namespace App\Livewire\Settings; use App\Models\DockerCleanupExecution; +use App\Models\ScheduledDatabaseBackup; use App\Models\ScheduledDatabaseBackupExecution; +use App\Models\ScheduledTask; use App\Models\ScheduledTaskExecution; +use App\Models\Server; use App\Services\SchedulerLogParser; use Illuminate\Support\Carbon; use Illuminate\Support\Collection; @@ -102,13 +105,84 @@ private function loadData(?int $teamId = null): void $parser = new SchedulerLogParser; $allSkips = $parser->getRecentSkips(500, $teamId); $this->skipTotalCount = $allSkips->count(); - $this->skipLogs = $allSkips->slice($this->skipPage, $this->skipDefaultTake)->values(); + $this->skipLogs = $this->enrichSkipLogsWithLinks( + $allSkips->slice($this->skipPage, $this->skipDefaultTake)->values() + ); $this->showSkipPrev = $this->skipPage > 0; $this->showSkipNext = ($this->skipPage + $this->skipDefaultTake) < $this->skipTotalCount; $this->skipCurrentPage = intval($this->skipPage / $this->skipDefaultTake) + 1; $this->managerRuns = $parser->getRecentRuns(30, $teamId); } + private function enrichSkipLogsWithLinks(Collection $skipLogs): Collection + { + $taskIds = $skipLogs->where('type', 'task')->pluck('context.task_id')->filter()->unique()->values(); + $backupIds = $skipLogs->where('type', 'backup')->pluck('context.backup_id')->filter()->unique()->values(); + $serverIds = $skipLogs->where('type', 'docker_cleanup')->pluck('context.server_id')->filter()->unique()->values(); + + $tasks = $taskIds->isNotEmpty() + ? ScheduledTask::with(['application.environment.project', 'service.environment.project'])->whereIn('id', $taskIds)->get()->keyBy('id') + : collect(); + + $backups = $backupIds->isNotEmpty() + ? ScheduledDatabaseBackup::with(['database.environment.project'])->whereIn('id', $backupIds)->get()->keyBy('id') + : collect(); + + $servers = $serverIds->isNotEmpty() + ? Server::whereIn('id', $serverIds)->get()->keyBy('id') + : collect(); + + return $skipLogs->map(function (array $skip) use ($tasks, $backups, $servers): array { + $skip['link'] = null; + $skip['resource_name'] = null; + + if ($skip['type'] === 'task') { + $task = $tasks->get($skip['context']['task_id'] ?? null); + if ($task) { + $skip['resource_name'] = $skip['context']['task_name'] ?? $task->name; + $resource = $task->application ?? $task->service; + $environment = $resource?->environment; + $project = $environment?->project; + if ($project && $environment && $resource) { + $routeName = $task->application_id + ? 'project.application.scheduled-tasks' + : 'project.service.scheduled-tasks'; + $routeKey = $task->application_id ? 'application_uuid' : 'service_uuid'; + $skip['link'] = route($routeName, [ + 'project_uuid' => $project->uuid, + 'environment_uuid' => $environment->uuid, + $routeKey => $resource->uuid, + 'task_uuid' => $task->uuid, + ]); + } + } + } elseif ($skip['type'] === 'backup') { + $backup = $backups->get($skip['context']['backup_id'] ?? null); + if ($backup) { + $database = $backup->database; + $skip['resource_name'] = $database?->name ?? 'Database backup'; + $environment = $database?->environment; + $project = $environment?->project; + if ($project && $environment && $database) { + $skip['link'] = route('project.database.backup.index', [ + 'project_uuid' => $project->uuid, + 'environment_uuid' => $environment->uuid, + 'database_uuid' => $database->uuid, + ]); + } + } + } elseif ($skip['type'] === 'docker_cleanup') { + $server = $servers->get($skip['context']['server_id'] ?? null); + if ($server) { + $skip['resource_name'] = $server->name; + $skip['link'] = route('server.show', ['server_uuid' => $server->uuid]); + } + } + + return $skip; + }); + } + private function getExecutions(?int $teamId = null): Collection { $dateFrom = $this->getDateFrom(); diff --git a/resources/views/livewire/settings/scheduled-jobs.blade.php b/resources/views/livewire/settings/scheduled-jobs.blade.php index 60acc9062..7c0db860f 100644 --- a/resources/views/livewire/settings/scheduled-jobs.blade.php +++ b/resources/views/livewire/settings/scheduled-jobs.blade.php @@ -213,8 +213,8 @@ class="border-b border-gray-200 dark:border-coolgray-400"> + - @@ -235,6 +235,17 @@ class="border-b border-gray-200 dark:border-coolgray-400"> {{ ucfirst(str_replace('_', ' ', $skip['type'])) }} + - @empty diff --git a/tests/Feature/ScheduledJobManagerShouldRunNowTest.php b/tests/Feature/ScheduledJobManagerShouldRunNowTest.php index 8862cc71e..f820c3777 100644 --- a/tests/Feature/ScheduledJobManagerShouldRunNowTest.php +++ b/tests/Feature/ScheduledJobManagerShouldRunNowTest.php @@ -30,8 +30,11 @@ expect($result)->toBeTrue(); }); -it('dispatches backup when job is delayed past the cron minute', function () { - // Freeze time at 02:07 — job was delayed 7 minutes past the 02:00 cron +it('catches delayed job when cache has a baseline from previous run', function () { + // Simulate a previous dispatch yesterday at 02:00 + Cache::put('test-backup:1', Carbon::create(2026, 2, 27, 2, 0, 0, 'UTC')->toIso8601String(), 86400); + + // Freeze time at 02:07 — job was delayed 7 minutes past today's 02:00 cron Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 7, 0, 'UTC')); $job = new ScheduledJobManager; @@ -45,8 +48,8 @@ $method = $reflection->getMethod('shouldRunNow'); $method->setAccessible(true); - // isDue() would return false at 02:07, but getPreviousRunDate() = 02:00 - // No lastDispatched in cache → should run + // isDue() would return false at 02:07, but getPreviousRunDate() = 02:00 today + // lastDispatched = 02:00 yesterday → 02:00 today > yesterday → fires $result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:1'); expect($result)->toBeTrue(); @@ -106,8 +109,27 @@ expect($result3)->toBeTrue(); }); -it('re-dispatches after cache loss instead of missing', function () { - // First run at 02:00 — dispatches +it('does not fire non-due jobs on restart when cache is empty', function () { + // Time is 10:00, cron is daily at 02:00 — NOT due right now + Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 0, 0, 'UTC')); + + $job = new ScheduledJobManager; + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + // Cache is empty (fresh restart) — should NOT fire daily backup at 10:00 + $result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:4'); + expect($result)->toBeFalse(); +}); + +it('fires due jobs on restart when cache is empty', function () { + // Time is exactly 02:00, cron is daily at 02:00 — IS due right now Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 0, 0, 'UTC')); $job = new ScheduledJobManager; @@ -120,16 +142,8 @@ $method = $reflection->getMethod('shouldRunNow'); $method->setAccessible(true); - $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:4'); - - // Simulate Redis restart — cache lost - Cache::forget('test-backup:4'); - - // Run again at 02:01 — should re-dispatch (lastDispatched = null after cache loss) - Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 1, 0, 'UTC')); - $executionTimeProp->setValue($job, Carbon::now()); - - $result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:4'); + // Cache is empty (fresh restart) — but cron IS due → should fire + $result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:4b'); expect($result)->toBeTrue(); }); diff --git a/tests/Feature/ScheduledJobMonitoringTest.php b/tests/Feature/ScheduledJobMonitoringTest.php index 90a3f57a4..036c3b638 100644 --- a/tests/Feature/ScheduledJobMonitoringTest.php +++ b/tests/Feature/ScheduledJobMonitoringTest.php @@ -234,3 +234,39 @@ // Cleanup @unlink($logPath); }); + +test('skipped jobs show fallback when resource is deleted', function () { + $this->actingAs($this->rootUser); + session(['currentTeam' => $this->rootTeam]); + + $logPath = storage_path('logs/scheduled-'.now()->format('Y-m-d').'.log'); + $logDir = dirname($logPath); + if (! is_dir($logDir)) { + mkdir($logDir, 0755, true); + } + + // Temporarily rename existing logs so they don't interfere + $existingLogs = glob(storage_path('logs/scheduled-*.log')); + $renamed = []; + foreach ($existingLogs as $log) { + $tmp = $log.'.bak'; + rename($log, $tmp); + $renamed[$tmp] = $log; + } + + $lines = [ + '['.now()->format('Y-m-d H:i:s').'] production.INFO: Task skipped {"type":"task","skip_reason":"application_not_running","task_id":99999,"task_name":"my-cron-job","team_id":0}', + ]; + file_put_contents($logPath, implode("\n", $lines)."\n"); + + Livewire::test(ScheduledJobs::class) + ->assertStatus(200) + ->assertSee('my-cron-job') + ->assertSee('Application not running'); + + // Cleanup + @unlink($logPath); + foreach ($renamed as $tmp => $original) { + rename($tmp, $original); + } +}); From 9a4b4280be5ad6e238cea4ffc267d64c8cd5289a Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sat, 28 Feb 2026 18:37:51 +0100 Subject: [PATCH 7/7] refactor(jobs): split task skip checks into critical and runtime phases Move expensive runtime checks (service/application status) after cron validation to avoid running them for tasks that aren't due. Critical checks (orphans, infrastructure) remain in first phase. Also fix database heading parameters to be built from the model. --- app/Jobs/ScheduledJobManager.php | 49 ++++++++++++++++------- app/Livewire/Project/Database/Heading.php | 6 ++- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/app/Jobs/ScheduledJobManager.php b/app/Jobs/ScheduledJobManager.php index 4195a1946..e68e3b613 100644 --- a/app/Jobs/ScheduledJobManager.php +++ b/app/Jobs/ScheduledJobManager.php @@ -214,10 +214,12 @@ private function processScheduledTasks(): void foreach ($tasks as $task) { try { $server = $task->server(); - $skipReason = $this->getTaskSkipReason($task, $server); - if ($skipReason !== null) { + + // Phase 1: Critical checks (always — cheap, handles orphans and infra issues) + $criticalSkip = $this->getTaskCriticalSkipReason($task, $server); + if ($criticalSkip !== null) { $this->skippedCount++; - $this->logSkip('task', $skipReason, [ + $this->logSkip('task', $criticalSkip, [ 'task_id' => $task->id, 'task_name' => $task->name, 'team_id' => $server?->team_id, @@ -237,16 +239,31 @@ private function processScheduledTasks(): void $frequency = VALID_CRON_STRINGS[$frequency]; } - if ($this->shouldRunNow($frequency, $serverTimezone, "scheduled-task:{$task->id}")) { - ScheduledTaskJob::dispatch($task); - $this->dispatchedCount++; - Log::channel('scheduled')->info('Task dispatched', [ + if (! $this->shouldRunNow($frequency, $serverTimezone, "scheduled-task:{$task->id}")) { + continue; + } + + // Phase 2: Runtime checks (only when cron is due — avoids noise for stopped resources) + $runtimeSkip = $this->getTaskRuntimeSkipReason($task); + if ($runtimeSkip !== null) { + $this->skippedCount++; + $this->logSkip('task', $runtimeSkip, [ 'task_id' => $task->id, 'task_name' => $task->name, 'team_id' => $server->team_id, - 'server_id' => $server->id, ]); + + continue; } + + ScheduledTaskJob::dispatch($task); + $this->dispatchedCount++; + Log::channel('scheduled')->info('Task dispatched', [ + 'task_id' => $task->id, + 'task_name' => $task->name, + 'team_id' => $server->team_id, + 'server_id' => $server->id, + ]); } catch (\Exception $e) { Log::channel('scheduled-errors')->error('Error processing task', [ 'task_id' => $task->id, @@ -281,11 +298,8 @@ private function getBackupSkipReason(ScheduledDatabaseBackup $backup, ?Server $s return null; } - private function getTaskSkipReason(ScheduledTask $task, ?Server $server): ?string + private function getTaskCriticalSkipReason(ScheduledTask $task, ?Server $server): ?string { - $service = $task->service; - $application = $task->application; - if (blank($server)) { $task->delete(); @@ -300,17 +314,22 @@ private function getTaskSkipReason(ScheduledTask $task, ?Server $server): ?strin return 'subscription_unpaid'; } - if (! $service && ! $application) { + if (! $task->service && ! $task->application) { $task->delete(); return 'resource_deleted'; } - if ($application && str($application->status)->contains('running') === false) { + return null; + } + + private function getTaskRuntimeSkipReason(ScheduledTask $task): ?string + { + if ($task->application && str($task->application->status)->contains('running') === false) { return 'application_not_running'; } - if ($service && str($service->status)->contains('running') === false) { + if ($task->service && str($task->service->status)->contains('running') === false) { return 'service_not_running'; } diff --git a/app/Livewire/Project/Database/Heading.php b/app/Livewire/Project/Database/Heading.php index 8d3d8e294..c6c9a3c48 100644 --- a/app/Livewire/Project/Database/Heading.php +++ b/app/Livewire/Project/Database/Heading.php @@ -69,7 +69,11 @@ public function manualCheckStatus() public function mount() { - $this->parameters = get_route_parameters(); + $this->parameters = [ + 'project_uuid' => $this->database->environment->project->uuid, + 'environment_uuid' => $this->database->environment->uuid, + 'database_uuid' => $this->database->uuid, + ]; } public function stop()
Time TypeResource ReasonDetails
+ @if($skip['link'] ?? null) + + {{ $skip['resource_name'] }} + + @elseif($skip['resource_name'] ?? null) + {{ $skip['resource_name'] }} + @else + {{ $skip['context']['task_name'] ?? $skip['context']['server_name'] ?? 'Deleted' }} + @endif + @php $reasonLabel = match($skip['reason']) { @@ -256,15 +267,6 @@ class="border-b border-gray-200 dark:border-coolgray-400"> @endphp {{ $reasonLabel }} - @php - $details = collect($skip['context']) - ->except(['type', 'skip_reason', 'execution_time']) - ->map(fn($v, $k) => str_replace('_', ' ', $k) . ': ' . $v) - ->implode(', '); - @endphp - {{ $details }} -