mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-19 09:21:37 +00:00
fix(doctor): keep automated repair from moving approval state (#103353)
* fix(doctor): isolate automated cross-state imports * test(update): cover isolated doctor finalization
This commit is contained in:
committed by
GitHub
parent
4e3d91a020
commit
cd9db5ed9a
@@ -182,6 +182,7 @@ describe("ensureConfigReady", () => {
|
||||
migrateState: true,
|
||||
migrateLegacyConfig: false,
|
||||
invalidConfigNote: false,
|
||||
crossStateDirImports: false,
|
||||
});
|
||||
}
|
||||
});
|
||||
@@ -203,6 +204,7 @@ describe("ensureConfigReady", () => {
|
||||
migrateLegacyConfig: false,
|
||||
invalidConfigNote: false,
|
||||
observe: false,
|
||||
crossStateDirImports: false,
|
||||
});
|
||||
});
|
||||
|
||||
@@ -217,6 +219,7 @@ describe("ensureConfigReady", () => {
|
||||
migrateLegacyConfig: false,
|
||||
invalidConfigNote: false,
|
||||
observe: false,
|
||||
crossStateDirImports: false,
|
||||
});
|
||||
});
|
||||
|
||||
@@ -227,6 +230,7 @@ describe("ensureConfigReady", () => {
|
||||
migrateState: true,
|
||||
migrateLegacyConfig: false,
|
||||
invalidConfigNote: false,
|
||||
crossStateDirImports: false,
|
||||
requireStartupMigrationCheckpoint: true,
|
||||
});
|
||||
});
|
||||
@@ -259,6 +263,7 @@ describe("ensureConfigReady", () => {
|
||||
migrateState: true,
|
||||
migrateLegacyConfig: false,
|
||||
invalidConfigNote: false,
|
||||
crossStateDirImports: false,
|
||||
});
|
||||
});
|
||||
|
||||
@@ -282,6 +287,7 @@ describe("ensureConfigReady", () => {
|
||||
migrateState: true,
|
||||
migrateLegacyConfig: false,
|
||||
invalidConfigNote: false,
|
||||
crossStateDirImports: false,
|
||||
});
|
||||
});
|
||||
|
||||
@@ -306,6 +312,7 @@ describe("ensureConfigReady", () => {
|
||||
migrateState: true,
|
||||
migrateLegacyConfig: false,
|
||||
invalidConfigNote: false,
|
||||
crossStateDirImports: false,
|
||||
});
|
||||
expect(setRuntimeConfigSnapshotMock).toHaveBeenCalledWith(
|
||||
migratedSnapshot.runtimeConfig,
|
||||
@@ -322,18 +329,36 @@ describe("ensureConfigReady", () => {
|
||||
expect(loadAndMaybeMigrateDoctorConfigMock).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("does not run doctor flow for default-state-dir exec approvals when a custom state dir is set", async () => {
|
||||
// Cross-state-dir imports are doctor-owned; the implicit preflight must not
|
||||
// trigger (and must never archive) files that belong to the default dir.
|
||||
const root = useTempOpenClawHome();
|
||||
const stateDir = path.join(root, "custom-state");
|
||||
setTestEnvValue("OPENCLAW_STATE_DIR", stateDir);
|
||||
writeStateMarker(root, "exec-approvals.json");
|
||||
it.each([
|
||||
{ commandPath: ["agent"], source: "exec-approvals.json" },
|
||||
{ commandPath: ["status"], source: "plugin-binding-approvals.json" },
|
||||
{ commandPath: ["plugins", "list"], source: "exec-approvals.json" },
|
||||
{ commandPath: ["tasks", "list"], source: "plugin-binding-approvals.json" },
|
||||
])(
|
||||
"runs notice-only preflight for $commandPath with default-state $source",
|
||||
async ({ commandPath, source }) => {
|
||||
const root = useTempOpenClawHome();
|
||||
const stateDir = path.join(root, "custom-state");
|
||||
setTestEnvValue("OPENCLAW_STATE_DIR", stateDir);
|
||||
writeStateMarker(root, source);
|
||||
const sourcePath = path.join(root, ".openclaw", source);
|
||||
const sourceRaw = fs.readFileSync(sourcePath, "utf8");
|
||||
|
||||
await runEnsureConfigReady(["agent"]);
|
||||
await runEnsureConfigReady(commandPath);
|
||||
|
||||
expect(loadAndMaybeMigrateDoctorConfigMock).not.toHaveBeenCalled();
|
||||
});
|
||||
expect(loadAndMaybeMigrateDoctorConfigMock).toHaveBeenCalledOnce();
|
||||
expect(loadAndMaybeMigrateDoctorConfigMock).toHaveBeenCalledWith({
|
||||
migrateState: true,
|
||||
migrateLegacyConfig: false,
|
||||
invalidConfigNote: false,
|
||||
...(commandPath[0] === "status" ? { observe: false } : {}),
|
||||
crossStateDirImports: false,
|
||||
});
|
||||
expect(fs.readFileSync(sourcePath, "utf8")).toBe(sourceRaw);
|
||||
expect(fs.existsSync(`${sourcePath}.migrated`)).toBe(false);
|
||||
expect(fs.existsSync(path.join(stateDir, "exec-approvals.json"))).toBe(false);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
["Discord model picker preferences", "discord/model-picker-preferences.json"],
|
||||
|
||||
@@ -4,7 +4,12 @@ import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { withSuppressedNotes } from "../../../packages/terminal-core/src/note.js";
|
||||
import { readConfigFileSnapshot, setRuntimeConfigSnapshot } from "../../config/config.js";
|
||||
import { resolveLegacyStateDirs, resolveOAuthDir, resolveStateDir } from "../../config/paths.js";
|
||||
import {
|
||||
resolveLegacyStateDirs,
|
||||
resolveNewStateDir,
|
||||
resolveOAuthDir,
|
||||
resolveStateDir,
|
||||
} from "../../config/paths.js";
|
||||
import type { ConfigFileSnapshot } from "../../config/types.js";
|
||||
import { resolveRequiredHomeDir } from "../../infra/home-dir.js";
|
||||
import { ExitError, type RuntimeEnv } from "../../runtime.js";
|
||||
@@ -98,6 +103,23 @@ function hasBundledChannelLegacyStateMigrationInputs(stateDir: string, oauthDir:
|
||||
return dirHasFile(oauthDir, isLegacyWhatsAppAuthFile);
|
||||
}
|
||||
|
||||
function hasCrossStateDirApprovalMigrationInputs(stateDir: string): boolean {
|
||||
if (!process.env.OPENCLAW_STATE_DIR?.trim()) {
|
||||
return false;
|
||||
}
|
||||
const homeDir = resolveRequiredHomeDir(process.env, os.homedir);
|
||||
const defaultStateDir = resolveNewStateDir(() => homeDir);
|
||||
if (path.resolve(defaultStateDir) === path.resolve(stateDir)) {
|
||||
return false;
|
||||
}
|
||||
const execApprovalsSource = path.join(defaultStateDir, "exec-approvals.json");
|
||||
const execApprovalsTarget = path.join(stateDir, "exec-approvals.json");
|
||||
return (
|
||||
(fileOrDirExists(execApprovalsSource) && !fileOrDirExists(execApprovalsTarget)) ||
|
||||
fileOrDirExists(path.join(defaultStateDir, "plugin-binding-approvals.json"))
|
||||
);
|
||||
}
|
||||
|
||||
function hasPendingSqliteSidecarArchive(sourcePath: string): boolean {
|
||||
return (
|
||||
fileOrDirExists(`${sourcePath}.migrated`) &&
|
||||
@@ -133,7 +155,8 @@ function hasLegacyStateMigrationInputs(): boolean {
|
||||
sqliteSidecarPaths.some(
|
||||
(sourcePath) => fileOrDirExists(sourcePath) || hasPendingSqliteSidecarArchive(sourcePath),
|
||||
) ||
|
||||
hasBundledChannelLegacyStateMigrationInputs(stateDir, oauthDir)
|
||||
hasBundledChannelLegacyStateMigrationInputs(stateDir, oauthDir) ||
|
||||
hasCrossStateDirApprovalMigrationInputs(stateDir)
|
||||
);
|
||||
}
|
||||
|
||||
@@ -209,6 +232,7 @@ export async function ensureConfigReady(params: {
|
||||
migrateLegacyConfig: false,
|
||||
invalidConfigNote: false,
|
||||
...(commandName === "status" ? { observe: false } : {}),
|
||||
crossStateDirImports: false,
|
||||
...(shouldRequireStartupMigrationCheckpoint(commandPath)
|
||||
? { requireStartupMigrationCheckpoint: true }
|
||||
: {}),
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// Register maintenance tests cover maintenance command registration in the CLI program.
|
||||
import { Command } from "commander";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV } from "../../commands/doctor-invocation.js";
|
||||
import { registerMaintenanceCommands } from "./register.maintenance.js";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
@@ -68,6 +69,10 @@ describe("registerMaintenanceCommands doctor action", () => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
it("exits with code 0 after successful doctor run", async () => {
|
||||
doctorCommand.mockResolvedValue(undefined);
|
||||
|
||||
@@ -101,6 +106,28 @@ describe("registerMaintenanceCommands doctor action", () => {
|
||||
const [runtimeArg, options] = commandCall(doctorCommand);
|
||||
expect(runtimeArg).toBe(runtime);
|
||||
expect(options.repair).toBe(true);
|
||||
expect(options.crossStateDirImports).toBe(true);
|
||||
});
|
||||
|
||||
it("denies cross-state imports when an automation parent disables them", async () => {
|
||||
doctorCommand.mockResolvedValue(undefined);
|
||||
vi.stubEnv(DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV, "1");
|
||||
|
||||
await runMaintenanceCli(["doctor", "--fix", "--non-interactive"]);
|
||||
|
||||
const [, options] = commandCall(doctorCommand);
|
||||
expect(options.repair).toBe(true);
|
||||
expect(options.crossStateDirImports).toBe(false);
|
||||
});
|
||||
|
||||
it("denies cross-state imports for older update parents", async () => {
|
||||
doctorCommand.mockResolvedValue(undefined);
|
||||
vi.stubEnv("OPENCLAW_UPDATE_IN_PROGRESS", "1");
|
||||
|
||||
await runMaintenanceCli(["doctor", "--fix", "--non-interactive"]);
|
||||
|
||||
const [, options] = commandCall(doctorCommand);
|
||||
expect(options.crossStateDirImports).toBe(false);
|
||||
});
|
||||
|
||||
it("runs doctor lint mode without invoking repair doctor", async () => {
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
import type { Command } from "commander";
|
||||
import { formatDocsLink } from "../../../packages/terminal-core/src/links.js";
|
||||
import { theme } from "../../../packages/terminal-core/src/theme.js";
|
||||
import { resolveDoctorCrossStateDirImports } from "../../commands/doctor-invocation.js";
|
||||
import { defaultRuntime } from "../../runtime.js";
|
||||
import { runCommandWithRuntime } from "../cli-utils.js";
|
||||
|
||||
@@ -96,6 +97,7 @@ export function registerMaintenanceCommands(program: Command) {
|
||||
deep: Boolean(opts.deep),
|
||||
postUpgrade: Boolean(opts.postUpgrade),
|
||||
json: Boolean(opts.json),
|
||||
crossStateDirImports: resolveDoctorCrossStateDirImports(),
|
||||
});
|
||||
defaultRuntime.exit(0);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user