diff --git a/CHANGELOG.md b/CHANGELOG.md index cf4a5df..be53ec2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,39 @@ a CI job fails when they disagree. - **`explore-night.ts --dry-run`** opens the tunnel, runs the build and time checks against the live brain and prints the plan, without enqueueing, posting, warming or booting anything. +- **A job that ran nothing now has a status of its own: `skipped`.** A runner + that could not run a job says `ok: true` with an error beginning `skipped:`, + and the queue closed that as `done`, so the job list, the status counts and + the digest counted it as a pass: `nightly-tvloop-hardware` and three explore + jobs a night "passed" in two seconds for two weeks while testing nothing. + The queue now closes it as `skipped`, with the reason in `last_error`; a host + row of plain `ok: true` over device rows that all skipped counts the same, + and a fan-out family where no child ran is `skipped`. `skipped` is accepted + by the `status` filter of `GET /api/jobs` and `GET /api/v1/jobs`, counted in + the status facets, `GET /metrics` and the Overview (`skipped 24h`, beside + done rather than in it), drawn as a grey chip on the dashboard, and given its + own line in the morning digest. The v1 summary's verdict, `fleetr`'s exit 4 + and the MCP verdict were already `skipped` and are unchanged. A database + opened by this release is migrated once: every `done` job with a `skipped:` + result is re-labelled by the same rule. Result rows are not touched. +- **A job waiting on a skipped job skips too.** A skipped build produced no + artifact, so the install behind it had nothing to install; it used to be + promoted and run against whatever was already on the phone. It now skips + with `last_error` `dependency skipped`, the whole way down the chain, + and a job enqueued behind an already-skipped one is skipped on arrival. A + failed dependency still fails its waiters, and wins over a skipped one. +- **A `schedule-skipping` alert.** A schedule whose last three runs + (`FLEET_ALERT_SKIP_STREAK`) all skipped raises one alert naming the reason it + keeps giving, for example "nightly-tvloop-hardware has skipped 3 runs in a + row: no Roku developer password on this host", and resolves on the first run + that is not a skip. The tvloop nightly needed it for two weeks. +- **Simulator rows are forgotten after a week offline.** The lease sweep deletes + a row whose descriptor says `kind` or `soc` is `simulator` once it has been + offline for `FLEET_SIMULATOR_TTL_DAYS` (default 7), because simulators are + recreated under new UUIDs and never come back; the live brain listed thirteen + of them. Real devices and Android emulators are never forgotten, a row that + holds a lock is left alone, and results and beacons are kept. `POST + /jobs/sweep` reports what it forgot. ### Changed @@ -160,6 +193,37 @@ a CI job fails when they disagree. `fleet-explore-tv` night "passed" in about a minute. A tvOS simulator is now `tvos-26.5` (watchOS and visionOS likewise). No TV night has run with the fix yet. +- **A failing `ui-test` now says which step failed.** `nightly-fleet-ui-smoke` + failed on the Galaxy S8+ every night from 10-10 with `failed: 1` and no + error, while `Assertion is false: "Fleet Runner" is visible` sat in the JUnit + report it had uploaded. The executor now reads the failing flow and its + assertion out of the report into the device row's `error` and + `test.failing_flow`; when Maestro produces no report, the error is its exit + code and last stderr line, never its command line, which carries the test + account's password. +- **No host result says `ok: false` with nothing beside it.** Every host + workload's rows go through one wrapper in the executor: a device row with + only counts says so ("1 of 1 tests failed; the runner reported no failing + step"), and a final host row with no error of its own gets the failing + devices' reasons. A subprocess failure nobody caught is reported as its exit + code and last stderr line instead of Node's message. The collector also + copies a failure's reason into the job's `last_error`, so the job list and + the `job-failed` alert show it. +- **`GET /api/artifacts/` answered "no such endpoint".** It is where + `DELETE` and `/pin` already live, so it is the URL a person reaches for, and + the 404 read as a missing artifact: the ui-smoke JUnit report was in the + store all along. It now redirects (308) to + `/api/v1/artifacts//content`, and says plainly when no artifact by + that hash was ever uploaded. +- **The executor registered Rokus as `platform: android`.** Its Roku + descriptor said `os: roku-15.3.4` but no platform, and the collector's + fallback for anything that is not iOS is Android. It now sends + `platform: "roku"`; `os ~ 'roku'` targeting and the tvloop workload are + unaffected. +- **A Roku seen first by address and later by serial was listed twice.** A + sweep where the box answered SSDP but not device-info left a + `roku-ip-192-168-50-218` row offline beside `roku-X01600EMFRJ9`. A + serial-named registration now retires the IP-named row at the same address. ## [0.8.0] — 2026-10-15 diff --git a/collector/dash/src/api.ts b/collector/dash/src/api.ts index 7346c9f..2e13d0a 100644 --- a/collector/dash/src/api.ts +++ b/collector/dash/src/api.ts @@ -166,7 +166,9 @@ export type Job = { job_id: string; workload: string; executor: "device" | "host"; - status: "waiting" | "queued" | "claimed" | "done" | "failed" | "cancelled"; + /** `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. */ @@ -214,6 +216,7 @@ export type Overview = { claimed: number; done_24h: number; failed_24h: number; + skipped_24h: number; last_attempt: number; oldest_queued_age_s: number | null; }; diff --git a/collector/dash/src/deviceKind.ts b/collector/dash/src/deviceKind.ts index a9aefce..1433201 100644 --- a/collector/dash/src/deviceKind.ts +++ b/collector/dash/src/deviceKind.ts @@ -7,7 +7,8 @@ * The model string comes first, because it is the one field that is right * about every device on the shelf today. `platform` is not: agents that * predate the field are reported as ios/android by the collector's fallback, - * so a Roku arrives as `android` and an Apple TV or Apple Watch as `ios`. + * so an Apple TV or Apple Watch arrives as `ios`, and a Roku registered by an + * executor older than 0.9 as `android` (newer ones say `roku`). * `kind` is the agent naming its own shape and is believed — but only to pick * the family; the model still chooses which member of it (a `tv` that says * "Roku Streaming Stick" is drawn as the stick, not a bare panel). diff --git a/collector/dash/src/pages/JobDetail.tsx b/collector/dash/src/pages/JobDetail.tsx index f690f78..8d98a71 100644 --- a/collector/dash/src/pages/JobDetail.tsx +++ b/collector/dash/src/pages/JobDetail.tsx @@ -87,7 +87,7 @@ function DependsOn({ job }: { job: Detail }) { .{" "} {statusOf(job) === "waiting" - ? "It is queued the moment the last of them is done, with any ${jobs.…} references in its spec filled in from their final rows. If one fails or is cancelled, this job fails with it rather than waiting forever." + ? "It is queued the moment the last of them is done, with any ${jobs.…} references in its spec filled in from their final rows. If one fails or is cancelled, this job fails with it rather than waiting forever; if one skips, this job skips too, since there is nothing for it to run on." : "The chain has already resolved; this is what it hung off."}

diff --git a/collector/dash/src/pages/Jobs.tsx b/collector/dash/src/pages/Jobs.tsx index 7e1ddd8..c1c363e 100644 --- a/collector/dash/src/pages/Jobs.tsx +++ b/collector/dash/src/pages/Jobs.tsx @@ -132,7 +132,7 @@ export function Jobs() { // 'waiting' leads because it is the one status a person goes // looking for: a chain that has not started is invisible in // every other view, including the overview's queue depth. - options={["waiting", "queued", "claimed", "done", "failed", "cancelled"].map((s) => ({ + options={["waiting", "queued", "claimed", "done", "failed", "skipped", "cancelled"].map((s) => ({ value: s, label: `${s} (${d.status_counts[s] ?? 0})`, }))} diff --git a/collector/dash/src/pages/Overview.tsx b/collector/dash/src/pages/Overview.tsx index 64c564b..93cdf40 100644 --- a/collector/dash/src/pages/Overview.tsx +++ b/collector/dash/src/pages/Overview.tsx @@ -83,6 +83,9 @@ export function Overview() { 0 ? "warn" : undefined} /> 0 ? "bad" : undefined} /> + {/* Its own number, never added to done: a skip ran nothing. No tone, + like the chip -- it is not good news and not a failure. */} + = {}) => ({ + schema: 1, job_id, workload: WL, executor: "device", targets: { device_id: DEV }, ...extra, + }); + const hostJob = (job_id: string) => ({ + schema: 1, job_id, workload: "install", executor: "host", + app: { name: "x", build: "1", sha256: "0".repeat(64) }, + targets: { executor: EXEC }, lease: { ttl_s: 60, max_attempts: 1 }, + }); + await json("POST", "/devices/register", { + device_id: DEV, descriptor: { model: "Skipper", os: "android-14" }, pools: [], capabilities: [WL], + }); + + // The plain case: the runner's final row says why nothing ran. + const SKIP = `smoke-skip-plain-${run}`; + await json("POST", "/jobs", devJob(SKIP)); + const closed = await json("POST", "/results", { + schema: 1, kind: "result", job_id: SKIP, device_id: DEV, iter: 0, final: true, ok: true, + error: "skipped: no Roku developer password on this host", + }); + check("skip: a skipped result closes the job as skipped, not done", closed.body?.status === "skipped", JSON.stringify(closed.body)); + const row = await json("GET", `/api/jobs/${SKIP}`); + check("skip: the job row says skipped", row.body?.status === "skipped", row.body?.status); + check("skip: and its last_error carries the reason", + row.body?.last_error === "skipped: no Roku developer password on this host", JSON.stringify(row.body?.last_error)); + const listed = await json("GET", `/api/jobs?status=skipped&q=${encodeURIComponent(SKIP)}`); + check("skip: /api/jobs filters on it", + (listed.body?.jobs ?? []).some((j: { job_id: string }) => j.job_id === SKIP), JSON.stringify(listed.body?.jobs?.length)); + check("skip: and the status facet counts it", (listed.body?.status_counts?.skipped ?? 0) >= 1, JSON.stringify(listed.body?.status_counts)); + const v1 = await json("GET", `/api/v1/jobs?status=skipped&q=${encodeURIComponent(SKIP)}`); + check("skip: /api/v1/jobs filters on it", + (v1.body?.items ?? []).some((j: { job_id: string; status: string }) => j.job_id === SKIP && j.status === "skipped"), + JSON.stringify(v1.body).slice(0, 300)); + const sum = await json("GET", `/api/v1/jobs/${SKIP}/summary`); + check("skip: the summary agrees (status skipped, verdict skipped, finished)", + sum.body?.status === "skipped" && sum.body?.verdict === "skipped" && sum.body?.finished === true, JSON.stringify(sum.body).slice(0, 300)); + const ov = await json("GET", "/api/overview"); + check("skip: the overview counts skips on their own", (ov.body?.queue?.skipped_24h ?? 0) >= 1, JSON.stringify(ov.body?.queue)); + + // A host job whose every device skipped, under a plain ok:true host row -- + // the shape a ui-test takes when no phone has the app installed. + const ALL = `smoke-skip-allhost-${run}`; + await json("POST", "/jobs", hostJob(ALL)); + await json("POST", "/results", { schema: 1, kind: "result", job_id: ALL, device_id: DEV, iter: 0, ok: true, error: "skipped: com.example not installed" }); + const allClosed = await json("POST", "/results", { schema: 1, kind: "result", job_id: ALL, device_id: `host:${EXEC}`, iter: 0, final: true, ok: true }); + check("skip: every device skipping under an ok host row is a skip", allClosed.body?.status === "skipped", JSON.stringify(allClosed.body)); + + // One device ran: the job ran. + const MIXED = `smoke-skip-mixed-${run}`; + await json("POST", "/jobs", hostJob(MIXED)); + await json("POST", "/results", { schema: 1, kind: "result", job_id: MIXED, device_id: DEV, iter: 0, ok: true, error: "skipped: device locked by another job" }); + await json("POST", "/results", { schema: 1, kind: "result", job_id: MIXED, device_id: `${DEV}-b`, iter: 0, ok: true, test: { passed: 1, failed: 0 } }); + const mixedClosed = await json("POST", "/results", { schema: 1, kind: "result", job_id: MIXED, device_id: `host:${EXEC}`, iter: 0, final: true, ok: true }); + check("skip: one device that ran makes the job done", mixedClosed.body?.status === "done", JSON.stringify(mixedClosed.body)); + + // A failure says why on the job row, from the failing device when the host + // row has nothing of its own. + const FAIL = `smoke-skip-fail-${run}`; + await json("POST", "/jobs", hostJob(FAIL)); + await json("POST", "/results", { schema: 1, kind: "result", job_id: FAIL, device_id: DEV, iter: 0, ok: false, error: 'smoke: Assertion is false: "Fleet Runner" is visible' }); + await json("POST", "/results", { schema: 1, kind: "result", job_id: FAIL, device_id: `host:${EXEC}`, iter: 0, final: true, ok: false }); + const failRow = await json("GET", `/api/jobs/${FAIL}`); + check("fail: a failed job's last_error names the failing device and step", + failRow.body?.status === "failed" && failRow.body?.last_error === `${DEV}: smoke: Assertion is false: "Fleet Runner" is visible`, + JSON.stringify(failRow.body?.last_error)); + + // Dependencies: a skipped dependency produced nothing, so its waiters skip + // -- the whole way down -- rather than run against nothing or fail. + const DA = `smoke-skip-da-${run}`, DB = `smoke-skip-db-${run}`, DC = `smoke-skip-dc-${run}`; + await json("POST", "/jobs", devJob(DA)); + await json("POST", "/jobs", devJob(DB, { depends_on: [DA] })); + await json("POST", "/jobs", devJob(DC, { depends_on: [DB] })); + const depClosed = await json("POST", "/results", { + schema: 1, kind: "result", job_id: DA, device_id: DEV, iter: 0, final: true, ok: true, error: "skipped: no build for this device", + }); + check("chain: a skipped dependency skips its waiter and the link beyond", + ((depClosed.body?.skipped ?? []) as string[]).includes(DB) && ((depClosed.body?.skipped ?? []) as string[]).includes(DC), + JSON.stringify(depClosed.body)); + const dbRow = await json("GET", `/api/jobs/${DB}`); + check("chain: the waiter says which dependency skipped", + dbRow.body?.status === "skipped" && dbRow.body?.last_error === `dependency ${DA} skipped`, JSON.stringify(dbRow.body?.last_error)); + const dbSum = await json("GET", `/api/v1/jobs/${DB}/summary`); + check("chain: and its summary is a skip, not a pass or a failure", dbSum.body?.verdict === "skipped", JSON.stringify(dbSum.body?.verdict)); + const lateDep = await json("POST", "/jobs", devJob(`smoke-skip-late-${run}`, { depends_on: [DA] })); + check("chain: a job enqueued behind an already-skipped job is skipped on arrival", + lateDep.status === 201 && lateDep.body?.status === "skipped", JSON.stringify(lateDep.body)); + check("chain: and a dry run says so first", + ((await json("POST", "/api/v1/jobs/validate", devJob(`smoke-skip-dry-${run}`, { depends_on: [DA] }))).body?.warnings ?? []) + .some((w: string) => w.includes("already skipped"))); + + // The artifact a person reaches for at /api/artifacts/ -- where DELETE + // and /pin already live -- used to answer "no such endpoint". + const blob = Buffer.from(`junit-${run}`.repeat(20)); + const blobSha = createHash("sha256").update(blob).digest("hex"); + await fetch(`${BASE}/artifacts`, { method: "POST", headers: { "content-type": "application/octet-stream", "x-artifact-name": "junit.xml" }, body: blob }); + const redir = await fetch(`${BASE}/api/artifacts/${blobSha}`, { redirect: "manual" }); + check("artifact: GET /api/artifacts/ points at the bytes", + redir.status === 308 && redir.headers.get("location") === `/api/v1/artifacts/${blobSha}/content`, `${redir.status} ${redir.headers.get("location")}`); + const followed = await fetch(`${BASE}/api/artifacts/${blobSha}`); + check("artifact: and following it gets them", followed.ok && Buffer.from(await followed.arrayBuffer()).equals(blob), String(followed.status)); + const unknown = await json("GET", `/api/artifacts/${"e".repeat(64)}`); + check("artifact: an unknown one is a plain 404 that says so", unknown.status === 404 && /no artifact/.test(String(unknown.body?.error)), JSON.stringify(unknown)); + + // Registry hygiene: a Roku first seen by address, then by serial, is one row. + const ip = `10.${run % 200}.${Math.floor(run / 200) % 200}.7`; + const ipId = `roku-ip-${ip.replace(/\./g, "-")}`; + await json("POST", "/devices/register", { device_id: ipId, descriptor: { model: "roku", os: "roku", ip, kind: "device" }, pools: [] }); + check("roku: the IP-named row exists first", (await json("GET", `/api/devices/${ipId}`)).status === 200); + await json("POST", "/devices/register", { + device_id: `roku-SMOKE${run}`, pools: [], + descriptor: { platform: "roku", model: "Roku Express 4K+", os: "roku-15.3.4", serial: `SMOKE${run}`, ip, kind: "device" }, + }); + check("roku: the serial-named registration retires the IP-named duplicate", (await json("GET", `/api/devices/${ipId}`)).status === 404); + const roku = await json("GET", `/api/devices/roku-SMOKE${run}`); + check("roku: and the Roku reads as platform roku", roku.body?.platform === "roku", JSON.stringify(roku.body?.platform)); + const swept = await json("POST", "/jobs/sweep"); + check("sweep: reports the simulators it forgot", Array.isArray(swept.body?.forgotten), JSON.stringify(swept.body)); +} + // --- preemption: a long job stands aside for higher-priority work --- // The collector never kills anything. It answers a beacon with preempt:true and @@ -2724,6 +2855,12 @@ runDbChecks(check); // the live brain's alert list with noise nobody could act on. runAlertChecks(check); +// Honest green: how a job closes (done, failed, skipped), every failure saying +// why, and the registry keeping to real things. +runSkipChecks(check); +await runReasonChecks(check); +runRegistryHygieneChecks(check); + // Where a host's Playwright lives and which Chromium it drives: the two // things a released executor on macOS 12 needed to run web work at all. await runBrowserChecks(check); diff --git a/collector/src/alerts.test.ts b/collector/src/alerts.test.ts index ccd7920..d2e8c6c 100644 --- a/collector/src/alerts.test.ts +++ b/collector/src/alerts.test.ts @@ -148,4 +148,66 @@ export function runAlertChecks(check: Check) { check("but a personal laptop that is thermally critical still alerts", thermal.includes("hot-daily-laptop"), JSON.stringify(thermal)); }); + + // A schedule that keeps skipping. The tvloop nightly skipped every night for + // two weeks on a missing Roku password and nothing said so; its job ids are + // the scheduler's `-`, which is how a run is found + // even on a brain whose old jobs carry no requested_by. + inTempDir((dir) => { + const db = openDb(dir); + const schedule = (id: string) => + db.prepare("INSERT INTO schedules (id, cron, template, enabled, last_run) VALUES (?, '30 6 * * *', '{}', 1, NULL)").run(id); + const job = (id: string, status: string, lastError: string | null = null) => + db.prepare( + `INSERT INTO jobs (job_id, executor, workload, spec, status, last_error, finished_at) + VALUES (?, 'host', 'tvloop', '{}', ?, ?, ${status === "queued" || status === "claimed" ? "NULL" : "datetime('now')"})`, + ).run(id, status, lastError); + const NO_PW = "skipped: no Roku developer password on this host"; + + schedule("nightly-tvloop-hardware"); + job("nightly-tvloop-hardware-202610120630", "done"); + job("nightly-tvloop-hardware-202610130630", "skipped", NO_PW); + job("nightly-tvloop-hardware-202610140630", "skipped", NO_PW); + job("nightly-tvloop-hardware-202610150630", "skipped", NO_PW); + // A schedule whose id is a prefix of this one must not borrow its runs. + schedule("nightly-tvloop"); + job("nightly-tvloop-202610150630", "done"); + // Two skips are not a streak yet. + schedule("nightly-two"); + job("nightly-two-202610140630", "skipped", "skipped: locked"); + job("nightly-two-202610150630", "skipped", "skipped: locked"); + // A fan-out where every child skipped, for different reasons, three nights. + schedule("nightly-fan"); + for (const day of ["13", "14", "15"]) { + job(`nightly-fan-202610${day}0230--phone-a`, "skipped", "skipped: com.example not installed"); + job(`nightly-fan-202610${day}0230--phone-b`, "skipped", day === "15" ? "skipped: device locked by another job" : "skipped: com.example not installed"); + } + // Three skips and then a run in flight: the in-flight one has not said + // anything yet, so the streak stands. + schedule("nightly-inflight"); + for (const day of ["12", "13", "14"]) job(`nightly-inflight-202610${day}0230`, "skipped", "skipped: no device matches this job on ultra"); + job("nightly-inflight-202610150230", "claimed"); + + const skipping = () => evaluate(new Date(), NO_SIZES).filter((f) => f.rule === "schedule-skipping"); + const found = skipping(); + const bySubject = new Map(found.map((f) => [f.subject, f])); + check("stale skip: three skips in a row raise one alert", bySubject.has("nightly-tvloop-hardware"), JSON.stringify(found)); + check("stale skip: naming the schedule and the reason it keeps giving", + bySubject.get("nightly-tvloop-hardware")?.message === + "nightly-tvloop-hardware has skipped 3 runs in a row: no Roku developer password on this host", + bySubject.get("nightly-tvloop-hardware")?.message); + check("stale skip: a schedule's runs are its own, not a longer id's", !bySubject.has("nightly-tvloop")); + check("stale skip: two skips are not a streak", !bySubject.has("nightly-two")); + check("stale skip: a fan-out whose every child skipped counts as a skipped run", bySubject.has("nightly-fan")); + check("stale skip: a run still in flight neither breaks nor extends the streak", bySubject.has("nightly-inflight")); + check("stale skip: one alert per schedule", found.length === 3, JSON.stringify(found.map((f) => f.subject))); + + // A run that is not a skip resolves it. + job("nightly-tvloop-hardware-202610160630", "failed", "smoke: Assertion is false"); + check("stale skip: the first run that is not a skip clears it", + !skipping().some((f) => f.subject === "nightly-tvloop-hardware")); + // Disabled schedules are not watched. + db.prepare("UPDATE schedules SET enabled = 0 WHERE id = 'nightly-fan'").run(); + check("stale skip: a disabled schedule is not watched", !skipping().some((f) => f.subject === "nightly-fan")); + }); } diff --git a/collector/src/alerts.ts b/collector/src/alerts.ts index a52ebfa..4980b48 100644 --- a/collector/src/alerts.ts +++ b/collector/src/alerts.ts @@ -13,6 +13,7 @@ import { db } from "./db.js"; import { beaconFields, deviceStatus, effectivePools, hasBattery, isExpired, parse } from "./api/shared.js"; import { minuteKey, prevRun } from "./cron.js"; +import { skipReasonText } from "./skip.js"; export type Severity = "warning" | "critical"; export type Finding = { rule: string; subject: string; severity: Severity; message: string }; @@ -38,8 +39,58 @@ export const THRESHOLDS = { // Below this many prior runs there is no median worth trusting, and a new // device would alarm on its second result forever. regressionMinSamples: env("FLEET_ALERT_REGRESSION_MIN_SAMPLES", 4), + // A schedule whose last this-many runs all skipped is not running, whatever + // its jobs' statuses say about the code. One skip is a phone on a charger + // somewhere else; three in a row is a missing password nobody has noticed. + skipStreak: Math.round(env("FLEET_ALERT_SKIP_STREAK", 3)), }; +/** + * The runs a schedule fired, newest first, each judged as a whole. + * + * A run is the job the scheduler enqueued (`-`) or, for + * a fan-out, that id's children; jobs from before provenance existed carry no + * `requested_by`, so the id is what identifies them. Read as a primary-key + * range rather than a LIKE, so it costs an index walk however long the history. + * A run still in flight is left out: it has not said whether it skipped. + */ +export function scheduleRuns(scheduleId: string): { run: string; skipped: boolean; reasons: string[] }[] { + const prefix = `${scheduleId}-`; + // '-' is 0x2D and '.' is 0x2E, so [prefix, prefix-with-'.') is every id that + // starts with it. + const upper = `${scheduleId}.`; + const escaped = scheduleId.replace(/[.*+?^$()|[\]{}\\]/g, "\\$&"); + const own = new RegExp("^" + escaped + "-\\d{12}(--.*)?$"); + const rows = (db + .prepare( + `SELECT job_id, status, last_error, created_at FROM jobs + WHERE job_id >= ? AND job_id < ?`, + ) + .all(prefix, upper) as { job_id: string; status: string; last_error: string | null; created_at: string }[]) + .filter((r) => own.test(r.job_id)); + const runs = new Map(); + for (const r of rows) { + const key = r.job_id.split("--")[0]; + const run = runs.get(key) ?? { created: r.created_at, statuses: [], reasons: [] }; + run.statuses.push(r.status); + if (r.status === "skipped" && r.last_error) run.reasons.push(r.last_error); + if (r.created_at < run.created) run.created = r.created_at; + runs.set(key, run); + } + const FINISHED = new Set(["done", "failed", "cancelled", "skipped"]); + return [...runs] + .filter(([, r]) => r.statuses.every((s) => FINISHED.has(s))) + // The id ends in the minute it fired, so it orders runs exactly; created_at + // has one-second resolution and a backfilled run would tie. + .sort(([a], [b]) => (a < b ? 1 : a > b ? -1 : 0)) + .map(([run, r]) => ({ + run, + // The family rule from the v1 summary: nothing in it ran. + skipped: !r.statuses.includes("done") && !r.statuses.includes("failed") && r.statuses.includes("skipped"), + reasons: r.reasons, + })); +} + /** * A metric watched for regression: where it lives, which direction is bad, and * what makes two runs comparable. The comparison key matters more than the @@ -258,6 +309,26 @@ export function evaluate(now = new Date(), sizes: { dbBytes: number; logBytes: n }); } + // A schedule that keeps skipping. Its jobs are no longer counted as passes, + // but a grey chip every morning is still easy to stop seeing: the tvloop + // nightly skipped for two weeks on a missing Roku password and nothing said + // so. One alert per schedule, naming the reason it keeps giving, resolved by + // the first run that is not a skip. + const streak = Math.max(1, THRESHOLDS.skipStreak); + for (const s of db.prepare("SELECT id FROM schedules WHERE enabled = 1").all() as { id: string }[]) { + const recent = scheduleRuns(s.id).slice(0, streak); + if (recent.length < streak || !recent.every((r) => r.skipped)) continue; + const reasons = recent.map((r) => skipReasonText(r.reasons[0])); + const distinct = [...new Set(reasons)]; + const why = distinct.length === 1 ? distinct[0] : `${reasons[0]} (the latest of ${distinct.length} different reasons)`; + out.push({ + rule: "schedule-skipping", + subject: s.id, + severity: "warning", + message: `${s.id} has skipped ${streak} runs in a row: ${why}`, + }); + } + // --- collector --- if (sizes.dbBytes > THRESHOLDS.dbBytes) out.push({ diff --git a/collector/src/api/jobs.ts b/collector/src/api/jobs.ts index 03f830e..35af083 100644 --- a/collector/src/api/jobs.ts +++ b/collector/src/api/jobs.ts @@ -3,7 +3,7 @@ import type { FastifyInstance } from "fastify"; import { db, type SqlParam } from "../db.js"; import { AGE, beaconFields, inClause, iso, paging, parse, sha256Refs } from "./shared.js"; -const STATUSES = new Set(["waiting", "queued", "claimed", "done", "failed", "cancelled"]); +const STATUSES = new Set(["waiting", "queued", "claimed", "done", "failed", "cancelled", "skipped"]); const EXECUTORS = new Set(["device", "host"]); const SORTS: Record = { created: "created_at", diff --git a/collector/src/api/mutations.ts b/collector/src/api/mutations.ts index 029adeb..4733ab9 100644 --- a/collector/src/api/mutations.ts +++ b/collector/src/api/mutations.ts @@ -34,11 +34,21 @@ type MatchingDevices = ( // inserted as 'waiting', which the claim loop does not look at, and becomes // 'queued' when the last of its dependencies is 'done'. // +// A dependency that skipped produced nothing -- no build, no artifact, no +// metric -- so a job waiting on it skips too, with `last_error` saying +// "dependency X skipped". Not a failure: nothing went wrong with the code, and +// a red install behind a build that never ran would be a lie in the other +// direction. +// // The machinery lives in this file because both of the places a job can close // have to settle whatever was waiting on it: a final result (server.ts) and a // cancellation (below), and only one of those can import the other. -export type DepSettlement = { promoted: string[]; failed: { job_id: string; reason: string }[] }; +export type DepSettlement = { + promoted: string[]; + failed: { job_id: string; reason: string }[]; + skipped: { job_id: string; reason: string }[]; +}; // `${jobs..artifact}` and `${jobs..metrics.}`. The id is matched // lazily and the suffix is what anchors the match, so a job id that itself @@ -180,17 +190,21 @@ export function validateDepRefs(specJson: string, deps: string[]): string | null /** What status a job with these dependencies should be inserted as. A chain * whose build already failed is failed on arrival rather than parked in * 'waiting' forever: nothing will ever close that dependency again, so no - * promotion event is coming. */ + * promotion event is coming. One whose build skipped is skipped on arrival, + * for the same reason. A failure outranks a skip. */ export function dependencyState( deps: string[], -): { status: "queued" | "waiting" } | { status: "failed"; reason: string } { +): { status: "queued" | "waiting" } | { status: "failed" | "skipped"; reason: string } { let allDone = true; + let skipped: string | null = null; for (const d of deps) { const row = db.prepare("SELECT status FROM jobs WHERE job_id = ?").get(d) as { status: string } | undefined; if (row?.status === "failed" || row?.status === "cancelled") return { status: "failed", reason: `dependency ${d} ${row.status}` }; + if (row?.status === "skipped") skipped ??= d; if (row?.status !== "done") allDone = false; } + if (skipped) return { status: "skipped", reason: `dependency ${skipped} skipped` }; return { status: allDone ? "queued" : "waiting" }; } @@ -217,13 +231,25 @@ function failWaiter(jobId: string, reason: string, out: DepSettlement) { settleWaiters(jobId, out); } +function skipWaiter(jobId: string, reason: string, out: DepSettlement) { + db.prepare( + `UPDATE jobs SET status = 'skipped', finished_at = datetime('now'), + lease_deadline = NULL, last_error = ? + WHERE job_id = ?`, + ).run(reason, jobId); + out.skipped.push({ job_id: jobId, reason }); + // An install that skipped has nothing for the ui-test behind it either. + settleWaiters(jobId, out); +} + /** * Resolve everything that was waiting on a job that just closed. * * A waiter whose last dependency is now 'done' is promoted to 'queued' with its * `${jobs.…}` references filled in from the dependency's final row. A waiter * whose dependency failed or was cancelled is failed with `last_error` naming - * it, and that failure cascades. + * it, and that failure cascades. One whose dependency skipped is skipped the + * same way -- unless another dependency failed, which wins. * * Callers run this inside the transaction that closed the job, and announce * what it returns after that transaction commits — a broken dashboard pipe must @@ -231,7 +257,7 @@ function failWaiter(jobId: string, reason: string, out: DepSettlement) { */ export function settleWaiters( closedJobId: string, - out: DepSettlement = { promoted: [], failed: [] }, + out: DepSettlement = { promoted: [], failed: [], skipped: [] }, ): DepSettlement { for (const w of waitersOn(closedJobId)) { const deps = parse(w.depends_on, []); @@ -251,6 +277,11 @@ export function settleWaiters( ); continue; } + const skippedDep = states.find((s) => s.status === "skipped"); + if (skippedDep) { + skipWaiter(w.job_id, `dependency ${skippedDep.id} skipped`, out); + continue; + } if (!states.every((s) => s.status === "done")) continue; const filled = substituteDepRefs(parse>(w.spec, {})); diff --git a/collector/src/api/overview.ts b/collector/src/api/overview.ts index 37f0b9c..ecf9f2e 100644 --- a/collector/src/api/overview.ts +++ b/collector/src/api/overview.ts @@ -77,6 +77,8 @@ function build() { COUNT(*) FILTER (WHERE status = 'claimed') AS claimed, COUNT(*) FILTER (WHERE status = 'done' AND finished_at >= datetime('now','-24 hours')) AS done_24h, COUNT(*) FILTER (WHERE status = 'failed' AND finished_at >= datetime('now','-24 hours')) AS failed_24h, + -- Its own count, never folded into done: a skip is a job that ran nothing. + COUNT(*) FILTER (WHERE status = 'skipped' AND finished_at >= datetime('now','-24 hours')) AS skipped_24h, COUNT(*) FILTER (WHERE status = 'claimed' AND attempts >= max_attempts) AS last_attempt, (SELECT ${AGE("MIN(created_at)")} FROM jobs WHERE status = 'queued') AS oldest_queued_age_s FROM jobs`, @@ -153,6 +155,7 @@ function build() { claimed: queue.claimed ?? 0, done_24h: queue.done_24h ?? 0, failed_24h: queue.failed_24h ?? 0, + skipped_24h: queue.skipped_24h ?? 0, last_attempt: queue.last_attempt ?? 0, oldest_queued_age_s: queue.oldest_queued_age_s ?? null, }, diff --git a/collector/src/api/peers.ts b/collector/src/api/peers.ts index dd1bc72..9bed22e 100644 --- a/collector/src/api/peers.ts +++ b/collector/src/api/peers.ts @@ -52,7 +52,7 @@ const PEER_TIMEOUT_MS = 4_000; * What may be proxied. * * An allow-list of prefixes rather than a rule about methods alone, because - * `GET /api/artifacts/:sha` streams bytes and `GET /api/stream` is an endless + * `GET /api/artifacts/:sha` leads to bytes and `GET /api/stream` is an endless * SSE response -- neither belongs on a path whose whole purpose is to fill in a * summary table. Everything here answers with a bounded JSON document. */ diff --git a/collector/src/api/system.ts b/collector/src/api/system.ts index 8be7033..c1b8e3e 100644 --- a/collector/src/api/system.ts +++ b/collector/src/api/system.ts @@ -242,6 +242,21 @@ export function registerSystem(app: FastifyInstance) { ).map((l) => ({ ...l, acquired_at: iso(l.acquired_at) })), })); + // One artifact, by the URL a person reaches for. DELETE and /pin live under + // /api/artifacts/:sha256, so a GET there is the obvious guess -- and it used + // to answer "no such endpoint", which read as "this artifact is gone" when + // the bytes were in the store all along (nightly-fleet-ui-smoke's JUnit + // report, October 2026). The bytes have two homes, /artifacts/:sha256 for + // runners and /api/v1/artifacts/:sha256/content for everyone else; this + // points at the second rather than becoming a third. + app.get("/api/artifacts/:sha256", async (req, reply) => { + const { sha256 } = req.params as { sha256: string }; + if (!/^[a-f0-9]{64}$/.test(sha256)) return reply.code(400).send({ error: "sha256 must be 64 lowercase hex characters" }); + const known = db.prepare("SELECT 1 FROM artifacts WHERE sha256 = ?").get(sha256); + if (!known) return reply.code(404).send({ error: `no artifact ${sha256} has been uploaded to this collector` }); + return reply.redirect(`/api/v1/artifacts/${sha256}/content`, 308); + }); + app.get("/api/artifacts", async (req) => { const q = req.query as Record; const { page, per_page, offset } = paging(q); diff --git a/collector/src/api/v1/prometheus.ts b/collector/src/api/v1/prometheus.ts index 3142fbb..310ecb1 100644 --- a/collector/src/api/v1/prometheus.ts +++ b/collector/src/api/v1/prometheus.ts @@ -32,7 +32,7 @@ export function metricsText(): string { for (const d of devices) if (!isExpired(d, d.age_s)) byStatus[deviceStatus(d.age_s)]++; const jobs = db.prepare("SELECT status, COUNT(*) AS n FROM jobs GROUP BY status").all() as { status: string; n: number }[]; - const jobCounts: Record = { waiting: 0, queued: 0, claimed: 0, done: 0, failed: 0, cancelled: 0 }; + const jobCounts: Record = { waiting: 0, queued: 0, claimed: 0, done: 0, failed: 0, cancelled: 0, skipped: 0 }; for (const j of jobs) jobCounts[j.status] = j.n; const oldest = db.prepare(`SELECT MAX(${AGE("created_at")}) AS s FROM jobs WHERE status = 'queued'`).get() as { s: number | null }; @@ -57,7 +57,7 @@ export function metricsText(): string { family("fleet_queue_oldest_seconds", "gauge", "Age of the oldest queued job; 0 when nothing is queued.", [[{}, oldest.s ?? 0]]), family("fleet_jobs_finished_24h", "gauge", "Jobs that finished in the last 24 hours, by workload and status.", finished.map((f) => [{ workload: f.workload, status: f.status }, f.n])), family("fleet_executors_live", "gauge", "Host executors that polled in the last two minutes.", [[{}, executors.n]]), - family("fleet_device_success_ratio_24h", "gauge", "done / (done + failed) per device over the last 24 hours.", success.map((s) => [{ device_id: s.device }, s.n ? s.ok / s.n : 0])), + family("fleet_device_success_ratio_24h", "gauge", "done / (done + failed) per device over the last 24 hours. Skipped jobs count as neither: nothing ran.", success.map((s) => [{ device_id: s.device }, s.n ? s.ok / s.n : 0])), family("fleet_alerts_open", "gauge", "Open alerts by severity.", alerts.map((a) => [{ severity: a.severity }, a.n])), family("fleet_artifacts", "gauge", "Artifacts recorded.", [[{}, artifacts.n]]), family("fleet_artifact_bytes", "gauge", "Total size of recorded artifacts.", [[{}, artifacts.bytes]]), diff --git a/collector/src/api/v1/routes.ts b/collector/src/api/v1/routes.ts index 60f749a..8207281 100644 --- a/collector/src/api/v1/routes.ts +++ b/collector/src/api/v1/routes.ts @@ -35,6 +35,7 @@ import { AGE, capabilityMatches, deviceCapabilities, inClause, iso, isExpired, O import { needsAdmin, notOwned } from "../policy.js"; import { schedulesView } from "../system.js"; import { SCHEMAS } from "./schemas.js"; +import { isSkipError } from "../../skip.js"; import { errorsOf, paramsValidator, validateSpec, type SchemaError } from "./spec-check.js"; // --- the table's types ------------------------------------------------------------ @@ -230,7 +231,7 @@ type SummaryJob = { const SUMMARY_COLUMNS = `job_id, workload, status, claimed_by, last_error, requested_by, source, spec, CASE WHEN claimed_at IS NOT NULL THEN CAST(strftime('%s', COALESCE(finished_at, datetime('now'))) - strftime('%s', claimed_at) AS INTEGER) END AS duration_s`; -const FINISHED = new Set(["done", "failed", "cancelled"]); +const FINISHED = new Set(["done", "failed", "cancelled", "skipped"]); const likePrefix = (p: string) => `${p.replace(/[\\%_]/g, "\\$&")}--%`; function familyOf(id: string): { jobs: SummaryJob[]; fanout: boolean } | null { @@ -268,16 +269,18 @@ function outcomesOf(job: SummaryJob): DeviceOutcome[] { const spec = parse>(job.spec, {}); if (entries.length === 0) { const device = String(job.claimed_by ?? spec.targets?.device_id ?? "(unassigned)"); - const skipped = typeof job.last_error === "string" && job.last_error.startsWith("skipped:"); + // A job skipped because its dependency skipped never reached a runner, so + // it has no rows; its status is the whole story. + const skipped = job.status === "skipped" || isSkipError(job.last_error); return [{ device_id: device, job_id: job.job_id, status: job.status, - ok: job.status === "done" ? !skipped : job.status === "failed" ? false : null, + ok: job.status === "skipped" ? true : job.status === "done" ? !skipped : job.status === "failed" ? false : null, skipped, error: job.last_error, metrics: null, test: null, failing_step: null, }]; } return entries.map(([device, p]) => { const error = typeof p.error === "string" ? p.error : null; - const skipped = !!error && error.startsWith("skipped:"); + const skipped = isSkipError(error); const test = p.test && typeof p.test === "object" ? { passed: p.test.passed ?? null, failed: p.test.failed ?? null, skipped: p.test.skipped ?? null } : null; @@ -341,7 +344,10 @@ export function summarize(id: string) { const has = (s: string) => jobs.some((j) => j.status === s); const familyStatus = !finished ? has("claimed") ? "claimed" : has("queued") ? "queued" : "waiting" - : jobs.every((j) => j.status === "cancelled") ? "cancelled" : has("failed") ? "failed" : "done"; + : jobs.every((j) => j.status === "cancelled") ? "cancelled" : has("failed") ? "failed" + // Nothing in the family ran: every child skipped (or skipped and was + // cancelled). One that ran is enough for the family to be done. + : !has("done") && has("skipped") ? "skipped" : "done"; const status = fanout ? familyStatus : head.status; return { job_id: id, @@ -592,7 +598,7 @@ export const ROUTES: Route[] = [ operationId: "listJobs", method: "GET", path: "/api/v1/jobs", tag: "jobs", scope: "read", readOnly: true, idempotent: true, summary: "Jobs, newest first, filterable.", params: [ - { name: "status", in: "query", type: "string", description: "Comma-separated: waiting, queued, claimed, done, failed, cancelled." }, + { name: "status", in: "query", type: "string", description: "Comma-separated: waiting, queued, claimed, done, failed, cancelled, skipped." }, { name: "workload", in: "query", type: "string", description: "Comma-separated workload names." }, { name: "executor", in: "query", type: "string", enum: ["device", "host"], description: "device or host." }, { name: "device", in: "query", type: "string", description: "Claimed by, or pinned to, this device." }, diff --git a/collector/src/api/v1/schemas.ts b/collector/src/api/v1/schemas.ts index 6091d8e..90c2500 100644 --- a/collector/src/api/v1/schemas.ts +++ b/collector/src/api/v1/schemas.ts @@ -133,7 +133,7 @@ export const SCHEMAS: Record = { Job: object( { job_id: str, workload: str, executor: { enum: ["device", "host"] }, - status: { enum: ["waiting", "queued", "claimed", "done", "failed", "cancelled"] }, + status: { enum: ["waiting", "queued", "claimed", "done", "failed", "cancelled", "skipped"] }, claimed_by: nstr, attempts: int, max_attempts: int, priority: int, template_id: nstr, depends_on: strs, lease_ttl_s: int, lease_deadline: nstr, lease_remaining_s: nint, duration_s: nint, last_error: nstr, created_at: nstr, claimed_at: nstr, finished_at: nstr, @@ -171,7 +171,7 @@ export const SCHEMAS: Record = { DeviceOutcome: object( { device_id: str, job_id: str, - status: { enum: ["waiting", "queued", "claimed", "done", "failed", "cancelled"] }, + status: { enum: ["waiting", "queued", "claimed", "done", "failed", "cancelled", "skipped"] }, ok: { type: ["boolean", "null"] }, skipped: { ...bool, description: "Nothing ran (no matching device, or a host refusing work). Not a pass." }, error: nstr, metrics: nobj, @@ -221,7 +221,7 @@ export const SCHEMAS: Record = { { valid: bool, job_id: str, - status: { type: "string", description: "What the job would be on enqueue: queued, waiting or failed." }, + status: { type: "string", description: "What the job would be on enqueue: queued, waiting, failed (a dependency failed) or skipped (a dependency skipped)." }, fanout: strs, spec: { ...obj, description: "The spec as it would be stored: `latest` resolved, lease filled in." }, candidates: arr(ref("CandidateDevice")), diff --git a/collector/src/client/api.gen.ts b/collector/src/client/api.gen.ts index 26a3314..4493d39 100644 --- a/collector/src/client/api.gen.ts +++ b/collector/src/client/api.gen.ts @@ -142,7 +142,7 @@ export type DeviceList = { export type DeviceOutcome = { device_id: string; job_id: string; - status: "waiting" | "queued" | "claimed" | "done" | "failed" | "cancelled"; + status: "waiting" | "queued" | "claimed" | "done" | "failed" | "cancelled" | "skipped"; ok: boolean | null; /** Nothing ran (no matching device, or a host refusing work). Not a pass. */ skipped: boolean; @@ -211,7 +211,7 @@ export type Job = { job_id: string; workload: string; executor: "device" | "host"; - status: "waiting" | "queued" | "claimed" | "done" | "failed" | "cancelled"; + status: "waiting" | "queued" | "claimed" | "done" | "failed" | "cancelled" | "skipped"; claimed_by?: string | null; attempts: number; max_attempts: number; @@ -486,7 +486,7 @@ export type TemplateList = { export type Validation = { valid: boolean; job_id?: string; - /** What the job would be on enqueue: queued, waiting or failed. */ + /** What the job would be on enqueue: queued, waiting, failed (a dependency failed) or skipped (a dependency skipped). */ status?: string; fanout?: string[]; /** The spec as it would be stored: `latest` resolved, lease filled in. */ diff --git a/collector/src/db.test.ts b/collector/src/db.test.ts index cf03e8a..d38b250 100644 --- a/collector/src/db.test.ts +++ b/collector/src/db.test.ts @@ -19,7 +19,7 @@ import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { DatabaseSync } from "node:sqlite"; -import { closeDb, db, dbFile, isUniqueViolation, openDb, withTransaction } from "./db.js"; +import { backfillSkipped, closeDb, db, dbFile, dbHandle, isUniqueViolation, openDb, withTransaction } from "./db.js"; type Check = (name: string, cond: boolean, detail?: string) => void; @@ -265,6 +265,75 @@ export function runDbChecks(check: Check) { closeDb(); }); + // --- 'skipped', on a database that holds two weeks of skips as 'done' ----- + // + // The 0.8 shape: every status but 'skipped', and the columns added since by + // ALTER. The live brain's tvloop nightly closed as done every night with a + // skipped: host row; explore's tvOS jobs closed as done over device rows that + // all skipped. Both must come out skipped, a real pass must not, and the + // result rows must be exactly what they were. + inTempDir((dir) => { + const file = path.join(dir, "fleet.db"); + const seed = new DatabaseSync(file); + seed.exec(PRE_DEPENDS_ON_JOBS.replace("('queued','claimed','done','failed','cancelled')", "('waiting','queued','claimed','done','failed','cancelled')")); + seed.exec("ALTER TABLE jobs ADD COLUMN depends_on TEXT"); + seed.exec("ALTER TABLE jobs ADD COLUMN requested_by TEXT"); + seed.exec("ALTER TABLE jobs ADD COLUMN source TEXT"); + seed.exec(`CREATE TABLE results (job_id TEXT NOT NULL, device_id TEXT NOT NULL, iter INTEGER NOT NULL DEFAULT 0, + payload TEXT NOT NULL, created_at TEXT NOT NULL DEFAULT (datetime('now')), PRIMARY KEY (job_id, device_id, iter))`); + const job = seed.prepare("INSERT INTO jobs (job_id, executor, workload, spec, status, requested_by, last_error) VALUES (?,?,?,?,?,?,?)"); + const result = seed.prepare("INSERT INTO results (job_id, device_id, iter, payload) VALUES (?,?,?,?)"); + job.run("nightly-tvloop-hardware-202610150630", "host", "tvloop", "{}", "done", "schedule:nightly-tvloop-hardware", null); + result.run("nightly-tvloop-hardware-202610150630", "host:fleet-host", 0, + JSON.stringify({ final: true, ok: true, error: "skipped: no Roku developer password on this host" })); + job.run("explore-tvos-1", "host", "explore", "{}", "done", null, "lease expired on ultra; requeued after attempt 1/3"); + result.run("explore-tvos-1", "sim-a", 0, JSON.stringify({ ok: true, error: "skipped: no device matches this job on ultra" })); + result.run("explore-tvos-1", "host:ultra", 0, JSON.stringify({ final: true, ok: true })); + job.run("real-pass", "host", "ui-test", "{}", "done", null, null); + result.run("real-pass", "phone-a", 0, JSON.stringify({ ok: true, error: "skipped: locked" })); + result.run("real-pass", "phone-b", 0, JSON.stringify({ ok: true, test: { passed: 2, failed: 0 } })); + result.run("real-pass", "host:mac", 0, JSON.stringify({ final: true, ok: true })); + job.run("real-fail", "host", "ui-test", "{}", "failed", null, null); + result.run("real-fail", "host:mac", 0, JSON.stringify({ final: true, ok: false, error: "skipped: odd but failed" })); + const before = (seed.prepare("SELECT job_id, device_id, iter, payload FROM results ORDER BY job_id, device_id").all() as unknown[]); + seed.close(); + + openDb(dir); + const ddl = (db.prepare("SELECT sql FROM sqlite_master WHERE type='table' AND name='jobs'").get() as { sql: string }).sql; + check("skipped: the rebuild widens the constraint to accept 'skipped'", ddl.includes("'skipped'"), ddl); + const status = (id: string) => db.prepare("SELECT status, last_error, requested_by FROM jobs WHERE job_id = ?").get(id) as + { status: string; last_error: string | null; requested_by: string | null }; + check("skipped: a done job whose final row said skipped is re-labelled", + status("nightly-tvloop-hardware-202610150630").status === "skipped" && + status("nightly-tvloop-hardware-202610150630").last_error === "skipped: no Roku developer password on this host", + JSON.stringify(status("nightly-tvloop-hardware-202610150630"))); + check("skipped: and keeps the columns added since 0.6 (requested_by survives the rebuild)", + status("nightly-tvloop-hardware-202610150630").requested_by === "schedule:nightly-tvloop-hardware"); + check("skipped: one whose every device skipped under an ok host row is re-labelled, its reason replacing a stale note", + status("explore-tvos-1").status === "skipped" && status("explore-tvos-1").last_error === "skipped: no device matches this job on ultra", + JSON.stringify(status("explore-tvos-1"))); + check("skipped: a job where one device ran stays done", status("real-pass").status === "done"); + check("skipped: a failed job stays failed", status("real-fail").status === "failed"); + const after = db.prepare("SELECT job_id, device_id, iter, payload FROM results ORDER BY job_id, device_id").all(); + check("skipped: result rows are untouched", JSON.stringify(after) === JSON.stringify(before)); + const indexes = (db.prepare("SELECT name FROM sqlite_master WHERE type='index' AND tbl_name='jobs'").all() as { name: string }[]).map((i) => i.name); + check("skipped: the rebuild leaves the jobs indexes in place", + ["idx_jobs_lease", "idx_jobs_claim", "idx_jobs_parent", "idx_jobs_requested_by"].every((n) => indexes.includes(n)), JSON.stringify(indexes)); + let accepted = true; + try { + db.prepare("INSERT INTO jobs (job_id, executor, workload, spec, status) VALUES ('s', 'host', 'x', '{}', 'skipped')").run(); + } catch { + accepted = false; + } + check("skipped: a migrated database accepts a skipped job", accepted); + check("skipped: the backfill is idempotent", backfillSkipped(dbHandle()) === 0); + closeDb(); + openDb(dir); + check("skipped: re-opening changes nothing", + (db.prepare("SELECT COUNT(*) AS n FROM jobs WHERE status = 'skipped'").get() as { n: number }).n === 3); + closeDb(); + }); + // --- withTransaction against a handle the module does not own ------------ inTempDir((dir) => { const file = path.join(dir, "standalone.db"); diff --git a/collector/src/db.ts b/collector/src/db.ts index bbb9e57..b1fc04d 100644 --- a/collector/src/db.ts +++ b/collector/src/db.ts @@ -32,6 +32,7 @@ import { DatabaseSync, type SQLInputValue, type StatementSync } from "node:sqlit import { mkdirSync } from "node:fs"; import path from "node:path"; import { DATA_DIR } from "./config.js"; +import { closingStatus } from "./skip.js"; /** * What may be bound to a `?` placeholder. @@ -199,6 +200,49 @@ export const db = { * because the collector is one SQLite file that a person owns and the * alternative is a version number somebody has to remember to bump. */ +/** + * Re-label the jobs that skipped before 'skipped' was a status. + * + * Candidates are the `done` jobs with any result row carrying a `"skipped:` + * string -- one pass over results with `instr`, no JSON parsing -- and each is + * then judged by the same `closingStatus` the queue uses when a job closes, so + * history and new jobs follow one rule. Result rows are not touched; the job's + * `last_error` gets the skip reason, which is where the job list, the alert and + * the digest read it from. + * + * Idempotent: a job it re-labels is no longer `done`, so a second pass finds + * nothing to do. Exported for the test, which runs it twice. + */ +export function backfillSkipped(h: DatabaseSync): number { + const candidates = h + .prepare( + `SELECT j.job_id FROM jobs j + WHERE j.status = 'done' + AND EXISTS (SELECT 1 FROM results r WHERE r.job_id = j.job_id AND instr(r.payload, '"skipped:') > 0)`, + ) + .all() as { job_id: string }[]; + const rowsOf = h.prepare("SELECT device_id, payload FROM results WHERE job_id = ? ORDER BY iter, rowid"); + const relabel = h.prepare("UPDATE jobs SET status = 'skipped', last_error = ? WHERE job_id = ? AND status = 'done'"); + let n = 0; + for (const { job_id } of candidates) { + const rows = (rowsOf.all(job_id) as { device_id: string; payload: string }[]).map((r) => { + let payload: Record = {}; + try { + payload = JSON.parse(r.payload) as Record; + } catch { + /* a row that is not JSON says nothing about a skip */ + } + return { device_id: r.device_id, payload }; + }); + const final = [...rows].reverse().find((r) => r.payload.final)?.payload; + if (!final) continue; + const closing = closingStatus(rows, final); + if (closing.status !== "skipped") continue; + n += Number(relabel.run(closing.reason, job_id).changes); + } + return n; +} + function migrate(db: DatabaseSync): void { db.exec(` CREATE TABLE IF NOT EXISTS devices ( @@ -234,8 +278,11 @@ CREATE TABLE IF NOT EXISTS jobs ( -- 'waiting' is a job whose dependencies have not finished. It is distinct -- from 'queued' because a queued job is one the claim loop should be looking -- at, and a waiting one is not eligible for anything yet. + -- 'skipped' is not 'done': the runner claimed it and could not run it (no + -- device matched, the app was not installed, no Roku password). Counting it + -- as done is how two nightlies "passed" for two weeks while testing nothing. status TEXT NOT NULL DEFAULT 'queued' - CHECK (status IN ('waiting','queued','claimed','done','failed','cancelled')), + CHECK (status IN ('waiting','queued','claimed','done','failed','cancelled','skipped')), created_at TEXT NOT NULL DEFAULT (datetime('now')), claimed_by TEXT, claimed_at TEXT, @@ -256,7 +303,8 @@ CREATE TABLE IF NOT EXISTS jobs ( parent_job_id TEXT, template_id TEXT, -- JSON array of job_ids this one waits for. Resolved to 'queued' when the - -- last of them closes; failed when any of them fails or is cancelled. + -- last of them closes; failed when any of them fails or is cancelled, + -- skipped when one of them skipped. depends_on TEXT ); @@ -673,6 +721,36 @@ CREATE TABLE IF NOT EXISTS findings_digests ( if (!artifactCols.has("media_type")) db.exec("ALTER TABLE artifacts ADD COLUMN media_type TEXT"); } +// 'skipped' as a job status, and the two weeks of skips that closed as 'done' +// before it existed. After every ALTER above, so the rebuild can copy whatever +// columns this database has by name instead of a fixed list that the next +// added column would silently drop. +{ + const ddl = (db.prepare("SELECT sql FROM sqlite_master WHERE type='table' AND name='jobs'").get() as + | { sql: string } + | undefined)?.sql; + if (ddl && !ddl.includes("'skipped'")) { + const widened = ddl + .replace(/^CREATE TABLE\s+("?)jobs\1/i, "CREATE TABLE jobs_migrating") + .replace(/CHECK\s*\(\s*status\s+IN\s*\(([^)]*)\)\s*\)/i, (_m, list: string) => `CHECK (status IN (${list.trim()},'skipped'))`); + if (!widened.includes("'skipped'") || !widened.startsWith("CREATE TABLE jobs_migrating")) + throw new Error("cannot widen the jobs status constraint: the table's DDL is not the shape this migration knows"); + const columns = (db.prepare("PRAGMA table_info(jobs)").all() as { name: string }[]).map((c) => `"${c.name}"`).join(", "); + db.exec("PRAGMA foreign_keys = off"); + withTransaction(db, () => { + db.exec(widened); + db.exec(`INSERT INTO jobs_migrating (${columns}) SELECT ${columns} FROM jobs`); + db.exec("DROP TABLE jobs"); + db.exec("ALTER TABLE jobs_migrating RENAME TO jobs"); + // Inside the same transaction as the rebuild, and only with it: the + // rebuild runs once per database, and a backfill that runs only then + // costs one scan of the results table, once, instead of one per start. + backfillSkipped(db); + })(); + db.exec("PRAGMA foreign_keys = on"); + } +} + // Every mutation that went through the control API, with who made it. Kept // apart from the jobs table because a cancel, a device edit or a refused // request is not a job, and "who did that" is asked of all of them. @@ -692,6 +770,10 @@ CREATE TABLE IF NOT EXISTS audit_log ( ); CREATE INDEX IF NOT EXISTS idx_audit_at ON audit_log (at DESC); CREATE INDEX IF NOT EXISTS idx_jobs_requested_by ON jobs (requested_by); +-- Again, because the 'skipped' rebuild above drops every index on jobs. +CREATE INDEX IF NOT EXISTS idx_jobs_lease ON jobs (status, lease_deadline); +CREATE INDEX IF NOT EXISTS idx_jobs_claim ON jobs (status, executor, priority DESC, created_at); +CREATE INDEX IF NOT EXISTS idx_jobs_parent ON jobs (parent_job_id); `); // Jobs claimed before leases existed have no deadline and would never be swept. diff --git a/collector/src/describe.test.ts b/collector/src/describe.test.ts index a1cbc38..9fcd36f 100644 --- a/collector/src/describe.test.ts +++ b/collector/src/describe.test.ts @@ -68,6 +68,13 @@ export async function runDescribeChecks(check: Check) { check("and is attached to this host, which host routing follows", typeof r1.attached_to === "string"); const r2 = await describeTarget({ id: "roku-gone", platform: "roku", kind: "device" } as never, null, null, []); check("a Roku discovery lost track of is still a Roku, not an iPhone", r2.os === "roku", JSON.stringify(r2)); + // It used to say nothing, and the collector's fallback guess for anything + // that is not iOS is Android: the shelf's Roku Express listed as android. + check("a Roku says its platform is roku", r1.platform === "roku" && r2.platform === "roku", + JSON.stringify([r1.platform, r2.platform])); + const { devicePlatform } = await import("./api/shared.js"); + check("and the collector believes it", devicePlatform(r1) === "roku", devicePlatform(r1)); + check("os ~ 'roku' still selects it", evalMatch("os ~ 'roku'", r1) && evalMatch("os ~ 'roku-15'", r1)); const { rokuPresenceHere } = await import("./drivers/roku.js"); check("Roku presence is off unless this host is told it owns them", diff --git a/collector/src/drivers/roku.ts b/collector/src/drivers/roku.ts index 724dca8..cfd35ef 100644 --- a/collector/src/drivers/roku.ts +++ b/collector/src/drivers/roku.ts @@ -403,9 +403,14 @@ export function rokuPresenceHere(env: NodeJS.ProcessEnv = process.env): boolean * `ios-18.4` have. `ip` is carried because tvloop and the dev installer both * address a Roku by it, and a workload should not have to re-run discovery to * learn what the registry already knew. + * + * `platform: "roku"` is said outright. Without it the collector fell back to + * its old guess -- iOS if `os` looks like iOS, Android otherwise -- and the + * Roku Express on the shelf registered as an Android device. */ export function describeRoku(d: RokuDevice): Record { return { + platform: "roku", model: d.modelName ?? d.modelNumber ?? "roku", os: d.softwareVersion ? `roku-${d.softwareVersion}` : "roku", ...(d.serial ? { serial: d.serial } : {}), diff --git a/collector/src/executor.ts b/collector/src/executor.ts index 4bc477d..10303e3 100644 --- a/collector/src/executor.ts +++ b/collector/src/executor.ts @@ -18,6 +18,8 @@ import { pressureReason, readPressure } from "./pressure.js"; import { runArchive } from "./web/archive/index.js"; import { runDigest } from "./web/digest.js"; import { countXcodebuildTests, xcodebuildDiagnostics } from "./xcparse.js"; +import { junitFailureReason, parseJunit } from "./junit.js"; +import { execFailureReason } from "./result-reason.js"; import { fleetOwned, physicalApple, simulatorName, simulatorPlatform, isAndroidEmulatorSerial, appleNotReadyReason, SIM_PREFIX, @@ -167,13 +169,6 @@ async function listTargets(ios?: IosDeviceInfo[], rokus: RokuDevice[] = []): Pro return dedupe([await adbDriver.list(), await simctlDriver.list(), targetsFrom(ios), rokus.map(rokuTarget)]); } -function parseJunit(xml: string): { passed: number; failed: number } { - const m = /tests="(\d+)"[^>]*failures="(\d+)"/.exec(xml); - if (!m) return { passed: 0, failed: 1 }; - const tests = Number(m[1]); - const failed = Number(m[2]); - return { passed: tests - failed, failed }; -} /** Is this target one the fleet may run work on? Presence and jobs must agree. */ @@ -598,6 +593,10 @@ async function runUiTest(job: Job) { const outDir = mkdtempSync(path.join(os.tmpdir(), "fleet-junit-")); const report = path.join(outDir, "report.xml"); let failedToRun = false; + // Why Maestro produced no report, when it did not: its exit code and the + // last thing it said. Never the error's message, which is the command + // line -- and the command line carries the test account's password. + let runError: string | undefined; // params.network, when the job carries one, is applied around the flow run // and restored afterwards whatever happens inside it. A profile that cannot // be applied fails the device's row by name rather than running the flow @@ -617,9 +616,10 @@ async function runUiTest(job: Job) { flows], { timeout: 600_000 }, ); - } catch { + } catch (e) { // Non-zero exit also just means failing flows; the report tells the truth. failedToRun = !existsSync(report); + if (failedToRun) runError = execFailureReason(e, creds ? [creds.password] : []); } }); } catch (e) { @@ -629,20 +629,34 @@ async function runUiTest(job: Job) { let passed = 0; let failed = 1; + let failure: string | null = null; + let failingFlow: string | undefined; const artifacts: string[] = []; if (!failedToRun && existsSync(report)) { const xml = readFileSync(report, "utf8"); - ({ passed, failed } = parseJunit(xml)); + const junit = parseJunit(xml); + ({ passed, failed } = junit); + // The failing flow and the step that broke, from the report itself: + // `smoke: Assertion is false: "Fleet Runner" is visible`. This used to + // be read by nobody, so a failure posted `failed: 1` and no reason. + failure = junitFailureReason(junit); + if (failure && creds) failure = redact(failure, [creds.password]); + failingFlow = junit.failures[0]?.name; // Scrub before upload: a failing step echoes the command it ran, and the // report is downloadable from the dashboard. if (creds) writeFileSync(report, redact(xml, [creds.password])); artifacts.push(await uploadArtifact(report, `${job.job_id}-${serial}-junit.xml`)); } if (failed > 0) allOk = false; + const error = shapeError + ? `params.network: ${shapeError}` + : failedToRun + ? `maestro produced no report: ${runError ?? "it never ran"}` + : failure ?? undefined; await postResult({ job_id: job.job_id, device_id: serial, iter: 0, - ok: failed === 0, test: { passed, failed, artifacts }, - ...(shapeError ? { error: `params.network: ${shapeError}` } : {}), + ok: failed === 0, test: { passed, failed, artifacts, ...(failingFlow ? { failing_flow: failingFlow } : {}) }, + ...(error ? { error } : {}), }); log(`ui-test on ${serial}: ${passed} passed / ${failed} failed${shapeError ? ` (network shaping: ${shapeError})` : ""}`); } @@ -2370,7 +2384,7 @@ export async function describeTarget( // registered as {model: "iphone", os: "ios"}. const d = rokus.find((r) => rokuTarget(r).id === t.id); return { - ...(d ? describeRoku(d) : { model: "roku", os: "roku", kind: "device" }), + ...(d ? describeRoku(d) : { platform: "roku", model: "roku", os: "roku", kind: "device" }), attached_to: NAME, attached_host: HOST, }; @@ -2828,11 +2842,17 @@ export async function startExecutor(): Promise { log(`job ${job.job_id} skipped: ${e.message}`); continue; } + // A subprocess that failed and was not caught by its workload: its + // exit code and last stderr line, not Node's message, which is the + // whole command line (arguments, and sometimes a password, included). + const reason = typeof (e as { cmd?: unknown }).cmd === "string" + ? execFailureReason(e) + : (e as Error).message.slice(0, 500); await postResult({ job_id: job.job_id, device_id: `host:${NAME}`, iter: 0, final: true, ok: false, - error: (e as Error).message.slice(0, 500), + error: reason, }); - log(`job ${job.job_id} failed: ${(e as Error).message}`); + log(`job ${job.job_id} failed: ${reason}`); } } } diff --git a/collector/src/findings.test.ts b/collector/src/findings.test.ts index e450ed7..9955d8f 100644 --- a/collector/src/findings.test.ts +++ b/collector/src/findings.test.ts @@ -265,6 +265,25 @@ export async function runFindingsChecks(check: Check) { check("digest: high severity first", d.apps[1].top[0].id === a.finding.id); check("digest: judged-not-a-bug is left out", !d.text.includes("Not actually a bug")); + // A skip is a job that ran nothing: its own count and its own line, with + // the schedule and the reason, never folded into done. + const job = (id: string, status: string, err: string | null) => + db.prepare( + "INSERT INTO jobs (job_id, executor, workload, spec, status, last_error, finished_at) VALUES (?, 'host', 'tvloop', '{}', ?, ?, datetime('now'))", + ).run(id, status, err); + job("nightly-ok-202610150230", "done", null); + job("nightly-ui-smoke-202610150230", "failed", 'smoke: Assertion is false: "Fleet Runner" is visible'); + job("nightly-tvloop-hardware-202610150630", "skipped", "skipped: no Roku developer password on this host"); + job("explore-tv-202610150300--sim-a", "skipped", "skipped: no device matches this job on ultra"); + job("explore-tv-202610150300--sim-b", "skipped", "skipped: no device matches this job on ultra"); + const withJobs = buildDigest(12, "http://fleet.test:8788"); + check("digest: jobs are counted done, failed and skipped", + withJobs.jobs.done === 1 && withJobs.jobs.failed === 1 && withJobs.jobs.skipped === 3, JSON.stringify(withJobs.jobs)); + check("digest: the skip line says nothing ran", withJobs.text.includes("Jobs, last 12h: 1 done, 1 failed, 3 skipped (nothing ran)."), withJobs.text); + check("digest: and names what skipped and why, by schedule", + withJobs.text.includes("- skipped: explore-tv x2 (no device matches this job on ultra)") && + withJobs.text.includes("- skipped: nightly-tvloop-hardware (no Roku developer password on this host)"), withJobs.text); + // The timer. const at7 = new Date(2026, 9, 3, 7, 0); check("digest timer: not before HH:MM", !digestDue(new Date(2026, 9, 3, 6, 59), "07:00", null)); diff --git a/collector/src/findings.ts b/collector/src/findings.ts index f1d45f7..0d92d49 100644 --- a/collector/src/findings.ts +++ b/collector/src/findings.ts @@ -39,6 +39,7 @@ import { db } from "./db.js"; import { iso, parse } from "./api/shared.js"; import { sendWebhook } from "./alerts.js"; +import { skipReasonText } from "./skip.js"; import { DASH_URL, FINDINGS_DIGEST_AT, @@ -825,11 +826,55 @@ export type Digest = { again: number; top: { id: number; severity: string; check: string; title: string; screen_name: string; replay: string; new: boolean; seen_count: number }[]; }[]; + /** Jobs that finished in the same window, by how they finished. A skip is + * its own count: it is a job that ran nothing, and folding it into done is + * how two nightlies read as passes for two weeks. */ + jobs: { done: number; failed: number; skipped: number; skipped_by: { name: string; count: number; reason: string }[] }; link: string; text: string; }; const TOP_PER_APP = 5; +const SKIPS_NAMED = 3; + +/** + * How the jobs in the window finished, and which schedules skipped and why. + * A scheduled job's id is `-`, so stripping the minute + * (and a fan-out child's `--device`) names the nightly a person recognises. + */ +export function jobsInWindow(sinceKey: string): Digest["jobs"] { + const counts = db + .prepare( + `SELECT COUNT(*) FILTER (WHERE status = 'done') AS done, + COUNT(*) FILTER (WHERE status = 'failed') AS failed, + COUNT(*) FILTER (WHERE status = 'skipped') AS skipped + FROM jobs WHERE finished_at >= ?`, + ) + .get(sinceKey) as { done: number; failed: number; skipped: number }; + const skips = db + .prepare("SELECT job_id, last_error FROM jobs WHERE status = 'skipped' AND finished_at >= ? ORDER BY finished_at DESC") + .all(sinceKey) as { job_id: string; last_error: string | null }[]; + const by = new Map(); + for (const j of skips) { + const name = j.job_id.split("--")[0].replace(/-\d{12}$/, ""); + const hit = by.get(name) ?? { name, count: 0, reason: skipReasonText(j.last_error) }; + hit.count += 1; + by.set(name, hit); + } + return { + done: counts.done ?? 0, failed: counts.failed ?? 0, skipped: counts.skipped ?? 0, + skipped_by: [...by.values()].sort((a, b) => b.count - a.count), + }; +} + +function jobsLines(j: Digest["jobs"], hours: number): string[] { + if (j.done + j.failed + j.skipped === 0) return []; + const lines = [`Jobs, last ${hours}h: ${j.done} done, ${j.failed} failed, ${j.skipped} skipped (nothing ran).`]; + for (const s of j.skipped_by.slice(0, SKIPS_NAMED)) + lines.push(`- skipped: ${s.name}${s.count > 1 ? ` x${s.count}` : ""} (${s.reason})`); + if (j.skipped_by.length > SKIPS_NAMED) lines.push(`- and ${j.skipped_by.length - SKIPS_NAMED} more skipping`); + return lines; +} /** ntfy turns a body past 4 KB into an attachment; a digest should read as a notification. */ const DIGEST_MAX_CHARS = 3_500; @@ -874,11 +919,14 @@ export function buildDigest(hours: number, dashUrl: string = DASH_URL, now = new const total = rows.length; const fresh = apps.reduce((n, a) => n + a.new, 0); const link = `${dashUrl}/dash/findings?status=open`; + const jobs = jobsInWindow(sinceKey); const plural = (n: number, one: string, many = `${one}s`) => `${n} ${n === 1 ? one : many}`; const lines: string[] = []; if (total === 0) { lines.push(`Night QA, last ${hours}h: nothing new.`); + const jl = jobsLines(jobs, hours); + if (jl.length) lines.push("", ...jl); } else { lines.push( `Night QA, last ${hours}h: ${plural(total, "finding")} (${fresh} new, ${total - fresh} seen again) in ${plural(apps.length, "app")}.`, @@ -892,11 +940,13 @@ export function buildDigest(hours: number, dashUrl: string = DASH_URL, now = new } if (a.count > a.top.length) lines.push(`- and ${a.count - a.top.length} more`); } + const jl = jobsLines(jobs, hours); + if (jl.length) lines.push("", ...jl); lines.push("", `Triage: ${link}`); } let text = lines.join("\n"); if (text.length > DIGEST_MAX_CHARS) text = `${text.slice(0, DIGEST_MAX_CHARS - 40).trimEnd()}\n...\nTriage: ${link}`; - return { hours, since: since.toISOString(), total, new: fresh, again: total - fresh, apps, link, text }; + return { hours, since: since.toISOString(), total, new: fresh, again: total - fresh, apps, jobs, link, text }; } /** Local calendar day, YYYY-MM-DD: "once a day" is the owner's day, not UTC's. */ diff --git a/collector/src/fleet-client.ts b/collector/src/fleet-client.ts index 80b6750..4a39792 100644 --- a/collector/src/fleet-client.ts +++ b/collector/src/fleet-client.ts @@ -8,6 +8,7 @@ import { promisify } from "node:util"; import { createHash } from "node:crypto"; import { readFileSync, writeFileSync } from "node:fs"; import os from "node:os"; +import { withFailureReasons } from "./result-reason.js"; export const exec = promisify(execFile); @@ -66,9 +67,14 @@ export async function post(url: string, body: unknown) { if (!res.ok) throw new Error(`POST ${url} -> ${res.status}`); } -export async function postResult(row: Record) { +/** + * Post one result row. Every host workload's rows go through here, so this is + * where a row that says `ok: false` and nothing else is given a reason -- see + * result-reason.ts. + */ +export const postResult = withFailureReasons(async (row: Record) => { await post("/results", { schema: 1, kind: "result", ...row }); -} +}); export async function postBeacon(jobId: string, deviceId: string, extra: Record) { await post("/results", { diff --git a/collector/src/junit.ts b/collector/src/junit.ts new file mode 100644 index 0000000..8b7490e --- /dev/null +++ b/collector/src/junit.ts @@ -0,0 +1,73 @@ +/** + * Reading a JUnit report: how many passed, how many failed, and what failed. + * + * Maestro writes one `` per flow with a `` inside naming the + * step that broke (`Assertion is false: "Fleet Runner" is visible`). The counts + * were always read; the failure was not, so a failing ui-test posted + * `failed: 1` and no reason while the reason sat in the report it uploaded. + * + * Regexes, not an XML parser: the reports come from a handful of known tools, + * the same pragmatism the counts have always had, and a dependency for one + * element would be a poor trade. + */ + +export type JunitFailure = { name: string; message: string }; +export type JunitSummary = { passed: number; failed: number; failures: JunitFailure[] }; + +const ENTITIES: Record = { quot: '"', apos: "'", lt: "<", gt: ">", amp: "&" }; + +function decode(s: string): string { + return s + .replace(//g, "$1") + .replace(/&(quot|apos|lt|gt|amp);/g, (_m, e: string) => ENTITIES[e]) + .replace(/&#(\d+);/g, (_m, n: string) => String.fromCodePoint(Number(n))) + .replace(/&#x([0-9a-f]+);/gi, (_m, n: string) => String.fromCodePoint(parseInt(n, 16))); +} + +const attr = (tag: string, name: string) => { + const m = new RegExp(`\\b${name}="([^"]*)"`).exec(tag); + return m ? decode(m[1]) : null; +}; + +/** + * Counts and failures. A report with no recognisable counts is one failure: + * a run that produced garbage did not pass. Counts come from the first + * `tests`/`failures` pair (Maestro's ``); `errors` count as + * failures, because an errored flow did not pass either. + */ +export function parseJunit(xml: string): JunitSummary { + const failures: JunitFailure[] = []; + for (const tc of xml.matchAll(/]*?)(?:\/>|>([\s\S]*?)<\/testcase>)/g)) { + const body = tc[2] ?? ""; + const f = /<(failure|error)\b([^>]*?)(?:\/>|>([\s\S]*?)<\/\1>)/.exec(body); + if (!f) continue; + const text = decode((f[3] ?? "").trim()); + const message = (text || attr(f[2], "message") || attr(f[2], "type") || f[1]).trim(); + failures.push({ name: attr(tc[1], "name") ?? attr(tc[1], "id") ?? "test", message }); + } + + const suite = /]*>/.exec(xml)?.[0] ?? /]*>/.exec(xml)?.[0] ?? ""; + const tests = Number(attr(suite, "tests")); + if (!suite || !Number.isFinite(tests)) { + return failures.length ? { passed: 0, failed: failures.length, failures } : { passed: 0, failed: 1, failures }; + } + const failed = Number(attr(suite, "failures") ?? 0) + Number(attr(suite, "errors") ?? 0); + return { passed: Math.max(0, tests - failed), failed: Math.max(failed, failures.length), failures }; +} + +/** + * One line for the result's `error`: the first failing test and its step, and + * how many more there were. Null when nothing failed. + */ +export function junitFailureReason(s: JunitSummary, max = 300): string | null { + if (s.failed === 0) return null; + const first = s.failures[0]; + let line: string; + if (!first) line = `${s.failed} of ${s.passed + s.failed} failed; the report names no failing step`; + else { + const msg = first.message.split("\n").map((l) => l.trim()).find(Boolean) ?? "failed"; + line = `${first.name}: ${msg}`; + if (s.failed > 1) line += ` (and ${s.failed - 1} more failing)`; + } + return line.length > max ? `${line.slice(0, max - 3)}...` : line; +} diff --git a/collector/src/queue.ts b/collector/src/queue.ts index 5941e0e..823d83a 100644 --- a/collector/src/queue.ts +++ b/collector/src/queue.ts @@ -81,7 +81,8 @@ export type JobSpec = { template_id?: string; // Job ids this one waits for. The job is inserted 'waiting' and promoted to // 'queued' when the last of them is 'done' — a build → install → ui-test - // chain is three POSTs and nothing watching the queue. + // chain is three POSTs and nothing watching the queue. If one of them fails + // the job fails; if one of them skipped, the job skips. depends_on?: string[]; // The runner will stand down and checkpoint if higher-priority work arrives // for the same claimant. Honoured by the collector's beacon reply; a runner @@ -271,7 +272,8 @@ export type CandidateDevice = { device_id: string; model: string | null; os: str export type EnqueueAccepted = { ok: true; http: 200 | 201; - /** queued, waiting (on depends_on), or failed (a dependency already failed). */ + /** queued, waiting (on depends_on), failed (a dependency already failed) or + * skipped (a dependency already skipped, so there is nothing to run on). */ status: string; job_id: string; /** Present when the spec fanned out: one child per device. */ @@ -480,6 +482,7 @@ export function enqueueJob( }; } + const closedOnArrival = depState.status === "failed" || depState.status === "skipped"; // Every child inherits the parent's dependencies, so a fan-out can hang off // a single build the same way one pinned job can. const insert = db.prepare( @@ -491,12 +494,12 @@ export function enqueueJob( const r = insert.run( child.job_id, child.executor, child.workload, JSON.stringify(child), ttlS, maxAttempts, priority, spec.job_id, templateId, dependsOn, depState.status, - depState.status === "failed" ? depState.reason : null, + closedOnArrival ? depState.reason : null, provenance.requested_by, provenance.source, ); if (r.changes > 0) created.push(child.job_id); } - if (depState.status === "failed") + if (closedOnArrival) for (const jobId of created) db.prepare("UPDATE jobs SET finished_at = datetime('now') WHERE job_id = ?").run(jobId); for (const jobId of created) @@ -517,10 +520,10 @@ export function enqueueJob( try { db.prepare( `INSERT INTO jobs (job_id, executor, workload, spec, lease_ttl_s, max_attempts, priority, template_id, depends_on, status, last_error, finished_at, requested_by, source) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ${depState.status === "failed" ? "datetime('now')" : "NULL"}, ?, ?)`, + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ${depState.status === "failed" || depState.status === "skipped" ? "datetime('now')" : "NULL"}, ?, ?)`, ).run( spec.job_id, spec.executor, spec.workload, JSON.stringify(spec), ttlS, maxAttempts, priority, templateId, - dependsOn, depState.status, depState.status === "failed" ? depState.reason : null, + dependsOn, depState.status, depState.status === "failed" || depState.status === "skipped" ? depState.reason : null, provenance.requested_by, provenance.source, ); } catch (e: unknown) { @@ -539,6 +542,7 @@ export function enqueueJob( function warningsFor(spec: JobSpec, candidates: number, deps: { status: string; reason?: string }): string[] { const out: string[] = []; if (deps.status === "failed") out.push(`a dependency already failed (${deps.reason ?? "unknown"}); the job would be failed at once`); + if (deps.status === "skipped") out.push(`a dependency already skipped (${deps.reason ?? "unknown"}); the job would be skipped at once, since there is nothing for it to run on`); if (deps.status === "waiting") out.push("waiting on depends_on; it starts when those jobs are done"); if (spec.executor === "device" && candidates === 0) { out.push("no registered device matches these targets and can run this workload; the job would queue until one does"); diff --git a/collector/src/registry-hygiene.test.ts b/collector/src/registry-hygiene.test.ts new file mode 100644 index 0000000..16df4ef --- /dev/null +++ b/collector/src/registry-hygiene.test.ts @@ -0,0 +1,74 @@ +/** + * Registry hygiene, against a real database. + * + * The rows are the live brain's, October 2026: an `iPhone 17 Pro` simulator + * UUID offline for three weeks, a `fleet-explore-tv` simulator that registered + * before `soc` was written, a real iPhone offline even longer (which must stay: + * a phone in a drawer is still a phone), an Android emulator, and the Roku that + * was listed twice. `last_seen` is written directly because a week of silence + * is the whole point and a suite cannot wait for it. + */ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { closeDb, openDb } from "./db.js"; +import { declaredSimulator, forgetStaleSimulators, retireRokuIpDuplicate, rokuIpId, simulatorTtlDays } from "./registry-hygiene.js"; + +type Check = (name: string, cond: boolean, detail?: string) => void; + +export function runRegistryHygieneChecks(check: Check) { + closeDb(); + const dir = mkdtempSync(path.join(tmpdir(), "fleet-hygiene-test-")); + try { + const db = openDb(dir); + const add = (id: string, descriptor: Record, daysAgo: number) => + db.prepare( + `INSERT INTO devices (device_id, descriptor, pools, last_seen) VALUES (?, ?, '[]', datetime('now', ?))`, + ).run(id, JSON.stringify(descriptor), `-${daysAgo * 86_400} seconds`); + const exists = (id: string) => !!db.prepare("SELECT 1 FROM devices WHERE device_id = ?").get(id); + + add("75619D0E-5B01-46FA-967B-0964985BCE93", { model: "iPhone 17 Pro", os: "ios-27.0", soc: "simulator", kind: "simulator" }, 21); + add("CECF9000-9FF1-4511-8D1E-CD3E2E34E1E0", { model: "fleet-explore-tv", os: "ios", kind: "simulator" }, 13); + add("73BF450F-DD86-4DBC-AE25-763DE93B7508", { model: "simulator", os: "ios", soc: "simulator" }, 19); + add("799804C0-17C7-430C-835D-AFCD50824381", { model: "iPhone 18 Pro", os: "ios-27.0", soc: "simulator", kind: "simulator" }, 1); + add("09A99EFE-56C4-5FCF-9170-305B974BF662", { model: "iPhone 12 Pro", os: "ios-18.7.8", soc: "iPhone13,3", kind: "device" }, 51); + add("emulator-5554", { model: "sdk_gphone64_arm64", os: "android-15", kind: "device" }, 30); + add("machine-intel", { model: "MacBook Pro", soc: "x86_64", os: "macos-12.7.6", kind: "laptop" }, 40); + add("LOCKED-SIM", { model: "fleet-sim-1", kind: "simulator", soc: "simulator" }, 30); + db.prepare("INSERT INTO device_locks (device_id, job_id) VALUES ('LOCKED-SIM', 'some-job')").run(); + + check("hygiene: the TTL defaults to 7 days and reads FLEET_SIMULATOR_TTL_DAYS", + simulatorTtlDays({}) === 7 && simulatorTtlDays({ FLEET_SIMULATOR_TTL_DAYS: "2" }) === 2 && simulatorTtlDays({ FLEET_SIMULATOR_TTL_DAYS: "nope" }) === 7); + check("hygiene: only a declared simulator counts", + declaredSimulator({ kind: "simulator" }) && declaredSimulator({ soc: "simulator" }) && + !declaredSimulator({ soc: "x86_64" }) && !declaredSimulator({ model: "sdk_gphone64_arm64" })); + + const gone = forgetStaleSimulators(7); + check("hygiene: simulators offline past the TTL are forgotten", + !exists("75619D0E-5B01-46FA-967B-0964985BCE93") && !exists("CECF9000-9FF1-4511-8D1E-CD3E2E34E1E0") && + !exists("73BF450F-DD86-4DBC-AE25-763DE93B7508"), JSON.stringify(gone)); + check("hygiene: and named in what the sweep returns", gone.length === 3, JSON.stringify(gone)); + check("hygiene: a simulator seen yesterday stays", exists("799804C0-17C7-430C-835D-AFCD50824381")); + check("hygiene: a real iPhone is never forgotten, however long it has been away", exists("09A99EFE-56C4-5FCF-9170-305B974BF662")); + check("hygiene: nor an Android emulator, which comes back under the same serial", exists("emulator-5554")); + check("hygiene: nor an Intel machine whose soc merely looks like a simulator's", exists("machine-intel")); + check("hygiene: nor a simulator that holds a lock", exists("LOCKED-SIM")); + check("hygiene: a second sweep has nothing left to do", forgetStaleSimulators(7).length === 0); + + // --- the Roku listed twice --- + check("roku: the IP-named id is the executor's", rokuIpId("192.168.50.218") === "roku-ip-192-168-50-218"); + add("roku-ip-192-168-50-218", { model: "roku", os: "roku", ip: "192.168.50.218", kind: "device" }, 3); + add("roku-ip-192-168-50-99", { model: "roku", os: "roku", ip: "192.168.50.99", kind: "device" }, 3); + const serial = { platform: "roku", model: "Roku Express 4K+", os: "roku-15.3.4", serial: "X01600EMFRJ9", ip: "192.168.50.218" }; + check("roku: an IP-named registration never displaces anything", retireRokuIpDuplicate("roku-ip-192-168-50-218", serial) === null); + check("roku: a serial-named registration retires the duplicate at its address", + retireRokuIpDuplicate("roku-X01600EMFRJ9", serial) === "roku-ip-192-168-50-218" && !exists("roku-ip-192-168-50-218")); + check("roku: and only that one", exists("roku-ip-192-168-50-99")); + check("roku: a device with no address retires nothing", retireRokuIpDuplicate("roku-X01600EMFRJ9", { model: "roku" }) === null); + check("roku: nor does a phone that happens to report an ip", + retireRokuIpDuplicate("988a1b3541354f565a", { ip: "192.168.50.99" }) === null && exists("roku-ip-192-168-50-99")); + } finally { + closeDb(); + rmSync(dir, { recursive: true, force: true }); + } +} diff --git a/collector/src/registry-hygiene.ts b/collector/src/registry-hygiene.ts new file mode 100644 index 0000000..2e7e9e5 --- /dev/null +++ b/collector/src/registry-hygiene.ts @@ -0,0 +1,92 @@ +/** + * Keeping the device registry down to real things. + * + * Two kinds of row outlive what they describe, and both were on the live brain + * in October 2026: thirteen offline iOS rows that were simulator UUIDs from + * runs long gone, and a Roku listed twice -- `roku-ip-192-168-50-218`, from a + * sweep where the box answered SSDP but not device-info, offline beside the + * `roku-X01600EMFRJ9` it really is. A fan-out with no pool reaches every row, + * and a shelf where a third of the entries are ghosts is a shelf nobody reads. + * + * Forgetting here is what `DELETE /api/devices/:id` does: the row goes, the + * results and beacons it posted stay, because they are measurements and a + * device leaving the shelf does not make them untrue. + */ +import { db } from "./db.js"; +import { parse } from "./api/shared.js"; + +/** How long a simulator may sit offline before it is forgotten. */ +export function simulatorTtlDays(env: NodeJS.ProcessEnv = process.env): number { + const v = Number(env.FLEET_SIMULATOR_TTL_DAYS); + return Number.isFinite(v) && v > 0 ? v : 7; +} + +/** + * Is this row a simulator by its own say-so? + * + * Only what the agent declared: `kind: "simulator"` or `soc: "simulator"`, + * which is what the host executor writes for every Apple simulator. Not the + * looser `isSimulator()` heuristic the dashboard uses to keep simulators out of + * hardware tables -- that one also fires on `x86_64`, which a real Intel + * machine can report, and a real device is never forgotten. Android emulators + * are not forgotten either: `emulator-5554` comes back under the same serial + * the next time an AVD boots, and they already carry a presence TTL that hides + * them while they are gone. + */ +export function declaredSimulator(descriptor: Record): boolean { + return descriptor.kind === "simulator" || descriptor.soc === "simulator"; +} + +/** + * Forget simulator rows that have been offline longer than the TTL. Run by the + * lease sweep. A simulator is recreated, never brought back: a UUID that has + * not checked in for a week belongs to a device that no longer exists. + * + * A row that holds a lock or a claim is left alone, however old -- the same + * rule the dashboard's Forget button follows. + */ +export function forgetStaleSimulators(days = simulatorTtlDays()): string[] { + const rows = db + .prepare( + `SELECT device_id, descriptor FROM devices + WHERE last_seen <= datetime('now', ?) + AND NOT EXISTS (SELECT 1 FROM device_locks l WHERE l.device_id = devices.device_id) + AND NOT EXISTS (SELECT 1 FROM jobs j WHERE j.status = 'claimed' AND j.claimed_by = devices.device_id)`, + ) + .all(`-${Math.round(days * 86_400)} seconds`) as { device_id: string; descriptor: string }[]; + const gone: string[] = []; + const del = db.prepare("DELETE FROM devices WHERE device_id = ?"); + for (const r of rows) { + if (!declaredSimulator(parse>(r.descriptor, {}))) continue; + if (del.run(r.device_id).changes > 0) gone.push(r.device_id); + } + return gone; +} + +/** `roku-ip-192-168-50-218` for 192.168.50.218: the id the executor gives a Roku it knows only by address. */ +export function rokuIpId(ip: string): string { + return `roku-ip-${ip.replace(/\./g, "-")}`; +} + +/** + * When a Roku registers under its serial, retire the row the same box left + * when it was known only by its IP. + * + * The executor names a Roku `roku-` when device-info answers and + * `roku-ip-
` when only SSDP did, so one flaky sweep is enough to leave + * a second row that never checks in again. The serial-named row re-registers + * every minute, so a duplicate that comes back from another flaky sweep is + * retired within the minute too. Not the other way round: an IP-named row + * never displaces a serial-named one, because the serial is the better name. + */ +export function retireRokuIpDuplicate(deviceId: string, descriptor: Record): string | null { + if (!/^roku-/.test(deviceId) || deviceId.startsWith("roku-ip-")) return null; + const ip = descriptor.ip; + if (typeof ip !== "string" || !/^\d{1,3}(\.\d{1,3}){3}$/.test(ip)) return null; + const dup = rokuIpId(ip); + const held = + db.prepare("SELECT 1 FROM device_locks WHERE device_id = ?").get(dup) ?? + db.prepare("SELECT 1 FROM jobs WHERE status = 'claimed' AND claimed_by = ?").get(dup); + if (held) return null; + return db.prepare("DELETE FROM devices WHERE device_id = ?").run(dup).changes > 0 ? dup : null; +} diff --git a/collector/src/result-reason.test.ts b/collector/src/result-reason.test.ts new file mode 100644 index 0000000..a9175e0 --- /dev/null +++ b/collector/src/result-reason.test.ts @@ -0,0 +1,95 @@ +/** + * Every failure says why: the JUnit report's failing step, a subprocess's exit + * code and last line, and a host row that would otherwise say only `ok: false`. + * + * The report below is the one `nightly-fleet-ui-smoke-202610150230` uploaded + * on 2026-10-15 for the Galaxy S8+ (988a1b3541354f565a), byte for byte apart + * from the path. The job posted `failed: 1` and no error while this sat in + * the store. + */ +import { junitFailureReason, parseJunit } from "./junit.js"; +import { execFailureReason, lastLine, withFailureReasons } from "./result-reason.js"; + +type Check = (name: string, cond: boolean, detail?: string) => void; + +const MAESTRO_REPORT = ` + + + + Assertion is false: "Fleet Runner" is visible + + + +`; + +export async function runReasonChecks(check: Check) { + // --- JUnit --------------------------------------------------------------- + const s = parseJunit(MAESTRO_REPORT); + check("junit: Maestro's counts are read", s.passed === 0 && s.failed === 1, JSON.stringify(s)); + check("junit: and so is the failing flow and step", + s.failures[0]?.name === "smoke" && s.failures[0]?.message === 'Assertion is false: "Fleet Runner" is visible', JSON.stringify(s.failures)); + check("junit: one line for the result's error", + junitFailureReason(s) === 'smoke: Assertion is false: "Fleet Runner" is visible', String(junitFailureReason(s))); + + const two = parseJunit(` + + + + `); + check("junit: a self-closing failure's message attribute is read", two.failures[0]?.message === "Element not found: Sign in", JSON.stringify(two.failures)); + check("junit: CDATA is unwrapped", two.failures[1]?.message.startsWith("Expected 3 rows"), JSON.stringify(two.failures[1])); + check("junit: more than one failure says how many more", + junitFailureReason(two) === "login: Element not found: Sign in (and 1 more failing)", String(junitFailureReason(two))); + check("junit: a passing report has no reason", junitFailureReason(parseJunit('')) === null); + const garbage = parseJunit("not xml at all"); + check("junit: a report with no counts is one failure, not a pass", garbage.failed === 1 && garbage.passed === 0); + check("junit: and says the report named no step", /names no failing step/.test(junitFailureReason(garbage) ?? "")); + check("junit: errors count as failures", + parseJunit('').failed === 1); + + // --- a subprocess that failed ------------------------------------------------ + const err = Object.assign(new Error("Command failed: maestro --device X test -e GREENFOLIO_TEST_PASSWORD=hunter2 flows\nboom"), { + cmd: "/opt/maestro/bin/maestro --device X test -e GREENFOLIO_TEST_PASSWORD=hunter2 flows", + code: 1, + stderr: "Running flow smoke\nError: device X is offline (hunter2)\n", + stdout: "", + }); + const r = execFailureReason(err, ["hunter2"]); + check("exec: the program, its exit code and its last stderr line", r === "maestro exited 1: Error: device X is offline ([redacted])", r); + check("exec: never the command line, which carries the password", !r.includes("PASSWORD") && !r.includes("hunter2"), r); + check("exec: a program that could not start says so", + execFailureReason(Object.assign(new Error("spawn maestro ENOENT"), { cmd: "maestro test", code: "ENOENT" })) === "maestro could not run (ENOENT): spawn maestro ENOENT"); + check("exec: a killed one says so", + /^xcodebuild was killed \(SIGTERM\)/.test(execFailureReason({ cmd: "xcodebuild test", killed: true, signal: "SIGTERM", stderr: "" }))); + check("exec: silence is said, not left blank", + execFailureReason({ cmd: "adb shell true", code: 255, stderr: "", stdout: "" }) === "adb exited 255 and printed nothing"); + check("lastLine skips trailing blank lines", lastLine("a\nb\n\n \n") === "b"); + + // --- no ok:false row goes out without a reason ------------------------------- + const posted: Record[] = []; + const post = withFailureReasons(async (row: Record) => { posted.push(row); }); + await post({ job_id: "j1", device_id: "988a1b35", iter: 0, ok: false, test: { passed: 0, failed: 1 }, error: 'smoke: Assertion is false: "Fleet Runner" is visible' }); + await post({ job_id: "j1", device_id: "host:fleet-host", iter: 0, final: true, ok: false }); + check("reasons: a failing host row with no error gets the failing device's", + posted[1]?.error === '988a1b35: smoke: Assertion is false: "Fleet Runner" is visible', JSON.stringify(posted[1])); + + await post({ job_id: "j2", device_id: "a", iter: 0, ok: true }); + await post({ job_id: "j2", device_id: "b", iter: 0, ok: false, test: { passed: 1, failed: 2 } }); + await post({ job_id: "j2", device_id: "c", iter: 0, ok: false, error: "install failed: INSTALL_FAILED_OLDER_SDK" }); + await post({ job_id: "j2", device_id: "host:mac", iter: 0, final: true, ok: false }); + check("reasons: a device row with only counts says what the counts say", + posted[3]?.error === "2 of 3 tests failed; the runner reported no failing step", JSON.stringify(posted[3])); + check("reasons: several failing devices are counted and named", + posted[5]?.error === "2 of 3 devices failed -- b: 2 of 3 tests failed; the runner reported no failing step; c: install failed: INSTALL_FAILED_OLDER_SDK", + String(posted[5]?.error)); + + await post({ job_id: "j3", device_id: "host:mac", iter: 0, final: true, ok: false, error: "executor mac has no browsers" }); + check("reasons: a row that already says why is left alone", posted[6]?.error === "executor mac has no browsers"); + await post({ job_id: "j4", device_id: "host:mac", iter: 0, final: true, ok: true }); + check("reasons: a pass is left alone", posted[7]?.error === undefined); + await post({ job_id: "j5", device_id: "host:mac", iter: 0, final: true, ok: false }); + check("reasons: a failure with nothing behind it still says so", posted[8]?.error === "failed; the workload reported no reason"); + await post({ job_id: "j1", device_id: "host:fleet-host", iter: 0, final: true, ok: false }); + check("reasons: a job's devices are forgotten once its final row is out", posted[9]?.error === "failed; the workload reported no reason"); +} diff --git a/collector/src/result-reason.ts b/collector/src/result-reason.ts new file mode 100644 index 0000000..17024f4 --- /dev/null +++ b/collector/src/result-reason.ts @@ -0,0 +1,119 @@ +/** + * Every failure says why. + * + * `nightly-fleet-ui-smoke` failed on the Galaxy S8+ every night from 10-10 with + * `test: {passed: 0, failed: 1}`, no `error`, and a host row that said only + * `ok: false`. The reason was sitting in the JUnit report it had uploaded -- + * `Assertion is false: "Fleet Runner" is visible` -- and nothing put it where a + * person or an agent reads. A failure with no reason costs whoever reads it a + * trip to a log on another machine, which is a trip nobody takes at 07:00. + * + * Two pieces. `execFailureReason` turns a failed subprocess into one line (the + * program, its exit code, the last line it printed to stderr). The tracker + * wraps the executor's `postResult` so that no host row can say `ok: false` + * with nothing beside it: a device row gets its test counts, and the final + * host row gets the failing devices' reasons, which is what the job list and + * the `job-failed` alert show. + * + * Pure apart from the closure's memory, so it is testable without a collector. + */ + +const ONE_LINE = 300; +const FINAL_MAX = 500; + +const clip = (s: string, n: number) => (s.length > n ? `${s.slice(0, n - 3)}...` : s); +const firstLine = (s: string) => s.split("\n").map((l) => l.trim()).find(Boolean) ?? ""; + +/** The last non-empty line of some output, trimmed. */ +export function lastLine(text: unknown): string { + if (typeof text !== "string" && !Buffer.isBuffer(text)) return ""; + const lines = String(text).split(/\r?\n/).map((l) => l.trim()).filter(Boolean); + return lines.at(-1) ?? ""; +} + +/** + * One line for a subprocess that failed, from what `execFile` rejects with. + * + * Built from the exit code and the output, never from `err.message`: Node puts + * the whole command line in the message, and a Maestro run carries the test + * account's password as `-e PASSWORD=...` on that line. `secrets` are scrubbed + * from what is used anyway, because a tool can echo its arguments. + */ +export function execFailureReason(err: unknown, secrets: string[] = []): string { + const e = (err ?? {}) as { + cmd?: unknown; code?: unknown; signal?: unknown; killed?: unknown; stderr?: unknown; stdout?: unknown; message?: unknown; + }; + const program = typeof e.cmd === "string" ? (e.cmd.trim().split(/\s+/)[0] ?? "").split("/").pop() || "the command" : "the command"; + let how: string; + if (e.killed === true || typeof e.signal === "string") how = `was killed${typeof e.signal === "string" ? ` (${e.signal})` : ""}`; + else if (typeof e.code === "number") how = `exited ${e.code}`; + else if (typeof e.code === "string") how = `could not run (${e.code})`; + else how = "failed"; + let said = lastLine(e.stderr) || lastLine(e.stdout); + // Not the message's first line, which is "Command failed: ". + if (!said && typeof e.message === "string" && !e.message.startsWith("Command failed")) said = firstLine(e.message); + for (const s of secrets) if (s) said = said.split(s).join("[redacted]"); + return clip(said ? `${program} ${how}: ${said}` : `${program} ${how} and printed nothing`, ONE_LINE); +} + +type Row = Record & { job_id?: unknown; device_id?: unknown; ok?: unknown; error?: unknown; final?: unknown }; + +const hasReason = (e: unknown) => typeof e === "string" && e.trim() !== ""; + +/** What a device row that failed with no error can still honestly say. */ +function deviceReason(row: Row): string { + const t = (row.test ?? null) as { passed?: unknown; failed?: unknown } | null; + const failed = typeof t?.failed === "number" ? t.failed : null; + const passed = typeof t?.passed === "number" ? t.passed : null; + if (failed !== null && failed > 0) { + const total = failed + (passed ?? 0); + return `${failed} of ${total} test${total === 1 ? "" : "s"} failed; the runner reported no failing step`; + } + return "failed; the workload reported no reason"; +} + +/** + * Wrap a `postResult` so that no row it posts says `ok: false` with an empty + * `error`. + * + * Device rows are remembered per job until the job's final row goes out. A + * final row that failed with no error of its own gets one built from them: + * `988a1b35...: smoke: Assertion is false: ...`, or `2 of 3 devices failed -- + * a: ...; b: ...`. A row that already says why is never rewritten. + */ +export function withFailureReasons(post: (row: Record) => Promise) { + const seen = new Map>(); + return async (row: Record): Promise => { + const r = row as Row; + const jobId = typeof r.job_id === "string" ? r.job_id : null; + const deviceId = typeof r.device_id === "string" ? r.device_id : ""; + let out: Record = row; + + if (r.final !== true) { + if (r.ok === false && !hasReason(r.error)) out = { ...row, error: deviceReason(r) }; + if (jobId && !deviceId.startsWith("host:")) { + const devices = seen.get(jobId) ?? new Map(); + devices.set(deviceId, { ok: out.ok !== false, error: hasReason(out.error) ? String(out.error) : null }); + seen.set(jobId, devices); + } + return post(out); + } + + const devices = jobId ? seen.get(jobId) : undefined; + if (jobId) seen.delete(jobId); + if (r.ok === false && !hasReason(r.error)) { + const failing = [...(devices ?? new Map()).entries()].filter(([, d]) => !d.ok); + const total = devices?.size ?? 0; + let error: string; + if (failing.length === 0) error = "failed; the workload reported no reason"; + else { + const parts = failing.map(([id, d]) => `${id}: ${firstLine(d.error ?? "no reason given")}`); + error = failing.length === 1 && total <= 1 + ? parts[0] + : `${failing.length} of ${total} devices failed -- ${parts.join("; ")}`; + } + out = { ...row, error: clip(error, FINAL_MAX) }; + } + return post(out); + }; +} diff --git a/collector/src/server.ts b/collector/src/server.ts index 6833b7b..1fb43f1 100644 --- a/collector/src/server.ts +++ b/collector/src/server.ts @@ -40,6 +40,8 @@ import { admit, isTailnetAddress, normaliseIp, whois } from "./tailnet.js"; // from this file, so there is no cycle. import { firstArtifactSha, settleWaiters, type DepSettlement } from "./api/mutations.js"; import { announce } from "./announce.js"; +import { closingStatus, type Closing } from "./skip.js"; +import { forgetStaleSimulators, retireRokuIpDuplicate } from "./registry-hygiene.js"; import { enqueueJob, matchingDevices, resolveLatestBuild, type JobSpec } from "./queue.js"; import { authOf, authWarnings, provenanceOf, registerAuth } from "./auth.js"; import { needsAdmin } from "./api/policy.js"; @@ -461,6 +463,13 @@ const sweepLeasesTx = db.transaction((): { requeued: SweptJob[]; failed: SweptJo function sweepLeases() { const swept = sweepLeasesTx(); + // Registry hygiene rides on the same timer: simulators offline past + // FLEET_SIMULATOR_TTL_DAYS are forgotten. See registry-hygiene.ts. + const forgotten = forgetStaleSimulators(); + for (const id of forgotten) { + app.log.info({ device_id: id }, "simulator offline past its TTL; forgotten"); + announce({ type: "device", device_id: id, event: "forget", reason: "simulator offline past FLEET_SIMULATOR_TTL_DAYS" }); + } for (const j of swept.requeued) { app.log.warn({ job_id: j.job_id, claimed_by: j.claimed_by, attempt: j.attempts }, "lease expired; requeued"); announce({ type: "job", job_id: j.job_id, status: "queued", reason: "lease expired", attempts: j.attempts }); @@ -469,7 +478,7 @@ function sweepLeases() { app.log.error({ job_id: j.job_id, claimed_by: j.claimed_by, attempts: j.attempts }, "lease expired; job failed"); announce({ type: "job", job_id: j.job_id, status: "failed", reason: "lease expired", attempts: j.attempts }); } - return swept; + return { ...swept, forgotten }; } // --- devices --- @@ -538,6 +547,12 @@ app.post("/devices/register", async (req, reply) => { type: "device", device_id: b.device_id, event: "register", pools: b.pools ?? [], capabilities: b.capabilities ?? null, }); + // The same Roku, seen earlier by address only: one row is the truth. + const retired = retireRokuIpDuplicate(b.device_id, (b.descriptor ?? {}) as Record); + if (retired) { + app.log.info({ device_id: retired, now: b.device_id }, "retired a Roku's IP-named duplicate"); + announce({ type: "device", device_id: retired, event: "forget", reason: `same Roku as ${b.device_id}` }); + } return { ok: true }; }); @@ -677,6 +692,7 @@ app.post("/jobs/sweep", async () => { ok: true, requeued: swept.requeued.map((j) => j.job_id), failed: swept.failed.map((j) => j.job_id), + forgotten: swept.forgotten, }; }); @@ -693,11 +709,15 @@ app.get("/jobs/:id", async (req, reply) => { * A crash between the two would leave a chain permanently stalled on a * dependency that is already finished, which nothing on the collector would * ever come back and fix. */ -const closeJobTx = db.transaction((jobId: string, status: "done" | "failed"): DepSettlement => { +const closeJobTx = db.transaction((jobId: string, status: Closing["status"], reason: string | null): DepSettlement => { // Dropping the deadline takes the job out of the sweep's reach for good. + // The reason replaces last_error only when there is one: a skip always has + // one, a failure usually does, and a pass has nothing to say. db.prepare( - "UPDATE jobs SET status = ?, finished_at = datetime('now'), lease_deadline = NULL WHERE job_id = ?", - ).run(status, jobId); + `UPDATE jobs SET status = ?, finished_at = datetime('now'), lease_deadline = NULL, + last_error = COALESCE(?, last_error) + WHERE job_id = ?`, + ).run(status, reason, jobId); db.prepare("DELETE FROM device_locks WHERE job_id = ?").run(jobId); return settleWaiters(jobId); }); @@ -819,8 +839,16 @@ app.post("/results", async (req, reply) => { app.log.warn({ job_id: b.job_id }, "preempted result for a job that is no longer claimed"); } - const status = b.ok === false ? "failed" : "done"; - const settled = closeJobTx(b.job_id, status); + // Failed, done or skipped, judged over every row the job posted: a host + // job's final row is the executor's verdict, and its devices may all have + // said `skipped:` underneath a plain `ok: true`. + const rows = (db + .prepare("SELECT device_id, payload FROM results WHERE job_id = ? ORDER BY iter, rowid") + .all(b.job_id) as { device_id: string; payload: string }[]) + .map((r) => ({ device_id: r.device_id, payload: JSON.parse(r.payload) as Record })); + const closing = closingStatus(rows, b as Record); + const status = closing.status; + const settled = closeJobTx(b.job_id, status, closing.reason); // A finished job has nothing left to show. Outside the transaction, because // closing viewer sockets must not be able to roll back a device's result. endMirror(b.job_id); @@ -831,14 +859,18 @@ app.post("/results", async (req, reply) => { announce({ type: "job", job_id: jobId, status: "queued", reason: `dependency ${b.job_id} finished` }); for (const f of settled.failed) announce({ type: "job", job_id: f.job_id, status: "failed", reason: f.reason }); + for (const s of settled.skipped) + announce({ type: "job", job_id: s.job_id, status: "skipped", reason: s.reason }); - // A skip -- nothing attached, a host under pressure -- is ok for the queue - // and says nothing about the code, so it must not become a green check. - const skipped = b.ok === true && typeof b.error === "string" && b.error.startsWith("skipped:"); - reportStatus(b.job_id, status === "failed" ? "failure" : "success", skipped).catch((e) => + // A skip -- nothing attached, a host under pressure -- says nothing about + // the code, so it must not become a green check. + reportStatus(b.job_id, status === "failed" ? "failure" : "success", status === "skipped").catch((e) => app.log.error(e, "status report failed"), ); - return { ok: true, status, promoted: settled.promoted, failed: settled.failed.map((f) => f.job_id) }; + return { + ok: true, status, promoted: settled.promoted, failed: settled.failed.map((f) => f.job_id), + skipped: settled.skipped.map((s) => s.job_id), + }; } return { ok: true }; }); diff --git a/collector/src/skip.test.ts b/collector/src/skip.test.ts new file mode 100644 index 0000000..6ca5cb5 --- /dev/null +++ b/collector/src/skip.test.ts @@ -0,0 +1,69 @@ +/** + * How a job closes: done, failed or skipped. + * + * The shapes below are the ones the live brain held for two weeks as `done`: + * the tvloop nightly's single host row saying `skipped: no Roku developer + * password on this host`, and explore's `skipped: no device matches this job`. + * Plus the one the summary always knew was a skip and the queue did not -- a + * ui-test whose every device said `skipped:` under a plain `ok: true` host row. + */ +import { closingStatus, isSkipError, skipReasonText, type ResultRow } from "./skip.js"; + +type Check = (name: string, cond: boolean, detail?: string) => void; + +const row = (device_id: string, payload: Record): ResultRow => ({ device_id, payload }); + +export function runSkipChecks(check: Check) { + check("a skip error is recognised by its prefix", isSkipError("skipped: locked") && !isSkipError("failed: skipped:") && !isSkipError(null)); + + // The tvloop nightly: one host row, ok, skipped. + const tv = row("host:fleet-host", { final: true, ok: true, error: "skipped: no Roku developer password on this host" }); + const c1 = closingStatus([tv], tv.payload); + check("a final row that says skipped closes the job as skipped", + c1.status === "skipped" && c1.reason === "skipped: no Roku developer password on this host", JSON.stringify(c1)); + + // Every device skipped, the host row says nothing. + const devs = [ + row("phone-a", { ok: true, error: "skipped: com.example not installed" }), + row("phone-b", { ok: true, error: "skipped: com.example not installed" }), + ]; + const host = row("host:mac", { final: true, ok: true }); + const c2 = closingStatus([...devs, host], host.payload); + check("every device skipping under a plain ok host row is a skip", c2.status === "skipped", JSON.stringify(c2)); + check("and the shared reason is named once", c2.reason === "skipped: com.example not installed", String(c2.reason)); + + const mixedReasons = [row("a", { ok: true, error: "skipped: locked" }), row("b", { ok: true, error: "skipped: not installed" })]; + const c3 = closingStatus([...mixedReasons, host], host.payload); + check("different reasons are counted, not concatenated", c3.status === "skipped" && /and 1 other reason/.test(c3.reason ?? ""), String(c3.reason)); + + // One device ran. + const ran = [row("a", { ok: true, error: "skipped: locked" }), row("b", { ok: true, test: { passed: 3, failed: 0 } })]; + check("one device that ran makes it done", closingStatus([...ran, host], host.payload).status === "done"); + + // A plain pass. + const pass = row("phone", { final: true, ok: true, metrics: { decode_tok_s: 12 } }); + check("a plain pass is done", closingStatus([pass], pass.payload).status === "done"); + + // Failures: the final row wins, and says why from wherever it can. + const failDev = row("988a1b35", { ok: false, test: { passed: 0, failed: 1 }, error: 'smoke: Assertion is false: "Fleet Runner" is visible' }); + const failHost = row("host:fleet-host", { final: true, ok: false }); + const c4 = closingStatus([failDev, failHost], failHost.payload); + check("a failing final row is a failure", c4.status === "failed"); + check("whose reason comes from the failing device when the host row has none", + c4.reason === '988a1b35: smoke: Assertion is false: "Fleet Runner" is visible', String(c4.reason)); + const own = row("host:x", { final: true, ok: false, error: "workload 'x' not supported by this executor yet" }); + check("a failing row's own error is kept", closingStatus([own], own.payload).reason === "workload 'x' not supported by this executor yet"); + const bare = row("phone", { final: true, ok: false }); + check("a failure with nothing to say has a null reason, not an invented one", closingStatus([bare], bare.payload).reason === null); + // ok:false with a skipped: error is a failure: ok is the verdict. + const odd = row("phone", { final: true, ok: false, error: "skipped: but the runner said ok false" }); + check("ok:false is a failure even when the text says skipped", closingStatus([odd], odd.payload).status === "failed"); + + // A device's progress rows do not outvote its final row. + const progress = [row("p", { iter: 1, ok: true }), row("p", { iter: 0, final: true, ok: true, error: "skipped: charging" })]; + check("a device's final row beats its progress rows", closingStatus(progress, progress[1].payload).status === "skipped"); + + check("the prefix comes off for a sentence that already says skipped", + skipReasonText("skipped: no Roku developer password on this host") === "no Roku developer password on this host" && + skipReasonText(null) === "no reason given" && skipReasonText("dependency b skipped") === "dependency b skipped"); +} diff --git a/collector/src/skip.ts b/collector/src/skip.ts new file mode 100644 index 0000000..70820cb --- /dev/null +++ b/collector/src/skip.ts @@ -0,0 +1,82 @@ +/** + * What a skip is, decided in one place. + * + * A runner that could not run a job says so with a result row whose `ok` is + * true and whose `error` begins `skipped:` -- the app is not installed, the + * device was locked, nothing attached matched, the host was under pressure, no + * Roku password. `ok: true` because nothing went wrong with the code; the + * prefix because nothing was tested either. + * + * For a long time that row closed the job as `done`, and the job list, the + * status counts and the morning digest all counted it as a pass. Two nightlies + * "passed" in two seconds every night for two weeks while testing nothing. Only + * the v1 summary and `fleetr`'s exit 4 read the prefix. Now the queue reads it + * too, when the job closes, and the job's status says `skipped`. + * + * Pure: no database, so `db.ts` can run it in the startup backfill without an + * import cycle, and a test can feed it rows directly. + */ + +export const SKIP_PREFIX = "skipped:"; + +/** True for an error string that says nothing ran. */ +export function isSkipError(error: unknown): error is string { + return typeof error === "string" && error.startsWith(SKIP_PREFIX); +} + +export type ResultRow = { device_id: string; payload: Record }; + +export type Closing = + | { status: "failed"; reason: string | null } + | { status: "done"; reason: null } + | { status: "skipped"; reason: string }; + +const clip = (s: string, n = 500) => (s.length > n ? `${s.slice(0, n - 3)}...` : s); + +/** + * How a job closes, from every result row it posted (in `iter, rowid` order) + * and the final row that closes it. + * + * - The final row says `ok: false`: **failed**. Its error, or the first failing + * device's, is the reason. + * - The final row's own error begins `skipped:`: **skipped**. + * - Otherwise the per-device verdicts decide, read the way the v1 summary reads + * them: the newest row per device, with the host executor's `host:` + * row standing in only when no device reported. Every one of those skipped + * and none failed: **skipped** -- a ui-test whose every phone lacked the app + * posts a plain `ok: true` host row over device rows that all say why. + * - Anything else: **done**. + * + * The same rule as `summarize()` in `api/v1/routes.ts`, so a job's status and + * its summary verdict cannot disagree about whether anything ran. + */ +export function closingStatus(rows: ResultRow[], final: Record): Closing { + const byDevice = new Map>(); + for (const r of rows) { + const prev = byDevice.get(r.device_id); + if (!prev || !prev.final || r.payload.final) byDevice.set(r.device_id, r.payload); + } + let entries = [...byDevice]; + if (entries.some(([d]) => !d.startsWith("host:"))) entries = entries.filter(([d]) => !d.startsWith("host:")); + + if (final.ok === false) { + const own = typeof final.error === "string" && final.error.trim() ? final.error : null; + const failing = entries.find(([, p]) => p.ok === false && typeof p.error === "string" && p.error.trim()); + const reason = own ?? (failing ? `${failing[0]}: ${String(failing[1].error)}` : null); + return { status: "failed", reason: reason ? clip(reason) : null }; + } + if (isSkipError(final.error)) return { status: "skipped", reason: clip(final.error) }; + + if (entries.length > 0 && entries.every(([, p]) => p.ok !== false && isSkipError(p.error))) { + const reasons = [...new Set(entries.map(([, p]) => String(p.error)))]; + const reason = reasons.length === 1 ? reasons[0] : `${reasons[0]} (and ${reasons.length - 1} other reason${reasons.length === 2 ? "" : "s"})`; + return { status: "skipped", reason: clip(reason) }; + } + return { status: "done", reason: null }; +} + +/** The reason without its `skipped: ` prefix, for a sentence that already says "skipped". */ +export function skipReasonText(reason: string | null | undefined): string { + if (!reason) return "no reason given"; + return reason.startsWith(SKIP_PREFIX) ? reason.slice(SKIP_PREFIX.length).trim() || "no reason given" : reason; +} diff --git a/collector/src/web/digest.ts b/collector/src/web/digest.ts index 95c6934..83c8842 100644 --- a/collector/src/web/digest.ts +++ b/collector/src/web/digest.ts @@ -115,7 +115,9 @@ async function runShelfBatch( await postBeacon(digestJob.job_id, "web:digest", {}).catch(() => {}); const j = await apiJson<{ status: string; last_error?: string | null }>(`/jobs/${encodeURIComponent(jobId)}`); if (j.status === "done") break; - if (j.status === "failed" || j.status === "cancelled") { + // A skipped batch ran nothing and has no outputs to read; say that, rather + // than "finished without an outputs artifact" one step later. + if (j.status === "failed" || j.status === "cancelled" || j.status === "skipped") { throw new Error(`${label} batch ${jobId} ${j.status}: ${j.last_error ?? "no detail"}`); } } diff --git a/docs/api.md b/docs/api.md index eacb50a..6d66b68 100644 --- a/docs/api.md +++ b/docs/api.md @@ -20,7 +20,7 @@ disagree: the test suite fails if they do. | `GET /api/v1/devices` · `/devices/{id}` | read | The shelf, with status (`online`, `stale`, `offline`) and current job | | `GET /api/v1/workloads` · `/workloads/{name}` | read | What the fleet can run: executor, params schema, a valid example spec, how many live agents can take it | | `GET /api/v1/templates` | read | Saved job templates | -| `GET /api/v1/jobs` | read | Jobs; filter by `status`, `workload`, `executor` (`device` or `host`), `device`, `pool`, `source`, `requested_by`, `mine=true`, `q` (substring of the job id), `from`/`to` | +| `GET /api/v1/jobs` | read | Jobs; filter by `status` (`waiting`, `queued`, `claimed`, `done`, `failed`, `cancelled`, `skipped`), `workload`, `executor` (`device` or `host`), `device`, `pool`, `source`, `requested_by`, `mine=true`, `q` (substring of the job id), `from`/`to` | | `POST /api/v1/jobs` | run | Enqueue. 201 with the job id, its status, and links to its summary and wait URLs. 422 `no_capable_agent` when no live agent declares the workload (or backend); 400 `no_matching_devices` for a fan-out that matches nothing, and 400 `no_build` when `latest` names no published build | | `POST /api/v1/jobs/validate` | run | Dry run: schema errors by path, the devices it would land on, warnings. Always 200 | | `GET /api/v1/jobs/{id}` | read | One job, with its spec | @@ -56,6 +56,13 @@ needs `admin`; cancel and retry are for the job's owner or `admin`); see ran, never folded into passed), `cancelled`, and while unfinished `queued`, `waiting` or `running`. +**Job statuses** are `waiting`, `queued`, `claimed`, `done`, `failed`, +`cancelled` and `skipped`. `skipped` means the job was claimed and nothing ran +(its runner said `skipped:`, or a dependency skipped); `last_error` says why. +A skipped job's summary verdict is `skipped`, as it always was; before 0.9 its +status said `done`. A fan-out family's `status` is `skipped` when no child ran. +See [Skipped is not done](concepts.md#skipped-is-not-done). + **Who asked.** Every job records `requested_by` and `source` (`cli`, `mcp`, `ci`, `dash`, `schedule`, `api`). With auth off both are what the caller said, labelled unverified; with tokens on, `requested_by` is the token's name, or the person a @@ -104,9 +111,9 @@ anything else `admin`. With auth off, all of it is open. | Method & path | Purpose | |---|---| -| `POST /jobs` | Enqueue. 409 on duplicate `job_id`; the reply's `status` says whether it queued or is `waiting` on `depends_on` | +| `POST /jobs` | Enqueue. 409 on duplicate `job_id`; the reply's `status` says whether it queued, is `waiting` on `depends_on`, or was closed on arrival as `failed` or `skipped` because a dependency already was | | `GET /jobs/:id` | Status, including `attempts`, `lease_deadline` and `last_error` | -| `POST /jobs/sweep` | Force a lease sweep now; returns what was requeued and failed | +| `POST /jobs/sweep` | Force a lease sweep now; returns what was requeued and failed, and the simulator rows it forgot (offline past `FLEET_SIMULATOR_TTL_DAYS`, default 7) | | `POST /jobs/:id/release` | The claimant hands a job back before starting it. Returns it to `queued` and **refunds the attempt**; 409 once any result exists. See [the protocol](protocol.md#handing-a-job-straight-back) | | `POST /locks/acquire` · `POST /locks/release` | Host-executor device locks for `targets.exclusive`; device claims lock implicitly | | `POST /events/:topic` | Publish a pipeline event; returns its id | @@ -130,7 +137,7 @@ Every endpoint is `GET` and side-effect free. | `GET /api/devices` | Registry with derived `online`/`stale`/`offline`, current job, lock, flattened beacon | | `GET /api/devices/:id` | Descriptor, job history, latest benchmarks, counts | | `GET /api/devices/:id/beacons?hours=24` | Beacon history for the battery and thermal charts | -| `GET /api/jobs` | Filters, paging, sorting; returns status facets | +| `GET /api/jobs` | Filters (`status` takes `skipped` too), paging, sorting; returns status facets, `skipped` among them | | `GET /api/jobs/:id` | Spec, results, beacons, artifacts, locks, fan-out family, status report | | `GET /api/results` | Filters: `job`, `device`, `workload`, `final`, `ok`, `from`, `to` | | `GET /api/results/bench` | Latest passing run per device per configuration, with history for trends | @@ -141,6 +148,7 @@ Every endpoint is `GET` and side-effect free. | `GET /api/evals` · `GET /api/evals/:input_sha` | Eval sets pivoted by model and device | | `GET /api/schedules` | Schedules with computed `next_run` and missed-fire detection | | `GET /api/artifacts` | Store listing with on-disk state and reference counts | +| `GET /api/artifacts/:sha256` | A 308 to `/api/v1/artifacts/:sha256/content`, the bytes; 404 when nothing by that hash was ever uploaded. Before 0.9 this answered "no such endpoint", which read as a missing artifact | | `GET /api/visual/baselines` · `/suites` · `/matrix?suite=` | Accepted baselines and the review grid | | `GET /api/locks` | Held locks and how long they have been held | | `GET /api/executors` | Host-executor liveness, derived from their long-poll traffic | @@ -239,7 +247,8 @@ already handle. Work in flight finishes; nothing new starts. A cancelled job is deliberately **not** `failed`. The overview's failure counts and every alert built on them would otherwise count deliberate stops as -breakage. +breakage. A skipped one is not `done` either, for the mirror-image reason: the +overview counts it on its own (`queue.skipped_24h`). ### Pool edits go to a different column diff --git a/docs/brand.md b/docs/brand.md index 4abce47..45b427a 100644 --- a/docs/brand.md +++ b/docs/brand.md @@ -70,7 +70,7 @@ dependency, so nothing new is installed. | `img/banner.png` | 2560×1280 | `figures/banner.html` | | `img/social-preview.png` | 2560×1280 (2× of GitHub's 1280×640) | `figures/social-preview.html` | | `img/architecture.png` | 2560×1280 | `figures/architecture.html` | -| `img/lifecycle.png` | 2560×1360 | `figures/lifecycle.html` | +| `img/lifecycle.png` | 2560×1464 | `figures/lifecycle.html` | | `img/protocol.png` | 2560×1120 | `figures/protocol.html` | | `img/workloads.png` | 2560×1360 | `figures/workloads.html` | | `img/platforms.png` | 2560×1400 | `figures/platforms.html` | diff --git a/docs/concepts.md b/docs/concepts.md index d2718ea..57164d6 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -4,7 +4,7 @@ How the queue thinks. Everything here is the collector's behaviour rather than any one runner's, and it is what you need before writing a job spec that does something more interesting than the one in [Get started](getting-started.md). -![The life of a job: waiting until its dependencies close, queued, claimed and running under a lease that beacons renew, then done — with a lapsed lease swept back to the queue, a preemption handing its attempt back, and ok: false or a last lapsed attempt ending in failed](img/lifecycle.png) +![The life of a job: waiting until its dependencies close, queued, claimed and running under a lease that beacons renew, then done — with a lapsed lease swept back to the queue, a preemption handing its attempt back, ok: false or a last lapsed attempt ending in failed, and a job that ran nothing closing as skipped](img/lifecycle.png) ## Two kinds of job @@ -220,6 +220,46 @@ Host jobs fan out too, and the executor's own target selection honours `targets.device_id`, so an `install` plus `ui-test` pair can cover the OS matrix with two job specs. +## Skipped is not done + +A runner that claims a job and cannot run it says so with a final row whose +`ok` is `true` and whose `error` begins `skipped:` — the app is not installed, +the device was locked by another job, nothing attached matched, the host was +under memory pressure, no Roku password. `ok: true` because nothing went wrong +with the code; the prefix because nothing was tested either. + +The job closes as **`skipped`**, a status of its own beside `done`, `failed` +and `cancelled`. Until 0.9 it closed as `done`, and the job list, the status +counts and the digest counted it as a pass: two nightlies passed in two seconds +a night for two weeks while testing nothing. + +How the queue decides, when the final row arrives: + +| The rows say | The job is | +|---|---| +| the final row has `ok: false` | `failed`, with `last_error` from that row or, if it has none, from the first failing device (`988a1b35…: smoke: Assertion is false: …`) | +| the final row's own `error` begins `skipped:` | `skipped` | +| every device row skipped and none failed, under a plain `ok: true` host row (a ui-test where no phone had the app) | `skipped` | +| anything else, including one device that ran while others skipped | `done` | + +A skipped job's `last_error` is the reason, so the job list, the +[`schedule-skipping` alert](deploy/alerts.md#a-schedule-that-keeps-skipping) +and the [morning digest](explore/findings.md#the-morning-digest) all read it +from one place. The result rows are stored exactly as the runner posted them. + +- **A fan-out family** is `skipped` when no child ran: every child skipped, or + skipped and cancelled. One child that ran makes the family `done`, and one + that failed makes it `failed`, as before. +- **The v1 summary's verdict** was already `skipped` for these jobs and still + is, so `fleetr`'s exit 4 and the MCP tools' verdict are unchanged. What + changed is that the job's status now agrees with it. +- **A skip posts no commit status**, as before. A green check where no device + ran would be a lie. +- **Jobs that closed as `done` before 0.9** are re-labelled once, when a 0.9 + collector first opens the database: every `done` job with a `skipped:` result + is judged by the same rule as above. It runs inside the migration that adds + the status, so it costs one pass over the results table, once. + ## Job chains Build, then install what it built, then run the UI suite against it. That shape @@ -251,6 +291,14 @@ not work the fleet is failing to pick up. `last_error` naming it, and that cascades the whole way down. A broken build must not leave an install and a ui-test sitting in `waiting` until somebody notices next week. +- When a dependency **[skipped](#skipped-is-not-done)**, the waiter skips too, + with `last_error` `dependency skipped`, and that cascades the same way. + A skipped build produced no artifact and no metric, so there is nothing for + the install to install; failing it would blame the code for a phone that was + on a charger somewhere else, and running it would test whatever was already + installed. If one dependency failed and another skipped, the failure wins. A + job enqueued behind a dependency that has already skipped is skipped on + arrival, and a dry run warns first. - A dependency that does not exist, and a cycle, are refused at enqueue with a 400. A chain naming a job nobody ever posted would otherwise sit in `waiting` forever, looking exactly like a chain whose build is merely slow. diff --git a/docs/dashboard.md b/docs/dashboard.md index 65d56af..4004b1c 100644 --- a/docs/dashboard.md +++ b/docs/dashboard.md @@ -45,8 +45,13 @@ artifacts. drawings, from phones and tablets through Roku boxes and sticks, an Apple TV, laptops, watches and a headset to a board and a browser. The drawing is chosen from the model string first, because agents that predate the `platform` field -are reported as `ios` or `android`, so a Roku arrives as `android` and an -Apple TV or Apple Watch as `ios`. +are reported as `ios` or `android`: an Apple TV or Apple Watch arrives as +`ios`, and a Roku registered by an executor older than 0.9 as `android`. + +**A skipped job has a grey chip**, neither green nor red: it was claimed and +nothing ran. The Jobs status filter has a `skipped` option with its count, and +the Overview's queue panel shows **skipped 24h** beside done and failed rather +than inside done. See [Skipped is not done](concepts.md#skipped-is-not-done). **Filters live in the URL**, so a filtered view is a link you can send. Jobs can be composed, enqueued, cancelled, retried and reprioritised; devices can be diff --git a/docs/deploy/alerts.md b/docs/deploy/alerts.md index 1008e43..d870037 100644 --- a/docs/deploy/alerts.md +++ b/docs/deploy/alerts.md @@ -22,6 +22,7 @@ notifications — and nothing is notified twice, ever. | `cold-start-regressed` | Launch time rose past the same threshold against its own median | | `batch-regressed` | Top-1 accuracy fell past the same threshold | | `schedule-missed` | An enabled schedule that has run before missed its firing by `FLEET_ALERT_SCHEDULE_LATE_S` (default 5 min) | +| `schedule-skipping` | An enabled schedule's last `FLEET_ALERT_SKIP_STREAK` (default 3) runs all [skipped](../concepts.md#skipped-is-not-done) | | `db-size` / `log-size` | Past `FLEET_ALERT_DB_BYTES` (default 2 GiB) / `FLEET_ALERT_LOG_BYTES` (200 MiB) | Every threshold is read once, at collector start; a value that is not a @@ -36,6 +37,33 @@ Battery and thermal are only judged on devices **still checking in** — a readi from a silent device describes whenever it went silent, not now. And a device reporting `-1` has no battery telemetry rather than a flat battery. +## A schedule that keeps skipping + +A skipped job is no longer counted as a pass, but a grey chip every morning is +still easy to stop seeing. `nightly-tvloop-hardware` skipped every night for two +weeks on a missing Roku password and nothing said so. `schedule-skipping` is +one alert per schedule, worded from the reason the runs keep giving: + +```text +nightly-tvloop-hardware has skipped 3 runs in a row: no Roku developer password on this host +``` + +What counts as a run, and as a skip: + +- **A run is what the scheduler enqueued**: the job `-`, + or for a fan-out every child of it. The id is what identifies it, so jobs + from before provenance was recorded still count. +- **A run skipped when nothing in it ran**: every job in it is `skipped` (or + skipped and cancelled). One device that ran makes the run count as run. +- **A run still in flight is left out**: it neither breaks the streak nor + extends it. +- If the runs gave different reasons, the alert names the latest and says how + many different reasons there were. + +It resolves on the first run that is not a skip, pass or fail. Disabled +schedules are not watched. The streak length is `FLEET_ALERT_SKIP_STREAK`, +read at start like the others. + ## The regression rules compare against history, not a threshold The comparison key matters more than the threshold. Two runs are only comparable diff --git a/docs/deploy/index.md b/docs/deploy/index.md index 9df08f1..e1fae34 100644 --- a/docs/deploy/index.md +++ b/docs/deploy/index.md @@ -136,6 +136,7 @@ port, bind list, tailnet allowlist, data and artifact directories and | `FLEET_PEER_TOKENS` | none | JSON object of peer URL to a `read` token that peer issued, for a peer with tokens on. See [more than one brain](#more-than-one-brain) | | `FLEET_POWER_CONFIG` | `power.json` | The pool-to-smart-plug map, see [Energy](#energy) | | `FLEET_SWEEP_MS` | 15000 | How often expired leases are swept | +| `FLEET_SIMULATOR_TTL_DAYS` | 7 | The same sweep forgets a simulator row (descriptor `kind` or `soc` is `simulator`) offline longer than this. Simulators are recreated under new UUIDs and never come back. Real devices and Android emulators are never forgotten; results and beacons stay either way | | `FLEET_SCHEDULER_TICK_MS` | 20000 | How often schedules are evaluated | | `FLEET_DASH_DIST` | `dash/dist` | The built dashboard. Missing, the collector serves a placeholder rather than failing | | `FLEET_LOG` | `info` | Log level | diff --git a/docs/explore/findings.md b/docs/explore/findings.md index ac47868..6e9b3a5 100644 --- a/docs/explore/findings.md +++ b/docs/explore/findings.md @@ -65,6 +65,19 @@ through the same sender as alerts. Nothing is sent on a day with nothing new, and a restart does not send twice. Findings judged not a bug, agent's mistake or duplicate are left out. `FLEET_DASH_URL` sets the address used in its links. +Below the findings, one line says how the jobs that finished in the same window +ended — done, failed, and skipped on a count of its own — and names up to three +schedules that skipped, with the reason each gave: + +```text +Jobs, last 24h: 14 done, 1 failed, 4 skipped (nothing ran). +- skipped: explore-tv x3 (no device matches this job on ultra) +- skipped: nightly-tvloop-hardware (no Roku developer password on this host) +``` + +The send rule is unchanged: a day with no findings sends nothing, whatever the +jobs did. A schedule that keeps skipping has [its own alert](../deploy/alerts.md#a-schedule-that-keeps-skipping). + ## GitHub issues (off by default) Fleet Runner stays disconnected from the app repositories until that is decided diff --git a/docs/figures/lifecycle.html b/docs/figures/lifecycle.html index 5558bcb..307cf2c 100644 --- a/docs/figures/lifecycle.html +++ b/docs/figures/lifecycle.html @@ -10,9 +10,9 @@ Rendered by `npm run assets` in collector/ to docs/img/lifecycle.png. --> -
- +
+ beacon · renews the lease · answers preempt @@ -69,6 +69,8 @@ A dependency that fails, fails everything waiting on it, the whole way down the chain. Nothing is ever killed. Cancelling sets the row to cancelled, and the next beacon answers lease_renewed: false — work in flight finishes, nothing new starts. + A runner that ran nothing posts ok: true with an error beginning skipped:, and the job closes + skipped, not done. A job waiting on a skipped one skips too.
diff --git a/docs/img/lifecycle.png b/docs/img/lifecycle.png index 9d72156..e520ece 100644 Binary files a/docs/img/lifecycle.png and b/docs/img/lifecycle.png differ diff --git a/docs/lab.md b/docs/lab.md index bd0d54c..b695974 100644 --- a/docs/lab.md +++ b/docs/lab.md @@ -118,10 +118,10 @@ back. | | | |---|---| -| The device | Roku Express 4K+ at `192.168.50.218`, Roku OS 15.3.4, developer mode on. Registered as `roku-` by fleet-host | +| The device | Roku Express 4K+ at `192.168.50.218`, Roku OS 15.3.4, developer mode on. Registered as `roku-` by fleet-host. An executor older than 0.9 registers it as `platform: android` and leaves an offline `roku-ip-192-168-50-218` beside it; from 0.9 it says `roku`, and the serial-named row retires the IP-named one | | The checkout | `~/tvloop` on fleet-host, **synced from the dev MacBook**, not cloned: the repo is private and fleet-host has no GitHub credential. Re-sync with `rsync -a --delete --exclude node_modules --exclude 'packages/*/dist' --exclude tvloop.toml / fleet-host:tvloop/`, then `corepack pnpm install --frozen-lockfile && corepack pnpm build` there | | `~/tvloop/tvloop.toml` | Not in the repo. Points `[project] source` at `examples/sample-channel`, the channel the flows expect. Keep it out of the rsync | -| The password | Keychain item `fleet-roku-dev` / account `rokudev` on fleet-host, the same one the dev installer reads. Missing means every run is `skipped:` and says how to add it | +| The password | Keychain item `fleet-roku-dev` / account `rokudev` on fleet-host, the same one the dev installer reads. Missing means every run is `skipped:` and says how to add it. On a 0.7 or 0.8 brain those runs show as `done`; from 0.9 they are `skipped`, and three in a row raise a `schedule-skipping` alert | | The steps | `doctor`, `install` (sideloads the sample channel, **replacing whatever is in the dev slot**), `hardware`, `replay`. A failed doctor or install stops the run | ## Real phones from CI @@ -148,7 +148,8 @@ Three things have to be true for a status to appear: `fleet mcp` serves the lab as MCP tools; register it once with `claude mcp add --scope user fleet -- ~/.fleet/bin/fleet mcp`. From the dev MacBook it reaches the brain through the tunnel, the same as the agent does. -A result whose error starts with `skipped:` means nothing ran. +A result whose error starts with `skipped:` means nothing ran. The 0.7.2 brain +closes those jobs as `done`; from 0.9 their status is `skipped`. From 0.8.0 there is also `fleetr`, whose exit code is the verdict, and a skill (`fleet skill install`) that tells an agent which commands to run in what @@ -194,7 +195,7 @@ Five enabled. All times local to fleet-host. | `nightly-self-check` | 02:15 | fanout, pool `machines` | Disk, clock drift, a tool that vanished, or the agent is not supervised | | `nightly-fleet-ui-smoke` | 02:30 | `fleet-host`, an Android device | **Red since 2026-08-21.** From 09-16 to early October it was `no targets attached`. The Galaxy S8+ is plugged in again and the suite now runs on it, and fails, every night; why is not diagnosed yet | | `nightly-aliquant-shots` | 04:15 | `fleet-host`, `chromium` and `mobile-chrome` | Green. The baseline was re-accepted on 2026-09-22 from fleet-host's own captures, after Aliquant's sign-in page gained a "Forgot password?" link; the old one had diverged 2.7% on desktop and 6.1% on mobile since 09-10. The `webkit` and `mobile-safari` baselines are still the August ones, because nothing runs those projects now | -| `nightly-tvloop-hardware` | 05:00 | `fleet-host`, the Roku, exclusive | tvloop's doctor, install, contract tests and flow replay. `skipped:` until the Roku password is in fleet-host's Keychain. Red from doctor means the Roku is asleep, off the network, or out of developer mode | +| `nightly-tvloop-hardware` | 05:00 | `fleet-host`, the Roku, exclusive | tvloop's doctor, install, contract tests and flow replay. `skipped:` until the Roku password is in fleet-host's Keychain (status `done` on the 0.7.2 brain, `skipped` from 0.9). Red from doctor means the Roku is asleep, off the network, or out of developer mode | **`nightly-aliquant-web` and `nightly-aliquant-audit` are retired** (disabled 2026-09-29). Aliquant's own CI already runs the same Playwright suite and diff --git a/docs/protocol.md b/docs/protocol.md index f96a45c..1b1046d 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -49,13 +49,13 @@ means "nothing", and they are different on purpose. ### `platform` and `kind` **`platform`** is which OS family you run. An open string: `android`, `ios`, -`tvos`, `watchos`, `visionos`, `macos`, `linux`, `windows`, `web` are the ones -in use. Send it. An agent that omits it gets the old rule — iOS if `os` looks +`tvos`, `watchos`, `visionos`, `macos`, `linux`, `windows`, `roku`, `web` are +the ones in use. Send it. An agent that omits it gets the old rule — iOS if `os` looks like iOS, Android otherwise — which is kept so that a collector upgrade does not relabel agents written before the field existed. That rule is also why the field had to exist: it does not report a platform, it reports a guess, and every desktop agent registered as an Android phone until it could say -otherwise. +otherwise, and so did the host executor's Rokus until 0.9. **`kind`** is the form factor: `phone`, `tablet`, `laptop`, `desktop`, `tv`, `watch`, `headset`, `sbc`, `browser`, `ci`, `container`. Null when you do not @@ -89,6 +89,12 @@ results you posted stay attributable and `GET /api/devices/:id` still resolves you. What changes is that the queue stops offering you work and you leave the shelf. +One exception, decided by the collector rather than the agent: a row whose +descriptor says `kind: "simulator"` or `soc: "simulator"` is **deleted** once +it has been offline for `FLEET_SIMULATOR_TTL_DAYS` (default 7), because a +simulator is recreated under a new UUID and never comes back. Its results and +beacons are kept, as with the dashboard's Forget. + ## 2. Claim work ``` @@ -243,6 +249,14 @@ The last row for a device carries `final`: the verdict. `error` carries the reason on a failure, and it is what the dashboard shows against the job — so make it a sentence a person can act on. +**If you could not run the job at all**, say `"ok": true` with an `error` that +begins `skipped:` and names why (`skipped: com.example not installed`). The job +closes as `skipped` rather than `done`, nothing waiting on it runs, and no +commit status is posted. A host row of plain `ok: true` over device rows that +all said `skipped:` counts the same. See +[Skipped is not done](concepts.md#skipped-is-not-done). Nothing about the row's +shape changed; a runner that already said `skipped:` needs no update. + ### Metric names are not free-form `metrics` keys come from diff --git a/docs/workloads/host.md b/docs/workloads/host.md index b61db8b..499df54 100644 --- a/docs/workloads/host.md +++ b/docs/workloads/host.md @@ -270,6 +270,15 @@ directory (`FLEET_FLOWS_DIR` to change). `exclusive` takes a device lock for the run, because two suites tapping the same phone is not a test. +**A failing Maestro run says which step broke.** The device row's `error` is +the first failing flow and its assertion, read from the JUnit report +(`smoke: Assertion is false: "Fleet Runner" is visible`), `test.failing_flow` +names the flow, and the report itself is the row's artifact +(`GET /api/v1/artifacts//content`). When Maestro produced no report at +all, the error is its exit code and the last line it printed to stderr, never +its command line, which carries the test account's password. Until 0.9 the +reason was in the uploaded report and nowhere else. + **iOS UI tests need a Mac with full Xcode.** `simctl` and `devicectl` ship with Xcode, not the Command Line Tools, so they run on a different executor from the Android work. [The iOS executor](../deploy/ios-executor.md) covers standing one