Fail doctor only on a piece a supported command needs

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),

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

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<string, { name: string; status: string; detail: string } | undefined>;
+  cleanup: () => Promise<void>;
+}> {
+  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<CliRuntime["runProcess"]>,
+    runProcess: fakeToolchainProbe(options) as unknown as NonNullable<CliRuntime["runProcess"]>,
+  });
+  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();
   }
 });