diff --git a/.gitignore b/.gitignore index bb42a3ad9..3fb68ddf4 100644 --- a/.gitignore +++ b/.gitignore @@ -217,11 +217,9 @@ mcp-schema.json docs/.vitepress/dist docs/.vitepress/cache -**/.junie/skills/ -**/.junie/agents/ - # Generated by Rulesync .rulesync/skills/.curated/ +.rulesync/.sources/ **/AGENTS.md **/.agents/ **/.augmentignore @@ -273,6 +271,8 @@ docs/.vitepress/cache **/.vscode/mcp.json **/.junie/guidelines.md **/.junie/mcp.json +**/.junie/skills/ +**/.junie/agents/ **/.kilocode/rules/ **/.kilocode/skills/ **/.kilocode/workflows/ diff --git a/docs/guide/declarative-sources.md b/docs/guide/declarative-sources.md index 664e4fe06..542cd537e 100644 --- a/docs/guide/declarative-sources.md +++ b/docs/guide/declarative-sources.md @@ -1,6 +1,6 @@ -# Declarative Skill Sources +# Declarative Sources -Rulesync can fetch skills from external repositories using the `install` command. Instead of manually running `fetch` for each skill source, declare them in your `rulesync.jsonc` and run `rulesync install` to resolve and fetch them. Then `rulesync generate` picks them up as local curated skills. Typical workflow: `rulesync install && rulesync generate`. +Rulesync can fetch features (skills, rules, commands, subagents, mcp, hooks, ignore) from external repositories using the `install` command. Instead of manually running `fetch` for each source, declare them in your `rulesync.jsonc` and run `rulesync install` to resolve and fetch them. Then `rulesync generate` picks them up alongside local definitions. Typical workflow: `rulesync install && rulesync generate`. ## Configuration @@ -12,21 +12,24 @@ Add a `sources` array to your `rulesync.jsonc`: "targets": ["copilot", "claudecode"], "features": ["rules", "skills"], "sources": [ - // Fetch all skills from a GitHub repository (default transport) + // Fetch all features from a GitHub repository (default transport) { "source": "owner/repo" }, + // Fetch only specific feature types + { "source": "owner/repo", "features": ["rules", "commands"] }, + // Fetch only specific skills by name { "source": "anthropics/skills", "skills": ["skill-creator"] }, // With ref pinning and subdirectory path (same syntax as fetch command) - { "source": "owner/repo@v1.0.0:path/to/skills" }, + { "source": "owner/repo@v1.0.0:path/to/rulesync" }, // Git transport — works with any git remote (Azure DevOps, Bitbucket, etc.) { "source": "https://dev.azure.com/org/project/_git/repo", "transport": "git", "ref": "main", - "path": "exports/skills", + "path": "exports/rulesync", }, // Git transport with a local repository @@ -37,44 +40,46 @@ Add a `sources` array to your `rulesync.jsonc`: Each entry in `sources` accepts: -| Property | Type | Description | -| ----------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------- | -| `source` | `string` | Repository source. For GitHub transport: `owner/repo` or `owner/repo@ref:path`. For git transport: a full git URL. | -| `skills` | `string[]` | Optional list of skill names to fetch. If omitted, all skills are fetched. | -| `transport` | `string` | `"github"` (default) uses the GitHub REST API. `"git"` uses git CLI and works with any git remote. | -| `ref` | `string` | Branch, tag, or ref to fetch from. Defaults to the remote's default branch. For GitHub transport, use the `@ref` source syntax. | -| `path` | `string` | Path to the skills directory within the repository. Defaults to `"skills"`. For GitHub transport, use the `:path` source syntax. | +| Property | Type | Description | +| ----------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `source` | `string` | Repository source. For GitHub transport: `owner/repo` or `owner/repo@ref:path`. For git transport: a full git URL. | +| `features` | `string[]` | Optional list of feature types to fetch (`skills`, `rules`, `commands`, `subagents`, `mcp`, `hooks`, `ignore`, or `*`). Defaults to all (`["*"]`). | +| `skills` | `string[]` | Optional list of skill names to fetch. If omitted, all skills are fetched. | +| `transport` | `string` | `"github"` (default) uses the GitHub REST API. `"git"` uses git CLI and works with any git remote. | +| `ref` | `string` | Branch, tag, or ref to fetch from. Defaults to the remote's default branch. For GitHub transport, use the `@ref` source syntax. | +| `path` | `string` | Base path within the repository where rulesync content lives. All feature directories (skills/, rules/, etc.) and feature files (mcp.json, etc.) are resolved relative to this path. Defaults to the repository root. For GitHub transport, use the `:path` source syntax. | ## How It Works When `rulesync install` runs and `sources` is configured: -1. **Lockfile resolution** — Each source's ref is resolved to a commit SHA and stored in `rulesync.lock` (at the project root). On subsequent runs the locked SHA is reused for deterministic builds. -2. **Remote skill listing** — The `skills/` directory (or the path specified in the source URL) is listed from the remote repository. -3. **Filtering** — If `skills` is specified, only matching skill directories are fetched. -4. **Precedence rules**: - - **Local skills always win** — Skills in `.rulesync/skills/` (not in `.curated/`) take precedence; a remote skill with the same name is skipped. - - **First-declared source wins** — If two sources provide a skill with the same name, the one declared first in the `sources` array is used. -5. **Output** — Fetched skills are written to `.rulesync/skills/.curated//`. This directory is automatically added to `.gitignore` by `rulesync gitignore`. +1. **Lockfile resolution** -- Each source's ref is resolved to a commit SHA and stored in `rulesync.lock` (at the project root). On subsequent runs the locked SHA is reused for deterministic builds. +2. **Feature fetching** -- For each source, the requested features are fetched from the remote repository. Directory features (skills, rules, commands, subagents) are fetched recursively. Single-file features (mcp.json, hooks.json, .aiignore) are fetched individually. +3. **Source cache** -- Fetched content is written to `.rulesync/.sources//`, preserving the rulesync directory structure. This cache is used by `rulesync generate`. +4. **Filtering** -- If `skills` is specified, only matching skill directories are fetched. If `features` is specified, only matching feature types are fetched. +5. **Precedence rules**: + - **Local items always win** -- Items in `.rulesync//` take precedence; a remote item with the same name is skipped. + - **First-declared source wins** -- If two sources provide an item with the same name, the one declared first in the `sources` array is used. + - **Single-file features merge** -- For mcp.json, hooks.json, and .aiignore, content is merged across sources. Local content is applied first, then sources in declaration order. Server entries, event keys, and ignore lines are merged with local values taking precedence. ## CLI Options The `install` command accepts these flags: -| Flag | Description | -| ----------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `--update` | Force re-resolve all source refs, ignoring the lockfile (useful to pull new updates). | -| `--frozen` | Fail if lockfile is missing or out of sync. Fetches missing skills using locked refs without updating the lockfile. Useful for CI to ensure reproducibility. | -| `--token ` | GitHub token for private repositories. | +| Flag | Description | +| ----------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `--update` | Force re-resolve all source refs, ignoring the lockfile (useful to pull new updates). | +| `--frozen` | Fail if lockfile is missing or out of sync. Fetches missing content using locked refs without updating the lockfile. Useful for CI to ensure reproducibility. | +| `--token ` | GitHub token for private repositories. | ```bash -# Install skills using locked refs +# Install sources using locked refs rulesync install # Force update to latest refs rulesync install --update -# Strict CI mode — fail if lockfile doesn't cover all sources (missing locked skills are fetched) +# Strict CI mode — fail if lockfile doesn't cover all sources rulesync install --frozen # Install then generate @@ -86,19 +91,20 @@ rulesync generate ## Lockfile -The lockfile at `rulesync.lock` (at the project root) records the resolved commit SHA and per-skill integrity hashes for each source so that builds are reproducible. It is safe to commit this file. An example: +The lockfile at `rulesync.lock` (at the project root) records the resolved commit SHA and per-file integrity hashes for each source so that builds are reproducible. It is safe to commit this file. An example: ```json { - "lockfileVersion": 1, + "lockfileVersion": 2, "sources": { - "owner/skill-repo": { + "owner/repo": { "requestedRef": "main", "resolvedRef": "abc123def456...", "resolvedAt": "2025-01-15T12:00:00.000Z", - "skills": { - "my-skill": { "integrity": "sha256-abcdef..." }, - "another-skill": { "integrity": "sha256-123456..." } + "files": { + "skills/my-skill/SKILL.md": { "integrity": "sha256-abcdef..." }, + "rules/coding-standards.md": { "integrity": "sha256-123456..." }, + "mcp.json": { "integrity": "sha256-789abc..." } } } } @@ -123,11 +129,11 @@ GITHUB_TOKEN=$(gh auth token) npx rulesync install > [!TIP] > The `install` command also accepts a `--token` flag for explicit authentication: `rulesync install --token ghp_xxxx`. -## Curated vs Local Skills +## Source Cache vs Local Content -| Location | Type | Precedence | Committed to Git | -| ----------------------------------- | ------- | ---------- | ---------------- | -| `.rulesync/skills//` | Local | Highest | Yes | -| `.rulesync/skills/.curated//` | Curated | Lower | No (gitignored) | +| Location | Type | Precedence | Committed to Git | +| ---------------------------------------- | ------ | ---------- | ---------------- | +| `.rulesync//` | Local | Highest | Yes | +| `.rulesync/.sources///` | Source | Lower | No (gitignored) | -When both a local and a curated skill share the same name, the local skill is used and the remote one is not fetched. +When both a local and a source item share the same name, the local item is used and the remote one is skipped. diff --git a/skills/rulesync/declarative-sources.md b/skills/rulesync/declarative-sources.md index 664e4fe06..542cd537e 100644 --- a/skills/rulesync/declarative-sources.md +++ b/skills/rulesync/declarative-sources.md @@ -1,6 +1,6 @@ -# Declarative Skill Sources +# Declarative Sources -Rulesync can fetch skills from external repositories using the `install` command. Instead of manually running `fetch` for each skill source, declare them in your `rulesync.jsonc` and run `rulesync install` to resolve and fetch them. Then `rulesync generate` picks them up as local curated skills. Typical workflow: `rulesync install && rulesync generate`. +Rulesync can fetch features (skills, rules, commands, subagents, mcp, hooks, ignore) from external repositories using the `install` command. Instead of manually running `fetch` for each source, declare them in your `rulesync.jsonc` and run `rulesync install` to resolve and fetch them. Then `rulesync generate` picks them up alongside local definitions. Typical workflow: `rulesync install && rulesync generate`. ## Configuration @@ -12,21 +12,24 @@ Add a `sources` array to your `rulesync.jsonc`: "targets": ["copilot", "claudecode"], "features": ["rules", "skills"], "sources": [ - // Fetch all skills from a GitHub repository (default transport) + // Fetch all features from a GitHub repository (default transport) { "source": "owner/repo" }, + // Fetch only specific feature types + { "source": "owner/repo", "features": ["rules", "commands"] }, + // Fetch only specific skills by name { "source": "anthropics/skills", "skills": ["skill-creator"] }, // With ref pinning and subdirectory path (same syntax as fetch command) - { "source": "owner/repo@v1.0.0:path/to/skills" }, + { "source": "owner/repo@v1.0.0:path/to/rulesync" }, // Git transport — works with any git remote (Azure DevOps, Bitbucket, etc.) { "source": "https://dev.azure.com/org/project/_git/repo", "transport": "git", "ref": "main", - "path": "exports/skills", + "path": "exports/rulesync", }, // Git transport with a local repository @@ -37,44 +40,46 @@ Add a `sources` array to your `rulesync.jsonc`: Each entry in `sources` accepts: -| Property | Type | Description | -| ----------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------- | -| `source` | `string` | Repository source. For GitHub transport: `owner/repo` or `owner/repo@ref:path`. For git transport: a full git URL. | -| `skills` | `string[]` | Optional list of skill names to fetch. If omitted, all skills are fetched. | -| `transport` | `string` | `"github"` (default) uses the GitHub REST API. `"git"` uses git CLI and works with any git remote. | -| `ref` | `string` | Branch, tag, or ref to fetch from. Defaults to the remote's default branch. For GitHub transport, use the `@ref` source syntax. | -| `path` | `string` | Path to the skills directory within the repository. Defaults to `"skills"`. For GitHub transport, use the `:path` source syntax. | +| Property | Type | Description | +| ----------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `source` | `string` | Repository source. For GitHub transport: `owner/repo` or `owner/repo@ref:path`. For git transport: a full git URL. | +| `features` | `string[]` | Optional list of feature types to fetch (`skills`, `rules`, `commands`, `subagents`, `mcp`, `hooks`, `ignore`, or `*`). Defaults to all (`["*"]`). | +| `skills` | `string[]` | Optional list of skill names to fetch. If omitted, all skills are fetched. | +| `transport` | `string` | `"github"` (default) uses the GitHub REST API. `"git"` uses git CLI and works with any git remote. | +| `ref` | `string` | Branch, tag, or ref to fetch from. Defaults to the remote's default branch. For GitHub transport, use the `@ref` source syntax. | +| `path` | `string` | Base path within the repository where rulesync content lives. All feature directories (skills/, rules/, etc.) and feature files (mcp.json, etc.) are resolved relative to this path. Defaults to the repository root. For GitHub transport, use the `:path` source syntax. | ## How It Works When `rulesync install` runs and `sources` is configured: -1. **Lockfile resolution** — Each source's ref is resolved to a commit SHA and stored in `rulesync.lock` (at the project root). On subsequent runs the locked SHA is reused for deterministic builds. -2. **Remote skill listing** — The `skills/` directory (or the path specified in the source URL) is listed from the remote repository. -3. **Filtering** — If `skills` is specified, only matching skill directories are fetched. -4. **Precedence rules**: - - **Local skills always win** — Skills in `.rulesync/skills/` (not in `.curated/`) take precedence; a remote skill with the same name is skipped. - - **First-declared source wins** — If two sources provide a skill with the same name, the one declared first in the `sources` array is used. -5. **Output** — Fetched skills are written to `.rulesync/skills/.curated//`. This directory is automatically added to `.gitignore` by `rulesync gitignore`. +1. **Lockfile resolution** -- Each source's ref is resolved to a commit SHA and stored in `rulesync.lock` (at the project root). On subsequent runs the locked SHA is reused for deterministic builds. +2. **Feature fetching** -- For each source, the requested features are fetched from the remote repository. Directory features (skills, rules, commands, subagents) are fetched recursively. Single-file features (mcp.json, hooks.json, .aiignore) are fetched individually. +3. **Source cache** -- Fetched content is written to `.rulesync/.sources//`, preserving the rulesync directory structure. This cache is used by `rulesync generate`. +4. **Filtering** -- If `skills` is specified, only matching skill directories are fetched. If `features` is specified, only matching feature types are fetched. +5. **Precedence rules**: + - **Local items always win** -- Items in `.rulesync//` take precedence; a remote item with the same name is skipped. + - **First-declared source wins** -- If two sources provide an item with the same name, the one declared first in the `sources` array is used. + - **Single-file features merge** -- For mcp.json, hooks.json, and .aiignore, content is merged across sources. Local content is applied first, then sources in declaration order. Server entries, event keys, and ignore lines are merged with local values taking precedence. ## CLI Options The `install` command accepts these flags: -| Flag | Description | -| ----------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `--update` | Force re-resolve all source refs, ignoring the lockfile (useful to pull new updates). | -| `--frozen` | Fail if lockfile is missing or out of sync. Fetches missing skills using locked refs without updating the lockfile. Useful for CI to ensure reproducibility. | -| `--token ` | GitHub token for private repositories. | +| Flag | Description | +| ----------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `--update` | Force re-resolve all source refs, ignoring the lockfile (useful to pull new updates). | +| `--frozen` | Fail if lockfile is missing or out of sync. Fetches missing content using locked refs without updating the lockfile. Useful for CI to ensure reproducibility. | +| `--token ` | GitHub token for private repositories. | ```bash -# Install skills using locked refs +# Install sources using locked refs rulesync install # Force update to latest refs rulesync install --update -# Strict CI mode — fail if lockfile doesn't cover all sources (missing locked skills are fetched) +# Strict CI mode — fail if lockfile doesn't cover all sources rulesync install --frozen # Install then generate @@ -86,19 +91,20 @@ rulesync generate ## Lockfile -The lockfile at `rulesync.lock` (at the project root) records the resolved commit SHA and per-skill integrity hashes for each source so that builds are reproducible. It is safe to commit this file. An example: +The lockfile at `rulesync.lock` (at the project root) records the resolved commit SHA and per-file integrity hashes for each source so that builds are reproducible. It is safe to commit this file. An example: ```json { - "lockfileVersion": 1, + "lockfileVersion": 2, "sources": { - "owner/skill-repo": { + "owner/repo": { "requestedRef": "main", "resolvedRef": "abc123def456...", "resolvedAt": "2025-01-15T12:00:00.000Z", - "skills": { - "my-skill": { "integrity": "sha256-abcdef..." }, - "another-skill": { "integrity": "sha256-123456..." } + "files": { + "skills/my-skill/SKILL.md": { "integrity": "sha256-abcdef..." }, + "rules/coding-standards.md": { "integrity": "sha256-123456..." }, + "mcp.json": { "integrity": "sha256-789abc..." } } } } @@ -123,11 +129,11 @@ GITHUB_TOKEN=$(gh auth token) npx rulesync install > [!TIP] > The `install` command also accepts a `--token` flag for explicit authentication: `rulesync install --token ghp_xxxx`. -## Curated vs Local Skills +## Source Cache vs Local Content -| Location | Type | Precedence | Committed to Git | -| ----------------------------------- | ------- | ---------- | ---------------- | -| `.rulesync/skills//` | Local | Highest | Yes | -| `.rulesync/skills/.curated//` | Curated | Lower | No (gitignored) | +| Location | Type | Precedence | Committed to Git | +| ---------------------------------------- | ------ | ---------- | ---------------- | +| `.rulesync//` | Local | Highest | Yes | +| `.rulesync/.sources///` | Source | Lower | No (gitignored) | -When both a local and a curated skill share the same name, the local skill is used and the remote one is not fetched. +When both a local and a source item share the same name, the local item is used and the remote one is skipped. diff --git a/src/cli/commands/generate.test.ts b/src/cli/commands/generate.test.ts index 7d6366f7d..0769a66a4 100644 --- a/src/cli/commands/generate.test.ts +++ b/src/cli/commands/generate.test.ts @@ -51,6 +51,7 @@ describe("generateCommand", () => { getSimulateCommands: vi.fn().mockReturnValue(false), getSimulateSubagents: vi.fn().mockReturnValue(false), getSimulateSkills: vi.fn().mockReturnValue(false), + getSources: vi.fn().mockReturnValue([]), getDryRun: vi.fn().mockReturnValue(false), getCheck: vi.fn().mockReturnValue(false), isPreviewMode: vi.fn().mockReturnValue(false), @@ -210,6 +211,7 @@ describe("generateCommand", () => { simulateSubagents: false, simulateSkills: false, skills: [], + sourceCaches: [], dryRun: false, }); }); @@ -229,6 +231,7 @@ describe("generateCommand", () => { simulateSubagents: true, simulateSkills: false, skills: [], + sourceCaches: [], dryRun: false, }); }); @@ -271,6 +274,7 @@ describe("generateCommand", () => { simulateSubagents: false, simulateSkills: false, skills: [], + sourceCaches: [], dryRun: false, }); expect(RulesProcessor).toHaveBeenCalledWith({ @@ -281,6 +285,7 @@ describe("generateCommand", () => { simulateSubagents: false, simulateSkills: false, skills: [], + sourceCaches: [], dryRun: false, }); }); @@ -310,6 +315,7 @@ describe("generateCommand", () => { baseDir: ".", toolTarget: "claudecode", global: false, + sourceCaches: [], dryRun: false, }); }); @@ -376,6 +382,7 @@ describe("generateCommand", () => { baseDir: ".", toolTarget: "claudecode", global: false, + sourceCaches: [], dryRun: false, }); }); @@ -417,6 +424,7 @@ describe("generateCommand", () => { expect(IgnoreProcessor).toHaveBeenCalledWith({ baseDir: ".", toolTarget: "claudecode", + sourceCaches: [], dryRun: false, }); }); @@ -431,6 +439,7 @@ describe("generateCommand", () => { expect(IgnoreProcessor).toHaveBeenCalledWith({ baseDir: ".", toolTarget: "claudecode", + sourceCaches: [], dryRun: false, }); @@ -476,6 +485,7 @@ describe("generateCommand", () => { baseDir: ".", toolTarget: "claudecode", global: false, + sourceCaches: [], dryRun: false, }); }); @@ -518,6 +528,7 @@ describe("generateCommand", () => { baseDir: ".", toolTarget: "claudecode", global: true, + sourceCaches: [], dryRun: false, }); }); @@ -538,6 +549,7 @@ describe("generateCommand", () => { baseDir: ".", toolTarget: "claudecode", global: true, + sourceCaches: [], dryRun: false, }); }); @@ -560,6 +572,7 @@ describe("generateCommand", () => { baseDir: ".", toolTarget: "claudecode", global: true, + sourceCaches: [], dryRun: false, }); }); @@ -754,6 +767,7 @@ describe("generateCommand", () => { simulateSubagents: true, simulateSkills: false, skills: [], + sourceCaches: [], dryRun: false, }); }); @@ -801,6 +815,7 @@ describe("generateCommand", () => { simulateSubagents: false, simulateSkills: false, skills: [], + sourceCaches: [], dryRun: false, }); expect(RulesProcessor).toHaveBeenCalledWith({ @@ -811,6 +826,7 @@ describe("generateCommand", () => { simulateSubagents: false, simulateSkills: false, skills: [], + sourceCaches: [], dryRun: false, }); expect(RulesProcessor).toHaveBeenCalledWith({ @@ -821,6 +837,7 @@ describe("generateCommand", () => { simulateSubagents: false, simulateSkills: false, skills: [], + sourceCaches: [], dryRun: false, }); expect(RulesProcessor).toHaveBeenCalledTimes(3); // Once for each baseDir @@ -847,6 +864,7 @@ describe("generateCommand", () => { baseDir: ".", toolTarget: "claudecode", global: true, + sourceCaches: [], dryRun: false, }); expect(CommandsProcessor.getToolTargets).toHaveBeenCalledWith( @@ -875,6 +893,7 @@ describe("generateCommand", () => { baseDir: ".", toolTarget: "claudecode", global: true, + sourceCaches: [], dryRun: false, }); }); diff --git a/src/cli/commands/gitignore.test.ts b/src/cli/commands/gitignore.test.ts index d0d050791..dfc7475e7 100644 --- a/src/cli/commands/gitignore.test.ts +++ b/src/cli/commands/gitignore.test.ts @@ -191,6 +191,7 @@ dist/`; it("should report that .gitignore is already up to date", async () => { const rulesyncBlock = `# Generated by Rulesync .rulesync/skills/.curated/ +.rulesync/.sources/ **/AGENTS.md **/.agents/ **/.augmentignore @@ -322,6 +323,7 @@ rulesync.local.jsonc it("should not log Antigravity workaround info when already up to date", async () => { const rulesyncBlock = `# Generated by Rulesync .rulesync/skills/.curated/ +.rulesync/.sources/ **/AGENTS.md **/.agents/ **/.augmentignore diff --git a/src/cli/commands/gitignore.ts b/src/cli/commands/gitignore.ts index 95f2f268c..d32a2202a 100644 --- a/src/cli/commands/gitignore.ts +++ b/src/cli/commands/gitignore.ts @@ -1,6 +1,9 @@ import { join } from "node:path"; -import { RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH } from "../../constants/rulesync-paths.js"; +import { + RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH, + RULESYNC_SOURCES_RELATIVE_DIR_PATH, +} from "../../constants/rulesync-paths.js"; import { fileExists, readFileContent, writeFileContent } from "../../utils/file.js"; import { logger } from "../../utils/logger.js"; @@ -10,6 +13,8 @@ const LEGACY_RULESYNC_HEADER = "# Generated by rulesync - AI tool configuration const RULESYNC_IGNORE_ENTRIES = [ // Rulesync curated (fetched) skills `${RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH}/`, + // Rulesync source cache + `${RULESYNC_SOURCES_RELATIVE_DIR_PATH}/`, // AGENTS.md "**/AGENTS.md", "**/.agents/", diff --git a/src/cli/commands/install.test.ts b/src/cli/commands/install.test.ts index ee23bb17f..dce91147f 100644 --- a/src/cli/commands/install.test.ts +++ b/src/cli/commands/install.test.ts @@ -37,7 +37,7 @@ describe("installCommand", () => { const sources: SourceEntry[] = [{ source: "owner/repo" }]; vi.mocked(ConfigResolver.resolve).mockResolvedValue(createMockConfig(sources)); vi.mocked(resolveAndFetchSources).mockResolvedValue({ - fetchedSkillCount: 3, + fetchedFileCount: 3, sourcesProcessed: 1, }); @@ -52,20 +52,20 @@ describe("installCommand", () => { token: undefined, }, }); - expect(logger.success).toHaveBeenCalledWith("Installed 3 skill(s) from 1 source(s)."); + expect(logger.success).toHaveBeenCalledWith("Installed 3 file(s) from 1 source(s)."); }); it("should report all up to date when no skills fetched", async () => { const sources: SourceEntry[] = [{ source: "owner/repo" }]; vi.mocked(ConfigResolver.resolve).mockResolvedValue(createMockConfig(sources)); vi.mocked(resolveAndFetchSources).mockResolvedValue({ - fetchedSkillCount: 0, + fetchedFileCount: 0, sourcesProcessed: 1, }); await installCommand({}); - expect(logger.success).toHaveBeenCalledWith("All skills up to date (1 source(s) checked)."); + expect(logger.success).toHaveBeenCalledWith("All sources up to date (1 source(s) checked)."); }); it("should warn and return early when no sources defined", async () => { @@ -85,7 +85,7 @@ describe("installCommand", () => { const sources: SourceEntry[] = [{ source: "owner/repo" }]; vi.mocked(ConfigResolver.resolve).mockResolvedValue(createMockConfig(sources)); vi.mocked(resolveAndFetchSources).mockResolvedValue({ - fetchedSkillCount: 0, + fetchedFileCount: 0, sourcesProcessed: 1, }); @@ -102,7 +102,7 @@ describe("installCommand", () => { const sources: SourceEntry[] = [{ source: "owner/repo" }]; vi.mocked(ConfigResolver.resolve).mockResolvedValue(createMockConfig(sources)); vi.mocked(resolveAndFetchSources).mockResolvedValue({ - fetchedSkillCount: 0, + fetchedFileCount: 0, sourcesProcessed: 1, }); @@ -119,7 +119,7 @@ describe("installCommand", () => { const sources: SourceEntry[] = [{ source: "owner/repo" }]; vi.mocked(ConfigResolver.resolve).mockResolvedValue(createMockConfig(sources)); vi.mocked(resolveAndFetchSources).mockResolvedValue({ - fetchedSkillCount: 0, + fetchedFileCount: 0, sourcesProcessed: 1, }); diff --git a/src/cli/commands/install.ts b/src/cli/commands/install.ts index 161c681b5..74b13b731 100644 --- a/src/cli/commands/install.ts +++ b/src/cli/commands/install.ts @@ -30,7 +30,7 @@ export async function installCommand(options: InstallCommandOptions): Promise 0) { + if (result.fetchedFileCount > 0) { logger.success( - `Installed ${result.fetchedSkillCount} skill(s) from ${result.sourcesProcessed} source(s).`, + `Installed ${result.fetchedFileCount} file(s) from ${result.sourcesProcessed} source(s).`, ); } else { - logger.success(`All skills up to date (${result.sourcesProcessed} source(s) checked).`); + logger.success(`All sources up to date (${result.sourcesProcessed} source(s) checked).`); } } diff --git a/src/config/config.test.ts b/src/config/config.test.ts index 5c91c8779..3d8dff015 100644 --- a/src/config/config.test.ts +++ b/src/config/config.test.ts @@ -1,7 +1,8 @@ import { describe, expect, it } from "vitest"; +import { ALL_FEATURES } from "../types/features.js"; import { ALL_TOOL_TARGETS } from "../types/tool-targets.js"; -import { Config, type ConfigParams } from "./config.js"; +import { Config, type ConfigParams, resolveSourceFeatures } from "./config.js"; describe("Config", () => { const defaultConfig: ConfigParams = { @@ -226,3 +227,28 @@ describe("Config", () => { }); }); }); + +describe("resolveSourceFeatures", () => { + it("should return all features when features is omitted", () => { + const result = resolveSourceFeatures({ source: "owner/repo" }); + expect(result).toEqual([...ALL_FEATURES]); + }); + + it("should return all features when features includes wildcard", () => { + const result = resolveSourceFeatures({ source: "owner/repo", features: ["*"] }); + expect(result).toEqual([...ALL_FEATURES]); + }); + + it("should return only specified features", () => { + const result = resolveSourceFeatures({ source: "owner/repo", features: ["skills", "rules"] }); + expect(result).toEqual(["skills", "rules"]); + }); + + it("should return all features when wildcard is mixed with others", () => { + const result = resolveSourceFeatures({ + source: "owner/repo", + features: ["skills", "*"], + }); + expect(result).toEqual([...ALL_FEATURES]); + }); +}); diff --git a/src/config/config.ts b/src/config/config.ts index 694f0246d..e3b5fabf6 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -4,6 +4,7 @@ import { minLength, optional, refine, z } from "zod/mini"; import { ALL_FEATURES, + ALL_FEATURES_WITH_WILDCARD, Feature, Features, PerTargetFeatures, @@ -21,11 +22,12 @@ import { hasControlCharacters } from "../utils/validation.js"; /** * Schema for a single source entry in the sources array. - * Declares an external repository from which skills can be fetched. + * Declares an external repository from which features can be fetched. */ export const SourceEntrySchema = z.object({ source: z.string().check(minLength(1, "source must be a non-empty string")), skills: optional(z.array(z.string())), + features: optional(z.array(z.enum(ALL_FEATURES_WITH_WILDCARD))), transport: optional(z.enum(["github", "git"])), ref: optional( z.string().check( @@ -43,6 +45,17 @@ export const SourceEntrySchema = z.object({ }); export type SourceEntry = z.infer; +/** + * Resolve which features a source entry provides. + * When features is omitted or includes "*", returns all features. + */ +export function resolveSourceFeatures(entry: SourceEntry): Feature[] { + if (!entry.features || entry.features.includes("*")) { + return [...ALL_FEATURES]; + } + return entry.features.filter((f): f is Feature => f !== "*"); +} + export const ConfigParamsSchema = z.object({ baseDirs: z.array(z.string()), targets: RulesyncTargetsSchema, diff --git a/src/constants/rulesync-paths.ts b/src/constants/rulesync-paths.ts index aacd6bf17..781098bdd 100644 --- a/src/constants/rulesync-paths.ts +++ b/src/constants/rulesync-paths.ts @@ -17,6 +17,7 @@ export const RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH = join( RULESYNC_SKILLS_RELATIVE_DIR_PATH, ".curated", ); +export const RULESYNC_SOURCES_RELATIVE_DIR_PATH = join(RULESYNC_RELATIVE_DIR_PATH, ".sources"); export const RULESYNC_SOURCES_LOCK_RELATIVE_FILE_PATH = "rulesync.lock"; // File names (without path) diff --git a/src/features/commands/commands-processor.ts b/src/features/commands/commands-processor.ts index 4ab28efcf..11b652c89 100644 --- a/src/features/commands/commands-processor.ts +++ b/src/features/commands/commands-processor.ts @@ -2,6 +2,7 @@ import { basename, join, relative } from "node:path"; import { z } from "zod/mini"; +import { type SourceCacheEntry, loadParsedFileItemsFromSources } from "../../lib/source-cache.js"; import { FeatureProcessor } from "../../types/feature-processor.js"; import { RulesyncFile } from "../../types/rulesync-file.js"; import { ToolFile } from "../../types/tool-file.js"; @@ -23,7 +24,7 @@ import { KiloCommand } from "./kilo-command.js"; import { KiroCommand } from "./kiro-command.js"; import { OpenCodeCommand } from "./opencode-command.js"; import { RooCommand } from "./roo-command.js"; -import { RulesyncCommand } from "./rulesync-command.js"; +import { RulesyncCommand, RulesyncCommandFrontmatterSchema } from "./rulesync-command.js"; import { ToolCommand, ToolCommandForDeletionParams, @@ -325,18 +326,21 @@ export class CommandsProcessor extends FeatureProcessor { private readonly toolTarget: CommandsProcessorToolTarget; private readonly global: boolean; private readonly getFactory: GetFactory; + private readonly sourceCaches: SourceCacheEntry[]; constructor({ baseDir = process.cwd(), toolTarget, global = false, getFactory = defaultGetFactory, + sourceCaches = [], dryRun = false, }: { baseDir?: string; toolTarget: ToolTarget; global?: boolean; getFactory?: GetFactory; + sourceCaches?: SourceCacheEntry[]; dryRun?: boolean; }) { super({ baseDir, dryRun }); @@ -349,6 +353,7 @@ export class CommandsProcessor extends FeatureProcessor { this.toolTarget = result.data; this.global = global; this.getFactory = getFactory; + this.sourceCaches = sourceCaches; } async convertRulesyncFilesToToolFiles(rulesyncFiles: RulesyncFile[]): Promise { @@ -428,6 +433,31 @@ export class CommandsProcessor extends FeatureProcessor { ), ); + // Load commands from source caches + const localCommandNames = new Set( + rulesyncCommands.map((c) => basename(c.getRelativeFilePath())), + ); + const parsedItems = await loadParsedFileItemsFromSources({ + sources: this.sourceCaches, + featureDirName: "commands", + globPattern: "**/*.md", + localNames: localCommandNames, + schema: RulesyncCommandFrontmatterSchema, + }); + + for (const item of parsedItems) { + rulesyncCommands.push( + new RulesyncCommand({ + baseDir: process.cwd(), + relativeDirPath: basePath, + relativeFilePath: item.name, + frontmatter: item.frontmatter, + body: item.body, + fileContent: item.content, + }), + ); + } + logger.debug(`Successfully loaded ${rulesyncCommands.length} rulesync commands`); return rulesyncCommands; } diff --git a/src/features/hooks/hooks-processor.test.ts b/src/features/hooks/hooks-processor.test.ts index 48a6a35b7..bed628052 100644 --- a/src/features/hooks/hooks-processor.test.ts +++ b/src/features/hooks/hooks-processor.test.ts @@ -103,9 +103,6 @@ describe("HooksProcessor", () => { const processor = new HooksProcessor({ baseDir: testDir, toolTarget: "cursor" }); const files = await processor.loadRulesyncFiles(); expect(files).toHaveLength(0); - expect(logger.error).toHaveBeenCalledWith( - expect.stringContaining("Failed to load Rulesync hooks file"), - ); }); it("should load rulesync files from cwd even when baseDir is different (global mode)", async () => { diff --git a/src/features/hooks/hooks-processor.ts b/src/features/hooks/hooks-processor.ts index ef5935de8..77236245a 100644 --- a/src/features/hooks/hooks-processor.ts +++ b/src/features/hooks/hooks-processor.ts @@ -1,6 +1,11 @@ import { z } from "zod/mini"; -import { RULESYNC_HOOKS_RELATIVE_FILE_PATH } from "../../constants/rulesync-paths.js"; +import { + RULESYNC_HOOKS_RELATIVE_FILE_PATH, + RULESYNC_RELATIVE_DIR_PATH, +} from "../../constants/rulesync-paths.js"; +import { mergeHooks } from "../../lib/merge-strategies.js"; +import { type SourceCacheEntry, loadAndMergeJsonFeature } from "../../lib/source-cache.js"; import { FeatureProcessor } from "../../types/feature-processor.js"; import { CLAUDE_HOOK_EVENTS, @@ -15,7 +20,7 @@ import { import type { RulesyncFile } from "../../types/rulesync-file.js"; import type { ToolFile } from "../../types/tool-file.js"; import type { ToolTarget } from "../../types/tool-targets.js"; -import { formatError } from "../../utils/error.js"; +import { formatError, isFileNotFoundError } from "../../utils/error.js"; import { logger } from "../../utils/logger.js"; import { ClaudecodeHooks } from "./claudecode-hooks.js"; import { CopilotHooks } from "./copilot-hooks.js"; @@ -164,16 +169,19 @@ const hooksProcessorToolTargetsGlobalImportable: ToolTarget[] = [...toolHooksFac export class HooksProcessor extends FeatureProcessor { private readonly toolTarget: HooksProcessorToolTarget; private readonly global: boolean; + private readonly sourceCaches: SourceCacheEntry[]; constructor({ baseDir = process.cwd(), toolTarget, global = false, + sourceCaches = [], dryRun = false, }: { baseDir?: string; toolTarget: ToolTarget; global?: boolean; + sourceCaches?: SourceCacheEntry[]; dryRun?: boolean; }) { super({ baseDir, dryRun }); @@ -185,22 +193,51 @@ export class HooksProcessor extends FeatureProcessor { } this.toolTarget = result.data; this.global = global; + this.sourceCaches = sourceCaches; } async loadRulesyncFiles(): Promise { + // Load local hooks.json + let localHooks: RulesyncHooks | undefined; try { - return [ - await RulesyncHooks.fromFile({ - baseDir: process.cwd(), - validate: true, - }), - ]; + localHooks = await RulesyncHooks.fromFile({ + baseDir: process.cwd(), + validate: true, + }); } catch (error) { - logger.error( - `Failed to load Rulesync hooks file (${RULESYNC_HOOKS_RELATIVE_FILE_PATH}): ${formatError(error)}`, - ); - return []; + if (!isFileNotFoundError(error)) { + logger.warn(`Failed to load local hooks.json: ${formatError(error)}`); + } + } + + // Merge with source caches + const localContent = localHooks ? localHooks.getJson() : undefined; + const merged = await loadAndMergeJsonFeature({ + sources: this.sourceCaches, + fileName: "hooks.json", + localContent, + mergeFn: mergeHooks, + }); + + if (!merged) { + if (!localHooks) return []; + return [localHooks]; } + + // loadAndMergeJsonFeature returns localContent by reference when no sources contribute + if (localHooks && merged === localContent) { + return [localHooks]; + } + + return [ + new RulesyncHooks({ + baseDir: process.cwd(), + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: "hooks.json", + fileContent: JSON.stringify(merged, null, 2), + validate: true, + }), + ]; } async loadToolFiles({ forDeletion = false }: { forDeletion?: boolean } = {}): Promise< diff --git a/src/features/ignore/ignore-processor.test.ts b/src/features/ignore/ignore-processor.test.ts index f5ebb92c7..0958ba953 100644 --- a/src/features/ignore/ignore-processor.test.ts +++ b/src/features/ignore/ignore-processor.test.ts @@ -26,6 +26,7 @@ import { WindsurfIgnore } from "./windsurf-ignore.js"; vi.mock("../../utils/logger.js", () => ({ logger: { debug: vi.fn(), + warn: vi.fn(), error: vi.fn(), }, })); @@ -33,6 +34,9 @@ vi.mock("../../utils/logger.js", () => ({ // Create a mock class for RulesyncIgnore class MockRulesyncIgnore { constructor(public params: any) {} + getFileContent() { + return this.params.fileContent; + } } vi.mock("./rulesync-ignore.js", () => ({ @@ -142,9 +146,6 @@ describe("IgnoreProcessor", () => { const files = await processor.loadRulesyncFiles(); expect(files).toHaveLength(0); - expect(logger.error).toHaveBeenCalledWith( - expect.stringContaining("Failed to load rulesync ignore file"), - ); }); }); diff --git a/src/features/ignore/ignore-processor.ts b/src/features/ignore/ignore-processor.ts index 705a4e5de..58f9906de 100644 --- a/src/features/ignore/ignore-processor.ts +++ b/src/features/ignore/ignore-processor.ts @@ -1,11 +1,17 @@ import { z } from "zod/mini"; -import { RULESYNC_AIIGNORE_RELATIVE_FILE_PATH } from "../../constants/rulesync-paths.js"; +import { + RULESYNC_AIIGNORE_RELATIVE_FILE_PATH, + RULESYNC_AIIGNORE_FILE_NAME, + RULESYNC_RELATIVE_DIR_PATH, +} from "../../constants/rulesync-paths.js"; +import { mergeAiignore } from "../../lib/merge-strategies.js"; +import { type SourceCacheEntry, loadAndMergeTextFeature } from "../../lib/source-cache.js"; import { FeatureProcessor } from "../../types/feature-processor.js"; import { RulesyncFile } from "../../types/rulesync-file.js"; import { ToolFile } from "../../types/tool-file.js"; import { ToolTarget } from "../../types/tool-targets.js"; -import { formatError } from "../../utils/error.js"; +import { formatError, isFileNotFoundError } from "../../utils/error.js"; import { logger } from "../../utils/logger.js"; import { AugmentcodeIgnore } from "./augmentcode-ignore.js"; import { ClaudecodeIgnore } from "./claudecode-ignore.js"; @@ -91,16 +97,19 @@ const defaultGetFactory: GetFactory = (target) => { export class IgnoreProcessor extends FeatureProcessor { private readonly toolTarget: IgnoreProcessorToolTarget; private readonly getFactory: GetFactory; + private readonly sourceCaches: SourceCacheEntry[]; constructor({ baseDir = process.cwd(), toolTarget, getFactory = defaultGetFactory, + sourceCaches = [], dryRun = false, }: { baseDir?: string; toolTarget: ToolTarget; getFactory?: GetFactory; + sourceCaches?: SourceCacheEntry[]; dryRun?: boolean; }) { super({ baseDir, dryRun }); @@ -112,6 +121,7 @@ export class IgnoreProcessor extends FeatureProcessor { } this.toolTarget = result.data; this.getFactory = getFactory; + this.sourceCaches = sourceCaches; } async writeToolIgnoresFromRulesyncIgnores(rulesyncIgnores: RulesyncIgnore[]): Promise { @@ -124,14 +134,43 @@ export class IgnoreProcessor extends FeatureProcessor { * Load and parse rulesync ignore files from .rulesync/ignore/ directory */ async loadRulesyncFiles(): Promise { + // Load local .aiignore + let localIgnore: RulesyncIgnore | undefined; try { - return [await RulesyncIgnore.fromFile()]; + localIgnore = await RulesyncIgnore.fromFile(); } catch (error) { - logger.error( - `Failed to load rulesync ignore file (${RULESYNC_AIIGNORE_RELATIVE_FILE_PATH}): ${formatError(error)}`, - ); - return []; + if (!isFileNotFoundError(error)) { + logger.warn(`Failed to load local .aiignore: ${formatError(error)}`); + } } + + // Merge with source caches + const localContent = localIgnore ? localIgnore.getFileContent() : undefined; + const merged = await loadAndMergeTextFeature({ + sources: this.sourceCaches, + fileName: RULESYNC_AIIGNORE_FILE_NAME, + localContent, + mergeFn: mergeAiignore, + }); + + if (!merged) { + if (!localIgnore) return []; + return [localIgnore]; + } + + // loadAndMergeTextFeature returns localContent by reference when no sources contribute + if (localIgnore && merged === localContent) { + return [localIgnore]; + } + + return [ + new RulesyncIgnore({ + baseDir: process.cwd(), + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: RULESYNC_AIIGNORE_FILE_NAME, + fileContent: merged, + }), + ]; } /** diff --git a/src/features/mcp/mcp-processor.ts b/src/features/mcp/mcp-processor.ts index 9d4c154c9..a8fed8404 100644 --- a/src/features/mcp/mcp-processor.ts +++ b/src/features/mcp/mcp-processor.ts @@ -1,11 +1,16 @@ import { z } from "zod/mini"; -import { RULESYNC_MCP_RELATIVE_FILE_PATH } from "../../constants/rulesync-paths.js"; +import { + RULESYNC_MCP_RELATIVE_FILE_PATH, + RULESYNC_RELATIVE_DIR_PATH, +} from "../../constants/rulesync-paths.js"; +import { mergeMcpServers } from "../../lib/merge-strategies.js"; +import { type SourceCacheEntry, loadAndMergeJsonFeature } from "../../lib/source-cache.js"; import { FeatureProcessor } from "../../types/feature-processor.js"; import { RulesyncFile } from "../../types/rulesync-file.js"; import { ToolFile } from "../../types/tool-file.js"; import { ToolTarget } from "../../types/tool-targets.js"; -import { formatError } from "../../utils/error.js"; +import { formatError, isFileNotFoundError } from "../../utils/error.js"; import { logger } from "../../utils/logger.js"; import { ClaudecodeMcp } from "./claudecode-mcp.js"; import { ClineMcp } from "./cline-mcp.js"; @@ -28,6 +33,12 @@ import { ToolMcpSettablePaths, } from "./tool-mcp.js"; +type McpJson = { mcpServers: Record; [key: string]: unknown }; + +function parseMcpJson(raw: string): McpJson { + return JSON.parse(raw); +} + /** * Supported tool targets for McpProcessor. * Using a tuple to preserve order for consistent iteration. @@ -272,18 +283,21 @@ export class McpProcessor extends FeatureProcessor { private readonly toolTarget: McpProcessorToolTarget; private readonly global: boolean; private readonly getFactory: GetFactory; + private readonly sourceCaches: SourceCacheEntry[]; constructor({ baseDir = process.cwd(), toolTarget, global = false, getFactory = defaultGetFactory, + sourceCaches = [], dryRun = false, }: { baseDir?: string; toolTarget: ToolTarget; global?: boolean; getFactory?: GetFactory; + sourceCaches?: SourceCacheEntry[]; dryRun?: boolean; }) { super({ baseDir, dryRun }); @@ -296,6 +310,7 @@ export class McpProcessor extends FeatureProcessor { this.toolTarget = result.data; this.global = global; this.getFactory = getFactory; + this.sourceCaches = sourceCaches; } /** @@ -303,14 +318,45 @@ export class McpProcessor extends FeatureProcessor { * Load and parse rulesync MCP files from .rulesync/ directory */ async loadRulesyncFiles(): Promise { + // Load local mcp.json + let localMcp: RulesyncMcp | undefined; try { - return [await RulesyncMcp.fromFile({})]; + localMcp = await RulesyncMcp.fromFile({}); } catch (error) { - logger.error( - `Failed to load a Rulesync MCP file (${RULESYNC_MCP_RELATIVE_FILE_PATH}): ${formatError(error)}`, - ); - return []; + if (!isFileNotFoundError(error)) { + logger.warn(`Failed to load local mcp.json: ${formatError(error)}`); + } + } + + // Merge with source caches + const localContent = localMcp ? localMcp.getJson() : undefined; + + const merged = await loadAndMergeJsonFeature({ + sources: this.sourceCaches, + fileName: "mcp.json", + localContent, + mergeFn: mergeMcpServers, + parseFn: parseMcpJson, + }); + + if (!merged) { + if (!localMcp) return []; + return [localMcp]; } + + // loadAndMergeJsonFeature returns localContent by reference when no sources contribute + if (localMcp && merged === localContent) { + return [localMcp]; + } + + return [ + new RulesyncMcp({ + baseDir: process.cwd(), + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: "mcp.json", + fileContent: JSON.stringify(merged, null, 2), + }), + ]; } /** diff --git a/src/features/rules/rules-processor.ts b/src/features/rules/rules-processor.ts index f9aed2a7c..e9ceaccbb 100644 --- a/src/features/rules/rules-processor.ts +++ b/src/features/rules/rules-processor.ts @@ -9,6 +9,7 @@ import { RULESYNC_RULES_RELATIVE_DIR_PATH, RULESYNC_SUBAGENTS_RELATIVE_DIR_PATH, } from "../../constants/rulesync-paths.js"; +import { type SourceCacheEntry, loadParsedFileItemsFromSources } from "../../lib/source-cache.js"; import { FeatureProcessor } from "../../types/feature-processor.js"; import { RulesyncFile } from "../../types/rulesync-file.js"; import { ToolFile } from "../../types/tool-file.js"; @@ -48,7 +49,7 @@ import { OpenCodeRule } from "./opencode-rule.js"; import { QwencodeRule } from "./qwencode-rule.js"; import { ReplitRule } from "./replit-rule.js"; import { RooRule } from "./roo-rule.js"; -import { RulesyncRule } from "./rulesync-rule.js"; +import { RulesyncRule, RulesyncRuleFrontmatterSchema } from "./rulesync-rule.js"; import { ToolRule, ToolRuleForDeletionParams, @@ -471,6 +472,7 @@ export class RulesProcessor extends FeatureProcessor { private readonly global: boolean; private readonly getFactory: GetFactory; private readonly skills?: RulesyncSkill[]; + private readonly sourceCaches: SourceCacheEntry[]; constructor({ baseDir = process.cwd(), @@ -481,6 +483,7 @@ export class RulesProcessor extends FeatureProcessor { global = false, getFactory = defaultGetFactory, skills, + sourceCaches = [], dryRun = false, }: { baseDir?: string; @@ -491,6 +494,7 @@ export class RulesProcessor extends FeatureProcessor { simulateSkills?: boolean; getFactory?: GetFactory; skills?: RulesyncSkill[]; + sourceCaches?: SourceCacheEntry[]; dryRun?: boolean; }) { super({ baseDir, dryRun }); @@ -507,6 +511,7 @@ export class RulesProcessor extends FeatureProcessor { this.simulateSkills = simulateSkills; this.getFactory = getFactory; this.skills = skills; + this.sourceCaches = sourceCaches; } async convertRulesyncFilesToToolFiles(rulesyncFiles: RulesyncFile[]): Promise { @@ -770,6 +775,30 @@ export class RulesProcessor extends FeatureProcessor { }), ); + // Load rules from source caches + const localRuleNames = new Set(rulesyncRules.map((r) => basename(r.getRelativeFilePath()))); + const parsedItems = await loadParsedFileItemsFromSources({ + sources: this.sourceCaches, + featureDirName: "rules", + globPattern: "**/*.md", + localNames: localRuleNames, + schema: RulesyncRuleFrontmatterSchema, + }); + + for (const item of parsedItems) { + // Source rules cannot be root or localRoot + const sourceFrontmatter = { ...item.frontmatter, root: undefined, localRoot: undefined }; + rulesyncRules.push( + new RulesyncRule({ + baseDir: process.cwd(), + relativeDirPath: RULESYNC_RULES_RELATIVE_DIR_PATH, + relativeFilePath: item.name, + frontmatter: sourceFrontmatter, + body: item.body, + }), + ); + } + const factory = this.getFactory(this.toolTarget); const rootRules = rulesyncRules.filter((rule) => rule.getFrontmatter().root); diff --git a/src/features/skills/skills-processor.test.ts b/src/features/skills/skills-processor.test.ts index 79e8f0d58..c887ed106 100644 --- a/src/features/skills/skills-processor.test.ts +++ b/src/features/skills/skills-processor.test.ts @@ -2,7 +2,10 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { RULESYNC_SKILLS_RELATIVE_DIR_PATH } from "../../constants/rulesync-paths.js"; +import { + RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH, + RULESYNC_SKILLS_RELATIVE_DIR_PATH, +} from "../../constants/rulesync-paths.js"; import { setupTestDirectory } from "../../test-utils/test-directories.js"; import { ensureDir, writeFileContent } from "../../utils/file.js"; import { ClaudecodeSkill } from "./claudecode-skill.js"; @@ -428,6 +431,66 @@ This is skill content`; const rulesyncSkill = rulesyncDirs[0] as RulesyncSkill; expect(rulesyncSkill.getFrontmatter().name).toBe("skill-1"); }); + + it("should load skills from legacy .curated/ directory (backward compat)", async () => { + const curatedDir = join(testDir, RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH); + await ensureDir(curatedDir); + + const skillDir = join(curatedDir, "curated-skill"); + await ensureDir(skillDir); + await writeFileContent( + join(skillDir, "SKILL.md"), + `--- +name: curated-skill +description: A curated skill +--- +Curated skill content`, + ); + + const rulesyncDirs = await processor.loadRulesyncDirs(); + + expect(rulesyncDirs).toHaveLength(1); + expect(rulesyncDirs[0]).toBeInstanceOf(RulesyncSkill); + const skill = rulesyncDirs[0] as RulesyncSkill; + expect(skill.getFrontmatter().name).toBe("curated-skill"); + }); + + it("should let local skills override curated skills with the same name", async () => { + // Create local skill + const skillsDir = join(testDir, RULESYNC_SKILLS_RELATIVE_DIR_PATH); + await ensureDir(skillsDir); + const localDir = join(skillsDir, "my-skill"); + await ensureDir(localDir); + await writeFileContent( + join(localDir, "SKILL.md"), + `--- +name: my-skill +description: Local version +--- +Local content`, + ); + + // Create curated skill with the same name + const curatedDir = join(testDir, RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH); + await ensureDir(curatedDir); + const curatedSkillDir = join(curatedDir, "my-skill"); + await ensureDir(curatedSkillDir); + await writeFileContent( + join(curatedSkillDir, "SKILL.md"), + `--- +name: my-skill +description: Curated version +--- +Curated content`, + ); + + const rulesyncDirs = await processor.loadRulesyncDirs(); + + // Only the local skill should be loaded, curated is skipped + expect(rulesyncDirs).toHaveLength(1); + const skill = rulesyncDirs[0] as RulesyncSkill; + expect(skill.getFrontmatter().description).toBe("Local version"); + }); }); describe("loadToolDirs", () => { diff --git a/src/features/skills/skills-processor.ts b/src/features/skills/skills-processor.ts index 1e8071dfb..85c95ddba 100644 --- a/src/features/skills/skills-processor.ts +++ b/src/features/skills/skills-processor.ts @@ -1,8 +1,9 @@ -import { basename, join } from "node:path"; +import { basename, dirname, join } from "node:path"; import { z } from "zod/mini"; import { RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH } from "../../constants/rulesync-paths.js"; +import { type SourceCacheEntry, loadDirItemsFromSources } from "../../lib/source-cache.js"; import { AiDir } from "../../types/ai-dir.js"; import { DirFeatureProcessor } from "../../types/dir-feature-processor.js"; import { ToolTarget } from "../../types/tool-targets.js"; @@ -251,18 +252,21 @@ export class SkillsProcessor extends DirFeatureProcessor { private readonly toolTarget: SkillsProcessorToolTarget; private readonly global: boolean; private readonly getFactory: GetFactory; + private readonly sourceCaches: SourceCacheEntry[]; constructor({ baseDir = process.cwd(), toolTarget, global = false, getFactory = defaultGetFactory, + sourceCaches = [], dryRun = false, }: { baseDir?: string; toolTarget: ToolTarget; global?: boolean; getFactory?: GetFactory; + sourceCaches?: SourceCacheEntry[]; dryRun?: boolean; }) { super({ baseDir, dryRun, avoidBlockScalars: toolTarget === "cursor" }); @@ -275,6 +279,7 @@ export class SkillsProcessor extends DirFeatureProcessor { this.toolTarget = result.data; this.global = global; this.getFactory = getFactory; + this.sourceCaches = sourceCaches; } async convertRulesyncDirsToToolDirs(rulesyncDirs: AiDir[]): Promise { @@ -364,9 +369,31 @@ export class SkillsProcessor extends DirFeatureProcessor { ); } - const allSkills = [...localSkills, ...curatedSkills]; + // Load skills from source caches + const allKnownNames = new Set([ + ...localSkillNames, + ...curatedSkills.map((s) => s.getDirName()), + ]); + const sourceItems = await loadDirItemsFromSources({ + sources: this.sourceCaches, + featureDirName: "skills", + localNames: allKnownNames, + }); + + const sourceSkills = await Promise.all( + sourceItems.map((item) => + RulesyncSkill.fromDir({ + baseDir: dirname(dirname(item.path)), + relativeDirPath: "skills", + dirName: item.name, + global: this.global, + }), + ), + ); + + const allSkills = [...localSkills, ...curatedSkills, ...sourceSkills]; logger.debug( - `Successfully loaded ${allSkills.length} rulesync skills (${localSkills.length} local, ${curatedSkills.length} curated)`, + `Successfully loaded ${allSkills.length} rulesync skills (${localSkills.length} local, ${curatedSkills.length} curated, ${sourceSkills.length} from sources)`, ); return allSkills; } diff --git a/src/features/subagents/subagents-processor.ts b/src/features/subagents/subagents-processor.ts index 505bbc0f0..7e6995400 100644 --- a/src/features/subagents/subagents-processor.ts +++ b/src/features/subagents/subagents-processor.ts @@ -2,6 +2,7 @@ import { basename, join } from "node:path"; import { z } from "zod/mini"; +import { type SourceCacheEntry, loadParsedFileItemsFromSources } from "../../lib/source-cache.js"; import { FeatureProcessor } from "../../types/feature-processor.js"; import { RulesyncFile } from "../../types/rulesync-file.js"; import { ToolFile } from "../../types/tool-file.js"; @@ -20,7 +21,7 @@ import { JunieSubagent } from "./junie-subagent.js"; import { KiroSubagent } from "./kiro-subagent.js"; import { OpenCodeSubagent } from "./opencode-subagent.js"; import { RooSubagent } from "./roo-subagent.js"; -import { RulesyncSubagent } from "./rulesync-subagent.js"; +import { RulesyncSubagent, RulesyncSubagentFrontmatterSchema } from "./rulesync-subagent.js"; import { SimulatedSubagent } from "./simulated-subagent.js"; import { ToolSubagent, @@ -204,18 +205,21 @@ export class SubagentsProcessor extends FeatureProcessor { private readonly toolTarget: SubagentsProcessorToolTarget; private readonly global: boolean; private readonly getFactory: GetFactory; + private readonly sourceCaches: SourceCacheEntry[]; constructor({ baseDir = process.cwd(), toolTarget, global = false, getFactory = defaultGetFactory, + sourceCaches = [], dryRun = false, }: { baseDir?: string; toolTarget: ToolTarget; global?: boolean; getFactory?: GetFactory; + sourceCaches?: SourceCacheEntry[]; dryRun?: boolean; }) { super({ baseDir, dryRun }); @@ -228,6 +232,7 @@ export class SubagentsProcessor extends FeatureProcessor { this.toolTarget = result.data; this.global = global; this.getFactory = getFactory; + this.sourceCaches = sourceCaches; } async convertRulesyncFilesToToolFiles(rulesyncFiles: RulesyncFile[]): Promise { @@ -283,49 +288,61 @@ export class SubagentsProcessor extends FeatureProcessor { async loadRulesyncFiles(): Promise { const subagentsDir = join(process.cwd(), RulesyncSubagent.getSettablePaths().relativeDirPath); - // Check if directory exists - const dirExists = await directoryExists(subagentsDir); - if (!dirExists) { - logger.debug(`Rulesync subagents directory not found: ${subagentsDir}`); - return []; - } - - // Read all markdown files from the directory - const entries = await listDirectoryFiles(subagentsDir); - const mdFiles = entries.filter((file) => file.endsWith(".md")); - - if (mdFiles.length === 0) { - logger.debug(`No markdown files found in rulesync subagents directory: ${subagentsDir}`); - return []; - } - - logger.debug(`Found ${mdFiles.length} subagent files in ${subagentsDir}`); - - // Parse all files and create RulesyncSubagent instances using fromFilePath + // Load local subagents from the project directory const rulesyncSubagents: RulesyncSubagent[] = []; - for (const mdFile of mdFiles) { - const filepath = join(subagentsDir, mdFile); - - try { - const rulesyncSubagent = await RulesyncSubagent.fromFile({ - relativeFilePath: mdFile, - validate: true, - }); - - rulesyncSubagents.push(rulesyncSubagent); - logger.debug(`Successfully loaded subagent: ${mdFile}`); - } catch (error) { - logger.warn(`Failed to load subagent file ${filepath}: ${formatError(error)}`); - continue; + const dirExists = await directoryExists(subagentsDir); + if (dirExists) { + const entries = await listDirectoryFiles(subagentsDir); + const mdFiles = entries.filter((file) => file.endsWith(".md")); + + logger.debug(`Found ${mdFiles.length} subagent files in ${subagentsDir}`); + + for (const mdFile of mdFiles) { + const filepath = join(subagentsDir, mdFile); + + try { + const rulesyncSubagent = await RulesyncSubagent.fromFile({ + relativeFilePath: mdFile, + validate: true, + }); + + rulesyncSubagents.push(rulesyncSubagent); + logger.debug(`Successfully loaded subagent: ${mdFile}`); + } catch (error) { + logger.warn(`Failed to load subagent file ${filepath}: ${formatError(error)}`); + continue; + } } } - if (rulesyncSubagents.length === 0) { - logger.debug(`No valid subagents found in ${subagentsDir}`); + if (rulesyncSubagents.length === 0 && this.sourceCaches.length === 0) { + logger.debug(`No subagents found locally or in source caches`); return []; } + // Load subagents from source caches + const localNames = new Set(rulesyncSubagents.map((s) => basename(s.getRelativeFilePath()))); + const parsedItems = await loadParsedFileItemsFromSources({ + sources: this.sourceCaches, + featureDirName: "subagents", + globPattern: "*.md", + localNames, + schema: RulesyncSubagentFrontmatterSchema, + }); + + for (const item of parsedItems) { + rulesyncSubagents.push( + new RulesyncSubagent({ + baseDir: process.cwd(), + relativeDirPath: RulesyncSubagent.getSettablePaths().relativeDirPath, + relativeFilePath: item.name, + frontmatter: item.frontmatter, + body: item.body, + }), + ); + } + logger.debug(`Successfully loaded ${rulesyncSubagents.length} rulesync subagents`); return rulesyncSubagents; } diff --git a/src/lib/generate.test.ts b/src/lib/generate.test.ts index 6ec0959b7..f0a969308 100644 --- a/src/lib/generate.test.ts +++ b/src/lib/generate.test.ts @@ -74,6 +74,7 @@ describe("generate", () => { getSimulateCommands: ReturnType; getSimulateSubagents: ReturnType; getSimulateSkills: ReturnType; + getSources: ReturnType; isPreviewMode: ReturnType; }; @@ -89,6 +90,7 @@ describe("generate", () => { getSimulateCommands: vi.fn().mockReturnValue(false), getSimulateSubagents: vi.fn().mockReturnValue(false), getSimulateSkills: vi.fn().mockReturnValue(false), + getSources: vi.fn().mockReturnValue([]), isPreviewMode: vi.fn().mockReturnValue(false), }; @@ -166,6 +168,7 @@ describe("generate", () => { simulateSubagents: false, simulateSkills: false, skills: [], + sourceCaches: [], dryRun: false, }); }); @@ -347,6 +350,7 @@ describe("generate", () => { baseDir: ".", toolTarget: "claudecode", global: false, + sourceCaches: [], dryRun: false, }); }); @@ -372,6 +376,7 @@ describe("generate", () => { baseDir: ".", toolTarget: "claudecode", global: false, + sourceCaches: [], dryRun: false, }); }); @@ -409,6 +414,7 @@ describe("generate", () => { baseDir: ".", toolTarget: "claudecode", global: false, + sourceCaches: [], dryRun: false, }); }); @@ -457,6 +463,7 @@ describe("generate", () => { baseDir: ".", toolTarget: "claudecode", global: false, + sourceCaches: [], dryRun: false, }); }); diff --git a/src/lib/generate.ts b/src/lib/generate.ts index 6b9198b18..37a089ce0 100644 --- a/src/lib/generate.ts +++ b/src/lib/generate.ts @@ -22,6 +22,7 @@ import { formatError } from "../utils/error.js"; import { fileExists } from "../utils/file.js"; import { logger } from "../utils/logger.js"; import type { FeatureGenerateResult } from "../utils/result.js"; +import { type SourceCacheEntry, getOrderedSourceCaches } from "./source-cache.js"; export type GenerateResult = { rulesCount: number; @@ -138,13 +139,24 @@ export async function checkRulesyncDirExists(params: { baseDir: string }): Promi export async function generate(params: { config: Config }): Promise { const { config } = params; - const ignoreResult = await generateIgnoreCore({ config }); - const mcpResult = await generateMcpCore({ config }); - const commandsResult = await generateCommandsCore({ config }); - const subagentsResult = await generateSubagentsCore({ config }); - const skillsResult = await generateSkillsCore({ config }); - const hooksResult = await generateHooksCore({ config }); - const rulesResult = await generateRulesCore({ config, skills: skillsResult.skills }); + // Resolve source caches once for all processors. + // Source caches are always resolved from the project root (process.cwd()), not per-baseDir. + const sourceCaches = await getOrderedSourceCaches({ + baseDir: process.cwd(), + sources: config.getSources(), + }); + + const ignoreResult = await generateIgnoreCore({ config, sourceCaches }); + const mcpResult = await generateMcpCore({ config, sourceCaches }); + const commandsResult = await generateCommandsCore({ config, sourceCaches }); + const subagentsResult = await generateSubagentsCore({ config, sourceCaches }); + const skillsResult = await generateSkillsCore({ config, sourceCaches }); + const hooksResult = await generateHooksCore({ config, sourceCaches }); + const rulesResult = await generateRulesCore({ + config, + sourceCaches, + skills: skillsResult.skills, + }); const hasDiff = ignoreResult.hasDiff || @@ -177,9 +189,10 @@ export async function generate(params: { config: Config }): Promise { - const { config, skills } = params; + const { config, sourceCaches, skills } = params; let totalCount = 0; const allPaths: string[] = []; @@ -204,6 +217,7 @@ async function generateRulesCore(params: { simulateSubagents: config.getSimulateSubagents(), simulateSkills: config.getSimulateSkills(), skills: skills, + sourceCaches, dryRun: config.isPreviewMode(), }); @@ -225,8 +239,11 @@ async function generateRulesCore(params: { return { count: totalCount, paths: allPaths, hasDiff }; } -async function generateIgnoreCore(params: { config: Config }): Promise { - const { config } = params; +async function generateIgnoreCore(params: { + config: Config; + sourceCaches: SourceCacheEntry[]; +}): Promise { + const { config, sourceCaches } = params; const supportedIgnoreTargets = IgnoreProcessor.getToolTargets(); warnUnsupportedTargets({ @@ -254,6 +271,7 @@ async function generateIgnoreCore(params: { config: Config }): Promise { - const { config } = params; +async function generateMcpCore(params: { + config: Config; + sourceCaches: SourceCacheEntry[]; +}): Promise { + const { config, sourceCaches } = params; let totalCount = 0; const allPaths: string[] = []; @@ -311,6 +332,7 @@ async function generateMcpCore(params: { config: Config }): Promise { - const { config } = params; +async function generateCommandsCore(params: { + config: Config; + sourceCaches: SourceCacheEntry[]; +}): Promise { + const { config, sourceCaches } = params; let totalCount = 0; const allPaths: string[] = []; @@ -361,6 +386,7 @@ async function generateCommandsCore(params: { config: Config }): Promise { - const { config } = params; +async function generateSubagentsCore(params: { + config: Config; + sourceCaches: SourceCacheEntry[]; +}): Promise { + const { config, sourceCaches } = params; let totalCount = 0; const allPaths: string[] = []; @@ -411,6 +440,7 @@ async function generateSubagentsCore(params: { config: Config }): Promise { - const { config } = params; + const { config, sourceCaches } = params; let totalCount = 0; const allPaths: string[] = []; @@ -464,6 +495,7 @@ async function generateSkillsCore(params: { baseDir: baseDir, toolTarget: toolTarget, global: config.getGlobal(), + sourceCaches, dryRun: config.isPreviewMode(), }); @@ -492,8 +524,11 @@ async function generateSkillsCore(params: { return { count: totalCount, paths: allPaths, skills: allSkills, hasDiff }; } -async function generateHooksCore(params: { config: Config }): Promise { - const { config } = params; +async function generateHooksCore(params: { + config: Config; + sourceCaches: SourceCacheEntry[]; +}): Promise { + const { config, sourceCaches } = params; let totalCount = 0; const allPaths: string[] = []; @@ -514,6 +549,7 @@ async function generateHooksCore(params: { config: Config }): Promise> { + const { url, ref, paths, basePath } = params; + if (paths.length === 0) return []; + + validateGitUrl(url); + validateRef(ref); + for (const p of paths) { + if (p.split(/[/\\]/).includes("..") || isAbsolute(p)) { + throw new GitClientError(`Invalid path "${p}": must be a relative path without ".."`); + } + const ctrl = findControlCharacter(p); + if (ctrl) { + throw new GitClientError( + `Path contains control character ${ctrl.hex} at position ${ctrl.position}`, + ); + } + } + + await checkGitAvailable(); + const tmpDir = await createTempDirectory("rulesync-git-"); + try { + await execFileAsync( + "git", + [ + "clone", + "--depth", + "1", + "--branch", + ref, + "--no-checkout", + "--filter=blob:none", + "--", + url, + tmpDir, + ], + { timeout: GIT_TIMEOUT_MS }, + ); + + // Sparse checkout all requested paths at once (git patterns use forward slashes) + const sparseArgs = paths.map((p) => (basePath ? posix.join(basePath, p) : p)); + await execFileAsync("git", ["-C", tmpDir, "sparse-checkout", "set", "--", ...sparseArgs], { + timeout: GIT_TIMEOUT_MS, + }); + await execFileAsync("git", ["-C", tmpDir, "checkout"], { timeout: GIT_TIMEOUT_MS }); + + // Walk from the base path (or repo root) and collect all files + const walkRoot = basePath ? join(tmpDir, basePath) : tmpDir; + if (!(await directoryExists(walkRoot))) return []; + return await walkDirectory(walkRoot, walkRoot); + } catch (error) { + if (error instanceof GitClientError) throw error; + throw new GitClientError(`Failed to fetch source cache files from ${url}`, error); + } finally { + await removeTempDirectory(tmpDir); + } +} + const MAX_WALK_DEPTH = 20; const MAX_TOTAL_FILES = 10_000; const MAX_TOTAL_SIZE = 100 * 1024 * 1024; // 100 MB @@ -226,7 +293,12 @@ async function walkDirectory( ); } const content = await readFileContent(fullPath); - results.push({ relativePath: relative(baseDir, fullPath), content, size }); + // Normalize to forward slashes for consistent lockfile keys across platforms + results.push({ + relativePath: relative(baseDir, fullPath).replaceAll("\\", "/"), + content, + size, + }); } } return results; diff --git a/src/lib/merge-strategies.test.ts b/src/lib/merge-strategies.test.ts new file mode 100644 index 000000000..ba0365249 --- /dev/null +++ b/src/lib/merge-strategies.test.ts @@ -0,0 +1,154 @@ +import { describe, expect, it } from "vitest"; + +import { mergeAiignore, mergeHooks, mergeMcpServers } from "./merge-strategies.js"; + +describe("mergeMcpServers", () => { + it("should keep base servers when no conflicts", () => { + const base = { mcpServers: { a: { command: "a" } } }; + const overlay = { mcpServers: { b: { command: "b" } } }; + + const result = mergeMcpServers(base, overlay); + + expect(result.mcpServers).toEqual({ + a: { command: "a" }, + b: { command: "b" }, + }); + }); + + it("should give precedence to base servers on name conflict", () => { + const base = { mcpServers: { shared: { command: "base-cmd" } } }; + const overlay = { mcpServers: { shared: { command: "overlay-cmd" } } }; + + const result = mergeMcpServers(base, overlay); + + expect(result.mcpServers).toEqual({ shared: { command: "base-cmd" } }); + }); + + it("should handle empty base", () => { + const base = { mcpServers: {} }; + const overlay = { mcpServers: { a: { command: "a" } } }; + + const result = mergeMcpServers(base, overlay); + + expect(result.mcpServers).toEqual({ a: { command: "a" } }); + }); + + it("should handle empty overlay", () => { + const base = { mcpServers: { a: { command: "a" } } }; + const overlay = { mcpServers: {} }; + + const result = mergeMcpServers(base, overlay); + + expect(result.mcpServers).toEqual({ a: { command: "a" } }); + }); +}); + +describe("mergeHooks", () => { + it("should concatenate hook arrays for the same event", () => { + const base = { hooks: { sessionStart: [{ command: "base-cmd" }] } }; + const overlay = { hooks: { sessionStart: [{ command: "overlay-cmd" }] } }; + + const result = mergeHooks(base, overlay); + + expect(result.hooks.sessionStart).toEqual([ + { command: "base-cmd" }, + { command: "overlay-cmd" }, + ]); + }); + + it("should add overlay events not present in base", () => { + const base = { hooks: { sessionStart: [{ command: "start" }] } }; + const overlay = { hooks: { sessionEnd: [{ command: "end" }] } }; + + const result = mergeHooks(base, overlay); + + expect(result.hooks.sessionStart).toEqual([{ command: "start" }]); + expect(result.hooks.sessionEnd).toEqual([{ command: "end" }]); + }); + + it("should merge per-tool hook sections", () => { + const base = { + hooks: {}, + cursor: { hooks: { preToolUse: [{ command: "base-hook" }] } }, + }; + const overlay = { + hooks: {}, + cursor: { hooks: { preToolUse: [{ command: "overlay-hook" }] } }, + }; + + const result = mergeHooks(base, overlay); + + expect(result.cursor?.hooks?.preToolUse).toEqual([ + { command: "base-hook" }, + { command: "overlay-hook" }, + ]); + }); + + it("should handle missing per-tool sections in base", () => { + const base = { hooks: {} }; + const overlay = { + hooks: {}, + claudecode: { hooks: { sessionStart: [{ command: "init" }] } }, + }; + + const result = mergeHooks(base, overlay); + + expect(result.claudecode?.hooks?.sessionStart).toEqual([{ command: "init" }]); + }); + + it("should handle empty hooks objects", () => { + const base = { hooks: {} }; + const overlay = { hooks: {} }; + + const result = mergeHooks(base, overlay); + + expect(result.hooks).toEqual({}); + }); +}); + +describe("mergeAiignore", () => { + it("should concatenate lines from overlay after base", () => { + const base = "*.log\n*.tmp\n"; + const overlay = "*.bak\n"; + + const result = mergeAiignore(base, overlay); + + expect(result).toBe("*.log\n*.tmp\n*.bak\n"); + }); + + it("should deduplicate identical lines", () => { + const base = "*.log\n*.tmp\n"; + const overlay = "*.log\n*.new\n"; + + const result = mergeAiignore(base, overlay); + + expect(result).toBe("*.log\n*.tmp\n*.new\n"); + }); + + it("should skip empty lines in overlay", () => { + const base = "*.log\n"; + const overlay = "\n\n*.bak\n\n"; + + const result = mergeAiignore(base, overlay); + + expect(result).toBe("*.log\n*.bak\n"); + }); + + it("should return base unchanged when overlay has nothing new", () => { + const base = "*.log\n*.tmp\n"; + const overlay = "*.log\n*.tmp\n"; + + const result = mergeAiignore(base, overlay); + + expect(result).toBe("*.log\n*.tmp\n"); + }); + + it("should handle base without trailing newline", () => { + const base = "*.log"; + const overlay = "*.bak\n"; + + const result = mergeAiignore(base, overlay); + + expect(result).toBe("*.log\n*.bak\n"); + }); +}); diff --git a/src/lib/merge-strategies.ts b/src/lib/merge-strategies.ts new file mode 100644 index 000000000..40e9a86b0 --- /dev/null +++ b/src/lib/merge-strategies.ts @@ -0,0 +1,108 @@ +import type { HookDefinition, HooksConfig } from "../types/hooks.js"; + +// --------------------------------------------------------------------------- +// MCP merge +// --------------------------------------------------------------------------- + +type McpJson = { + mcpServers: Record; + [key: string]: unknown; +}; + +/** + * Merge MCP JSON objects at the server-name level. + * `base` entries take precedence over `overlay` entries with the same name. + */ +export function mergeMcpServers(base: McpJson, overlay: McpJson): McpJson { + const mergedServers = { ...overlay.mcpServers, ...base.mcpServers }; + return { ...overlay, ...base, mcpServers: mergedServers }; +} + +// --------------------------------------------------------------------------- +// Hooks merge +// --------------------------------------------------------------------------- + +/** + * Merge hooks config objects. + * For each event key, concatenates hook arrays (base hooks first, then overlay hooks). + * Per-tool override sections are merged the same way. + */ +export function mergeHooks(base: HooksConfig, overlay: HooksConfig): HooksConfig { + const result: HooksConfig = { ...base }; + + // Merge the top-level hooks record + result.hooks = mergeHookRecords(base.hooks, overlay.hooks); + + // Merge per-tool override sections + const toolKeys = [ + "cursor", + "claudecode", + "copilot", + "opencode", + "factorydroid", + "geminicli", + ] as const; + for (const tool of toolKeys) { + const baseToolSection = base[tool]; + const overlayToolSection = overlay[tool]; + + if (overlayToolSection?.hooks) { + const baseToolHooks = baseToolSection?.hooks ?? {}; + result[tool] = { + ...baseToolSection, + hooks: mergeHookRecords(baseToolHooks, overlayToolSection.hooks), + }; + } + } + + return result; +} + +/** + * Merge two hook records. Base entries come first; overlay entries are appended. + * For events present only in overlay, they are added as-is. + */ +function mergeHookRecords( + base: Record, + overlay: Record, +): Record { + const result: Record = { ...base }; + + for (const [event, hooks] of Object.entries(overlay)) { + if (result[event]) { + // Base hooks first, then overlay hooks + result[event] = [...result[event], ...hooks]; + } else { + result[event] = [...hooks]; + } + } + + return result; +} + +// --------------------------------------------------------------------------- +// Aiignore merge +// --------------------------------------------------------------------------- + +/** + * Merge .aiignore text content. + * Concatenates lines (base first, then overlay), deduplicating identical non-empty lines. + */ +export function mergeAiignore(base: string, overlay: string): string { + const baseLines = base.split("\n"); + const seen = new Set(baseLines.filter((line) => line.trim() !== "")); + const newLines: string[] = []; + + for (const line of overlay.split("\n")) { + if (line.trim() === "") continue; + if (seen.has(line)) continue; + seen.add(line); + newLines.push(line); + } + + if (newLines.length === 0) return base; + + // Ensure base ends with newline before appending + const normalizedBase = base.endsWith("\n") ? base : base + "\n"; + return normalizedBase + newLines.join("\n") + "\n"; +} diff --git a/src/lib/source-cache.test.ts b/src/lib/source-cache.test.ts new file mode 100644 index 000000000..79895d1aa --- /dev/null +++ b/src/lib/source-cache.test.ts @@ -0,0 +1,433 @@ +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { RULESYNC_SOURCES_RELATIVE_DIR_PATH } from "../constants/rulesync-paths.js"; +import { setupTestDirectory } from "../test-utils/test-directories.js"; +import { ensureDir, writeFileContent } from "../utils/file.js"; +import { + type SourceCacheEntry, + getOrderedSourceCaches, + loadAndMergeJsonFeature, + loadAndMergeTextFeature, + loadDirItemsFromSources, + loadFileItemsFromSources, + sourceKeyToDirName, +} from "./source-cache.js"; + +vi.mock("../utils/logger.js", () => ({ + logger: { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + success: vi.fn(), + }, +})); + +describe("sourceKeyToDirName", () => { + it("should replace slashes with double dashes", () => { + expect(sourceKeyToDirName("owner/repo")).toBe("owner--repo"); + }); + + it("should normalize the key before converting", () => { + expect(sourceKeyToDirName("https://github.com/Owner/Repo")).toBe("owner--repo"); + }); + + it("should handle git URLs", () => { + expect(sourceKeyToDirName("https://github.com/owner/repo.git")).toBe("owner--repo"); + }); + + it("should handle keys without slashes", () => { + expect(sourceKeyToDirName("single-segment")).toBe("single-segment"); + }); + + it("should normalize Azure DevOps URLs to org--project--repo", () => { + expect(sourceKeyToDirName("https://user@dev.azure.com/org/project/_git/repo")).toBe( + "org--project--repo", + ); + }); +}); + +describe("getOrderedSourceCaches", () => { + let testDir: string; + let cleanup: () => Promise; + + beforeEach(async () => { + ({ testDir, cleanup } = await setupTestDirectory()); + vi.spyOn(process, "cwd").mockReturnValue(testDir); + }); + + afterEach(async () => { + await cleanup(); + vi.restoreAllMocks(); + }); + + it("should return entries for sources whose cache directories exist", async () => { + const sourcesDir = join(testDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH); + await ensureDir(join(sourcesDir, "owner--repo")); + await ensureDir(join(sourcesDir, "other--repo")); + + const result = await getOrderedSourceCaches({ + baseDir: testDir, + sources: [{ source: "owner/repo" }, { source: "other/repo" }], + }); + + expect(result).toHaveLength(2); + expect(result[0]!.sourceKey).toBe("owner/repo"); + expect(result[1]!.sourceKey).toBe("other/repo"); + }); + + it("should skip sources whose cache directories do not exist", async () => { + const sourcesDir = join(testDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH); + await ensureDir(join(sourcesDir, "owner--repo")); + + const result = await getOrderedSourceCaches({ + baseDir: testDir, + sources: [{ source: "owner/repo" }, { source: "missing/repo" }], + }); + + expect(result).toHaveLength(1); + expect(result[0]!.sourceKey).toBe("owner/repo"); + }); + + it("should preserve declaration order from sources array", async () => { + const sourcesDir = join(testDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH); + await ensureDir(join(sourcesDir, "second--repo")); + await ensureDir(join(sourcesDir, "first--repo")); + + const result = await getOrderedSourceCaches({ + baseDir: testDir, + sources: [{ source: "first/repo" }, { source: "second/repo" }], + }); + + expect(result[0]!.sourceKey).toBe("first/repo"); + expect(result[1]!.sourceKey).toBe("second/repo"); + }); +}); + +describe("loadDirItemsFromSources", () => { + let testDir: string; + let cleanup: () => Promise; + + beforeEach(async () => { + ({ testDir, cleanup } = await setupTestDirectory()); + }); + + afterEach(async () => { + await cleanup(); + }); + + it("should load skill directories from source caches", async () => { + const cachePath = join(testDir, "source-a"); + await ensureDir(join(cachePath, "skills", "skill-one")); + await writeFileContent(join(cachePath, "skills", "skill-one", "SKILL.md"), "# Skill One"); + + const sources: SourceCacheEntry[] = [{ sourceKey: "source-a", cachePath }]; + const result = await loadDirItemsFromSources({ + sources, + featureDirName: "skills", + localNames: new Set(), + }); + + expect(result).toHaveLength(1); + expect(result[0]!.name).toBe("skill-one"); + expect(result[0]!.sourceKey).toBe("source-a"); + }); + + it("should skip items that exist locally", async () => { + const cachePath = join(testDir, "source-a"); + await ensureDir(join(cachePath, "skills", "local-skill")); + await ensureDir(join(cachePath, "skills", "remote-skill")); + + const sources: SourceCacheEntry[] = [{ sourceKey: "source-a", cachePath }]; + const result = await loadDirItemsFromSources({ + sources, + featureDirName: "skills", + localNames: new Set(["local-skill"]), + }); + + expect(result).toHaveLength(1); + expect(result[0]!.name).toBe("remote-skill"); + }); + + it("should apply first-source-wins for duplicate names", async () => { + const cacheA = join(testDir, "source-a"); + const cacheB = join(testDir, "source-b"); + await ensureDir(join(cacheA, "skills", "shared-skill")); + await ensureDir(join(cacheB, "skills", "shared-skill")); + + const sources: SourceCacheEntry[] = [ + { sourceKey: "source-a", cachePath: cacheA }, + { sourceKey: "source-b", cachePath: cacheB }, + ]; + const result = await loadDirItemsFromSources({ + sources, + featureDirName: "skills", + localNames: new Set(), + }); + + expect(result).toHaveLength(1); + expect(result[0]!.sourceKey).toBe("source-a"); + }); +}); + +describe("loadFileItemsFromSources", () => { + let testDir: string; + let cleanup: () => Promise; + + beforeEach(async () => { + ({ testDir, cleanup } = await setupTestDirectory()); + }); + + afterEach(async () => { + await cleanup(); + }); + + it("should load rule files from source caches", async () => { + const cachePath = join(testDir, "source-a"); + await ensureDir(join(cachePath, "rules")); + await writeFileContent(join(cachePath, "rules", "coding.md"), "# Coding"); + + const sources: SourceCacheEntry[] = [{ sourceKey: "source-a", cachePath }]; + const result = await loadFileItemsFromSources({ + sources, + featureDirName: "rules", + globPattern: "*.md", + localNames: new Set(), + }); + + expect(result).toHaveLength(1); + expect(result[0]!.name).toBe("coding.md"); + }); + + it("should skip files that exist locally", async () => { + const cachePath = join(testDir, "source-a"); + await ensureDir(join(cachePath, "rules")); + await writeFileContent(join(cachePath, "rules", "local.md"), "# Local"); + await writeFileContent(join(cachePath, "rules", "remote.md"), "# Remote"); + + const sources: SourceCacheEntry[] = [{ sourceKey: "source-a", cachePath }]; + const result = await loadFileItemsFromSources({ + sources, + featureDirName: "rules", + globPattern: "*.md", + localNames: new Set(["local.md"]), + }); + + expect(result).toHaveLength(1); + expect(result[0]!.name).toBe("remote.md"); + }); + + it("should let local rules override source rules of the same name", async () => { + const cacheA = join(testDir, "source-a"); + await ensureDir(join(cacheA, "rules")); + await writeFileContent( + join(cacheA, "rules", "coding-standards.md"), + "# Remote coding standards", + ); + await writeFileContent(join(cacheA, "rules", "security.md"), "# Remote security"); + + const sources: SourceCacheEntry[] = [{ sourceKey: "source-a", cachePath: cacheA }]; + const result = await loadFileItemsFromSources({ + sources, + featureDirName: "rules", + globPattern: "*.md", + localNames: new Set(["coding-standards.md"]), + }); + + // Only security.md should be loaded; coding-standards.md is overridden by local + expect(result).toHaveLength(1); + expect(result[0]!.name).toBe("security.md"); + }); + + it("should apply first-source-wins for duplicate file names", async () => { + const cacheA = join(testDir, "source-a"); + const cacheB = join(testDir, "source-b"); + await ensureDir(join(cacheA, "rules")); + await ensureDir(join(cacheB, "rules")); + await writeFileContent(join(cacheA, "rules", "shared.md"), "# From A"); + await writeFileContent(join(cacheB, "rules", "shared.md"), "# From B"); + + const sources: SourceCacheEntry[] = [ + { sourceKey: "source-a", cachePath: cacheA }, + { sourceKey: "source-b", cachePath: cacheB }, + ]; + const result = await loadFileItemsFromSources({ + sources, + featureDirName: "rules", + globPattern: "*.md", + localNames: new Set(), + }); + + expect(result).toHaveLength(1); + expect(result[0]!.sourceKey).toBe("source-a"); + }); +}); + +describe("loadAndMergeJsonFeature", () => { + let testDir: string; + let cleanup: () => Promise; + + beforeEach(async () => { + ({ testDir, cleanup } = await setupTestDirectory()); + }); + + afterEach(async () => { + await cleanup(); + }); + + it("should merge JSON from multiple sources with local precedence", async () => { + const cacheA = join(testDir, "source-a"); + const cacheB = join(testDir, "source-b"); + await ensureDir(cacheA); + await ensureDir(cacheB); + await writeFileContent(cacheA + "/mcp.json", JSON.stringify({ servers: { a: 1 } })); + await writeFileContent(cacheB + "/mcp.json", JSON.stringify({ servers: { b: 2 } })); + + const localContent = { servers: { local: 0 } }; + const sources: SourceCacheEntry[] = [ + { sourceKey: "source-a", cachePath: cacheA }, + { sourceKey: "source-b", cachePath: cacheB }, + ]; + + type TestJson = { servers: Record }; + const result = await loadAndMergeJsonFeature({ + sources, + fileName: "mcp.json", + localContent, + mergeFn: (base, overlay) => ({ + servers: { ...overlay.servers, ...base.servers }, + }), + }); + + expect(result).toEqual({ servers: { local: 0, a: 1, b: 2 } }); + }); + + it("should give first-source-wins for overlapping MCP server names", async () => { + const cacheA = join(testDir, "source-a"); + const cacheB = join(testDir, "source-b"); + await ensureDir(cacheA); + await ensureDir(cacheB); + await writeFileContent( + cacheA + "/mcp.json", + JSON.stringify({ mcpServers: { shared: { command: "from-a" }, unique_a: { command: "a" } } }), + ); + await writeFileContent( + cacheB + "/mcp.json", + JSON.stringify({ mcpServers: { shared: { command: "from-b" }, unique_b: { command: "b" } } }), + ); + + const sources: SourceCacheEntry[] = [ + { sourceKey: "source-a", cachePath: cacheA }, + { sourceKey: "source-b", cachePath: cacheB }, + ]; + + type McpJson = { mcpServers: Record }; + const result = await loadAndMergeJsonFeature({ + sources, + fileName: "mcp.json", + localContent: undefined, + mergeFn: (base, overlay) => ({ + mcpServers: { ...overlay.mcpServers, ...base.mcpServers }, + }), + }); + + // source-a wins for "shared" since it is declared first + expect(result!.mcpServers["shared"]!.command).toBe("from-a"); + expect(result!.mcpServers["unique_a"]!.command).toBe("a"); + expect(result!.mcpServers["unique_b"]!.command).toBe("b"); + }); + + it("should let local MCP content override all source servers on conflict", async () => { + const cacheA = join(testDir, "source-a"); + await ensureDir(cacheA); + await writeFileContent( + cacheA + "/mcp.json", + JSON.stringify({ + mcpServers: { overlap: { command: "remote" }, remote_only: { command: "r" } }, + }), + ); + + const sources: SourceCacheEntry[] = [{ sourceKey: "source-a", cachePath: cacheA }]; + type McpJson = { mcpServers: Record }; + const result = await loadAndMergeJsonFeature({ + sources, + fileName: "mcp.json", + localContent: { mcpServers: { overlap: { command: "local" } } }, + mergeFn: (base, overlay) => ({ + mcpServers: { ...overlay.mcpServers, ...base.mcpServers }, + }), + }); + + // Local "overlap" wins over remote + expect(result!.mcpServers["overlap"]!.command).toBe("local"); + // Remote-only server is still included + expect(result!.mcpServers["remote_only"]!.command).toBe("r"); + }); + + it("should return undefined when no sources and no local content", async () => { + const result = await loadAndMergeJsonFeature({ + sources: [], + fileName: "mcp.json", + localContent: undefined, + mergeFn: (base) => base, + }); + + expect(result).toBeUndefined(); + }); + + it("should use first source as base when no local content", async () => { + const cachePath = join(testDir, "source-a"); + await ensureDir(cachePath); + await writeFileContent(cachePath + "/data.json", JSON.stringify({ value: 42 })); + + const result = await loadAndMergeJsonFeature<{ value: number }>({ + sources: [{ sourceKey: "source-a", cachePath }], + fileName: "data.json", + localContent: undefined, + mergeFn: (base) => base, + }); + + expect(result).toEqual({ value: 42 }); + }); +}); + +describe("loadAndMergeTextFeature", () => { + let testDir: string; + let cleanup: () => Promise; + + beforeEach(async () => { + ({ testDir, cleanup } = await setupTestDirectory()); + }); + + afterEach(async () => { + await cleanup(); + }); + + it("should merge text from sources after local content", async () => { + const cachePath = join(testDir, "source-a"); + await ensureDir(cachePath); + await writeFileContent(join(cachePath, ".aiignore"), "*.bak\n"); + + const result = await loadAndMergeTextFeature({ + sources: [{ sourceKey: "source-a", cachePath }], + fileName: ".aiignore", + localContent: "*.log\n", + mergeFn: (base, overlay) => base + overlay, + }); + + expect(result).toBe("*.log\n*.bak\n"); + }); + + it("should return undefined when no sources and no local content", async () => { + const result = await loadAndMergeTextFeature({ + sources: [], + fileName: ".aiignore", + localContent: undefined, + mergeFn: (base, overlay) => base + overlay, + }); + + expect(result).toBeUndefined(); + }); +}); diff --git a/src/lib/source-cache.ts b/src/lib/source-cache.ts new file mode 100644 index 000000000..174077963 --- /dev/null +++ b/src/lib/source-cache.ts @@ -0,0 +1,293 @@ +import { basename, join } from "node:path"; + +import type { SourceEntry } from "../config/config.js"; +import { RULESYNC_SOURCES_RELATIVE_DIR_PATH } from "../constants/rulesync-paths.js"; +import { formatError } from "../utils/error.js"; +import { directoryExists, fileExists, findFilesByGlobs, readFileContent } from "../utils/file.js"; +import { parseFrontmatter } from "../utils/frontmatter.js"; +import { logger } from "../utils/logger.js"; +import { normalizeSourceKey } from "./sources-lock.js"; + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +export type SourceCacheEntry = { + /** The original source key (e.g. "owner/repo"). */ + sourceKey: string; + /** Absolute path to the source's cache directory. */ + cachePath: string; +}; + +export type SourceItem = { + /** Item name (e.g. "my-skill" for a dir, "coding.md" for a file). */ + name: string; + /** Absolute path to the item (directory for skills, file for rules/commands/subagents). */ + path: string; + /** The source this item came from. */ + sourceKey: string; +}; + +// --------------------------------------------------------------------------- +// Cache directory naming +// --------------------------------------------------------------------------- + +/** + * Convert a normalized source key to a filesystem-safe directory name. + * Replaces `/` with `--` to avoid nested directories. + */ +export function sourceKeyToDirName(sourceKey: string): string { + const normalized = normalizeSourceKey(sourceKey); + return normalized.replace(/\//g, "--"); +} + +// --------------------------------------------------------------------------- +// Ordered source cache resolution +// --------------------------------------------------------------------------- + +/** + * Get ordered source cache entries based on config's sources array. + * Returns only entries whose cache directories exist on disk. + */ +export async function getOrderedSourceCaches(params: { + baseDir: string; + sources: SourceEntry[]; +}): Promise { + const { baseDir, sources } = params; + const entries: SourceCacheEntry[] = []; + + for (const source of sources) { + const dirName = sourceKeyToDirName(source.source); + const cachePath = join(baseDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH, dirName); + + if (await directoryExists(cachePath)) { + entries.push({ sourceKey: source.source, cachePath }); + } + } + + return entries; +} + +// --------------------------------------------------------------------------- +// Directory-item loading (skills: subdirectories, rules/commands/subagents: files) +// --------------------------------------------------------------------------- + +/** + * Load directory-based items (each item is a subdirectory) from source caches. + * Used for skills where each skill is a directory containing files. + */ +export async function loadDirItemsFromSources(params: { + sources: SourceCacheEntry[]; + featureDirName: string; + localNames: Set; +}): Promise { + const { sources, featureDirName, localNames } = params; + const items: SourceItem[] = []; + const seen = new Set(localNames); + + for (const source of sources) { + const featureDir = join(source.cachePath, featureDirName); + if (!(await directoryExists(featureDir))) continue; + + const dirPaths = await findFilesByGlobs(join(featureDir, "*"), { type: "dir" }); + for (const dirPath of dirPaths) { + const name = basename(dirPath); + if (seen.has(name)) { + logger.debug( + `Skipping ${featureDirName} "${name}" from ${source.sourceKey}: already provided.`, + ); + continue; + } + seen.add(name); + items.push({ name, path: dirPath, sourceKey: source.sourceKey }); + } + } + + return items; +} + +/** + * Load file-based items (each item is a single file) from source caches. + * Used for rules, commands, subagents where each item is a .md file. + */ +export async function loadFileItemsFromSources(params: { + sources: SourceCacheEntry[]; + featureDirName: string; + globPattern: string; + localNames: Set; +}): Promise { + const { sources, featureDirName, globPattern, localNames } = params; + const items: SourceItem[] = []; + const seen = new Set(localNames); + + for (const source of sources) { + const featureDir = join(source.cachePath, featureDirName); + if (!(await directoryExists(featureDir))) continue; + + const filePaths = await findFilesByGlobs(join(featureDir, globPattern)); + for (const filePath of filePaths) { + const name = basename(filePath); + if (seen.has(name)) { + logger.debug( + `Skipping ${featureDirName} "${name}" from ${source.sourceKey}: already provided.`, + ); + continue; + } + seen.add(name); + items.push({ name, path: filePath, sourceKey: source.sourceKey }); + } + } + + return items; +} + +/** + * Parsed source item returned by loadParsedFileItemsFromSources. + * Contains the validated frontmatter, body, and raw content for each item. + */ +export type ParsedSourceItem = { + name: string; + sourceKey: string; + frontmatter: T; + body: string; + content: string; +}; + +/** + * Load file items from source caches, then read, parse frontmatter, and validate. + * Returns only items that pass schema validation. Skips and warns on failures. + * This eliminates the duplicated read/parse/validate loop across processors. + */ +export async function loadParsedFileItemsFromSources(params: { + sources: SourceCacheEntry[]; + featureDirName: string; + globPattern: string; + localNames: Set; + schema: { safeParse(data: unknown): { success: true; data: T } | { success: false } }; +}): Promise[]> { + const { sources, featureDirName, globPattern, localNames, schema } = params; + const items = await loadFileItemsFromSources({ + sources, + featureDirName, + globPattern, + localNames, + }); + const results: ParsedSourceItem[] = []; + + for (const item of items) { + try { + const content = await readFileContent(item.path); + const { frontmatter, body } = parseFrontmatter(content, item.path); + const result = schema.safeParse(frontmatter); + if (!result.success) { + logger.warn( + `Skipping source ${featureDirName} "${item.name}" from ${item.sourceKey}: invalid frontmatter.`, + ); + continue; + } + results.push({ + name: item.name, + sourceKey: item.sourceKey, + frontmatter: result.data, + body: body.trim(), + content, + }); + } catch (error) { + logger.warn( + `Failed to load source ${featureDirName} "${item.name}" from ${item.sourceKey}: ${formatError(error)}`, + ); + } + } + + return results; +} + +// --------------------------------------------------------------------------- +// Single-file feature merging +// --------------------------------------------------------------------------- + +/** + * Load and merge a JSON feature file across source caches. + * Local content is the base; sources overlay in declaration order. + * The mergeFn receives (accumulated, overlay) and returns the merged result. + */ +export async function loadAndMergeJsonFeature(params: { + sources: SourceCacheEntry[]; + fileName: string; + localContent: T | undefined; + mergeFn: (base: T, overlay: T) => T; + parseFn?: (raw: string) => T; +}): Promise { + const { sources, fileName, localContent, mergeFn, parseFn } = params; + const parse = parseFn ?? ((raw: string): T => JSON.parse(raw)); + let result: T | undefined = localContent; + + for (const source of sources) { + const filePath = join(source.cachePath, fileName); + if (!(await fileExists(filePath))) continue; + + try { + const content = await readFileContent(filePath); + const parsed = parse(content); + + if (result === undefined) { + result = parsed; + } else { + result = mergeFn(result, parsed); + } + logger.debug(`Merged ${fileName} from source ${source.sourceKey}`); + } catch (error) { + logger.warn( + `Failed to parse ${fileName} from source ${source.sourceKey}: ${formatError(error)}`, + ); + } + } + + return result; +} + +/** + * Load and merge a text feature file across source caches. + * Local content is the base; sources overlay in declaration order. + */ +export async function loadAndMergeTextFeature(params: { + sources: SourceCacheEntry[]; + fileName: string; + localContent: string | undefined; + mergeFn: (base: string, overlay: string) => string; +}): Promise { + const { sources, fileName, localContent, mergeFn } = params; + let result: string | undefined = localContent; + + for (const source of sources) { + const filePath = join(source.cachePath, fileName); + if (!(await fileExists(filePath))) continue; + + try { + const content = await readFileContent(filePath); + + if (result === undefined) { + result = content; + } else { + result = mergeFn(result, content); + } + logger.debug(`Merged ${fileName} from source ${source.sourceKey}`); + } catch (error) { + logger.warn( + `Failed to read ${fileName} from source ${source.sourceKey}: ${formatError(error)}`, + ); + } + } + + return result; +} + +/** + * List all files under a source cache directory for integrity tracking. + * Returns relative paths within the source cache (e.g. "skills/my-skill/SKILL.md"). + */ +export async function listSourceCacheFiles(cachePath: string): Promise { + if (!(await directoryExists(cachePath))) return []; + const files = await findFilesByGlobs(join(cachePath, "**", "*"), { type: "file" }); + return files.map((f) => f.substring(cachePath.length + 1)); +} diff --git a/src/lib/sources-lock.test.ts b/src/lib/sources-lock.test.ts index bd8fe29a8..c0cea4f0b 100644 --- a/src/lib/sources-lock.test.ts +++ b/src/lib/sources-lock.test.ts @@ -7,8 +7,10 @@ import { setupTestDirectory } from "../test-utils/test-directories.js"; import { readFileContent, writeFileContent } from "../utils/file.js"; import { LOCKFILE_VERSION, + computeFileIntegrity, computeSkillIntegrity, createEmptyLock, + getLockedFiles, getLockedSkillNames, getLockedSource, normalizeSourceKey, @@ -39,7 +41,7 @@ describe("sources-lock", () => { describe("createEmptyLock", () => { it("should return an empty lock structure", () => { const lock = createEmptyLock(); - expect(lock).toEqual({ lockfileVersion: 1, sources: {} }); + expect(lock).toEqual({ lockfileVersion: 2, sources: {} }); }); }); @@ -58,18 +60,19 @@ describe("sources-lock", () => { it("should return empty lock when file does not exist", async () => { const lock = await readLockFile({ baseDir: testDir }); - expect(lock).toEqual({ lockfileVersion: 1, sources: {} }); + expect(lock).toEqual({ lockfileVersion: 2, sources: {} }); }); - it("should parse a valid lockfile", async () => { + it("should parse a valid v2 lockfile", async () => { const lockContent = JSON.stringify({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "https://github.com/org/repo": { resolvedRef: VALID_SHA, - skills: { - "skill-a": { integrity: "sha256-abc" }, - "skill-b": { integrity: "sha256-def" }, + files: { + "skills/skill-a": { integrity: "sha256-abc" }, + "skills/skill-b": { integrity: "sha256-def" }, + "rules/coding.md": { integrity: "sha256-ghi" }, }, }, }, @@ -81,9 +84,10 @@ describe("sources-lock", () => { expect(lock.sources["https://github.com/org/repo"]).toEqual({ resolvedRef: VALID_SHA, - skills: { - "skill-a": { integrity: "sha256-abc" }, - "skill-b": { integrity: "sha256-def" }, + files: { + "skills/skill-a": { integrity: "sha256-abc" }, + "skills/skill-b": { integrity: "sha256-def" }, + "rules/coding.md": { integrity: "sha256-ghi" }, }, }); }); @@ -92,7 +96,7 @@ describe("sources-lock", () => { await writeFileContent(join(testDir, RULESYNC_SOURCES_LOCK_RELATIVE_FILE_PATH), "not-json"); const lock = await readLockFile({ baseDir: testDir }); - expect(lock).toEqual({ lockfileVersion: 1, sources: {} }); + expect(lock).toEqual({ lockfileVersion: 2, sources: {} }); }); it("should return empty lock for invalid schema", async () => { @@ -102,11 +106,11 @@ describe("sources-lock", () => { ); const lock = await readLockFile({ baseDir: testDir }); - expect(lock).toEqual({ lockfileVersion: 1, sources: {} }); + expect(lock).toEqual({ lockfileVersion: 2, sources: {} }); }); - it("should migrate legacy lockfile format", async () => { - const legacyContent = JSON.stringify({ + it("should migrate v0 lockfile format", async () => { + const v0Content = JSON.stringify({ sources: { "org/repo": { resolvedRef: "abc123", @@ -115,27 +119,59 @@ describe("sources-lock", () => { }, }); - await writeFileContent( - join(testDir, RULESYNC_SOURCES_LOCK_RELATIVE_FILE_PATH), - legacyContent, - ); + await writeFileContent(join(testDir, RULESYNC_SOURCES_LOCK_RELATIVE_FILE_PATH), v0Content); const lock = await readLockFile({ baseDir: testDir }); expect(lock).toEqual({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "org/repo": { resolvedRef: "abc123", + files: { + "skills/skill-a": { integrity: "" }, + "skills/skill-b": { integrity: "" }, + }, + }, + }, + }); + expect(logger.info).toHaveBeenCalledWith( + expect.stringContaining("Migrated v0 sources lockfile"), + ); + }); + + it("should migrate v1 lockfile format", async () => { + const v1Content = JSON.stringify({ + lockfileVersion: 1, + sources: { + "org/repo": { + resolvedRef: VALID_SHA, skills: { - "skill-a": { integrity: "" }, - "skill-b": { integrity: "" }, + "skill-a": { integrity: "sha256-abc" }, + "skill-b": { integrity: "sha256-def" }, + }, + }, + }, + }); + + await writeFileContent(join(testDir, RULESYNC_SOURCES_LOCK_RELATIVE_FILE_PATH), v1Content); + + const lock = await readLockFile({ baseDir: testDir }); + + expect(lock).toEqual({ + lockfileVersion: 2, + sources: { + "org/repo": { + resolvedRef: VALID_SHA, + files: { + "skills/skill-a": { integrity: "sha256-abc" }, + "skills/skill-b": { integrity: "sha256-def" }, }, }, }, }); expect(logger.info).toHaveBeenCalledWith( - expect.stringContaining("Migrated legacy sources lockfile"), + expect.stringContaining("Migrated v1 sources lockfile"), ); }); }); @@ -159,7 +195,7 @@ describe("sources-lock", () => { sources: { "https://github.com/org/repo": { resolvedRef: "abc123", - skills: { "skill-a": { integrity: "sha256-x" } }, + files: { "skills/skill-a": { integrity: "sha256-x" } }, }, }, }; @@ -226,6 +262,24 @@ describe("sources-lock", () => { it("should strip gitlab: prefix", () => { expect(normalizeSourceKey("gitlab:org/repo")).toBe("org/repo"); }); + + it("should normalize Azure DevOps URL to org/project/repo", () => { + expect(normalizeSourceKey("https://dev.azure.com/org/project/_git/repo")).toBe( + "org/project/repo", + ); + }); + + it("should normalize Azure DevOps URL with user prefix", () => { + expect(normalizeSourceKey("https://user@dev.azure.com/org/project/_git/repo")).toBe( + "org/project/repo", + ); + }); + + it("should normalize Azure DevOps URL with .git suffix", () => { + expect(normalizeSourceKey("https://dev.azure.com/org/project/_git/repo.git")).toBe( + "org/project/repo", + ); + }); }); describe("getLockedSource", () => { @@ -235,7 +289,7 @@ describe("sources-lock", () => { sources: { "https://github.com/org/repo": { resolvedRef: "abc123", - skills: { "skill-a": { integrity: "sha256-x" } }, + files: { "skills/skill-a": { integrity: "sha256-x" } }, }, }, }; @@ -244,7 +298,7 @@ describe("sources-lock", () => { expect(result).toEqual({ resolvedRef: "abc123", - skills: { "skill-a": { integrity: "sha256-x" } }, + files: { "skills/skill-a": { integrity: "sha256-x" } }, }); }); @@ -262,7 +316,7 @@ describe("sources-lock", () => { sources: { "org/repo": { resolvedRef: "abc123", - skills: { "skill-a": { integrity: "sha256-x" } }, + files: { "skills/skill-a": { integrity: "sha256-x" } }, }, }, }; @@ -271,7 +325,7 @@ describe("sources-lock", () => { const result = getLockedSource(lock, "https://github.com/org/repo"); expect(result).toEqual({ resolvedRef: "abc123", - skills: { "skill-a": { integrity: "sha256-x" } }, + files: { "skills/skill-a": { integrity: "sha256-x" } }, }); }); }); @@ -282,12 +336,12 @@ describe("sources-lock", () => { const result = setLockedSource(lock, "https://github.com/org/repo", { resolvedRef: "abc123", - skills: { "skill-a": { integrity: "sha256-x" } }, + files: { "skills/skill-a": { integrity: "sha256-x" } }, }); expect(result.sources["org/repo"]).toEqual({ resolvedRef: "abc123", - skills: { "skill-a": { integrity: "sha256-x" } }, + files: { "skills/skill-a": { integrity: "sha256-x" } }, }); }); @@ -297,24 +351,24 @@ describe("sources-lock", () => { sources: { "https://github.com/org/repo": { resolvedRef: "old-sha", - skills: { "skill-a": { integrity: "sha256-x" } }, + files: { "skills/skill-a": { integrity: "sha256-x" } }, }, }, }; const result = setLockedSource(lock, "https://github.com/org/repo", { resolvedRef: "new-sha", - skills: { - "skill-a": { integrity: "sha256-x" }, - "skill-b": { integrity: "sha256-y" }, + files: { + "skills/skill-a": { integrity: "sha256-x" }, + "skills/skill-b": { integrity: "sha256-y" }, }, }); expect(result.sources["org/repo"]).toEqual({ resolvedRef: "new-sha", - skills: { - "skill-a": { integrity: "sha256-x" }, - "skill-b": { integrity: "sha256-y" }, + files: { + "skills/skill-a": { integrity: "sha256-x" }, + "skills/skill-b": { integrity: "sha256-y" }, }, }); }); @@ -325,27 +379,27 @@ describe("sources-lock", () => { sources: { "https://github.com/org/repo": { resolvedRef: "old-sha", - skills: { "skill-a": { integrity: "sha256-x" } }, + files: { "skills/skill-a": { integrity: "sha256-x" } }, }, }, }; const result = setLockedSource(lock, "org/repo", { resolvedRef: "new-sha", - skills: { "skill-b": { integrity: "sha256-y" } }, + files: { "skills/skill-b": { integrity: "sha256-y" } }, }); // Old URL-format entry should be removed, replaced with normalized key expect(result.sources["https://github.com/org/repo"]).toBeUndefined(); expect(result.sources["org/repo"]).toEqual({ resolvedRef: "new-sha", - skills: { "skill-b": { integrity: "sha256-y" } }, + files: { "skills/skill-b": { integrity: "sha256-y" } }, }); }); it("should not mutate the original lock", () => { const lock = { lockfileVersion: LOCKFILE_VERSION, sources: {} }; - const result = setLockedSource(lock, "key", { resolvedRef: "sha", skills: {} }); + const result = setLockedSource(lock, "key", { resolvedRef: "sha", files: {} }); expect(lock.sources).toEqual({}); expect(result.sources["key"]).toBeDefined(); @@ -381,25 +435,76 @@ describe("sources-lock", () => { }); describe("getLockedSkillNames", () => { - it("should return skill names from a locked source entry", () => { + it("should extract skill names from files keyed with skills/ prefix", () => { const entry = { resolvedRef: "sha", - skills: { - a: { integrity: "sha256-x" }, - b: { integrity: "sha256-y" }, + files: { + "skills/a": { integrity: "sha256-x" }, + "skills/b": { integrity: "sha256-y" }, + "rules/coding.md": { integrity: "sha256-z" }, }, }; expect(getLockedSkillNames(entry)).toEqual(["a", "b"]); }); - it("should return empty array for entry with no skills", () => { + it("should deduplicate skills with nested file paths", () => { + const entry = { + resolvedRef: "sha", + files: { + "skills/my-skill/SKILL.md": { integrity: "sha256-x" }, + "skills/my-skill/README.md": { integrity: "sha256-y" }, + }, + }; + + expect(getLockedSkillNames(entry)).toEqual(["my-skill"]); + }); + + it("should return empty array for entry with no skill files", () => { + const entry = { + resolvedRef: "sha", + files: { + "rules/coding.md": { integrity: "sha256-z" }, + }, + }; + + expect(getLockedSkillNames(entry)).toEqual([]); + }); + + it("should return empty array for entry with no files", () => { const entry = { resolvedRef: "sha", - skills: {}, + files: {}, }; expect(getLockedSkillNames(entry)).toEqual([]); }); }); + + describe("getLockedFiles", () => { + it("should return all files from a locked source entry", () => { + const files = { + "skills/a": { integrity: "sha256-x" }, + "rules/coding.md": { integrity: "sha256-y" }, + }; + const entry = { resolvedRef: "sha", files }; + + expect(getLockedFiles(entry)).toEqual(files); + }); + }); + + describe("computeFileIntegrity", () => { + it("should return sha256-prefixed hash", () => { + const result = computeFileIntegrity("hello world"); + expect(result).toMatch(/^sha256-[0-9a-f]+$/); + }); + + it("should produce different hash for different content", () => { + expect(computeFileIntegrity("hello")).not.toBe(computeFileIntegrity("world")); + }); + + it("should produce same hash for same content", () => { + expect(computeFileIntegrity("test")).toBe(computeFileIntegrity("test")); + }); + }); }); diff --git a/src/lib/sources-lock.ts b/src/lib/sources-lock.ts index d8b6c3eeb..2d58d4f60 100644 --- a/src/lib/sources-lock.ts +++ b/src/lib/sources-lock.ts @@ -8,18 +8,20 @@ import { fileExists, readFileContent, writeFileContent } from "../utils/file.js" import { logger } from "../utils/logger.js"; /** Current lockfile format version. Bump when the schema changes. */ -export const LOCKFILE_VERSION = 1; +export const LOCKFILE_VERSION = 2; /** - * Schema for a single locked skill entry with content integrity. + * Schema for a single locked file entry with content integrity. */ -export const LockedSkillSchema = z.object({ +export const LockedFileSchema = z.object({ integrity: z.string(), }); -export type LockedSkill = z.infer; +export type LockedFile = z.infer; /** - * Schema for a single locked source entry. + * Schema for a single locked source entry (v2: per-file integrity). + * Keys in `files` are relative paths within the source cache + * (e.g. "skills/my-skill/SKILL.md", "rules/coding.md", "mcp.json"). */ export const LockedSourceSchema = z.object({ requestedRef: optional(z.string()), @@ -27,7 +29,7 @@ export const LockedSourceSchema = z.object({ .string() .check(refine((v) => /^[0-9a-f]{40}$/.test(v), "resolvedRef must be a 40-character hex SHA")), resolvedAt: optional(z.string()), - skills: z.record(z.string(), LockedSkillSchema), + files: z.record(z.string(), LockedFileSchema), }); export type LockedSource = z.infer; @@ -40,37 +42,69 @@ export const SourcesLockSchema = z.object({ }); export type SourcesLock = z.infer; -/** - * Schema for the legacy v0 lockfile format (skills as string array, no version field). - */ -const LegacyLockedSourceSchema = z.object({ +// --------------------------------------------------------------------------- +// Legacy schemas for migration +// --------------------------------------------------------------------------- + +/** v0 lockfile: skills as string array, no version field. */ +const V0LockedSourceSchema = z.object({ resolvedRef: z.string(), skills: z.array(z.string()), }); +const V0SourcesLockSchema = z.object({ + sources: z.record(z.string(), V0LockedSourceSchema), +}); -const LegacySourcesLockSchema = z.object({ - sources: z.record(z.string(), LegacyLockedSourceSchema), +/** v1 lockfile: skills as Record, lockfileVersion present. */ +const V1LockedSourceSchema = z.object({ + requestedRef: optional(z.string()), + resolvedRef: z.string(), + resolvedAt: optional(z.string()), + skills: z.record(z.string(), LockedFileSchema), +}); +const V1SourcesLockSchema = z.object({ + lockfileVersion: z.number(), + sources: z.record(z.string(), V1LockedSourceSchema), }); /** - * Migrate a legacy lockfile (string[] skills, no version) to the current format. - * Skills get empty integrity since we can't compute it retroactively. + * Migrate v0 lockfile (string[] skills, no version) to v2. + * Skill names mapped to files["skills/{name}"] with empty integrity. */ -function migrateLegacyLock(legacy: z.infer): SourcesLock { +function migrateV0Lock(legacy: z.infer): SourcesLock { const sources: Record = {}; for (const [key, entry] of Object.entries(legacy.sources)) { - const skills: Record = {}; + const files: Record = {}; for (const name of entry.skills) { - skills[name] = { integrity: "" }; + files[`skills/${name}`] = { integrity: "" }; + } + sources[key] = { resolvedRef: entry.resolvedRef, files }; + } + logger.info( + "Migrated v0 sources lockfile to version 2. Run 'rulesync install --update' to populate integrity hashes.", + ); + return { lockfileVersion: LOCKFILE_VERSION, sources }; +} + +/** + * Migrate v1 lockfile (skills record with integrity) to v2 (files record). + * Skill names mapped to files["skills/{name}"] preserving integrity. + */ +function migrateV1Lock(v1: z.infer): SourcesLock { + const sources: Record = {}; + for (const [key, entry] of Object.entries(v1.sources)) { + const files: Record = {}; + for (const [name, skill] of Object.entries(entry.skills)) { + files[`skills/${name}`] = { integrity: skill.integrity }; } sources[key] = { resolvedRef: entry.resolvedRef, - skills, + requestedRef: entry.requestedRef, + resolvedAt: entry.resolvedAt, + files, }; } - logger.info( - "Migrated legacy sources lockfile to version 1. Run 'rulesync install --update' to populate integrity hashes.", - ); + logger.info("Migrated v1 sources lockfile to version 2."); return { lockfileVersion: LOCKFILE_VERSION, sources }; } @@ -97,16 +131,22 @@ export async function readLockFile(params: { baseDir: string }): Promise): string { @@ -148,6 +188,15 @@ export function computeSkillIntegrity(files: Array<{ path: string; content: stri return `sha256-${hash.digest("hex")}`; } +/** + * Compute a SHA-256 integrity hash for a single file's content. + */ +export function computeFileIntegrity(content: string): string { + const hash = createHash("sha256"); + hash.update(content); + return `sha256-${hash.digest("hex")}`; +} + /** * Normalize a source key for consistent lockfile lookups. * Strips URL prefixes, provider prefixes, trailing slashes, .git suffix, and lowercases. @@ -172,6 +221,15 @@ export function normalizeSourceKey(source: string): string { } } + // Normalize Azure DevOps URLs to org/project/repo + // Matches: https://[user@]dev.azure.com/org/project/_git/repo + const adoMatch = key.match( + /^https?:\/\/(?:[^@]+@)?dev\.azure\.com\/([^/]+)\/([^/]+)\/_git\/([^/]+?)(?:\.git)?(?:\/.*)?$/i, + ); + if (adoMatch) { + key = `${adoMatch[1]}/${adoMatch[2]}/${adoMatch[3]}`; + } + // Strip provider prefix for (const provider of ["github:", "gitlab:"]) { if (key.startsWith(provider)) { @@ -233,7 +291,25 @@ export function setLockedSource( /** * Get the skill names from a locked source entry. + * Extracts skill directory names from files keyed as "skills/{name}" or "skills/{name}/...". */ export function getLockedSkillNames(entry: LockedSource): string[] { - return Object.keys(entry.skills); + const names = new Set(); + for (const filePath of Object.keys(entry.files)) { + if (filePath.startsWith("skills/")) { + const rest = filePath.substring("skills/".length); + const name = rest.split("/")[0]; + if (name) { + names.add(name); + } + } + } + return [...names]; +} + +/** + * Get all locked file paths for a source entry. + */ +export function getLockedFiles(entry: LockedSource): Record { + return entry.files; } diff --git a/src/lib/sources.test.ts b/src/lib/sources.test.ts index 4bed08dbd..201374c29 100644 --- a/src/lib/sources.test.ts +++ b/src/lib/sources.test.ts @@ -2,11 +2,12 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH } from "../constants/rulesync-paths.js"; +import { RULESYNC_SOURCES_RELATIVE_DIR_PATH } from "../constants/rulesync-paths.js"; import { setupTestDirectory } from "../test-utils/test-directories.js"; import { directoryExists, - findFilesByGlobs, + ensureDir, + listDirectoryFiles, removeDirectory, writeFileContent, } from "../utils/file.js"; @@ -46,9 +47,11 @@ vi.mock("../utils/file.js", async (importOriginal) => { return { ...actual, directoryExists: vi.fn(), - findFilesByGlobs: vi.fn(), + ensureDir: vi.fn(), + listDirectoryFiles: vi.fn().mockResolvedValue([]), removeDirectory: vi.fn(), writeFileContent: vi.fn(), + checkPathTraversal: actual.checkPathTraversal, }; }); @@ -78,14 +81,27 @@ vi.mock("./git-client.js", () => ({ resetGitCheck: vi.fn(), resolveDefaultRef: vi.fn(), resolveRefToSha: vi.fn(), - fetchSkillFiles: vi.fn(), + fetchSourceCacheFiles: vi.fn(), })); +vi.mock("./github-utils.js", () => ({ + listDirectoryRecursive: vi.fn().mockResolvedValue([]), + withSemaphore: vi.fn((_semaphore: any, fn: () => any) => fn()), +})); + +vi.mock("./source-cache.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + sourceKeyToDirName: actual.sourceKeyToDirName, + }; +}); + vi.mock("./sources-lock.js", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, - readLockFile: vi.fn().mockResolvedValue({ lockfileVersion: 1, sources: {} }), + readLockFile: vi.fn().mockResolvedValue({ lockfileVersion: 2, sources: {} }), writeLockFile: vi.fn().mockResolvedValue(undefined), }; }); @@ -105,11 +121,11 @@ describe("resolveAndFetchSources", () => { getFileContent: vi.fn().mockResolvedValue("file content"), }; - // Default: no curated dir, no local skills + // Default: no source cache dir exists vi.mocked(directoryExists).mockResolvedValue(false); - vi.mocked(findFilesByGlobs).mockResolvedValue([]); vi.mocked(removeDirectory).mockResolvedValue(undefined); vi.mocked(writeFileContent).mockResolvedValue(undefined); + vi.mocked(ensureDir).mockResolvedValue(undefined as any); }); afterEach(async () => { @@ -123,7 +139,7 @@ describe("resolveAndFetchSources", () => { baseDir: testDir, }); - expect(result).toEqual({ fetchedSkillCount: 0, sourcesProcessed: 0 }); + expect(result).toEqual({ fetchedFileCount: 0, sourcesProcessed: 0 }); }); it("should skip fetching when skipSources is true", async () => { @@ -133,68 +149,65 @@ describe("resolveAndFetchSources", () => { options: { skipSources: true }, }); - expect(result).toEqual({ fetchedSkillCount: 0, sourcesProcessed: 0 }); + expect(result).toEqual({ fetchedFileCount: 0, sourcesProcessed: 0 }); expect(mockClientInstance.getDefaultBranch).not.toHaveBeenCalled(); }); - it("should clean per-source locked skill directories before re-fetching", async () => { + it("should clean source cache before re-fetching", async () => { const { readLockFile } = await import("./sources-lock.js"); - const curatedDir = join(testDir, RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH); + const sourceCacheDir = join(testDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH, "org--repo"); - // Pre-existing lock with previously fetched skills + // Pre-existing lock with previously fetched files vi.mocked(readLockFile).mockResolvedValue({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "https://github.com/org/repo": { resolvedRef: "locked-sha", - skills: { - "old-skill-a": { integrity: "sha256-aaa" }, - "old-skill-b": { integrity: "sha256-bbb" }, + files: { + "skills/old-skill-a/SKILL.md": { integrity: "sha256-aaa" }, }, }, }, }); - // old-skill-a exists on disk, old-skill-b does not. - // Because old-skill-b is missing, the SHA-match skip check fails, - // and per-source cleanup runs for old-skill-a (which exists). - vi.mocked(directoryExists).mockImplementation(async (path: string) => { - if (path === join(curatedDir, "old-skill-a")) return true; - return false; - }); + // Source cache dir does not exist, so SHA-match skip fails and re-fetch triggers + vi.mocked(directoryExists).mockResolvedValue(false); - // No remote skills after cleanup + // No remote content after cleanup mockClientInstance.listDirectory.mockResolvedValue([]); + // getFileContent throws 404 for single-file features + const { GitHubClientError } = await import("./github-client.js"); + mockClientInstance.getFileContent.mockRejectedValue(new GitHubClientError("Not Found", 404)); await resolveAndFetchSources({ sources: [{ source: "https://github.com/org/repo" }], baseDir: testDir, }); - // Only old-skill-a should be removed (it existed on disk) - expect(removeDirectory).toHaveBeenCalledWith(join(curatedDir, "old-skill-a")); - // Should NOT do a blanket removal of the curated dir - expect(removeDirectory).not.toHaveBeenCalledWith(curatedDir); + // cleanSourceCache should call removeDirectory then ensureDir on the source cache path + // Since directoryExists returns false, removeDirectory may not be called, + // but ensureDir should be called for the cache path + expect(ensureDir).toHaveBeenCalledWith(sourceCacheDir); }); - it("should skip re-fetch when SHA matches lockfile and skills exist on disk", async () => { + it("should skip re-fetch when SHA matches lockfile and source cache exists on disk", async () => { const { readLockFile } = await import("./sources-lock.js"); - const curatedDir = join(testDir, RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH); + const sourceCacheDir = join(testDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH, "org--repo"); - // Lock has a source with resolved SHA and skills + // Lock has a source with resolved SHA vi.mocked(readLockFile).mockResolvedValue({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "https://github.com/org/repo": { resolvedRef: "locked-sha-123", - skills: { "cached-skill": { integrity: "sha256-cached" } }, + files: { "skills/cached-skill/SKILL.md": { integrity: "sha256-cached" } }, }, }, }); - // All locked skill dirs exist on disk + // Source cache directory exists on disk vi.mocked(directoryExists).mockImplementation(async (path: string) => { - if (path === join(curatedDir, "cached-skill")) return true; + if (path === sourceCacheDir) return true; return false; }); @@ -205,60 +218,52 @@ describe("resolveAndFetchSources", () => { // Should not call listDirectory (no re-fetch) expect(mockClientInstance.listDirectory).not.toHaveBeenCalled(); - // fetchedSkillCount is 0 because nothing was newly fetched - expect(result.fetchedSkillCount).toBe(0); + // fetchedFileCount is 0 because nothing was newly fetched + expect(result.fetchedFileCount).toBe(0); expect(result.sourcesProcessed).toBe(1); // removeDirectory should not have been called (no cleanup needed) expect(removeDirectory).not.toHaveBeenCalled(); }); it("should fetch skills from a remote source", async () => { - // Mock: remote has one skill directory with one file + const { listDirectoryRecursive } = await import("./github-utils.js"); + + // Mock: listDirectory returns skill dirs for "skills" path, empty/404 for others + const { GitHubClientError } = await import("./github-client.js"); mockClientInstance.listDirectory.mockImplementation( async (_owner: string, _repo: string, path: string) => { if (path === "skills") { return [{ name: "my-skill", path: "skills/my-skill", type: "dir" }]; } - if (path === "skills/my-skill") { - return [{ name: "SKILL.md", path: "skills/my-skill/SKILL.md", type: "file", size: 100 }]; - } - return []; + // Other directory features return 404 + throw new GitHubClientError("Not Found", 404); }, ); - mockClientInstance.getFileContent.mockResolvedValue("# My Skill\nContent here."); - - const result = await resolveAndFetchSources({ - sources: [{ source: "https://github.com/org/repo" }], - baseDir: testDir, - }); - - expect(result.fetchedSkillCount).toBe(1); - expect(result.sourcesProcessed).toBe(1); - - const expectedFilePath = join( - testDir, - RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH, - "my-skill", - "SKILL.md", - ); - expect(writeFileContent).toHaveBeenCalledWith(expectedFilePath, "# My Skill\nContent here."); - }); - - it("should skip skills that exist locally", async () => { - // Local skill "my-skill" exists - vi.mocked(directoryExists).mockImplementation(async (path: string) => { - if (path.endsWith("skills")) return true; - return false; - }); - vi.mocked(findFilesByGlobs).mockResolvedValue([join(testDir, ".rulesync/skills/my-skill")]); + // Single-file features return 404 + mockClientInstance.getFileContent.mockRejectedValue(new GitHubClientError("Not Found", 404)); + + // listDirectoryRecursive returns files within the skill dir + vi.mocked(listDirectoryRecursive).mockResolvedValue([ + { + name: "SKILL.md", + path: "skills/my-skill/SKILL.md", + type: "file", + size: 100, + sha: "abc123", + download_url: null, + }, + ]); - // Remote has same skill name - mockClientInstance.listDirectory.mockImplementation( + // Mock getFileContent to succeed when called via withSemaphore for skill files + const { withSemaphore } = await import("./github-utils.js"); + vi.mocked(withSemaphore).mockImplementation(async (_sem: any, fn: () => any) => fn()); + // Re-mock getFileContent to handle both skill files and single-file features + mockClientInstance.getFileContent.mockImplementation( async (_owner: string, _repo: string, path: string) => { - if (path === "skills") { - return [{ name: "my-skill", path: "skills/my-skill", type: "dir" }]; + if (path === "skills/my-skill/SKILL.md") { + return "# My Skill\nContent here."; } - return []; + throw new GitHubClientError("Not Found", 404); }, ); @@ -267,12 +272,21 @@ describe("resolveAndFetchSources", () => { baseDir: testDir, }); - // Skill should be skipped since local takes precedence - expect(result.fetchedSkillCount).toBe(0); + expect(result.fetchedFileCount).toBeGreaterThanOrEqual(1); + expect(result.sourcesProcessed).toBe(1); + + const sourceCacheDir = join(testDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH, "org--repo"); + const expectedFilePath = join(sourceCacheDir, "skills", "my-skill", "SKILL.md"); + expect(writeFileContent).toHaveBeenCalledWith(expectedFilePath, "# My Skill\nContent here."); }); it("should respect skill filter", async () => { - // Remote has two skills + const { listDirectoryRecursive } = await import("./github-utils.js"); + const { GitHubClientError } = await import("./github-client.js"); + const { withSemaphore } = await import("./github-utils.js"); + vi.mocked(withSemaphore).mockImplementation(async (_sem: any, fn: () => any) => fn()); + + // Remote has two skill dirs mockClientInstance.listDirectory.mockImplementation( async (_owner: string, _repo: string, path: string) => { if (path === "skills") { @@ -281,96 +295,120 @@ describe("resolveAndFetchSources", () => { { name: "skill-b", path: "skills/skill-b", type: "dir" }, ]; } - if (path === "skills/skill-a") { - return [{ name: "SKILL.md", path: "skills/skill-a/SKILL.md", type: "file", size: 50 }]; - } - return []; + throw new GitHubClientError("Not Found", 404); + }, + ); + mockClientInstance.getFileContent.mockImplementation( + async (_owner: string, _repo: string, path: string) => { + if (path.startsWith("skills/")) return "content"; + throw new GitHubClientError("Not Found", 404); }, ); - mockClientInstance.getFileContent.mockResolvedValue("content"); - const result = await resolveAndFetchSources({ + // listDirectoryRecursive returns files for whichever skill dir is requested + vi.mocked(listDirectoryRecursive).mockImplementation(async (params: any) => { + if (params.path === "skills/skill-a") { + return [ + { + name: "SKILL.md", + path: "skills/skill-a/SKILL.md", + type: "file", + size: 50, + sha: "abc123", + download_url: null, + }, + ]; + } + return []; + }); + + await resolveAndFetchSources({ sources: [{ source: "https://github.com/org/repo", skills: ["skill-a"] }], baseDir: testDir, }); // Only skill-a should be fetched - expect(result.fetchedSkillCount).toBe(1); const writeArgs = vi.mocked(writeFileContent).mock.calls.map((call) => call[0]); expect(writeArgs.some((p) => p.includes("skill-a"))).toBe(true); expect(writeArgs.some((p) => p.includes("skill-b"))).toBe(false); }); - it("should skip duplicate skills from later sources", async () => { - // Both sources have "shared-skill" - mockClientInstance.listDirectory.mockImplementation( - async (_owner: string, _repo: string, path: string) => { - if (path === "skills") { - return [{ name: "shared-skill", path: "skills/shared-skill", type: "dir" }]; - } - if (path === "skills/shared-skill") { - return [ - { name: "SKILL.md", path: "skills/shared-skill/SKILL.md", type: "file", size: 50 }, - ]; - } - return []; - }, - ); - mockClientInstance.getFileContent.mockResolvedValue("content"); + it("should handle 404 for skills directory gracefully", async () => { + const { GitHubClientError } = await import("./github-client.js"); + // All listDirectory and getFileContent calls return 404 + mockClientInstance.listDirectory.mockRejectedValue(new GitHubClientError("Not Found", 404)); + mockClientInstance.getFileContent.mockRejectedValue(new GitHubClientError("Not Found", 404)); const result = await resolveAndFetchSources({ - sources: [ - { source: "https://github.com/org/repo-a" }, - { source: "https://github.com/org/repo-b" }, - ], + sources: [{ source: "https://github.com/org/repo" }], baseDir: testDir, }); - // First source fetches it, second source skips it - expect(result.fetchedSkillCount).toBe(1); + // Should not throw, just skip features that are not found + expect(result.fetchedFileCount).toBe(0); + expect(result.sourcesProcessed).toBe(1); }); - it("should handle 404 for skills directory gracefully", async () => { + it("should remove empty cache directory when no files are fetched", async () => { const { GitHubClientError } = await import("./github-client.js"); + // All features return 404 mockClientInstance.listDirectory.mockRejectedValue(new GitHubClientError("Not Found", 404)); + mockClientInstance.getFileContent.mockRejectedValue(new GitHubClientError("Not Found", 404)); - const result = await resolveAndFetchSources({ - sources: [{ source: "https://github.com/org/repo" }], + await resolveAndFetchSources({ + sources: [{ source: "https://github.com/org/empty-repo" }], baseDir: testDir, }); - // Should not throw, just skip the source - expect(result.fetchedSkillCount).toBe(0); - expect(result.sourcesProcessed).toBe(1); + const sourceCacheDir = join(testDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH, "org--empty-repo"); + // ensureDir was called (cleanSourceCache creates it), then removeDirectory cleans it up + expect(ensureDir).toHaveBeenCalledWith(sourceCacheDir); + expect(removeDirectory).toHaveBeenCalledWith(sourceCacheDir); }); it("should re-resolve refs when updateSources is true", async () => { const { readLockFile } = await import("./sources-lock.js"); + const { GitHubClientError } = await import("./github-client.js"); // Pre-existing lock has a different SHA for the same source vi.mocked(readLockFile).mockResolvedValue({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "https://github.com/org/repo": { resolvedRef: "old-locked-sha-should-be-ignored", - skills: { "my-skill": { integrity: "sha256-xxx" } }, + files: { "skills/my-skill/SKILL.md": { integrity: "sha256-xxx" } }, }, }, }); - // Set up mock: remote has one skill + const { listDirectoryRecursive, withSemaphore } = await import("./github-utils.js"); + vi.mocked(withSemaphore).mockImplementation(async (_sem: any, fn: () => any) => fn()); + + // Remote has one skill mockClientInstance.listDirectory.mockImplementation( async (_owner: string, _repo: string, path: string) => { if (path === "skills") { return [{ name: "my-skill", path: "skills/my-skill", type: "dir" }]; } - if (path === "skills/my-skill") { - return [{ name: "SKILL.md", path: "skills/my-skill/SKILL.md", type: "file", size: 100 }]; - } - return []; + throw new GitHubClientError("Not Found", 404); + }, + ); + vi.mocked(listDirectoryRecursive).mockResolvedValue([ + { + name: "SKILL.md", + path: "skills/my-skill/SKILL.md", + type: "file", + size: 100, + sha: "abc123", + download_url: null, + }, + ]); + mockClientInstance.getFileContent.mockImplementation( + async (_owner: string, _repo: string, path: string) => { + if (path.startsWith("skills/")) return "content"; + throw new GitHubClientError("Not Found", 404); }, ); - mockClientInstance.getFileContent.mockResolvedValue("content"); const result = await resolveAndFetchSources({ sources: [{ source: "https://github.com/org/repo" }], @@ -379,12 +417,15 @@ describe("resolveAndFetchSources", () => { }); // updateSources: true creates empty lock, so resolveRefToSha must be called - // (proving the pre-existing lock entry "old-locked-sha-should-be-ignored" was ignored) expect(mockClientInstance.resolveRefToSha).toHaveBeenCalled(); - expect(result.fetchedSkillCount).toBe(1); + expect(result.fetchedFileCount).toBeGreaterThanOrEqual(1); }); it("should continue processing other sources when one source fails", async () => { + const { GitHubClientError } = await import("./github-client.js"); + const { listDirectoryRecursive, withSemaphore } = await import("./github-utils.js"); + vi.mocked(withSemaphore).mockImplementation(async (_sem: any, fn: () => any) => fn()); + let resolveCallCount = 0; mockClientInstance.resolveRefToSha.mockImplementation(async () => { resolveCallCount++; @@ -400,13 +441,25 @@ describe("resolveAndFetchSources", () => { if (path === "skills") { return [{ name: "good-skill", path: "skills/good-skill", type: "dir" }]; } - if (path === "skills/good-skill") { - return [{ name: "SKILL.md", path: "skills/good-skill/SKILL.md", type: "file", size: 50 }]; - } - return []; + throw new GitHubClientError("Not Found", 404); + }, + ); + vi.mocked(listDirectoryRecursive).mockResolvedValue([ + { + name: "SKILL.md", + path: "skills/good-skill/SKILL.md", + type: "file", + size: 50, + sha: "abc123", + download_url: null, + }, + ]); + mockClientInstance.getFileContent.mockImplementation( + async (_owner: string, _repo: string, path: string) => { + if (path.startsWith("skills/")) return "content"; + throw new GitHubClientError("Not Found", 404); }, ); - mockClientInstance.getFileContent.mockResolvedValue("content"); const result = await resolveAndFetchSources({ sources: [ @@ -417,42 +470,48 @@ describe("resolveAndFetchSources", () => { }); // Second source should succeed despite first failing - expect(result.fetchedSkillCount).toBe(1); + expect(result.fetchedFileCount).toBeGreaterThanOrEqual(1); expect(result.sourcesProcessed).toBe(2); }); it("should handle GitLab source gracefully", async () => { + const { GitHubClientError } = await import("./github-client.js"); + // Mock getFileContent for non-gitlab sources, but the gitlab source won't reach it + mockClientInstance.listDirectory.mockRejectedValue(new GitHubClientError("Not Found", 404)); + mockClientInstance.getFileContent.mockRejectedValue(new GitHubClientError("Not Found", 404)); + const result = await resolveAndFetchSources({ sources: [{ source: "gitlab:org/repo" }], baseDir: testDir, }); - // Should not throw, but log error and skip - expect(result.fetchedSkillCount).toBe(0); + // Should not throw, but log warning and skip + expect(result.fetchedFileCount).toBe(0); expect(result.sourcesProcessed).toBe(1); }); it("should prune stale lockfile entries and preserve current sources", async () => { const { readLockFile, writeLockFile } = await import("./sources-lock.js"); + const sourceCacheDir = join(testDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH, "org--new-repo"); // Pre-existing lock has entries for both a removed and a current source vi.mocked(readLockFile).mockResolvedValue({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "org/old-removed-repo": { resolvedRef: "old-sha", - skills: { "old-skill": { integrity: "sha256-old" } }, + files: { "skills/old-skill/SKILL.md": { integrity: "sha256-old" } }, }, "org/new-repo": { resolvedRef: "existing-sha", - skills: { "kept-skill": { integrity: "sha256-kept" } }, + files: { "skills/kept-skill/SKILL.md": { integrity: "sha256-kept" } }, }, }, }); - // All locked skill dirs exist on disk (for SHA-match skip) + // Source cache exists on disk (for SHA-match skip) vi.mocked(directoryExists).mockImplementation(async (path: string) => { - if (path.includes("kept-skill")) return true; + if (path === sourceCacheDir) return true; return false; }); @@ -470,23 +529,61 @@ describe("resolveAndFetchSources", () => { expect(writtenLock.sources["org/new-repo"]).toBeDefined(); }); + it("should remove orphaned source cache directories for removed sources", async () => { + const { readLockFile } = await import("./sources-lock.js"); + const sourcesDir = join(testDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH); + const currentCacheDir = join(sourcesDir, "org--current-repo"); + + vi.mocked(readLockFile).mockResolvedValue({ + lockfileVersion: 2, + sources: { + "org/current-repo": { + resolvedRef: "sha-123", + files: { "skills/my-skill/SKILL.md": { integrity: "sha256-x" } }, + }, + }, + }); + + // Source cache exists on disk for SHA-match skip + vi.mocked(directoryExists).mockImplementation(async (path: string) => { + if (path === currentCacheDir) return true; + if (path === sourcesDir) return true; + return false; + }); + + // .sources/ contains both the current source dir and an orphaned one + vi.mocked(listDirectoryFiles).mockResolvedValue(["org--current-repo", "org--removed-repo"]); + + await resolveAndFetchSources({ + sources: [{ source: "org/current-repo" }], + baseDir: testDir, + }); + + // The orphaned directory should be removed + expect(removeDirectory).toHaveBeenCalledWith(join(sourcesDir, "org--removed-repo")); + // The current source directory should NOT be removed + const removeCalls = vi.mocked(removeDirectory).mock.calls.map((c) => c[0]); + expect(removeCalls).not.toContain(join(sourcesDir, "org--current-repo")); + }); + it("should not prune current sources even when config uses different URL format than lock key", async () => { const { readLockFile, writeLockFile } = await import("./sources-lock.js"); + const sourceCacheDir = join(testDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH, "org--repo"); // Lock stored under normalized key vi.mocked(readLockFile).mockResolvedValue({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "org/repo": { resolvedRef: "sha-123", - skills: { "my-skill": { integrity: "sha256-xxx" } }, + files: { "skills/my-skill/SKILL.md": { integrity: "sha256-xxx" } }, }, }, }); - // All locked skill dirs exist + // Source cache exists on disk vi.mocked(directoryExists).mockImplementation(async (path: string) => { - if (path.includes("my-skill")) return true; + if (path === sourceCacheDir) return true; return false; }); @@ -505,40 +602,56 @@ describe("resolveAndFetchSources", () => { } }); - it("should skip skill directories with path traversal characters in name", async () => { - // Remote has skills with suspicious names + it("should reject files with path traversal via checkPathTraversal", async () => { + const { listDirectoryRecursive, withSemaphore } = await import("./github-utils.js"); + const { GitHubClientError } = await import("./github-client.js"); + vi.mocked(withSemaphore).mockImplementation(async (_sem: any, fn: () => any) => fn()); + + // Remote has a skill with a path-traversal name mockClientInstance.listDirectory.mockImplementation( async (_owner: string, _repo: string, path: string) => { if (path === "skills") { - return [ - { name: "../../evil", path: "skills/../../evil", type: "dir" }, - { name: "good-skill", path: "skills/good-skill", type: "dir" }, - ]; - } - if (path === "skills/good-skill") { - return [{ name: "SKILL.md", path: "skills/good-skill/SKILL.md", type: "file", size: 50 }]; + return [{ name: "../../evil", path: "skills/../../evil", type: "dir" }]; } - return []; + throw new GitHubClientError("Not Found", 404); + }, + ); + vi.mocked(listDirectoryRecursive).mockResolvedValue([ + { + name: "EVIL.md", + path: "skills/../../evil/EVIL.md", + type: "file", + size: 50, + sha: "abc123", + download_url: null, + }, + ]); + mockClientInstance.getFileContent.mockImplementation( + async (_owner: string, _repo: string, path: string) => { + if (path.startsWith("skills/")) return "content"; + throw new GitHubClientError("Not Found", 404); }, ); - mockClientInstance.getFileContent.mockResolvedValue("content"); + // The path traversal causes writeAndTrackFile to throw via checkPathTraversal, + // which propagates as a source-level error caught by the outer try/catch const result = await resolveAndFetchSources({ sources: [{ source: "https://github.com/org/repo" }], baseDir: testDir, }); - // Only the good skill should be fetched; the traversal one is skipped - expect(result.fetchedSkillCount).toBe(1); + // The evil file should never be written const writeArgs = vi.mocked(writeFileContent).mock.calls.map((call) => call[0]); expect(writeArgs.some((p) => p.includes("evil"))).toBe(false); - expect(writeArgs.some((p) => p.includes("good-skill"))).toBe(true); + // The source-level error means fetchedFileCount is 0 + expect(result.fetchedFileCount).toBe(0); + expect(result.sourcesProcessed).toBe(1); }); it("should throw when frozen and source not in lockfile", async () => { const { readLockFile } = await import("./sources-lock.js"); - vi.mocked(readLockFile).mockResolvedValue({ lockfileVersion: 1, sources: {} }); + vi.mocked(readLockFile).mockResolvedValue({ lockfileVersion: 2, sources: {} }); await expect( resolveAndFetchSources({ @@ -550,22 +663,22 @@ describe("resolveAndFetchSources", () => { expect(mockClientInstance.getDefaultBranch).not.toHaveBeenCalled(); }); - it("should succeed in frozen mode when lockfile covers all sources and skills exist on disk", async () => { + it("should succeed in frozen mode when lockfile covers all sources and cache exists on disk", async () => { const { readLockFile } = await import("./sources-lock.js"); - const curatedDir = join(testDir, RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH); + const sourceCacheDir = join(testDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH, "org--repo"); vi.mocked(readLockFile).mockResolvedValue({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "org/repo": { resolvedRef: "sha-123", - skills: { "my-skill": { integrity: "sha256-xxx" } }, + files: { "skills/my-skill/SKILL.md": { integrity: "sha256-xxx" } }, }, }, }); vi.mocked(directoryExists).mockImplementation(async (path: string) => { - if (path === join(curatedDir, "my-skill")) return true; + if (path === sourceCacheDir) return true; return false; }); @@ -575,24 +688,27 @@ describe("resolveAndFetchSources", () => { options: { frozen: true }, }); - expect(result.fetchedSkillCount).toBe(0); + expect(result.fetchedFileCount).toBe(0); expect(result.sourcesProcessed).toBe(1); }); - it("should fetch missing locked skills in frozen mode without writing lockfile", async () => { + it("should fetch missing locked source in frozen mode without writing lockfile", async () => { const { readLockFile, writeLockFile } = await import("./sources-lock.js"); + const { listDirectoryRecursive, withSemaphore } = await import("./github-utils.js"); + const { GitHubClientError } = await import("./github-client.js"); + vi.mocked(withSemaphore).mockImplementation(async (_sem: any, fn: () => any) => fn()); vi.mocked(readLockFile).mockResolvedValue({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "org/repo": { resolvedRef: "sha-123", - skills: { "missing-skill": { integrity: "sha256-xxx" } }, + files: { "skills/missing-skill/SKILL.md": { integrity: "sha256-xxx" } }, }, }, }); - // Skill dir does not exist on disk + // Source cache dir does not exist on disk vi.mocked(directoryExists).mockResolvedValue(false); mockClientInstance.listDirectory.mockImplementation( @@ -600,15 +716,25 @@ describe("resolveAndFetchSources", () => { if (path === "skills") { return [{ name: "missing-skill", path: "skills/missing-skill", type: "dir" }]; } - if (path === "skills/missing-skill") { - return [ - { name: "SKILL.md", path: "skills/missing-skill/SKILL.md", type: "file", size: 42 }, - ]; - } - return []; + throw new GitHubClientError("Not Found", 404); + }, + ); + vi.mocked(listDirectoryRecursive).mockResolvedValue([ + { + name: "SKILL.md", + path: "skills/missing-skill/SKILL.md", + type: "file", + size: 42, + sha: "abc123", + download_url: null, + }, + ]); + mockClientInstance.getFileContent.mockImplementation( + async (_owner: string, _repo: string, path: string) => { + if (path.startsWith("skills/")) return "locked skill content"; + throw new GitHubClientError("Not Found", 404); }, ); - mockClientInstance.getFileContent.mockResolvedValue("locked skill content"); const result = await resolveAndFetchSources({ sources: [{ source: "https://github.com/org/repo" }], @@ -616,7 +742,8 @@ describe("resolveAndFetchSources", () => { options: { frozen: true }, }); - expect(result).toEqual({ fetchedSkillCount: 1, sourcesProcessed: 1 }); + expect(result.fetchedFileCount).toBeGreaterThanOrEqual(1); + expect(result.sourcesProcessed).toBe(1); expect(mockClientInstance.getDefaultBranch).not.toHaveBeenCalled(); expect(mockClientInstance.resolveRefToSha).not.toHaveBeenCalled(); expect(writeLockFile).not.toHaveBeenCalled(); @@ -624,19 +751,22 @@ describe("resolveAndFetchSources", () => { it("should warn when computed integrity differs from locked hash", async () => { const { readLockFile } = await import("./sources-lock.js"); + const { listDirectoryRecursive, withSemaphore } = await import("./github-utils.js"); + const { GitHubClientError } = await import("./github-client.js"); + vi.mocked(withSemaphore).mockImplementation(async (_sem: any, fn: () => any) => fn()); // Lock has a source with a specific integrity hash vi.mocked(readLockFile).mockResolvedValue({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "org/repo": { resolvedRef: "locked-sha-123", - skills: { "my-skill": { integrity: "sha256-old-hash" } }, + files: { "skills/my-skill/SKILL.md": { integrity: "sha256-old-hash" } }, }, }, }); - // Skill dir is missing on disk so re-fetch is triggered + // Source cache is missing so re-fetch is triggered vi.mocked(directoryExists).mockResolvedValue(false); // Mock: remote has one skill with different content than what was locked @@ -646,145 +776,98 @@ describe("resolveAndFetchSources", () => { if (path === "skills") { return [{ name: "my-skill", path: "skills/my-skill", type: "dir" }]; } - if (path === "skills/my-skill") { - return [{ name: "SKILL.md", path: "skills/my-skill/SKILL.md", type: "file", size: 100 }]; - } - return []; + throw new GitHubClientError("Not Found", 404); }, ); - mockClientInstance.getFileContent.mockResolvedValue("tampered content"); - - await resolveAndFetchSources({ - sources: [{ source: "https://github.com/org/repo" }], - baseDir: testDir, - }); - - // Should have warned about integrity mismatch - expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining("Integrity mismatch")); - }); - - it("should preserve lock entries for skipped skills", async () => { - const { readLockFile, writeLockFile } = await import("./sources-lock.js"); - - // Lock has two skills for this source - vi.mocked(readLockFile).mockResolvedValue({ - lockfileVersion: 1, - sources: { - "org/repo": { - resolvedRef: "locked-sha", - skills: { - "local-skill": { integrity: "sha256-local" }, - "remote-skill": { integrity: "sha256-remote" }, - }, - }, + vi.mocked(listDirectoryRecursive).mockResolvedValue([ + { + name: "SKILL.md", + path: "skills/my-skill/SKILL.md", + type: "file", + size: 100, + sha: "abc123", + download_url: null, }, - }); - - // local-skill exists locally, so it will be skipped - vi.mocked(directoryExists).mockImplementation(async (path: string) => { - if (path.endsWith("skills")) return true; - return false; - }); - vi.mocked(findFilesByGlobs).mockResolvedValue([join(testDir, ".rulesync/skills/local-skill")]); - - // remote-skill doesn't exist on disk, so SHA-match skip fails and re-fetch happens - // Remote has only remote-skill - mockClientInstance.listDirectory.mockImplementation( + ]); + mockClientInstance.getFileContent.mockImplementation( async (_owner: string, _repo: string, path: string) => { - if (path === "skills") { - return [ - { name: "local-skill", path: "skills/local-skill", type: "dir" }, - { name: "remote-skill", path: "skills/remote-skill", type: "dir" }, - ]; - } - if (path === "skills/remote-skill") { - return [ - { - name: "SKILL.md", - path: "skills/remote-skill/SKILL.md", - type: "file", - size: 50, - }, - ]; - } - return []; + if (path.startsWith("skills/")) return "tampered content"; + throw new GitHubClientError("Not Found", 404); }, ); - mockClientInstance.getFileContent.mockResolvedValue("content"); await resolveAndFetchSources({ sources: [{ source: "https://github.com/org/repo" }], baseDir: testDir, }); - // The written lock should still have both skills - const writeCalls = vi.mocked(writeLockFile).mock.calls; - expect(writeCalls.length).toBeGreaterThan(0); - const writtenLock = writeCalls[0]![0].lock; - const sourceEntry = writtenLock.sources["org/repo"]; - expect(sourceEntry).toBeDefined(); - // local-skill should be preserved from locked entry (it was skipped due to local precedence) - expect(sourceEntry?.skills["local-skill"]).toBeDefined(); - // remote-skill should have been re-fetched with new integrity - expect(sourceEntry?.skills["remote-skill"]).toBeDefined(); + // Should have warned about integrity mismatch + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining("Integrity mismatch")); }); - it("should fetch skills via git transport", async () => { - const { resolveDefaultRef, fetchSkillFiles } = await import("./git-client.js"); + it("should fetch files via git transport", async () => { + const { resolveDefaultRef, fetchSourceCacheFiles } = await import("./git-client.js"); vi.mocked(resolveDefaultRef).mockResolvedValue({ ref: "main", sha: "abc123def456" }); - vi.mocked(fetchSkillFiles).mockResolvedValue([ - { relativePath: "my-skill/SKILL.md", content: "# My Skill", size: 100 }, + vi.mocked(fetchSourceCacheFiles).mockResolvedValue([ + { relativePath: "skills/my-skill/SKILL.md", content: "# My Skill", size: 100 }, ]); - const result = await resolveAndFetchSources({ + await resolveAndFetchSources({ sources: [{ source: "https://dev.azure.com/org/project/_git/repo", transport: "git" }], baseDir: testDir, }); - expect(result.fetchedSkillCount).toBe(1); expect(mockClientInstance.listDirectory).not.toHaveBeenCalled(); - expect(writeFileContent).toHaveBeenCalledWith( - join(testDir, RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH, "my-skill", "SKILL.md"), - "# My Skill", - ); + // File should be written to source cache, not curated dir + const writeCalls = vi.mocked(writeFileContent).mock.calls; + const writtenPaths = writeCalls.map((call) => call[0]); + expect(writtenPaths.some((p) => p.includes("skills") && p.includes("my-skill"))).toBe(true); }); it("should use explicit ref and path for git transport", async () => { - const { resolveRefToSha, fetchSkillFiles } = await import("./git-client.js"); - vi.mocked(resolveRefToSha).mockResolvedValue("def456abc789"); - vi.mocked(fetchSkillFiles).mockResolvedValue([ - { relativePath: "my-skill/SKILL.md", content: "# Custom Path", size: 50 }, + const { resolveRefToSha: gitResolveRefToSha, fetchSourceCacheFiles } = + await import("./git-client.js"); + vi.mocked(gitResolveRefToSha).mockResolvedValue("def456abc789"); + vi.mocked(fetchSourceCacheFiles).mockResolvedValue([ + { relativePath: "skills/my-skill/SKILL.md", content: "# Custom Path", size: 50 }, ]); await resolveAndFetchSources({ sources: [ - { source: "file:///local/clone", transport: "git", ref: "develop", path: "exports/skills" }, + { + source: "file:///local/clone", + transport: "git", + ref: "develop", + path: "exports/skills", + }, ], baseDir: testDir, }); - expect(vi.mocked(resolveRefToSha)).toHaveBeenCalledWith("file:///local/clone", "develop"); - expect(vi.mocked(fetchSkillFiles)).toHaveBeenCalledWith({ - url: "file:///local/clone", - ref: "develop", - skillsPath: "exports/skills", - }); + expect(vi.mocked(gitResolveRefToSha)).toHaveBeenCalledWith("file:///local/clone", "develop"); + expect(vi.mocked(fetchSourceCacheFiles)).toHaveBeenCalledWith( + expect.objectContaining({ + url: "file:///local/clone", + ref: "develop", + basePath: "exports/skills", + }), + ); }); it("should error in frozen mode when git source lockfile entry lacks requestedRef", async () => { const { readLockFile } = await import("./sources-lock.js"); vi.mocked(readLockFile).mockResolvedValue({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "https://dev.azure.com/org/_git/repo": { resolvedRef: "a".repeat(40), - skills: { "my-skill": { integrity: "sha256-x" } }, + files: { "skills/my-skill/SKILL.md": { integrity: "sha256-x" } }, }, }, }); - // Skill dir missing so SHA-match skip fails + // Source cache dir missing so SHA-match skip fails vi.mocked(directoryExists).mockResolvedValue(false); const result = await resolveAndFetchSources({ @@ -793,28 +876,28 @@ describe("resolveAndFetchSources", () => { options: { frozen: true }, }); - expect(result.fetchedSkillCount).toBe(0); + expect(result.fetchedFileCount).toBe(0); expect(logger.error).toHaveBeenCalledWith(expect.stringContaining("missing requestedRef")); }); - it("should skip re-fetch for git transport when locked SHA matches and skills exist", async () => { + it("should skip re-fetch for git transport when locked SHA matches and cache exists", async () => { const { readLockFile } = await import("./sources-lock.js"); - const { fetchSkillFiles } = await import("./git-client.js"); - const curatedDir = join(testDir, RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH); + const { fetchSourceCacheFiles } = await import("./git-client.js"); vi.mocked(readLockFile).mockResolvedValue({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "https://dev.azure.com/org/_git/repo": { resolvedRef: "b".repeat(40), requestedRef: "main", - skills: { "cached-skill": { integrity: "sha256-cached" } }, + files: { "skills/cached-skill/SKILL.md": { integrity: "sha256-cached" } }, }, }, }); + // Source cache exists on disk - sourceKeyToDirName for the URL vi.mocked(directoryExists).mockImplementation(async (path: string) => { - if (path === join(curatedDir, "cached-skill")) return true; + if (path.includes(".sources")) return true; return false; }); @@ -823,19 +906,19 @@ describe("resolveAndFetchSources", () => { baseDir: testDir, }); - expect(result.fetchedSkillCount).toBe(0); - expect(vi.mocked(fetchSkillFiles)).not.toHaveBeenCalled(); + expect(result.fetchedFileCount).toBe(0); + expect(vi.mocked(fetchSourceCacheFiles)).not.toHaveBeenCalled(); }); it("should apply skill filter for git transport", async () => { - const { resolveDefaultRef, fetchSkillFiles } = await import("./git-client.js"); + const { resolveDefaultRef, fetchSourceCacheFiles } = await import("./git-client.js"); vi.mocked(resolveDefaultRef).mockResolvedValue({ ref: "main", sha: "c".repeat(40) }); - vi.mocked(fetchSkillFiles).mockResolvedValue([ - { relativePath: "skill-a/SKILL.md", content: "A", size: 10 }, - { relativePath: "skill-b/SKILL.md", content: "B", size: 10 }, + vi.mocked(fetchSourceCacheFiles).mockResolvedValue([ + { relativePath: "skills/skill-a/SKILL.md", content: "A", size: 10 }, + { relativePath: "skills/skill-b/SKILL.md", content: "B", size: 10 }, ]); - const result = await resolveAndFetchSources({ + await resolveAndFetchSources({ sources: [ { source: "https://dev.azure.com/org/_git/repo", @@ -846,74 +929,62 @@ describe("resolveAndFetchSources", () => { baseDir: testDir, }); - expect(result.fetchedSkillCount).toBe(1); + // Only skill-a should be written const writeArgs = vi.mocked(writeFileContent).mock.calls.map((call) => call[0]); expect(writeArgs.some((p) => p.includes("skill-a"))).toBe(true); expect(writeArgs.some((p) => p.includes("skill-b"))).toBe(false); }); - it("should skip git transport skill when local skill takes precedence", async () => { - const { resolveDefaultRef, fetchSkillFiles } = await import("./git-client.js"); - vi.mocked(resolveDefaultRef).mockResolvedValue({ ref: "main", sha: "d".repeat(40) }); - vi.mocked(fetchSkillFiles).mockResolvedValue([ - { relativePath: "local-skill/SKILL.md", content: "remote", size: 10 }, - ]); + it("should not write files outside requested features for git transport", async () => { + const { resolveDefaultRef, fetchSourceCacheFiles } = await import("./git-client.js"); + vi.mocked(resolveDefaultRef).mockResolvedValue({ ref: "main", sha: "a".repeat(40) }); - // local-skill exists locally - vi.mocked(directoryExists).mockImplementation(async (path: string) => { - if (path.endsWith("skills")) return true; - return false; - }); - vi.mocked(findFilesByGlobs).mockResolvedValue([join(testDir, ".rulesync/skills/local-skill")]); - - const result = await resolveAndFetchSources({ - sources: [{ source: "https://dev.azure.com/org/_git/repo", transport: "git" }], - baseDir: testDir, - }); - - expect(result.fetchedSkillCount).toBe(0); - expect(writeFileContent).not.toHaveBeenCalled(); - }); - - it("should skip duplicate git transport skill from later source", async () => { - const { resolveDefaultRef, fetchSkillFiles } = await import("./git-client.js"); - vi.mocked(resolveDefaultRef).mockResolvedValue({ ref: "main", sha: "e".repeat(40) }); - vi.mocked(fetchSkillFiles).mockResolvedValue([ - { relativePath: "shared-skill/SKILL.md", content: "content", size: 10 }, + // Simulate sparse checkout returning extra files outside the requested feature + // (e.g. mcp.json sitting alongside subagents/ under the basePath) + vi.mocked(fetchSourceCacheFiles).mockResolvedValue([ + { relativePath: "subagents/reviewer.md", content: "# Reviewer", size: 50 }, + { relativePath: "mcp.json", content: '{"mcpServers":{}}', size: 20 }, ]); - const result = await resolveAndFetchSources({ + await resolveAndFetchSources({ sources: [ - { source: "https://dev.azure.com/org/_git/repo-a", transport: "git" }, - { source: "https://dev.azure.com/org/_git/repo-b", transport: "git" }, + { + source: "https://dev.azure.com/org/_git/repo", + transport: "git", + features: ["subagents"], + path: ".rulesync", + }, ], baseDir: testDir, }); - // First source fetches it, second source skips it - expect(result.fetchedSkillCount).toBe(1); + const writeArgs = vi.mocked(writeFileContent).mock.calls.map((call) => call[0]); + // subagents/reviewer.md should be written + expect(writeArgs.some((p) => p.includes("subagents") && p.includes("reviewer.md"))).toBe(true); + // mcp.json should NOT be written since "mcp" is not in the features list + expect(writeArgs.some((p) => p.endsWith("mcp.json"))).toBe(false); }); - it("should warn on integrity mismatch for git transport skill", async () => { + it("should warn on integrity mismatch for git transport file", async () => { const { readLockFile } = await import("./sources-lock.js"); - const { fetchSkillFiles } = await import("./git-client.js"); + const { fetchSourceCacheFiles } = await import("./git-client.js"); const lockedSha = "f".repeat(40); vi.mocked(readLockFile).mockResolvedValue({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "https://dev.azure.com/org/_git/repo": { resolvedRef: lockedSha, requestedRef: "main", - skills: { "my-skill": { integrity: "sha256-original" } }, + files: { "skills/my-skill/SKILL.md": { integrity: "sha256-original" } }, }, }, }); - // Skill dir missing so re-fetch is triggered + // Source cache dir missing so re-fetch is triggered vi.mocked(directoryExists).mockResolvedValue(false); - vi.mocked(fetchSkillFiles).mockResolvedValue([ - { relativePath: "my-skill/SKILL.md", content: "tampered", size: 10 }, + vi.mocked(fetchSourceCacheFiles).mockResolvedValue([ + { relativePath: "skills/my-skill/SKILL.md", content: "tampered", size: 10 }, ]); await resolveAndFetchSources({ @@ -926,7 +997,7 @@ describe("resolveAndFetchSources", () => { it("should handle GitClientError gracefully and continue processing", async () => { const { GitClientError } = await import("./git-client.js"); - const { resolveDefaultRef, fetchSkillFiles } = await import("./git-client.js"); + const { resolveDefaultRef, fetchSourceCacheFiles } = await import("./git-client.js"); let callCount = 0; vi.mocked(resolveDefaultRef).mockImplementation(async () => { @@ -936,8 +1007,8 @@ describe("resolveAndFetchSources", () => { } return { ref: "main", sha: "a".repeat(40) }; }); - vi.mocked(fetchSkillFiles).mockResolvedValue([ - { relativePath: "good-skill/SKILL.md", content: "ok", size: 10 }, + vi.mocked(fetchSourceCacheFiles).mockResolvedValue([ + { relativePath: "skills/good-skill/SKILL.md", content: "ok", size: 10 }, ]); const result = await resolveAndFetchSources({ @@ -948,32 +1019,107 @@ describe("resolveAndFetchSources", () => { baseDir: testDir, }); - expect(result.fetchedSkillCount).toBe(1); + expect(result.fetchedFileCount).toBeGreaterThanOrEqual(1); expect(result.sourcesProcessed).toBe(2); expect(logger.error).toHaveBeenCalledWith(expect.stringContaining("not installed")); expect(logger.info).toHaveBeenCalledWith(expect.stringContaining("Hint")); }); - it("should drop renamed/deleted skills from lockfile when upstream removes them", async () => { + it("should fetch skills, rules, and mcp.json from a single source into .sources/", async () => { + const { listDirectoryRecursive, withSemaphore } = await import("./github-utils.js"); + vi.mocked(withSemaphore).mockImplementation(async (_sem: any, fn: () => any) => fn()); + const { GitHubClientError } = await import("./github-client.js"); + + // Remote has skills, rules, and mcp.json; other features return 404 + mockClientInstance.listDirectory.mockImplementation( + async (_owner: string, _repo: string, path: string) => { + if (path === "skills") { + return [{ name: "my-skill", path: "skills/my-skill", type: "dir" }]; + } + if (path === "rules") { + return [{ name: "coding.md", path: "rules/coding.md", type: "file" }]; + } + throw new GitHubClientError("Not Found", 404); + }, + ); + vi.mocked(listDirectoryRecursive).mockImplementation(async (params: any) => { + if (params.path === "skills/my-skill") { + return [ + { + name: "SKILL.md", + path: "skills/my-skill/SKILL.md", + type: "file", + size: 50, + sha: "abc", + download_url: null, + }, + ]; + } + if (params.path === "rules") { + return [ + { + name: "coding.md", + path: "rules/coding.md", + type: "file", + size: 30, + sha: "def", + download_url: null, + }, + ]; + } + return []; + }); + mockClientInstance.getFileContent.mockImplementation( + async (_owner: string, _repo: string, path: string) => { + if (path === "skills/my-skill/SKILL.md") return "# My Skill"; + if (path === "rules/coding.md") return "# Coding Rules"; + if (path === ".rulesync/mcp.json" || path === "mcp.json") + return JSON.stringify({ mcpServers: { s1: { command: "cmd" } } }); + throw new GitHubClientError("Not Found", 404); + }, + ); + + const result = await resolveAndFetchSources({ + sources: [{ source: "https://github.com/org/multi-feature" }], + baseDir: testDir, + }); + + expect(result.sourcesProcessed).toBe(1); + expect(result.fetchedFileCount).toBeGreaterThanOrEqual(2); + + const sourceCacheDir = join(testDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH, "org--multi-feature"); + // Verify skill was written + expect(writeFileContent).toHaveBeenCalledWith( + join(sourceCacheDir, "skills", "my-skill", "SKILL.md"), + "# My Skill", + ); + // Verify rule was written + expect(writeFileContent).toHaveBeenCalledWith( + join(sourceCacheDir, "rules", "coding.md"), + "# Coding Rules", + ); + }); + + it("should drop renamed/deleted files from lockfile when upstream removes them", async () => { const { readLockFile, writeLockFile } = await import("./sources-lock.js"); - const { resolveDefaultRef, fetchSkillFiles } = await import("./git-client.js"); + const { resolveDefaultRef, fetchSourceCacheFiles } = await import("./git-client.js"); // Lock has "old-skill" from a previous install vi.mocked(readLockFile).mockResolvedValue({ - lockfileVersion: 1, + lockfileVersion: 2, sources: { "https://dev.azure.com/org/_git/repo": { resolvedRef: "a".repeat(40), requestedRef: "main", - skills: { "old-skill": { integrity: "sha256-old" } }, + files: { "skills/old-skill/SKILL.md": { integrity: "sha256-old" } }, }, }, }); // Remote now has "new-skill" instead of "old-skill" (renamed upstream) vi.mocked(resolveDefaultRef).mockResolvedValue({ ref: "main", sha: "b".repeat(40) }); - vi.mocked(fetchSkillFiles).mockResolvedValue([ - { relativePath: "new-skill/SKILL.md", content: "renamed", size: 10 }, + vi.mocked(fetchSourceCacheFiles).mockResolvedValue([ + { relativePath: "skills/new-skill/SKILL.md", content: "renamed", size: 10 }, ]); vi.mocked(directoryExists).mockResolvedValue(false); @@ -988,7 +1134,7 @@ describe("resolveAndFetchSources", () => { expect(writeCalls).toHaveLength(1); const writtenLock = writeCalls[0]![0].lock; const sourceEntry = Object.values(writtenLock.sources)[0]!; - expect(sourceEntry.skills).toHaveProperty("new-skill"); - expect(sourceEntry.skills).not.toHaveProperty("old-skill"); + expect(sourceEntry.files).toHaveProperty("skills/new-skill/SKILL.md"); + expect(sourceEntry.files).not.toHaveProperty("skills/old-skill/SKILL.md"); }); }); diff --git a/src/lib/sources.ts b/src/lib/sources.ts index bb4334f49..effcc7ff6 100644 --- a/src/lib/sources.ts +++ b/src/lib/sources.ts @@ -1,39 +1,44 @@ -import { join, resolve, sep } from "node:path"; +import { join, posix, resolve, sep } from "node:path"; import { Semaphore } from "es-toolkit/promise"; -import type { SourceEntry } from "../config/config.js"; +import { type SourceEntry, resolveSourceFeatures } from "../config/config.js"; import { FETCH_CONCURRENCY_LIMIT, MAX_FILE_SIZE, - RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH, + RULESYNC_AIIGNORE_FILE_NAME, + RULESYNC_HOOKS_FILE_NAME, + RULESYNC_MCP_FILE_NAME, + RULESYNC_SOURCES_RELATIVE_DIR_PATH, } from "../constants/rulesync-paths.js"; -import { getLocalSkillDirNames } from "../features/skills/skills-utils.js"; +import type { Feature } from "../types/features.js"; import { formatError } from "../utils/error.js"; import { checkPathTraversal, directoryExists, + ensureDir, + listDirectoryFiles, removeDirectory, writeFileContent, } from "../utils/file.js"; import { logger } from "../utils/logger.js"; import { GitClientError, - fetchSkillFiles, + fetchSourceCacheFiles, resolveDefaultRef, resolveRefToSha, validateRef, } from "./git-client.js"; import { GitHubClient, GitHubClientError, logGitHubAuthHints } from "./github-client.js"; import { listDirectoryRecursive, withSemaphore } from "./github-utils.js"; +import { sourceKeyToDirName } from "./source-cache.js"; import { parseSource } from "./source-parser.js"; import { - type LockedSkill, + type LockedFile, type LockedSource, type SourcesLock, - computeSkillIntegrity, + computeFileIntegrity, createEmptyLock, - getLockedSkillNames, getLockedSource, normalizeSourceKey, readLockFile, @@ -41,6 +46,57 @@ import { writeLockFile, } from "./sources-lock.js"; +// --------------------------------------------------------------------------- +// Feature path mapping +// --------------------------------------------------------------------------- + +/** Directory features whose remote content lives under a subdirectory. */ +const DIRECTORY_FEATURES: Feature[] = ["skills", "rules", "commands", "subagents"]; + +/** Single-file features mapped to their file names. Only non-directory features. */ +const FILE_FEATURE_NAMES: Partial> = { + mcp: RULESYNC_MCP_FILE_NAME, + hooks: RULESYNC_HOOKS_FILE_NAME, + ignore: RULESYNC_AIIGNORE_FILE_NAME, +}; + +/** Get the file name for a single-file feature. */ +function getFileFeatureName(feature: Feature): string { + const name = FILE_FEATURE_NAMES[feature]; + if (!name) { + throw new Error(`Feature "${feature}" is not a single-file feature.`); + } + return name; +} + +/** + * Check if a relative path belongs to any of the requested features. + * Used to filter out files that sparse checkout may include from sibling + * directories when the basePath is a parent of multiple feature dirs. + */ +function isPathInFeatures(relativePath: string, features: Feature[]): boolean { + for (const feature of features) { + if (DIRECTORY_FEATURES.includes(feature)) { + if (relativePath.startsWith(feature + "/")) return true; + } else { + if (relativePath === FILE_FEATURE_NAMES[feature]) return true; + } + } + return false; +} + +/** Map a feature to its remote path(s) relative to the source root. */ +function featureToRemotePath(feature: Feature): string { + if (DIRECTORY_FEATURES.includes(feature)) { + return feature; + } + return getFileFeatureName(feature); +} + +// --------------------------------------------------------------------------- +// Public API +// --------------------------------------------------------------------------- + export type ResolveAndFetchSourcesOptions = { /** Force re-resolve all refs, ignoring the lockfile. */ updateSources?: boolean; @@ -53,12 +109,12 @@ export type ResolveAndFetchSourcesOptions = { }; export type ResolveAndFetchSourcesResult = { - fetchedSkillCount: number; + fetchedFileCount: number; sourcesProcessed: number; }; /** - * Resolve declared sources, fetch remote skills into .rulesync/skills/.curated/, + * Resolve declared sources, fetch remote content into .rulesync/.sources/, * and update the lockfile. */ export async function resolveAndFetchSources(params: { @@ -69,12 +125,12 @@ export async function resolveAndFetchSources(params: { const { sources, baseDir, options = {} } = params; if (sources.length === 0) { - return { fetchedSkillCount: 0, sourcesProcessed: 0 }; + return { fetchedFileCount: 0, sourcesProcessed: 0 }; } if (options.skipSources) { logger.info("Skipping source fetching."); - return { fetchedSkillCount: 0, sourcesProcessed: 0 }; + return { fetchedFileCount: 0, sourcesProcessed: 0 }; } // Read existing lockfile @@ -82,14 +138,11 @@ export async function resolveAndFetchSources(params: { ? createEmptyLock() : await readLockFile({ baseDir }); - // Frozen mode: validate lockfile covers all declared sources. - // Missing curated skills are fetched using locked refs. + // Frozen mode: validate lockfile covers all declared sources if (options.frozen) { const missingKeys: string[] = []; - for (const source of sources) { - const locked = getLockedSource(lock, source.source); - if (!locked) { + if (!getLockedSource(lock, source.source)) { missingKeys.push(source.source); } } @@ -106,44 +159,32 @@ export async function resolveAndFetchSources(params: { const token = GitHubClient.resolveToken(options.token); const client = new GitHubClient({ token }); - // Determine local skills (in .rulesync/skills/ but not in .curated/) - const localSkillNames = await getLocalSkillDirNames(baseDir); - - let totalSkillCount = 0; - const allFetchedSkillNames = new Set(); + let totalFileCount = 0; for (const sourceEntry of sources) { try { const transport = sourceEntry.transport ?? "github"; - let result: { skillCount: number; fetchedSkillNames: string[]; updatedLock: SourcesLock }; + let result: { fileCount: number; updatedLock: SourcesLock }; if (transport === "git") { result = await fetchSourceViaGit({ sourceEntry, baseDir, lock, - localSkillNames, - alreadyFetchedSkillNames: allFetchedSkillNames, updateSources: options.updateSources ?? false, frozen: options.frozen ?? false, }); } else { - result = await fetchSource({ + result = await fetchSourceViaGitHub({ sourceEntry, client, baseDir, lock, - localSkillNames, - alreadyFetchedSkillNames: allFetchedSkillNames, updateSources: options.updateSources ?? false, }); } - const { skillCount, fetchedSkillNames, updatedLock } = result; - lock = updatedLock; - totalSkillCount += skillCount; - for (const name of fetchedSkillNames) { - allFetchedSkillNames.add(name); - } + lock = result.updatedLock; + totalFileCount += result.fileCount; } catch (error) { logger.error(`Failed to fetch source "${sourceEntry.source}": ${formatError(error)}`); if (error instanceof GitHubClientError) { @@ -154,7 +195,7 @@ export async function resolveAndFetchSources(params: { } } - // Prune stale lockfile entries whose keys are not in the current sources (immutable) + // Prune stale lockfile entries const sourceKeys = new Set(sources.map((s) => normalizeSourceKey(s.source))); const prunedSources: typeof lock.sources = {}; for (const [key, value] of Object.entries(lock.sources)) { @@ -166,6 +207,19 @@ export async function resolveAndFetchSources(params: { } lock = { lockfileVersion: lock.lockfileVersion, sources: prunedSources }; + // Remove orphaned source cache directories + const sourcesDir = join(baseDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH); + if (await directoryExists(sourcesDir)) { + const expectedDirNames = new Set(sources.map((s) => sourceKeyToDirName(s.source))); + const existingDirs = await listDirectoryFiles(sourcesDir); + for (const dirName of existingDirs) { + if (!expectedDirNames.has(dirName)) { + logger.debug(`Removing orphaned source cache directory: ${dirName}`); + await removeDirectory(join(sourcesDir, dirName)); + } + } + } + // Only write lockfile if it has changed (and not in frozen mode) if (!options.frozen && JSON.stringify(lock) !== originalLockJson) { await writeLockFile({ baseDir, lock }); @@ -173,12 +227,13 @@ export async function resolveAndFetchSources(params: { logger.debug("Lockfile unchanged, skipping write."); } - return { fetchedSkillCount: totalSkillCount, sourcesProcessed: sources.length }; + return { fetchedFileCount: totalFileCount, sourcesProcessed: sources.length }; } -/** - * Log contextual hints for GitClientError to help users troubleshoot. - */ +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + function logGitClientHints(error: GitClientError): void { if (error.message.includes("not installed")) { logger.info("Hint: Install git and ensure it is available on your PATH."); @@ -188,193 +243,121 @@ function logGitClientHints(error: GitClientError): void { } /** - * Check if all locked skills exist on disk in the curated directory. + * Check if a source cache directory exists on disk. */ -async function checkLockedSkillsExist(curatedDir: string, skillNames: string[]): Promise { - if (skillNames.length === 0) return true; - for (const name of skillNames) { - if (!(await directoryExists(join(curatedDir, name)))) { - return false; - } - } - return true; +async function sourceCacheExists(baseDir: string, sourceKey: string): Promise { + const cachePath = join( + baseDir, + RULESYNC_SOURCES_RELATIVE_DIR_PATH, + sourceKeyToDirName(sourceKey), + ); + return directoryExists(cachePath); } -// --------------------------------------------------------------------------- -// Shared helpers for fetchSource and fetchSourceViaGit -// --------------------------------------------------------------------------- - /** - * Remove previously curated skill directories for a source before re-fetching. - * Validates that each path resolves within the curated directory to prevent traversal. + * Clean and recreate a source cache directory. */ -async function cleanPreviousCuratedSkills( - curatedDir: string, - lockedSkillNames: string[], -): Promise { - const resolvedCuratedDir = resolve(curatedDir); - for (const prevSkill of lockedSkillNames) { - const prevDir = join(curatedDir, prevSkill); - if (!resolve(prevDir).startsWith(resolvedCuratedDir + sep)) { - logger.warn( - `Skipping removal of "${prevSkill}": resolved path is outside the curated directory.`, - ); - continue; - } - if (await directoryExists(prevDir)) { - await removeDirectory(prevDir); - } +async function cleanSourceCache(params: { baseDir: string; sourceKey: string }): Promise { + const cachePath = join( + params.baseDir, + RULESYNC_SOURCES_RELATIVE_DIR_PATH, + sourceKeyToDirName(params.sourceKey), + ); + const resolvedCachePath = resolve(cachePath); + const resolvedSourcesDir = resolve(join(params.baseDir, RULESYNC_SOURCES_RELATIVE_DIR_PATH)); + if (!resolvedCachePath.startsWith(resolvedSourcesDir + sep)) { + throw new Error(`Source cache path "${cachePath}" escapes the sources directory.`); + } + if (await directoryExists(cachePath)) { + await removeDirectory(cachePath); } + await ensureDir(cachePath); + return cachePath; } /** - * Check whether a skill should be skipped during fetching. - * Returns true (with appropriate logging) if the skill should be skipped. + * Finalize a source fetch: update lockfile, clean empty cache, log result. */ -function shouldSkipSkill(params: { - skillName: string; +async function finalizeFetch(params: { + cachePath: string; + files: Record; + lock: SourcesLock; sourceKey: string; - localSkillNames: Set; - alreadyFetchedSkillNames: Set; -}): boolean { - const { skillName, sourceKey, localSkillNames, alreadyFetchedSkillNames } = params; - if (skillName.includes("..") || skillName.includes("/") || skillName.includes("\\")) { - logger.warn( - `Skipping skill with invalid name "${skillName}" from ${sourceKey}: contains path traversal characters.`, - ); - return true; - } - if (localSkillNames.has(skillName)) { - logger.debug( - `Skipping remote skill "${skillName}" from ${sourceKey}: local skill takes precedence.`, - ); - return true; - } - if (alreadyFetchedSkillNames.has(skillName)) { - logger.warn( - `Skipping duplicate skill "${skillName}" from ${sourceKey}: already fetched from another source.`, - ); - return true; + requestedRef: string | undefined; + resolvedSha: string; +}): Promise<{ fileCount: number; updatedLock: SourcesLock }> { + const { cachePath, files, lock, sourceKey, requestedRef, resolvedSha } = params; + const fileCount = Object.keys(files).length; + + if (fileCount === 0) { + await removeDirectory(cachePath); } - return false; + + const updatedLock = setLockedSource(lock, sourceKey, { + requestedRef, + resolvedRef: resolvedSha, + resolvedAt: new Date().toISOString(), + files, + }); + + logger.info(`Fetched ${fileCount} file(s) from ${sourceKey}.`); + return { fileCount, updatedLock }; } /** - * Write skill files to disk, compute integrity, and check against the lockfile. - * Returns the computed LockedSkill entry. + * Write a file to the source cache and compute its integrity. */ -async function writeSkillAndComputeIntegrity(params: { - skillName: string; - files: Array<{ relativePath: string; content: string }>; - curatedDir: string; +async function writeAndTrackFile(params: { + cachePath: string; + relativePath: string; + content: string; locked: LockedSource | undefined; resolvedSha: string; sourceKey: string; -}): Promise { - const { skillName, files, curatedDir, locked, resolvedSha, sourceKey } = params; - const written: Array<{ path: string; content: string }> = []; +}): Promise<{ relativePath: string; integrity: string }> { + const { cachePath, relativePath, content, locked, resolvedSha, sourceKey } = params; - for (const file of files) { - checkPathTraversal({ - relativePath: file.relativePath, - intendedRootDir: join(curatedDir, skillName), - }); - await writeFileContent(join(curatedDir, skillName, file.relativePath), file.content); - written.push({ path: file.relativePath, content: file.content }); - } + checkPathTraversal({ relativePath, intendedRootDir: cachePath }); + await writeFileContent(join(cachePath, relativePath), content); - const integrity = computeSkillIntegrity(written); - const lockedSkillEntry = locked?.skills[skillName]; + const integrity = computeFileIntegrity(content); + const lockedEntry = locked?.files[relativePath]; if ( - lockedSkillEntry?.integrity && - lockedSkillEntry.integrity !== integrity && + lockedEntry?.integrity && + lockedEntry.integrity !== integrity && resolvedSha === locked?.resolvedRef ) { logger.warn( - `Integrity mismatch for skill "${skillName}" from ${sourceKey}: expected "${lockedSkillEntry.integrity}", got "${integrity}". Content may have been tampered with.`, + `Integrity mismatch for "${relativePath}" from ${sourceKey}: expected "${lockedEntry.integrity}", got "${integrity}".`, ); } - return { integrity }; -} - -/** - * Merge newly fetched skills with existing locked skills and update the lockfile. - */ -function buildLockUpdate(params: { - lock: SourcesLock; - sourceKey: string; - fetchedSkills: Record; - locked: LockedSource | undefined; - requestedRef: string | undefined; - resolvedSha: string; - remoteSkillNames: string[]; -}): { updatedLock: SourcesLock; fetchedNames: string[] } { - const { lock, sourceKey, fetchedSkills, locked, requestedRef, resolvedSha, remoteSkillNames } = - params; - const fetchedNames = Object.keys(fetchedSkills); - - // Merge back locked skills that still exist in the remote but were skipped - // (due to local precedence, already-fetched, etc.). Skills no longer present - // in the remote (e.g. renamed or deleted upstream) are intentionally dropped. - const remoteSet = new Set(remoteSkillNames); - const mergedSkills: Record = { ...fetchedSkills }; - if (locked) { - for (const [skillName, skillEntry] of Object.entries(locked.skills)) { - if (!(skillName in mergedSkills) && remoteSet.has(skillName)) { - mergedSkills[skillName] = skillEntry; - } - } - } - - const updatedLock = setLockedSource(lock, sourceKey, { - requestedRef, - resolvedRef: resolvedSha, - resolvedAt: new Date().toISOString(), - skills: mergedSkills, - }); - - logger.info( - `Fetched ${fetchedNames.length} skill(s) from ${sourceKey}: ${fetchedNames.join(", ") || "(none)"}`, - ); - - return { updatedLock, fetchedNames }; + return { relativePath, integrity }; } // --------------------------------------------------------------------------- -// Transport-specific fetch functions +// GitHub API transport // --------------------------------------------------------------------------- -/** - * Fetch skills from a single source entry via the GitHub REST API. - */ -async function fetchSource(params: { +async function fetchSourceViaGitHub(params: { sourceEntry: SourceEntry; client: GitHubClient; baseDir: string; lock: SourcesLock; - localSkillNames: Set; - alreadyFetchedSkillNames: Set; updateSources: boolean; -}): Promise<{ - skillCount: number; - fetchedSkillNames: string[]; - updatedLock: SourcesLock; -}> { - const { sourceEntry, client, baseDir, localSkillNames, alreadyFetchedSkillNames, updateSources } = - params; +}): Promise<{ fileCount: number; updatedLock: SourcesLock }> { + const { sourceEntry, client, baseDir, updateSources } = params; const { lock } = params; const parsed = parseSource(sourceEntry.source); - if (parsed.provider === "gitlab") { logger.warn(`GitLab sources are not yet supported. Skipping "${sourceEntry.source}".`); - return { skillCount: 0, fetchedSkillNames: [], updatedLock: lock }; + return { fileCount: 0, updatedLock: lock }; } const sourceKey = sourceEntry.source; const locked = getLockedSource(lock, sourceKey); - const lockedSkillNames = locked ? getLockedSkillNames(locked) : []; + const features = resolveSourceFeatures(sourceEntry); // Resolve the ref to a commit SHA let ref: string; @@ -382,165 +365,177 @@ async function fetchSource(params: { let requestedRef: string | undefined; if (locked && !updateSources) { - // Use the locked SHA for deterministic fetching ref = locked.resolvedRef; resolvedSha = locked.resolvedRef; requestedRef = locked.requestedRef; logger.debug(`Using locked ref for ${sourceKey}: ${resolvedSha}`); } else { - // Resolve the ref (or default branch) to a SHA requestedRef = parsed.ref ?? (await client.getDefaultBranch(parsed.owner, parsed.repo)); resolvedSha = await client.resolveRefToSha(parsed.owner, parsed.repo, requestedRef); ref = resolvedSha; logger.debug(`Resolved ${sourceKey} ref "${requestedRef}" to SHA: ${resolvedSha}`); } - const curatedDir = join(baseDir, RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH); - - // Skip re-fetch if SHA matches lockfile and curated skills exist on disk + // Skip re-fetch if SHA matches and cache exists if (locked && resolvedSha === locked.resolvedRef && !updateSources) { - const allExist = await checkLockedSkillsExist(curatedDir, lockedSkillNames); - if (allExist) { + if (await sourceCacheExists(baseDir, sourceKey)) { logger.debug(`SHA unchanged for ${sourceKey}, skipping re-fetch.`); - return { - skillCount: 0, - fetchedSkillNames: lockedSkillNames, - updatedLock: lock, - }; + return { fileCount: 0, updatedLock: lock }; } } - // Determine which skills to fetch - const skillFilter = sourceEntry.skills ?? ["*"]; - const isWildcard = skillFilter.length === 1 && skillFilter[0] === "*"; - - // List the skills/ directory in the remote repo. - // If a path is given in the source URL, it points directly to the skills directory. - // Otherwise, look for "skills/" at the repo root. - const skillsBasePath = parsed.path ?? "skills"; - let remoteSkillDirs: Array<{ name: string; path: string }>; - - try { - const entries = await client.listDirectory(parsed.owner, parsed.repo, skillsBasePath, ref); - remoteSkillDirs = entries - .filter((e) => e.type === "dir") - .map((e) => ({ name: e.name, path: e.path })); - } catch (error) { - if (error instanceof GitHubClientError && error.statusCode === 404) { - logger.warn(`No skills/ directory found in ${sourceKey}. Skipping.`); - return { skillCount: 0, fetchedSkillNames: [], updatedLock: lock }; - } - throw error; - } - - // Filter skills by name - const filteredDirs = isWildcard - ? remoteSkillDirs - : remoteSkillDirs.filter((d) => skillFilter.includes(d.name)); - + // Clean and prepare cache directory + const cachePath = await cleanSourceCache({ baseDir, sourceKey }); const semaphore = new Semaphore(FETCH_CONCURRENCY_LIMIT); - const fetchedSkills: Record = {}; - - if (locked) { - await cleanPreviousCuratedSkills(curatedDir, lockedSkillNames); - } - - for (const skillDir of filteredDirs) { - if ( - shouldSkipSkill({ - skillName: skillDir.name, - sourceKey, - localSkillNames, - alreadyFetchedSkillNames, - }) - ) { - continue; - } - - // Recursively fetch all files in this skill directory - const allFiles = await listDirectoryRecursive({ - client, - owner: parsed.owner, - repo: parsed.repo, - path: skillDir.path, - ref, - semaphore, - }); - - // Filter out files exceeding MAX_FILE_SIZE - const files = allFiles.filter((file) => { - if (file.size > MAX_FILE_SIZE) { - logger.warn( - `Skipping file "${file.path}" (${(file.size / 1024 / 1024).toFixed(2)}MB exceeds ${MAX_FILE_SIZE / 1024 / 1024}MB limit).`, + const files: Record = {}; + const basePath = parsed.path; + const skillFilter = sourceEntry.skills ?? ["*"]; + const isSkillWildcard = skillFilter.length === 1 && skillFilter[0] === "*"; + + for (const feature of features) { + // Use posix.join for remote repo paths (GitHub API always uses forward slashes) + const remotePath = basePath + ? posix.join(basePath, featureToRemotePath(feature)) + : featureToRemotePath(feature); + + if (DIRECTORY_FEATURES.includes(feature)) { + // Directory feature: recursively list and fetch files + try { + if (feature === "skills") { + // Skills: list top-level to find skill subdirectories, then recurse each + const entries = await client.listDirectory(parsed.owner, parsed.repo, remotePath, ref); + const skillDirs = entries + .filter((e) => e.type === "dir") + .filter((d) => isSkillWildcard || skillFilter.includes(d.name)); + + for (const skillDir of skillDirs) { + const allFiles = await listDirectoryRecursive({ + client, + owner: parsed.owner, + repo: parsed.repo, + path: skillDir.path, + ref, + semaphore, + }); + + for (const file of allFiles) { + if (file.size > MAX_FILE_SIZE) { + logger.warn( + `Skipping file "${file.path}" (${(file.size / 1024 / 1024).toFixed(2)}MB exceeds limit).`, + ); + continue; + } + const content = await withSemaphore(semaphore, () => + client.getFileContent(parsed.owner, parsed.repo, file.path, ref), + ); + const relPath = posix.join( + "skills", + skillDir.name, + file.path.substring(skillDir.path.length + 1), + ); + const result = await writeAndTrackFile({ + cachePath, + relativePath: relPath, + content, + locked, + resolvedSha, + sourceKey, + }); + files[result.relativePath] = { integrity: result.integrity }; + } + } + } else { + // rules/commands/subagents: recursively list all files + const allFiles = await listDirectoryRecursive({ + client, + owner: parsed.owner, + repo: parsed.repo, + path: remotePath, + ref, + semaphore, + }); + + for (const file of allFiles) { + if (file.size > MAX_FILE_SIZE) { + logger.warn( + `Skipping file "${file.path}" (${(file.size / 1024 / 1024).toFixed(2)}MB exceeds limit).`, + ); + continue; + } + const content = await withSemaphore(semaphore, () => + client.getFileContent(parsed.owner, parsed.repo, file.path, ref), + ); + const relPath = posix.join(feature, file.path.substring(remotePath.length + 1)); + const result = await writeAndTrackFile({ + cachePath, + relativePath: relPath, + content, + locked, + resolvedSha, + sourceKey, + }); + files[result.relativePath] = { integrity: result.integrity }; + } + } + } catch (error) { + if (error instanceof GitHubClientError && error.statusCode === 404) { + logger.debug(`No ${feature}/ directory found in ${sourceKey}.`); + continue; + } + throw error; + } + } else { + // Single-file feature (mcp.json, hooks.json, .aiignore) + try { + const content = await withSemaphore(semaphore, () => + client.getFileContent(parsed.owner, parsed.repo, remotePath, ref), ); - return false; + const fileName = getFileFeatureName(feature); + const result = await writeAndTrackFile({ + cachePath, + relativePath: fileName, + content, + locked, + resolvedSha, + sourceKey, + }); + files[result.relativePath] = { integrity: result.integrity }; + } catch (error) { + if (error instanceof GitHubClientError && error.statusCode === 404) { + logger.debug(`No ${featureToRemotePath(feature)} found in ${sourceKey}.`); + continue; + } + throw error; } - return true; - }); - - // Fetch all file contents - const skillFiles: Array<{ relativePath: string; content: string }> = []; - for (const file of files) { - const relativeToSkill = file.path.substring(skillDir.path.length + 1); - const content = await withSemaphore(semaphore, () => - client.getFileContent(parsed.owner, parsed.repo, file.path, ref), - ); - skillFiles.push({ relativePath: relativeToSkill, content }); } - - fetchedSkills[skillDir.name] = await writeSkillAndComputeIntegrity({ - skillName: skillDir.name, - files: skillFiles, - curatedDir, - locked, - resolvedSha, - sourceKey, - }); - logger.debug(`Fetched skill "${skillDir.name}" from ${sourceKey}`); } - const result = buildLockUpdate({ - lock, - sourceKey, - fetchedSkills, - locked, - requestedRef, - resolvedSha, - remoteSkillNames: filteredDirs.map((d) => d.name), - }); - - return { - skillCount: result.fetchedNames.length, - fetchedSkillNames: result.fetchedNames, - updatedLock: result.updatedLock, - }; + return finalizeFetch({ cachePath, files, lock, sourceKey, requestedRef, resolvedSha }); } -/** - * Fetch skills from a single source using git CLI (works with any git remote). - */ +// --------------------------------------------------------------------------- +// Git CLI transport +// --------------------------------------------------------------------------- + async function fetchSourceViaGit(params: { sourceEntry: SourceEntry; baseDir: string; lock: SourcesLock; - localSkillNames: Set; - alreadyFetchedSkillNames: Set; updateSources: boolean; frozen: boolean; -}): Promise<{ skillCount: number; fetchedSkillNames: string[]; updatedLock: SourcesLock }> { - const { sourceEntry, baseDir, localSkillNames, alreadyFetchedSkillNames, updateSources, frozen } = - params; +}): Promise<{ fileCount: number; updatedLock: SourcesLock }> { + const { sourceEntry, baseDir, updateSources, frozen } = params; const { lock } = params; const url = sourceEntry.source; const locked = getLockedSource(lock, url); - const lockedSkillNames = locked ? getLockedSkillNames(locked) : []; + const features = resolveSourceFeatures(sourceEntry); + // Resolve ref let resolvedSha: string; let requestedRef: string | undefined; if (locked && !updateSources) { resolvedSha = locked.resolvedRef; requestedRef = locked.requestedRef; - // Validate locked ref before passing to git commands if (requestedRef) { validateRef(requestedRef); } @@ -553,14 +548,14 @@ async function fetchSourceViaGit(params: { resolvedSha = def.sha; } - const curatedDir = join(baseDir, RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH); + // Skip re-fetch if SHA matches and cache exists if (locked && resolvedSha === locked.resolvedRef && !updateSources) { - if (await checkLockedSkillsExist(curatedDir, lockedSkillNames)) { - return { skillCount: 0, fetchedSkillNames: lockedSkillNames, updatedLock: lock }; + if (await sourceCacheExists(baseDir, url)) { + return { fileCount: 0, updatedLock: lock }; } } - // Resolve requestedRef lazily (deferred from locked path to avoid unnecessary network calls) + // Resolve requestedRef lazily if needed if (!requestedRef) { if (frozen) { throw new Error( @@ -572,61 +567,56 @@ async function fetchSourceViaGit(params: { resolvedSha = def.sha; } - const skillFilter = sourceEntry.skills ?? ["*"]; - const isWildcard = skillFilter.length === 1 && skillFilter[0] === "*"; - const remoteFiles = await fetchSkillFiles({ + // Build paths to fetch via sparse checkout + const remotePaths = features.map((f) => featureToRemotePath(f)); + const remoteFiles = await fetchSourceCacheFiles({ url, ref: requestedRef, - skillsPath: sourceEntry.path ?? "skills", + paths: remotePaths, + basePath: sourceEntry.path, }); - // Group files by skill directory (first path component) - const skillFileMap = new Map>(); - for (const file of remoteFiles) { - const idx = file.relativePath.indexOf("/"); - if (idx === -1) continue; - const name = file.relativePath.substring(0, idx); - const inner = file.relativePath.substring(idx + 1); - const arr = skillFileMap.get(name) ?? []; - arr.push({ relativePath: inner, content: file.content }); - skillFileMap.set(name, arr); - } + // Clean and prepare cache directory + const cachePath = await cleanSourceCache({ baseDir, sourceKey: url }); + const files: Record = {}; - const allNames = [...skillFileMap.keys()]; - const filteredNames = isWildcard ? allNames : allNames.filter((n) => skillFilter.includes(n)); - - if (locked) { - await cleanPreviousCuratedSkills(curatedDir, lockedSkillNames); - } + // Apply skill filter + const skillFilter = sourceEntry.skills ?? ["*"]; + const isSkillWildcard = skillFilter.length === 1 && skillFilter[0] === "*"; - const fetchedSkills: Record = {}; - for (const skillName of filteredNames) { - if (shouldSkipSkill({ skillName, sourceKey: url, localSkillNames, alreadyFetchedSkillNames })) { + for (const file of remoteFiles) { + // Filter out files that don't belong to the requested features. + // Sparse checkout from a basePath may include sibling files. + if (!isPathInFeatures(file.relativePath, features)) { continue; } - fetchedSkills[skillName] = await writeSkillAndComputeIntegrity({ - skillName, - files: skillFileMap.get(skillName) ?? [], - curatedDir, + // Apply skill filter for skills + if (file.relativePath.startsWith("skills/")) { + const parts = file.relativePath.split("/"); + const skillName = parts[1]; + if (skillName && !isSkillWildcard && !skillFilter.includes(skillName)) { + continue; + } + } + + const result = await writeAndTrackFile({ + cachePath, + relativePath: file.relativePath, + content: file.content, locked, resolvedSha, sourceKey: url, }); + files[result.relativePath] = { integrity: result.integrity }; } - const result = buildLockUpdate({ + return finalizeFetch({ + cachePath, + files, lock, sourceKey: url, - fetchedSkills, - locked, requestedRef, resolvedSha, - remoteSkillNames: filteredNames, }); - return { - skillCount: result.fetchedNames.length, - fetchedSkillNames: result.fetchedNames, - updatedLock: result.updatedLock, - }; } diff --git a/src/utils/error.test.ts b/src/utils/error.test.ts index 566f806a7..7aed9860f 100644 --- a/src/utils/error.test.ts +++ b/src/utils/error.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { z } from "zod/mini"; -import { formatError } from "./error.js"; +import { formatError, isFileNotFoundError } from "./error.js"; describe("formatError", () => { it("should format a single validation error without path", () => { @@ -139,3 +139,25 @@ describe("formatError", () => { expect(formatted).toBe("[object Object]"); }); }); + +describe("isFileNotFoundError", () => { + it("should return true for ENOENT errors", () => { + const error = Object.assign(new Error("no such file"), { code: "ENOENT" }); + expect(isFileNotFoundError(error)).toBe(true); + }); + + it("should return false for other error codes", () => { + const error = Object.assign(new Error("permission denied"), { code: "EACCES" }); + expect(isFileNotFoundError(error)).toBe(false); + }); + + it("should return false for plain errors without code", () => { + expect(isFileNotFoundError(new Error("generic"))).toBe(false); + }); + + it("should return false for non-Error values", () => { + expect(isFileNotFoundError("ENOENT")).toBe(false); + expect(isFileNotFoundError(null)).toBe(false); + expect(isFileNotFoundError(undefined)).toBe(false); + }); +}); diff --git a/src/utils/error.ts b/src/utils/error.ts index 7594f8edd..7f91a4844 100644 --- a/src/utils/error.ts +++ b/src/utils/error.ts @@ -42,6 +42,13 @@ function isZodErrorLike(error: unknown): error is { ); } +/** + * Check if an error is a file-not-found (ENOENT) error from the filesystem. + */ +export function isFileNotFoundError(error: unknown): boolean { + return error instanceof Error && "code" in error && error.code === "ENOENT"; +} + export function formatError(error: unknown): string { // Check for ZodError by duck typing (handles both zod and zod/mini) if (error instanceof ZodError || isZodErrorLike(error)) {