microsoft / microsoft/SkillOpt

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

Open
#247 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
17.3k
Forks
1.6k
Avg merge
2d 7h
Merged PRs (30d)
17

Description

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with skillopt_sleep/staging.py around _backup() and adopt(), then trace command registration in skillopt_sleep/main.py and manifest handling. Implement the proposed revert flow, adoption tracking, safe backups, and first-adoption removal behavior; verify that default and explicit staging targets restore the recorded live paths without overwriting the original backup.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
cli, tooling
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.