From 20a9855c04573530cd2772c5c4c4610cd87c492f Mon Sep 17 00:00:00 2001
From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com>
Date: Sat, 13 Dec 2025 21:42:19 +0100
Subject: [PATCH] Address CodeRabbit review feedback
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Fix null-safe operator on currentTeam() call in Upgrade.php
- Add --rm flag to docker run in upgrade.sh for cleanup consistency
- Store beforeunload handler as named reference and clean up on success
- Add clarifying comments for upgrade method calls
- Add error state handling with close option in upgrade modal
- Add step mapping documentation comment in upgrade-progress component

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 app/Livewire/Upgrade.php                      |  2 +-
 .../components/upgrade-progress.blade.php     |  9 +++
 resources/views/livewire/upgrade.blade.php    | 72 +++++++++++++++++--
 scripts/upgrade.sh                            |  2 +-
 4 files changed, 78 insertions(+), 7 deletions(-)

diff --git a/app/Livewire/Upgrade.php b/app/Livewire/Upgrade.php
index 70723135f..37cc8ec28 100644
--- a/app/Livewire/Upgrade.php
+++ b/app/Livewire/Upgrade.php
@@ -55,7 +55,7 @@ public function upgrade()
     public function getUpgradeStatus(): array
     {
         // Only root team members can view upgrade status
-        if (auth()->user()?->currentTeam()->id !== 0) {
+        if (auth()->user()?->currentTeam()?->id !== 0) {
             return ['status' => 'none'];
         }
 
diff --git a/resources/views/components/upgrade-progress.blade.php b/resources/views/components/upgrade-progress.blade.php
index 13eca4f5b..d65273cce 100644
--- a/resources/views/components/upgrade-progress.blade.php
+++ b/resources/views/components/upgrade-progress.blade.php
@@ -1,5 +1,14 @@
 @props(['step' => 0])
 
+{{--
+    Step Mapping (Backend → UI):
+    Backend steps 1-2 (config download, env update) → UI Step 1: Preparing
+    Backend step 3 (pulling images) → UI Step 2: Helper + UI Step 3: Image
+    Backend steps 4-5 (stop/start containers) → UI Step 4: Restart
+    Backend step 6 (complete) → mapped in JS mapStepToUI() in upgrade.blade.php
+
+    The currentStep variable is inherited from parent Alpine component (upgradeModal).
+--}}
 <div class="w-full max-w-md mx-auto" x-data="{ activeStep: {{ $step }} }" x-effect="activeStep = $el.closest('[x-data]')?.__x?.$data?.currentStep ?? {{ $step }}">
     <div class="flex items-center justify-between">
         {{-- Step 1: Preparing --}}
diff --git a/resources/views/livewire/upgrade.blade.php b/resources/views/livewire/upgrade.blade.php
index d9d717f44..dcc664e84 100644
--- a/resources/views/livewire/upgrade.blade.php
+++ b/resources/views/livewire/upgrade.blade.php
@@ -51,7 +51,7 @@ class="relative w-full py-6 border rounded-sm min-w-full lg:min-w-[36rem] max-w-
                                     {{ $currentVersion }} <span class="mx-1">&rarr;</span> {{ $latestVersion }}
                                 </div>
                             </div>
-                            <button x-show="!showProgress" @click="modalOpen=false"
+                            <button x-show="!showProgress || upgradeError" @click="upgradeError ? closeErrorModal() : modalOpen=false"
                                 class="absolute top-0 right-0 flex items-center justify-center w-8 h-8 mt-5 mr-5 text-gray-600 rounded-full hover:text-gray-800 hover:bg-gray-50 dark:text-neutral-400 dark:hover:text-white dark:hover:bg-coolgray-300">
                                 <svg class="w-5 h-5" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24"
                                     stroke-width="1.5" stroke="currentColor">
@@ -79,7 +79,7 @@ class="absolute top-0 right-0 flex items-center justify-center w-8 h-8 mt-5 mr-5
                                     {{-- Current Status Message --}}
                                     <div class="p-4 rounded-lg bg-neutral-200 dark:bg-coolgray-200">
                                         <div class="flex items-center gap-3">
-                                            <template x-if="!upgradeComplete">
+                                            <template x-if="!upgradeComplete && !upgradeError">
                                                 <svg class="w-5 h-5 text-warning animate-spin"
                                                     xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24">
                                                     <circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor"
@@ -97,6 +97,14 @@ class="absolute top-0 right-0 flex items-center justify-center w-8 h-8 mt-5 mr-5
                                                         clip-rule="evenodd" />
                                                 </svg>
                                             </template>
+                                            <template x-if="upgradeError">
+                                                <svg class="w-5 h-5 text-error" xmlns="http://www.w3.org/2000/svg"
+                                                    viewBox="0 0 20 20" fill="currentColor">
+                                                    <path fill-rule="evenodd"
+                                                        d="M18 10a8 8 0 11-16 0 8 8 0 0116 0zm-7 4a1 1 0 11-2 0 1 1 0 012 0zm-1-9a1 1 0 00-1 1v4a1 1 0 102 0V6a1 1 0 00-1-1z"
+                                                        clip-rule="evenodd" />
+                                                </svg>
+                                            </template>
                                             <span x-text="currentStatus" class="text-sm"></span>
                                         </div>
                                     </div>
@@ -113,6 +121,18 @@ class="font-bold text-warning"></span> seconds...
                                             </x-forms.button>
                                         </div>
                                     </template>
+
+                                    {{-- Error State with Close Button --}}
+                                    <template x-if="upgradeError">
+                                        <div class="flex flex-col items-center gap-4">
+                                            <p class="text-sm text-neutral-600 dark:text-neutral-400">
+                                                Check the logs on the server at /data/coolify/source/upgrade*.
+                                            </p>
+                                            <x-forms.button @click="closeErrorModal()" type="button">
+                                                Close
+                                            </x-forms.button>
+                                        </div>
+                                    </template>
                                 </div>
                             </template>
 
@@ -168,6 +188,7 @@ class="w-24 dark:bg-coolgray-200 dark:hover:bg-coolgray-300">Cancel
             elapsedTime: 0,
             currentStep: 0,
             upgradeComplete: false,
+            upgradeError: false,
             successCountdown: 3,
             currentVersion: config.currentVersion || '',
             latestVersion: config.latestVersion || '',
@@ -178,12 +199,16 @@ class="w-24 dark:bg-coolgray-200 dark:hover:bg-coolgray-300">Cancel
                 this.currentStep = 1;
                 this.currentStatus = 'Starting upgrade...';
                 this.startTimer();
+                // Trigger server-side upgrade script via Livewire
                 this.$wire.$call('upgrade');
+                // Start client-side status polling
                 this.upgrade();
-                window.addEventListener('beforeunload', (event) => {
+                // Prevent accidental navigation during upgrade
+                this.beforeUnloadHandler = (event) => {
                     event.preventDefault();
                     event.returnValue = '';
-                });
+                };
+                window.addEventListener('beforeunload', this.beforeUnloadHandler);
             },
 
             startTimer() {
@@ -259,6 +284,11 @@ class="w-24 dark:bg-coolgray-200 dark:hover:bg-coolgray-300">Cancel
                     clearInterval(this.elapsedInterval);
                     this.elapsedInterval = null;
                 }
+                // Remove beforeunload handler now that upgrade is complete
+                if (this.beforeUnloadHandler) {
+                    window.removeEventListener('beforeunload', this.beforeUnloadHandler);
+                    this.beforeUnloadHandler = null;
+                }
 
                 this.upgradeComplete = true;
                 this.currentStep = 5;
@@ -278,6 +308,38 @@ class="w-24 dark:bg-coolgray-200 dark:hover:bg-coolgray-300">Cancel
                 window.location.reload();
             },
 
+            showError(message) {
+                // Stop all intervals
+                if (this.checkHealthInterval) {
+                    clearInterval(this.checkHealthInterval);
+                    this.checkHealthInterval = null;
+                }
+                if (this.checkUpgradeStatusInterval) {
+                    clearInterval(this.checkUpgradeStatusInterval);
+                    this.checkUpgradeStatusInterval = null;
+                }
+                if (this.elapsedInterval) {
+                    clearInterval(this.elapsedInterval);
+                    this.elapsedInterval = null;
+                }
+                // Remove beforeunload handler so user can close modal
+                if (this.beforeUnloadHandler) {
+                    window.removeEventListener('beforeunload', this.beforeUnloadHandler);
+                    this.beforeUnloadHandler = null;
+                }
+
+                this.upgradeError = true;
+                this.currentStatus = `Error: ${message}`;
+            },
+
+            closeErrorModal() {
+                this.modalOpen = false;
+                this.showProgress = false;
+                this.upgradeError = false;
+                this.currentStatus = '';
+                this.currentStep = 0;
+            },
+
             upgrade() {
                 if (this.checkUpgradeStatusInterval) return true;
                 this.currentStep = 1;
@@ -294,7 +356,7 @@ class="w-24 dark:bg-coolgray-200 dark:hover:bg-coolgray-300">Cancel
                         } else if (data.status === 'complete') {
                             this.showSuccess();
                         } else if (data.status === 'error') {
-                            this.currentStatus = `Error: ${data.message}`;
+                            this.showError(data.message);
                         }
                     } catch (error) {
                         // Service is down - switch to health check mode
diff --git a/scripts/upgrade.sh b/scripts/upgrade.sh
index 8ade89669..ad3f32009 100644
--- a/scripts/upgrade.sh
+++ b/scripts/upgrade.sh
@@ -244,7 +244,7 @@ nohup bash -c "
     else
         log 'Using standard docker-compose configuration'
         log 'Running docker compose up...'
-        docker run -v /data/coolify/source:/data/coolify/source -v /var/run/docker.sock:/var/run/docker.sock \${DOCKER_CONFIG_MOUNT} \${REGISTRY_URL:-ghcr.io}/coollabsio/coolify-helper:\${LATEST_HELPER_VERSION} bash -c \"LATEST_IMAGE=\${LATEST_IMAGE} docker compose --project-name coolify --env-file /data/coolify/source/.env -f /data/coolify/source/docker-compose.yml -f /data/coolify/source/docker-compose.prod.yml up -d --remove-orphans --wait --wait-timeout 60\" >>\"\$LOGFILE\" 2>&1
+        docker run -v /data/coolify/source:/data/coolify/source -v /var/run/docker.sock:/var/run/docker.sock \${DOCKER_CONFIG_MOUNT} --rm \${REGISTRY_URL:-ghcr.io}/coollabsio/coolify-helper:\${LATEST_HELPER_VERSION} bash -c \"LATEST_IMAGE=\${LATEST_IMAGE} docker compose --project-name coolify --env-file /data/coolify/source/.env -f /data/coolify/source/docker-compose.yml -f /data/coolify/source/docker-compose.prod.yml up -d --remove-orphans --wait --wait-timeout 60\" >>\"\$LOGFILE\" 2>&1
     fi
     log 'Docker compose up completed'