diff --git a/src/commands/doctor-completion.test.ts b/src/commands/doctor-completion.test.ts new file mode 100644 index 00000000000..4b5e32ef01e --- /dev/null +++ b/src/commands/doctor-completion.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "vitest"; +import { + shellCompletionStatusToHealthFindings, + shellCompletionStatusToRepairEffects, + type ShellCompletionStatus, +} from "./doctor-completion.js"; + +function status(overrides: Partial = {}): ShellCompletionStatus { + return { + shell: "zsh", + profileInstalled: true, + cacheExists: true, + cachePath: "/tmp/openclaw.zsh", + usesSlowPattern: false, + ...overrides, + }; +} + +describe("shell completion health mapping", () => { + it("reports slow dynamic shell completion with dry-run effects", () => { + const current = status({ usesSlowPattern: true, cacheExists: false }); + + expect(shellCompletionStatusToHealthFindings(current)).toEqual([ + expect.objectContaining({ + checkId: "core/doctor/shell-completion", + severity: "info", + path: "shellCompletion.zsh", + }), + ]); + expect(shellCompletionStatusToRepairEffects(current)).toEqual([ + { + kind: "state", + action: "would-generate-completion-cache", + target: "/tmp/openclaw.zsh", + dryRunSafe: true, + }, + { + kind: "file", + action: "would-upgrade-shell-profile-completion", + target: "zsh", + dryRunSafe: false, + }, + ]); + }); + + it("reports missing completion cache with a dry-run cache effect", () => { + const current = status({ profileInstalled: true, cacheExists: false }); + + expect(shellCompletionStatusToHealthFindings(current)).toEqual([ + expect.objectContaining({ + severity: "info", + message: expect.stringContaining("cache is missing"), + fixHint: expect.stringContaining("openclaw doctor --fix"), + }), + ]); + expect(shellCompletionStatusToRepairEffects(current)).toEqual([ + { + kind: "state", + action: "would-regenerate-completion-cache", + target: "/tmp/openclaw.zsh", + dryRunSafe: true, + }, + ]); + }); + + it("keeps healthy shell completion quiet", () => { + const current = status(); + + expect(shellCompletionStatusToHealthFindings(current)).toEqual([]); + expect(shellCompletionStatusToRepairEffects(current)).toEqual([]); + }); +}); diff --git a/src/commands/doctor-completion.ts b/src/commands/doctor-completion.ts index dae0d9374f3..aff789bf8cd 100644 --- a/src/commands/doctor-completion.ts +++ b/src/commands/doctor-completion.ts @@ -11,6 +11,7 @@ import { resolveShellFromEnv, usesSlowDynamicCompletion, } from "../cli/completion-runtime.js"; +import type { HealthFinding, HealthRepairEffect } from "../flows/health-checks.js"; import { resolveOpenClawPackageRoot } from "../infra/openclaw-root.js"; import type { RuntimeEnv } from "../runtime.js"; import { note } from "../terminal/note.js"; @@ -85,6 +86,66 @@ export async function checkShellCompletionStatus( }; } +export function shellCompletionStatusToHealthFindings( + status: ShellCompletionStatus, +): readonly HealthFinding[] { + const checkId = "core/doctor/shell-completion"; + const path = `shellCompletion.${status.shell}`; + if (status.usesSlowPattern) { + return [ + { + checkId, + severity: "info", + message: `Your ${status.shell} profile uses slow dynamic completion (source <(...)).`, + path, + fixHint: "Run `openclaw doctor --fix` to upgrade to cached completion.", + }, + ]; + } + if (status.profileInstalled && !status.cacheExists) { + return [ + { + checkId, + severity: "info", + message: `Shell completion is configured in your ${status.shell} profile but the cache is missing.`, + path, + fixHint: `Run \`openclaw completion --write-state\` or \`openclaw doctor --fix\` to regenerate ${status.cachePath}.`, + }, + ]; + } + return []; +} + +export function shellCompletionStatusToRepairEffects( + status: ShellCompletionStatus, +): readonly HealthRepairEffect[] { + const effects: HealthRepairEffect[] = []; + if (status.usesSlowPattern && !status.cacheExists) { + effects.push({ + kind: "state", + action: "would-generate-completion-cache", + target: status.cachePath, + dryRunSafe: true, + }); + } + if (status.usesSlowPattern) { + effects.push({ + kind: "file", + action: "would-upgrade-shell-profile-completion", + target: status.shell, + dryRunSafe: false, + }); + } else if (status.profileInstalled && !status.cacheExists) { + effects.push({ + kind: "state", + action: "would-regenerate-completion-cache", + target: status.cachePath, + dryRunSafe: true, + }); + } + return effects; +} + export type DoctorCompletionOptions = { nonInteractive?: boolean; }; diff --git a/src/flows/doctor-core-checks.ts b/src/flows/doctor-core-checks.ts index 1c3f8313c69..bf8d4aefc45 100644 --- a/src/flows/doctor-core-checks.ts +++ b/src/flows/doctor-core-checks.ts @@ -7,6 +7,11 @@ import { type LegacyClawdBrowserProfileResidue, } from "../commands/doctor-browser.js"; import { hasConfiguredCommandOwners } from "../commands/doctor-command-owner.js"; +import { + checkShellCompletionStatus, + shellCompletionStatusToHealthFindings, + shellCompletionStatusToRepairEffects, +} from "../commands/doctor-completion.js"; import { disableUnavailableSkillsInConfig } from "../commands/doctor-skills-core.js"; import type { ConfigValidationIssue, OpenClawConfig } from "../config/types.openclaw.js"; import { resolveSecretInputRef } from "../config/types.secrets.js"; @@ -715,6 +720,29 @@ const finalConfigValidationCheck: HealthCheck = { }, }; +const shellCompletionCheck: HealthCheck = { + id: "core/doctor/shell-completion", + kind: "core", + description: "Shell completion uses the cached completion path when configured.", + source: "doctor", + async detect() { + return shellCompletionStatusToHealthFindings(await checkShellCompletionStatus()); + }, + async repair(ctx) { + const status = await checkShellCompletionStatus(); + const effects = shellCompletionStatusToRepairEffects(status); + if (ctx.dryRun === true) { + return { status: "repaired", changes: [], effects }; + } + return { + status: "skipped", + reason: "legacy doctor shell-completion repair owns real mutations", + changes: [], + effects, + }; + }, +}; + function createWorkspaceSuggestionsCheck(deps: CoreHealthCheckDeps): HealthCheck { return { id: "core/doctor/workspace-suggestions", @@ -742,6 +770,7 @@ function createConvertedWorkflowChecks(deps: CoreHealthCheckDeps): readonly Heal gatewayAuthCheck, legacyStateCheck, legacyWhatsAppCrontabCheck, + shellCompletionCheck, gatewayPlatformNotesCheck, createSecurityCheck(deps), browserCheck, diff --git a/src/flows/doctor-health-contributions.test.ts b/src/flows/doctor-health-contributions.test.ts index 4d3428b105b..5f56889d57f 100644 --- a/src/flows/doctor-health-contributions.test.ts +++ b/src/flows/doctor-health-contributions.test.ts @@ -8,6 +8,12 @@ import { const mocks = vi.hoisted(() => ({ maybeRunConfiguredPluginInstallReleaseStep: vi.fn(), + registerCoreHealthChecks: vi.fn(), + registerBundledHealthChecks: vi.fn(), + runDoctorHealthRepairs: vi.fn(), + listHealthChecks: vi.fn(), + resolveAgentWorkspaceDir: vi.fn(() => "/tmp/openclaw-workspace"), + resolveDefaultAgentId: vi.fn(() => "default"), note: vi.fn(), replaceConfigFile: vi.fn().mockResolvedValue(undefined), readConfigFileSnapshot: vi.fn().mockResolvedValue({ @@ -26,6 +32,27 @@ vi.mock("../commands/doctor/shared/release-configured-plugin-installs.js", () => maybeRunConfiguredPluginInstallReleaseStep: mocks.maybeRunConfiguredPluginInstallReleaseStep, })); +vi.mock("./doctor-core-checks.js", () => ({ + registerCoreHealthChecks: mocks.registerCoreHealthChecks, +})); + +vi.mock("./bundled-health-checks.js", () => ({ + registerBundledHealthChecks: mocks.registerBundledHealthChecks, +})); + +vi.mock("./doctor-repair-flow.js", () => ({ + runDoctorHealthRepairs: mocks.runDoctorHealthRepairs, +})); + +vi.mock("./health-check-registry.js", () => ({ + listHealthChecks: mocks.listHealthChecks, +})); + +vi.mock("../agents/agent-scope.js", () => ({ + resolveAgentWorkspaceDir: mocks.resolveAgentWorkspaceDir, + resolveDefaultAgentId: mocks.resolveDefaultAgentId, +})); + vi.mock("../terminal/note.js", () => ({ note: mocks.note, })); @@ -86,6 +113,30 @@ function buildDoctorPrompter(shouldRepair: boolean): DoctorPrompter { describe("doctor health contributions", () => { beforeEach(() => { mocks.maybeRunConfiguredPluginInstallReleaseStep.mockReset(); + mocks.registerCoreHealthChecks.mockReset(); + mocks.registerBundledHealthChecks.mockReset(); + mocks.runDoctorHealthRepairs.mockReset(); + mocks.runDoctorHealthRepairs.mockResolvedValue({ + config: {}, + findings: [], + remainingFindings: [], + changes: [], + warnings: [], + diffs: [], + effects: [], + checksRun: 0, + checksRepaired: 0, + checksValidated: 0, + }); + mocks.listHealthChecks.mockReset(); + mocks.listHealthChecks.mockReturnValue([ + { id: "core/doctor/shell-completion" }, + { id: "core/doctor/unrelated" }, + ]); + mocks.resolveAgentWorkspaceDir.mockReset(); + mocks.resolveAgentWorkspaceDir.mockReturnValue("/tmp/openclaw-workspace"); + mocks.resolveDefaultAgentId.mockReset(); + mocks.resolveDefaultAgentId.mockReturnValue("default"); mocks.note.mockReset(); mocks.readConfigFileSnapshot.mockReset(); mocks.readConfigFileSnapshot.mockResolvedValue({ @@ -210,6 +261,27 @@ describe("doctor health contributions", () => { ); }); + it("keeps legacy positional shell completion out of the broad structured repair pass", async () => { + const contribution = requireDoctorContribution("doctor:structured-health-repairs"); + const ctx = { + cfg: {}, + configResult: { cfg: {} }, + sourceConfigValid: true, + prompter: buildDoctorPrompter(true), + runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() }, + options: {}, + cfgForPersistence: {}, + configPath: "/tmp/fake-openclaw.json", + env: {}, + } as Parameters<(typeof contribution)["run"]>[0]; + + await contribution.run(ctx); + + expect(mocks.runDoctorHealthRepairs).toHaveBeenCalledWith(expect.any(Object), { + checks: [{ id: "core/doctor/unrelated" }], + }); + }); + it("skips doctor config writes under legacy update parents", () => { expect( shouldSkipLegacyUpdateDoctorConfigWrite({ diff --git a/src/flows/doctor-health-contributions.ts b/src/flows/doctor-health-contributions.ts index 14d69aab14b..357d757b115 100644 --- a/src/flows/doctor-health-contributions.ts +++ b/src/flows/doctor-health-contributions.ts @@ -50,6 +50,8 @@ type DoctorHealthContribution = FlowContribution & { run: (ctx: DoctorHealthFlowContext) => Promise; }; +const LEGACY_POSITIONAL_REPAIR_CHECK_IDS = new Set(["core/doctor/shell-completion"]); + function isUpdateDoctorRun(env: NodeJS.ProcessEnv | Record): boolean { const value = env.OPENCLAW_UPDATE_IN_PROGRESS; return value === "1" || value === "true"; @@ -243,6 +245,7 @@ async function runStructuredHealthRepairs(ctx: DoctorHealthFlowContext): Promise } const { registerCoreHealthChecks } = await import("./doctor-core-checks.js"); const { registerBundledHealthChecks } = await import("./bundled-health-checks.js"); + const { listHealthChecks } = await import("./health-check-registry.js"); const { runDoctorHealthRepairs } = await import("./doctor-repair-flow.js"); const { resolveAgentWorkspaceDir, resolveDefaultAgentId } = await import("../agents/agent-scope.js"); @@ -251,13 +254,19 @@ async function runStructuredHealthRepairs(ctx: DoctorHealthFlowContext): Promise registerCoreHealthChecks(); const workspaceDir = resolveAgentWorkspaceDir(ctx.cfg, resolveDefaultAgentId(ctx.cfg)); registerBundledHealthChecks({ cfg: ctx.cfg, cwd: workspaceDir }); - const result = await runDoctorHealthRepairs({ - mode: "fix", - runtime: ctx.runtime, - cfg: ctx.cfg, - cwd: workspaceDir, - configPath: ctx.configPath, - }); + const checks = listHealthChecks().filter( + (check) => !LEGACY_POSITIONAL_REPAIR_CHECK_IDS.has(check.id), + ); + const result = await runDoctorHealthRepairs( + { + mode: "fix", + runtime: ctx.runtime, + cfg: ctx.cfg, + cwd: workspaceDir, + configPath: ctx.configPath, + }, + { checks }, + ); ctx.cfg = result.config; if (result.changes.length > 0) { note(result.changes.join("\n"), "Doctor changes"); diff --git a/src/flows/doctor-lint-flow.test.ts b/src/flows/doctor-lint-flow.test.ts index 0189e13db13..3c3383fd2cf 100644 --- a/src/flows/doctor-lint-flow.test.ts +++ b/src/flows/doctor-lint-flow.test.ts @@ -92,4 +92,10 @@ describe("exitCodeFromFindings", () => { expect(exitCodeFromFindings(findings, "warning")).toBe(1); expect(exitCodeFromFindings(findings, "error")).toBe(0); }); + + it("does not fail default lint for informational findings", () => { + const findings = [{ checkId: "a", severity: "info" as const, message: "info" }]; + + expect(exitCodeFromFindings(findings)).toBe(0); + }); });