feat: Wave 1 — search-first skill, confidence thresholds, version manifest

[dean0x]

Summary

Implements the three independent Wave 1 features from the post-v1.0 roadmap (#137):

• Search-first skill (Search-First Skill: Research Before Building #111) — New skill enforcing a 4-phase research loop (Need Analysis → Search → Evaluate → Decide) before writing custom utility code. Added to core-skills plugin and ambient-router BUILD primary skills. 31 skills total (was 30).

• Reviewer confidence thresholds (Reviewer Confidence Thresholds: Reduce Noise in Code Review Output #113) — Each review finding now includes a visible confidence score (0-100%). Only ≥80% findings appear in main report sections; 60-79% go to a capped Suggestions section. Adds consolidation rules (group similar issues, skip stylistic preferences, skip unchanged code unless CRITICAL). Fixes synthesizer review glob bug that matched zero reviewer files.

• Version manifest (Version manifest for upgrade tracking #91) — Tracks installed version, plugins, scope, and feature flags in manifest.json. Enables upgrade detection during devflow init and shows install status in devflow list. Partial installs merge plugins instead of overwriting. 17 new tests.

Closes #111, closes #113, closes #91

Test plan

• npm run build passes — 31 skills, 17 plugins
• npm test passes — 208 tests (was 191, +17 manifest tests)
• search-first skill distributed to plugins/devflow-core-skills/skills/
• Zero regressions on existing tests
• Manual: run node dist/cli.js list to verify install status display
• Manual: run /code-review on a test branch to verify confidence percentages and suggestions section

[dean0x]

Manifest Validation Gap (BLOCKING)

The readManifest function validates only 3 fields (version, plugins, scope) but casts the full object via as ManifestData. Missing validation for features object, installedAt, and updatedAt.

Risk: Partial/malformed manifests pass validation but cause runtime crashes in list.ts:41 when accessing manifest.features.teams.

Fix: Add full field validation:

if (
  \!data.version ||
  \!Array.isArray(data.plugins) ||
  \!data.scope ||
  typeof data.features \!== "object" ||
  data.features === null ||
  typeof data.features.teams \!== "boolean" ||
  typeof data.features.ambient \!== "boolean" ||
  typeof data.features.memory \!== "boolean" ||
  \!data.installedAt ||
  \!data.updatedAt
) {
  return null;
}

Or use Zod schema (project convention per CLAUDE.md).

Confidence: 85% | Consolidated: Reported in 5 reviews (security, architecture, consistency, tests, regression)

Claude Code Review

[dean0x]

Missing Error Handling for writeManifest (BLOCKING)

The writeManifest() call is not wrapped in try/catch. If the write fails (permissions, disk full), the entire init command crashes after plugins are already installed.

Impact: User sees CLI error despite successful installation, confusion about install state.

Fix: Wrap in try/catch with graceful degradation:

try {
  const installedPluginNames = pluginsToInstall.map(pl => pl.name);
  const now = new Date().toISOString();
  const manifestData = {
    version,
    plugins: existingManifest && options.plugin
      ? mergeManifestPlugins(existingManifest.plugins, installedPluginNames)
      : installedPluginNames,
    scope,
    features: { teams: teamsEnabled, ambient: ambientEnabled, memory: memoryEnabled },
    installedAt: existingManifest?.installedAt ?? now,
    updatedAt: now,
  };
  await writeManifest(devflowDir, manifestData);
} catch (error) {
  p.log.warn(`Could not write manifest: ${error instanceof Error ? error.message : error}`);
}

Manifest is a non-critical feature (upgrade tracking only), so write failure should not crash the installer.

Confidence: 82% | Categories: Architecture, Regression

Claude Code Review

[dean0x]

Code Review Summary — Blocking & Should-Fix Issues

Blocking Issues (2 inline comments created above)

✅ Manifest Validation Gap (src/cli/utils/manifest.ts:27-30) - FIXED via inline comment

• Validates only 3 fields but casts full object; can crash when accessing undefined features properties
• Confidence: 85% | Consolidated from 5 reviews

✅ Missing Error Handling (src/cli/commands/init.ts:624) - FIXED via inline comment

• writeManifest() not wrapped in try/catch; write failure crashes after successful install
• Confidence: 82% | Consolidated from 2 reviews

Documentation Issues (HIGH - Not in PR diff)

⚠️ README.md outdated — These files are not modified in this PR but need updates:

• Line 27: Says "30 quality skills" but CLAUDE.md updated to 31
  • Fix: Change to "31 quality skills" and note "9 auto-activating core" (was 8)
• Lines 109-122: Auto-activating skills table missing search-first entry
  • The skill was added to devflow-core-skills plugin.json but README table was not updated
  • Confidence: 95% | Category: HIGH

Action: Please update README.md in a follow-up commit to prevent documentation-reality drift.

---

Should-Fix Issues

📊 Test Coverage Gaps (HIGH Risk)

Missing edge case tests (tests/manifest.test.ts / src/cli/utils/manifest.ts)

• readManifest missing test for partial manifest (missing features/installedAt/updatedAt)
• compareSemver v-prefix path untested (regex has v? but no test exercises it)
• Suggested fixes in review reports (tests.md)

Zero test coverage for integration logic (src/cli/commands/list.ts, src/cli/commands/init.ts)

• list.ts manifest reading + scope resolution + feature string logic untested
• init.ts conditional plugin merge logic untested (especially risky: existingManifest && options.plugin)
• Recommended: Extract pure functions (formatFeatures, resolvePluginList) for testability

Confidence: 85-88% | Category: HIGH

⚡ Performance Opportunity

Sequential I/O in list command (src/cli/commands/list.ts:17-21)

• getGitRoot() and readManifest(userDevflowDir) run sequentially but are independent
• Could parallelize with Promise.all() for faster execution on slow filesystems
• Confidence: 82% | Category: MEDIUM

🎯 Complexity Reduction

Extract formatInstallStatus() helper (src/cli/commands/list.ts:54)

• Manifest display logic (127-char ternary) could move to dedicated function
• Would reduce action handler nesting and improve readability
• Confidence: 80% | Category: MEDIUM

Extract manifest helpers from init.ts (src/cli/commands/init.ts:296-305,611-624)

• logUpgradeStatus() and saveManifest() could separate concerns
• Keeps action handler from growing further (already 515 lines with 15+ responsibilities)
• Confidence: 85% | Category: MEDIUM

---

Summary by Severity

Category	Count	Details
Blocking	2	Manifest validation, error handling (inlined above)
HIGH	3	README drift (2 items) + test coverage gaps
MEDIUM	4	Performance, complexity extraction (2 files)

Claude Code Review Assistant