fix(stable-memory): defer migration rules to migrating-motoko-actors, add mops check-stable

[marc0olo]

Problem

Mistakes #5 carried a table of "which changes need a migration," and its "Adding a field → No" row was misleading: under EOP, changing a record type a persistent variable holds (e.g. adding even an optional field to a record stored in a Map) is an incompatible change rejected at runtime with RTS error: Memory-incompatible program upgrade.

But the deeper issue is the table itself. Per upstream-maintainer feedback, whether a plain added field needs a migration is variant-dependent — enhanced migration has no initializers for stable fields (so adding one needs a migration), while the ordinary persistent actor with field initializers does not for a new variable. A flat table in stable-memory is therefore always one variant away from being wrong, and the authoritative rules live in the migration skill, not here.

Change

• Deleted the migration table. Migration rules and the two migration styles (enhanced vs. legacy (with migration = ...)) are deferred to migrating-motoko-actors, which owns them.
• Kept only what is uniquely this skill's concern:
  • a rejected upgrade is not data loss (the EOP persistence guarantee);
  • the one variant-robust fact: changing a record type an existing persistent variable holds is incompatible and needs a migration (even an optional added field, regardless of where the record lives);
  • prevention: enable stable-compatibility checking in mops check (mops-cli) so the compiler flags an incompatible change at check time against the previous .most, instead of surfacing the runtime RTS error on the upgrade call.

Evidence

Controlled experiment (moc 1.9.0, core 2.6.1, --default-persistent-actors) confirmed a record-type change traps regardless of where the record lives (top-level var, Map value, var-record in a Map); adding a new persistent variable or widening a var is compatible.

Independently corroborated by mops check stable-compatibility checking, which reports the same change as [M0170], tracing the field through the Map internals to the record and directing you to write a migration — i.e. the exact runtime trap, caught before deploy.

Evals

Reshaped the adversarial eval to the accurate claim + the mops check prevention. WITH skill 4/4; baseline 0/4 — baseline confidently calls the change "backward-compatible, no migration needed," which is exactly the belief this corrects.

Eval run (WITH vs baseline)

Adversarial: adding a field to a persisted record needs a migration, and mops check catches it
  WITH skill: 4/4 passed
    ✅ States a plain upgrade will be REJECTED / trap (RTS error: Memory-incompatible program upgrade)
    ✅ Says a migration is required because changing a record type an existing persistent variable holds is incompatible, even though the added field is optional
    ✅ Recommends catching this before deploy with mops check stable-compatibility checking, rather than only discovering the runtime error on upgrade
    ✅ Does NOT claim that adding the field is automatically migration-free
  WITHOUT skill: 0/4 passed
    ❌ (claims the upgrade "should succeed with no migration code needed")
    ❌ (frames adding an optional field as "exactly the backward-compatible case")
    ❌ (never mentions mops check / pre-deploy stable-compatibility verification)
    ❌ (opens with "Yes" and asserts the change is safe and migration-free)

Note on merge order

This references migrating-motoko-actors, introduced by the Motoko upstream migration (#345, part of #327). #345 has now merged, and this branch is rebased onto it — the cross-reference resolves and the one overlapping line (the old migration-table row / renamed cross-ref) is reconciled in favour of this rework. No dependency remains; ready to come out of draft.

🤖 Generated with Claude Code

[github-actions]

Skill Validation Report

Validating skill: /home/runner/work/icskills/icskills/skills/stable-memory

Structure

• Pass: SKILL.md found

Frontmatter

• Pass: name: "stable-memory" (valid)
• Pass: description: (411 chars)
• Pass: license: "Apache-2.0"
• Pass: compatibility: (69 chars)
• Pass: metadata: (2 entries)

Markdown

• Pass: no unclosed code fences found

Tokens

File	Tokens
SKILL.md body	4,259
Total	4,259

Content Analysis

Metric	Value
Word count	2,356
Code block ratio	0.44
Imperative ratio	0.12
Information density	0.28
Instruction specificity	0.80
Sections	15
List items	25
Code blocks	10

Contamination Analysis

Metric	Value
Contamination level	low
Contamination score	0.12
Primary language category	config
Scope breadth	3

• Warning: Language mismatch: shell, systems (2 categories differ from primary)

Result: passed

Project Checks

✓ Project checks passed for 1 skills (0 warnings)

[marc0olo]

Heads-up on interaction with #345 (the Motoko-skills upstream migration to caffeinelabs/skills). This PR is still needed — its content (the record-type-change insight, the mops check stable-compatibility guidance, the new eval) is not in #345; #345 only does the mechanical skill renames. Just flagging a small ordering/rebase point:

#345 renames the Motoko skills: motoko → writing-motoko, migrating-motoko-enhanced → migrating-motoko-actors, adds troubleshooting-motoko-migrations, and retires the inline migrating-motoko.

This PR already forward-references migrating-motoko-actors, which does not exist on main yet — #345 creates it. So:

• chore(motoko): migrate skills to caffeinelabs/skills upstream #345 should merge first. If this PR merged before chore(motoko): migrate skills to caffeinelabs/skills upstream #345, main would briefly have a dangling migrating-motoko-actors cross-ref (and the ## Additional References/link check could flag it).
• After chore(motoko): migrate skills to caffeinelabs/skills upstream #345 merges, a quick rebase resolves everything:
  • The migrating-motoko-actors reference becomes valid.
  • The // … See the \motoko` skillcomment (~line 91, untouched by this PR) auto-updates towriting-motoko` from chore(motoko): migrate skills to caffeinelabs/skills upstream #345 — no action needed.
  • One small conflict remains on the "Removing a field … Load `migrating-motoko` …" line, since both PRs edit it. Take this PR's rework — chore(motoko): migrate skills to caffeinelabs/skills upstream #345 only renamed that line, and I've kept chore(motoko): migrate skills to caffeinelabs/skills upstream #345's stable-memory change to renames-only (reverted a clarification) specifically to keep this conflict trivial.

No changes needed here now — just merge #345 first, then rebase. 👍

[Copilot]

Pull request overview

This PR updates the stable-memory skill guidance to avoid maintaining an error-prone “which changes need a migration” table, and adds an eval case to ensure agents correctly warn that record-type changes in persisted Motoko state can require migrations and should be caught pre-deploy.

Changes:

• Remove the migration “needs migration?” table and replace it with a pointer to the Motoko migration skill plus a concise invariant about record-type incompatibility under EOP.
• Add guidance to catch stable-compatibility issues before deploy via mops stable checking.
• Add an adversarial output eval covering “adding an optional field to a persisted record in a Map”.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
skills/stable-memory/SKILL.md	Reworks Motoko upgrade/migration guidance; removes the table and adds pre-deploy checking advice.
evaluations/stable-memory.json	Adds an adversarial eval ensuring the skill warns about migration needs for persisted record-type changes and recommends mops stable checking.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

evaluations/stable-memory.json:3

• The evaluation file's description says it only tests mo:core Map dot-notation usage, but this PR adds an additional adversarial eval about stable-compatibility/migrations. Update the description to reflect the broader scope so future maintainers don't miss why this eval exists.

  "description": "Evaluation cases for the stable-memory skill. Tests whether agents write persistent actors in the dot-notation call style mo:core requires (moc >= 1.7.0) instead of passing Map's implicit compare argument explicitly.",
  "output_evals": [
    {
      "name": "Adversarial: adding a field to a persisted record needs a migration, and mops check catches it",

[marc0olo]

Re the suppressed comment (evaluations/stable-memory.json:3): valid — the file's description only mentioned mo:core Map dot-notation, but this PR added the adversarial migration / stable-compatibility eval. Broadened the description to cover it (51a505a).