fix(selfedit): a config-only restart leaves an undo behind

.gitignore

@@ -17,6 +17,8 @@ dist/
 /data/
 # Self-edit rollback marker — runtime state, survives restart, never committed.
 /.selfedit-pending.json
+# The config a failed boot was rolled back from — holds owner_handles, like config.yaml.
+/.config-failed.yaml
 # Secrets — keep dir structure, never the real secret files
 /secrets/*
 !/secrets/README.md

docs/CONFIG.md

@@ -151,12 +151,13 @@ matters because a bare `owner_handles: +15551234567` parses as the integer
   and lists work. It exists so `install.sh` scripts set keys byte-exactly instead
   of the model retyping YAML (#185).
 
-**`config.yaml` is gitignored, so the self-edit seatbelt cannot roll back a bad
-config write** — git only restores what it tracks, and tracking this file would
-push `owner_handles` to a published repo. The pre-restart config gate stops a
-config that will not *load*; `data/config-history/` is what a config that loads
-but is wrong gets restored from. Never hand-edit config from a package install;
-use `config_apply`.
+**`config.yaml` is gitignored, so git is not what rolls back a bad config
+write** — git only restores what it tracks, and tracking this file would push
+`owner_handles` to a published repo. `data/config-history/` is the seatbelt
+instead: on a boot that fails after a restart, the newest snapshot is put back
+automatically (see below). The pre-restart config gate still stops a config
+that will not *load*. Never hand-edit config from a package install; use
+`config_apply`.
 
 ### Recovering a bad config
 
@@ -172,11 +173,19 @@ diff data/config-history/<prev>.yaml config.yaml   # what this change did
 cp data/config-history/<prev>.yaml config.yaml     # put it back, then restart
 ```
 
-The newest entry is what is live now, so the one to restore is usually the
-second-to-last. The last 20 changes are kept. A config too broken to boot never
-enters the history — every entry is one that *booted*, which is not the same as
-one that was right: the config you most often need to restore *from* is a bad
-one that came up fine (`gate: {approved: ["*"]}` parses and boots).
+Restoring by hand, the newest entry is usually what is live now, so the one to
+copy back is the second-to-last. The last 20 changes are kept. A config too
+broken to boot never enters the history — every entry is one that *booted*,
+which is not the same as one that was right: the config you most often need to
+restore *from* is a bad one that came up fine (`gate: {approved: ["*"]}` parses
+and boots).
+
+**Automatic restore.** When a restart's boot fails, the recovery puts the
+**newest** snapshot back on its own — newest, not second-to-last, because the
+config that just failed never booted and so never got snapshotted. The config
+it replaces is kept at `.config-failed.yaml` in the repo root (gitignored, same
+handles as `config.yaml`), so a hand edit is never silently discarded, and the
+restart notice on the requesting thread names both.
 
 ## On-disk layout
 
@@ -197,7 +206,8 @@ one that came up fine (`gate: {approved: ["*"]}` parses and boots).
 | `restart_notice.json` | the thread that asked for a restart |
 | `chief.log` | daemon stdout/stderr on macOS / `--no-service` (systemd uses journald) |
 
-Also gitignored at repo root: `.selfedit-pending.json` — the rollback marker.
+Also gitignored at repo root: `.selfedit-pending.json` (the rollback marker) and
+`.config-failed.yaml` (the config a failed boot was rolled back *from*).
 Runtime state that must survive restart and must never be committed.
 
 ### `secrets/` — `secrets/*` ignored, `README.md` tracked

src/chief/adapters/imessage.py

@@ -32,7 +32,7 @@
     run_jxa_subprocess,
 )
 from chief.adapters.imessage_store import RecentDedup
-from chief.selfedit.recovery import RestartBoundary
+from chief.selfedit.restart import RestartBoundary
 
 logger = logging.getLogger(__name__)
 

src/chief/adapters/imessage_fifo.py

@@ -11,7 +11,7 @@
 from collections.abc import Awaitable, Callable
 
 from chief.adapters.base import Message
-from chief.selfedit.recovery import RestartBoundary
+from chief.selfedit.restart import RestartBoundary
 
 logger = logging.getLogger(__name__)
 

src/chief/agent/restart_gate.py

@@ -2,7 +2,7 @@
 it before exec.
 
 Kept apart from the session so the turn path stays focused on assembling and
-running turns. ``chief.selfedit.recovery.RestartController`` is the production
+running turns. ``chief.selfedit.restart.RestartController`` is the production
 implementation; ``_NullGate`` is the no-op used by tests and non-daemon runs.
 """
 

src/chief/app.py

@@ -33,7 +33,7 @@
 from chief.persistence.store import MessageStore
 from chief.provider.base import Provider
 from chief.selfedit.pipeline import SelfEditPipeline
-from chief.selfedit.recovery import RestartController
+from chief.selfedit.restart import RestartController
 from chief.skills import SkillLibrary
 from chief.strangers import StrangerLog
 from chief.tools import ToolContext, ToolDispatcher, ToolRegistry

src/chief/config/history.py

@@ -18,6 +18,7 @@
 record of every *change* rather than of every restart.
 """
 
+import os
 from datetime import datetime
 from pathlib import Path
 
@@ -31,13 +32,70 @@
 
 
 def snapshots(history_dir: Path) -> list[Path]:
-    """Every snapshot, oldest first. The last entry is what is live now, so
-    the previous config is the second-to-last."""
+    """Every snapshot, oldest first.
+
+    Name order is write order (see :data:`STAMP_FORMAT`), and only a config
+    that booted is ever written here — so the last entry is the last config
+    known to come up, which is what :func:`restore_newest` puts back.
+    """
     if not history_dir.is_dir():
         return []
     return sorted(history_dir.glob("*.yaml"))
 
 
+def _write_private(path: Path, data: bytes, mode: int) -> None:
+    """Write ``data`` to ``path`` at ``mode``, never wider and never partial.
+
+    Created at 0600 rather than written and chmod'd after: these files hold
+    ``owner_handles``, and the plain write would leave one world-readable at
+    the umask default for the breath in between. Renamed into place because
+    the restore overwrites the live config at the moment the rollback marker
+    is already gone — a crash mid-write there would leave a truncated config
+    and nothing left to undo it with.
+    """
+    tmp = path.with_name(path.name + ".tmp")
+    fd = os.open(tmp, os.O_CREAT | os.O_WRONLY | os.O_TRUNC, 0o600)
+    with os.fdopen(fd, "wb") as handle:
+        handle.write(data)
+    tmp.chmod(mode)
+    os.replace(tmp, path)
+
+
+def restore_newest(
+    history_dir: Path, config_path: Path, failed_copy: Path
+) -> Path | None:
+    """Put the newest snapshot back, keeping the config it replaced.
+
+    The boot-failure undo for a config change, since git cannot roll back a
+    file it does not track. The **newest** entry is right here, not the
+    second-to-last: a config that failed to boot never got snapshotted, so
+    the last entry is still the last one that came up.
+
+    The config being replaced is saved to ``failed_copy`` first — it may hold
+    an edit the owner still wants, and a failed boot is not a reason to
+    discard it silently. Returns the snapshot restored, or ``None`` when
+    there is no history or the config already matches it (nothing to undo,
+    and the caller must not reboot into an unchanged state).
+    """
+    kept = snapshots(history_dir)
+    if not kept:
+        return None
+    newest = kept[-1]
+    mode = newest.stat().st_mode & 0o777
+    if config_path.is_file():
+        current = config_path.read_bytes()
+        if current == newest.read_bytes():
+            return None
+        live = config_path.stat().st_mode & 0o777
+        failed_copy.parent.mkdir(parents=True, exist_ok=True)
+        _write_private(failed_copy, current, live)
+        # The narrower of the two: an owner who ran `chmod 600 config.yaml`
+        # must not have it widened again by a snapshot taken before they did.
+        mode &= live
+    _write_private(config_path, newest.read_bytes(), mode)
+    return newest
+
+
 def snapshot(config_path: Path, history_dir: Path, now: datetime) -> Path | None:
     """Copy ``config_path`` into the history, unless it is already the newest.
 
@@ -64,8 +122,7 @@ def snapshot(config_path: Path, history_dir: Path, now: datetime) -> Path | None
     # ``copyfile``'s mode handling — which drops the source's bits, so a
     # ``chmod 600 config.yaml`` would have yielded a 0644 copy of the handles
     # the owner had just narrowed.
-    written.write_bytes(current)
-    written.chmod(config_path.stat().st_mode & 0o777)
+    _write_private(written, current, config_path.stat().st_mode & 0o777)
     # Skip the file just written: a box that boots with its clock set ahead
     # leaves a future-stamped entry sorting last forever, and once KEEP of
     # them exist this loop would otherwise unlink the live snapshot.

src/chief/dispatch.py

@@ -31,7 +31,7 @@
 from chief.hub import ObserverHub
 from chief.policy import StreamPolicy, delta_frame, resolve, result_frame, tool_frame
 from chief.provider.base import ProviderError, ToolCall
-from chief.selfedit.recovery import RestartBoundary
+from chief.selfedit.restart import RestartBoundary
 from chief.strangers import StrangerLog
 
 

src/chief/entrypoint.py

@@ -12,7 +12,8 @@
 from chief.install.migrate import migrate_instance
 from chief.instance_lock import AlreadyRunning, acquire_instance_lock
 from chief.selfedit.notice import mark_notice_rolled_back, take_restart_notice
-from chief.selfedit.recovery import clear_marker, restart_daemon, rollback_if_marked
+from chief.selfedit.recovery import clear_marker, rollback_if_marked
+from chief.selfedit.restart import restart_daemon
 
 if TYPE_CHECKING:  # the runtime import stays inside the seatbelt below.
     from chief.daemon import App
@@ -64,6 +65,10 @@ async def amain() -> None:
         level=logging.INFO, format="%(asctime)s %(name)s %(levelname)s %(message)s"
     )
     repo_root = Path.cwd()
+    # Where the rollback looks for the last config that booted. Rebound below
+    # once the real config is loaded; the default covers a boot that dies
+    # before that, when db_path is unknown anyway.
+    history = repo_root / "data" / "config-history"
     try:
         # Imported inside the seatbelt: a self-edit that breaks chief.app's
         # import (yet somehow passes the done-check) still rolls back instead
@@ -77,6 +82,7 @@ async def amain() -> None:
         for step in migrate_instance(repo_root):
             logger.info("update migration: %s", step)
         config = load_config()
+        history = config.db_path.parent / "config-history"
         # One daemon per data dir: a stray second instance would double-poll
         # chat.db and answer every iMessage twice. Held for the whole process.
         _lock = acquire_instance_lock(config.db_path.parent / "chief.lock")
@@ -86,11 +92,14 @@ async def amain() -> None:
         logger.error("another chief instance is already running — refusing to start")
         raise SystemExit(1) from None
     except Exception:
-        # A failed boot right after a self-edit rolls back and re-execs.
-        if rollback_if_marked(repo_root):
+        # A failed boot right after a restart undoes it and reboots — the
+        # commit via git, the config from its newest snapshot.
+        if undone := rollback_if_marked(repo_root, history):
             # The next boot reports the rollback on the requesting thread
-            # instead of a "restart success" that never happened.
-            mark_notice_rolled_back(repo_root)
+            # instead of a "restart success" that never happened — naming
+            # what moved, since a restart changes the commit, the config, or
+            # both, and only the recovery knows which.
+            mark_notice_rolled_back(repo_root, undone)
             restart_daemon()
         raise
     clear_marker(repo_root)

src/chief/selfedit/notice.py

@@ -42,14 +42,16 @@ class RestartNotice:
     thread_key: str
     rationale: str = ""
     rolled_back: bool = False
+    #: What the rollback actually undid, from ``rollback_if_marked``. A
+    #: restart moves the commit, the config, or both, so the report cannot
+    #: assume — it used to claim a previous commit even when none had moved.
+    undone: str = ""
 
     def text(self) -> str:
         """What the fresh daemon says on the requesting thread."""
         if self.rolled_back:
-            return (
-                "⚠️ restart failed — the new code did not boot, so it was "
-                "rolled back. Back up on the previous commit."
-            )
+            what = self.undone or "it was rolled back"
+            return f"⚠️ restart failed — the new code did not boot, so {what}."
         detail = f" ({self.rationale})" if self.rationale else ""
         return f"✅ restart success — back up{detail}"
 
@@ -65,6 +67,7 @@ def write_restart_notice(repo_root: Path, notice: RestartNotice) -> None:
                 "thread_key": notice.thread_key,
                 "rationale": notice.rationale,
                 "rolled_back": notice.rolled_back,
+                "undone": notice.undone,
                 "written_at": time.time(),
             }
         )
@@ -92,6 +95,7 @@ def take_restart_notice(repo_root: Path) -> RestartNotice | None:
                 thread_key=str(data["thread_key"]),
                 rationale=str(data.get("rationale", "")),
                 rolled_back=bool(data.get("rolled_back", False)),
+                undone=str(data.get("undone", "")),
             )
     except Exception:
         # A corrupt notice is dropped, not raised: it must never block a boot.
@@ -100,8 +104,12 @@ def take_restart_notice(repo_root: Path) -> RestartNotice | None:
     return notice
 
 
-def mark_notice_rolled_back(repo_root: Path) -> None:
-    """A failed boot: the pending notice now reports the rollback instead."""
+def mark_notice_rolled_back(repo_root: Path, undone: str = "") -> None:
+    """A failed boot: the pending notice now reports the rollback instead.
+
+    ``undone`` is what the recovery actually put back, carried through so the
+    owner is told which half moved — and where the config that failed went.
+    """
     notice = take_restart_notice(repo_root)
     if notice is None:
         return
@@ -112,5 +120,6 @@ def mark_notice_rolled_back(repo_root: Path) -> None:
             thread_key=notice.thread_key,
             rationale=notice.rationale,
             rolled_back=True,
+            undone=undone,
         ),
     )

src/chief/selfedit/pipeline.py

@@ -4,9 +4,10 @@
 until ``restart``: dirty-tree file-list confirmation, the full done-check,
 then a live ``config.yaml`` load (fixture configs alone would let a bad real
 value boot-loop launchd). Only on all green does it commit, write the
-rollback marker, and reboot into the new code (a boot failure rolls back —
-``recovery.py``). On red the edits are **kept** and the failure returned so
-the agent fixes forward. Restarts serialize behind a lock.
+rollback marker (always — a config-only restart still needs an undo), and
+reboot into the new code (a boot failure rolls back — ``recovery.py``). On red
+the edits are **kept** and the failure returned so the agent fixes forward.
+Restarts serialize behind a lock.
 
 Subprocess plumbing (checks, git, argv-only — no shell) lives in
 ``gitops.py``.
@@ -146,16 +147,22 @@ async def _commit_if_dirty(self, rationale: str, base: str) -> bool:
         """Commit tracked working-tree changes and drop a rollback marker.
 
         Returns whether anything was committed. Gitignored writes (``data``,
-        ``secrets``, ``config.yaml``, off-repo) never enter the commit, so they
-        are unversioned — the rollback marker only rewinds repo files (#198).
+        ``secrets``, ``config.yaml``, off-repo) never enter the commit, so git
+        cannot rewind them (#198) — hence the recorded config-history path,
+        and the marker being written **even when nothing was committed**: a
+        config-only restart used to leave none, so it had no recovery at all.
         """
-        if not await self._dirty_files():
-            return False
-        await self._git("add", "-A")
-        await self._git("commit", "-m", f"self-edit: {rationale}")
-        marker = {"rollback_to": base, "rationale": rationale}
+        committed = bool(await self._dirty_files())
+        if committed:
+            await self._git("add", "-A")
+            await self._git("commit", "-m", f"self-edit: {rationale}")
+        marker = {
+            "rollback_to": base,
+            "rationale": rationale,
+            "committed": committed,
+        }
         (self._root / MARKER_NAME).write_text(json.dumps(marker))
-        return True
+        return committed
 
     async def revert_edits(self) -> str:
         """Discard uncommitted changes to tracked repo files (back to HEAD).