From c5f0387045bc9cdf800f5f934af60c6f157be931 Mon Sep 17 00:00:00 2001
From: jamesread <contact@jread.com>
Date: Sun, 19 Jul 2026 00:03:12 +0100
Subject: [PATCH] chore: coderabbit suggestions

---
 .pre-commit-config.yaml                       |  8 ++--
 frontend/package-lock.json                    |  2 +-
 frontend/package.json                         |  2 +-
 frontend/resources/vue/ActionButton.vue       | 24 ++++++++++-
 frontend/resources/vue/Dashboard.vue          | 28 +++++++++---
 frontend/resources/vue/ExecutionButton.vue    | 10 +++--
 frontend/resources/vue/views/ArgumentForm.vue |  2 +
 .../resources/vue/views/DiagnosticsView.vue   | 15 +++++--
 frontend/resources/vue/views/LoginView.vue    | 43 +++++++++++++++++--
 service/.golangci.yml                         | 14 +++---
 service/Makefile                              |  4 +-
 .../auth/otoauth2/restapi_auth_oauth2.go      |  4 +-
 .../config/config_reloader_user_test.go       | 25 +++++------
 .../internal/filehelper/file_change_notify.go |  6 ++-
 service/internal/filehelper/file_write.go     |  7 ++-
 .../internal/installationinfo/runtimeinfo.go  | 12 +++++-
 service/scripts/find-flakey-tests-inf/main.go |  9 ++--
 17 files changed, 161 insertions(+), 54 deletions(-)

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index db0bff1..8e38c30 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -38,28 +38,28 @@ repos:
         entry: make service-codestyle
         language: system
         pass_filenames: false
-        files: ^(service/|proto/|lang/)
+        files: ^(service/|proto/|lang/|Makefile)
 
       - id: frontend-codestyle
         name: frontend-codestyle
         entry: make frontend-codestyle
         language: system
         pass_filenames: false
-        files: ^frontend/
+        files: ^(frontend/|Makefile)
 
       - id: service-unittests
         name: service-unittests
         entry: make service-unittests
         language: system
         pass_filenames: false
-        files: ^(service/|proto/|lang/)
+        files: ^(service/|proto/|lang/|Makefile)
 
       - id: service-build
         name: service-build
         entry: make service
         language: system
         pass_filenames: false
-        files: ^(service/|proto/|lang/)
+        files: ^(service/|proto/|lang/|Makefile)
 
       - id: it
         name: integration-tests
diff --git a/frontend/package-lock.json b/frontend/package-lock.json
index 96ded27..4916cf4 100644
--- a/frontend/package-lock.json
+++ b/frontend/package-lock.json
@@ -33,7 +33,7 @@
 				"stylelint-config-standard": "^40.0.0"
 			},
 			"engines": {
-				"node": ">=22.0.0"
+				"node": "^20.19.0 || >=22.12.0"
 			}
 		},
 		"node_modules/@babel/code-frame": {
diff --git a/frontend/package.json b/frontend/package.json
index 13ebbbb..e812077 100644
--- a/frontend/package.json
+++ b/frontend/package.json
@@ -41,6 +41,6 @@
 		"vue-router": "^5.1.0"
 	},
 	"engines": {
-		"node": ">=22.0.0"
+		"node": "^20.19.0 || >=22.12.0"
 	}
 }
diff --git a/frontend/resources/vue/ActionButton.vue b/frontend/resources/vue/ActionButton.vue
index 28b4c16..436556d 100644
--- a/frontend/resources/vue/ActionButton.vue
+++ b/frontend/resources/vue/ActionButton.vue
@@ -247,10 +247,16 @@ function updateFromJson (json) {
   if (json.datetimeRateLimitExpires) {
     const date = new Date(json.datetimeRateLimitExpires.replace(' ', 'T'))
     rateLimitExpires.value = date.getTime() / 1000
+    if (bindingId.value) {
+      rateLimits[bindingId.value] = rateLimitExpires.value
+    }
     updateRateLimitStatus()
   } else if (json.datetimeRateLimitExpires === '') {
     // Explicitly clear if empty string
     rateLimitExpires.value = 0
+    if (bindingId.value) {
+      rateLimits[bindingId.value] = 0
+    }
     updateRateLimitStatus()
   }
 }
@@ -359,6 +365,15 @@ async function pollExecutionUntilDone (trackingId) {
   }
 }
 
+let stopButtonResultWatch = null
+
+function stopWatchingButtonResult () {
+  if (stopButtonResultWatch) {
+    stopButtonResultWatch()
+    stopButtonResultWatch = null
+  }
+}
+
 async function startAction (actionArgs) {
   buttonClasses.value = [] // Removes old animation classes
 
@@ -376,7 +391,8 @@ async function startAction (actionArgs) {
 
   console.log('Watching buttonResults for', startActionArgs.uniqueTrackingId)
 
-  watch(
+  stopWatchingButtonResult()
+  stopButtonResultWatch = watch(
     () => buttonResults[startActionArgs.uniqueTrackingId],
     (newResult, oldResult) => {
 	  onLogEntryChanged(newResult)
@@ -397,12 +413,18 @@ async function startAction (actionArgs) {
 	  await pollExecutionUntilDone(trackingId)
     }
   } catch (err) {
+    stopWatchingButtonResult()
     console.error('Failed to start action:', err)
   }
 }
 
 function onLogEntryChanged (logEntry) {
+  if (!logEntry) {
+    return
+  }
+
   if (logEntry.executionFinished) {
+    stopWatchingButtonResult()
     onExecutionFinished(logEntry)
   } else if (logEntry.queued && !logEntry.executionStarted) {
     onExecutionQueued(logEntry)
diff --git a/frontend/resources/vue/Dashboard.vue b/frontend/resources/vue/Dashboard.vue
index 2ec8bc0..8f427cc 100644
--- a/frontend/resources/vue/Dashboard.vue
+++ b/frontend/resources/vue/Dashboard.vue
@@ -1,6 +1,6 @@
 <template>
   <section
-    v-if="!dashboard && !initError"
+    v-if="!dashboard && !initError && !loadError"
     style="text-align: center; padding: 2em;"
   >
     <HugeiconsIcon
@@ -27,6 +27,19 @@
       Please check your configuration and try again.
     </p>
   </section>
+  <section
+    v-else-if="loadError"
+    style="text-align: center; padding: 2em;"
+    class="bad"
+  >
+    <h2 style="color: var(--error);">
+      Failed to Load Dashboard
+    </h2>
+    <p>{{ loadError }}</p>
+    <p style="color: var(--fg2);">
+      Please check your configuration and try again.
+    </p>
+  </section>
   <template v-else-if="dashboard">
     <section v-if="dashboard.contents.length == 0">
       <div
@@ -135,6 +148,7 @@ const router = useRouter()
 const dashboard = ref(null)
 const loadingTime = ref(0)
 const initError = ref(null)
+const loadError = ref(null)
 let loadingTimer = null
 let checkInitInterval = null
 let dashboardRequestId = 0
@@ -194,8 +208,9 @@ async function getDashboard () {
     const pageTitle = window.initResponse?.pageTitle || 'OliveTin'
     document.title = ret.dashboard.title + ' - ' + pageTitle
 
-    // Clear any previous init error since we successfully loaded
+    // Clear any previous errors since we successfully loaded
     initError.value = null
+    loadError.value = null
 
     // Stop the loading timer once dashboard is loaded
     if (loadingTimer) {
@@ -210,9 +225,9 @@ async function getDashboard () {
       return
     }
 
-    // On error, provide a safe fallback state
     console.error('Failed to load dashboard', e)
-    dashboard.value = { title: title || 'Default', contents: [] }
+    dashboard.value = null
+    loadError.value = e.message || 'Failed to load dashboard'
     const pageTitle = window.initResponse?.pageTitle || 'OliveTin'
     document.title = 'Error - ' + pageTitle
 
@@ -221,14 +236,13 @@ async function getDashboard () {
       clearInterval(loadingTimer)
       loadingTimer = null
     }
-
-    // Set attribute even on error so tests can proceed
-    document.body.setAttribute('loaded-dashboard', title || 'error')
   }
 }
 
 function waitForInitAndLoadDashboard () {
   document.body.removeAttribute('loaded-dashboard')
+  loadError.value = null
+  dashboard.value = null
 
   if (loadingTimer) {
     clearInterval(loadingTimer)
diff --git a/frontend/resources/vue/ExecutionButton.vue b/frontend/resources/vue/ExecutionButton.vue
index 241fb27..edd7403 100644
--- a/frontend/resources/vue/ExecutionButton.vue
+++ b/frontend/resources/vue/ExecutionButton.vue
@@ -24,6 +24,7 @@ export default {
       required: true
     }
   },
+  emits: ['show'],
   data () {
     return {
       trackingId: '',
@@ -51,11 +52,11 @@ export default {
       this.isWaiting = true
     },
 
-    show () {
+    async show () {
       this.$emit('show')
 
       if (window.executionDialog) {
-        window.executionDialog.reset()
+        await window.executionDialog.reset()
         window.executionDialog.show()
         window.executionDialog.fetchExecutionResult(this.trackingId)
       }
@@ -89,7 +90,10 @@ export default {
       // For execution button, we don't need to update classes as much
       // since it's a simpler component
       if (resultCssClass) {
-        this.$el.classList.add(resultCssClass)
+        const button = this.$el.querySelector('button')
+        if (button) {
+          button.classList.add(resultCssClass)
+        }
       }
     }
   }
diff --git a/frontend/resources/vue/views/ArgumentForm.vue b/frontend/resources/vue/views/ArgumentForm.vue
index 5b104ae..0588358 100644
--- a/frontend/resources/vue/views/ArgumentForm.vue
+++ b/frontend/resources/vue/views/ArgumentForm.vue
@@ -319,11 +319,13 @@ function getArgumentValue (arg) {
 function handleJustificationInput (event) {
   justificationValue.value = event.target.value
   justificationEditedManually.value = true
+  event.target.setCustomValidity('')
 }
 
 function handleInput (arg, event) {
   const value = event.target.type === 'checkbox' ? event.target.checked : event.target.value
   argValues.value[arg.name] = value
+  event.target.setCustomValidity('')
   updateUrlWithArg(arg.name, value)
   updateJustificationFromTemplate()
 }
diff --git a/frontend/resources/vue/views/DiagnosticsView.vue b/frontend/resources/vue/views/DiagnosticsView.vue
index 02115c2..ab80c7b 100644
--- a/frontend/resources/vue/views/DiagnosticsView.vue
+++ b/frontend/resources/vue/views/DiagnosticsView.vue
@@ -117,9 +117,18 @@ async function fetchDiagnostics () {
 }
 
 async function generateServerDiagnostics () {
-  const response = await window.client.serverDiagnostics()
-  console.log('response', response)
-  serverDiagnostics.value = `\`\`\`\n${response.alert}\n\`\`\`\n`
+  loading.value = true
+
+  try {
+    const response = await window.client.serverDiagnostics()
+    console.log('response', response)
+    serverDiagnostics.value = `\`\`\`\n${response.alert}\n\`\`\`\n`
+  } catch (err) {
+    console.error('Failed to generate server diagnostics:', err)
+    serverDiagnostics.value = ''
+  } finally {
+    loading.value = false
+  }
 }
 
 async function copyServerDiagnostics () {
diff --git a/frontend/resources/vue/views/LoginView.vue b/frontend/resources/vue/views/LoginView.vue
index 938d6bb..a2d5708 100644
--- a/frontend/resources/vue/views/LoginView.vue
+++ b/frontend/resources/vue/views/LoginView.vue
@@ -24,10 +24,15 @@
             @click="loginWithOAuth(provider)"
           >
             <span
-              v-if="provider.icon"
+              v-if="providerIcon(provider)"
               class="provider-icon"
-              v-html="provider.icon"
-            />
+            >
+              <iconify-icon
+                v-if="providerIcon(provider).kind === 'iconify'"
+                :icon="providerIcon(provider).id"
+              />
+              <span v-else>{{ providerIcon(provider).text }}</span>
+            </span>
             <span class="provider-name">Login with {{ provider.title }}</span>
           </button>
         </div>
@@ -96,6 +101,38 @@ const hasOAuth = ref(false)
 const hasLocalLogin = ref(false)
 const oauthProviders = ref([])
 
+const trustedProviderIconifyIds = {
+  github: 'simple-icons:github',
+  google: 'simple-icons:google'
+}
+
+function providerIcon (provider) {
+  const raw = (provider?.icon || '').trim()
+  if (!raw) {
+    return null
+  }
+
+  const iconifyTagMatch = raw.match(/<iconify-icon\b[^>]*\bicon=["']([^"']+)["'][^>]*>/i)
+  if (iconifyTagMatch) {
+    return { kind: 'iconify', id: iconifyTagMatch[1] }
+  }
+
+  if (/^[a-z0-9-]+:[a-z0-9-]+$/i.test(raw)) {
+    return { kind: 'iconify', id: raw }
+  }
+
+  const trustedId = trustedProviderIconifyIds[provider.key]
+  if (trustedId && (raw.includes('<') || raw === provider.key)) {
+    return { kind: 'iconify', id: trustedId }
+  }
+
+  if (!raw.includes('<')) {
+    return { kind: 'text', text: raw }
+  }
+
+  return trustedId ? { kind: 'iconify', id: trustedId } : null
+}
+
 function loadLoginOptions () {
   // Use the init response data that was loaded in App.vue
   if (window.initResponse) {
diff --git a/service/.golangci.yml b/service/.golangci.yml
index d59908d..fc14403 100644
--- a/service/.golangci.yml
+++ b/service/.golangci.yml
@@ -15,11 +15,9 @@ linters:
     - staticcheck
     - unconvert
     - unused
-
-linters-settings:
-  gocyclo:
-    min-complexity: 5
-
-issues:
-  exclude-dirs:
-    - gen
+  settings:
+    gocyclo:
+      min-complexity: 5
+  exclusions:
+    paths:
+      - gen
diff --git a/service/Makefile b/service/Makefile
index 66c066b..b52a5e1 100644
--- a/service/Makefile
+++ b/service/Makefile
@@ -53,11 +53,11 @@ find-flakey-tests-inf:
 	go run ./scripts/find-flakey-tests-inf
 
 go-tools:
-	go install "github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest"
+	go install "github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2"
 
 .PHONY: unittests unittests-fast find-flakey-tests find-flakey-tests-inf
 
 go-tools-all:
 	go install "github.com/bufbuild/buf/cmd/buf"
-	go install "github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest"
+	go install "github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2"
 	go install "google.golang.org/protobuf/cmd/protoc-gen-go"
diff --git a/service/internal/auth/otoauth2/restapi_auth_oauth2.go b/service/internal/auth/otoauth2/restapi_auth_oauth2.go
index 54eccc1..5073b7e 100644
--- a/service/internal/auth/otoauth2/restapi_auth_oauth2.go
+++ b/service/internal/auth/otoauth2/restapi_auth_oauth2.go
@@ -349,13 +349,13 @@ func getUserInfo(cfg *config.Config, client *http.Client, provider *config.OAuth
 		return ret
 	}
 
+	defer func() { _ = res.Body.Close() }()
+
 	if res.StatusCode != http.StatusOK {
 		log.Errorf("Failed to get user data: %v", res.StatusCode)
 		return ret
 	}
 
-	defer func() { _ = res.Body.Close() }()
-
 	contents, err := io.ReadAll(res.Body)
 
 	if err != nil {
diff --git a/service/internal/config/config_reloader_user_test.go b/service/internal/config/config_reloader_user_test.go
index 362897b..69746b4 100644
--- a/service/internal/config/config_reloader_user_test.go
+++ b/service/internal/config/config_reloader_user_test.go
@@ -8,6 +8,7 @@ import (
 	"github.com/knadh/koanf/providers/file"
 	"github.com/knadh/koanf/v2"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestUserLoadingFromConfig(t *testing.T) {
@@ -30,13 +31,13 @@ actions:
 
 	// Create temporary file
 	tmpFile, err := os.CreateTemp("", "test_config_*.yaml")
-	assert.NoError(t, err, "Should create temporary file")
+	require.NoError(t, err, "Should create temporary file")
 	defer func() { _ = os.Remove(tmpFile.Name()) }()
 
 	// Write test config to file
 	_, err = tmpFile.WriteString(testConfig)
-	assert.NoError(t, err, "Should write test config to file")
-	assert.NoError(t, tmpFile.Close())
+	require.NoError(t, err, "Should write test config to file")
+	require.NoError(t, tmpFile.Close())
 
 	// Load config using koanf
 	k := koanf.New(".")
@@ -81,12 +82,12 @@ actions:
 `
 
 	tmpFile, err := os.CreateTemp("", "test_config_empty_*.yaml")
-	assert.NoError(t, err, "Should create temporary file")
+	require.NoError(t, err, "Should create temporary file")
 	defer func() { _ = os.Remove(tmpFile.Name()) }()
 
 	_, err = tmpFile.WriteString(testConfig)
-	assert.NoError(t, err, "Should write test config to file")
-	assert.NoError(t, tmpFile.Close())
+	require.NoError(t, err, "Should write test config to file")
+	require.NoError(t, tmpFile.Close())
 
 	k := koanf.New(".")
 	err = k.Load(file.Provider(tmpFile.Name()), yaml.Parser())
@@ -116,12 +117,12 @@ actions:
 `
 
 	tmpFile, err := os.CreateTemp("", "test_config_disabled_*.yaml")
-	assert.NoError(t, err, "Should create temporary file")
+	require.NoError(t, err, "Should create temporary file")
 	defer func() { _ = os.Remove(tmpFile.Name()) }()
 
 	_, err = tmpFile.WriteString(testConfig)
-	assert.NoError(t, err, "Should write test config to file")
-	assert.NoError(t, tmpFile.Close())
+	require.NoError(t, err, "Should write test config to file")
+	require.NoError(t, tmpFile.Close())
 
 	k := koanf.New(".")
 	err = k.Load(file.Provider(tmpFile.Name()), yaml.Parser())
@@ -147,12 +148,12 @@ actions:
 `
 
 	tmpFile, err := os.CreateTemp("", "test_config_no_auth_*.yaml")
-	assert.NoError(t, err, "Should create temporary file")
+	require.NoError(t, err, "Should create temporary file")
 	defer func() { _ = os.Remove(tmpFile.Name()) }()
 
 	_, err = tmpFile.WriteString(testConfig)
-	assert.NoError(t, err, "Should write test config to file")
-	assert.NoError(t, tmpFile.Close())
+	require.NoError(t, err, "Should write test config to file")
+	require.NoError(t, tmpFile.Close())
 
 	k := koanf.New(".")
 	err = k.Load(file.Provider(tmpFile.Name()), yaml.Parser())
diff --git a/service/internal/filehelper/file_change_notify.go b/service/internal/filehelper/file_change_notify.go
index 9374b11..a39eaa1 100644
--- a/service/internal/filehelper/file_change_notify.go
+++ b/service/internal/filehelper/file_change_notify.go
@@ -74,7 +74,11 @@ func watchPath(ctx *watchContext) {
 		return
 	}
 
-	defer func() { _ = watcher.Close() }()
+	defer func() {
+		if err := watcher.Close(); err != nil {
+			log.Errorf("Failed to close file watcher: %v", err)
+		}
+	}()
 
 	done := make(chan bool)
 
diff --git a/service/internal/filehelper/file_write.go b/service/internal/filehelper/file_write.go
index 8d894d3..40e6798 100644
--- a/service/internal/filehelper/file_write.go
+++ b/service/internal/filehelper/file_write.go
@@ -23,7 +23,12 @@ func WriteFile(filename string, out []byte) {
 			return
 		}
 
-		_ = handle.Close()
+		if err := handle.Close(); err != nil {
+			log.WithFields(log.Fields{
+				"error":    err,
+				"filename": filename,
+			}).Errorf("Failed to close %v", filename)
+		}
 	}
 
 	err := os.WriteFile(filename, out, 0600)
diff --git a/service/internal/installationinfo/runtimeinfo.go b/service/internal/installationinfo/runtimeinfo.go
index 87218a7..ea8f74e 100644
--- a/service/internal/installationinfo/runtimeinfo.go
+++ b/service/internal/installationinfo/runtimeinfo.go
@@ -8,6 +8,8 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
+
+	log "github.com/sirupsen/logrus"
 )
 
 type RuntimeInfo struct {
@@ -85,6 +87,14 @@ func getOsReleasePrettyName() string {
 		return ""
 	}
 
+	defer func() {
+		if closeErr := handle.Close(); closeErr != nil {
+			log.WithFields(log.Fields{
+				"error": closeErr,
+			}).Warn("Failed to close /etc/os-release")
+		}
+	}()
+
 	scanner := bufio.NewScanner(handle)
 	scanner.Split(bufio.ScanLines)
 
@@ -96,7 +106,5 @@ func getOsReleasePrettyName() string {
 		}
 	}
 
-	_ = handle.Close()
-
 	return "notfound"
 }
diff --git a/service/scripts/find-flakey-tests-inf/main.go b/service/scripts/find-flakey-tests-inf/main.go
index d0368c8..a890bd9 100644
--- a/service/scripts/find-flakey-tests-inf/main.go
+++ b/service/scripts/find-flakey-tests-inf/main.go
@@ -202,10 +202,13 @@ func appendFile(path, content string) error {
 	if err != nil {
 		return err
 	}
-	defer func() { _ = file.Close() }()
 
-	_, err = file.WriteString(content)
-	return err
+	_, writeErr := file.WriteString(content)
+	closeErr := file.Close()
+	if writeErr != nil {
+		return writeErr
+	}
+	return closeErr
 }
 
 func newTestRunState() *testRunState {