ci: validate cross-skill references in lint-skill-entry

.github/scripts/check-public-refs.mjs

@@ -0,0 +1,85 @@
+#!/usr/bin/env node
+//
+// Every repository named in this repo must be fetchable by an anonymous reader.
+//
+// This repo is public. Naming a repository here discloses that it exists, who owns it and
+// roughly what is in it — and a prohibition discloses exactly as much as a recommendation:
+// "do not re-host to acme/secret-notes, it is private" publishes the name either way. So the
+// rule is about the mention, not the sentiment attached to it.
+//
+// The check is a request, not a list. An owner allowlist looked cheaper and was wrong on its
+// first run: it cleared nothing useful and flagged `nock/nock` and `phishfort/phishfort-lists`,
+// because "is this owner well known" is not the property that matters. The property is whether
+// a reader who is not you can open the link — which an unauthenticated request answers exactly.
+// 404 means private or absent; both are unresolvable for a public reader, and both are defects.
+//
+// Deliberately unauthenticated: a token would see private repos and pass them, which is the
+// failure this exists to prevent.
+//
+//   0  every referenced repository resolves anonymously
+//   1  one or more do not
+//   2  could not run (offline) — reported, not silently passed
+import { readdirSync, readFileSync, statSync } from 'node:fs';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const ROOT = process.env.SKILLS_LINT_ROOT
+  ? path.resolve(process.env.SKILLS_LINT_ROOT)
+  : path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..');
+
+// `orgs/`, `sponsors/` and friends are github.com paths that are not repositories.
+const NOT_A_REPO = new Set(['orgs', 'sponsors', 'users', 'settings', 'apps', 'topics', 'features', 'pricing']);
+// Org-internal repos are private to the public but readable by colleagues, and naming them is a
+// deliberate call: they are load-bearing context for the audience this repo is written for. The
+// rule being enforced is about *personal* repos, which are unreachable by colleagues too.
+const INTERNAL_OWNERS = new Set(['MetaMask', 'Consensys']);
+// Template placeholders in contributor docs are meant to be substituted, not resolved.
+const PLACEHOLDER = /^(YOUR|MY|<|\$\{)/u;
+const REPO_REF = /https?:\/\/(?:www\.)?github\.com\/([A-Za-z0-9][\w.-]*)\/([A-Za-z0-9][\w.-]*)/gu;
+const TEXT = /\.(md|sh|py|mjs|js|ya?ml|json|tsx?)$/u;
+
+function walk(dir, out = []) {
+  for (const e of readdirSync(dir, { withFileTypes: true })) {
+    if (e.name === '.git' || e.name === 'node_modules') continue;
+    const full = path.join(dir, e.name);
+    if (e.isDirectory()) walk(full, out);
+    else if (TEXT.test(e.name)) out.push(full);
+  }
+  return out;
+}
+
+const refs = new Map(); // "owner/repo" -> Set of relative paths
+for (const file of walk(ROOT)) {
+  let text;
+  try { text = readFileSync(file, 'utf8'); } catch { continue; }
+  for (const [, owner, repo] of text.matchAll(REPO_REF)) {
+    if (NOT_A_REPO.has(owner) || INTERNAL_OWNERS.has(owner) || PLACEHOLDER.test(owner)) continue;
+    const key = `${owner}/${repo.replace(/\.git$/u, '')}`;
+    if (!refs.has(key)) refs.set(key, new Set());
+    refs.get(key).add(path.relative(ROOT, file));
+  }
+}
+
+if (refs.size === 0) { console.log('check-public-refs: no repository references found'); process.exit(0); }
+
+let bad = 0, unknown = 0;
+for (const [key, files] of [...refs].sort()) {
+  let status;
+  try {
+    const res = await fetch(`https://github.com/${key}`, { method: 'HEAD', redirect: 'follow' });
+    status = res.status;
+  } catch {
+    console.error(`  ????  ${key} — request failed; cannot conclude`);
+    unknown += 1;
+    continue;
+  }
+  if (status === 200) continue;
+  bad += 1;
+  console.error(`  FAIL  ${key} — HTTP ${status} anonymously; a public reader cannot open this`);
+  for (const f of files) console.error(`          ${f}`);
+}
+
+console.log(`\ncheck-public-refs: ${refs.size} repository reference(s) checked`);
+if (unknown > 0 && bad === 0) { console.error(`${unknown} could not be checked — offline?`); process.exit(2); }
+if (bad > 0) { console.error(`${bad} unresolvable. Cite an org-owned location, or state the rule without the example.`); process.exit(1); }
+console.log('every referenced repository resolves anonymously');

.github/scripts/lint-skill-entry.mjs

@@ -9,7 +9,7 @@
 // Run against the repo:        node .github/scripts/lint-skill-entry.mjs
 // Run against another tree:     SKILLS_LINT_ROOT=/path node .github/scripts/lint-skill-entry.mjs
 
-import { readFileSync, readdirSync, statSync } from 'node:fs';
+import { existsSync, readFileSync, readdirSync, statSync } from 'node:fs';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 
@@ -40,8 +40,10 @@ export function lintSkill(skill) {
   const dirName = skill.id.slice(skill.domain.length + 1);
 
   let raw;
+  let source = '';
   try {
-    raw = parseFrontmatter(readFileSync(path.join(skill.path, 'skill.md'), 'utf8'));
+    source = readFileSync(path.join(skill.path, 'skill.md'), 'utf8');
+    raw = parseFrontmatter(source);
   } catch (error) {
     return { errors: [`could not read skill.md: ${error.message}`], warnings };
   }
@@ -121,9 +123,84 @@ export function lintSkill(skill) {
     }
   }
 
+  crossReferenceChecks(skill, raw, source, errors, warnings);
+
   return { errors, warnings };
 }
 
+// Lane IDs (`B7`, `C4`) are addresses into evidence-catalog.md, not names. They carry no
+// meaning to a reader who has not opened the catalog, and a `description` cannot link out
+// to it — frontmatter is plain text. So: never in a description, and in the body only on a
+// line that also links the catalog. The catalog's own skill is exempt: it defines them.
+const LANE_ID = /(?<![\w-])[A-G]\d(?![\w-])/u;
+const CATALOG = 'evidence-catalog';
+
+// `## Related` is by convention a list of sibling skills, so every backticked kebab-case
+// token in it must name one. This is what catches a rename that swept the owning branch
+// and left every branch that referenced it pointing at a name that no longer resolves.
+// `$(?![\s\S])`, not `\z` — JS has no absolute-end anchor, and under `m` a bare `$` would
+// stop the section at its own first line break.
+const RELATED_SECTION = /^#{1,4}\s+Related\s*$([\s\S]*?)(?=^#{1,4}\s|$(?![\s\S]))/imu;
+const BACKTICKED = /`([a-z][a-z0-9]*(?:-[a-z0-9]+)+)`/gu;
+
+function crossReferenceChecks(skill, raw, source, errors, warnings) {
+  const ownsCatalog = existsSync(path.join(skill.path, 'references', `${CATALOG}.md`));
+
+  if (!ownsCatalog) {
+    if (raw.description && LANE_ID.test(raw.description)) {
+      errors.push(
+        `\`description\` cites a bare lane id (${raw.description.match(LANE_ID)[0]}); frontmatter cannot link ${CATALOG}.md, so name the category instead of indexing it`,
+      );
+    }
+    // `skill.body` is frontmatter-stripped, so its indices are not the line numbers a reader
+    // sees when they open the file. Offset by the stripped prefix — a lint message that
+    // points at the wrong line is the same unresolvable-reference defect this rule exists
+    // to catch.
+    const bodyLines = skill.body.split('\n');
+    // Locate the body in the source rather than subtracting line counts: the two differ by
+    // trailing-newline handling, which silently shifts every reported line by one.
+    const start = source.indexOf(skill.body);
+    const offset = start < 0 ? 0 : source.slice(0, start).split('\n').length - 1;
+    for (const [index, line] of bodyLines.entries()) {
+      const hit = line.match(LANE_ID);
+      if (hit && !line.includes(CATALOG)) {
+        warnings.push(
+          `line ${index + 1 + offset} cites lane ${hit[0]} without linking ${CATALOG}.md; an unresolvable index reads as noise`,
+        );
+      }
+    }
+  }
+
+  // `[[snake_case]]` is wiki-link syntax from a private authoring vault. It renders as
+  // literal brackets on GitHub and resolves nowhere for any reader here. Underscores are
+  // what separate it from JS array literals (`[[signer1.address, …]]`), which are common
+  // in workflow snippets and must not trip this.
+  for (const [, link] of skill.body.matchAll(/\[\[([a-z][a-z0-9]*(?:_[a-z0-9]+)+)\]\]/gu)) {
+    errors.push(`\`[[${link}]]\` is a private-vault wiki link; it resolves for no reader here — use a real path or URL`);
+  }
+
+  const related = skill.body.match(RELATED_SECTION);
+  if (related) {
+    const known = knownSkillNames();
+    for (const [, name] of related[1].matchAll(BACKTICKED)) {
+      if (!known.has(name) && name !== skill.name) {
+        // Warning, not error: the gate runs on the PR's own branch, where a sibling skill
+        // that ships in a concurrent PR does not exist yet. Blocking would fail a PR for a
+        // forward reference that resolves on merge.
+        warnings.push(`\`## Related\` links \`${name}\`, which is not a skill on this branch (renamed, removed, or still in an open PR?)`);
+      }
+    }
+  }
+}
+
+let nameCache;
+function knownSkillNames() {
+  // `sources` is an array of roots; passing a bare string iterates its characters and
+  // silently yields zero skills, which would make every check below vacuously pass.
+  nameCache ??= new Set(collectSkills([ROOT]).map((skill) => skill.name));
+  return nameCache;
+}
+
 // Restrict to skills touched by the given file paths (the CI gate passes the
 // PR's changed files, so pre-existing drift in untouched skills never blocks an
 // unrelated change). With no paths, every skill is linted (a full audit).

.github/workflows/lint-skill-entry.yml

@@ -6,7 +6,11 @@ on:
       - 'domains/**'
       - 'tools/**'
       - '.github/scripts/lint-skill-entry.mjs'
+      - '.github/scripts/check-public-refs.mjs'
       - '.github/workflows/lint-skill-entry.yml'
+      - 'test/**'
+      - 'CONTRIBUTING.md'
+      - 'README.md'
 
 permissions:
   contents: read
@@ -64,3 +68,13 @@ jobs:
         run: |
           mapfile -d '' -t files < changed-skill-files.bin
           node .github/scripts/lint-skill-entry.mjs "${files[@]}"
+
+      # Runs on the WHOLE tree, not the changed files: a private-repo reference is a
+      # property of what this repository publishes, and a PR that touches nothing can
+      # still be the moment someone notices one. Unauthenticated by construction — a
+      # token would see private repos and pass them, which is the failure it prevents.
+      - name: Every referenced repository resolves anonymously
+        env:
+          GH_TOKEN: ''
+          GITHUB_TOKEN: ''
+        run: node .github/scripts/check-public-refs.mjs

tools/skill-audit.mjs

@@ -0,0 +1,106 @@
+#!/usr/bin/env node
+//
+// What actually loaded, and what published without its gate.
+//
+// Skill loading is not deterministic. A description is matched by a model, so "did the right
+// skill load" is a question about a probabilistic event, and the only honest way to answer it
+// is to look at what happened rather than at what the description says should happen.
+//
+// Two reports:
+//   loaded    every skill that entered context, by route. Three routes exist and they leave
+//             different traces, which is why counting only one of them reads as silence.
+//   ungated   every outward-facing publish with no gate run before it in the same session.
+//             This one is deterministic and is the reason the script exists: whether a gate
+//             ran before a write is a fact about the transcript, not a judgement.
+//
+// Usage: node tools/skill-audit.mjs <transcript.jsonl> [--json]
+import { createReadStream } from 'node:fs';
+import { createInterface } from 'node:readline';
+
+const [file, ...flags] = process.argv.slice(2);
+if (!file) {
+  console.error('usage: skill-audit.mjs <transcript.jsonl> [--json]');
+  process.exit(2);
+}
+
+const ROUTES = [
+  // Skill tool call — the explicit path.
+  [/"skill"\s*:\s*"([a-z0-9-]+)"/g, 'skill-tool'],
+  // Slash command injected into the turn.
+  [/<command-name>\/?([a-z0-9-]+)<\/command-name>/g, 'slash-command'],
+  // Description match / directory scope: the loader announces where it read the file from.
+  [/Base directory for this skill:[^\n"]*?skills\/([a-z0-9-]+)/g, 'auto-load'],
+];
+
+// An outward-facing write. Deliberately broader than the porcelain: `gh api` with a body
+// field is the path that bypasses `gh pr comment`, and it is the one that got used.
+const PUBLISH = /gh\s+(?:pr|issue)\s+(?:comment|edit|create)\b|gh\s+api\b[^"']*(?:-F|-f|--field|--raw-field)\s+body=/;
+const GATE = /attest-gate\.sh|pr-evidence-gate\.py/;
+
+const loaded = new Map();
+const events = [];
+let line = 0;
+
+const rl = createInterface({ input: createReadStream(file), crlfDelay: Infinity });
+for await (const raw of rl) {
+  line += 1;
+  for (const [re, route] of ROUTES) {
+    re.lastIndex = 0;
+    let m;
+    while ((m = re.exec(raw)) !== null) {
+      const key = `${m[1]} (${route})`;
+      loaded.set(key, (loaded.get(key) ?? 0) + 1);
+    }
+  }
+  const isGate = GATE.test(raw);
+  const isPublish = PUBLISH.test(raw);
+  if (isGate) events.push({ line, kind: 'gate' });
+  // Chained means the gate and the write are one command, so the shell enforces the
+  // ordering. A gate that merely ran EARLIER proves nothing: the verdict can be read after
+  // the write, or not read at all, which is how a blocked artifact reached a public PR in
+  // the session this script was written from.
+  if (isPublish) events.push({ line, kind: 'publish', chained: isGate });
+}
+
+// A publish is gated if a gate invocation appears earlier in the transcript. This is
+// deliberately generous — same session, any distance — because the failure it looks for is
+// "no gate at all", and a stricter window would produce arguments about proximity rather
+// than findings.
+let lastGate = -1;
+const ungated = [];
+const unchained = [];
+for (const e of events) {
+  if (e.kind === 'gate') { lastGate = e.line; continue; }
+  if (lastGate < 0) ungated.push(e.line);
+  if (!e.chained) unchained.push(e.line);
+}
+
+const report = {
+  transcript: file,
+  loaded: Object.fromEntries([...loaded].sort((a, b) => b[1] - a[1])),
+  publishes: events.filter((e) => e.kind === 'publish').length,
+  gateRuns: events.filter((e) => e.kind === 'gate').length,
+  ungatedPublishLines: ungated,
+  unchainedPublishLines: unchained,
+};
+
+if (flags.includes('--json')) {
+  console.log(JSON.stringify(report, null, 2));
+} else {
+  console.log(`skill-audit: ${file}\n`);
+  console.log('loaded:');
+  for (const [k, v] of Object.entries(report.loaded)) console.log(`  ${String(v).padStart(4)}  ${k}`);
+  if (!Object.keys(report.loaded).length) console.log('  (none)');
+  console.log(`\npublishes: ${report.publishes}   gate runs: ${report.gateRuns}`);
+  console.log(`unchained publishes: ${unchained.length} of ${report.publishes}`);
+  if (ungated.length) {
+    console.log(`\nUNGATED (${ungated.length}) — no gate ran at all before these:`);
+    for (const l of ungated.slice(0, 10)) console.log(`  line ${l}`);
+  }
+  if (unchained.length) {
+    console.log(`\nUNCHAINED (${unchained.length}) — a gate ran earlier, but not as the same`);
+    console.log('command, so nothing forced the write to depend on its verdict:');
+    for (const l of unchained.slice(0, 10)) console.log(`  line ${l}`);
+  }
+}
+process.exit(ungated.length || unchained.length ? 1 : 0);