From c4e53740462f07907e725a761f62b913b0b06215 Mon Sep 17 00:00:00 2001
From: dtzp555 <dtzp555@gmail.com>
Date: Mon, 11 May 2026 03:51:03 +1000
Subject: [PATCH] fix(doctor): handle unparseable version + empty health body
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three issues raised by code-quality reviewer on b65201b:

1. semverCompare returned 0 for unparseable input, causing fromSupported=true
   and kind=noop for an install with unreadable package.json. Now treats
   unparseable currentVersion as fresh_install candidate.
2. mockHealth: { status: 200, body: null } routed to fix_oauth (because
   health.body?.auth?.ok was undefined → falsy). 200 with empty body is
   server-broken, not OAuth-broken; now routes to fix_service.
3. Removed unused KIND_ENUM declaration (dead code).

Two regression tests added.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 scripts/doctor.mjs | 14 +++++++++-----
 test-features.mjs  | 16 ++++++++++++++++
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/scripts/doctor.mjs b/scripts/doctor.mjs
index d1da648..04ffd83 100644
--- a/scripts/doctor.mjs
+++ b/scripts/doctor.mjs
@@ -17,7 +17,6 @@ import { homedir } from "node:os";
 import { execSync } from "node:child_process";
 
 const SCHEMA_VERSION = "1";
-const KIND_ENUM = ["noop", "update", "upgrade", "fresh_install", "fix_oauth", "fix_service"];
 
 function semverParts(v) {
   const m = String(v).replace(/^v/, "").match(/^(\d+)\.(\d+)\.(\d+)/);
@@ -53,7 +52,7 @@ export async function runDoctor(opts = {}) {
   push("current_version", "PASS", `current=${currentVersion}`);
 
   // --- from-version supported? ---
-  const fromSupported = semverCompare(currentVersion, "v3.4.0") >= 0;
+  const fromSupported = !!semverParts(currentVersion) && semverCompare(currentVersion, "v3.4.0") >= 0;
   push("from_version_supported", fromSupported ? "PASS" : "FAIL",
        fromSupported ? "≥ v3.4.0" : `${currentVersion} < v3.4.0; in-place upgrade not supported`);
 
@@ -75,6 +74,9 @@ export async function runDoctor(opts = {}) {
     if (health.error || health.status !== 200) {
       healthOk = false;
       push("service_running", "FAIL", `service unreachable: ${health.error || `status ${health.status}`}`);
+    } else if (!health.body || typeof health.body !== "object") {
+      healthOk = false;
+      push("service_running", "FAIL", "service /health returned 200 but empty/non-JSON body");
     } else {
       push("service_running", "PASS", "service responding on /health");
       const authOk = health.body?.auth?.ok;
@@ -95,11 +97,13 @@ export async function runDoctor(opts = {}) {
     kind = "fix_service";
   } else if (!opts.skipNetwork && !oauthOk) {
     kind = "fix_oauth";
-  } else if (semverCompare(currentVersion, latestVersion) === 0) {
-    kind = "noop";
   } else {
     const cur = semverParts(currentVersion), lat = semverParts(latestVersion);
-    if (cur && lat && cur.major === lat.major && cur.minor === lat.minor) {
+    if (!cur) {
+      kind = "fresh_install";
+    } else if (semverCompare(currentVersion, latestVersion) === 0) {
+      kind = "noop";
+    } else if (lat && cur.major === lat.major && cur.minor === lat.minor) {
       kind = "update";
     } else {
       kind = "upgrade";
diff --git a/test-features.mjs b/test-features.mjs
index b970fac..a67c3ec 100644
--- a/test-features.mjs
+++ b/test-features.mjs
@@ -639,6 +639,22 @@ test("doctor service down → fix_service kind", async () => {
   assert.equal(result.next_action.kind, "fix_service");
 });
 
+test("doctor unparseable version → fresh_install", async () => {
+  const result = await runDoctor({ skipNetwork: true, mockVersion: "garbage", mockLatest: "v3.14.0" });
+  assert.equal(result.from_version_supported, false);
+  assert.equal(result.next_action.kind, "fresh_install");
+});
+
+test("doctor empty health body → fix_service (not fix_oauth)", async () => {
+  const result = await runDoctor({
+    skipNetwork: false,
+    mockVersion: "v3.10.0",
+    mockLatest: "v3.14.0",
+    mockHealth: { status: 200, body: null }
+  });
+  assert.equal(result.next_action.kind, "fix_service");
+});
+
 // ── Cleanup ──
 closeDb();