mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-01 19:42:03 +00:00
fix: address review feedback on doctor --json
Follows up on jrusso1020's review in #320. Exit code no longer gated on check health --------------------------------------- `doctor --json` previously set exitCode=1 when any check failed. Two problems: - `checkVersion` returns ok:false whenever a newer npm version is available, so any pipeline using `hyperframes doctor --json || fail` would start failing the next time a new CLI version was published. - Asymmetric with bare `doctor` which always exits 0. Exit code now strictly reflects whether the command executed, not whether the environment is healthy. Consumers who want to gate do: hyperframes doctor --json | jq -e '.ok' > /dev/null || handle_failure Documented that pattern in docs/packages/cli.mdx. Schema locked with a snapshot test ---------------------------------- Extracted `buildDoctorReport()` as a pure function and added `doctor.test.ts` covering: - top-level key set (any accidental rename/addition fails the test) - shape of each CheckOutcome entry - ok flag true/false semantics - check-order preservation - hint field: omitted when absent, preserved when present - redact option both on and off Any future refactor that silently breaks the documented JSON contract will now fail CI. $HOME redaction for JSON mode ----------------------------- JSON output is explicitly designed to be pasted into bug reports and agent contexts. Added `redactHome()` so the user's home directory is replaced with the literal `$HOME` in `detail`/`hint` when --json is set. Human mode is unchanged (shows real paths). Import grouping --------------- Moved `node:os` + `_examples` imports up with the rest so `export const examples` no longer sits between imports.
This commit is contained in:
Dylan woo
@@ -525,8 +525,20 @@ This is suppressed in CI environments, non-TTY shells, and when `HYPERFRAMES_NO_
|
||||
◇ All checks passed
|
||||
```
|
||||
|
||||
| Flag | Description |
|
||||
|------|-------------|
|
||||
| `--json` | Output as JSON (includes `_meta` envelope) |
|
||||
|
||||
Verifies CLI version, Node.js, FFmpeg, FFprobe, Chrome, and Docker availability. If a newer CLI version is available, the version row shows an upgrade hint.
|
||||
|
||||
**CI gating.** `hyperframes doctor --json` always exits 0 on successful execution — the command succeeded if it produced valid output. Whether the environment is healthy is carried in the `ok` field of the payload, so a new CLI release (which flips `Version.ok` to `false`) never breaks your pipeline. Pipe through `jq` to gate on the payload instead:
|
||||
|
||||
```bash
|
||||
hyperframes doctor --json | jq -e '.ok' > /dev/null || handle_failure
|
||||
```
|
||||
|
||||
Paths in `detail` and `hint` are redacted in JSON mode — the user's home directory is replaced with the literal `$HOME` so output is safe to paste into bug reports and agent contexts.
|
||||
|
||||
### `info`
|
||||
|
||||
Display project metadata:
|
||||
|
||||
@@ -0,0 +1,141 @@
|
||||
import { describe, it, expect, beforeEach, afterEach } from "vitest";
|
||||
import { buildDoctorReport, redactHome, type CheckOutcome } from "./doctor.js";
|
||||
|
||||
// ── Fixtures ────────────────────────────────────────────────────────────────
|
||||
|
||||
const OUTCOMES_ALL_OK: CheckOutcome[] = [
|
||||
{ name: "Version", ok: true, detail: "0.4.4 (latest)" },
|
||||
{ name: "Node.js", ok: true, detail: "v22.0.0 (darwin arm64)" },
|
||||
{ name: "FFmpeg", ok: true, detail: "ffmpeg version 8.1" },
|
||||
];
|
||||
|
||||
const OUTCOMES_WITH_FAILURE: CheckOutcome[] = [
|
||||
{ name: "Version", ok: true, detail: "0.4.4 (latest)" },
|
||||
{
|
||||
name: "Docker",
|
||||
ok: false,
|
||||
detail: "Not found",
|
||||
hint: "https://docs.docker.com/get-docker/",
|
||||
},
|
||||
];
|
||||
|
||||
describe("redactHome", () => {
|
||||
const originalHome = process.env["HOME"];
|
||||
const originalUserProfile = process.env["USERPROFILE"];
|
||||
|
||||
afterEach(() => {
|
||||
if (originalHome !== undefined) process.env["HOME"] = originalHome;
|
||||
else delete process.env["HOME"];
|
||||
if (originalUserProfile !== undefined) process.env["USERPROFILE"] = originalUserProfile;
|
||||
else delete process.env["USERPROFILE"];
|
||||
});
|
||||
|
||||
it("replaces HOME paths with the literal $HOME", () => {
|
||||
process.env["HOME"] = "/Users/alice";
|
||||
delete process.env["USERPROFILE"];
|
||||
expect(redactHome("system: /Users/alice/Library/Caches/chrome")).toBe(
|
||||
"system: $HOME/Library/Caches/chrome",
|
||||
);
|
||||
});
|
||||
|
||||
it("replaces all occurrences, not just the first", () => {
|
||||
process.env["HOME"] = "/home/bob";
|
||||
delete process.env["USERPROFILE"];
|
||||
expect(redactHome("/home/bob/a and /home/bob/b")).toBe("$HOME/a and $HOME/b");
|
||||
});
|
||||
|
||||
it("falls back to USERPROFILE when HOME is unset (Windows)", () => {
|
||||
delete process.env["HOME"];
|
||||
process.env["USERPROFILE"] = "C:\\Users\\carol";
|
||||
expect(redactHome("C:\\Users\\carol\\AppData")).toBe("$HOME\\AppData");
|
||||
});
|
||||
|
||||
it("is a no-op when neither HOME nor USERPROFILE is set", () => {
|
||||
delete process.env["HOME"];
|
||||
delete process.env["USERPROFILE"];
|
||||
expect(redactHome("/Users/someone/path")).toBe("/Users/someone/path");
|
||||
});
|
||||
|
||||
it("leaves strings without HOME unchanged", () => {
|
||||
process.env["HOME"] = "/Users/alice";
|
||||
expect(redactHome("brew install ffmpeg")).toBe("brew install ffmpeg");
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildDoctorReport", () => {
|
||||
it("emits the locked schema shape", () => {
|
||||
const report = buildDoctorReport(OUTCOMES_ALL_OK);
|
||||
|
||||
expect(report).toMatchObject({
|
||||
ok: expect.any(Boolean),
|
||||
platform: expect.any(String),
|
||||
arch: expect.any(String),
|
||||
checks: expect.any(Array),
|
||||
_meta: expect.objectContaining({
|
||||
version: expect.any(String),
|
||||
updateAvailable: expect.any(Boolean),
|
||||
}),
|
||||
});
|
||||
|
||||
// Top-level keys are exactly these — any accidental addition or rename
|
||||
// should force an explicit update to this test + PR review.
|
||||
expect(Object.keys(report).sort()).toEqual(["_meta", "arch", "checks", "ok", "platform"]);
|
||||
});
|
||||
|
||||
it("reports ok=true when all checks pass", () => {
|
||||
expect(buildDoctorReport(OUTCOMES_ALL_OK).ok).toBe(true);
|
||||
});
|
||||
|
||||
it("reports ok=false when any check fails", () => {
|
||||
expect(buildDoctorReport(OUTCOMES_WITH_FAILURE).ok).toBe(false);
|
||||
});
|
||||
|
||||
it("preserves check order exactly as provided", () => {
|
||||
const report = buildDoctorReport(OUTCOMES_ALL_OK);
|
||||
expect(report.checks.map((c) => c.name)).toEqual(["Version", "Node.js", "FFmpeg"]);
|
||||
});
|
||||
|
||||
it("omits hint when not provided (doesn't emit hint:undefined)", () => {
|
||||
const report = buildDoctorReport([{ name: "X", ok: true, detail: "fine" }]);
|
||||
expect(report.checks[0]).toEqual({ name: "X", ok: true, detail: "fine" });
|
||||
expect("hint" in report.checks[0]!).toBe(false);
|
||||
});
|
||||
|
||||
it("preserves hint when provided", () => {
|
||||
const report = buildDoctorReport(OUTCOMES_WITH_FAILURE);
|
||||
const docker = report.checks.find((c) => c.name === "Docker");
|
||||
expect(docker?.hint).toBe("https://docs.docker.com/get-docker/");
|
||||
});
|
||||
|
||||
describe("redact option", () => {
|
||||
const originalHome = process.env["HOME"];
|
||||
beforeEach(() => {
|
||||
process.env["HOME"] = "/Users/alice";
|
||||
});
|
||||
afterEach(() => {
|
||||
if (originalHome !== undefined) process.env["HOME"] = originalHome;
|
||||
else delete process.env["HOME"];
|
||||
});
|
||||
|
||||
it("redacts HOME in detail and hint when redact=true", () => {
|
||||
const outcomes: CheckOutcome[] = [
|
||||
{
|
||||
name: "Chrome",
|
||||
ok: false,
|
||||
detail: "system: /Users/alice/Applications/Chrome",
|
||||
hint: "Try /Users/alice/bin/chrome",
|
||||
},
|
||||
];
|
||||
const report = buildDoctorReport(outcomes, { redact: true });
|
||||
expect(report.checks[0]?.detail).toBe("system: $HOME/Applications/Chrome");
|
||||
expect(report.checks[0]?.hint).toBe("Try $HOME/bin/chrome");
|
||||
});
|
||||
|
||||
it("leaves HOME alone when redact is off (default)", () => {
|
||||
const outcomes: CheckOutcome[] = [
|
||||
{ name: "Chrome", ok: true, detail: "/Users/alice/Chrome" },
|
||||
];
|
||||
expect(buildDoctorReport(outcomes).checks[0]?.detail).toBe("/Users/alice/Chrome");
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,12 +1,7 @@
|
||||
import { defineCommand } from "citty";
|
||||
import type { Example } from "./_examples.js";
|
||||
import { execSync } from "node:child_process";
|
||||
|
||||
export const examples: Example[] = [
|
||||
["Check system dependencies", "hyperframes doctor"],
|
||||
["Output as JSON for CI / agents", "hyperframes doctor --json"],
|
||||
];
|
||||
import { freemem, platform } from "node:os";
|
||||
import type { Example } from "./_examples.js";
|
||||
import { c } from "../ui/colors.js";
|
||||
import { findBrowser } from "../browser/manager.js";
|
||||
import { findFFmpeg } from "../browser/ffmpeg.js";
|
||||
@@ -14,6 +9,11 @@ import { VERSION } from "../version.js";
|
||||
import { getUpdateMeta, withMeta } from "../utils/updateCheck.js";
|
||||
import { getSystemMeta, getShmSizeMb, getFreeDiskMb, bytesToMb } from "../telemetry/system.js";
|
||||
|
||||
export const examples: Example[] = [
|
||||
["Check system dependencies", "hyperframes doctor"],
|
||||
["Output as JSON for CI / agents", "hyperframes doctor --json"],
|
||||
];
|
||||
|
||||
interface Check {
|
||||
name: string;
|
||||
run: () => CheckResult | Promise<CheckResult>;
|
||||
@@ -181,13 +181,53 @@ function checkEnvironment(): CheckResult {
|
||||
return { ok: true, detail: parts.join(" \u00B7 ") };
|
||||
}
|
||||
|
||||
interface CheckOutcome {
|
||||
export interface CheckOutcome {
|
||||
name: string;
|
||||
ok: boolean;
|
||||
detail: string;
|
||||
hint?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Replace the user's home directory path with the literal string `$HOME` so
|
||||
* JSON output pasted into bug reports or agent contexts doesn't leak usernames.
|
||||
* Safe no-op when HOME/USERPROFILE is unset.
|
||||
*/
|
||||
export function redactHome(s: string): string {
|
||||
const home = process.env["HOME"] || process.env["USERPROFILE"];
|
||||
if (!home) return s;
|
||||
return s.split(home).join("$HOME");
|
||||
}
|
||||
|
||||
function redactOutcome(o: CheckOutcome): CheckOutcome {
|
||||
return {
|
||||
name: o.name,
|
||||
ok: o.ok,
|
||||
detail: redactHome(o.detail),
|
||||
...(o.hint ? { hint: redactHome(o.hint) } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the JSON report payload from raw check outcomes. Pure function so the
|
||||
* output schema can be locked down with a snapshot test — any future refactor
|
||||
* that renames fields, drops `hint`, or reorders `checks[]` will fail that
|
||||
* test before it reaches users or agents parsing the output.
|
||||
*
|
||||
* @param options.redact - when true, replaces HOME paths in `detail`/`hint`
|
||||
* with the literal `$HOME`. Default off so tests can assert on raw values;
|
||||
* the CLI turns it on for `--json` output.
|
||||
*/
|
||||
export function buildDoctorReport(outcomes: CheckOutcome[], options: { redact?: boolean } = {}) {
|
||||
const checks = options.redact ? outcomes.map(redactOutcome) : outcomes;
|
||||
return withMeta({
|
||||
ok: checks.every((o) => o.ok),
|
||||
platform: process.platform,
|
||||
arch: process.arch,
|
||||
checks,
|
||||
});
|
||||
}
|
||||
|
||||
export default defineCommand({
|
||||
meta: { name: "doctor", description: "Check system dependencies and environment" },
|
||||
args: {
|
||||
@@ -229,20 +269,13 @@ export default defineCommand({
|
||||
const allOk = outcomes.every((o) => o.ok);
|
||||
|
||||
if (args.json) {
|
||||
console.log(
|
||||
JSON.stringify(
|
||||
withMeta({
|
||||
ok: allOk,
|
||||
platform: process.platform,
|
||||
arch: process.arch,
|
||||
checks: outcomes,
|
||||
}),
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
// Non-zero exit so `hyperframes doctor --json` is usable as a CI gate.
|
||||
if (!allOk) process.exitCode = 1;
|
||||
// Exit code intentionally reflects command success, not environment
|
||||
// health — `checkVersion` returns ok:false when an npm update is
|
||||
// available, which would poison any CI pipeline doing
|
||||
// `hyperframes doctor --json || fail` the next time a new version is
|
||||
// published. Consumers who want a gate can do:
|
||||
// hyperframes doctor --json | jq -e '.ok' > /dev/null || handle_failure
|
||||
console.log(JSON.stringify(buildDoctorReport(outcomes, { redact: true }), null, 2));
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user