diff --git a/AGENTS.md b/AGENTS.md index 7b8247ba..b68f6055 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -73,6 +73,17 @@ installation, players, backend services, the site, or production deployment. reports both ffmpeg binaries, the `libx265`, `libx264`, and `libwebp` encoders, the `cwebp` fallback, and the `zscale` / `tonemap` filters. Do not claim ffmpeg has `libwebp` when it does not. +- **`doctor` check status is `pass`, `warn`, or `fail`, and only a `fail` moves + the exit code.** `data.status` is the worst row. A clean host must exit 0, so + an optional piece the CLI has a documented path without is a warning: `cwebp` + beside an ffmpeg with `libwebp`, `encoder_libwebp` beside a usable `cwebp`, + `encoder_libx265` (only `--codec hevc`), `filter_hdr_tonemap`, and the + server-advertised `feedback` feature. The two WebP rows fail together when + the host has neither encoder, because then no image can be transcoded at all. + `encoder_libx264`, `ffmpeg`, `ffprobe`, `node`, `config_permissions`, + `token`, `api_url`, and the four control-plane rows stay `fail`. Never widen + a `fail` to something the host cannot fix or a supported command never needs; + never soften a piece a default path requires. - Delivery profiles are fixed. Video defaults to H.264 (`libx264`, High, level 4.2, `-preset fast -crf 23 -maxrate 8M -bufsize 16M`, `-bf 2`, GOP two seconds, `yuv420p`, Rec. 709 limited range, AAC 192 kbit/s 48 kHz stereo), diff --git a/README.md b/README.md index 386672cf..81ccafaf 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,8 @@ screenrig --json version Node.js 20.11 or newer is required. `media upload` additionally requires ffmpeg and ffprobe 6.0 or newer; the other commands do not. Run `screenrig --json doctor` -to inspect the optional media toolchain before an upload. +to inspect the optional media toolchain before an upload. It exits 0 on a host +that is missing nothing required and reports the rest as warnings. This global package is the official developer-shell distribution. Agent workflows that load the ScreenRig plugin must keep using the plugin-relative launcher. The @@ -537,6 +538,45 @@ page or stream with nothing to print writes no human output. `--json` is a JSON envelope or stream. After redaction it may still include a server `message` field when that field is data. +## Preflight checks (`doctor`) + +`screenrig --json doctor` probes this installation and the configured control +plane, and reports one row per check with a `name`, a `detail`, and a `status`: + +| Status | Meaning | Effect on the exit code | +| --- | --- | --- | +| `pass` | Present and usable. | none | +| `warn` | An optional piece is absent and the CLI has a documented path without it. | none | +| `fail` | A defect that breaks a supported command until it is fixed. | exit 1 | + +`data.status` is the worst row in the list. The command exits 0 unless +something required failed, so a clean host — installed through the plugin or +through the npm package — passes even when optional pieces are absent. Stdout +is a success envelope either way: branch on `data.status` and the rows, not on +`ok`. + +`node`, `config_permissions`, `token`, `api_url`, `ffmpeg`, `ffprobe`, +`encoder_libx264`, `health`, `ready`, `version`, and `capabilities` fail when +they are not satisfied. These rows warn instead: + +| Check | Why it is optional | +| --- | --- | +| `cwebp` | The standalone WebP encoder is only the fallback for an ffmpeg build without libwebp, so a host whose ffmpeg carries the `libwebp` encoder never runs it. | +| `encoder_libwebp` | Where the binary above is installed, stills are encoded with it instead. Animation still needs `libwebp_anim`. | +| `encoder_libx265` | Only `--codec hevc` uses it. | +| `filter_hdr_tonemap` | Without `zscale` and `tonemap` an HDR source converts without tone mapping, and the upload envelope carries a warning. | +| `feedback` | The server advertises it; the `feedback` commands are unavailable where it is absent. | + +`encoder_libwebp` and `cwebp` are the two ways to encode the one delivery +format for images, so each warns while the other is usable and the two fail +together when the host has neither. That case is a real defect: no image can be +transcoded at all. The remedy the `cwebp` row names is an ffmpeg built with +libwebp, or `cwebp` on `PATH`, or `SCREENRIG_CWEBP` pointing at it. + +Neither the plugin launcher nor the npm package installs ffmpeg, ffprobe, or +cwebp. They stay host dependencies, resolved from `PATH` or from +`SCREENRIG_FFMPEG`, `SCREENRIG_FFPROBE`, and `SCREENRIG_CWEBP`. + ## Media transcoding `media upload` transcodes the source by default before it declares the upload, @@ -552,8 +592,9 @@ version but does not enforce a minimum. What it does enforce is the presence of the encoder each profile needs. `screenrig doctor` reports the resolved binaries and versions, the `libx265`, `libx264`, and `libwebp` encoders, the `cwebp` fallback, and whether the build carries the `zscale` and `tonemap` filters that -HDR tone mapping needs. `encoder_libwebp` is the ffmpeg encoder only; a fail -there does not mean stills cannot transcode when `cwebp` passes. +HDR tone mapping needs. `encoder_libwebp` is the ffmpeg encoder only, and its +detail never claims the fallback covers it: a warning there means stills still +transcode through `cwebp` while animation does not. The command also checks the filename. A low-information name such as `video.mp4` or `IMG_1234.jpg` adds an advisory `generic_filename` warning to @@ -664,8 +705,8 @@ Node.js 20.11 or newer is required by the package. The commands below are source-checkout development gates, not installed-plugin commands; run them from this repository checkout. Default `media upload` transcoding additionally requires ffmpeg and ffprobe on the host. `--no-transcode` bypasses both. No -other command requires them; `screenrig doctor` only probes and reports the -optional toolchain. +other command requires them; `screenrig doctor` probes and reports the +toolchain, and fails only on a piece a supported command actually needs. ```sh npm ci diff --git a/src/cli.test.ts b/src/cli.test.ts index 2f618343..5430500a 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -2261,49 +2261,149 @@ test("doctor reports checks over the published foundation routes", async () => { await rm(configDir, { recursive: true, force: true }); }); -test("doctor reports encoder_libwebp independently of the cwebp fallback", async () => { - resetFfmpegToolchainCache(); - const transport = memoryBackend(); - const configDir = await testTemp("doctor-cwebp-"); - const fsLike = { mkdir, open, rename, rm, chmod, stat, homedir: () => configDir, env: { XDG_CONFIG_HOME: configDir } }; - await writeConfigAtomic( - path.join(configDir, "screenrig", "config.json"), - { api_url: "https://api.screenrig.ai", token: "sr_live_tokidAAAAAAAAAAAAAAAA_secretsecretsecretsecretsecr" }, - fsLike, - ); - const runProcess = async (request: { command: string; args: string[] }) => { +/** + * Fake toolchain probe. `encoders` and `filters` are the ffmpeg capability + * listings, and `cwebp` decides whether the standalone binary answers + * `-version` at all. + */ +function fakeToolchainProbe(options: { encoders: string[]; filters: string[]; cwebp: boolean }) { + return async (request: { command: string; args: string[] }) => { if (request.args.includes("-version") && !request.args.includes("-hide_banner")) { - return { code: 0, signal: null, stdout: "1.6.0\nlibsharpyuv: 0.4.2\n", stderrTail: "" }; + return options.cwebp + ? { code: 0, signal: null, stdout: "1.6.0\nlibsharpyuv: 0.4.2\n", stderrTail: "" } + : { code: null, signal: null, stdout: "", stderrTail: "", spawnError: "spawn cwebp ENOENT" }; } if (request.args.includes("-version")) { return { code: 0, signal: null, stdout: `${path.basename(request.command)} version n8.1.2\n`, stderrTail: "" }; } if (request.args.includes("-encoders")) { - return { code: 0, signal: null, stdout: " V....D libx264 libx264 H.264\n", stderrTail: "" }; + const rows = options.encoders.map((encoder) => ` V....D ${encoder} ${encoder}\n`).join(""); + return { code: 0, signal: null, stdout: rows, stderrTail: "" }; } if (request.args.includes("-filters")) { - return { code: 0, signal: null, stdout: " .S scale V->V Scale.\n", stderrTail: "" }; + const rows = options.filters.map((filter) => ` .S ${filter} V->V Filter.\n`).join(""); + return { code: 0, signal: null, stdout: rows, stderrTail: "" }; } return { code: 1, signal: null, stdout: "", stderrTail: "" }; }; +} + +async function doctorWithToolchain( + prefix: string, + options: { encoders: string[]; filters: string[]; cwebp: boolean }, +): Promise<{ + code: number; + status: string; + byName: Record; + cleanup: () => Promise; +}> { + resetFfmpegToolchainCache(); + const transport = memoryBackend(); + const configDir = await testTemp(prefix); + const fsLike = { mkdir, open, rename, rm, chmod, stat, homedir: () => configDir, env: { XDG_CONFIG_HOME: configDir } }; + await writeConfigAtomic( + path.join(configDir, "screenrig", "config.json"), + { api_url: "https://api.screenrig.ai", token: "sr_live_tokidAAAAAAAAAAAAAAAA_secretsecretsecretsecretsecr" }, + fsLike, + ); const { code, stdout } = await withRuntime(["--json", "doctor"], transport, { fs: fsLike, - runProcess: runProcess as unknown as NonNullable, + runProcess: fakeToolchainProbe(options) as unknown as NonNullable, + }); + const envelope = JSON.parse(stdout) as { + ok: boolean; + data: { status: string; checks: Array<{ name: string; status: string; detail: string }> }; + }; + return { + code, + status: envelope.data.status, + byName: Object.fromEntries(envelope.data.checks.map((check) => [check.name, check])), + cleanup: () => rm(configDir, { recursive: true, force: true }), + }; +} + +test("doctor exits 0 on a clean host whose ffmpeg has libwebp and which has no cwebp", async () => { + const { code, status, byName, cleanup } = await doctorWithToolchain("doctor-no-cwebp-", { + encoders: ["libx264", "libx265", "libwebp", "libwebp_anim"], + filters: ["scale", "zscale", "tonemap"], + cwebp: false, }); try { - const envelope = JSON.parse(stdout) as { - ok: boolean; - data: { checks: Array<{ name: string; status: string; detail: string }> }; - }; - const byName = Object.fromEntries(envelope.data.checks.map((check) => [check.name, check])); - assert.equal(byName.encoder_libwebp?.status, "fail"); + assert.equal(byName.encoder_libwebp?.status, "pass"); + assert.equal(byName.cwebp?.status, "warn", "cwebp is only the fallback for a build without libwebp"); + assert.match(byName.cwebp?.detail ?? "", /not required because this ffmpeg build has the libwebp encoder/); + assert.equal(status, "warn"); + assert.equal(code, ExitCode.Success, "a host missing nothing required must exit 0"); + } finally { + await cleanup(); + } +}); + +test("doctor warns rather than fails for the encoders and filters that only optional paths need", async () => { + const { code, byName, cleanup } = await doctorWithToolchain("doctor-optional-", { + encoders: ["libx264", "libwebp", "libwebp_anim"], + filters: ["scale"], + cwebp: true, + }); + try { + assert.equal(byName.encoder_libx264?.status, "pass"); + assert.equal(byName.encoder_libx265?.status, "warn"); + assert.match(byName.encoder_libx265?.detail ?? "", /--codec hevc is unavailable/); + assert.equal(byName.filter_hdr_tonemap?.status, "warn"); + assert.equal(byName.cwebp?.status, "pass"); + assert.equal(code, ExitCode.Success); + } finally { + await cleanup(); + } +}); + +test("doctor reports encoder_libwebp independently of the cwebp fallback", async () => { + const { code, byName, cleanup } = await doctorWithToolchain("doctor-cwebp-", { + encoders: ["libx264"], + filters: ["scale"], + cwebp: true, + }); + try { + assert.equal(byName.encoder_libwebp?.status, "warn", "the cwebp fallback still encodes stills"); assert.match(byName.encoder_libwebp?.detail ?? "", /libwebp missing/); assert.doesNotMatch(byName.encoder_libwebp?.detail ?? "", /cwebp/); assert.equal(byName.cwebp?.status, "pass"); assert.match(byName.cwebp?.detail ?? "", /1\.6\.0/); - assert.equal(code, ExitCode.Unexpected, "encoder_libwebp fail still makes doctor non-zero"); + assert.equal(code, ExitCode.Success); } finally { - await rm(configDir, { recursive: true, force: true }); + await cleanup(); + } +}); + +test("doctor fails when neither libwebp nor cwebp can produce WebP", async () => { + const { code, status, byName, cleanup } = await doctorWithToolchain("doctor-no-webp-", { + encoders: ["libx264"], + filters: ["scale"], + cwebp: false, + }); + try { + assert.equal(byName.encoder_libwebp?.status, "fail"); + assert.equal(byName.cwebp?.status, "fail"); + assert.match(byName.cwebp?.detail ?? "", /no libwebp encoder either/); + assert.match(byName.cwebp?.detail ?? "", /SCREENRIG_CWEBP/); + assert.equal(status, "fail"); + assert.equal(code, ExitCode.Unexpected, "no WebP encoder at all is a real defect"); + } finally { + await cleanup(); + } +}); + +test("doctor fails a missing libx264 because it is the default video profile", async () => { + const { code, byName, cleanup } = await doctorWithToolchain("doctor-no-x264-", { + encoders: ["libwebp"], + filters: ["zscale", "tonemap"], + cwebp: false, + }); + try { + assert.equal(byName.encoder_libx264?.status, "fail"); + assert.equal(code, ExitCode.Unexpected); + } finally { + await cleanup(); } }); diff --git a/src/commands.ts b/src/commands.ts index f7f4a490..99e38b03 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -97,7 +97,14 @@ import { } from "./playlist-templates.js"; import { composeCatalog, formatComposeCatalog } from "./compose/catalog.js"; import { composeSpec } from "./compose/compose.js"; -import { cwebpLookup, ffmpegLookup, resolveCwebpToolchain, resolveFfmpegToolchain } from "./media/ffmpeg.js"; +import { + cwebpLookup, + ffmpegLookup, + resolveCwebpToolchain, + resolveFfmpegToolchain, + type CwebpToolchain, + type FfmpegToolchain, +} from "./media/ffmpeg.js"; import { createProgressReporter, silentProgressReporter, type ProgressReporter } from "./media/progress.js"; import { DEFAULT_CODEC, @@ -3250,12 +3257,33 @@ async function eventsFollow(args: ParsedArgs, runtime: CliRuntime, resolved: Awa }; } +/** + * `pass` is present and usable and `fail` is a defect the operator has to fix. + * `warn` is an optional piece the CLI has a documented path without, so it is + * reported without moving the exit code: a host that is missing nothing + * required exits 0. + */ +type DoctorStatus = "pass" | "warn" | "fail"; + +interface DoctorCheck { + name: string; + status: DoctorStatus; + detail: string; +} + +function probeFailureDetail(err: unknown, fallback: string): string { + if (err instanceof CliError) { + return err.problem.detail; + } + return err instanceof Error ? redactText(err.message) : fallback; +} + async function doctor( args: ParsedArgs, runtime: CliRuntime, resolved: Awaited>, ): Promise { - const checks: Array<{ name: string; status: "pass" | "fail"; detail: string }> = []; + const checks: DoctorCheck[] = []; const nodeMajor = Number(process.versions.node.split(".")[0]); checks.push({ name: "node", @@ -3295,8 +3323,30 @@ async function doctor( detail: resolved.logSocket ?? "(none)", }); const lookup = ffmpegLookup(runtime.env); + const webpLookup = cwebpLookup(runtime.env); + let toolchain: FfmpegToolchain | undefined; + let toolchainDetail: string | undefined; try { - const toolchain = await resolveFfmpegToolchain(runtime); + toolchain = await resolveFfmpegToolchain(runtime); + } catch (err) { + toolchainDetail = probeFailureDetail(err, "ffmpeg probe failed"); + } + let cwebp: CwebpToolchain | undefined; + let cwebpDetail: string | undefined; + try { + cwebp = await resolveCwebpToolchain(runtime); + } catch (err) { + cwebpDetail = probeFailureDetail(err, "cwebp probe failed"); + } + // WebP stills have two independent encoders: ffmpeg's libwebp, and the + // `cwebp` binary the image planner falls back to. Neither is required on its + // own, so each is a warning while the other is usable, and the pair fails + // only when the CLI has no way to produce WebP at all. `undefined` means the + // ffmpeg probe never answered, which the ffmpeg check already reports. + const libwebp = toolchain?.encoders.has("libwebp"); + const webpEncodable = libwebp === true || cwebp !== undefined; + + if (toolchain) { checks.push({ name: "ffmpeg", status: "pass", @@ -3307,48 +3357,70 @@ async function doctor( status: "pass", detail: `${toolchain.ffprobe} ${toolchain.ffprobeVersion}${lookup.ffprobeFromEnv ? " (SCREENRIG_FFPROBE)" : ""}`, }); - for (const [name, encoder] of [ - ["encoder_libx265", "libx265"], - ["encoder_libx264", "libx264"], - ["encoder_libwebp", "libwebp"], - ] as const) { - checks.push({ - name, - status: toolchain.encoders.has(encoder) ? "pass" : "fail", - detail: toolchain.encoders.has(encoder) ? `${encoder} available` : `${encoder} missing from this ffmpeg build`, - }); - } + const encoders = toolchain.encoders; + checks.push({ + name: "encoder_libx264", + status: encoders.has("libx264") ? "pass" : "fail", + detail: encoders.has("libx264") + ? "libx264 available" + : "libx264 missing from this ffmpeg build; the default video profile cannot encode", + }); + checks.push({ + name: "encoder_libx265", + status: encoders.has("libx265") ? "pass" : "warn", + detail: encoders.has("libx265") + ? "libx265 available" + : "libx265 missing from this ffmpeg build; --codec hevc is unavailable", + }); + checks.push({ + name: "encoder_libwebp", + status: encoders.has("libwebp") ? "pass" : webpEncodable ? "warn" : "fail", + detail: encoders.has("libwebp") + ? "libwebp available" + : "libwebp missing from this ffmpeg build; animation cannot be encoded", + }); const tonemap = toolchain.filters.has("zscale") && toolchain.filters.has("tonemap"); checks.push({ name: "filter_hdr_tonemap", - status: tonemap ? "pass" : "fail", + status: tonemap ? "pass" : "warn", detail: tonemap ? "zscale and tonemap available" : "zscale or tonemap missing; HDR sources convert without tone mapping", }); - } catch (err) { - const detail = err instanceof CliError ? err.problem.detail : err instanceof Error ? redactText(err.message) : "ffmpeg probe failed"; - checks.push({ name: "ffmpeg", status: "fail", detail }); + } else { + checks.push({ name: "ffmpeg", status: "fail", detail: toolchainDetail ?? "ffmpeg probe failed" }); } - const webpLookup = cwebpLookup(runtime.env); - try { - const cwebp = await resolveCwebpToolchain(runtime); - if (cwebp) { + + if (cwebp) { + checks.push({ + name: "cwebp", + status: "pass", + detail: `${cwebp.cwebp} ${cwebp.version}${cwebp.fromEnv ? " (SCREENRIG_CWEBP)" : ""}`, + }); + } else { + const missing = + cwebpDetail ?? `${webpLookup.cwebp} not available${webpLookup.cwebpFromEnv ? " (SCREENRIG_CWEBP)" : ""}`; + if (libwebp === true) { checks.push({ name: "cwebp", - status: "pass", - detail: `${cwebp.cwebp} ${cwebp.version}${cwebp.fromEnv ? " (SCREENRIG_CWEBP)" : ""}`, + status: "warn", + detail: `${missing}; not required because this ffmpeg build has the libwebp encoder`, }); - } else { + } else if (libwebp === false) { checks.push({ name: "cwebp", status: "fail", - detail: `${webpLookup.cwebp} not available${webpLookup.cwebpFromEnv ? " (SCREENRIG_CWEBP)" : ""}`, + detail: + `${missing}; this ffmpeg build has no libwebp encoder either, so image transcode cannot produce WebP. ` + + "Install an ffmpeg built with libwebp, or install cwebp on PATH (or set SCREENRIG_CWEBP).", + }); + } else { + checks.push({ + name: "cwebp", + status: "warn", + detail: `${missing}; it is the fallback for an ffmpeg build without libwebp, so fix ffmpeg first`, }); } - } catch (err) { - const detail = err instanceof CliError ? err.problem.detail : err instanceof Error ? redactText(err.message) : "cwebp probe failed"; - checks.push({ name: "cwebp", status: "fail", detail }); } const client = clientFor(runtime, args, resolved.apiUrl, resolved.token); @@ -3364,7 +3436,8 @@ async function doctor( const supported = features.feedback === true; checks.push({ name: "feedback", - status: supported ? "pass" : "fail", + // An optional server feature, so its absence is not a local defect. + status: supported ? "pass" : "warn", detail: supported ? "server advertises feedback support" : "server does not advertise feedback support; feedback commands are unavailable", @@ -3377,8 +3450,10 @@ async function doctor( } } const failed = checks.some((check) => check.status === "fail"); + const warned = checks.some((check) => check.status === "warn"); + const status: DoctorStatus = failed ? "fail" : warned ? "warn" : "pass"; return { - envelope: successEnvelope({ checks, version: CLI_VERSION }), + envelope: successEnvelope({ status, checks, version: CLI_VERSION }), exitCode: failed ? ExitCode.Unexpected : ExitCode.Success, human: checks.map((check) => `${check.status.toUpperCase()} ${check.name}: ${check.detail}`).join("\n"), }; diff --git a/src/localhost-smoke.ts b/src/localhost-smoke.ts index afbd2225..9a2a65d8 100644 --- a/src/localhost-smoke.ts +++ b/src/localhost-smoke.ts @@ -92,14 +92,26 @@ async function main(): Promise { const result = await invoke(["doctor"]); const envelope = JSON.parse(result.stdout) as { ok: boolean; - data?: { checks?: Array<{ name: string; status: string; detail: string }> }; + data?: { status?: string; checks?: Array<{ name: string; status: string; detail: string }> }; }; assert.equal(envelope.ok, true, result.stdout); const checks = envelope.data?.checks ?? []; assert.ok(checks.length > 0, result.stdout); for (const check of checks) { if (TOOLCHAIN_CHECKS.has(check.name)) continue; - assert.equal(check.status, "pass", `doctor check ${check.name} failed: ${check.detail}`); + assert.notEqual(check.status, "fail", `doctor check ${check.name} failed: ${check.detail}`); + } + const byName = new Map(checks.map((check) => [check.name, check])); + // Host-dependent, but never skipped on a host that has ffmpeg: `cwebp` is + // only the fallback for a build without libwebp, so an ffmpeg that carries + // the encoder must not leave doctor failing over a binary it never runs. + if (byName.get("encoder_libwebp")?.status === "pass") { + const cwebp = byName.get("cwebp"); + assert.notEqual(cwebp?.status, "fail", `cwebp is optional beside libwebp: ${cwebp?.detail}`); + } + if (!checks.some((check) => check.status === "fail")) { + assert.equal(envelope.data?.status !== "fail", true, result.stdout); + assert.equal(result.code, 0, `doctor with no failing check must exit 0: ${result.stdout}`); } };