TAKT skill orphan deletion is incompatible with dir-based DirFeatureProcessor

[dyoshikawa]

Deferred from PR #1539 (review findings #2 / #3).

Background

TaktSkill emits flat Markdown files (one .md per skill) under
.takt/facets/{instructions,knowledge,output-contracts}/ rather than a
nested per-skill directory like every other tool. To stay compatible with
the directory-based AiDir abstraction we kept dirName for routing, but
the orphan-deletion machinery in DirFeatureProcessor is fundamentally
dir-shaped:

• SkillsProcessor.loadToolDirsToDelete (src/features/skills/skills-processor.ts:458-486)
  enumerates subdirectories via findFilesByGlobs(..., { type: "dir" }).
  TAKT writes flat files, so this glob always returns [] — TAKT skill
  orphans are never picked up by --delete.
• DirFeatureProcessor.removeOrphanAiDirs / removeAiDirs
  (src/types/dir-feature-processor.ts:164-188) keys orphan diff on
  getDirPath() and deletes whole directories. All TAKT skills sharing a
  facet collapse to the same getDirPath() (e.g.
  .takt/facets/instructions), so a naive fix to (1) would either still
  produce empty diffs or — worse — wipe the entire facet directory
  (including command files written under the same instructions/
  directory).

Net effect today: stale .takt/facets/{instructions,knowledge,output-contracts}/<stem>.md
files are never removed when the source rulesync skill is renamed or
deleted. Not a security issue (no data corruption, no escape from
baseDir), but a real functional gap in --delete mode.

Details

Reviewer findings from PR #1539:

Finding #2 (mid)

File: src/features/skills/takt-skill.ts:115-119,
src/features/skills/skills-processor.ts:458-486

loadToolDirsToDelete searches for subdirectories. TAKT skills are flat
files. Orphans never match → never cleaned up.

Finding #3 (mid)

File: src/features/skills/takt-skill.ts:128-143,
src/types/dir-feature-processor.ts:174-188

All TaktSkill instances under one facet collapse to the same
getDirPath(). If #2 is fixed by reusing the AiDir machinery as-is,
removeAiDirs would removeDirectory(".takt/facets/instructions"),
wiping every skill and every command file written under that dir.

Solution / Next Steps

The two findings have to be solved together — you cannot fix #2 without
also fixing #3, otherwise the bigger blast radius of #3 becomes live.

Possible approaches (pick one):

1. File-based deletion path for TAKT skills. Introduce a per-target
   override hook on SkillsProcessor (or the factory) such that for
   takt:

   • loadToolDirsToDelete globs *.md under all three facet
     directories and constructs TaktSkill.forDeletion(...) per file
     with dirName = stem.
   • Orphan diff and removal operate on file paths, not directories.
     Override removeOrphanAiDirs / removeAiDirs for the takt skill
     case so they never call removeDirectory(...) on the shared facet
     directory.

2. Promote a per-class deletion identity. Add an optional
   getOrphanIdentity() method to AiDir (default: getDirPath()) and
   a corresponding removeForOrphan() (default: removeDirectory).
   TaktSkill overrides both to use the per-file path. Cleaner
   long-term, slightly more invasive.

Either way, add regression tests covering:

• Generate two TAKT skills under the same facet, delete one source, run
  generate --delete → only the orphan file is removed; the other
  skill file and any colliding-but-distinct command file under
  instructions/ remain intact.
• The same scenario for knowledge/ and output-contracts/ facets.
• Global-mode (~/.takt/facets/...) equivalent.

Related

• PR feat: add takt as a target tool (map rulesync features to TAKT facets) #1539 — initial TAKT target implementation (these findings were
  knowingly deferred to keep the original PR scoped).
• Reviewer round 2 of the agent-team loop also flagged this as the only
  remaining mid-tier design gap.