From a15ebf696907270740972db8f2e328408bff0708 Mon Sep 17 00:00:00 2001 From: Alix-007 Date: Mon, 14 Sep 2026 13:37:00 +0800 Subject: [PATCH] fix(doctor): surface directory inspection failures (#147287) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Problem This Solves CLI health diagnostics silently treat every failed directory metadata lookup as an absent directory. A cyclic project-directory symlink, an inaccessible workspace, or a file blocking a parent component can consequently produce no directory warning. ## Why This Change Was Made Keep only `ENOENT` on the existing quiet missing-directory path. Other `statSync` failures use the existing unreadable-directory diagnostic and repair guidance, through the shared errno helper. Successful regular-file and read/write checks remain unchanged. This is independent of the configuration-root work in #145895: it does not change directory selection, authentication, backend registration, transcript lookup, or persistence. ## User Impact Operators receive a directory warning instead of silence when the configured CLI workspace/project metadata cannot be inspected. Healthy directories and genuinely absent first-use directories remain quiet. Doctor does not create, repair, remove, or change permissions on any directory in this patch; update execution and rollback behavior are unchanged. ## Evidence ### Historical September 14 integration refresh Refresh head: `1e4d36ce602446a2821aea6fa2e4be1a17a17f9e`. Original branch head `4b256621d378b969f52c3c7fb1aa1479df656384` and pinned main `8b917bc499607eb94bb77358ddd69f6494d26c65` are both preserved as parents; no force-push. Refresh the existing accepted patch onto pinned main to remove stale integration inputs and obtain new-head CI. No new feature or unrelated failure workaround is added. The resulting tree exactly matches the conflict-free git merge-tree result; there are no manual source or test edits. Validation on Linux / Node 26.1.0 with frozen-lockfile dependencies: No new local runtime test is claimed for this mechanical integration; existing proof keeps its original revision and exact-head hosted CI is required. The native staged-content/format guard, main-relative diff check, and one fresh independent P0-P2 source review passed. No full build or full type-aware check was rerun locally; new-head CI is tracked separately. Earlier runtime proof below retains its recorded SHA; it is not relabeled as a new execution. Candidate: `4b256621d378b969f52c3c7fb1aa1479df656384`, based on `e9c1cb7e71b7e0fb17a0e0f53491349b8faeb669`. Linux, Node 26.1.0; isolated synthetic HOME/config/state and a physically independent dependency installation. The production file was rechecked against current main `0e4b4d1fa38c6d78d8cd4f8520d0335457fd07c7`; its metadata catch still has the same defect. ### Observed directory diagnostics The source-runtime driver imports the actual `` entry and project-directory resolver through `scripts/tsx.mjs`. It creates a real cyclic project symlink (`ELOOP`) and a real file-parent/child path (`ENOTDIR`), captures the entry's note output, and checks real filesystem contents afterwards. No filesystem result or directory-health function is mocked. Only the external CLI executable is a synthetic executable returning a logged-in status; the production auth subprocess launches it normally. ```sh node --import ./scripts/tsx.mjs "$PWD" baseline node --import ./scripts/tsx.mjs "$PWD" candidate ``` | Same native filesystem scenario | Before | After | | --- | --- | --- | | Cyclic project-directory symlink | No note | Project directory is not readable; existing repair hint | | Workspace below a regular-file parent | No note | Workspace is not readable; existing repair hint | | Healthy directory | No note | No note | | Missing workspace | No note | No note | Actual candidate output, with temporary paths redacted: ```text - project dir: /./projects/ is not readable by this user. - Fix: make the project dir readable, or remove the broken path and let recreate it. - Workspace: /file-parent/child is not readable by this user. - Fix: make the workspace a readable, writable directory for the gateway user. ``` Both driver runs recorded exactly four `auth status --json` fixture invocations, unchanged configuration/executable/blocking-file bytes, and successful scratch removal. Candidate production SHA-256 `c3eac4e054ffdd415b4b35fd3e55aee3e492a25bcde62ecc0beb26ffd8ff2075` remained unchanged during proof and matches the committed file. The unbuilt source checkout emitted bundled backend setup-entry loading warnings and used the unchanged default CLI-command fallback. This is directory-owner proof with a real subprocess fixture, not verification of bundled backend discovery, installed Doctor bootstrap, a real account, an updater, or model execution. The warnings were retained rather than counted as successful plugin loading. ### Historical repository validation - Baseline existing-suite run: 9 passed / 5 failed. Four metadata-error cases observed zero notes, and the real blocked-parent case also observed no note. The candidate uses the existing unreadable wording for an unresolvable child, without claiming the child exists. - Candidate `node scripts/run-vitest.mjs src/commands/doctor--cli.test.ts --reporter=json --outputFile=`: **14/14 passed**. Existing tests are retained; new cases cover EACCES, EPERM, EIO, ELOOP, actual ENOTDIR and a missing-path control. - `node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.core.json src/commands/doctor--cli.ts src/commands/doctor--cli.test.ts`: **2 files, 263 rules, zero warnings/errors**. - Changed-file `oxfmt` and the normal formatting commit hook passed without changing the proof-covered production bytes. - One fresh independent P0–P2 autoreview of the staged diff: scoped-clean, no actionable findings. The reviewer did not rerun tests or builds. The historical execution above does not claim a full build, full typecheck, Windows execution, published-updater test or all-green hosted CI. There are no new configuration keys, public APIs, stored formats, log policies or dependencies. The production change is confined to classifying an existing diagnostic failure. --- ## Current validation at 937d7f2a The contributor commits and refresh remain intact. One additional commit replaces the helper-only regression cases with registered Doctor dispatch tests. The historical results above retain their original identities; the following evidence describes the current head. Backend names and local proof locations are generalized for publication. ## Problem and fix Doctor could silently omit its directory-health warning for an unreadable or cyclic project directory. Only an absent path now stays quiet after a failed metadata lookup; other failures reach the existing warning and repair guidance. ## Impact Readable and first-use paths stay quiet. Directory contents and permissions are unchanged. The contributor's production fix and authorship are retained; the correction changes only tests. ## Evidence At `937d7f2a5e6c830782164c3453efea77c8556367`, isolated `pnpm openclaw doctor --non-interactive` runs deliver warnings and matching hints for native cyclic, permission-denied and blocked-parent paths. Readable/missing controls stay quiet; regular-file behavior is unchanged. The structured cyclic check returns one warning and exit 1. Registered dispatch tests fail for both broken directories on main `8b917bc499607eb94bb77358ddd69f6494d26c65` and pass on the correction. The owner plus three Doctor sibling files pass 213 tests. The required local type-aware preflight passes. Hosted CI run [34807143893](https://github.com/openclaw/openclaw/actions/runs/34807143893) passed at this exact head after one retry of a shutdown-timing test group. The same group passed locally on the head and plain main (172 tests). Historical captures remain attributed separately: original candidate `4b256621`, historical merge `193a40f08164`, and their recorded main baselines. The merge's 214-test record was recovered without rerunning it; its real CLI matrix now has matching source/build identities. ## Consumers Normal Doctor output and structured findings share the corrected directory-health owner. Directory selection, authentication and transcript readers are unchanged. Tests exercise registered dispatch, runtime selection and final warning delivery, with readable/missing controls. AI-assisted. Co-authored-by: Ayaan Zaidi --- src/commands/doctor-claude-cli.test.ts | 131 ++++++++++++++++++++++++- src/commands/doctor-claude-cli.ts | 5 +- 2 files changed, 131 insertions(+), 5 deletions(-) diff --git a/src/commands/doctor-claude-cli.test.ts b/src/commands/doctor-claude-cli.test.ts index 4ad507b52acc..48b931b65359 100644 --- a/src/commands/doctor-claude-cli.test.ts +++ b/src/commands/doctor-claude-cli.test.ts @@ -1,22 +1,31 @@ // Doctor Claude CLI tests cover CLI discovery, version checks, and repair guidance. +import childProcess from "node:child_process"; import fs from "node:fs"; +import { syncBuiltinESMExports } from "node:module"; import os from "node:os"; import path from "node:path"; import { expectDefined } from "@openclaw/normalization-core/expect"; import { afterEach, describe, expect, it, vi } from "vitest"; import { resolveClaudeCliProjectDirForWorkspace } from "../agents/command/claude-cli-project-dir.js"; +import { clearHealthChecksForTest } from "../flows/health-check-registry.js"; +import { withEnvAsync } from "../test-utils/env.js"; import { noteClaudeCliHealth } from "./doctor-claude-cli.js"; +import { createTestRuntime } from "./test-runtime-config-helpers.js"; const resolveCliBackendConfigMock = vi.hoisted(() => vi.fn()); const resolveModelAgentRuntimeMetadataMock = vi.hoisted(() => - vi.fn((_params: { agentId: string }) => ({ id: "openclaw", source: "implicit" })), + vi + .fn() + .mockReturnValue({ id: "openclaw", source: "implicit" }), ); -vi.mock("../agents/cli-backends.js", () => ({ +vi.mock("../agents/cli-backends.js", async (importOriginal) => ({ + ...(await importOriginal()), resolveCliBackendConfig: resolveCliBackendConfigMock, })); -vi.mock("../agents/agent-runtime-metadata.js", () => ({ +vi.mock("../agents/agent-runtime-metadata.js", async (importOriginal) => ({ + ...(await importOriginal()), resolveModelAgentRuntimeMetadata: resolveModelAgentRuntimeMetadataMock, })); @@ -58,6 +67,8 @@ describe("noteClaudeCliHealth", () => { .mockReset() .mockReturnValue({ id: "openclaw", source: "implicit" }); vi.restoreAllMocks(); + syncBuiltinESMExports(); + clearHealthChecksForTest(); }); it("probes the executable resolved by the owning backend", async () => { @@ -327,4 +338,118 @@ describe("noteClaudeCliHealth", () => { expect(body).not.toContain(`Agent zeta workspace: ${zetaWorkspace}`); }); }); + + // Registered CLI entry; routed by test/vitest/vitest.commands.config.ts. + it.each(["cyclic project", "blocked workspace", "readable", "missing"])( + "doctor --lint --only core/doctor/claude-cli reports a %s directory at final output", + async (scenario) => { + clearHealthChecksForTest(); + await withTempHome(async ({ homeDir, workspaceDir }) => { + const configPath = path.join(homeDir, "openclaw.json"); + let configuredWorkspace = workspaceDir; + if (scenario === "blocked workspace") { + const parent = path.join(workspaceDir, "parent"); + fs.writeFileSync(parent, "not a directory"); + configuredWorkspace = path.join(parent, "child"); + } else if (scenario === "missing") { + configuredWorkspace = path.join(workspaceDir, "missing"); + } + const projectDir = resolveClaudeCliProjectDirForWorkspace({ + workspaceDir: configuredWorkspace, + homeDir, + }); + fs.mkdirSync(path.dirname(projectDir), { recursive: true }); + if (scenario === "cyclic project") { + fs.symlinkSync(projectDir, projectDir, process.platform === "win32" ? "junction" : "dir"); + } else if (scenario === "readable") { + fs.mkdirSync(projectDir); + } + fs.writeFileSync( + configPath, + JSON.stringify({ + agents: { + ownership: "explicit", + defaults: { + model: "anthropic/fixture", + models: { "anthropic/fixture": { agentRuntime: { id: "claude-cli" } } }, + workspace: configuredWorkspace, + }, + entries: { main: {} }, + }, + }), + ); + const actualRuntime = await vi.importActual< + typeof import("../agents/agent-runtime-metadata.js") + >("../agents/agent-runtime-metadata.js"); + resolveModelAgentRuntimeMetadataMock.mockImplementation( + actualRuntime.resolveModelAgentRuntimeMetadata, + ); + resolveCliBackendConfigMock.mockReturnValue({ + id: "claude-cli", + config: { command: process.execPath }, + }); + const spawnSync = childProcess.spawnSync; + vi.spyOn(childProcess, "spawnSync").mockImplementation((...args) => { + if ( + args[0] === process.execPath && + args[1]?.[0] === "auth" && + args[1]?.[1] === "status" && + args[1]?.[2] === "--json" + ) { + return { + pid: 1, + status: 0, + signal: null, + stdout: '{"loggedIn":true}', + stderr: "", + output: [null, '{"loggedIn":true}', ""], + }; + } + return spawnSync(...args); + }); + syncBuiltinESMExports(); + const stdout = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + await withEnvAsync( + { + HOME: homeDir, + OPENCLAW_HOME: homeDir, + OPENCLAW_STATE_DIR: path.join(homeDir, ".openclaw"), + OPENCLAW_CONFIG_PATH: configPath, + }, + async () => { + const { runDoctorLintCli } = await import("./doctor-lint.js"); + const exitCode = await runDoctorLintCli(createTestRuntime(), { + json: true, + onlyIds: ["core/doctor/claude-cli"], + }); + const output: unknown = JSON.parse( + stdout.mock.calls.map(([chunk]) => String(chunk)).join(""), + ); + const broken = scenario === "cyclic project" || scenario === "blocked workspace"; + expect(exitCode).toBe(broken ? 1 : 0); + expect(output).toMatchObject({ + ok: !broken, + checksRun: 1, + findings: broken + ? [ + { + checkId: "core/doctor/claude-cli", + severity: "warning", + message: + scenario === "cyclic project" + ? `Claude project dir: $OPENCLAW_HOME${projectDir.slice(homeDir.length)} is not readable by this user.` + : `Workspace: ${configuredWorkspace} is not readable by this user.`, + fixHint: + scenario === "cyclic project" + ? "- Fix: make the Claude project dir readable, or remove the broken path and let Claude recreate it." + : "- Fix: make the workspace a readable, writable directory for the gateway user.", + }, + ] + : [], + }); + }, + ); + }); + }, + ); }); diff --git a/src/commands/doctor-claude-cli.ts b/src/commands/doctor-claude-cli.ts index 71d96e458f75..7d270ad1b5c4 100644 --- a/src/commands/doctor-claude-cli.ts +++ b/src/commands/doctor-claude-cli.ts @@ -17,6 +17,7 @@ import { resolveCliBackendConfig } from "../agents/cli-backends.js"; import { resolveClaudeCliProjectDirForWorkspace } from "../agents/command/claude-cli-project-dir.js"; import { formatCliCommand } from "../cli/command-format.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { hasErrnoCode } from "../infra/errno.js"; import { resolveExecutablePath } from "../infra/executable-path.js"; import { shortenHomePath } from "../utils.js"; @@ -59,8 +60,8 @@ function probeDirectoryHealth(dirPath: string): ClaudeCliDirHealth { if (!stat.isDirectory()) { return "not_directory"; } - } catch { - return "missing"; + } catch (error) { + return hasErrnoCode(error, "ENOENT") ? "missing" : "unreadable"; } try { fs.accessSync(dirPath, fs.constants.R_OK);