fix(setup): refuse linked skill installation directories (#3108)
Validate final selected skill roots and references before preview, approval, and client/Hopper changes, with a repeated installation-boundary check. Preserve interactive unselected-client choices and registration-only setup. Share typed directory/read observations across setup, doctor, and update; report unmanaged paths and unknown reads with actionable paths/reasons. Retain uninstall child outcomes and ownership manifests when cleanup remains. Shared single-pass inspection retains all unreadable paths after stale or missing files and keeps observed metadata, without duplicate manifest reads. Absent installed files do not depend on generated bundled bytes. Validation: 217 related tests, 41 final CLI/MCP and filesystem checks, and all 7,446 source tests in 508 files pass (eight platform/condition skips). Static/docs checks pass. Normal four-call install/rerun/repair/uninstall tasks preserve results and output token counts. No native Windows follow-up or arbitrary concurrent swap containment claim. Fixes #3057. Co-authored-by: geekwolf <3125615+geekwolf@users.noreply.github.com> Co-authored-by: N0zoM1z0 <161784452+N0zoM1z0@users.noreply.github.com> Co-authored-by: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Co-authored-by: akushonkamen <74654766+akushonkamen@users.noreply.github.com>
This commit is contained in:
12 files changed
+718
-89
No files matched your search
@@ -122,6 +122,16 @@ See [client configuration](client-configuration.md) for client-specific roots,
|
||||
profiles, and environment overrides. A mixed selection plans each distinct
|
||||
destination. Selecting the skill without a client uses `~/.agents/skills`.
|
||||
Setup leaves existing skill copies in other locations untouched.
|
||||
Setup refuses symbolic links at the selected skill directory or its managed
|
||||
`references` directory, matching uninstall and update's ownership rules. Use a
|
||||
regular skill directory, or select `--skill=false` to configure the client
|
||||
registration without installing the bundled skill.
|
||||
The final selected directories are checked before approval and again before
|
||||
setup changes any client or Hopper installation. Unselected client destinations
|
||||
do not block other setup choices. Doctor reports linked or non-directory skill
|
||||
paths as `unmanaged`, names the affected paths, and provides those alternatives;
|
||||
an unreadable skill identity remains `unknown`. Uninstall retains and reports
|
||||
files it cannot safely remove, including their ownership manifest.
|
||||
|
||||
`rea setup --skill-client claude_code --yes --json` refreshes and verifies the
|
||||
selected skill destination independently of MCP registration selection. The
|
||||
|
||||
+52
-18
@@ -23,6 +23,7 @@ import {
|
||||
import {
|
||||
readInstalledSkillIdentity,
|
||||
type InstalledSkillIdentity,
|
||||
type SkillPathIssue,
|
||||
} from "./SetupSkill.js";
|
||||
import { CATALOG_IDENTITY } from "../catalogIdentity.js";
|
||||
import { PRODUCT_IDENTITY, SDK_IDENTITY } from "../identity.js";
|
||||
@@ -132,6 +133,13 @@ export interface DoctorScope {
|
||||
}
|
||||
|
||||
/** Installed-package, skill, client, and runtime identity observations. */
|
||||
export type DoctorSkillState =
|
||||
| "aligned"
|
||||
| "stale"
|
||||
| "missing"
|
||||
| "unmanaged"
|
||||
| "unknown";
|
||||
|
||||
interface DoctorIdentity {
|
||||
readonly cli_package_version: string;
|
||||
readonly expected_skill_version: string;
|
||||
@@ -148,7 +156,8 @@ interface DoctorIdentity {
|
||||
readonly skill: {
|
||||
readonly installed_version: string | null;
|
||||
readonly installed_tool_count: number | null;
|
||||
readonly state: "aligned" | "stale" | "missing";
|
||||
readonly state: DoctorSkillState;
|
||||
readonly issues?: readonly SkillPathIssue[];
|
||||
readonly remediation: string | null;
|
||||
};
|
||||
readonly registrations: readonly ClientRegistrationStatus[];
|
||||
@@ -311,15 +320,13 @@ const collectDoctorIdentity = async (
|
||||
skill: {
|
||||
installed_version: installedSkillIdentity?.version ?? null,
|
||||
installed_tool_count: installedSkillIdentity?.toolCount ?? null,
|
||||
state:
|
||||
installedSkillIdentity === undefined
|
||||
? "missing"
|
||||
: skillAligned
|
||||
? "aligned"
|
||||
: "stale",
|
||||
state: skillIdentityState(installedSkillIdentity),
|
||||
...(installedSkillIdentity?.issues === undefined
|
||||
? {}
|
||||
: { issues: installedSkillIdentity.issues }),
|
||||
remediation: skillAligned
|
||||
? null
|
||||
: "Run rea setup to update the installed REA skill.",
|
||||
: skillIdentityRemediation(installedSkillIdentity),
|
||||
},
|
||||
registrations,
|
||||
},
|
||||
@@ -333,18 +340,45 @@ const skillIdentityAligned = (
|
||||
identity.toolCount === CATALOG_IDENTITY.counts.mcp_tools &&
|
||||
identity.canonical;
|
||||
|
||||
const skillIdentityState = (
|
||||
identity: InstalledSkillIdentity | undefined,
|
||||
): DoctorSkillState => {
|
||||
if (identity?.issues?.some(({ kind }) => kind === "unreadable"))
|
||||
return "unknown";
|
||||
if ((identity?.issues?.length ?? 0) > 0) return "unmanaged";
|
||||
return identity === undefined
|
||||
? "missing"
|
||||
: skillIdentityAligned(identity)
|
||||
? "aligned"
|
||||
: "stale";
|
||||
};
|
||||
|
||||
const skillIdentityRemediation = (
|
||||
identity: InstalledSkillIdentity | undefined,
|
||||
): string => {
|
||||
const issues = identity?.issues;
|
||||
if (issues !== undefined && issues.length > 0)
|
||||
return `${issues.map(({ path, detail }) => `${detail} (${path})`).join(" ")} Verify the reported paths and use regular skill directories, or run rea setup with --skill=false to configure the client without installing the skill.`;
|
||||
return "Run rea setup to update the installed REA skill.";
|
||||
};
|
||||
|
||||
const skillIdentityCheck = (
|
||||
identity: InstalledSkillIdentity | undefined,
|
||||
): DoctorCheck => ({
|
||||
name: "skill:identity",
|
||||
ok: false,
|
||||
classification: "config_drift",
|
||||
detail:
|
||||
identity === undefined
|
||||
? "Installed REA skill identity is missing."
|
||||
: "Installed REA skill identity is stale.",
|
||||
remediation: "Run rea setup to update the installed REA skill.",
|
||||
});
|
||||
): DoctorCheck => {
|
||||
const issues = identity?.issues;
|
||||
return {
|
||||
name: "skill:identity",
|
||||
ok: false,
|
||||
classification: "config_drift",
|
||||
detail:
|
||||
issues !== undefined && issues.length > 0
|
||||
? `Selected skill identity is ${skillIdentityState(identity)}: ${issues.map(({ path }) => path).join(", ")}.`
|
||||
: identity === undefined
|
||||
? "Installed REA skill identity is missing."
|
||||
: "Installed REA skill identity is stale.",
|
||||
remediation: skillIdentityRemediation(identity),
|
||||
};
|
||||
};
|
||||
|
||||
const registrationCheck = (
|
||||
registration: UnhealthyClientRegistrationStatus,
|
||||
|
||||
@@ -8,6 +8,7 @@ import type {
|
||||
DoctorCheck,
|
||||
DoctorProviderInspection,
|
||||
DoctorScope,
|
||||
DoctorSkillState,
|
||||
} from "./Doctor.js";
|
||||
|
||||
/** Normalized readiness boundary included in every doctor response. */
|
||||
@@ -29,7 +30,7 @@ interface DoctorProviderState {
|
||||
interface DoctorScopeInput {
|
||||
readonly checks: readonly DoctorCheck[];
|
||||
readonly registrations: readonly ClientRegistrationStatus[];
|
||||
readonly skillState: "aligned" | "stale" | "missing";
|
||||
readonly skillState: DoctorSkillState;
|
||||
readonly providers: DoctorProviderState;
|
||||
readonly scope: DoctorScopeReport;
|
||||
}
|
||||
@@ -188,7 +189,14 @@ const selectedSkillCheck = (
|
||||
classification: state === "aligned" ? "healthy" : "config_drift",
|
||||
...(state === "aligned"
|
||||
? { detail: "Installed REA skill identity is aligned." }
|
||||
: { remediation: "Run rea setup to update the installed REA skill." }),
|
||||
: {
|
||||
remediation:
|
||||
state === "unmanaged"
|
||||
? "Use regular skill directories, or run rea setup with --skill=false to configure the client without installing the skill."
|
||||
: state === "unknown"
|
||||
? "Verify access to the reported skill files before retrying."
|
||||
: "Run rea setup to update the installed REA skill.",
|
||||
}),
|
||||
};
|
||||
|
||||
const doctorProviderCheckName = (providerId: string, name: string): boolean =>
|
||||
|
||||
@@ -8,7 +8,7 @@ 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 { err, ok, type Result } from "../domain/result.js";
|
||||
import { SetupSkillInstallError } from "./SetupSkill.js";
|
||||
import type { DoctorCheck, DoctorReport, DoctorScope } from "./Doctor.js";
|
||||
import type { LinuxDistribution } from "./LinuxHopper.js";
|
||||
@@ -43,6 +43,7 @@ export class FakeSetupHost implements SetupHost {
|
||||
hopperReplaceRequests: boolean[] = [];
|
||||
configurations = 0;
|
||||
skillInstalls = 0;
|
||||
skillDirectoryValidations: Array<readonly string[]> = [];
|
||||
doctorCalls = 0;
|
||||
doctorScopes: Array<DoctorScope | undefined> = [];
|
||||
checkedHopperPaths: Array<string | undefined> = [];
|
||||
@@ -129,6 +130,12 @@ export class FakeSetupHost implements SetupHost {
|
||||
);
|
||||
skillNeedsInstall = (): Promise<boolean> =>
|
||||
Promise.resolve(this.skill !== "unchanged");
|
||||
validateSkillDirectories = (
|
||||
clientIds: readonly string[],
|
||||
): Promise<Result<void, SetupSkillInstallError>> => {
|
||||
this.skillDirectoryValidations.push(clientIds);
|
||||
return Promise.resolve(ok(undefined));
|
||||
};
|
||||
installSkill = () => {
|
||||
this.skillInstalls += 1;
|
||||
return Promise.resolve(
|
||||
|
||||
@@ -3,6 +3,97 @@ import { describe, expect, it } from "vitest";
|
||||
import { runSetup } from "./Setup.js";
|
||||
import type { SetupProgressEvent } from "./SetupTypes.js";
|
||||
import { FakeSetupHost, options } from "./Setup.fixture.js";
|
||||
import { SetupSkillInstallError } from "./SetupSkill.js";
|
||||
import { err, ok } from "../domain/result.js";
|
||||
|
||||
const linkedSkillFailure = () =>
|
||||
new SetupSkillInstallError({
|
||||
path: "/fixture/home/.claude/skills/reverse-engineer-anything",
|
||||
stage: "prepare",
|
||||
reason: "Skill directory is a symbolic link",
|
||||
rollbackFailures: [],
|
||||
recoveryBackupPaths: [],
|
||||
cause: undefined,
|
||||
});
|
||||
|
||||
describe("selected skill admission", () => {
|
||||
it.each(["codex", "shared"] as const)(
|
||||
"keeps %s choices when an unselected client is linked",
|
||||
async (choice) => {
|
||||
const host = new FakeSetupHost();
|
||||
host.hopper = "/Applications/Hopper";
|
||||
host.clients = [
|
||||
{ name: "claude_code", configPath: "/claude.json" },
|
||||
{ name: "codex", configPath: "/codex.toml", format: "toml" },
|
||||
];
|
||||
host.validateSkillDirectories = async (ids) => {
|
||||
host.skillDirectoryValidations.push(ids);
|
||||
return ids.includes("claude_code")
|
||||
? err(linkedSkillFailure())
|
||||
: ok(undefined);
|
||||
};
|
||||
const result = await runSetup(
|
||||
{ ...options(false), structured: false },
|
||||
host,
|
||||
async (actions, context) => {
|
||||
if (context?.stage === "select") {
|
||||
expect(actions.some(({ kind }) => kind === "install_skill")).toBe(
|
||||
true,
|
||||
);
|
||||
return {
|
||||
approved: false,
|
||||
selectedActionIds:
|
||||
choice === "codex"
|
||||
? ["configure_client:codex"]
|
||||
: ["install_skill"],
|
||||
};
|
||||
}
|
||||
return true;
|
||||
},
|
||||
);
|
||||
expect(result.status).toBe("ready");
|
||||
expect(host.skillInstalls).toBe(1);
|
||||
expect(host.configurations).toBe(choice === "codex" ? 1 : 0);
|
||||
expect(host.skillDirectoryValidations).toEqual(
|
||||
choice === "codex" ? [["codex"], ["codex"]] : [[], []],
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it("rechecks directory admission after approval before changing clients or Hopper", async () => {
|
||||
const host = new FakeSetupHost("linux");
|
||||
host.distribution = {
|
||||
id: "ubuntu",
|
||||
versionId: "24.04",
|
||||
packageFamily: "deb",
|
||||
hopperSetupSupported: true,
|
||||
};
|
||||
host.clients = [{ name: "claude_code", configPath: "/claude.json" }];
|
||||
let linked = false;
|
||||
host.validateSkillDirectories = async () =>
|
||||
linked ? err(linkedSkillFailure()) : ok(undefined);
|
||||
const result = await runSetup(
|
||||
{
|
||||
...options(false, true),
|
||||
clientIds: ["claude_code"],
|
||||
structured: false,
|
||||
},
|
||||
host,
|
||||
async () => {
|
||||
linked = true;
|
||||
return true;
|
||||
},
|
||||
);
|
||||
expect(result).toMatchObject({
|
||||
status: "needs_human",
|
||||
code: "skill_install_failed",
|
||||
appliedActions: [],
|
||||
});
|
||||
expect(host.configurations).toBe(0);
|
||||
expect(host.hopperInstalls).toBe(0);
|
||||
expect(host.skillInstalls).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("skill plan destinations", () => {
|
||||
it("plans a Claude-only skill in its personal location", async () => {
|
||||
|
||||
@@ -99,6 +99,12 @@ const runSetupWorkflow = async (
|
||||
[host, plannedActions, appliedActions, clients, clientStates],
|
||||
readinessScope,
|
||||
);
|
||||
const failSkill = async (
|
||||
error: SetupSkillInstallError,
|
||||
): Promise<SetupResult> => ({
|
||||
...(await fail(error.remediation)),
|
||||
code: "skill_install_failed",
|
||||
});
|
||||
const unsupported = await hostRemediation(host, false);
|
||||
if (unsupported !== undefined) return fail(unsupported);
|
||||
const initialState = await host.initialSetupState(options.readinessScope);
|
||||
@@ -235,6 +241,12 @@ const runSetupWorkflow = async (
|
||||
);
|
||||
|
||||
const manualRemediation = manualClientsRemediation(planned.selectedClients);
|
||||
if (skillSelected) {
|
||||
const admission = await host.validateSkillDirectories(
|
||||
selectedSkillClientIds,
|
||||
);
|
||||
if (!admission.ok) return failSkill(admission.error);
|
||||
}
|
||||
if (options.dryRun === true)
|
||||
return {
|
||||
status: "planned",
|
||||
@@ -285,6 +297,13 @@ const runSetupWorkflow = async (
|
||||
: {}),
|
||||
};
|
||||
|
||||
// Approval may pause while a skill manager changes a selected directory.
|
||||
if (planSelection.skillSelected) {
|
||||
const admission = await host.validateSkillDirectories(
|
||||
selectedSkillClientIds,
|
||||
);
|
||||
if (!admission.ok) return failSkill(admission.error);
|
||||
}
|
||||
let selectedClients: readonly SetupClient[] =
|
||||
planSelection.clientsToConfigure;
|
||||
if (
|
||||
@@ -325,10 +344,7 @@ const runSetupWorkflow = async (
|
||||
selectedSkillClientIds,
|
||||
);
|
||||
if (skillFailure !== undefined) {
|
||||
return {
|
||||
...(await fail(skillFailure.remediation)),
|
||||
code: "skill_install_failed",
|
||||
};
|
||||
return failSkill(skillFailure);
|
||||
}
|
||||
}
|
||||
const doctor = summarizeDoctor(await host.doctor(readinessScope));
|
||||
|
||||
@@ -31,6 +31,7 @@ import {
|
||||
skillDestinations,
|
||||
canonicalSkillNeedsInstall,
|
||||
installCanonicalSkill,
|
||||
validateCanonicalSkillDirectories,
|
||||
} from "./SetupSkill.js";
|
||||
import {
|
||||
clientConfigurationAligned,
|
||||
@@ -224,6 +225,13 @@ export const systemSetupHost = (
|
||||
platform,
|
||||
resources,
|
||||
),
|
||||
validateSkillDirectories: (clientIds) =>
|
||||
validateCanonicalSkillDirectories(
|
||||
homeDirectory,
|
||||
clientIds,
|
||||
environment,
|
||||
platform,
|
||||
),
|
||||
installSkill: (clientIds) =>
|
||||
installCanonicalSkill(
|
||||
homeDirectory,
|
||||
|
||||
+193
-61
@@ -178,6 +178,14 @@ export interface InstalledSkillIdentity {
|
||||
readonly version: string | null;
|
||||
readonly toolCount: number | null;
|
||||
readonly canonical: boolean;
|
||||
readonly issues?: readonly SkillPathIssue[];
|
||||
}
|
||||
|
||||
/** Known management blockers and unresolved filesystem observations. */
|
||||
export interface SkillPathIssue {
|
||||
readonly path: string;
|
||||
readonly kind: "symbolic_link" | "not_directory" | "unreadable";
|
||||
readonly detail: string;
|
||||
}
|
||||
|
||||
/** Resolve the skill roots required by a client selection, or all owned roots. */
|
||||
@@ -209,8 +217,10 @@ export const existingSkillDestinations = async (
|
||||
platform,
|
||||
)) {
|
||||
try {
|
||||
const root = await lstat(destination.path);
|
||||
if (root.isSymbolicLink() || !root.isDirectory()) continue;
|
||||
const issues = await skillDirectoryIssues([destination]);
|
||||
if (issues.some(({ kind }) => kind === "unreadable"))
|
||||
throw skillDirectoryFailure(issues);
|
||||
if (issues.length > 0) continue;
|
||||
const manifest = join(destination.path, "SKILL.md");
|
||||
const content = await readSkillFile(manifest, resources);
|
||||
if (isManagedSkillManifest(content)) existing.push(destination);
|
||||
@@ -242,49 +252,187 @@ const readOptionalSkillState = async (
|
||||
}
|
||||
};
|
||||
|
||||
const inspectSkillDirectory = async (
|
||||
path: string,
|
||||
): Promise<Result<void, SkillPathIssue>> => {
|
||||
try {
|
||||
const directory = await lstat(path);
|
||||
if (directory.isSymbolicLink())
|
||||
return err({
|
||||
path,
|
||||
kind: "symbolic_link",
|
||||
detail: "Skill directory is a symbolic link.",
|
||||
});
|
||||
if (!directory.isDirectory())
|
||||
return err({
|
||||
path,
|
||||
kind: "not_directory",
|
||||
detail: "Skill path is not a directory.",
|
||||
});
|
||||
return ok(undefined);
|
||||
} catch (cause: unknown) {
|
||||
if (cause instanceof Error && "code" in cause && cause.code === "ENOENT")
|
||||
return ok(undefined);
|
||||
return err({
|
||||
path,
|
||||
kind: "unreadable",
|
||||
detail: cause instanceof Error ? cause.message : String(cause),
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
const skillDirectoryIssues = async (
|
||||
destinations: readonly SkillDestination[],
|
||||
): Promise<readonly SkillPathIssue[]> => {
|
||||
const issues: SkillPathIssue[] = [];
|
||||
for (const { path } of destinations) {
|
||||
const root = await inspectSkillDirectory(path);
|
||||
if (!root.ok) {
|
||||
issues.push(root.error);
|
||||
continue;
|
||||
}
|
||||
const references = await inspectSkillDirectory(join(path, "references"));
|
||||
if (!references.ok) issues.push(references.error);
|
||||
}
|
||||
return issues;
|
||||
};
|
||||
|
||||
const skillDirectoryFailure = (
|
||||
issues: readonly SkillPathIssue[],
|
||||
): SetupSkillInstallError =>
|
||||
new SetupSkillInstallError({
|
||||
path: issues[0]?.path ?? "skill bundle",
|
||||
stage: "prepare",
|
||||
reason: `${issues.map(({ path, detail }) => `${detail} (${path})`).join(" ")} Use a regular skill directory, or run setup with --skill=false to configure the client without installing the skill`,
|
||||
rollbackFailures: [],
|
||||
recoveryBackupPaths: [],
|
||||
cause: issues,
|
||||
});
|
||||
|
||||
/** Validate only final selected roots; permission to install is separate from freshness. */
|
||||
export const validateCanonicalSkillDirectories = async (
|
||||
home: string,
|
||||
clientIds: readonly string[],
|
||||
environment: Readonly<NodeJS.ProcessEnv> = {},
|
||||
platform: NodeJS.Platform = process.platform,
|
||||
): Promise<Result<void, SetupSkillInstallError>> => {
|
||||
const issues = await skillDirectoryIssues(
|
||||
skillDestinations(home, clientIds, environment, platform),
|
||||
);
|
||||
return issues.length === 0
|
||||
? ok(undefined)
|
||||
: err(skillDirectoryFailure(issues));
|
||||
};
|
||||
|
||||
const canonicalSkillFiles = async (
|
||||
home: string,
|
||||
clientIds: readonly string[],
|
||||
environment: Readonly<NodeJS.ProcessEnv> = {},
|
||||
platform: NodeJS.Platform = process.platform,
|
||||
resources?: ArtifactResourceScope,
|
||||
): Promise<readonly CanonicalSkillFile[]> =>
|
||||
Promise.all(
|
||||
skillDestinations(home, clientIds, environment, platform).flatMap(
|
||||
({ path: root }) =>
|
||||
MANAGED_SKILL_FILES.map(async (relativePath) => {
|
||||
const destination = join(root, relativePath);
|
||||
return {
|
||||
destination,
|
||||
content: await canonicalSkillContent(relativePath),
|
||||
original: await readOptionalSkillState(destination, resources),
|
||||
};
|
||||
}),
|
||||
): Promise<readonly CanonicalSkillFile[]> => {
|
||||
const destinations = skillDestinations(
|
||||
home,
|
||||
clientIds,
|
||||
environment,
|
||||
platform,
|
||||
);
|
||||
const admission = await validateCanonicalSkillDirectories(
|
||||
home,
|
||||
clientIds,
|
||||
environment,
|
||||
platform,
|
||||
);
|
||||
if (!admission.ok) throw admission.error;
|
||||
return Promise.all(
|
||||
destinations.flatMap(({ path: root }) =>
|
||||
MANAGED_SKILL_FILES.map(async (relativePath) => {
|
||||
const destination = join(root, relativePath);
|
||||
return {
|
||||
destination,
|
||||
content: await canonicalSkillContent(relativePath),
|
||||
original: await readOptionalSkillState(destination, resources),
|
||||
};
|
||||
}),
|
||||
),
|
||||
);
|
||||
};
|
||||
|
||||
const canonicalSkillNeedsInstallByReading = async (
|
||||
interface CanonicalSkillInspection {
|
||||
readonly canonical: boolean;
|
||||
readonly manifest: string | undefined;
|
||||
}
|
||||
|
||||
interface SkillInspectionFailure {
|
||||
readonly _tag: "SkillInspectionFailure";
|
||||
readonly issues: readonly SkillPathIssue[];
|
||||
readonly cause: unknown;
|
||||
readonly manifest: string | undefined;
|
||||
}
|
||||
|
||||
const inspectCanonicalSkill = async (
|
||||
home: string,
|
||||
clientIds: readonly string[],
|
||||
environment: Readonly<NodeJS.ProcessEnv>,
|
||||
platform: NodeJS.Platform,
|
||||
resources?: ArtifactResourceScope,
|
||||
): Promise<boolean> => {
|
||||
for (const { path: root } of skillDestinations(
|
||||
): Promise<Result<CanonicalSkillInspection, SkillInspectionFailure>> => {
|
||||
const destinations = skillDestinations(
|
||||
home,
|
||||
clientIds,
|
||||
environment,
|
||||
platform,
|
||||
)) {
|
||||
);
|
||||
const issues = [...(await skillDirectoryIssues(destinations))];
|
||||
if (issues.length > 0)
|
||||
return err({
|
||||
_tag: "SkillInspectionFailure",
|
||||
issues,
|
||||
cause: issues,
|
||||
manifest: undefined,
|
||||
});
|
||||
let canonical = true;
|
||||
let manifest: string | undefined;
|
||||
let failureCause: unknown;
|
||||
for (const { path: root } of destinations) {
|
||||
for (const relativePath of MANAGED_SKILL_FILES) {
|
||||
const destination = join(root, relativePath);
|
||||
const content = await canonicalSkillContent(relativePath);
|
||||
const original = await readOptionalSkillState(destination, resources);
|
||||
if (original?.bytes.equals(Buffer.from(content, "utf8")) !== true)
|
||||
return true;
|
||||
let original: SkillFileState | undefined;
|
||||
try {
|
||||
original = await readOptionalSkillState(destination, resources);
|
||||
} catch (cause: unknown) {
|
||||
if (
|
||||
cause instanceof RegularFileCleanupFailure ||
|
||||
cause instanceof ArtifactReaderFailure
|
||||
)
|
||||
throw cause;
|
||||
issues.push({
|
||||
path: destination,
|
||||
kind: "unreadable",
|
||||
detail: causeReason(cause),
|
||||
});
|
||||
if (issues.length === 1) failureCause = cause;
|
||||
canonical = false;
|
||||
continue;
|
||||
}
|
||||
if (relativePath === "SKILL.md" && manifest === undefined)
|
||||
manifest = original?.bytes.toString("utf8");
|
||||
// A mismatch settles freshness, but not coverage of later read failures.
|
||||
if (original === undefined) canonical = false;
|
||||
else if (canonical) {
|
||||
const content = await canonicalSkillContent(relativePath);
|
||||
canonical = original.bytes.equals(Buffer.from(content, "utf8"));
|
||||
}
|
||||
}
|
||||
}
|
||||
return false;
|
||||
return issues.length === 0
|
||||
? ok({ canonical, manifest })
|
||||
: err({
|
||||
_tag: "SkillInspectionFailure",
|
||||
issues,
|
||||
cause: failureCause,
|
||||
manifest,
|
||||
});
|
||||
};
|
||||
|
||||
/** Report whether setup would change any file in the managed REA skill bundle. */
|
||||
@@ -296,20 +444,21 @@ export const canonicalSkillNeedsInstall = async (
|
||||
resources?: ArtifactResourceScope,
|
||||
): Promise<boolean> => {
|
||||
try {
|
||||
return await canonicalSkillNeedsInstallByReading(
|
||||
const inspection = await inspectCanonicalSkill(
|
||||
home,
|
||||
clientIds,
|
||||
environment,
|
||||
platform,
|
||||
resources,
|
||||
);
|
||||
return !inspection.ok || !inspection.value.canonical;
|
||||
} catch (cause: unknown) {
|
||||
if (
|
||||
cause instanceof RegularFileCleanupFailure ||
|
||||
cause instanceof ArtifactReaderFailure
|
||||
)
|
||||
throw cause;
|
||||
// Unreadable skill state fails open to install so setup can repair it.
|
||||
// A copy needs attention; selected-root admission decides whether it may be installed.
|
||||
void cause;
|
||||
return true;
|
||||
}
|
||||
@@ -431,6 +580,8 @@ export const installCanonicalSkill = async (
|
||||
}
|
||||
return ok("installed");
|
||||
} catch (cause: unknown) {
|
||||
if (stage === "prepare" && cause instanceof SetupSkillInstallError)
|
||||
return err(cause);
|
||||
failingPath = causePath(cause) ?? failingPath;
|
||||
const rollbackFailures = await restoreSkillFiles(attempted, operations);
|
||||
return err(
|
||||
@@ -458,48 +609,29 @@ export const readInstalledSkillIdentity = async (
|
||||
platform: NodeJS.Platform = process.platform,
|
||||
resources?: ArtifactResourceScope,
|
||||
): Promise<InstalledSkillIdentity | undefined> => {
|
||||
const destinations = skillDestinations(
|
||||
const inspection = await inspectCanonicalSkill(
|
||||
home,
|
||||
clientIds,
|
||||
environment,
|
||||
platform,
|
||||
resources,
|
||||
);
|
||||
let content: string | undefined;
|
||||
for (const destination of destinations) {
|
||||
try {
|
||||
content = await readSkillFile(
|
||||
join(destination.path, "SKILL.md"),
|
||||
resources,
|
||||
);
|
||||
break;
|
||||
} catch (cause: unknown) {
|
||||
if (
|
||||
cause instanceof RegularFileCleanupFailure ||
|
||||
cause instanceof ArtifactReaderFailure
|
||||
)
|
||||
throw cause;
|
||||
// Another selected copy may provide metadata; canonical validation below
|
||||
// still marks missing or unreadable copies as stale.
|
||||
}
|
||||
}
|
||||
if (content === undefined) return undefined;
|
||||
const toolCount = /^\s{2}tool_count:\s*(\d+)\s*$/mu.exec(content)?.[1];
|
||||
let canonical: boolean;
|
||||
try {
|
||||
canonical = !(await canonicalSkillNeedsInstall(
|
||||
home,
|
||||
clientIds,
|
||||
environment,
|
||||
platform,
|
||||
resources,
|
||||
));
|
||||
} catch (cause: unknown) {
|
||||
if (!(cause instanceof ArtifactReaderFailure)) throw cause;
|
||||
canonical = false;
|
||||
}
|
||||
const content = inspection.ok
|
||||
? inspection.value.manifest
|
||||
: inspection.error.manifest;
|
||||
const issues = inspection.ok ? [] : inspection.error.issues;
|
||||
if (content === undefined && issues.length === 0) return undefined;
|
||||
const toolCount =
|
||||
content === undefined
|
||||
? undefined
|
||||
: /^\s{2}tool_count:\s*(\d+)\s*$/mu.exec(content)?.[1];
|
||||
return {
|
||||
canonical,
|
||||
version: /^\s{2}version:\s*"([^"]+)"\s*$/mu.exec(content)?.[1] ?? null,
|
||||
canonical: inspection.ok && inspection.value.canonical,
|
||||
version:
|
||||
content === undefined
|
||||
? null
|
||||
: (/^\s{2}version:\s*"([^"]+)"\s*$/mu.exec(content)?.[1] ?? null),
|
||||
toolCount: toolCount === undefined ? null : Number.parseInt(toolCount, 10),
|
||||
...(issues.length === 0 ? {} : { issues }),
|
||||
};
|
||||
};
|
||||
@@ -1,6 +1,11 @@
|
||||
import type { DoctorReport, DoctorScope, runDoctor } from "./Doctor.js";
|
||||
import type { LinuxDistribution } from "./LinuxHopper.js";
|
||||
import type { SkillDestination, SkillInstallResult } from "./SetupSkill.js";
|
||||
import type {
|
||||
SkillDestination,
|
||||
SkillInstallResult,
|
||||
SetupSkillInstallError,
|
||||
} from "./SetupSkill.js";
|
||||
import type { Result } from "../domain/result.js";
|
||||
import type { SetupClient } from "./SupportedClients.js";
|
||||
import type {
|
||||
SetupFailureCode,
|
||||
@@ -89,6 +94,9 @@ export interface SetupHost {
|
||||
command: readonly string[],
|
||||
): Promise<ClientConfigurationInspection>;
|
||||
skillNeedsInstall(clientIds: readonly string[]): Promise<boolean>;
|
||||
validateSkillDirectories(
|
||||
clientIds: readonly string[],
|
||||
): Promise<Result<void, SetupSkillInstallError>>;
|
||||
installSkill(clientIds: readonly string[]): Promise<SkillInstallResult>;
|
||||
doctor(scope?: DoctorScope): Promise<Awaited<ReturnType<typeof runDoctor>>>;
|
||||
}
|
||||
|
||||
@@ -768,6 +768,12 @@ const removeManagedSkill = async (
|
||||
const failed = results.find(({ status }) => status === "failed");
|
||||
if (failed !== undefined)
|
||||
return item(name, "failed", results.map(({ detail }) => detail).join(" "));
|
||||
if (results.some(({ status }) => status === "retained"))
|
||||
return item(
|
||||
name,
|
||||
"retained",
|
||||
[...new Set(results.map(({ detail }) => detail))].join(" "),
|
||||
);
|
||||
const removed = results.some(({ status }) => status === "removed");
|
||||
return item(
|
||||
name,
|
||||
|
||||
@@ -1,11 +1,190 @@
|
||||
import { access, readFile, readdir, writeFile } from "node:fs/promises";
|
||||
import { join, relative, resolve } from "node:path";
|
||||
import {
|
||||
access,
|
||||
cp,
|
||||
lstat,
|
||||
mkdir,
|
||||
readFile,
|
||||
readdir,
|
||||
symlink,
|
||||
writeFile,
|
||||
} from "node:fs/promises";
|
||||
import { dirname, join, relative, resolve } from "node:path";
|
||||
|
||||
import { expect } from "vitest";
|
||||
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
import { cliTest } from "../../support/cli/cliFixture.js";
|
||||
|
||||
cliTest.for(["root", "references"] as const)(
|
||||
"rejects a linked skill %s during planning before configuring a client",
|
||||
async (kind, { cli }) => {
|
||||
const directory = await createTestTempDirectory("rea-skill-preflight-");
|
||||
const selected = join(
|
||||
directory,
|
||||
".claude/skills/reverse-engineer-anything",
|
||||
);
|
||||
const managed = join(directory, "user-managed");
|
||||
const source = resolve("skills/reverse-engineer-anything");
|
||||
await mkdir(kind === "root" ? dirname(selected) : selected, {
|
||||
recursive: true,
|
||||
});
|
||||
await cp(kind === "root" ? source : join(source, "references"), managed, {
|
||||
recursive: true,
|
||||
});
|
||||
if (kind === "references")
|
||||
await cp(join(source, "SKILL.md"), join(selected, "SKILL.md"));
|
||||
const link = kind === "root" ? selected : join(selected, "references");
|
||||
await symlink(
|
||||
managed,
|
||||
link,
|
||||
process.platform === "win32" ? "junction" : "dir",
|
||||
);
|
||||
const run = (arguments_: readonly string[]) =>
|
||||
cli.run({
|
||||
arguments: [...arguments_, "--json"],
|
||||
cwd: directory,
|
||||
environment: { USERPROFILE: directory },
|
||||
});
|
||||
const preview = await run([
|
||||
"setup",
|
||||
"--client",
|
||||
"claude_code",
|
||||
"--dry-run",
|
||||
]);
|
||||
const applied = await run(["setup", "--client", "claude_code", "--yes"]);
|
||||
await expect(access(join(directory, ".claude.json"))).rejects.toMatchObject(
|
||||
{
|
||||
code: "ENOENT",
|
||||
},
|
||||
);
|
||||
for (const result of [preview, applied]) {
|
||||
expect(result.exitCode, result.stdout).toBe(1);
|
||||
expect(result.json).toMatchObject({
|
||||
status: "needs_human",
|
||||
code: "skill_install_failed",
|
||||
appliedActions: [],
|
||||
});
|
||||
expect(result.stdout).toContain(link);
|
||||
expect(result.stdout).toContain("--skill=false");
|
||||
}
|
||||
const doctor = await run(["doctor", "--client", "claude_code", "--skill"]);
|
||||
expect(doctor.exitCode, doctor.stdout).toBe(1);
|
||||
expect(doctor.json).toMatchObject({
|
||||
healthy: false,
|
||||
identity: { skill: { state: "unmanaged" } },
|
||||
});
|
||||
expect(doctor.stdout).toContain(link);
|
||||
expect((await lstat(link)).isSymbolicLink()).toBe(true);
|
||||
},
|
||||
);
|
||||
|
||||
cliTest(
|
||||
"setup refuses a linked skill while allowing registration-only setup",
|
||||
async ({ cli }) => {
|
||||
const directory = await createTestTempDirectory("rea-linked-skill-cli-");
|
||||
const managed = join(directory, "managed");
|
||||
const selected = join(
|
||||
directory,
|
||||
".claude/skills/reverse-engineer-anything",
|
||||
);
|
||||
await mkdir(managed);
|
||||
await mkdir(join(directory, ".claude/skills"), { recursive: true });
|
||||
await cp(resolve("skills/reverse-engineer-anything"), managed, {
|
||||
recursive: true,
|
||||
});
|
||||
const original = await readFile(join(managed, "SKILL.md"), "utf8");
|
||||
await symlink(
|
||||
managed,
|
||||
selected,
|
||||
process.platform === "win32" ? "junction" : "dir",
|
||||
);
|
||||
const run = (arguments_: readonly string[]) =>
|
||||
cli.run({
|
||||
arguments: [...arguments_, "--json"],
|
||||
cwd: directory,
|
||||
environment: { USERPROFILE: directory },
|
||||
});
|
||||
|
||||
const refused = await run([
|
||||
"setup",
|
||||
"--skill-client",
|
||||
"claude_code",
|
||||
"--yes",
|
||||
]);
|
||||
expect(refused.exitCode, refused.stdout).toBe(1);
|
||||
expect(refused.json).toMatchObject({
|
||||
status: "needs_human",
|
||||
code: "skill_install_failed",
|
||||
});
|
||||
expect(refused.stdout).toContain("symbolic link");
|
||||
const doctor = await run(["doctor", "--client", "claude_code", "--skill"]);
|
||||
expect(doctor.exitCode, doctor.stdout).toBe(1);
|
||||
expect(doctor.json).toMatchObject({
|
||||
healthy: false,
|
||||
identity: { skill: { state: "unmanaged" } },
|
||||
});
|
||||
const registration = await run([
|
||||
"setup",
|
||||
"--client",
|
||||
"claude_code",
|
||||
"--skill=false",
|
||||
"--yes",
|
||||
]);
|
||||
expect(registration.exitCode, registration.stdout).toBe(0);
|
||||
expect(registration.json).toMatchObject({ status: "ready" });
|
||||
await access(join(directory, ".claude.json"));
|
||||
expect((await lstat(selected)).isSymbolicLink()).toBe(true);
|
||||
expect((await readdir(managed)).sort()).toEqual(["SKILL.md", "references"]);
|
||||
expect(await readFile(join(managed, "SKILL.md"), "utf8")).toBe(original);
|
||||
},
|
||||
);
|
||||
|
||||
cliTest(
|
||||
"uninstall reports retained references and preserves their ownership manifest",
|
||||
async ({ cli }) => {
|
||||
const directory = await createTestTempDirectory("rea-skill-retained-cli-");
|
||||
const selected = join(
|
||||
directory,
|
||||
".claude/skills/reverse-engineer-anything",
|
||||
);
|
||||
const managed = join(directory, "user-managed-references");
|
||||
const source = resolve("skills/reverse-engineer-anything");
|
||||
await mkdir(selected, { recursive: true });
|
||||
await cp(join(source, "SKILL.md"), join(selected, "SKILL.md"));
|
||||
await cp(join(source, "references"), managed, { recursive: true });
|
||||
await symlink(
|
||||
managed,
|
||||
join(selected, "references"),
|
||||
process.platform === "win32" ? "junction" : "dir",
|
||||
);
|
||||
const before = await readFile(join(managed, "native-and-artifacts.md"));
|
||||
const result = await cli.run({
|
||||
arguments: ["uninstall", "--json"],
|
||||
cwd: directory,
|
||||
environment: { USERPROFILE: directory },
|
||||
});
|
||||
expect(result.exitCode, result.stdout).toBe(0);
|
||||
expect(result.json).toMatchObject({
|
||||
items: expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
name: "skill",
|
||||
status: "retained",
|
||||
detail: expect.stringContaining("retained"),
|
||||
}),
|
||||
]),
|
||||
});
|
||||
expect(await readFile(join(selected, "SKILL.md"))).toEqual(
|
||||
await readFile(join(source, "SKILL.md")),
|
||||
);
|
||||
expect(await readFile(join(managed, "native-and-artifacts.md"))).toEqual(
|
||||
before,
|
||||
);
|
||||
expect((await lstat(join(selected, "references"))).isSymbolicLink()).toBe(
|
||||
true,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
cliTest(
|
||||
"skill-only setup verifies its selected client copy on install, rerun and repair",
|
||||
async ({ cli }) => {
|
||||
|
||||
@@ -3,10 +3,14 @@ import {
|
||||
access,
|
||||
chmod,
|
||||
copyFile,
|
||||
lstat,
|
||||
mkdir,
|
||||
readFile,
|
||||
readdir,
|
||||
rename,
|
||||
rm,
|
||||
stat,
|
||||
symlink,
|
||||
writeFile,
|
||||
} from "node:fs/promises";
|
||||
import { constants as fsConstants } from "node:fs";
|
||||
@@ -16,7 +20,9 @@ import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
|
||||
import {
|
||||
canonicalSkillNeedsInstall,
|
||||
existingSkillDestinations,
|
||||
installCanonicalSkill,
|
||||
readInstalledSkillIdentity,
|
||||
skillDestinations,
|
||||
type SkillInstallFileOperations,
|
||||
} from "../../../src/application/SetupSkill.js";
|
||||
@@ -123,6 +129,130 @@ describe("skill recovery byte preservation", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("skill directory admission", () => {
|
||||
it("excludes a linked references directory from update maintenance", async () => {
|
||||
const directory = await createTestTempDirectory(
|
||||
"rea-skill-linked-maintenance-",
|
||||
);
|
||||
const installed = await installCanonicalSkill(directory, ["claude_code"]);
|
||||
expect(installed.ok).toBe(true);
|
||||
const [selected] = skillDestinations(directory, ["claude_code"]);
|
||||
if (selected === undefined) throw new Error("Missing skill destination");
|
||||
const references = join(selected.path, "references");
|
||||
const retained = join(directory, "retained-references");
|
||||
await rename(references, retained);
|
||||
await symlink(
|
||||
retained,
|
||||
references,
|
||||
process.platform === "win32" ? "junction" : "dir",
|
||||
);
|
||||
expect(await existingSkillDestinations(directory)).toEqual([]);
|
||||
expect((await lstat(references)).isSymbolicLink()).toBe(true);
|
||||
await access(join(retained, "native-and-artifacts.md"));
|
||||
});
|
||||
|
||||
it
|
||||
.skipIf(process.platform === "win32" || process.getuid?.() === 0)
|
||||
.each(["aligned", "stale", "missing"] as const)(
|
||||
"reports every unreadable reference with a %s manifest",
|
||||
async (manifestState) => {
|
||||
const directory = await createTestTempDirectory(
|
||||
"rea-skill-unreadable-reference-",
|
||||
);
|
||||
expect((await installCanonicalSkill(directory, ["claude_code"])).ok).toBe(
|
||||
true,
|
||||
);
|
||||
const [selected] = skillDestinations(directory, ["claude_code"]);
|
||||
if (selected === undefined) throw new Error("Missing skill destination");
|
||||
const manifest = join(selected.path, "SKILL.md");
|
||||
if (manifestState === "stale")
|
||||
await writeFile(manifest, "\nlocal changes\n", { flag: "a" });
|
||||
if (manifestState === "missing") await rm(manifest);
|
||||
const references = [
|
||||
"references/connection-and-recovery.md",
|
||||
"references/evidence-workflows.md",
|
||||
].map((relative) => join(selected.path, relative));
|
||||
for (const reference of references) await chmod(reference, 0o000);
|
||||
try {
|
||||
const identity = await readInstalledSkillIdentity(directory, [
|
||||
"claude_code",
|
||||
]);
|
||||
const issues = references.map((path) =>
|
||||
expect.objectContaining({
|
||||
path,
|
||||
kind: "unreadable",
|
||||
detail: expect.stringContaining("EACCES"),
|
||||
}),
|
||||
);
|
||||
expect(identity).toMatchObject({
|
||||
canonical: false,
|
||||
version:
|
||||
manifestState === "missing" ? null : PRODUCT_IDENTITY.skillVersion,
|
||||
issues,
|
||||
});
|
||||
const report = await runDoctor(
|
||||
undefined,
|
||||
createDoctorHostFixture({
|
||||
installedSkillIdentity: () => Promise.resolve(identity),
|
||||
}),
|
||||
);
|
||||
expect(report.identity?.skill).toMatchObject({
|
||||
state: "unknown",
|
||||
issues,
|
||||
remediation: expect.stringContaining("evidence-workflows.md"),
|
||||
});
|
||||
} finally {
|
||||
for (const reference of references) await chmod(reference, 0o600);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["root", "references"] as const)(
|
||||
"refuses a linked skill %s before changing any selected destination",
|
||||
async (linkedDirectory) => {
|
||||
const home = await createTestTempDirectory("rea-skill-linked-root-");
|
||||
const destinations = skillDestinations(home, ["shared", "claude_code"]);
|
||||
const selected = destinations.find(
|
||||
({ client }) => client === "claude_code",
|
||||
);
|
||||
if (selected === undefined)
|
||||
throw new Error("Missing Claude Code skill root");
|
||||
const managed = join(home, "user-managed");
|
||||
await mkdir(managed);
|
||||
const original = "user-owned instructions\n";
|
||||
await writeFile(join(managed, "SKILL.md"), original);
|
||||
const linked =
|
||||
linkedDirectory === "root"
|
||||
? selected.path
|
||||
: join(selected.path, "references");
|
||||
await mkdir(dirname(linked), { recursive: true });
|
||||
await symlink(
|
||||
managed,
|
||||
linked,
|
||||
process.platform === "win32" ? "junction" : "dir",
|
||||
);
|
||||
|
||||
const result = await installCanonicalSkill(home, [
|
||||
"shared",
|
||||
"claude_code",
|
||||
]);
|
||||
if (result.ok) throw new Error("Expected linked directory refusal");
|
||||
expect(result.error.stage).toBe("prepare");
|
||||
expect(result.error.path).toBe(linked);
|
||||
expect(result.error.reason).toContain("symbolic link");
|
||||
expect(result.error.rollbackFailures).toEqual([]);
|
||||
expect((await lstat(linked)).isSymbolicLink()).toBe(true);
|
||||
expect(await readFile(join(managed, "SKILL.md"), "utf8")).toBe(original);
|
||||
expect(await readdir(managed)).toEqual(["SKILL.md"]);
|
||||
const shared = destinations.find(({ client }) => client === "shared");
|
||||
if (shared === undefined) throw new Error("Missing shared skill root");
|
||||
await expect(access(shared.path)).rejects.toMatchObject({
|
||||
code: "ENOENT",
|
||||
});
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe("canonical skill transaction", () => {
|
||||
it("does not roll back files when backup preparation fails", async () => {
|
||||
const home = await createTestTempDirectory("rea-skill-backup-failure-");
|
||||
|
||||
Reference in new issue
Block a user