From e6cd747d38a78627fab1350a7e1ee2f1ad795575 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Tue, 6 Oct 2026 03:06:32 +1100 Subject: [PATCH] fix(registry): land behavior line links on the line GitHub renders Markdown, so a #L anchor on SKILL.md opened the preview at the top. Markdown links now use the source view. The detail cache key moves to v4, so pages cached before deploy never claim that no rule matched. A Skill whose upstream file is gone gets no links, and the panel renders whenever the rail links to it. --- .../registry/app/components/SkillDetail.vue | 2 +- .../server/api/skills/[...slug].get.ts | 13 ++--- .../registry/server/utils/skill-behaviors.ts | 5 +- test/unit/skill-detail-behaviors.test.ts | 49 +++++++++++++------ test/unit/skill-detail-swr.test.ts | 2 +- test/unit/skill-page-behaviors.test.ts | 6 +-- 6 files changed, 51 insertions(+), 26 deletions(-) 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' }]], ]) })