0021. UI: read and approval screens on the generated TS client

Status: Accepted, amended twice · Date: 2026-10-25 · Design section: CLI and UI, roadmap item 5

Decided: Built the four read/approval UI screens on a committed, generated TS client, with the approval dialog disclosing identity/hardware/role before use

Context

ADR 0017 built the UI skeleton (routing, build, preactcn wiring) with one placeholder page. ADR 0016 defined the Connect API and committed its Go client; ADR 0018 implemented the server. Nothing yet in ui/ called it, and no screen existed. This unit is "Read-only UI, approval UI" from roadmap item 5, scoped to exactly four screens the design names in "CLI and UI": cluster list and detail (status, rollout progress, hand-over state), the machine list with pending approvals and the approve/reject actions, and a node detail page (hardware, disks, network, live resource usage, Kubernetes state, effective config annotated by origin with secrets masked, logs). Config editing, the GitOps page, a merged multi-node log viewer and deep links into other tools are explicitly later phases and out of scope here.

Versions consulted (docs read, not recalled from memory):

  • buf 1.73.0, protoc-gen-go v1.36.12, protoc-gen-connect-go v1.21.0 - unchanged from ADR 0016, still pinned in flake.nix.
  • @bufbuild/protoc-gen-es and @bufbuild/protobuf 2.16.0, @connectrpc/connect and @connectrpc/connect-web 2.2.0 - current published versions at the time of this work (npm view <pkg> version), installed into ui/'s own package.json, not nix (ui/ already has its own npm world per ADR 0017; these packages are tied to ui/'s own @bufbuild/protobuf runtime version, which nixpkgs has no reason to track). connectrpc.com's "Getting started" and "Generating code" pages (current, no version selector) confirmed the createClient(Service, transport) and buf.gen.yaml shapes below.
  • shadcn CLI 4.22.1 - same version ADR 0017 already used.

Decision

TypeScript client generation, and why it's committed

buf.gen.yaml gains one plugin entry:

  - local: ./ui/node_modules/.bin/protoc-gen-es
    out: ui/src/gen
    opt:
      - target=ts
      - erasable_syntax=true

Connect-ES v2 does not need a second, separate connect-es plugin the way v1 did: protoc-gen-es v2 itself generates both the message types and a GenService const (e.g. export const ClusterService = ...) that createClient(ClusterService, transport) consumes directly. Adding @connectrpc/protoc-gen-connect-es (still published, at v1.7.0, for the older style) would have been a second plugin generating code nothing here calls - rejected on the same "does this need to exist" ground as any other dependency.

erasable_syntax=true is not cosmetic: ui/tsconfig.app.json already sets erasableSyntaxOnly (ADR 0017's scaffold), which rejects a real TypeScript enum because it compiles to runtime code a type-stripper can't erase. Protobuf-ES's default enum output is exactly that real enum; without this option, tsc -b fails on every generated enum (RolloutKind, HandoverPhase, MachineRole, DiskRole, ConfigLayer) with TS1294: This syntax is not allowed when 'erasableSyntaxOnly' is enabled, confirmed by generating once without the option first. With it, enums generate as as const objects instead, and ui/src/lib/enum.ts's enumLabel() does the reverse (value -> name) lookup for display that a real TS enum would otherwise have given for free.

The generated output (ui/src/gen/) is committed, for the same reason ADR 0016 committed gen/go/: regenerating it needs buf (from nix develop) and ui/node_modules/.bin/protoc-gen-es (from cd ui && npm ci), and nobody who is only touching a screen - the normal case from here on - should need either just to open the project. The counter-pressure ADR 0016 didn't have to weigh is that this generator writes into ui/, which make build test lint never touches (ADR 0017); committing it is still the right call, because the alternative is a ui/ checkout that doesn't type-check or build until someone runs buf generate first, silently, with no error pointing at why.

One consequence worth stating plainly: buf.gen.yaml is shared by both plugins, so make proto-generate (buf generate, unchanged as a target - only its template grew a new plugin) now fails for anyone who hasn't run cd ui && npm ci first, because the TS plugin's binary won't exist on disk. This is the same category of pre-requisite proto-generate already had (nix develop, for the Go plugins), not a new kind of trap, but it is a new instance of it for whoever next edits proto/ without having ever touched ui/. make ui-proto-generate (new, ui-install as a prerequisite) is the one-command fix; documented here rather than solved by making ui/node_modules a dependency of the untouched proto-generate target, which this phase's own instructions say not to rewrite.

ui/eslint.config.js's existing radix-only exemption (@typescript-eslint/no-explicit-any off for src/lib/radix/**, ADR 0017) widens to also cover src/components/ui/** (also CLI-generated, never hand-written) and to turn off no-empty-object-type/no-unused-vars, both of which the newly-added select/collection/input shims trip; src/gen/** is added to globalIgnores outright, since it's regenerated output with its own /* eslint-disable */ header, not code this project's style rules apply to.

Screens

Four routes, matching the design's four screens one-for-one:

  • / - ClusterList: table of clusters with Talos/Kubernetes versions, control-plane count, rollout progress (kind, phase, nodes done/total) and hand-over state per component, each a Badge.
  • /clusters/:namespace/:name - ClusterDetail: the same Cluster message's full detail (conditions, rollout, hand-over), reached from a list row; there is no separate nav entry for it, same as there's none for an individual PR page.
  • /machines - MachineList: pending and approved machines in two tables (machine.proto's own comment: pending machines are listed unconditionally, never behind a flag), with the approve/reject dialogs below.
  • /nodes/:namespace/:name - NodeDetail: hardware, disks (with the WWID/serial/by-id-path/bus-path fallback chain node.proto's own comment describes - any of the first three may be empty), network interfaces, resource usage, Kubernetes status, effective config, and a live log tail.

router.tsx (ADR 0017's ~30-line History-API router) grows exactly two functions, matchRoute (a :param matcher - split, compare, no dependency) and objectRefPath (builds a /base/namespace/name path from an ObjectRef-shaped value) - still no routing library; a route table with two dynamic patterns doesn't need one either.

Data fetching is useEffect + useState per page component, calling the generated client directly; no data-fetching library was added. ui/src/lib/connect.ts is the only place a ConnectTransport is constructed, baseUrl: window.location.origin - same-origin, because cmd/ichor-ui serves both the SPA and the Connect handlers off the same mux (ADR 0018), and "the browser talks only to the backend" (architecture.md, "Trust") rules out pointing it anywhere else.

Eight more preactcn components were added via npx shadcn@latest add @preactcn/{table,badge,dialog,input,label,select, alert,textarea} (joining the three ADR 0017 already installed: button, card, separator) - the set these four screens actually use for lists, status chips, the approve/reject dialogs and their form fields. A Tabs component was considered for NodeDetail's sections and dropped: stacking them as separate Cards shows the same data with one fewer component to install and no interaction logic to get right for a phase that's read-only everywhere except two buttons.

Stream resume

ui/src/lib/resumableStream.ts exports streamWithResume, a generator generic over one streaming call: it loops, reconnecting with the latest resume token seen whenever the call throws a ConnectError with Code.Unavailable (exactly what ADR 0018's StreamNodeLogs returns on a graceful drain), re-throwing anything else, and returning when the stream ends cleanly. NodeDetail's log tail is its only caller in this phase - one AbortController per mount, passed as the call's signal, torn down on unmount or on a namespace/name change.

This is tested without a server, as the task requires: ui/src/lib/ resumableStream.test.ts passes a fake call (a hand-written async generator/iterable, not a mock library) that yields two lines, throws CodeUnavailable, and on the fake's own second invocation asserts it received the second line's token, not the first's or an empty one - the exact behavior a backend drain needs. A second test confirms a non-Unavailable error propagates instead of looping forever; a third confirms a clean stream end doesn't reconnect. Run with node --test "src/**/*.test.ts" (Node 24.21.0's native TypeScript support, confirmed by running it - ui/tsconfig.app.json's erasableSyntaxOnly already commits this project to code Node can strip and run directly) - no test framework dependency added. ui/package.json gained one script, "test"; Makefile gained ui-test (new, appended, ui-install as its prerequisite). ui/tsconfig.app.json's types array gained "node" (alongside "vite/client") so node:test/node:assert type-check too - @types/node was already a dependency, just not wired to the app project.

The approval screen

Approving is treated as the near-irreversible decision architecture.md's own "Trust" section implies (the controller starts applying configuration the moment it returns): the dialog shows identity (UUID, serial, MAC addresses - with a visible warning when any are missing, re-checking ADR 0011's own rule client-side via isStrongIdentity()), the found address (Machine.endpoint), and hardware (fetched via GetNode, which ADR 0018 notes works even pre-approval through Talos maintenance mode - if that fetch fails, the dialog says so and still shows everything else, since a hardware-detail fetch failing is not a reason to block showing identity). Role is a required Select with no default selection; the submit button is disabled until a role is chosen and the identity is strong - client-side, before ever calling the server, using the same two conditions ApproveMachine (ADR 0018) checks server-side. If the server still refuses (a race: two people approving the same machine, or an identity that weakened between list and dialog), ui/src/lib/errors.ts's describeError turns the ConnectError's code into a short headline (CodeFailedPrecondition -> "Not allowed right now", CodeInvalidArgument -> "Missing or invalid input", etc.) shown above the server's own message - ADR 0018's refusal messages are already plain English, so the headline adds the "what kind of problem" context without replacing or parsing them. This was chosen over hand-writing a distinct message per one of the four refusal reasons: string-matching the server's message to tell "already approved" apart from "weak identity" (both CodeFailedPrecondition) would be the client depending on wording the server is free to change; the client-side pre-check already gives the two most common cases their precise reason, and the code-level headline covers whatever reaches the server anyway.

Reject's dialog states, before the button exists to click, what rejecting does and does not do: it is recorded as an AuditRecord only; the TalosMachine is neither deleted nor mutated. Checked directly against machine.proto's Machine message before writing this copy: the only status field is approved bool - there is no separate "rejected" value - so the dialog says plainly that a rejected machine can reappear in the pending list indistinguishable from one that was never looked at, if its own discovery loop runs again. This restates ADR 0018's own "Gap to report" in a form a person approving or rejecting machines actually reads, rather than leaving it as something only the ADR says.

Consequences

  • Four screens, reachable from the nav (Clusters, Machines) or from a list row (ClusterDetail, NodeDetail), all reading through the generated TS client; approve/reject both work end to end against the generated request/response shapes.
  • make build test lint passes with no UI build present (verified: rm -rf cmd/ichor-ui/dist && make build test lint, unchanged from before this unit). make ui-ci (ui-typecheck ui-typecheck ui-lint ui-build, ADR 0017's existing target, unmodified) and the new make ui-test both pass. ui/node_modules and cmd/ichor-ui/dist remain untracked (ui/ .gitignore/root .gitignore, both from ADR 0017, needed no changes).
  • react and preact/compat do not appear anywhere in ui/package-lock. json (grep -i 'react\|compat' ui/package-lock.json after every npm install/npx shadcn add in this unit - empty every time, including after installing @floating-ui/dom and lucide-preact, which shadcn add pulled in on its own for the select/dialog positioning and icons the new components need).
  • buf lint passes unchanged; proto/ was not touched. Nothing was missing from the proto for these four screens - every field the design describes for them (status, rollout, hand-over, identity, endpoint, role, hardware, disks, network, resource usage, Kubernetes status, effective config with origin and masking, log lines with a resume token) already exists on the wire types ADR 0016 defined.
  • No live end-to-end run against cmd/ichor-ui/internal/uiapi was done: both are explicitly read-only for this unit, and a verification harness for either would have meant adding files under cmd/ or internal/, which this unit was told not to touch. Confidence that the wire format matches rests on both sides generating from the same .proto files (confirmed field-by-field against the generated .ts/comparable .go during this work) and on internal/uiapi's own 18 Ginkgo specs (ADR 0018) already exercising the server side of every RPC these screens call. The first real integration check is left to whoever next runs the UI against a live backend.
  • ResourceUsage.cpuCoresUsed's server-side formula is unverified against a live node (ADR 0018's own caveat); NodeDetail displays whatever it receives without re-deriving or second-guessing it.

Alternatives considered

A separate buf.gen.ts.yaml template, invoked by its own Makefile target, instead of adding a plugin entry to the existing buf.gen.yaml. Would have kept make proto-generate working without ui/node_modules present. Not taken: the task's own instruction was to add the TS plugin to buf.gen.yaml specifically, and one generate command producing both outputs from one source of truth is simpler to reason about than two templates that could drift out of sync with each other's plugin options. The cost (documented above) is paid once per ui/ checkout, not per screen.

@connectrpc/protoc-gen-connect-es (the v1-style connect plugin) alongside protoc-gen-es. Still published (v1.7.0) for projects on the older code-generation style; unnecessary here since protoc-gen-es v2 already generates the GenService createClient needs - confirmed by reading the generated output, not assumed from a version number.

A data-fetching library (e.g. a small SWR-style hook) instead of useEffect/useState per page. Four pages, each with at most two independent fetches and no shared cache requirement across them (a cluster list and a node detail page never need to agree on the same cached Cluster object) - ponytail's "does this need to exist" answer was no, matching ADR 0017's own precedent of preferring zero dependencies over a reach for one.

A Tabs component for NodeDetail. See "Screens" above - stacked Cards instead.

Amendment (same date): approval tri-state and service-selectable logs

ADR 0018's own amendment (same date) landed on main while this unit was in flight, changing the two proto messages this unit's screens read: Machine.approved (bool) became Machine.approval_state (MachineApprovalState: pending/approved/rejected), and StreamNodeLogsRequest gained service (empty selects the kernel log, otherwise a Talos service name). ui/src/gen was regenerated against the new proto/; no buf.gen.yaml/plugin change was needed, since both are ordinary field changes the existing target=ts + erasable_syntax=true generation already handles.

MachineList now has three buckets - pending, rejected, approved - instead of two. This closes the exact gap "the approval screen" section above documents working around: a rejected machine used to have no distinct state, so this screen's own copy warned that rejecting could look like it "did nothing" the next time the list loaded. With approval_state, a rejected machine gets its own section and stops reappearing as "pending." Approve and Reject both stay available on a rejected row - re-approving is the only "un-reject" this design defines (ADR 0018's amendment: reversible by design, not a dead end for a misclick), and re-rejecting an already-rejected machine is allowed server-side and just records the newest reason. The Reject dialog's copy changed to match: it no longer claims there's no durable rejected state, since there now is one.

NodeDetail's log tail gained a source Select (kernel/kubelet/etcd/ trustd/apid/machined, the names node.proto's own comment lists) above the log view. Switching it resets the resume token and restarts streamWithResume against the new service value - a different log source is a different stream, not a resumable continuation of the old one. "kernel" is this component's own sentinel for the wire value "": @preactcn/select's own convention reserves an empty string for "nothing selected" (confirmed earlier in this unit, debugging the role picker in the approve dialog - see the git history for that fix), so a legitimate empty-string wire value needs a non-empty UI-side stand-in, mapped back to "" only when building the request.

No further proto gaps were found while making this change: both new fields were exactly what the two screens needed, with nothing extra to add and nothing missing to report.

Second amendment (same date): the decision record lives on the CR, not only in the RPC response

A rejected machine's reason was visible exactly once, in RejectMachine's own AuditRecord response - gone the next time anyone loaded the list, so revisiting a rejected machine later showed that it was rejected but never why. Closed the way approval_state itself was closed (ADR 0018's amendment): on the CR, the single source of truth every reader (this UI, the CLI, kubectl get -o yaml) agrees on, not duplicated per- caller state that only the original caller ever saw.

TalosMachineSpec.ApprovalDecision (api/v1alpha1/talosmachine_types.go, a new *TalosMachineApprovalDecision{Actor, Reason, At}) sits beside ApprovalState, not inside it: the struct asserts no approved/rejected state of its own, so ApprovalState stays the only field a reader needs to check for that. ApproveMachine and RejectMachine (internal/uiapi/machine_service.go) both set it now; IsApproved()'s own behavior is untouched. actor stays the fixed "ichor-ui" string (ADR 0018) - added now, not deferred, so no decision recorded from here on is ever missing an actor once a real per-person one exists to fill it.

Symmetric reason, by design. ApproveMachineRequest gained an optional reason field alongside RejectMachineRequest's existing one, additive on the wire (buf breaking reports nothing). Rejecting almost always benefits from a stated reason; approving usually doesn't - the dialog already discloses identity, hardware and role before the button is even clickable, so the "why" is usually self-evident. But AuditRecord.reason was never documented as reject-only ("mainly used for rejections," ADR 0016 - not exclusive to them), and giving approve a reason field costs one optional string, not a parallel code path. Asymmetry would have meant writing down a reason why only one half of a symmetric action could explain itself, which this had no good answer for.

MachineList's Rejected section shows Reason and When per row now, read straight from Machine.approval_decision. The approve dialog gained its own optional, non-blocking Reason textarea for the same symmetry.

proto/, api/v1alpha1/talosmachine_types.go and internal/uiapi/ were all in scope for this amendment specifically (widened from this unit's original boundary, which had treated them as read-only) - internal/ controller/ was not touched, and IsApproved()'s meaning did not change, both confirmed by internal/controller's own test suite passing unchanged.

Amendment 2026-11-05 (ADR 0130): the "symmetric reason" is gone. ApproveMachineRequest.reason, RejectMachineRequest.reason and MachineApprovalDecision.reason are deprecated and unused, TalosMachine.spec.approvalDecision.reason is kept in the schema but no longer written or shown, the Approve and Reject dialogs have no reason box, and the Rejected list has no Reason column. ichor machine approve|reject lost --reason. The decision still records the actor and the time.

Search the docs

Search pages and headings