diff --git a/src/application/Setup.fixture.ts b/src/application/Setup.fixture.ts index a4c82678e..a99897494 100644 --- a/src/application/Setup.fixture.ts +++ b/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 => 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 => { this.doctorCalls += 1; diff --git a/src/application/Setup.recovery.test.ts b/src/application/Setup.recovery.test.ts index 7a37d4afe..951ef3a54 100644 --- a/src/application/Setup.recovery.test.ts +++ b/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 () => { diff --git a/src/application/Setup.ts b/src/application/Setup.ts index a9f89a30a..7244824db 100644 --- a/src/application/Setup.ts +++ b/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 => { +): Promise => { 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 ( diff --git a/src/application/SetupInstallFailure.ts b/src/application/SetupInstallFailure.ts index b330ba005..fe2a2739e 100644 --- a/src/application/SetupInstallFailure.ts +++ b/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 = diff --git a/src/application/SetupSkill.ts b/src/application/SetupSkill.ts index 50d0d96f9..6bbcdd0c7 100644 --- a/src/application/SetupSkill.ts +++ b/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 => - readRegularFileText(path, { symlinks: "reject" }); + readSkillFileState(path).then(({ content }) => content); + +interface SkillFileState { + readonly content: string; + readonly mode: number; +} + +const readSkillFileState = async (path: string): Promise => + 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; + readonly backup: (source: string, destination: string) => Promise; + readonly write: ( + path: string, + content: string, + mode: number, + ) => Promise; + readonly remove: (path: string) => Promise; +} + +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 => { +const readOptionalSkillState = async ( + path: string, +): Promise => { 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 => - 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 => { - 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 => { - 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 => { + 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 = {}, platform: NodeJS.Platform = process.platform, -): Promise<"installed" | "unchanged" | "failed"> => { - let changed: readonly CanonicalSkillFile[] = []; + operations: SkillInstallFileOperations = systemSkillInstallFileOperations, +): Promise => { + 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, + }), + ); } }; diff --git a/src/application/SetupTypes.ts b/src/application/SetupTypes.ts index 6b5b7a290..da58abbff 100644 --- a/src/application/SetupTypes.ts +++ b/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; skillNeedsInstall(clientIds: readonly string[]): Promise; - installSkill( - clientIds: readonly string[], - ): Promise<"installed" | "unchanged" | "failed">; + installSkill(clientIds: readonly string[]): Promise; doctor(scope?: DoctorScope): Promise>>; } /** Structured setup outcome carrying remediation instead of prompting. */ diff --git a/tests/boundary/filesystem/setupSkill.test.ts b/tests/boundary/filesystem/setupSkill.test.ts index 4da0225c3..af4df0625 100644 --- a/tests/boundary/filesystem/setupSkill.test.ts +++ b/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 => { + 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(); + 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", );