From b5209e8ac15db922d5ee15ccb598cc5a63f194ff Mon Sep 17 00:00:00 2001 From: Aditya Rao Date: Thu, 29 Jan 2026 02:04:09 +0530 Subject: [PATCH 1/2] fix: fail fast when both state dirs exist --- src/cli/run-main.ts | 12 +++ src/commands/doctor-config-flow.ts | 7 +- src/commands/doctor-state-migrations.test.ts | 16 ++-- src/commands/doctor-state-migrations.ts | 2 + src/config/paths.test.ts | 91 ++++++++++++++++++++ src/config/paths.ts | 54 ++++++++++++ src/infra/state-migrations.ts | 67 ++++++++++++-- 7 files changed, 233 insertions(+), 16 deletions(-) diff --git a/src/cli/run-main.ts b/src/cli/run-main.ts index c3a9ae466..1e4b92e0c 100644 --- a/src/cli/run-main.ts +++ b/src/cli/run-main.ts @@ -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. diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index fda4673d9..ec8e91767 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -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 { 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; diff --git a/src/commands/doctor-state-migrations.test.ts b/src/commands/doctor-state-migrations.test.ts index 2ae7faf05..b1ae6fdc6 100644 --- a/src/commands/doctor-state-migrations.test.ts +++ b/src/commands/doctor-state-migrations.test.ts @@ -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 () => { diff --git a/src/commands/doctor-state-migrations.ts b/src/commands/doctor-state-migrations.ts index 50c59a3a0..a8571188d 100644 --- a/src/commands/doctor-state-migrations.ts +++ b/src/commands/doctor-state-migrations.ts @@ -4,9 +4,11 @@ export { autoMigrateLegacyAgentDir, autoMigrateLegacyState, detectLegacyStateMigrations, + formatStateDirConflictMessage, migrateLegacyAgentDir, resetAutoMigrateLegacyStateDirForTest, resetAutoMigrateLegacyAgentDirForTest, resetAutoMigrateLegacyStateForTest, runLegacyStateMigrations, + StateDirConflictError, } from "../infra/state-migrations.js"; diff --git a/src/config/paths.test.ts b/src/config/paths.test.ts index 806d29f92..c9bfbe604 100644 --- a/src/config/paths.test.ts +++ b/src/config/paths.test.ts @@ -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 }); + } + }); +}); diff --git a/src/config/paths.ts b/src/config/paths.ts index f6e451596..353b2d8bb 100644 --- a/src/config/paths.ts +++ b/src/config/paths.ts @@ -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). diff --git a/src/infra/state-migrations.ts b/src/infra/state-migrations.ts index f5e50740e..aa8254826 100644 --- a/src/infra/state-migrations.ts +++ b/src/infra/state-migrations.ts @@ -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; 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 = { ...canonicalizedTarget.store }; + const merged: Record = { + ...canonicalizedTarget.store, + }; for (const [key, entry] of Object.entries(canonicalizedLegacy.store)) { merged[key] = mergeSessionEntry({ existing: merged[key], From 275db8ef8a37fbfd6af89af83134d15ebb97cef6 Mon Sep 17 00:00:00 2001 From: Aditya Rao Date: Thu, 29 Jan 2026 02:24:09 +0530 Subject: [PATCH 2/2] fix: remove unused imports --- src/cli/run-main.ts | 2 +- src/commands/doctor-config-flow.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cli/run-main.ts b/src/cli/run-main.ts index 1e4b92e0c..5c878c876 100644 --- a/src/cli/run-main.ts +++ b/src/cli/run-main.ts @@ -10,7 +10,7 @@ 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 { formatStateDirConflictMessage } from "../infra/state-migrations.js"; import { detectStateDirConflict } from "../config/paths.js"; import { enableConsoleCapture } from "../logging.js"; import { getPrimaryCommand, hasHelpOrVersion } from "./argv.js"; diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index ec8e91767..058a18ce3 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -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, StateDirConflictError } from "./doctor-state-migrations.js"; +import { autoMigrateLegacyStateDir } from "./doctor-state-migrations.js"; function isRecord(value: unknown): value is Record { return Boolean(value && typeof value === "object" && !Array.isArray(value));