From 18ce604ace8b46819381e450cd03e17b6977adbb Mon Sep 17 00:00:00 2001 From: Rohit Ghumare Date: Wed, 10 Jun 2026 19:30:00 +0100 Subject: [PATCH] fix(doctor): make engine checks private-install-aware and honor manualOnly The engine-version-mismatch and iii-on-path-not-local-bin checks probed only the PATH-resolved iii binary, while their fixes install the pinned engine to ~/.agentmemory/bin, which is only prepended to PATH in-process. Both checks therefore failed forever and re-prompted on every doctor run, and the offered fixes were no-ops. Both checks now consult the private install first and pass when it exists at the pinned version, noting a mismatched PATH iii as informational detail since the private install wins at runtime. The version-mismatch fix now converges: install then recheck passes. runDoctor consults a new canAutoFix helper in both interactive and --all paths, so manualOnly diagnostics print their manual fix hint and count as skipped instead of prompting or auto-applying. Covered by new cases in test/cli-doctor-fixes.test.ts including a stateful installer stub proving fix convergence. --- src/cli.ts | 10 ++++-- src/cli/doctor-diagnostics.ts | 52 ++++++++++++++++++++++++-------- test/cli-doctor-fixes.test.ts | 57 +++++++++++++++++++++++++++++++++-- 3 files changed, 101 insertions(+), 18 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index ae5be0035..f243cf759 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -24,6 +24,7 @@ import * as p from "@clack/prompts"; import { generateId } from "./state/schema.js"; import { buildDiagnostics, + canAutoFix, dryRunPlan, parseEnvFile, type Diagnostic, @@ -1798,12 +1799,15 @@ async function runDoctor() { } failed++; p.log.warn(`${d.id} ✗ ${status.detail ?? ""}`.trim()); - p.log.info(`why: ${d.fixPreview}`); - if (d.manualOnly) { - p.log.info(`(manual fix only — see "${d.id}" docs)`); + if (!canAutoFix(d)) { + p.log.info(`manual fix: ${d.fixPreview}`); + skipped++; + continue; } + p.log.info(`why: ${d.fixPreview}`); + if (applyAll) { const r = await applyFixWithReport(d, ctx, false); if (r.ok) fixed++; diff --git a/src/cli/doctor-diagnostics.ts b/src/cli/doctor-diagnostics.ts index 10d95136d..e7cf7e1e4 100644 --- a/src/cli/doctor-diagnostics.ts +++ b/src/cli/doctor-diagnostics.ts @@ -59,6 +59,11 @@ export type Diagnostic = { manualOnly?: boolean; }; +/** True when doctor may apply this diagnostic's fix without user involvement. */ +export function canAutoFix(d: Diagnostic): boolean { + return !d.manualOnly; +} + // Diagnostic ids are stable for testing and machine-readable doctor output. export const DIAGNOSTIC_IDS = [ "env-missing", @@ -215,21 +220,37 @@ export function buildDiagnostics(effects: DoctorEffects): Diagnostic[] { }, { id: "engine-version-mismatch", - message: "iii binary on PATH doesn't match the version agentmemory pins to.", + message: "No iii binary matches the version agentmemory pins to.", fixPreview: "Re-run the iii installer for the pinned version and restart the engine.", moreInfo: "agentmemory pins the iii engine to a specific release because newer engines " + "use a different worker model. Running a mismatched binary surfaces as EPIPE " + - "reconnect loops and empty search results.", + "reconnect loops and empty search results. At runtime agentmemory prefers the " + + "private install at ~/.agentmemory/bin/iii when the iii on PATH mismatches the pin.", check: async (ctx) => { + const privateVersion = effects.iiiBinaryVersion(effects.localBinIiiPath()); const bin = effects.findIiiBinary(); - if (!bin) return { ok: false, detail: "iii not on PATH" }; - const v = effects.iiiBinaryVersion(bin); - if (!v) return { ok: false, detail: "iii on PATH but --version failed" }; + const pathVersion = bin ? effects.iiiBinaryVersion(bin) : null; + if (privateVersion === ctx.pinnedVersion) { + const note = + pathVersion && pathVersion !== ctx.pinnedVersion + ? `; PATH iii is ${pathVersion} but the private install wins at runtime` + : ""; + return { + ok: true, + detail: `private install ${privateVersion} (pinned ${ctx.pinnedVersion})${note}`, + }; + } + if (!bin) { + return { ok: false, detail: "iii not on PATH and no private install" }; + } + if (!pathVersion) { + return { ok: false, detail: "iii on PATH but --version failed" }; + } return { - ok: v === ctx.pinnedVersion, - detail: `${v} (pinned ${ctx.pinnedVersion})`, + ok: pathVersion === ctx.pinnedVersion, + detail: `${pathVersion} (pinned ${ctx.pinnedVersion})`, }; }, fix: async () => { @@ -308,7 +329,7 @@ export function buildDiagnostics(effects: DoctorEffects): Diagnostic[] { { id: "iii-on-path-not-local-bin", message: - "iii is on PATH but not at agentmemory's private install path.", + "iii is on PATH but agentmemory's private install is absent or mismatched.", fixPreview: "Install the pinned version to ~/.agentmemory/bin — won't touch your PATH.", moreInfo: @@ -317,14 +338,19 @@ export function buildDiagnostics(effects: DoctorEffects): Diagnostic[] { "When agentmemory needs the pin and PATH doesn't have it, it falls back to the " + "private install. If neither exists, run the installer.", manualOnly: true, - check: async () => { + check: async (ctx) => { const bin = effects.findIiiBinary(); if (!bin) return { ok: true, detail: "iii not on PATH (handled elsewhere)" }; const localBin = effects.localBinIiiPath(); - return { - ok: bin === localBin, - detail: bin === localBin ? undefined : `iii at: ${bin}`, - }; + if (bin === localBin) return { ok: true }; + const privateVersion = effects.iiiBinaryVersion(localBin); + if (privateVersion === ctx.pinnedVersion) { + return { + ok: true, + detail: `private install pinned; PATH iii at ${bin} stays untouched`, + }; + } + return { ok: false, detail: `iii at: ${bin}` }; }, fix: async () => effects.runIiiInstaller().then((r) => ({ diff --git a/test/cli-doctor-fixes.test.ts b/test/cli-doctor-fixes.test.ts index f7d671fb0..0c9a4334b 100644 --- a/test/cli-doctor-fixes.test.ts +++ b/test/cli-doctor-fixes.test.ts @@ -8,6 +8,7 @@ import { describe, it, expect } from "vitest"; import { buildDiagnostics, + canAutoFix, DIAGNOSTIC_IDS, dryRunPlan, parseEnvFile, @@ -169,11 +170,13 @@ describe("doctor v2 diagnostic catalog", () => { expect(status.detail).toContain("ANTHROPIC_API_KEY"); }); - it("iii-on-path-not-local-bin warns when iii lives in another location", async () => { + it("iii-on-path-not-local-bin warns when iii lives in another location and the private install is missing", async () => { + const privatePath = "/Users/test/.agentmemory/bin/iii"; const diagnostics = buildDiagnostics( stubEffects({ findIiiBinary: () => "/opt/homebrew/bin/iii", - localBinIiiPath: () => "/Users/test/.local/bin/iii", + localBinIiiPath: () => privatePath, + iiiBinaryVersion: (bin) => (bin === privatePath ? null : "0.11.2"), }), ); const check = diagnostics.find((d) => d.id === "iii-on-path-not-local-bin")!; @@ -182,6 +185,56 @@ describe("doctor v2 diagnostic catalog", () => { expect(check.manualOnly).toBe(true); }); + it("both engine checks pass when the private install matches the pin despite a mismatched PATH iii", async () => { + const privatePath = "/Users/test/.agentmemory/bin/iii"; + const diagnostics = buildDiagnostics( + stubEffects({ + findIiiBinary: () => "/opt/homebrew/bin/iii", + localBinIiiPath: () => privatePath, + iiiBinaryVersion: (bin) => (bin === privatePath ? "0.11.2" : "0.99.99"), + }), + ); + const versionCheck = diagnostics.find((d) => d.id === "engine-version-mismatch")!; + const versionStatus = await versionCheck.check(stubCtx()); + expect(versionStatus.ok).toBe(true); + expect(versionStatus.detail).toContain("private install wins at runtime"); + const pathCheck = diagnostics.find((d) => d.id === "iii-on-path-not-local-bin")!; + const pathStatus = await pathCheck.check(stubCtx()); + expect(pathStatus.ok).toBe(true); + expect(pathStatus.detail).toContain("/opt/homebrew/bin/iii"); + }); + + it("engine-version-mismatch fix converges once the installer writes the private binary", async () => { + const privatePath = "/Users/test/.agentmemory/bin/iii"; + let privateVersion: string | null = null; + const diagnostics = buildDiagnostics( + stubEffects({ + findIiiBinary: () => "/opt/homebrew/bin/iii", + localBinIiiPath: () => privatePath, + iiiBinaryVersion: (bin) => (bin === privatePath ? privateVersion : "0.99.99"), + runIiiInstaller: async () => { + privateVersion = "0.11.2"; + return { ok: true, message: "installed" }; + }, + }), + ); + const check = diagnostics.find((d) => d.id === "engine-version-mismatch")!; + const before = await check.check(stubCtx()); + expect(before.ok).toBe(false); + const fixResult = await check.fix(stubCtx()); + expect(fixResult.ok).toBe(true); + const after = await check.check(stubCtx()); + expect(after.ok).toBe(true); + }); + + it("canAutoFix is false for manualOnly diagnostics and true otherwise", () => { + const diagnostics = buildDiagnostics(stubEffects()); + const manual = diagnostics.find((d) => d.id === "iii-on-path-not-local-bin")!; + expect(canAutoFix(manual)).toBe(false); + const auto = diagnostics.find((d) => d.id === "engine-version-mismatch")!; + expect(canAutoFix(auto)).toBe(true); + }); + it("dryRunPlan lists each failing diagnostic with the fix preview", () => { const diagnostics = buildDiagnostics(stubEffects()); const results = diagnostics.map((d) => ({