From 8cf0dd2bf645d18afe6a9c8493178bdd905c0647 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Fri, 17 Jul 2026 01:28:42 +0000 Subject: [PATCH] refactor(skill): remove legacy skill compatibility --- ...tion_SetupSkill.installCanonicalSkill.html | 2 +- scripts/verify-package.mjs | 25 ++------- src/application/SetupPlan.ts | 2 +- src/application/SetupSkill.ts | 46 ++-------------- src/application/Uninstall.ts | 32 +++-------- src/identity.ts | 1 - tests/setup.test.ts | 4 -- tests/setupLifecycle.test.ts | 4 -- tests/setupSkill.test.ts | 54 +------------------ 9 files changed, 17 insertions(+), 153 deletions(-) diff --git a/docs/api/functions/application_SetupSkill.installCanonicalSkill.html b/docs/api/functions/application_SetupSkill.installCanonicalSkill.html index a8d1fa056..8385130c0 100644 --- a/docs/api/functions/application_SetupSkill.installCanonicalSkill.html +++ b/docs/api/functions/application_SetupSkill.installCanonicalSkill.html @@ -24,7 +24,7 @@
  • home: string

    • Returns Promise<"failed" | "unchanged" | "installed">

    +
  • Defined in src/application/SetupSkill.ts:46
    • diff --git a/scripts/verify-package.mjs b/scripts/verify-package.mjs index 13869b7a7..8fde6da1d 100644 --- a/scripts/verify-package.mjs +++ b/scripts/verify-package.mjs @@ -760,14 +760,9 @@ try { home, ".agents/skills/reverse-engineer-anything/SKILL.md", ); - const legacySkillPath = join(home, ".agents/skills/rea-analysis/SKILL.md"); const siblingSkillPath = join(home, ".agents/skills/unrelated/SKILL.md"); if (supportedSetupHost) { - await mkdir(join(home, ".agents/skills/rea-analysis"), { - recursive: true, - }); await mkdir(join(home, ".agents/skills/unrelated"), { recursive: true }); - await writeFile(legacySkillPath, "stale managed skill\n"); await writeFile(siblingSkillPath, "unrelated skill\n"); } const plannedExecution = await runWithStatus( @@ -780,23 +775,18 @@ try { const plannedClaudeConfig = await readFile(claudeConfig, "utf8"); const plannedCodexConfig = await readFile(codexTarget, "utf8"); const plannedCursorConfig = await readFile(cursorConfig, "utf8"); - const plannedSkill = await readFile(legacySkillPath, "utf8"); if ( planned.status !== "needs_confirmation" || plannedExecution.status !== 1 || planned.appliedActions.length !== 0 || !planned.plannedActions.some(({ kind }) => kind === "configure_client") || - !planned.plannedActions.some( - ({ kind, detail }) => - kind === "install_skill" && detail.includes("back up its SKILL.md"), - ) || + !planned.plannedActions.some(({ kind }) => kind === "install_skill") || plannedClaudeConfig !== '{"existing":true}\n' || plannedCodexConfig !== 'model = "gpt-5"\n' || - plannedCursorConfig !== '{"existing":true}\n' || - plannedSkill !== "stale managed skill\n" + plannedCursorConfig !== '{"existing":true}\n' ) throw new Error( - `packaged setup plan was not read-only: ${JSON.stringify({ status: planned.status, exitCode: plannedExecution.status, plannedKinds: planned.plannedActions.map(({ kind }) => kind), appliedKinds: planned.appliedActions.map(({ kind }) => kind), claudeConfig: plannedClaudeConfig, codexConfig: plannedCodexConfig, cursorConfig: plannedCursorConfig, skill: plannedSkill })}`, + `packaged setup plan was not read-only: ${JSON.stringify({ status: planned.status, exitCode: plannedExecution.status, plannedKinds: planned.plannedActions.map(({ kind }) => kind), appliedKinds: planned.appliedActions.map(({ kind }) => kind), claudeConfig: plannedClaudeConfig, codexConfig: plannedCodexConfig, cursorConfig: plannedCursorConfig })}`, ); } const firstExecution = await runWithStatus( @@ -868,13 +858,8 @@ try { ); if (skill !== canonicalSkill) throw new Error("packaged skill did not match its canonical source"); - if ( - (await pathExists(legacySkillPath)) || - (await readFile(`${legacySkillPath}.rea.backup`, "utf8")) !== - "stale managed skill\n" || - (await readFile(siblingSkillPath, "utf8")) !== "unrelated skill\n" - ) - throw new Error("packaged skill-name migration was not isolated"); + if ((await readFile(siblingSkillPath, "utf8")) !== "unrelated skill\n") + throw new Error("packaged skill installation modified a sibling skill"); const alignedDoctorExecution = await runWithStatus( cli, ["doctor", "--json"], diff --git a/src/application/SetupPlan.ts b/src/application/SetupPlan.ts index a81aac2c5..7a16ff908 100644 --- a/src/application/SetupPlan.ts +++ b/src/application/SetupPlan.ts @@ -45,7 +45,7 @@ export const setupPlan = ( kind: "install_skill" as const, target: `~/.agents/skills/${PRODUCT_IDENTITY.skillName}/SKILL.md`, detail: - "Install or update the bundled REA reverse-engineering skill. If the retired rea-analysis skill exists, back up its SKILL.md beside the retired path, then remove that legacy entry.", + "Install or update the bundled REA reverse-engineering skill.", external: false, }, ] diff --git a/src/application/SetupSkill.ts b/src/application/SetupSkill.ts index 78b59ae7b..ca08af3da 100644 --- a/src/application/SetupSkill.ts +++ b/src/application/SetupSkill.ts @@ -1,4 +1,4 @@ -import { lstat, mkdir, readFile, rm } from "node:fs/promises"; +import { mkdir, readFile, rm } from "node:fs/promises"; import { dirname, join } from "node:path"; import writeFileAtomic from "write-file-atomic"; @@ -36,11 +36,7 @@ export const canonicalSkillNeedsInstall = async ( ): Promise => { try { const { content, destination } = await canonicalSkillFiles(home); - if ((await readOptionalText(destination)) !== content) return true; - for (const name of PRODUCT_IDENTITY.legacySkillNames) - if ((await readOptionalText(skillDestination(home, name))) !== undefined) - return true; - return false; + return (await readOptionalText(destination)) !== content; } catch { return true; } @@ -54,30 +50,13 @@ export const installCanonicalSkill = async ( let backup = `${destination}.rea.backup`; let original: string | undefined; let canonicalChanged = false; - const removedLegacy: Array<{ - readonly content: string; - readonly destination: string; - }> = []; try { const canonical = await canonicalSkillFiles(home); destination = canonical.destination; backup = `${destination}.rea.backup`; const { content } = canonical; original = await readOptionalText(destination); - const legacy: Array<{ - readonly content: string; - readonly destination: string; - }> = []; - for (const name of PRODUCT_IDENTITY.legacySkillNames) { - const legacyDestination = skillDestination(home, name); - const legacyContent = await readOptionalText(legacyDestination); - if (legacyContent !== undefined) { - if ((await lstat(dirname(legacyDestination))).isSymbolicLink()) - throw new Error("legacy skill directory is a symbolic link"); - legacy.push({ content: legacyContent, destination: legacyDestination }); - } - } - if (original === content && legacy.length === 0) return "unchanged"; + if (original === content) return "unchanged"; await mkdir(dirname(destination), { recursive: true }); if (original !== undefined && original !== content) await writeFileAtomic(backup, original, { @@ -92,15 +71,6 @@ export const installCanonicalSkill = async ( }); if ((await readFile(destination, "utf8")) !== content) throw new Error("skill readback mismatch"); - for (const legacySkill of legacy) { - await writeFileAtomic( - `${legacySkill.destination}.rea.backup`, - legacySkill.content, - { encoding: "utf8", mode: 0o600 }, - ); - await rm(legacySkill.destination, { force: true }); - removedLegacy.push(legacySkill); - } return "installed"; } catch { try { @@ -114,16 +84,6 @@ export const installCanonicalSkill = async ( } catch { // The canonical backup remains beside the skill for operator recovery. } - for (const legacySkill of removedLegacy) - try { - await mkdir(dirname(legacySkill.destination), { recursive: true }); - await writeFileAtomic(legacySkill.destination, legacySkill.content, { - encoding: "utf8", - mode: 0o600, - }); - } catch { - // The legacy backup remains beside the skill for operator recovery. - } return "failed"; } }; diff --git a/src/application/Uninstall.ts b/src/application/Uninstall.ts index 869d2428d..2a1dd36f7 100644 --- a/src/application/Uninstall.ts +++ b/src/application/Uninstall.ts @@ -260,32 +260,12 @@ const removeManagedPath = async ( const removeManagedSkills = async ( home: string, fileSystem: UninstallFileSystem, -): Promise => { - const results: UninstallItem[] = []; - for (const name of [ - PRODUCT_IDENTITY.skillName, - ...PRODUCT_IDENTITY.legacySkillNames, - ]) - results.push( - await removeManagedPath( - join(home, ".agents/skills", name), - "skill", - fileSystem, - ), - ); - const failed = results.find(({ status }) => status === "failed"); - if (failed !== undefined) return failed; - const retained = results.find(({ status }) => status === "retained"); - if (retained !== undefined) return retained; - const removed = results.filter(({ status }) => status === "removed"); - if (removed.length > 0) - return item( - "skill", - "removed", - removed.map(({ detail }) => detail).join(" "), - ); - return item("skill", "skipped", "No managed REA skill installation exists."); -}; +): Promise => + removeManagedPath( + join(home, ".agents/skills", PRODUCT_IDENTITY.skillName), + "skill", + fileSystem, + ); const item = ( name: string, diff --git a/src/identity.ts b/src/identity.ts index 7097c2bb3..b1c9dd811 100644 --- a/src/identity.ts +++ b/src/identity.ts @@ -12,7 +12,6 @@ export const PRODUCT_IDENTITY = { mcpCommand: `npx -y ${PACKAGE_METADATA.name}@latest mcp`, mcpServerKey: "rea", skillName: "reverse-engineer-anything", - legacySkillNames: ["rea-analysis"], skillVersion: PACKAGE_METADATA.skillVersion, configFileName: "rea.json", } as const; diff --git a/tests/setup.test.ts b/tests/setup.test.ts index 416138666..d686cfd6c 100644 --- a/tests/setup.test.ts +++ b/tests/setup.test.ts @@ -152,10 +152,6 @@ describe("setup workflow", () => { "configure_client", "install_skill", ]); - expect( - result.plannedActions.find(({ kind }) => kind === "install_skill") - ?.detail, - ).toContain("back up its SKILL.md"); expect(host.hopperInstalls).toBe(0); expect(host.configurations).toBe(0); expect(result.remediation).toBe( diff --git a/tests/setupLifecycle.test.ts b/tests/setupLifecycle.test.ts index 589417fcc..1ac81c274 100644 --- a/tests/setupLifecycle.test.ts +++ b/tests/setupLifecycle.test.ts @@ -150,10 +150,8 @@ describe("agent lifecycle", () => { home, ".agents/skills/reverse-engineer-anything/SKILL.md", ); - const legacySkill = join(home, ".agents/skills/rea-analysis/SKILL.md"); await mkdir(dirname(cursor), { recursive: true }); await mkdir(dirname(skill), { recursive: true }); - await mkdir(dirname(legacySkill), { recursive: true }); await writeFile( cursor, JSON.stringify({ @@ -164,7 +162,6 @@ describe("agent lifecycle", () => { }), ); await writeFile(skill, "managed"); - await writeFile(legacySkill, "legacy managed"); await mkdir(join(home, ".rea"), { recursive: true }); await import("node:fs/promises").then(({ symlink }) => symlink(home, join(home, ".rea/cache")), @@ -175,7 +172,6 @@ describe("agent lifecycle", () => { mcpServers: { other: { command: "other" } }, }); await expect(readFile(skill, "utf8")).rejects.toThrow(); - await expect(readFile(legacySkill, "utf8")).rejects.toThrow(); expect(first.items).toContainEqual( expect.objectContaining({ name: "cache", diff --git a/tests/setupSkill.test.ts b/tests/setupSkill.test.ts index ba9f2c4ad..b9daef1af 100644 --- a/tests/setupSkill.test.ts +++ b/tests/setupSkill.test.ts @@ -1,11 +1,4 @@ -import { - mkdtemp, - mkdir, - readFile, - rm, - symlink, - writeFile, -} from "node:fs/promises"; +import { mkdtemp, mkdir, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; @@ -49,49 +42,4 @@ describe("canonical skill transaction", () => { expect(await canonicalSkillNeedsInstall(home)).toBe(false); expect(await installCanonicalSkill(home)).toBe("unchanged"); }); - - it("backs up and retires the legacy skill name", async () => { - const home = await mkdtemp(join(tmpdir(), "rea-skill-test-")); - roots.push(home); - const legacy = join(home, ".agents/skills/rea-analysis/SKILL.md"); - const destination = join( - home, - ".agents/skills/reverse-engineer-anything/SKILL.md", - ); - await mkdir(dirname(legacy), { recursive: true }); - await writeFile(legacy, "legacy managed skill\n"); - - expect(await canonicalSkillNeedsInstall(home)).toBe(true); - expect(await installCanonicalSkill(home)).toBe("installed"); - expect(await readFile(destination, "utf8")).toContain( - "name: reverse-engineer-anything", - ); - expect(await readFile(`${legacy}.rea.backup`, "utf8")).toBe( - "legacy managed skill\n", - ); - await expect(readFile(legacy, "utf8")).rejects.toThrow(); - expect(await canonicalSkillNeedsInstall(home)).toBe(false); - }); - - it("does not migrate a legacy skill through a directory symlink", async () => { - const home = await mkdtemp(join(tmpdir(), "rea-skill-test-")); - roots.push(home); - const skillsRoot = join(home, ".agents/skills"); - const external = join(home, "external-skill"); - const legacyRoot = join(skillsRoot, "rea-analysis"); - const legacy = join(legacyRoot, "SKILL.md"); - const destination = join(skillsRoot, "reverse-engineer-anything/SKILL.md"); - await mkdir(skillsRoot, { recursive: true }); - await mkdir(external, { recursive: true }); - await writeFile(join(external, "SKILL.md"), "external skill\n"); - await symlink( - external, - legacyRoot, - process.platform === "win32" ? "junction" : "dir", - ); - - expect(await installCanonicalSkill(home)).toBe("failed"); - expect(await readFile(legacy, "utf8")).toBe("external skill\n"); - await expect(readFile(destination, "utf8")).rejects.toThrow(); - }); });