feat(01KW5WPD): computeSkillEffectiveness resolves retired skills correctly (re-rooted path + injected IO seam) so the retro analyst stops proposing to retire already-deleted or moved skills (#423, #403)

[jackmcintyre]

What changed

computeSkillEffectiveness resolves retired skills correctly (re-rooted path + injected IO seam) so the retro analyst stops proposing to retire already-deleted or moved skills (#423, #403)

This change delivers the following acceptance criteria:

• AC1: Given a per_skill entry whose skill is no longer installed, When computeSkillEffectiveness resolves installed status, Then it re-roots the absolute, invoke-time-captured skill_path against targetRepoRoot and excludes (or flags installed:false) only skills genuinely absent - a live skill whose path was renamed or moved is NOT misclassified as retired.
• AC2: Given the helper is deliberately pure with an injected IO seam (raw fs is forbidden; tests inject a fake ROOT), When the installed-check runs, Then it goes through that injected IO seam so the helper stays pure and the live-skills assertion remains testable.
• AC3: Given a skill still installed on disk, When computeSkillEffectiveness runs, Then its invoke_count and effectiveness_ratio are unaffected by the filter.
• AC4: Given a retired skill whose historical telemetry sits inside the 50-story window alongside a skill_path that matches neither candidate dir, When the helper runs, Then the retired skill does not appear as a 0-ratio active skill, and the ambiguous other-plugin/relative path defaults to treated-as-installed so it is never wrongly dropped.

Why

As a retro-analyst, I want the skill-effectiveness signal to reflect only skills still installed on disk, resolved correctly against the repo, so that I stop drafting skill-retire proposals for commands that were already removed (flow:author, flow:judge, flow:status, flow:board, crew:board, flow:accept-proposal) WITHOUT wrongly hiding a live skill whose path moved.

Closes #423. Closes #403 (duplicate of #423).

How it works

• Injected IO seam: Added existsImpl?: (absolutePath: string) => boolean and pluginRoot?: string to ComputeSkillEffectivenessOptions. When existsImpl is absent the helper is fully backward-compatible (no filtering). Tests inject a fake that never touches the real filesystem (AC2).
• Re-rooted path resolution: Added extractSkillCommand (regex /[/\\]skills[/\\]([^/\\]+)[/\\]SKILL\.md$/i) to pull the command name from an absolute, invoke-time-captured skill_path. Added checkInstalled that probes two candidate dirs: (1) {targetRepoRoot}/plugins/flow/skills/{cmd}/SKILL.md (dev/monorepo) and (2) {pluginRoot}/skills/{cmd}/SKILL.md (installed-plugin). A live skill whose path moved is NOT misclassified as retired (AC1, AC3).
• Ambiguous-path default: A skill_path matching neither /skills/{cmd}/SKILL.md pattern defaults to installed = true — never wrongly dropped (AC4).
• gatherRetroInputs wired up: Now passes existsImpl: existsSync and pluginRoot: getPluginRoot() (wrapped in try/catch for edge cases), activating the check in production.
• retro-skill-effectiveness.test.ts updated: Existing tests were using retired skills (flow:author, flow:judge); replaced with live ones (flow:run) so they continue to pass after existsImpl is active.
• dist rebuilt: pnpm build re-run; dist/index.js and dist/cli.js committed in the same change (CI fails on drift).

How to check it yourself

Step 1 — Run the targeted test file. From the repo root: cd plugins/flow/mcp-server && pnpm exec vitest run src/tools/tests/compute-skill-effectiveness.test.ts. Expect all tests to pass, including the new describe block "computeSkillEffectiveness — retired-skill installed-check (native:01KW5WPDPJY5DK6JV810307E0J)" with 4 tests covering AC1-AC4.

Step 2 — Run the full test suite. From plugins/flow/mcp-server: pnpm exec vitest run. Expect all approximately 3900 tests to pass with no regressions.

Step 3 — Verify retro-skill-effectiveness tests. From plugins/flow/mcp-server: pnpm exec vitest run src/tools/tests/retro-skill-effectiveness.test.ts. Expect both tests to pass (they now use flow:run instead of the retired flow:author and flow:judge commands).

Step 4 — Verify the production fix. Reinstall the plugin from this branch, open a project with retro telemetry containing invocations of removed commands (e.g. flow:author), then run /flow:retro. Confirm the retro output no longer proposes retiring already-deleted skills like flow:author or flow:judge.

Risk and blast radius

Risk tier: medium

The change is scoped to compute-skill-effectiveness.ts, its test file, gather-retro-inputs.ts, and retro-skill-effectiveness.test.ts. The seam is opt-in (existsImpl absent = no filtering), so all existing callers that do not pass existsImpl are fully backward-compatible. Review the diff to judge its actual blast radius — including any effect on shared state, data schemas, or authentication paths — before approving.

What is explicitly not covered: reviewer verification of this summary's accuracy is handled by a separate companion story.

Evidence

The pre-pull-request build-and-test gate passed before this pull request was opened. No pull request can be opened by the automated flow unless both pnpm build and pnpm test exit 0 in the developer's working directory.

Per-criterion covering checks:

• AC1 → plugins/flow/mcp-server/src/tools/__tests__/compute-skill-effectiveness.test.ts (automated test)
• AC2 → plugins/flow/mcp-server/src/tools/__tests__/compute-skill-effectiveness.test.ts (automated test)
• AC3 → plugins/flow/mcp-server/src/tools/__tests__/compute-skill-effectiveness.test.ts (automated test)
• AC4 → plugins/flow/mcp-server/src/tools/__tests__/compute-skill-effectiveness.test.ts (automated test)

Story ref: native:01KW5WPDPJY5DK6JV810307E0J