fix(setup): preserve skill files and report rollback failures

Track attempted writes, preserve original content and modes from one regular-file observation, and continue rollback after individual failures. Return tagged failure details and retained backup paths through setup.

Validation: setup filesystem and recovery tests, combined compile/typecheck/lint/format/metadata/architecture checks. Filesystem fault injection covers backup, ambiguous publication, and rollback failures.

src/application/Setup.fixture.ts

@@ -8,6 +8,8 @@ import type {
 } from "./SetupTypes.js";
 import type { SetupClient } from "./SupportedClients.js";
 import type { SetupHopperInstallResult } from "./SetupInstallFailure.js";
+import { err, ok } from "../domain/result.js";
+import { SetupSkillInstallError } from "./SetupSkill.js";
 import type { DoctorCheck, DoctorReport, DoctorScope } from "./Doctor.js";
 import type { LinuxDistribution } from "./LinuxHopper.js";
 import { PRODUCT_IDENTITY, SDK_IDENTITY } from "../identity.js";
@@ -122,9 +124,22 @@ export class FakeSetupHost implements SetupHost {
     );
   skillNeedsInstall = (): Promise<boolean> =>
     Promise.resolve(this.skill !== "unchanged");
-  installSkill = (): Promise<"installed" | "unchanged" | "failed"> => {
+  installSkill = () => {
     this.skillInstalls += 1;
-    return Promise.resolve(this.skill);
+    return Promise.resolve(
+      this.skill === "failed"
+        ? err(
+            new SetupSkillInstallError({
+              path: "/fixture/home/.agents/skills/reverse-engineer-anything/SKILL.md",
+              stage: "write",
+              reason: "injected skill write failure",
+              rollbackFailures: [],
+              recoveryBackupPaths: [],
+              cause: new Error("injected skill write failure"),
+            }),
+          )
+        : ok(this.skill),
+    );
   };
   doctor = (scope?: DoctorScope): Promise<DoctorReport> => {
     this.doctorCalls += 1;

src/application/Setup.recovery.test.ts

@@ -6,6 +6,7 @@ import { ConfigurationError } from "../domain/configurationErrors.js";
 import { SUPPORTED_NODE_VERSION_PROSE } from "../domain/runtimeVersion.js";
 import { FakeSetupHost, options } from "./Setup.fixture.js";
 import { runSetup } from "./Setup.js";
+import type { SetupProgressEvent } from "./SetupTypes.js";
 
 describe("setup workflow", () => {
   it("rejects Node 25 before applying setup actions", async () => {
@@ -57,14 +58,27 @@ describe("setup workflow", () => {
     const host = new FakeSetupHost();
     host.hopper = "/Applications/Hopper";
     host.skill = "failed";
+    const progress: SetupProgressEvent[] = [];
     const result = await runSetup(
-      { ...options(true), installSkill: true, clientIds: [] },
+      {
+        ...options(true),
+        installSkill: true,
+        clientIds: [],
+        onProgress: (event) => {
+          if (event.actionId === "install_skill") progress.push(event);
+        },
+      },
       host,
     );
     expect(result.status).toBe("needs_human");
-    expect(result.remediation).toBe(
-      "REA analysis skill could not be installed or verified. Check permissions for its planned skill directories, then rerun setup.",
-    );
+    expect(result.code).toBe("skill_install_failed");
+    expect(result.remediation).toContain("REA skill write failed at");
+    expect(result.remediation).toContain("injected skill write failure");
+    expect(result.remediation).toContain("Check permissions and rerun setup.");
+    expect(progress.at(-1)).toMatchObject({
+      state: "failed",
+      detail: expect.stringContaining("injected skill write failure"),
+    });
   });
 
   it("delegates remaining unhealthy checks to doctor remediation", async () => {

src/application/Setup.ts

@@ -26,6 +26,7 @@ import {
 import { AnalysisError } from "../domain/analysisErrorBase.js";
 import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js";
 import { ProviderCleanupError } from "../domain/providerCleanupError.js";
+import type { SetupSkillInstallError } from "./SetupSkill.js";
 
 /** Discover, approve, and apply setup actions idempotently. */
 export const runSetup = async (
@@ -315,18 +316,20 @@ const runSetupWorkflow = async (
       : { onProgress: options.onProgress }),
   });
   if (clientFailure !== undefined) return fail(clientFailure);
-  if (
-    planSelection.installSkill &&
-    !(await installSkillAction(
+  if (planSelection.installSkill) {
+    const skillFailure = await installSkillAction(
       host,
       options,
       appliedActions,
       selectedSkillClientIds,
-    ))
-  )
-    return fail(
-      "REA analysis skill could not be installed or verified. Check permissions for its planned skill directories, then rerun setup.",
     );
+    if (skillFailure !== undefined) {
+      return {
+        ...(await fail(skillFailure.remediation)),
+        code: "skill_install_failed",
+      };
+    }
+  }
   const doctor = summarizeDoctor(await host.doctor(readinessScope));
   const remediation = combineRemediation(
     finalSetupRemediation(
@@ -417,7 +420,7 @@ const installSkillAction = async (
   options: SetupOptions,
   appliedActions: string[],
   skillClientIds: readonly string[],
-): Promise<boolean> => {
+): Promise<SetupSkillInstallError | undefined> => {
   const label = "REA reverse-engineering skill";
   emitProgress(options, {
     actionId: "install_skill",
@@ -425,22 +428,23 @@ const installSkillAction = async (
     state: "started",
   });
   const skill = await host.installSkill(skillClientIds);
-  if (skill === "failed") {
+  if (!skill.ok) {
     emitProgress(options, {
       actionId: "install_skill",
       label,
       state: "failed",
+      detail: skill.error.message,
     });
-    return false;
+    return skill.error;
   }
-  if (skill === "installed") appliedActions.push("installed_skill");
+  if (skill.value === "installed") appliedActions.push("installed_skill");
   emitProgress(options, {
     actionId: "install_skill",
     label,
-    state: skill === "installed" ? "completed" : "warning",
-    ...(skill === "unchanged" ? { detail: "Already current" } : {}),
+    state: skill.value === "installed" ? "completed" : "warning",
+    ...(skill.value === "unchanged" ? { detail: "Already current" } : {}),
   });
-  return true;
+  return undefined;
 };
 
 const setupFailure = async (

src/application/SetupInstallFailure.ts

@@ -11,6 +11,7 @@ export type SetupFailureCode =
   | "mount_failed"
   | "bundle_invalid"
   | "copy_failed"
+  | "skill_install_failed"
   | "cleanup_failed";
 
 export type SetupHopperInstallResult =

src/application/SetupSkill.ts

@@ -2,10 +2,15 @@ import { copyFile, lstat, mkdir, readFile, rm } from "node:fs/promises";
 import { constants as fsConstants } from "node:fs";
 import { dirname, join } from "node:path";
 import writeFileAtomic from "write-file-atomic";
+import { err, ok, type Result } from "../domain/result.js";
+import { readBoundedFileBytes } from "../process/BoundedFileBytes.js";
 
 import { PRODUCT_IDENTITY } from "../identity.js";
-import { NonRegularFileReadError } from "../filesystem/RegularFile.js";
-import { readRegularFileText } from "./RegularFileRead.js";
+import {
+  NonRegularFileReadError,
+  RegularFileChangedError,
+} from "../filesystem/RegularFile.js";
+import { withRegularFile } from "./RegularFileRead.js";
 import { clientSkillDirectories } from "./SupportedClients.js";
 
 export const MANAGED_SKILL_FILES = [
@@ -23,14 +28,119 @@ export const isManagedSkillManifest = (content: string): boolean =>
   content.startsWith(`---\r\nname: ${PRODUCT_IDENTITY.skillName}\r\n`);
 
 export const readSkillFile = (path: string): Promise<string> =>
-  readRegularFileText(path, { symlinks: "reject" });
+  readSkillFileState(path).then(({ content }) => content);
+
+interface SkillFileState {
+  readonly content: string;
+  readonly mode: number;
+}
+
+const readSkillFileState = async (path: string): Promise<SkillFileState> =>
+  withRegularFile(
+    path,
+    async (handle, stats) => {
+      const bytes = await readBoundedFileBytes(
+        handle,
+        stats.size,
+        undefined,
+        stats.size,
+      );
+      if (bytes === undefined || bytes.length !== stats.size)
+        throw new RegularFileChangedError(path);
+      return { content: bytes.toString("utf8"), mode: stats.mode & 0o7777 };
+    },
+    { symlinks: "reject" },
+  );
 
 interface CanonicalSkillFile {
   readonly content: string;
   readonly destination: string;
-  readonly original: string | undefined;
+  readonly original: SkillFileState | undefined;
 }
 
+export interface SkillInstallFileOperations {
+  readonly mkdir: (path: string) => Promise<void>;
+  readonly backup: (source: string, destination: string) => Promise<void>;
+  readonly write: (
+    path: string,
+    content: string,
+    mode: number,
+  ) => Promise<void>;
+  readonly remove: (path: string) => Promise<void>;
+}
+
+export interface SkillInstallRollbackFailure {
+  readonly path: string;
+  readonly reason: string;
+}
+
+/** Expected failure while replacing a managed skill bundle. */
+export class SetupSkillInstallError extends Error {
+  readonly _tag = "SetupSkillInstallError";
+  readonly path: string;
+  readonly stage: "prepare" | "backup" | "write" | "readback";
+  readonly reason: string;
+  readonly rollbackFailures: readonly SkillInstallRollbackFailure[];
+  readonly recoveryBackupPaths: readonly string[];
+
+  constructor(input: {
+    readonly path: string;
+    readonly stage: "prepare" | "backup" | "write" | "readback";
+    readonly reason: string;
+    readonly rollbackFailures: readonly SkillInstallRollbackFailure[];
+    readonly recoveryBackupPaths: readonly string[];
+    readonly cause: unknown;
+  }) {
+    const message = [
+      `REA skill ${input.stage} failed at ${input.path}: ${input.reason}.`,
+      ...(input.rollbackFailures.length === 0
+        ? []
+        : [
+            `Rollback also failed for ${input.rollbackFailures.map(({ path, reason }) => `${path} (${reason})`).join(", ")}.`,
+            ...(input.recoveryBackupPaths.length === 0
+              ? []
+              : [
+                  `Existing-file backups remain at ${input.recoveryBackupPaths.join(", ")}.`,
+                ]),
+          ]),
+    ].join(" ");
+    super(message, { cause: input.cause });
+    this.path = input.path;
+    this.stage = input.stage;
+    this.reason = input.reason;
+    this.rollbackFailures = input.rollbackFailures;
+    this.recoveryBackupPaths = input.recoveryBackupPaths;
+  }
+
+  get remediation(): string {
+    return `${this.message} Check permissions and rerun setup.`;
+  }
+}
+
+export type SkillInstallValue = "installed" | "unchanged";
+export type SkillInstallResult = Result<
+  SkillInstallValue,
+  SetupSkillInstallError
+>;
+
+const systemSkillInstallFileOperations: SkillInstallFileOperations = {
+  mkdir: async (path) => {
+    await mkdir(path, { recursive: true });
+  },
+  backup: async (source, destination) => {
+    try {
+      await copyFile(source, destination, fsConstants.COPYFILE_EXCL);
+    } catch (cause: unknown) {
+      if (cause instanceof Error && "code" in cause && cause.code === "EEXIST")
+        return;
+      throw cause;
+    }
+  },
+  write: (path, content, mode) =>
+    writeFileAtomic(path, content, { encoding: "utf8", mode }),
+  remove: (path) => rm(path, { force: true }),
+};
+
 /** Managed skill location selected for a client family. */
 export interface SkillDestination {
   readonly client: string;
@@ -87,9 +197,11 @@ export const existingSkillDestinations = async (
   return existing;
 };
 
-const readOptionalText = async (path: string): Promise<string | undefined> => {
+const readOptionalSkillState = async (
+  path: string,
+): Promise<SkillFileState | undefined> => {
   try {
-    return await readSkillFile(path);
+    return await readSkillFileState(path);
   } catch (cause: unknown) {
     if (cause instanceof Error && "code" in cause && cause.code === "ENOENT")
       return undefined;
@@ -117,7 +229,7 @@ const canonicalSkillFiles = async (
               ),
               "utf8",
             ),
-            original: await readOptionalText(destination),
+            original: await readOptionalSkillState(destination),
           };
         }),
     ),
@@ -133,7 +245,7 @@ export const canonicalSkillNeedsInstall = async (
   try {
     return (
       await canonicalSkillFiles(home, clientIds, environment, platform)
-    ).some(({ content, original }) => original !== content);
+    ).some(({ content, original }) => original?.content !== content);
   } catch (cause: unknown) {
     // Unreadable skill state fails open to install so setup can repair it.
     void cause;
@@ -141,39 +253,42 @@ export const canonicalSkillNeedsInstall = async (
   }
 };
 
-const writeText = (path: string, content: string): Promise<void> =>
-  writeFileAtomic(path, content, { encoding: "utf8", mode: 0o600 });
+const causeReason = (cause: unknown): string =>
+  cause instanceof Error ? cause.message : String(cause);
 
-const preserveSkillBackup = async (
-  source: string,
-  destination: string,
-): Promise<void> => {
-  try {
-    await copyFile(source, destination, fsConstants.COPYFILE_EXCL);
-  } catch (cause: unknown) {
-    if (cause instanceof Error && "code" in cause && cause.code === "EEXIST")
-      return;
-    throw cause;
-  }
-};
+const causePath = (cause: unknown): string | undefined =>
+  cause instanceof Error && "path" in cause && typeof cause.path === "string"
+    ? cause.path
+    : undefined;
 
 const restoreSkillFiles = async (
-  changed: readonly CanonicalSkillFile[],
-): Promise<void> => {
-  for (const { destination, original } of [...changed].reverse()) {
-    if (original === undefined) await rm(destination, { force: true });
-    else await writeText(destination, original);
+  attempted: readonly CanonicalSkillFile[],
+  operations: SkillInstallFileOperations,
+): Promise<readonly SkillInstallRollbackFailure[]> => {
+  const failures: SkillInstallRollbackFailure[] = [];
+  for (const { destination, original } of [...attempted].reverse()) {
+    try {
+      if (original === undefined) await operations.remove(destination);
+      else await operations.write(destination, original.content, original.mode);
+    } catch (cause: unknown) {
+      failures.push({ path: destination, reason: causeReason(cause) });
     }
+  }
+  return failures;
 };
 
 /** Transactionally install or upgrade the canonical REA skill and references. */
+/** Transactionally install the canonical skill bundle and optional references. */
 export const installCanonicalSkill = async (
   home: string,
   clientIds: readonly string[] = [],
   environment: Readonly<NodeJS.ProcessEnv> = {},
   platform: NodeJS.Platform = process.platform,
-): Promise<"installed" | "unchanged" | "failed"> => {
-  let changed: readonly CanonicalSkillFile[] = [];
+  operations: SkillInstallFileOperations = systemSkillInstallFileOperations,
+): Promise<SkillInstallResult> => {
+  let attempted: CanonicalSkillFile[] = [];
+  let stage: "prepare" | "backup" | "write" | "readback" = "prepare";
+  let failingPath = "skill bundle";
   try {
     const canonical = await canonicalSkillFiles(
       home,
@@ -181,30 +296,52 @@ export const installCanonicalSkill = async (
       environment,
       platform,
     );
-    changed = canonical.filter(({ content, original }) => original !== content);
-    if (changed.length === 0) return "unchanged";
+    const changed = canonical.filter(
+      ({ content, original }) => original?.content !== content,
+    );
+    if (changed.length === 0) return ok("unchanged");
 
+    stage = "backup";
     for (const { destination, original } of changed) {
-      await mkdir(dirname(destination), { recursive: true });
+      failingPath = destination;
+      await operations.mkdir(dirname(destination));
       if (original !== undefined)
-        await preserveSkillBackup(destination, `${destination}.rea.backup`);
+        await operations.backup(destination, `${destination}.rea.backup`);
     }
-    for (const { destination, content } of changed)
-      await writeText(destination, content);
-    for (const { destination, content } of changed)
+    stage = "write";
+    for (const file of changed) {
+      failingPath = file.destination;
+      // An atomic writer can publish and then reject; include the target before
+      // entering it so rollback covers that ambiguous outcome.
+      attempted.push(file);
+      await operations.write(
+        file.destination,
+        file.content,
+        file.original?.mode ?? 0o600,
+      );
+    }
+    stage = "readback";
+    for (const { destination, content } of changed) {
+      failingPath = destination;
       if ((await readSkillFile(destination)) !== content)
         throw new Error(`skill readback mismatch: ${destination}`);
-    return "installed";
-  } catch (cause: unknown) {
-    // Install failure preserves the original error outcome; report cause inline.
-    void cause;
-    try {
-      await restoreSkillFiles(changed);
-    } catch (restoreCause: unknown) {
-      // Per-file backups remain beside changed files for operator recovery.
-      void restoreCause;
     }
-    return "failed";
+    return ok("installed");
+  } catch (cause: unknown) {
+    failingPath = causePath(cause) ?? failingPath;
+    const rollbackFailures = await restoreSkillFiles(attempted, operations);
+    return err(
+      new SetupSkillInstallError({
+        path: failingPath,
+        stage,
+        reason: causeReason(cause),
+        rollbackFailures,
+        recoveryBackupPaths: attempted.flatMap(({ destination, original }) =>
+          original === undefined ? [] : [`${destination}.rea.backup`],
+        ),
+        cause,
+      }),
+    );
   }
 };
 

src/application/SetupTypes.ts

@@ -1,6 +1,6 @@
 import type { DoctorReport, DoctorScope, runDoctor } from "./Doctor.js";
 import type { LinuxDistribution } from "./LinuxHopper.js";
-import type { SkillDestination } from "./SetupSkill.js";
+import type { SkillDestination, SkillInstallResult } from "./SetupSkill.js";
 import type { SetupClient } from "./SupportedClients.js";
 import type {
   SetupFailureCode,
@@ -70,9 +70,7 @@ export interface SetupHost {
     command: readonly string[],
   ): Promise<ClientConfigurationInspection>;
   skillNeedsInstall(clientIds: readonly string[]): Promise<boolean>;
-  installSkill(
-    clientIds: readonly string[],
-  ): Promise<"installed" | "unchanged" | "failed">;
+  installSkill(clientIds: readonly string[]): Promise<SkillInstallResult>;
   doctor(scope?: DoctorScope): Promise<Awaited<ReturnType<typeof runDoctor>>>;
 }
 /** Structured setup outcome carrying remediation instead of prompting. */

tests/boundary/filesystem/setupSkill.test.ts

@@ -2,11 +2,14 @@ import { createHash } from "node:crypto";
 import {
   access,
   chmod,
+  copyFile,
   mkdir,
   readFile,
   rm,
+  stat,
   writeFile,
 } from "node:fs/promises";
+import { constants as fsConstants } from "node:fs";
 import { dirname, join } from "node:path";
 import { describe, expect, it } from "vitest";
 import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
@@ -14,6 +17,8 @@ import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
 import {
   canonicalSkillNeedsInstall,
   installCanonicalSkill,
+  skillDestinations,
+  type SkillInstallFileOperations,
 } from "../../../src/application/SetupSkill.js";
 import {
   runDoctor,
@@ -22,10 +27,141 @@ import {
 import { createDoctorHostFixture } from "../../../src/application/Doctor.fixture.js";
 import { TOOL_CONTRACTS } from "../../../src/contracts/toolContracts.js";
 import { PRODUCT_IDENTITY } from "../../../src/identity.js";
+import writeFileAtomic from "write-file-atomic";
 import { z } from "zod";
 import { skillReferenceIssues } from "../../../scripts/lib/docs-facts.mjs";
 
+const expectPermissionMode = async (
+  path: string,
+  expected: number,
+): Promise<void> => {
+  if (process.platform === "win32") return;
+  expect((await stat(path)).mode & 0o777).toBe(expected);
+};
+
 describe("canonical skill transaction", () => {
+  it("does not roll back files when backup preparation fails", async () => {
+    const home = await createTestTempDirectory("rea-skill-backup-failure-");
+    const root = join(home, ".agents/skills/reverse-engineer-anything");
+    const manifest = join(root, "SKILL.md");
+    const connection = join(root, "references/connection-and-recovery.md");
+    const original = "operator-owned manifest bytes\n";
+    const secondOriginal = "operator-owned reference bytes\n";
+    await mkdir(dirname(connection), { recursive: true });
+    await writeFile(manifest, original, { mode: 0o640 });
+    await writeFile(connection, secondOriginal, { mode: 0o644 });
+    const writes: string[] = [];
+    let backups = 0;
+    const operations: SkillInstallFileOperations = {
+      mkdir: async (path) => {
+        await mkdir(path, { recursive: true });
+      },
+      backup: async (source, target) => {
+        backups += 1;
+        if (backups === 2) throw new Error("injected backup failure");
+        await copyFile(source, target, fsConstants.COPYFILE_EXCL);
+      },
+      write: async (path, content, mode) => {
+        writes.push(path);
+        await writeFile(path, content, { mode });
+      },
+      remove: async (path) => rm(path, { force: true }),
+    };
+
+    const result = await installCanonicalSkill(
+      home,
+      [],
+      {},
+      process.platform,
+      operations,
+    );
+
+    if (result.ok) throw new Error("Expected backup failure");
+    expect(result.error.stage).toBe("backup");
+    expect(result.error.rollbackFailures).toEqual([]);
+    expect(writes).toEqual([]);
+    expect(await readFile(manifest, "utf8")).toBe(original);
+    expect(await readFile(connection, "utf8")).toBe(secondOriginal);
+    expect(await readFile(`${manifest}.rea.backup`, "utf8")).toBe(original);
+    await expect(
+      readFile(`${connection}.rea.backup`, "utf8"),
+    ).rejects.toMatchObject({
+      code: "ENOENT",
+    });
+    await expectPermissionMode(manifest, 0o640);
+    await expectPermissionMode(connection, 0o644);
+  });
+
+  it("rolls back attempted writes independently and preserves original modes", async () => {
+    const home = await createTestTempDirectory("rea-skill-rollback-failure-");
+    const [destination] = skillDestinations(home, []);
+    if (destination === undefined) throw new Error("Missing shared skill root");
+    const root = destination.path;
+    const manifest = join(root, "SKILL.md");
+    const connection = join(root, "references/connection-and-recovery.md");
+    const native = join(root, "references/native-and-artifacts.md");
+    await mkdir(dirname(native), { recursive: true });
+    await writeFile(manifest, "old manifest\n", { mode: 0o640 });
+    await writeFile(connection, "old connection\n", { mode: 0o644 });
+    await writeFile(native, "old native\n", { mode: 0o600 });
+    const writes = new Map<string, number>();
+    const nativeWrites: string[] = [];
+    const operations: SkillInstallFileOperations = {
+      mkdir: async (path) => {
+        await mkdir(path, { recursive: true });
+      },
+      backup: async (source, target) =>
+        copyFile(source, target, fsConstants.COPYFILE_EXCL),
+      write: async (path, content, mode) => {
+        const attempt = (writes.get(path) ?? 0) + 1;
+        writes.set(path, attempt);
+        if (path === native) nativeWrites.push(content);
+        if (path === native && attempt === 2)
+          throw new Error("injected rollback failure");
+        await writeFileAtomic(path, content, { encoding: "utf8", mode });
+        if (path === native && attempt === 1)
+          throw new Error("injected ambiguous publication failure");
+      },
+      remove: async (path) => rm(path, { force: true }),
+    };
+
+    const result = await installCanonicalSkill(
+      home,
+      [],
+      {},
+      process.platform,
+      operations,
+    );
+
+    if (result.ok) throw new Error("Expected write failure");
+    expect(result.error.stage).toBe("write");
+    expect(result.error.path).toBe(native);
+    expect(result.error.rollbackFailures).toEqual([
+      { path: native, reason: "injected rollback failure" },
+    ]);
+    expect(result.error.recoveryBackupPaths).toEqual([
+      `${manifest}.rea.backup`,
+      `${connection}.rea.backup`,
+      `${native}.rea.backup`,
+    ]);
+    expect(result.error.message).toContain(
+      `${native} (injected rollback failure)`,
+    );
+    expect(await readFile(manifest, "utf8")).toBe("old manifest\n");
+    expect(await readFile(connection, "utf8")).toBe("old connection\n");
+    await expectPermissionMode(manifest, 0o640);
+    await expectPermissionMode(connection, 0o644);
+    expect(await readFile(native, "utf8")).not.toBe("old native\n");
+    expect(await readFile(`${native}.rea.backup`, "utf8")).toBe("old native\n");
+    expect(nativeWrites).toHaveLength(2);
+    expect(nativeWrites[1]).toBe("old native\n");
+    expect(result.error.message).toContain(
+      `Existing-file backups remain at ${manifest}.rea.backup`,
+    );
+  });
+});
+
+describe("canonical skill discovery", () => {
   it.skipIf(process.platform === "win32" || process.getuid?.() === 0)(
     "preserves an unreadable existing skill instead of treating it as absent",
     async () => {
@@ -39,7 +175,9 @@ describe("canonical skill transaction", () => {
       await writeFile(destination, original, { mode: 0o200 });
       try {
         await chmod(destination, 0o200);
-        expect(await installCanonicalSkill(home)).toBe("failed");
+        const result = await installCanonicalSkill(home);
+        if (result.ok) throw new Error("Expected unreadable skill failure");
+        expect(result.error.stage).toBe("prepare");
       } finally {
         await chmod(destination, 0o600);
       }
@@ -89,7 +227,7 @@ describe("canonical skill transaction", () => {
       await installCanonicalSkill(home, ["claude_code"], {
         CLAUDE_CONFIG_DIR: dirname(skillsDirectory),
       }),
-    ).toBe("installed");
+    ).toMatchObject({ ok: true, value: "installed" });
     expect(
       await canonicalSkillNeedsInstall(home, ["claude_code"], {
         CLAUDE_CONFIG_DIR: dirname(skillsDirectory),
@@ -125,24 +263,24 @@ describe("canonical skill transaction", () => {
     });
     const doctorScope = { clients: ["claude_code", "codex"], skill: true };
 
-    expect(await installCanonicalSkill(home, ["claude_code", "codex"])).toBe(
-      "installed",
-    );
+    expect(
+      await installCanonicalSkill(home, ["claude_code", "codex"]),
+    ).toMatchObject({ ok: true, value: "installed" });
     await rm(sharedSkill);
     expect(
       (await runDoctor(undefined, host, doctorScope)).identity?.skill.state,
     ).toBe("stale");
-    expect(await installCanonicalSkill(home, ["claude_code", "codex"])).toBe(
-      "installed",
-    );
+    expect(
+      await installCanonicalSkill(home, ["claude_code", "codex"]),
+    ).toMatchObject({ ok: true, value: "installed" });
     for (const skill of [sharedSkill, claudeSkill]) {
       await writeFile(skill, "selected copy changed\n");
       expect(
         (await runDoctor(undefined, host, doctorScope)).identity?.skill.state,
       ).toBe("stale");
-      expect(await installCanonicalSkill(home, ["claude_code", "codex"])).toBe(
-        "installed",
-      );
+      expect(
+        await installCanonicalSkill(home, ["claude_code", "codex"]),
+      ).toMatchObject({ ok: true, value: "installed" });
       expect(
         (await runDoctor(undefined, host, doctorScope)).identity?.skill.state,
       ).toBe("aligned");
@@ -165,12 +303,15 @@ describe("canonical skill transaction", () => {
     await mkdir(dirname(destination), { recursive: true });
     await mkdir(dirname(nativeGuide), { recursive: true });
     await mkdir(dirname(sibling), { recursive: true });
-    await writeFile(destination, "stale managed skill\n");
+    await writeFile(destination, "stale managed skill\n", { mode: 0o640 });
     await writeFile(nativeGuide, "stale filesystem-write permission grant\n");
     await writeFile(sibling, "unrelated skill\n");
 
     expect(await canonicalSkillNeedsInstall(home)).toBe(true);
-    expect(await installCanonicalSkill(home)).toBe("installed");
+    expect(await installCanonicalSkill(home)).toMatchObject({
+      ok: true,
+      value: "installed",
+    });
     expect(await readFile(`${destination}.rea.backup`, "utf8")).toBe(
       "stale managed skill\n",
     );
@@ -187,6 +328,7 @@ describe("canonical skill transaction", () => {
     expect(installedSkill).toContain(
       `version: "${PRODUCT_IDENTITY.skillVersion}"`,
     );
+    await expectPermissionMode(destination, 0o640);
     expect(await readFile(`${nativeGuide}.rea.backup`, "utf8")).toBe(
       "stale filesystem-write permission grant\n",
     );
@@ -232,13 +374,19 @@ describe("canonical skill transaction", () => {
       ),
     ).toContain("analyze_javascript_application");
     expect(await canonicalSkillNeedsInstall(home)).toBe(false);
-    expect(await installCanonicalSkill(home)).toBe("unchanged");
+    expect(await installCanonicalSkill(home)).toMatchObject({
+      ok: true,
+      value: "unchanged",
+    });
     expect(await skillReferenceIssues(dirname(destination))).toEqual([]);
   });
 
   it("binds portable conformance to the generated skill bytes that setup installs", async () => {
     const home = await createTestTempDirectory("rea-skill-commitment-");
-    expect(await installCanonicalSkill(home)).toBe("installed");
+    expect(await installCanonicalSkill(home)).toMatchObject({
+      ok: true,
+      value: "installed",
+    });
     const paths = [
       "SKILL.md",
       "references/android-applications.md",
@@ -298,7 +446,10 @@ it("doctor verifies installed instructions and references even when metadata is
   expect((await runDoctor(undefined, host)).identity?.skill.state).toBe(
     "missing",
   );
-  expect(await installCanonicalSkill(home)).toBe("installed");
+  expect(await installCanonicalSkill(home)).toMatchObject({
+    ok: true,
+    value: "installed",
+  });
   expect((await runDoctor(undefined, host)).identity?.skill).toMatchObject({
     state: "aligned",
   });
@@ -312,7 +463,10 @@ it("doctor verifies installed instructions and references even when metadata is
     expect((await runDoctor(undefined, host)).identity?.skill.state).toBe(
       "stale",
     );
-    expect(await installCanonicalSkill(home)).toBe("installed");
+    expect(await installCanonicalSkill(home)).toMatchObject({
+      ok: true,
+      value: "installed",
+    });
     expect((await runDoctor(undefined, host)).identity?.skill.state).toBe(
       "aligned",
     );