skillopt-sleep: adopt() writes a backup that nothing can restore — add a revert command #247

Summary

adopt() carefully backs up every live file before overwriting it — and then nothing in SkillOpt-Sleep can ever restore that backup. There is no revert, no rollback, no --undo. A user whose nightly run adopted a skill that made things worse has to figure out the staging layout themselves and hand-copy files out of a timestamped directory.

$ grep -rn "revert\|rollback\|restore" skillopt_sleep/
skillopt_sleep/harvest.py:34:    "still failing", "still fails", "not fixed", "revert", "undo",
skillopt_sleep/prompts.py:194:    that override (reverting the template to its default). Returns the new

Both hits are unrelated — one is a feedback-phrase list, the other a docstring. The restore path simply does not exist.

Current behavior

staging.py:518 writes the backup:

def _backup(path: str, backup_dir: str) -> None:
    if os.path.exists(path):
        os.makedirs(backup_dir, exist_ok=True)
        shutil.copy2(path, os.path.join(backup_dir, os.path.basename(path)))

staging.py:524 adopt() calls it for the skill and the memory file, then copies the proposals over the live paths. So after adoption the pre-adopt content is sitting right there at <project>/.skillopt-sleep/staging/<ts>/backup/SKILL.md, alongside a manifest.json that already records exactly where it came from (live_skill_path, live_memory_path).

Everything a revert needs is on disk. There is just no command that reads it.

The CLI (skillopt_sleep/__main__.py:539) exposes run, dry-run, status, adopt, harvest, schedule, unschedule — and that's the full list.

Why this matters more than it looks

Adoption can happen unattended. --auto-adopt (config.py:84, applied at cycle.py:696) adopts as soon as the gate accepts, and cmd_schedule forwards that flag straight into the installed cron entry / Scheduled Task (__main__.py:520):

extra=("--auto-adopt" if getattr(args, "auto_adopt", False) else "")

So the documented happy path is: install a nightly job, go to sleep, and let it rewrite your live SKILL.md at 03:17. The gate is a held-out validation gate, not an oracle — a night can accept an edit that scores better on a handful of mined tasks and still be worse in daily use. When that happens, the recovery story is "go read the source to find out where the backup went."

Related bug: the backup is destroyed if you adopt the same staging dir twice

_backup unconditionally overwrites backup/<basename>. Adopt the same directory a second time and the "backup" becomes a copy of the already-adopted proposal — the original is gone for good:

import json, os, tempfile
from skillopt_sleep.staging import adopt

d = tempfile.mkdtemp()
st, live = os.path.join(d, "staging"), os.path.join(d, "live", "SKILL.md")
os.makedirs(st); os.makedirs(os.path.dirname(live))
open(live, "w").write("ORIGINAL hand-written skill\n")
open(os.path.join(st, "proposed_SKILL.md"), "w").write("PROPOSED (regressed) skill\n")
json.dump({"has_skill": True, "has_memory": False, "live_skill_path": live,
           "live_memory_path": "", "accepted": True},
          open(os.path.join(st, "manifest.json"), "w"))

adopt(st)
print(open(os.path.join(st, "backup", "SKILL.md")).read())  # ORIGINAL hand-written skill
adopt(st)
print(open(os.path.join(st, "backup", "SKILL.md")).read())  # PROPOSED (regressed) skill  <-- original lost

skillopt-sleep adopt with no --staging resolves to latest_staging(project), so running it twice in a row is an easy thing to do by accident. Any revert feature has to fix this too, or it will happily restore the regression it was meant to undo.

Proposed

A revert subcommand mirroring adopt:

skillopt-sleep revert [--project PATH] [--staging DIR]

• Default target: the most recently adopted staging dir for the project, not merely the most recent one (latest_staging currently sorts by mtime and only checks for manifest.json).
• Read manifest.json, and for each live path that has a corresponding file under backup/, copy it back.
• Print what was restored, in the same shape adopt prints what it updated.
• --staging DIR to revert a specific night, so a user can walk back more than one step.

Edge cases worth handling explicitly:

1. No backup exists. _backup is a no-op when the live file didn't exist before adoption (first-ever adopt creates SKILL.md from nothing). A correct revert should remove the adopted file in that case, not silently leave it. This needs to be distinguishable from "backup missing because something went wrong", so the manifest should record which live paths existed pre-adopt.
2. Never adopted. Reverting a staging dir that was only ever staged should be a clear no-op message, not a crash on a missing backup/.
3. Idempotent backups. Make _backup refuse to overwrite an existing backup (or version it), per the bug above.
4. Adoption state. Nothing currently records that a staging dir was adopted — status can't tell an adopted proposal from a staged one. Writing an adopted_at into the manifest (or a sibling file) would make both revert's default target and status's output honest.

Happy to send a PR if the shape above sounds right — in particular whether revert should default to "last adopted" (my preference) or require an explicit --staging.

Environment

main @ 9c776fc, but this is a structural gap rather than a version-specific one.