diff --git a/AGENTS.md b/AGENTS.md index 6b37a0c3..700d6097 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,3 +1,13 @@ +## How the CLI should look and sound + +`cli-ux` records the conventions every command follows: one accent colour used +only for the tool's own chrome, a stdout a program consumes carrying nothing +else, decoration only where there is a terminal to draw it on, errors that name +the fix, help as a lowercase imperative phrase, British spelling, and what a +long operation must keep saying while it works. Read it before adding a +surface, rather than copying whichever neighbour is nearest. + +- `cmd/spinloop/palette.go` — the colours and the spinner every surface draws from, in two groups that must not be swapped: the brand colours for the tool's own chrome, the state colours for what an engine is doing. (`cli-ux`) # AGENTS.md This file provides guidance to coding agents, such as Claude Code, when working with code in this repository. @@ -49,7 +59,7 @@ The binary lives under `cmd/`; domain logic is split into `internal/` packages s - `internal/pi` — Pi's `models.json` IO: deep-merge of one managed provider, preserving siblings and unknown fields. (`pi-integration`) - `internal/lucinate` — lucinate's `connections.json` IO: one managed connection, no secret ever written to disk. (`lucinate-integration`) - `cmd/spinloop/fleet.go`, `metrics_render.go`, `status_render.go`, `fleet_dashboard.go`, `dashboard_*.go` — the `fleet` command group and its Bubble Tea dashboard (the CLI's only TUI). (`fleet-client`, `fleet-config`) -- `internal/fleet` — the fleet client: the `Node` interface (`daemonNode`/`remoteNode`), concurrent fan-out, and routing/waking a node for a launch. (`fleet-client`, `fleet-config`, `fleet-routing`, `remote-node`) +- `internal/fleet` — the fleet client: the `Node` interface (`daemonNode`/`remoteNode`), concurrent fan-out (each reading stamped with the time its call returned), the `StartPhase` a start reports and the one function that renders it, and routing/waking a node for a launch. (`fleet-client`, `fleet-config`, `fleet-routing`, `remote-node`) - `examples/fleet-docker/` — a runnable multi-node fleet that doubles as the fleet integration test, run per PR by CI. (`fleet-docker-example`) - `cmd/spinloop/remote.go` + `internal/remote` — the `remote` command group and the scale-to-zero cloud GPU control plane (SigV4-signed Lambda Function URL calls — the repo's only AWS/network dependency). (`remote-environments`, `endpoint-lifecycle`, `endpoint-provisioning`, `remote-endpoint`, `remote-seed`, `weight-seeding`, `remote-keep`, `remote-start-probe`) - `internal/daemon` — the engine supervisor and the HTTP control API. `Routes()` in `api.go` is checked against `docs/openapi.yaml` by `openapi_test.go` — keep them in sync when adding an endpoint. (`daemon-api`, `daemon-api-contract`, `engine-activity`, `engine-metrics`, `api-logging`, `serve-daemon`) diff --git a/cmd/spinloop/dashboard_detail.go b/cmd/spinloop/dashboard_detail.go index 7f202994..9139ea30 100644 --- a/cmd/spinloop/dashboard_detail.go +++ b/cmd/spinloop/dashboard_detail.go @@ -1,7 +1,7 @@ // The full-screen node detail view: enter opens it on the node under the // cursor, replacing the grid with that node's unclipped metrics, its tailed // engine log, and a footer naming the keys the view answers to; escape -// closes it. The metrics section is dashNodeContentLines — the exact lines +// closes it. The metrics section is dashNodeView's lines — the exact lines // the tile draws, unclipped — so the two surfaces cannot disagree; the log // section follows fleet.LogsCall the same way `fleet logs -f` follows one // node. Everything else about the board — the grid's own refresh, and any @@ -166,12 +166,11 @@ func (m *dashModel) detailLogCapacity() int { } // detailSectionHeights splits the frame's rows between the metrics section -// (dashNodeContentLines' natural length for the node in view) and the log -// section (whatever remains after the header, footer, and the three dividers -// around the three sections), floored at one row each. +// (dashNodeView's natural length for the node in view) and the log section +// (whatever remains after the header, footer, and the three dividers around +// the three sections), floored at one row each. func (m *dashModel) detailSectionHeights() (metrics, log int) { - e := m.entries[m.cursor] - metrics = len(dashNodeContentLines(e.name, m.results[m.cursor], m.actions[m.cursor])) + metrics = len(m.detailNodeLines()) if metrics < 1 { metrics = 1 } @@ -183,6 +182,15 @@ func (m *dashModel) detailSectionHeights() (metrics, log int) { return metrics, log } +// detailNodeLines is the metrics section: the same lines the node's tile draws, +// for the node the view is open on. +func (m *dashModel) detailNodeLines() []string { + e := m.entries[m.cursor] + lines, _ := dashNodeView(e.name, m.results[m.cursor], m.actions[m.cursor], + dashNow(), dashStaleAfter(e.kind)) + return lines +} + // detailLogLines is the log section's content: the tailed lines, most recent // last, or the standing note when there is nothing to show yet. func detailLogLines(content, note string) []string { @@ -203,7 +211,7 @@ func (m dashModel) detailView() string { e := m.entries[m.cursor] metricsLines, avail := m.detailSectionHeights() - title := fmt.Sprintf("fleet dashboard %s node: %s", m.fleetPath, e.name) + detail := m.fleetPath if e.node != nil { // A standing node never polls at all, follow flag or not, so its // header says nothing about a log state that can never change. @@ -211,12 +219,12 @@ func (m dashModel) detailView() string { if !m.detailLogFollow { logState = "paused" } - title += " log: " + logState + detail += " log: " + logState } - header := dashClip(title, w) + header := dashTitleBar("fleet dashboard · node: "+e.name, detail, w) divider := strings.Repeat("─", w) - content := dashNodeContentLines(e.name, m.results[m.cursor], m.actions[m.cursor]) + content := m.detailNodeLines() for len(content) < metricsLines { content = append(content, "") } diff --git a/cmd/spinloop/dashboard_model.go b/cmd/spinloop/dashboard_model.go index 20c62348..39d000e9 100644 --- a/cmd/spinloop/dashboard_model.go +++ b/cmd/spinloop/dashboard_model.go @@ -41,12 +41,16 @@ type dashEntry struct { } // dashAction is the board's account of a start or stop in flight on one -// node: the verb, the last line the call has reported, when the action began, -// and the call's own context — the abort's door. A node with nothing in -// flight carries the zero value. +// node: the verb, the call's current phase, when the action began, and the +// call's own context — the abort's door. A node with nothing in flight carries +// the zero value. +// +// The phase is one value that each report replaces outright, so a situation +// the call has moved on from is never left on the tile; it is nil until the +// call reports one, and for a stop, whose call reports none. type dashAction struct { verb string // "start" or "stop" - line string // the call's latest status line; empty until it reports one + phase *fleet.StartPhase // what the call is doing now; nil until it reports since time.Time // when the action was issued; zero where the board has no clock on it cancel context.CancelFunc // end the wait on the call; nil where the zero value sits aborted bool // the operator ended the wait, so the line says so @@ -74,9 +78,8 @@ type dashModel struct { // is driven directly and nothing is wired to the program. send func(msg tea.Msg) - fastGen, slowGen int // rounds started in each group; each answer carries its number - fastBusy, slowBusy bool // a round in flight in that group - nextSlowAt time.Time // when the cloud group is next due; zero means due now + fastBusy, slowBusy bool // a round in flight in that group + nextReadAt []time.Time // parallel to entries; when each node is next due to be read width, height int @@ -97,12 +100,11 @@ type dashModel struct { type dashTickMsg time.Time // dashRefreshMsg is one completed round of one group. idx and results are -// parallel: the entries this round re-read, and what each answered. An -// answer whose round is no longer the newest in its group is dropped: a late -// reply must not paint over a fresher board. +// parallel: the entries this round re-read, and what each answered. Which of +// them are drawn is decided by each reading's own time, not by the round's: +// see the message's handling in Update. type dashRefreshMsg struct { remote bool // the cloud group's round - gen int idx []int results []fleet.NodeResult } @@ -116,39 +118,38 @@ func dashVerbProgress(verb string) string { return verb + "ing" } -// dashNow is the board's clock. A variable so a test can pin what an in-flight -// tile draws for elapsed time. +// dashNow returns the board's current time. A variable so a test can fix the +// elapsed time an in-flight tile renders. var dashNow = time.Now -// dashActionProgress is the tile's in-flight heading: the verb, with how long -// the action has been running once the board has a clock on it. +// dashActionProgress returns the tile's in-flight heading: the spinner (the +// tool's own, shared with `fleet deploy`), the verb, and how long the action +// has been running when a.since is set. // -// The elapsed time is the tile's heartbeat. A start's own status lines can -// legitimately stand unchanged for minutes — the attempt that finds capacity -// holds one request for the whole boot and says nothing while it does — so -// without a moving number there is no way to tell a tile that is waiting from -// one that is wedged. It is computed on every repaint rather than baked into a -// line when that line arrives, which is the difference between a heartbeat and -// another thing that can go stale; the board's tick repaints a couple of times -// a second's worth of the way there, often enough for the number to move. -func dashActionProgress(a dashAction) string { - verb := dashVerbProgress(a.verb) +// A start's phase can hold unchanged for minutes: the attempt that obtains +// capacity holds one request open for the duration of the boot and reports +// nothing while it does. Without a spinner and an elapsed time, a tile in that +// state draws identically to one whose start has stopped making progress. Both +// are computed from now on each repaint rather than stored when a phase +// arrives, so neither can itself go stale. +func dashActionProgress(a dashAction, now time.Time) string { + verb := spinnerFrame(now) + " " + dashVerbProgress(a.verb) if a.since.IsZero() { return verb } - elapsed := dashNow().Sub(a.since) + elapsed := now.Sub(a.since) if elapsed < 0 { elapsed = 0 } return verb + " " + formatDuration(int(elapsed.Seconds())) } -// dashActionProgressMsg is one intermediate line of a call behind an in- -// flight action, sent to the program by the call's own goroutine as the work -// proceeds. +// dashActionProgressMsg is one phase of a call behind an in-flight action, +// sent to the program by the call's own goroutine as the work proceeds. Each +// replaces the phase the node's action carries. type dashActionProgressMsg struct { - node string - line string + node string + phase fleet.StartPhase } // dashActionMsg is one completed start or stop. @@ -166,6 +167,31 @@ func dashTickCmd() tea.Cmd { return tea.Tick(dashboardRefreshInterval, func(time.Time) tea.Msg { return dashTickMsg{} }) } +// dashSpinInterval is how often the board repaints while an action is in +// flight. The spinner and the elapsed time beside a verb are computed when the +// tile is drawn, so they advance only as often as the board redraws, and the +// refresh tick alone is far too slow for a spinner to read as one. A variable, +// so a test never waits on it. +var dashSpinInterval = 100 * time.Millisecond + +// dashSpinTickMsg fires on that interval while something is in flight. +type dashSpinTickMsg time.Time + +func dashSpinTickCmd() tea.Cmd { + return tea.Tick(dashSpinInterval, func(time.Time) tea.Msg { return dashSpinTickMsg{} }) +} + +// actionInFlight reports whether any node has a start or stop running, which +// is what the repaint chain runs for. +func (m *dashModel) actionInFlight() bool { + for _, a := range m.actions { + if a.verb != "" { + return true + } + } + return false +} + // Init starts the rounds the board is due for — the local round plus the // cloud round, because on a cold open the cloud deadline has never been // spent — and the tick that keeps them coming. @@ -186,28 +212,43 @@ func (m *dashModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, tea.Batch(append([]tea.Cmd{dashTickCmd()}, m.startRounds()...)...) case dashRefreshMsg: if msg.remote { - if msg.gen != m.slowGen { - return m, nil - } m.slowBusy = false } else { - if msg.gen != m.fastGen { - return m, nil - } m.fastBusy = false } + // One rule for every reading: draw it only if it was taken later than + // the one already on the board for that node. Reads run concurrently + // and take differing times, so a reading can land after one taken + // later than it — including a round issued before an action finished + // and landing after, which would otherwise repaint the node's + // pre-action state over its post-action report. for i, idx := range msg.idx { - m.results[idx] = msg.results[i] + if r := msg.results[i]; r.At.After(m.results[idx].At) { + m.results[idx] = r + } + } + case dashSpinTickMsg: + // The repaint chain runs only while there is something to animate; a + // tick that finds nothing in flight stops it, and the next action + // starts it again. + if !m.actionInFlight() { + return m, nil } + return m, dashSpinTickCmd() case dashActionProgressMsg: if i := m.indexOf(msg.node); i >= 0 && m.actions[i].verb != "" { - m.actions[i].line = msg.line + phase := msg.phase + m.actions[i].phase = &phase } case dashActionMsg: aborted := false if i := m.indexOf(msg.node); i >= 0 { aborted = m.actions[i].aborted m.actions[i] = dashAction{} + // The node returns to its kind's own cadence, and is read once + // more now: what the action changed is what the operator is + // waiting to see. + m.scheduleRead(i, time.Time{}) } m.statusLine = dashActionLine(msg, aborted) case detailLogTickMsg: @@ -313,7 +354,7 @@ func (m *dashModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case "r": // A manual refresh is due for every node, cloud or local, // whatever their own deadlines say. - m.nextSlowAt = time.Time{} + m.nextReadAt = make([]time.Time, len(m.entries)) cmds := m.startRounds() if len(cmds) > 0 { cmd = tea.Batch(cmds...) @@ -328,43 +369,93 @@ func (m *dashModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } -// startRounds starts the rounds the board is due for: the local machines -// whenever the tick fires, the cloud environments when their own deadline -// has passed. A round is never started over one still in flight in the same -// group, and a group with nothing to read starts nothing. +// startRounds starts the rounds the board is due for, one per group, over +// whichever of that group's nodes have reached their own next-read time. A +// round is never started over one still in flight in the same group, and a +// group with nothing due starts nothing. func (m *dashModel) startRounds() []tea.Cmd { var cmds []tea.Cmd - if cmd := m.refreshRemoteGroup(false); cmd != nil { - cmds = append(cmds, cmd) - } - if !time.Now().Before(m.nextSlowAt) { - if cmd := m.refreshRemoteGroup(true); cmd != nil { + for _, remote := range []bool{false, true} { + if cmd := m.refreshRemoteGroup(remote); cmd != nil { cmds = append(cmds, cmd) } } return cmds } -// refreshRemoteGroup starts one round over one group of live nodes — the -// local daemon machines (remote false) or the cloud environments (remote -// true) — and returns the round's command, or nil when the group is empty or -// already has a round in flight. Starting the cloud round spends its -// deadline: it moves to one interval away, so the next due round is a -// whole interval later. +// dashNodeInterval is how often one node is read: the short interval while an +// action is in flight on it, and its kind's own cadence otherwise. The reasons +// a cloud environment is read once a minute — each read is a signed call +// through the control plane, and an idle environment's state changes on the +// scale of minutes — hold for an environment nobody is touching and hold for +// neither one the operator has just started. +func dashNodeInterval(kind string, a dashAction) time.Duration { + if a.verb == "" && kind == fleet.KindRemote { + return dashboardRemoteRefreshInterval + } + return dashboardRefreshInterval +} + +// isDue reports whether node i is to be read this tick. A node whose cadence +// is the board's own tick interval is due on every tick — the tick is its +// cadence, and comparing a deadline against it would skip a round to the +// tick's own jitter and halve the rate. Only a node read less often than the +// tick carries a deadline that can defer it, which is a cloud environment with +// nothing in flight on it. +func (m *dashModel) isDue(i int, now time.Time) bool { + if dashNodeInterval(m.entries[i].kind, m.actions[i]) <= dashboardRefreshInterval { + return true + } + return !now.Before(m.dueAt(i)) +} + +// dueAt is when node i is next due to be read, and scheduleRead records the +// next one. The times are per node rather than per group because a node with +// an action in flight is read on the short interval whatever its kind, while +// the rest of its group keeps its own cadence. The slice grows to fit rather +// than being required up front, so a model built without it reads every node +// on its first round. +func (m *dashModel) dueAt(i int) time.Time { + if i < len(m.nextReadAt) { + return m.nextReadAt[i] + } + return time.Time{} +} + +func (m *dashModel) scheduleRead(i int, at time.Time) { + for len(m.nextReadAt) <= i { + m.nextReadAt = append(m.nextReadAt, time.Time{}) + } + m.nextReadAt[i] = at +} + +// refreshRemoteGroup starts one round over the due nodes of one group of live +// nodes — the local daemon machines (remote false) or the cloud environments +// (remote true) — and returns the round's command, or nil when nothing in the +// group is due or a round is already in flight there. Starting the round +// spends each read node's deadline: each moves to one of its own intervals +// away, so a node the operator is acting on comes round again on the short +// interval while its neighbours keep their own cadence. // -// The round carries a context with a deadline of its group's interval, so a -// node slower than the cadence shows an outcome this round and gets its turn -// again next. Each node answers independently — the fan-out calls them -// concurrently — so one slow node delays no other, and a slow cloud round -// stretches only its own group. +// The round carries a context with a deadline of its group's kind interval, +// not of the cadence it was started on: a cloud read is a signed call through +// the control plane and takes what it takes, so shortening the cadence during +// an action must not shorten what the call is given to answer in. Each node +// answers independently — the fan-out calls them concurrently — so one slow +// node delays no other, and a slow cloud round stretches only its own group. func (m *dashModel) refreshRemoteGroup(remote bool) tea.Cmd { + now := time.Now() idx := make([]int, 0, len(m.entries)) nodes := make([]fleet.Node, 0, len(m.entries)) for i, e := range m.entries { - if e.node != nil && (e.kind == fleet.KindRemote) == remote { - idx = append(idx, i) - nodes = append(nodes, e.node) + if e.node == nil || (e.kind == fleet.KindRemote) != remote { + continue } + if !m.isDue(i, now) { + continue + } + idx = append(idx, i) + nodes = append(nodes, e.node) } if len(nodes) == 0 { return nil @@ -374,31 +465,23 @@ func (m *dashModel) refreshRemoteGroup(remote bool) tea.Cmd { return nil } m.slowBusy = true - m.slowGen++ - m.nextSlowAt = time.Now().Add(dashboardRemoteRefreshInterval) } else { if m.fastBusy { return nil } m.fastBusy = true - m.fastGen++ } - gen := m.generationFor(remote) + for _, i := range idx { + m.scheduleRead(i, now.Add(dashNodeInterval(m.entries[i].kind, m.actions[i]))) + } interval := m.intervalFor(remote) return func() tea.Msg { ctx, cancel := context.WithTimeout(context.Background(), interval) defer cancel() - return dashRefreshMsg{remote: remote, gen: gen, idx: idx, results: fleet.FanOutNodes(ctx, fleet.MetricsCall, nodes)} + return dashRefreshMsg{remote: remote, idx: idx, results: fleet.FanOutNodes(ctx, fleet.MetricsCall, nodes)} } } -func (m *dashModel) generationFor(remote bool) int { - if remote { - return m.slowGen - } - return m.fastGen -} - func (m *dashModel) intervalFor(remote bool) time.Duration { if remote { return dashboardRemoteRefreshInterval @@ -451,13 +534,19 @@ func (m *dashModel) beginAction(verb string) tea.Cmd { m.statusLine = e.name + ": still " + dashVerbProgress(m.actions[m.cursor].verb) return nil } + // The repaint chain runs for as long as anything is in flight, so it is + // started only when this action is the first. + spin := !m.actionInFlight() ctx, cancel := context.WithCancel(context.Background()) m.actions[m.cursor] = dashAction{verb: verb, since: dashNow(), cancel: cancel} + // The node is read on the short interval for the duration of the action, + // starting now rather than at its kind's next due time. + m.scheduleRead(m.cursor, time.Time{}) m.statusLine = dashVerbProgress(verb) + " " + e.name + "…" send := m.send - progress := func(line string) { + report := func(p fleet.StartPhase) { if send != nil { - send(dashActionProgressMsg{node: e.name, line: line}) + send(dashActionProgressMsg{node: e.name, phase: p}) } } starter, isStarter := e.node.(fleet.ProgressStarter) @@ -467,16 +556,20 @@ func (m *dashModel) beginAction(verb string) tea.Cmd { act = e.node.Stop case isStarter: act = func(ctx context.Context) (daemon.StatusResponse, error) { - return starter.StartWithProgress(ctx, progress) + return starter.StartWithProgress(ctx, report) } default: act = e.node.Start } - return func() tea.Msg { + run := func() tea.Msg { status, err := act(ctx) cancel() return dashActionMsg{node: e.name, verb: verb, status: status, err: err} } + if spin { + return tea.Batch(run, dashSpinTickCmd()) + } + return run } // abortAction ends the wait on the selected node's in-flight start. Only a @@ -561,9 +654,11 @@ func (m dashModel) View() string { return m.detailView() } w, h := m.effWidth(), m.effHeight() + now := dashNow() tiles := make([]string, len(m.entries)) for i := range m.entries { - tiles[i] = dashTile(m.entries[i].name, m.results[i], i == m.cursor, m.actions[i]) + tiles[i] = dashTile(m.entries[i].name, m.results[i], i == m.cursor, m.actions[i], + now, dashStaleAfter(m.entries[i].kind)) } rows := dashGridRows(tiles, dashCols(w)) lo := m.scrollRow @@ -587,7 +682,8 @@ func (m dashModel) headerLine(w int) string { if len(m.entries) == 1 { word = "node" } - return dashClip(fmt.Sprintf("fleet dashboard %s (%d %s)", m.fleetPath, len(m.entries), word), w) + return dashTitleBar("fleet dashboard", + fmt.Sprintf("%s (%d %s)", m.fleetPath, len(m.entries), word), w) } // dashGridKeys is the grid's own key help; the detail view's footer shares @@ -598,10 +694,15 @@ const dashGridKeys = "↑↓←→ move s start a abort x stop r refresh // stop confirmation prompt while one is pending, with the status line and a // "refreshing" marker appended — shared by the grid and the detail view so // the two cannot word the confirmation or a status outcome differently. +// +// Only the key help is drawn as key-and-meaning pairs. The prompt's own +// question, the status line and the refreshing marker are prose, not keys, and +// splitting them on a space would draw their first word as though it were one. func (m dashModel) footerLine(w int, keys string) string { - line := keys + line := dashKeyHints(keys) if m.confirm && len(m.entries) > 0 { - line = "stop " + m.entries[m.cursor].name + "? y yes n no" + line = "stop " + m.entries[m.cursor].name + "?" + dashHintGap + + dashKeyHints("y yes"+dashHintGap+"n no") } if m.statusLine != "" { line += " " + m.statusLine diff --git a/cmd/spinloop/dashboard_render.go b/cmd/spinloop/dashboard_render.go index 2ed13951..027cfc5e 100644 --- a/cmd/spinloop/dashboard_render.go +++ b/cmd/spinloop/dashboard_render.go @@ -15,6 +15,7 @@ import ( "fmt" "io" "strings" + "time" "github.com/charmbracelet/lipgloss" ansi "github.com/charmbracelet/x/ansi" @@ -90,6 +91,10 @@ func dashClip(line string, width int) string { return ansi.CutWc(line, 0, width) } +// dashHintGap separates one key-help entry from the next, and is what both +// dashFooterHints and dashKeyHints split a hint line on. +const dashHintGap = " " + // dashFooterHints drops the "a abort" entry from a key-help line when // nothing is currently abortable, so the footer never advertises a key that // would do nothing for the node it describes. @@ -97,14 +102,35 @@ func dashFooterHints(hints string, abortable bool) string { if abortable { return hints } - parts := strings.Split(hints, " ") + parts := strings.Split(hints, dashHintGap) kept := make([]string, 0, len(parts)) for _, p := range parts { if p != "a abort" { kept = append(kept, p) } } - return strings.Join(kept, " ") + return strings.Join(kept, dashHintGap) +} + +// dashKeyHints draws a key-help line: each entry is a key and what that key +// does, and entries are three spaces apart (dashHintGap). The key keeps the +// terminal's own text colour and what it does is drawn a step back in the +// muted ink, so the keys themselves are what a glance over the footer picks +// out. An entry that is one word — no key and its meaning — is left alone. +func dashKeyHints(hints string) string { + if hints == "" { + return "" + } + dim := lipgloss.NewStyle().Foreground(lipgloss.Color(brandInkDim)) + parts := strings.Split(hints, dashHintGap) + for i, part := range parts { + key, does, ok := strings.Cut(part, " ") + if !ok { + continue + } + parts[i] = key + " " + dim.Render(does) + } + return strings.Join(parts, dashHintGap) } // dashHealthTier is a panel's health at a glance, distinct from the state @@ -120,44 +146,8 @@ const ( dashUnknown ) -// dashHealthTierFor derives a panel's health tier from its node result and -// any action in flight on it. Priority order matches dashNodeContentLines' -// own shape switch, so the tier and the shape it is rendered into never -// disagree: an action in flight is always attention, regardless of the last -// completed refresh; then no refresh yet is unknown — there is no status to -// read, so the tile shows a grey "?"; then a crashed engine or a failed -// outcome is unhealthy; then a running engine the daemon has explicitly -// reported not ready is attention — the case this tier exists for, a cloud -// node whose process is up but still loading weights; then an answer that -// carries no state at all is unknown; then a node that answered with nothing -// serving is not serving, a faded dot rather than the green of a node that -// is up and serving — idle, the daemon with nothing started; stopped, a -// daemon engine that was stopped; undeployed, a remote environment with no -// instance at all; anything else, including a running engine the daemon -// reports no readiness for at all (an older daemon, or a runner with no -// known health check), is healthy, so this degrades to the pre-readiness -// behaviour rather than showing a tier the daemon cannot actually back. -func dashHealthTierFor(r fleet.NodeResult, a dashAction) dashHealthTier { - switch { - case a.verb != "": - return dashAttention - case r.Outcome == "": - return dashUnknown - case !r.OK() || r.Metrics.State == "crashed": - return dashUnhealthy - case r.Metrics.State == "running" && r.Metrics.Ready == "not-ready": - return dashAttention - case r.Metrics.State == "": - return dashUnknown - case r.Metrics.State == "idle" || r.Metrics.State == "stopped" || r.Metrics.State == "undeployed": - return dashNotServing - default: - return dashHealthy - } -} - -// dashHealthGlyph is the coloured status dot dashTileContent prepends to a -// panel's name line, in the same raw-ANSI style renderBar already uses for +// dashHealthGlyph is the coloured status mark the tile's header bar carries +// beside a node's name, in the same raw-ANSI style renderBar already uses for // the resource bars inside the tile — the tile body is one plain string // wrapped in a single lipgloss style at the border, so per-character colour // here has to be ANSI, not lipgloss.Color. Not serving shares unknown's @@ -165,69 +155,230 @@ func dashHealthTierFor(r fleet.NodeResult, a dashAction) dashHealthTier { // a filled dot for a known undeployed node against the ? for one that has // not answered yet. func dashHealthGlyph(tier dashHealthTier) string { + return dashHealthColour(tier) + dashHealthMark(tier) + ansiReset +} + +// dashHealthColour and dashHealthMark are the glyph's two halves, kept apart +// because the header bar draws them either side of its own foreground colour +// and cannot use a mark that has already reset the background behind it. +func dashHealthColour(tier dashHealthTier) string { switch tier { case dashHealthy: - return "\033[92m●\033[0m" + return ansiGreen case dashAttention: - return "\033[33m●\033[0m" - case dashNotServing: - return "\033[90m●\033[0m" - case dashUnknown: - return "\033[90m?\033[0m" + return ansiYellow + case dashNotServing, dashUnknown: + return ansiGrey default: - return "\033[31m●\033[0m" + return ansiRed + } +} + +func dashHealthMark(tier dashHealthTier) string { + if tier == dashUnknown { + return "?" } + return "●" } -// dashNodeContentLines is the facts the bar format prints for one node, as -// plain lines with no width clipping or height padding — the tile and the -// detail view's metrics section both draw from this, so the two can never -// word a number differently. The shapes are the state of the node's business -// — a start or stop in flight (the call's own status lines beside the node's -// last report: the call says what the operator asked for, the report says -// what the node is doing — a boot half done already carries a state and -// whatever it measures, and that is worth seeing while the call works), not +// The tile header bar's own codes. The bar is one dark background across the +// tile's full width with light text on it, so a node's name reads as the +// panel's title rather than as another line of its body; the health glyph +// keeps its own colour on top of it, which is what the tier is read from. The +// surface it sits on is the board's own (see palette.go). +const ( + dashHeaderBG = "\033[48;5;" + barSurface + "m" + dashHeaderFG = "\033[97m" + dashHeaderGlyphW = 2 // the glyph and the space after it +) + +// dashBarStyle is text on a bar's surface, in one of the inks above. +func dashBarStyle(fg string) lipgloss.Style { + return lipgloss.NewStyle(). + Background(lipgloss.Color(barSurface)). + Foreground(lipgloss.Color(fg)) +} + +// dashTitleBar is the board's own top line, on the same lifted surface a +// tile's header bar sits on: the product and the screen on the left, and what +// is on that screen — the fleet file, the node count, the log's state — on the +// right. The grid and the detail view both draw their header from this, so the +// two screens cannot title themselves differently. +// +// A terminal too narrow for both keeps the left half and drops the right, +// rather than wrapping the bar onto a second row. +func dashTitleBar(screen, detail string, w int) string { + if w < 1 { + return "" + } + // At least this much between the two halves, so they read as two groups + // rather than as one run of text that happens to have a space in it. + const minGap = 3 + const brand = " spinloop" + left := brand + " " + screen + right := detail + " " + pad := w - lipgloss.Width(left) - lipgloss.Width(right) + if pad < minGap { + right, pad = "", w-lipgloss.Width(left) + } + bar := dashBarStyle(brandAccent).Bold(true).Render(brand) + if pad < 0 { + return dashClip(bar+dashBarStyle(brandInk).Render(" "+screen), w) + } + bar += dashBarStyle(brandInk).Render(" " + screen + strings.Repeat(" ", pad)) + if right != "" { + bar += dashBarStyle(brandInkDim).Render(right) + } + return bar +} + +// dashTileHeader draws a tile's first line as that bar: the health glyph, the +// line's text, and background to the tile's full width. The text is clipped +// before the padding is measured, so the bar is exactly dashTileW columns +// whatever it carries. +func dashTileHeader(text string, tier dashHealthTier) string { + body := dashClip(text, dashTileW-dashHeaderGlyphW) + pad := dashTileW - dashHeaderGlyphW - lipgloss.Width(body) + if pad < 0 { + pad = 0 + } + return dashHeaderBG + dashHealthColour(tier) + dashHealthMark(tier) + + dashHeaderFG + " " + body + strings.Repeat(" ", pad) + ansiReset +} + +// dashStaleThreshold is how many of a node's own intervals its newest reading +// may age before a panel draws it as out of date rather than as the node's +// current state. Three rather than one, so an ordinary late round does not +// flicker the board grey. +const dashStaleThreshold = 3 + +// dashStaleAfter is how old a reading of a node of this kind may be before the +// panel says so. +func dashStaleAfter(kind string) time.Duration { + if kind == fleet.KindRemote { + return dashStaleThreshold * dashboardRemoteRefreshInterval + } + return dashStaleThreshold * dashboardRefreshInterval +} + +// dashNodeView is everything a panel says about one node: the lines the bar +// format prints for it, with no width clipping or height padding, and the +// health tier its glyph is coloured from. The tile and the detail view's +// metrics section both draw from this, so the two can never word a number +// differently, and the lines and the tier are produced together, so the colour +// and the text it sits beside cannot disagree about what the node is doing. +// +// Every input is an argument — the reading, the action in flight, the time the +// panel is drawn at, and how old a reading of this node may be before it is +// out of date. No clock is read here, so the same arguments always produce the +// same panel, and every combination of an action's phase against a reading can +// be enumerated in a test. +// +// The shapes are the state of the node's business: a start or stop in flight +// (the action's own account beside the node's last report — the action says +// what the operator asked for, the report says what the node is doing, and a +// boot half done already carries a state and whatever it measures), not // answered yet (an empty panel naming the node), answered and working (the -// full bar block), or answered and not working (outcome plus reason, which -// is what every one-shot surface shows). -func dashNodeContentLines(name string, r fleet.NodeResult, a dashAction) []string { +// full bar block), or answered and not working (outcome plus reason, which is +// what every one-shot surface shows). A reading older than staleAfter carries +// its age wherever it is drawn, and takes the panel to the unknown tier: a +// stale reading is not a wrong reading, but drawing it identically to a +// current one is. +func dashNodeView(name string, r fleet.NodeResult, a dashAction, now time.Time, staleAfter time.Duration) ([]string, dashHealthTier) { + age := dashReadingAge(r, now, staleAfter) var b strings.Builder switch { case a.verb != "": - fmt.Fprintf(&b, "%s %s\n", name, dashActionProgress(a)) - if a.line != "" { - fmt.Fprintln(&b, a.line) + fmt.Fprintf(&b, "%s %s\n", name, dashActionProgress(a, now)) + if a.phase != nil { + fmt.Fprintln(&b, fleet.RenderPhase(*a.phase, now)) } if r.OK() { if s := r.Metrics.State; s != "" { - fmt.Fprintln(&b, dashStateLine(s, r.Metrics)) + fmt.Fprintln(&b, dashStateLine(s, r.Metrics)+age) } dashTileReportBody(&b, r.Metrics, true) } case r.Outcome == "": fmt.Fprintf(&b, "%s\nwaiting for first refresh…\n", name) case !r.OK(): - fmt.Fprintf(&b, "%s %s\n", name, r.Outcome) + fmt.Fprintf(&b, "%s %s%s\n", name, r.Outcome, age) if d := r.Detail(); d != "" { fmt.Fprintln(&b, d) } default: - fmt.Fprintf(&b, "%s %s\n", name, dashStateLine(r.Metrics.State, r.Metrics)) + fmt.Fprintf(&b, "%s %s%s\n", name, dashStateLine(r.Metrics.State, r.Metrics), age) dashTileReportBody(&b, r.Metrics, r.Metrics.State == "running") } lines := strings.Split(b.String(), "\n") - return lines[:len(lines)-1] // the trailing newline splits an extra empty piece + lines = lines[:len(lines)-1] // the trailing newline splits an extra empty piece + return lines, dashHealthTierFor(r, a, age != "") +} + +// dashReadingAge is the "· 3m ago" a panel carries once its newest reading has +// aged past staleAfter, or "" while the reading is current. A reading with no +// time on it — one built outside the fan-out — is never called stale, since +// there is nothing to measure its age against. +func dashReadingAge(r fleet.NodeResult, now time.Time, staleAfter time.Duration) string { + if r.At.IsZero() || staleAfter <= 0 { + return "" + } + age := now.Sub(r.At) + if age < staleAfter { + return "" + } + return " · " + formatDuration(int(age.Seconds())) + " ago" +} + +// dashHealthTierFor derives a panel's health tier. Priority order matches +// dashNodeView's own shape switch, so the tier and the shape it is rendered +// into never disagree: an action in flight is always attention, regardless of +// the last completed refresh; then no refresh yet is unknown — there is no +// status to read, so the panel shows a grey "?"; then a reading that has aged +// past its cadence is unknown too, since it no longer describes the node now; +// then a crashed engine or a failed outcome is unhealthy; then a running +// engine the daemon has explicitly reported not ready is attention — the case +// this tier exists for, a cloud node whose process is up but still loading +// weights; then an answer that carries no state at all is unknown; then a node +// that answered with nothing serving is not serving, a faded dot rather than +// the green of a node that is up and serving — idle, the daemon with nothing +// started; stopped, a daemon engine that was stopped; undeployed, a remote +// environment with no instance at all; anything else, including a running +// engine the daemon reports no readiness for at all (an older daemon, or a +// runner with no known health check), is healthy, so this degrades to the +// pre-readiness behaviour rather than showing a tier the daemon cannot +// actually back. +func dashHealthTierFor(r fleet.NodeResult, a dashAction, stale bool) dashHealthTier { + switch { + case a.verb != "": + return dashAttention + case r.Outcome == "": + return dashUnknown + case stale: + return dashUnknown + case !r.OK() || r.Metrics.State == "crashed": + return dashUnhealthy + case r.Metrics.State == "running" && r.Metrics.Ready == "not-ready": + return dashAttention + case r.Metrics.State == "": + return dashUnknown + case r.Metrics.State == "idle" || r.Metrics.State == "stopped" || r.Metrics.State == "undeployed": + return dashNotServing + default: + return dashHealthy + } } -// dashTileContent is one tile's inside: dashNodeContentLines padded to the -// tile's fixed height and clipped to its fixed width, with the health glyph -// prepended to the name line — tile-only, not part of dashNodeContentLines, -// so the detail view (which draws the same lines full-screen) is unaffected. -func dashTileContent(name string, r fleet.NodeResult, a dashAction) string { - lines := dashNodeContentLines(name, r, a) - if len(lines) > 0 { - lines[0] = dashHealthGlyph(dashHealthTierFor(r, a)) + " " + lines[0] +// dashTileContent is one tile's inside: dashNodeView's lines padded to the +// tile's fixed height and clipped to its fixed width, with the first line +// drawn as the header bar — tile-only, not part of dashNodeView, so the detail +// view (which draws the same lines full-screen) keeps a plain first line. +func dashTileContent(name string, r fleet.NodeResult, a dashAction, now time.Time, staleAfter time.Duration) string { + lines, tier := dashNodeView(name, r, a, now, staleAfter) + if len(lines) == 0 { + lines = []string{""} } + lines[0] = dashTileHeader(lines[0], tier) for len(lines) < dashTileH { lines = append(lines, "") } @@ -287,16 +438,16 @@ func dashTileReportBody(w io.Writer, m metrics.Stats, resources bool) { } // dashTile frames one panel; the selected one carries a lit border. -func dashTile(name string, r fleet.NodeResult, selected bool, a dashAction) string { +func dashTile(name string, r fleet.NodeResult, selected bool, a dashAction, now time.Time, staleAfter time.Duration) string { style := lipgloss.NewStyle(). Width(dashTileW).Height(dashTileH). Border(lipgloss.RoundedBorder()) if selected { - style = style.BorderForeground(lipgloss.Color("214")) + style = style.BorderForeground(lipgloss.Color(brandAccent)) } else { style = style.BorderForeground(lipgloss.Color("240")) } - return style.Render(dashTileContent(name, r, a)) + return style.Render(dashTileContent(name, r, a, now, staleAfter)) } // dashGridRows lays tiles out left to right, top to bottom, in fleet-file diff --git a/cmd/spinloop/fleet.go b/cmd/spinloop/fleet.go index 1a05aeb6..dbca2431 100644 --- a/cmd/spinloop/fleet.go +++ b/cmd/spinloop/fleet.go @@ -422,7 +422,7 @@ func fleetDeployCmd() *cobra.Command { every kind: remote node with --all, deriving what to serve from each node's own Spinloop source: its file field, or its name resolved as a registered alias or a same-named subdirectory beside the fleet file. Reuses the same -derivation, consent, and registration behavior as "spinloop remote deploy".`, +derivation, consent, and registration behaviour as "spinloop remote deploy".`, Args: cobra.ArbitraryArgs, SilenceErrors: true, SilenceUsage: true, @@ -548,18 +548,6 @@ func runFleetDeploy(path string, all bool, names []string, opts deployOpts) erro return nil } -// deploySpinnerFrames are the classic Braille dots, cycled while a node's -// deploy is still in flight. -var deploySpinnerFrames = [...]string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"} - -const ( - ansiGreen = "\033[92m" - ansiRed = "\033[31m" - ansiYellow = "\033[33m" - ansiGrey = "\033[90m" - ansiReset = "\033[0m" -) - // renderDeploySpinner redraws one line per target in place — a spinner // beside whichever nodes are still deploying, a coloured mark beside // whichever have finished — until stop is closed, then erases its own @@ -580,7 +568,7 @@ func renderDeploySpinner(targets []string, results []fleetDeployResult, done []b if done[i] { fmt.Printf("%s %s\n", nodeGlyph(results[i].outcome), name) } else { - fmt.Printf("%s%s%s %s deploying...\n", ansiGrey, deploySpinnerFrames[frame%len(deploySpinnerFrames)], ansiReset, name) + fmt.Printf("%s%s%s %s deploying...\n", ansiGrey, string(spinnerFrames[frame%len(spinnerFrames)]), ansiReset, name) } } mu.Unlock() diff --git a/cmd/spinloop/fleet_dashboard_test.go b/cmd/spinloop/fleet_dashboard_test.go index 46df9fe4..7e77d9e8 100644 --- a/cmd/spinloop/fleet_dashboard_test.go +++ b/cmd/spinloop/fleet_dashboard_test.go @@ -25,8 +25,8 @@ import ( // fakeDashNode is one in-memory fleet node: Start and Stop flip its state and // its Metrics answer follows, so a program-level test can watch a tile turn // on and off. metricsErr makes the node answer no metrics at all. A non-nil -// hold keeps StartWithProgress on its first status line until the test -// closes it — an in-flight start the test can watch. +// hold keeps StartWithProgress on its first phase until the test closes it — +// an in-flight start the test can watch. type fakeDashNode struct { mu sync.Mutex state string @@ -129,13 +129,13 @@ func (n plainDashNode) Logs(ctx context.Context, offset int64, limit int) (daemo return n.f.Logs(ctx, offset, limit) } -// The fake also reports one progress line on the way, so the dashboard's -// progress path is exercised end to end, through the program's Send — and, -// when told to hold, stays on that line until released. A wait that ends on -// the context — the abort — comes back with the context's error, the way the -// control plane's own loop does. -func (f *fakeDashNode) StartWithProgress(ctx context.Context, progress func(string)) (daemon.StatusResponse, error) { - progress("instance starting; retrying in 1s") +// The fake also reports one phase on the way, so the dashboard's progress +// path is exercised end to end, through the program's Send — and, when told to +// hold, stays on that phase until released. A wait that ends on the context — +// the abort — comes back with the context's error, the way the control plane's +// own loop does. +func (f *fakeDashNode) StartWithProgress(ctx context.Context, report func(fleet.StartPhase)) (daemon.StatusResponse, error) { + report(fleet.StartPhase{Kind: fleet.PhaseBooting, Since: time.Now(), Detail: "starting"}) if f.hold != nil { select { case <-f.hold: @@ -208,6 +208,22 @@ func startFastRound(t *testing.T, m *dashModel) (dashRefreshMsg, bool) { return r, true } +// runAction runs the command a key that sets off a start or stop returned, and +// gives back the action's own message. That command is a batch when the action +// also starts the board's repaint chain: the action is the first command in +// it, and the chain's timer is left unrun, since a test has no use for it and +// would only wait on it. +func runAction(t *testing.T, cmd tea.Cmd) tea.Msg { + t.Helper() + if cmd == nil { + t.Fatal("no action command to run") + } + if batch, ok := cmd().(tea.BatchMsg); ok { + return batch[0]() + } + return cmd() +} + // landRounds runs each round the model started to completion and applies its // answer, one at a time. func landRounds(t *testing.T, m *dashModel, cmds []tea.Cmd) *dashModel { @@ -256,6 +272,54 @@ func dashBar(label string, pct float64) string { strings.Repeat("░", width-filled) + fmt.Sprintf(" %.0f%%", pct) } +// dashTestClock is a time whose spinner frame is the cycle's first, so a test +// that pins the clock to it can spell an in-flight heading out in full rather +// than asking the renderer what it drew. +var dashTestClock = time.Unix(1756900000, 0).UTC() + +const dashTestSpinner = "⠋" + +// dashFixNow pins the board's clock for one test. An in-flight tile draws its +// spinner frame and its elapsed time from that clock, so pinning it is what +// makes such a tile the same bytes every run. +func dashFixNow(t *testing.T, at time.Time) { + t.Helper() + dashNow = func() time.Time { return at } + t.Cleanup(func() { dashNow = time.Now }) +} + +// dashTestTile draws one tile at the board's current clock, with the staleness +// bound of a local node — the tile tests supply readings with no time on them, +// which are never called stale, so the bound is not what any of them is about. +func dashTestTile(name string, r fleet.NodeResult, selected bool, a dashAction) string { + return dashTile(name, r, selected, a, dashNow(), dashStaleAfter(fleet.KindDaemon)) +} + +// dashExpectedHeader is the tile's header bar as a test spells it out: the +// dark background across the tile's full width, the health mark in its own +// colour on top of it, and light text after. Written out here rather than +// taken from the renderer, so the bar is pinned by the test rather than by +// itself. +func dashExpectedHeader(text string, tier dashHealthTier) string { + colour, mark := "\033[31m", "●" + switch tier { + case dashHealthy: + colour = "\033[92m" + case dashAttention: + colour = "\033[33m" + case dashNotServing: + colour = "\033[90m" + case dashUnknown: + colour, mark = "\033[90m", "?" + } + pad := dashTileW - 2 - lipgloss.Width(text) + if pad < 0 { + pad = 0 + } + return "\033[48;5;235m" + colour + mark + "\033[97m " + text + + strings.Repeat(" ", pad) + "\033[0m" +} + func dashTileExpected(lines []string) string { var b strings.Builder b.WriteString("╭" + strings.Repeat("─", dashTileW) + "╮\n") @@ -283,7 +347,7 @@ func TestDashTileRunningByteStable(t *testing.T) { }, } want := dashTileExpected([]string{ - dashHealthGlyph(dashHealthy) + " up running (up 2h 0m 0s)", + dashExpectedHeader("up running (up 2h 0m 0s)", dashHealthy), "llamacpp org/qwen:q4", " last active 12s ago", dashBar("CPU", 42), @@ -296,7 +360,7 @@ func TestDashTileRunningByteStable(t *testing.T) { " generation tokens: 1024", " requests: 17", }) - if got := dashTile("up", r, false, dashAction{}); got != want { + if got := dashTestTile("up", r, false, dashAction{}); got != want { t.Errorf("tile mismatch:\ngot:\n%q\nwant:\n%q", got, want) } } @@ -307,16 +371,16 @@ func TestDashTileOutcomeAndEmpty(t *testing.T) { Name: "down", Outcome: fleet.OutcomeUnreachable, Err: errors.New("connection refused (127.0.0.1:1)"), } - if got := dashTile("down", dead, false, dashAction{}); got != dashTileExpected([]string{ - dashHealthGlyph(dashUnhealthy) + " down unreachable", + if got := dashTestTile("down", dead, false, dashAction{}); got != dashTileExpected([]string{ + dashExpectedHeader("down unreachable", dashUnhealthy), "connection refused (127.0.0.1:1)", "", "", "", "", "", "", "", "", "", "", }) { t.Errorf("outcome tile mismatch:\n%q", got) } // A node not answered yet is an empty panel naming the node. - if got := dashTile("down", fleet.NodeResult{Name: "down"}, false, dashAction{}); got != dashTileExpected([]string{ - dashHealthGlyph(dashUnknown) + " down", "waiting for first refresh…", + if got := dashTestTile("down", fleet.NodeResult{Name: "down"}, false, dashAction{}); got != dashTileExpected([]string{ + dashExpectedHeader("down", dashUnknown), "waiting for first refresh…", "", "", "", "", "", "", "", "", "", "", }) { t.Errorf("empty tile mismatch:\n%q", got) @@ -330,10 +394,10 @@ func TestDashTileStoppedByteStable(t *testing.T) { Metrics: metrics.Stats{State: "idle"}, } want := dashTileExpected([]string{ - dashHealthGlyph(dashNotServing) + " idle idle", + dashExpectedHeader("idle idle", dashNotServing), "", "", "", "", "", "", "", "", "", "", "", }) - if got := dashTile("idle", r, false, dashAction{}); got != want { + if got := dashTestTile("idle", r, false, dashAction{}); got != want { t.Errorf("stopped tile mismatch:\n%q\nwant:\n%q", got, want) } // A remote environment with no instance at all reports undeployed and @@ -344,74 +408,89 @@ func TestDashTileStoppedByteStable(t *testing.T) { Metrics: metrics.Stats{State: "undeployed", Runner: "llamacpp", ModelID: "unsloth/Qwen3.8-27B-GGUF"}, } wantUndeployed := dashTileExpected([]string{ - dashHealthGlyph(dashNotServing) + " dev-1 undeployed", + dashExpectedHeader("dev-1 undeployed", dashNotServing), "llamacpp unsloth/Qwen3.8-27B-GGUF", "", "", "", "", "", "", "", "", "", "", }) - if got := dashTile("dev-1", u, false, dashAction{}); got != wantUndeployed { + if got := dashTestTile("dev-1", u, false, dashAction{}); got != wantUndeployed { t.Errorf("undeployed tile mismatch:\ngot:\n%q\nwant:\n%q", got, wantUndeployed) } } -// A node with an action in flight and no report yet shows the verb and the -// call's own lines; a stop conjugates: the p of stop drops before -ing. +// A node with an action in flight and no report yet shows the verb, a spinner +// beside it, and the call's own phase; a stop conjugates: the p of stop drops +// before -ing. func TestDashTileActionInFlight(t *testing.T) { lipgloss.SetColorProfile(termenv.Ascii) - if got := dashTile("dev-2", fleet.NodeResult{Name: "dev-2"}, false, - dashAction{verb: "start", line: "instance starting; retrying in 42s"}); got != dashTileExpected([]string{ - dashHealthGlyph(dashAttention) + " dev-2 starting", - "instance starting; retrying in 42s", + dashFixNow(t, dashTestClock) + wait := fleet.StartPhase{Kind: fleet.PhaseWaitingCapacity, + Since: dashTestClock, RetryAt: dashTestClock.Add(42 * time.Second)} + if got := dashTestTile("dev-2", fleet.NodeResult{Name: "dev-2"}, false, + dashAction{verb: "start", phase: &wait}); got != dashTileExpected([]string{ + dashExpectedHeader("dev-2 "+dashTestSpinner+" starting", dashAttention), + "waiting for capacity — retrying in 42s", "", "", "", "", "", "", "", "", "", "", }) { t.Errorf("in-flight tile mismatch:\n%q", got) } // A stop conjugates: the p of stop drops before -ing. - if got := dashTile("dev-2", fleet.NodeResult{Name: "dev-2"}, false, + if got := dashTestTile("dev-2", fleet.NodeResult{Name: "dev-2"}, false, dashAction{verb: "stop"}); got != dashTileExpected([]string{ - dashHealthGlyph(dashAttention) + " dev-2 stopping", + dashExpectedHeader("dev-2 "+dashTestSpinner+" stopping", dashAttention), "", "", "", "", "", "", "", "", "", "", "", }) { t.Errorf("bare in-flight tile mismatch:\n%q", got) } } -// The elapsed time beside the verb is the tile's heartbeat. A start's own -// status lines can legitimately stand unchanged for minutes — the attempt that -// finds capacity holds one request for the whole boot and says nothing while -// it does — so the moving number is what tells the operator the tile is -// waiting rather than wedged. It is drawn from the board's clock on every -// repaint, not baked into a line when that line arrives. +// A start's phase can hold unchanged for minutes: the attempt that obtains +// capacity holds one request open for the duration of the boot and reports +// nothing while it does. Everything about time on the tile is therefore +// computed from the board's clock on each repaint rather than stored when the +// phase arrived — the elapsed time beside the verb counts up, and the wait +// counts down towards the attempt it is waiting for. func TestDashTileActionInFlightShowsElapsed(t *testing.T) { lipgloss.SetColorProfile(termenv.Ascii) - now := time.Now() + now := dashTestClock dashNow = func() time.Time { return now } t.Cleanup(func() { dashNow = time.Now }) - a := dashAction{verb: "start", since: now.Add(-150 * time.Second), - line: "instance no-capacity; retrying in 120s"} + wait := fleet.StartPhase{Kind: fleet.PhaseWaitingCapacity, + Since: now, RetryAt: now.Add(120 * time.Second), Detail: "no-capacity"} + a := dashAction{verb: "start", since: now.Add(-150 * time.Second), phase: &wait} want := dashTileExpected([]string{ - dashHealthGlyph(dashAttention) + " dev-2 starting 2m 30s", - "instance no-capacity; retrying in 120s", + dashExpectedHeader("dev-2 "+dashTestSpinner+" starting 2m 30s", dashAttention), + "waiting for capacity — retrying in 2m 0s", "", "", "", "", "", "", "", "", "", "", }) - if got := dashTile("dev-2", fleet.NodeResult{Name: "dev-2"}, false, a); got != want { + if got := dashTestTile("dev-2", fleet.NodeResult{Name: "dev-2"}, false, a); got != want { t.Errorf("elapsed in-flight tile mismatch:\ngot:\n%q\nwant:\n%q", got, want) } - // The same tile a minute later, with nothing else having changed: the - // number has moved, which is the whole point of it. + // The same tile a minute later, with nothing about the action having + // changed: the action has counted up and the wait has counted down, which + // is the whole point of computing both when the tile is drawn. now = now.Add(time.Minute) - if got := dashTile("dev-2", fleet.NodeResult{Name: "dev-2"}, false, a); !strings.Contains(got, "starting 3m 30s") { + got := dashTestTile("dev-2", fleet.NodeResult{Name: "dev-2"}, false, a) + if !strings.Contains(got, "starting 3m 30s") { t.Errorf("the elapsed time did not move with the clock:\n%q", got) } + if !strings.Contains(got, "retrying in 1m 0s") { + t.Errorf("the wait did not count down with the clock:\n%q", got) + } } // A report that lands while an action is in flight shows on the tile beside -// the call's own lines: the call says what the operator asked for, the report +// the call's own account: the call says what the operator asked for, the report // says what the node is doing — a boot half done already carries a state and // whatever it measures, and that is the truth the tile should keep showing // while the start still works. func TestDashTileActionInFlightWithReport(t *testing.T) { lipgloss.SetColorProfile(termenv.Ascii) + dashFixNow(t, dashTestClock) + capacity := fleet.StartPhase{Kind: fleet.PhaseWaitingCapacity, + Since: dashTestClock, RetryAt: dashTestClock.Add(120 * time.Second), Detail: "no-capacity"} + booting := fleet.StartPhase{Kind: fleet.PhaseBooting, + Since: dashTestClock.Add(-60 * time.Second), Detail: "starting"} // The instance is up and measuring, the engine not serving yet: the // start still works, and the report carries the state and the bars. r := fleet.NodeResult{ @@ -425,8 +504,8 @@ func TestDashTileActionInFlightWithReport(t *testing.T) { }, } want := dashTileExpected([]string{ - dashHealthGlyph(dashAttention) + " vllm-1 starting", - "instance no-capacity; retrying in 120s", + dashExpectedHeader("vllm-1 "+dashTestSpinner+" starting", dashAttention), + "waiting for capacity — retrying in 2m 0s", "running (up 4m 0s)", "vllm org/qwen3:32b", dashBar("CPU", 12), @@ -435,8 +514,8 @@ func TestDashTileActionInFlightWithReport(t *testing.T) { dashBar("GPU mem", 45), "", "", "", "", }) - if got := dashTile("vllm-1", r, false, - dashAction{verb: "start", line: "instance no-capacity; retrying in 120s"}); got != want { + if got := dashTestTile("vllm-1", r, false, + dashAction{verb: "start", phase: &capacity}); got != want { t.Errorf("in-flight tile with report mismatch:\ngot:\n%q\nwant:\n%q", got, want) } // Early in a boot: the report has a state and the serving, nothing @@ -446,14 +525,14 @@ func TestDashTileActionInFlightWithReport(t *testing.T) { Metrics: metrics.Stats{State: "pending", Runner: "vllm", ModelID: "org/qwen3:32b"}, } wantEarly := dashTileExpected([]string{ - dashHealthGlyph(dashAttention) + " vllm-1 starting", - "instance starting; retrying in 60s", + dashExpectedHeader("vllm-1 "+dashTestSpinner+" starting", dashAttention), + "booting (1m 0s)", "pending", "vllm org/qwen3:32b", "", "", "", "", "", "", "", "", }) - if got := dashTile("vllm-1", early, false, - dashAction{verb: "start", line: "instance starting; retrying in 60s"}); got != wantEarly { + if got := dashTestTile("vllm-1", early, false, + dashAction{verb: "start", phase: &booting}); got != wantEarly { t.Errorf("early-boot in-flight tile mismatch:\ngot:\n%q\nwant:\n%q", got, wantEarly) } // A round that failed this time says nothing on the tile: the call's own @@ -463,16 +542,198 @@ func TestDashTileActionInFlightWithReport(t *testing.T) { Err: errors.New("stats returned HTTP 503: instance is not running"), } wantFailed := dashTileExpected([]string{ - dashHealthGlyph(dashAttention) + " vllm-1 starting", - "instance starting; retrying in 60s", + dashExpectedHeader("vllm-1 "+dashTestSpinner+" starting", dashAttention), + "booting (1m 0s)", "", "", "", "", "", "", "", "", "", "", }) - if got := dashTile("vllm-1", failed, false, - dashAction{verb: "start", line: "instance starting; retrying in 60s"}); got != wantFailed { + if got := dashTestTile("vllm-1", failed, false, + dashAction{verb: "start", phase: &booting}); got != wantFailed { t.Errorf("in-flight tile over a failed round mismatch:\ngot:\n%q\nwant:\n%q", got, wantFailed) } } +// The combination that produced the original defect, and every other pairing +// of what a start is doing against what the node's last reading says. One +// function produces both halves of a panel, so the pairings can be listed here +// rather than only being reachable through a rendered tile. +// +// The case to look for: a start waiting for capacity beside a reading that +// reports the node running. Both are true — the reading is of the instance the +// previous attempt left behind — and the panel has to say both without either +// reading as the other. +func TestDashNodeViewEveryPhaseAgainstEveryReading(t *testing.T) { + now := dashTestClock + const staleAfter = 6 * time.Second + + fresh := fleet.NodeResult{Name: "n", Outcome: fleet.OutcomeOK, + Metrics: metrics.Stats{State: "running", Ready: "ready", UptimeSeconds: 240}, + At: now.Add(-time.Second)} + stale := fresh + stale.At = now.Add(-3 * time.Minute) + failed := fleet.NodeResult{Name: "n", Outcome: fleet.OutcomeUnreachable, + Err: errors.New("connection refused"), At: now.Add(-time.Second)} + none := fleet.NodeResult{Name: "n"} + + phases := []struct { + name string + phase *fleet.StartPhase + line string // the phase's own line, or "" where the action reports none + }{ + {"no phase yet", nil, ""}, + {"attempting", &fleet.StartPhase{Kind: fleet.PhaseAttempting, Since: now}, + "waking the instance…"}, + {"waiting for capacity", &fleet.StartPhase{Kind: fleet.PhaseWaitingCapacity, + Since: now, RetryAt: now.Add(90 * time.Second), Detail: "no-capacity"}, + "waiting for capacity — retrying in 1m 30s"}, + {"booting", &fleet.StartPhase{Kind: fleet.PhaseBooting, Since: now.Add(-time.Minute), + Detail: "starting"}, "booting (1m 0s)"}, + {"reconnecting", &fleet.StartPhase{Kind: fleet.PhaseReconnecting, Since: now, + RetryAt: now.Add(5 * time.Second), Detail: "unexpected EOF"}, + "connection dropped (unexpected EOF) — retrying in 5s"}, + } + readings := []struct { + name string + r fleet.NodeResult + reports string // what the reading puts on the panel, or "" for none + settled dashHealthTier // the tier with nothing in flight + }{ + {"no reading yet", none, "", dashUnknown}, + {"a fresh answer", fresh, "running (up 4m 0s)", dashHealthy}, + {"a stale answer", stale, "running (up 4m 0s) · 3m 0s ago", dashUnknown}, + {"a failed round", failed, "", dashUnhealthy}, + } + + for _, ph := range phases { + for _, rd := range readings { + t.Run(ph.name+" over "+rd.name, func(t *testing.T) { + a := dashAction{verb: "start", since: now.Add(-30 * time.Second), phase: ph.phase} + lines, tier := dashNodeView("n", rd.r, a, now, staleAfter) + joined := strings.Join(lines, "\n") + // The action's own account leads, and the node's report + // follows it where the reading has one to give. + if want := "n " + dashTestSpinner + " starting 30s"; lines[0] != want { + t.Errorf("heading = %q, want %q", lines[0], want) + } + if ph.line != "" && !strings.Contains(joined, ph.line) { + t.Errorf("the phase is not on the panel:\n%s", joined) + } + if rd.reports != "" && !strings.Contains(joined, rd.reports) { + t.Errorf("the reading is not on the panel:\n%s", joined) + } + // A failed round says nothing here: the action's account + // stands, and the next round will say more. + if rd.name == "a failed round" && strings.Contains(joined, "connection refused") { + t.Errorf("a failed round painted over the action's account:\n%s", joined) + } + // An action in flight is always attention, whatever the + // reading says — including a reading that reports the node + // running while the start is still waiting for capacity. + if tier != dashAttention { + t.Errorf("tier = %v, want attention while an action is in flight", tier) + } + // Nothing about the action is shown once it settles, and the + // reading alone then decides the tier. + settledLines, settledTier := dashNodeView("n", rd.r, dashAction{}, now, staleAfter) + if settledTier != rd.settled { + t.Errorf("settled tier = %v, want %v", settledTier, rd.settled) + } + if ph.line != "" && strings.Contains(strings.Join(settledLines, "\n"), ph.line) { + t.Errorf("a finished action left its phase on the panel:\n%s", + strings.Join(settledLines, "\n")) + } + }) + } + } +} + +// A reading that has aged past a few of its node's own intervals is drawn with +// its age and reads unknown, rather than being drawn identically to a current +// one — and goes back to plain once the node answers again. +func TestDashTileStaleReadingShowsItsAgeAndRecovers(t *testing.T) { + lipgloss.SetColorProfile(termenv.Ascii) + now := dashTestClock + dashFixNow(t, now) + + r := fleet.NodeResult{Name: "dev-1", Outcome: fleet.OutcomeOK, + Metrics: metrics.Stats{State: "running", Ready: "ready"}, + At: now.Add(-4 * time.Minute)} + staleAfter := dashStaleAfter(fleet.KindRemote) // three minutes, on the minute cadence + got := dashTile("dev-1", r, false, dashAction{}, now, staleAfter) + want := dashTileExpected([]string{ + dashExpectedHeader("dev-1 running · 4m 0s ago", dashUnknown), + "", "", "", "", "", "", "", "", "", "", "", + }) + if got != want { + t.Errorf("stale tile mismatch:\ngot:\n%q\nwant:\n%q", got, want) + } + // The node answers again: the age goes, and so does the grey. + r.At = now.Add(-time.Second) + wantFresh := dashTileExpected([]string{ + dashExpectedHeader("dev-1 running", dashHealthy), + "", "", "", "", "", "", "", "", "", "", "", + }) + if got := dashTile("dev-1", r, false, dashAction{}, now, staleAfter); got != wantFresh { + t.Errorf("recovered tile mismatch:\ngot:\n%q\nwant:\n%q", got, wantFresh) + } +} + +// The board's own title bar: the brand in the accent, the screen beside it, +// and what is on that screen pushed to the right, all on one surface running +// the terminal's full width. A terminal too narrow for both halves keeps the +// left one. +func TestDashTitleBar(t *testing.T) { + lipgloss.SetColorProfile(termenv.ANSI256) + const ( + accent = "\x1b[1;38;5;43;48;5;235m" // the logo's mint, bold, on the bar + dim = "\x1b[38;5;109;48;5;235m" // the muted ink for the right half + ) + for _, w := range []int{120, 80, 40, 24, 1} { + bar := dashTitleBar("fleet dashboard", "examples/fleet.yaml (3 nodes)", w) + if got := lipgloss.Width(bar); got != w { + t.Errorf("at width %d the bar is %d columns: %q", w, got, bar) + } + if !strings.HasPrefix(bar, accent) { + t.Errorf("at width %d the bar does not open in the accent: %q", w, bar) + } + } + wide := dashTitleBar("fleet dashboard", "examples/fleet.yaml (3 nodes)", 120) + if !strings.HasPrefix(wide, accent+" spinloop") { + t.Errorf("the brand is not in the accent: %q", wide) + } + if !strings.Contains(wide, dim+"examples/fleet.yaml (3 nodes) ") { + t.Errorf("the right half is not in the muted ink: %q", wide) + } + // Too narrow for both: the right half goes rather than wrapping. + if narrow := dashTitleBar("fleet dashboard", "examples/fleet.yaml", 40); strings.Contains(narrow, "fleet.yaml") { + t.Errorf("a narrow bar kept its right half: %q", narrow) + } + // A width of nothing draws nothing, rather than a bar of negative padding. + if got := dashTitleBar("fleet dashboard", "x", 0); got != "" { + t.Errorf("dashTitleBar at width 0 = %q, want empty", got) + } +} + +// The header bar runs the tile's full width and is the same colour on every +// tile, whatever the node's health: the glyph is what health is read from, so +// a bar that changed colour with the tier would compete with it. +func TestDashTileHeaderIsOneColourWhateverTheHealth(t *testing.T) { + lipgloss.SetColorProfile(termenv.Ascii) + healthy := dashTileHeader("up running", dashHealthy) + unhealthy := dashTileHeader("down unreachable", dashUnhealthy) + for _, bar := range []string{healthy, unhealthy} { + if !strings.HasPrefix(bar, "\033[48;5;235m") { + t.Errorf("header bar does not open with the background: %q", bar) + } + if w := lipgloss.Width(bar); w != dashTileW { + t.Errorf("header bar is %d columns, want the tile's %d: %q", w, dashTileW, bar) + } + } + // The two differ only in their glyph and their text, never in the bar. + if !strings.Contains(healthy, "\033[92m●") || !strings.Contains(unhealthy, "\033[31m●") { + t.Errorf("the glyphs lost their own colours:\n%q\n%q", healthy, unhealthy) + } +} + func TestDashHealthTierFor(t *testing.T) { cases := []struct { name string @@ -513,28 +774,33 @@ func TestDashHealthTierFor(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - if got := dashHealthTierFor(c.r, c.a); got != c.want { + if got := dashHealthTierFor(c.r, c.a, false); got != c.want { t.Errorf("dashHealthTierFor() = %v, want %v", got, c.want) } }) } } -// The selection is carried by the lit border; a colour profile that keeps -// colour is needed to see it, since the byte-stable profile strips it. +// The selection is carried by a border in the brand accent; a colour profile +// that keeps colour is needed to see it, since the byte-stable profile strips +// it. The accent is the mint of the spinloop logo, which a 256-colour terminal +// renders as index 43 — spelled out here so a change of accent has to be a +// deliberate one. func TestDashTileSelectedBorderLit(t *testing.T) { lipgloss.SetColorProfile(termenv.ANSI256) r := fleet.NodeResult{Name: "n"} - sel, unsel := dashTile("n", r, true, dashAction{}), dashTile("n", r, false, dashAction{}) - if !strings.Contains(sel, "\x1b[38;5;214m") { - t.Errorf("selected tile carries no lit border:\n%q", sel) + sel, unsel := dashTestTile("n", r, true, dashAction{}), dashTestTile("n", r, false, dashAction{}) + const accent = "\x1b[38;5;43m" + if !strings.Contains(sel, accent) { + t.Errorf("selected tile carries no accented border:\n%q", sel) } - if strings.Contains(unsel, "\x1b[38;5;214m") { - t.Errorf("unselected tile carries the lit border:\n%q", unsel) + if strings.Contains(unsel, accent) { + t.Errorf("unselected tile carries the accented border:\n%q", unsel) } // The health glyph is unaffected by selection: same tier, same colour, - // whether or not the border is lit. - glyph := dashHealthGlyph(dashUnknown) + // whether or not the border is lit. It is read off the header bar, where + // the mark sits between the bar's background and its text colour. + glyph := "\033[48;5;235m\033[90m?\033[97m" if !strings.Contains(sel, glyph) { t.Errorf("selected tile's glyph changed:\n%q", sel) } @@ -551,7 +817,7 @@ func TestDashTileClipsLongLines(t *testing.T) { Name: "n", Outcome: fleet.OutcomeOK, Metrics: metrics.Stats{State: "running", ModelID: strings.Repeat("m", 60)}, } - rows := strings.Split(dashTile("n", r, false, dashAction{}), "\n") + rows := strings.Split(dashTestTile("n", r, false, dashAction{}), "\n") if len(rows) != dashTileH+2 { t.Fatalf("tile is %d rows, want %d", len(rows), dashTileH+2) } @@ -574,8 +840,8 @@ func TestDashTileUptimeSurvivesLongServingLine(t *testing.T) { UptimeSeconds: 125, }, } - lines := strings.Split(dashTile("n", r, false, dashAction{}), "\n") - want := "│" + dashHealthGlyph(dashHealthy) + " n running (up 2m 5s)" + lines := strings.Split(dashTestTile("n", r, false, dashAction{}), "\n") + want := "│" + dashExpectedHeader("n running (up 2m 5s)", dashHealthy) if got := lines[1]; !strings.HasPrefix(got, want) { t.Errorf("state line = %q, want prefix %q", got, want) } @@ -627,7 +893,7 @@ func TestDashTileTruncatesTallContent(t *testing.T) { Tokens: &metrics.TokenStats{Running: 1, PromptTokens: 100, GenerationTokens: 50, Requests: 3}, }, } - lines := strings.Split(dashTile("many", r, false, dashAction{}), "\n") + lines := strings.Split(dashTestTile("many", r, false, dashAction{}), "\n") if len(lines) != dashTileH+2 { t.Errorf("a tall node broke the tile geometry: %d lines (want %d)", len(lines), dashTileH+2) } @@ -1058,10 +1324,10 @@ func TestDashModelResizeChangesGrid(t *testing.T) { } } -// A tick while a group's round is in flight is absorbed; a round's answer -// lands by generation, and a superseded answer is dropped rather than -// painted over the board. -func TestDashModelRefreshGenerations(t *testing.T) { +// A tick while a group's round is in flight is absorbed; an answer lands by +// the time its own read was taken, and an older reading is dropped rather than +// painted over a newer one. +func TestDashModelRefreshOrdersReadingsByTheirTime(t *testing.T) { node := newFakeDashNode("running") m := &dashModel{ entries: []dashEntry{{name: "a", kind: fleet.KindDaemon, node: node}}, @@ -1072,27 +1338,28 @@ func TestDashModelRefreshGenerations(t *testing.T) { // In flight: the tick reschedules itself but must not begin a second // round in the group. m.fastBusy = true - m.fastGen = 7 m2, _ := m.Update(dashTickMsg{}) - if m2.(*dashModel).fastGen != 7 { + m = m2.(*dashModel) + if !m.fastBusy { t.Fatal("tick while refreshing started another round") } - m = m2.(*dashModel) - // A stale answer: discarded. - stale := []fleet.NodeResult{{Outcome: fleet.OutcomeOK}} - fresh := []fleet.NodeResult{{Outcome: fleet.OutcomeUnreachable}} - m2, _ = m.Update(dashRefreshMsg{gen: 6, idx: []int{0}, results: stale}) - if m2.(*dashModel).results[0].Outcome != "" { - t.Fatalf("stale round painted the board: %v", m2.(*dashModel).results[0].Outcome) - } - m3, _ := m2.(*dashModel).Update(dashRefreshMsg{gen: 7, idx: []int{0}, results: fresh}) - mm := m3.(*dashModel) + now := time.Now() + fresh := []fleet.NodeResult{{Outcome: fleet.OutcomeUnreachable, At: now}} + older := []fleet.NodeResult{{Outcome: fleet.OutcomeOK, At: now.Add(-time.Second)}} + m2, _ = m.Update(dashRefreshMsg{idx: []int{0}, results: fresh}) + mm := m2.(*dashModel) if mm.fastBusy { t.Fatal("round completion did not clear the flag") } if mm.results[0].Outcome != fleet.OutcomeUnreachable { t.Fatalf("current round not applied: %v", mm.results[0].Outcome) } + // A reading taken before the one on screen: dropped. + m3, _ := mm.Update(dashRefreshMsg{idx: []int{0}, results: older}) + mm = m3.(*dashModel) + if mm.results[0].Outcome != fleet.OutcomeUnreachable { + t.Fatalf("an older reading painted the board: %v", mm.results[0].Outcome) + } // A live round: per-node answers, in entry order. msg, ok := startFastRound(t, mm) if !ok { @@ -1139,8 +1406,10 @@ func TestDashModelRemoteCadence(t *testing.T) { if m.fastBusy || m.slowBusy { t.Fatal("rounds still marked in flight after their answers landed") } - if !m.nextSlowAt.After(time.Now()) { - t.Fatal("starting the cloud round did not spend its deadline") + for i := 1; i < 3; i++ { + if !m.dueAt(i).After(time.Now()) { + t.Fatalf("reading %s did not spend its deadline", m.entries[i].name) + } } // Before the deadline, a tick is due for the local round only. cmds = m.startRounds() @@ -1149,23 +1418,80 @@ func TestDashModelRemoteCadence(t *testing.T) { } m = landRounds(t, m, cmds) // At the deadline, both groups are due again. - m.nextSlowAt = time.Now().Add(-time.Millisecond) + m.scheduleRead(1, time.Now().Add(-time.Millisecond)) + m.scheduleRead(2, time.Now().Add(-time.Millisecond)) if cmds = m.startRounds(); len(cmds) != 2 { t.Fatalf("due tick started %d rounds, want both groups", len(cmds)) } m = landRounds(t, m, cmds) - // A manual refresh is due for every node, whatever the deadline says. - m.nextSlowAt = time.Now().Add(time.Hour) + // A manual refresh is due for every node, whatever the deadlines say. + m.scheduleRead(1, time.Now().Add(time.Hour)) + m.scheduleRead(2, time.Now().Add(time.Hour)) m2, _ := m.Update(dashKey("r")) mm := m2.(*dashModel) - if mm.nextSlowAt.IsZero() { - t.Error("the manual refresh did not bring the cloud deadline forward") - } if !mm.fastBusy || !mm.slowBusy { t.Errorf("the manual refresh did not start both groups: fastBusy=%v slowBusy=%v", mm.fastBusy, mm.slowBusy) } } +// A node the operator is acting on is read on the short interval whatever its +// kind, and returns to its own cadence once the action settles. Its +// neighbours in the same group keep their own cadence throughout. +func TestDashModelActedOnNodeIsReadMoreOften(t *testing.T) { + orig := dashboardRemoteRefreshInterval + dashboardRemoteRefreshInterval = time.Minute + defer func() { dashboardRemoteRefreshInterval = orig }() + + r1, r2 := newFakeDashNode("stopped"), newFakeDashNode("running") + hold := make(chan struct{}) + r1.hold = hold + m := &dashModel{ + entries: []dashEntry{ + {name: "r1", kind: fleet.KindRemote, node: r1}, + {name: "r2", kind: fleet.KindRemote, node: r2}, + }, + results: make([]fleet.NodeResult, 2), + actions: make([]dashAction, 2), + width: 120, height: 40, + } + // A cold round reads both, and puts both on the cloud cadence. + m = landRounds(t, m, m.startRounds()) + for i := range m.entries { + if !m.dueAt(i).After(time.Now().Add(30 * time.Second)) { + t.Fatalf("%s is not on the cloud cadence", m.entries[i].name) + } + } + // A start on r1 brings it forward at once and holds it on the short + // interval; r2 is untouched. + m.cursor = 0 + cmd := m.beginAction("start") + if m.dueAt(0).After(time.Now()) { + t.Error("the started node was not brought forward for reading") + } + if !m.dueAt(1).After(time.Now().Add(30 * time.Second)) { + t.Error("the neighbour lost its own cadence") + } + m = landRounds(t, m, m.startRounds()) + if got := m.dueAt(0).Sub(time.Now()); got > 2*dashboardRefreshInterval { + t.Errorf("the started node is next read in %s, want the short interval", got) + } + if !m.dueAt(1).After(time.Now().Add(30 * time.Second)) { + t.Errorf("the neighbour was dragged onto the short interval: due in %s", time.Until(m.dueAt(1))) + } + // The start finishes: the node is read once more now, then returns to + // its kind's own cadence. + close(hold) + next, _ := m.Update(runAction(t, cmd)) + m = next.(*dashModel) + if m.dueAt(0).After(time.Now()) { + t.Error("the finished action did not bring its node forward for one more read") + } + m = landRounds(t, m, m.startRounds()) + if !m.dueAt(0).After(time.Now().Add(30 * time.Second)) { + t.Errorf("the node did not return to its own cadence: due in %s", time.Until(m.dueAt(0))) + } +} + // One dead node does not keep its neighbour's answer waiting: each node // answers on its own, and the fan-out calls them concurrently. func TestDashRoundDoesNotWaitOnDeadNode(t *testing.T) { @@ -1216,7 +1542,7 @@ func TestDashModelStartAndStop(t *testing.T) { if _, cmd2 := mm.Update(dashKey("s")); cmd2 != nil { t.Fatal("a node with an action in flight started again") } - smsg, _ := cmd().(dashActionMsg) + smsg, _ := runAction(t, cmd).(dashActionMsg) if node.starts != 1 { t.Fatal("start not called") } @@ -1246,7 +1572,7 @@ func TestDashModelStartAndStop(t *testing.T) { if cmd2 == nil { t.Fatal("y did not stop") } - pmsg, _ := cmd2().(dashActionMsg) + pmsg, _ := runAction(t, cmd2).(dashActionMsg) if node.stops != 1 { t.Fatal("stop not called") } @@ -1296,6 +1622,7 @@ func TestDashModelConcurrentStarts(t *testing.T) { actions: make([]dashAction, 2), width: 120, height: 40, } + dashFixNow(t, dashTestClock) var caught []tea.Msg m.send = func(msg tea.Msg) { caught = append(caught, msg) } @@ -1323,22 +1650,22 @@ func TestDashModelConcurrentStarts(t *testing.T) { if m.actions[0].verb != "start" || m.actions[1].verb != "start" { t.Fatalf("both actions should be in flight: %+v", m.actions) } - if v := m.View(); !strings.Contains(v, "a starting") || !strings.Contains(v, "b starting") { + if v := m.View(); !strings.Contains(v, "a "+dashTestSpinner+" starting") || + !strings.Contains(v, "b "+dashTestSpinner+" starting") { t.Errorf("the in-flight tiles do not carry the verb:\n%s", v) } // Run both calls: each reports its own line through the send door. - msgA := cmd() - msgB := cmdB() + msgA := runAction(t, cmd) + msgB := runAction(t, cmdB) for _, msg := range caught { next, _ = m.Update(msg) m = next.(*dashModel) } - if m.actions[0].line != "instance starting; retrying in 1s" || - m.actions[1].line != "instance starting; retrying in 1s" { - t.Errorf("progress lines not on the nodes: %+v", m.actions) + if m.actions[0].phase == nil || m.actions[1].phase == nil { + t.Errorf("the calls' phases are not on the nodes: %+v", m.actions) } - if v := m.View(); !strings.Contains(v, "retrying in 1s") { - t.Errorf("the tile does not show the call's line:\n%s", v) + if v := m.View(); !strings.Contains(v, "booting") { + t.Errorf("the tile does not show the call's phase:\n%s", v) } // Each final clears its own node and leaves its line in the footer. next, _ = m.Update(msgA) @@ -1360,9 +1687,10 @@ func TestDashModelConcurrentStarts(t *testing.T) { } // A round that lands while a start is in flight paints on the tile beside -// the call's own lines: the node's report and the call's account both show, +// the call's own account: the node's report and the call's phase both show, // until the call returns and the report stands alone. func TestDashModelLandedRoundShowsBesideInFlightAction(t *testing.T) { + dashFixNow(t, dashTestClock) f := newFakeDashNode("stopped") m := &dashModel{ entries: []dashEntry{{name: "a", kind: fleet.KindRemote, node: f}}, @@ -1375,8 +1703,10 @@ func TestDashModelLandedRoundShowsBesideInFlightAction(t *testing.T) { if m.actions[0].verb != "start" { t.Fatalf("no action recorded: %+v", m.actions[0]) } - // The call's own line, as its goroutine would send it. - next, _ = m.Update(dashActionProgressMsg{node: "a", line: "instance starting; retrying in 1s"}) + // The call's own phase, as its goroutine would send it. + next, _ = m.Update(dashActionProgressMsg{node: "a", + phase: fleet.StartPhase{Kind: fleet.PhaseWaitingCapacity, + Since: dashTestClock, RetryAt: dashTestClock.Add(time.Second)}}) m = next.(*dashModel) // The cloud round lands while the start is in flight. cmd := m.refreshRemoteGroup(true) @@ -1388,7 +1718,8 @@ func TestDashModelLandedRoundShowsBesideInFlightAction(t *testing.T) { m = next.(*dashModel) v := m.View() for _, want := range []string{ - "a starting", "instance starting; retrying in 1s", "stopped", "llamacpp org/qwen", + "a " + dashTestSpinner + " starting", "waiting for capacity — retrying in 1s", + "stopped", "llamacpp org/qwen", } { if !strings.Contains(v, want) { t.Errorf("the in-flight tile does not carry %q:\n%s", want, v) @@ -1481,7 +1812,11 @@ func TestDashModelQuitDuringConfirmation(t *testing.T) { // A slow-group answer from a superseded round is discarded, not painted — // the generation guard works for the cloud group, not only the local one. -func TestDashModelSlowStaleRoundDiscarded(t *testing.T) { +// The case a per-group counter cannot order: a round issued while a start was +// in flight, landing after the start finished and the board took the node's +// post-action report. The round carries the node's state as of before the +// action, and must not repaint the newer report with it. +func TestDashModelLateRoundDoesNotOverwriteAPostActionReport(t *testing.T) { node := newFakeDashNode("stopped") m := &dashModel{ entries: []dashEntry{{name: "a", kind: fleet.KindRemote, node: node}}, @@ -1489,22 +1824,20 @@ func TestDashModelSlowStaleRoundDiscarded(t *testing.T) { actions: make([]dashAction, 1), width: 120, height: 40, } - m.slowBusy = true - m.slowGen = 3 - before := m.results[0] - stale := dashRefreshMsg{ - remote: true, - gen: 2, // superseded: the model is on generation 3 - idx: []int{0}, - results: []fleet.NodeResult{{Name: "a", Outcome: fleet.OutcomeOK, Status: daemon.StatusResponse{State: "running"}}}, - } - next, _ := m.Update(stale) + issued := time.Now() + // The start finishes, and the report that follows it lands. + after := fleet.NodeResult{Name: "a", Outcome: fleet.OutcomeOK, + Metrics: metrics.Stats{State: "running"}, At: issued.Add(3 * time.Second)} + next, _ := m.Update(dashRefreshMsg{remote: true, idx: []int{0}, results: []fleet.NodeResult{after}}) m = next.(*dashModel) - if m.results[0].Name != before.Name || m.results[0].Outcome != before.Outcome || m.results[0].Err != before.Err { - t.Errorf("a superseded slow round was painted: %+v", m.results[0]) - } - if !m.slowBusy { - t.Error("a stale slow round cleared the in-flight flag") + // The round issued before the action finished now answers, carrying the + // node as it was then. + before := fleet.NodeResult{Name: "a", Outcome: fleet.OutcomeOK, + Metrics: metrics.Stats{State: "stopped"}, At: issued} + next, _ = m.Update(dashRefreshMsg{remote: true, idx: []int{0}, results: []fleet.NodeResult{before}}) + m = next.(*dashModel) + if got := m.results[0].Metrics.State; got != "running" { + t.Errorf("the late round repainted the node's older report: state = %q, want running", got) } } @@ -1519,7 +1852,7 @@ func TestDashModelSlowRoundInFlightGuard(t *testing.T) { width: 120, height: 40, } m.slowBusy = true - m.nextSlowAt = time.Time{} // the deadline is due + m.scheduleRead(0, time.Time{}) // the node is due if cmd := m.refreshRemoteGroup(true); cmd != nil { t.Fatal("started a second slow round over one in flight") } @@ -1542,7 +1875,7 @@ func TestDashModelStartOutcomeWordings(t *testing.T) { width: 120, height: 40, } _, cmd := m.Update(dashKey("s")) - msgA, _ := cmd().(dashActionMsg) + msgA, _ := runAction(t, cmd).(dashActionMsg) next, _ := m.Update(msgA) m = next.(*dashModel) if m.statusLine != "a: start failed — boot exploded" { @@ -1558,7 +1891,7 @@ func TestDashModelStartOutcomeWordings(t *testing.T) { next, _ = m.Update(dashKey("right")) m = next.(*dashModel) _, cmd = m.Update(dashKey("s")) - msgB, _ := cmd().(dashActionMsg) + msgB, _ := runAction(t, cmd).(dashActionMsg) next, _ = m.Update(msgB) m = next.(*dashModel) if m.statusLine != "b: start — done" { @@ -1586,10 +1919,10 @@ func TestDashModelStartOnPlainNode(t *testing.T) { if m.actions[0].verb != "start" { t.Fatalf("no action recorded: %+v", m.actions[0]) } - if v := m.View(); !strings.Contains(v, "a starting") { + if v := m.View(); !strings.Contains(v, "a "+spinnerFrame(dashNow())+" starting") { t.Errorf("the in-flight tile does not carry the verb:\n%s", v) } - msg, _ := cmd().(dashActionMsg) + msg, _ := runAction(t, cmd).(dashActionMsg) next, _ = m.Update(msg) m = next.(*dashModel) if f.starts != 1 { @@ -1655,7 +1988,7 @@ func TestDashModelAbortsAnInFlightStart(t *testing.T) { } // The call's loop returns on the done context, and its final message lands // as for any finished action. - msg, _ := cmd().(dashActionMsg) + msg, _ := runAction(t, cmd).(dashActionMsg) if msg.err == nil { t.Fatalf("the aborted start came back as a success: %+v", msg) } @@ -1677,7 +2010,7 @@ func TestDashModelAbortsAnInFlightStart(t *testing.T) { t.Fatalf("the freed node did not take a second start: %+v", m.actions[0]) } close(hold) - msg2, _ := cmd2().(dashActionMsg) + msg2, _ := runAction(t, cmd2).(dashActionMsg) next, _ = m.Update(msg2) m = next.(*dashModel) if m.statusLine != "a: start — running" { @@ -1758,7 +2091,7 @@ func TestDashActionLineAbortedWording(t *testing.T) { } } -// A line for a node the board does not know is dropped, and a final for one +// A phase for a node the board does not know is dropped, and a final for one // still leaves its line on the footer without touching a real node. func TestDashModelUnknownNodeMessagesIgnored(t *testing.T) { node := newFakeDashNode("running") @@ -1768,10 +2101,11 @@ func TestDashModelUnknownNodeMessagesIgnored(t *testing.T) { actions: make([]dashAction, 1), width: 120, height: 40, } - next, _ := m.Update(dashActionProgressMsg{node: "ghost", line: "a line"}) + next, _ := m.Update(dashActionProgressMsg{node: "ghost", + phase: fleet.StartPhase{Kind: fleet.PhaseBooting, Since: time.Now()}}) m = next.(*dashModel) - if m.actions[0].line != "" { - t.Errorf("a stranger's line landed on the wrong tile: %+v", m.actions[0]) + if m.actions[0].phase != nil { + t.Errorf("a stranger's phase landed on the wrong tile: %+v", m.actions[0]) } next, _ = m.Update(dashActionMsg{node: "ghost", verb: "start", status: daemon.StatusResponse{State: "running"}}) m = next.(*dashModel) @@ -1846,7 +2180,7 @@ func TestDashProgramStartsAndStopsANode(t *testing.T) { tm.Type("s") // The call reports itself mid-flight — through the program's Send, onto // the tile — and stays there until it finishes. - seen("instance starting; retrying in 1s", 5*time.Second) + seen("booting", 5*time.Second) close(release) seen("alpha running", 5*time.Second) tm.Type("x") @@ -1978,7 +2312,7 @@ func TestDashModelDetailKeysDriveTheNodeInView(t *testing.T) { if cmd == nil || mm.actions[0].verb != "start" { t.Fatalf("s from the detail view did not start the node: %+v", mm.actions[0]) } - smsg, _ := cmd().(dashActionMsg) + smsg, _ := runAction(t, cmd).(dashActionMsg) m4, _ := mm.Update(smsg) mm = m4.(*dashModel) if !strings.Contains(mm.statusLine, "a: start — running") { @@ -1998,7 +2332,7 @@ func TestDashModelDetailKeysDriveTheNodeInView(t *testing.T) { if cmd2 == nil { t.Fatal("y did not stop from the detail view") } - pmsg, _ := cmd2().(dashActionMsg) + pmsg, _ := runAction(t, cmd2).(dashActionMsg) m7, _ := mm.Update(pmsg) mm = m7.(*dashModel) if !strings.Contains(mm.statusLine, "a: stop — stopped") { @@ -2125,6 +2459,44 @@ func TestDashFooterHints(t *testing.T) { } } +// Each entry in the key help is a key and what it does: the key keeps the +// terminal's own text colour, and what it does is drawn a step back in the +// muted ink, so the keys are what a glance picks out. The prompt's question +// and the status line beside them are prose and are left alone. +func TestDashKeyHintsDimWhatEachKeyDoes(t *testing.T) { + lipgloss.SetColorProfile(termenv.ANSI256) + const dim = "\x1b[38;5;109m" // the muted ink, in a 256-colour terminal + got := dashKeyHints("↑↓←→ move s start q quit") + want := "↑↓←→ " + dim + "move\x1b[0m s " + dim + "start\x1b[0m q " + dim + "quit\x1b[0m" + if got != want { + t.Errorf("dashKeyHints:\ngot: %q\nwant: %q", got, want) + } + // An entry with no meaning after its key is left as it is, rather than + // having its only word dimmed as though it were one. + if got := dashKeyHints("refreshing"); got != "refreshing" { + t.Errorf("a one-word entry was styled: %q", got) + } + if got := dashKeyHints(""); got != "" { + t.Errorf("dashKeyHints(\"\") = %q, want empty", got) + } + + // In the footer, only the key help is drawn this way. + m := dashModel{ + entries: []dashEntry{{name: "dev-1", kind: fleet.KindDaemon}}, + results: make([]fleet.NodeResult, 1), + actions: make([]dashAction, 1), + confirm: true, statusLine: "dev-1: start — running", + width: 120, height: 24, + } + footer := m.footerLine(120, "q quit") + if !strings.HasPrefix(footer, "stop dev-1? y "+dim+"yes") { + t.Errorf("the confirmation's keys are not drawn as pairs: %q", footer) + } + if !strings.HasSuffix(footer, " dev-1: start — running") { + t.Errorf("the status line was styled as a key pair: %q", footer) + } +} + // The footer only advertises abort while a start is actually in flight on // the node it describes — not for an idle or running node, and not for one // whose in-flight action is a stop, both of which would make the key a @@ -2646,18 +3018,20 @@ func TestDashDetailViewNoLogYet(t *testing.T) { } // An action in flight on the node in view replaces the metrics section with -// its verb and latest status line, the same wording the tile uses. +// its verb and its call's latest phase, the same wording the tile uses. func TestDashDetailViewActionInFlight(t *testing.T) { lipgloss.SetColorProfile(termenv.Ascii) m := dashModel{ entries: []dashEntry{{name: "n", kind: fleet.KindDaemon}}, results: []fleet.NodeResult{{Name: "n"}}, - actions: []dashAction{{verb: "start", line: "instance starting; retrying in 5s"}}, - detail: true, - width: 80, height: 24, + actions: []dashAction{{verb: "start", + phase: &fleet.StartPhase{Kind: fleet.PhaseBooting, Since: time.Now(), Detail: "starting"}}}, + detail: true, + width: 80, height: 24, } view := m.detailView() - if !strings.Contains(view, "n starting") || !strings.Contains(view, "instance starting; retrying in 5s") { + if !strings.Contains(view, "n "+spinnerFrame(dashNow())+" starting") || + !strings.Contains(view, "booting (") { t.Errorf("in-flight metrics section missing:\n%s", view) } } diff --git a/cmd/spinloop/palette.go b/cmd/spinloop/palette.go new file mode 100644 index 00000000..4467f25b --- /dev/null +++ b/cmd/spinloop/palette.go @@ -0,0 +1,62 @@ +// The colours and the spinner every surface of the CLI draws from — the +// dashboard's tiles and title bar, `fleet deploy`'s progress lines, the +// resource bars `fleet metrics` and `remote metrics` print. They live together +// so a second surface finds them rather than writing its own copy, which is +// what left the same ten spinner frames declared twice in this package. +// +// The two groups here are read differently and must not be swapped for one +// another. A state colour answers "how is this going" and belongs to whatever +// it is reporting; the brand colours answer "which tool is this" and belong to +// the tool's own chrome. See `openspec/specs/cli-ux/spec.md`. + +package main + +import "time" + +// The brand colours, taken from the spinloop logo: a near-black ground, a mint +// accent. They carry the same values as the site's `--accent`, `--ink` and +// `--ink-2` tokens, so the CLI and the site are recognisably one product. +// +// The accent says nothing about a node: the board's title bar and the border +// of the selected panel are drawn from it, and nothing else is. +// +// The inks are written as hex and downsampled by lipgloss to whatever the +// terminal can show. The surface is a 256-colour index instead, because a +// tile's header bar is raw ANSI — the tile body is one plain string under a +// single lipgloss style — while the board's title bar takes the same surface +// through lipgloss, so the two are written from one value. +const ( + brandAccent = "#1DE2AD" + brandInk = "#EEF3F2" + brandInkDim = "#9AA7AC" + barSurface = "235" +) + +// The state colours, as raw ANSI. They report what an engine or a call is +// doing — a resource bar's fill, a node's health mark, a deploy's outcome — +// and are the terminal's own green, amber and red rather than anything of the +// brand's. renderBar and the dashboard's health glyph both draw from these. +const ( + ansiGreen = "\033[92m" + ansiRed = "\033[31m" + ansiYellow = "\033[33m" + ansiGrey = "\033[90m" + ansiReset = "\033[0m" +) + +// spinnerFrames is the braille cycle drawn beside work still in flight, and +// spinnerStep how long each frame holds. One cycle for the whole tool, so a +// node deploying under `fleet deploy` and a node starting on the dashboard +// show the same thing. +var spinnerFrames = []rune("⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏") + +const spinnerStep = 125 * time.Millisecond + +// spinnerFrame is the frame showing at time now. It is a function of the time +// alone, so a surface that redraws simply asks again rather than holding a +// counter of its own. +func spinnerFrame(now time.Time) string { + n := int64(len(spinnerFrames)) + i := (now.UnixNano()/int64(spinnerStep))%n + n + return string(spinnerFrames[i%n]) +} diff --git a/cmd/spinloop/remote.go b/cmd/spinloop/remote.go index 0d9887bd..2a67dd63 100644 --- a/cmd/spinloop/remote.go +++ b/cmd/spinloop/remote.go @@ -21,6 +21,7 @@ import ( "github.com/spf13/cobra" "github.com/spinloop-ai/spinloop/internal/contextsize" + "github.com/spinloop-ai/spinloop/internal/fleet" "github.com/spinloop-ai/spinloop/internal/opencode" "github.com/spinloop-ai/spinloop/internal/preset" "github.com/spinloop-ai/spinloop/internal/remote" @@ -271,13 +272,16 @@ func spinloopArg(args []string) string { // so without this the command looks hung. A variable so tests can shorten it. var heartbeatEvery = 30 * time.Second -// startProgress reports what a slow start is doing. Everything it writes goes -// to stderr, so `spinloop remote start | grep '^export '` still yields just the -// exports while the user watching the terminal still sees progress. +// startProgress reports what a slow start is doing, as a renderer over the +// phases fleet.StartPhases builds from remote.Start's callbacks — the same +// phases the dashboard tile draws, so the two surfaces cannot word one +// situation differently. Everything it writes goes to stderr, so `spinloop +// remote start | grep '^export '` still yields just the exports while the user +// watching the terminal still sees progress. type startProgress struct { mu sync.Mutex since time.Time - state string // most recent state the endpoint reported; "" until the first poll + phase fleet.StartPhase // what the start is doing now; each report replaces it done chan struct{} stop sync.Once } @@ -299,26 +303,31 @@ func newStartProgress(every time.Duration) *startProgress { return p } -// setState records the state of the latest poll so the heartbeat can describe -// what is actually happening. Called from remote.Start on every poll. -func (p *startProgress) setState(state string) { +// report prints one phase as the start enters it, and keeps it for the +// heartbeat to redraw. Called from remote.Start's own goroutine. +func (p *startProgress) report(phase fleet.StartPhase) { p.mu.Lock() - p.state = state + p.phase = phase p.mu.Unlock() + p.line(fleet.RenderPhase(phase, time.Now())) } -// heartbeat is the periodic line. It reflects the latest state so it does not -// claim the instance is booting when it is really blocked on capacity. Any -// state other than no-capacity (including the unset state before the first -// poll) reads as a normal cold start. +// callbacks are the pair to hand remote.Start: the phases they build are what +// report renders. +func (p *startProgress) callbacks() (progress func(string), onState func(string)) { + return fleet.StartPhases(p.report) +} + +// heartbeat is the periodic line. A phase can hold for minutes — the attempt +// that obtains capacity keeps one request open for the whole boot and reports +// nothing while it does — so the line is the current phase redrawn at the +// current time, which counts a wait down and a boot up, plus how long the +// whole start has been running. func (p *startProgress) heartbeat() string { p.mu.Lock() - state := p.state + phase := p.phase p.mu.Unlock() - if state == "no-capacity" { - return fmt.Sprintf("still waiting for capacity (%s elapsed)", p.elapsed()) - } - return fmt.Sprintf("still starting (%s elapsed)", p.elapsed()) + return fmt.Sprintf("%s (%s elapsed)", fleet.RenderPhase(phase, time.Now()), p.elapsed()) } func (p *startProgress) elapsed() time.Duration { @@ -438,7 +447,8 @@ func runRemoteStart(args []string, timeout time.Duration, printEnv bool, keepD s ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() - resp, err := remote.Start(ctx, cfg, progress.line, progress.setState, retainUntil) + onProgress, onState := progress.callbacks() + resp, err := remote.Start(ctx, cfg, onProgress, onState, retainUntil) if err != nil { return err } @@ -660,7 +670,8 @@ func runRemoteRestart(args []string, force bool, timeout time.Duration) error { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() - resp, err := remote.Restart(ctx, cfg, force, progress.line, progress.setState) + onProgress, onState := progress.callbacks() + resp, err := remote.Restart(ctx, cfg, force, onProgress, onState) if err != nil { return err } diff --git a/cmd/spinloop/remote_deploy_test.go b/cmd/spinloop/remote_deploy_test.go index dbc6b1b7..8d4cb87a 100644 --- a/cmd/spinloop/remote_deploy_test.go +++ b/cmd/spinloop/remote_deploy_test.go @@ -762,76 +762,95 @@ func TestStartProgress_HeartbeatsAndStops(t *testing.T) { p.close() time.Sleep(40 * time.Millisecond) }) - if !strings.Contains(stderr, "still starting") { + // Before any poll has reported, the start is one attempt in flight. + if !strings.Contains(stderr, "waking the instance") { t.Errorf("expected a heartbeat while waiting, got:\n%s", stderr) } // Closing must stop it: 40ms of 10ms ticks after close would add ~4 more. - if got := strings.Count(stderr, "still starting"); got > 8 { + if got := strings.Count(stderr, "waking the instance"); got > 8 { t.Errorf("heartbeat kept running after close (%d lines):\n%s", got, stderr) } } // The heartbeat must describe what is really happening: while the endpoint // reports no capacity, nothing is booting, so it must not claim the instance is -// still starting. -func TestStartProgress_HeartbeatReflectsState(t *testing.T) { +// coming up. It renders the phase the start is in, so the wording is the tile's +// wording and the numbers in it move as the heartbeat repeats. +func TestStartProgress_HeartbeatReflectsPhase(t *testing.T) { + // driveStart feeds one start's callbacks the way remote.Start would. + driveStart := func(p *startProgress, do func(progress, onState func(string))) { + progress, onState := p.callbacks() + do(progress, onState) + } + t.Run("no-capacity says waiting for capacity", func(t *testing.T) { stderr := captureStderr(t, func() { p := newStartProgress(10 * time.Millisecond) - p.setState("no-capacity") + driveStart(p, func(progress, onState func(string)) { + onState("no-capacity") + progress("instance no-capacity; retrying in 120s") + }) time.Sleep(45 * time.Millisecond) p.close() }) if !strings.Contains(stderr, "waiting for capacity") { t.Errorf("expected a capacity-wait heartbeat, got:\n%s", stderr) } - if strings.Contains(stderr, "still starting") { - t.Errorf("must not claim it is starting while out of capacity, got:\n%s", stderr) + if !strings.Contains(stderr, "retrying in") { + t.Errorf("the wait does not say when the next attempt is due, got:\n%s", stderr) + } + if strings.Contains(stderr, "booting") { + t.Errorf("must not claim it is booting while out of capacity, got:\n%s", stderr) } }) - t.Run("a booting state says still starting", func(t *testing.T) { + t.Run("a booting state says booting", func(t *testing.T) { stderr := captureStderr(t, func() { p := newStartProgress(10 * time.Millisecond) - p.setState("starting") + driveStart(p, func(progress, onState func(string)) { onState("starting") }) time.Sleep(45 * time.Millisecond) p.close() }) - if !strings.Contains(stderr, "still starting") { - t.Errorf("expected a starting heartbeat while booting, got:\n%s", stderr) + if !strings.Contains(stderr, "booting") { + t.Errorf("expected a booting heartbeat, got:\n%s", stderr) } if strings.Contains(stderr, "waiting for capacity") { t.Errorf("a booting instance is not a capacity wait, got:\n%s", stderr) } }) - // The line tracks the latest poll: once capacity is found and the instance - // starts booting, the heartbeat must stop reporting a capacity wait. - t.Run("the latest state wins after a transition", func(t *testing.T) { + // The heartbeat tracks the latest phase: once capacity is found and the + // instance starts booting, it must stop reporting a capacity wait. + t.Run("the latest phase wins after a transition", func(t *testing.T) { p := newStartProgress(time.Hour) // no ticks; drive the line directly - p.setState("no-capacity") + progress, onState := p.callbacks() + onState("no-capacity") + progress("instance no-capacity; retrying in 120s") if got := p.heartbeat(); !strings.Contains(got, "waiting for capacity") { t.Errorf("after no-capacity, heartbeat = %q, want a capacity wait", got) } - p.setState("starting") - if got := p.heartbeat(); !strings.Contains(got, "still starting") { - t.Errorf("after booting, heartbeat = %q, want a starting line", got) + onState(remote.StateInFlight) + onState("starting") + if got := p.heartbeat(); !strings.Contains(got, "booting") { + t.Errorf("after booting, heartbeat = %q, want a booting line", got) } p.close() }) // The attempt that finds capacity reports no state until it is ready — its - // in-flight report is what tells the heartbeat the capacity wait is over. - t.Run("in-flight after a capacity wait says starting", func(t *testing.T) { + // in-flight report is what retires the capacity wait. + t.Run("in-flight after a capacity wait retires it", func(t *testing.T) { p := newStartProgress(time.Hour) // no ticks; drive the line directly - p.setState("no-capacity") + progress, onState := p.callbacks() + onState("no-capacity") + progress("instance no-capacity; retrying in 120s") if got := p.heartbeat(); !strings.Contains(got, "waiting for capacity") { t.Errorf("after no-capacity, heartbeat = %q, want a capacity wait", got) } - p.setState(remote.StateInFlight) + onState(remote.StateInFlight) got := p.heartbeat() - if !strings.Contains(got, "still starting") { - t.Errorf("in-flight after a capacity wait, heartbeat = %q, want a starting line", got) + if !strings.Contains(got, "waking the instance") { + t.Errorf("in-flight after a capacity wait, heartbeat = %q, want the attempt", got) } if strings.Contains(got, "waiting for capacity") { t.Errorf("an in-flight attempt supersedes the capacity wait, got:\n%q", got) @@ -842,10 +861,9 @@ func TestStartProgress_HeartbeatReflectsState(t *testing.T) { // The end-to-end shape of the capacity-wait bug: the first attempt is refused // for lack of capacity, the retry finds capacity and holds its request while -// the instance boots. The heartbeat must say it is waiting for capacity during -// the wait and switch to saying it is starting once the booting attempt is in -// flight — the refused attempt's report must not outlive the attempt it -// described. +// the instance boots. The output must say it is waiting for capacity during the +// wait and stop saying so once the booting attempt is in flight — the refused +// attempt's report must not outlive the attempt it described. func TestRemoteStart_HeartbeatTracksTheCapacityWaitEnding(t *testing.T) { isolateConfig(t) stubAWSEnv(t) @@ -899,20 +917,20 @@ func TestRemoteStart_HeartbeatTracksTheCapacityWaitEnding(t *testing.T) { }) lines := strings.Split(strings.TrimSpace(stderr), "\n") - lastWaiting, startingAfterWaiting := -1, false + lastWaiting, attemptAfterWaiting := -1, false for i, line := range lines { switch { case strings.Contains(line, "waiting for capacity"): lastWaiting = i - case lastWaiting != -1 && strings.Contains(line, "still starting"): - startingAfterWaiting = true + case lastWaiting != -1 && strings.Contains(line, "waking the instance"): + attemptAfterWaiting = true } } if lastWaiting == -1 { - t.Fatalf("no capacity-wait heartbeat in the output, so the wait was not observed:\n%s", stderr) + t.Fatalf("no capacity wait in the output, so the wait was not observed:\n%s", stderr) } - if !startingAfterWaiting { - t.Errorf("no 'still starting' heartbeat after the last capacity-wait line; the booting attempt still reads as a capacity wait:\n%s", stderr) + if !attemptAfterWaiting { + t.Errorf("nothing after the last capacity-wait line says a further attempt went out; the booting attempt still reads as a capacity wait:\n%s", stderr) } if calls != 2 { t.Errorf("expected the start to be attempted twice, got %d", calls) diff --git a/docs/internals.md b/docs/internals.md index 36a05a0b..a763672f 100644 --- a/docs/internals.md +++ b/docs/internals.md @@ -1,3 +1,4 @@ +- The colours and the spinner live in `palette.go`, in two groups that must not be swapped for one another. The brand colours (`brandAccent`, `brandInk`, `brandInkDim`) carry the same values as the web repo's `--accent`, `--ink` and `--ink-2` tokens, written as hex and downsampled by lipgloss, and the accent is used only where nothing about a node is being reported — the title bar's product name and the selected panel's border. The state colours (`ansiGreen` and friends) are raw ANSI and report what an engine is doing: the resource bars and the health glyph draw from them. `spinnerFrames` is one cycle for the whole tool, shared by `fleet deploy`'s progress lines and the dashboard's in-flight tiles. # Implementation notes This is maintainer reference, not a user guide (see [`docs/README.md`](README.md) @@ -30,9 +31,12 @@ These are mistakes already made here; each was silent rather than loud, which is A few Bubble Tea/lipgloss specifics that are easy to break by "simplifying": -- The `tea.Program` holds the model by **pointer**: Bubble Tea never reads a value model's `Init` back, so the first round's mutations (its generation, its deadline spend — real cloud calls, for remote environments) would be silently discarded. -- The tick reschedules itself whenever it fires; a one-shot `tea.Tick` without the reschedule leaves the board still after the second round. -- Answers from a fan-out round are tagged with the round's generation, so a superseded reply (from a slower node, or a since-closed detail view) is discarded rather than overwriting newer state. +- The `tea.Program` holds the model by **pointer**: Bubble Tea never reads a value model's `Init` back, so the first round's mutations (its deadline spend — real cloud calls, for remote environments) would be silently discarded. +- The tick reschedules itself whenever it fires; a one-shot `tea.Tick` without the reschedule leaves the board still after the second round. A second, faster chain (`dashSpinTickMsg`) runs only while an action is in flight, so the spinner and the elapsed time beside a verb advance; it stops on the first tick that finds nothing in flight. +- Every reading carries the time its own call returned (`fleet.NodeResult.At`, set in the fan-out), and the board draws a reading only when it was taken later than the one already on screen. Reads run concurrently and take differing times, so a reading can land after one taken later than it — including a round issued before an action finished and landing after it, which would otherwise repaint the node's pre-action state. +- A start reports a `fleet.StartPhase` — what it is doing, when that began, when the next attempt is due — rather than a line of text, and `fleet.RenderPhase(phase, now)` is the only place it becomes text. A wait therefore counts down and a boot counts up on every repaint, and a situation the start has moved on from cannot be left on the tile: each phase replaces the one before it. `spinloop remote start` renders the same phases to stderr, so the tile and the CLI cannot word one situation differently. +- One function, `dashNodeView`, produces both a panel's lines and its health tier, from the reading, the action, the current time, and how old a reading of that node may be. Nothing in it reads a clock, so every pairing of a start's phase against a reading can be enumerated in a test. +- A tile's first line is a header bar drawn in raw ANSI — the body is one plain string under a single lipgloss style, so per-character colour cannot be lipgloss's. The board's own title bar (`dashTitleBar`) uses lipgloss instead, and the two share one surface index (`barSurface`) because they are set through different mechanisms and would otherwise drift. - A grid row joins the *corresponding lines* of the tiles it places, not the tile blocks — joining whole blocks glues the second tile's top border to the first tile's bottom border and shifts its body down a line. - A tile's content is exactly the lines `fleet metrics` bar format prints (`renderStatBars`/`renderTokenLines` are shared, not reimplemented), so the panel and `fleet metrics` can never disagree on a number. diff --git a/internal/fleet/fanout.go b/internal/fleet/fanout.go index 0471aa5c..5039cba3 100644 --- a/internal/fleet/fanout.go +++ b/internal/fleet/fanout.go @@ -3,6 +3,7 @@ package fleet import ( "context" "sync" + "time" "github.com/spinloop-ai/spinloop/internal/daemon" ) @@ -47,6 +48,11 @@ func LogsCall(offsets map[string]int64, limit int) Call { // result per position, in the order given, so the rendering is stable between // refreshes. It never returns an error: a producer that fails is a typed // NodeResult, so one bad position is a row rather than a blanked view. +// +// Each result is stamped with the time its own call returned, not the time the +// round did: the calls run concurrently and take differing times, so a round's +// results are readings of different moments, and a display that orders them +// needs each one's own. func fanOutEach(producers []func() NodeResult) []NodeResult { results := make([]NodeResult, len(producers)) var wg sync.WaitGroup @@ -54,7 +60,9 @@ func fanOutEach(producers []func() NodeResult) []NodeResult { wg.Add(1) go func(i int) { defer wg.Done() - results[i] = produce() + r := produce() + r.At = time.Now() + results[i] = r }(i) } wg.Wait() diff --git a/internal/fleet/node.go b/internal/fleet/node.go index b31e4556..1be6af2a 100644 --- a/internal/fleet/node.go +++ b/internal/fleet/node.go @@ -5,6 +5,7 @@ import ( "errors" "net/http" "os" + "time" "github.com/spinloop-ai/spinloop/internal/daemon" "github.com/spinloop-ai/spinloop/internal/metrics" @@ -40,13 +41,17 @@ const ( ) // ProgressStarter is an optional node capability: a start whose call reports -// its situation as it works, one status line at a time. A node whose start is -// a single request and a single reply (a daemon that queues the work and +// its situation as it works, one phase at a time. A node whose start is a +// single request and a single reply (a daemon that queues the work and // returns) has no such situation and does not implement it. A caller that can // show progress (the dashboard) asserts for it and falls back to Start when // it is absent. +// +// report is called with each phase the start enters, and each call replaces +// the one before it: a start has one situation at a time, so the value a +// caller holds is the whole of what the start is doing. type ProgressStarter interface { - StartWithProgress(ctx context.Context, progress func(string)) (daemon.StatusResponse, error) + StartWithProgress(ctx context.Context, report func(StartPhase)) (daemon.StatusResponse, error) } // Node is one member of the fleet. Only daemonNode implements it today; the @@ -144,6 +149,14 @@ type NodeResult struct { Status daemon.StatusResponse Metrics metrics.Stats Logs daemon.LogsResponse + + // At is when this reading was taken — set by the fan-out as the call + // returns. Reads are concurrent and of uneven duration, so a reading can + // land after one taken later than it; a caller that draws the newest + // reading orders them by this rather than by arrival. It is also the + // reading's age, which is what tells a display an answer no longer + // describes the node now. Zero on a result built outside the fan-out. + At time.Time } // OK reports whether the node answered. diff --git a/internal/fleet/remote_node.go b/internal/fleet/remote_node.go index 6eb3d512..b20f395b 100644 --- a/internal/fleet/remote_node.go +++ b/internal/fleet/remote_node.go @@ -63,25 +63,22 @@ func (n *remoteNode) Metrics(ctx context.Context) (metrics.Stats, error) { } func (n *remoteNode) Start(ctx context.Context) (daemon.StatusResponse, error) { - return n.StartWithProgress(ctx, func(string) {}) + return n.StartWithProgress(ctx, func(StartPhase) {}) } -// StartWithProgress is Start carrying the control plane's own status lines up -// as the boot proceeds — the environment's state and the wait to the next -// poll, one line per retry, plus a line of its own each time a fresh attempt -// goes out. The caller shows or drops them. -func (n *remoteNode) StartWithProgress(ctx context.Context, progress func(string)) (daemon.StatusResponse, error) { - if progress == nil { - // remote.Start writes its lines unconditionally; a caller that wants - // none says so by passing nil rather than by being lucky about which - // paths the start happens to take. - progress = func(string) {} - } - onState := func(state string) { - if line := startStateLine(state); line != "" { - progress(line) - } +// StartWithProgress is Start with each phase of the boot passed to report as +// the start enters it: the attempt going out, a refusal for want of capacity +// and when the next attempt is due, the instance coming up, a dropped +// connection. Each phase replaces the one before it. Callers may render or +// discard them. +func (n *remoteNode) StartWithProgress(ctx context.Context, report func(StartPhase)) (daemon.StatusResponse, error) { + if report == nil { + // remote.Start invokes its callbacks on every retry path, so nil is + // substituted with a no-op here rather than left to whichever paths a + // given start takes. + report = func(StartPhase) {} } + progress, onState := StartPhases(report) resp, err := remote.Start(ctx, n.cfg, progress, onState, nil) if err != nil { return daemon.StatusResponse{}, err @@ -89,25 +86,6 @@ func (n *remoteNode) StartWithProgress(ctx context.Context, progress func(string return statusFromRemote(*resp), nil } -// startStateLine is the progress line a poll's state earns, or "" for the -// states that already have one. -// -// Only StateInFlight earns one, and it exists to retire a stale wait notice. -// remote.Start's own progress lines are written immediately before a wait -// ("instance no-capacity; retrying in 120s") and are true only until the next -// attempt goes out — but the attempt that finds capacity holds its single -// request for the whole boot, minutes, and writes nothing more. A caller that -// scrolls its lines is fine either way; a caller that shows the latest line as -// the node's current situation — the dashboard tile — would otherwise go on -// reporting a capacity wait long after the instance was up and serving. This -// is the client-side report StateInFlight was added for. -func startStateLine(state string) string { - if state == remote.StateInFlight { - return "waking the instance…" - } - return "" -} - // StartWith is how a router wakes a node to serve something. A remote environment // is not woken: what it serves is set by `spinloop remote deploy`, a heavier flow // (provisioning, weight seeding, ingress) that a node start must not conflate. diff --git a/internal/fleet/remote_node_test.go b/internal/fleet/remote_node_test.go index bdfbe4fc..9ffea62b 100644 --- a/internal/fleet/remote_node_test.go +++ b/internal/fleet/remote_node_test.go @@ -375,13 +375,13 @@ func (n *failingNode) Logs(context.Context, int64, int) (daemon.LogsResponse, er return daemon.LogsResponse{}, nil } -// StartWithProgress carries the control plane's own lines up to the caller: -// the retry during a boot, then the final verdict. +// StartWithProgress carries the boot up to the caller as phases: the attempt +// going out, then the instance coming up once a reply reports it. func TestRemoteNodeStartWithProgressReportsTheBoot(t *testing.T) { stubAWSCreds(t) var ( mu sync.Mutex - lines []string + phases []StartPhase attempts int ) mux := http.NewServeMux() @@ -409,9 +409,9 @@ func TestRemoteNodeStartWithProgressReportsTheBoot(t *testing.T) { if !ok { t.Fatal("the remote node does not carry progress") } - resp, err := starter.StartWithProgress(context.Background(), func(line string) { + resp, err := starter.StartWithProgress(context.Background(), func(p StartPhase) { mu.Lock() - lines = append(lines, line) + phases = append(phases, p) mu.Unlock() }) if err != nil { @@ -425,27 +425,35 @@ func TestRemoteNodeStartWithProgressReportsTheBoot(t *testing.T) { if attempts != 2 { t.Errorf("start attempts = %d (want 2)", attempts) } - // Each attempt announces itself, and the retry between them carries the - // control plane's own wording. - want := []string{"waking the instance…", "instance starting; retrying in 1s", "waking the instance…"} - if !slices.Equal(lines, want) { - t.Errorf("progress lines = %q (want %q)", lines, want) + // The attempt goes out, and the reply reporting the instance coming up + // replaces it. The second attempt is a poll of that same boot, so it does + // not take the phase back to an attempt. + want := []StartPhaseKind{PhaseAttempting, PhaseBooting} + got := make([]StartPhaseKind, len(phases)) + for i, p := range phases { + got[i] = p.Kind + } + if !slices.Equal(got, want) { + t.Errorf("phases = %v (want %v)", got, want) + } + if last := phases[len(phases)-1]; last.Detail != "starting" { + t.Errorf("the boot carries detail %q, want the control plane's state", last.Detail) } } -// The bug this guards: a start refused for capacity, then granted. The refusal -// writes "instance no-capacity; retrying in 120s" — true until the next -// attempt goes out, and the attempt that finds capacity then holds its single -// request for the whole boot without writing anything more. A caller that -// shows the latest line as the node's current situation (the dashboard tile) -// would go on reporting a capacity wait for the rest of the start, beside its -// own refreshes reporting the instance running. The last line a start reports -// must never be the retired capacity notice. +// The defect this covers: a start refused for capacity, then granted. The +// refusal describes the attempt it refused, and is accurate only until the next +// attempt is issued; the attempt that obtains capacity then holds a single +// request open for the duration of the boot and reports nothing further. A +// caller that draws the start's latest situation in a fixed place (the +// dashboard tile) would show a capacity wait for the remainder of the start, +// alongside its own refreshes reporting the instance running. The last phase a +// start reports must therefore not be the capacity wait. func TestRemoteNodeStartWithProgressRetiresACapacityWait(t *testing.T) { stubAWSCreds(t) var ( mu sync.Mutex - lines []string + phases []StartPhase attempts int ) mux := http.NewServeMux() @@ -470,31 +478,34 @@ func TestRemoteNodeStartWithProgressRetiresACapacityWait(t *testing.T) { t.Fatal(err) } starter := node.(ProgressStarter) - if _, err := starter.StartWithProgress(context.Background(), func(line string) { + if _, err := starter.StartWithProgress(context.Background(), func(p StartPhase) { mu.Lock() - lines = append(lines, line) + phases = append(phases, p) mu.Unlock() }); err != nil { t.Fatalf("StartWithProgress: %v", err) } mu.Lock() defer mu.Unlock() - if len(lines) == 0 { + if len(phases) == 0 { t.Fatal("the start reported nothing at all") } - // The wait was reported when it was true... - if !slices.ContainsFunc(lines, func(l string) bool { return strings.Contains(l, "no-capacity") }) { - t.Errorf("progress lines = %q (want the capacity refusal reported)", lines) + // The wait was reported when it was true, with the refusal's own delay... + waited := slices.IndexFunc(phases, func(p StartPhase) bool { + return p.Kind == PhaseWaitingCapacity && !p.RetryAt.IsZero() + }) + if waited < 0 { + t.Errorf("phases = %+v (want the capacity refusal reported with its due time)", phases) } // ...and retired by the attempt that superseded it. - if last := lines[len(lines)-1]; strings.Contains(last, "no-capacity") { - t.Errorf("the start's last line is the retired capacity wait: %q (all: %q)", last, lines) + if last := phases[len(phases)-1]; last.Kind == PhaseWaitingCapacity { + t.Errorf("the start's last phase is the retired capacity wait: %+v (all: %+v)", last, phases) } } -// A start that wants no progress lines says so with a nil callback, and is not -// relying on which paths the start happens to take: remote.Start writes its -// lines unconditionally, so every one of them has to land somewhere safe. +// A nil progress callback is valid. remote.Start invokes progress on every +// retry path, so StartWithProgress must substitute a no-op rather than depend +// on which paths a given start takes; this exercises a start that retries. func TestRemoteNodeStartWithProgressAcceptsNoReporter(t *testing.T) { stubAWSCreds(t) var attempts int diff --git a/internal/fleet/start_phase.go b/internal/fleet/start_phase.go new file mode 100644 index 00000000..697c190c --- /dev/null +++ b/internal/fleet/start_phase.go @@ -0,0 +1,242 @@ +// A start's situation as data, and the one place it becomes text. +// +// A start that waits — for capacity, for a boot, for a dropped connection — +// has one situation at a time, and it holds for minutes. A caller that appends +// a line per transition to a scrolling log reads correctly whichever way that +// is carried; a caller that draws the most recent line in a fixed place (the +// dashboard tile) draws whatever the last write left there, including a +// situation the start has moved on from. A StartPhase is replaced outright on +// each transition and carries times rather than rendered numbers, so what is +// drawn is computed from the phase and the current time on every repaint. + +package fleet + +import ( + "fmt" + "strings" + "time" + + "github.com/spinloop-ai/spinloop/internal/remote" +) + +// StartPhaseKind identifies what a start is currently doing. Exactly one value +// applies at a time. +type StartPhaseKind int + +const ( + // PhaseAttempting: a request has been sent and no reply has come back. + PhaseAttempting StartPhaseKind = iota + // PhaseWaitingCapacity: the control plane refused the attempt for want of + // capacity, and the next attempt is due at RetryAt. + PhaseWaitingCapacity + // PhaseBooting: the control plane reported the instance coming up. + PhaseBooting + // PhaseReconnecting: the attempt's connection dropped, and the next one is + // due at RetryAt. + PhaseReconnecting +) + +// StartPhase is a start's current situation. It carries no rendered text and no +// rendered number: Since and RetryAt are the inputs RenderPhase computes one +// from, so a phase that holds for minutes still draws a value that moves. +type StartPhase struct { + Kind StartPhaseKind + // Since is when this phase began. + Since time.Time + // RetryAt is when the next attempt is due; zero outside a wait, and zero + // during a wait whose reply named no delay. + RetryAt time.Time + // Detail is the control plane's state string, or a transport error. + Detail string +} + +// The two control-plane states the mapping reads by name. stateNoCapacity is +// an attempt refused because no zone had a GPU to give it — the one state that +// means the instance is not coming up at all. stateReady ends the start, and +// the caller reports its outcome rather than a phase. +const ( + stateNoCapacity = "no-capacity" + stateReady = "ready" +) + +// RenderPhase is the phase's line at time now. A wait counts down towards +// RetryAt and a boot counts up from Since, so nothing here is fixed when the +// phase is built. The dashboard tile and `spinloop remote start` both draw +// their line from this, so the two cannot word one phase differently. +func RenderPhase(p StartPhase, now time.Time) string { + switch p.Kind { + case PhaseWaitingCapacity: + return "waiting for capacity" + retryIn(p.RetryAt, now) + case PhaseBooting: + if p.Since.IsZero() { + return "booting" + } + return "booting (" + formatPhaseDuration(now.Sub(p.Since)) + ")" + case PhaseReconnecting: + line := "connection dropped" + if p.Detail != "" { + line += " (" + p.Detail + ")" + } + return line + retryIn(p.RetryAt, now) + default: + return "waking the instance…" + } +} + +// retryIn is the tail a waiting phase carries: the time left until the next +// attempt is due, "" where the phase records no due time, and "retrying now" +// once the due time has passed — the moment between the wait ending and the +// next attempt reporting itself. +func retryIn(retryAt, now time.Time) string { + if retryAt.IsZero() { + return "" + } + left := retryAt.Sub(now) + if left < time.Second { + return " — retrying now" + } + return " — retrying in " + formatPhaseDuration(left) +} + +// formatPhaseDuration renders a duration in whole seconds, in the shape the +// fleet surfaces already print an uptime in. +func formatPhaseDuration(d time.Duration) string { + if d < 0 { + d = 0 + } + secs := int(d / time.Second) + h, m, s := secs/3600, (secs/60)%60, secs%60 + switch { + case h > 0: + return fmt.Sprintf("%dh %dm %ds", h, m, s) + case m > 0: + return fmt.Sprintf("%dm %ds", m, s) + default: + return fmt.Sprintf("%ds", s) + } +} + +// StartPhases adapts remote.Start's progress and onState callbacks onto a +// stream of phases: it returns the pair to hand remote.Start, and calls report +// once per transition. The two callbacks are separate and neither carries a +// phase on its own — onState carries the state of a reply, and the progress +// line that follows a 503 carries that reply's retry-after — so the mapping +// holds the state between them. +// +// It is here rather than in either caller because both `spinloop remote start` +// and the dashboard drive remote.Start and render the result. +func StartPhases(report func(StartPhase)) (progress func(string), onState func(string)) { + t := &startPhases{report: report, now: time.Now} + return t.progress, t.state +} + +// startPhases holds the current phase between the two callbacks. +type startPhases struct { + report func(StartPhase) + now func() time.Time + phase StartPhase + begun bool +} + +// set replaces the phase and reports it. +func (t *startPhases) set(p StartPhase) { + t.phase = p + t.begun = true + t.report(p) +} + +// enter moves to a phase kind, or leaves the current phase alone when it is +// already that kind — so Since keeps counting from when the situation began +// rather than restarting on every poll that reports the same thing. +func (t *startPhases) enter(kind StartPhaseKind, detail string) { + if t.begun && t.phase.Kind == kind { + return + } + t.set(StartPhase{Kind: kind, Since: t.now(), Detail: detail}) +} + +// state maps one reply's state onto a phase. +func (t *startPhases) state(s string) { + switch { + case s == remote.StateInFlight: + // A fresh attempt supersedes a capacity wait and a dropped + // connection: each described the attempt before it. It does not + // supersede a boot — once a reply has reported the instance coming + // up, the attempts that follow are polls of that same boot, and its + // elapsed time counts from the first reply that reported it. + if t.phase.Kind != PhaseBooting || !t.begun { + t.enter(PhaseAttempting, "") + } + case s == stateReady: + // The reply that ends the start. Its outcome is the caller's to + // report; entering a phase here would draw a line for a situation + // that is over before it can be read. + case s == stateNoCapacity: + // The reply's retry-after reaches this caller only on the progress + // line remote.Start writes next, so the wait carries no due time + // until that line arrives. + t.enter(PhaseWaitingCapacity, s) + default: + t.enter(PhaseBooting, s) + } +} + +// droppedPrefix is how remote.Start opens the line it writes when an attempt's +// connection drops mid-request. +const droppedPrefix = "connection dropped" + +// progress maps one of remote.Start's status lines onto a phase. The lines are +// the only place the 503's retry-after and a transport error reach this +// caller; which state a retry line refers to came through onState immediately +// before it, so only the delay is read off the line itself. +func (t *startPhases) progress(line string) { + wait, hasWait := parseRetryIn(line) + if strings.HasPrefix(line, droppedPrefix) { + p := StartPhase{Kind: PhaseReconnecting, Since: t.now(), Detail: parenDetail(line)} + if hasWait { + p.RetryAt = p.Since.Add(wait) + } + t.set(p) + return + } + if hasWait && t.phase.Kind == PhaseWaitingCapacity { + t.phase.RetryAt = t.now().Add(wait) + t.report(t.phase) + } +} + +// retryInMarker precedes the delay in every line remote.Start writes before a +// wait. +const retryInMarker = "retrying in " + +// parseRetryIn reads the delay a status line names, in either of the forms +// remote.Start writes it — a whole number of seconds from the reply's +// retry-after, or a Go duration for the fixed wait after a dropped connection. +// A line naming no delay reports false, and the phase then carries no due time +// rather than a wrong one. +func parseRetryIn(line string) (time.Duration, bool) { + i := strings.Index(line, retryInMarker) + if i < 0 { + return 0, false + } + fields := strings.Fields(line[i+len(retryInMarker):]) + if len(fields) == 0 { + return 0, false + } + d, err := time.ParseDuration(strings.TrimRight(fields[0], ".")) + if err != nil || d < 0 { + return 0, false + } + return d, true +} + +// parenDetail is the parenthesised part of a status line — the transport error +// on a dropped connection — or "" when the line carries none. +func parenDetail(line string) string { + open := strings.Index(line, "(") + shut := strings.LastIndex(line, ")") + if open < 0 || shut < open { + return "" + } + return line[open+1 : shut] +} diff --git a/internal/fleet/start_phase_test.go b/internal/fleet/start_phase_test.go new file mode 100644 index 00000000..0f345509 --- /dev/null +++ b/internal/fleet/start_phase_test.go @@ -0,0 +1,134 @@ +package fleet + +import ( + "strings" + "testing" + "time" + + "github.com/spinloop-ai/spinloop/internal/remote" +) + +// Every line a phase renders is computed from the phase and the time it is +// drawn at, so a phase that holds while the clock moves draws a different line +// each time. The clock here is fixed and moved by hand: nothing in a phase is +// read from the wall clock, so the same phase at two times is the whole test. +func TestRenderPhaseIsComputedAtDrawTime(t *testing.T) { + base := time.Date(2026, 9, 3, 12, 0, 0, 0, time.UTC) + + // A capacity wait counts down towards the attempt it is waiting for. + wait := StartPhase{Kind: PhaseWaitingCapacity, Since: base, RetryAt: base.Add(120 * time.Second), Detail: "no-capacity"} + if got, want := RenderPhase(wait, base), "waiting for capacity — retrying in 2m 0s"; got != want { + t.Errorf("RenderPhase(wait, base) = %q, want %q", got, want) + } + if got, want := RenderPhase(wait, base.Add(73*time.Second)), "waiting for capacity — retrying in 47s"; got != want { + t.Errorf("the wait did not count down: %q, want %q", got, want) + } + // Past its due time, with the next attempt not yet reported. + if got, want := RenderPhase(wait, base.Add(130*time.Second)), "waiting for capacity — retrying now"; got != want { + t.Errorf("RenderPhase past the due time = %q, want %q", got, want) + } + // A refusal whose reply named no delay says what it is waiting for and + // nothing about when, rather than a wrong number. + noDue := StartPhase{Kind: PhaseWaitingCapacity, Since: base, Detail: "no-capacity"} + if got, want := RenderPhase(noDue, base.Add(time.Minute)), "waiting for capacity"; got != want { + t.Errorf("RenderPhase with no due time = %q, want %q", got, want) + } + + // A boot counts up from when it began. + boot := StartPhase{Kind: PhaseBooting, Since: base, Detail: "starting"} + if got, want := RenderPhase(boot, base.Add(4*time.Minute+12*time.Second)), "booting (4m 12s)"; got != want { + t.Errorf("RenderPhase(boot) = %q, want %q", got, want) + } + if got, want := RenderPhase(boot, base.Add(2*time.Hour)), "booting (2h 0m 0s)"; got != want { + t.Errorf("the boot did not count up: %q, want %q", got, want) + } + + // An attempt in flight, and a dropped connection carrying the transport + // error it dropped with. + if got, want := RenderPhase(StartPhase{Kind: PhaseAttempting, Since: base}, base), "waking the instance…"; got != want { + t.Errorf("RenderPhase(attempting) = %q, want %q", got, want) + } + dropped := StartPhase{Kind: PhaseReconnecting, Since: base, RetryAt: base.Add(5 * time.Second), Detail: "EOF"} + if got, want := RenderPhase(dropped, base.Add(time.Second)), "connection dropped (EOF) — retrying in 4s"; got != want { + t.Errorf("RenderPhase(reconnecting) = %q, want %q", got, want) + } +} + +// The mapping from remote.Start's two callbacks onto phases: an attempt goes +// out, is refused for capacity with a due time for the next one, and the +// attempt that follows retires the refusal rather than leaving it standing +// while the instance boots — the defect this phase stream exists for. +func TestStartPhasesRetiresACapacityWait(t *testing.T) { + var got []StartPhase + progress, onState := StartPhases(func(p StartPhase) { got = append(got, p) }) + + onState(remote.StateInFlight) + onState("no-capacity") + progress("instance no-capacity; retrying in 120s") + onState(remote.StateInFlight) + + kinds := make([]StartPhaseKind, len(got)) + for i, p := range got { + kinds[i] = p.Kind + } + want := []StartPhaseKind{PhaseAttempting, PhaseWaitingCapacity, PhaseWaitingCapacity, PhaseAttempting} + if len(kinds) != len(want) { + t.Fatalf("phases = %v, want %v", kinds, want) + } + for i := range want { + if kinds[i] != want[i] { + t.Fatalf("phases = %v, want %v", kinds, want) + } + } + // The retry-after reaches this caller only on the progress line, and it + // lands on the wait the state established. + due := got[2].RetryAt.Sub(got[2].Since) + if due < 119*time.Second || due > 121*time.Second { + t.Errorf("the capacity wait's due time is %s after it began, want about 2m", due) + } + if last := got[len(got)-1]; last.Kind == PhaseWaitingCapacity { + t.Error("the capacity wait outlived the attempt that superseded it") + } +} + +// A reply reporting the instance coming up enters the boot, and the polls that +// follow are that same boot: its elapsed time counts from the first reply that +// reported it, rather than restarting on each poll. +func TestStartPhasesHoldsTheBootAcrossPolls(t *testing.T) { + var got []StartPhase + progress, onState := StartPhases(func(p StartPhase) { got = append(got, p) }) + + onState(remote.StateInFlight) + onState("starting") + progress("instance starting; retrying in 5s") + onState(remote.StateInFlight) + onState("starting") + + if len(got) != 2 { + t.Fatalf("phases = %+v, want the attempt and the boot only", got) + } + if got[1].Kind != PhaseBooting || got[1].Detail != "starting" { + t.Errorf("second phase = %+v, want a boot reporting the control plane's state", got[1]) + } +} + +// A dropped connection is its own phase, carrying the transport error and the +// wait before the retry, and the attempt that follows retires it. +func TestStartPhasesReportsADroppedConnection(t *testing.T) { + var got []StartPhase + progress, onState := StartPhases(func(p StartPhase) { got = append(got, p) }) + + onState(remote.StateInFlight) + progress("connection dropped (unexpected EOF); retrying in 5s") + + last := got[len(got)-1] + if last.Kind != PhaseReconnecting { + t.Fatalf("last phase = %+v, want a reconnect", last) + } + if last.Detail != "unexpected EOF" { + t.Errorf("detail = %q, want the transport error", last.Detail) + } + if line := RenderPhase(last, last.Since.Add(time.Second)); !strings.Contains(line, "retrying in 4s") { + t.Errorf("rendered = %q, want the wait counting down", line) + } +} diff --git a/openspec/changes/archive/2026-09-04-cli-ux-conventions/.openspec.yaml b/openspec/changes/archive/2026-09-04-cli-ux-conventions/.openspec.yaml new file mode 100644 index 00000000..1d9aeef9 --- /dev/null +++ b/openspec/changes/archive/2026-09-04-cli-ux-conventions/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-04 diff --git a/openspec/changes/archive/2026-09-04-cli-ux-conventions/design.md b/openspec/changes/archive/2026-09-04-cli-ux-conventions/design.md new file mode 100644 index 00000000..2076a25c --- /dev/null +++ b/openspec/changes/archive/2026-09-04-cli-ux-conventions/design.md @@ -0,0 +1,134 @@ +## Context + +The conventions this records are already in the code. A survey of +`cmd/spinloop` before writing the spec found them held consistently, with two +duplications and one slip: + +| convention | where it is already held | +| --- | --- | +| machine-readable output on stdout, everything else on stderr | `remote start`'s exports against its progress; `fleet status`/`metrics` writing their tables and JSON to stdout | +| decoration only on a terminal | `fleet deploy`'s spinner and `fleet dashboard`'s refusal both check `term.IsTerminal` | +| errors lowercase, quoting the value, naming the command that fixes it | `main.go`'s alias and provider errors; `fleet.go`'s unknown-node error | +| help as a lowercase imperative phrase | every `Short:` in the command tree | +| confirmation with a `--yes` escape | `remote bootstrap` | +| one accent, used only for chrome | the dashboard's title bar and selected border | +| a long operation reporting a situation that moves | `remote start`'s phases, and the dashboard tile rendering the same ones | + +The duplications: the ten braille spinner frames are written out in +`fleet.go` and again in `dashboard_model.go`, in the same package. The slip: +one `Long:` description in `fleet.go` says "behavior". + +So the work is mostly writing the spec. The code changes are the two things +writing it down makes obviously wrong. + +## Findings from the survey + +Reviewing the drafted spec against `cmd/spinloop`, command by command: + +- **`fleet deploy` draws its spinner on stdout, not stderr.** The first draft + of the stdout requirement listed spinners among the things that go to stderr, + which would have made this a violation. It is not one: the spinner is drawn + only when stdout is a terminal, so a redirected or piped run never sees it, + and `fleet deploy` has no machine-readable stdout for it to corrupt. The + requirement was reworded to bite where it matters — a stdout carrying a + result a program consumes — and the rule about redrawing was left with the + requirement that already owns it. This is a correction to the spec, not a + finding against the code. +- **One American spelling.** `fleet deploy`'s long description says "behavior". + It is the only one in the package, and it is fixed here rather than recorded, + so the spec is not landed already violated. +- **Two definitions of the spinner.** `deploySpinnerFrames` in `fleet.go` and + `dashSpinnerFrames` in `dashboard_model.go` hold the same ten braille frames. + Consolidated by this change. +- **Everything else holds.** Every `Short:` in the command tree is a lowercase + imperative phrase with no trailing full stop, and so is every flag usage + string. Errors are lowercase, quote the operator's value with `%q`, and name + the command that fixes them. `remote start` keeps its exports on stdout and + its progress on stderr. `remote bootstrap` confirms before it creates + anything and takes `--yes`. The dashboard's accent is used only for its title + bar and its selection. + +## Goals / Non-Goals + +**Goals:** + +- A spec a reviewer can point at, and an author of a new command can read + instead of guessing from a neighbour. +- Record the brand accent's value in this repo, so it need not be looked up in + the web repo. +- Remove the duplicate spinner definition, and put the palette where a second + command would look for it. + +**Non-Goals:** + +- Changing any wording the CLI currently prints, beyond the one American + spelling. Where a command and the spec disagree, that is a finding for + someone to raise later. +- Restating what `remote-metrics-bar-format` and `fleet-client` already + specify. Those keep the resource bars' colour thresholds and the dashboard's + own behaviour; `cli-ux` states the general rules they are instances of. +- A shared internal package for terminal rendering. Everything here lives in + `package main` under `cmd/spinloop`, which is one package already. + +## Decisions + +### Decision 1: one capability, not one per surface + +`cli-ux` covers the whole tool — one-shot commands and the full-screen view +alike — because the conventions are what the two have in common. Splitting it +per surface would put "errors are lowercase" in one spec and the same sentence +in another, which is the situation this change exists to end. + +The full-screen affordances that have no one-shot equivalent (key help, +staleness) are a requirement within it rather than a separate capability, since +the rule is one sentence each and both are about the same tool. + +**Alternative rejected:** folding these into `fleet-client`. That spec is about +one command's behaviour; a rule for every command does not belong under it, and +a second command wanting the same rule would have to reference a spec it has +nothing else to do with. + +### Decision 2: the spec names the accent's value, not its source + +`#1DE2AD` is written into the spec, with a note that it is the value the site's +accent token carries. The alternative — "the accent is whatever the web repo's +token says" — would make this repo's spec unreadable without the other one to +hand, and would not be checkable here. + +The two are kept in step by being the same short hex value in two places, both +naming the logo they were sampled from. That is a copy, and copies drift; the +cost of the alternative is worse. + +### Decision 3: the accent is defined by what it may not do + +The requirement is not "use mint for highlights" but "use it only where nothing +about a node is being reported, and never use a state colour for chrome". The +prohibition is the useful half: the failure it prevents is a board where the +selected panel and a panel wanting attention are the same colour, which is what +the selection border was before this. + +### Decision 4: one spinner, in the file the palette moves to + +The frames and the brand colours are the two things every surface that draws +anything needs, so both go in one small file, and the two current definitions +of the frames become one. Nothing else moves. + +### Decision 5: British spelling is a requirement, and the one slip is fixed + +The rule is stated, and the single `Long:` description that says "behavior" is +corrected in the same change, so the spec is not written already violated. That +is the only wording this change touches. + +## Risks / Trade-offs + +- **A spec recording current practice can be read as blessing it.** Some of + what is written down is simply what was done first. The requirements are + worded as rules rather than as descriptions, so a later change that wants to + do differently has to argue with the rule rather than quietly diverge. +- **The accent's value is duplicated across two repos.** Accepted, with both + copies naming the logo as their source. A shared token file across two repos + with different build systems would cost more than the drift it prevents. +- **The spec is broad and the tests for it are indirect.** Most of these + requirements are already pinned by existing tests of the commands they apply + to; a few (help wording, British spelling) are conventions a reviewer checks. + Adding a linter for those is possible and is not in this change. diff --git a/openspec/changes/archive/2026-09-04-cli-ux-conventions/proposal.md b/openspec/changes/archive/2026-09-04-cli-ux-conventions/proposal.md new file mode 100644 index 00000000..1b4c7ec6 --- /dev/null +++ b/openspec/changes/archive/2026-09-04-cli-ux-conventions/proposal.md @@ -0,0 +1,69 @@ +## Why + +The CLI's conventions exist, are followed fairly consistently, and are written +down nowhere. What goes on stdout and what goes on stderr, how an error is +worded, when a spinner is drawn, which colour means what — each was decided +once and has since been copied from whatever nearby code the next command was +modelled on. That works while there is nearby code to copy; it fails the first +time someone adds a surface with no neighbour, and it gives a reviewer nothing +to point at. + +The web repo has a `design-language` spec for the site. This repo has no +counterpart, and cannot simply adopt that one: a CLI has affordances a web page +does not — a stdout that may be piped into another program, a terminal that may +not be a terminal, output that scrolls away rather than being re-read, a full +screen redrawn in place — and lacks most of what that spec governs (typefaces, +breakpoints, layout gaps, vector icons). + +The dashboard work that precedes this made the gap concrete: the brand accent +had to be looked up in another repo, and the same spinner is now defined twice +in one package because there was nothing saying there should be one. + +## What Changes + +- **A `cli-ux` capability records the conventions.** One accent colour and the + rule that it is only ever used where nothing about a node is being reported; + the split between what a program may parse and what a person reads; when + colour, spinners and in-place redraws are drawn at all; how an error is + worded; British spelling; what a long operation must keep saying; what a + destructive action must ask; and the affordances that belong to a full-screen + view. +- **The brand palette is stated in this repo.** The accent is the mint of the + spinloop logo, the same value the site's `--accent` token carries. The spec + records the value and, more importantly, records that the terminal's own + green, amber and red are not part of it — they report an engine's state and + are specified by `remote-metrics-bar-format`. +- **The spinner is defined once.** `deploySpinnerFrames` in `fleet.go` and + `dashSpinnerFrames` in `dashboard_model.go` are the same ten braille frames + written out twice, in one package. One definition replaces both. +- **The brand colours move to their own file.** They are currently in + `dashboard_render.go`, which is where they were first needed rather than + where a second command would look for them. + +No command's behaviour changes. The spec records what the CLI already does, and +the two code changes are consolidations that follow from writing it down. + +## Capabilities + +### New Capabilities + +- `cli-ux`: how the CLI and its full-screen views look, what they write where, + how they word what they say, and what they must keep saying while they work. + +### Modified Capabilities + +(None. `remote-metrics-bar-format` keeps the resource bars' colour thresholds +and `fleet-client` keeps the dashboard's own behaviour; `cli-ux` states the +general rules those two are instances of, and does not restate either.) + +## Impact + +- Spec: a new `openspec/specs/cli-ux/spec.md`. +- Code: `cmd/spinloop/fleet.go` and `cmd/spinloop/dashboard_model.go` share one + spinner definition; the brand colours move out of `dashboard_render.go` into + a file of their own. No behaviour changes, and the existing tests pin that. +- Docs: `AGENTS.md`'s spec pointers, and a note in `docs/internals.md` on where + the palette lives. +- Not in scope: changing any wording the CLI currently prints. Where the spec + and a command disagree, that is a finding to raise, not something this change + fixes. diff --git a/openspec/changes/archive/2026-09-04-cli-ux-conventions/specs/cli-ux/spec.md b/openspec/changes/archive/2026-09-04-cli-ux-conventions/specs/cli-ux/spec.md new file mode 100644 index 00000000..1c9063ed --- /dev/null +++ b/openspec/changes/archive/2026-09-04-cli-ux-conventions/specs/cli-ux/spec.md @@ -0,0 +1,239 @@ +## Purpose + +How the command-line tool and its full-screen views look, what they write to +which stream, how they word what they say, and what they must keep saying while +they work. It is what makes a command added later recognisable as part of the +same tool without its author having to read the others first. + +It is the counterpart to the web repo's `design-language` spec, not a copy of +it. A terminal has affordances a page does not — a stdout another program may +be parsing, an output stream that may not be a terminal at all, lines that +scroll away rather than staying to be re-read, a screen redrawn in place — and +has none of what that spec governs: typefaces, breakpoints, layout gaps, vector +icons. + +## ADDED Requirements + +### Requirement: One accent colour, and it never reports a state + +The tool SHALL use a single brand accent — the mint of the spinloop logo, +`#1DE2AD`, the value the site's accent token carries — defined once and derived +from that definition everywhere it is used. It SHALL be used only where nothing +about an engine, a node or an operation is being reported: the product's name, +the mark on the thing the operator has selected, and no more. + +Colours that report a state — the green, amber and red of a resource bar or a +health mark — SHALL NOT be used for the tool's own chrome, and the accent SHALL +NOT be used for a state. The two are read differently: a state colour answers +"how is this going", the accent answers "which tool is this", and a surface +that uses one for the other makes both unreadable. + +The tool SHALL be legible on a light terminal as well as a dark one. Text that +carries no meaning of its own SHALL be left in the terminal's own foreground +colour rather than set to a near-white or near-black of the tool's choosing. + +#### Scenario: The accent marks a selection, not a state + +- **WHEN** a full-screen view marks which item the operator has selected +- **THEN** it uses the accent, and no colour that reports what that item is + doing + +#### Scenario: A state keeps the terminal's own state colours + +- **WHEN** a resource bar, a health mark or an outcome is drawn +- **THEN** its colour is the green, amber or red that reports what it is + reporting, and never the brand accent + +#### Scenario: Changing the accent re-tints the tool + +- **WHEN** the accent's definition is changed +- **THEN** every accented surface changes with it, because no surface carries + its own copy of the value + +### Requirement: A stdout a program consumes carries nothing else + +A command whose stdout carries a machine-readable result — the exports an +`eval` consumes, a `--format=json` document — SHALL write that result to stdout +and everything else to stderr: progress, explanation and warnings. A command +whose whole output is a report for a person MAY write that report to stdout, +which is what stdout is for, and SHALL keep prompts and warnings off it. + +A command whose stdout is being consumed SHALL NOT be made to say less: what it +reports on stderr does not change with where its stdout goes. + +#### Scenario: A pipeline gets only what it can parse + +- **WHEN** a command that prints both progress and a machine-readable result is + piped into another program +- **THEN** only the result reaches that program, and the progress is still + shown to the person who ran it + +#### Scenario: A prompt does not land in a capture + +- **WHEN** a command asks the operator a question +- **THEN** the question is written to stderr, so a captured stdout holds the + command's output and not its conversation + +#### Scenario: A shell-completion request stays silent + +- **WHEN** the shell asks the tool for completions +- **THEN** the tool writes completions and nothing else, whatever the state of + its configuration + +### Requirement: Decoration is drawn only where it can be seen + +Colour, spinners, cursor movement and in-place redrawing SHALL be used only +when the stream being written to is a terminal. A run whose output is +redirected — to a file, to a log, to CI — SHALL get plain lines in the same +order, rather than a file full of escape codes. + +This is what keeps a spinner out of a capture, whichever stream it is drawn +on: a redirected stream is not a terminal, so nothing is drawn on it. + +A full-screen view SHALL refuse to start when it has no terminal to draw on, +and SHALL name the command that reports the same information into a pipe. + +#### Scenario: A redirected run gets plain lines + +- **WHEN** a command that draws a spinner has its output redirected to a file +- **THEN** the file holds the same information as plain lines, with no spinner + and no escape codes + +#### Scenario: A full-screen view refuses a pipe + +- **WHEN** a full-screen view is invoked with its output piped +- **THEN** it fails before drawing anything, and its error names the command + that carries the same information into a pipe + +### Requirement: A long operation keeps saying what it is doing + +An operation that can take longer than a few seconds SHALL report its current +situation, and SHALL replace that report outright at each transition rather +than adding to it: what is shown is what is happening now, never what was +happening before. + +Everything such a report says about time SHALL be computed when it is drawn or +written, not when the situation arose — a wait counts down towards what it is +waiting for, and elapsed time counts up. A situation that holds unchanged for +minutes is the normal case, so a moving value is what distinguishes an +operation that is waiting from one that has stopped making progress. Where the +surface redraws in place, an operation in flight SHALL also carry a spinner. + +The tool SHALL use one spinner, defined once, so every surface that shows work +in progress shows the same thing. + +#### Scenario: A wait counts down as it is watched + +- **WHEN** an operation is waiting for a retry and the operator watches without + pressing anything +- **THEN** the time until that retry counts down, and the time the operation + has been running counts up + +#### Scenario: A superseded situation is not left on screen + +- **WHEN** an operation moves on from one situation to another +- **THEN** what is shown is the new one, with no trace of the one it replaced + +### Requirement: An error names what to do about it + +An error SHALL be a lowercase phrase with no trailing full stop. It SHALL quote +the value the operator supplied, name the file, flag or environment variable +it concerns, and — where a command would fix it — give that command. + +An error SHALL be reported without a usage dump: the failure is what the +operator needs to read, and burying it under the command's full help hides it. + +#### Scenario: An unknown name says what the known ones are + +- **WHEN** the operator names something that does not exist +- **THEN** the error quotes what they typed, says where it was looked for, and + either lists what is there or names the command that would + +#### Scenario: A broken reference names its repair + +- **WHEN** a stored reference points at something that has gone +- **THEN** the error says what it points at and gives the command that would + re-point or remove it + +### Requirement: Help text is a lowercase imperative phrase + +A command's one-line description SHALL be a lowercase phrase in the +imperative, without a trailing full stop, naming what the command does rather +than what it is. A flag's usage string SHALL follow the same form. A command's +longer description SHALL add what the short one could not carry, and SHALL NOT +repeat it. + +#### Scenario: A new command reads like the others + +- **WHEN** a command is added +- **THEN** its description is a lowercase imperative phrase with no trailing + full stop, as its neighbours' are + +### Requirement: Copy is plain, specific and British + +Everything the tool writes — help, errors, progress, prompts and the text +inside a full-screen view — SHALL use ordinary English in British spelling. It +SHALL name the actual thing rather than describe it abstractly, and SHALL use +technical vocabulary only where it is more precise or more concise than plain +wording. + +Where the same fact is reported by more than one surface, those surfaces SHALL +word it from one place rather than each writing their own version, so two +screens cannot describe one situation differently. + +#### Scenario: British spelling throughout + +- **WHEN** any text the tool prints is written +- **THEN** it uses British spellings + +#### Scenario: Two surfaces cannot word one fact differently + +- **WHEN** the same fact is shown both in a full-screen view and by a one-shot + command +- **THEN** both draw their wording from the same place + +### Requirement: A destructive action asks first, and can be told not to + +An action that destroys or replaces something the operator cannot trivially +recreate SHALL ask for confirmation before it is sent, and SHALL offer a flag +that skips the question for an unattended run. The question SHALL be written to +stderr and SHALL default to not proceeding, so a bare newline or a closed input +declines. + +A declined or abandoned confirmation SHALL send nothing and SHALL say that +nothing was done. + +#### Scenario: A declined confirmation changes nothing + +- **WHEN** the operator is asked to confirm a destructive action and declines +- **THEN** nothing is sent, and the tool says so + +#### Scenario: An unattended run is not blocked by a prompt + +- **WHEN** a destructive action is run with the flag that skips confirmation +- **THEN** it proceeds without asking + +### Requirement: A full-screen view offers only keys that would do something + +A full-screen view SHALL name its keys on screen, and SHALL name a key only +where pressing it would do something in the context that key help describes. A +key that would do nothing for what is currently selected SHALL NOT be +advertised there. + +Such a view SHALL make plain how current what it is showing is: information +that has aged well past the rate it is refreshed at SHALL be shown with its age +rather than drawn identically to information just read, and SHALL NOT be +reported as the present state of anything. + +#### Scenario: The key help drops a key that would do nothing + +- **WHEN** the selected item has nothing for a given key to act on +- **THEN** the key help does not name that key +- **WHEN** the selection moves to an item that key does act on +- **THEN** the key help names it + +#### Scenario: Aged information says how old it is + +- **WHEN** what a panel shows has not been refreshed for well past its own + refresh rate +- **THEN** the panel shows how old it is and stops presenting it as current diff --git a/openspec/changes/archive/2026-09-04-cli-ux-conventions/tasks.md b/openspec/changes/archive/2026-09-04-cli-ux-conventions/tasks.md new file mode 100644 index 00000000..09a55b98 --- /dev/null +++ b/openspec/changes/archive/2026-09-04-cli-ux-conventions/tasks.md @@ -0,0 +1,16 @@ +## 1. The spec + +- [x] 1.1 Review the drafted `cli-ux` spec against `cmd/spinloop` command by command, and note any command that already contradicts it — the finding goes in the change, the fix does not +- [x] 1.2 Run `concord check` and `concord overlap` so no requirement here claims something `remote-metrics-bar-format` or `fleet-client` already owns + +## 2. The consolidations the spec makes obvious + +- [x] 2.1 Move the brand colours and the spinner frames out of `dashboard_render.go` and `dashboard_model.go` into one file of their own under `cmd/spinloop` +- [x] 2.2 Replace `deploySpinnerFrames` in `fleet.go` with that one definition, and check `fleet deploy`'s spinner still draws the same frames in the same order +- [x] 2.3 Correct the one American spelling in `fleet.go`'s `fleet deploy` long description + +## 3. Docs and checks + +- [x] 3.1 Add `cli-ux` to `AGENTS.md`'s spec pointers, and note in `docs/internals.md` where the palette and the spinner now live +- [x] 3.2 Run `gofmt`, `go vet ./...` and `go test ./... -cover`, keeping total coverage at or above 80% +- [x] 3.3 Run `openspec validate cli-ux-conventions --strict` diff --git a/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/.openspec.yaml b/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/.openspec.yaml new file mode 100644 index 00000000..9696e00f --- /dev/null +++ b/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-03 diff --git a/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/design.md b/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/design.md new file mode 100644 index 00000000..0dc1415e --- /dev/null +++ b/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/design.md @@ -0,0 +1,161 @@ +## Context + +Three sources describe a node, updated on three different schedules: + +| source | update rate | current representation | +| --- | --- | --- | +| the start call's progress | irregular — written only before a wait | `string`; each write replaces the previous, and the last one is retained indefinitely | +| the refresh round | 2s local, 60s remote | `fleet.NodeResult`, no timestamp | +| the operator's action | on keypress | `dashAction{verb, line, cancel, aborted, since}` | + +The tile concatenates the first two. No code compares them, and only the second +has any ordering control: per-group counters that order refresh rounds against +each other and against nothing else. + +## Decision 1: a start reports a phase, not a line + +```go +// StartPhaseKind identifies what a start is currently doing. Exactly one value +// applies at a time, and each new value replaces the previous one. +type StartPhaseKind int + +const ( + PhaseAttempting StartPhaseKind = iota // a request has been sent, no reply yet + PhaseWaitingCapacity // refused for want of capacity, waiting to retry + PhaseBooting // accepted; the instance is starting + PhaseReconnecting // the request's connection dropped, waiting to retry +) + +// StartPhase holds a start's current situation as data rather than text. The +// display layer formats it, so the dashboard and the CLI render the same phase +// identically. +type StartPhase struct { + Kind StartPhaseKind + Since time.Time // when this phase began + RetryAt time.Time // when the next attempt is due; zero except while waiting + Detail string // the control plane's state string, or a transport error +} +``` + +`ProgressStarter` becomes: + +```go +type ProgressStarter interface { + StartWithProgress(ctx context.Context, report func(StartPhase)) (daemon.StatusResponse, error) +} +``` + +A value rather than a more carefully written string, because the defect was not +the wording of the line: a `string` field records no expiry. A phase holds one +value per start and each write replaces the previous one, so a superseded +situation is not retained. Achieving the same result with a string requires +every write site to overwrite at every transition. + +`remoteNode` maps `remote.Start`'s existing callbacks onto phases: +`onState(StateInFlight)` → `PhaseAttempting`; `onState("no-capacity")` plus the +503's `RetryAfterSeconds` → `PhaseWaitingCapacity` with `RetryAt` set; +`onState` with any other state on a held request → `PhaseBooting`; the +`connection dropped` progress line → `PhaseReconnecting`. `internal/remote` is +unchanged. + +**Alternative rejected:** keep `func(string)` and write a line at every +transition. That is what the quick fix does. It is correct only while every +write site covers every transition, and an omission produces no error and no +test failure — the output is a well-formed line carrying an out-of-date value. + +## Decision 2: the rendered text is a function of the phase and the current time + +```go +func RenderPhase(p StartPhase, now time.Time) string +``` + +`PhaseWaitingCapacity` renders `waiting for capacity — retrying in 47s`, +recomputed on each repaint from `RetryAt.Sub(now)`. `PhaseBooting` renders +`booting (4m 12s)` from `now.Sub(p.Since)`. The board already repaints on its +2-second tick, so this needs no additional timer. + +No number is stored, so no number can be left at a stale value. The elapsed +counter added by the quick fix applies the same approach to the verb, and is +replaced by this. + +## Decision 3: record when each reading was taken, and never go backwards + +`fleet.NodeResult` gains `At time.Time`, set in `FanOutNodes` as each read +returns. The dashboard then applies one rule: + +> Display a reading only if it was taken later than the one currently displayed +> for that node. + +This replaces the `fastGen`/`slowGen` counters and covers a case they cannot: +the counters order refresh rounds relative to each other, but not relative to an +action completing. Concretely — a refresh round is issued at T0 during a start, +the start completes at T3 and the tile is cleared to the node's post-start +report, the round returns at T5 carrying the node's state as of T0, and the +current code displays it. + +`At` also supplies the reading's age, which Decision 4 requires. + +**Alternative rejected:** a per-node counter. It orders readings equally well, +but carries no age, so displaying a reading's age would require a second field +alongside it. + +## Decision 4: an old reading is displayed with its age + +A node whose newest reading is older than three times its kind's interval is +drawn with its age (`· 3m ago`) and assigned the unknown health tier. Three +intervals rather than one, so an ordinary late round does not flicker the board +grey. + +This generalises the reported defect. The problem was not that the board +displayed an old reading; it was that an old reading was rendered identically to +a current one. With the age displayed, an operator can distinguish the two in +cases not anticipated here. + +## Decision 5: read a node more often while an action is in flight + +A node with an action in flight is read on `dashboardRefreshInterval` regardless +of kind, and returns to its kind's interval once the action completes. + +A remote node is read once a minute because each read is a signed Lambda call +and an idle environment's state changes slowly. Neither applies to an +environment the operator has just started. The additional call volume is +proportional to the number of nodes with an action in flight. + +## Decision 6: one function produces a tile's contents + +```go +func dashNodeView(p *StartPhase, reading Reading, now time.Time) (lines []string, tier dashHealthTier) +``` + +Every input the tile depends on is an argument. No clock is read and no field is +taken from the model, so the same arguments always produce the same output. This +merges the existing `dashNodeContentLines` and `dashHealthTierFor` switches, +which must already evaluate their conditions in the same order and currently +depend on a comment in each to keep them aligned. + +The reason to merge them is testability. One function gives a single place to +enumerate every phase against every state a reading can be in — no reading yet, +a fresh answer, a stale answer, a failed round — which is twenty cases, and +includes `PhaseWaitingCapacity` alongside a reading reporting the node running. +That is the combination that produced this defect, and the current code has no +place to test it, because the two descriptions of the node are combined only +inside a `strings.Builder`. + +## Risks + +- **The interface changes.** `ProgressStarter` has one implementation + (`remoteNode`) and two call sites (the dashboard, and `n.Start`), so the + change is small, but the CLI's `startProgress` is rewritten in the same change + to render the same phases. +- **The CLI's output changes.** `spinloop remote start`'s stderr lines are + reworded. Nothing parses them (the eval-able `export` lines go to stdout and + are unchanged), but the change is visible to users and some CLI tests assert + on them. +- **The staleness threshold is a guess.** Three intervals is chosen to avoid + flicker. If slow cloud rounds routinely exceed it, the board will grey out + more often than intended, and the multiplier is the value to adjust. + +## Migration + +No configuration, no on-disk state, no API changes. The change is internal, plus +the text rendered on two screens. diff --git a/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/proposal.md b/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/proposal.md new file mode 100644 index 00000000..adc1a349 --- /dev/null +++ b/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/proposal.md @@ -0,0 +1,123 @@ +## Why + +The dashboard maintains two separate descriptions of a node — the status lines a +start writes as it runs, and the last completed refresh round — and no code +compares them for recency or resolves a contradiction between them. The tile +renders both as though each were current. + +The reported symptom: a remote start refused for capacity, then granted, left the +tile displaying + +``` +dev-1 starting +instance no-capacity; retrying in 120s +running (up 2m 58s) +``` + +for the remainder of the boot — a capacity wait and a running instance on one +tile, rendered identically. That line was corrected in `03718af` by passing +`remote.Start`'s `onState` callback through to the tile, and an elapsed counter +was added so that a line which stays constant is still visibly updating. The +structure that allowed the defect is unchanged, and it allows more than one: + +- **A log line is rendered as current state.** `dashAction.line` is a `string` + with no timestamp and no expiry. A line written before a wait ("retrying in + 120s") is accurate in a scrolling terminal, where it is one entry in a + sequence, and inaccurate on a tile that redraws in place, where it is the only + text shown. Keeping it accurate requires every write site to overwrite it at + every transition; `remoteNode.StartWithProgress` did not, which produced this + defect. +- **No reading records when it was taken.** `fleet.NodeResult` carries no + timestamp, so a 60-second-old `running (up 58s)` is drawn identically to one + read a moment ago. It also means a refresh round issued before an action + completes and returned after it will overwrite the post-action report with + pre-action data: `dashRefreshMsg` checks only a per-group counter, which does + not order a round against an action completing. +- **The read interval does not vary with what the operator is doing.** A remote + node stays on its 60-second interval throughout a start — the period in which + its state changes most and is being watched. +- **No single function produces a tile's contents.** `dashNodeContentLines` + concatenates the action's lines and the refresh report through a switch over + several fields. No function takes every input the tile depends on as + arguments, so the combinations in which the two descriptions contradict each + other cannot be enumerated in a test. + +## What Changes + +- **A start reports a phase, not a line.** `fleet.ProgressStarter` carries a + `StartPhase` value — what the start is doing, when that began, and when the + next attempt is due — in place of `func(string)`. Each transition replaces the + value, and nothing accumulates, so a superseded situation is not retained. + `remoteNode` builds phases from the `progress` and `onState` callbacks + `remote.Start` already provides. +- **Wait text is computed at draw time.** The rendered text is a pure function of + the phase and the current time, so "retrying in 120s" counts down and "waiting + for capacity" counts up, rather than remaining at the value held when the line + was written. +- **Both screens render the same phases.** `cmd/spinloop`'s `startProgress` (the + CLI's stderr output) formats the same values, so the dashboard and `spinloop + remote start` cannot render the same phase differently — the same reason the + tile and the detail view already share `dashNodeContentLines`. +- **Every reading records when it was taken.** `fleet.NodeResult` gains the time + of its read, and the dashboard applies one rule throughout: never display a + reading older than the one currently displayed. This replaces the existing + counters and corrects the case where a slow round returns late and overwrites + newer data. +- **An old reading is no longer rendered as current.** A node whose last answer + is older than a few of its own intervals displays its age and is assigned the + unknown health tier, rather than presenting an out-of-date state as current. +- **A node with an action in flight is read more often.** It refreshes on the + short interval until the action completes, then returns to its kind's + interval. The additional call volume is limited to nodes the operator is + acting on. +- **One function produces a tile's contents.** It takes the phase, the last + reading, that reading's age and the current time, and returns the tile's lines + and its health tier. Every input is an argument, so every combination can be + enumerated in one test — including the contradictory ones the current code + cannot test. + +## Capabilities + +### New Capabilities + +(None) + +### Modified Capabilities + +- `fleet-client`: the "The dashboard drives the selected node" requirement's + in-flight clause is restated in terms of a start's phase rather than its + status lines, with the countdown and the elapsed time; "The fleet refreshes + without stalling" gains the rule that an older reading never replaces a newer + one, and the shorter interval for a node with an action in flight; "Dashboard + panels show a health indicator" adds an out-of-date reading as a reason to + show unknown. + +### Unchanged + +The keys, the grid, the detail view, the abort behaviour, the confirmation on +stop, and the layout of a tile with no action in flight. + +## Impact + +- Code: `internal/fleet/node.go` (`ProgressStarter` signature, `StartPhase`, + `NodeResult.At`), `internal/fleet/remote_node.go` (building phases), + `internal/fleet/fanout.go` (timestamping results as each read returns), + `cmd/spinloop/dashboard_model.go` (displaying only newer readings, the shorter + interval during an action, and the single function producing a tile's + contents), `cmd/spinloop/dashboard_render.go` (rendering a phase), + `cmd/spinloop/remote.go` (`startProgress` rendering the same phases). +- `internal/remote` is unchanged: `Start`'s `progress` and `onState` callbacks + already supply everything the phases are built from, `StateInFlight` included. +- Tests: one test enumerating every phase against every state a reading can be + in, tests that an older reading never replaces a newer one, and a test that a + remote node is read more often while an action is in flight. +- Docs: `AGENTS.md`'s dashboard entry, and `docs/internals.md` if the phase + contract needs a note there. + +## Non-goals + +- Changing what the control plane reports, or adding states to it. +- Any change to `fleet metrics --watch`, the one-shot commands, or the daemon + API. +- Cancelling a start's work. An abort still ends only the dashboard's wait, as + currently specified. diff --git a/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/specs/fleet-client/spec.md b/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/specs/fleet-client/spec.md new file mode 100644 index 00000000..4cbefa3c --- /dev/null +++ b/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/specs/fleet-client/spec.md @@ -0,0 +1,473 @@ +## MODIFIED Requirements + +### Requirement: The dashboard drives the selected node + +The dashboard SHALL let the operator start and stop the engine of the node +currently selected, from the keyboard, through the same node operations the one- +shot `fleet start` and `fleet stop` commands use. An action SHALL reach the +node under the cursor and no other. An action is one per node, not one per +board: while one node is starting, the operator selects another and starts it, +and the wakes run side by side. A node that already has an action in flight +SHALL take no further start or stop; the dashboard says it is still working +and drives nothing. + +Start SHALL proceed without confirmation. Stop SHALL require an explicit +confirmation before it is sent, because it ends an engine that may be serving +work: a declined or abandoned confirmation SHALL send nothing. + +While an action is in flight, the node's own tile SHALL carry it: the verb, the +action's current situation, and how long the action has been running, beside the +node's last report rather than in place of it — the action's own account says +what the operator asked for, the report says what the node is doing. For a node +whose last completed refresh answered, the tile SHALL show that answer's state, +what it serves, its last-active record, and its resource usage and token and +request counters whenever the answer carries them, whatever the node's state — a +boot half done is already measuring, and that is the truth the tile keeps showing +while the call works. A node whose action reports nothing and whose refresh has +not yet answered SHALL show the verb alone, and a latest refresh that failed +SHALL change nothing on the tile. Each finished action SHALL clear the action +from its node's tile, which is then the node's report alone, and leave its +outcome on the status line, the one-shot wording. + +An action SHALL have exactly one current situation at a time, and a new one +SHALL replace its predecessor outright rather than being added to it. A +situation that was true of an attempt the action has moved on from SHALL NOT be +shown: the tile reports what the action is doing now, never what it was doing +before. In particular, a start refused for want of capacity SHALL stop reporting +that refusal once a further attempt is under way — so a tile never shows a +capacity wait beside a refresh reporting the node up and running. + +Anything the tile says about time SHALL be computed when the tile is drawn, not +when the situation it describes arose: a wait counts down towards the attempt it +is waiting for, and an action counts up from when the operator issued it. A +start's situation can hold unchanged for minutes — the attempt that succeeds +keeps one request open for the whole boot and reports nothing while it does — so +these are what distinguish an action that is waiting from one that is wedged. + +An action in flight SHALL also carry a spinner beside its verb, whose frame is +likewise chosen when the tile is drawn, and the board SHALL redraw often enough +for it to turn. It says the same thing as the elapsed time to an operator +glancing rather than reading, and it says it on a tile whose every other line +can hold still for minutes. + +The outcome of an action SHALL be shown inside the dashboard (a status line the +operator can read before the next refresh replaces attention), and a refused or +unreachable action SHALL NOT close the dashboard. What an action changes in state +— an engine coming up after a start, coming down after a stop — SHALL appear +through the normal refresh, without the operator asking for it. + +A start in flight SHALL be abortable from the keyboard, on the node under the +cursor. A start carries no deadline — a cold cloud wake takes minutes, and a +deadline would report a failure to a slow success — so the operator's abort is +its exit: an abort SHALL end the dashboard's wait on the start, return the +node's tile to the node's report, free the node to be started or stopped again, +and show its outcome on the status line, without closing the dashboard. An abort +ends the wait, not the work it set in motion: the dashboard SHALL NOT present it +as a cancellation of the wake, and what the wake goes on to do — the node's +state — SHALL appear through the normal refresh. + +A stop in flight SHALL NOT be abortable: it targets an engine that is already +running rather than a cold wake with no deadline of its own, and abandoning +the dashboard's wait on it would leave the operator unsure whether the stop +still went ahead. The abort key SHALL drive nothing on a node whose in-flight +action is a stop. + +The dashboard's key help SHALL name the abort key only while a start is in +flight on the node it describes — the node under the cursor on the grid, the +node in view in the detail view — and SHALL NOT name it for an idle or +running node, or one whose in-flight action is a stop, so the operator is +never invited to press a key that would do nothing there. + +#### Scenario: Starting a cold node + +- **WHEN** the operator selects a node with no engine running and issues the + start +- **THEN** the start is sent without a prompt, its outcome is shown in the + status line, and the panel shows the node's new state as the refreshes come + around + +#### Scenario: Stopping asks first + +- **WHEN** the operator issues the stop on the selected node +- **THEN** the dashboard asks for confirmation and nothing is sent +- **WHEN** the operator declines +- **THEN** the stop is not sent and the node's state is unchanged + +#### Scenario: A confirmed stop is sent + +- **WHEN** the operator issues the stop and confirms +- **THEN** the stop is sent to the selected node only, its outcome is shown in + the status line, and the panel follows the node's state on subsequent refreshes + +#### Scenario: A start is watched on its own tile + +- **WHEN** the operator starts a node whose start reports its situation as it + works +- **THEN** the node's tile shows the verb and the start's current situation + while the start is in flight +- **AND** a refresh that answers while the start is in flight shows its state + and whatever it measures on the same tile, beneath the start's own account +- **AND** when the start finishes, the tile is the node's report alone and the + outcome is on the status line + +#### Scenario: A refused start does not outlive its refusal + +- **WHEN** a node's start is refused for want of capacity, and a later attempt + is issued once capacity is free +- **THEN** the tile reports the capacity wait while it holds, and stops + reporting it once the next attempt is under way, rather than showing it + beside a refresh that reports the node running + +#### Scenario: A start's elapsed time keeps moving + +- **WHEN** a start is in flight and its situation does not change for some time +- **THEN** the elapsed time beside the verb keeps advancing on the tile, so the + operator can tell the start is still waiting rather than wedged + +#### Scenario: A wait counts down and an action counts up + +- **WHEN** a start is waiting for its next attempt and the operator watches + without pressing anything +- **THEN** the time until that attempt counts down on the tile, and the time + since the operator issued the start counts up, both advancing as the tile is + redrawn rather than standing at the values they held when the wait began +- **AND** the spinner beside the verb turns while they do + +#### Scenario: An in-flight start before any report + +- **WHEN** the operator starts a node before any refresh of it has answered +- **THEN** its tile shows the verb and the start's own account alone +- **AND** the first answered refresh appears on the tile beside them + +#### Scenario: Two nodes wake at once + +- **WHEN** the operator starts one node, selects another, and starts it +- **THEN** both wakes run at the same time, each reported on its own tile +- **AND** each finishing action clears its own node and leaves its outcome on + the status line without disturbing the other + +#### Scenario: A node still starting is not started again + +- **WHEN** the operator presses start on a node whose start is in flight +- **THEN** the dashboard drives nothing and says the node is still starting + +#### Scenario: An in-flight start can be abandoned + +- **WHEN** the operator's start on a node is still in flight — the cloud waiting + for capacity, the boot slow, or the connection behind it dropping and + retrying — and the operator issues the abort on that node +- **THEN** the dashboard stops waiting on the start, the node's tile returns to + the node's report, and the outcome is shown on the status line +- **AND** the node may be started or stopped again, and the dashboard keeps + running with its refreshes + +#### Scenario: An abort is not a cancellation of the wake + +- **WHEN** the operator aborts a start whose wake the cloud is already carrying + on +- **THEN** the dashboard reports that it stopped waiting, not that the wake was + cancelled +- **AND** the node's state, whatever the wake goes on to do, appears through the + normal refresh + +#### Scenario: A stop in flight cannot be aborted + +- **WHEN** the operator issues the abort on a node whose stop is in flight +- **THEN** the dashboard drives nothing: the stop keeps running, the tile keeps + showing it as stopping, and the node's state comes back through the normal + refresh + +#### Scenario: The key help hides abort when nothing is abortable + +- **WHEN** the node under the cursor (or shown in the detail view) is idle, + running with nothing in flight, or has a stop in flight +- **THEN** the key help does not name the abort key +- **WHEN** that node has a start in flight +- **THEN** the key help names the abort key + +#### Scenario: An action that fails keeps the dashboard open + +- **WHEN** an action is sent to a node that cannot be reached, or the node + refuses it +- **THEN** the failure is shown in the status line with the daemon's own reason + and the dashboard keeps running with its refreshes + +### Requirement: The fleet refreshes without stalling + +The dashboard SHALL refresh the fleet continuously, and SHALL also refresh +immediately on the operator's request. The cadence SHALL be by node kind: a +local daemon machine SHALL refresh on a short interval — seconds, not the +watch mode's minute — and a `kind: remote` environment SHALL refresh on a +much slower cadence, a 60-second interval, one status call a minute, because +its status is a signed call through the cloud control plane rather than a +local socket, and its state changes on the scale of minutes. A manual refresh +SHALL read every node, whether or not its kind's cadence was due. A refresh of a +group SHALL read that group's nodes concurrently, as fan-out does elsewhere. + +A node with an action in flight SHALL refresh on the short interval whatever its +kind, and SHALL return to its kind's own cadence once the action settles. The +reasons a cloud environment is read once a minute — the call is expensive and +its state changes slowly — hold for an environment nobody is touching and hold +for neither one the operator has just started. + +Every reading of a node SHALL carry the time it was taken, and the dashboard +SHALL NOT show a reading older than the one it is already showing for that node. +Reads are concurrent and of uneven duration, so a reading can land after one +taken later than it; showing it would replace what the node is doing with what +it was doing. + +A node whose newest reading has aged well past its cadence SHALL be shown as +such — its age on the panel, and its health unknown rather than whatever the +aged reading said. A stale reading is not a wrong reading, but presenting it +indistinguishably from a current one is: the operator SHALL be able to tell how +old what they are looking at is. + +One node that is slow or unanswerable SHALL NOT hold the rest of the fleet +hostage to it: a refresh SHALL give up on a node that has not answered within +the budget of the node's group, show that node with its outcome for this +round, and the other nodes SHALL keep their cadence. The dashboard SHALL NOT +start a second read of a group while that group's previous one has not +finished; the two groups' reads MAY run at the same time, and a slow cloud +round SHALL NOT stretch the local machines' cadence. + +#### Scenario: Continuous refresh without input + +- **WHEN** the dashboard is open and no key is pressed +- **THEN** the local panels keep updating on the short interval, and each + cloud panel is re-read once a minute, on the 60-second cadence + +#### Scenario: An immediate refresh + +- **WHEN** the operator presses the refresh key +- **THEN** every node is re-read, local and cloud alike, rather than waiting + for each kind's next cadence + +#### Scenario: A node being acted on is read more often + +- **WHEN** the operator starts a cloud environment +- **THEN** that environment is re-read on the short interval for as long as the + start is in flight, and returns to its 60-second cadence once the start + settles +- **AND** the other cloud environments keep their own cadence throughout + +#### Scenario: A late reading does not overwrite a newer one + +- **WHEN** a read of a node is issued, a later read of the same node answers + first, and the earlier read then answers +- **THEN** the panel keeps showing the later read, and the earlier one is + discarded + +#### Scenario: A panel with an out-of-date reading shows its age + +- **WHEN** a node stops answering and its newest reading ages well past its + cadence +- **THEN** its panel shows how old that reading is and its health reads + unknown, rather than continuing to present the aged state as current +- **WHEN** the node answers again +- **THEN** the panel returns to showing its state without an age + +#### Scenario: One hung node does not stall the board + +- **WHEN** one node does not answer within its group's budget while the others + do +- **THEN** that node's panel shows its outcome, the other panels refreshed on + their normal cadence, and no second read of its group started over the + in-flight one + +#### Scenario: A slow cloud round keeps the local cadence + +- **WHEN** a cloud environment is slow to answer while its group's read runs +- **THEN** the local machines keep refreshing on the short interval, and the + cloud panel shows its outcome on the cloud cadence + +### Requirement: Dashboard panels show a health indicator + +Each panel SHALL show a coloured status glyph alongside its node's name, +distinct from the border colour that marks the selected panel, so a node's +health reads at a glance across a grid of many panels without reading each +panel's text. The glyph SHALL be shown in every panel shape: a settled +answer, an action in flight, a panel awaiting its first refresh, and a panel +showing a failed outcome. + +A tile SHALL draw its first line — the glyph, the node's name, and what the +node is doing — as a header bar: light text on one background colour running +the tile's full width, so the name reads as the panel's title rather than as +another line of the panel's body. That background SHALL be a single neutral +colour, the same on every tile, so it never competes with the glyph's own +colour for what a node's health is read from. + +A node's health SHALL fall into exactly one of five tiers: healthy, +attention, unhealthy, not serving, and unknown. Healthy is coloured green, +attention yellow, and unhealthy red. Not serving and unknown are both +coloured grey and read as "nothing to watch" rather than "something is +wrong"; the two stay apart by their marks — not serving is the same filled +dot as the other tiers in a faded shade, and unknown keeps its `?` — so a +node known to be undeployed never reads as a node the dashboard has not +heard from yet. + +- **Healthy**: the node answered its last refresh, that answer is current, its + engine is not crashed, not `idle`, not `stopped`, and not `undeployed`, and — + when the daemon reports readiness for it — the engine is ready. A `running` + node whose daemon reports no readiness (an older daemon, or a runner with no + known health check) counts as healthy too, rather than reporting a health tier + the daemon cannot actually back. +- **Attention**: the node has a start or stop action in flight for it, or is + `running` with its daemon explicitly reporting the engine not yet ready. +- **Unhealthy**: the node's engine has crashed, or its last refresh's + outcome was a failure (`unreachable`, `unauthorized`, `config-error`, + `failed`, or `unsupported`). +- **Not serving**: the node answered its last refresh, that answer is current, + no action is in flight for it, and its engine is not serving — its state is + `idle`, the daemon has started nothing, `stopped`, a daemon engine that was + stopped, or `undeployed`, a remote environment with no instance at all. +- **Unknown**: no current status can be determined for the node — it has not yet + answered any refresh, its last refresh answered without reporting an engine + state, or its newest answer has aged well past its cadence and no longer + describes the node now. + +#### Scenario: A running, ready node reads healthy + +- **WHEN** a node's last completed refresh reports its engine `running` and + its daemon reports the engine ready +- **THEN** its panel's status glyph is green + +#### Scenario: A running node still loading reads attention + +- **WHEN** a node's last completed refresh reports its engine `running` and + its daemon reports the engine not yet ready +- **THEN** its panel's status glyph is yellow, even though its engine state + reads `running` + +#### Scenario: A running node with no readiness signal reads healthy + +- **WHEN** a node's last completed refresh reports its engine `running` and + its daemon reports no readiness for it +- **THEN** its panel's status glyph is green, the same as before this + daemon-side signal existed + +#### Scenario: A crashed node reads unhealthy + +- **WHEN** a node's last completed refresh reports its engine `crashed` +- **THEN** its panel's status glyph is red + +#### Scenario: An unreachable node reads unhealthy + +- **WHEN** a node's last refresh could not reach its daemon +- **THEN** its panel's status glyph is red, alongside the outcome and reason + already shown + +#### Scenario: A node awaiting its first refresh reads unknown + +- **WHEN** the dashboard opens and a node has not yet answered any refresh +- **THEN** its panel's status glyph is grey, until its first refresh lands + +#### Scenario: An answer without a state reads unknown + +- **WHEN** a node's last refresh answered but reported no engine state +- **THEN** its panel's status glyph is grey + +#### Scenario: A node whose answer has gone stale reads unknown + +- **WHEN** a node last answered `running` and has not answered since, long + enough that the answer no longer describes the node now +- **THEN** its panel's status glyph is grey rather than the green that answer + earned when it was current + +#### Scenario: An action in flight reads attention + +- **WHEN** the operator starts or stops a node and that action has not yet + finished +- **THEN** that node's panel's status glyph is yellow while the action is in + flight, whatever the node's last completed refresh reported + +#### Scenario: An undeployed node reads not serving + +- **WHEN** a node's last completed refresh reports its engine `idle` and no + start or stop is in flight for it +- **THEN** its panel's status glyph is the faded grey dot, not the green of + a serving node + +#### Scenario: A stopped node reads not serving + +- **WHEN** a node's last completed refresh reports its engine `stopped` — a + daemon engine that was stopped — and no start or stop is in flight for it +- **THEN** its panel's status glyph is the faded grey dot, not the green of + a serving node + +#### Scenario: An undeployed remote environment reads not serving + +- **WHEN** a remote environment's last completed refresh reports it + `undeployed` — it has no instance at all — and no start or stop is in + flight for it +- **THEN** its panel's status glyph is the faded grey dot, not the green of + a serving node + +#### Scenario: An undeployed node with a start in flight reads attention + +- **WHEN** a node's last completed refresh reports its engine `idle` or + `stopped` and a start for it has not yet finished +- **THEN** its panel's status glyph is yellow while the start is in flight, + never the grey of not serving + +#### Scenario: Not serving and unknown keep different marks + +- **WHEN** a grid holds both a node that has answered with `idle` and a node + that has not yet answered any refresh +- **THEN** the first shows the faded grey filled dot and the second the grey + `?`, so the two greys are told apart by their mark + +The board SHALL carry a title bar of its own along the top of the frame, on +the same surface a tile's header bar uses: the product's name, the screen in +view, and — pushed to the right — the fleet file and what is on that screen. A +terminal too narrow for both halves SHALL keep the left one rather than wrap +the bar onto a second row. The grid and the detail view SHALL draw their title +bar from one place, so the two screens cannot title themselves differently. + +The board SHALL use exactly one accent colour, the mint of the spinloop logo, +and SHALL use it only where nothing about a node is being reported: the title +bar's product name, and the border of the selected panel. A node's own state — +the health glyph, the resource bars — SHALL keep the terminal's green, amber +and red, which say what an engine is doing rather than whose product this is. + +In the frame's key help, each entry SHALL be drawn as the key and what that +key does: the key in the terminal's own text colour, and what it does a step +back in the muted ink, so the keys are what a glance over the footer picks out. +The prose beside them — a confirmation's question, an action's outcome — SHALL +be left as it is. + +#### Scenario: A key stands out from what it does + +- **WHEN** the key help is drawn +- **THEN** each key keeps the terminal's own text colour and what it does is + drawn in the muted ink beside it +- **AND** the status line and a confirmation's question are not drawn that way + +#### Scenario: The selected panel is marked in the accent + +- **WHEN** the operator moves the selection onto a panel +- **THEN** that panel's border is drawn in the brand accent, and no colour that + reports a node's state is used for it + +#### Scenario: The title bar names the product and the screen + +- **WHEN** the dashboard is open on the grid or on a node's detail view +- **THEN** the top of the frame carries a title bar naming the product and that + screen, with the fleet file and the screen's own details to the right + +#### Scenario: A narrow terminal keeps the left half of the title bar + +- **WHEN** the terminal is too narrow for both halves of the title bar +- **THEN** the product and the screen are kept and the right half is dropped, + and the bar stays one row + +#### Scenario: The header bar does not carry the health colour + +- **WHEN** two nodes of different health are shown side by side +- **THEN** both tiles' header bars are the same colour, and the two nodes are + told apart by their glyphs + +#### Scenario: The glyph is distinct from the selection border + +- **WHEN** the operator moves the selection onto a panel +- **THEN** the selected panel's border colour changes as it does today, and + every panel's status glyph colour is unaffected by which panel is selected diff --git a/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/tasks.md b/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/tasks.md new file mode 100644 index 00000000..ef7b48a0 --- /dev/null +++ b/openspec/changes/archive/2026-09-04-dashboard-start-status-rebuild/tasks.md @@ -0,0 +1,44 @@ +## 1. The start phase + +- [x] 1.1 Add `StartPhaseKind`, `StartPhase` and `RenderPhase(p, now)` to `internal/fleet`, with `RenderPhase` a pure function of the phase and the time +- [x] 1.2 Change `ProgressStarter` to `StartWithProgress(ctx, report func(StartPhase))`, and update `remoteNode.Start` to pass a no-op reporter +- [x] 1.3 Map `remote.Start`'s `progress`/`onState` pair onto phases in `remoteNode.StartWithProgress`: `StateInFlight` → attempting, `no-capacity` + the 503's retry-after → waiting-for-capacity with its due time, a held request → booting, a dropped connection → reconnecting. Leave `internal/remote` untouched +- [x] 1.4 Unit-test `RenderPhase` against a fixed clock: the capacity wait counts down, the boot counts up, and no rendering carries a number that was fixed when the phase was built + +## 2. Recording when each reading was taken + +- [x] 2.1 Add `At time.Time` to `fleet.NodeResult` and set it in `internal/fleet/fanout.go` as each read returns +- [x] 2.2 Replace the dashboard's `fastGen`/`slowGen` drop rule with a simpler one: show a reading only when it was taken later than the one on screen +- [x] 2.3 Test the case the counters miss: a round that starts before an action finishes and lands after it does not repaint the node's older report + +## 3. Old readings, and how often a node is read + +- [x] 3.1 Draw a node's age on its tile once its newest reading is older than three of its kind's intervals, and drop it to the unknown health tier +- [x] 3.2 Read a node with an action in flight on the short interval whatever its kind, returning to its kind's cadence once the action settles +- [x] 3.3 Test that a remote node is read more often for the duration of a start and returns to its own cadence afterwards, and that a node with an old reading greys out and recovers when it answers again + +## 4. One function produces a tile's contents + +- [x] 4.1 Merge `dashNodeContentLines` and `dashHealthTierFor` into one function taking the phase, the reading, how old it is and the current time, returning the tile's lines and its health colour +- [x] 4.2 Write one test listing every phase against every state a reading can be in — no reading yet, fresh answer, stale answer, failed round — including a start waiting for capacity next to a reading that says the node is running, which is the case that caused the original bug +- [x] 4.3 Keep the settled (no action in flight) tile byte-for-byte unchanged, pinned by the existing tile tests. Superseded during implementation: the operator asked for a full-width coloured header row on every tile, which changes the settled tile's first line. The existing tile tests were updated to that header and still pin every tile byte for byte; the rest of a settled tile is unchanged. + +## 5. Shared wording + +- [x] 5.1 Rewrite `cmd/spinloop`'s `startProgress` as a renderer over the same phase stream, dropping its own state field and heartbeat wording +- [x] 5.2 Update the CLI tests that pin `remote start`'s stderr lines, and confirm nothing on the eval-able stdout path changes + +## 6. Docs and checks + +- [x] 6.1 Update the dashboard pointers in `AGENTS.md`, and note the phase contract in `docs/internals.md` if it is not obvious from the code +- [x] 6.2 Run `gofmt`, `go vet ./...` and `go test ./... -cover`, keeping total coverage at or above 80% + +## 7. The tile's own appearance + +Asked for during implementation, and specified alongside the rest. + +- [x] 7.1 Draw a braille spinner beside an in-flight verb, its frame chosen when the tile is drawn, and repaint the board on a fast tick while any action is in flight +- [x] 7.2 Draw a tile's first line as a header bar: light text on one neutral background across the tile's full width, with the health glyph keeping its own colour on top, so the bar never competes with the glyph for what a node's health is read from +- [x] 7.3 Give the board a title bar of its own on the same surface, shared by the grid and the detail view: the product in the brand accent, the screen beside it, the fleet file and the screen's details to the right +- [x] 7.4 Draw the selected panel's border in the brand accent rather than the amber it shared with the attention health tier, and keep the accent off anything that reports a node's state +- [x] 7.5 Draw each key-help entry as a key and what it does: the key in the terminal's own text colour, its meaning in the muted ink, leaving the status line and a confirmation's question as prose diff --git a/openspec/specs/cli-ux/spec.md b/openspec/specs/cli-ux/spec.md new file mode 100644 index 00000000..188d30d5 --- /dev/null +++ b/openspec/specs/cli-ux/spec.md @@ -0,0 +1,239 @@ +# cli-ux Specification + +## Purpose +How the command-line tool and its full-screen views look, what they write to +which stream, how they word what they say, and what they must keep saying while +they work. It is what makes a command added later recognisable as part of the +same tool without its author having to read the others first. + +It is the counterpart to the web repo's `design-language` spec, not a copy of +it. A terminal has affordances a page does not — a stdout another program may +be parsing, an output stream that may not be a terminal at all, lines that +scroll away rather than staying to be re-read, a screen redrawn in place — and +has none of what that spec governs: typefaces, breakpoints, layout gaps, vector +icons. +## Requirements +### Requirement: One accent colour, and it never reports a state + +The tool SHALL use a single brand accent — the mint of the spinloop logo, +`#1DE2AD`, the value the site's accent token carries — defined once and derived +from that definition everywhere it is used. It SHALL be used only where nothing +about an engine, a node or an operation is being reported: the product's name, +the mark on the thing the operator has selected, and no more. + +Colours that report a state — the green, amber and red of a resource bar or a +health mark — SHALL NOT be used for the tool's own chrome, and the accent SHALL +NOT be used for a state. The two are read differently: a state colour answers +"how is this going", the accent answers "which tool is this", and a surface +that uses one for the other makes both unreadable. + +The tool SHALL be legible on a light terminal as well as a dark one. Text that +carries no meaning of its own SHALL be left in the terminal's own foreground +colour rather than set to a near-white or near-black of the tool's choosing. + +#### Scenario: The accent marks a selection, not a state + +- **WHEN** a full-screen view marks which item the operator has selected +- **THEN** it uses the accent, and no colour that reports what that item is + doing + +#### Scenario: A state keeps the terminal's own state colours + +- **WHEN** a resource bar, a health mark or an outcome is drawn +- **THEN** its colour is the green, amber or red that reports what it is + reporting, and never the brand accent + +#### Scenario: Changing the accent re-tints the tool + +- **WHEN** the accent's definition is changed +- **THEN** every accented surface changes with it, because no surface carries + its own copy of the value + +### Requirement: A stdout a program consumes carries nothing else + +A command whose stdout carries a machine-readable result — the exports an +`eval` consumes, a `--format=json` document — SHALL write that result to stdout +and everything else to stderr: progress, explanation and warnings. A command +whose whole output is a report for a person MAY write that report to stdout, +which is what stdout is for, and SHALL keep prompts and warnings off it. + +A command whose stdout is being consumed SHALL NOT be made to say less: what it +reports on stderr does not change with where its stdout goes. + +#### Scenario: A pipeline gets only what it can parse + +- **WHEN** a command that prints both progress and a machine-readable result is + piped into another program +- **THEN** only the result reaches that program, and the progress is still + shown to the person who ran it + +#### Scenario: A prompt does not land in a capture + +- **WHEN** a command asks the operator a question +- **THEN** the question is written to stderr, so a captured stdout holds the + command's output and not its conversation + +#### Scenario: A shell-completion request stays silent + +- **WHEN** the shell asks the tool for completions +- **THEN** the tool writes completions and nothing else, whatever the state of + its configuration + +### Requirement: Decoration is drawn only where it can be seen + +Colour, spinners, cursor movement and in-place redrawing SHALL be used only +when the stream being written to is a terminal. A run whose output is +redirected — to a file, to a log, to CI — SHALL get plain lines in the same +order, rather than a file full of escape codes. + +This is what keeps a spinner out of a capture, whichever stream it is drawn +on: a redirected stream is not a terminal, so nothing is drawn on it. + +A full-screen view SHALL refuse to start when it has no terminal to draw on, +and SHALL name the command that reports the same information into a pipe. + +#### Scenario: A redirected run gets plain lines + +- **WHEN** a command that draws a spinner has its output redirected to a file +- **THEN** the file holds the same information as plain lines, with no spinner + and no escape codes + +#### Scenario: A full-screen view refuses a pipe + +- **WHEN** a full-screen view is invoked with its output piped +- **THEN** it fails before drawing anything, and its error names the command + that carries the same information into a pipe + +### Requirement: A long operation keeps saying what it is doing + +An operation that can take longer than a few seconds SHALL report its current +situation, and SHALL replace that report outright at each transition rather +than adding to it: what is shown is what is happening now, never what was +happening before. + +Everything such a report says about time SHALL be computed when it is drawn or +written, not when the situation arose — a wait counts down towards what it is +waiting for, and elapsed time counts up. A situation that holds unchanged for +minutes is the normal case, so a moving value is what distinguishes an +operation that is waiting from one that has stopped making progress. Where the +surface redraws in place, an operation in flight SHALL also carry a spinner. + +The tool SHALL use one spinner, defined once, so every surface that shows work +in progress shows the same thing. + +#### Scenario: A wait counts down as it is watched + +- **WHEN** an operation is waiting for a retry and the operator watches without + pressing anything +- **THEN** the time until that retry counts down, and the time the operation + has been running counts up + +#### Scenario: A superseded situation is not left on screen + +- **WHEN** an operation moves on from one situation to another +- **THEN** what is shown is the new one, with no trace of the one it replaced + +### Requirement: An error names what to do about it + +An error SHALL be a lowercase phrase with no trailing full stop. It SHALL quote +the value the operator supplied, name the file, flag or environment variable +it concerns, and — where a command would fix it — give that command. + +An error SHALL be reported without a usage dump: the failure is what the +operator needs to read, and burying it under the command's full help hides it. + +#### Scenario: An unknown name says what the known ones are + +- **WHEN** the operator names something that does not exist +- **THEN** the error quotes what they typed, says where it was looked for, and + either lists what is there or names the command that would + +#### Scenario: A broken reference names its repair + +- **WHEN** a stored reference points at something that has gone +- **THEN** the error says what it points at and gives the command that would + re-point or remove it + +### Requirement: Help text is a lowercase imperative phrase + +A command's one-line description SHALL be a lowercase phrase in the +imperative, without a trailing full stop, naming what the command does rather +than what it is. A flag's usage string SHALL follow the same form. A command's +longer description SHALL add what the short one could not carry, and SHALL NOT +repeat it. + +#### Scenario: A new command reads like the others + +- **WHEN** a command is added +- **THEN** its description is a lowercase imperative phrase with no trailing + full stop, as its neighbours' are + +### Requirement: Copy is plain, specific and British + +Everything the tool writes — help, errors, progress, prompts and the text +inside a full-screen view — SHALL use ordinary English in British spelling. It +SHALL name the actual thing rather than describe it abstractly, and SHALL use +technical vocabulary only where it is more precise or more concise than plain +wording. + +Where the same fact is reported by more than one surface, those surfaces SHALL +word it from one place rather than each writing their own version, so two +screens cannot describe one situation differently. + +#### Scenario: British spelling throughout + +- **WHEN** any text the tool prints is written +- **THEN** it uses British spellings + +#### Scenario: Two surfaces cannot word one fact differently + +- **WHEN** the same fact is shown both in a full-screen view and by a one-shot + command +- **THEN** both draw their wording from the same place + +### Requirement: A destructive action asks first, and can be told not to + +An action that destroys or replaces something the operator cannot trivially +recreate SHALL ask for confirmation before it is sent, and SHALL offer a flag +that skips the question for an unattended run. The question SHALL be written to +stderr and SHALL default to not proceeding, so a bare newline or a closed input +declines. + +A declined or abandoned confirmation SHALL send nothing and SHALL say that +nothing was done. + +#### Scenario: A declined confirmation changes nothing + +- **WHEN** the operator is asked to confirm a destructive action and declines +- **THEN** nothing is sent, and the tool says so + +#### Scenario: An unattended run is not blocked by a prompt + +- **WHEN** a destructive action is run with the flag that skips confirmation +- **THEN** it proceeds without asking + +### Requirement: A full-screen view offers only keys that would do something + +A full-screen view SHALL name its keys on screen, and SHALL name a key only +where pressing it would do something in the context that key help describes. A +key that would do nothing for what is currently selected SHALL NOT be +advertised there. + +Such a view SHALL make plain how current what it is showing is: information +that has aged well past the rate it is refreshed at SHALL be shown with its age +rather than drawn identically to information just read, and SHALL NOT be +reported as the present state of anything. + +#### Scenario: The key help drops a key that would do nothing + +- **WHEN** the selected item has nothing for a given key to act on +- **THEN** the key help does not name that key +- **WHEN** the selection moves to an item that key does act on +- **THEN** the key help names it + +#### Scenario: Aged information says how old it is + +- **WHEN** what a panel shows has not been refreshed for well past its own + refresh rate +- **THEN** the panel shows how old it is and stops presenting it as current + diff --git a/openspec/specs/fleet-client/spec.md b/openspec/specs/fleet-client/spec.md index 5b6913c0..f1b9ba9d 100644 --- a/openspec/specs/fleet-client/spec.md +++ b/openspec/specs/fleet-client/spec.md @@ -681,32 +681,40 @@ Start SHALL proceed without confirmation. Stop SHALL require an explicit confirmation before it is sent, because it ends an engine that may be serving work: a declined or abandoned confirmation SHALL send nothing. -While an action is in flight, the node's own tile SHALL carry it: the verb -and the action's status lines as the call reports them, beside the node's last -report rather than in place of it — the call's lines say what the operator -asked for, the report says what the node is doing. For a node whose last -completed refresh answered, the tile SHALL show that answer's state, what it -serves, its last-active record, and its resource usage and token and request -counters whenever the answer carries them, whatever the node's state — a boot -half done is already measuring, and that is the truth the tile keeps showing -while the call works. A node whose call reports nothing and whose refresh has +While an action is in flight, the node's own tile SHALL carry it: the verb, the +action's current situation, and how long the action has been running, beside the +node's last report rather than in place of it — the action's own account says +what the operator asked for, the report says what the node is doing. For a node +whose last completed refresh answered, the tile SHALL show that answer's state, +what it serves, its last-active record, and its resource usage and token and +request counters whenever the answer carries them, whatever the node's state — a +boot half done is already measuring, and that is the truth the tile keeps showing +while the call works. A node whose action reports nothing and whose refresh has not yet answered SHALL show the verb alone, and a latest refresh that failed SHALL change nothing on the tile. Each finished action SHALL clear the action from its node's tile, which is then the node's report alone, and leave its outcome on the status line, the one-shot wording. -The tile SHALL show the verb with how long the action has been running, counted -from when the operator issued it and recomputed as the tile is repainted rather -than fixed when a status line arrives. A start's status lines can stand -unchanged for minutes — the attempt that succeeds holds one request for the -whole boot and reports nothing while it does — so the elapsed time is what -distinguishes a tile that is waiting from one that is wedged. - -A status line a node reports SHALL describe the node's situation now, not a -situation that has since been superseded. A line that is true only until the -node's next attempt — a wait before a retry, and the reason for it — SHALL be -replaced when that attempt is issued, so a tile never goes on reporting a -refused start beside its own refreshes reporting the node running. +An action SHALL have exactly one current situation at a time, and a new one +SHALL replace its predecessor outright rather than being added to it. A +situation that was true of an attempt the action has moved on from SHALL NOT be +shown: the tile reports what the action is doing now, never what it was doing +before. In particular, a start refused for want of capacity SHALL stop reporting +that refusal once a further attempt is under way — so a tile never shows a +capacity wait beside a refresh reporting the node up and running. + +Anything the tile says about time SHALL be computed when the tile is drawn, not +when the situation it describes arose: a wait counts down towards the attempt it +is waiting for, and an action counts up from when the operator issued it. A +start's situation can hold unchanged for minutes — the attempt that succeeds +keeps one request open for the whole boot and reports nothing while it does — so +these are what distinguish an action that is waiting from one that is wedged. + +An action in flight SHALL also carry a spinner beside its verb, whose frame is +likewise chosen when the tile is drawn, and the board SHALL redraw often enough +for it to turn. It says the same thing as the elapsed time to an operator +glancing rather than reading, and it says it on a tile whose every other line +can hold still for minutes. The outcome of an action SHALL be shown inside the dashboard (a status line the operator can read before the next refresh replaces attention), and a refused or @@ -759,20 +767,15 @@ never invited to press a key that would do nothing there. #### Scenario: A start is watched on its own tile -- **WHEN** the operator starts a node whose start reports progress as it works -- **THEN** the node's tile shows the verb and the start's status lines while - the start is in flight +- **WHEN** the operator starts a node whose start reports its situation as it + works +- **THEN** the node's tile shows the verb and the start's current situation + while the start is in flight - **AND** a refresh that answers while the start is in flight shows its state - and whatever it measures on the same tile, beneath the start's lines + and whatever it measures on the same tile, beneath the start's own account - **AND** when the start finishes, the tile is the node's report alone and the outcome is on the status line -#### Scenario: A start's elapsed time keeps moving - -- **WHEN** a start is in flight and reports no new status line for some time -- **THEN** the elapsed time beside the verb keeps advancing on the tile, so the - operator can tell the start is still waiting rather than wedged - #### Scenario: A refused start does not outlive its refusal - **WHEN** a node's start is refused for want of capacity, and a later attempt @@ -781,10 +784,25 @@ never invited to press a key that would do nothing there. reporting it once the next attempt is under way, rather than showing it beside a refresh that reports the node running +#### Scenario: A start's elapsed time keeps moving + +- **WHEN** a start is in flight and its situation does not change for some time +- **THEN** the elapsed time beside the verb keeps advancing on the tile, so the + operator can tell the start is still waiting rather than wedged + +#### Scenario: A wait counts down and an action counts up + +- **WHEN** a start is waiting for its next attempt and the operator watches + without pressing anything +- **THEN** the time until that attempt counts down on the tile, and the time + since the operator issued the start counts up, both advancing as the tile is + redrawn rather than standing at the values they held when the wait began +- **AND** the spinner beside the verb turns while they do + #### Scenario: An in-flight start before any report - **WHEN** the operator starts a node before any refresh of it has answered -- **THEN** its tile shows the verb and the start's status lines alone +- **THEN** its tile shows the verb and the start's own account alone - **AND** the first answered refresh appears on the tile beside them #### Scenario: Two nodes wake at once @@ -972,8 +990,26 @@ watch mode's minute — and a `kind: remote` environment SHALL refresh on a much slower cadence, a 60-second interval, one status call a minute, because its status is a signed call through the cloud control plane rather than a local socket, and its state changes on the scale of minutes. A manual refresh -SHALL read every node, whether or not its kind's cadence was due. A refresh of a group SHALL read that group's nodes -concurrently, as fan-out does elsewhere. +SHALL read every node, whether or not its kind's cadence was due. A refresh of a +group SHALL read that group's nodes concurrently, as fan-out does elsewhere. + +A node with an action in flight SHALL refresh on the short interval whatever its +kind, and SHALL return to its kind's own cadence once the action settles. The +reasons a cloud environment is read once a minute — the call is expensive and +its state changes slowly — hold for an environment nobody is touching and hold +for neither one the operator has just started. + +Every reading of a node SHALL carry the time it was taken, and the dashboard +SHALL NOT show a reading older than the one it is already showing for that node. +Reads are concurrent and of uneven duration, so a reading can land after one +taken later than it; showing it would replace what the node is doing with what +it was doing. + +A node whose newest reading has aged well past its cadence SHALL be shown as +such — its age on the panel, and its health unknown rather than whatever the +aged reading said. A stale reading is not a wrong reading, but presenting it +indistinguishably from a current one is: the operator SHALL be able to tell how +old what they are looking at is. One node that is slow or unanswerable SHALL NOT hold the rest of the fleet hostage to it: a refresh SHALL give up on a node that has not answered within @@ -995,6 +1031,30 @@ round SHALL NOT stretch the local machines' cadence. - **THEN** every node is re-read, local and cloud alike, rather than waiting for each kind's next cadence +#### Scenario: A node being acted on is read more often + +- **WHEN** the operator starts a cloud environment +- **THEN** that environment is re-read on the short interval for as long as the + start is in flight, and returns to its 60-second cadence once the start + settles +- **AND** the other cloud environments keep their own cadence throughout + +#### Scenario: A late reading does not overwrite a newer one + +- **WHEN** a read of a node is issued, a later read of the same node answers + first, and the earlier read then answers +- **THEN** the panel keeps showing the later read, and the earlier one is + discarded + +#### Scenario: A panel with an out-of-date reading shows its age + +- **WHEN** a node stops answering and its newest reading ages well past its + cadence +- **THEN** its panel shows how old that reading is and its health reads + unknown, rather than continuing to present the aged state as current +- **WHEN** the node answers again +- **THEN** the panel returns to showing its state without an age + #### Scenario: One hung node does not stall the board - **WHEN** one node does not answer within its group's budget while the others @@ -1031,6 +1091,13 @@ panel's text. The glyph SHALL be shown in every panel shape: a settled answer, an action in flight, a panel awaiting its first refresh, and a panel showing a failed outcome. +A tile SHALL draw its first line — the glyph, the node's name, and what the +node is doing — as a header bar: light text on one background colour running +the tile's full width, so the name reads as the panel's title rather than as +another line of the panel's body. That background SHALL be a single neutral +colour, the same on every tile, so it never competes with the glyph's own +colour for what a node's health is read from. + A node's health SHALL fall into exactly one of five tiers: healthy, attention, unhealthy, not serving, and unknown. Healthy is coloured green, attention yellow, and unhealthy red. Not serving and unknown are both @@ -1040,24 +1107,25 @@ dot as the other tiers in a faded shade, and unknown keeps its `?` — so a node known to be undeployed never reads as a node the dashboard has not heard from yet. -- **Healthy**: the node answered its last refresh, its engine is not - crashed, not `idle`, not `stopped`, and not `undeployed`, and — when the - daemon reports readiness for it — the engine is ready. A `running` node whose daemon - reports no readiness (an older daemon, or a runner with no known health - check) counts as healthy too, rather than reporting a health tier the - daemon cannot actually back. +- **Healthy**: the node answered its last refresh, that answer is current, its + engine is not crashed, not `idle`, not `stopped`, and not `undeployed`, and — + when the daemon reports readiness for it — the engine is ready. A `running` + node whose daemon reports no readiness (an older daemon, or a runner with no + known health check) counts as healthy too, rather than reporting a health tier + the daemon cannot actually back. - **Attention**: the node has a start or stop action in flight for it, or is `running` with its daemon explicitly reporting the engine not yet ready. - **Unhealthy**: the node's engine has crashed, or its last refresh's outcome was a failure (`unreachable`, `unauthorized`, `config-error`, `failed`, or `unsupported`). -- **Not serving**: the node answered its last refresh, no action is in - flight for it, and its engine is not serving — its state is `idle`, the - daemon has started nothing, `stopped`, a daemon engine that was stopped, - or `undeployed`, a remote environment with no instance at all. -- **Unknown**: no status can be determined for the node — it has not yet - answered any refresh, or its last refresh answered without reporting an - engine state. +- **Not serving**: the node answered its last refresh, that answer is current, + no action is in flight for it, and its engine is not serving — its state is + `idle`, the daemon has started nothing, `stopped`, a daemon engine that was + stopped, or `undeployed`, a remote environment with no instance at all. +- **Unknown**: no current status can be determined for the node — it has not yet + answered any refresh, its last refresh answered without reporting an engine + state, or its newest answer has aged well past its cadence and no longer + describes the node now. #### Scenario: A running, ready node reads healthy @@ -1100,6 +1168,13 @@ heard from yet. - **WHEN** a node's last refresh answered but reported no engine state - **THEN** its panel's status glyph is grey +#### Scenario: A node whose answer has gone stale reads unknown + +- **WHEN** a node last answered `running` and has not answered since, long + enough that the answer no longer describes the node now +- **THEN** its panel's status glyph is grey rather than the green that answer + earned when it was current + #### Scenario: An action in flight reads attention - **WHEN** the operator starts or stops a node and that action has not yet @@ -1143,8 +1218,59 @@ heard from yet. - **THEN** the first shows the faded grey filled dot and the second the grey `?`, so the two greys are told apart by their mark +The board SHALL carry a title bar of its own along the top of the frame, on +the same surface a tile's header bar uses: the product's name, the screen in +view, and — pushed to the right — the fleet file and what is on that screen. A +terminal too narrow for both halves SHALL keep the left one rather than wrap +the bar onto a second row. The grid and the detail view SHALL draw their title +bar from one place, so the two screens cannot title themselves differently. + +The board SHALL use exactly one accent colour, the mint of the spinloop logo, +and SHALL use it only where nothing about a node is being reported: the title +bar's product name, and the border of the selected panel. A node's own state — +the health glyph, the resource bars — SHALL keep the terminal's green, amber +and red, which say what an engine is doing rather than whose product this is. + +In the frame's key help, each entry SHALL be drawn as the key and what that +key does: the key in the terminal's own text colour, and what it does a step +back in the muted ink, so the keys are what a glance over the footer picks out. +The prose beside them — a confirmation's question, an action's outcome — SHALL +be left as it is. + +#### Scenario: A key stands out from what it does + +- **WHEN** the key help is drawn +- **THEN** each key keeps the terminal's own text colour and what it does is + drawn in the muted ink beside it +- **AND** the status line and a confirmation's question are not drawn that way + +#### Scenario: The selected panel is marked in the accent + +- **WHEN** the operator moves the selection onto a panel +- **THEN** that panel's border is drawn in the brand accent, and no colour that + reports a node's state is used for it + +#### Scenario: The title bar names the product and the screen + +- **WHEN** the dashboard is open on the grid or on a node's detail view +- **THEN** the top of the frame carries a title bar naming the product and that + screen, with the fleet file and the screen's own details to the right + +#### Scenario: A narrow terminal keeps the left half of the title bar + +- **WHEN** the terminal is too narrow for both halves of the title bar +- **THEN** the product and the screen are kept and the right half is dropped, + and the bar stays one row + +#### Scenario: The header bar does not carry the health colour + +- **WHEN** two nodes of different health are shown side by side +- **THEN** both tiles' header bars are the same colour, and the two nodes are + told apart by their glyphs + #### Scenario: The glyph is distinct from the selection border - **WHEN** the operator moves the selection onto a panel - **THEN** the selected panel's border colour changes as it does today, and every panel's status glyph colour is unaffected by which panel is selected +