Skip to content

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

Merged
edheltzel merged 2 commits into
mainfrom
worktree-verify-floor-235
Jul 13, 2026
Merged

harden: fail install verification on zero/short skill canonicals (#235)#239
edheltzel merged 2 commits into
mainfrom
worktree-verify-floor-235

Conversation

@edheltzel

Copy link
Copy Markdown
Owner

Closes #235 (P1 from the #230 post-merge audit — reviewer report finding 1, the strongest-corroborated item in the set).

Problem

Since #228, the agent skills are the sole command surface. But recall_verify_install (lib/install-lib.sh) and recall doctor's buildSymlinkProbes (src/commands/doctor.ts) derived their expected-symlink list from whatever canonicals exist under ~/.agents/Recall/shared/skills/ with no lower bound. A bad npm pack, partial checkout, or an interrupt before _recall_copy_skill_files produces zero probes, missing stays empty, and install prints "All symlinks verified" over a blank surface. Pre-#230 the commands loop backstopped this; that backstop is gone.

Fix (surgical — a guard + tests, no refactor)

  1. recall_verify_install asserts a floor: if $RECALL_REPO_DIR/agent-skills ships skill dirs, the canonical count under $RECALL_SHARED_SKILLS_DIR must match — else push a synthetic entry into missing so verification fails red. (canon < src, so partial packs are caught too.)
  2. doctor: extracted listSkillCanonicalFiles() (shared with buildSymlinkProbes, DRY) + new exported path-injected probeSkillSurface()WARN when the install root exists with zero skill canonicals; wired into runDoctor.
  3. npm-pack.test.ts: assert all 9 agent-skills/<name>/SKILL.md are packed; assert commands/ is excluded.
  4. install-sentinel.test.ts: Skills added to the unconditional-step guard; regression test for the empty-canonical floor.
  5. doctor-skill-surface.test.ts: focused unit test for probeSkillSurface.
  6. CHANGELOG [Unreleased] → Fixed.

Verification

  • Red→green demonstrated for the floor guard: with the install-lib.sh hunk stashed, verify fails red when source ships skills but zero canonicals exist fails (recall_verify_install returns 0 — silent pass); with the fix restored it passes. Test pins the distinctive agent skill canonicals message so an unrelated missing symlink can't green it for the wrong reason.
  • bash -n lib/install-lib.sh clean · bun run lint clean.
  • Touched test files: 23 pass / 0 fail.
  • Full bun test: 1235 pass. The 10 failures are pre-existing and unrelated — recall_configure_opencode_mcp / recall_configure_pi_mcp MCP-config tests (files not touched here); confirmed identical failures on the baseline with this branch's changes stashed.

Refs #230, #228. Do not merge — Themis gates.

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 produced zero probes
and install printed "All symlinks verified" over a blank surface.

- lib/install-lib.sh: recall_verify_install asserts a floor — if the
  source tree ships skill dirs, the canonical count must match, else push
  a synthetic `missing` entry so verification fails red.
- src/commands/doctor.ts: extract listSkillCanonicalFiles() (shared with
  buildSymlinkProbes) and add exported probeSkillSurface() → WARN when the
  install root exists with zero skill canonicals; wired into runDoctor.
- tests/install/npm-pack.test.ts: assert all 9 agent-skills/<name>/SKILL.md
  are packed and commands/ is excluded.
- tests/install/install-sentinel.test.ts: cover the Skills step in the
  unconditional-step guard; regression test for the empty-canonical floor.
- tests/commands/doctor-skill-surface.test.ts: probeSkillSurface unit test.
- CHANGELOG: [Unreleased] Fixed entry.

Refs #235, #230, #228.
@edheltzel
edheltzel merged commit 58cb1d1 into main Jul 13, 2026
2 checks passed
@edheltzel
edheltzel deleted the worktree-verify-floor-235 branch July 13, 2026 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

harden: install verification passes green with zero skill canonicals — silent-pass on the sole command surface (P1 from #230 audit)

2 participants