Skip to content

feat(hub): development hub — fleet dashboard, goal dispatch, and box spawning#75

Merged
andybons merged 40 commits into
mainfrom
feat/hub
Jul 13, 2026
Merged

feat(hub): development hub — fleet dashboard, goal dispatch, and box spawning#75
andybons merged 40 commits into
mainfrom
feat/hub

Conversation

@andybons

@andybons andybons commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds harness hub — a local, single-operator control surface over a fleet of
harness serve boxes: a live dashboard for "what are my agents doing right
now" and for dispatching new goal-supervised sessions. It serves one embedded,
CSP-self-contained page (tools/hub/index.html, go:embed) on localhost:7777
and talks to each box directly over its normal HTTP+SSE API. The hub keeps no
server-side state
— every box, run token, and selection lives in the browser
tab's URL fragment — and exposes exactly one Go API, POST /spawn, which execs
the deployment's provision command and streams its output live over SSE.

What's here

  • Fleet view + session drill-down + dispatch/goal workflow + notifications
    the core hub: a live fleet dashboard, per-session timeline, and the goal-first
    dispatch flow.
  • Tactical-telemetry design language — a committed dark-only brutalist
    archetype (documented in AGENTS.md): one substrate, two semantic colors
    (hazard red, terminal green), monospace dominance, zero border-radius.
  • Incremental, from-tip rendering — keyed append-only timeline and per-card
    fleet reconciliation, event stream connected from the journal tip, renders
    coalesced to one per frame; open sessions pin to the tail as intent, not
    geometry.
  • Spawn contractPOST /spawn streams combined stdout+stderr over SSE and
    parses TUNNEL_URL / RUN_TOKEN / PORT_URL_<port> lines; box-name
    passthrough via HARNESS_HUB_BOX_NAME; lineage grouping, paused goals, the
    process strip, and Respawn/ADOPT.
  • Browser-security hardeningPOST /spawn execs a real provision command,
    so handleSpawn rejects browser cross-origin requests before any exec via an
    Origin-vs-Host gate (OWASP verify-origin): the page's own same-origin
    fetch("/spawn") and non-browser callers pass, an attacker page's fetch
    gets 403. The served page also carries a strict Content-Security-Policy
    (default-src 'none' + 'unsafe-inline' script/style + connect-src * for
    arbitrary box origins + frame-ancestors/base-uri/form-action 'none') —
    defense-in-depth for a page holding run tokens in its URL fragment.

Testing

  • Pure hub logicnode --test tools/hub/*_test.mjs and go test ./tools/hub/ (CSRF, CSP, spawn contract).
  • Real-backend end-to-endgo test -race ./tools/hub/e2e/... starts an
    actual server.Server + hub.NewHandler and drives the real, served
    index.html with jsdom and an unmocked fetch. Self-installing.
  • The CSP and CSRF guard were additionally verified in a real browser (page
    renders, inline script/style run, same-origin and cross-origin box fetches
    behave correctly, zero CSP violations).

Not included (deferred)

An interactive terminal (GET /term PTY-over-WebSocket plus a hub-side xterm
pane) was explored on this branch and then removed (commit aacc771): the
server half shipped without a client and the pane was never built. It drops the
coder/websocket and creack/pty dependencies. Revive both halves from git
history (the endpoint is commit c08e81b) when the hub pane is built.

andybons added 30 commits July 10, 2026 22:21
…ract

Adds tools/hub, a new package holding the Go side of the local fleet hub:
NewHandler serves the (placeholder, for now) embedded single-file page at
/ and implements the sole API, POST /spawn, which execs a configured
shell command and streams its stdout live over SSE, parsing the
TUNNEL_URL=/RUN_TOKEN= contract lines documented in spawn.go. harness hub
wires this up as a new CLI subcommand, loopback-only by default.
tools/hub/index.html gains the inspector-matched dark/light palette and
overall layout (fleet list, drill-down timeline/composer/goal panel,
header actions, modal host), plus the full set of pure, unit-tested
helpers the app logic will build on: fmtRelative, sessionBadge,
countByState, goalSnippet, lastTurnSummary, fmtTokens/usageSummary,
reduceGoal (extended with goal.stalled), goalFromSession, notifyForEvent,
sortByLastActivity, and the URL-fragment hub-state codec
(encodeHubState/decodeHubState — no server-side registry, no config file,
tolerant of garbage fragments). The SSE frame parser and shortId/
prettyJSON/partsText/maxSeq are carried over verbatim from the inspector.

tools/hub/hub_test.mjs unit-tests all of the above (36 cases, all green).
…tifications

Wires the pure helpers up into a full app:

- Per-box lifecycle: health polling + GET /session snapshot every 5s, plus
  the box-wide SSE /event stream (reconnect with backoff, mirroring the
  inspector) for near-real-time fleet-card updates. Fleet view renders box
  cards (health dot, vcs_revision, session counts by composite state) and
  session cards ordered by last_activity_at (state badge, goal snippet,
  last_turn outcome, cumulative usage, relative time).
- Session drill-down opens a SECOND, session-scoped SSE connection
  (/event?from=0&session=<id>) that replays a session's entire durable
  history — messages AND every goal.* record — so the timeline renders as
  a goal narrative (goal.set condition, each goal.eval verdict, goal.stalled
  distinguishing "waiting out provider weather" from a real stall,
  goal.achieved, goal.cleared) alongside the inspector-style message
  rendering (streaming deltas, collapsed tool_call/reasoning, error/abort
  banners). Composer actions: send prompt, set/clear goal, abort.
- Dispatch flow: one modal creates a session AND starts a goal in one step
  (large textarea for long multi-paragraph conditions, max_turns, optional
  model). A one-click re-arm affordance appears on ended/errored goal
  sessions, pre-filling the previous condition for editing.
- Box management: add box (base URL + token + name), remove box, and
  spawn box — the spawn modal POSTs this hub's own same-origin /spawn and
  streams the live output, adding the box automatically once TUNNEL_URL=/
  RUN_TOKEN= are parsed.
- Opt-in browser Notifications (permission requested on first toggle):
  fired on goal.achieved or a turn.end error for any connected box's
  session, so the hub is the monitor — nobody has to poll.
- All state (box list, current selection, notify toggle) persists only in
  the URL fragment via encodeHubState/persist(); nothing touches
  localStorage or a server-side registry.
… contract

Adds a public-safe AGENTS.md section for `harness hub`: what it is (a
local, single-operator fleet dashboard, not a deployed product), the
URL-fragment-only state model and why run tokens riding the URL is an
accepted tradeoff, the -cors-origin requirement for every box, and the
POST /spawn TUNNEL_URL=/RUN_TOKEN= contract that is the only coupling
between this repo and any deployment-specific provisioning tool.

Also applies gofmt's smart-quote comment normalization to hub.go.
…ield

Spawning a box no longer prompts for a name — openSpawnModal generates
a friendly slug (randomSlug, injectable rand so it's unit-tested) and
shows it read-only; the operator just clicks Spawn.
An empty bordered .spawnlog rendered as a stray input-field-looking gray
bar in the spawn dialog. :empty { display: none } keeps it out until the
first stdout line arrives.
connectBox fired pollBoxOnce (which sets lastSeq from the /session
snapshot's high-water mark) WITHOUT awaiting it, then opened
/event?from=lastSeq while lastSeq was still 0 — replaying the entire
journal (every event of every historical session on the shared volume)
into the UI on every box connect. Await the first poll before opening
the stream so it tails from the tip; the recurring poll reconciles the
tiny snapshot-to-stream window. Session drill-down still replays its own
session scoped from=0, which is bounded and intended.
Opening a long session hung the tab: handleDrillEvent rebuilt the full
timeline after EVERY event of the from=0 session replay, and
handleBoxEvent repainted the fleet per event — O(events x DOM).
createCoalescer (testable, injectable scheduler) now funnels both
event-driven repaints through requestAnimationFrame, one render per
frame no matter how many events land in a chunk. A 365-message session
replay goes from a frozen tab to main-thread-responsive in ~100ms.
Low-frequency paths (selection, poll ticks, reconnect) still render
directly.
Multi-thousand-character goal conditions rendered at full height in the
pinned goal panel, burying the timeline. Collapse to a two-line clamp
with a show-full-goal/show-less toggle; the expanded state lives on
drill.goalExpanded so poll- and event-driven re-renders don't snap it
shut, and it resets on session change. Short single-line conditions get
no toggle.
planAppend, isPinnedToBottom, shouldResort, boxCardSignature, and
sessionRowSignature give the upcoming keyed-diff DOM code (renderTimeline,
renderFleet) pure, unit-tested building blocks: which items are new,
whether the viewport is pinned to the bottom, whether a reorder pass is
due, and whether a card/row's content actually changed. DOM code to
consume them lands next.
renderTimeline() used to empty and rebuild #timeline from scratch on every
coalesced render; a from=0 session replay is dozens of such rebuilds over
~2s, which flickered, snapped every tool_call/reasoning <details> back to
collapsed, and forced the scroll position to the bottom regardless of
where the operator had scrolled to. renderFleet() had the matching bug for
box cards and session rows, with live activity re-sorting rows so they
jumped around under the cursor during a stream burst.

Timeline: durable messages/goal-narrative entries/banners are now each
rendered exactly once, keyed (message id / event seq / banner index) via
the new planAppend helper, and a render pass only appends the ones not
already in the DOM — existing nodes, and their <details> open/closed
state, are left untouched. The streaming draft is now a single persistent
element (createDraftEl/updateDraftEl) whose text/reasoning/tool children
are patched in place instead of recreated, so expanding its tool_call
block survives further deltas. Autoscroll now only sticks to the bottom
if the viewport was already pinned there (isPinnedToBottom) before this
render's DOM changes; a scrolled-up operator keeps their place. The one
legitimate full rebuild is a selection change (resetTimelineDom).

Fleet: box cards and session rows are keyed by id and only touched when
boxCardSignature/sessionRowSignature actually changed. Session row
re-sorting by last_activity_at is damped to at most once per second
(shouldResort) — between allowed sorts, rows refresh their content in
place and new rows are spliced in without disturbing the existing order,
so cards/rows stop shuffling under the cursor during bursts.

Load: a box's health starts "unknown" until its first poll resolves;
that state now renders as a neutral, dimmed dot (.dot.loading) with the
session area showing a "loading…" placeholder instead of "no
sessions", so the layout doesn't jump once data arrives. Bootstrap
already decoded the URL fragment and rendered synchronously before any
network I/O; this just makes that first paint not look broken.
Operator-reported bug: a box running an older harness binary returns
last_activity_at: null for every session. The old sortByLastActivity
compared new Date(null) - new Date(null), which is NaN; Array#sort's
behavior on a NaN-returning comparator is effectively undefined, so the
sort degenerated to server/insertion order and could bury the one
actively running session at the bottom of the fleet list.

Replace it with sortSessions (plus the sessionTimeRank/sessionPriorityRank
helpers it's built from): active states sort first (goal-running, then
busy, then idle) regardless of timestamps, and within a state sessions
order by last_activity_at, falling back to created_at, falling back to
-Infinity (sorts last) for a session with neither. sessionTimeRank never
returns NaN, so the comparator is total no matter how many timestamps are
missing. Update the fleet's damped re-sort (updateSessList) to call
sortSessions instead of the retired sortByLastActivity, and add the
header-comment hand-test checklist entries for the incremental-rendering
behaviors landed alongside it.
Previously this was a throwaway, uncommitted manual session: a Go program
starting a real server.Server + hub.NewHandler, and a Node/jsdom script
driving the actual served index.html against them with a real, unmocked
fetch. That only proved the incremental-rendering fix works in a one-off
local run; committing it is what makes it reproducible and checkable by
anyone (or any automated verifier) from a fresh clone.

tools/hub/e2e/stub.go: exports Start(), building a real box server (the
same server.New wiring as `harness serve`, fed by a small scripted
provider so no model API key is needed) and a real hub server (the same
hub.NewHandler wiring as `harness hub`, serving the actual embedded
tools/hub/index.html) on loopback.

tools/hub/e2e/hubverify: a small `go run`-able command for driving the
stub by hand (prints its addresses/token, then blocks).

tools/hub/e2e/real_e2e.mjs: loads the REAL served page in jsdom with
Node's own unmocked fetch (plus a real AbortController — jsdom's own
produces AbortSignal instances real fetch rejects as foreign) and checks,
against the real backend: the hub server serves index.html byte-for-byte;
a populated URL fragment renders its box skeleton synchronously with no
empty-state flash; a real health/session poll resolves to a healthy dot
with the box card's DOM node surviving a real session being created; a
real engine turn renders as a keyed durable message whose expanded
reasoning block survives a second real, server-driven turn (keyed
append-only against actual SSE traffic); and a scrolled-up viewport is
left alone by a real subsequent render.

tools/hub/e2e/e2e_test.go wires this into `go test ./tools/hub/e2e/...`
(already part of the standard `go test -race ./...`), so no extra command
is needed — it skips cleanly (not a failure) when node isn't on PATH or
`npm install` hasn't been run in this directory yet (see README.md), so
the ordinary recipe stays green without the opt-in step, while actually
exercising the real thing wherever that one-time setup has been done.

package.json/package-lock.json declare jsdom as this isolated verification
tool's own dependency, kept out of the shipped hub page entirely — tools/hub/index.html
remains the single build-free file it always was.
The previous version skipped TestRealEndToEnd whenever jsdom wasn't
already installed in tools/hub/e2e, which meant a plain `go test -race
./...` on a fresh clone silently skipped the real end-to-end check rather
than running it — 'green' didn't actually mean 'verified' for that piece
without a separate, undocumented-at-the-command-line `npm install` step.

TestRealEndToEnd now runs `npm ci` (using the package-lock.json already
committed alongside it) itself the first time jsdom is missing, then
proceeds to drive the real check — no manual setup required. It only
skips in the one case where `node --test tools/hub/*_test.mjs` (already a
hard, documented requirement of this repo) would ALSO be unrunnable: no
Node toolchain on PATH at all. If node/npm ARE present but the dependency
install itself fails (e.g. no network access to npm's registry), it fails
loudly rather than skipping — an offline environment is a real
verification gap, not something to paper over as a pass.

Updated the e2e README and AGENTS.md's Development hub section to match:
plain `go test -race ./...` is now the complete, unconditional recipe.
…ot geometry

Opening a session left the view at/near the top: pinnedness was inferred
from scroll geometry on every render, so (a) a mid-replay scroll event
delivered after the next chunk's append saw a huge gap and released the
pin, and (b) post-render layout growth (fonts, wrapping) after the LAST
render stranded the view shy of the bottom with nothing left to re-pin.

Now a session opens stuck-to-bottom (tlDom.stick) and only user action
changes that: the scroll listener ignores events within 120ms of a
programmatic pin, wheel-up releases immediately, and scrolling back to
the bottom re-sticks. A ResizeObserver on the content blocks re-pins on
late growth while stuck, and never touches a released view.
Adds a WebSocket relay to a real PTY-backed shell, gated by the same run
token every other endpoint uses. Browsers cannot set arbitrary headers on a
WebSocket handshake, so the token also rides the Sec-WebSocket-Protocol
subprotocol as bearer.<token> (echoed back on accept), validated with the
same constant-time comparison as the Authorization header path; a plain
Authorization: Bearer header still works for non-browser clients. The token
never rides the URL/query string.

session=<id> starts the shell in that session's own workdir (its dedicated
git worktree when isolation=worktree); absent, it defaults to the server
process's cwd.

Client disconnect kills the shell's whole process group, reusing the
retry-until-ESRCH SIGKILL pattern from engine/bash_unix.go's killProcessGroup
(a pty-hosted shell is its own session/process-group leader via
pty.StartWithSize's Setsid, same reasoning as the bash tool's Setpgid). Shell
exit closes the socket with a normal closure frame. The Origin header is
validated against -cors-origin by hand (a WS handshake isn't subject to
browser CORS preflight at all, so this is the only thing stopping a random
page from riding a browser's connection).

PTY code lives in term_unix.go (//go:build unix); term_other.go stubs
runTerminal/ptySupported so GET /term returns a plain 501 on every other
GOOS without ever upgrading the connection or touching a PTY, mirroring
engine/bash_unix.go/bash_other.go.

Dependencies: github.com/creack/pty for the PTY itself, and
github.com/coder/websocket for the WebSocket transport — chosen over
gorilla/websocket for being smaller, actively maintained, dependency-free,
and because its context-based Accept/Read/Write API natively supports
negotiating a specific subprotocol on accept, which the bearer-token trick
above needs directly instead of reimplementing subprotocol selection by
hand.

Tests: an end-to-end unix test dials the real WS endpoint against a live
serve instance — round-trips terminal-e2e-10671, confirms a resize
control message actually changes the PTY winsize (via 80), confirms
session=<id> starts the shell in that session's workdir, and confirms both a
bad token and a mismatched Origin are rejected before any upgrade (structurally
before any PTY spawn). The non-unix 501 path is covered by a platform-agnostic
test that skips itself on unix and asserts the 501 contract for real when built
on a !unix GOOS.
… HARNESS_HUB_BOX_NAME

Also extends the spawn contract's line-scanning to PORT_URL_<port>=<url>
lines (collected into a port_urls map on the done event), ahead of the
hub page wiring them up in a later commit.
…ss/port strips

Adds buildLineages, collectActiveGoals, goalPauseTreatment, rearmNeeded,
canRedispatch, normalizePorts/normalizeProcesses/processStateBadge/
portLinksFor, parsePortURLLines, and mergePortURLs to the TESTABLE block,
plus reduceGoal/goalFromSession support for goal.paused and the
paused/pause_reason fields (docs/design/fleet-model.md §7). All red-first
in tools/hub/hub_test.mjs; DOM wiring lands in a follow-up commit.
…d the active-goals bar

Wires the pure helpers from the previous commit into rendering:

- LINEAGE GROUPING: box-card session rows now group via buildLineages —
  the lineage tip is the primary row, prior attempts collapse behind an
  'N earlier attempts' toggle. '↗ Re-dispatch' on an ended/errored session
  creates a NEW session (parent_session set to the old one) and opens the
  goal form pre-filled with its prior condition, distinct from the
  existing in-place '🔁 Re-arm'.
- PAUSED GOALS: session rows, the drill goal panel, and the timeline
  narrative all render goal.paused/pause_reason: provider-backoff gets a
  calm 'waiting out provider weather' note, restart gets a prominent
  '🔁 Re-arm now' CTA (rearmNeeded/goalPauseTreatment).
- PROCESS STRIP: each box card polls GET /process and renders a compact
  per-process row (name, ready/starting/exited-with-code dot, a clickable
  preview link when the process reports a port matching the box's new
  port_urls map). Absent/404 renders no strip at all. Clicking a process
  opens a small panel with its state and log path (no log-content
  endpoint exists to call). port_urls is editable in the add-box modal and
  auto-populated from PORT_URL_<port>= spawn output.
- RESPAWN/ADOPT: a box whose health is down gets a '⟲ Respawn' button that
  POSTs /spawn with the SAME name and updates that box's base URL/token in
  place on success.
- ACTIVE-GOALS STRIP: a slim fleet-wide bar above the box list listing
  every active (running or paused) goal across every box, click-through to
  its session — collectActiveGoals.

Palette-consistent light+dark CSS for every new element. Hand-test
checklist in the header comment extended with six new numbered steps.
node --test tools/hub/*_test.mjs (117 tests), go build/vet clean.
… real scroll event

Two real bugs the new rendering work surfaced under `go test -race ./...`:

- index.html's resetTimelineDom called `new ResizeObserver(...)`
  unconditionally. Any host lacking that global (confirmed via jsdom,
  which tools/hub/e2e's real backend check drives the page through) threw
  inside the selectSession click handler and silently aborted the rest of
  it — including the connectDrillStream() call that actually streams a
  session's messages in. Feature-detect it instead; every real browser
  this page targets has it, so behavior there is unchanged.
- real_e2e.mjs's pinned-tail check set `tl.scrollTop = 0` to simulate the
  operator scrolling up, but jsdom does not synthesize a "scroll" event
  from a plain property write the way a real browser's layout engine does
  — the page's own scroll listener (which is what flips stickiness off)
  never saw it. Dispatch a real "scroll" event after the property write,
  same as an actual user scroll would.

Both were pre-existing (present before this branch's lineage/paused-goal/
process-strip/respawn/active-goals work), intermittently masked because
the first bug's exception vs. the second bin's assertion depended on which
one a given run happened to reach — go test -race ./tools/hub/e2e/...
is now green across repeated runs. Verified: go build/vet/test -race
./... all green; node --test tools/hub/*_test.mjs (117 tests) green.
Applies the brutalist tactical-telemetry archetype disciplined by
anti-slop taste rules: single dark substrate (phosphor on #0a0a0a, the
light theme is removed rather than half-supported), hazard red as the
sole accent, terminal green reserved for exactly one semantic (live and
succeeded goal execution), monospace dominance with uppercase tracked
micro-labels, zero border-radius, hairline compartment borders, square
status markers, static scanline texture (no motion without cause), and
inverted-video hover states. Emoji and em-dashes purged from all UI
strings; every piece of telemetry shown is real data. All 113 helper
tests unchanged and green; selectors untouched so renderer code is
unaffected.
… dialogs

The page's job is reading fleet state, but ~14 actions were always
visible. Now visibility follows frequency and context:

- Header: Spawn stays; Add box / Copy link / Notify move to a native
  <details class=menu> overflow with light-dismiss.
- Box cards: Dispatch stays; New session / Remove behind a per-box
  overflow; Respawn still surfaces as a primary button exactly when the
  box is down (the moment it is the primary action).
- Session rows are pure state displays: inline Re-arm/Re-dispatch
  removed; both live in the opened session's goal panel (the paused
  urgency already reaches the operator via the row badge and the
  active-goals bar's urgent chip).
- Composer: Abort renders only while busy, Clear goal only while a goal
  is active.
- Zero-count chips no longer render.
- Modals are a native <dialog> (focus trap, Escape, ::backdrop free);
  jsdom 29 in the e2e suite supports showModal.

mjs suite, real-backend e2e, and a Playwright pass (menu counts,
light-dismiss, dialog open/Escape, zero row actions) all green.
The third element on a left-nav box card was an unlabeled hex blob,
mangled further by the uppercase micro-label transform. It is the box
binary's vcs_revision from /health — now reads 'rev caafc53'; the
unreachable/loading states in the same slot are unchanged.
A dead box's URL is unclickable and will be replaced on respawn anyway;
the card shows name, 'unreachable', and the Respawn action instead.
The prior check hid the URL only on health=down, so the loading/unknown
state still showed it — visible on every page load for boxes that turn
out dead. Whitelist reachable instead of blacklisting down.
An idle-reaped box is the fleet model's normal lifecycle state (compute
released, volume intact, Respawn resumes) — 'unreachable' framed it as
a fault. A genuine network fault looks identical and shares the remedy,
so the calm label serves both.
The copy referenced internal docs (fleet-model.md section numbers),
implementation nouns (fragment entry), used an em-dash, and repeated
the box name the title already carries. Replaced with three plain
sentences plus the honest caveat: sessions ride the volume back,
working directories and processes do not.
…, gate Escape

runSpawn now announces the in-flight state (button label 'Spawning…',
disabled) and disables the dialog's Close while a spawn streams —
abandoning mid-spawn would orphan the server-side process. Escape is
gated the same way via the dialog cancel event. Both the Spawn and
Respawn modals thread their Close button through. The spawn log clears
its placeholder on first real stdout line.
andybons added 6 commits July 12, 2026 12:20
…kout clobber)

The in-flight treatment committed in 6e97ddf was silently reverted: the
spawn script's git checkout origin/main cycle in the working checkout
clobbered runSpawn's body between edit and commit, so only the call-site
threading and Escape gate survived. runSpawn now again disables + relabels
the start button to 'Spawning…', disables Close, and clears the log
placeholder on first stdout; verified via a real respawn (label, Close,
Escape all correct, adoption completes). Hand-test checklist refreshed
for the current IA (menu, inactive boxes, in-flight states).

The root cause is fixed separately in the spawn script: box binaries now
build from a dedicated /tmp/harness-build clone, never the operator's
working checkout.
The .notice class was red for every message, so 'box added'/'box updated'
success read as an error (see the red 'box updated' in the respawn
dialog). Add a .notice.ok green variant, reset the class per spawn
attempt (red stays the error default), apply ok on both success paths.
Full UI audit findings:
- Secondary text (--dim #7d7d7d) was 4.44:1 on the lightest card bg,
  just under WCAG AA. Bumped to #888888 (5.15:1); box name, base, meta,
  count chips, and goal snippets now all clear AA.
- The fixed 390px fleet pane overflowed viewports narrower than itself.
  Below 820px the panes now stack (fleet on top, capped height) so a
  narrow window never scrolls horizontally.

Audited and already passing: keyboard reach + visible focus, single h1,
all buttons accessibly named, lang/title present, no infinite animations,
long-name truncation, live badges/timeline/streaming, Enter-to-send,
contextual Abort. State colors (green 12.6:1, red 4.9:1) pass AA.
The card's overflow menu was a <details> dropdown clipped two ways: the
card's overflow:hidden (pointless at radius 0) and #fleet's scroll
container. Converted to a popover (top-layer render, escapes all ancestor
clipping) with CSS anchor positioning under the trigger and
position-try-fallbacks:flip-block so it flips above near the viewport
bottom. Verified: opens fully within the viewport, items readable,
light-dismisses. Dropped the now-unneeded box-card overflow:hidden.
…er runs

The card overflow menu (commit 2715143) calls hidePopover() at the top of
its item handlers, before the item's real work. jsdom does not implement
the Popover API, so the real-backend e2e threw mid-handler and the '+ New
session' item never called quickNewSession — failing two steps later as
'a real session row should appear'. Provide minimal showPopover/hidePopover/
togglePopover stubs in the jsdom beforeParse, the same category as the
existing fetch/requestAnimationFrame/AbortController capability patches, so
the served page runs unmodified.
# Conflicts:
#	AGENTS.md
#	server/server.go
@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown
📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 4m 41s —— View job


Review — PR #75 development hub

Read AGENTS.md and the full diff. Reviewed the concurrency, protocol, auth, and startup-speed surfaces most likely to hide bugs. This is a well-engineered, heavily-tested PR; the /term PTY relay and the SSE plumbing are careful. One security issue is worth addressing before merge, plus two minor notes.

  • Read AGENTS.md + gather diff context
  • Review server/term*.go (PTY / WebSocket / process-group kill)
  • Review tools/hub/hub.go, spawn.go (SSE, exec, addr binding)
  • Review tools/hub/index.html (XSS, token handling, stream teardown)
  • Review tests (TDD, sleeps, synctest)
  • Post review

Findings

1. CSRF on POST /spawn (medium) — inline
handleSpawn has no auth and no Origin check, yet it execs the deployment's provision command via sh -c. Loopback binding doesn't help: any page the operator visits can fetch("http://localhost:7777/spawn",{method:"POST"}) as a no-preflight simple request and trigger a real box provision. A missing/malformed body is tolerated, so no valid JSON is even needed. Fix: gate on Origin (reject when present and not same-origin) — transparent to the real page.

2. /term output-writer can block indefinitely on a half-open client (low)
In runTerminal (server/term_unix.go:116), the PTY→WebSocket goroutine calls conn.Write(context.Background(), …) with no deadline. If the TCP peer is silently gone (half-open) the write parks, the PTY read buffer applies backpressure, and the handler's final <-outDone waits on it. The shellDone/ctx.Done() branches both call conn.Close, which unblocks the write, so this only bites if neither fires — bounded in practice but worth a write deadline (or conn.SetWriteTimeout-style guard) for robustness.

3. Sleep-based test synchronization (minor, AGENTS.md) — inline
TestTermResizeAccepted (line 96) and TestTermClientDisconnectKillsShell (line 220) use raw time.Sleep. The resize case doesn't even need it — a single server-side reader goroutine guarantees the resize frame is applied before the echo command; readUntil is the real sync. These drive a real subprocess PTY, so they're not the pure in-process case the rule targets, but they can be dropped.

Things I checked that are solid

  • Auth: run-token compared with subtle.ConstantTimeCompare; empty token impossible (NewServer rejects it at server.go:333). Token rides subprotocol/header, never the query string. /term re-checks Origin against -cors-origin before websocket.Accept, closing the WS-has-no-preflight gap.
  • PTY lifecycle: shell is its own session/pgroup leader (Setsid), client disconnect / server shutdown both killProcessGroup the whole tree with the same bounded-retry pattern as bash_unix.go. No goroutine leak across the three coroutines — every exit path closes conn, unblocking the reader (buffered wsErr).
  • XSS: the el() helper renders via textContent; the one innerHTML path (html: key, index.html:1349) is never passed remote data — grep confirms no call site. Still, a Content-Security-Policy response header on the served page would be cheap defense-in-depth given it holds run tokens.
  • Stream teardown: disconnectBox clears both timers, aborts the fetch, and flips rt.running before aborting so the reconnect closure early-returns — no reconnect storm. SSE parser handles \r\n, comments, and partial frames across chunk boundaries correctly.
  • Startup speed: go:embed index.html and the two new pure-Go deps (coder/websocket, creack/pty — no cgo, good) add no init-time network/disk/subprocess work. /term route registration is trivial.

Comment thread tools/hub/spawn.go
Comment thread server/term_unix_test.go Outdated
TestGoalPausedRestartYieldsIdleAndUsable's final assertion fires a
fire-and-forget prompt_async (202 admission) and returns immediately.
handlePrompt spawns a runPrompt goroutine, tracked by srv.wg, that keeps
appending to the session journal in dir (== t.TempDir()). Nothing awaited
it: srv.Close only closes the journal file, and the test never called
Drain, so under load the goroutine was still writing when t.TempDir()'s
RemoveAll ran — failing cleanup with 'directory not empty'.

Drain(context.Background()) blocks on wg.Wait until the goroutine exits, so
no writer survives into RemoveAll. Same idiom as goal_rollback_test.go.
@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown
📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 3m 8s —— View job


Review — PR #75 development hub

  • Read AGENTS.md + gather diff context
  • Review server/term*.go (PTY / WebSocket / process-group kill)
  • Review tools/hub/hub.go, spawn.go (SSE, exec, addr binding)
  • Review tools/hub/index.html (XSS, token handling, stream teardown)
  • Review tests (TDD, sleeps, synctest)
  • Post review

Re-reviewed the concurrency, protocol, auth, and startup-speed surfaces against the current tip. This is a well-engineered, heavily-tested PR — the /term PTY relay, SSE plumbing, and process-group teardown are careful, and the auth story is solid. Three findings remain from the prior round and are still unaddressed (filed inline again below).

Findings

1. CSRF on POST /spawn (medium) — inline
handleSpawn execs the deployment provision command via sh -c with no auth and no Origin check. Loopback binding doesn't help: any page the operator visits can fetch("http://localhost:7777/spawn",{method:"POST"}) as a no-preflight simple request and trigger a real provision — the SSE response is unreadable cross-origin, but the side effect fires. A missing/malformed body is tolerated, so no valid JSON is needed. Fix: gate on Origin (reject when present and not same-origin); transparent to the real page.

2. /term output writer can block on a half-open client (low) — inline
In runTerminal (server/term_unix.go:122), the PTY→WebSocket write uses context.Background() with no deadline. The wsErr branch kills the shell and waits <-shellDone/<-outDone but never conn.Close, so a goroutine already parked in conn.Write on a dead-but-not-reset peer never unblocks and the handler leaks. The shellDone/ctx.Done() branches both conn.Close (so they're fine); only the wsErr path bites. Fix: bounded write context, or conn.Close() in the wsErr branch.

3. Sleep-based test sync (minor, AGENTS.md) — inline
TestTermResizeAccepted (line 96) and TestTermClientDisconnectKillsShell (line 220) use raw time.Sleep. The resize case doesn't need it — a single server-side reader goroutine + TCP ordering guarantees the resize frame is applied before the echo command; readUntil is the real sync. These drive a real subprocess PTY so they're outside the rule's core target, but they can be dropped.

Things I checked that are solid

  • Auth: run-token via subtle.ConstantTimeCompare; empty token impossible (New rejects it, server.go:333). Token rides subprotocol/header, never the query string. /term re-checks Origin against -cors-origin before websocket.Accept, closing the WS-has-no-preflight gap (auth + Origin + workdir all resolved before any PTY spawn).
  • PTY lifecycle: shell is its own session/pgroup leader (Setsid); client disconnect and server shutdown both killProcessGroup the whole tree with the same bounded-retry pattern as bash_unix.go. No goroutine leak across the three coroutines except the half-open case in finding fix(cli): explicit -model flag wins over persisted model on resume #2.
  • runSpawn: correct StdoutPipe→scan→Wait ordering (no premature pipe close); combined stdout+stderr; scanner buffer grown to 1 MiB; always emits exactly one terminal done; CommandContext kills the process on client disconnect. Contract regexes require the marker on its own trimmed line — documented.
  • XSS: el() renders via textContent; the one innerHTML path (html: key, index.html:1349) has no call site passing remote data (grep confirms). No eval/Function/document.write.
  • Startup speed: go:embed index.html; the two new deps (coder/websocket, creack/pty) are pure-Go, no cgo; no init-time network/disk/subprocess work; /term route registration is trivial.

Nits (non-blocking)

  • The served hub page (handleIndex) sets no Content-Security-Policy. The page holds run tokens in its URL fragment, so a CSP (default-src 'self'; connect-src * for the box APIs) would be cheap defense-in-depth. Same for the X-Content-Type-Options: nosniff header.

Comment thread tools/hub/hub.go
Comment thread server/term_unix.go Outdated
Comment thread server/term_unix_test.go Outdated
The interactive terminal (GET /term over WebSocket, its unix PTY relay, and
the vendored xterm.js that was never wired into the hub page) is pulled for
now: the server half shipped without a client, and the hub terminal pane was
still unbuilt. Removing it drops the coder/websocket and creack/pty
dependencies (nothing else uses them) and moots the two /term review
findings (the unbounded PTY-writer conn.Write and the raw time.Sleep in the
term tests). The managed-process machinery from #71 is untouched — it never
depended on the terminal.

Revive from history when the hub-side pane is built.
@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown
📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 2m 29s —— View job


Review — PR #75 development hub

  • Read AGENTS.md + gather diff context
  • Review tools/hub/hub.go, spawn.go (SSE, exec, addr binding)
  • Review tools/hub/index.html (XSS, token handling)
  • Review tests (TDD, sleeps, synctest)
  • Confirm /term revert; re-check prior findings
  • Post review

Re-reviewed against the current tip. The latest commit (aacc771) reverts the /term PTY endpoint and the vendored xterm, so two of the three prior-round findings are now moot — server/term_unix.go (half-open-client blocking write) and server/term_unix_test.go (raw time.Sleep) no longer exist. Confirmed no leftover term/xterm references remain in server/ or tools/.

That leaves one substantive finding, still unaddressed from three prior rounds.

Findings

1. CSRF on POST /spawn (medium) — inline
handleSpawn (hub.go:82) execs SpawnCommand via sh -c with no auth and no Origin check. In a real deployment that provisions a cloud box (real cost/side effects). Loopback binding does not help: any page the operator visits can fetch("http://localhost:7777/spawn",{method:"POST"}) as a CORS simple request (no preflight) — the SSE body is unreadable cross-origin, but the provision side effect has already fired. A missing/malformed body is deliberately tolerated (hub.go:100), so no valid JSON is even needed.

Fix (inline suggestion attached): reject a request whose Origin header is present and not same-origin. The real page always sends Origin: http://<host>, so a same-origin-only gate is transparent to it — and worth a red-verified test per AGENTS.md.

Things I checked that are solid

  • /term revert is clean — no dangling references, no half-wired xterm imports. The known follow-up in the PR body (xterm pane) is correctly absent from the tree now.
  • runSpawn — correct StdoutPipe→scan→Wait ordering (no premature pipe close); combined stdout+stderr; scanner buffer grown to 1 MiB; always emits exactly one terminal done; CommandContext kills the process on client disconnect; empty name leaves the env untouched (no empty-valued HARNESS_HUB_BOX_NAME). Contract regexes anchor on a trimmed whole line — documented.
  • Addr bindingresolveAddr keeps the loopback-by-default promise; explicit -addr passes through as the operator asked. spawnCommandFromEnv correctly distinguishes flag-set-empty (disable) from flag-absent (env fallback) via fs.Visit.
  • XSSel() renders via textContent; the lone innerHTML path (html: key, index.html:1349) now has zero call sites (grep confirms). No eval/Function/document.write/insertAdjacentHTML. No localStorage/sessionStorage; state lives only in the URL fragment as documented.
  • Startup speedgo:embed index.html, no init-time network/disk/subprocess work; harness hub is a separate subcommand off the hot path.
  • Tests — no raw time.Sleep for synchronization in the hub tests; TestRunSpawnCancelKillsProcess uses the emitted stdout event as the readiness signal and the done channel as the kill proof. The goal_paused_test.go change adds a srv2.Drain(...) to drain the in-flight prompt before TempDir cleanup — correct fix for the journal-write-vs-RemoveAll race.

Nit (non-blocking)

  • handleIndex still sets no Content-Security-Policy / X-Content-Type-Options: nosniff. The page holds run tokens in its URL fragment, so a CSP (default-src 'self'; connect-src *) would be cheap defense-in-depth. Purely optional.

Net: one medium CSRF finding to address (or explicitly accept, given the single-operator local-tool threat model) before merge; everything else is clean and well-tested.

Comment thread tools/hub/hub.go
POST /spawn execs the deployment provision command via sh -c — a real,
costly side effect — with no auth and no Origin check. Loopback binding did
not help: any page the operator visited could
fetch("http://localhost:7777/spawn",{method:"POST"}) as a no-preflight
CORS simple request and trigger a box spawn (the attacker cannot read the
SSE response, but the exec has already fired; a missing/malformed body is
tolerated, so no valid JSON is even needed).

Gate the handler on Origin-vs-Host before any exec (OWASP's verify-origin
pattern): a browser attaches Origin to every cross-origin POST, so an
attacker page's Origin never matches the hub's Host and is rejected 403,
while the page's own same-origin fetch("/spawn") (Origin == Host) and
non-browser clients (no Origin — not a CSRF vector) pass unchanged.
@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown
📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 3m 11s —— View job


Review — PR #75 development hub

  • Read AGENTS.md + gather diff context
  • Verify the new CSRF guard on POST /spawn (latest commit)
  • Review tools/hub/hub.go, spawn.go
  • Review index.html / /term revert
  • Verify tests (red-verified the CSRF regression)
  • Post review

Re-reviewed against the current tip (9c9d972). The one substantive finding from the previous four rounds — CSRF on POST /spawn — is now fixed, and correctly. With that resolved, this PR looks ready to merge.

The CSRF fix is correct and well-tested

isCrossOrigin (tools/hub/hub.go:91) implements OWASP's verify-origin pattern cleanly, gating the exec before any side effect (hub.go:113):

  • Attacker page → rejected. A browser attaches Origin to every cross-origin POST and can't spoof it, so the attacker's origin never matches r.Host → 403 before runSpawn execs.
  • Real page → transparent. Same-origin fetch sends Origin == Host → passes.
  • Non-browser clients → pass. No Origin header (curl, scripts) means no ambient browser credentials, so not a CSRF vector — correctly allowed.
  • Edge cases handled explicitly. The opaque "null" origin (sandboxed/file:// contexts) and unparseable origins fall to the safe side (u.Host == "" → treated as cross-origin), and TestIsCrossOrigin covers the different-port and unparseable cases.

I red-verified the regression per AGENTS.md: neutralizing the guard flips TestHandleSpawnRejectsCrossOrigin to status = 200, want 403, and restoring it goes green. Full go test -race ./tools/hub/ passes and go build ./... is clean.

Other surfaces confirmed solid this round

  • /term revert is clean — no dangling term/xterm references anywhere in server/, tools/, or cmd/; the deferred xterm pane is correctly absent from the tree.
  • runSpawn (spawn.go:85) — correct StdoutPipe→scan→Wait ordering; combined stdout+stderr; 1 MiB scanner buffer; always emits exactly one terminal done; CommandContext kills the process on client disconnect; empty name leaves the env untouched (no empty-valued HARNESS_HUB_BOX_NAME).
  • Addr bindingresolveAddr keeps the loopback-by-default promise; spawnCommandFromEnv distinguishes flag-set-empty (disable) from flag-absent (env fallback) via fs.Visit.
  • XSS / startup speed / tests — unchanged from the prior clean rounds: el() renders via textContent, no live-data innerHTML sink, no init-time network/disk/subprocess work, no raw time.Sleep for synchronization.

Nit (non-blocking, unchanged)

  • handleIndex (hub.go:54) still sets no Content-Security-Policy / X-Content-Type-Options: nosniff. The page holds run tokens in its URL fragment, so a CSP (default-src 'self'; connect-src *) would be cheap defense-in-depth — purely optional, and safe to defer.

Net: the blocking finding is resolved and verified. LGTM.

The hub page holds run tokens in its URL fragment, so add a strict CSP
response header on the served index: default-src 'none' blocks every
external resource load (the page is a single inline no-build file, so
'unsafe-inline' covers its own script/style — a per-response nonce/hash is
not viable on a byte-for-byte go:embed), connect-src * permits the fetch/SSE
to arbitrary operator-added box origins the stateless hub cannot enumerate,
and frame-ancestors/base-uri/form-action are pinned to 'none'. Verified in a
real browser: page renders, inline script/style run, same-origin /spawn and
cross-origin box fetches are permitted, no CSP violations. Documents the
CSRF and CSP hardening in AGENTS.md's Development hub section.
@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown

Claude finished @andybons's task in 1m 36s —— View job


Review — PR #75 development hub

  • Read AGENTS.md + gather diff context
  • Review the two latest commits (CSRF guard, CSP header)
  • Re-check tools/hub/hub.go, spawn.go, index.html
  • Review tests
  • Post review

Re-reviewed against the current tip (fe28660). The two remaining nits from the prior LGTM round are now both resolved, and correctly. Nothing new to flag — this is ready to merge.

The CSP header is correct

handleIndex (hub.go:77) now sets a strict Content-Security-Policy on the served page. The policy is well-reasoned for this specific page:

  • default-src 'none' blocks every external resource load — right, since the page is a single go:embed'd file with zero external references.
  • script-src/style-src 'unsafe-inline' is the only viable choice for a byte-for-byte, no-build embedded file (no per-response nonce/hash hook), and the comment says so.
  • connect-src * is genuinely required: the stateless hub cannot enumerate the arbitrary operator-added box origins the page fetches/streams from, so it can't be tightened to an allowlist. * still covers the same-origin /spawn SSE.
  • frame-ancestors/base-uri/form-action 'none' are pinned explicitly (they don't inherit from default-src) — clickjacking + base-tag + form-hijack all closed. Good defense-in-depth for a page holding run tokens in its URL fragment.

TestHandleIndexSetsCSP asserts each directive is present.

The CSRF guard is correct (confirmed from prior round)

isCrossOrigin (hub.go:104) gates the exec at hub.go:127, before any side effect. Edge cases are handled the safe way — unparseable Origin and the opaque "null" origin both fall to u.Host == "" → treated as cross-origin (TestIsCrossOrigin covers different-host, different-port, null, and unparseable). Absent Origin (non-browser callers, no ambient credentials) passes; same-origin passes. Transparent to the real page.

Verified this round

  • go build ./... clean; go test ./tools/hub/ passes.
  • The /term PTY endpoint and vendored xterm remain reverted (commit aacc771) — no dangling references; the deferred xterm pane is correctly absent from the tree, matching the PR body's "known follow-up".
  • runSpawn (spawn.go:85) unchanged and still solid: correct StdoutPipe→scan→Wait ordering, combined stdout+stderr, 1 MiB scanner buffer, exactly one terminal done, CommandContext kills the process on client disconnect, and empty name leaves the env untouched (no empty-valued HARNESS_HUB_BOX_NAME).
  • XSS / startup-speed surfaces unchanged from the prior clean rounds: el() renders via textContent, no live-data innerHTML sink, no init-time network/disk/subprocess work, harness hub off the hot path.

Net: both outstanding nits are addressed; no blocking or new findings. LGTM.

@andybons andybons changed the title feat(hub): development hub — fleet dashboard, goal dispatch, and /term backend feat(hub): development hub — fleet dashboard, goal dispatch, and box spawning Jul 13, 2026
@andybons andybons merged commit 83082e2 into main Jul 13, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant