From bd8d8c359ac7ffc0badfa1f5cc9ab83ad1851343 Mon Sep 17 00:00:00 2001 From: Kevin Cui Date: Mon, 27 Jul 2026 23:49:20 -0400 Subject: [PATCH] chore(skills): remove the legacy gpt-image-2 startup cleanup The TODO(gpt-image-2-removal) compatibility routine deleted the oo-managed `@alwaysmavs/gpt-image-2` skills that older releases had materialized into AI agents and kept as canonical registry sources after the built-in preset was retired. Enough releases have shipped since then, so the startup cleanup has done its job. Drop `legacy-gpt-image-2-cleanup.ts`, its call site in `run-cli.ts`, and the bootstrap test that only guarded the cleanup against `OO_SKILLS_SYNC_DISABLED`; startup now goes straight to the managed skills synchronization. Signed-off-by: Kevin Cui --- src/application/bootstrap/run-cli.test.ts | 42 ---- src/application/bootstrap/run-cli.ts | 11 +- .../skills/legacy-gpt-image-2-cleanup.test.ts | 208 ------------------ .../skills/legacy-gpt-image-2-cleanup.ts | 138 ------------ 4 files changed, 2 insertions(+), 397 deletions(-) delete mode 100644 src/application/commands/skills/legacy-gpt-image-2-cleanup.test.ts delete mode 100644 src/application/commands/skills/legacy-gpt-image-2-cleanup.ts diff --git a/src/application/bootstrap/run-cli.test.ts b/src/application/bootstrap/run-cli.test.ts index ef27b139..e24268b0 100644 --- a/src/application/bootstrap/run-cli.test.ts +++ b/src/application/bootstrap/run-cli.test.ts @@ -282,48 +282,6 @@ describe("runCli bootstrap", () => { } }); - test("preserves legacy managed skills when OO_SKILLS_SYNC_DISABLED is set", async () => { - const sandbox = await createCliSandbox(); - - sandbox.env.OO_SKILLS_SYNC_DISABLED = "1"; - - try { - const managedRegistryPath = join( - resolveManagedSkillAgentHomeDirectory(sandbox.env, "universal"), - "skills", - "gpt-image-2", - ); - - await mkdir(managedRegistryPath, { recursive: true }); - await writeFile(join(managedRegistryPath, "SKILL.md"), "skill\n"); - await writeFile( - join(managedRegistryPath, ".oo-metadata.json"), - JSON.stringify({ - kind: "registry", - packageName: "@alwaysmavs/gpt-image-2", - schemaVersion: 1, - version: "1.0.0", - }), - ); - - const result = await sandbox.run(["--help"]); - - expect(result.exitCode).toBe(0); - // Legacy cleanup is part of the guarded block, so the managed skill - // must remain untouched. - expect((await stat(managedRegistryPath)).isDirectory()).toBe(true); - expect( - (await stat(join(managedRegistryPath, "SKILL.md"))).isFile(), - ).toBe(true); - expect( - (await stat(join(managedRegistryPath, ".oo-metadata.json"))).isFile(), - ).toBe(true); - } - finally { - await sandbox.cleanup(); - } - }); - test("writes debug logs to the log directory during cli startup", async () => { const sandbox = await createCliSandbox(); diff --git a/src/application/bootstrap/run-cli.ts b/src/application/bootstrap/run-cli.ts index c34d1e6e..6626193f 100644 --- a/src/application/bootstrap/run-cli.ts +++ b/src/application/bootstrap/run-cli.ts @@ -36,7 +36,6 @@ import { import { createTranslator } from "../../i18n/translator.ts"; import { createCliCatalog } from "../commands/catalog.ts"; import { synchronizeManagedSkillsForAvailableHosts } from "../commands/skills/auto-sync.ts"; -import { removeLegacyGptImage2ManagedSkills } from "../commands/skills/legacy-gpt-image-2-cleanup.ts"; import { APP_NAME } from "../config/app-config.ts"; import { formatCliVersionText, @@ -301,15 +300,9 @@ export async function executeCli(invocation: CliInvocation): Promise { const adapter = new CommanderCliAdapter(); // OO_SKILLS_SYNC_DISABLED suppresses the startup skills synchronization - // and legacy-cleanup side effects so embedded callers never write skill - // files into other agents' home directories (e.g. ~/.agents, ~/.claude). + // so embedded callers never write skill files into other agents' home + // directories (e.g. ~/.agents, ~/.claude). if (readEnvBoolean(invocation.env.OO_SKILLS_SYNC_DISABLED) !== true) { - // TODO(gpt-image-2-removal): Temporary compatibility cleanup. Remove this - // call and `legacy-gpt-image-2-cleanup.ts` once enough releases have - // shipped that no user still has the oo-managed `@alwaysmavs/gpt-image-2` - // skills materialized in an AI agent. Must run before the sync below so - // the canonical sources are gone before re-publishing could re-create them. - await removeLegacyGptImage2ManagedSkills(context); await synchronizeManagedSkillsForAvailableHosts(context); } diff --git a/src/application/commands/skills/legacy-gpt-image-2-cleanup.test.ts b/src/application/commands/skills/legacy-gpt-image-2-cleanup.test.ts deleted file mode 100644 index 529b059f..00000000 --- a/src/application/commands/skills/legacy-gpt-image-2-cleanup.test.ts +++ /dev/null @@ -1,208 +0,0 @@ -import type { Logger } from "pino"; - -import { mkdir, rm, stat } from "node:fs/promises"; -import { join } from "node:path"; - -import { describe, expect, test } from "bun:test"; - -import { - createLogCapture, - createTemporaryDirectory, -} from "../../../../__tests__/helpers.ts"; -import { removeLegacyGptImage2ManagedSkills } from "./legacy-gpt-image-2-cleanup.ts"; - -const gptImage2Metadata = { - kind: "registry", - packageName: "@alwaysmavs/gpt-image-2", - schemaVersion: 1, - version: "0.0.3", -}; -const otherRegistryMetadata = { - kind: "registry", - packageName: "@alice/demo", - schemaVersion: 1, - version: "1.0.0", -}; -const bundledMetadata = { kind: "bundled", schemaVersion: 1, version: "1.2.3" }; -const localMetadata = { kind: "local", schemaVersion: 1 }; - -describe("legacy @alwaysmavs/gpt-image-2 managed skill cleanup", () => { - test("removes oo-managed gpt-image-2 skills from AI agents while preserving other, local, and unmanaged skills", async () => { - const rootDirectory = await createTemporaryDirectory("oo-legacy-gpt-image-2"); - const configDirectoryPath = join(rootDirectory, "config"); - const settingsFilePath = join(configDirectoryPath, "settings.toml"); - const universalSkillsDirectoryPath = join(rootDirectory, ".agents", "skills"); - const gptImage2SkillPath = join(universalSkillsDirectoryPath, "gpt-image-2"); - const gptImage2EditSkillPath = join(universalSkillsDirectoryPath, "gpt-image-2-edit"); - const otherRegistrySkillPath = join(universalSkillsDirectoryPath, "demo"); - const bundledSkillPath = join(universalSkillsDirectoryPath, "oo"); - const localSkillPath = join(universalSkillsDirectoryPath, "mine"); - const unmanagedSkillPath = join(universalSkillsDirectoryPath, "custom"); - const logCapture = createLogCapture(); - - try { - await writeManagedSkill(gptImage2SkillPath, gptImage2Metadata); - await writeManagedSkill(gptImage2EditSkillPath, gptImage2Metadata); - await writeManagedSkill(otherRegistrySkillPath, otherRegistryMetadata); - await writeManagedSkill(bundledSkillPath, bundledMetadata); - await writeManagedSkill(localSkillPath, localMetadata); - await writeUnmanagedSkill(unmanagedSkillPath); - - await removeLegacyGptImage2ManagedSkills( - createCleanupContext({ - env: { HOME: rootDirectory, USERPROFILE: rootDirectory }, - logger: logCapture.logger as unknown as Logger, - settingsFilePath, - }), - ); - - await expect(stat(gptImage2SkillPath)).rejects.toMatchObject({ - code: "ENOENT", - }); - await expect(stat(gptImage2EditSkillPath)).rejects.toMatchObject({ - code: "ENOENT", - }); - expect((await stat(otherRegistrySkillPath)).isDirectory()).toBe(true); - expect((await stat(bundledSkillPath)).isDirectory()).toBe(true); - expect((await stat(localSkillPath)).isDirectory()).toBe(true); - expect((await stat(unmanagedSkillPath)).isDirectory()).toBe(true); - } - finally { - logCapture.close(); - await rm(rootDirectory, { force: true, recursive: true }); - } - }); - - test("removes the canonical registry sources for gpt-image-2 while preserving other packages", async () => { - const rootDirectory = await createTemporaryDirectory("oo-legacy-gpt-image-2"); - const configDirectoryPath = join(rootDirectory, "config"); - const settingsFilePath = join(configDirectoryPath, "settings.toml"); - const canonicalRegistryPath = join(configDirectoryPath, "skills", "registry"); - const canonicalGptImage2Path = join(canonicalRegistryPath, "gpt-image-2"); - const canonicalOtherPath = join(canonicalRegistryPath, "demo"); - const logCapture = createLogCapture(); - - try { - await writeManagedSkill(canonicalGptImage2Path, gptImage2Metadata); - await writeManagedSkill(canonicalOtherPath, otherRegistryMetadata); - - await removeLegacyGptImage2ManagedSkills( - createCleanupContext({ - env: { HOME: rootDirectory, USERPROFILE: rootDirectory }, - logger: logCapture.logger as unknown as Logger, - settingsFilePath, - }), - ); - - await expect(stat(canonicalGptImage2Path)).rejects.toMatchObject({ - code: "ENOENT", - }); - expect((await stat(canonicalOtherPath)).isDirectory()).toBe(true); - } - finally { - logCapture.close(); - await rm(rootDirectory, { force: true, recursive: true }); - } - }); - - test("is a silent no-op when there is nothing to clean up", async () => { - const rootDirectory = await createTemporaryDirectory("oo-legacy-gpt-image-2"); - const configDirectoryPath = join(rootDirectory, "config"); - const settingsFilePath = join(configDirectoryPath, "settings.toml"); - const logCapture = createLogCapture(); - - try { - await mkdir(configDirectoryPath, { recursive: true }); - - await removeLegacyGptImage2ManagedSkills( - createCleanupContext({ - env: { HOME: rootDirectory, USERPROFILE: rootDirectory }, - logger: logCapture.logger as unknown as Logger, - settingsFilePath, - }), - ); - - logCapture.close(); - expect(logCapture.read()).toBe(""); - } - finally { - await rm(rootDirectory, { force: true, recursive: true }); - } - }); - - test("completes the canonical cleanup even when the host cleanup fails", async () => { - const rootDirectory = await createTemporaryDirectory("oo-legacy-gpt-image-2"); - const configDirectoryPath = join(rootDirectory, "config"); - const settingsFilePath = join(configDirectoryPath, "settings.toml"); - const universalHomeDirectory = join(rootDirectory, ".agents"); - const universalSkillsPath = join(universalHomeDirectory, "skills"); - const canonicalGptImage2Path = join( - configDirectoryPath, - "skills", - "registry", - "gpt-image-2", - ); - const logCapture = createLogCapture(); - - try { - // Make the universal skills path a file so readdir fails with a - // non-ENOENT error, forcing the host-cleanup branch to reject. - await mkdir(universalHomeDirectory, { recursive: true }); - await Bun.write(universalSkillsPath, "not a directory\n"); - await writeManagedSkill(canonicalGptImage2Path, gptImage2Metadata); - - await removeLegacyGptImage2ManagedSkills( - createCleanupContext({ - env: { HOME: rootDirectory, USERPROFILE: rootDirectory }, - logger: logCapture.logger as unknown as Logger, - settingsFilePath, - }), - ); - - // The host branch rejected, but the canonical branch must still have - // run to completion (Promise.allSettled, not a fail-fast Promise.all). - await expect(stat(canonicalGptImage2Path)).rejects.toMatchObject({ - code: "ENOENT", - }); - - logCapture.close(); - expect(logCapture.read()).toContain( - "Legacy @alwaysmavs/gpt-image-2 managed skill cleanup failed.", - ); - } - finally { - await rm(rootDirectory, { force: true, recursive: true }); - } - }); -}); - -async function writeManagedSkill( - skillDirectoryPath: string, - metadata: object, -): Promise { - await mkdir(skillDirectoryPath, { recursive: true }); - await Bun.write(join(skillDirectoryPath, "SKILL.md"), "skill\n"); - await Bun.write( - join(skillDirectoryPath, ".oo-metadata.json"), - JSON.stringify(metadata), - ); -} - -async function writeUnmanagedSkill(skillDirectoryPath: string): Promise { - await mkdir(skillDirectoryPath, { recursive: true }); - await Bun.write(join(skillDirectoryPath, "SKILL.md"), "user authored\n"); -} - -function createCleanupContext(options: { - env: Record; - logger: Logger; - settingsFilePath: string; -}): Parameters[0] { - return { - env: options.env, - logger: options.logger, - settingsStore: { - getFilePath: () => options.settingsFilePath, - } as never, - }; -} diff --git a/src/application/commands/skills/legacy-gpt-image-2-cleanup.ts b/src/application/commands/skills/legacy-gpt-image-2-cleanup.ts deleted file mode 100644 index d520ae7d..00000000 --- a/src/application/commands/skills/legacy-gpt-image-2-cleanup.ts +++ /dev/null @@ -1,138 +0,0 @@ -import type { CliExecutionContext } from "../../contracts/cli.ts"; - -import { join } from "node:path"; -import { removePath } from "./bundled-skill-filesystem.ts"; -import { resolveAvailableManagedSkillHosts } from "./managed-skill-hosts.ts"; -import { readSkillsDirectoryEntries } from "./managed-skill-listings.ts"; -import { - resolveManagedSkillCanonicalRootDirectoryPath, - resolveManagedSkillsDirectoryPath, -} from "./managed-skill-paths.ts"; -import { - managedMetadataOfKind, - readSkillDirectoryState, -} from "./skill-directory-state.ts"; - -// Registry skill package the CLI used to ship as a built-in preset. -const legacyGptImage2PackageName = "@alwaysmavs/gpt-image-2"; - -type LegacyGptImage2CleanupContext = Pick< - CliExecutionContext, - "env" | "logger" | "settingsStore" ->; - -/** - * TODO(gpt-image-2-removal): Temporary compatibility cleanup. Remove this file - * and its call site in `run-cli.ts` once enough releases have shipped that no - * user still has the oo-managed `@alwaysmavs/gpt-image-2` skills materialized in - * an AI agent or kept as canonical registry sources. - * - * `@alwaysmavs/gpt-image-2` used to be a built-in preset: at startup the CLI - * downloaded the package and released its skills into every AI agent alongside - * the bundled skills, keeping canonical registry sources under - * `/skills/registry`. That preset path is gone, so this routine deletes - * the skills oo itself materialized for that package — both the per-agent - * installs and the canonical sources that `synchronizeManagedSkillsForAvailableHosts` - * would otherwise keep re-publishing. Skills are identified strictly by their - * `.oo-metadata.json` (registry metadata whose `packageName` matches the legacy - * preset package); user-authored and unrelated skills are preserved. - */ -export async function removeLegacyGptImage2ManagedSkills( - context: LegacyGptImage2CleanupContext, -): Promise { - // Best-effort cleanup: run both branches to completion and never short-circuit - // on the first failure, so a failing branch cannot leave the other unfinished. - const results = await Promise.allSettled([ - removeLegacyGptImage2HostInstalls(context), - removeLegacyGptImage2CanonicalSources(context), - ]); - - for (const result of results) { - if (result.status === "rejected") { - context.logger.warn( - { err: result.reason }, - "Legacy @alwaysmavs/gpt-image-2 managed skill cleanup failed.", - ); - } - } -} - -async function removeLegacyGptImage2HostInstalls( - context: LegacyGptImage2CleanupContext, -): Promise { - const hosts = await resolveAvailableManagedSkillHosts(context.env); - - await Promise.all( - hosts.map(host => - removeLegacyGptImage2SkillsInDirectory( - resolveManagedSkillsDirectoryPath(host.homeDirectory), - context, - ), - ), - ); -} - -async function removeLegacyGptImage2CanonicalSources( - context: LegacyGptImage2CleanupContext, -): Promise { - await removeLegacyGptImage2SkillsInDirectory( - resolveManagedSkillCanonicalRootDirectoryPath( - context.settingsStore.getFilePath(), - ), - context, - ); -} - -async function removeLegacyGptImage2SkillsInDirectory( - skillsDirectoryPath: string, - context: LegacyGptImage2CleanupContext, -): Promise { - const entryNames = await readSkillsDirectoryEntries(skillsDirectoryPath); - - await Promise.all( - entryNames.map(entryName => - removeLegacyGptImage2Skill( - join(skillsDirectoryPath, entryName), - entryName, - context, - ), - ), - ); -} - -async function removeLegacyGptImage2Skill( - skillDirectoryPath: string, - skillName: string, - context: LegacyGptImage2CleanupContext, -): Promise { - try { - if (!(await isLegacyGptImage2SkillDirectory(skillDirectoryPath))) { - return; - } - - await removePath(skillDirectoryPath); - context.logger.info( - { path: skillDirectoryPath, skillName }, - "Removed oo-managed @alwaysmavs/gpt-image-2 skill.", - ); - } - catch (error) { - context.logger.warn( - { err: error, path: skillDirectoryPath, skillName }, - "Failed to remove oo-managed @alwaysmavs/gpt-image-2 skill.", - ); - } -} - -async function isLegacyGptImage2SkillDirectory( - skillDirectoryPath: string, -): Promise { - // Only registry metadata is matched, so bundled and local skills (and - // directories without `.oo-metadata.json`) never match. - const metadata = managedMetadataOfKind( - await readSkillDirectoryState(skillDirectoryPath), - "registry", - ); - - return metadata?.packageName === legacyGptImage2PackageName; -}