feat(cli): detect superseded skill directories on startup (swamp-club#326)

[stack72]

Summary

• Adds a startup check that detects superseded skill directories (e.g. swamp-extension-model) across all enrolled tools and emits a warning prompting the user to run swamp repo upgrade
• Uses the existing DeferredWarning infrastructure alongside checkForMissingPulledExtensions, so the warning appears after logging initializes and never blocks startup
• Falls back to the primary tool (default: claude) for repos without an explicit tools array in their marker

Test Plan

• deno check — passes
• deno lint — passes
• deno fmt --check — passes
• deno run test — 5877 tests pass
• Compiled binary tested against ~/code/swamp-ubiquity — warning fires with stale dirs, disappears after swamp repo upgrade

🤖 Generated with Claude Code

[github-actions]

Adversarial Review

Critical / High

No critical or high severity findings.

Medium

1. src/cli/mod.ts:868 — SKILL_DIRS lookup uses AiTool type but the map is Record<string, string>, silently skipping "none"

   When marker.tools contains "none" (which should be tools: [] per the type docs, but the union type still includes it for read-side compat), the code hits SKILL_DIRS["none"] which returns undefined, so the if (!dir) continue skips it harmlessly. This is correct behavior, but the intent is implicit — it works by accident of "none" not being a key in the map. Not a bug, but fragile if someone later adds a "none" mapping to SKILL_DIRS.

2. src/cli/mod.ts:863-865 — resolvePrimaryTool fallback when marker?.tools is an empty array

   If marker.tools is [] (the normalized form of tool: "none"), marker?.tools?.length evaluates to 0 (falsy), so the code falls through to resolvePrimaryTool(marker) which returns marker?.tools?.[0] ?? "claude". With tools: [], tools?.[0] is undefined, so it falls back to "claude" — scanning the claude skill dir even though the user explicitly opted out of tool integration. This is a minor inconsistency: an explicitly-no-tools repo would still get superseded-skill warnings for .claude/skills/. In practice this is harmless noise, but it's semantically incorrect.

Low

1. src/domain/repo/repo_service_test.ts:2508-2510 — test for nonexistent directory relies on Deno.stat throwing for each entry rather than the parent being absent

   detectSupersededSkills("/tmp/nonexistent-dir-326") works because every Deno.stat(join("/tmp/nonexistent-dir-326", name)) throws NotFound. This is fine, but the test doesn't verify the behavior it implies (that a missing parent dir returns empty) — it just happens to work because child paths inside a nonexistent parent also don't exist. If detectSupersededSkills were ever changed to Deno.readDir the parent first, this test would break. Very minor — the current implementation makes this a non-issue.

Verdict

PASS — Clean, well-scoped change. The new detectSupersededSkills function is straightforward, the error handling follows the established non-fatal pattern, and the deferred-warning integration is correct. The Set-based dedup across multiple tools is a nice touch. The medium findings are cosmetic edge cases in uncommon configurations, not correctness issues.

[github-actions]

Code Review

Clean, well-scoped PR. The new startup check follows existing patterns (checkForMissingPulledExtensions), the domain logic lives in the right place, and the tests cover the key cases.

Blocking Issues

None.

Suggestions

1. Minor — SUPERSEDED_SKILLS export widening: The type annotation export const SUPERSEDED_SKILLS: readonly string[] widens the inferred tuple type. This works fine since the constant is only iterated, but as const with satisfies readonly string[] would preserve the literal types if they ever become useful downstream. Purely cosmetic — no action needed.

What looks good

• DDD: detectSupersededSkills is a stateless domain service function on repo_service.ts, orchestrated by the CLI layer in mod.ts. Clean separation.
• Import boundary: The CLI imports from ../domain/repo/ paths, which is the established pattern (matches resolve_extension_files.ts, commands/audit.ts, etc.). The libswamp/mod.ts restriction applies only to libswamp internals — no violation here.
• Test coverage: Three tests covering empty dir, detection of superseded dirs (with a non-superseded control), and nonexistent directory. The withTempDir helper already has the Windows EBUSY catch.
• Error handling: Both detectSupersededSkills (per-entry catch) and checkForSupersededSkills (outer catch) are intentionally non-fatal, matching the deferred-warning design.
• LogTape formatting: Warning message uses bare interpolation (logger.warn\${warning.error}``) per project convention.
• Design doc: design/repo.md updated with the feature description.