feat: name your machines from web settings (#1214)

Machines are labelled by hostname with no way to give them a friendlier name. `MachineMetadataSchema` has declared `displayName` all along and the whole read path already honours it (`displayName → host → id`), but nothing could ever write it: the CLI never sends the field, the hub exposed no route that sets it, and the web UI had no editor. Add the missing write path: - `PATCH /api/machines/:id` with `{ displayName }`, guarded by the existing `requireMachine`. An empty value removes the key so the label falls back to the hostname; the empty string is never stored. - `machineCache.renameMachine` merges that one key into the stored metadata and lets `refreshMachine` publish `machine-updated`, which `useSSE` already invalidates on — so every connected client relabels without new plumbing. - A `/settings/machines` page listing online machines with inline rename, placed between Voice and About so the existing preference pages keep their order. Each row keeps the hostname visible, so a renamed machine is still identifiable. The merge reads the raw stored metadata rather than the cached `Machine` view. That view is narrowed by `MachineMetadataSchema`, which strips unknown keys and yields `null` for a row that fails validation — reachable, since the CLI's `machine-update-metadata` handler accepts `z.unknown()`. Merging against it would have written those fields out of existence. The row's save is guarded by a ref rather than `isPending`: disabling the focused input forces a blur, so Enter otherwise reaches `save` twice and fires two PATCHes, the second of which can lose the version race and report a failure for a rename that succeeded. `mergeMachineMetadata` already preserves hub-side fields on CLI re-registration, so a reconnect does not clobber the name. Closes #1210
2026-08-05 06:24:37 +00:00 · 2026-07-29 10:04:33 +08:00
parent e32fe146c3
commit 8d1f84e20b
14 changed files with 750 additions and 0 deletions
@@ -0,0 +1,151 @@
+import { describe, expect, it } from 'bun:test'
+import type { SyncEvent } from '@hapi/protocol/types'
+import { Store } from '../store'
+import type { SSEManager } from '../sse/sseManager'
+import { EventPublisher } from './eventPublisher'
+import { MachineCache } from './machineCache'
+
+function createCache() {
+    const store = new Store(':memory:')
+    const broadcast: SyncEvent[] = []
+    const sseManager = { broadcast: (event: SyncEvent) => { broadcast.push(event) } } as unknown as SSEManager
+    const publisher = new EventPublisher(sseManager, () => 'ns')
+    const cache = new MachineCache(store, publisher)
+    return { store, publisher, cache, broadcast }
+}
+
+function seedMachine(store: Store, metadata: unknown) {
+    store.machines.getOrCreateMachine('machine-1', metadata, null, 'ns')
+}
+
+const BASE_METADATA = {
+    host: 'workstation.local',
+    platform: 'linux',
+    happyCliVersion: '1.0.0',
+    workspaceRoots: ['/home/user']
+}
+
+describe('MachineCache.renameMachine', () => {
+    it('sets displayName without touching CLI-reported fields', async () => {
+        const { store, cache } = createCache()
+        seedMachine(store, BASE_METADATA)
+        cache.reloadAll()
+
+        await cache.renameMachine('machine-1', 'Workstation')
+
+        expect(store.machines.getMachine('machine-1')?.metadata).toEqual({
+            ...BASE_METADATA,
+            displayName: 'Workstation'
+        })
+    })
+
+    it('replaces an existing displayName', async () => {
+        const { store, cache } = createCache()
+        seedMachine(store, { ...BASE_METADATA, displayName: 'Old' })
+        cache.reloadAll()
+
+        await cache.renameMachine('machine-1', 'New')
+
+        expect((store.machines.getMachine('machine-1')?.metadata as { displayName?: string })?.displayName).toBe('New')
+    })
+
+    it('removes the key when given an empty name rather than storing an empty string', async () => {
+        const { store, cache } = createCache()
+        seedMachine(store, { ...BASE_METADATA, displayName: 'Workstation' })
+        cache.reloadAll()
+
+        await cache.renameMachine('machine-1', '')
+
+        const metadata = store.machines.getMachine('machine-1')?.metadata as Record<string, unknown>
+        expect(metadata).toEqual(BASE_METADATA)
+        expect('displayName' in metadata).toBe(false)
+    })
+
+    it('survives a CLI re-registration afterwards', async () => {
+        const { store, cache } = createCache()
+        seedMachine(store, BASE_METADATA)
+        cache.reloadAll()
+
+        await cache.renameMachine('machine-1', 'Workstation')
+        store.machines.getOrCreateMachine('machine-1', { ...BASE_METADATA, host: 'renamed-host' }, null, 'ns')
+
+        const metadata = store.machines.getMachine('machine-1')?.metadata as Record<string, unknown>
+        expect(metadata.displayName).toBe('Workstation')
+        expect(metadata.host).toBe('renamed-host')
+    })
+
+    it('publishes machine-updated so clients refetch', async () => {
+        const { store, publisher, cache } = createCache()
+        seedMachine(store, BASE_METADATA)
+        cache.reloadAll()
+
+        const seen: string[] = []
+        publisher.subscribe((event) => {
+            if (event.type === 'machine-updated') {
+                seen.push(event.machineId)
+            }
+        })
+
+        await cache.renameMachine('machine-1', 'Workstation')
+
+        expect(seen).toContain('machine-1')
+    })
+
+    it('throws when the machine is unknown', async () => {
+        const { cache } = createCache()
+
+        await expect(cache.renameMachine('missing', 'Nope')).rejects.toThrow('Machine not found')
+    })
+
+    it('preserves fields the metadata schema does not recognise', async () => {
+        const { store, cache } = createCache()
+        seedMachine(store, { ...BASE_METADATA, futureField: 'keep me' })
+        cache.reloadAll()
+
+        await cache.renameMachine('machine-1', 'Workstation')
+
+        expect(store.machines.getMachine('machine-1')?.metadata).toEqual({
+            ...BASE_METADATA,
+            futureField: 'keep me',
+            displayName: 'Workstation'
+        })
+    })
+
+    it('does not wipe stored metadata that fails schema validation', async () => {
+        const { store, cache } = createCache()
+        // A CLI can write metadata the hub never validates (machineHandlers takes
+        // z.unknown()), so a row missing required fields is reachable in practice.
+        seedMachine(store, { host: 'workstation.local' })
+        cache.reloadAll()
+
+        await cache.renameMachine('machine-1', 'Workstation')
+
+        expect(store.machines.getMachine('machine-1')?.metadata).toEqual({
+            host: 'workstation.local',
+            displayName: 'Workstation'
+        })
+    })
+
+    it('renames a machine that has no metadata at all', async () => {
+        const { store, cache } = createCache()
+        seedMachine(store, null)
+        cache.reloadAll()
+
+        await cache.renameMachine('machine-1', 'Workstation')
+
+        expect(store.machines.getMachine('machine-1')?.metadata).toEqual({ displayName: 'Workstation' })
+    })
+
+    it('writes against the stored version even when the cache is stale', async () => {
+        const { store, cache } = createCache()
+        seedMachine(store, BASE_METADATA)
+        cache.reloadAll()
+
+        // Another writer bumps the version behind the cache's back.
+        store.machines.getOrCreateMachine('machine-1', { ...BASE_METADATA, host: 'other-writer' }, null, 'ns')
+
+        await cache.renameMachine('machine-1', 'Workstation')
+
+        expect((store.machines.getMachine('machine-1')?.metadata as { displayName?: string })?.displayName).toBe('Workstation')
+    })
+})
@@ -10,6 +10,12 @@ type MachineAlivePayload = {
    health?: unknown
 }

+const METADATA_RETRY_ATTEMPTS = 5
+
+function isPlainObject(value: unknown): value is Record<string, unknown> {
+    return typeof value === 'object' && value !== null && !Array.isArray(value)
+}
+
 function parseMachineHealth(value: unknown): Machine['health'] {
    const parsed = MachineHealthSchema.safeParse(value)
    return parsed.success ? parsed.data : null
@@ -76,6 +82,57 @@ export class MachineCache {
        return this.refreshMachine(stored.id) ?? (() => { throw new Error('Failed to load machine') })()
    }

+    /**
+     * Set or clear a machine's user-facing name.
+     *
+     * An empty `displayName` removes the key so the label falls back to the
+     * hostname; the empty string is never stored. Only that one key is touched,
+     * leaving everything the CLI reported intact.
+     *
+     * Reads the raw stored metadata rather than `this.machines`, because the
+     * cached view is narrowed by `MachineMetadataSchema`: it strips unknown keys
+     * and collapses to `null` when a row fails validation (which is reachable —
+     * the CLI's `machine-update-metadata` handler accepts `z.unknown()`). Merging
+     * against that view would write those fields out of existence.
+     *
+     * Retries on version-mismatch. Reading the version straight from the store
+     * makes contention with this process impossible (both calls are synchronous
+     * SQLite), so the retry only matters when another process writes the same
+     * database between the read and the write. `refreshMachine` publishes the
+     * `machine-updated` event that makes web clients refetch.
+     */
+    async renameMachine(machineId: string, displayName: string): Promise<void> {
+        for (let attempt = 0; attempt < METADATA_RETRY_ATTEMPTS; attempt += 1) {
+            const stored = this.store.machines.getMachine(machineId)
+            if (!stored) {
+                throw new Error('Machine not found')
+            }
+
+            const current = isPlainObject(stored.metadata) ? stored.metadata : {}
+            const { displayName: _previous, ...rest } = current
+            const newMetadata = displayName.length > 0 ? { ...rest, displayName } : rest
+
+            const result = this.store.machines.updateMachineMetadata(
+                machineId,
+                newMetadata,
+                stored.metadataVersion,
+                stored.namespace
+            )
+
+            if (result.result === 'error') {
+                throw new Error('Failed to update machine metadata')
+            }
+
+            this.refreshMachine(machineId)
+
+            if (result.result === 'success') {
+                return
+            }
+        }
+
+        throw new Error('Machine was modified concurrently. Please try again.')
+    }
+
    refreshMachine(machineId: string): Machine | null {
        const stored = this.store.machines.getMachine(machineId)
        if (!stored) {
@@ -321,6 +321,10 @@ export class SyncEngine {
        return this.machineCache.getOnlineMachinesByNamespace(namespace)
    }

+    async renameMachine(machineId: string, displayName: string): Promise<void> {
+        return this.machineCache.renameMachine(machineId, displayName)
+    }
+
    getMessagesPage(
        sessionId: string,
        options: {
@@ -314,4 +314,140 @@ describe('machines routes', () => {
            currentModelId: 'composer-2.5[fast=true]'
        })
    })
+
+    describe('PATCH /machines/:id', () => {
+        function createApp(engine: Partial<SyncEngine>) {
+            const app = new Hono<WebAppEnv>()
+            app.use('*', async (c, next) => {
+                c.set('namespace', 'default')
+                await next()
+            })
+            app.route('/api', createMachinesRoutes(() => engine as SyncEngine))
+            return app
+        }
+
+        function patch(app: Hono<WebAppEnv>, body: unknown, machineId = 'machine-1') {
+            return app.request(`/api/machines/${machineId}`, {
+                method: 'PATCH',
+                headers: { 'Content-Type': 'application/json' },
+                body: JSON.stringify(body)
+            })
+        }
+
+        it('renames a machine', async () => {
+            const machine = createMachine()
+            let captured: { id: string; displayName: string } | undefined
+            const app = createApp({
+                getMachine: () => machine,
+                renameMachine: async (id: string, displayName: string) => {
+                    captured = { id, displayName }
+                }
+            } as Partial<SyncEngine>)
+
+            const response = await patch(app, { displayName: 'Workstation' })
+
+            expect(response.status).toBe(200)
+            expect(captured).toEqual({ id: 'machine-1', displayName: 'Workstation' })
+        })
+
+        it('trims the name before storing it', async () => {
+            const machine = createMachine()
+            let captured: string | undefined
+            const app = createApp({
+                getMachine: () => machine,
+                renameMachine: async (_id: string, displayName: string) => {
+                    captured = displayName
+                }
+            } as Partial<SyncEngine>)
+
+            await patch(app, { displayName: '  Workstation  ' })
+
+            expect(captured).toBe('Workstation')
+        })
+
+        it('clears the name when given an empty string', async () => {
+            const machine = createMachine()
+            let captured: string | undefined
+            const app = createApp({
+                getMachine: () => machine,
+                renameMachine: async (_id: string, displayName: string) => {
+                    captured = displayName
+                }
+            } as Partial<SyncEngine>)
+
+            const response = await patch(app, { displayName: '   ' })
+
+            expect(response.status).toBe(200)
+            expect(captured).toBe('')
+        })
+
+        it('rejects a name longer than 64 characters', async () => {
+            const machine = createMachine()
+            let called = false
+            const app = createApp({
+                getMachine: () => machine,
+                renameMachine: async () => {
+                    called = true
+                }
+            } as Partial<SyncEngine>)
+
+            const response = await patch(app, { displayName: 'x'.repeat(65) })
+
+            expect(response.status).toBe(400)
+            expect(called).toBe(false)
+        })
+
+        it('rejects a body without displayName', async () => {
+            const machine = createMachine()
+            const app = createApp({
+                getMachine: () => machine,
+                renameMachine: async () => {}
+            } as Partial<SyncEngine>)
+
+            expect((await patch(app, {})).status).toBe(400)
+        })
+
+        it('returns 404 for an unknown machine', async () => {
+            const app = createApp({
+                getMachine: () => undefined,
+                renameMachine: async () => {}
+            } as Partial<SyncEngine>)
+
+            expect((await patch(app, { displayName: 'Nope' }, 'missing')).status).toBe(404)
+        })
+
+        it('returns 403 for a machine in another namespace', async () => {
+            const machine = createMachine({ namespace: 'other' })
+            const app = createApp({
+                getMachine: () => machine,
+                renameMachine: async () => {}
+            } as Partial<SyncEngine>)
+
+            expect((await patch(app, { displayName: 'Nope' })).status).toBe(403)
+        })
+
+        it('maps a concurrency failure to 409', async () => {
+            const machine = createMachine()
+            const app = createApp({
+                getMachine: () => machine,
+                renameMachine: async () => {
+                    throw new Error('Machine was modified concurrently. Please try again.')
+                }
+            } as Partial<SyncEngine>)
+
+            expect((await patch(app, { displayName: 'Workstation' })).status).toBe(409)
+        })
+
+        it('maps an unexpected failure to 500', async () => {
+            const machine = createMachine()
+            const app = createApp({
+                getMachine: () => machine,
+                renameMachine: async () => {
+                    throw new Error('disk on fire')
+                }
+            } as Partial<SyncEngine>)
+
+            expect((await patch(app, { displayName: 'Workstation' })).status).toBe(500)
+        })
+    })
 })
@@ -1,6 +1,8 @@
 import {
+    MACHINE_DISPLAY_NAME_MAX_LENGTH,
    MachineListDirectoryRequestSchema,
    MachinePathsExistsRequestSchema,
+    RenameMachineRequestSchema,
    SpawnSessionRequestSchema
 } from '@hapi/protocol'
 import { Hono } from 'hono'
@@ -22,6 +24,44 @@ export function createMachinesRoutes(getSyncEngine: () => SyncEngine | null): Ho
        return c.json({ machines })
    })

+    app.patch('/machines/:id', async (c) => {
+        const engine = getSyncEngine()
+        if (!engine) {
+            return c.json({ error: 'Not connected' }, 503)
+        }
+
+        const machineId = c.req.param('id')
+        const machine = requireMachine(c, engine, machineId)
+        if (machine instanceof Response) {
+            return machine
+        }
+
+        const body = await c.req.json().catch(() => null)
+        const parsed = RenameMachineRequestSchema.safeParse(body)
+        if (!parsed.success) {
+            return c.json({ error: 'Invalid body: displayName is required' }, 400)
+        }
+
+        // Trim first: a name is stored trimmed, so the ceiling applies to what
+        // actually gets stored. An empty result clears the custom name.
+        const displayName = parsed.data.displayName.trim()
+        if (displayName.length > MACHINE_DISPLAY_NAME_MAX_LENGTH) {
+            return c.json({ error: `displayName must be at most ${MACHINE_DISPLAY_NAME_MAX_LENGTH} characters` }, 400)
+        }
+
+        try {
+            await engine.renameMachine(machineId, displayName)
+            return c.json({ ok: true })
+        } catch (error) {
+            const message = error instanceof Error ? error.message : 'Failed to rename machine'
+            // Match the session rename contract: contention maps to 409.
+            if (message.includes('concurrently') || message.includes('version')) {
+                return c.json({ error: message }, 409)
+            }
+            return c.json({ error: message }, 500)
+        }
+    })
+
    app.post('/machines/:id/spawn', async (c) => {
        const engine = getSyncEngine()
        if (!engine) {