harden: fail install verification on zero/short skill canonicals (#235)

CHANGELOG.md

@@ -49,6 +49,20 @@ note in the 0.9.0 entry.
   `files` whitelist, so packaged installs shipped no skills; `commands/`
   dropped from the whitelist with the migration).
 
+### Fixed
+
+- **Install verification now fails on a blank/partial skill surface** (#235,
+  from the #230 audit). Since #228 the agent skills are the sole command
+  surface, but `recall_verify_install` and `recall doctor` derived their probes
+  from whatever canonicals existed with no lower bound — a bad npm pack, partial
+  checkout, or an interrupt before the skill copy step yielded zero probes and
+  install printed "All symlinks verified" over an empty surface.
+  `recall_verify_install` now asserts a floor (source ships skill dirs ⇒ the
+  canonical count must match), and `recall doctor` WARNs when the install root
+  exists with zero skill canonicals. `tests/install/npm-pack.test.ts` now
+  asserts all 9 `agent-skills/<name>/SKILL.md` are packed and `commands/` is
+  excluded; the install-sentinel unconditional-step guard now covers `Skills`.
+
 ## [0.9.2] — 2026-07-13 — "hybrid search at scale"
 
 ### Added

lib/install-lib.sh

@@ -1344,6 +1344,31 @@ recall_verify_install() {
     _check_symlink "$CLAUDE_DIR/MEMORY/extract_prompt.md" "$RECALL_SHARED_DIR/extract_prompt.md"
   fi
 
+  # Skill command surface floor (#235). Since #228 the agent skills are the
+  # SOLE command surface, and the loop above derives its probes from whatever
+  # canonicals happen to exist — so a bad npm pack, partial checkout, or an
+  # interrupt before _recall_copy_skill_files yields zero/fewer probes, `missing`
+  # stays empty, and verification would pass green on a blank surface. Assert a
+  # floor instead: if the source tree ships skill dirs, the canonical dir must
+  # hold at least as many, else the copy step didn't complete — fail red. The
+  # glob matches directories only, so agent-skills/AGENTS.md never inflates the
+  # source count (the copy loop excludes it identically).
+  local skills_src="$RECALL_REPO_DIR/agent-skills"
+  if [[ -d "$skills_src" ]]; then
+    local sd src_count=0 canon_count=0
+    for sd in "$skills_src"/*/; do
+      [[ -d "$sd" ]] && src_count=$((src_count + 1))
+    done
+    if [[ -d "$RECALL_SHARED_SKILLS_DIR" ]]; then
+      for sd in "$RECALL_SHARED_SKILLS_DIR"/*/; do
+        [[ -d "$sd" ]] && canon_count=$((canon_count + 1))
+      done
+    fi
+    if [[ $src_count -gt 0 ]] && [[ $canon_count -lt $src_count ]]; then
+      missing+=("agent skill canonicals: found $canon_count of $src_count under $RECALL_SHARED_SKILLS_DIR (command surface incomplete)")
+    fi
+  fi
+
   if [[ ${#missing[@]} -eq 0 ]] && [[ ${#wrong_target[@]} -eq 0 ]]; then
     return 0
   fi

src/commands/doctor.ts

@@ -470,6 +470,24 @@ export interface SymlinkProbe {
   canonical: string;  // path the symlink should point at
 }
 
+// Enumerate the depth-1 skill canonical files (name/file pairs) under the
+// install root's shared/skills dir. Shared by buildSymlinkProbes (to derive
+// per-file symlink probes) and probeSkillSurface (to floor the command surface,
+// #235) so both agree on what counts as a shipped skill file.
+function listSkillCanonicalFiles(root: string): { name: string; file: string }[] {
+  const skillsCanonicalDir = join(root, 'shared', 'skills');
+  if (!existsSync(skillsCanonicalDir)) return [];
+  let skillNames: string[] = [];
+  try { skillNames = readdirSync(skillsCanonicalDir); } catch { return []; }
+  const out: { name: string; file: string }[] = [];
+  for (const name of skillNames) {
+    let files: string[] = [];
+    try { files = readdirSync(join(skillsCanonicalDir, name)); } catch { continue; }
+    for (const file of files) out.push({ name, file });
+  }
+  return out;
+}
+
 function buildSymlinkProbes(): SymlinkProbe[] {
   const home = homedir();
   const root = join(home, '.agents', 'Recall');
@@ -502,27 +520,39 @@ function buildSymlinkProbes(): SymlinkProbe[] {
   // recall_copy_canonical but never reached recall_link); covering it here
   // lets `recall doctor --fix` repair without reinstall. (The former slash
   // commands migrated to these skills — #228.)
-  const skillsCanonicalDir = join(root, 'shared', 'skills');
-  if (existsSync(skillsCanonicalDir)) {
-    let skillNames: string[] = [];
-    try { skillNames = readdirSync(skillsCanonicalDir); } catch { skillNames = []; }
-    for (const name of skillNames) {
-      const skillDir = join(skillsCanonicalDir, name);
-      let files: string[] = [];
-      try { files = readdirSync(skillDir); } catch { continue; }
-      for (const file of files) {
-        probes.push({
-          label: `agent skill: ${name}/${file}`,
-          target: join(home, '.claude', 'skills', name, file),
-          canonical: join(skillDir, file),
-        });
-      }
-    }
+  for (const { name, file } of listSkillCanonicalFiles(root)) {
+    probes.push({
+      label: `agent skill: ${name}/${file}`,
+      target: join(home, '.claude', 'skills', name, file),
+      canonical: join(root, 'shared', 'skills', name, file),
+    });
   }
 
   return probes;
 }
 
+// Skill command surface floor (#235). Since #228 the agent skills are the sole
+// command surface, and buildSymlinkProbes derives probes from whatever
+// canonicals exist — so a missing/empty shared/skills dir yields zero probes and
+// doctor stays green on a blanked surface. Surface it as a WARN when the install
+// root exists but no skill canonicals are present. Exported + path-injected for
+// unit testing, mirroring probeInstallSentinel.
+export function probeSkillSurface(root: string): CheckResult {
+  const label = 'Agent skill command surface';
+  if (!existsSync(root)) {
+    return { label, status: 'INFO', message: 'Recall install root not present — skipping skill surface check' };
+  }
+  const count = listSkillCanonicalFiles(root).length;
+  if (count === 0) {
+    return {
+      label,
+      status: 'WARN',
+      message: 'No agent skill canonicals under shared/skills — the recall-* command surface is blank; re-run ./install.sh to converge',
+    };
+  }
+  return { label, status: 'PASS', message: `${count} agent skill file(s) present` };
+}
+
 function hashFile(path: string): string | null {
   try {
     const buf = readFileSync(path);
@@ -889,6 +919,11 @@ export async function runDoctor(opts: DoctorOptions = {}): Promise<void> {
     }
   }
 
+  // Skill command surface floor (#235): the per-file probes above go silent
+  // when zero skill canonicals exist, so add an explicit WARN for a blanked
+  // command surface — the sole command surface since #228.
+  results.push(probeSkillSurface(join(homedir(), '.agents', 'Recall')));
+
   // MCP env health (issue #28): the recall-memory registration must carry
   // env.RECALL_DB_PATH matching the resolved DB path, or the MCP server can
   // diverge from the CLI after the next Claude restart. Same repair contract as

tests/commands/doctor-skill-surface.test.ts

@@ -0,0 +1,42 @@
+// Unit tests for the skill-surface floor probe used by `recall doctor` (#235).
+// probeSkillSurface() is pure with respect to its arg (the install root), so we
+// drive it directly against a temp root rather than invoking runDoctor
+// end-to-end — same approach as doctor-install-sentinel.test.ts.
+
+import { afterEach, beforeEach, describe, expect, test } from 'bun:test';
+import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'fs';
+import { tmpdir } from 'os';
+import { join } from 'path';
+import { probeSkillSurface } from '../../src/commands/doctor';
+
+let root: string;
+
+beforeEach(() => {
+  root = mkdtempSync(join(tmpdir(), 'recall-doctor-skills-'));
+});
+
+afterEach(() => {
+  if (existsSync(root)) rmSync(root, { recursive: true, force: true });
+});
+
+describe('probeSkillSurface', () => {
+  test('install root absent → INFO (nothing to check)', () => {
+    const r = probeSkillSurface(join(root, 'does-not-exist'));
+    expect(r.status).toBe('INFO');
+  });
+
+  test('root present but zero skill canonicals → WARN (blank command surface)', () => {
+    const r = probeSkillSurface(root);
+    expect(r.status).toBe('WARN');
+    expect(r.message).toContain('command surface');
+  });
+
+  test('root with a skill canonical → PASS', () => {
+    const skillDir = join(root, 'shared', 'skills', 'recall-scout');
+    mkdirSync(skillDir, { recursive: true });
+    writeFileSync(join(skillDir, 'SKILL.md'), '# scout\n');
+    const r = probeSkillSurface(root);
+    expect(r.status).toBe('PASS');
+    expect(r.message).toContain('1 agent skill file');
+  });
+});

tests/install/install-sentinel.test.ts

@@ -165,6 +165,22 @@ describe('install.sh completion sentinel (#27)', () => {
     expect(r.stderr).toContain('re-run ./install.sh'); // recovery via log_error → stderr
   });
 
+  // ── Skill command surface floor (#235) ──────────────────────────────────────
+  //
+  // Since #228 the agent skills are the SOLE command surface. recall_verify_install
+  // derives its skill probes from whatever canonicals exist, so before this floor a
+  // partial/absent copy step (bad pack, partial checkout, interrupt) left `missing`
+  // empty and verification passed green on a blank surface. The fakeRepo fixture
+  // ships one source skill (agent-skills/recall-scout/SKILL.md) but no canonical was
+  // copied here, so the floor must fail red.
+  test('verify fails red when source ships skills but zero canonicals exist', () => {
+    const r = runDriver(['recall_verify_install']);
+    expect(r.status).not.toBe(0);
+    // Pin the distinctive floor message so an unrelated missing symlink can't
+    // green this test for the wrong reason.
+    expect(r.stderr).toContain('agent skill canonicals');
+  });
+
   // ── DRY: one canonical recovery story ───────────────────────────────────────
 
   test('recovery wording has a single source of truth (recall_print_recovery)', () => {
@@ -188,7 +204,7 @@ describe('install.sh completion sentinel (#27)', () => {
     const src = readFileSync(INSTALL_SH, 'utf-8');
     for (const step of [
       'Backup', 'Migrate', 'Installing', 'Building', 'Linking', 'Database',
-      'MCP', 'Hooks', 'Guide', 'Commands', 'CLAUDE.md',
+      'MCP', 'Hooks', 'Guide', 'Commands', 'Skills', 'CLAUDE.md',
     ]) {
       expect(src).toContain(`_step "${step}"`);
     }

tests/install/npm-pack.test.ts

@@ -66,6 +66,20 @@ describe('npm package: files whitelist', () => {
     expect(has('FOR_PI.md')).toBe(true);
   });
 
+  // #235: the agent skills are the sole command surface since #228, so the pack
+  // must ship every one — a partial pack (the pre-#230 failure mode) silently
+  // blanks the surface. Enumerate all 9 explicitly rather than trusting the
+  // agent-skills/ prefix alone (which passes with just 1 of 9 present).
+  test('bundles all 9 agent-skills/<name>/SKILL.md', () => {
+    const skills = [
+      'recall-add', 'recall-doctor', 'recall-dump', 'recall-loa', 'recall-recent',
+      'recall-scout', 'recall-search', 'recall-stats', 'recall-update',
+    ];
+    for (const name of skills) {
+      expect(has(`agent-skills/${name}/SKILL.md`)).toBe(true);
+    }
+  });
+
   test('excludes dev / local bloat', () => {
     for (const bad of [
       'tests/',
@@ -75,6 +89,7 @@ describe('npm package: files whitelist', () => {
       'scripts/',
       '.agents/',
       '.claude/',
+      'commands/', // #235: the migrated slash-command surface must not ship
     ]) {
       expect(hasUnder(bad)).toBe(false);
     }