feat: name your machines from web settings - #1214
Merged
Merged
Conversation
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 tiann#1210
There was a problem hiding this comment.
Findings
None.
Summary
Review mode: initial
No issues found in the added/modified lines. Residual risk: tests were not run in this review pass; assessment is based on the GitHub diff plus surrounding source context. The PR includes focused hub cache/route coverage and web editor coverage for the new machine rename flow.
Testing
Not run (automation)
HAPI Bot
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1210
Problem
Machines are labelled by hostname, and there is no way to give one a friendlier name.
MachineMetadataSchema(shared/src/schemas.ts:286) has declareddisplayNameall along, and the entire read path already honours it with the fallbackdisplayName → host → id.slice(0, 8)—useMachineLabels.ts:5,MachineSelector.tsx:5,WorkspaceBrowser.tsx:56,telegram/sessionView.ts:99. 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.This adds the missing write path. Reads are untouched — they already work.
What's here
PATCH /api/machines/:idtaking{ displayName }, behind the existingrequireMachineguard. An empty value removes the key so the label falls back to the hostname; the empty string is never stored. Length is capped at 64 after trimming.machineCache.renameMachinemerges that one key into the stored metadata.refreshMachinethen publishesmachine-updated, whichuseSSE.ts:275already invalidatesqueryKeys.machineson — so every connected client relabels with no new plumbing./settings/machines, listing online machines with inline rename. Placed between Voice and About so the four existing preference pages keep their order. Each row keepshost · platformvisible, so a renamed machine stays identifiable. Offline machines are not listed, becauseGET /machinesreturnsgetOnlineMachinesByNamespace; widening that would affect every other consumer, so it stayed out of scope.Every file is either new or gains an additive change — 750 insertions, 0 deletions.
Two things worth calling out
The merge reads raw stored metadata, not the cached
Machine. That cached view is narrowed byMachineMetadataSchema, which strips unknown keys and collapses tonullfor a row that fails validation. Both are reachable — the CLI'smachine-update-metadatahandler (machineHandlers.ts:57-95) acceptsmetadata: z.unknown()and writes it unvalidated. Merging against the narrowed view would have silently droppedhost/platform/workspaceRoots, or any field a newer CLI reports that this hub's schema doesn't know yet. Two regression tests cover it.The row's save is guarded by a ref, not
isPending. Disabling the focused input on submit forces a blur, so Enter reachessavea second time and fires two PATCHes; the second can lose the optimistic-version race and report a failure for a rename that actually succeeded. jsdom doesn't reproduce disable-forces-blur, so there's an explicit test driving that sequence.mergeMachineMetadata(store/machines.ts:41-46) already preserves hub-side fields when the CLI re-registers — withdisplayNameas its own test fixture — so a reconnect doesn't clobber the name. Covered by a test here too.Tests
31 new tests: 10 in
machineCache.test.ts(merge semantics, clearing, schema-invalid rows, unknown-key preservation, event publication, CLI re-registration), 9 inmachines.test.ts(route: trim, clear, over-length, 404/403, 409/500 mapping), 12 inweb/.../machines.test.tsx(rendering, editing, blur/Enter/Escape, single-submit, error state, empty state).Verified against this commit:
bun testinhub/: 662 pass, 3 skip, 0 failbun run testinweb/: 1601 pass, 0 failbun run typecheckinshared/,hub/,web/,cli/: all cleanThe web tests were checked for real coverage by mutation — removing the
trim, making Escape save, closing the editor on error, and dropping the double-submit guard each fail the corresponding test.i18n
English and Simplified Chinese strings added for the new page and its nav summary.