From 1dcdc380fde76c0da9053a465237e65b4a0cb05f Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Wed, 7 Oct 2026 01:53:34 +1100 Subject: [PATCH] fix(registry): store the canonical copy of each Skill Agent folders and plugin mirrors copy one Skill under one name. The sync wrote every copy over the one row, so the stored path was whichever copy it wrote last, and the next push moved it again. modern-web-design was admitted from a hidden .claude/ folder this way. The sync now reads only the copy canonicalSkillFolder picks, the rule the resolver uses for skilld run by name. Continuation offsets still count every SKILL.md, so an in-flight checkpoint keeps its meaning. --- .../server/utils/skill-frontmatter.ts | 9 ++++++ layers/registry/server/utils/sync-repo.ts | 15 ++++++++-- scripts/deep-sync-clone.ts | 8 +++-- scripts/sync-skills-gh.ts | 8 +++-- shared/skill-path.ts | 23 ++++++++++++++ test/unit/sync-repo-correctness.test.ts | 30 +++++++++++++++++++ 6 files changed, 84 insertions(+), 9 deletions(-) diff --git a/layers/registry/server/utils/skill-frontmatter.ts b/layers/registry/server/utils/skill-frontmatter.ts index 81d9571f..8d4f7013 100644 --- a/layers/registry/server/utils/skill-frontmatter.ts +++ b/layers/registry/server/utils/skill-frontmatter.ts @@ -155,6 +155,15 @@ export function slugifySkillName(s: string): string { .replace(SLUGIFY_TRIM_DASH_RE, '') } +/** + * The registry name of the Skill in a folder, with `.` for the Repository + * root. A root Skill takes the Repository name. {@link parseSkillFile} names a + * Skill the same way. + */ +export function registrySkillName(folder: string, repository: string): string { + return slugifySkillName(folder === '.' ? repository : folder.slice(folder.lastIndexOf('/') + 1)) +} + export interface ParsedSkill { name: string displayName: string diff --git a/layers/registry/server/utils/sync-repo.ts b/layers/registry/server/utils/sync-repo.ts index f7844afa..0833df80 100644 --- a/layers/registry/server/utils/sync-repo.ts +++ b/layers/registry/server/utils/sync-repo.ts @@ -2,13 +2,13 @@ import type { GithubBindings, RepoMeta } from './github-client' import type { SkillTrustTier } from './skill-trust' -import { isRegistrySkillPath, isSkilldCacheSkill } from '#shared/skill-path' +import { canonicalSkillPaths, isRegistrySkillPath, isSkilldCacheSkill } from '#shared/skill-path' import { isCategoryPinned } from '../data/clusters' import { getBlobsBatch, getCommitsBatch, getRepoSummary, getTree, logRateLimit } from './github-client' import { repoStarObservationStatements } from './repo-history' import { resolveRepoSourceIdentityFromRow } from './repo-source-identity' import { skillContentSha256 } from './skill-content-hash' -import { parseSkillFile } from './skill-frontmatter' +import { parseSkillFile, registrySkillName } from './skill-frontmatter' import { isOfficialSkillRepo, scoreSkillIndexability } from './skill-indexability' import { parseSkillMd } from './skill-md-render' import { resolveSkillTrust } from './skill-trust' @@ -879,6 +879,13 @@ export async function syncRepo( }) } stats.skillsSeen = skillFiles.length + // Agent folders and plugin mirrors copy one Skill under one name. Writing + // every copy over the one row stored whichever was written last, and the + // next push moved it again. Only the copy `skilld run` resolves is read. + const canonicalPaths = new Set(canonicalSkillPaths( + skillFiles.map(file => file.path), + folder => registrySkillName(folder, repo), + )) const now = checkedAt const stars = meta.stargazers_count ?? 0 @@ -976,7 +983,9 @@ export async function syncRepo( ? skillFiles.length : Math.max(1, Math.floor(opts.maxSkillFiles)) const chunkEnd = Math.min(skillFiles.length, chunkStart + chunkSize) - const chunkFiles = skillFiles.slice(chunkStart, chunkEnd) + // Offsets count every SKILL.md in tree order, copies included, so a + // checkpoint keeps its meaning whichever copies a chunk then skips. + const chunkFiles = skillFiles.slice(chunkStart, chunkEnd).filter(file => canonicalPaths.has(file.path)) const isFinalChunk = chunkEnd >= skillFiles.length for (const slice of slices(chunkFiles, SKILL_SLICE_SIZE)) { diff --git a/scripts/deep-sync-clone.ts b/scripts/deep-sync-clone.ts index 4405f81a..e8aca960 100644 --- a/scripts/deep-sync-clone.ts +++ b/scripts/deep-sync-clone.ts @@ -23,10 +23,10 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import process from 'node:process' import { promisify } from 'node:util' -import { parseSkillFile } from '#layers/registry/server/utils/skill-frontmatter' +import { parseSkillFile, registrySkillName } from '#layers/registry/server/utils/skill-frontmatter' import { isOfficialSkillRepo, scoreSkillIndexability } from '#layers/registry/server/utils/skill-indexability' import { resolveSkillTrust } from '#layers/registry/server/utils/skill-trust' -import { isRegistrySkillPath, isSkilldCacheSkill } from '#shared/skill-path' +import { canonicalSkillPaths, isRegistrySkillPath, isSkilldCacheSkill } from '#shared/skill-path' const execFileP = promisify(execFile) async function runP(cmd: string, args: string[], opts: { timeout?: number, maxBuffer?: number, cwd?: string } = {}): Promise<{ stdout: string, stderr: string, code: number }> { @@ -311,7 +311,9 @@ async function syncRepo(target: TopRepo, meta: RepoMetaNode | null, kindOv: Map< const seenNames: string[] = [] - for (const skillPath of skillPaths) { + // One copy per Skill name, the one the production sync and `skilld run` pick. + const canonicalPaths = canonicalSkillPaths(skillPaths, folder => registrySkillName(folder, repo)) + for (const skillPath of canonicalPaths) { const skillDir = skillPath.includes('/') ? skillPath.slice(0, skillPath.lastIndexOf('/')) : '' const dirName = skillDir.split('/').pop() ?? repo const raw = await gitReadFile(dir, skillPath) diff --git a/scripts/sync-skills-gh.ts b/scripts/sync-skills-gh.ts index adc527df..6a0437fb 100644 --- a/scripts/sync-skills-gh.ts +++ b/scripts/sync-skills-gh.ts @@ -18,8 +18,8 @@ import { execFileSync } from 'node:child_process' import process from 'node:process' -import { parseSkillFile } from '../layers/registry/server/utils/skill-frontmatter' -import { isRegistrySkillPath, isSkilldCacheSkill } from '../shared/skill-path' +import { parseSkillFile, registrySkillName } from '../layers/registry/server/utils/skill-frontmatter' +import { canonicalSkillPaths, isRegistrySkillPath, isSkilldCacheSkill } from '../shared/skill-path' const args = process.argv.slice(2) const target = args[0] @@ -81,7 +81,9 @@ interface Skill { } const skills: Skill[] = [] -for (const file of skillFiles) { +// One copy per Skill name, the one the production sync and `skilld run` pick. +const canonicalPaths = new Set(canonicalSkillPaths(skillFiles.map(file => file.path), folder => registrySkillName(folder, repo))) +for (const file of skillFiles.filter(file => canonicalPaths.has(file.path))) { const segments = file.path.split('/') if (segments.length < 2) continue diff --git a/shared/skill-path.ts b/shared/skill-path.ts index 307fd21f..586a6e3c 100644 --- a/shared/skill-path.ts +++ b/shared/skill-path.ts @@ -62,6 +62,29 @@ export function canonicalSkillFolder(folders: readonly string[]): string | null return ranked[0] ?? null } +/** + * The canonical SKILL.md path of each Skill name, in input order. + * + * `paths` are SKILL.md paths from one Repository tree. `nameOf` names the + * Skill in a folder, with `.` for the root. Copies that share a name keep the + * one {@link canonicalSkillFolder} picks, the folder `skilld run` resolves by + * name, so the registry admits the same copy every sync. + */ +export function canonicalSkillPaths(paths: readonly string[], nameOf: (folder: string) => string): string[] { + const foldersByName = new Map() + for (const path of paths) { + const folder = skillFolderOf(path) + const name = nameOf(folder) + foldersByName.set(name, [...(foldersByName.get(name) ?? []), folder]) + } + const canonical = new Set([...foldersByName.values()].map(canonicalSkillFolder)) + return paths.filter(path => canonical.has(skillFolderOf(path))) +} + +function skillFolderOf(path: string): string { + return path === SKILL_FILE ? '.' : path.slice(0, -`/${SKILL_FILE}`.length) +} + function compareSkillFolders(left: string, right: string): number { const leftSegments = left === '.' ? [] : left.split('/') const rightSegments = right === '.' ? [] : right.split('/') diff --git a/test/unit/sync-repo-correctness.test.ts b/test/unit/sync-repo-correctness.test.ts index cb5edc7d..f9c67dab 100644 --- a/test/unit/sync-repo-correctness.test.ts +++ b/test/unit/sync-repo-correctness.test.ts @@ -564,6 +564,36 @@ describe('syncRepo content acknowledgement', () => { })).toBe('/gh/acme/skills') }) + // Repositories copy one Skill into Agent folders and plugin mirrors. Each + // sync used to write every copy over the one row, so the stored path moved + // to whichever copy was written last, and the next push moved it again. + it('stores the canonical copy of a Skill copied into Agent folders, push after push', async () => { + const copies = [ + '.claude/skills/modern-web-design/SKILL.md', + 'plugins/web/skills/modern-web-design/SKILL.md', + 'skills/modern-web-design/SKILL.md', + ] + insertRepo(sqlite, 'old-tree') + github.getBlobsBatch.mockImplementation(async (_owner, _repo, _branch, paths: string[]) => ({ + status: 200, + data: new Map(paths.map(path => [path, rawSkill('modern-web-design')])), + unreadable: new Set(), + rateLimit: null, + notModified: false, + })) + + const storedPaths: unknown[] = [] + for (const pushedAt of ['2026-07-12T12:00:00Z', '2026-07-12T13:00:00Z']) { + const summary = repoSummary(`tree-at-${pushedAt}`) + github.getTree.mockResolvedValue(tree(copies.map((path, index) => ({ path, sha: `copy-${index}` })), `tree-at-${pushedAt}`)) + github.getRepoSummary.mockResolvedValue({ ...summary, data: { ...summary.data, meta: { ...summary.data.meta, pushed_at: pushedAt } } }) + await syncRepo('acme', 'skills', {}, db, { ownerVerified: true }) + storedPaths.push(sqlite.prepare(`SELECT rendered_skill_path FROM skills WHERE name = 'modern-web-design'`).pluck().get()) + } + + expect(storedPaths).toEqual(['skills/modern-web-design/SKILL.md', 'skills/modern-web-design/SKILL.md']) + }) + it('refreshes the repository description when its skill tree is unchanged', async () => { insertRepo(sqlite, 'new-tree') insertSkill(sqlite, 'one', 'one-old')