From 4e0f295f90d81ce2cf7bf38056bfd60d2dae1741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Neum=C3=BCller?= Date: Mon, 13 Jul 2026 16:44:03 +0200 Subject: [PATCH 1/4] AI impl --- docs/user-guide/asset-registry-commands.md | 32 +++ .../asset-registry/asset-registry-api.ts | 7 + .../asset-registry.interfaces.ts | 5 + .../asset-registry/asset-registry.service.ts | 64 ++++- src/commands/asset-registry/module.ts | 13 + src/core/utils/file-service.ts | 2 +- .../asset-registry-skills-download.spec.ts | 222 ++++++++++++++++++ .../commands/asset-registry.spec.ts | 35 +++ 8 files changed, 378 insertions(+), 2 deletions(-) create mode 100644 tests/commands/asset-registry/asset-registry-skills-download.spec.ts diff --git a/docs/user-guide/asset-registry-commands.md b/docs/user-guide/asset-registry-commands.md index 7d5e289..67bcb21 100644 --- a/docs/user-guide/asset-registry-commands.md +++ b/docs/user-guide/asset-registry-commands.md @@ -214,6 +214,38 @@ Behavior: - On success the command logs a single confirmation line with the absolute path of the written file. - A missing skill or file returns a clear error such as `Problem getting SKILL.md for 'platform/missing': ...`. +### Download an Entire Skill + +Download a complete skill bundle — its `SKILL.md` plus all reference files — into a new directory named after the skill. Folder structure inside the skill is preserved. Use this to vendor a full copy of a skill into a repo or filesystem in a single command. + +Download a platform skill into the current directory: + +``` +content-cli asset-registry skills download --path platform/content-cli-setup +``` + +Download an asset skill into a specific parent directory: + +``` +content-cli asset-registry skills download \ + --path asset/BOARD_V2/asset-studio-board-v2 \ + --output ./skills +``` + +The command creates a new directory named after the last segment of `--path` (the skill name) inside `--output`. For example, the second command above writes to `./skills/asset-studio-board-v2/`. + +Options: + +- `--path ` (required) – Skill path from `asset-registry skills list` (e.g. `platform/` or `asset//`). +- `--output ` – Parent directory in which the skill directory will be created. Defaults to the current working directory. Created automatically if it does not exist. + +Behavior: + +- The CLI first fetches the skill's file manifest, then downloads each file to `{output}/{skillName}/{relativePath}`, creating intermediate directories as needed. +- Existing local files are overwritten without prompting (consistent with `skills get`). +- On success the command logs a summary line with the number of files written and the absolute path of the skill directory. +- A missing skill returns a clear error such as `Problem listing skill files for 'platform/missing': ...`. If a listed file cannot be downloaded, the command stops on the first failure and reports which file failed; files already written are left in place. + ## Troubleshooting If the asset registry is disabled on your team, commands fail with: diff --git a/src/commands/asset-registry/asset-registry-api.ts b/src/commands/asset-registry/asset-registry-api.ts index 1a50524..eb2e4fc 100644 --- a/src/commands/asset-registry/asset-registry-api.ts +++ b/src/commands/asset-registry/asset-registry-api.ts @@ -63,6 +63,13 @@ export class AssetRegistryApi { .catch((e) => handleAssetRegistryApiError(operation, e)); } + public async listSkillFiles(skillPath: string): Promise { + const url = AssetRegistryApi.endpointUrl("skills", encodePathSegments(skillPath), "files"); + return this.httpClient() + .get(url) + .catch((e) => handleAssetRegistryApiError(`listing skill files for '${skillPath}'`, e)); + } + private buildSkillFileUrl(skillPath: string, filePath?: string): string { const segments = ["skills", encodePathSegments(skillPath)]; if (filePath) { diff --git a/src/commands/asset-registry/asset-registry.interfaces.ts b/src/commands/asset-registry/asset-registry.interfaces.ts index baf0c54..6e50829 100644 --- a/src/commands/asset-registry/asset-registry.interfaces.ts +++ b/src/commands/asset-registry/asset-registry.interfaces.ts @@ -63,3 +63,8 @@ export interface GetSkillFileOptions { file?: string; output?: string; } + +export interface DownloadSkillOptions { + path: string; + output?: string; +} diff --git a/src/commands/asset-registry/asset-registry.service.ts b/src/commands/asset-registry/asset-registry.service.ts index 2890e0d..0a0684d 100644 --- a/src/commands/asset-registry/asset-registry.service.ts +++ b/src/commands/asset-registry/asset-registry.service.ts @@ -1,5 +1,11 @@ import { AssetRegistryApi } from "./asset-registry-api"; -import { AgentSkill, AssetRegistryDescriptor, GetSkillFileOptions, ValidateOptions } from "./asset-registry.interfaces"; +import { + AgentSkill, + AssetRegistryDescriptor, + DownloadSkillOptions, + GetSkillFileOptions, + ValidateOptions, +} from "./asset-registry.interfaces"; import { Context } from "../../core/command/cli-context"; import { fileService, FileService } from "../../core/utils/file-service"; import { FatalError, logger } from "../../core/utils/logger"; @@ -43,6 +49,30 @@ export class AssetRegistryService { logger.info(FileService.fileDownloadedMessage + absolutePath); } + public async downloadSkill(opts: DownloadSkillOptions): Promise { + const skillName = this.resolveSkillName(opts.path); + const parentDir = opts.output ?? "."; + const skillDir = path.resolve(process.cwd(), parentDir, skillName); + + const files = await this.api.listSkillFiles(opts.path); + if (!Array.isArray(files) || files.length === 0) { + logger.info(`No files found for skill '${opts.path}'.`); + return; + } + + for (const rawFilePath of files) { + const filePath = this.normalizeManifestPath(rawFilePath, opts.path); + this.assertPathWithinSkillDir(skillDir, filePath, opts.path); + + const buffer = await this.api.getSkillFile(opts.path, filePath); + fileService.writeBufferToPath(skillDir, filePath, buffer); + } + + logger.info( + `Downloaded ${files.length} file(s) for skill '${opts.path}' to ${skillDir}` + ); + } + private resolveLocalFilename(file?: string): string { if (!file) { return "SKILL.md"; @@ -55,6 +85,38 @@ export class AssetRegistryService { return base; } + private resolveSkillName(skillPath: string): string { + const trimmed = trimSlashes(skillPath ?? ""); + const name = trimmed ? path.basename(trimmed) : ""; + if (!name) { + throw new FatalError(`--path must identify a skill, got '${skillPath}'.`); + } + return name; + } + + private normalizeManifestPath(rawFilePath: unknown, skillPath: string): string { + if (typeof rawFilePath !== "string") { + throw new FatalError( + `Skill manifest for '${skillPath}' contained a non-string entry: ${JSON.stringify(rawFilePath)}.` + ); + } + const trimmed = trimSlashes(rawFilePath); + if (!trimmed) { + throw new FatalError(`Skill manifest for '${skillPath}' contained an empty file path.`); + } + return trimmed; + } + + private assertPathWithinSkillDir(skillDir: string, relativeFilePath: string, skillPath: string): void { + const resolved = path.resolve(skillDir, relativeFilePath); + const rel = path.relative(skillDir, resolved); + if (!rel || rel.startsWith("..") || path.isAbsolute(rel)) { + throw new FatalError( + `Refusing to write file '${relativeFilePath}' from skill '${skillPath}': path escapes the skill directory.` + ); + } + } + public async listSkills(jsonResponse: boolean): Promise { const response = await this.api.listSkills(); diff --git a/src/commands/asset-registry/module.ts b/src/commands/asset-registry/module.ts index 3de46aa..49532b9 100644 --- a/src/commands/asset-registry/module.ts +++ b/src/commands/asset-registry/module.ts @@ -56,6 +56,12 @@ class Module extends IModule { .option("--file ", "Relative path of a reference file within the skill (defaults to SKILL.md)") .option("--output ", "Destination directory (defaults to current working directory)") .action(this.getSkillFile); + + skillsCommand.command("download") + .description("Download an entire skill (SKILL.md and all reference files) into a new directory, preserving folder structure. Existing files are overwritten.") + .requiredOption("--path ", "Skill path from 'skills list' (e.g. platform/ or asset//)") + .option("--output ", "Parent directory in which the skill directory will be created (defaults to current working directory)") + .action(this.downloadSkill); } private async listTypes(context: Context, command: Command, options: OptionValues): Promise { @@ -96,6 +102,13 @@ class Module extends IModule { output: options.output, }); } + + private async downloadSkill(context: Context, command: Command, options: OptionValues): Promise { + await new AssetRegistryService(context).downloadSkill({ + path: options.path, + output: options.output, + }); + } } export = Module; diff --git a/src/core/utils/file-service.ts b/src/core/utils/file-service.ts index 5b30550..414f258 100644 --- a/src/core/utils/file-service.ts +++ b/src/core/utils/file-service.ts @@ -25,7 +25,7 @@ export class FileService { public writeBufferToPath(targetDir: string, filename: string, data: Buffer): string { const resolvedDir = path.resolve(process.cwd(), targetDir); const absolutePath = path.join(resolvedDir, filename); - this.mkdirRecursive(resolvedDir); + this.mkdirRecursive(path.dirname(absolutePath)); this.writeBufferToFileWithGivenName(data, absolutePath); return absolutePath; } diff --git a/tests/commands/asset-registry/asset-registry-skills-download.spec.ts b/tests/commands/asset-registry/asset-registry-skills-download.spec.ts new file mode 100644 index 0000000..eb42d16 --- /dev/null +++ b/tests/commands/asset-registry/asset-registry-skills-download.spec.ts @@ -0,0 +1,222 @@ +import * as fs from "node:fs"; +import * as path from "path"; +import { mockAxiosGet, mockAxiosGetError } from "../../utls/http-requests-mock"; +import { AssetRegistryService } from "../../../src/commands/asset-registry/asset-registry.service"; +import { testContext } from "../../utls/test-context"; +import { loggingTestTransport } from "../../jest.setup"; +import { FatalError } from "../../../src/core/utils/logger"; +import { uniqueDirName } from "../../utls/fs-utils"; + +const SKILLS_BASE_URL = "https://myTeam.celonis.cloud/pacman/api/core/asset-registry/skills"; + +function absoluteOutputDir(outputDir: string): string { + return path.resolve(process.cwd(), outputDir); +} + +describe("Asset registry skills download", () => { + const skillMdContent = Buffer.from("# Hello SKILL\n", "utf-8"); + const styleContent = Buffer.from("body { color: red; }\n", "utf-8"); + const exampleContent = Buffer.from("example content\n", "utf-8"); + + it("Should download all files listed by the manifest into a new skill directory (platform skill)", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/files`, [ + "SKILL.md", + "refs/style.md", + "refs/nested/example.md", + ]); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/SKILL.md`, skillMdContent); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/refs/style.md`, styleContent); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/refs/nested/example.md`, exampleContent); + + const output = uniqueDirName(); + + await new AssetRegistryService(testContext).downloadSkill({ + path: "platform/foo", + output, + }); + + const skillDir = path.join(absoluteOutputDir(output), "foo"); + expect(fs.existsSync(skillDir)).toBe(true); + + const skillMd = path.join(skillDir, "SKILL.md"); + const style = path.join(skillDir, "refs", "style.md"); + const example = path.join(skillDir, "refs", "nested", "example.md"); + + expect(fs.readFileSync(skillMd).equals(skillMdContent)).toBe(true); + expect(fs.readFileSync(style).equals(styleContent)).toBe(true); + expect(fs.readFileSync(example).equals(exampleContent)).toBe(true); + + const summaryLog = loggingTestTransport.logMessages[loggingTestTransport.logMessages.length - 1]; + expect(summaryLog.message).toContain("Downloaded 3 file(s) for skill 'platform/foo'"); + expect(summaryLog.message).toContain(skillDir); + }); + + it("Should use the last path segment as the skill directory name for asset skills", async () => { + mockAxiosGet( + `${SKILLS_BASE_URL}/asset/BOARD_V2/board-authoring/files`, + ["SKILL.md"] + ); + mockAxiosGet(`${SKILLS_BASE_URL}/asset/BOARD_V2/board-authoring/SKILL.md`, skillMdContent); + + const output = uniqueDirName(); + + await new AssetRegistryService(testContext).downloadSkill({ + path: "asset/BOARD_V2/board-authoring", + output, + }); + + const skillDir = path.join(absoluteOutputDir(output), "board-authoring"); + expect(fs.existsSync(path.join(skillDir, "SKILL.md"))).toBe(true); + }); + + it("Should default --output to the current working directory", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/cwd-default/files`, ["SKILL.md"]); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/cwd-default/SKILL.md`, skillMdContent); + + await new AssetRegistryService(testContext).downloadSkill({ + path: "platform/cwd-default", + }); + + const skillDir = path.join(process.cwd(), "cwd-default"); + expect(fs.readFileSync(path.join(skillDir, "SKILL.md")).equals(skillMdContent)).toBe(true); + }); + + it("Should create the --output directory if it does not exist", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/files`, ["SKILL.md"]); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/SKILL.md`, skillMdContent); + + const output = path.join(uniqueDirName(), "nested", "deep"); + expect(fs.existsSync(absoluteOutputDir(output))).toBe(false); + + await new AssetRegistryService(testContext).downloadSkill({ + path: "platform/foo", + output, + }); + + const skillDir = path.join(absoluteOutputDir(output), "foo"); + expect(fs.existsSync(path.join(skillDir, "SKILL.md"))).toBe(true); + }); + + it("Should overwrite existing local files on re-download", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/files`, ["SKILL.md", "refs/style.md"]); + const newSkill = Buffer.from("NEW SKILL", "utf-8"); + const newStyle = Buffer.from("NEW STYLE", "utf-8"); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/SKILL.md`, newSkill); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/refs/style.md`, newStyle); + + const output = uniqueDirName(); + const skillDir = path.join(absoluteOutputDir(output), "foo"); + fs.mkdirSync(path.join(skillDir, "refs"), { recursive: true }); + fs.writeFileSync(path.join(skillDir, "SKILL.md"), "OLD SKILL"); + fs.writeFileSync(path.join(skillDir, "refs", "style.md"), "OLD STYLE"); + + await new AssetRegistryService(testContext).downloadSkill({ + path: "platform/foo", + output, + }); + + expect(fs.readFileSync(path.join(skillDir, "SKILL.md")).equals(newSkill)).toBe(true); + expect(fs.readFileSync(path.join(skillDir, "refs", "style.md")).equals(newStyle)).toBe(true); + }); + + it("Should URI-encode skill and file segments while preserving slashes", async () => { + const listUrl = `${SKILLS_BASE_URL}/asset/BOARD_V2/${encodeURIComponent("with space")}/files`; + const fileUrl = `${SKILLS_BASE_URL}/asset/BOARD_V2/${encodeURIComponent("with space")}/${encodeURIComponent("dir with space")}/a.md`; + mockAxiosGet(listUrl, ["dir with space/a.md"]); + mockAxiosGet(fileUrl, skillMdContent); + + const output = uniqueDirName(); + + await new AssetRegistryService(testContext).downloadSkill({ + path: "asset/BOARD_V2/with space", + output, + }); + + const written = path.join(absoluteOutputDir(output), "with space", "dir with space", "a.md"); + expect(fs.existsSync(written)).toBe(true); + }); + + it("Should log an info message and skip disk writes when the manifest is empty", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/empty/files`, []); + const output = uniqueDirName(); + + await new AssetRegistryService(testContext).downloadSkill({ + path: "platform/empty", + output, + }); + + expect(loggingTestTransport.logMessages).toHaveLength(1); + expect(loggingTestTransport.logMessages[0].message).toContain( + "No files found for skill 'platform/empty'." + ); + expect(fs.existsSync(path.join(absoluteOutputDir(output), "empty"))).toBe(false); + }); + + it("Should surface a clear FatalError when the manifest endpoint returns 404", async () => { + mockAxiosGetError(`${SKILLS_BASE_URL}/platform/missing/files`, 404, { + error: "Skill not found", + }); + + await expect( + new AssetRegistryService(testContext).downloadSkill({ + path: "platform/missing", + output: uniqueDirName(), + }) + ).rejects.toThrow(/Problem listing skill files for 'platform\/missing':/); + }); + + it("Should surface a clear FatalError when one of the listed files fails to download", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/partial/files`, [ + "SKILL.md", + "refs/missing.md", + ]); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/partial/SKILL.md`, skillMdContent); + mockAxiosGetError(`${SKILLS_BASE_URL}/platform/partial/refs/missing.md`, 404, { + error: "File not found", + }); + + await expect( + new AssetRegistryService(testContext).downloadSkill({ + path: "platform/partial", + output: uniqueDirName(), + }) + ).rejects.toThrow(/Problem getting skill file 'refs\/missing\.md' for 'platform\/partial':/); + }); + + it("Should refuse to write manifest entries that escape the skill directory", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/evil/files`, ["../escape.md"]); + + await expect( + new AssetRegistryService(testContext).downloadSkill({ + path: "platform/evil", + output: uniqueDirName(), + }) + ).rejects.toThrow( + new FatalError( + "Refusing to write file '../escape.md' from skill 'platform/evil': path escapes the skill directory." + ) + ); + }); + + it("Should throw a synchronous FatalError when --path cannot yield a skill name", async () => { + await expect( + new AssetRegistryService(testContext).downloadSkill({ + path: "/", + output: uniqueDirName(), + }) + ).rejects.toThrow(new FatalError("--path must identify a skill, got '/'.")); + }); + + it("Should reject an empty file path in the manifest", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/bad/files`, [""]); + + await expect( + new AssetRegistryService(testContext).downloadSkill({ + path: "platform/bad", + output: uniqueDirName(), + }) + ).rejects.toThrow( + new FatalError("Skill manifest for 'platform/bad' contained an empty file path.") + ); + }); +}); diff --git a/tests/integration/commands/asset-registry.spec.ts b/tests/integration/commands/asset-registry.spec.ts index 0f8c39e..eac4697 100644 --- a/tests/integration/commands/asset-registry.spec.ts +++ b/tests/integration/commands/asset-registry.spec.ts @@ -12,6 +12,8 @@ describe("asset-registry command integration", () => { mockService = { listTypes: jest.fn().mockResolvedValue(undefined), listSkills: jest.fn().mockResolvedValue(undefined), + getSkillFile: jest.fn().mockResolvedValue(undefined), + downloadSkill: jest.fn().mockResolvedValue(undefined), getType: jest.fn().mockResolvedValue(undefined), getSchema: jest.fn().mockResolvedValue(undefined), validate: jest.fn().mockResolvedValue(undefined), @@ -137,6 +139,39 @@ describe("asset-registry command integration", () => { }); }); + describe("asset-registry skills download", () => { + it("calls downloadSkill with --path only", async () => { + const result = await runCli([ + "asset-registry", "skills", "download", + "--path", "platform/foo", + ]); + + expect(result.exitCode).toBe(0); + expect(mockService.downloadSkill).toHaveBeenCalledWith({ + path: "platform/foo", + output: undefined, + }); + }); + + it("forwards --output when provided", async () => { + await runCli([ + "asset-registry", "skills", "download", + "--path", "asset/BOARD_V2/board-authoring", + "--output", "./skills", + ]); + expect(mockService.downloadSkill).toHaveBeenCalledWith({ + path: "asset/BOARD_V2/board-authoring", + output: "./skills", + }); + }); + + it("fails when --path is missing", async () => { + const result = await runCli(["asset-registry", "skills", "download"]); + expect(result.exitCode).not.toBe(0); + expect(mockService.downloadSkill).not.toHaveBeenCalled(); + }); + }); + describe("asset-registry get", () => { it("calls getType with the requested assetType", async () => { const result = await runCli(["asset-registry", "get", "--assetType", "BOARD_V2"]); From a14cd68d6db68e43007d5a20b8fa659735bc5a3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Neum=C3=BCller?= Date: Tue, 14 Jul 2026 14:48:01 +0200 Subject: [PATCH 2/4] Rename GetSkillFileOptions to GetSkillOptions --- src/commands/asset-registry/asset-registry.interfaces.ts | 7 +------ src/commands/asset-registry/asset-registry.service.ts | 4 ++-- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/commands/asset-registry/asset-registry.interfaces.ts b/src/commands/asset-registry/asset-registry.interfaces.ts index 6e50829..a7be8c1 100644 --- a/src/commands/asset-registry/asset-registry.interfaces.ts +++ b/src/commands/asset-registry/asset-registry.interfaces.ts @@ -58,13 +58,8 @@ export interface AgentSkillsResponse { skills: AgentSkill[]; } -export interface GetSkillFileOptions { +export interface GetSkillOptions { path: string; file?: string; output?: string; } - -export interface DownloadSkillOptions { - path: string; - output?: string; -} diff --git a/src/commands/asset-registry/asset-registry.service.ts b/src/commands/asset-registry/asset-registry.service.ts index 0a0684d..983f889 100644 --- a/src/commands/asset-registry/asset-registry.service.ts +++ b/src/commands/asset-registry/asset-registry.service.ts @@ -3,7 +3,7 @@ import { AgentSkill, AssetRegistryDescriptor, DownloadSkillOptions, - GetSkillFileOptions, + GetSkillOptions, ValidateOptions, } from "./asset-registry.interfaces"; import { Context } from "../../core/command/cli-context"; @@ -39,7 +39,7 @@ export class AssetRegistryService { } } - public async getSkillFile(opts: GetSkillFileOptions): Promise { + public async getSkillFile(opts: GetSkillOptions): Promise { const filename = this.resolveLocalFilename(opts.file); const targetDir = opts.output ?? "."; From 147bcefbe2fb1f550d6ea37e221382c0578d686d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Neum=C3=BCller?= Date: Tue, 14 Jul 2026 17:01:50 +0200 Subject: [PATCH 3/4] Replace skill download with --all flag in skill get --- docs/user-guide/asset-registry-commands.md | 44 +--- .../asset-registry/asset-registry-api.ts | 1 + .../asset-registry.interfaces.ts | 1 + .../asset-registry/asset-registry.service.ts | 31 ++- src/commands/asset-registry/module.ts | 23 +- .../asset-registry-skills-download.spec.ts | 222 ------------------ .../asset-registry-skills-get.spec.ts | 122 +++++++++- .../commands/asset-registry.spec.ts | 103 ++++++-- 8 files changed, 239 insertions(+), 308 deletions(-) delete mode 100644 tests/commands/asset-registry/asset-registry-skills-download.spec.ts diff --git a/docs/user-guide/asset-registry-commands.md b/docs/user-guide/asset-registry-commands.md index 67bcb21..1372221 100644 --- a/docs/user-guide/asset-registry-commands.md +++ b/docs/user-guide/asset-registry-commands.md @@ -201,50 +201,32 @@ content-cli asset-registry skills get \ --output ./skills ``` -Options: - -- `--path ` (required) – Skill path from `asset-registry skills list` (e.g. `platform/` or `asset//`). -- `--file ` – Relative path of a reference file within the skill. Defaults to `SKILL.md` when omitted. -- `--output ` – Destination directory. Defaults to the current working directory. Created automatically if it does not exist. - -Behavior: - -- The local filename is the basename of `--file` (or `SKILL.md` when `--file` is omitted). Subdirectories in `--file` are not preserved on the local side. -- Re-running the command overwrites the existing local file without prompting. -- On success the command logs a single confirmation line with the absolute path of the written file. -- A missing skill or file returns a clear error such as `Problem getting SKILL.md for 'platform/missing': ...`. - -### Download an Entire Skill - -Download a complete skill bundle — its `SKILL.md` plus all reference files — into a new directory named after the skill. Folder structure inside the skill is preserved. Use this to vendor a full copy of a skill into a repo or filesystem in a single command. - -Download a platform skill into the current directory: - -``` -content-cli asset-registry skills download --path platform/content-cli-setup -``` - -Download an asset skill into a specific parent directory: +Download an entire skill directory: ``` -content-cli asset-registry skills download \ +content-cli asset-registry skills get \ --path asset/BOARD_V2/asset-studio-board-v2 \ + --all \ --output ./skills ``` -The command creates a new directory named after the last segment of `--path` (the skill name) inside `--output`. For example, the second command above writes to `./skills/asset-studio-board-v2/`. +The command creates a new directory named after the last segment of `--path` (the skill name) inside `--output`. For example, the command above writes to `./skills/asset-studio-board-v2/`. + Options: - `--path ` (required) – Skill path from `asset-registry skills list` (e.g. `platform/` or `asset//`). -- `--output ` – Parent directory in which the skill directory will be created. Defaults to the current working directory. Created automatically if it does not exist. +- `--file ` – Relative path of a reference file within the skill. Defaults to `SKILL.md` when omitted. +- `--output ` – Destination directory. Defaults to the current working directory. Created automatically if it does not exist. +- `--all` - Download all skill files (SKILL.md and all reference files). Files are stored to a new directory identical to the skill name under the path provided with --output. Behavior: -- The CLI first fetches the skill's file manifest, then downloads each file to `{output}/{skillName}/{relativePath}`, creating intermediate directories as needed. -- Existing local files are overwritten without prompting (consistent with `skills get`). -- On success the command logs a summary line with the number of files written and the absolute path of the skill directory. -- A missing skill returns a clear error such as `Problem listing skill files for 'platform/missing': ...`. If a listed file cannot be downloaded, the command stops on the first failure and reports which file failed; files already written are left in place. +- The local filename is the basename of `--file` (or `SKILL.md` when `--file` is omitted). Subdirectories in `--file` are not preserved on the local side. --file is mutually exclusive with --all. +- When `--all` is provided, all files are downloaded into a new subdirectory to the provided output dir, the name of which is identical to the name of the skill. --all is mutually exclusive with --file. +- Re-running the command overwrites the existing local file without prompting. +- On success the command logs a single confirmation line with the absolute path of the written file. +- A missing skill or file returns a clear error such as `Problem getting SKILL.md for 'platform/missing': ...`. ## Troubleshooting diff --git a/src/commands/asset-registry/asset-registry-api.ts b/src/commands/asset-registry/asset-registry-api.ts index eb2e4fc..7998ab1 100644 --- a/src/commands/asset-registry/asset-registry-api.ts +++ b/src/commands/asset-registry/asset-registry-api.ts @@ -67,6 +67,7 @@ export class AssetRegistryApi { const url = AssetRegistryApi.endpointUrl("skills", encodePathSegments(skillPath), "files"); return this.httpClient() .get(url) + .then(result => result?.files ?? []) .catch((e) => handleAssetRegistryApiError(`listing skill files for '${skillPath}'`, e)); } diff --git a/src/commands/asset-registry/asset-registry.interfaces.ts b/src/commands/asset-registry/asset-registry.interfaces.ts index a7be8c1..86639cc 100644 --- a/src/commands/asset-registry/asset-registry.interfaces.ts +++ b/src/commands/asset-registry/asset-registry.interfaces.ts @@ -62,4 +62,5 @@ export interface GetSkillOptions { path: string; file?: string; output?: string; + all?: boolean; } diff --git a/src/commands/asset-registry/asset-registry.service.ts b/src/commands/asset-registry/asset-registry.service.ts index 983f889..83ee2ff 100644 --- a/src/commands/asset-registry/asset-registry.service.ts +++ b/src/commands/asset-registry/asset-registry.service.ts @@ -2,7 +2,6 @@ import { AssetRegistryApi } from "./asset-registry-api"; import { AgentSkill, AssetRegistryDescriptor, - DownloadSkillOptions, GetSkillOptions, ValidateOptions, } from "./asset-registry.interfaces"; @@ -39,7 +38,24 @@ export class AssetRegistryService { } } - public async getSkillFile(opts: GetSkillOptions): Promise { + public async getSkill(opts: GetSkillOptions): Promise { + const hasFileOption = !!opts.file + const hasAllOption = opts.all + + if (hasFileOption && hasAllOption) { + throw new FatalError( + "Options --file and --all are mutually exclusive. Use --file to download an individual file (defaults to SKILL.md) or --all to download all files (SKILL.md and reference files)." + ); + } + + if (hasAllOption) { + await this.getSkillDirectory(opts); + } else { + await this.getSingleSkillFile(opts); + } + } + + private async getSingleSkillFile(opts: GetSkillOptions): Promise { const filename = this.resolveLocalFilename(opts.file); const targetDir = opts.output ?? "."; @@ -49,7 +65,7 @@ export class AssetRegistryService { logger.info(FileService.fileDownloadedMessage + absolutePath); } - public async downloadSkill(opts: DownloadSkillOptions): Promise { + private async getSkillDirectory(opts: GetSkillOptions): Promise { const skillName = this.resolveSkillName(opts.path); const parentDir = opts.output ?? "."; const skillDir = path.resolve(process.cwd(), parentDir, skillName); @@ -68,9 +84,7 @@ export class AssetRegistryService { fileService.writeBufferToPath(skillDir, filePath, buffer); } - logger.info( - `Downloaded ${files.length} file(s) for skill '${opts.path}' to ${skillDir}` - ); + logger.info(`Downloaded ${files.length} file(s) for skill '${opts.path}' to ${skillDir}`); } private resolveLocalFilename(file?: string): string { @@ -171,13 +185,10 @@ export class AssetRegistryService { const hasFile = !!opts.file; if (hasFile && (hasNodeKey || hasConfig || !!opts.packageKey)) { - throw new FatalError( - "Option -f is mutually exclusive with --packageKey, --nodeKey and --configuration." - ); + throw new FatalError("Option -f is mutually exclusive with --packageKey, --nodeKey and --configuration."); } if (hasFile) { - return this.parseJson(fileService.readFile(opts.file), `-f ${opts.file}`); } diff --git a/src/commands/asset-registry/module.ts b/src/commands/asset-registry/module.ts index 49532b9..95c1b71 100644 --- a/src/commands/asset-registry/module.ts +++ b/src/commands/asset-registry/module.ts @@ -51,17 +51,12 @@ class Module extends IModule { .action(this.listSkills); skillsCommand.command("get") - .description("Download a skill file (defaults to SKILL.md)") + .description("Download a skill file or the entire skill directory.") .requiredOption("--path ", "Skill path from 'skills list' (e.g. platform/ or asset//)") .option("--file ", "Relative path of a reference file within the skill (defaults to SKILL.md)") .option("--output ", "Destination directory (defaults to current working directory)") - .action(this.getSkillFile); - - skillsCommand.command("download") - .description("Download an entire skill (SKILL.md and all reference files) into a new directory, preserving folder structure. Existing files are overwritten.") - .requiredOption("--path ", "Skill path from 'skills list' (e.g. platform/ or asset//)") - .option("--output ", "Parent directory in which the skill directory will be created (defaults to current working directory)") - .action(this.downloadSkill); + .option("--all", "Download all skill files (SKILL.md and all reference files). Files are stored to a new directory identical to the skill name under the path provided with --output.") + .action(this.getSkill); } private async listTypes(context: Context, command: Command, options: OptionValues): Promise { @@ -95,18 +90,12 @@ class Module extends IModule { await new AssetRegistryService(context).listSkills(!!options.json); } - private async getSkillFile(context: Context, command: Command, options: OptionValues): Promise { - await new AssetRegistryService(context).getSkillFile({ + private async getSkill(context: Context, command: Command, options: OptionValues): Promise { + await new AssetRegistryService(context).getSkill({ path: options.path, file: options.file, output: options.output, - }); - } - - private async downloadSkill(context: Context, command: Command, options: OptionValues): Promise { - await new AssetRegistryService(context).downloadSkill({ - path: options.path, - output: options.output, + all: options.all, }); } } diff --git a/tests/commands/asset-registry/asset-registry-skills-download.spec.ts b/tests/commands/asset-registry/asset-registry-skills-download.spec.ts deleted file mode 100644 index eb42d16..0000000 --- a/tests/commands/asset-registry/asset-registry-skills-download.spec.ts +++ /dev/null @@ -1,222 +0,0 @@ -import * as fs from "node:fs"; -import * as path from "path"; -import { mockAxiosGet, mockAxiosGetError } from "../../utls/http-requests-mock"; -import { AssetRegistryService } from "../../../src/commands/asset-registry/asset-registry.service"; -import { testContext } from "../../utls/test-context"; -import { loggingTestTransport } from "../../jest.setup"; -import { FatalError } from "../../../src/core/utils/logger"; -import { uniqueDirName } from "../../utls/fs-utils"; - -const SKILLS_BASE_URL = "https://myTeam.celonis.cloud/pacman/api/core/asset-registry/skills"; - -function absoluteOutputDir(outputDir: string): string { - return path.resolve(process.cwd(), outputDir); -} - -describe("Asset registry skills download", () => { - const skillMdContent = Buffer.from("# Hello SKILL\n", "utf-8"); - const styleContent = Buffer.from("body { color: red; }\n", "utf-8"); - const exampleContent = Buffer.from("example content\n", "utf-8"); - - it("Should download all files listed by the manifest into a new skill directory (platform skill)", async () => { - mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/files`, [ - "SKILL.md", - "refs/style.md", - "refs/nested/example.md", - ]); - mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/SKILL.md`, skillMdContent); - mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/refs/style.md`, styleContent); - mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/refs/nested/example.md`, exampleContent); - - const output = uniqueDirName(); - - await new AssetRegistryService(testContext).downloadSkill({ - path: "platform/foo", - output, - }); - - const skillDir = path.join(absoluteOutputDir(output), "foo"); - expect(fs.existsSync(skillDir)).toBe(true); - - const skillMd = path.join(skillDir, "SKILL.md"); - const style = path.join(skillDir, "refs", "style.md"); - const example = path.join(skillDir, "refs", "nested", "example.md"); - - expect(fs.readFileSync(skillMd).equals(skillMdContent)).toBe(true); - expect(fs.readFileSync(style).equals(styleContent)).toBe(true); - expect(fs.readFileSync(example).equals(exampleContent)).toBe(true); - - const summaryLog = loggingTestTransport.logMessages[loggingTestTransport.logMessages.length - 1]; - expect(summaryLog.message).toContain("Downloaded 3 file(s) for skill 'platform/foo'"); - expect(summaryLog.message).toContain(skillDir); - }); - - it("Should use the last path segment as the skill directory name for asset skills", async () => { - mockAxiosGet( - `${SKILLS_BASE_URL}/asset/BOARD_V2/board-authoring/files`, - ["SKILL.md"] - ); - mockAxiosGet(`${SKILLS_BASE_URL}/asset/BOARD_V2/board-authoring/SKILL.md`, skillMdContent); - - const output = uniqueDirName(); - - await new AssetRegistryService(testContext).downloadSkill({ - path: "asset/BOARD_V2/board-authoring", - output, - }); - - const skillDir = path.join(absoluteOutputDir(output), "board-authoring"); - expect(fs.existsSync(path.join(skillDir, "SKILL.md"))).toBe(true); - }); - - it("Should default --output to the current working directory", async () => { - mockAxiosGet(`${SKILLS_BASE_URL}/platform/cwd-default/files`, ["SKILL.md"]); - mockAxiosGet(`${SKILLS_BASE_URL}/platform/cwd-default/SKILL.md`, skillMdContent); - - await new AssetRegistryService(testContext).downloadSkill({ - path: "platform/cwd-default", - }); - - const skillDir = path.join(process.cwd(), "cwd-default"); - expect(fs.readFileSync(path.join(skillDir, "SKILL.md")).equals(skillMdContent)).toBe(true); - }); - - it("Should create the --output directory if it does not exist", async () => { - mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/files`, ["SKILL.md"]); - mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/SKILL.md`, skillMdContent); - - const output = path.join(uniqueDirName(), "nested", "deep"); - expect(fs.existsSync(absoluteOutputDir(output))).toBe(false); - - await new AssetRegistryService(testContext).downloadSkill({ - path: "platform/foo", - output, - }); - - const skillDir = path.join(absoluteOutputDir(output), "foo"); - expect(fs.existsSync(path.join(skillDir, "SKILL.md"))).toBe(true); - }); - - it("Should overwrite existing local files on re-download", async () => { - mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/files`, ["SKILL.md", "refs/style.md"]); - const newSkill = Buffer.from("NEW SKILL", "utf-8"); - const newStyle = Buffer.from("NEW STYLE", "utf-8"); - mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/SKILL.md`, newSkill); - mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/refs/style.md`, newStyle); - - const output = uniqueDirName(); - const skillDir = path.join(absoluteOutputDir(output), "foo"); - fs.mkdirSync(path.join(skillDir, "refs"), { recursive: true }); - fs.writeFileSync(path.join(skillDir, "SKILL.md"), "OLD SKILL"); - fs.writeFileSync(path.join(skillDir, "refs", "style.md"), "OLD STYLE"); - - await new AssetRegistryService(testContext).downloadSkill({ - path: "platform/foo", - output, - }); - - expect(fs.readFileSync(path.join(skillDir, "SKILL.md")).equals(newSkill)).toBe(true); - expect(fs.readFileSync(path.join(skillDir, "refs", "style.md")).equals(newStyle)).toBe(true); - }); - - it("Should URI-encode skill and file segments while preserving slashes", async () => { - const listUrl = `${SKILLS_BASE_URL}/asset/BOARD_V2/${encodeURIComponent("with space")}/files`; - const fileUrl = `${SKILLS_BASE_URL}/asset/BOARD_V2/${encodeURIComponent("with space")}/${encodeURIComponent("dir with space")}/a.md`; - mockAxiosGet(listUrl, ["dir with space/a.md"]); - mockAxiosGet(fileUrl, skillMdContent); - - const output = uniqueDirName(); - - await new AssetRegistryService(testContext).downloadSkill({ - path: "asset/BOARD_V2/with space", - output, - }); - - const written = path.join(absoluteOutputDir(output), "with space", "dir with space", "a.md"); - expect(fs.existsSync(written)).toBe(true); - }); - - it("Should log an info message and skip disk writes when the manifest is empty", async () => { - mockAxiosGet(`${SKILLS_BASE_URL}/platform/empty/files`, []); - const output = uniqueDirName(); - - await new AssetRegistryService(testContext).downloadSkill({ - path: "platform/empty", - output, - }); - - expect(loggingTestTransport.logMessages).toHaveLength(1); - expect(loggingTestTransport.logMessages[0].message).toContain( - "No files found for skill 'platform/empty'." - ); - expect(fs.existsSync(path.join(absoluteOutputDir(output), "empty"))).toBe(false); - }); - - it("Should surface a clear FatalError when the manifest endpoint returns 404", async () => { - mockAxiosGetError(`${SKILLS_BASE_URL}/platform/missing/files`, 404, { - error: "Skill not found", - }); - - await expect( - new AssetRegistryService(testContext).downloadSkill({ - path: "platform/missing", - output: uniqueDirName(), - }) - ).rejects.toThrow(/Problem listing skill files for 'platform\/missing':/); - }); - - it("Should surface a clear FatalError when one of the listed files fails to download", async () => { - mockAxiosGet(`${SKILLS_BASE_URL}/platform/partial/files`, [ - "SKILL.md", - "refs/missing.md", - ]); - mockAxiosGet(`${SKILLS_BASE_URL}/platform/partial/SKILL.md`, skillMdContent); - mockAxiosGetError(`${SKILLS_BASE_URL}/platform/partial/refs/missing.md`, 404, { - error: "File not found", - }); - - await expect( - new AssetRegistryService(testContext).downloadSkill({ - path: "platform/partial", - output: uniqueDirName(), - }) - ).rejects.toThrow(/Problem getting skill file 'refs\/missing\.md' for 'platform\/partial':/); - }); - - it("Should refuse to write manifest entries that escape the skill directory", async () => { - mockAxiosGet(`${SKILLS_BASE_URL}/platform/evil/files`, ["../escape.md"]); - - await expect( - new AssetRegistryService(testContext).downloadSkill({ - path: "platform/evil", - output: uniqueDirName(), - }) - ).rejects.toThrow( - new FatalError( - "Refusing to write file '../escape.md' from skill 'platform/evil': path escapes the skill directory." - ) - ); - }); - - it("Should throw a synchronous FatalError when --path cannot yield a skill name", async () => { - await expect( - new AssetRegistryService(testContext).downloadSkill({ - path: "/", - output: uniqueDirName(), - }) - ).rejects.toThrow(new FatalError("--path must identify a skill, got '/'.")); - }); - - it("Should reject an empty file path in the manifest", async () => { - mockAxiosGet(`${SKILLS_BASE_URL}/platform/bad/files`, [""]); - - await expect( - new AssetRegistryService(testContext).downloadSkill({ - path: "platform/bad", - output: uniqueDirName(), - }) - ).rejects.toThrow( - new FatalError("Skill manifest for 'platform/bad' contained an empty file path.") - ); - }); -}); diff --git a/tests/commands/asset-registry/asset-registry-skills-get.spec.ts b/tests/commands/asset-registry/asset-registry-skills-get.spec.ts index 2b2d202..772eb4d 100644 --- a/tests/commands/asset-registry/asset-registry-skills-get.spec.ts +++ b/tests/commands/asset-registry/asset-registry-skills-get.spec.ts @@ -12,6 +12,8 @@ const SKILLS_BASE_URL = "https://myTeam.celonis.cloud/pacman/api/core/asset-regi describe("Asset registry skills get", () => { const skillContent = Buffer.from("# Hello SKILL\n\nLine 2.\n", "utf-8"); + const styleContent = Buffer.from("body { color: red; }\n", "utf-8"); + const exampleContent = Buffer.from("example content\n", "utf-8"); function absoluteOutputDir(outputDir: string): string { return path.resolve(process.cwd(), outputDir); @@ -21,7 +23,7 @@ describe("Asset registry skills get", () => { mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo`, skillContent); const output = uniqueDirName(); - await new AssetRegistryService(testContext).getSkillFile({ + await new AssetRegistryService(testContext).getSkill({ path: "platform/foo", output, }); @@ -40,7 +42,7 @@ describe("Asset registry skills get", () => { mockAxiosGet(`${SKILLS_BASE_URL}/asset/BOARD_V2/board-authoring`, skillContent); const output = uniqueDirName(); - await new AssetRegistryService(testContext).getSkillFile({ + await new AssetRegistryService(testContext).getSkill({ path: "asset/BOARD_V2/board-authoring", output, }); @@ -55,7 +57,7 @@ describe("Asset registry skills get", () => { mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/refs/style.md`, refContent); const output = uniqueDirName(); - await new AssetRegistryService(testContext).getSkillFile({ + await new AssetRegistryService(testContext).getSkill({ path: "platform/foo", file: "refs/style.md", output, @@ -74,7 +76,7 @@ describe("Asset registry skills get", () => { const output = path.join(uniqueDirName(), "nested", "deep"); expect(fs.existsSync(absoluteOutputDir(output))).toBe(false); - await new AssetRegistryService(testContext).getSkillFile({ + await new AssetRegistryService(testContext).getSkill({ path: "platform/foo", output, }); @@ -93,7 +95,7 @@ describe("Asset registry skills get", () => { const target = path.join(absoluteOutputDir(output), "SKILL.md"); fs.writeFileSync(target, "OLD"); - await new AssetRegistryService(testContext).getSkillFile({ + await new AssetRegistryService(testContext).getSkill({ path: "platform/foo", output, }); @@ -104,7 +106,7 @@ describe("Asset registry skills get", () => { it("Should default --output to the current working directory", async () => { mockAxiosGet(`${SKILLS_BASE_URL}/platform/cwd-default`, skillContent); - await new AssetRegistryService(testContext).getSkillFile({ + await new AssetRegistryService(testContext).getSkill({ path: "platform/cwd-default", }); @@ -118,7 +120,7 @@ describe("Asset registry skills get", () => { mockAxiosGet(url, skillContent); const output = uniqueDirName(); - await new AssetRegistryService(testContext).getSkillFile({ + await new AssetRegistryService(testContext).getSkill({ path: "asset/BOARD_V2/with space", output, }); @@ -131,7 +133,7 @@ describe("Asset registry skills get", () => { mockAxiosGetError(`${SKILLS_BASE_URL}/platform/missing`, 404, { error: "Skill not found" }); await expect( - new AssetRegistryService(testContext).getSkillFile({ + new AssetRegistryService(testContext).getSkill({ path: "platform/missing", output: uniqueDirName(), }) @@ -144,7 +146,7 @@ describe("Asset registry skills get", () => { }); await expect( - new AssetRegistryService(testContext).getSkillFile({ + new AssetRegistryService(testContext).getSkill({ path: "platform/foo", file: "refs/missing.md", output: uniqueDirName(), @@ -154,11 +156,111 @@ describe("Asset registry skills get", () => { it("Should throw a synchronous FatalError when --file resolves to an empty basename", async () => { await expect( - new AssetRegistryService(testContext).getSkillFile({ + new AssetRegistryService(testContext).getSkill({ path: "platform/foo", file: "/", output: uniqueDirName(), }) ).rejects.toThrow(new FatalError("--file must point to a file, got '/'.")); }); + + it("Should download all files listed by the manifest into a new skill directory (platform skill)", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/files`, { files: ["SKILL.md", "refs/style.md", "refs/nested/example.md"] }); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/SKILL.md`, skillContent); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/refs/style.md`, styleContent); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/refs/nested/example.md`, exampleContent); + + const output = uniqueDirName(); + + await new AssetRegistryService(testContext).getSkill({ + path: "platform/foo", + output, + all: true, + }); + + const skillDir = path.join(absoluteOutputDir(output), "foo"); + expect(fs.existsSync(skillDir)).toBe(true); + + const skillMd = path.join(skillDir, "SKILL.md"); + const style = path.join(skillDir, "refs", "style.md"); + const example = path.join(skillDir, "refs", "nested", "example.md"); + + expect(fs.readFileSync(skillMd).equals(skillContent)).toBe(true); + expect(fs.readFileSync(style).equals(styleContent)).toBe(true); + expect(fs.readFileSync(example).equals(exampleContent)).toBe(true); + + const summaryLog = loggingTestTransport.logMessages[loggingTestTransport.logMessages.length - 1]; + expect(summaryLog.message).toContain("Downloaded 3 file(s) for skill 'platform/foo'"); + expect(summaryLog.message).toContain(skillDir); + }); + + it("Should use the last path segment as the skill directory name for asset skills", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/asset/BOARD_V2/board-authoring/files`, { files: ["SKILL.md"] }); + mockAxiosGet(`${SKILLS_BASE_URL}/asset/BOARD_V2/board-authoring/SKILL.md`, skillContent); + + const output = uniqueDirName(); + + await new AssetRegistryService(testContext).getSkill({ + path: "asset/BOARD_V2/board-authoring", + output, + all: true, + }); + + const skillDir = path.join(absoluteOutputDir(output), "board-authoring"); + expect(fs.existsSync(path.join(skillDir, "SKILL.md"))).toBe(true); + }); + + it("Should surface a clear FatalError when one of the listed files fails to download", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/partial/files`, { files: ["SKILL.md", "refs/missing.md"] }); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/partial/SKILL.md`, skillContent); + mockAxiosGetError(`${SKILLS_BASE_URL}/platform/partial/refs/missing.md`, 404, { + error: "File not found", + }); + + await expect( + new AssetRegistryService(testContext).getSkill({ + path: "platform/partial", + output: uniqueDirName(), + all: true, + }) + ).rejects.toThrow(/Problem getting skill file 'refs\/missing\.md' for 'platform\/partial':/); + }); + + it("Should refuse to write manifest entries that escape the skill directory", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/evil/files`, { files: ["../escape.md"] }); + + await expect( + new AssetRegistryService(testContext).getSkill({ + path: "platform/evil", + output: uniqueDirName(), + all: true, + }) + ).rejects.toThrow( + new FatalError( + "Refusing to write file '../escape.md' from skill 'platform/evil': path escapes the skill directory." + ) + ); + }); + + it("Should throw a synchronous FatalError when --path cannot yield a skill name", async () => { + await expect( + new AssetRegistryService(testContext).getSkill({ + path: "/", + output: uniqueDirName(), + all: true, + }) + ).rejects.toThrow(new FatalError("--path must identify a skill, got '/'.")); + }); + + it("Should reject an empty file path in the manifest", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/bad/files`, { files: [""] }); + + await expect( + new AssetRegistryService(testContext).getSkill({ + path: "platform/bad", + output: uniqueDirName(), + all: true, + }) + ).rejects.toThrow(new FatalError("Skill manifest for 'platform/bad' contained an empty file path.")); + }); }); diff --git a/tests/integration/commands/asset-registry.spec.ts b/tests/integration/commands/asset-registry.spec.ts index eac4697..ef4f390 100644 --- a/tests/integration/commands/asset-registry.spec.ts +++ b/tests/integration/commands/asset-registry.spec.ts @@ -12,8 +12,7 @@ describe("asset-registry command integration", () => { mockService = { listTypes: jest.fn().mockResolvedValue(undefined), listSkills: jest.fn().mockResolvedValue(undefined), - getSkillFile: jest.fn().mockResolvedValue(undefined), - downloadSkill: jest.fn().mockResolvedValue(undefined), + getSkill: jest.fn().mockResolvedValue(undefined), getType: jest.fn().mockResolvedValue(undefined), getSchema: jest.fn().mockResolvedValue(undefined), validate: jest.fn().mockResolvedValue(undefined), @@ -139,36 +138,104 @@ describe("asset-registry command integration", () => { }); }); - describe("asset-registry skills download", () => { - it("calls downloadSkill with --path only", async () => { + describe("asset-registry skills get", () => { + it("calls getSkill with --path only", async () => { const result = await runCli([ - "asset-registry", "skills", "download", - "--path", "platform/foo", + "asset-registry", "skills", "get", + "--path", "platform/foo" ]); expect(result.exitCode).toBe(0); - expect(mockService.downloadSkill).toHaveBeenCalledWith({ + expect(mockService.getSkill).toHaveBeenCalledWith({ path: "platform/foo", output: undefined, - }); + all: undefined, + file: undefined, + }) }); it("forwards --output when provided", async () => { - await runCli([ - "asset-registry", "skills", "download", - "--path", "asset/BOARD_V2/board-authoring", - "--output", "./skills", + const result = await runCli([ + "asset-registry", + "skills", + "get", + "--path", + "platform/foo", + "--output", + "./skills", ]); - expect(mockService.downloadSkill).toHaveBeenCalledWith({ - path: "asset/BOARD_V2/board-authoring", + + expect(result.exitCode).toBe(0); + expect(mockService.getSkill).toHaveBeenCalledWith({ + path: "platform/foo", output: "./skills", + all: undefined, + file: undefined, }); }); - it("fails when --path is missing", async () => { - const result = await runCli(["asset-registry", "skills", "download"]); - expect(result.exitCode).not.toBe(0); - expect(mockService.downloadSkill).not.toHaveBeenCalled(); + it("forwards --all when provided", async () => { + const result = await runCli([ + "asset-registry", + "skills", + "get", + "--path", + "platform/foo", + "--all", + ]); + + expect(result.exitCode).toBe(0); + expect(mockService.getSkill).toHaveBeenCalledWith({ + path: "platform/foo", + all: true, + file: undefined, + output: undefined, + }); + }); + + it("forwards --file when provided", async () => { + const result = await runCli([ + "asset-registry", + "skills", + "get", + "--path", + "platform/foo", + "--file", + "file", + ]); + + expect(result.exitCode).toBe(0); + expect(mockService.getSkill).toHaveBeenCalledWith({ + path: "platform/foo", + all: undefined, + file: "file", + output: undefined, + }); + }); + + it("fails when both --all and --file are provided", async () => { + mockService.getSkill.mockRejectedValueOnce( + new Error("Options --file and --all are mutually exclusive.") + ); + + const result = await runCli([ + "asset-registry", + "skills", + "get", + "--path", + "platform/foo", + "--file", + "file", + "--all", + ]); + + expect(result.exitCode).toBe(1); + expect(mockService.getSkill).toHaveBeenCalledWith({ + path: "platform/foo", + all: true, + file: "file", + output: undefined, + }); }); }); From 2069fcdfb1632711794093a625e04d3225ad9624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Neum=C3=BCller?= Date: Tue, 14 Jul 2026 17:06:45 +0200 Subject: [PATCH 4/4] AI improvements --- docs/user-guide/asset-registry-commands.md | 16 +-- .../asset-registry-skills-get.spec.ts | 117 ++++++++++++++++++ .../commands/asset-registry.spec.ts | 2 +- 3 files changed, 126 insertions(+), 9 deletions(-) diff --git a/docs/user-guide/asset-registry-commands.md b/docs/user-guide/asset-registry-commands.md index 1372221..bc4359f 100644 --- a/docs/user-guide/asset-registry-commands.md +++ b/docs/user-guide/asset-registry-commands.md @@ -212,21 +212,21 @@ content-cli asset-registry skills get \ The command creates a new directory named after the last segment of `--path` (the skill name) inside `--output`. For example, the command above writes to `./skills/asset-studio-board-v2/`. - Options: - `--path ` (required) – Skill path from `asset-registry skills list` (e.g. `platform/` or `asset//`). -- `--file ` – Relative path of a reference file within the skill. Defaults to `SKILL.md` when omitted. +- `--file ` – Relative path of a reference file within the skill. Defaults to `SKILL.md` when omitted. Mutually exclusive with `--all`. - `--output ` – Destination directory. Defaults to the current working directory. Created automatically if it does not exist. -- `--all` - Download all skill files (SKILL.md and all reference files). Files are stored to a new directory identical to the skill name under the path provided with --output. +- `--all` – Download every file in the skill (`SKILL.md` and all reference files). Files are written into a new directory named after the skill under `--output`, and their folder structure inside the skill is preserved. Mutually exclusive with `--file`. Behavior: -- The local filename is the basename of `--file` (or `SKILL.md` when `--file` is omitted). Subdirectories in `--file` are not preserved on the local side. --file is mutually exclusive with --all. -- When `--all` is provided, all files are downloaded into a new subdirectory to the provided output dir, the name of which is identical to the name of the skill. --all is mutually exclusive with --file. -- Re-running the command overwrites the existing local file without prompting. -- On success the command logs a single confirmation line with the absolute path of the written file. -- A missing skill or file returns a clear error such as `Problem getting SKILL.md for 'platform/missing': ...`. +- Without `--all`, the command downloads a single file (defaults to `SKILL.md`, or the file identified by `--file`). The local filename is the basename of `--file`; subdirectories in `--file` are not preserved on the local side. +- With `--all`, the command first fetches the skill's file manifest, then downloads each file to `{output}/{skillName}/{relativePath}`, creating intermediate directories as needed. If the manifest is empty, the command logs `No files found for skill ''.` and writes nothing. +- Re-running the command overwrites existing local files without prompting. +- On success the command logs the absolute path of the written file (single-file mode), or a summary line like `Downloaded N file(s) for skill '' to ` (`--all` mode). +- Passing both `--file` and `--all` fails with `Options --file and --all are mutually exclusive. ...`. +- A missing skill or file returns a clear error. In single-file mode this is `Problem getting SKILL.md for '': ...` (or `Problem getting skill file '' for '': ...` when `--file` is set). In `--all` mode a missing manifest reports `Problem listing skill files for '': ...`; a manifest entry that fails to download reports `Problem getting skill file '' for '': ...` and stops on the first failure (files already written are left in place). ## Troubleshooting diff --git a/tests/commands/asset-registry/asset-registry-skills-get.spec.ts b/tests/commands/asset-registry/asset-registry-skills-get.spec.ts index 772eb4d..9220706 100644 --- a/tests/commands/asset-registry/asset-registry-skills-get.spec.ts +++ b/tests/commands/asset-registry/asset-registry-skills-get.spec.ts @@ -263,4 +263,121 @@ describe("Asset registry skills get", () => { }) ).rejects.toThrow(new FatalError("Skill manifest for 'platform/bad' contained an empty file path.")); }); + + it("Should throw a FatalError when both --file and --all are provided", async () => { + await expect( + new AssetRegistryService(testContext).getSkill({ + path: "platform/foo", + file: "SKILL.md", + output: uniqueDirName(), + all: true, + }) + ).rejects.toThrow( + new FatalError( + "Options --file and --all are mutually exclusive. Use --file to download an individual file (defaults to SKILL.md) or --all to download all files (SKILL.md and reference files)." + ) + ); + }); + + it("Should default --output to the current working directory when --all is set", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/cwd-default-all/files`, { files: ["SKILL.md"] }); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/cwd-default-all/SKILL.md`, skillContent); + + await new AssetRegistryService(testContext).getSkill({ + path: "platform/cwd-default-all", + all: true, + }); + + const skillDir = path.join(process.cwd(), "cwd-default-all"); + expect(fs.readFileSync(path.join(skillDir, "SKILL.md")).equals(skillContent)).toBe(true); + }); + + it("Should create a nested --output directory that does not exist when --all is set", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/files`, { files: ["SKILL.md"] }); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/SKILL.md`, skillContent); + + const output = path.join(uniqueDirName(), "nested", "deep"); + expect(fs.existsSync(absoluteOutputDir(output))).toBe(false); + + await new AssetRegistryService(testContext).getSkill({ + path: "platform/foo", + output, + all: true, + }); + + const skillDir = path.join(absoluteOutputDir(output), "foo"); + expect(fs.readFileSync(path.join(skillDir, "SKILL.md")).equals(skillContent)).toBe(true); + }); + + it("Should overwrite existing local files on re-download when --all is set", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/files`, { files: ["SKILL.md", "refs/style.md"] }); + const newSkill = Buffer.from("NEW SKILL", "utf-8"); + const newStyle = Buffer.from("NEW STYLE", "utf-8"); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/SKILL.md`, newSkill); + mockAxiosGet(`${SKILLS_BASE_URL}/platform/foo/refs/style.md`, newStyle); + + const output = uniqueDirName(); + const skillDir = path.join(absoluteOutputDir(output), "foo"); + fs.mkdirSync(path.join(skillDir, "refs"), { recursive: true }); + fs.writeFileSync(path.join(skillDir, "SKILL.md"), "OLD SKILL"); + fs.writeFileSync(path.join(skillDir, "refs", "style.md"), "OLD STYLE"); + + await new AssetRegistryService(testContext).getSkill({ + path: "platform/foo", + output, + all: true, + }); + + expect(fs.readFileSync(path.join(skillDir, "SKILL.md")).equals(newSkill)).toBe(true); + expect(fs.readFileSync(path.join(skillDir, "refs", "style.md")).equals(newStyle)).toBe(true); + }); + + it("Should URI-encode multi-segment skill and file paths under --all", async () => { + const listUrl = `${SKILLS_BASE_URL}/asset/BOARD_V2/${encodeURIComponent("with space")}/files`; + const fileUrl = `${SKILLS_BASE_URL}/asset/BOARD_V2/${encodeURIComponent("with space")}/${encodeURIComponent("dir with space")}/a.md`; + mockAxiosGet(listUrl, { files: ["dir with space/a.md"] }); + mockAxiosGet(fileUrl, skillContent); + + const output = uniqueDirName(); + + await new AssetRegistryService(testContext).getSkill({ + path: "asset/BOARD_V2/with space", + output, + all: true, + }); + + const written = path.join(absoluteOutputDir(output), "with space", "dir with space", "a.md"); + expect(fs.existsSync(written)).toBe(true); + }); + + it("Should log 'No files found' and write nothing when the manifest is empty", async () => { + mockAxiosGet(`${SKILLS_BASE_URL}/platform/empty/files`, { files: [] }); + const output = uniqueDirName(); + + await new AssetRegistryService(testContext).getSkill({ + path: "platform/empty", + output, + all: true, + }); + + expect(loggingTestTransport.logMessages).toHaveLength(1); + expect(loggingTestTransport.logMessages[0].message).toContain( + "No files found for skill 'platform/empty'." + ); + expect(fs.existsSync(path.join(absoluteOutputDir(output), "empty"))).toBe(false); + }); + + it("Should surface a clear FatalError when the manifest endpoint returns 404", async () => { + mockAxiosGetError(`${SKILLS_BASE_URL}/platform/missing/files`, 404, { + error: "Skill not found", + }); + + await expect( + new AssetRegistryService(testContext).getSkill({ + path: "platform/missing", + output: uniqueDirName(), + all: true, + }) + ).rejects.toThrow(/Problem listing skill files for 'platform\/missing':/); + }); }); diff --git a/tests/integration/commands/asset-registry.spec.ts b/tests/integration/commands/asset-registry.spec.ts index ef4f390..e77c2f0 100644 --- a/tests/integration/commands/asset-registry.spec.ts +++ b/tests/integration/commands/asset-registry.spec.ts @@ -151,7 +151,7 @@ describe("asset-registry command integration", () => { output: undefined, all: undefined, file: undefined, - }) + }); }); it("forwards --output when provided", async () => {