diff --git a/api/dev-watchdog.ts b/api/dev-watchdog.ts index 2c5b6ca..afdae68 100644 --- a/api/dev-watchdog.ts +++ b/api/dev-watchdog.ts @@ -78,28 +78,34 @@ let timer: ReturnType | null = null; async function check(): Promise { state.lastCheckedAt = new Date().toISOString(); - if (state.surrendered) return; - - // Inside the cooldown: it was just restarted and is probably still starting. - if (state.lastRestartAt && Date.now() - Date.parse(state.lastRestartAt) < COOLDOWN_MS) return; - const servers = await readServers().catch(() => null); const dev = servers?.find((one: ServerStatus) => one.name === 'development'); if (!dev || dev.port === null) return; /* - Only the lie. `online` and answering is fine; anything else is somebody's decision. + ── Answering again clears everything, INCLUDING having given up ── - `reachable` is null when there is no port to try, which is not a failure — it is a server that has - never been configured, and starting that would be inventing an intention nobody had. + This branch used to sit below the `surrendered` check, which made surrender permanent: once it + stopped trying it also stopped noticing, so a server somebody fixed by hand left a watchdog that + would never watch again until the whole platform restarted. Reading the state first and clearing + it here is what makes giving up a pause rather than a death. + + `online` and answering is fine; anything else is somebody's decision — see the note at the top. + `reachable` is null when there is no port to try, which is a server nobody has configured. */ if (dev.status !== 'online' || dev.reachable !== false) { - if (state.strikes) console.log('[fofos-admin] dev server answering again'); + if (state.strikes || state.surrendered) console.log('[fofos-admin] dev server answering again'); state.strikes = 0; state.consecutiveRestarts = 0; + state.surrendered = false; return; } + if (state.surrendered) return; + + // Inside the cooldown: it was just restarted and is probably still starting. + if (state.lastRestartAt && Date.now() - Date.parse(state.lastRestartAt) < COOLDOWN_MS) return; + state.strikes += 1; console.log(`[fofos-admin] dev server is online but not answering (${state.strikes}/${STRIKES})`); if (state.strikes < STRIKES) return; @@ -122,7 +128,24 @@ async function check(): Promise { output: error instanceof Error ? error.message : String(error), })); - if (!result.ok) console.warn('[fofos-admin] the restart failed:', result.output.slice(0, 400)); + /* + A failed start is the dangerous outcome, not the loud one. + + `startServer` DELETES the pm2 entry before starting it. If the start then fails there is no + process at all — and `status: null` reads, one tick later, as "a server somebody stopped", which + this deliberately leaves alone. So a single bad restart would turn the watchdog off and take the + server with it, silently. + + Counted as a failure so `GIVE_UP_AFTER` still bounds the retries, and said loudly, because a + development server that no longer exists is not something to find out about tomorrow. + */ + if (!result.ok) { + console.error( + '[fofos-admin] the dev server FAILED to start and no process is left — `pm2 start` was preceded ' + + 'by a delete. Start it from the panel, or `pm2 logs`:', + result.output.slice(0, 400), + ); + } if (state.consecutiveRestarts >= GIVE_UP_AFTER) { state.surrendered = true; diff --git a/api/lighthouse.ts b/api/lighthouse.ts index 0ad6c33..3ed7cfe 100644 --- a/api/lighthouse.ts +++ b/api/lighthouse.ts @@ -33,7 +33,8 @@ const CATEGORIES = ['performance', 'accessibility', 'best-practices', 'seo'] as export type FormFactor = 'mobile' | 'desktop'; -export type AuditMetric = { id: string; label: string; value: number; display: string }; +/** `value` is null when the metric did not run — which is not the same as it being zero. */ +export type AuditMetric = { id: string; label: string; value: number | null; display: string }; export type AuditFinding = { id: string; @@ -200,12 +201,48 @@ export async function audit( logLevel: 'error', onlyCategories: [...CATEGORIES], formFactor, - // Lighthouse will not apply mobile emulation to a desktop form factor or the reverse, and - // getting this wrong is the classic way to produce a number that means nothing. + /* + ── `formFactor` alone does NOT make a desktop audit ── + + It changes which of Lighthouse's own thresholds a metric is scored against and nothing else. + The THROTTLING and the USER AGENT keep their defaults, which are mobile: a 4x CPU slowdown and + slow 4G, announced as a moto-g. So a `--desktop` run was a desktop-sized window crawling on an + emulated phone, and it read 86 where the same page read 100 in a real desktop Lighthouse. + + That number was believed for a while and blamed on network latency. It was not latency; it was + this. All four have to agree or the run is about nothing. + + The numbers are Lighthouse's own — `desktopDense4G` from its constants: 40 ms RTT, 10 Mbps, no + CPU slowdown. Copied rather than imported because the constants module is not part of its + public surface and pinning to it would break on an upgrade that moved the file. + */ screenEmulation: formFactor === 'desktop' ? { mobile: false, width: 1350, height: 940, deviceScaleFactor: 1, disabled: false } : { mobile: true, width: 412, height: 823, deviceScaleFactor: 1.75, disabled: false }, + throttling: + formFactor === 'desktop' + ? { + rttMs: 40, + throughputKbps: 10 * 1024, + cpuSlowdownMultiplier: 1, + requestLatencyMs: 0, + downloadThroughputKbps: 0, + uploadThroughputKbps: 0, + } + : { + rttMs: 150, + throughputKbps: 1.6 * 1024, + cpuSlowdownMultiplier: 4, + requestLatencyMs: 150 * 3.75, + downloadThroughputKbps: 1.6 * 1024 * 0.9, + uploadThroughputKbps: 750 * 0.9, + }, + emulatedUserAgent: + formFactor === 'desktop' + ? 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) ' + + 'Chrome/131.0.0.0 Safari/537.36' + : undefined, throttlingMethod: 'simulate', // Never. It ships to Sentry, and a build tool that phones home about this site is not something // anybody asked for. Off by default in lighthouse too; said here so it stays off. @@ -213,6 +250,17 @@ export async function audit( } as any); if (!run?.lhr) throw new Error('lighthouse returned nothing'); + + /* + A run that could not load the page still returns an `lhr`. + + Every category comes back null, every audit is filtered out, and the report reads "nothing + failing" — the most reassuring possible output for a site that is down. `runtimeError` is the one + field that says so. + */ + if (run.lhr.runtimeError?.code && run.lhr.runtimeError.code !== 'NO_ERROR') { + throw new Error(`${run.lhr.runtimeError.code}: ${run.lhr.runtimeError.message ?? 'the page did not load'}`); + } const lhr = run.lhr; const scores: Record = {}; @@ -226,7 +274,9 @@ export async function audit( return { id, label: found?.title ?? id, - value: typeof found?.numericValue === 'number' ? found.numericValue : 0, + // NULL, not 0. A metric that did not run is not a metric that scored perfectly, and a `--json` + // consumer reading 0 for Total Blocking Time would believe the best possible news. + value: typeof found?.numericValue === 'number' ? found.numericValue : null, display: found?.displayValue ?? '—', }; }); @@ -240,6 +290,12 @@ export async function audit( */ const findings: AuditFinding[] = Object.values(lhr.audits) .filter((one) => { + /* + `error` is a failure and is listed. It nulls the whole category score, so filtering it out + alongside the diagnostics meant a category could read 0 with an empty findings list — the + one case where "nothing failing" is exactly wrong. + */ + if (one.scoreDisplayMode === 'error') return true; if (one.scoreDisplayMode === 'informative' || one.scoreDisplayMode === 'manual') return false; if (one.scoreDisplayMode === 'notApplicable') return false; return typeof one.score === 'number' && one.score < 0.9; @@ -261,8 +317,19 @@ export async function audit( if (typeof item.wastedMs === 'number') parts.push(`${Math.round(item.wastedMs)} ms`); const node = item.node as { nodeLabel?: string } | undefined; if (node?.nodeLabel) parts.push(node.nodeLabel); + /* + `source` is a string on some audits and a `source-location` object on others — + `deprecations` and the insight audits are the ones that bite. `String()` on the object + prints `[object Object]`, which is worse than printing nothing. + */ + const source = item.source as { url?: unknown } | string | undefined; + const from = + typeof source === 'string' ? source + : source && typeof source.url === 'string' ? source.url + : ''; + return { - url: typeof item.url === 'string' ? item.url : String(item.source ?? ''), + url: typeof item.url === 'string' ? item.url : from, detail: parts.join(', '), }; }); diff --git a/bin/lighthouse.ts b/bin/lighthouse.ts index 27f67ab..c56883c 100644 --- a/bin/lighthouse.ts +++ b/bin/lighthouse.ts @@ -32,9 +32,20 @@ const factors: FormFactor[] = args.includes('--mobile') ? ['desktop'] : ['mobile', 'desktop']; -/** Lighthouse's own bands, so a number reads as good or not without looking them up. */ +/** + * Lighthouse's own bands, so a number reads as good or not without looking them up. + * + * `✓` is 90 and up, which is what Lighthouse calls green — NOT the same as the exit gate below, which + * is 100. The two disagreeing is deliberate and worth saying out loud: green is Lighthouse's word for + * good, and this tool's exit code answers a different question, which is whether there is anything + * left to do. + */ const band = (score: number | null) => (score === null ? ' ? ' : score >= 90 ? ' ✓ ' : score >= 50 ? ' ~ ' : ' ✗ '); +/** Every category on every form factor at 100 — see the note on `band`. */ +const perfect = (results: { scores: Record }[]) => + results.every((one) => Object.values(one.scores).every((score) => score === 100)); + const results = []; for (const formFactor of factors) { // To stderr, so `--json` on stdout stays a clean pipe while the wait is still narrated. @@ -43,7 +54,14 @@ for (const formFactor of factors) { if (json) { console.log(JSON.stringify(results, null, 2)); - process.exit(0); + /* + The same gate as below, not `exit(0)`. + + `--json` is the mode a script uses, and a script is exactly the caller that reads an exit code. + Exiting 0 here meant the documented "non-zero when anything is short of 100" was true for a person + watching the table and false for the only consumer that could act on it. + */ + process.exit(perfect(results) ? 0 : 1); } for (const result of results) { @@ -74,5 +92,4 @@ for (const result of results) { } // A non-zero exit when anything is short of 100, so this can gate a build without being parsed. -const perfect = results.every((one) => Object.values(one.scores).every((score) => score === 100)); -process.exit(perfect ? 0 : 1); +process.exit(perfect(results) ? 0 : 1); diff --git a/skills/fofos-lighthouse/SKILL.md b/skills/fofos-lighthouse/SKILL.md index 9786066..475dd3f 100644 --- a/skills/fofos-lighthouse/SKILL.md +++ b/skills/fofos-lighthouse/SKILL.md @@ -6,8 +6,8 @@ version: 1 --- `bin/lighthouse.ts` runs Lighthouse against a URL in a real Chrome and prints what failed, with the -resources each finding names. It is the same engine the panel uses (`api/lighthouse.ts`), so a number -you get here is the number the panel would show. +resources each finding names. The engine is `api/lighthouse.ts`, which is where the panel will read +from when it grows a screen for this — today the CLI is the only caller. ## Running it @@ -34,8 +34,14 @@ static files under `dist/`, served by nginx behind a proxy, with different cachi compression and a trailing-slash redirect. **They are not the same page** and their numbers do not agree. -So: `bun run build:site` first, then audit `https://web.fofosfluffybastards.com/...`. A score from the -dev server is not wrong, it is about something nobody visits. +So: build first, then audit the public URL. The build has to run in the website checkout and with the +site's own environment — see `api/static-site.ts`, which documents both: + +```bash +cd ~/officerdev/data/*/FofosWebsite/website && bun run build:site +``` + +A score from the dev server is not wrong, it is about something nobody visits. ## Reading the output @@ -78,10 +84,17 @@ Performance is computed from these and nothing else. Insights are diagnostics; t ## Traps -**A desktop number from another machine includes that machine's latency.** Auditing from officer-dev -against fofos crosses a network the owner's browser does not. Compare like with like: two runs from -the same place, before and after a change. An absolute score from somewhere else is a different -question. +**A desktop audit is desktop in four ways, and `formFactor` is only one of them.** Setting +`formFactor: 'desktop'` changes which thresholds a metric is scored against and NOTHING else — the +throttling and the user agent keep their defaults, which are mobile. A run configured that way is a +desktop-sized window crawling on an emulated phone, and it reads about 86 where the same page reads +100. `api/lighthouse.ts` sets `screenEmulation`, `throttling` and `emulatedUserAgent` together for +this reason; if you write your own runner, set all four or the number is about nothing. + +**A number from another machine includes that machine's network.** Compare like with like: two runs +from the same place, before and after a change. An absolute score from somewhere else is a different +question — though check the paragraph above first, because that explains far more of a gap than +latency usually does. **Run-to-run variance is real** — a point or two either way on the same build. Chase a finding, not a number. If a score moves and no finding changed, nothing happened.