fix: fail fast when both state dirs exist
This commit is contained in:
parent
109ac1c549
commit
b5209e8ac1
@ -10,6 +10,8 @@ import { ensureMoltbotCliOnPath } from "../infra/path-env.js";
|
||||
import { assertSupportedRuntime } from "../infra/runtime-guard.js";
|
||||
import { formatUncaughtError } from "../infra/errors.js";
|
||||
import { installUnhandledRejectionHandler } from "../infra/unhandled-rejections.js";
|
||||
import { formatStateDirConflictMessage, StateDirConflictError } from "../infra/state-migrations.js";
|
||||
import { detectStateDirConflict } from "../config/paths.js";
|
||||
import { enableConsoleCapture } from "../logging.js";
|
||||
import { getPrimaryCommand, hasHelpOrVersion } from "./argv.js";
|
||||
import { tryRouteCli } from "./route.js";
|
||||
@ -32,6 +34,16 @@ export async function runCli(argv: string[] = process.argv) {
|
||||
// Enforce the minimum supported runtime before doing any work.
|
||||
assertSupportedRuntime();
|
||||
|
||||
// Check for state directory conflict (split-brain condition).
|
||||
// This must happen early to prevent inconsistent state across the system.
|
||||
const stateDirConflict = detectStateDirConflict();
|
||||
if (stateDirConflict.hasConflict) {
|
||||
console.error(
|
||||
formatStateDirConflictMessage(stateDirConflict.legacyDir, stateDirConflict.newDir),
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
if (await tryRouteCli(normalizedArgv)) return;
|
||||
|
||||
// Capture all console output into structured logs while keeping stdout/stderr behavior.
|
||||
|
||||
@ -14,7 +14,7 @@ import { formatCliCommand } from "../cli/command-format.js";
|
||||
import { note } from "../terminal/note.js";
|
||||
import { normalizeLegacyConfigValues } from "./doctor-legacy-config.js";
|
||||
import type { DoctorOptions } from "./doctor-prompter.js";
|
||||
import { autoMigrateLegacyStateDir } from "./doctor-state-migrations.js";
|
||||
import { autoMigrateLegacyStateDir, StateDirConflictError } from "./doctor-state-migrations.js";
|
||||
|
||||
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||
return Boolean(value && typeof value === "object" && !Array.isArray(value));
|
||||
@ -214,7 +214,10 @@ export async function loadAndMaybeMigrateDoctorConfig(params: {
|
||||
}
|
||||
}
|
||||
|
||||
const autoEnable = applyPluginAutoEnable({ config: candidate, env: process.env });
|
||||
const autoEnable = applyPluginAutoEnable({
|
||||
config: candidate,
|
||||
env: process.env,
|
||||
});
|
||||
if (autoEnable.changes.length > 0) {
|
||||
note(autoEnable.changes.join("\n"), "Doctor changes");
|
||||
candidate = autoEnable.config;
|
||||
|
||||
@ -12,6 +12,7 @@ import {
|
||||
resetAutoMigrateLegacyStateDirForTest,
|
||||
resetAutoMigrateLegacyStateForTest,
|
||||
runLegacyStateMigrations,
|
||||
StateDirConflictError,
|
||||
} from "./doctor-state-migrations.js";
|
||||
|
||||
let tempRoot: string | null = null;
|
||||
@ -346,20 +347,19 @@ describe("doctor legacy state migrations", () => {
|
||||
expect(result.migrated).toBe(true);
|
||||
});
|
||||
|
||||
it("skips state dir migration when target exists", async () => {
|
||||
it("throws StateDirConflictError when both state dirs exist", async () => {
|
||||
const root = await makeTempRoot();
|
||||
const legacyDir = path.join(root, ".clawdbot");
|
||||
const targetDir = path.join(root, ".moltbot");
|
||||
fs.mkdirSync(legacyDir, { recursive: true });
|
||||
fs.mkdirSync(targetDir, { recursive: true });
|
||||
|
||||
const result = await autoMigrateLegacyStateDir({
|
||||
env: {} as NodeJS.ProcessEnv,
|
||||
homedir: () => root,
|
||||
});
|
||||
|
||||
expect(result.migrated).toBe(false);
|
||||
expect(result.warnings.length).toBeGreaterThan(0);
|
||||
await expect(
|
||||
autoMigrateLegacyStateDir({
|
||||
env: {} as NodeJS.ProcessEnv,
|
||||
homedir: () => root,
|
||||
}),
|
||||
).rejects.toThrow(StateDirConflictError);
|
||||
});
|
||||
|
||||
it("skips state dir migration when env override is set", async () => {
|
||||
|
||||
@ -4,9 +4,11 @@ export {
|
||||
autoMigrateLegacyAgentDir,
|
||||
autoMigrateLegacyState,
|
||||
detectLegacyStateMigrations,
|
||||
formatStateDirConflictMessage,
|
||||
migrateLegacyAgentDir,
|
||||
resetAutoMigrateLegacyStateDirForTest,
|
||||
resetAutoMigrateLegacyAgentDirForTest,
|
||||
resetAutoMigrateLegacyStateForTest,
|
||||
runLegacyStateMigrations,
|
||||
StateDirConflictError,
|
||||
} from "../infra/state-migrations.js";
|
||||
|
||||
@ -4,6 +4,7 @@ import path from "node:path";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
import {
|
||||
detectStateDirConflict,
|
||||
resolveDefaultConfigCandidates,
|
||||
resolveConfigPath,
|
||||
resolveOAuthDir,
|
||||
@ -140,3 +141,93 @@ describe("state + config path candidates", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("detectStateDirConflict", () => {
|
||||
it("returns hasConflict: true when both legacy and new dirs exist", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "moltbot-conflict-"));
|
||||
try {
|
||||
const legacyDir = path.join(root, ".clawdbot");
|
||||
const newDir = path.join(root, ".moltbot");
|
||||
await fs.mkdir(legacyDir, { recursive: true });
|
||||
await fs.mkdir(newDir, { recursive: true });
|
||||
|
||||
const result = detectStateDirConflict({} as NodeJS.ProcessEnv, () => root);
|
||||
expect(result.hasConflict).toBe(true);
|
||||
if (result.hasConflict) {
|
||||
expect(result.legacyDir).toBe(legacyDir);
|
||||
expect(result.newDir).toBe(newDir);
|
||||
}
|
||||
} finally {
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("returns hasConflict: false when only legacy dir exists", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "moltbot-conflict-"));
|
||||
try {
|
||||
const legacyDir = path.join(root, ".clawdbot");
|
||||
await fs.mkdir(legacyDir, { recursive: true });
|
||||
|
||||
const result = detectStateDirConflict({} as NodeJS.ProcessEnv, () => root);
|
||||
expect(result.hasConflict).toBe(false);
|
||||
} finally {
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("returns hasConflict: false when only new dir exists", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "moltbot-conflict-"));
|
||||
try {
|
||||
const newDir = path.join(root, ".moltbot");
|
||||
await fs.mkdir(newDir, { recursive: true });
|
||||
|
||||
const result = detectStateDirConflict({} as NodeJS.ProcessEnv, () => root);
|
||||
expect(result.hasConflict).toBe(false);
|
||||
} finally {
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("returns hasConflict: false when neither dir exists", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "moltbot-conflict-"));
|
||||
try {
|
||||
const result = detectStateDirConflict({} as NodeJS.ProcessEnv, () => root);
|
||||
expect(result.hasConflict).toBe(false);
|
||||
} finally {
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("returns hasConflict: false when env override is set", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "moltbot-conflict-"));
|
||||
try {
|
||||
const legacyDir = path.join(root, ".clawdbot");
|
||||
const newDir = path.join(root, ".moltbot");
|
||||
await fs.mkdir(legacyDir, { recursive: true });
|
||||
await fs.mkdir(newDir, { recursive: true });
|
||||
|
||||
const env = { MOLTBOT_STATE_DIR: "/custom/path" } as NodeJS.ProcessEnv;
|
||||
const result = detectStateDirConflict(env, () => root);
|
||||
expect(result.hasConflict).toBe(false);
|
||||
} finally {
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("returns hasConflict: false when legacy is symlink to new", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "moltbot-conflict-"));
|
||||
try {
|
||||
const legacyDir = path.join(root, ".clawdbot");
|
||||
const newDir = path.join(root, ".moltbot");
|
||||
await fs.mkdir(newDir, { recursive: true });
|
||||
// On Windows, use 'junction' which doesn't require admin privileges
|
||||
const symlinkType = process.platform === "win32" ? "junction" : "dir";
|
||||
await fs.symlink(newDir, legacyDir, symlinkType);
|
||||
|
||||
const result = detectStateDirConflict({} as NodeJS.ProcessEnv, () => root);
|
||||
expect(result.hasConflict).toBe(false);
|
||||
} finally {
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@ -37,6 +37,60 @@ export function resolveNewStateDir(homedir: () => string = os.homedir): string {
|
||||
return newStateDir(homedir);
|
||||
}
|
||||
|
||||
export type StateDirConflict =
|
||||
| {
|
||||
hasConflict: true;
|
||||
legacyDir: string;
|
||||
newDir: string;
|
||||
}
|
||||
| {
|
||||
hasConflict: false;
|
||||
};
|
||||
|
||||
/**
|
||||
* Detect if both legacy (~/.clawdbot) and new (~/.moltbot) state directories exist.
|
||||
* This creates a split-brain condition where different parts of the system may
|
||||
* read/write to different directories.
|
||||
*
|
||||
* Returns conflict info if both directories exist and no explicit override is set.
|
||||
*/
|
||||
export function detectStateDirConflict(
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
homedir: () => string = os.homedir,
|
||||
): StateDirConflict {
|
||||
// If explicit override is set, no conflict possible
|
||||
const override = env.MOLTBOT_STATE_DIR?.trim() || env.CLAWDBOT_STATE_DIR?.trim();
|
||||
if (override) return { hasConflict: false };
|
||||
|
||||
const legacyDir = legacyStateDir(homedir);
|
||||
const newDir = newStateDir(homedir);
|
||||
|
||||
// Check if legacy is a symlink to new (already migrated)
|
||||
try {
|
||||
const legacyStat = fs.lstatSync(legacyDir);
|
||||
if (legacyStat.isSymbolicLink()) {
|
||||
const target = fs.readlinkSync(legacyDir);
|
||||
const resolvedTarget = path.resolve(path.dirname(legacyDir), target);
|
||||
if (path.resolve(resolvedTarget) === path.resolve(newDir)) {
|
||||
return { hasConflict: false };
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Legacy dir doesn't exist, no conflict
|
||||
return { hasConflict: false };
|
||||
}
|
||||
|
||||
// Check if both directories exist
|
||||
const hasLegacy = fs.existsSync(legacyDir);
|
||||
const hasNew = fs.existsSync(newDir);
|
||||
|
||||
if (hasLegacy && hasNew) {
|
||||
return { hasConflict: true, legacyDir, newDir };
|
||||
}
|
||||
|
||||
return { hasConflict: false };
|
||||
}
|
||||
|
||||
/**
|
||||
* State directory for mutable data (sessions, logs, caches).
|
||||
* Can be overridden via MOLTBOT_STATE_DIR (preferred) or CLAWDBOT_STATE_DIR (legacy).
|
||||
|
||||
@ -156,7 +156,11 @@ function normalizeSessionEntry(entry: SessionEntryLike): SessionEntry | null {
|
||||
typeof entry.updatedAt === "number" && Number.isFinite(entry.updatedAt)
|
||||
? entry.updatedAt
|
||||
: Date.now();
|
||||
const normalized = { ...(entry as unknown as SessionEntry), sessionId, updatedAt };
|
||||
const normalized = {
|
||||
...(entry as unknown as SessionEntry),
|
||||
sessionId,
|
||||
updatedAt,
|
||||
};
|
||||
const rec = normalized as unknown as Record<string, unknown>;
|
||||
if (typeof rec.groupChannel !== "string" && typeof rec.room === "string") {
|
||||
rec.groupChannel = rec.room;
|
||||
@ -207,7 +211,10 @@ function canonicalizeSessionStore(params: {
|
||||
const existing = canonical[canonicalKey];
|
||||
if (!existing) {
|
||||
canonical[canonicalKey] = entry;
|
||||
meta.set(canonicalKey, { isCanonical, updatedAt: resolveUpdatedAt(entry) });
|
||||
meta.set(canonicalKey, {
|
||||
isCanonical,
|
||||
updatedAt: resolveUpdatedAt(entry),
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
@ -284,6 +291,49 @@ type StateDirMigrationResult = {
|
||||
warnings: string[];
|
||||
};
|
||||
|
||||
/**
|
||||
* Error thrown when both legacy (~/.clawdbot) and new (~/.moltbot) state directories
|
||||
* exist simultaneously, creating a split-brain condition where different parts of
|
||||
* the system may read/write to different directories.
|
||||
*/
|
||||
export class StateDirConflictError extends Error {
|
||||
constructor(
|
||||
message: string,
|
||||
public readonly legacyDir: string,
|
||||
public readonly targetDir: string,
|
||||
) {
|
||||
super(message);
|
||||
this.name = "StateDirConflictError";
|
||||
}
|
||||
}
|
||||
|
||||
export function formatStateDirConflictMessage(legacyDir: string, targetDir: string): string {
|
||||
return [
|
||||
`State directory conflict: both legacy and new directories exist.`,
|
||||
``,
|
||||
` Legacy: ${legacyDir}`,
|
||||
` New: ${targetDir}`,
|
||||
``,
|
||||
`This creates a split-brain condition where different parts of the system`,
|
||||
`may read/write to different directories, causing pairing and auth failures.`,
|
||||
``,
|
||||
`To fix, choose ONE of these options:`,
|
||||
``,
|
||||
` 1. Keep the legacy directory (recommended if you have existing data):`,
|
||||
` Remove or rename ${targetDir}`,
|
||||
``,
|
||||
` 2. Use the new directory:`,
|
||||
` Remove or rename ${legacyDir}`,
|
||||
``,
|
||||
` 3. Merge manually, then remove the legacy directory`,
|
||||
``,
|
||||
` 4. Set an explicit state directory:`,
|
||||
` export MOLTBOT_STATE_DIR="${targetDir}"`,
|
||||
``,
|
||||
`After resolving, restart Moltbot.`,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
function resolveSymlinkTarget(linkPath: string): string | null {
|
||||
try {
|
||||
const target = fs.readlinkSync(linkPath);
|
||||
@ -352,10 +402,13 @@ export async function autoMigrateLegacyStateDir(params: {
|
||||
}
|
||||
|
||||
if (isDirPath(targetDir)) {
|
||||
warnings.push(
|
||||
`State dir migration skipped: target already exists (${targetDir}). Remove or merge manually.`,
|
||||
// Both directories exist - this creates a split-brain condition.
|
||||
// Refuse to continue to prevent inconsistent state across the system.
|
||||
throw new StateDirConflictError(
|
||||
formatStateDirConflictMessage(legacyDir, targetDir),
|
||||
legacyDir,
|
||||
targetDir,
|
||||
);
|
||||
return { migrated: false, skipped: false, changes, warnings };
|
||||
}
|
||||
|
||||
try {
|
||||
@ -520,7 +573,9 @@ async function migrateLegacySessions(
|
||||
scope: detected.targetScope,
|
||||
});
|
||||
|
||||
const merged: Record<string, SessionEntryLike> = { ...canonicalizedTarget.store };
|
||||
const merged: Record<string, SessionEntryLike> = {
|
||||
...canonicalizedTarget.store,
|
||||
};
|
||||
for (const [key, entry] of Object.entries(canonicalizedLegacy.store)) {
|
||||
merged[key] = mergeSessionEntry({
|
||||
existing: merged[key],
|
||||
|
||||
Loading…
Reference in New Issue
Block a user