Skip to content

Make an archive the catalog cannot see an error, and recover the ones it already lost - #24

Merged
andrei-hasna merged 3 commits into
mainfrom
fix/2c4541e1-reconcile-orphan-runs
Jul 28, 2026
Merged

Make an archive the catalog cannot see an error, and recover the ones it already lost#24
andrei-hasna merged 3 commits into
mainfrom
fix/2c4541e1-reconcile-orphan-runs

Conversation

@andrei-hasna

@andrei-hasna andrei-hasna commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Closes the reporting side of todos 2c4541e1.

The defect

Registration is a single commit after the archive loop. src/backup.ts copies each archive to the destination inside for (const source of sources), then writes the catalog manifest, the destination manifest.json and the runs.jsonl entry only once every source has finished:

const manifestPath = join(home.manifestsDir, `${id}.json`);
if (!options.dryRun) {
  writeJsonAtomic(manifestPath, manifest);
  await copyManifestToDestination(manifestPath, destination, `${id}/manifest.json`);
  appendRun(home, manifest);
}

So a run that dies part-way leaves real archives that nothing describes. pruneBackups enumerates catalog manifests and nothing else, so such an archive is never expired and never reported (unbounded disk growth), verify and restore cannot reach it, and backup hold cannot protect it because holds key on a manifest id. It is simultaneously undeletable by policy and unprotectable — and nothing in the tool noticed.

This is the mirror of the empty-manifest husk fixed in 0.2.1: there a manifest described archives that were gone; here archives exist and no manifest describes them.

This is not a recent regression. appendRun(home, manifest) has sat after the loop since the initial commit 27ede7d (2026-06-29), and none of #19 / #21 / #23 touched it.

What it had already cost, measured

On one machine: 62 archives across 11 run directories, 1.26 GB, spanning 2026-07-24 to 2026-07-27 — including two runs of 22 archives each. Three of those runs died at exactly 60 s, evidenced by truncated partial archives left in tmp/ (which the run path never cleans): e.g. a 1,425,710,386-byte archive stopped at 762,930,877 and at 981,565,810 on two separate runs.

The uncatalogued runs interleave with successful ones minutes apart, which is why nothing looked broken. Registration was never broken; it is conditional on the run completing.

backup reconcile

Compares each local destination against the catalog and exits non-zero on any mismatch:

state fails?
orphan-run (archives, no manifest) yes
manifest-missing-archives yes
manifest-extra-archives yes
run-dir-missing yes
empty-run-dir no
content-deleted no
  • empty-run-dir is deliberately not a finding. prune deletes the archives and the catalog manifest but leaves the directory and never rewrites the append-only ledger, so empty directories are expected residue. Flagging them would train operators to ignore the command — on the affected machine 9 of the 20 manifest-less directories are exactly this.
  • Non-local destinations report enumerated: false with a reason. No network listing is performed, so silence about an S3 destination is not evidence that it is consistent, and the report says so instead of implying a clean bill of health it never established.

backup reconcile --adopt

Rebuilds catalog manifests from the artifacts on disk — and only from what they can support.

Recovered because it is genuinely derivable: run id, UTC start time (the run-id stamp is minted from toISOString()), destination, archive names, byte sizes, recomputed sha256, and the owning source — archive names are <runId>-<sanitizeName(source.name)>.tgz, so the reverse mapping is exact whenever exactly one configured source sanitizes to that key.

Never fabricated: the run-time per-source inventory (left []), the host that wrote the archives, and any claim that the archive matched its source when written. A recomputed digest shows the artifact is internally intact now; it cannot show the backup was ever good. Manifests carry reconstructedAt / reconstructedFrom, and the limitations are returned in the result payload rather than living only in documentation.

Two safety properties worth reviewing closely:

  1. A run with any unattributable archive is skipped whole, not partially adopted. sanitizeName folds case and punctuation, so two configured sources can collide on one key (this repo already ships archiveNameCollisions to detect that). Attributing an archive to the wrong source would let retention treat it as a copy of something it is not, and delete the last real copy of that source while reporting it retained.
  2. Adopted runs are held (hold:reconstructed-manifest) by default. Making an orphan visible to the catalog also makes it visible to prune, so recovery must not convert an invisibility bug into a deletion risk. --no-hold opts out.

Also: --home with no value silently selected the default home

--home was read with stringFlag, which returns undefined for a bare --home — and undefined means "use the default". So backup <command> --home (trailing flag, or a shell variable that expanded to nothing) ran against the real backup home while the operator believed it had been redirected. That is the shape behind a live-home incident that destroyed 139 artifacts. Now read with stringFlagStrict, and resolved inside the error boundary so the refusal is reported as ok: false + exit 1 rather than thrown at the shell.

Verification

bun run check green (typecheck + 122 tests + contracts conformance + build); 13 new tests in tests/reconcile.test.ts, 109 pre-existing still passing.

The new tests reproduce the production shape by deleting all three registration artifacts after a successful run — exactly what a mid-loop kill leaves — and cover: orphan detection, a green complete run, UTC start-time derivation (asserting no offset shift), empty-dir-is-benign, missing archives, a missing run directory, non-local destinations reported as not-enumerated, adopt→verify passing, the hold blocking prune expiry, dry-run-by-default writing nothing, refusal on an unattributable archive, non-zero CLI exit, and the bare---home refusal.

Exercised end-to-end against a hardlink-isolated copy of a real 17 GB backup home and then the live one: 11 orphan runs / 62 orphan archives detected → adopted → reconcile green (22 consistent, 0 orphans) → verify green on the recovered runs (22/22 and 2/2 checksums matched, archives that verify previously could not see at all) → prune --plan shows 22 retained, 0 expired, 0 expired artifacts, each adopted run carrying hold:reconstructed-manifest. Before/after asserted additions only, with a comm positive control proving the detector fires on a removed entry: 0 pre-existing manifests changed or lost.

Not merging this myself — ready for an adversarial review pass. The --home strictness change is the one intentional behaviour break: any caller passing a bare --home was silently hitting the live home and will now get an error.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

… ones it already lost

Registration is a single commit AFTER the archive loop. `runBackup` copies each
archive to the destination inside `for (const source of sources)`, then writes the
catalog manifest, the destination `manifest.json` and the `runs.jsonl` entry only
once every source has finished. A run that dies part-way therefore leaves real
archives that nothing describes.

That state is worse than it reads. `pruneBackups` enumerates catalog manifests and
nothing else, so an unregistered archive is never expired and never reported,
`verify` and `restore` cannot reach it, and `backup hold` cannot protect it because
holds key on a manifest id. It is at once undeletable by policy and unprotectable,
and nothing in the tool noticed. It is the mirror of the empty-manifest husk fixed
in 0.2.1: there a manifest described archives that were gone, here archives exist
and no manifest describes them.

On one machine this had accumulated 62 archives across 11 run directories over three
weeks (1.26 GB), including two runs of 22 archives each. The runs interleaved with
successful ones minutes apart, so nothing looked broken: registration was never
broken, it is conditional on the run completing.

`backup reconcile` compares each local destination against the catalog and exits
non-zero on `orphan-run`, `manifest-missing-archives`, `manifest-extra-archives` and
`run-dir-missing`. An empty run directory with no manifest is deliberately NOT a
finding: prune removes the archives and the manifest but leaves the directory and
never rewrites the append-only ledger, so empty directories are expected residue and
flagging them would train operators to ignore the command. Non-local destinations
report `enumerated: false` with a reason, because no network listing is performed and
silence about an S3 destination is not evidence that it is consistent.

`backup reconcile --adopt` rebuilds manifests from the artifacts on disk, and only
from what they can actually support: run id, UTC start time (the run-id stamp is
minted from `toISOString()`), destination, archive names, sizes, recomputed sha256,
and the owning source via the same `sanitizeName` the writer used. What is gone is
not invented — the run-time inventory is left empty, and a recomputed digest proves
the artifact is intact now, not that it ever matched its source, so these manifests
carry `reconstructedAt` and the limitation is returned in the payload rather than
living only in docs. A run with any archive that does not map to exactly one
configured source is skipped whole, because `sanitizeName` folds case and
punctuation and mis-attributing an archive would let retention treat it as a copy of
the wrong source and delete that source's last real copy while reporting it
retained. Adopted runs are held: making an orphan visible to the catalog also makes
it visible to prune, and recovery must not turn an invisibility bug into a deletion
risk.

Also fixes `--home` with no value silently selecting the default home. It was read
with `stringFlag`, which returns undefined for a bare `--home`, and undefined means
"use the default" — so `backup <command> --home` ran against the real backup home
while the operator believed it had been redirected. That is the shape behind a
live-home incident that destroyed 139 artifacts. It now uses `stringFlagStrict` and
is resolved inside the error boundary, so the refusal is reported rather than thrown.
`gzip -t` is not a completeness test for these artifacts, and trusting it is an
active trap. When `tar -czf` is killed mid-stream its gzip child sees EOF on the
pipe and writes a valid gzip trailer, so the truncated file passes `gzip -t`
while its tar payload stops mid-member. All three partials left by the incident
behave exactly that way: they decompress cleanly and `tar -tzf` reports
`Unexpected EOF in archive`.

That matters specifically for reconstruction. A rebuilt manifest records a
sha256 computed from the file as it is now, so adopting a truncated archive
would make `verify` agree with itself forever — permanently green about data
that cannot be restored. That is strictly worse than leaving the run
unregistered, where at least `reconcile` keeps reporting it.

`adopt` now proves completeness by listing the archive rather than by trusting
the gzip envelope, and a run with any incomplete payload is refused whole and
reported in `unadoptable` with its reason.

The regression test reproduces the real artifact shape rather than a generic
truncation: it gunzips a real archive, cuts the payload mid-member, re-gzips it,
and asserts `gzip -t` exits 0 while `tar -tzf` does not, before asserting the
run is refused and no manifest is written.
@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Adversarial review pass landed; one finding was load-bearing enough to change the code, and it is worth calling out for whoever reviews this.

gzip -t is not a completeness test for these artifacts, and trusting it would have made this fix dangerous. When tar -czf is killed mid-stream, its gzip child sees EOF on the pipe and writes a valid gzip trailer — so the truncated file passes gzip -t while its tar payload stops mid-member. All three partials from the incident behave exactly that way.

That interacts badly with reconstruction specifically: a rebuilt manifest records a sha256 computed from the file as it is now, so adopting a truncated archive would make verify agree with itself forever — permanently green about data that cannot be restored. Strictly worse than leaving the run unregistered, where reconcile at least keeps reporting it.

Fixed in 4dbe3f5: adopt now proves completeness by listing the archive (tar -tzf) instead of trusting the gzip envelope, and refuses a run whole if any payload is incomplete. The regression test reproduces the real artifact shape rather than a generic truncation — gunzip a real archive, cut mid-member, re-gzip, then assert gzip -t exits 0 while tar -tzf does not, before asserting the run is refused and no manifest is written. 123 tests green.

Checked against the affected machine before and after: all 62 recovered archives are tar-complete (0 incomplete), with a positive control confirming tar -tzf rejects a truncated copy. So no manifest already written describes a truncated archive.

Two review findings I did not fix here, deliberately, as they are separate concerns rather than scope for this PR:

  1. reconcile does not cover ledger-only runs. One runs.jsonl entry has no manifest and no run directory. prune refuses directory artifacts, so a second disposal path exists that this PR's model does not explain, and reconcile enumerates destinations and the catalog but not the run ledger.
  2. findDestination returns destinations[0] when --destination is omitted. On the affected machine destinations[0] is the local destination, so a bare backup run writes locally without anyone naming it. Given how sharp the destructive paths on this CLI are, an implicit destination default deserves the same treatment --home just got.

Also corrected in the record: I had claimed elsewhere that the end-of-loop registration was unrelated to the recent merges. The design is original (first commit of src/backup.ts, 27ede7d), which stands — but the installed build at the time of the failures was 8ce32b1 itself and had completed zero runs, so "not a regression" is unproven rather than established. No mechanism in 8ce32b1 was found that would cause an external kill.

Not merging this myself.

…ost archive

Adversarial review of the reconcile guard found three states where it reported
`ok: true` — or a wrong diagnosis — for a destination holding unregistered
archives or a damaged catalog. Each was measured, and each now has a regression
test that fails without the fix.

1. An unreadable run directory was an empty one. `archiveFilesInRunDir` caught
   its own error and returned `[]`, so a directory whose listing was denied was
   classified `empty-run-dir` — "expected prune residue", exit 0 — while an
   archive sat in it unregistered. Measured: state `empty-run-dir`,
   `archivesOnDisk: 0`, one archive actually present. With a manifest also
   present the same swallow produced a false data-loss claim instead
   (`manifest-missing-archives`: "archives that are not at the destination",
   when they were merely unreadable).

2. A broken catalog manifest was a missing one. `readCatalogManifest` caught the
   parse error and returned `null`, which collapsed a partially written manifest
   into "no manifest": reported as an orphan when archives remained — inviting
   `--adopt` to overwrite the only surviving record of the run's real start time,
   inventory and as-written digests — and reported as benign `empty-run-dir`
   when the archives had already been pruned, i.e. catalog corruption with
   exit 0. Parsing is no longer the bar either: a manifest missing `id`,
   `createdAt` or `archives` is a partial write. A broken manifest whose run
   directory is also gone was reachable by neither walk (the destination walk
   cannot see it, the manifest walk skips what it cannot parse), so a
   catalog-wide census now reports it. `--adopt` refuses to overwrite one.

3. A run still in flight was reported as a lost archive. Registration is one
   commit after the loop, so a healthy run is in the orphan shape for its entire
   duration; a scheduled reconcile overlapping a scheduled run would have failed
   every time, and `--adopt` would have written a manifest describing a partial
   archive set and held a run about to register itself. A run directory written
   within `--active-within-seconds` (default 900, 0 disables) is now
   `run-in-progress`: not a failure, not adopted, reported as `skipped` with the
   reason stated. The window only delays detection — verified on live data: a
   real orphan run read `run-in-progress` at 908 s before its last write and
   `orphan-run` immediately after the window elapsed.

Also, on the payload gate added in 4dbe3f5: its `tar -tzf` check is kept because
it is strictly stronger than a gzip-envelope test, and that is now recorded with
the measurement behind it — 4 of the 6 real partials on the affected machine pass
`gzip -t` and fail `tar -tzf`. A second truncation shape is pinned alongside the
tar-payload one: `copyArchiveToDestination` writes a local archive with
`Bun.write(target, Bun.file(staged))` straight onto the FINAL name, so a kill
during the copy also leaves a truncated archive under the name it keeps, which
means "the destination copy happens after the digest, so destination archives
are whole" does not hold. The refusal moves out of `unresolvedArchives` into
`incompleteArchives`, because a consumer reading the former would conclude the
archive's SOURCE was unresolvable; the two refusals have different remedies.

`--active-within-seconds` is read with a strict number flag, so its bare form is
an error rather than a silent fall-back to the default — the same shape as the
bare `--home` this branch fixes. And `src/reconcile.ts` joins the build's module
entry list alongside `src/preservation.ts`.

The root cause is still only detected, not fixed: registration remains a single
commit after the archive loop, so the next killed run orphans its archives
exactly as before. It recurred during this review — run bkp_20260728004309_gptrq1
started 2026-07-28T00:43:09Z, wrote 22 archives, and died 60 s later leaving no
manifest. Recorded in the changelog rather than left to be inferred.

132 tests pass (was 123); typecheck, contracts conformance and build green.
@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Adversarial review — APPROVE-WITH-FIXES (fixes pushed as 8e0e635)

Reviewed against a fresh clone, not the shared checkout. Verdict: the recovery half is sound and correct — I round-tripped a reconstruction and every field checks out — but the guard half failed open in three states, which I fixed here. The root cause is detected, not fixed, and it recurred during this review.

Timestamps are UTC unless marked; this box is UTC+0300 (EEST), verified date -u 00:38Z vs date 03:38 EEST.


1. Round-trip on a reconstruction — PASSES

bkp_20260727220933_azrqsj, checked field by field against the artifacts rather than against the report:

field manifest claims measured independently verdict
createdAt 2026-07-27T22:09:33Z archive mtime 2026-07-28 01:09:33.757 +0300 = 22:09:33Z exact to the second
bytes 557632 / 1096 stat -c%s → 557632 / 1096 match
sha256 9ca5ddce…fc2e / a0514d77…bb42 sha256sum → identical match
sourceName/sourcePath hasna-projects-db…/projects/projects.db; ops-state-snapshot-script…/ops-state-snapshot.sh tar -tzvfprojects.db, ops-state-snapshot.sh contents match the claimed source
destination local-smoke~/.hasna/backup/local archives are there match
inventory [] not recoverable correctly left empty, not invented

The createdAt derivation is the field the brief flags as able to silently change GFS retention, so I checked the premise in code too: uniqueId (src/config.ts:587) stamps new Date().toISOString(), so the run-id digits are UTC. A local-time reading on this box would have been 3 h off; the mtime agreeing to the second rules that out.

Container integrity of the whole recovered set (read-only): all 62/62 adopted archives pass gzip -t and tar -tzf. No truncated archive was adopted in production.

The recovered set is honestly bounded. My own census of all 32 run directories: 22 hold archives (all consistent), 9 are genuinely empty (0 files — prune residue), 1 is tonight's new orphan. Adopted .tgz per run: 2×9 + 22×2 = 62. That reproduces the 62/11 figure exactly and independently. Note the task framing itself was wrong and the PR is right to correct it: it is 62 archives across 11 directories, not 22; and there is no 14:14→22:09 breakage boundary — on 07-27 alone the local runs go 10:31 empty, 10:33 orphan, 10:40 registered, 10:44 empty, 10:46 registered, 10:50 registered, 22:09 orphan.

2. No pre-existing manifest was modified or removed

Asserted by content, not by count (a count is not a control on this machine — run directories appear on a timer).

$ cat original62-abs.txt | xargs md5sum | md5sum      # the 62 pre-recovery manifests
84aacd70b5313b70ed913a09a96fc0ef                       # reproduces, before and after my work
$ find …/manifests -type f -printf '%P %s\n' | LC_ALL=C sort | md5sum   # name+size, same 62
dc00e84fd543e3610fbb67c76296899a
$ md5sum -c prework-manifests-md5.txt | grep -cv ': OK$'
0                                                      # all 73, byte-identical
$ LC_ALL=C comm -23 pre.keys final.keys | wc -l
0                                                      # whole home: nothing disappeared
$ LC_ALL=C comm -23 <(sort -u <(cat pre.keys; echo "f local/CONTROL-ENTRY-THAT-VANISHED")) final.keys
f local/CONTROL-ENTRY-THAT-VANISHED                    # positive control: comm does detect a removal

Both fingerprints the ops agent published reproduce exactly, with the caveat that the recipe is path-sensitive: run from inside manifests/ the same 62 files give df2f00ce…, because md5sum hashes the filename it printed. Use absolute paths.

3. Findings — three fail-opens in the guard, fixed in 8e0e635

Every one was measured, and every one now has a regression test that fails without the fix.

F1 (high) — an unreadable run directory was an empty one. archiveFilesInRunDir caught its own error and returned []. Measured with the manifest also deleted and the directory chmod 000:

PROBE H  ok=true  state=empty-run-dir  archivesOnDisk=0
         REALITY: archives still on disk = 1

ok: true, exit 0, message "no manifest and no archives: expected prune residue" — the guard declared an unregistered archive benign. That is the exact blindness this command exists to end. With a manifest present the same swallow produced a false data-loss claim instead: manifest-missing-archives, "manifest lists 1 archive(s) that are not at the destination", when they were merely unreadable.

F2 (high) — a broken catalog manifest was a missing one. readCatalogManifest caught the parse error and returned null, collapsing three outcomes into two. Two measured consequences:

PROBE B  (truncated manifest, archives present)   ok=false  state=orphan-run
PROBE C  (truncated manifest, archives pruned)    ok=true   state=empty-run-dir

PROBE C is catalog corruption reported as benign with exit 0. PROBE B is worse than it looks: classified as an orphan, --adopt would have overwritten the damaged manifest — the only surviving record of that run's real start time, inventory, and as-written digests — with a weaker reconstruction. And a broken manifest whose run directory is also gone was reachable by neither walk (the destination walk cannot see it; catalogRunIdsForDestination skips what it cannot parse), so it was invisible to the command whose whole job is to notice the catalog and reality disagreeing.

Fixed: manifest-unreadable and run-dir-unreadable are failing states; parsing is no longer the bar (a manifest missing id/createdAt/archives is a partial write); a catalog-wide census reports orphaned broken manifests; --adopt refuses to overwrite one and says so in skipped.

F3 (high) — a run in flight was reported as a lost archive, and would have been adopted. The brief asked specifically for this and the command failed it:

PROBE D  ok=false  state=orphan-run
         adopt dry-run → adopted=[bkp_…_inprog]  unadoptable=0

Because registration is one commit after the loop, a healthy run is in the orphan shape for its entire duration. A scheduled reconcile overlapping a scheduled run would have failed every time — and --adopt would have written a manifest describing a partial archive set and held a run that was about to register itself.

Fixed with --active-within-seconds (default 900, 0 disables): such a directory is run-in-progress, benign, and never adopted. Verified on live data rather than only in tests — tonight's real orphan run, same directory, two readings:

00:51:37Z (508 s after its last write)  → runsInProgress=1  orphanRuns=0  ok=true   rc=0
00:58:58Z (908 s after its last write)  → runsInProgress=0  orphanRuns=1  ok=false  rc=1

Detection is delayed, never suppressed. A bare --active-within-seconds is now an error rather than a silent fall-back to the default — the same shape as the bare --home this branch fixes.

F4 — a correction to 4dbe3f5's premise, and to my own first attempt. I had implemented the completeness gate as a streamed gzip check. That was wrong, and 4dbe3f5's tar -tzf is right. I tested my own check against the six real partials in tmp/:

   762930877  gzip=PASS tar=FAIL  bkp_20260727225717_1cpj14-browserplan-git-bundle.tgz
   981565810  gzip=PASS tar=FAIL  bkp_20260727235000_g7r7pg-browserplan-git-bundle.tgz
  1008337116  gzip=PASS tar=FAIL  bkp_20260728004309_gptrq1-browserplan-git-bundle.tgz
   358795032  gzip=PASS tar=FAIL  bkp_20260727220933_azrqsj-charter-critical-dbs.tgz
   247201792  gzip=FAIL tar=FAIL  bkp_20260724094911_br6s5g-charter-critical-dbs.tgz
   169869312  gzip=FAIL tar=FAIL  bkp_20260727103354_4654gj-charter-critical-dbs.tgz

4 of 6 pass a gzip test. My DecompressionStream check passed all four as intact. So the gzip envelope is not the bar; the tar listing is. I removed my helper and kept yours.

Two things I did add on top of it. First, a second truncation shape is now pinned: copyArchiveToDestination (src/backup.ts:756) writes a local archive with Bun.write(target, Bun.file(archivePath)) straight onto the final name, so a kill during the copy also leaves a truncated archive at the destination under the name it keeps. That refutes the standing claim that "the destination copy happens after createArchive+sha256 complete, so destination archives are whole" — it holds for a kill during tar -czf, not for a kill during the copy. Both shapes now have a test. Second, the refusal moved out of unresolvedArchives into a dedicated incompleteArchives: a consumer reading the former would conclude the archive's source was unresolvable, which is a different refusal with a different remedy.

F5 (low)src/reconcile.ts was missing from the build's multi-entry module list, so dist/src/reconcile.js was not emitted while dist/src/preservation.js was. No functional break (it is inlined into the bundles), but it is drift from the pattern. Added.

4. Root cause: DETECTED, NOT FIXED — and it recurred during this review

Nothing in this PR makes registration incremental or transactional; the single end-of-loop commit is untouched. It recurred while I was reviewing:

run                bkp_20260728004309_gptrq1   started 2026-07-28T00:43:09Z
22 archives at the destination, no destination manifest.json, no catalog manifest
last destination write   00:43:50.9Z
tmp partial mtime        00:44:09Z      ← exactly 60 s after the run started
tmp partial              1,008,337,116 bytes of the 1,425,710,386 seen complete elsewhere
no live backup process

Same 60-second kill signature, a fourth time. This was not caused by anything I ran — I invoked only reconcile (read-only) and the test suite against /tmp homes. That is 22 more orphaned archives on the floor tonight, all of them intact (22/22 pass gzip -t and tar -tzf), none of them catalogued or held. I did not adopt them — that is the owner's call, and the standing prohibition on mutating archives applies.

Two consequences worth writing down: this will keep recurring until registration records what a partial run actually completed, and reconcile is not a guard until something calls it on a schedule and alerts on non-zero — the installed global CLI is still 0.2.2 with no reconcile subcommand. Both are now stated in CHANGELOG.md rather than left to be inferred.

5. Things I confirmed rather than refuted

  • Live reconcile before my changes: ok=true, 22 consistent, 0 orphans, 9 empty, 3 destinations enumerated: false. Matches the reported post-recovery state exactly.
  • pruneBackups really does enumerate catalog manifests only, and really does delete the archives, the destination manifest.json and the catalog manifest (src/backup.ts:1216-1232), leaving the directory genuinely empty — so empty-run-dir as benign residue is correct, and all 9 such directories in production are in fact 0-file.
  • verifyBackup compares sha256 and nothing else (src/backup.ts:546-557) — which is exactly why the payload gate has to live in --adopt.
  • A dry run creates no run directory at all, so it cannot be mis-reported as a lost archive. Verified.
  • An unreadable destination root throws through to the CLI error boundary → ok: false, exit 1. Fails closed.
  • 123 → 132 tests, bun run check green (typecheck + tests + contracts conformance + build); main has 109, so this branch adds 23.

6. Unverifiable / left open

  • Which caller imposes the 60-second timeout is still unidentified. No cron entry and no loop is scheduled at 00:43Z; I can confirm the pattern, not name the process.
  • Whether a reconstructed archive ever matched its source is not knowable from disk, and the PR correctly refuses to claim it.
  • host and policy on a reconstructed manifest are the current machine and the current policy, i.e. plausible-looking values for fields that are not recoverable. host is non-optional on BackupManifest, so it cannot simply be omitted; reconstructedAt on the manifest plus the limitations payload is the mitigation, and I added the policy caveat to limitations. Making both nullable is a follow-up, not a blocker.
  • runs.jsonl and the catalog legitimately disagree about the 11 recovered runs (72 lines, unchanged) and reconcile does not read the ledger at all. Deliberate and defensible; now written down.
  • Non-local destinations are never enumerated, so a home whose only destinations are S3 returns ok: true having checked nothing. The JSON says so via enumerated: false; an operator wiring this to an alert should also assert destinationsNotEnumerated.

7. Merge order

Prohibitions honoured

No backup run, no prune --apply, no prune --help. Nothing deleted, expired, moved or renamed; the uncatalogued run directories and the tmp/ partials are untouched (they are the evidence). Every live invocation carried an explicit --home on the same command line. Staged secrets scan clean before both commits. No Co-Authored-By. Nothing pushed to main.

Fixes are in 8e0e635; merging on that basis.

@andrei-hasna
andrei-hasna merged commit bad7d52 into main Jul 28, 2026
1 check passed
@andrei-hasna
andrei-hasna deleted the fix/2c4541e1-reconcile-orphan-runs branch July 28, 2026 01:17
andrei-hasna added a commit that referenced this pull request Jul 28, 2026
#24 (`bad7d52`) landed on main first, so this branch integrates it rather than
preceding it. Merge, not rebase: the review record cites this branch's head
`2350364` and the two commits before it, and rewriting those SHAs would leave
three rounds of adversarial evidence pointing at commits the PR no longer
lists.

The interaction predicted in the merge-order analysis is real, and it was
machine-caught rather than reasoned about:

- `reconcile` and its flags are now declared in `src/cli/spec.ts`. Without the
  declaration the closed allowlist exits 1 on `--adopt`, silently removing
  #24's recovery path for orphaned runs. #24 also added a fourth flag the
  merge-order note did not list, `--active-within-seconds`, and the drift test
  named it.
- `tests/cli-spec-drift.test.ts` failed on the merge because #24 added a fifth
  flag-reader helper, `numberFlagStrict`, that the scanner did not scan for —
  the guard-on-the-guard doing its job. Added, longest-first so `numberFlag`
  cannot match inside it, plus a pin that reconcile's four flags are declared
  on the verb that reads them.

Conflict resolutions, all in favour of the spec-driven surface:
- `main()` keeps the pre-dispatch help/validation gate; `--home` is resolved
  with `stringFlagStrict` INSIDE the try, as #24 does, so the refusal is
  reported as JSON rather than thrown at the shell.
- `doctor`'s `commands` stays `commandNames()`. #24's hand-maintained array
  (which had gained `reconcile`) is exactly the duplicate that had already
  drifted; the derived list now includes `reconcile` because the spec declares
  it.
- `printHelp()`'s hand-written blob is gone, as on this branch; #24's
  `reconcile` help prose is ported into the spec's notes, so
  `backup reconcile --help` prints it and MUTATING is marked.

Also corrected here, since both are claims in this PR's own diff: the CHANGELOG
and docs/COMMANDS.md said `--profile preservation` was the only way to archive
secrets. An explicit `--exclude` list that omits the secrets patterns does it
too — measured — which is why the notice is keyed on the resolved list.

Verified through the real CLI, not only the suite: `reconcile --help` prints
without executing; `reconcile`, `--adopt --no-hold --destination local
--active-within-seconds 0` all accepted; `--activewithinseconds` refused with
`Did you mean --active-within-seconds?`; `doctor` lists reconcile among 22
commands.

bun test 242 pass / 0 fail / 20 files; typecheck, contracts:conformance, build
all rc=0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant