fix(seeder): keep an installed app's host port across catalog sync

The catalog-sync loop overwrote container_config and ui_location for every
non-user-modified curated service, resetting the published host port of an
install that had been moved off the catalog default. The row then disagreed
with the running container: the Open link pointed at a dead port and the
next recreate collided.

Preserve the live host ports for installed rows and apply every other
catalog change, so these installs keep receiving catalog updates instead of
being opted out of them.

Diagnosis and the original fix are @johno10661's work in #1005.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8Mgg1vd2eKSs8StZiuyMV

admin/app/utils/service_catalog_merge.ts

@@ -0,0 +1,147 @@
+/**
+ * Merging catalog config onto an already-installed curated service.
+ *
+ * `ServiceSeeder.run()` keeps curated services in sync with the catalog by
+ * overwriting `container_config` and `ui_location` on every boot. That is correct
+ * for anything not yet installed, and wrong for anything that is: an install whose
+ * published host port differs from the catalog default gets its port reset, which
+ * desyncs the row from the container that is actually running. The Open link then
+ * points at a port with nothing behind it, and the next recreate collides with
+ * whatever process took the catalog port in the meantime.
+ *
+ * A host port diverges for an ordinary reason: the default was already taken on
+ * that machine, so the user moved it. That is a property of the machine, not a
+ * preference the catalog should get a vote on.
+ *
+ * These helpers keep the running install's host ports while still applying every
+ * other catalog change, so an app on an alternate port continues to receive image,
+ * env, metadata and scheme updates. The alternative considered was flagging such
+ * rows `is_user_modified` so the sync skips them entirely, which also protects the
+ * port but permanently opts that install out of all future catalog updates. This
+ * is the narrower fix: preserve the one field the machine owns, sync the rest.
+ */
+
+/** `{ "8080/tcp": [{ "HostPort": "8090" }] }` keyed by container port. */
+type PortBindings = Record<string, Array<{ HostPort?: string }> | undefined>
+
+/**
+ * `services.container_config` is a MySQL **json** column, so the driver hands back
+ * an already-parsed object at runtime even though the model declares it
+ * `string | null`. The catalog side of the comparison is a `JSON.stringify` string.
+ * Both shapes reach these helpers, so accept either.
+ *
+ * This is not a hypothetical: a string-only version of this function silently
+ * no-opped against a real database while passing every string-fixture unit test.
+ */
+type SerializedConfig = string | Record<string, any> | null | undefined
+
+function parseConfig(value: SerializedConfig): Record<string, any> | null {
+  if (!value) return null
+  if (typeof value === 'object') return value
+  try {
+    const parsed = JSON.parse(value)
+    return parsed && typeof parsed === 'object' ? parsed : null
+  } catch {
+    // A row we cannot parse gets the catalog value verbatim, which is the
+    // pre-existing behaviour and no worse than what it has now.
+    return null
+  }
+}
+
+function readPortBindings(containerConfig: SerializedConfig): PortBindings | null {
+  const bindings = parseConfig(containerConfig)?.HostConfig?.PortBindings
+  if (!bindings || typeof bindings !== 'object') return null
+  return bindings as PortBindings
+}
+
+function firstHostPort(bindings: PortBindings | null): string | null {
+  if (!bindings) return null
+  for (const list of Object.values(bindings)) {
+    const port = Array.isArray(list) ? list[0]?.HostPort : undefined
+    if (port) return String(port)
+  }
+  return null
+}
+
+/**
+ * Catalog `container_config` with the live install's published host ports kept.
+ *
+ * Matching is per container-side port, so if the catalog changes which port the
+ * app listens on *inside* the container, that is a genuine catalog change and the
+ * catalog value wins. Only the host side of a binding the install already has is
+ * preserved.
+ */
+export function mergeContainerConfigPreservingHostPorts<T extends string | null>(
+  catalogConfig: T,
+  liveConfig: SerializedConfig
+): T {
+  if (!catalogConfig) return catalogConfig
+  const live = readPortBindings(liveConfig)
+  if (!live) return catalogConfig
+
+  const parsed = parseConfig(catalogConfig)
+  if (!parsed) return catalogConfig
+
+  const catalogBindings: PortBindings | undefined = parsed?.HostConfig?.PortBindings
+  if (!catalogBindings || typeof catalogBindings !== 'object') return catalogConfig
+
+  let changed = false
+  for (const [containerPort, catalogList] of Object.entries(catalogBindings)) {
+    const livePort = Array.isArray(live[containerPort]) ? live[containerPort]![0]?.HostPort : undefined
+    if (!livePort) continue
+
+    const catalogPort = Array.isArray(catalogList) ? catalogList[0]?.HostPort : undefined
+    if (catalogPort === undefined || String(catalogPort) === String(livePort)) continue
+
+    catalogBindings[containerPort] = [{ ...(Array.isArray(catalogList) ? catalogList[0] : {}), HostPort: String(livePort) }]
+    changed = true
+  }
+
+  return (changed ? JSON.stringify(parsed) : catalogConfig) as T
+}
+
+/**
+ * Catalog `ui_location` with the live install's port kept, when it is a port at all.
+ *
+ * `ui_location` is one of `"8090"`, `"https:8480"`, or a path like `"/chat"`. Only
+ * the first two forms carry a port, and only those are rewritten. The catalog's
+ * scheme is always taken, so a catalog change like Vaultwarden moving to `https:`
+ * still reaches an install running on an alternate port.
+ *
+ * The live port is only honoured when it matches a host port the install actually
+ * publishes. That keeps a stale or hand-edited `ui_location` from pinning the link
+ * to a port nothing is listening on.
+ */
+export function mergeUiLocationPreservingHostPort(
+  catalogUiLocation: string | null,
+  liveUiLocation: string | null | undefined,
+  liveConfig: SerializedConfig
+): string | null {
+  if (!catalogUiLocation || !liveUiLocation) return catalogUiLocation
+
+  const catalogMatch = catalogUiLocation.match(/^(?:(https?):)?(\d+)$/)
+  const liveMatch = liveUiLocation.match(/^(?:(https?):)?(\d+)$/)
+  if (!catalogMatch || !liveMatch) return catalogUiLocation
+
+  const livePort = liveMatch[2]
+  if (livePort === catalogMatch[2]) return catalogUiLocation
+
+  const bindings = readPortBindings(liveConfig)
+  const publishedPorts = new Set(
+    bindings
+      ? Object.values(bindings)
+          .flatMap((list) => (Array.isArray(list) ? list : []))
+          .map((b) => (b?.HostPort ? String(b.HostPort) : null))
+          .filter((p): p is string => p !== null)
+      : []
+  )
+  if (!publishedPorts.has(livePort)) return catalogUiLocation
+
+  const scheme = catalogMatch[1]
+  return scheme ? `${scheme}:${livePort}` : livePort
+}
+
+/** Exported for tests: the first published host port in a serialized config. */
+export function firstPublishedHostPort(containerConfig: SerializedConfig): string | null {
+  return firstHostPort(readPortBindings(containerConfig))
+}

admin/database/seeders/service_seeder.ts

@@ -4,6 +4,10 @@ import { ModelAttributes } from '@adonisjs/lucid/types/model'
 import env from '#start/env'
 import { SERVICE_NAMES } from '../../constants/service_names.js'
 import { KIWIX_LIBRARY_CMD } from '../../constants/kiwix.js'
+import {
+  mergeContainerConfigPreservingHostPorts,
+  mergeUiLocationPreservingHostPort,
+} from '../../app/utils/service_catalog_merge.js'
 
 type ServiceSeedRecord = Omit<
   ModelAttributes<Service>,
@@ -545,6 +549,10 @@ export default class ServiceSeeder extends BaseSeeder {
       'service_name',
       'is_custom',
       'is_user_modified',
+      // Needed to keep an installed app's published host port across catalog sync.
+      'installed',
+      'container_config',
+      'ui_location',
     ])
     const existingServiceMap = new Map(existingServices.map((s) => [s.service_name, s]))
 
@@ -564,12 +572,32 @@ export default class ServiceSeeder extends BaseSeeder {
     for (const service of ServiceSeeder.DEFAULT_SERVICES) {
       const existing = existingServiceMap.get(service.service_name)
       if (existing && !existing.is_custom && !existing.is_user_modified) {
+        // An installed app's published host port belongs to the machine, not the
+        // catalog: it diverges because the default was already taken on that host.
+        // Overwriting it desyncs the row from the running container, which breaks
+        // the Open link now and collides on the next recreate (#1005). Keep the
+        // live host ports and apply every other catalog change. Not-yet-installed
+        // rows take the catalog verbatim, since nothing is running to conflict.
+        const containerConfig = existing.installed
+          ? mergeContainerConfigPreservingHostPorts(
+              service.container_config,
+              existing.container_config
+            )
+          : service.container_config
+        const uiLocation = existing.installed
+          ? mergeUiLocationPreservingHostPort(
+              service.ui_location,
+              existing.ui_location,
+              existing.container_config
+            )
+          : service.ui_location
+
         await Service.query().where('service_name', service.service_name).update({
-          container_config: service.container_config,
+          container_config: containerConfig,
           container_command: service.container_command ?? null,
           metadata: (service as any).metadata ?? null,
           category: service.category,
-          ui_location: service.ui_location,
+          ui_location: uiLocation,
         })
       }
     }

admin/tests/unit/service_catalog_merge.spec.ts

@@ -0,0 +1,128 @@
+import * as assert from 'node:assert/strict'
+import { test } from 'node:test'
+
+import {
+  firstPublishedHostPort,
+  mergeContainerConfigPreservingHostPorts,
+  mergeUiLocationPreservingHostPort,
+} from '../../app/utils/service_catalog_merge.js'
+
+const config = (bindings: Record<string, string>, extra: Record<string, unknown> = {}) =>
+  JSON.stringify({
+    Image: 'ghcr.io/example/app:1.0.0',
+    ...extra,
+    HostConfig: {
+      PortBindings: Object.fromEntries(
+        Object.entries(bindings).map(([containerPort, hostPort]) => [
+          containerPort,
+          [{ HostPort: hostPort }],
+        ])
+      ),
+    },
+  })
+
+// ── container_config ────────────────────────────────────────────────────────
+
+test('keeps the live host port when it diverges from the catalog default', () => {
+  const merged = mergeContainerConfigPreservingHostPorts(
+    config({ '8080/tcp': '8090' }),
+    config({ '8080/tcp': '9999' })
+  )
+  assert.equal(firstPublishedHostPort(merged), '9999')
+})
+
+test('applies other catalog changes while keeping the port', () => {
+  const merged = mergeContainerConfigPreservingHostPorts(
+    config({ '8080/tcp': '8090' }, { Image: 'ghcr.io/example/app:2.0.0' }),
+    config({ '8080/tcp': '9999' }, { Image: 'ghcr.io/example/app:1.0.0' })
+  )
+  assert.equal(JSON.parse(merged).Image, 'ghcr.io/example/app:2.0.0')
+  assert.equal(firstPublishedHostPort(merged), '9999')
+})
+
+test('returns the catalog value untouched when the port already matches', () => {
+  const catalog = config({ '8080/tcp': '8090' })
+  assert.equal(mergeContainerConfigPreservingHostPorts(catalog, config({ '8080/tcp': '8090' })), catalog)
+})
+
+test('a catalog change to the CONTAINER-side port wins', () => {
+  // The app now listens on 3000 inside the container. The live 8080 binding is a
+  // different key, so there is nothing to preserve and the catalog is authoritative.
+  const merged = mergeContainerConfigPreservingHostPorts(
+    config({ '3000/tcp': '8090' }),
+    config({ '8080/tcp': '9999' })
+  )
+  assert.equal(firstPublishedHostPort(merged), '8090')
+})
+
+test('preserves each port independently on a multi-port app', () => {
+  const merged = mergeContainerConfigPreservingHostPorts(
+    config({ '8080/tcp': '8310', '8081/tcp': '8311' }),
+    config({ '8080/tcp': '9310', '8081/tcp': '8311' })
+  )
+  const bindings = JSON.parse(merged).HostConfig.PortBindings
+  assert.equal(bindings['8080/tcp'][0].HostPort, '9310')
+  assert.equal(bindings['8081/tcp'][0].HostPort, '8311')
+})
+
+// `services.container_config` is a MySQL json column, so the live value arrives as
+// an already-parsed OBJECT, not a string, despite the model typing it `string | null`.
+// A string-only implementation passes every other test in this file and silently
+// no-ops against a real database. Verified live before adding these.
+test('accepts an already-parsed live config object (MySQL json column)', () => {
+  const merged = mergeContainerConfigPreservingHostPorts(
+    config({ '80/tcp': '8410' }),
+    JSON.parse(config({ '80/tcp': '9410' }))
+  )
+  assert.equal(firstPublishedHostPort(merged), '9410')
+})
+
+test('ui_location honours a parsed live config object too', () => {
+  assert.equal(
+    mergeUiLocationPreservingHostPort('8410', '9410', JSON.parse(config({ '80/tcp': '9410' }))),
+    '9410'
+  )
+})
+
+test('unparseable or portless live config falls back to the catalog', () => {
+  const catalog = config({ '8080/tcp': '8090' })
+  assert.equal(mergeContainerConfigPreservingHostPorts(catalog, 'not json'), catalog)
+  assert.equal(mergeContainerConfigPreservingHostPorts(catalog, null), catalog)
+  assert.equal(mergeContainerConfigPreservingHostPorts(catalog, JSON.stringify({})), catalog)
+})
+
+// ── ui_location ─────────────────────────────────────────────────────────────
+
+test('rewrites a bare port to the live one', () => {
+  assert.equal(
+    mergeUiLocationPreservingHostPort('8090', '9999', config({ '8080/tcp': '9999' })),
+    '9999'
+  )
+})
+
+test('takes the catalog scheme and the live port', () => {
+  // Vaultwarden moved to https in the catalog; this install runs on 9480.
+  assert.equal(
+    mergeUiLocationPreservingHostPort('https:8480', '9480', config({ '80/tcp': '9480' })),
+    'https:9480'
+  )
+})
+
+test('a path ui_location is never rewritten', () => {
+  assert.equal(mergeUiLocationPreservingHostPort('/chat', '/chat', null), '/chat')
+  assert.equal(mergeUiLocationPreservingHostPort('/chat', '9999', config({ '80/tcp': '9999' })), '/chat')
+})
+
+test('ignores a live port the install does not actually publish', () => {
+  // Stale or hand-edited ui_location: pinning the link to it would point the
+  // Open button at a port with nothing behind it.
+  assert.equal(
+    mergeUiLocationPreservingHostPort('8090', '7777', config({ '8080/tcp': '8090' })),
+    '8090'
+  )
+})
+
+test('null or missing values fall back to the catalog', () => {
+  assert.equal(mergeUiLocationPreservingHostPort('8090', null, null), '8090')
+  assert.equal(mergeUiLocationPreservingHostPort(null, '9999', null), null)
+})