test(skills): guard guide-to-CLI parity, description shape, references, and size

This commit is contained in:
Jinwoo-H
2026-09-04 17:24:12 -04:00
parent 70b4811267
commit dca9336a4a
4 changed files with 317 additions and 1 deletions
@@ -1,5 +1,5 @@
import { execFile } from 'node:child_process'
import { cp, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'
import { cp, mkdir, mkdtemp, readFile, readdir, rm, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import path from 'node:path'
import { promisify } from 'node:util'
@@ -351,3 +351,55 @@ describe('bundled skill guide generator', () => {
await expect(buildArtifacts(root)).rejects.toThrow('Guide reference is empty')
})
})
// Why generalized: `orchestration-skill-guidance.test.mjs` pins this both-directions routing for
// orchestration alone. Any guide that grows a `references/` directory needs the same contract, or a
// reference can ship unroutable or a gate can route a file that does not exist.
describe('guide reference routing', () => {
async function guidesWithReferences() {
const guideRoot = path.join(projectDir, 'skill-guides')
const entries = await readdir(guideRoot, { withFileTypes: true })
const owners = []
for (const entry of entries.filter((candidate) => candidate.isDirectory())) {
const referenceRoot = path.join(guideRoot, entry.name, 'references')
const shipped = await readdir(referenceRoot).catch(() => null)
if (shipped === null) {
continue
}
owners.push({
name: entry.name,
referenceRoot,
shipped: shipped.filter((file) => file.endsWith('.md')).sort()
})
}
return owners
}
it('routes every shipped reference from its own guide, in both directions', async () => {
const owners = await guidesWithReferences()
// A vacuous loop would pass forever; orchestration is the guide that owns references today.
expect(owners.map((owner) => owner.name)).toContain('orchestration')
const mismatches = []
for (const owner of owners) {
const guidePath = path.join(projectDir, 'skill-guides', `${owner.name}.md`)
const guide = await readFile(guidePath, 'utf8').catch(() => null)
if (guide === null) {
mismatches.push(`${owner.name}: references/ exists with no ${owner.name}.md beside it`)
continue
}
const routed = [
...new Set([...guide.matchAll(/`references\/([^`]+\.md)`/gu)].map((match) => match[1]))
].sort()
const unshipped = routed.filter((file) => !owner.shipped.includes(file))
const unrouted = owner.shipped.filter((file) => !routed.includes(file))
if (unshipped.length > 0) {
mismatches.push(`${owner.name}: routes missing ${unshipped}`)
}
if (unrouted.length > 0) {
mismatches.push(`${owner.name}: ships unrouted ${unrouted}`)
}
}
expect(mismatches).toEqual([])
})
})
@@ -7,6 +7,10 @@ const skillsDir = resolve(import.meta.dirname, '../../skills')
// Why: the Agent Skills spec caps `description` at 1024 chars and conforming installers
// reject the whole skill (#17935); the frontmatter is what the installer parses, so check it.
const MAX_DESCRIPTION_LENGTH = 1024
// Why raw, not backtick-stripped: NVIDIA SkillEvaluator rejects `<tag>` in a description as a
// schema error, and Cowork's validator parses descriptions as HTML and fails the whole plugin
// silently (compound-engineering #602). Neither honors backticks, so placeholders belong in the body.
const ANGLE_BRACKET_TOKEN = /<[A-Za-z][\w.-]*>/u
function readDescription(skillName) {
const skillMarkdown = readFileSync(join(skillsDir, skillName, 'SKILL.md'), 'utf8')
@@ -36,4 +40,13 @@ describe('bundled skill descriptions', () => {
`${name}: description is ${description.length} chars`
).toBeLessThanOrEqual(MAX_DESCRIPTION_LENGTH)
})
it.each(skillNames)('%s keeps angle-bracket placeholders out of its description', (name) => {
const token = ANGLE_BRACKET_TOKEN.exec(readDescription(name) ?? '')
expect(
token?.[0],
`${name}: rephrase or move "${token?.[0] ?? ''}" into the skill body`
).toBeUndefined()
})
})
@@ -0,0 +1,74 @@
import { readdirSync, readFileSync } from 'node:fs'
import { join, resolve } from 'node:path'
import { describe, expect, it } from 'vitest'
const guideRoot = resolve(import.meta.dirname, '../../skill-guides')
/**
* Provenance: the Agent Skills spec's "keep your main SKILL.md under 500 lines" is an explicit
* recommendation, not a limit, and nothing rejects a longer guide. 300 is the tighter bound this
* repo already practices — six of eight guides sit under it, and `orchestration.md` holds 201 lines
* by routing detail into `references/`, which is the restructure this budget is meant to push.
* A line count is not a token count; treat a green run as a shape check, not a context-budget proof.
*/
const MAX_GUIDE_LINES = 300
/**
* Guides that already exceed the bound, with the size they may not grow past. Recorded sizes are a
* ratchet ceiling, not a target: shrink them freely and delete the entry once the guide fits.
* A name may leave this set. A name may never join it — split the guide into `references/` instead.
*/
const OVER_BUDGET = new Map([
['orca-cli', 424],
['orca-per-workspace-env', 794]
])
/** Matches `wc -l`: a trailing newline ends the last line rather than starting a new one. */
function lineCount(contents) {
const lines = contents.split(/\r?\n/u)
return lines.at(-1) === '' ? lines.length - 1 : lines.length
}
function guideSizes() {
return new Map(
readdirSync(guideRoot, { withFileTypes: true })
.filter((entry) => entry.isFile() && entry.name.endsWith('.md'))
.map((entry) => [
entry.name.replace(/\.md$/u, ''),
lineCount(readFileSync(join(guideRoot, entry.name), 'utf8'))
])
)
}
describe('always-loaded skill guide size budget', () => {
const sizes = guideSizes()
it('measures every shipped guide', () => {
expect(sizes.size).toBeGreaterThanOrEqual(8)
expect(sizes.get('orchestration')).toBeGreaterThan(0)
})
it('keeps every guide outside OVER_BUDGET under the bound', () => {
const violations = [...sizes]
.filter(([name, size]) => size > MAX_GUIDE_LINES && !OVER_BUDGET.has(name))
.map(([name, size]) => `${name}: ${size} lines > ${MAX_GUIDE_LINES}`)
expect(violations).toEqual([])
})
it('never lets an OVER_BUDGET guide grow past its recorded size', () => {
const grown = [...OVER_BUDGET]
.filter(([name, ceiling]) => (sizes.get(name) ?? 0) > ceiling)
.map(([name, ceiling]) => `${name}: ${sizes.get(name)} lines > recorded ${ceiling}`)
expect(grown).toEqual([])
})
it('drops OVER_BUDGET entries that now fit, so the set only ratchets down', () => {
const stale = [...OVER_BUDGET.keys()].filter(
(name) => !sizes.has(name) || (sizes.get(name) ?? 0) <= MAX_GUIDE_LINES
)
expect(stale).toEqual([])
})
})
+177
View File
@@ -0,0 +1,177 @@
import { readdirSync, readFileSync } from 'node:fs'
import { join, relative, resolve } from 'node:path'
import { describe, expect, it } from 'vitest'
import { CLI_GLOBAL_FLAGS } from '../shared/cli-argument-boundary'
import { specPaths } from './command-spec'
import { COMMAND_SPECS } from './specs'
// Why: a guide is the version-matched surface for the binary that shipped it, so a command
// path or flag it names must exist in COMMAND_SPECS. `orca emulator camera --webcam` was
// documented for months without ever existing (#16904 review C1).
// Why __dirname: it works under both Vitest and the CommonJS tsc emit that build:cli type-checks
// this file against; import.meta.dirname does not (TS1470).
const projectDir = resolve(__dirname, '..', '..')
const guideRoot = join(projectDir, 'skill-guides')
const MAX_COMMAND_DEPTH = 3
type Invocation = { file: string; line: number; text: string }
function guideFiles(directory: string): string[] {
return readdirSync(directory, { withFileTypes: true }).flatMap((entry) => {
const full = join(directory, entry.name)
if (entry.isDirectory()) {
return guideFiles(full)
}
return entry.isFile() && entry.name.endsWith('.md') ? [full] : []
})
}
/**
* The invocation span is the command text only — never the surrounding prose or table cell.
* `skill-guides/orca-emulator.md` describes serve-sim's own `--detach` in a Notes column beside
* an `ORCA ...` cell, and that is correct prose a line-scoped check would flag.
*/
function invocationSpans(contents: string, file: string): Invocation[] {
const found: Invocation[] = []
let inFence = false
contents.split(/\r?\n/u).forEach((line, index) => {
if (/^\s*(?:```|~~~)/u.test(line)) {
inFence = !inFence
return
}
const spans = inFence ? [line] : [...line.matchAll(/`([^`]+)`/gu)].map((match) => match[1])
for (const span of spans) {
const starts = [...span.matchAll(/\bORCA\b/gu)].map((match) => match.index)
starts.forEach((start, position) => {
found.push({
file,
line: index + 1,
text: span.slice(start, starts[position + 1] ?? span.length).trim()
})
})
}
})
return found
}
/** Blank out quoted values so a nested `--model` inside `--command "codex --model ..."` is not read as a flag. */
function maskQuotedValues(text: string): string {
let masked = ''
let quote: string | null = null
for (const character of text) {
if (quote) {
masked += character === quote ? character : ' '
if (character === quote) {
quote = null
}
} else if (character === '"' || character === "'") {
quote = character
masked += character
} else {
masked += character
}
}
return masked
}
const specByPath = new Map<string, (typeof COMMAND_SPECS)[number]>()
const pathPrefixes = new Set<string>()
for (const spec of COMMAND_SPECS) {
for (const path of specPaths(spec)) {
specByPath.set(path.join(' '), spec)
for (let length = 1; length < path.length; length += 1) {
pathPrefixes.add(path.slice(0, length).join(' '))
}
}
}
function longestKnownPrefix(tokens: string[]): string | null {
for (let length = tokens.length; length >= 1; length -= 1) {
const candidate = tokens.slice(0, length).join(' ')
if (specByPath.has(candidate) || pathPrefixes.has(candidate)) {
return candidate
}
}
return null
}
function allowedFlagsFor(prefix: string): Set<string> {
const exact = specByPath.get(prefix)
const flags = new Set<string>(CLI_GLOBAL_FLAGS)
const specs = exact
? [exact]
: COMMAND_SPECS.filter((spec) =>
specPaths(spec).some((path) => path.join(' ').startsWith(`${prefix} `))
)
for (const spec of specs) {
for (const flag of spec.allowedFlags) {
flags.add(flag)
}
}
return flags
}
function describeFailure(invocation: Invocation, detail: string): string {
const location = `${relative(projectDir, invocation.file)}:${invocation.line}`
return `${location}: ${detail}\n ${invocation.text}`
}
function parityFailures(invocation: Invocation): string[] {
const masked = maskQuotedValues(invocation.text).replace(/\s#.*$/u, '')
const tokens: string[] = []
for (const token of masked.slice('ORCA'.length).trim().split(/\s+/u)) {
if (!/^[a-z][a-z0-9-]*$/u.test(token) || tokens.length === MAX_COMMAND_DEPTH) {
break
}
tokens.push(token)
}
if (tokens.length === 0) {
return []
}
const failures: string[] = []
let command: string | null = null
for (let length = tokens.length; length >= 1 && command === null; length -= 1) {
const candidate = tokens.slice(0, length).join(' ')
if (specByPath.has(candidate)) {
command = candidate
}
}
if (command === null) {
// An incomplete reference like `ORCA emulator ...` names a real prefix and pins no flags.
if (pathPrefixes.has(tokens.join(' '))) {
return failures
}
failures.push(
describeFailure(invocation, `no COMMAND_SPECS path or alias for "${tokens.join(' ')}"`)
)
command = longestKnownPrefix(tokens)
if (command === null) {
return failures
}
}
const allowed = allowedFlagsFor(command)
for (const match of masked.matchAll(/--([a-z][a-z0-9-]*)/gu)) {
if (!allowed.has(match[1])) {
failures.push(describeFailure(invocation, `--${match[1]} is not a flag of "${command}"`))
}
}
return failures
}
describe('skill guides only name commands and flags the CLI defines', () => {
const invocations = guideFiles(guideRoot).flatMap((file) =>
invocationSpans(readFileSync(file, 'utf8'), file)
)
it('extracts invocations from every guide and reference', () => {
expect(invocations.length).toBeGreaterThan(150)
expect(new Set(invocations.map((invocation) => invocation.file)).size).toBeGreaterThan(8)
})
it('resolves every ORCA invocation against COMMAND_SPECS', () => {
expect(invocations.flatMap(parityFailures)).toEqual([])
})
})