fix(registry): never link a Skill whose source is gone

layers/registry/server/api/skill-asset/[...slug].get.ts

@@ -4,7 +4,7 @@ import { normalizeSkillAssetFilePath } from '#shared/skill-asset-path'
 import { resolveRepoSourceIdentityFromRow } from '../../utils/repo-source-identity'
 import { skillImagePolicyForEvent } from '../../utils/skill-image-policy'
 import { parseSkillMd } from '../../utils/skill-md-render'
-import { loadStoredSkillRow, parseStoredAssets, resolveReferencedFileTarget } from '../../utils/skill-stored-source'
+import { loadLinkableRepoSkillNames, loadStoredSkillRow, parseStoredAssets, resolveReferencedFileTarget } from '../../utils/skill-stored-source'
 import { findSkill } from '../../utils/skills-registry'
 import { fetchUpstreamText } from '../../utils/upstream-text'
 
@@ -22,10 +22,6 @@ interface AssetCache {
   skillPath: string | null
 }
 
-interface RepoSkillNameRow {
-  name: string
-}
-
 interface RegisteredAsset {
   path: string
   size: number
@@ -70,12 +66,9 @@ export default defineApiHandler({
     if (!skill)
       throw createError({ statusCode: 404, message: 'Skill not found' })
 
-    const [row, repoSkillRows] = await Promise.all([
+    const [row, repoSkillNames] = await Promise.all([
       loadStoredSkillRow(platform.db, skill),
-      platform.db
-        .prepare(`SELECT name FROM skills WHERE owner = ? AND repo = ? ORDER BY name`)
-        .bind(skill.owner, skill.repo)
-        .all<RepoSkillNameRow>(),
+      loadLinkableRepoSkillNames(platform.db, skill),
     ])
 
     if (!row)
@@ -107,7 +100,8 @@ export default defineApiHandler({
     const branch = row.rendered_commit_sha!
     const skillMdPath = row.rendered_skill_path!
     const skillDir = skillMdPath.replace(/(?:^|\/)SKILL\.md$/i, '')
-    const cacheKey = `skills:asset:v5:${source.owner}/${source.repo}/${skill.name}:${filePath}:${branch}`
+    // v6: v5 renders could link a sibling Skill whose source is gone.
+    const cacheKey = `skills:asset:v6:${source.owner}/${source.repo}/${skill.name}:${filePath}:${branch}`
     const cached = await readCache<AssetCache>(useStorage('edge-cache'), cacheKey)
     if (cached) {
       if (cached.status === 'missing')
@@ -151,7 +145,7 @@ export default defineApiHandler({
         branch,
         skillDir,
         filePath,
-        skillNames: (repoSkillRows.results ?? []).map(candidate => candidate.name),
+        skillNames: repoSkillNames,
         registryOwner: skill.owner,
         registryRepo: skill.repo,
       }, await skillImagePolicyForEvent(event))

layers/registry/server/api/skills/[...slug].get.ts

@@ -19,6 +19,7 @@ import { getGeneratedKinds } from '../../utils/skill-generated'
 import { skillImagePolicyForEvent } from '../../utils/skill-image-policy'
 import { readRepositoryLicense } from '../../utils/skill-license'
 import { parseSkillMd } from '../../utils/skill-md-render'
+import { loadLinkableRepoSkillNames } from '../../utils/skill-stored-source'
 import { findDuplicateGroupForSkill, findSkillWithRow } from '../../utils/skills-registry'
 import { tagLinkPath } from '../../utils/tag-quality'
 import { isSkillIndexable, noteAdmissionFallback, SKILL_INDEX_INPUT_COLUMNS_SQL } from '../../utils/trending-admission'
@@ -207,10 +208,6 @@ interface SkillDetailRow {
   latest_revision_sha: string | null
 }
 
-interface RepoSkillNameRow {
-  name: string
-}
-
 const skillDetailHandler = defineApiHandler({
   response: SkillDetailResponseSchema,
   handler: async ({ event, platform }) => {
@@ -242,14 +239,10 @@ async function loadSkillDetail(event: H3Event, platform: Platform, slug: string)
   const { skill, row } = found
   const curators: CuratorEndorsement[] = []
 
-  const [duplicateGroup, generated, repoSkillRows] = await Promise.all([
+  const [duplicateGroup, generated, repoSkillNames] = await Promise.all([
     findDuplicateGroupForSkill(event, `${skill.owner}/${skill.repo}/${skill.name}`),
     getGeneratedKinds(platform.db, { owner: skill.owner, repo: skill.repo, name: skill.name }, ['faq', 'tags', 'summary']),
-    // Dependency links must only target Skills whose source still exists.
-    platform.db
-      .prepare(`SELECT name FROM skills WHERE owner = ? AND repo = ? AND source_resolved = 1 ORDER BY name`)
-      .bind(skill.owner, skill.repo)
-      .all<RepoSkillNameRow>(),
+    loadLinkableRepoSkillNames(platform.db, skill),
   ])
   const faqRow = generated.get('faq') as { payload: FaqPayload } | undefined
   const tagRow = generated.get('tags') as { payload: TagPayload } | undefined
@@ -258,7 +251,6 @@ async function loadSkillDetail(event: H3Event, platform: Platform, slug: string)
   const source = resolveRepoSourceIdentityFromRow(skill, row)
   const githubUrl = `https://github.com/${source.owner}/${source.repo}`
   const branch = row.rendered_commit_sha || row.default_branch || 'main'
-  const repoSkillNames = (repoSkillRows.results ?? []).map(candidate => candidate.name)
 
   // Warm path: render is in D1. Cold path (no usable stored render): render
   // live so the page still has content. A page view never writes the render

layers/registry/server/utils/skill-stored-source.ts

@@ -52,6 +52,22 @@ export async function loadStoredSkillRow(
     .first<StoredSkillRow>()
 }
 
+/**
+ * The Skill names in one repository that a rendered document may link.
+ *
+ * A Markdown render turns a `/name` mention of a sibling Skill into a link.
+ * A Skill whose SKILL.md is gone upstream answers 410, so its name stays out.
+ * The Skill page and each file page share this read, so neither can link a
+ * sibling the other keeps as text.
+ */
+export async function loadLinkableRepoSkillNames(db: D1Database, skill: RepoIdentity): Promise<string[]> {
+  const rows = await db
+    .prepare(`SELECT name FROM skills WHERE owner = ? AND repo = ? AND source_resolved = 1 ORDER BY name`)
+    .bind(skill.owner, skill.repo)
+    .all<{ name: string }>()
+  return (rows.results ?? []).map(row => row.name)
+}
+
 export type StoredSkillMd
   = | { _tag: 'ok', body: string, source: string }
     | { _tag: 'gone' }

layers/registry/server/utils/skills-registry.ts

@@ -19,6 +19,13 @@ import { SUPPORTED_SKILL_SQL } from './supported-sources'
 import { noteAdmissionFallback, SKILL_ADMISSIONS_POPULATED_SQL, SKILL_INDEXABLE_SQL } from './trending-admission'
 
 const NOT_BROKEN_SQL = notBrokenSql('r')
+/**
+ * A Skill that a page may link. A Skill whose source is gone upstream
+ * (`source_resolved = 0`) answers 410 (server/middleware/skill-source-gone.ts),
+ * so a link to it is a broken internal link. Listings, related Skills, and
+ * featured sections read through this one predicate.
+ */
+const LINKABLE_SKILL_SQL = `(${NOT_BROKEN_SQL} AND s.source_resolved = 1)`
 const NOT_AGGREGATOR_SQL = notAggregatorSql('r')
 const FROM_SKILLS_JOIN_REPOS = `FROM skills s ${JOIN_REPOS_SQL}`
 const SELECT_SKILL_ROW_BASE = `
@@ -250,13 +257,10 @@ export async function querySkills(event: H3Event, opts: SkillsQuery): Promise<Sk
   const offset = opts.offset ?? (page - 1) * limit
   const selectSkillRow = includeDependencies ? SELECT_SKILL_ROW_WITH_BODY : SELECT_SKILL_ROW
 
-  // Listings exclude skills whose source is unresolved or deleted upstream
-  // (`source_resolved = 0`): their detail pages serve 410 tombstones
-  // (server/middleware/skill-source-gone.ts), so linking them from a hub is a
-  // self-inflicted broken internal link. Same convention as community,
-  // homepage and collection queries. Tombstone pages load the skill directly,
-  // not through this query.
-  const conditions: string[] = [NOT_BROKEN_SQL, 's.source_resolved = 1']
+  // A listing links every row, so it reads linkable Skills only, like the
+  // community, homepage and collection queries. Tombstone pages load the
+  // skill directly, not through this query.
+  const conditions: string[] = [LINKABLE_SKILL_SQL]
   const params: (string | number)[] = []
 
   if (maintainerRepos && !search && !owner && !tags?.length && !official && !supportedOnly && !category && !trustTier)
@@ -524,6 +528,9 @@ export interface SkillLookup {
  * When owner is known, the exact (owner, name) row is returned. When owner
  * is missing (legacy collection entries), the highest-starred repository row
  * name wins. Result is keyed by packageName.
+ *
+ * By default only linkable Skills match. `includeBroken` also matches broken
+ * repositories and gone sources, for a name lookup that renders no link.
  */
 export async function findSkillsByLookups(
   event: H3Event,
@@ -535,7 +542,7 @@ export async function findSkillsByLookups(
   const db = getDB(event)
   const uniqueNames = [...new Set(lookups.map(l => l.packageName))]
   const placeholders = uniqueNames.map(() => '?').join(',')
-  const brokenClause = opts.includeBroken ? '' : ` AND ${NOT_BROKEN_SQL}`
+  const brokenClause = opts.includeBroken ? '' : ` AND ${LINKABLE_SKILL_SQL}`
   const supportedClause = opts.supportedOnly ? ` AND (${SUPPORTED_SKILL_SQL})` : ''
   const rows = await db
     .prepare(`SELECT ${SELECT_SKILL_ROW} ${FROM_SKILLS_JOIN_REPOS} WHERE s.name IN (${placeholders})${brokenClause}${supportedClause}`)
@@ -703,7 +710,7 @@ export async function getFeaturedOfficialSections(
           ROW_NUMBER() OVER (PARTITION BY s.owner, s.repo ORDER BY s.modified_at DESC, s.name ASC) AS rn,
           COUNT(*) OVER (PARTITION BY s.owner, s.repo) AS repo_total
         ${FROM_SKILLS_JOIN_REPOS}
-        WHERE (${filter.sql}) AND ${NOT_BROKEN_SQL}
+        WHERE (${filter.sql}) AND ${LINKABLE_SKILL_SQL}
       ) ranked WHERE rn <= ?`,
     )
     .bind(...filter.params, perOrg)
@@ -964,12 +971,12 @@ export async function findRelatedSkills(
           AND repo_skills.repo = ?2
           AND repo_skills.source_resolved = 1
       ) AS repo_skill_count FROM (
-        SELECT ${SELECT_SKILL_ROW_BASE} ${FROM_SKILLS_JOIN_REPOS} WHERE s.owner = ?1 AND s.repo = ?2 AND s.name != ?3 AND s.source_resolved = 1 AND ${NOT_BROKEN_SQL} ORDER BY s.modified_at DESC, s.name ASC LIMIT ?4
+        SELECT ${SELECT_SKILL_ROW_BASE} ${FROM_SKILLS_JOIN_REPOS} WHERE s.owner = ?1 AND s.repo = ?2 AND s.name != ?3 AND ${LINKABLE_SKILL_SQL} ORDER BY s.modified_at DESC, s.name ASC LIMIT ?4
       ) paged ORDER BY modified_at DESC, name ASC`)
       .bind(owner, repo, excludeName, limit),
     db
       .prepare(`SELECT paged.*, ${repoSkillCountSql('paged')} FROM (
-        SELECT ${SELECT_SKILL_ROW_BASE} ${FROM_SKILLS_JOIN_REPOS} WHERE s.owner = ? AND NOT (s.repo = ?) AND s.name != ? AND s.source_resolved = 1 AND ${NOT_BROKEN_SQL} ORDER BY r.stars DESC, s.modified_at DESC, s.name ASC LIMIT ?
+        SELECT ${SELECT_SKILL_ROW_BASE} ${FROM_SKILLS_JOIN_REPOS} WHERE s.owner = ? AND NOT (s.repo = ?) AND s.name != ? AND ${LINKABLE_SKILL_SQL} ORDER BY r.stars DESC, s.modified_at DESC, s.name ASC LIMIT ?
       ) paged ORDER BY stars DESC, modified_at DESC, name ASC`)
       .bind(owner, repo, excludeName, limit),
   ])

test/unit/skill-asset-tree-outage.test.ts

@@ -98,6 +98,23 @@ describe('skill-asset source gone', () => {
   })
 })
 
+describe('skill-asset Skill references', () => {
+  it('links a live sibling Skill and keeps a removed one as text', async () => {
+    fixture.raw.prepare(
+      `INSERT INTO skills (owner, repo, name, slug, display_name, source_resolved)
+       VALUES ('owner', 'repo', 'live', 'owner/live', 'Live', 1),
+              ('owner', 'repo', 'removed', 'owner/removed', 'Removed', 0)`,
+    ).run()
+    vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response('Run `/live`, then `/removed`.')))
+    const handler = (await import('../../layers/registry/server/api/skill-asset/[...slug].get')).default
+
+    const body = await handler(event()) as { html: string }
+
+    expect([...body.html.matchAll(/href="([^"]+)"/g)].map(match => match[1])).toEqual(['/gh/owner/repo/live'])
+    expect(body.html).toContain('<code>/removed</code>')
+  })
+})
+
 function event(): H3Event {
   return {
     method: 'GET',

test/unit/skill-links-skip-gone-sources.test.ts

@@ -0,0 +1,47 @@
+import type { H3Event } from 'h3'
+import type { SqliteD1 } from './helpers/d1-sqlite'
+import { afterEach, beforeEach, describe, expect, it } from 'vitest'
+import { findSkillsByLookups, getFeaturedOfficialSections } from '../../layers/registry/server/utils/skills-registry'
+import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite'
+
+// A Skill whose SKILL.md is gone upstream answers 410. Every page that links
+// it carries a broken internal link, so a Skill card read must skip it.
+
+let fixture: SqliteD1
+
+beforeEach(() => {
+  fixture = createSqliteD1(allMigrations())
+  fixture.raw.exec(`
+    INSERT INTO repos (owner, repo, stars) VALUES ('acme', 'skills', 10);
+    INSERT INTO skills (owner, repo, name, slug, display_name, source_resolved, modified_at) VALUES
+      ('acme', 'skills', 'live', 'acme/live', 'Live', 1, 1),
+      ('acme', 'skills', 'removed', 'acme/removed', 'Removed', 0, 2);
+  `)
+})
+
+afterEach(() => fixture.close())
+
+function event(): H3Event {
+  return { context: { platform: { db: fixture.db } } } as unknown as H3Event
+}
+
+describe('skill card reads skip a Skill whose source is gone', () => {
+  it('resolves related Skill names to live Skills only', async () => {
+    const found = await findSkillsByLookups(event(), [{ packageName: 'live' }, { packageName: 'removed' }])
+
+    expect([...found.keys()]).toEqual(['live'])
+  })
+
+  it('still resolves a removed Skill for the CLI name lookup', async () => {
+    const found = await findSkillsByLookups(event(), [{ packageName: 'removed' }], { includeBroken: true })
+
+    expect(found.get('removed')?.repo).toBe('skills')
+  })
+
+  it('features live Skills only and counts only those', async () => {
+    const sections = await getFeaturedOfficialSections(event(), [{ owner: 'acme', repo: 'skills' }], 5)
+
+    expect(sections.map(section => ({ total: section.totalSkills, names: section.skills.map(skill => skill.name) })))
+      .toEqual([{ total: 1, names: ['live'] }])
+  })
+})