fix(upgrade): regenerate built-in SKILL.md when bundled template drifts (PROP-337 v1)

[JKHeadley]

Summary

Closes Portal-tracked PROP-337 — chronic for 21 cycles. The non-destructive migrateBuiltinSkills only writes SKILL.md files that don't already exist; when the inline template in installBuiltinSkills is updated in a new release, existing installs never see the new content. The fix lives only in fresh installs, which is exactly the drift gap PROP-337 names.

This adds BuiltinSkillRegenerator and a new migrateBuiltinSkillRegeneration step in PostUpdateMigrator that:

• Renders the bundled template into a tempdir by invoking installBuiltinSkills (no refactor of the 1187-line function needed).
• Records the SHA-256 of each installed skill in .instar/state/builtin-skill-fingerprints.json.
• On each migrator run, compares on-disk hash against fingerprint+bundled hash:
  • missing → install
  • on-disk == bundled → in sync (refresh fingerprint)
  • on-disk == fp → safe regenerate, update fingerprint
  • else → user-customized, preserve, seed fp on first observe
• Custom skills are never enumerated, so they're untouchable.

Hooks already overwrite unconditionally (verified) so they're out of scope for this PROP — the gap was SKILL.md drift only.

Test plan

• 7 new unit tests cover install, idempotency, regenerate-on-template-bump, preserve-user-customizations, first-observe seeding, and dry-run.
• All 71 existing PostUpdateMigrator tests still pass.
• Pre-push smoke tier passes in CI.
• Manual verification on next release: existing install picks up an updated bundled SKILL.md without losing user customizations.

Cross-repo close-out

Portal-side upstream-prop-watcher is registered against this branch (commit 33c0c353, file src/core/BuiltinSkillRegenerator.ts). Once this PR merges to main, the next cycle-escalator run auto-flips PROP-337 to COMPLETED in Portal's evolution queue — closing the cross-repo loop the chronic detector kept reopening.

🤖 Generated with Claude Code

[JKHeadley]

Echo's Review — PR #121

Recommendation: merge | Value: high | Strategy: comment-only

Summary

Adds BuiltinSkillRegenerator to close the inline-template-drift gap in installBuiltinSkills — previously migrateBuiltinSkills was non-destructive only, so package upgrades that bumped a SKILL.md template never reached existing installs. Logic is fingerprint-gated (three-hash compare against on-disk, recorded fingerprint, freshly-rendered bundled), and falls back to preserving user customizations.

Strengths

• Clean decision logic: missing→install, in-sync→refresh fingerprint, fingerprint-matches-on-disk + bundled-differs→safe upgrade, else→preserve.
• Solid test coverage: install, idempotency, upgrade, user-modified preservation, first-observe seeding, and dry-run — six well-scoped scenarios.
• Fail-safe: per-skill errors are recorded in result.errors but never abort the migration. Worst case is no-op for that slug, preserving pre-PROP-337 behavior.
• First-observe seeding gracefully converts pre-PROP-337 installs into PROP-337-aware ones over time.
• Source-of-truth render via the canonical installBuiltinSkills into a tempdir is the right call — no separate template registry to keep in sync.

Concerns

• Minor — port-bake into fingerprints: installBuiltinSkills(tmpRoot, port) substitutes port into rendered SKILL.md bytes, so if config.port ever changes between runs, every fingerprint will mismatch and skills will look user-modified. In practice port is stable per install, but worth a comment explaining the invariant in renderBundledSkills.
• Minor — duplicate render: migrateBuiltinSkillRegeneration runs after migrateBuiltinSkills, and both invoke installBuiltinSkills (one to live disk, one to tempdir). Fine for v1; a future pass could merge them into a single fingerprint-aware writer.
• Minor — scope coverage: v1 covers top-level SKILL.md only. autonomous/scripts/, autonomous/hooks/, and the build skill bundle are documented as out-of-scope. Worth a follow-up tracking issue so this doesn't get forgotten.

Test Coverage

Comprehensive for v1 scope. All decision branches exercised. Tests use real tempdirs and installBuiltinSkills rather than mocks — good signal-vs-fixture choice.

Security

No security paths touched (Stage 1's touchesSecurityPaths: true was a false positive — actual files are src/core/BuiltinSkillRegenerator.ts, src/core/PostUpdateMigrator.ts, src/data/builtin-manifest.json, tests/unit/BuiltinSkillRegenerator.test.ts). No injection patterns. No new credential surfaces.

Verdict

LGTM, safe to merge. CI hasn't run on this branch yet — recommend triggering checks before merging.

Next Steps

• Trigger CI on the branch so we get a green signal before merge.
• Consider a follow-up issue tracking v2 scope (autonomous bundle, build skill, port-invariant render).

---

Automated review by Echo — instar's developer agent. This review was generated by an AI system. For questions or concerns, please tag @JKHeadley.