fix(setup): verify independently selected skill destinations

This commit is contained in:
morluto
2026-10-12 01:31:32 +08:00
parent 1dfb933e47
commit bb508134ef
9 changed files with 223 additions and 3 deletions
+6
View File
@@ -352,6 +352,12 @@ Other integrations, including Qwen Code, use `~/.agents/skills`.
A mixed selection plans each distinct destination. Selecting the skill without a client uses the shared
directory. Setup leaves existing skill copies in other locations untouched.
`rea setup --skill-client claude_code --yes --json` refreshes and verifies the
selected skill destination independently of MCP registration selection. The
final `doctor.scope.skill_clients` records which skill destinations were
checked; `doctor.scope.clients` continues to describe required registrations.
An unselected shared skill does not affect a client-only refresh.
`doctor --skill --json` verifies the selected copies against the bundled
instructions and references. Consumers should use `identity.skill.state`,
`installed_version`, and `installed_tool_count` for skill readiness. The
+36
View File
@@ -281,6 +281,42 @@ describe("doctor installation identity", () => {
});
describe("doctor scoped readiness", () => {
it("checks explicitly selected skill destinations independently of registrations", async () => {
const selected: (readonly string[] | undefined)[] = [];
const result = await runDoctor(
undefined,
host({
clientRegistrations: () => Promise.resolve([]),
installedSkillIdentity: (clients) => {
selected.push(clients);
return Promise.resolve({
version: "stale",
toolCount: 0,
canonical: false,
});
},
}),
{
clients: [],
skillClients: ["claude_code", "claude_code"],
skill: true,
},
);
expect(selected).toEqual([["claude_code"]]);
expect(result.scope).toMatchObject({
mode: "explicit",
clients: [],
skill_clients: ["claude_code"],
skill: true,
});
expect(result.healthy).toBe(false);
expect(result.scope_checks.map(({ name }) => name)).toEqual([
"node",
"skill:identity",
]);
});
it("scopes effective readiness to selected clients without hiding drift", async () => {
const registrations: readonly ClientRegistrationStatus[] = [
{
+4 -2
View File
@@ -120,6 +120,7 @@ export interface DoctorReport {
/** Caller-selected readiness boundary; omitted values retain audit semantics. */
export interface DoctorScope {
readonly clients?: readonly string[];
readonly skillClients?: readonly string[];
readonly providers?: readonly string[];
readonly skill?: boolean;
}
@@ -206,9 +207,10 @@ const collectDoctorIdentity = async (
const installationPaths = (await host.installationPaths?.()) ?? [];
const registrations = (await host.clientRegistrations?.()) ?? [];
const skillClientIds =
scope.mode === "audit-wide"
scope.skill_clients ??
(scope.mode === "audit-wide"
? registrations.map(({ client }) => client)
: scope.clients;
: scope.clients);
const installedSkillIdentity =
await host.installedSkillIdentity?.(skillClientIds);
const skillAligned = skillIdentityAligned(installedSkillIdentity);
+4
View File
@@ -14,6 +14,7 @@ import type {
export interface DoctorScopeReport {
readonly mode: "audit-wide" | "explicit";
readonly clients: readonly string[];
readonly skill_clients?: readonly string[];
readonly providers: readonly string[];
readonly skill: boolean;
readonly target: string | null;
@@ -40,6 +41,9 @@ export const normalizeDoctorScope = (
): DoctorScopeReport => ({
mode: requested === undefined ? "audit-wide" : "explicit",
clients: uniqueSorted(requested?.clients ?? []),
...(requested?.skillClients === undefined
? {}
: { skill_clients: uniqueSorted(requested.skillClients) }),
providers: uniqueSorted(requested?.providers ?? []),
skill: requested?.skill === true,
target: target ?? null,
+4
View File
@@ -159,6 +159,7 @@ export class FakeSetupHost implements SetupHost {
scope === undefined
? environmentHealthy
: (scope.clients?.length ?? 0) === 0 &&
(scope.skillClients?.length ?? 0) === 0 &&
(scope.providers?.length ?? 0) === 0 &&
scope.skill !== true
? true
@@ -169,6 +170,9 @@ export class FakeSetupHost implements SetupHost {
scope: {
mode: scope === undefined ? "audit-wide" : "explicit",
clients: scope?.clients ?? [],
...(scope?.skillClients === undefined
? {}
: { skill_clients: scope.skillClients }),
providers: scope?.providers ?? [],
skill: scope?.skill === true,
target: null,
+1
View File
@@ -232,6 +232,7 @@ describe("setup scoped readiness", () => {
clients: ["codex"],
providers: [],
skill: true,
skillClients: ["codex"],
});
});
+9 -1
View File
@@ -125,7 +125,10 @@ const runSetupWorkflow = async (
: (options.clientIds ??
options.readinessScope?.clients ??
discovery.defaultClientIds);
let selectedSkillClientIds = options.skillClientIds ?? selectedClientIds;
let selectedSkillClientIds =
options.skillClientIds ??
options.readinessScope?.skillClients ??
selectedClientIds;
let skillNeedsInstall = await host.skillNeedsInstall(selectedSkillClientIds);
let skillSelected =
options.installSkill === true ||
@@ -220,6 +223,7 @@ const runSetupWorkflow = async (
installHopper: planDiscovery.installHopper,
installSkill,
skillSelected,
selectedSkillClientIds,
selectedClients: planned.selectedClients,
clientsToConfigure: planned.clientsToConfigure,
};
@@ -519,6 +523,7 @@ const resolvedReadinessScope = (
readonly selectedClients: readonly SetupClient[];
readonly installHopper: boolean;
readonly skillSelected?: boolean;
readonly selectedSkillClientIds: readonly string[];
},
): DoctorScope => ({
clients: selection.selectedClients.map(({ name }) => name),
@@ -526,6 +531,9 @@ const resolvedReadinessScope = (
? ["hopper"]
: (requested?.providers ?? []),
skill: selection.skillSelected === true || requested?.skill === true,
...(selection.skillSelected === true || requested?.skill === true
? { skillClients: selection.selectedSkillClientIds }
: {}),
});
const summarizeDoctor = (report: DoctorReport): SetupDoctorSummary => ({
+3
View File
@@ -263,6 +263,9 @@ const setupRunOptions = (
? {
readinessScope: {
clients: options.client,
...(options.skillClient.length > 0
? { skillClients: options.skillClient }
: {}),
providers: options.installHopper ? ["hopper"] : [],
skill: options.skill ?? agentIntegrationSelected,
},
@@ -0,0 +1,156 @@
import { access, readFile, readdir, writeFile } from "node:fs/promises";
import { join, relative, resolve } from "node:path";
import { expect } from "vitest";
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
import { cliTest } from "../../support/cli/cliFixture.js";
cliTest(
"skill-only setup verifies its selected client copy on install, rerun and repair",
async ({ cli }) => {
const directory = await createTestTempDirectory("rea-selected-skill-");
const selected = join(
directory,
".claude/skills/reverse-engineer-anything",
);
const shared = join(directory, ".agents/skills/reverse-engineer-anything");
const run = (args: readonly string[]) =>
cli.run({
arguments: [...args, "--json"],
cwd: directory,
environment: { USERPROFILE: directory },
});
const preview = await run([
"setup",
"--skill-client",
"claude_code",
"--dry-run",
]);
expect(preview.exitCode, preview.stdout).toBe(0);
expect(preview.json).toMatchObject({
status: "planned",
doctor: {
scope: {
clients: [],
skill: true,
skill_clients: ["claude_code"],
},
},
});
const registered = await run([
"setup",
"--client",
"claude_code",
"--skill=false",
"--yes",
]);
expect(registered.exitCode, registered.stderr).toBe(0);
const configuration = join(directory, ".claude.json");
const before = await readFile(configuration, "utf8");
const args = ["setup", "--skill-client", "claude_code", "--yes"];
const installed = await run(args);
expect(installed.exitCode, installed.stdout).toBe(0);
expect(installed.json).toMatchObject({
status: "ready",
appliedActions: ["installed_skill"],
doctor: {
healthy: true,
scope: { clients: [], skill: true, skill_clients: ["claude_code"] },
identity: { skill: { state: "aligned" } },
},
});
const bundle = resolve("skills/reverse-engineer-anything");
const assertAligned = async () => {
for (const entry of await readdir(bundle, {
recursive: true,
withFileTypes: true,
})) {
if (!entry.isFile()) continue;
const path = relative(bundle, join(entry.parentPath, entry.name));
expect(await readFile(join(selected, path))).toEqual(
await readFile(join(bundle, path)),
);
}
};
await assertAligned();
const repeated = await run(args);
expect(repeated.exitCode, repeated.stdout).toBe(0);
expect(repeated.json).toMatchObject({
status: "ready",
appliedActions: [],
});
await writeFile(
join(selected, "references/native-and-artifacts.md"),
"damaged selected copy\n",
);
const damaged = await run(["doctor", "--client", "claude_code", "--skill"]);
expect(damaged.json).toMatchObject({
healthy: false,
identity: { skill: { state: "stale" } },
});
const repaired = await run(args);
expect(repaired.exitCode, repaired.stdout).toBe(0);
expect(repaired.json).toMatchObject({
status: "ready",
appliedActions: ["installed_skill"],
});
await assertAligned();
await expect(access(shared)).rejects.toMatchObject({ code: "ENOENT" });
expect(await readFile(configuration, "utf8")).toBe(before);
const sharedControl = await run([
"setup",
"--skill-client",
"shared",
"--yes",
]);
expect(sharedControl.exitCode, sharedControl.stdout).toBe(0);
expect(sharedControl.json).toMatchObject({ status: "ready" });
await access(join(shared, "SKILL.md"));
},
);
cliTest(
"setup keeps registration and skill readiness selections independent",
async ({ cli }) => {
const directory = await createTestTempDirectory("rea-independent-skill-");
const result = await cli.run({
arguments: [
"setup",
"--client",
"codex",
"--skill-client",
"claude_code",
"--yes",
"--json",
],
cwd: directory,
environment: { USERPROFILE: directory },
});
expect(result.exitCode, result.stdout).toBe(0);
expect(result.json).toMatchObject({
status: "ready",
doctor: {
healthy: true,
scope: {
clients: ["codex"],
skill_clients: ["claude_code"],
skill: true,
},
identity: { skill: { state: "aligned" } },
},
});
await access(join(directory, ".codex/config.toml"));
await access(
join(directory, ".claude/skills/reverse-engineer-anything/SKILL.md"),
);
await expect(access(join(directory, ".claude.json"))).rejects.toMatchObject(
{ code: "ENOENT" },
);
await expect(
access(join(directory, ".agents/skills/reverse-engineer-anything")),
).rejects.toMatchObject({ code: "ENOENT" });
},
);