diff --git a/layers/registry/app/components/SkillDetail.vue b/layers/registry/app/components/SkillDetail.vue index de541e3b..c91768fd 100644 --- a/layers/registry/app/components/SkillDetail.vue +++ b/layers/registry/app/components/SkillDetail.vue @@ -2244,7 +2244,7 @@ useHead(computed(() => ({ role="region" > loadSkillDetail(event, platform, slug), @@ -328,11 +328,6 @@ async function loadSkillDetail(event: H3Event, platform: Platform, slug: string) } const allowedTools = parseAllowedTools(rendered.frontmatter) const capability = classifyAllowedTools(allowedTools) - const behaviors = skillPageBehaviors({ - raw: rendered.raw, - assetPaths: assets.map(asset => asset.path), - source: { owner: source.owner, repo: source.repo, branch, skillPath: rendered.skillPath }, - }) const selectedAssets = selectSkillFiles(assets) // `rendered.*` describes the cached copy, which survives the file being // deleted upstream, so it can only ever say "we can still render this". The @@ -346,6 +341,12 @@ async function loadSkillDetail(event: H3Event, platform: Platform, slug: string) raw: rendered.raw, }) const sourceGone = !sourceResolved && row.source_resolved === 0 + const behaviors = skillPageBehaviors({ + raw: rendered.raw, + assetPaths: assets.map(asset => asset.path), + // A deleted upstream file has nothing to link to. + source: { owner: source.owner, repo: source.repo, branch, skillPath: sourceGone ? null : rendered.skillPath }, + }) // `current_sha` is the blob sha of SKILL.md, not a commit, so it cannot pin a link. const sourceCommitSha = row.latest_revision_sha ?? null const pushedAtIso = epochToIso(row.pushed_at) diff --git a/layers/registry/server/utils/skill-behaviors.ts b/layers/registry/server/utils/skill-behaviors.ts index 2da3a14e..b56a2b3a 100644 --- a/layers/registry/server/utils/skill-behaviors.ts +++ b/layers/registry/server/utils/skill-behaviors.ts @@ -32,7 +32,10 @@ export function skillPageBehaviors(input: { skillPath: directory ? `${directory}/${location.path}` : location.path, branch: input.source.branch, }) - return file && location.line !== null ? `${file}#L${location.line}` : file + if (!file || location.line === null) + return file + // GitHub renders Markdown, and a line anchor only works on its source view. + return /\.(?:md|mdx|markdown)$/i.test(location.path) ? `${file}?plain=1#L${location.line}` : `${file}#L${location.line}` } return detectBehaviors(files).map(behavior => ({ ...behavior, diff --git a/test/unit/skill-detail-behaviors.test.ts b/test/unit/skill-detail-behaviors.test.ts index a49bb264..fec7a0d4 100644 --- a/test/unit/skill-detail-behaviors.test.ts +++ b/test/unit/skill-detail-behaviors.test.ts @@ -20,30 +20,40 @@ vi.stubGlobal('useStorage', () => ({ let harness: SqliteD1 -beforeEach(() => { - vi.resetModules() - cacheMap.clear() - harness = createSqliteD1(allMigrations()) +function seed(sourceResolved: number) { harness.raw.prepare(`INSERT INTO repos (owner, repo, default_branch) VALUES ('acme', 'skills', 'main')`).run() const raw = '---\nname: setup\ndescription: Set up the project.\n---\n\n```sh\nsudo apt-get install jq\n```\n' harness.raw.prepare( `INSERT INTO skills (owner, repo, name, slug, display_name, source_resolved, rendered_skill_path, rendered_status, rendered_raw, rendered_html, rendered_at, assets) - VALUES ('acme', 'skills', 'setup', 'acme/skills/setup', 'setup', 1, + VALUES ('acme', 'skills', 'setup', 'acme/skills/setup', 'setup', ?, 'skills/setup/SKILL.md', 'ok', ?, '

ok

', ?, ?)`, - ).run(raw, NOW_SEC, JSON.stringify([{ path: 'scripts/install.sh', size: 12, type: 'code' }])) + ).run(sourceResolved, raw, NOW_SEC, JSON.stringify([{ path: 'scripts/install.sh', size: 12, type: 'code' }])) +} + +interface Body { + sourceFacts: { behaviors: Array<{ id: string, tier: string, locations: Array<{ path: string, line: number | null, url: string | null }> }> } +} + +async function detail(): Promise { + const handler = (await import('../../layers/registry/server/api/skills/[...slug].get')).default as (event: H3Event) => Promise + return handler({ + context: { platform: { db: harness.db } }, + node: { req: { headers: {} } }, + } as unknown as H3Event) +} + +beforeEach(() => { + vi.resetModules() + cacheMap.clear() + harness = createSqliteD1(allMigrations()) }) describe('skill detail behaviors', () => { it('names each behavior in SKILL.md and the stored file names, with a link to where it appears', async () => { - const handler = (await import('../../layers/registry/server/api/skills/[...slug].get')).default as (event: H3Event) => Promise<{ - sourceFacts: { behaviors: Array<{ id: string, tier: string, locations: Array<{ path: string, line: number | null, url: string | null }> }> } - }> + seed(1) - const body = await handler({ - context: { platform: { db: harness.db } }, - node: { req: { headers: {} } }, - } as unknown as H3Event) + const body = await detail() expect(body.sourceFacts.behaviors.map(behavior => [behavior.id, behavior.tier])).toEqual([ ['privilege', 'ask'], @@ -54,8 +64,19 @@ describe('skill detail behaviors', () => { expect(body.sourceFacts.behaviors[0]?.locations).toEqual([{ path: 'SKILL.md', line: 7, - url: expect.stringMatching(/^https:\/\/github\.com\/acme\/skills\/blob\/[^/]+\/skills\/setup\/SKILL\.md#L7$/), + url: expect.stringMatching(/^https:\/\/github\.com\/acme\/skills\/blob\/[^/]+\/skills\/setup\/SKILL\.md\?plain=1#L7$/), }]) expect(body.sourceFacts.behaviors[2]?.locations[0]?.url).toMatch(/\/skills\/setup\/scripts\/install\.sh$/) }) + + it('links nothing once the upstream SKILL.md is gone', async () => { + seed(0) + + const body = await detail() + + expect(body.sourceFacts.behaviors.length).toBeGreaterThan(0) + expect(body.sourceFacts.behaviors.flatMap(behavior => behavior.locations.map(location => location.url))).toEqual( + body.sourceFacts.behaviors.flatMap(behavior => behavior.locations.map(() => null)), + ) + }) }) diff --git a/test/unit/skill-detail-swr.test.ts b/test/unit/skill-detail-swr.test.ts index 161ca840..26f3255f 100644 --- a/test/unit/skill-detail-swr.test.ts +++ b/test/unit/skill-detail-swr.test.ts @@ -14,7 +14,7 @@ import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' describe('skill detail SWR cache', () => { const NOW_SEC = Math.floor(Date.now() / 1000) const slug = 'ericzakariasson/scandinavian-design/alpha' - const cacheKey = `skills:detail:v3:${slug}` + const cacheKey = `skills:detail:v4:${slug}` let harness: SqliteD1 let handler: (event: H3Event) => Promise> let prepareCalls: number diff --git a/test/unit/skill-page-behaviors.test.ts b/test/unit/skill-page-behaviors.test.ts index 73a82089..7ff29829 100644 --- a/test/unit/skill-page-behaviors.test.ts +++ b/test/unit/skill-page-behaviors.test.ts @@ -12,10 +12,10 @@ describe('skillPageBehaviors', () => { }) expect(behaviors.map(behavior => [behavior.id, behavior.tier, behavior.locations])).toEqual([ - ['privilege', 'ask', [{ path: 'SKILL.md', line: 7, url: 'https://github.com/acme/skills/blob/main/skills/setup/SKILL.md#L7' }]], - ['shell', 'show', [{ path: 'SKILL.md', line: 6, url: 'https://github.com/acme/skills/blob/main/skills/setup/SKILL.md#L6' }]], + ['privilege', 'ask', [{ path: 'SKILL.md', line: 7, url: 'https://github.com/acme/skills/blob/main/skills/setup/SKILL.md?plain=1#L7' }]], + ['shell', 'show', [{ path: 'SKILL.md', line: 6, url: 'https://github.com/acme/skills/blob/main/skills/setup/SKILL.md?plain=1#L6' }]], ['scripts', 'show', [{ path: 'scripts/install.sh', line: null, url: 'https://github.com/acme/skills/blob/main/skills/setup/scripts/install.sh' }]], - ['packages', 'show', [{ path: 'SKILL.md', line: 7, url: 'https://github.com/acme/skills/blob/main/skills/setup/SKILL.md#L7' }]], + ['packages', 'show', [{ path: 'SKILL.md', line: 7, url: 'https://github.com/acme/skills/blob/main/skills/setup/SKILL.md?plain=1#L7' }]], ]) })