fix(setup): preserve concurrent client configuration edits (#4855)

* fix(setup): preserve concurrent client configuration edits

* fix(setup): normalize configuration transaction queue paths

* fix(setup): report concurrent snapshot read conflicts

* test(setup): isolate Pi workflows from installed Hopper

* test: align inherited fixtures with current analysis contracts

* test(ida): assert search recovery instead of diagnostic prose

* fix(setup): share configuration queue keys across directory aliases

Resolve parent-directory aliases for regular and missing configuration
paths so a directory symlink and its real directory serialize setup
instead of racing the same file.

Co-authored-by: morluto <morluto@users.noreply.github.com>

---------

Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: morluto <morluto@users.noreply.github.com>

docs/installation.md

@@ -135,6 +135,15 @@ exits 1. Installing missing Hopper non-interactively additionally requires
 rea setup --yes --all-detected --install-hopper --json
 ```
 
+Client configuration writes detect changes to the file read by setup, including
+when a matching recovery backup already exists. A detected conflict reports
+`reason: "conflict"` and preserves the newer file; review it and rerun setup.
+Rollback likewise requires the file identity and content installed by that
+transaction. Requests for the same resolved path are serialized within one REA
+process. These checks do not provide an operating-system compare-and-swap or
+coordinate arbitrary external writers, so avoid editing client configuration
+while setup is applying it.
+
 ### Skill destinations
 
 The bundled skill is installed in each selected client's personal skill directory.

src/application/ClientConfigPath.ts

@@ -1,4 +1,5 @@
 import { lstat, realpath } from "node:fs/promises";
+import { basename, dirname, join, resolve } from "node:path";
 
 interface ClientConfigPathStats {
   readonly uid?: number;
@@ -14,21 +15,32 @@ interface ClientConfigPathFileSystem {
 const systemFileSystem: ClientConfigPathFileSystem = { lstat, realpath };
 
 /**
- * Resolve an existing, user-owned config symlink to one stable transaction
- * path. Missing regular paths remain eligible for first-run creation.
+ * Resolve one stable transaction path for a configuration file.
+ * An existing user-owned config symlink resolves to its target file.
+ * Parent-directory aliases resolve for regular and missing files, so a
+ * directory symlink and its real directory share one queue key. Missing
+ * paths stay eligible for first-run creation.
  */
 export const resolveClientConfigTransactionPath = async (
   requestedPath: string,
   fileSystem: ClientConfigPathFileSystem = systemFileSystem,
 ): Promise<string | undefined> => {
-  let requestedStats: ClientConfigPathStats;
+  let requestedStats: ClientConfigPathStats | undefined;
   try {
     requestedStats = await fileSystem.lstat(requestedPath);
   } catch (cause: unknown) {
-    return isMissing(cause) ? requestedPath : undefined;
+    if (!isMissing(cause)) return undefined;
   }
-  if (!requestedStats.isSymbolicLink()) return requestedPath;
+  if (requestedStats?.isSymbolicLink() === true)
+    return resolveOwnedConfigSymlink(requestedPath, requestedStats, fileSystem);
+  return canonicalRegularConfigPath(requestedPath, fileSystem);
+};
 
+const resolveOwnedConfigSymlink = async (
+  requestedPath: string,
+  requestedStats: ClientConfigPathStats,
+  fileSystem: ClientConfigPathFileSystem,
+): Promise<string | undefined> => {
   const currentUid =
     typeof process.getuid === "function" ? process.getuid() : undefined;
   if (
@@ -51,5 +63,33 @@ export const resolveClientConfigTransactionPath = async (
   }
 };
 
+/**
+ * Resolve parent-directory aliases, including when the file or an intermediate
+ * directory is absent. A realpath failure keeps the requested spelling.
+ */
+const canonicalRegularConfigPath = async (
+  requestedPath: string,
+  fileSystem: ClientConfigPathFileSystem,
+): Promise<string> => {
+  if (fileSystem.realpath === undefined) return requestedPath;
+  let ancestor = resolve(requestedPath);
+  const missing: string[] = [];
+  while (true) {
+    try {
+      return join(await fileSystem.realpath(ancestor), ...missing);
+    } catch (cause: unknown) {
+      if (!isAbsentPath(cause) || dirname(ancestor) === ancestor)
+        return requestedPath;
+      missing.unshift(basename(ancestor));
+      ancestor = dirname(ancestor);
+    }
+  }
+};
+
 const isMissing = (cause: unknown): boolean =>
   cause instanceof Error && "code" in cause && cause.code === "ENOENT";
+
+const isAbsentPath = (cause: unknown): boolean =>
+  cause instanceof Error &&
+  "code" in cause &&
+  (cause.code === "ENOENT" || cause.code === "ENOTDIR");

src/application/ClientConfigurationPublication.ts

@@ -8,7 +8,7 @@ import {
   rename,
   type FileHandle,
 } from "node:fs/promises";
-import { basename, dirname, join, sep } from "node:path";
+import { basename, dirname, join, resolve, sep } from "node:path";
 
 import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js";
 import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js";
@@ -20,6 +20,14 @@ import {
 import type { RegularFileOperationOutcome } from "../filesystem/OwnedRegularFile.js";
 import { readRegularFileTextSnapshot } from "./RegularFileRead.js";
 import { ClientConfigurationPublicationFailure } from "./ClientConfigurationCleanup.js";
+import { resolveClientConfigTransactionPath } from "./ClientConfigPath.js";
+import { ConfigurationError } from "../domain/configurationErrors.js";
+
+/** The original read owner's selection, before merging and preparing a backup. */
+export interface ClientConfigurationSelection {
+  readonly requestedPath: string;
+  readonly state: Stats | undefined;
+}
 
 /** The exact staged file retained through publication, verification and recovery. */
 export class ClientConfigurationPublication {
@@ -36,10 +44,20 @@ export class ClientConfigurationPublication {
     contents: string,
     previous: string | undefined,
     mode?: number,
+    selection?: ClientConfigurationSelection,
   ): Promise<ClientConfigurationPublication> {
     const parent = await realpath(dirname(path));
     const destination = join(parent, basename(path));
     const input = await selectedConfiguration(destination, previous);
+    if (selection !== undefined) {
+      await assertRequestedConfigurationPath(path, selection.requestedPath);
+      if (
+        selection.state === undefined
+          ? input !== undefined
+          : input === undefined || !sameRegularFileState(selection.state, input)
+      )
+        throw new RegularFileChangedError(destination);
+    }
     const directory = await mkdtemp(`${parent}${sep}.rea-config-`);
     const staging = await OwnedStagedOutput.create(directory);
     const cleanup = new AnalysisCleanupLease(
@@ -72,10 +90,23 @@ export class ClientConfigurationPublication {
       if ((await owner.handle.stat()).ino === 0)
         throw new RegularFileChangedError(stagedPath);
       await staging.assertReadyForPublication(stagedPath);
+      if (selection !== undefined)
+        await assertRequestedConfigurationPath(path, selection.requestedPath);
       await assertSelectedConfiguration(destination, input);
       if (input === undefined) {
         // An exclusive link cannot overwrite a file created after absence was checked.
-        await link(stagedPath, destination);
+        try {
+          await link(stagedPath, destination);
+        } catch (cause: unknown) {
+          if (
+            selection !== undefined &&
+            cause instanceof Error &&
+            "code" in cause &&
+            cause.code === "EEXIST"
+          )
+            throw new RegularFileChangedError(destination, { cause });
+          throw cause;
+        }
       } else {
         // The destination is explicit: rename never follows its final symlink.
         await rename(stagedPath, destination);
@@ -107,11 +138,12 @@ export class ClientConfigurationPublication {
     return outcome.value;
   }
 
-  async assertCurrent(): Promise<void> {
+  async assertCurrent(readback?: string): Promise<void> {
     const owned = await this.owner.handle.stat();
     const named = await lstat(this.path);
     if (
       owned.ino === 0 ||
+      (readback !== undefined && readback !== this.contents) ||
       named.isSymbolicLink() ||
       !sameRegularFileState(owned, named)
     )
@@ -120,10 +152,6 @@ export class ClientConfigurationPublication {
 
   /** State checks do not exclude a concurrent writer to the same inode. */
   async restore(original: string | undefined): Promise<void> {
-    if (original === undefined)
-      throw new Error(
-        "First-run configuration recovery requires manual removal; pathname deletion cannot prove ownership",
-      );
     const handle = this.owner.handle;
     const before = await handle.stat();
     const named = await lstat(this.path);
@@ -137,6 +165,10 @@ export class ClientConfigurationPublication {
       !sameRegularFileState(before, await lstat(this.path))
     )
       throw new RegularFileChangedError(this.path);
+    if (original === undefined)
+      throw new Error(
+        "First-run configuration recovery requires manual removal; pathname deletion cannot prove ownership",
+      );
     const bytes = Buffer.from(original, "utf8");
     for (let position = 0; position < bytes.length;) {
       const { bytesWritten } = await handle.write(
@@ -178,6 +210,53 @@ export class ClientConfigurationPublication {
   }
 }
 
+const assertRequestedConfigurationPath = async (
+  path: string,
+  requestedPath: string,
+): Promise<void> => {
+  const current = await resolveClientConfigTransactionPath(requestedPath);
+  if (current === undefined)
+    throw new ConfigurationError(
+      `Client configuration path could not be safely verified: ${requestedPath}`,
+    );
+  if (resolve(current) === resolve(path)) return;
+  // `path` is the canonical file selected for this transaction. A later
+  // retarget of a parent-directory alias keeps that file. A configuration
+  // symlink that now names another target, or a path whose parent is already
+  // canonical, still conflicts.
+  if (await isParentAliasRetarget(requestedPath, path, current)) return;
+  throw new RegularFileChangedError(path);
+};
+
+const isParentAliasRetarget = async (
+  requestedPath: string,
+  path: string,
+  current: string,
+): Promise<boolean> => {
+  if (basename(current) !== basename(path)) return false;
+  let requestedStats: { isSymbolicLink(): boolean };
+  try {
+    requestedStats = await lstat(requestedPath);
+  } catch (cause: unknown) {
+    if (isNodeErrorCode(cause, "ENOENT")) return false;
+    throw new ConfigurationError(
+      `Client configuration path could not be safely verified: ${requestedPath}`,
+    );
+  }
+  if (requestedStats.isSymbolicLink()) return false;
+  const requestedParent = dirname(resolve(requestedPath));
+  try {
+    const canonicalParent = await realpath(requestedParent);
+    return resolve(canonicalParent) !== requestedParent;
+  } catch (cause: unknown) {
+    void cause;
+    return false;
+  }
+};
+
+const isNodeErrorCode = (cause: unknown, code: string): boolean =>
+  cause instanceof Error && "code" in cause && cause.code === code;
+
 const selectedConfiguration = async (
   path: string,
   previous: string | undefined,

src/application/ClientConfigurationTransaction.ts

@@ -0,0 +1,27 @@
+import { resolve } from "node:path";
+
+const activeTransactions = new Map<string, Promise<void>>();
+
+/**
+ * Serialize the full read/merge/publish operation for one resolved
+ * configuration path.
+ */
+export const withClientConfigurationTransaction = async <Value>(
+  path: string,
+  operation: () => Promise<Value>,
+): Promise<Value> => {
+  const key = resolve(path);
+  const previous = activeTransactions.get(key);
+  let release!: () => void;
+  const current = new Promise<void>((complete) => {
+    release = complete;
+  });
+  activeTransactions.set(key, current);
+  await previous;
+  try {
+    return await operation();
+  } finally {
+    release();
+    if (activeTransactions.get(key) === current) activeTransactions.delete(key);
+  }
+};

src/application/SetupClientConfiguration.ts

@@ -23,6 +23,8 @@ import {
 import { dshDocument, DshCordisDocument } from "./DshCordisPatch.js";
 import { mkdir } from "node:fs/promises";
 import { dirname } from "node:path";
+import { RegularFileChangedError } from "../filesystem/RegularFile.js";
+import { withClientConfigurationTransaction } from "./ClientConfigurationTransaction.js";
 import { isClientConfigurationCleanupFailure } from "./ClientConfigurationCleanup.js";
 import { ClientConfigurationPublication } from "./ClientConfigurationPublication.js";
 import {
@@ -42,7 +44,11 @@ import {
 import { resolveClientConfigTransactionPath } from "./ClientConfigPath.js";
 import { packageReadTargetChanged } from "./MsixPackageStorage.js";
 import { RegularFileCleanupFailure } from "../filesystem/OwnedRegularFile.js";
-import { readRegularFileText } from "./RegularFileRead.js";
+import {
+  readRegularFileText,
+  readRegularFileTextSnapshot,
+  InvalidUtf8FileError,
+} from "./RegularFileRead.js";
 import {
   preserveCurrentRecoveryBackup,
   selectCurrentRecoveryBackup,
@@ -63,7 +69,7 @@ export class ClientConfigurationCleanupRollbackFailure extends RegularFileCleanu
     failure: RegularFileCleanupFailure<string>,
     readonly configurationFailure: Extract<
       ClientConfigurationResult,
-      { readonly status: "failed"; readonly reason: "rollback" }
+      { readonly status: "failed" }
     >,
   ) {
     super(
@@ -71,8 +77,8 @@ export class ClientConfigurationCleanupRollbackFailure extends RegularFileCleanu
       failure.owner,
       failure.outcome,
       new AggregateError(
-        [failure.cleanupCause, new Error(configurationFailure.failure.detail)],
-        `Configuration readback cleanup and rollback failed: ${configurationFailure.failure.path}`,
+        [failure.cleanupCause, configurationFailure],
+        `Configuration readback cleanup and rollback failed: ${failure.path}`,
       ),
       failure.retention,
       failure.completionCancellation,
@@ -81,24 +87,34 @@ export class ClientConfigurationCleanupRollbackFailure extends RegularFileCleanu
 }
 
 /** Configure one supported client's native stdio MCP registration shape. */
-export const configureClientConfiguration = (
+export const configureClientConfiguration = async (
   client: SetupClient,
   environment: SetupProviderEnvironment = {},
   command: readonly string[] = npxRegistrationCommand(),
   resolveProviderRequestBudget?: ProviderRequestBudgetResolver,
 ): Promise<ClientConfigurationResult> => {
   if (client.configPathError !== undefined)
-    return Promise.resolve({ status: "failed", reason: "path" });
-  return configureClientDocument(
-    client,
-    environment,
-    command,
-    resolveProviderRequestBudget,
+    return { status: "failed", reason: "path" };
+  const transactionPath = await resolveClientConfigTransactionPath(
+    client.configPath,
+  );
+  if (transactionPath === undefined)
+    return { status: "failed", reason: "path" };
+  return withClientConfigurationTransaction(transactionPath, () =>
+    configureClientDocument(
+      { client, transactionPath },
+      environment,
+      command,
+      resolveProviderRequestBudget,
+    ),
   );
 };
 
 const configureClientDocument = async (
-  client: SetupClient,
+  {
+    client,
+    transactionPath,
+  }: { readonly client: SetupClient; readonly transactionPath: string },
   environment: SetupProviderEnvironment,
   command: readonly string[],
   resolveProviderRequestBudget?: ProviderRequestBudgetResolver,
@@ -138,18 +154,18 @@ const configureClientDocument = async (
   }
   if (policy.value !== undefined)
     return { status: "failed", reason: "readback" };
-  const transactionPath = await resolveClientConfigTransactionPath(
-    client.configPath,
-  );
-  if (transactionPath === undefined)
-    return { status: "failed", reason: "path" };
-  let original: string | undefined;
+  let snapshot:
+    | Awaited<ReturnType<typeof readRegularFileTextSnapshot>>
+    | undefined;
   try {
-    original = await readRegularFileText(transactionPath);
+    snapshot = await readRegularFileTextSnapshot(transactionPath, {
+      symlinks: "reject",
+    });
   } catch (cause: unknown) {
     if (cause instanceof RegularFileCleanupFailure) throw cause;
     if (!isMissing(cause)) return { status: "failed", reason: "readback" };
   }
+  const original = snapshot?.text;
   let parsed: ClientConfigurationDocument;
   try {
     parsed = parseClientConfiguration(
@@ -203,6 +219,7 @@ const configureClientDocument = async (
   return persistClientConfiguration({
     client,
     transactionPath,
+    snapshot,
     format,
     original,
     parsed,
@@ -214,6 +231,9 @@ const configureClientDocument = async (
 interface ClientConfigurationUpdate {
   readonly client: SetupClient;
   readonly transactionPath: string;
+  readonly snapshot:
+    | Awaited<ReturnType<typeof readRegularFileTextSnapshot>>
+    | undefined;
   readonly format: NonNullable<SetupClient["format"]>;
   readonly original: string | undefined;
   readonly parsed: ClientConfigurationDocument;
@@ -224,6 +244,7 @@ interface ClientConfigurationUpdate {
 const persistClientConfiguration = async ({
   client,
   transactionPath,
+  snapshot,
   format,
   original,
   parsed,
@@ -328,20 +349,21 @@ const persistClientConfiguration = async ({
       serialized.value,
       original,
       0o600,
+      { requestedPath: client.configPath, state: snapshot?.state },
     );
   } catch (cause: unknown) {
     if (isClientConfigurationCleanupFailure(cause)) throw cause;
+    if (isConfigurationConflict(cause))
+      return conflictFailure(recoveryBackupPath);
     // Write failure is reported by the status reason.
     void cause;
     return { status: "failed", reason: "write" };
   }
   return publication.verify(async () => {
     try {
-      const readback = parseClientConfiguration(
-        await readRegularFileText(publication.path),
-        format,
-      );
-      await publication.assertCurrent();
+      const text = await readRegularFileText(publication.path);
+      await publication.assertCurrent(text);
+      const readback = parseClientConfiguration(text, format);
       if (
         !registrationCurrent(readback, desired) ||
         Object.entries(fallbackServers).some(
@@ -357,17 +379,15 @@ const persistClientConfiguration = async ({
         );
       }
     } catch (cause: unknown) {
+      if (isConfigurationConflict(cause) || isMissing(cause))
+        return conflictFailure(recoveryBackupPath);
       if (cause instanceof RegularFileCleanupFailure) {
         const rollback = await restoreConfig(publication, original);
         if (!rollback.ok)
-          throw new ClientConfigurationCleanupRollbackFailure(cause, {
-            status: "failed",
-            reason: "rollback",
-            ...(recoveryBackupPath === undefined
-              ? {}
-              : { backupPath: recoveryBackupPath }),
-            failure: rollback.error,
-          });
+          throw new ClientConfigurationCleanupRollbackFailure(
+            cause,
+            rollbackFailure(rollback.error, recoveryBackupPath),
+          );
         throw cause;
       }
       // Readback failure restores the transaction before reporting.
@@ -627,19 +647,24 @@ const readZcodeFallbackServers = async (
   }
 };
 
+type ConfigurationRestoreFailure =
+  | { readonly kind: "conflict" }
+  | {
+      readonly kind: "restore";
+      readonly path: string;
+      readonly detail: string;
+    };
+
 const restoreConfig = async (
   publication: ClientConfigurationPublication,
   original: string | undefined,
-): Promise<
-  Result<
-    void,
-    { readonly kind: "restore"; readonly path: string; readonly detail: string }
-  >
-> => {
+): Promise<Result<void, ConfigurationRestoreFailure>> => {
   try {
     await publication.restore(original);
     return ok(undefined);
   } catch (cause: unknown) {
+    if (isConfigurationConflict(cause) || isMissing(cause))
+      return err({ kind: "conflict" });
     return err({
       kind: "restore",
       path: publication.path,
@@ -660,13 +685,33 @@ const rollbackOrReadbackFailure = async (
         reason: "readback",
         ...(backupPath === undefined ? {} : { backupPath }),
       }
+    : rollbackFailure(rollback.error, backupPath);
+};
+
+const rollbackFailure = (
+  failure: ConfigurationRestoreFailure,
+  backupPath?: string,
+): Extract<ClientConfigurationResult, { readonly status: "failed" }> =>
+  failure.kind === "conflict"
+    ? conflictFailure(backupPath)
     : {
         status: "failed",
         reason: "rollback",
+        failure,
         ...(backupPath === undefined ? {} : { backupPath }),
-        failure: rollback.error,
       };
-};
+
+const isConfigurationConflict = (cause: unknown): boolean =>
+  cause instanceof RegularFileChangedError ||
+  cause instanceof InvalidUtf8FileError;
+
+const conflictFailure = (
+  backupPath?: string,
+): Extract<ClientConfigurationResult, { readonly status: "failed" }> => ({
+  status: "failed",
+  reason: "conflict",
+  ...(backupPath === undefined ? {} : { backupPath }),
+});
 
 const errorDetail = (cause: unknown): string =>
   cause instanceof Error ? cause.message : String(cause);

src/application/SetupClients.ts

@@ -10,6 +10,8 @@ export const failedConfigurationMessage = (
   result: Extract<ClientConfigurationResult, { readonly status: "failed" }>,
 ): string => {
   switch (result.reason) {
+    case "conflict":
+      return "Agent configuration changed during setup. Review the current file and rerun setup.";
     case "target_changed":
       return "The client began reading a different configuration file after the plan was shown, so no change was made. Rerun setup to review the new target.";
     case "target_unavailable":

src/application/SetupTypes.ts

@@ -29,6 +29,11 @@ export interface SetupInitialState {
 
 /** Result of one backup/write/readback transaction. */
 export type ClientConfigurationResult =
+  | {
+      readonly status: "failed";
+      readonly reason: "conflict";
+      readonly backupPath?: string;
+    }
   | {
       readonly status: "unchanged" | "configured" | "skipped";
       readonly backupPath?: string;

tests/acceptance/setup/piClientSetup.test.ts

@@ -226,6 +226,7 @@ cliTest(
         environment: {
           HOME: home,
           USERPROFILE: home,
+          HOPPER_LAUNCHER_PATH: join(home, "unconfigured-hopper"),
           PI_CODING_AGENT_DIR: "custom-agent",
           OMP_PROFILE: "work",
           PI_PROFILE: "legacy",
@@ -341,6 +342,7 @@ cliTest(
         environment: {
           HOME: home,
           USERPROFILE: home,
+          HOPPER_LAUNCHER_PATH: join(home, "unconfigured-hopper"),
           PI_CODING_AGENT_DIR: "custom-agent",
         },
         timeoutMs: 20_000,
@@ -449,6 +451,7 @@ cliTest(
         environment: {
           HOME: home,
           USERPROFILE: home,
+          HOPPER_LAUNCHER_PATH: join(home, "unconfigured-hopper"),
           PI_CODING_AGENT_DIR: "file:///agent%2fdir",
         },
         timeoutMs: 20_000,

tests/boundary/filesystem/setupConfig.test.ts

@@ -50,6 +50,38 @@ describe("JSON client configuration transaction: writing configuration through s
     },
   );
 
+  it.skipIf(process.platform === "win32")(
+    "resolves a directory symlink and its real directory to one transaction path",
+    async () => {
+      const root = await createTestTempDirectory("rea-config-dir-alias-");
+      const realDirectory = join(root, "real");
+      const alias = join(root, "alias");
+      await mkdir(realDirectory);
+      await symlink(realDirectory, alias, "dir");
+      const realPath = join(realDirectory, "config.json");
+      const aliasPath = join(alias, "config.json");
+      const nestedRealPath = join(realDirectory, "nested", "config.json");
+      const nestedAliasPath = join(alias, "nested", "config.json");
+
+      expect(await resolveClientConfigTransactionPath(aliasPath)).toBe(
+        realPath,
+      );
+      expect(await resolveClientConfigTransactionPath(realPath)).toBe(realPath);
+      expect(await resolveClientConfigTransactionPath(nestedAliasPath)).toBe(
+        nestedRealPath,
+      );
+      expect(await resolveClientConfigTransactionPath(nestedRealPath)).toBe(
+        nestedRealPath,
+      );
+
+      await writeFile(realPath, "{}\n");
+      expect(await resolveClientConfigTransactionPath(aliasPath)).toBe(
+        realPath,
+      );
+      expect(await resolveClientConfigTransactionPath(realPath)).toBe(realPath);
+    },
+  );
+
   it("preserves existing keys, creates a backup, and reads back the MCP entry", async () => {
     const directory = await createTestTempDirectory("rea-setup-");
     const configPath = join(directory, "mcp.json");

tests/process-global/setupConfigurationConflicts.test.ts

@@ -0,0 +1,454 @@
+import {
+  chmod,
+  lstat,
+  mkdir,
+  readFile,
+  readdir,
+  rename,
+  rm,
+  symlink,
+  writeFile,
+} from "node:fs/promises";
+import { join, relative } from "node:path";
+import { afterEach, expect, it, vi } from "vitest";
+
+const writers = vi.hoisted(() => ({
+  before: undefined as undefined | ((path: string) => Promise<void>),
+  after: undefined as undefined | ((path: string) => Promise<void>),
+  duringRead: undefined as undefined | (() => Promise<void>),
+}));
+
+vi.mock(
+  "../../src/application/ClientConfigurationPublication.js",
+  async (importOriginal) => {
+    const actual =
+      await importOriginal<
+        typeof import("../../src/application/ClientConfigurationPublication.js")
+      >();
+    return {
+      ...actual,
+      ClientConfigurationPublication: {
+        write: async (
+          ...args: Parameters<
+            typeof actual.ClientConfigurationPublication.write
+          >
+        ) => {
+          await writers.before?.(args[0]);
+          const publication = await actual.ClientConfigurationPublication.write(
+            ...args,
+          );
+          await writers.after?.(args[0]);
+          return publication;
+        },
+      },
+    };
+  },
+);
+
+vi.mock("../../src/process/BoundedFileBytes.js", async (importOriginal) => {
+  const actual =
+    await importOriginal<
+      typeof import("../../src/process/BoundedFileBytes.js")
+    >();
+  return {
+    ...actual,
+    readBoundedFileBytes: async (
+      ...args: Parameters<typeof actual.readBoundedFileBytes>
+    ) => {
+      const bytes = await actual.readBoundedFileBytes(...args);
+      const mutation = writers.duringRead;
+      writers.duringRead = undefined;
+      await mutation?.();
+      return bytes;
+    },
+  };
+});
+
+import { configureClientConfiguration } from "../../src/application/SetupClientConfiguration.js";
+import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js";
+
+afterEach(() => {
+  writers.before = undefined;
+  writers.after = undefined;
+  writers.duringRead = undefined;
+});
+
+it("reports a conflict when a file changes while the freshness snapshot is being read", async () => {
+  const root = await createTestTempDirectory("rea-config-read-conflict-");
+  const path = join(root, "config.json");
+  const changed = '{"external":"changed during read"}\n';
+  await writeFile(path, "{}\n");
+  writers.before = async (target) => {
+    writers.duringRead = () => writeFile(target, changed);
+  };
+
+  expect(
+    await configureClientConfiguration({
+      name: "cursor",
+      configPath: path,
+      format: "json",
+    }),
+  ).toMatchObject({ status: "failed", reason: "conflict" });
+  expect(await readFile(path, "utf8")).toBe(changed);
+});
+
+it.each(["before", "after"] as const)(
+  "preserves an invalid UTF-8 replacement written %s installation and reports a conflict",
+  async (phase) => {
+    const root = await createTestTempDirectory(
+      "rea-config-invalid-replacement-",
+    );
+    const path = join(root, "config.json");
+    const original = "{}\n";
+    const changed = Buffer.from([0xff, 0xfe]);
+    await writeFile(path, original);
+    writers[phase] = (target) => writeFile(target, changed);
+
+    expect(
+      await configureClientConfiguration({
+        name: "cursor",
+        configPath: path,
+        format: "json",
+      }),
+    ).toMatchObject({ status: "failed", reason: "conflict" });
+    expect(await readFile(path)).toEqual(changed);
+    expect(await readFile(`${path}.rea.backup`, "utf8")).toBe(original);
+    expect(await readdir(root)).toEqual([
+      "config.json",
+      "config.json.rea.backup",
+    ]);
+  },
+);
+
+it("preserves an unrelated concurrent edit when a matching recovery backup already exists", async () => {
+  const root = await createTestTempDirectory("rea-config-conflict-");
+  const path = join(root, "config.json");
+  const original = '{"theme":"dark"}\n';
+  const changed =
+    '{"theme":"light","mcpServers":{"other":{"command":"other"}}}\n';
+  await writeFile(path, original);
+  await writeFile(`${path}.rea.backup`, original);
+  writers.before = (target) => writeFile(target, changed);
+
+  const result = await configureClientConfiguration({
+    name: "cursor",
+    configPath: path,
+    format: "json",
+  });
+
+  expect(result).toMatchObject({ status: "failed", reason: "conflict" });
+  expect(await readFile(path, "utf8")).toBe(changed);
+  expect(await readFile(`${path}.rea.backup`, "utf8")).toBe(original);
+  expect(await readdir(root)).toEqual([
+    "config.json",
+    "config.json.rea.backup",
+  ]);
+  writers.before = undefined;
+  const retry = await configureClientConfiguration({
+    name: "cursor",
+    configPath: path,
+    format: "json",
+  });
+  expect(retry.status).toBe("configured");
+  if (retry.status !== "configured" || retry.backupPath === undefined)
+    throw new Error("Retry did not preserve its current recovery snapshot");
+  expect(await readFile(retry.backupPath, "utf8")).toBe(changed);
+  expect(JSON.parse(await readFile(path, "utf8"))).toMatchObject({
+    theme: "light",
+    mcpServers: { other: { command: "other" } },
+  });
+  expect(
+    await configureClientConfiguration({
+      name: "cursor",
+      configPath: path,
+      format: "json",
+    }),
+  ).toEqual({ status: "unchanged" });
+});
+
+it("does not overwrite a file created by another writer during first-run setup", async () => {
+  const root = await createTestTempDirectory("rea-config-first-run-conflict-");
+  const path = join(root, "config.json");
+  const changed = '{"theme":"light"}\n';
+  writers.before = (target) => writeFile(target, changed);
+
+  expect(
+    await configureClientConfiguration({
+      name: "cursor",
+      configPath: path,
+      format: "json",
+    }),
+  ).toMatchObject({ status: "failed", reason: "conflict" });
+  expect(await readFile(path, "utf8")).toBe(changed);
+  expect(await readdir(root)).toEqual(["config.json"]);
+});
+
+it("does not recreate a configuration deleted during setup", async () => {
+  const root = await createTestTempDirectory("rea-config-deleted-");
+  const path = join(root, "config.json");
+  await writeFile(path, "{}\n");
+  writers.before = (target) => rm(target);
+
+  expect(
+    await configureClientConfiguration({
+      name: "cursor",
+      configPath: path,
+      format: "json",
+    }),
+  ).toMatchObject({ status: "failed", reason: "conflict" });
+  await expect(lstat(path)).rejects.toMatchObject({ code: "ENOENT" });
+});
+
+it("preserves an external edit made after the atomic write without rolling it back", async () => {
+  const root = await createTestTempDirectory("rea-config-after-write-");
+  const path = join(root, "config.json");
+  const changed = '{"theme":"external"}\n';
+  await writeFile(path, "{}\n");
+  writers.after = (target) => writeFile(target, changed);
+
+  expect(
+    await configureClientConfiguration({
+      name: "cursor",
+      configPath: path,
+      format: "json",
+    }),
+  ).toMatchObject({ status: "failed", reason: "conflict" });
+  expect(await readFile(path, "utf8")).toBe(changed);
+});
+
+it("does not adopt another writer's replacement merely because its bytes match", async () => {
+  const root = await createTestTempDirectory("rea-config-replaced-");
+  const path = join(root, "config.json");
+  const replacement = join(root, "replacement.json");
+  let replacementInode: number | undefined;
+  writers.after = async (target) => {
+    await writeFile(replacement, await readFile(target));
+    replacementInode = (await lstat(replacement)).ino;
+    await rename(replacement, target);
+  };
+
+  expect(
+    await configureClientConfiguration({
+      name: "cursor",
+      configPath: path,
+      format: "json",
+    }),
+  ).toMatchObject({ status: "failed", reason: "conflict" });
+  expect((await lstat(path)).ino).toBe(replacementInode);
+});
+
+it("detects a replacement of the original file even when its contents are identical", async () => {
+  const root = await createTestTempDirectory("rea-config-original-replaced-");
+  const path = join(root, "config.json");
+  const replacement = join(root, "replacement.json");
+  await writeFile(path, "{}\n");
+  let replacementInode: number | undefined;
+  writers.before = async (target) => {
+    await writeFile(replacement, await readFile(target));
+    replacementInode = (await lstat(replacement)).ino;
+    await rename(replacement, target);
+  };
+  expect(
+    await configureClientConfiguration({
+      name: "cursor",
+      configPath: path,
+      format: "json",
+    }),
+  ).toMatchObject({ status: "failed", reason: "conflict" });
+  expect((await lstat(path)).ino).toBe(replacementInode);
+  expect(await readFile(path, "utf8")).toBe("{}\n");
+});
+
+it.skipIf(process.platform === "win32")(
+  "does not follow a retargeted configuration symlink",
+  async () => {
+    const root = await createTestTempDirectory("rea-config-retargeted-");
+    const path = join(root, "config.json");
+    const first = join(root, "first.json");
+    const second = join(root, "second.json");
+    await writeFile(first, "{}\n");
+    await writeFile(second, '{"external":true}\n');
+    await symlink(first, path);
+    writers.before = async () => {
+      await rm(path);
+      await symlink(second, path);
+    };
+
+    expect(
+      await configureClientConfiguration({
+        name: "cursor",
+        configPath: path,
+        format: "json",
+      }),
+    ).toMatchObject({ status: "failed", reason: "conflict" });
+    expect(await readFile(first, "utf8")).toBe("{}\n");
+    expect(await readFile(second, "utf8")).toBe('{"external":true}\n');
+  },
+);
+
+it.each(["absolute", "relative"])(
+  "merges queued setup requests using a %s path to the same configuration",
+  async (pathKind) => {
+    const root = await createTestTempDirectory("rea-config-queued-");
+    const path = join(root, "config.json");
+    const client = {
+      name: "cursor",
+      configPath: path,
+      format: "json",
+    } as const;
+    const results = await Promise.all([
+      configureClientConfiguration(client, { FIRST: "1" }),
+      configureClientConfiguration(
+        {
+          ...client,
+          configPath:
+            pathKind === "relative" ? relative(process.cwd(), path) : path,
+        },
+        { SECOND: "2" },
+      ),
+    ]);
+
+    expect(results.every((result) => result.status === "configured")).toBe(
+      true,
+    );
+    expect(JSON.parse(await readFile(path, "utf8"))).toMatchObject({
+      mcpServers: { rea: { env: { FIRST: "1", SECOND: "2" } } },
+    });
+  },
+);
+
+it.skipIf(process.platform === "win32")(
+  "serializes setup through a directory symlink and its real directory",
+  async () => {
+    for (const presence of ["missing", "existing"] as const) {
+      const root = await createTestTempDirectory(
+        `rea-config-dir-alias-${presence}-`,
+      );
+      const realDirectory = join(root, "real");
+      const alias = join(root, "alias");
+      await mkdir(realDirectory);
+      await symlink(realDirectory, alias, "dir");
+      const realPath = join(realDirectory, "config.json");
+      const aliasPath = join(alias, "config.json");
+      if (presence === "existing") await writeFile(realPath, "{}\n");
+
+      const entered: string[] = [];
+      let releaseFirst!: () => void;
+      const release = new Promise<void>((resolve) => {
+        releaseFirst = resolve;
+      });
+      let markStarted!: () => void;
+      const started = new Promise<void>((resolve) => {
+        markStarted = resolve;
+      });
+      writers.before = async (target) => {
+        entered.push(target);
+        if (entered.length === 1) {
+          markStarted();
+          await release;
+        }
+      };
+
+      const first = configureClientConfiguration(
+        { name: "cursor", configPath: realPath, format: "json" },
+        { FIRST: "1" },
+      );
+      try {
+        await started;
+        const second = configureClientConfiguration(
+          { name: "cursor", configPath: aliasPath, format: "json" },
+          { SECOND: "2" },
+        );
+        const secondReachedWriter = await new Promise<boolean>((resolve) => {
+          let giveUp: ReturnType<typeof setTimeout> | undefined;
+          const timer = setInterval(() => {
+            if (entered.length > 1) {
+              clearInterval(timer);
+              clearTimeout(giveUp);
+              resolve(true);
+            }
+          }, 15);
+          giveUp = setTimeout(() => {
+            clearInterval(timer);
+            resolve(false);
+          }, 1_000);
+        });
+        expect(secondReachedWriter).toBe(false);
+        expect(entered).toEqual([realPath]);
+        releaseFirst();
+        const [firstResult, secondResult] = await Promise.all([first, second]);
+        expect([firstResult.status, secondResult.status]).toEqual([
+          "configured",
+          "configured",
+        ]);
+        expect(JSON.parse(await readFile(realPath, "utf8"))).toMatchObject({
+          mcpServers: { rea: { env: { FIRST: "1", SECOND: "2" } } },
+        });
+        expect(await readFile(aliasPath, "utf8")).toBe(
+          await readFile(realPath, "utf8"),
+        );
+      } finally {
+        releaseFirst();
+        writers.before = undefined;
+      }
+    }
+  },
+);
+
+it.skipIf(process.platform === "win32")(
+  "preserves a concurrent permission change even when the configuration bytes match",
+  async () => {
+    const root = await createTestTempDirectory("rea-config-mode-conflict-");
+    const path = join(root, "config.json");
+    const original = "{}\n";
+    await writeFile(path, original, { mode: 0o600 });
+    writers.before = (target) => chmod(target, 0o640);
+
+    expect(
+      await configureClientConfiguration({
+        name: "cursor",
+        configPath: path,
+        format: "json",
+      }),
+    ).toMatchObject({ status: "failed", reason: "conflict" });
+    expect(await readFile(path, "utf8")).toBe(original);
+    expect((await lstat(path)).mode & 0o777).toBe(0o640);
+    expect(await readFile(`${path}.rea.backup`, "utf8")).toBe(original);
+  },
+);
+
+it("releases a failed setup transaction so a queued request can configure the file", async () => {
+  const root = await createTestTempDirectory("rea-config-queued-failure-");
+  const path = join(root, "config.json");
+  const client = { name: "cursor", configPath: path, format: "json" } as const;
+  let markStarted!: () => void;
+  const started = new Promise<void>((resolve) => {
+    markStarted = resolve;
+  });
+  let releaseWriter!: () => void;
+  const release = new Promise<void>((resolve) => {
+    releaseWriter = resolve;
+  });
+  writers.before = async () => {
+    writers.before = undefined;
+    markStarted();
+    await release;
+    throw new Error("The first writer could not stage its configuration");
+  };
+
+  const first = configureClientConfiguration(client, { FIRST: "1" });
+  await started;
+  const second = configureClientConfiguration(client, { SECOND: "2" });
+  releaseWriter();
+  const [failed, queued] = await Promise.all([first, second]);
+
+  expect(failed).toEqual({ status: "failed", reason: "write" });
+  expect(queued.status).toBe("configured");
+  expect(JSON.parse(await readFile(path, "utf8")).mcpServers.rea.env).toEqual({
+    SECOND: "2",
+  });
+  expect(await readdir(root)).toEqual(["config.json"]);
+  expect(await configureClientConfiguration(client)).toEqual({
+    status: "unchanged",
+  });
+});

tests/process-global/setupConfigurationRollback.test.ts

@@ -395,7 +395,7 @@ it.each(["existing", "first-run"] as const)(
 
     expect(await fs.readFile(replacement, "utf8")).toBe(foreign);
     expect((await fs.lstat(path)).isSymbolicLink()).toBe(true);
-    expect(result).toMatchObject({ status: "failed", reason: "rollback" });
+    expect(result).toMatchObject({ status: "failed", reason: "conflict" });
     if (kind === "existing")
       expect(await fs.readFile(`${path}.rea.backup`, "utf8")).toBe(original);
   },
@@ -423,7 +423,7 @@ it("retains a replacement regular file when readback fails", async () => {
   );
 
   expect(await fs.readFile(path, "utf8")).toBe(foreign);
-  expect(result).toMatchObject({ status: "failed", reason: "rollback" });
+  expect(result).toMatchObject({ status: "failed", reason: "conflict" });
   expect(await fs.readFile(`${path}.rea.backup`, "utf8")).toBe(original);
 });
 
@@ -502,7 +502,7 @@ it("rejects a replaced publication even when the replacement passes registration
       {},
       ["rea", "mcp"],
     ),
-  ).toMatchObject({ status: "failed", reason: "rollback" });
+  ).toMatchObject({ status: "failed", reason: "conflict" });
   expect(await fs.readFile(path, "utf8")).toBe(foreign);
   expect(await fs.readFile(`${path}.rea.backup`, "utf8")).toBe(original);
 });
@@ -525,7 +525,7 @@ it("retains concurrent edits to the published inode rather than overwriting them
       {},
       ["rea", "mcp"],
     ),
-  ).toMatchObject({ status: "failed", reason: "rollback" });
+  ).toMatchObject({ status: "failed", reason: "conflict" });
   expect(await fs.readFile(path, "utf8")).toBe(edited);
   expect(await fs.readFile(`${path}.rea.backup`, "utf8")).toBe(original);
 });