diff --git a/AGENTS.md b/AGENTS.md index 17f1eb8..4cc206a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,7 +11,7 @@ from an agent, see [`skills/fleet-runner/SKILL.md`](skills/fleet-runner/SKILL.md | `collector/` | The brain: Fastify + `node:sqlite`, the queue, the dashboard (`dash/`), the host executor | | `collector/src/workloads/explore/` | Exploratory QA: a vision model drives an app from mission cards (`collector/examples/missions/`); see `docs/explore/internals.md` | | `collector/src/api/v1/` | The control API. `routes.ts` is the single route table: it registers the routes, generates `openapi.json` and the typed client | -| `collector/src/client/` | The generated client (`api.gen.ts`) and its transport. The CLI and the MCP server use nothing else | +| `collector/src/client/` | The generated client (`api.gen.ts`) and its transport. The CLI and the MCP server use nothing else; the dashboard (`dash/src/client.ts`) uses it for everything `/api/v1` offers, and `scripts/check-dash-requests.ts` fails the suite if it calls a deprecated route | | `fleet/` | The `fleet` / `fleetr` command: runs a brain or a runner, plus the work commands, `fleet mcp` and the remote gateway | | `desktop/` | A Tauri menu-bar app around `fleet`. A front door, not a runner; compiled in CI on macOS and launched in CI on Linux (`desktop/ci/launch-smoke.sh`), never run on a Mac | | `runner-machine/`, `runner-android/`, `runner-ios/`, `runner-roku/`, `collector/runner-web/` | Runners. They speak the runner protocol (`/devices/register`, `/devices/:id/next-job`, `/results`), not `/api/v1` | diff --git a/CHANGELOG.md b/CHANGELOG.md index 0743d71..a891b37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -174,6 +174,24 @@ a CI job fails when they disagree. `npm run shoot:live`, filtered to what was online and to the `bug-garden` fixture app; docs/brand.md says why a throwaway collector cannot make them. ([#90](https://github.com/addisdev/fleet-runner/pull/90)) +- **The dashboard reads devices, jobs, templates and schedules through + `/api/v1`.** It called the deprecated `GET /api/devices`, `/api/jobs` and + their detail routes, and enqueued with `POST /api/jobs`, with response types + written by hand, so the server could change a field and the dashboard would + show a blank column with nothing failing. It now uses the generated client + for every read and change `/api/v1` offers, and its types are the client's. + The `/api/v1` schemas state every field the dashboard reads, so + `check-api-v1` fails when a handler stops sending one, and a new suite step + fails if `dash/src` calls a route the deprecation headers mark. To carry the + Devices and Jobs filter menus, `GET /api/v1/devices` and `/api/v1/jobs` now + return `facets`, and `/api/v1/jobs` takes `has_error=true`. Two things a + person can see: the composer's spec is checked strictly, so an unknown + top-level field or params outside the workload's manifest is refused with + the path named instead of failing later, and its jobs record `source: dash`. + Enqueueing is no longer behind `FLEET_DASH_TOKEN`, as `POST /jobs` never + was. Findings, the Overview, results, system, events, the live stream and the + other changes stay on `/api/*` until `/api/v1` has them. + ([#97](https://github.com/addisdev/fleet-runner/pull/97)) ### Fixed @@ -288,6 +306,12 @@ a CI job fails when they disagree. case-sensitively where the harness does not. It now reads `absent_text` and matches case-insensitively, as the bug garden's README says. ([#88](https://github.com/addisdev/fleet-runner/pull/88)) +- **`/api/v1` cancel and retry were always refused with `FLEET_DASH_TOKEN` + set.** They hand the request to the guarded `/api/jobs/:id/cancel` and + `/retry`, and dropped `X-Fleet-Token` on the way, so with the guard on every + one answered 401 whatever the caller sent. The header is now carried, and + the refusal says `unauthorized` rather than `internal`. + ([#97](https://github.com/addisdev/fleet-runner/pull/97)) ### Now verified diff --git a/collector/dash/src/TokenBanner.tsx b/collector/dash/src/TokenBanner.tsx index 6697f8d..792f520 100644 --- a/collector/dash/src/TokenBanner.tsx +++ b/collector/dash/src/TokenBanner.tsx @@ -3,15 +3,16 @@ // // The alternative is what happened before it existed: you click something // ordinary, like renaming a device, and get a 401 naming an HTTP header. The -// requirement is knowable up front — /api/health reports it — so say so up +// requirement is knowable up front — /api/v1/health reports it — so say so up // front, and take the token right here rather than sending someone to another // screen to find the field. import { useState } from "preact/hooks"; -import { useApi, type Health } from "./api.js"; +import { useLoad } from "./api.js"; +import { fleet, read } from "./client.js"; import { useToken } from "./mutate.js"; export function TokenBanner() { - const health = useApi("/api/health", [], 0); + const health = useLoad("health", () => read(fleet.getHealth()), [], 0); const [token, setToken] = useToken(); const [draft, setDraft] = useState(""); diff --git a/collector/dash/src/api.ts b/collector/dash/src/api.ts index 2e13d0a..35cd59b 100644 --- a/collector/dash/src/api.ts +++ b/collector/dash/src/api.ts @@ -2,6 +2,7 @@ import { useEffect, useRef, useState } from "preact/hooks"; import { onLive } from "./live.js"; import { tokenHeader } from "./token.js"; +import type { JobDetail } from "./client.js"; export class ApiError extends Error { constructor( @@ -38,18 +39,31 @@ export type Loadable = { data: T | null; error: Error | null; loading: boolea * of "3m ago" and lease countdowns goes stale on its own, with no event to say so. */ export function useApi(path: string | null, topics: string[] = [], refreshMs = 0): Loadable { + return useLoad(path, () => api(path!), topics, refreshMs); +} + +/** + * useApi for anything that returns a promise -- the generated /api/v1 client, + * mostly. `key` stands in for the path: it changes exactly when the request + * would, and null means "not yet". + */ +export function useLoad(key: string | null, load: () => Promise, topics: string[] = [], refreshMs = 0): Loadable { const [data, setData] = useState(null); const [error, setError] = useState(null); - const [loading, setLoading] = useState(path !== null); + const [loading, setLoading] = useState(key !== null); const [nonce, setNonce] = useState(0); // Guards against a slow first response overwriting a fast second one. const seq = useRef(0); + // The loader closes over the render that made it; the effect below runs on + // key changes only, so it reads the newest one from here. + const loader = useRef(load); + loader.current = load; useEffect(() => { - if (path === null) return; + if (key === null) return; const mine = ++seq.current; setLoading(true); - api(path) + loader.current() .then((d) => { if (seq.current === mine) { setData(d); @@ -62,7 +76,7 @@ export function useApi(path: string | null, topics: string[] = [], refreshMs .finally(() => { if (seq.current === mine) setLoading(false); }); - }, [path, nonce]); + }, [key, nonce]); const reload = () => setNonce((n) => n + 1); @@ -88,28 +102,16 @@ export function useApi(path: string | null, topics: string[] = [], refreshMs return { data, error, loading, reload }; } -// --- response shapes (mirrors src/api/*.ts) --- - -export type DeviceStatus = "online" | "stale" | "offline"; +// --- response shapes --- +// +// Devices, jobs and their details come from /api/v1, so their types are the +// generated client's (collector/src/client/api.gen.ts), re-exported here for +// the pages that import them from this file. Do not redeclare them: a copy is +// how the dashboard drifted from the server before. +export type { Device, DeviceDetail, DeviceList, Job, JobDetail, JobList } from "./client.js"; +export type ResultRow = JobDetail["results"][number]; -export type Beacon = { - battery_pct: number | null; - charging: boolean | null; - thermal: string | null; - mem_mb: number | null; - mem_method: string | null; - process_alive: boolean | null; - job_id: string | null; - /** - * Set when this device is running a job for a DIFFERENT collector. - * - * Rendered rather than ignored because "busy for another brain" and "idle, - * and nothing here is offering it work" look identical on this screen and are - * opposite problems -- one is working, the other is a queue that is not - * matching. - */ - busy: { job_id: string | null; collector: string | null } | null; -} | null; +// The rest mirror unversioned /api/* routes that /api/v1 does not offer yet. export type Peer = { url: string; @@ -126,76 +128,6 @@ export type Peers = { configured: number; }; -export type Device = { - device_id: string; - name: string | null; - notes: string | null; - descriptor: Record; - /** Effective pools: the override when set, otherwise what the runner reports. */ - pools: string[]; - /** null = an agent that predates capability routing; it is offered everything. */ - capabilities: string[] | null; - /** /24 the agent last registered from. Distinguishes "offline" from "elsewhere". */ - last_net: string | null; - pools_reported: string[]; - pools_override: string[] | null; - /** - * What the agent says it runs. An open string, not a union: the fleet gained - * a laptop, a TV, a watch and a browser tab after this type was written, and - * a union here would have had to be edited before any of them could show up - * correctly. Agents that predate the field are still reported as ios/android - * by the collector's fallback. - */ - platform: string; - /** Form factor, when the agent declared one: phone, tablet, laptop, tv, watch, sbc, browser… */ - kind: string | null; - /** Seconds of silence an ephemeral agent survives; null for a shelf device. */ - ttl_s: number | null; - /** True once an ephemeral agent's window closed. Such devices are hidden unless asked for. */ - expired: boolean; - simulator: boolean; - status: DeviceStatus; - age_s: number | null; - last_seen: string | null; - beacon: Beacon; - current_job: string | null; - lock: { job_id: string; acquired_at: string | null } | null; -}; - -export type Job = { - job_id: string; - workload: string; - executor: "device" | "host"; - /** `skipped`: claimed and closed without running anything (no device matched, - * the app was not installed, a dependency skipped). Not a pass. */ - status: "waiting" | "queued" | "claimed" | "done" | "failed" | "cancelled" | "skipped"; - /** Job ids this one waits for; it stays `waiting` until they are all done. */ - depends_on?: string[] | null; - /** Whether a higher-priority job may ask this one to checkpoint and step aside. */ - preemptible?: boolean; - claimed_by: string | null; - attempts: number; - max_attempts: number; - priority: number; - template_id: string | null; - lease_ttl_s: number; - lease_deadline: string | null; - lease_remaining_s: number | null; - duration_s: number | null; - last_error: string | null; - created_at: string | null; - claimed_at: string | null; - finished_at: string | null; - pool: string | null; - match: string | null; - device_id: string | null; - exclusive: boolean; - backend: string | null; - model: string | null; - app: string | null; - report_to: Record | null; -}; - export type Overview = { generated_at: string; devices: { @@ -252,38 +184,6 @@ export type Overview = { health: Health; }; -export type DeviceList = { - total: number; - /** Facets over every device, not the filtered set, so a filter never empties its own menu. */ - pools: string[]; - platforms: string[]; - kinds: string[]; - devices: Device[]; -}; - -export type DeviceDetail = Device & { - counts: { results: number; beacons: number }; - jobs: { - job_id: string; - workload: string; - executor: string; - status: Job["status"]; - created_at: string | null; - finished_at: string | null; - attempts: number; - max_attempts: number; - last_error: string | null; - }[]; - benchmarks: { - job_id: string; - at: string | null; - model: string; - quant: string | null; - backend: string; - metrics: Record | null; - }[]; -}; - export type BeaconSample = { ts: string | null; battery_pct: number | null; @@ -297,44 +197,6 @@ export type BeaconSample = { export type BeaconHistory = { device_id: string; hours: number; count: number; samples: BeaconSample[] }; -export type JobList = { - page: number; - per_page: number; - total: number; - pages: number; - status_counts: Record; - workloads: string[]; - pools: string[]; - jobs: Job[]; -}; - -export type ResultRow = { - device_id: string; - iter: number; - created_at: string | null; - payload: Record; -}; - -export type JobDetail = Job & { - spec: Record; - parent: string | null; - children: Job[]; - siblings: Job[]; - results: ResultRow[]; - beacons: (BeaconSample & { device_id: string })[]; - artifacts: { - sha256: string; - name: string | null; - size: number | null; - created_at: string | null; - in_store: boolean; - role: "input" | "output"; - }[]; - locks: { device_id: string; acquired_at: string | null }[]; - status_report: { target: string; state: string; posted: boolean; detail: string | null; created_at: string | null } | null; - derived_timeline: { at: string | null; what: string }[]; -}; - export type RecentResults = { results: { job_id: string; @@ -351,6 +213,7 @@ export type RecentResults = { export type Locks = { locks: { device_id: string; job_id: string; acquired_at: string | null; held_s: number }[] }; +/** The health block inside /api/overview. The token banner reads /api/v1/health instead. */ export type Health = { ok: boolean; instance: string; diff --git a/collector/dash/src/client.ts b/collector/dash/src/client.ts new file mode 100644 index 0000000..0be0e04 --- /dev/null +++ b/collector/dash/src/client.ts @@ -0,0 +1,64 @@ +// The dashboard's door into /api/v1: the generated client, with the dashboard's +// own way of carrying a token. +// +// The client and its types are generated from the route table, so a field the +// server renames is a compile error here rather than a blank column. Where the +// dashboard needs something /api/v1 does not offer, the page still calls the +// unversioned /api/* route through api.ts or mutate.ts. +import { FleetApiError, FleetClient } from "../../src/client/index.js"; +import type { DeviceList, JobSpecInput } from "../../src/client/index.js"; +import { getToken, tokenHeader } from "./token.js"; + +export type * from "../../src/client/index.js"; + +/** + * Same origin, and the token as `X-Fleet-Token` read at call time: that is the + * header FLEET_DASH_TOKEN's guard checks, and FLEET_AUTH=tokens accepts it as + * readily as `Authorization: Bearer`. The banner can change the token at any + * moment, so it is not baked into the client. + */ +export const fleet = new FleetClient({ + baseUrl: location.origin, + source: "dash", + fetch: (input, init) => + fetch(input, { + ...init, + headers: { accept: "application/json", ...(init?.headers as Record | undefined), ...tokenHeader() }, + }), +}); + +/** Turn a v1 refusal into the sentence a person should read. */ +function explain(e: unknown, mutation: boolean): Error { + if (!(e instanceof FleetApiError)) return e as Error; + // Naming the missing header is true and useless. Say what to do -- the same + // words mutate.ts uses for the routes not yet in /api/v1. + if (mutation && e.status === 401) + return new FleetApiError(401, e.code, getToken() + ? "That dashboard token was rejected. Check it on the System screen." + : "This collector needs a dashboard token — paste it in the banner above, or on the System screen."); + // A spec the strict schema refuses: say where, not only that. + const errors = (e.details as { schema_errors?: { path: string; message: string }[] } | undefined)?.schema_errors; + if (errors?.length) + return new FleetApiError(e.status, e.code, `${e.message}: ${errors.map((x) => `${x.path || "/"} ${x.message}`).join("; ")}`, e.hint, e.details); + return e; +} + +/** A read through the client, with errors in the dashboard's words. */ +export const read = (p: Promise): Promise => p.catch((e) => Promise.reject(explain(e, false))); + +/** A change through the client: a 401 says to paste a token, as every other button does. */ +export const change = (p: Promise): Promise => p.catch((e) => Promise.reject(explain(e, true))); + +/** + * Every device, however many pages that takes. The shelf is one screen, and + * the unversioned route it replaced answered with all of them at once. + */ +export async function allDevices(query: Parameters[0] = {}): Promise { + const first = await fleet.listDevices({ ...query, per_page: 500 }); + const items = [...first.items]; + for (let page = 2; page <= first.pages; page++) items.push(...(await fleet.listDevices({ ...query, per_page: 500, page })).items); + return { ...first, items, page: 1, per_page: items.length || first.per_page, pages: 1 }; +} + +/** The composer's spec is free-form JSON a person typed; the server is what checks it. */ +export const asSpec = (spec: Record) => spec as JobSpecInput; diff --git a/collector/dash/src/names.ts b/collector/dash/src/names.ts index 77523ac..61a42fd 100644 --- a/collector/dash/src/names.ts +++ b/collector/dash/src/names.ts @@ -9,6 +9,7 @@ // Every screen that prints a device id goes through here instead. One fetch, // shared, refreshed when a device changes — not one request per table row. import { useEffect, useState } from "preact/hooks"; +import { allDevices } from "./client.js"; import { onLive } from "./live.js"; type Names = Record; @@ -18,10 +19,9 @@ let inflight: Promise | null = null; const watchers = new Set<(n: Names) => void>(); async function load(): Promise { - const res = await fetch("/api/devices"); - const body = (await res.json()) as { devices: { device_id: string; name: string | null }[] }; + const body = await allDevices(); cache = Object.fromEntries( - body.devices.filter((d) => d.name).map((d) => [d.device_id, d.name as string]), + body.items.filter((d) => d.name).map((d) => [d.device_id, d.name as string]), ); for (const w of watchers) w(cache); return cache; diff --git a/collector/dash/src/pages/Alerts.tsx b/collector/dash/src/pages/Alerts.tsx index 20c73dc..b1884f0 100644 --- a/collector/dash/src/pages/Alerts.tsx +++ b/collector/dash/src/pages/Alerts.tsx @@ -1,10 +1,14 @@ // Alerts: what the collector thinks is wrong right now, and what it thought // was wrong earlier. -import { useApi } from "../api.js"; +import { useApi, useLoad } from "../api.js"; +import { fleet, read } from "../client.js"; import { mutate, useMutation } from "../mutate.js"; import { useQuery } from "../router.js"; import { Actions, Button, ErrorBox, Link, Loaded, Panel, Pill, Stat, agoFrom, clock, duration } from "../ui.js"; +// This screen reads /api/alerts, not /api/v1/alerts: it shows the counts per +// state, whether a webhook is set and the thresholds, which v1 does not return. +// The banner below needs only the list, and reads it from v1. export type Alert = { id: number; rule: string; @@ -147,8 +151,8 @@ export function Alerts() { /** The banner every screen carries. Silent unless something is actually open. */ export function AlertBanner() { - const state = useApi("/api/alerts?state=open", ["alert", "job", "device"], 60_000); - const open = state.data?.alerts ?? []; + const state = useLoad("alerts open", () => read(fleet.listAlerts({ state: "open" })), ["alert", "job", "device"], 60_000); + const open = state.data?.items ?? []; if (open.length === 0) return null; const worst = open.some((a) => a.severity === "critical") ? "critical" : "warning"; return ( diff --git a/collector/dash/src/pages/Compose.tsx b/collector/dash/src/pages/Compose.tsx index c053258..54755ff 100644 --- a/collector/dash/src/pages/Compose.tsx +++ b/collector/dash/src/pages/Compose.tsx @@ -5,11 +5,14 @@ // before anything is enqueued, and the equivalent curl is shown throughout — so // the form teaches the API rather than hiding it. import { useEffect, useState } from "preact/hooks"; -import { api, useApi, type DeviceList } from "../api.js"; +import { useLoad } from "../api.js"; +import { allDevices, asSpec, change, fleet, read } from "../client.js"; import { mutate, useMutation } from "../mutate.js"; import { navigate, useQuery } from "../router.js"; import { Actions, Button, ErrorBox, Field, Json, Loaded, Panel, Pill, Select } from "../ui.js"; +// The generated Template says only that spec is an object; the composer reads +// a saved spec the way it reads its own free-form half, as loosely typed JSON. type Template = { id: string; name: string | null; spec: Record }; // Defaults per workload, matching schemas/job.schema.json. These are starting @@ -94,8 +97,12 @@ function stamp(workload: string) { export function Compose() { const [q] = useQuery(); - const devices = useApi("/api/devices", ["device"]); - const templates = useApi<{ templates: Template[] }>("/api/templates", ["template"]); + const devices = useLoad("devices", () => read(allDevices()), ["device"]); + const templates = useLoad( + "templates", + () => read(fleet.listTemplates({ per_page: 500 })).then((t) => ({ templates: t.items as Template[] })), + ["template"], + ); const [workload, setWorkload] = useState(q.get("workload") ?? "benchmark"); const [jobId, setJobId] = useState(stamp(q.get("workload") ?? "benchmark")); @@ -190,7 +197,9 @@ export function Compose() { }, [pool, matchExpr, deviceId, executor, workload]); const enqueue = useMutation(async () => { - const out = await mutate<{ ok: boolean; job_id?: string; fanout?: string[] }>("POST", "/api/jobs", spec); + // /api/v1 checks the spec strictly: an unknown top-level field or params + // the workload's manifest does not allow is refused, with the paths named. + const out = await change(fleet.createJob(asSpec(spec ?? {}))); // Fan-out returns children; a single job returns its id. Land on whichever // page can actually show what just happened. if (out.fanout?.length) navigate(`/jobs?q=${encodeURIComponent(jobId)}`); @@ -208,7 +217,7 @@ export function Compose() { }); const curl = spec - ? `curl -X POST ${location.origin}/jobs \\\n -H 'content-type: application/json' \\\n -d '${JSON.stringify(spec)}'` + ? `curl -X POST ${location.origin}/api/v1/jobs \\\n -H 'content-type: application/json' \\\n -d '${JSON.stringify(spec)}'` : ""; return ( @@ -235,11 +244,11 @@ export function Compose() { {(d) => ( <>
- setQuery({ status: v })} /> - setQuery({ pool: v })} /> {/* Faceted from the response, never hard-coded: the platform set is open, so a literal list here would hide every device the fleet learned to run on after this file was written. */} - setQuery({ kind: v })} /> + setQuery({ kind: v })} /> )} setQuery({ q: v })} />