fix: four second-tier correctness defects (--json truncation, badge NaN, score buckets)

.changeset/quiet-planets-listen.md

@@ -0,0 +1,7 @@
+---
+"@docker-doctor/cli": patch
+---
+
+Piped or redirected `--json`/`--score` output is no longer silently truncated. The scan action's output paths used `process.exit()` right after `console.log()`, which could abort the process before the async write to a pipe/file finished flushing. They now set `process.exitCode` and return, letting Node exit naturally once stdout has fully drained. Exit codes (`--score` below 50 exits 1; `--json`/default exit 1 on any error-severity diagnostic) are unchanged.
+
+The score threshold table (labels/emoji at 90/75/50/0) is now exported as `SCORE_BUCKETS` (and `getScoreBucket`) from `@docker-doctor/core`, replacing what used to be duplicated inline logic in `calculateScore`. The printed/JSON `label` strings are byte-identical to before.

apps/web/app/share/badge/route.ts

@@ -1,21 +1,33 @@
+import { getScoreData, parseScoreQuery } from "@/lib/score";
+
 const LABEL = "Docker Doctor";
 
+// Color-only variant of the same thresholds as `getScoreData` in
+// `@/lib/score` (which mirrors `SCORE_BUCKETS` in
+// `packages/core/src/scoring.ts` -- see the note there on why this
+// isn't unified via an `@docker-doctor/core` import).
 const getScoreColor = (score: number): string => {
-  if (score >= 90) {
-    return "#4c1";
-  }
-  if (score >= 75) {
-    return "#dfb317";
+  const { label } = getScoreData(score);
+  switch (label) {
+    case "Excellent": {
+      return "#4c1";
+    }
+    case "Good": {
+      return "#dfb317";
+    }
+    case "Needs Work": {
+      return "#fe7d37";
+    }
+    default: {
+      // Critical
+      return "#e05d44";
+    }
   }
-  return "#e05d44";
 };
 
-const parseQuery = (value: string | null, fallback: number): number =>
-  Math.min(100, Math.max(0, Math.trunc(Number(value ?? fallback))));
-
 export const GET = (req: Request): Response => {
   const { searchParams } = new URL(req.url);
-  const score = parseQuery(searchParams.get("s"), 100);
+  const score = parseScoreQuery(searchParams.get("s"), 100);
 
   const labelWidth = 108;
   const valueWidth = 76;

apps/web/app/share/og/route.tsx

@@ -4,16 +4,11 @@ import path from "node:path";
 import { ImageResponse } from "next/og";
 import type { NextRequest } from "next/server";
 
-import { getScoreData } from "@/lib/score";
-
-const parseScore = (raw: string | null): number => {
-  const value = Math.trunc(Number(raw || "100"));
-  return Math.min(100, Math.max(0, value));
-};
+import { getScoreData, parseScoreQuery } from "@/lib/score";
 
 export const GET = async (request: NextRequest) => {
   const { searchParams } = new URL(request.url);
-  const score = parseScore(searchParams.get("s"));
+  const score = parseScoreQuery(searchParams.get("s"), 100);
   const { color } = getScoreData(score);
 
   const [regularFont, semiboldFont] = await Promise.all([

apps/web/lib/score.ts

@@ -5,27 +5,74 @@ export interface ScoreData {
   label: string;
 }
 
+// NOTE: this mirrors `SCORE_BUCKETS` in `packages/core/src/scoring.ts`
+// (thresholds 90/75/50/0). It is intentionally NOT imported from
+// `@docker-doctor/core`: that package's raw-TS sources use regex named
+// capturing groups (e.g. dockerfile-parser.ts, rules/security.ts), which
+// require `target >= ES2018`, but this app's tsconfig targets ES2017 --
+// importing the package breaks `tsc --noEmit` for the whole app. See the
+// plan 007 report for the exact error; fixing this (bumping the web
+// target, or dropping named groups in core) is a decision for the
+// maintainer, not made here.
+const SCORE_BUCKETS = [
+  {
+    background: "bg-green-500/10",
+    border: "border-green-500",
+    color: "#22c55e",
+    label: "Excellent",
+    min: 90,
+  },
+  {
+    background: "bg-yellow-500/10",
+    border: "border-yellow-500",
+    color: "#eab308",
+    label: "Good",
+    min: 75,
+  },
+  {
+    background: "bg-orange-500/10",
+    border: "border-orange-500",
+    color: "#f97316",
+    label: "Needs Work",
+    min: 50,
+  },
+  {
+    background: "bg-red-600/10",
+    border: "border-red-600",
+    color: "#dc2626",
+    label: "Critical",
+    min: 0,
+  },
+] as const;
+
 export const getScoreData = (score: number): ScoreData => {
-  if (score >= 90) {
-    return {
-      background: "bg-green-500/10",
-      border: "border-green-500",
-      color: "#22c55e",
-      label: "Excellent",
-    };
+  for (const bucket of SCORE_BUCKETS) {
+    if (score >= bucket.min) {
+      return bucket;
+    }
   }
-  if (score >= 75) {
-    return {
-      background: "bg-yellow-500/10",
-      border: "border-yellow-500",
-      color: "#eab308",
-      label: "Good",
-    };
+  return SCORE_BUCKETS.at(-1) as ScoreData;
+};
+
+const clampScore = (n: number): number => Math.min(100, Math.max(0, n));
+
+/**
+ * Parses a raw `s` query-param value into a score clamped to [0, 100].
+ *
+ * `value === null` (the param is absent) falls back to `fallback`.
+ * Any present-but-non-finite value (e.g. "abc") also falls back to
+ * `fallback`. An empty string ("") is a *present* value: `Number("")`
+ * is `0`, so it clamps to `0` rather than falling back -- this is a
+ * deliberate choice, not an oversight.
+ */
+export const parseScoreQuery = (
+  value: string | null,
+  fallback = 100
+): number => {
+  if (value === null) {
+    return clampScore(fallback);
   }
-  return {
-    background: "bg-red-500/10",
-    border: "border-red-500",
-    color: "#ef4444",
-    label: "Needs Work",
-  };
+  const parsed = Number(value);
+  const n = Number.isFinite(parsed) ? Math.trunc(parsed) : fallback;
+  return clampScore(n);
 };

packages/core/src/index.ts

@@ -10,7 +10,7 @@ export {
   findRule,
 } from "./rules/index";
 export { loadConfig } from "./config/loader";
-export { calculateScore } from "./scoring";
+export { calculateScore, getScoreBucket, SCORE_BUCKETS } from "./scoring";
 export * from "./errors";
 export * from "./types/index";
 export { toJsonReport, type JsonReport } from "./report";

packages/core/src/scoring.ts

@@ -1,5 +1,23 @@
 import type { Diagnostic } from "./types/index";
 
+export const SCORE_BUCKETS = [
+  { emoji: "🏆", label: "Excellent", min: 90 },
+  { emoji: "✅", label: "Good", min: 75 },
+  { emoji: "⚠️", label: "Needs Work", min: 50 },
+  { emoji: "🚨", label: "Critical", min: 0 },
+] as const;
+
+export const getScoreBucket = (
+  score: number
+): (typeof SCORE_BUCKETS)[number] => {
+  for (const bucket of SCORE_BUCKETS) {
+    if (score >= bucket.min) {
+      return bucket;
+    }
+  }
+  return SCORE_BUCKETS.at(-1) as (typeof SCORE_BUCKETS)[number];
+};
+
 export const calculateScore = (
   diagnostics: Diagnostic[]
 ): {
@@ -19,15 +37,8 @@ export const calculateScore = (
   }
 
   const score = Math.max(0, 100 - penalty);
-
-  let label = "Critical 🚨";
-  if (score >= 90) {
-    label = "Excellent 🏆";
-  } else if (score >= 75) {
-    label = "Good ✅";
-  } else if (score >= 50) {
-    label = "Needs Work ⚠️";
-  }
+  const bucket = getScoreBucket(score);
+  const label = `${bucket.label} ${bucket.emoji}`;
 
   return { label, score };
 };

packages/core/test/scoring.test.ts

@@ -0,0 +1,58 @@
+import { describe, expect, test } from "bun:test";
+
+import { calculateScore, getScoreBucket, SCORE_BUCKETS } from "../src/scoring";
+import type { Diagnostic } from "../src/types/index";
+
+const errorDiag = (n: number): Diagnostic[] =>
+  Array.from({ length: n }, () => ({
+    file: "Dockerfile",
+    help: "help",
+    message: "message",
+    rule: "docker-doctor/test-rule",
+    severity: "error" as const,
+  }));
+
+describe("calculateScore", () => {
+  test("no diagnostics scores 100 and labels Excellent", () => {
+    const { score, label } = calculateScore([]);
+    expect(score).toBe(100);
+    expect(label).toBe("Excellent 🏆");
+  });
+
+  test("scores >= 75 label Good", () => {
+    // penalty 20 -> 80
+    const { score, label } = calculateScore(errorDiag(2));
+    expect(score).toBe(80);
+    expect(label).toBe("Good ✅");
+  });
+
+  test("scores >= 50 label Needs Work", () => {
+    // penalty 50 -> 50
+    const { score, label } = calculateScore(errorDiag(5));
+    expect(score).toBe(50);
+    expect(label).toBe("Needs Work ⚠️");
+  });
+
+  test("scores below 50 label Critical", () => {
+    // penalty 100 -> 0
+    const { score, label } = calculateScore(errorDiag(10));
+    expect(score).toBeLessThan(50);
+    expect(label).toBe("Critical 🚨");
+  });
+});
+
+describe("getScoreBucket", () => {
+  test("returns the matching bucket for a boundary score", () => {
+    expect(getScoreBucket(0).label).toBe("Critical");
+    expect(getScoreBucket(49).label).toBe("Critical");
+    expect(getScoreBucket(50).label).toBe("Needs Work");
+    expect(getScoreBucket(75).label).toBe("Good");
+    expect(getScoreBucket(90).label).toBe("Excellent");
+    expect(getScoreBucket(100).label).toBe("Excellent");
+  });
+
+  test("SCORE_BUCKETS is ordered from highest to lowest threshold", () => {
+    const mins = SCORE_BUCKETS.map((b) => b.min);
+    expect(mins).toEqual([...mins].toSorted((a, b) => b - a));
+  });
+});

packages/docker-doctor/package.json

@@ -54,6 +54,7 @@
   "scripts": {
     "build": "NODE_OPTIONS='--max-old-space-size=4096' tsdown",
     "dev": "NODE_OPTIONS='--max-old-space-size=4096' tsdown --watch",
+    "test": "bun test",
     "typecheck": "tsc --noEmit",
     "clean": "git clean -xdf .turbo node_modules dist"
   },

packages/docker-doctor/src/cli.ts

@@ -476,7 +476,8 @@ program
 
         if (options.score) {
           console.log(score);
-          process.exit(score < 50 ? 1 : 0);
+          process.exitCode = score < 50 ? 1 : 0;
+          return;
         } else if (options.json) {
           const report = toJsonReport(
             filteredDiagnostics,
@@ -488,27 +489,27 @@ program
           const hasErrors = filteredDiagnostics.some(
             (d) => d.severity === "error"
           );
-          process.exit(hasErrors ? 1 : 0);
-        } else {
-          await formatTerminal(
-            filteredDiagnostics,
-            score,
-            label,
-            project,
-            options.verbose,
-            fileContents
-          );
+          process.exitCode = hasErrors ? 1 : 0;
+          return;
+        }
+        await formatTerminal(
+          filteredDiagnostics,
+          score,
+          label,
+          project,
+          options.verbose,
+          fileContents
+        );
 
-          // Exit with non-zero code if there are any error severity diagnostics
-          const hasErrors = filteredDiagnostics.some(
-            (d) => d.severity === "error"
-          );
+        // Exit with non-zero code if there are any error severity diagnostics
+        const hasErrors = filteredDiagnostics.some(
+          (d) => d.severity === "error"
+        );
 
-          if (process.stdout.isTTY && process.stdin.isTTY) {
-            await runInteractiveWizard();
-          }
-          process.exit(hasErrors ? 1 : 0);
+        if (process.stdout.isTTY && process.stdin.isTTY) {
+          await runInteractiveWizard();
         }
+        process.exitCode = hasErrors ? 1 : 0;
       } finally {
         if (spinnerInterval !== null) {
           clearInterval(spinnerInterval);