test(pi): validate package integrity in CI (PF-3852)

.github/workflows/pi-ci.yml

@@ -0,0 +1,125 @@
+name: Pi CI
+
+# No `branches:` filter on pull_request, unlike the other CI here: that filter matches the
+# *base*, so a stacked PR gets no validation until it retargets main — including the PR that
+# adds these jobs. Validation should run wherever pi/ changes.
+on:
+  pull_request:
+    paths:
+      - ".github/workflows/pi-ci.yml"
+      - "pi/**"
+  push:
+    branches: [main]
+    paths:
+      - ".github/workflows/pi-ci.yml"
+      - "pi/**"
+
+permissions:
+  contents: read
+
+jobs:
+  validate:
+    runs-on: ubuntu-latest
+
+    defaults:
+      run:
+        working-directory: pi
+
+    steps:
+      - uses: actions/checkout@v6
+        with:
+          persist-credentials: false
+
+      - uses: actions/setup-node@v5
+        with:
+          node-version: "24"
+
+      # Static integrity: manifest, skill frontmatter, skill-relative references, and
+      # that every tool the skills teach is actually registered in directTools.
+      - name: Validate package
+        run: node test/validate.mjs
+
+      # The package ships no code, so a dropped `files` entry is the whole failure mode:
+      # the tarball still publishes and installs, just with skills silently missing.
+      # pi-publish.yml repeats this at publish time; here it runs on the PR that breaks it.
+      - name: Tarball carries every shipped component
+        run: |
+          npm pack --dry-run --json > /tmp/pack.json
+          node -e "
+            const [pack] = require('/tmp/pack.json');
+            const paths = new Set(pack.files.map((f) => f.path));
+            const required = [
+              'mcp.json',
+              'README.md',
+              'rules/security.md',
+              'skills/tinyfish-web/SKILL.md',
+              'skills/tinyfish-research/SKILL.md',
+              'skills/tinyfish-automation/SKILL.md',
+              'skills/tinyfish-authenticated/SKILL.md',
+              'skills/tinyfish-browser/SKILL.md',
+            ];
+            const missing = required.filter((p) => !paths.has(p));
+            if (missing.length) {
+              console.error('Missing from tarball:\n  ' + missing.join('\n  '));
+              process.exit(1);
+            }
+            const leaked = [...paths].filter((p) => p === 'test' || p.startsWith('test/'));
+            if (leaked.length) {
+              console.error('test/ must not ship in the tarball: ' + leaked.join(', '));
+              process.exit(1);
+            }
+            console.log('Tarball OK: ' + pack.files.length + ' files');
+          "
+
+  install:
+    name: Pi loads the package
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v6
+        with:
+          persist-credentials: false
+
+      - uses: actions/setup-node@v5
+        with:
+          node-version: "24"
+
+      - name: Install pi
+        run: npm i -g @earendil-works/pi-coding-agent
+
+      # Proves the real thing: pi accepts the package, and the adapter derives the
+      # server name and directTools the skills are written against. Both are installed
+      # the way a user installs them. No model, no network calls to us, no credentials.
+      - name: Register the package and assert what the adapter resolves
+        run: |
+          set -euo pipefail
+          export PI_CODING_AGENT_DIR="$(mktemp -d)"
+          pi install ./pi
+          pi install npm:pi-mcp-adapter
+          pi list
+          node -e "
+            const path = require('node:path');
+            const loader = path.join(
+              process.env.PI_CODING_AGENT_DIR,
+              'npm/node_modules/pi-mcp-adapter/dist/package-mcp-loader.js',
+            );
+            const { loadPackageMcpConfigs } = require(loader);
+            const cfg = loadPackageMcpConfigs(process.cwd());
+            const names = Object.keys(cfg.mcpServers);
+            if (!names.includes('tiny-fish_pi__tinyfish')) {
+              console.error('Expected server tiny-fish_pi__tinyfish, adapter resolved: ' + JSON.stringify(names));
+              process.exit(1);
+            }
+            // Compare the exact set: a truncated list would still be 'non-empty'.
+            const expected = require(path.join(process.cwd(), 'pi/mcp.json'))
+              .mcpServers.tinyfish.directTools;
+            const direct = cfg.mcpServers['tiny-fish_pi__tinyfish'].directTools;
+            const norm = (a) => JSON.stringify([...(a ?? [])].sort());
+            if (norm(direct) !== norm(expected)) {
+              console.error('directTools changed in transit through the adapter');
+              console.error('  mcp.json: ' + norm(expected));
+              console.error('  resolved: ' + norm(direct));
+              process.exit(1);
+            }
+            console.log('Adapter resolved ' + names[0] + ' with all ' + expected.length + ' direct tools intact');
+          "

pi/test/validate.mjs

@@ -0,0 +1,239 @@
+// Static integrity checks for the @tiny-fish/pi package. Run: node test/validate.mjs
+import { readFileSync, existsSync, readdirSync, statSync } from 'node:fs';
+import { join, dirname, resolve } from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..');
+const failures = [];
+const checks = [];
+
+const fail = (name, msg) => failures.push(`${name}: ${msg}`);
+const ok = (name) => checks.push(name);
+const read = (p) => readFileSync(join(ROOT, p), 'utf8');
+const readJson = (p) => JSON.parse(read(p));
+
+const pkg = readJson('package.json');
+const mcp = readJson('mcp.json');
+
+// Pi's skill name rules, from its docs/skills.md: 1-64 chars, lowercase alnum and
+// single hyphens, no leading/trailing hyphen. A violating name still loads but warns.
+const NAME_RE = /^[a-z0-9]+(-[a-z0-9]+)*$/;
+const MAX_DESC = 1024;
+
+// Deliberately not a YAML parser. It recognises plain scalars and rejects everything else,
+// so a form it cannot measure correctly fails loudly instead of being mis-measured: a block
+// scalar (`description: |`) would otherwise read as the literal "|" and sail past the length
+// cap that pi's real YAML parser would enforce against the full text.
+function parseFrontmatter(text) {
+  const m = text.match(/^---\r?\n([\s\S]*?)\r?\n---/);
+  if (!m) return { error: 'no YAML frontmatter' };
+  const out = {};
+  const lines = m[1].split(/\r?\n/);
+  for (let i = 0; i < lines.length; i++) {
+    const line = lines[i];
+    if (!line.trim() || line.startsWith('#')) continue;
+    const kv = line.match(/^([A-Za-z][A-Za-z0-9_-]*):(.*)$/);
+    if (!kv) {
+      if (/^\s/.test(line)) return { error: `frontmatter uses multi-line YAML at "${line.trim().slice(0, 40)}" — this package keeps name and description as single-line plain scalars` };
+      return { error: `unparsable frontmatter line: ${line.slice(0, 60)}` };
+    }
+    const raw = kv[2].trim();
+    if (/^[|>]/.test(raw)) {
+      return { error: `"${kv[1]}" uses a block scalar (${raw[0]}); keep it a single-line plain scalar so its length can be checked` };
+    }
+    if (raw.startsWith('&') || raw.startsWith('*')) {
+      return { error: `"${kv[1]}" uses a YAML anchor/alias; keep it a plain scalar` };
+    }
+    out[kv[1]] = raw.replace(/^["']|["']$/g, '');
+  }
+  return out;
+}
+
+function skillDirs() {
+  const base = join(ROOT, 'skills');
+  if (!existsSync(base)) return null;
+  return readdirSync(base).filter((d) => statSync(join(base, d)).isDirectory());
+}
+
+// --- manifest ------------------------------------------------------------
+{
+  const name = 'manifest';
+  if (pkg.name !== '@tiny-fish/pi') fail(name, `package name is ${pkg.name}`);
+  if (!pkg.keywords?.includes('pi-package')) {
+    fail(name, 'keywords must include "pi-package" — it is the entire gallery discovery mechanism');
+  }
+  if (!pkg.pi?.mcp) fail(name, 'pi.mcp is not declared');
+  if (!pkg.pi?.skills?.length) fail(name, 'pi.skills is not declared');
+  for (const p of [pkg.pi?.mcp, ...(pkg.pi?.skills ?? [])].filter(Boolean)) {
+    if (!existsSync(join(ROOT, p))) fail(name, `pi manifest points at missing path ${p}`);
+  }
+  ok(name);
+}
+
+// --- mcp.json ------------------------------------------------------------
+{
+  const name = 'mcp';
+  const server = mcp.mcpServers?.tinyfish;
+  if (!server) {
+    fail(name, 'mcpServers.tinyfish is missing');
+  } else {
+    if ('type' in server) {
+      fail(name, 'ServerEntry has no "type" field — the adapter ignores it; a bare url means HTTP');
+    }
+    // The adapter interpolates /\$\{(\w+)\}/ only; ${VAR:-} ships as a literal header value.
+    for (const [h, v] of Object.entries(server.headers ?? {})) {
+      if (/\$\{[^}]*[^\w}][^}]*\}/.test(v)) {
+        fail(name, `header ${h} uses an interpolation form the adapter cannot parse: ${v}`);
+      }
+    }
+    // Without directTools the adapter registers nothing top-level; everything hides
+    // behind the mcp proxy and the skills' tool names stop resolving.
+    if (!Array.isArray(server.directTools) || server.directTools.length === 0) {
+      fail(name, 'directTools must be a non-empty array or no tools register top-level');
+    } else if (new Set(server.directTools).size !== server.directTools.length) {
+      fail(name, 'directTools contains duplicates');
+    }
+    ok(name);
+  }
+}
+
+// --- skills --------------------------------------------------------------
+const dirs = skillDirs();
+if (!dirs) {
+  fail('skills', 'skills/ directory is missing');
+} else {
+  const seen = new Map();
+  for (const dir of dirs) {
+    const rel = `skills/${dir}/SKILL.md`;
+    if (!existsSync(join(ROOT, rel))) {
+      fail('skills', `${dir}/ has no SKILL.md`);
+      continue;
+    }
+    const text = read(rel);
+    const fm = parseFrontmatter(text);
+    if (fm.error) {
+      fail(rel, fm.error);
+      continue;
+    }
+    if (!fm.name) fail(rel, 'frontmatter has no name');
+    else {
+      if (!NAME_RE.test(fm.name) || fm.name.length > 64) {
+        fail(rel, `name "${fm.name}" violates pi's rules (lowercase alnum, single hyphens, <= 64)`);
+      }
+      if (fm.name !== dir) fail(rel, `name "${fm.name}" does not match its directory "${dir}"`);
+      if (seen.has(fm.name)) fail(rel, `duplicate skill name "${fm.name}", also in ${seen.get(fm.name)}`);
+      seen.set(fm.name, rel);
+    }
+    // Pi does not load a skill whose description is missing — it vanishes silently.
+    if (!fm.description) fail(rel, 'frontmatter has no description; pi will not load the skill');
+    else if (fm.description.length > MAX_DESC) {
+      fail(rel, `description is ${fm.description.length} chars, over pi's ${MAX_DESC} cap`);
+    }
+
+    // Skill-relative paths, the bug class that shipped a dangling rules/security.md pointer.
+    // Both spellings pi documents: a backticked path, and a Markdown link destination.
+    const refs = [
+      ...[...text.matchAll(/`([^`\s]+\.md)`/g)].map((m) => m[1]),
+      ...[...text.matchAll(/\]\(([^)\s]+\.md)(?:\s[^)]*)?\)/g)].map((m) => m[1]),
+    ];
+    for (const ref of refs) {
+      if (/^(https?:|#|\/)/.test(ref)) continue;
+      const target = resolve(ROOT, 'skills', dir, ref);
+      if (!existsSync(target)) fail(rel, `references missing file ${ref}`);
+    }
+
+    // The adapter derives the prefix from the install, so a hardcoded proxy name is correct
+    // for package installs and wrong for CLI ones. Skills must use the name search returns.
+    // A `<placeholder>` is the correct thing to write here; a literal derived name is not.
+    for (const m of text.matchAll(/mcp\(\{\s*tool:\s*["']([^"']+)["']/g)) {
+      if (!m[1].includes('<') && /^(tiny-?fish|.*__)/.test(m[1])) {
+        fail(rel, `hardcodes a proxy tool name (${m[1]}); call the name mcp({ search }) returned`);
+      }
+    }
+  }
+  if (seen.size) ok(`skills (${seen.size} valid)`);
+}
+
+// --- skills vs directTools ----------------------------------------------
+// A tool the skills teach but that is not registered forces a proxy call the
+// capability skills never explain.
+if (dirs) {
+  const name = 'skills-vs-directTools';
+  const registered = new Set(mcp.mcpServers?.tinyfish?.directTools ?? []);
+  // Every TinyFish tool name the skills could plausibly name. An incomplete list silently
+  // narrows the check, which is how `list_browser_sessions` slipped through review once —
+  // the guard below keeps it from falling behind mcp.json again.
+  const KNOWN = [
+    'search', 'fetch_content', 'run_web_automation', 'run_web_automation_async',
+    'get_run', 'cancel_run', 'list_runs', 'poll_status', 'discover_run', 'get_steps',
+    'guide_next_step', 'batch_status', 'batch_cancel',
+    'create_browser_session', 'list_browser_sessions', 'close_browser_session',
+    'get_wallet', 'get_search_usage', 'list_fetch_usage',
+  ];
+  const unknownRegistered = [...registered].filter((t) => !KNOWN.includes(t));
+  if (unknownRegistered.length) {
+    fail(name, `directTools names not in this file's KNOWN list — add them: ${unknownRegistered.join(', ')}`);
+  }
+  const taught = new Set();
+  for (const dir of dirs) {
+    const files = [`skills/${dir}/SKILL.md`];
+    const refs = join(ROOT, 'skills', dir, 'references');
+    if (existsSync(refs)) for (const f of readdirSync(refs)) files.push(`skills/${dir}/references/${f}`);
+    for (const f of files) {
+      if (!existsSync(join(ROOT, f))) continue;
+      const text = read(f);
+      for (const t of KNOWN) if (text.includes(`\`${t}\``)) taught.add(t);
+    }
+  }
+  const missing = [...taught].filter((t) => !registered.has(t)).sort();
+  if (missing.length) fail(name, `taught by the skills but not in directTools: ${missing.join(', ')}`);
+  else ok(`${name} (${taught.size} taught, all registered)`);
+}
+
+// --- README claims -------------------------------------------------------
+// The stated tool count has drifted twice already: once when directTools grew, once in review.
+{
+  const name = 'readme';
+  const readme = existsSync(join(ROOT, 'README.md')) ? read('README.md') : null;
+  const count = mcp.mcpServers?.tinyfish?.directTools?.length;
+  if (!readme) {
+    fail(name, 'README.md is missing');
+  } else if (count) {
+    const WORDS = ['zero', 'one', 'two', 'three', 'four', 'five', 'six', 'seven', 'eight',
+      'nine', 'ten', 'eleven', 'twelve', 'thirteen', 'fourteen', 'fifteen', 'sixteen',
+      'seventeen', 'eighteen', 'nineteen', 'twenty'];
+    const m = readme.match(/registers\s+([a-z]+|\d+)\s+tools directly/i);
+    if (!m) {
+      fail(name, 'could not find the "registers N tools directly" claim to check');
+    } else {
+      const stated = /^\d+$/.test(m[1]) ? Number(m[1]) : WORDS.indexOf(m[1].toLowerCase());
+      if (stated !== count) {
+        fail(name, `README says it registers ${m[1]} tools but directTools has ${count}`);
+      }
+    }
+  }
+  ok(name);
+}
+
+// --- files ---------------------------------------------------------------
+{
+  const name = 'files';
+  for (const entry of ['skills', 'rules', 'mcp.json', 'README.md', 'LICENSE']) {
+    if (!pkg.files?.includes(entry)) fail(name, `"${entry}" is missing from package.json files`);
+  }
+  for (const entry of pkg.files ?? []) {
+    if (entry === 'test' || entry.startsWith('test/')) {
+      fail(name, `"${entry}" would ship the test directory in the tarball`);
+    }
+  }
+  ok(name);
+}
+
+// --- report --------------------------------------------------------------
+for (const c of checks) console.log(`  ok  ${c}`);
+if (failures.length) {
+  console.error(`\n${failures.length} failure(s):`);
+  for (const f of failures) console.error(`  FAIL  ${f}`);
+  process.exit(1);
+}
+console.log(`\nAll checks passed (${checks.length}).`);