fix: address CodeRabbit review feedback on PR #1115

Harden entity watcher reconciliation, theme loading, and CI parallel job
waiting, and fix related UI and documentation issues raised in review.

Co-authored-by: Cursor <cursoragent@cursor.com>

.github/workflows/build-and-release.yml

@@ -103,7 +103,13 @@ jobs:
           webui_pid=$!
           make -w service-unittests &
           service_tests_pid=$!
-          wait "$webui_pid" "$service_tests_pid"
+          webui_status=0
+          service_tests_status=0
+          wait "$webui_pid" || webui_status=$?
+          wait "$service_tests_pid" || service_tests_status=$?
+          if (( webui_status != 0 || service_tests_status != 0 )); then
+            exit 1
+          fi
           make -w frontend-unittests
 
       - name: integration tests

docs/modules/ROOT/examples/reverse-proxies/etc/reverse_proxy_nginx_dns.conf

@@ -1,3 +1,5 @@
+# Requires Nginx 1.25.1+ with ngx_http_v2_module for "http2 on;".
+# On older Nginx, use "listen 443 ssl http2;" instead and omit the http2 directive.
 server {
     listen 443 ssl;
     http2 on;
@@ -23,4 +25,3 @@ server {
         proxy_send_timeout 600s;
     }
 }
-

docs/modules/ROOT/pages/config.adoc

@@ -37,7 +37,7 @@ All configuration options are covered in the solution sections
 | Option | Description | Default | Live Reloadable | Documentation
 
 | `actions` | The list of available actions. | `-` | Live Reloadable, but refreshing the web browser is recommended. | xref:action_examples/intro.adoc[Action examples]
-| `entities` | A list of "things" you can attach actions to. | `-` | Entity data files reload live; new entity definitions in config reload live. Restart if removing types or changing file paths. | xref:entities/intro.adoc[Entities]
+| `entities` | A list of "things" you can attach actions to. | `-` | Entity data files and file path changes reload live; new entity definitions in config reload live. Restart if removing or renaming entity types (stale data may remain otherwise). | xref:entities/intro.adoc[Entities]
 | `dashboards` | A grouping of actions, with optional displays, or actions generated from entities. | `-` | Live Reloadable | xref:dashboards/intro.adoc[Dashboards]
 |===
 

docs/modules/ROOT/pages/entities/intro.adoc

@@ -25,7 +25,8 @@ The chosen field is shown as the instance name in lists and dashboards. On the e
 
 * **Entity file content** — when a watched data file is updated on disk, OliveTin reloads instances and notifies the web UI. No restart is required.
 * **New entity types in `config.yaml`** — when you add a new entry under `entities:` and reload config, OliveTin starts watching that file without a full restart.
-* **Removing or renaming entity types** — restart OliveTin if you remove an entity definition or change its file path.
+* **Entity file path changes** — when you change an entity's file path in config and reload, OliveTin switches to the new file without a restart.
+* **Removing or renaming entity types** — restart OliveTin if you remove an entity definition or rename its type; otherwise stale entity data may remain in memory until restart.
 
 [NOTE]
 ====

docs/modules/ROOT/pages/reverse-proxies/nginx.adoc

@@ -5,6 +5,8 @@ include::partial$reverse-proxies/diagram.adoc[]
 
 This is an example of DNS based proxying with Nginx.
 
+The example uses `http2 on;`, which requires Nginx 1.25.1 or later with `ngx_http_v2_module`. On older Nginx versions, replace it with `listen 443 ssl http2;` and remove the separate `http2` directive.
+
 ./etc/nginx/cond.d/OliveTin.conf
 [source,nginx]
 ----
@@ -39,4 +41,3 @@ These "custom path" instructions are for when you want to use OliveTin with a cu
 ----
 
 include::partial$reverse-proxies/external-rest.adoc[]
-

frontend/main.js

@@ -119,7 +119,11 @@ async function main () {
   try {
     const i18nSettings = await initClient()
 
-    await applyThemeStyles(getStoredThemePreference())
+    try {
+      await applyThemeStyles(getStoredThemePreference())
+    } catch (err) {
+      console.warn('Failed to load theme CSS:', err)
+    }
 
     initWebsocket()
 

frontend/resources/vue/App.vue

@@ -578,11 +578,19 @@ async function applyTheme () {
     await applyThemeStyles(themePreference.value)
   } catch (err) {
     console.warn('Failed to load theme CSS:', err)
-    const themeStyle = document.getElementById('theme-style')
-    if (themeStyle) {
-      themeStyle.textContent = ''
+    localStorage.removeItem('olivetin-theme')
+    themePreference.value = ''
+    selectedTheme.value = ''
+    try {
+      await applyThemeStyles('')
+    } catch (fallbackErr) {
+      console.warn('Failed to load default theme CSS:', fallbackErr)
+      const themeStyle = document.getElementById('theme-style')
+      if (themeStyle) {
+        themeStyle.textContent = ''
+      }
+      document.body.removeAttribute('loaded-theme')
     }
-    document.body.removeAttribute('loaded-theme')
   }
 }
 

frontend/resources/vue/components/ExecutionLogsTable.vue

@@ -31,7 +31,9 @@
         :to="`/logs/${row.executionTrackingId}`"
         class="execution-id-link"
       >
-        <LogActionTitle :justification="row.justification">
+        <LogActionTitle
+          :justification="variant === 'action-history' ? row.justification : ''"
+        >
           {{ row.executionTrackingId }}
         </LogActionTitle>
       </router-link>
@@ -89,7 +91,7 @@
     <template #cell-status="{ row }">
       <span class="exit-code">
         <span
-          v-if="variant === 'standard' && row.queuePosition != null && !row.executionFinished"
+          v-if="variant === 'standard' && row.queuePosition != null && !row.executionFinished && !row.executionStarted"
           class="queue-position"
         >
           {{ t('logs.queue-position', { position: row.queuePosition }) }}

frontend/resources/vue/utils/themeLoader.js

@@ -1,4 +1,16 @@
-export async function applyThemeStyles (themePreference = '') {
+const DEFAULT_THEME_FETCH_TIMEOUT_MS = 5000
+
+async function fetchWithTimeout (url, options, timeoutMs) {
+  const controller = new AbortController()
+  const timeoutId = setTimeout(() => controller.abort(), timeoutMs)
+  try {
+    return await fetch(url, { ...options, signal: controller.signal })
+  } finally {
+    clearTimeout(timeoutId)
+  }
+}
+
+export async function applyThemeStyles (themePreference = '', timeoutMs = DEFAULT_THEME_FETCH_TIMEOUT_MS) {
   let themeStyle = document.getElementById('theme-style')
 
   if (!themeStyle) {
@@ -12,7 +24,7 @@ export async function applyThemeStyles (themePreference = '') {
     ? `/custom-webui/themes/${encodeURIComponent(themePreference)}/theme.css`
     : '/theme.css'
 
-  const response = await fetch(themeUrl, { cache: 'no-store' })
+  const response = await fetchWithTimeout(themeUrl, { cache: 'no-store' }, timeoutMs)
   if (!response.ok) {
     throw new Error(`theme fetch failed: ${response.status}`)
   }

frontend/resources/vue/views/EntityDetailsView.vue

@@ -26,7 +26,10 @@
         Back
       </button>
     </template>
-    <div v-if="!entityDetails">
+    <div v-if="entityNotFound">
+      <p>Entity not found.</p>
+    </div>
+    <div v-else-if="!entityDetails">
       <p>Loading entity details...</p>
     </div>
     <template v-else>
@@ -130,6 +133,7 @@ import ActionIconGlyph from '../components/ActionIconGlyph.vue'
 
 const router = useRouter()
 const entityDetails = ref(null)
+const entityNotFound = ref(false)
 
 const props = defineProps({
   entityType: String,
@@ -151,6 +155,10 @@ function goBack () {
   router.push({ name: 'Entities' })
 }
 
+function isEntityNotFoundError (err) {
+  return err.status === 404 || err.code === 'NotFound' || err.message?.includes('not found')
+}
+
 async function fetchEntityDetails () {
   try {
     const response = await window.client.getEntity({
@@ -159,7 +167,14 @@ async function fetchEntityDetails () {
     })
 
     entityDetails.value = response
+    entityNotFound.value = false
   } catch (err) {
+    if (isEntityNotFoundError(err)) {
+      entityDetails.value = null
+      entityNotFound.value = true
+      return
+    }
+
     console.error('Failed to fetch entity details:', err)
     window.showBigError('fetch-entity-details', 'getting entity details', err, false)
   }

frontend/resources/vue/views/ExecutionView.vue

@@ -157,7 +157,7 @@ import LogActionTitle from '../components/LogActionTitle.vue'
 import Section from 'picocrank/vue/components/Section.vue'
 import { OutputTerminal } from '../../../js/OutputTerminal.js'
 import { HugeiconsIcon } from '@hugeicons/vue'
-import { WorkoutRunIcon, Cancel02Icon, ArrowLeftIcon, DashboardSquare01Icon, Copy01Icon } from '@hugeicons/core-free-icons'
+import { WorkoutRunIcon, Cancel02Icon, ArrowLeftIcon, DashboardSquare01Icon, Copy01Icon, ComputerTerminal01Icon } from '@hugeicons/core-free-icons'
 import { useRouter } from 'vue-router'
 import { buttonResults } from '../stores/buttonResults'
 import { requestReconnectNow } from '../../../js/websocket.js'

integration-tests/runner.mjs

@@ -174,6 +174,8 @@ class OliveTinTestRunnerVm extends OliveTinTestRunnerEnv {
   }
 
   async start (cfg) {
+    this.pageGeneration += 1
+
     console.log("vagrant changing config")
     spawn('vagrant', ['ssh', '-c', '"ln -sf /etc/OliveTin/ /opt/OliveTin-configs/' + cfg + '/config.yaml"'])
     spawn('vagrant', ['ssh', '-c', '"systemctl restart OliveTin"'])

service/internal/api/api.go

@@ -41,6 +41,8 @@ type oliveTinAPI struct {
 	// We use a map for efficient membership and deletion; ordering is not required.
 	streamingClients      map[*streamingClient]struct{}
 	streamingClientsMutex sync.RWMutex
+
+	entityListenerRegistered bool
 }
 
 const maxEventStreamClients = 16
@@ -1733,10 +1735,12 @@ func entityListFields(data any, properties []config.EntityProperty) map[string]s
 		return nil
 	}
 
+	dataMap, _ := data.(map[string]any)
 	displayFieldKey := entities.DisplayNameFieldKey(data)
 	fields := make(map[string]string, len(properties))
 	for _, property := range properties {
-		if entityPropertyIsDisplayName(property.Name, displayFieldKey) {
+		actualKey := entityDataPropertyKey(dataMap, property.Name)
+		if entityFieldIsSelectedDisplayName(actualKey, displayFieldKey) {
 			continue
 		}
 		fields[property.Name] = entityPropertyValue(data, property.Name)
@@ -1745,6 +1749,25 @@ func entityListFields(data any, properties []config.EntityProperty) map[string]s
 	return fields
 }
 
+func entityDataPropertyKey(dataMap map[string]any, propertyName string) string {
+	if dataMap == nil {
+		return propertyName
+	}
+
+	if _, found := dataMap[propertyName]; found {
+		return propertyName
+	}
+
+	propertyNameLower := strings.ToLower(propertyName)
+	for key := range dataMap {
+		if strings.ToLower(key) == propertyNameLower {
+			return key
+		}
+	}
+
+	return propertyName
+}
+
 func entityPropertyValue(data any, propertyName string) string {
 	dataMap, ok := data.(map[string]any)
 	if !ok {
@@ -1810,10 +1833,6 @@ func entityFieldIsSelectedDisplayName(fieldName, displayFieldKey string) bool {
 	return displayFieldKey != "" && fieldName == displayFieldKey
 }
 
-func entityPropertyIsDisplayName(propertyName, displayFieldKey string) bool {
-	return displayFieldKey != "" && strings.EqualFold(propertyName, displayFieldKey)
-}
-
 func (api *oliveTinAPI) RestartAction(ctx ctx.Context, req *connect.Request[apiv1.RestartActionRequest]) (*connect.Response[apiv1.StartActionResponse], error) {
 	execReqLogEntry, err := api.restartActionLogEntry(req.Msg.ExecutionTrackingId)
 	if err != nil {
@@ -1871,6 +1890,11 @@ var (
 // Call this before background goroutines that may trigger RebuildActionMap.
 func RegisterExecutorListener(ex *executor.Executor) {
 	server := ensureExecutorListener(ex)
+	if server.entityListenerRegistered {
+		return
+	}
+
+	server.entityListenerRegistered = true
 	entities.AddListener(server.onEntityChanged)
 }
 

service/internal/api/api_entities_list_test.go

@@ -184,6 +184,50 @@ func TestGetEntityRetainsNonSelectedDisplayNameCasingVariant(t *testing.T) {
 	assert.Equal(t, "parked", resp.Msg.Fields["status"])
 }
 
+func TestGetEntitiesRetainsNonSelectedDisplayNameCasingVariantInListFields(t *testing.T) {
+	entities.ClearEntitiesOfType("vehicle")
+	entities.AddEntity("vehicle", "0", map[string]any{
+		"title":  "lower-title-value",
+		"Title":  "upper-title-value",
+		"status": "parked",
+	})
+	t.Cleanup(func() {
+		entities.ClearEntitiesOfType("vehicle")
+	})
+
+	cfg := config.DefaultConfig()
+	cfg.Entities = []*config.EntityFile{
+		{
+			Name: "vehicle",
+			Properties: []config.EntityProperty{
+				{Name: "title", Title: "Title"},
+				{Name: "status", Title: "Status"},
+			},
+		},
+	}
+	cfg.Sanitize()
+
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+	ts, client := getNewTestServerAndClientWithExecutor(cfg, ex)
+	defer ts.Close()
+
+	resp, err := client.GetEntities(context.Background(), connect.NewRequest(&apiv1.GetEntitiesRequest{
+		EntityType: "vehicle",
+		Page:       1,
+		PageSize:   10,
+	}))
+	require.NoError(t, err)
+
+	vehicleDef := findEntityDefinition(resp.Msg.EntityDefinitions, "vehicle")
+	require.NotNil(t, vehicleDef)
+	require.Len(t, vehicleDef.Instances, 1)
+	assert.Equal(t, "upper-title-value", vehicleDef.Instances[0].Title)
+	assert.Equal(t, "lower-title-value", vehicleDef.Instances[0].Fields["title"])
+	assert.Equal(t, "parked", vehicleDef.Instances[0].Fields["status"])
+	assert.NotContains(t, vehicleDef.Instances[0].Fields, "Title")
+}
+
 func TestGetEntityOmitsDisplayNamePropertyFromConfiguredFields(t *testing.T) {
 	entities.ClearEntitiesOfType("vehicle")
 	entities.AddEntity("vehicle", "0", map[string]any{

service/internal/entities/entities.go

@@ -19,6 +19,7 @@ var (
 	EntityChangedSender chan bool
 	listeners           []func()
 
+	reconcileMu     sync.Mutex
 	watchedMu       sync.Mutex
 	watchedBindings = map[string]entityWatchBinding{}
 )
@@ -45,6 +46,9 @@ func SetupEntityFileWatchers(cfg *config.Config) {
 // SyncEntityFileWatchers ensures each configured entity file has a watcher and reloads
 // entity data. Safe to call on config reload; obsolete watchers are stopped.
 func SyncEntityFileWatchers(cfg *config.Config) {
+	reconcileMu.Lock()
+	defer reconcileMu.Unlock()
+
 	baseDir := ResolveEntitiesBaseDir(cfg.GetDir())
 	reconcileEntityWatchers(desiredEntityWatchers(baseDir, cfg.Entities))
 }
@@ -102,6 +106,20 @@ func reconcileEntityWatchers(desired map[string]entityWatchBinding) {
 	for path, binding := range desired {
 		ensureEntityFileWatcher(path, binding)
 	}
+	clearUnreferencedEntityTypes(desired)
+}
+
+func clearUnreferencedEntityTypes(desired map[string]entityWatchBinding) {
+	referenced := make(map[string]struct{}, len(desired))
+	for _, binding := range desired {
+		referenced[binding.entityName] = struct{}{}
+	}
+
+	for entityName := range GetEntities() {
+		if _, ok := referenced[entityName]; !ok {
+			ClearEntitiesOfType(entityName)
+		}
+	}
 }
 
 func stopObsoleteEntityWatchers(desired map[string]entityWatchBinding) {
@@ -126,12 +144,22 @@ func ensureEntityFileWatcher(path string, binding entityWatchBinding) {
 		loadEntityFile(path, binding.entityName)
 		return
 	}
-	watchedBindings[path] = binding
 	watchedMu.Unlock()
 
-	filehelper.WatchFileWrite(path, makeEntityFileWatchCallback(path), filehelper.WatchMeta{
+	if err := filehelper.WatchFileWrite(path, makeEntityFileWatchCallback(path), filehelper.WatchMeta{
 		ConfigFile: binding.sourceFile,
-	})
+	}); err != nil {
+		filehelper.StopFileWatch(path)
+		watchedMu.Lock()
+		delete(watchedBindings, path)
+		watchedMu.Unlock()
+		return
+	}
+
+	watchedMu.Lock()
+	watchedBindings[path] = binding
+	watchedMu.Unlock()
+
 	loadEntityFile(path, binding.entityName)
 }
 

service/internal/entities/entities_test.go

@@ -134,8 +134,8 @@ func TestSyncEntityFileWatchers_removedPathDoesNotReload(t *testing.T) {
 	cfg.Entities = nil
 	SyncEntityFileWatchers(cfg)
 	assert.Equal(t, 0, watchedPathCountForTests())
+	assert.Empty(t, GetEntityInstancesOrdered("vehicle"))
 
-	ClearEntitiesOfType("vehicle")
 	require.NoError(t, os.WriteFile(vehiclePath, []byte("- title: car2\n"), 0o600))
 	entityFileWatchCallback(vehiclePath)
 	assert.Empty(t, GetEntityInstancesOrdered("vehicle"))

service/internal/filehelper/file_change_notify.go

@@ -70,19 +70,34 @@ func WatchDirectoryWrite(fullpath string, callback func(filename string), meta W
 	}, nil)
 }
 
-func WatchFileWrite(fullpath string, callback func(filename string), meta WatchMeta) {
+func WatchFileWrite(fullpath string, callback func(filename string), meta WatchMeta) error {
 	filename := filepath.Base(fullpath)
 	filedir := filepath.Dir(fullpath)
 	watchKey := filepath.Join(filedir, filename)
 
-	done := registerFileWatch(watchKey)
-	go watchPath(&watchContext{
+	ctx := &watchContext{
 		filedir:         filedir,
 		filename:        filename,
 		callback:        callback,
 		interestedEvent: fsnotify.Write,
 		meta:            meta,
-	}, done)
+	}
+
+	watcher, err := fsnotify.NewWatcher()
+	if err != nil {
+		reportWatcherFailure(ctx, err)
+		return err
+	}
+
+	if err := watcher.Add(filedir); err != nil {
+		reportWatcherFailure(ctx, err)
+		closeWatcher(watcher)
+		return err
+	}
+
+	done := registerFileWatch(watchKey)
+	go runWatcher(ctx, watcher, done)
+	return nil
 }
 
 // StopFileWatch stops a file write watcher started by WatchFileWrite.
@@ -127,13 +142,18 @@ func watchPath(ctx *watchContext, done <-chan struct{}) {
 		return
 	}
 
-	defer closeWatcher(watcher)
-
 	if err := watcher.Add(ctx.filedir); err != nil {
 		reportWatcherFailure(ctx, err)
+		closeWatcher(watcher)
 		return
 	}
 
+	runWatcher(ctx, watcher, done)
+}
+
+func runWatcher(ctx *watchContext, watcher *fsnotify.Watcher, done <-chan struct{}) {
+	defer closeWatcher(watcher)
+
 	for processEvent(ctx, watcher, done) {
 	}
 }

service/internal/oncalendarfile/calendar.go

@@ -37,7 +37,11 @@ func Schedule(cfg *config.Config, ex *executor.Executor) {
 				parseCalendarFile(captured, cfg, ex, filename)
 			}
 
-			go filehelper.WatchFileWrite(action.ExecOnCalendarFile, x, filehelper.WatchMeta{
+			go func(calendarFile string, callback func(string), meta filehelper.WatchMeta) {
+				if err := filehelper.WatchFileWrite(calendarFile, callback, meta); err != nil {
+					log.WithFields(log.Fields{"calendarFile": calendarFile}).Errorf("Could not watch calendar file: %v", err)
+				}
+			}(action.ExecOnCalendarFile, x, filehelper.WatchMeta{
 				ActionID:    action.ID,
 				ActionTitle: action.Title,
 				ConfigFile:  action.SourceFile,