mirror of
https://github.com/openclaw/openclaw.git
synced 2026-10-03 01:29:56 +00:00
fix(doctor): surface directory inspection failures (#147287)
## 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 `8b917bc499` 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 `e9c1cb7e71`. Linux, Node 26.1.0; isolated synthetic HOME/config/state and a physically independent dependency installation. The production file was rechecked against current main `0e4b4d1fa3`; its metadata catch still has the same defect. ### Observed directory diagnostics The source-runtime driver imports the actual `<directory-health-owner>` 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 <proof-driver> "$PWD" baseline node --import ./scripts/tsx.mjs <proof-driver> "$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 - <backend> project dir: <isolated-home>/.<backend>/projects/<workspace-key> is not readable by this user. - Fix: make the <backend> project dir readable, or remove the broken path and let <backend> recreate it. - Workspace: <fixture>/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-<backend>-cli.test.ts --reporter=json --outputFile=<receipt>`: **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-<backend>-cli.ts src/commands/doctor-<backend>-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 `8b917bc499` 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 <hi@obviy.us>
This commit is contained in:
Alix-007
GitHub
parent
ad157baa20
commit
a15ebf6969
2 changed files with 131 additions and 5 deletions
|
|
@ -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<typeof import("../agents/agent-runtime-metadata.js").resolveModelAgentRuntimeMetadata>()
|
||||
.mockReturnValue({ id: "openclaw", source: "implicit" }),
|
||||
);
|
||||
|
||||
vi.mock("../agents/cli-backends.js", () => ({
|
||||
vi.mock("../agents/cli-backends.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../agents/cli-backends.js")>()),
|
||||
resolveCliBackendConfig: resolveCliBackendConfigMock,
|
||||
}));
|
||||
|
||||
vi.mock("../agents/agent-runtime-metadata.js", () => ({
|
||||
vi.mock("../agents/agent-runtime-metadata.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../agents/agent-runtime-metadata.js")>()),
|
||||
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.",
|
||||
},
|
||||
]
|
||||
: [],
|
||||
});
|
||||
},
|
||||
);
|
||||
});
|
||||
},
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue