fix(dsh): verify — строгая проверка scripts/*, дубликаты frontmatter, тесты (T2.1-T2.6, #69)

dsh/sync/verify-skills.mjs

@@ -14,18 +14,22 @@
  *      the scalar one vendored from LayneChai's `parseFrontmatter` (MIT); no
  *      provider is imported, because on the DeepSeek Harness the registry
  *      belongs to the built-in `skill-filesystem`, not to this repository.
+ *      A duplicate scalar key is an error too: the last value silently wins
+ *      in a naive parser, and a name/description pair that appears twice is
+ *      a broken skill file, not a style choice.
  *   2. The body carries no leftover upstream `superpowers:` namespace
  *      prefix — the port strips it during sync, and the DSH registry
  *      addresses skills by bare name.
  *   3. Every relative reference in the body resolves inside `<skills-dir>`:
  *      markdown links (`[x](../<skill>/y.md)`) and path-like inline code
- *      (`references/y.md`, `../<skill>/references/y.md`, `scripts/y`). A
- *      sibling skill's `scripts/` invocation only needs that skill directory
- *      to exist: the reduced vendor tree under `dsh/tests/fixtures/` keeps
- *      the upstream command without shipping the executable, and a mistyped
- *      skill name still fails. Names that are illustrative examples rather
- *      than real files (`scripts/tool.sh`, …) are skipped, as in the
- *      vendored reference implementation.
+ *      (`references/y.md`, `../<skill>/references/y.md`, `scripts/y`,
+ *      `../<skill>/scripts/y`). Both the skill directory and the file must
+ *      exist — a renamed or dropped upstream script is exactly the
+ *      regression this check exists for (T2.1/T2.2). Names that are
+ *      illustrative examples rather than real files (`scripts/tool.sh`, …)
+ *      are skipped, as in the vendored reference implementation; that
+ *      whitelist is a deliberate compromise, not a general escape hatch
+ *      (T2.3 in the SDD ledger).
  *
  * The prefix check is scoped to SKILL.md bodies on purpose: reference files
  * such as `using-superpowers/references/dsh-tools.md` quote `superpowers:` in
@@ -53,6 +57,11 @@ const TRAILING_PUNCTUATION = /[.,;:)\]]+$/
 /**
  * Names that appear only as illustrative examples in prose, never as real
  * files — the same list the vendored reference implementation keeps.
+ *
+ * Deliberate compromise: upstream `writing-skills` shows `bash scripts/tool.sh`
+ * as an example, and no such file ships (see T2.3 in the SDD ledger). The list
+ * is closed and exact-path matched, so it cannot hide a real dangling link in
+ * another skill.
  */
 const ILLUSTRATIVE = new Set(['scripts/tool.sh', 'scripts/tool.js', 'scripts/helper.py', 'scripts/helper.sh'])
 
@@ -61,8 +70,11 @@ const ILLUSTRATIVE = new Set(['scripts/tool.sh', 'scripts/tool.js', 'scripts/hel
  * `parseFrontmatter`: the file starts with `---`, the block ends at the first
  * line that starts with `---`, and each `key: value` line becomes a string
  * (matching quotes stripped). Anything else is left for a real YAML parser.
+ * Keys seen more than once are collected in `duplicates`: the vendored parser
+ * would let the last value win silently, and a duplicated `name` or
+ * `description` is a broken skill file.
  *
- * @returns {{metadata: Record<string, string>, body: string} | null}
+ * @returns {{metadata: Record<string, string>, body: string, duplicates: string[]} | null}
  */
 function parseFrontmatter(text) {
   if (!text.startsWith('---')) return null
@@ -71,6 +83,7 @@ function parseFrontmatter(text) {
   const block = text.slice(3, end)
   const body = text.slice(end + 4).replace(/^\n+/, '')
   const metadata = {}
+  const duplicates = []
   for (const line of block.split('\n')) {
     const match = /^([A-Za-z][\w-]*):\s*(.*)$/.exec(line.trim())
     if (!match) continue
@@ -78,9 +91,10 @@ function parseFrontmatter(text) {
     if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) {
       value = value.slice(1, -1)
     }
+    if (Object.hasOwn(metadata, match[1])) duplicates.push(match[1])
     metadata[match[1]] = value
   }
-  return { metadata, body }
+  return { metadata, body, duplicates }
 }
 
 /** Map every line inside a ``` / ~~~ fence to an empty line: examples are not prose. */
@@ -116,20 +130,16 @@ async function isDirectory(path) {
 }
 
 /**
- * Classify one whitespace-separated token of an inline code span.
+ * Return the in-tree reference a whitespace-separated inline-code token names,
+ * or `null` when it is prose (`foo`, `PLAN_FILE`, `skills/…` from the checkout
+ * root) rather than a path relative to the skill directory.
  *
- * `file` — a path that must resolve; `sibling-script` — a `../<skill>/scripts/`
- * invocation, where only the skill directory has to exist (see the header).
- *
- * @returns {{kind: 'file' | 'sibling-script', path: string} | null}
+ * @returns {string | null}
  */
 function inlineReference(token) {
-  if (/^(?:references|scripts)\//.test(token)) return { kind: 'file', path: token }
-  if (!token.startsWith('../')) return null
-  const parts = token.split('/')
-  const scripts = parts.indexOf('scripts')
-  if (scripts > 0) return { kind: 'sibling-script', path: parts.slice(0, scripts).join('/') }
-  return { kind: 'file', path: token }
+  if (/^(?:references|scripts)\//.test(token)) return token
+  if (token.startsWith('../')) return token
+  return null
 }
 
 function fail(skill, reason) {
@@ -151,6 +161,7 @@ async function verifySkill(skillsRoot, skill) {
 
   const parsed = parseFrontmatter(text)
   if (parsed === null) fail(skill, 'missing YAML frontmatter')
+  if (parsed.duplicates.length > 0) fail(skill, `duplicate key "${parsed.duplicates[0]}" in frontmatter`)
   if (!parsed.metadata.name) fail(skill, 'empty name in frontmatter')
   if (!parsed.metadata.description) fail(skill, 'empty description in frontmatter')
 
@@ -180,9 +191,9 @@ async function verifySkill(skillsRoot, skill) {
         if (token === '' || ILLUSTRATIVE.has(token)) continue
         const reference = inlineReference(token)
         if (reference === null) continue
-        const target = resolve(skillDir, reference.path)
-        const resolved = reference.kind === 'sibling-script' ? await isDirectory(target) : await exists(target)
-        if (!resolved) fail(skill, `dangling relative reference: ${token} (line ${lineNumber})`)
+        if (!(await exists(resolve(skillDir, reference)))) {
+          fail(skill, `dangling relative reference: ${token} (line ${lineNumber})`)
+        }
       }
     }
   }

dsh/tests/fixtures/superpowers-min/skills/subagent-driven-development/scripts/review-package

@@ -0,0 +1,2 @@
+#!/usr/bin/env bash
+echo "review-package: stub, dsh tests only"

dsh/tests/fixtures/superpowers-min/skills/subagent-driven-development/scripts/sdd-workspace

@@ -0,0 +1,2 @@
+#!/usr/bin/env bash
+echo "sdd-workspace: stub, dsh tests only"

dsh/tests/verify.test.mjs

@@ -1,6 +1,6 @@
 import assert from 'node:assert/strict'
 import { spawnSync } from 'node:child_process'
-import { cpSync, readFileSync } from 'node:fs'
+import { cpSync, readFileSync, rmSync } from 'node:fs'
 import { dirname, join, resolve } from 'node:path'
 import test from 'node:test'
 import { fileURLToPath } from 'node:url'
@@ -138,3 +138,130 @@ test('verify ok when superpowers: survives in a references file (T1.5)', () => {
     cleanup(work)
   }
 })
+
+test('verify fails when a sibling skill script is missing', () => {
+  const work = tempDir('dsh-verify-missing-script-')
+  try {
+    const dir = syncedCopy(work, syncedTree(work))
+    rmSync(join(dir, 'subagent-driven-development', 'scripts', 'sdd-workspace'))
+
+    const result = verify(dir)
+
+    assert.equal(result.status, 1)
+    assert.match(result.stderr, /executing-plans\/SKILL\.md/)
+    assert.match(result.stderr, /sdd-workspace/)
+  } finally {
+    cleanup(work)
+  }
+})
+
+test('verify fails on a mistyped sibling skill name', () => {
+  const work = tempDir('dsh-verify-mistyped-skill-')
+  try {
+    const dir = syncedCopy(work, syncedTree(work))
+    const file = join(dir, 'executing-plans', 'SKILL.md')
+    write(
+      file,
+      `${readFileSync(file, 'utf8')}\nRun \`bash ../subagent-driven-developmen/scripts/sdd-workspace PLAN_FILE\`.\n`,
+    )
+
+    const result = verify(dir)
+
+    assert.equal(result.status, 1)
+    assert.match(result.stderr, /subagent-driven-developmen\/scripts\/sdd-workspace/)
+  } finally {
+    cleanup(work)
+  }
+})
+
+test('verify fails on a duplicate frontmatter key', () => {
+  const work = tempDir('dsh-verify-duplicate-key-')
+  try {
+    const dir = syncedCopy(work, syncedTree(work))
+    const file = join(dir, 'brainstorming', 'SKILL.md')
+    write(file, readFileSync(file, 'utf8').replace(/^name: brainstorming$/m, 'name: brainstorming\nname: duplicate'))
+
+    const result = verify(dir)
+
+    assert.equal(result.status, 1)
+    assert.match(result.stderr, /brainstorming\/SKILL\.md/)
+    assert.match(result.stderr, /duplicate key/)
+  } finally {
+    cleanup(work)
+  }
+})
+
+test('verify fails without frontmatter delimiters', () => {
+  const work = tempDir('dsh-verify-no-frontmatter-')
+  try {
+    const dir = syncedCopy(work, syncedTree(work))
+    write(join(dir, 'brainstorming', 'SKILL.md'), '# Brainstorming\n\nNo frontmatter here.\n')
+
+    const result = verify(dir)
+
+    assert.equal(result.status, 1)
+    assert.match(result.stderr, /brainstorming\/SKILL\.md/)
+    assert.match(result.stderr, /missing YAML frontmatter/)
+  } finally {
+    cleanup(work)
+  }
+})
+
+test('verify CLI without a directory exits 2', () => {
+  const result = verify('')
+
+  assert.equal(result.status, 2)
+  assert.match(result.stderr, /usage:/)
+})
+
+test('verify an empty skills directory reports zero skills', () => {
+  const work = tempDir('dsh-verify-empty-')
+  try {
+    const dir = join(work, 'empty')
+    write(join(dir, '.keep'), '')
+
+    const result = verify(dir)
+
+    assert.equal(result.status, 0, `stderr: ${result.stderr}`)
+    assert.match(result.stdout, /verified 0 skills/)
+  } finally {
+    cleanup(work)
+  }
+})
+
+test('verify ignores prefixes and paths inside fenced examples', () => {
+  const work = tempDir('dsh-verify-fenced-')
+  try {
+    const dir = syncedCopy(work, syncedTree(work))
+    const file = join(dir, 'writing-plans', 'SKILL.md')
+    const fence = [
+      '```',
+      'See superpowers:brainstorming, [x](../brainstorming/nope.md) and `references/nope.md`.',
+      '```',
+    ].join('\n')
+    write(file, `${readFileSync(file, 'utf8')}\n${fence}\n`)
+
+    const result = verify(dir)
+
+    assert.equal(result.status, 0, `stderr: ${result.stderr}`)
+    assert.match(result.stdout, /verified 15 skills/)
+  } finally {
+    cleanup(work)
+  }
+})
+
+test('verify does not flag a resolving inline reference', () => {
+  const work = tempDir('dsh-verify-inline-reference-')
+  try {
+    const dir = syncedCopy(work, syncedTree(work))
+    const file = join(dir, 'using-superpowers', 'SKILL.md')
+    write(file, `${readFileSync(file, 'utf8')}\nSee \`references/dsh-tools.md\` again.\n`)
+
+    const result = verify(dir)
+
+    assert.equal(result.status, 0, `stderr: ${result.stderr}`)
+    assert.match(result.stdout, /verified 15 skills/)
+  } finally {
+    cleanup(work)
+  }
+})