fix(client): make the disaster-recovery path actually work

Review found the read path scoped to the CURRENT device's uuid, which breaks
the exact case snapshot mode exists for: when the laptop dies, the
replacement machine has a new id, so snapshots printed 'no restore points
yet' while the backups sat on the forge, and restore errored. Reproduced,
then fixed by unscoping the READ only. Writing stays device-scoped (two
machines must not overwrite each other) and pruning stays device-scoped
(machine A must not apply its clock to machine B's refs); the docstring now
says why the three differ.

Also from the same review:
- mirror mode printed a %cI timestamp that restore could not accept, so
  copying the first column looped the user back to snapshots. It now matches
  the log, and refuses an ambiguous timestamp (two commits in one second)
  with the candidate ids instead of guessing.
- the size guard advised 'add a .gitignore' while measuring with a plain
  walk that ignored one. It now measures what git would sync, through a
  throwaway git dir outside the folder so a refused add leaves no .git
  behind.
- get --all caught only SystemExit, so a RuntimeError from any git call
  abandoned the remaining repos.
- get --all mapped alice/notes and bob/notes to one path and reported the
  second as 'already present'. Clashes now clone to <owner>-<name> and say so.
- the prune clock was in-memory, so watch --once under launchd pruned every
  run. Persisted in config.

153 tests. Live-verified on the beta forge: machine A backed up uncommitted
work and was deleted; machine B, different device id, cloned the repo, listed
A's snapshot and restored both files.
This commit is contained in:
claude
2026-08-23 11:42:19 -04:00
parent ddb829d701
commit eb70ca08ad
3 changed files with 374 additions and 32 deletions
+144 -26
View File
@@ -354,25 +354,41 @@ def _remember_snapshot_tree(folder, dev, tree):
pass # a lost marker costs one redundant push, never correctness
def list_snapshots(folder, dev, remote="granthi"):
"""Backup refs for this device on the remote, newest first.
def list_snapshots(folder, dev=None, remote="granthi"):
"""Backup refs on the remote, newest first.
Read from the REMOTE, not a local cache: the point of the feature is
surviving the loss of this machine, so what the server holds is the
answer that matters.
`dev=None` means EVERY device, and that is what the read paths pass.
The three scopes differ on purpose:
* writing is device-scoped, so two machines never overwrite each
other's snapshots;
* pruning is device-scoped, so machine A never applies its own clock
and retention to machine B's refs;
* reading is NOT scoped, because the case this feature exists for is
"the laptop died". A new machine has a new id, and scoping the read
to it would show an empty list while the backups sit on the forge.
"""
rc, out = git(folder, "ls-remote", remote, f"{BACKUP_NS}/{dev}/*",
check=False)
pattern = f"{BACKUP_NS}/{dev}/*" if dev else f"{BACKUP_NS}/*"
rc, out = git(folder, "ls-remote", remote, pattern, check=False)
if rc != 0:
return []
prefix = f"{BACKUP_NS}/{dev}/" if dev else f"{BACKUP_NS}/"
snaps = []
for line in out.splitlines():
sha, _, ref = line.partition("\t")
ref = ref.strip()
if not ref.startswith(f"{BACKUP_NS}/{dev}/"):
if not ref.startswith(prefix):
continue
snaps.append({"sha": sha.strip(), "ref": ref,
"ts": ref.rsplit("/", 1)[-1]})
rest = ref[len(f"{BACKUP_NS}/"):]
device, _, ts = rest.partition("/")
if not ts:
continue
snaps.append({"sha": sha.strip(), "ref": ref, "ts": ts,
"device": device})
return sorted(snaps, key=lambda s: s["ts"], reverse=True)
@@ -831,21 +847,45 @@ def _get_all(cfg, args):
"""
repos, truncated = list_repos(cfg)
base = os.path.abspath(args.into or ".")
linked = set(cfg.get("folders", {}))
folders = cfg.get("folders", {})
# full_name already living at a path, so "already here" can be told apart
# from "a different repo wants that same path".
owner_of = {os.path.abspath(p): m.get("full_name")
for p, m in folders.items()}
cloned = skipped = failed = 0
for repo in sorted(repos, key=lambda r: r.get("full_name") or ""):
full_name = repo.get("full_name")
if not full_name:
continue
dest = os.path.join(base, full_name.rsplit("/", 1)[-1])
if dest in linked or (os.path.exists(dest) and os.listdir(dest)):
owner, _, name = full_name.rpartition("/")
dest = os.path.join(base, name)
if owner_of.get(dest) == full_name:
log(f"skip {full_name}: already present at {dest}")
skipped += 1
continue
if dest in owner_of or (os.path.exists(dest) and os.listdir(dest)):
# alice/notes and bob/notes both want <base>/notes. Reporting the
# second as "already here" would be a silent collision -- the user
# would believe they had pulled both.
taken_by = owner_of.get(dest)
qualified = os.path.join(base, f"{owner}-{name}" if owner else name)
if owner_of.get(qualified) == full_name or (
os.path.exists(qualified) and os.listdir(qualified)):
log(f"skip {full_name}: already present at {qualified}")
skipped += 1
continue
log(f"name clash on {dest}"
+ (f" (held by {taken_by})" if taken_by else "")
+ f": cloning {full_name} to {qualified} instead")
dest = qualified
try:
clone_one(cfg, full_name, dest, args.mode)
owner_of[os.path.abspath(dest)] = full_name
cloned += 1
except SystemExit as e: # one bad repo must not abandon the rest
except (SystemExit, RuntimeError) as e:
# One bad repo must not abandon the rest. clone_one raises
# SystemExit for a refused destination and RuntimeError from any
# failing git call -- both are one repo's problem, not the run's.
log(f"FAILED {full_name}: {e}")
failed += 1
log(f"get --all: {cloned} cloned, {skipped} already here, {failed} failed")
@@ -857,12 +897,65 @@ def _get_all(cfg, args):
def measure_folder(folder, max_files=ADD_MAX_FILES):
"""(files, bytes) below .git, stopping once max_files is exceeded.
"""(files, bytes) that would actually sync, stopping past max_files.
Ignore rules are honoured, because the guard's own advice is "add a
.gitignore for what should not sync" -- and advice that changes nothing
is worse than no advice. git is asked with a THROWAWAY git dir outside
the folder, so a refused `add` leaves no .git behind in a directory the
user never agreed to turn into a repo.
Bounded on purpose: the caller only needs to know whether the folder is
surprisingly big, and walking a 2 TB drive to answer that would be the
same mistake as syncing it.
"""
listing = _git_would_sync(folder)
if listing is None:
return _walk_folder(folder, max_files)
files = total = 0
for rel in listing:
path = os.path.join(folder, rel)
try:
if os.path.islink(path):
continue
total += os.path.getsize(path)
except OSError:
continue
files += 1
if files > max_files:
break
return files, total
def _git_would_sync(folder):
"""Paths git would take from `folder`, honouring .gitignore. None if git
could not answer, so the caller falls back to a plain walk."""
scratch = tempfile.mkdtemp(prefix="granthi-measure-")
try:
git_dir = os.path.join(scratch, "git")
init = subprocess.run(["git", "init", "-q", "--bare", git_dir],
capture_output=True, text=True)
if init.returncode != 0:
return None
proc = subprocess.run(
["git", f"--git-dir={git_dir}", f"--work-tree={folder}",
"status", "--porcelain", "--untracked-files=all",
"--ignored=no", "-z"],
capture_output=True, text=True)
if proc.returncode != 0:
return None
out = []
for entry in proc.stdout.split("\0"):
if len(entry) > 3:
out.append(entry[3:])
return out
except OSError:
return None
finally:
subprocess.run(["rm", "-rf", scratch], capture_output=True)
def _walk_folder(folder, max_files):
files = total = 0
for root, dirs, names in os.walk(folder):
dirs[:] = [d for d in dirs if d != ".git"]
@@ -989,16 +1082,15 @@ def _sigterm(signum, frame):
log(f"signal {signum} received; finishing current pass then exiting")
_LAST_PRUNE = 0.0
def watch_pass(now=None):
global _LAST_PRUNE
cfg = load_config()
dev = device_id(cfg)
# Pruning talks to the remote for every folder, so it runs on a slow
# clock of its own rather than on every 30s pass.
due = (now or time.time()) - _LAST_PRUNE >= PRUNE_EVERY_SECONDS
# clock of its own rather than on every 30s pass. The clock is PERSISTED:
# `watch --once` under launchd or cron is a fresh process every time, and
# an in-memory timer would make "at most hourly" mean "every run".
now = now or time.time()
due = now - float(cfg.get("last_prune") or 0) >= PRUNE_EVERY_SECONDS
for folder, meta in sorted(cfg.get("folders", {}).items()):
if not os.path.isdir(folder):
log(f"SKIP {folder}: missing")
@@ -1024,7 +1116,7 @@ def watch_pass(now=None):
# Retention failing must never stop the backup itself.
log(f"prune failed in {folder} (backups unaffected): {e}")
if due:
_LAST_PRUNE = now or time.time()
cfg["last_prune"] = now
save_config(cfg)
@@ -1058,7 +1150,7 @@ def cmd_snapshots(args):
"""Restore points, newest first. Both modes answer the same question."""
cfg = require_linked(load_config())
folder, meta = _folder_meta(cfg, args.folder)
dev = device_id(cfg)
this_dev = cfg.get("device_id")
if meta.get("mode", "mirror") == "mirror":
rc, out = git(folder, "log", "--format=%H\t%cI\t%s", "-n",
str(args.limit), check=False)
@@ -1070,13 +1162,16 @@ def cmd_snapshots(args):
sha, ts, subject = (line.split("\t") + ["", ""])[:3]
print(f" {ts} {sha[:12]} {subject}")
return 0
snaps = list_snapshots(folder, dev)
# Every device's backups, not just this one's -- see list_snapshots.
snaps = list_snapshots(folder)
if not snaps:
print("no restore points yet (nothing uncommitted has been backed up)")
return 0
print(f"restore points for {folder} (mode snapshot, device {dev[:8]}):")
print(f"restore points for {folder} (mode snapshot):")
for snap in snaps[:args.limit]:
print(f" {snap['ts']} {snap['sha'][:12]}")
here = " (this device)" if snap["device"] == this_dev else ""
print(f" {snap['ts']} {snap['sha'][:12]} "
f"device {snap['device'][:8]}{here}")
if len(snaps) > args.limit:
print(f" ... {len(snaps) - args.limit} older (use --limit)")
return 0
@@ -1091,15 +1186,36 @@ def cmd_restore(args):
"""
cfg = require_linked(load_config())
folder, meta = _folder_meta(cfg, args.folder)
dev = device_id(cfg)
target = args.at
sha = None
if meta.get("mode", "mirror") == "snapshot":
for snap in list_snapshots(folder, dev):
# Across ALL devices: restoring is exactly the moment the device that
# took the backup may no longer exist.
for snap in list_snapshots(folder):
if snap["ts"] == target or snap["sha"].startswith(target):
sha = snap["sha"]
git(folder, "fetch", "granthi", f"{snap['ref']}")
break
else:
# Mirror mode lists commits by their %cI timestamp, and that first
# column is what a person copies. It is not a commit-ish, so match it
# against the log before falling through to rev-parse -- otherwise
# `restore --at <what snapshots just printed>` fails and sends the
# user back to `snapshots`, which prints the same thing again.
rc, out = git(folder, "log", "--format=%H\t%cI", check=False)
if rc == 0:
hits = [line.partition("\t")[0] for line in out.splitlines()
if line.partition("\t")[2] == target]
if len(hits) > 1:
# Two commits in the same second share a %cI. Silently taking
# the newest would restore something the user did not pick.
listed = "\n".join(f" {h[:12]}" for h in hits)
raise SystemExit(
f"{target} matches {len(hits)} restore points in "
f"{folder}. Re-run with one of these ids instead:\n"
f"{listed}")
if hits:
sha = hits[0]
if sha is None:
rc, resolved = git(folder, "rev-parse", "--verify", "--quiet",
f"{target}^{{commit}}", check=False)
@@ -1108,8 +1224,10 @@ def cmd_restore(args):
f"no restore point {target!r} for {folder} "
f"(see: granthi-sync snapshots {folder})")
sha = resolved.strip()
# A %cI timestamp carries ':' and '+', which make an awkward folder name.
suffix = re.sub(r"[^A-Za-z0-9]+", "-", target).strip("-") or sha[:12]
dest = os.path.abspath(
args.into or f"{folder.rstrip(os.sep)}-restore-{target}")
args.into or f"{folder.rstrip(os.sep)}-restore-{suffix}")
if os.path.exists(dest) and os.listdir(dest):
raise SystemExit(f"refusing to restore over a non-empty path: {dest}")
os.makedirs(dest, exist_ok=True)