API Middleware Tests and Cleanup

.github/workflows/ci.yml

@@ -49,6 +49,12 @@ jobs:
       - uses: DeterminateSystems/nix-installer-action@main
       - uses: DeterminateSystems/magic-nix-cache-action@main
 
+      # The socket tests send real multicast over loopback, which Ubuntu
+      # ships with multicast off. They fail rather than skip under CI, so a
+      # missing step here can't hide as a skip.
+      - name: Enable loopback multicast
+        run: sudo scripts/lo-multicast.sh on
+
       - name: Format, vet, build, test
         run: |
           nix develop --command bash -c '
@@ -58,6 +64,10 @@ jobs:
             make vet
             make build
             make go-test
+            # Committed generated files (doc comments, openapi.json, alert
+            # codes) must match the code.
+            make generate
+            git diff --exit-code
           '
 
   frontend:
@@ -81,3 +91,32 @@ jobs:
             make frontend-test
             make frontend
           '
+
+  gui-e2e:
+    name: End-to-end (gui)
+    runs-on: ubuntu-latest
+    steps:
+      # Same prerequisites as gui-go: generated protobuf and the frontend
+      # build, both via nix. The browser is the runner's own Google Chrome
+      # (Playwright's "chrome" channel), so nothing is downloaded for it.
+      - uses: actions/checkout@v4
+        with:
+          submodules: recursive
+
+      - uses: DeterminateSystems/nix-installer-action@main
+      - uses: DeterminateSystems/magic-nix-cache-action@main
+
+      - name: Smoke tests
+        run: |
+          nix develop --command bash -c '
+            cd gui
+            make e2e
+          '
+
+      - name: Upload traces
+        if: failure()
+        uses: actions/upload-artifact@v4
+        with:
+          name: playwright-traces
+          path: gui/frontend/test-results/
+          retention-days: 7

gui/ARCHITECTURE.md

@@ -84,6 +84,7 @@ before returning.
 | `internal/config`                            | `vision.yml`: the working document, save and load, locked calibrations, the disk watcher, and generated `config.yml` files.             |
 | `internal/geometry`                          | The live field template, published calibrations, and the 1Hz publish loop. Holds no file state.                                         |
 | `internal/multicast`                         | The vision, game controller, and video sockets: interface selection, reopening on changes, retrying while unhealthy, suspend detection. |
+| `internal/api`                               | Typed route registration and the OpenAPI 3.1 spec reflected from the handlers' own types and doc comments.                              |
 | `internal/hub`                               | In-process topic pub/sub and the `/ws` WebSocket handler.                                                                               |
 | `internal/detections`                        | Which camera_ids the vision socket hears, from which address, at what rate. Feeds the Camera Layout page.                               |
 | `internal/referee`                           | The current match's teams, from the game controller, and their robot heights from `bot_heights_file`.                                   |
@@ -193,31 +194,64 @@ JSON float and YAML decodes integers as ints, so without this every edit would s
 
 ## HTTP API
 
-All routes are registered in `cmd/ssl-vision-processor-gui/routes.go`.
-
-| Method | Path                                   | Purpose                                                                             |
-| ------ | -------------------------------------- | ----------------------------------------------------------------------------------- |
-| GET    | `/api/health`                          | Liveness check.                                                                     |
-| GET    | `/api/geometry`                        | The full wrapper packet, as canonical protojson.                                    |
-| GET    | `/api/geometry/presets`                | The rulebook presets, read live from `config/legacy/geometry-div{A,B}.yml`.         |
-| GET    | `/api/config`                          | The working document and the store's state.                                         |
-| PUT    | `/api/config`                          | Replace the working document and apply it live. 409 on a stale revision.            |
-| POST   | `/api/config/save`                     | Write the working document to disk. 409 if the file changed on disk, unless forced. |
-| POST   | `/api/config/save-as`                  | Write to a new path and edit that file from then on.                                |
-| POST   | `/api/config/load`                     | Load another file, discarding unsaved changes.                                      |
-| POST   | `/api/config/reload`                   | Accept the file as it is on disk, discarding unsaved changes.                       |
-| POST   | `/api/config/cameras/{id}/calibration` | Lock the camera's latest live calibration.                                          |
-| DELETE | `/api/config/cameras/{id}/calibration` | Unlock it and stop publishing any calibration for the camera.                       |
-| GET    | `/api/camera/devices`                  | This host's capture devices: stable by-id and by-path links, then raw nodes.        |
-| GET    | `/api/snapshots`                       | List of debug images currently on disk.                                             |
-| GET    | `/api/snapshot/{camID}/{view}`         | One debug image.                                                                    |
-| GET    | `/ws`                                  | WebSocket, see below.                                                               |
-| GET    | `/ws/video/{id}?mode=full\|keyframes`  | One camera's live video, see Live video below.                                      |
+Every route is registered in `cmd/ssl-vision-processor-gui/routes.go` through `internal/api`, which builds an
+OpenAPI 3.1 spec from each handler's request and response types, and its summary, descriptions and field docs
+from their Go doc comments. The host serves the spec at `GET /api/openapi.json` and a reference for it at
+`GET /api/docs` (Redoc, embedded, so it works offline), linked from the GUI's settings menu. `gui/openapi.json`
+is the committed copy; `make generate` rewrites it, and CI fails when it, `internal/api/comments_gen.go`, the
+frontend's generated API types (`frontend/src/lib/generated/api.ts`, from the spec by openapi-typescript) or the
+alert codes are stale. The frontend imports its API types from `frontend/src/lib/apiTypes.ts`, never a hand copy.
+
+A handler takes one input struct and returns one output. Input fields tagged `path:`, `query:` or `header:` are
+parameters, and a field tagged `body:"<content types>"` is the body; an input with only JSON fields is itself the
+body. Output fields tagged `header:` are response headers. Each route lists the error codes it can return, which
+the spec shows per status; the `cmd` tests fail if any route answers a code it doesn't list. Routes whose
+responses aren't one JSON value (the snapshot image, the two WebSocket upgrades) use `api.Raw` and are
+documented the same way.
 
 An unrouted `/api/*` path returns 404 rather than falling through to the frontend, so a typo'd endpoint fails
 with a clear status instead of returning HTML to a caller expecting JSON. Every other path serves the embedded
 frontend, with a fallback to `index.html` for client side routes.
 
+Every error is JSON, `{"error": code, "message": text, "issues": [...]}` (`internal/httpjson`). Clients branch
+on `error`: `bad_request`, `invalid` (with one `issues` entry per problem), `not_found`, `stale`,
+`disk_changed`, `no_live_calibration`, `precondition`, `unsupported_media`, `cross_origin`, `internal`.
+Request bodies must be `Content-Type: application/json` (PATCH also takes `application/merge-patch+json`), and
+unknown keys are rejected, so a misspelled key fails instead of silently resetting the value it named.
+
+`GET /api/config` returns the revision as its `ETag`, and every config write (PUT, PATCH, save, save-as, load,
+reload, lock, unlock) answers `{"revision": n}` with the new revision as its `ETag`. PATCH follows RFC 7396: objects
+merge, `null` removes a key, and arrays (`cameras`) are replaced whole. PATCH is the safer write for a client that
+doesn't model the whole document: PUT replaces it, so a key the client dropped is removed.
+
+Some document keys only matter to this GUI and no vision_processor reads them: a camera's `display` (video
+orientation) and `seed.goalSideMarker` (which corner marker was picked). Other clients should leave them as they
+are. `teams` and `competitionField` are read by the host's alerts. Each `Change` in `config.state` has both `path`
+(vision.yml's spelling, `field.field_width`) and `jsonPath` (the JSON document's, `field.fieldWidth`); in both,
+`cameras[N]` is the camera with camera_id N, not a list index. `section` is the GUI page the change belongs to. Protobuf messages appear in two spellings: `/api/geometry` and
+`wrapper_packet.out` use canonical protojson (`pixelImageWidth`), while the document's `models` and
+`calibration.camera` keep proto field names (`pixel_image_width`), matching how they're stored in vision.yml.
+
+"Host-local" endpoints describe the machine running the host, and only describe a camera when its
+vision_processor runs on the same machine: device paths and debug images aren't available for remote instances.
+
+Browser pages from other origins are handled by `internal/origins`. The host's own pages are always allowed. In a
+lab, so is any page served from this network: `localhost`, a loopback, private or link-local IP, a `.local` name, or
+a single-label name such as `laptop`. No public website can have such an origin, because the browser sets `Origin`
+from the page's real address. On a competition field (`competitionField`, switched live) only the host's own pages
+and the `-allowOrigin http://host:port,...` list are allowed; `*` allows any. A page that isn't allowed can't change
+state (the `http.CrossOriginProtection` test: `Sec-Fetch-Site`, else `Origin` against `Host`) or open `/ws` and
+`/ws/video`. An allowed page from another origin gets CORS headers (preflights answered, `ETag` exposed). Clients
+outside a browser send no `Origin` and are unaffected.
+
+Every request must also name the host by something it is: any IP literal, `localhost`, a `.local` or single-label
+name, its hostname, the names its addresses reverse-resolve to, or a `-allowHost` alias (`*` turns the check off).
+Anything else gets 421 `unknown_host`. This stops DNS rebinding, where an attacker points their own domain at this
+host so the browser treats their page as same-origin.
+
+There is no authentication: anyone who can reach the port, including another team on a venue network using this
+UI, can administer the host.
+
 ## WebSocket protocol
 
 `/ws` is a single connection carrying a topic based publish/subscribe protocol, implemented in `internal/hub`.
@@ -228,15 +262,24 @@ frontend, with a fallback to `index.html` for client side routes.
 { "action": "unsubscribe", "topic": "wrapper_packet.out" }
 // server -> client
 { "topic": "wrapper_packet.out", "data": { /* protojson */ } }
+{ "error": "unknown_topic", "message": "...", "topic": "wrapper_pakcet.out" }
 ```
 
+`GET /api/topics` lists the topics. A new subscriber gets the topic's latest value at once, then every later one.
+A message the server can't act on is answered with an error (`bad_request`, `unknown_topic`, `unknown_action`)
+and the connection stays open.
+
 `wrapper_packet.out` carries the current `SSL_WrapperPacket` as canonical protojson, published once per second.
 `config.state` carries `config.Store`'s state: the revision, unsaved changes, any change made to the file on disk,
 and per camera calibration status. It is published on every change and once per second, since live calibration
 status comes from the network rather than the store.
 `network.state` carries each multicast socket's address and what it has heard there: vision detection packets
 (the host's own looped back geometry doesn't count) and game controller referee messages. It is published once per
 second and on every config change.
+`alerts.state` carries every problem the host can see (`internal/alerts`), on the same schedule: a stable `id`,
+a `code`, a `severity` (`error`, `warning`, `caution`), an optional `cameraId`, the facts behind it as `params`,
+and a plain English `message`. The host decides which alerts exist and how serious they are; a client words and
+links them by `code`, falling back to `message` for codes it doesn't know.
 
 Every channel in this path, from a topic's own subscriber channel to a connection's shared outbound channel, is
 size limited and drops the oldest queued value in favor of the newest one. A slow client sees only the latest

gui/CLAUDE.md

@@ -16,6 +16,8 @@ All run from `gui/`.
 ```
 make run       # regenerate proto, build frontend, run the Go host on :8085 (-config vision.yml)
 make test      # frontend check/lint/format + go test -race, all packages
+make e2e       # Playwright smoke tests (frontend/e2e/): real host, installed Google Chrome
+make generate  # rewrite generated files: comments_gen.go, openapi.json, the frontend's API types, alert codes
 make install   # go install after a frontend build
 make proto     # regenerate internal/{vision,gamecontroller} and frontend/src/proto
 make clean     # wipe dist/, internal/{vision,gamecontroller}, frontend/src/proto
@@ -44,6 +46,7 @@ internal/
   geometry/    live field template + published calibrations + 1Hz publish loop
   multicast/   vision + game controller sockets, interface selection,
                reopened on address/interface changes
+  api/         typed routes + the OpenAPI spec (/api/openapi.json, /api/docs)
   hub/         topic pub/sub + the /ws handler
   snapshot/    debug image listing/serving
   video/       live video: RTP H.264 -> fMP4 over /ws/video/{id} for MSE
@@ -171,7 +174,7 @@ without this every edit diffs as `1920 -> 1920`.
 **buf reads the proto submodule directly**: `inputs: [{directory:
 ../proto/proto}]` in `buf.gen.yaml`. No vendored copy to drift. Generated
 output (`internal/vision`, `internal/gamecontroller`, `frontend/src/proto`) is
-*not* committed -- `buf.gen.yaml`'s plugins are both `local:` (`go tool
+_not_ committed -- `buf.gen.yaml`'s plugins are both `local:` (`go tool
 protoc-gen-go`; `frontend/node_modules/.bin/protoc-gen-es`), not buf.build
 remotes, so `make proto` needs no network beyond the already-checked-out
 `proto/` submodule and runs as an ordinary Makefile prerequisite (a `.proto`
@@ -183,7 +186,7 @@ the reason to.
 `gui/frontend/dist/.gitkeep` exists for a similar but distinct reason on the
 frontend side -- `//go:embed` fails to compile if `dist/` doesn't exist, so a
 fresh clone with no `npm run build` yet must still be able to `go test`. That
-one stays committed: `dist/` is the frontend's actual build *output*, not
+one stays committed: `dist/` is the frontend's actual build _output_, not
 codegen from a source of truth already in the tree, so there's nothing to
 regenerate it from without first running the build it's the output of.
 
@@ -243,6 +246,36 @@ calibrated against a real skewed webcam view, and the calibration came back
 and was absorbed with no wire-format surprises between C++ and Go protobuf.
 Not automated -- there is no CI hardware to run it on.
 
+Routes go through `api.Route` (typed input and output) or `api.Raw`, never
+`mux.Handle`, so the spec stays complete. A handler's doc comment is its API
+summary (first sentence) and description: write it for API readers, keep the
+first sentence short, and run `make generate` after changing it. List every
+error code a route can return in its `api.Doc`; the `cmd` tests fail otherwise.
+
+The frontend's API types are generated from `openapi.json` into
+`frontend/src/lib/generated/api.ts` (`npm run gen:api`, part of `make
+generate`). Import them by name from `frontend/src/lib/apiTypes.ts`; never
+write a TS copy of a Go type. For the generated types to be exact, a Go
+response never sends `null` for a slice, array or map, or for an `omitempty`
+field, and fixed-value strings carry an `enum:` tag. `responses_test.go`
+checks real responses against the spec for that.
+
+`frontend/src/lib/alerts/alert-codes.json` is generated by
+`internal/alerts`' tests and read by `alerts.test.ts`, which checks the GUI
+words every alert code the host can raise. After adding a code, regenerate it
+with `go test ./internal/alerts -run Codes -update`.
+
+Real multicast is tested over loopback (`internal/multicast/loopback_test.go`,
+`internal/video/loopback_test.go`, `cmd/.../realsockets_test.go`, helpers in
+`internal/multicast/mctest`). Ubuntu ships loopback with multicast off, so
+these skip until `../scripts/lo-multicast.sh on`; under `CI` they fail
+instead, and the workflow turns it on first.
+
+`make e2e` runs `frontend/e2e/` with Playwright against the real host on a
+scratch copy of `cmd/ssl-vision-processor-gui/testdata/vision.yml`, in the
+installed Google Chrome (no browser download). A `test.fail()` there marks a
+known bug: when it starts passing, the bug is fixed and the marker comes out.
+
 ## Not yet built
 
 - **`internal/discovery`** -- parsing `SSL_VPConfig` announces off multicast

gui/Makefile

@@ -11,7 +11,7 @@ FRONTEND_SOURCES = $(shell find frontend -type f \
 
 PROTO_SOURCES = $(shell find ../proto/proto -type f 2>/dev/null)
 
-.PHONY: all frontend install test go-test frontend-test vet build run proto clean
+.PHONY: all frontend install test go-test frontend-test e2e generate redoc vet build run proto clean
 
 all: install
 
@@ -59,6 +59,33 @@ go-test: frontend
 frontend-test: frontend
 	cd frontend && npm run check && npm run lint && npm run check:text && npm test && npm run format:check
 
+# End-to-end smoke tests: the real host, serving the built frontend, driven in
+# the installed Google Chrome by Playwright (frontend/e2e/). The binary goes to
+# a temp dir so a stale one in the tree is never what's tested.
+E2E_BIN := $(shell mktemp -u -t vision-gui-e2e-bin.XXXXXX)
+
+e2e: frontend
+	go build -o $(E2E_BIN) ./cmd/$(BINARY)
+	cd frontend && GUI_BIN=$(E2E_BIN) npx playwright test; status=$$?; rm -f $(E2E_BIN); exit $$status
+
+# Regenerates every committed file derived from the code: the doc comments the
+# OpenAPI spec uses, the spec itself (openapi.json), and the alert codes the
+# frontend's tests check. CI runs this and fails on any diff.
+generate: frontend
+	cd internal/api && go run ./gen
+	go test ./cmd/$(BINARY) -run OpenAPISpecIsCurrent -update
+	cd frontend && npm run gen:api
+	go test ./internal/alerts -run CodesFileIsCurrent -update
+
+# Refreshes the embedded API docs viewer (Redoc's standalone bundle, MIT).
+REDOC_VERSION = 2.5.1
+
+redoc:
+	cd $$(mktemp -d) && npm pack redoc@$(REDOC_VERSION) --silent && \
+		tar -xzf redoc-$(REDOC_VERSION).tgz package/LICENSE package/bundles/redoc.standalone.js package/bundles/redoc.standalone.js.LICENSE.txt && \
+		cp package/bundles/redoc.standalone.js package/bundles/redoc.standalone.js.LICENSE.txt $(CURDIR)/internal/api/viewer/ && \
+		cp package/LICENSE $(CURDIR)/internal/api/viewer/REDOC-LICENSE
+
 # CI runs go-test and frontend-test as separate parallel jobs (see
 # .github/workflows/ci.yml) -- this is the single "everything before you
 # push" target for local use, composed of exactly those same two so the