Skip to content

Stop --help from running verbs, report what excludes drop, and add a real restore proof - #22

Open
andrei-hasna wants to merge 13 commits into
mainfrom
fix/cc893efa-cli-argument-and-verify-semantics
Open

Stop --help from running verbs, report what excludes drop, and add a real restore proof#22
andrei-hasna wants to merge 13 commits into
mainfrom
fix/cc893efa-cli-argument-and-verify-semantics

Conversation

@andrei-hasna

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

Copy link
Copy Markdown
Contributor

Closes the four CLI/verification defects in todos task cc893efa. All four were found by using the
tool, and all four were reproduced against main @ 8ce32b1 before anything was changed.

No backup run, prune --apply, sources add, or restore apply touched the live registry. Every
command below ran with an explicit --home <fixture> on the same command line — no env var, no $(...)
subshell — and the redirection was confirmed by grepping the init output for the fixture path before
any mutating call. $HOME/.hasna/backup still holds its 62 manifests, and its config.json mtime
(2026-07-27 16:54:35) is unchanged.

The defects, as reproduced

Exit codes measured unpiped (cmd; rc=$?).

Before After
run --help rc=0, wrote a real archive + manifest rc=0, prints usage, manifests=0 destfiles=0
run --dryrun (typo) rc=0, real run, "dryRun": false rc=1, Unknown flag --dryrun. Did you mean --dry-run?
run --home --dry-run --home read as absent → resolved the LIVE home rc=1, Missing value for --home <path>.
run -h ignored → real run prints usage
sources add <p> --excludes x rc=0, silently stored the default excludes incl. .git rc=1, Did you mean --exclude?
<verb> --help for 5 verbs rc=0, all printed JSON (the verb ran) usage for every verb and subcommand
sources add <repo> attached 12 excludes silently DROPPING 27 file(s), 26890 bytes … .git → 26 files, dist → 1 file
plan excluded files reported nowhere preflight: 1 file(s) to archive, 27 file(s) EXCLUDED
verify latest on an incomplete backup rc=0 ok: true "checksum matched" rc=0 ok: true proof: checksum-only + doesNotProve[]
the same archive, verify --deep did not exist rc=1, 30 source files / 1 restored / 29 missing

run --home --dry-run silently targeting the live home is a new find, not in the task text: it is
the same "the home was not the one you named" shape as the incident that destroyed 139 live artifacts.
It was read off the code path (src/cli/index.ts parseArgs → stringFlagbackupHome(undefined)
defaultBackupHomeRoot()), never executed against the live home.

What changed

1 — Help and argument discipline (c6f3b71, defects 1 + 4). New src/cli/spec.ts declares the whole
command surface as data — verbs, subcommands, flags with kinds, positionals, help text, and a
mutating classification. resolveInvocation() answers help and validates arguments before the
dispatch switch, so no path can reach a verb with an argument list the spec does not accept. Refusal is
the failure mode for unknown flags, missing/empty flag values, unexpected positionals, and single-dash
args — on every verb, not just the mutating ones, because the failure mode is "the CLI ignored part
of what you typed". doctor's hardcoded command array is now derived from the spec; the array it
replaces had already drifted, omitting status, manifest, and holds.

2 — Excludes are measured and reported, and there is a profile with none (e022ebb, defect 2).
sources add walks the tree and reports each pattern with the file and byte count it drops, plus
examples, on stderr and in the result, with a source-excludes-drop-content finding.
--profile preservation attaches no excludes; combining it with --exclude is refused rather than
resolved. plan/run carry a preflight block with kept-vs-excluded totals. The counts are labelled
estimate: true — they use the inventory's matcher, not GNU tar's globber — because the authoritative
measurement is verify --deep. preservation also drops the secrets patterns and these archives are
unencrypted, so the CLI says that out loud when it is used.

3 — verify stops reading as a proof, and verify --deep is one (402a3fb, defect 3).
VerifyResult carries a required proof field plus proves/doesNotProve in the payload; the
per-check message is now recorded checksum matched (archive contents NOT inspected). A checksum-only
proof-bundle contract carries the limitation as a residualRisks entry even when it passes — the
contract is called a proof bundle and a green one was being filed as evidence of restorability.
verify --deep materializes each archive, re-checks its checksum, extracts to scratch, builds a sha256
manifest over every restored file, diffs it against the live source tree, and clones any recovered git
object database to compare ref SHAs and tree hashes against git ls-remote. Cloning is the load-bearing
git step: a shallow bundle passes git bundle verify and then fails to clone.

And it stays honest about the gap: when no source tree is available — the normal state of a pre-deletion
archive, and exactly when the proof matters most — it reports completeness: "not-verified" and names
the archives in doesNotProve instead of returning a green result that reads as a completeness proof.
--require-source-diff turns that into a failure.

Evidence

bun test --timeout 60000        rc=0   120 pass / 0 fail / 9 files   (main baseline: 80 pass)
bun run typecheck               rc=0
bun run contracts:conformance   rc=0
bun run build                   rc=0

Fail markers cross-checked per the colorized-output caveat: grep -c '✗' = 0 and grep -c '(fail)'
= 0, and the summary line reads 120 pass / 0 fail.

Mutation-proven, not assertion-counted. Each regression suite was run against planted old behaviour
and had to go red:

planted mutation result
main's src/cli/index.ts restored (help executes, flags ignored) rc=1, 0 pass / 16 fail
--profile silently ignored → falls back to DEFAULT_EXCLUDES rc=1, 5 pass / 5 fail
exclude measurement disabled (excludeReport early-returns) rc=1, 6 pass / 4 fail
--deep degenerated to checksum-only (no diff, no git proof) rc=1, 7 pass / 6 fail
plain verify reverted to "checksum matched" with no claims rc=1, 10 pass / 3 fail
all restored rc=0, all green

Two separate plants for defect 2 because neither alone covers both halves — one leaves the reporting
intact, the other leaves the profile intact.

The defect-2 acceptance test is the check that caught the original bug: stage a tree containing a
real .git and dist, register it via the documented preservation path, run, restore, and assert a
zero-difference sha256 manifest on both sides ({staged, restored, missing: [], extra: [], corrupt: []} as one object, so the counts appear in the failure diff). It then recovers the history —
restored git rev-parse HEAD equals the live HEAD and git fsck exits 0 — because a hash-identical
.git is not by itself proof the history is usable.

The git-remote comparison was verified against the real GitHub API, not only against a fixture:
verify latest --deep --git-remote hasna/backup expanded to https://github.com/hasna/backup.git,
fetched 30 refs via git ls-remote, and correctly reported 0/30 matched, 29 missing locally, 1 mismatched for an unrelated fixture repository (rc=1). The suite substitutes a local bare repo for
the endpoint, since git ls-remote takes a path and a URL through the same code path.

Existing tests touched, and why

Nothing was weakened to a substring match.

  • 6 assertions in tests/backup.test.ts read the full addSource warning list with toEqual([...]).
    The new exclude finding is appended last so positional warnings[0] readers still see the coverage
    finding, and a gitFindingCodes() helper filters the new code at the five exact-list sites. One site
    additionally asserts the new code is present, so the interaction stays pinned in that file.
  • tests/backup.test.ts:1111 asserted "checksum matched". That string was the defect; the
    assertion now pins the new wording and proof: "checksum-only".
  • tests/live-home-guard.test.ts:253 asserted that --confirm-home " " produces the deep
    LiveBackupHomeRefusedError naming the live home. Argument validation now rejects a whitespace-only
    path value one layer earlier. Still refused, nothing deleted, and the deep guard still refuses blank
    independently (confirmsLiveHome trims); the two path-shaped confirmations still assert the
    home-naming message. Split into its own expectation rather than deleted.
  • tests/mcp.test.ts tool-name lists gained backup_verify_deep.
  • tests/contracts.test.ts fixture gained the now-required proof field.

Bugs found while doing this

  • A correctness bug in my own first version, caught by an existing test: the exclude-drop finding
    fired for inventory-only sources, which claims files are missing from an archive that is never
    written. excludeReport() now returns an empty report unless mode === "archive"; pinned by its own test.
  • A harness bug affecting this repo's existing test helpers: on Bun 1.3.14 process.exitCode = undefined is a no-op once the code has been set (verified: process.exitCode = 1; process.exitCode = undefined still reads 1). tests/backup.test.ts:1112 and :1320 both reset with
    undefined, so after the first CLI refusal in that file every later exit-code read is stale. The new
    helpers reset to 0. This produced 7 phantom failures in my first run before I found it.

Overlap with #21

#21 (1f202268, preservation prune guard + --include-git-history) is open and touches some of the same
files. The scope split held: this PR adds --profile/reporting only. It does not add, edit, or
reference --include-git-history, and it does not touch prune, preservation markers, or
src/preservation.ts. Expect textual conflicts in src/config.ts (addSource), src/cli/index.ts
(handleSources), src/types.ts, CHANGELOG.md, and docs/COMMANDS.md — adjacent hunks, no semantic
overlap. Whoever merges second resolves; the two flags compose, since preservation excludes nothing and
therefore keeps git history by construction.

Not done

🤖 Generated with Claude Code


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


Rebase onto d32e5cb, and the review's blockers closed

Appended after the adversarial review (verdict, REJECT). The
text above is the original submission and is left intact; the retractions below say plainly which of
its claims did not survive.
Timestamps are UTC+0300 (EEST); this box is UTC+0300.

Rebased from base 8ce32b1 onto d32e5cb (PR #21). git merge-base --is-ancestor d32e5cb HEAD
→ rc=0. PR #22 is now MERGEABLE (was CONFLICTING).

Retractions from the original submission

  1. "Textual conflicts with PR Refuse to prune preservation-marked archives, and let a source capture git history #21 … adjacent hunks, no semantic overlap" was WRONG, exactly as the
    review found. There were three genuine semantic collisions, not zero: the two flags below, plus
    --preserve-markers=.
  2. "Commit 2 conflicts only in src/config.ts, src/cli/index.ts, src/types.ts, CHANGELOG and
    docs" undercounted.
    Commit e022ebb conflicted in five files including src/backup.ts and
    src/runtime.ts — the two the notes claimed were kept clear of Refuse to prune preservation-marked archives, and let a source capture git history #21.
  3. "--git-remote … reported 0/30 matched … correct, since the fixture repo is not hasna/backup"
    was a misread of its own output.
    The review reproduced the same shape against a clone of the real
    hasna/backup, so "different repo" never explained the numbers. A namespace mismatch did. Fixed
    below.
  4. The "mechanical" label on the remaining conflicts was too generous. Git auto-merged both sides'
    additions of DEFAULT_EXCLUDES into src/runtime.ts twice (import block and re-export block) and
    produced duplicate identifiers. bun install failed rc=1 until deduped. Those were not textual
    conflicts — git resolved them silently and wrongly.

BLOCKER 1 — #21's flags are declared, and the drift is now a test failure

main reads include-git-history and preserve-markers; the spec at 402a3fb declared neither, and
this branch makes flags a closed allowlist that fails closed. The printHelp() conflict is the same
hazard mirrored: #21 extended that blob and this branch deletes it wholesale, so taking this branch's
side is what dropped them. Their prose moved into the spec's notes.

Measured through the CLI, --home <fixture> on the same command line, redirection confirmed before any
mutating call:

invocation rc result
sources add <p> --include-git-history 0 excludeProfile=default-with-git-history, 11 patterns, no .git
policy set --preserve-markers a,b 0 preserveMarkers set
policy set --preserve-markers= 0 extras cleared — #21's documented clear form
policy set --preserve-markers (bare) 1 Missing value for --preserve-markers

The third row is the collision the review did not have to find because it fell out of the rebase: this
branch's empty-value refusal broke #21's documented clear form. Fixed with a declared
FlagSpec.emptyValueMeansNone that permits the exactly empty string only — whitespace-only stays
refused, so nothing that merely looks empty can read as an opt-in.

tests/cli-spec-drift.test.ts generalises the review's by-hand verb×flag sentinel probe into a static
scan: every flag any handler reads must be declared, or the test names it.

The composition decision — --profile × --include-git-history

The review declined to author this and merge it, on the grounds that it is the code path that lost 68
files and 139 artifacts. I authored it; I am not merging it. Here it is, stated explicitly so it
can be attacked.

The model: --profile/--exclude choose a BASE list. --include-git-history is a SUBTRACTIVE
MODIFIER on that base — it removes the patterns that drop .git, and nothing else.

Every cell falls out of that one rule. All eleven measured through the real CLI:

invocation rc resolution
(nothing) 0 default, 12 patterns
--include-git-history 0 default-with-git-history, 11 (no .git)
--profile default 0 default, 12 — identical to (nothing)
--profile default --include-git-history 0 default-with-git-history, 11 — identical to the bare modifier. This was the undefined cell.
--profile preservation 0 preservation, none
--profile preservation --include-git-history 0 preservation, none — modifier is a no-op
--exclude dist 0 explicit, dist
--exclude dist --include-git-history 0 explicit, dist (no .git pattern to subtract)
--exclude .git --include-git-history 1 refused — #21's rule kept verbatim
--profile default --exclude dist 1 refused
--profile preservation --exclude d --include-git-history 1 refused — profile+exclude beats the modifier

Two properties are load-bearing, each pinned by a test:

  1. --profile default is a pure synonym for passing nothing, with or without the modifier. If
    naming the default changed behaviour the profile name would be a lie, and --include-git-history
    alone already resolved to default-minus-.git in shipped main. Refusing the explicit spelling
    while honouring the implicit one would be a regression dressed as strictness.
  2. The modifier never removes a secrets pattern. Verified by measurement, not assumed:
    DEFAULT_EXCLUDES_WITH_GIT_HISTORY removes exactly ['.git'] (12 → 11) and keeps .env,
    .env.*, .secrets, .connect, *.pem, *_TOKEN, *_KEY. A new SECRETS_EXCLUDES +
    assertKeepsSecrets() makes the modifier fail closed if a future change to excludesGitDir
    ever broadened the subtraction. So this does not widen what gets archived beyond the .git
    directory the caller asked for; --profile preservation remains the only way to archive secrets,
    and it still says so out loud.

Why the asymmetry (the modifier silently trims a default base but refuses an explicit one):
with default the operator never named the patterns, so subtracting one is the tool honouring the
flag. With --exclude they did name them, so subtracting one would discard a written instruction —
#21 already ruled that a refusal, and resolving it either way produces a source that lies.

The recorded label is default-with-git-history, not default. Alternative rejected: record it
as default. The stored list is 11 patterns, not the 12 that label denotes, and sources add prints
the profile name
— so labelling it default would print a false statement in the very report this
branch added to stop the tool making false statements about what it archived. It is not selectable via
--profile (one spelling per meaning); passing it is refused with a message naming
--profile default --include-git-history.

Second alternative rejected: make --profile default --include-git-history a refusal, on the
grounds that a profile "sets the exclude list" so a modifier contradicts it. Rejected for reason (1)
above — it would make naming the default a breaking change and regress #21's shipped behaviour. It is
the defensible runner-up, and it is the one a reviewer should push on.

BLOCKER 2 — a repeated flag no longer collapses

BEFORE  sources add <src> --exclude '.env' --exclude '*.pem'   rc=0
          stored excludes = ['*.pem']        <- '.env' discarded, no warning
        run                                                    rc=0
          tar tzf <archive>:  src/  src/.env  src/app.js       <- the .env IS archived

AFTER   sources add <src> --exclude '.env' --exclude '*.pem'   rc=0
          stderr: patterns=.env,*.pem        stored = ['.env', '*.pem']
        run                                                    rc=0
          tar tzf <archive>:  src/  src/app.js                 <- .env and .pem both absent

The parser records every occurrence and the spec decides what a repeat means. The four comma-separated
list flags accumulate (sources add --exclude, policy set --preserve-markers,
verify --compare-source, aws --regions), so --exclude a --exclude b--exclude a,b. Every
other flag refuses
, because there is no honest resolution for --home /a --home /b and quietly
taking the last one is the shape that destroyed 139 artifacts:

status --home <a> --home <b>   rc=1  --home was given 2 times ("<a>", "<b>"), and it accepts one value.

Per the review's instruction, every other repeatable flag was audited rather than just --exclude:
every declared flag on every verb and subcommand was probed with a repeat. All refuse except the four
accumulators. The single exception is the help verb, which resolves before validation by design
because it executes nothing — measured: help --home a --home b prints help at rc=0 and writes nothing.
That audit is a test, driven off the spec, so a flag added later is covered without editing it.

The three further defects

--deep was blind to symlinks and empty directories — the review's most damaging finding, since
the half designated authoritative was the wrong one.

BEFORE  sources add --exclude 'danger-link'   stderr: DROPPING 1 file(s), 13 bytes
        verify latest --deep                  rc=0 ok=true completeness=verified-against-source missing=0
AFTER   verify latest --deep                  rc=1 ok=false missing=1  diff.missing=['danger-link']

A symlink is fingerprinted by its target string, not followed — otherwise a symlink and a regular
file of identical content compare equal, and the walk could escape the tree or loop. Only empty
directories are recorded; a non-empty one is implied by its contents. MANIFEST_COMMAND now covers
-type l and -type d -empty, because it ships in the result for a human to redo by hand and
publishing a command narrower than the measurement would restate the defect as documentation.

--git-remote false-negatived non-mirror clones — a namespace mismatch, as the review established
by refuting its own stronger hypothesis. Refs are now compared by logical name, with
refs/remotes/<remote>/<X>heads/<X>; the remote name is discarded, since the restored clone's
remote could be called anything. Each logical name maps to a set of local SHAs, so a local branch
ahead of its upstream still satisfies the remote's SHA — that commit is in the restored object
database, which is what the proof asks.

Two things worth reading carefully here:

  • My first version of this fix introduced the same class of bug it was fixing. Returning
    undefined for unrecognized namespaces dropped GitHub's refs/pull/<n>/head, which a --mirror
    holds verbatim — 26 of 35 refs on this repository — making a faithful mirror report 9/35 with 26
    "missing". Caught only by measuring against the real remote rather than the fixture. Unrecognized
    namespaces now fall back to the literal refname. Pinned by its own test.
  • A working clone genuinely lacks those PR refs (git clone never fetches them), so reporting them
    as missing is true but says nothing about the archive. They now go to
    refsMissingLocallyAdvisory: reported and counted in the message, not failed on. Only
    refs/heads/* and refs/tags/* count as history a restore must reproduce.

Measured against the real GitHub API, rc=0: working clone ok=true, 9/35 matched, 0 history refs missing, 26 advisory; --mirror ok=true, 35/35, 0 advisory. Before: 1/32, ok=false.

--target exposure — the review's ruling was warn and refuse, not cleanup, and that is what
this does. A group- or world-accessible target is refused before anything is written, naming the
path, the mode, the chmod 700 that fixes it, and --allow-insecure-target.

BEFORE  chmod 0777 <t>; verify latest --deep --target <t>   rc=0, no warning
          after: <t> still 0777, restored files 0664 incl. a credentials fixture
AFTER   same invocation                                      rc=1, nothing restored
        with --allow-insecure-target                         rc=0 + 2 TARGET: notices on stderr

Refusing rather than chmod-ing is deliberate: silently changing the permissions of a directory the
caller named is the same class of surprise as silently deleting it. A target the command creates is
chmod-ed to 0700 explicitly, since mkdirSync's mode is masked by the umask (verified under
umask 000). Every named target gets a TARGET: notice that it is not removed and may hold plaintext
credentials — silence about that was the defect.

Judgements the review settled, kept as ruled

The exclude estimate stays; fail-closed on read-only verbs stays (blast radius empirically
zero); ok:true with completeness: not-verified stays. Not relitigated.

Verification

bun test --timeout 60000        rc=0   180 pass / 0 fail / 16 files / 1380 expect()   (main baseline 80)
bun run typecheck               rc=0
bun run contracts:conformance   rc=0
bun run build                   rc=0

Marker caveat, since FORCE_COLOR=3 is set and this repo prints no per-test markers on a green
run: grep -c '(pass)' = 0 and grep -c '✓' = 0 on a fully passing log, so the ANSI-stripped
summary line is authoritative and both fail-marker forms are checked — grep -c '✗' = 0 and
grep -c '(fail)' = 0.

Every new fix is mutation-proven red against its planted old behaviour, with needle counts asserted
changed:

plant result
spec without #21's two flags (402a3fb state) 1 pass / 3 fail
flat-Record parser, repeat refusal off 1 pass / 6 fail
isFile()-only manifest walk 3 pass / 7 fail
literal ref comparison 3 pass / 7 fail
target mode never inspected, no notice 3 pass / 5 fail
D1 replant (8ce32b1:src/cli/index.ts) on the rebased tree 0 pass / 16 fail
D3 replant (comparePath=undefined + proveGit off) on the rebased tree 20 pass / 13 fail

In each case the tests that stayed green are the ones that do not depend on the planted code path —
the anti-vacuity scanner check, the pure spec assertions, MANIFEST_COMMAND. One plant attempt
failed its own assertion and did not write the file, so that run's rc=0 was the unplanted tree;
recorded because it would otherwise read as evidence.

Live registry untouched

Fingerprint recipe reconciled with the brief first: 7e4d2912f84e55ee369ad7fd1dfcd5c2 is md5 over the
sorted md5sum lines (hash + filename); md5 over the hashes alone gives d7119a19…. Identical
before and after all work:

manifests_count=62
manifests_md5=7e4d2912f84e55ee369ad7fd1dfcd5c2
config_sha256=60ffe3b3a191e47da58db9c58489cebc40cb7498ebafb725b87d04eb24eaf325
config_mtime=2026-07-27 16:54:35.753425863 +0300

swarm-final-archive-20260727 still present in config.json. No backup run against real data, no
prune --apply, no archive deleted, expired, moved or renamed. Every command carried an explicit
--home <fixture> on the same command line, with redirection confirmed by grepping the init
output for the fixture path before any mutating call, in the same process.

One measurement I could not reproduce: the review's sha256 measure 698aaf2ee69dc21e…. My recipes
give 2f6951f9… (hash+name lines) and 7ff933dc… (hashes only). Its recipe is undocumented, so I rely
on the three measures that do reproduce rather than claim agreement I cannot show.

Overlap with PR #23

No conflict. PR #23 (fix/9c93913b-prune-guard-wiring-tests, now at 85c0155) touches
src/backup.ts and tests/prune-guard-wiring.test.ts. Its src/backup.ts hunks are all inside
pruneBackups (lines 318–469); mine are in the import block, runBackup, verifyBackup, and
helpers — disjoint. A trial merge of #23 into this branch auto-merged at rc=0 ("Automatic merge
went well"), and the trial branch was discarded. Correction to the brief I was given: #23 touches
tests/prune-guard-wiring.test.ts, not src/preservation.ts.

Not done

NOT MERGED — this goes back for a second adversarial review pass. No release or version bump.
verify --deep is still not wired into any loop or into doctor. The completeness mislabel on a
failed extract (a92b20ec) and the confirmsLiveHome coverage regression (03e39cbf) are still open
as their own tasks, untouched here.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Adversarial review of PR #22 — VERDICT: REJECT (do not merge as-is)

Reviewed at 402a3fb from a fresh clone at $HOME/.hasna/repos/worktrees/backup/cc893efa-adv-pr22
(the shared checkout is gutted — filed as 7bf35997). Timestamps below are UTC+0300 (EEST)
unless marked UTC.

The PR's core thesis reproduces and holds. The headline defect is real, the fixes bind, and
the mutation evidence survives re-planting. It is rejected for a merge-blocking semantic
conflict with the already-merged PR #21
, plus four measured defects.


Merge order — already settled, not by me

PR #21 merged at 2026-07-27 22:43:50 UTC (2026-07-28 01:43:50 EEST) as d32e5cb. main has
moved off this PR's base 8ce32b1. So #22 lands second and must be rebased onto d32e5cb.

BLOCKER 1 — "textual conflicts, no semantic overlap" is refuted; a naive resolution silently drops #21's guard

This is exactly the failure the merge-order warning anticipated, and it is now measured, not
hypothetical.

main (post-#21) reads two flags at runtime:

  • src/cli/index.ts:179includeGitHistory: booleanFlag(parsed, "include-git-history"), feeding
    resolveExcludes (src/config.ts:439-440)
  • src/cli/index.ts:228preserveMarkers: stringFlagStrict(parsed, "preserve-markers"), feeding
    the preservation-marker policy replacement (src/config.ts:348-383) — i.e. the configuration
    surface of the prune preservation guard Refuse to prune preservation-marked archives, and let a source capture git history #21 exists to provide

src/cli/spec.ts at 402a3fb declares neither (git show 402a3fb:src/cli/spec.ts | grep -E 'include-git-history|preserve-markers' → rc=1, no output). Since #22 makes the flag list a
closed allowlist that fails closed, both of #21's flags will exit 1 once #22 lands.

The single conflict in src/cli/index.ts is printHelp() — which #21 extended with those two
flags and #22 deletes wholesale. Taking #22's side (the obvious resolution) is what drops them.

The rebase is worse than reported. Commit 2 (e022ebb) conflicts in five files, including
src/backup.ts and src/runtime.ts — the file the PR notes say were deliberately kept clear of
#21. Those two, plus src/cli/index.ts and docs/COMMANDS.md, are mechanical (import lists; one
line passing both profile and includeGitHistory to addSource).

src/config.ts (6 conflicts) is not mechanical: it leaves two competing exclude resolvers,
resolveExcludes({excludes, includeGitHistory}) from #21 and resolveSourceExcludes({excludes, profile}) from #22, that must become one. The composition matrix needs an owner's decision, and
one case has no defined answer today:

invocation resolution
--exclude a,b + --include-git-history #21's rule: refuse if a pattern drops .git
--profile preservation + --include-git-history compose (preservation excludes nothing) — as the PR notes state
--profile X --exclude Y #22's rule: refuse
--profile default + --include-git-history undefined — needs excludeProfile semantics decided

I am not authoring that resolution and merging it: it sits in the exact code path that lost 68
files and 139 artifacts, and a reviewer's own unreviewed design change would have had no
adversarial pass at all.

BLOCKER 2 — a repeated flag is silently collapsed, and it archives a secret in plaintext

Same defect class the PR closes ("an argument list this function cannot fully account for does not
execute"), left open, with a credentials consequence.

sources add <src> --exclude '.env' --exclude '*.pem' --home <fixture>     rc=0
  stderr: excludes: profile=explicit patterns=*.pem
  stored: excludes = ['*.pem']            <- '.env' silently discarded, no warning
run --home <fixture>                                                      rc=0
  tar tzf <archive>:  src/  src/.env  src/app.js   <- DB_PASSWORD archived in plaintext

parseArgs (src/cli/index.ts:381-405) writes flags into a flat Record, so the last occurrence
wins. The exclude report then prints a confident "DROPPING 1 file(s)" naming only one of the two
patterns the operator asked for. Fix is small: refuse a repeated flag, or accumulate for
list-valued flags.

DEFECT 3 — verify --deep reports 0 missing / verified-against-source on an archive provably missing a symlink

hashManifest (src/verification.ts:296-322) walks with withFileTypes and does
if (!entry.isFile()) continue, so symlinks and empty directories are never hashed on either
side
. They cannot appear as missing, extra, or corrupt.

source: 1 regular file + 2 symlinks; 'danger-link' excluded from the archive
sources add ... --exclude 'danger-link'   stderr: "DROPPING 1 file(s), 10 bytes"
verify latest --deep                      rc=0  ok=True
    completeness=verified-against-source  missing=0 extra=0 corrupt=0
    restoredFiles=1  sourceFiles=1

The two halves of this PR contradict each other on one tree: sources add says one file is
dropped; the measurement the PR designates as authoritative over the estimate says nothing is
missing. That is the antipattern the PR exists to kill, narrowed to symlinks and empty dirs.
Not a regression from main (which had no --deep), but it undercuts the central claim, and it
should at minimum be named in doesNotProve.

DEFECT 4 — --git-remote false-negatives on a non-mirror clone (and the PR's own evidence for this path was misread)

proveGit compares refnames literally. A working clone stores upstream branches under
refs/remotes/origin/<X>; git ls-remote reports refs/heads/<X>. Measured on a clone of
this repo:

verify latest --deep --git-remote hasna/backup      rc=1  ok=False
  1/32 ref SHAs matched, 31 missing from the restored copy, 9 present locally but not on the remote
  e.g. refs/heads/fix/cc893efa-...        reported MISSING locally
  while refs/remotes/origin/fix/cc893efa-...  is present with the identical SHA 402a3fb80e

The same ref is double-counted as both "missing from the restored copy" and "present locally but
not on the remote". archiveOk fails the archive on git.ok === false, so a faithful,
hash-identical archive of a working clone reports FAILED — and a proof tool that cries wolf gets
ignored.

I also refuted my own stronger hypothesis by measurement: a true --mirror capture gives
32/32 matched, ok=True, rc=0, so the documented remedy path is correct. The gap is only the
shape --profile preservation newly makes easy.

Two corrections to the PR record, both about evidence quality rather than code:

  1. The note reads the fixture result — "0/30 matched, 29 missing locally, 1 mismatched, rc=1 —
    correct, since the fixture repo is not hasna/backup" — as confirmation. My run against a
    clone of the actual hasna/backup produced the same shape (1/32, 31 missing). "Different repo"
    did not explain those numbers; the namespace mismatch did. The conclusion rested on a misread
    of its own output.
  2. The note says the blank---confirm-home case "still refuses independently (confirmsLiveHome
    trims)". True of the code (src/config.ts:224-225) but no longer true of the suite: the
    only test that exercised that path through the CLI now stops at the new argument validation, and
    confirmsLiveHome appears in tests/ only inside a comment. That is a coverage regression.

DEFECT 5 — verify --deep --target <path> leaves restored content, silently, at whatever permissions the path already had

The judgement is right that deleting a caller-named path would make a verification command
destructive. But it is currently silent, and it does not protect the path:

mkdir -p <target>; chmod 0777 <target>
verify latest --deep --target <target> --home <fixture>    rc=0, no stderr warning about the target
after: <target> still 0777;  restored files mode 664, incl. creds.env.sample

resolveTarget passes mode: 0o700 to mkdirSync, which is a no-op on an existing directory.
Ruling: warn loudly (name the path, say it is not removed, say it may hold plaintext
credentials) and refuse a group/world-accessible target unless an explicit override is passed.
Not automatic cleanup.


Rulings on the five judgements put up for attack

  1. Exclude counts as an estimate — SAFE, keep. estimate: true ships in the payload, the
    stderr text names verify --deep as authoritative, and the drop itself is real even if a count
    is off. Rejecting tar -cf /dev/null -v because it reads every byte is the right trade. One
    caveat: the estimate is only safe while the authoritative measurement is sound — see DEFECT 3,
    where the estimate was right and --deep was wrong.
  2. Fail closed on read-only verbs — CORRECT, and the blast radius is empirically zero here.
    Audited every caller on this machine: loops.db has 5 rows matching backup in
    target_json, none of them CLI invocations ("backup exists", "backup of", "backup paths"); the
    3 status='active' loops touch iapp-geo/communities/seo, not this CLI. All 34 workflow_specs
    hits are archived prose task descriptions — the backup verify --json --contract string is a
    proposal inside a task body, not a call. No crontab or systemd user unit invokes it.
    Separately, I probed every verb×flag pair with a --zzz-sentinel forcing refusal before
    execution: every flag any handler reads is declared on that verb, and nothing else is. No
    spec drift within the PR
    — the drift is against Refuse to prune preservation-marked archives, and let a source capture git history #21 (BLOCKER 1).
  3. ok: true with completeness: "not-verified" — I AGREE, keep it. Splitting the claims is
    right: ok answers "did what I measured pass", completeness answers "what did I measure", and
    --require-source-diff exists for callers needing the stronger claim. The alternative would
    fail every pre-deletion archive, which is the main use case, and a check that always fails gets
    ignored just as fast as one that always passes. Related mislabel worth fixing separately: in the
    source-name-collision case one archive never extracts, yet completeness still reads
    verified-against-source (summarize counts only extracted archives as comparable). ok is
    false, so nothing passes wrongly.
  4. Needs a warning AND a refusal, not cleanup. See DEFECT 5.
  5. CONFIRMED — the live registry is untouched. Three independent measures, before and after
    the entire review: config.json sha256 60ffe3b3a191e47d… unchanged; manifests/ = 62
    entries; sha256 over the sorted hashes of every file in manifests/ =
    698aaf2ee69dc21e… unchanged. config.json mtime 2026-07-27 16:54:35 EEST (13:54:35 UTC),
    exactly as reported. Every command carried an explicit --home <fixture> on the same command
    line, redirection confirmed by grepping the fixture path out of init output before any
    mutating call. No backup run against real data, no prune --apply, no archive deleted.

What DOES hold (verified, not taken on trust)

  • Headline reproduced. Same archive, same moment, default excludes dropping .git + dist:
    verify latestrc=0 ok=true proof=checksum-only; verify latest --deeprc=1
    ok=false
    , restoredFiles=4 sourceFiles=36 missing=32 extra=0 corrupt=0. Plain verify passes
    on an archive missing 32 of 36 files.
  • D1 refusals bind, with the fixture's manifest count and destination file count unchanged
    across all of them: run --help rc=0 prints usage + MUTATING, writes nothing · run --dryrun
    rc=1 "Did you mean --dry-run?" · run --dry-run --extra rc=1 · run stray-positional rc=1 ·
    run -h rc=0 help · run --home --dry-run rc=1 "Missing value for --home" · run --home rc=1 ·
    --home= and --home=' ' rc=1 "Empty value" · --dryrun=1 rc=1 · -home <p> rc=1
    "Flags use two dashes". Inline --home=<path> resolves correctly.
  • --git-remote genuinely hits the GitHub API, not a stub: expands to
    https://github.com/hasna/backup.git, fetches 32 refs — matching my independent
    git ls-remote count exactly — and a nonexistent repo propagates the server's real
    remote: Repository not found at rc=1.
  • Mutation evidence survives re-planting (two plants, needle counts asserted changed both
    times, so no silently-skipped sed):
    • plant 8ce32b1:src/cli/index.ts (resolveInvocation/validateArguments/dashArgs counts
      3/2/… → 0/0/0) → bun test tests/cli-arguments.test.ts --timeout 60000 rc=1, 0 pass / 16
      fail
      , 16 named failures, all substantive (run --help returned a real manifest;
      run --dryrun exit 0). One failure is a 60005ms timeout, i.e. the documented --timeout hole
      — still a genuine red.
    • plant comparePath = undefined + proveGit disabled in src/verification.ts (needles 1→0
      and 1→0) → bun test tests/verify-restore-proof.test.ts rc=1, 7 pass / 6 fail, matching
      the reported 7/6 including "THE DEFECT: plain verify passes …".
    • both files restored, git status --porcelain empty.
  • No assertion was weakened. All 6 gitFindingCodes sites keep an exact toEqual([...]) on
    the filtered list, and the filtered code is separately pinned with toContain at
    tests/backup.test.ts:1583. The verify message went from toBe("checksum matched") to
    toBe("recorded checksum matched (archive contents NOT inspected)") plus a new
    expect(verification.proof).toBe("checksum-only") — strengthened. mcp.test.ts and
    contracts.test.ts changes are additive. live-home-guard.test.ts splits the blank case out and
    still asserts refusal (see the coverage caveat in DEFECT 4).
  • Suite at 402a3fb: bun test --timeout 60000 rc=0, 120 pass / 0 fail, 9 files, 1090
    expect() calls
    . Fail-marker cross-check per the FORCE_COLOR caveat: ANSI-stripped grep -c '✗'
    = 0 and grep -c '(fail)' = 0, summary line 120 pass / 0 fail.
  • Source-name collision ("My Repo" / "my-repo") does not produce a false pass: --deep
    rc=1 with checksum mismatch: the archive at the destination is not the one that was written.

To merge

  1. Rebase onto d32e5cb; unify the two exclude resolvers; decide --profile default +
    --include-git-history.
  2. Declare --include-git-history (sources add) and --preserve-markers (policy set) in
    src/cli/spec.ts, and add a test that fails if a flag a handler reads is not declared — the
    --zzz-sentinel probe above generalises into one.
  3. Refuse repeated flags.
  4. Re-run the suite and the D1/D3 replants on the rebased tree.

Nothing was pushed; origin/fix/cc893efa-cli-argument-and-verify-semantics is still 402a3fb.

`backup run --help` performed a REAL backup: --help was parsed into an unused
flag and the `run` case ran anyway. It happened three times on 2026-07-27
(bkp_20260727104035_3xihad, bkp_20260727104414_l8kxir,
bkp_20260727104643_06r19b), and because retention keeps one backup per calendar
day per source, each accidental run evicted the earlier same-day archives from
the daily slot — asking for help could destroy a restore-proven backup.

The same root cause made every unknown flag a silent no-op. `run --dryrun`, one
character from the safety flag, performed a live backup with dryRun: false.
`sources add <path> --excludes node_modules` (plural) was accepted and stored
the default exclude list including `.git`. And a flag whose value went missing
read as absent, so `run --home --dry-run` resolved $HOME/.hasna/backup — the
live installation, the same shape as the incident that destroyed 139 artifacts.

Help is now resolved before dispatch and never executes. Argument validation
runs before dispatch too and fails closed: unknown flags, missing or empty flag
values, unexpected positionals, and single-dash arguments all refuse with exit 1
and name the accepted flag. Every verb and subcommand has help, and `doctor`
derives its command list from the same spec instead of a hand-maintained copy
that had already drifted (it omitted status, manifest, and holds).

The surface is declared as data in src/cli/spec.ts so help text and validation
cannot disagree with each other.

Refs todos cc893efa (defects 1 and 4).
`sources add` attached twelve default excludes without saying what they cost.
On 2026-07-27 that turned a 155-file capture of a repository into 87 restorable
files — 68 missing, all under a staged `dist/`, 0 extra and 0 corrupt — while
`backup verify` reported ok, because verify only compares the archive against
its own recorded checksum and never opens it. The workaround at the time was
renaming the staged directory, which is not a fix. For repository preservation,
`.git` and `dist` are exactly the content you must keep.

Three changes, none of which relies on remembering anything:

- `sources add` measures the tree and reports each pattern with the file and
  byte count it drops, plus examples, on stderr and in the result, together with
  a `source-excludes-drop-content` finding. The counts carry `estimate: true`:
  they use the inventory's matcher rather than GNU tar's globber, so the
  authoritative measurement stays `verify --deep`.
- `--profile preservation` attaches NO excludes. Combining it with `--exclude`
  is refused rather than resolved. It also drops the secrets patterns, and these
  archives are unencrypted, so the CLI says that out loud when it is used.
- `plan` and `run` report a `preflight`: files and bytes to archive, files and
  bytes excluded, which patterns drop them, and per-source detail. `plan` is
  documented as the pre-flight and previously reported the excluded files
  nowhere at all.

An inventory-only source reports no drop — it writes no archive for anything to
be dropped from. Five assertions in tests/backup.test.ts read the full
`addSource` warning list exactly; they now filter the new code so each stays
about the git classification it was written to pin.

Refs todos cc893efa (defect 2).
`backup verify` compares each archive at the destination against the sha256
recorded when it was written, and never opens it. That catches media rot and a
truncated upload. It cannot catch content that was never archived — and on
2026-07-27 it returned ok for a backup of a repository missing 68 files, because
the archive was a byte-perfect copy of an incomplete capture. This is vacuous
verification in its purest form: a check that passes while measuring nothing it
is read as measuring, whose output wording invites the wrong conclusion.

Two halves.

Say what it measured. VerifyResult now carries a REQUIRED `proof` field
(`checksum-only`), plus `proves` and `doesNotProve` lists that ship in the
payload rather than living in a doc nobody reads at the moment of the decision.
The per-check message changed from "checksum matched" to "recorded checksum
matched (archive contents NOT inspected)". The CLI prints the boundary on
stderr. A checksum-only proof-bundle contract carries the limitation as a
residualRisks entry EVEN WHEN IT PASSES — the contract is literally called a
proof bundle and a green one was being filed as evidence of restorability.

Then measure it. `verify --deep` does what every real proof in that wave did by
hand: materialize each archive, re-check its checksum, extract it to a scratch
tree, build a sha256 manifest over every restored file, diff it against the live
source tree, and where the restored tree carries a git object database clone from
it — the step that catches a shallow or truncated repository, which `git bundle
verify` does not — comparing ref SHAs and tree hashes against `git ls-remote`.
Non-zero exit on any missing or corrupt file.

And stay honest about the gap. When no source tree is available — the normal
state of a pre-deletion archive, and exactly when the proof matters most — it
reports `completeness: "not-verified"` and names the archives in `doesNotProve`
rather than returning a green result that reads as a completeness proof.
`--require-source-diff` makes that a failure for callers needing the stronger
claim. A caller-supplied `--target` is never removed; only a scratch directory
the verifier created under the backup home's tmp.

Deep verification lives in a new src/verification.ts, and the claim strings in
src/lib/proof-claims.ts so both paths can state their own boundary without an
import cycle. New MCP tool backup_verify_deep, grouped with inspection.

Refs todos cc893efa (defect 3).
…ext drift

The spec introduced here is a CLOSED ALLOWLIST that fails closed, so a flag a
handler reads but the spec does not declare exits 1. PR #21 landed
`sources add --include-git-history` and `policy set --preserve-markers` on main
while this branch was open and the spec declared neither, so rebasing without
this commit would have made both exit 1 — silently removing the entire
configuration surface of #21's prune preservation guard.

The `printHelp()` conflict is the same hazard in the other direction: #21
extended that blob with both flags and this branch deletes it wholesale in
favour of spec-derived help, so taking this branch's side is what drops them.
Their help prose moves into the spec's notes rather than being lost.

- Declare `--include-git-history` on `sources add` and `--preserve-markers` on
  `policy set`, carrying #21's explanatory text into the spec notes.
- `--preserve-markers=` (inline, empty) is #21's documented way to clear the
  per-installation markers, which this branch's empty-value refusal broke. Add
  `FlagSpec.emptyValueMeansNone` so the exception is declared on the flag rather
  than tested by name in the validator. It permits the exactly-empty string
  only: a bare `--preserve-markers` is still "missing value" and a
  whitespace-only value is still refused, so neither can read as an opt-in.
- New tests/cli-spec-drift.test.ts generalises the reviewer's by-hand
  verb x flag sentinel probe into a static scan: every flag any handler reads
  must be declared, or the test names it. Includes an anti-vacuity test, since a
  scanner matching nothing would be green for the wrong reason.

Mutation-proven: planting 402a3fb's spec state (both declarations deleted,
needle counts asserted 1 -> 0 for each) turns the new suite red at 1 pass / 3
fail; the anti-vacuity test correctly stays green because it does not depend on
the spec.

`--profile default-with-git-history` is refused with a message naming the
spelling that works, and the profile-refusal test now iterates the SELECTABLE
names rather than every key of SOURCE_EXCLUDE_PROFILES, which now also holds the
derived label.
`parseArgs` wrote flags into a flat Record, so a repeated flag silently
collapsed to its last occurrence. Measured before this change:

  sources add <src> --exclude '.env' --exclude '*.pem'   rc=0
    stderr: excludes: profile=explicit patterns=*.pem
    stored: excludes = ['*.pem']          <- '.env' discarded, no warning
  run                                                    rc=0
    tar tzf <archive>: src/  src/.env  src/app.js        <- the .env is archived

That is the defect class this branch exists to close — an argument list the CLI
cannot fully account for must not execute — left open, with a credentials
consequence, since these archives are not encrypted.

`ParsedArgs.flags` now records EVERY occurrence, and the spec decides what a
repeat means:

- `FlagSpec.repeatable` accumulates. On for the four comma-separated list flags
  only: `sources add --exclude`, `policy set --preserve-markers`,
  `verify --compare-source`, `aws --regions`. `--exclude a --exclude b` and
  `--exclude a,b` now resolve identically.
- Every other flag REFUSES a repeat, naming each value. There is no honest
  resolution for `--home /a --home /b`, and quietly taking the last one is the
  "the home was not the one you named" shape that destroyed 139 live artifacts.

`listFlag` also throws on a valueless occurrence rather than letting it filter
down to `[]`, which would be indistinguishable from the deliberate
`--preserve-markers=` clear. `validateArguments` already refuses that, so this is
defence in depth against a mistake becoming an instruction.

Audited by measurement, not by reading: every declared flag on every verb and
subcommand was probed with a repeat. All refuse except the four accumulators.
The one exception is the `help` VERB, which resolves before validation by design
because it executes nothing.

tests/cli-repeated-flags.test.ts asserts the archive LISTING, not just the stored
config — the bug was that the tool reported confidently while content went
elsewhere, so checking the report would be weaker than the bug. The exhaustive
audit is driven off the spec, so a flag added later is covered without editing
the test, and the repeatable set is pinned exhaustively so marking a non-list
flag repeatable is a reviewed change.

Mutation-proven: planting the flat-Record behaviour (accumulate needle 1 -> 0,
repeat refusal guarded off) turns the suite red at 1 pass / 6 fail. The one green
is the pure spec assertion, which does not depend on the parser.

Suite: bun test --timeout 60000 rc=0, 152 pass / 0 fail / 13 files / 1304
expect(). typecheck rc=0.
`hashManifest` walked with `withFileTypes` and did `if (!entry.isFile()) continue`,
so symlinks and empty directories were hashed on NEITHER side and could not
appear as missing, extra, or corrupt. Measured on a tree of one regular file plus
two symlinks, with one symlink excluded from the archive:

  sources add ... --exclude 'danger-link'   stderr: "DROPPING 1 file(s), 13 bytes"
  verify latest --deep                      rc=0  ok=true
      completeness=verified-against-source  missing=0 extra=0 corrupt=0

The pre-flight estimate and the deep measurement contradicted each other on one
tree, and the half this branch designates AUTHORITATIVE over the estimate was the
wrong one. That is the most damaging form the gap could take, because being the
authority is the entire point of `--deep`.

After: rc=1, ok=false, missing=1, `diff.missing = ["danger-link"]`. Same for an
excluded empty directory: `diff.missing = ["dropme"]`.

- A symlink is fingerprinted by its TARGET STRING, not the content it resolves
  to. Following it would make a symlink and a regular file of identical content
  compare equal — a real difference a restore must reproduce — and would let the
  walk escape the tree or loop. Checked before `isDirectory()`, so a symlink to a
  directory is recorded rather than descended into.
- Only EMPTY directories are recorded. A non-empty directory is already implied by
  the entries beneath it; an empty one is implied by nothing. The root itself is
  never recorded, since `relative(root, root)` is not a path either side can miss.
- Sockets, FIFOs, and device nodes are still skipped: tar does not carry their
  content, so recording them would report a difference no restore could resolve.
- `totalBytes` skips symlinks and empty dirs. `statSync` FOLLOWS a link, so
  counting one would double-count its target's bytes or invent bytes for a
  dangling link.
- MANIFEST_COMMAND now covers `-type l` and `-type d -empty`. It ships in the
  result so a human can redo the proof by hand; publishing a command narrower
  than the measurement would restate the fixed defect as documentation.

Mutation-proven: planting the `isFile()`-only walk (needles `entry.isSymbolicLink()`
1 -> 0 and the empty-dir set 1 -> 0) turns the new suite red at 3 pass / 7 fail.
The three greens are the root-never-recorded, MANIFEST_COMMAND, and restoredBytes
assertions, none of which depends on the two recording paths.

The suite also pins the no-false-alarm direction: a faithful `--profile
preservation` archive of a tree containing a symlink and an empty directory still
passes with restoredFiles=3. A check that fails on a good archive gets ignored as
fast as one that always passes.

Suite: bun test --timeout 60000 rc=0, 162 pass / 0 fail / 14 files / 1329 expect().
typecheck rc=0.
`proveGit` compared refnames literally. A working clone files an upstream branch
under `refs/remotes/origin/<X>` while `git ls-remote` reports `refs/heads/<X>`, so
the same branch was counted twice in opposite directions. Measured on a clone of
this repository before the fix:

  verify latest --deep --git-remote hasna/backup   rc=1 ok=false
    1/32 matched, 31 missing from the restored copy,
    9 present locally but not on the remote
    refs/heads/fix/cc893efa-...          reported MISSING
    refs/remotes/origin/fix/cc893efa-... present at the IDENTICAL SHA

`archiveOk` fails an archive on `git.ok === false`, so a faithful, hash-identical
archive of a working clone reported FAILED. A `--mirror` capture was unaffected,
so the documented remedy was right and only the comparison was wrong.

`logicalRefName` normalizes `refs/remotes/<remote>/<X>` to `heads/<X>`, discarding
the remote NAME: the restored clone's remote could be called anything, and its
recorded URL describes the machine that made the clone, not the objects.
`refs/remotes/<remote>/HEAD` is skipped as a symbolic alias.

Each logical name maps to a SET of local SHAs, not one, so a working clone whose
`refs/heads/main` is ahead of `refs/remotes/origin/main` still satisfies the
remote's SHA — the remote's commit is in the restored object database, which is
what a restore proof asks.

TWO THINGS THIS SURFACED THAT ARE WORTH READING CAREFULLY:

1. My first version of `logicalRefName` returned undefined for unrecognized
   namespaces, and that INTRODUCED the same false negative it was fixing: GitHub
   advertises `refs/pull/<n>/head`, a `--mirror` holds all 26 of them verbatim on
   this repository, and dropping them made a faithful mirror report 9/35 with 26
   "missing". Caught by measuring against the real remote rather than only the
   fixture. Unrecognized namespaces now fall back to the LITERAL refname, so the
   mirror path stays at full match. Pinned by its own test.
2. A working clone genuinely lacks those PR refs — `git clone` never fetches them
   — so reporting them as missing is true but says nothing about the archive.
   They are now split into `refsMissingLocallyAdvisory`: reported, counted in the
   message, and NOT failed on. Only `refs/heads/*` and `refs/tags/*` are treated
   as history a restore must reproduce.

Measured against the real GitHub API after the fix, rc=0:
  working clone  ok=true  9/35 matched, 0 history refs missing, 26 advisory
  --mirror       ok=true  35/35 matched, 0 advisory

Mutation-proven: planting the literal comparison (`logicalRefName` short-circuited
to identity, needle asserted present) turns the new suite red at 3 pass / 7 fail.
The suite also pins both no-false-pass directions — a branch pushed after the
capture is still reported missing and fails, and a diverged branch is still
reported mismatched.

Suite: bun test --timeout 60000 rc=0, 172 pass / 0 fail / 15 files / 1356 expect().
typecheck rc=0.
…behind

`--deep` writes a REAL restored copy of the data to the scratch tree, and a
caller-named `--target` is never removed — correctly, since deleting a path the
caller named would make a verification command destructive. But it was silent, and
it did not protect the path: `mkdirSync`'s `mode` is a no-op on an existing
directory. Measured on a `--profile preservation` archive, which excludes nothing:

  mkdir -p <t>; chmod 0777 <t>
  verify latest --deep --target <t>   rc=0, no warning about the target
  after: <t> still 0777; restored files 0664, including a credentials fixture

Per the review's ruling — warn AND refuse, not automatic cleanup:

- A group- or world-accessible target is REFUSED before anything is written. The
  message names the path, the mode, the `chmod 700` that fixes it, the
  `--allow-insecure-target` override, and why it matters.
- Refusing rather than `chmod`-ing the path is deliberate. Silently changing the
  permissions of a directory the caller named is the same class of surprise as
  silently deleting it, and the operator may have chosen that mode for other
  things living there.
- `--allow-insecure-target` proceeds and prints the exposure. The override accepts
  the risk; it does not alter the path.
- A target the command CREATES is chmod-ed to 0700 explicitly, because
  `mkdirSync`'s mode argument is masked by the umask. Verified under `umask 000`.
- Every named target gets a `TARGET:` notice on stderr and in `targetNotices`,
  even a secure one: the path is not removed and may hold plaintext credentials.
  Silence about that was the defect.

The default scratch tree is unchanged in contract — 0700 under the home's tmp,
removed after the run, no notices since no caller named it — and that is pinned so
the guard cannot have made the ephemeral path destructive or leaky.

Mutation-proven: planting the original behaviour (mode never inspected, no
not-removed notice; needles asserted 1 -> 0 and 0) turns the suite red at 3 pass /
5 fail.

Also fixed a type hole in my own test: a `.catch(e => e as Error)` union let the
refusal assertions read a resolved result. It now asserts `toBeInstanceOf(Error)`
first, so a future change that stops throwing fails by name instead of silently.

Docs: docs/VERIFICATION.md gains the target rules and the logical-ref-comparison
section; docs/COMMANDS.md gains the repeated-flag rows and the full
profile x --include-git-history composition matrix. CHANGELOG covers all five.

Suite: bun test --timeout 60000 rc=0, 180 pass / 0 fail / 16 files / 1380 expect().
typecheck rc=0, contracts:conformance rc=0, build rc=0.
…-target guard

Adversarial review round 2 returned APPROVE-WITH-CONDITIONS from two reviewers.
This closes every blocking condition, plus the new `prune --help` defect
(todos 9a2b3a22) which the pre-dispatch help gate already covered.

REFUSE A DUPLICATE --compare-source KEY (reviewer 1, worst finding). Marking the
flag repeatable fixed the argv layer, but a duplicate KEY still let the last
mapping win: `--compare-source t=<decoy> --compare-source t=<real>` returned rc=0
ok=true missing=0, while the reverse order returned rc=1 — the same input, order
dependent, one of them a GREEN verification, and nothing said about the discarded
mapping. Which tree an archive is diffed against decides what the proof means.

SPEC-DRIFT SCANNER NOW COVERS listFlag. The alternation omitted it, leaving the
readers for ALL FOUR repeatable flags unscanned — the highest-risk class, since an
undeclared flag fails closed at runtime. Planting `listFlag(parsed,
"sentinel-list")` left the suite green at 180/180 while the flag exited 1. The
reader list is now asserted to match the helpers the CLI actually defines, so a
fifth reader cannot silently narrow the scan again.

assertKeepsSecrets IS NOW ACTUALLY PINNED, and its docblock no longer overstates.
It was live (broadening the subtraction made it refuse) but untested, while the
comment claimed "a test pins it" and that the SECRETS_EXCLUDES-subset property
"is itself asserted". Neither was true. Added: the subset property, that the
modifier removes exactly `.git` while retaining every secrets pattern, that the
guard throws for each secrets pattern, and that it does NOT throw for the real
subtraction. Exported so it can be asserted directly.

Deleting the guard's CALL still left the suite green, because the guard only fires
when the invariant is already violated — so the call site is asserted at the
SOURCE level, and the comment now says that is what it is rather than implying a
behavioural test.

INLINE `=` NO LONGER TRUNCATES. `split("=", 2)` discarded everything past the
second `=`: `--name=a=b=c` stored "a", `--exclude='p=1,q=2'` stored ["p"]. Two
patterns silently reduced to one at rc=0 — the same class as the repeated-flag
collapse. Now splits on the first `=` and keeps the remainder. `--home=`,
`--preserve-markers=` and inline `--home=<path>` all still behave.

ONE SHARED RESTORE-TARGET GUARD, src/lib/restore-target.ts, used by BOTH
`verify --deep` and `restore apply`, which had drifted exactly as a duplicated
rule does: `--deep` refused a 0777 target while `restore apply --target <the same
path> --yes` returned rc=0 and left `creds.env` at mode 664.

- CONTAINMENT, new and the more serious half: a target inside the backup home (or
  containing it) is refused on both commands, with no override. Measured before:
  `verify --deep --target <home>/manifests` rc=0, leaving a restored directory
  among the manifest files — content mixed into the catalog `verify` and `restore`
  both read. Symlink-resolved, because a symlink into the home reads as outside it
  lexically.
- EXPOSURE differs by command, deliberately and recorded rather than hidden.
  `--deep` refuses (throwaway scratch tree the tool owns; 0700 is natural).
  `restore apply` WARNS (the caller's destination for their own data; an ordinary
  `mkdir` is 0755). Refusing in both places was my first attempt and it broke
  tests/backup.test.ts:201, whose target is a plainly-created directory — a guard
  that fails the common case only trains everyone to pass the override.

`verify` stays `mutating: false` because plain verify only reads, but its notes now
say `--deep` WRITES a restored copy, and the read-only-commands sentence in
UnnamedBackupHomeError is qualified the same way.

The `plan` note claiming the pre-flight "names every pattern that drops content"
now says it names the patterns and is an ESTIMATE using the inventory's matcher
rather than GNU tar's globber — the CHANGELOG already carried that caveat and the
spec contradicted it.

Mutation-proven, each with needle counts asserted changed: duplicate-key refusal +
`=` truncation planted together -> 9 pass / 2 fail; containment disabled -> 12 pass
/ 3 fail; the reviewer's `listFlag` plant now fails the drift test naming
"sentinel-list" (it previously left 180/180 green); deleting the
assertKeepsSecrets call now fails the wiring assertion (previously green);
broadening the subtraction to drop `.env` -> 14 pass / 2 fail.

Rebased onto b2c4809 (PR #23 merged) with ZERO conflicts.

Gates: bun test --timeout 60000 rc=0, 206 pass / 0 fail / 17 files / 1531 expect().
typecheck rc=0, contracts:conformance rc=0, build rc=0.
@andrei-hasna
andrei-hasna force-pushed the fix/cc893efa-cli-argument-and-verify-semantics branch from 0266051 to 2350364 Compare July 28, 2026 00:32
@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Round 2 — review conditions closed, and two corrections to my own PR body

Head is now 2350364, rebased onto b2c4809 (PR #23 merged) with zero conflicts.

Two adversarial reviewers returned APPROVE-WITH-CONDITIONS. Every blocking condition is closed
below. Corrections to the PR body go here rather than as a body edit, because both reviewers have
already read and acted on that text — GitHub keeps no body revision history, so an edit would make the
correction unverifiable by anyone but me.

Corrections to the PR body — both reviewers were right, I was wrong

  1. The policy set --preserve-markers a,b row is wrong as written. a and b are one character
    each, below PR Refuse to prune preservation-marked archives, and let a source capture git history #21's MIN_PRESERVATION_MARKER_LENGTH of 3, so that literal invocation is rc=1
    (Invalid preserveMarkers[0]) on d32e5cb and on this branch alike. Re-measured just now. The
    underlying claim still holds — the flag works — but I demonstrated it with a failing example:
    --preserve-markers golden-copy,legal-holdrc=0, stored ['golden-copy','legal-hold']. The
    row should read that.
  2. "main baseline 80" is stale. Measured by building each ref: 8ce32b1 = 80 pass / 6 files,
    d32e5cb = 100 pass / 8 files. 80 was this PR's original base. Against the base it actually
    lands on, the delta is 100 → 206.

Conditions closed

# condition fix
R1-1 duplicate --compare-source key collapsed last-wins, returning a green proof refused, naming both paths
R1-2 spec-drift scanner omitted listFlag — the reader for all four repeatable flags added; the reader list is now asserted against the helpers the CLI defines
R1-3 assertKeepsSecrets untested while its docblock claimed a test pinned it four real assertions + a source-level wiring assertion; docblock now states exactly what is pinned and how
R2-1 --target inside the backup home accepted, mutating the catalog refused on both verify --deep and restore apply, no override, symlink-resolved
R2-3 credential warnings blamed --profile preservation exclusively reworded to name what the archive actually held, since an explicit --exclude list can keep every secrets pattern
R2-4 restore apply --target unguarded while verify --deep refused one shared guard, src/lib/restore-target.ts
R2-5 verify is mutating: false yet --deep writes notes and UnnamedBackupHomeError now say --deep writes a restored copy; the flag stays false because plain verify genuinely only reads
R1-6 inline = truncated values splits on the first =, keeps the remainder

Also folded in, from the coordinator: backup prune --help (todos 9a2b3a22). Already closed by
this PR's pre-dispatch help gate, and I verified the stronger claim in that task — no verb's --help
resolves a home at all: prune --help --home <nonexistent> prints usage at rc=0 and does not create
the directory
. Same for run, sources add, restore apply, plan, init.

The one condition I did NOT implement as stated, and why

R2-4 asked to extend the target guard to restore. I did — for containment. But extending the
permission refusal there broke tests/backup.test.ts:201, whose target is an ordinary
mkdir-created directory, i.e. 0755 under a standard umask. So the rules split by purpose, and the
asymmetry is now pinned by a test rather than left implicit:

rule verify --deep --target restore apply --target
inside the backup home refuse, no override refuse, no override
group/world-readable refuse, --allow-insecure-target overrides warn, naming the mode

--deep's target is a throwaway scratch tree the tool owns, where 0700 is natural. restore apply's
is the caller's destination for their own data; refusing 0755 would reject the normal invocation of
the primary way to get data back, and a guard that fails the common case trains everyone to pass the
override. If the reviewers disagree, this is the decision to push on.

Findings acknowledged and NOT fixed here — filed instead

--deep is red-by-default on a default-profile source (excluded files count as missing), the
--include-git-history refusal is glob-bypassable (--exclude '.gi?', pre-existing — git diff d32e5cb..HEAD -- src/git.ts is 0 lines), restoredFiles counts entries rather than files, the
target guard does not inspect parent directories, prune has no --source, and hashManifest reads
whole files. All are follow-up tasks, none introduced by this PR. Per reviewer 1, --deep must not
be wired into any loop or doctor until missing-because-excluded is distinguished from
missing-unexpectedly
— this PR wires it into neither.

Verification

bun test --timeout 60000        rc=0   206 pass / 0 fail / 17 files / 1531 expect()
bun run typecheck               rc=0
bun run contracts:conformance   rc=0
bun run build                   rc=0

New mutation plants, needle counts asserted changed each time: duplicate-key + = truncation reverted
together → 9 pass / 2 fail; containment disabled → 12 pass / 3 fail; broadening the subtraction
to drop .env14 pass / 2 fail. And the two plants that used to pass silently now fail:
reviewer 1's listFlag(parsed, "sentinel-list") plant now fails the drift test naming the flag
(previously 180/180 green), and deleting the assertKeepsSecrets call now fails the wiring assertion
(previously green).

Reviewer 2 independently reproduced the target-guard plant at exactly 3 pass / 5 fail, and reviewer
1 reproduced two plants at 3 pass / 7 fail each and confirmed MANIFEST_COMMAND actually
reproduces --deep (diff rc=0). Reviewer 1 also refuted its own hashManifest collision
hypotheses by measurement, and confirmed logicalRefName cannot produce a false pass.

Live catalog — a correction to the control itself

The manifests count is not a valid control on this machine, and neither is the config.json
sha256.
During this session the live catalog went 62 → 73 manifests and config.json was
rewritten twice by a concurrent session. Neither was me: every command I ran carried an explicit
--home <fixture> on the same command line, and runs.jsonl is still at its pre-session mtime
(2026-07-27 17:14:40 +0300), so no run occurred from here. The 11 added manifests were written at
03:22:56–03:23:01 but carry backup IDs from 07-24 and 07-27 — a backfill, not new runs.

The control that held, and the one to use: no entry disappeared.
comm -23 before after0 lost, 11 added. swarm-final-archive-20260727 still present in
config.json.

One thing worth reclaiming: the brief's fingerprint 7e4d2912f84e55ee369ad7fd1dfcd5c2 was declared
unreproducible after eleven recipes were tried. It is reproducible — the missing detail is
relative paths, because md5sum output embeds the path:

cd $HOME/.hasna/backup && find manifests -type f | sort | xargs md5sum | md5sum

I reproduced it three times. The absolute-path variants the coordinator issued
(dc00e84f…, 84aacd70…) also reproduced exactly at the time they were taken, and have since moved
with the backfill — which is the point: a digest over a directory a scheduled loop writes to is not a
control. "Nothing disappeared" is.

No backup run against real data, no prune --apply, nothing deleted, expired, moved or renamed.

NOT MERGED. Ready for a third pass, focused on the exposure-asymmetry decision above.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Round 3 — the five unverified plants reproduce; two defects block the merge

Third adversarial pass, on head 2350364 / base b2c4809. Fresh clone at
$HOME/.hasna/repos/worktrees/backup/cc893efa-adv3-verify (the shared checkout is a hollow .git
skeleton). Tree confirmed byte-identical to the PR head after all plant/revert cycles:
git diff 23503640 lines.

Verdict: APPROVE-WITH-FIXES. Not merged. The engineering is real and the mutation evidence holds
under independent replication. Two defects must be fixed first, and both are in code this PR
introduces: restore apply --help prints a refusal rule the code does not implement, and the
warning that replaces that refusal never reaches stderr. A CLI making a false statement about what
it refuses is the exact defect class this PR exists to close.

Baseline

bun test --timeout 60000        rc=0   206 pass / 0 fail / 17 files / 1531 expect()
bun run typecheck               rc=0
bun run contracts:conformance   rc=0
bun run build                   rc=0

grep -c '✗' = 0 and grep -c '(fail)' = 0; summary line authoritative per the marker caveat.


1. The plants — seven reproduced independently, five of them for the first time

Measured by building each ref, targeted file runs, cmd; rc=$?.

plant claimed I measured note
spec without #21's two flags 1 pass / 3 fail 2 pass / 3 fail 3 fail exact; pass count rose because R1-2 added a 5th test
flat-Record parser, repeat refusal off 1 pass / 6 fail 4 pass / 7 fail stronger than claimed
duplicate --compare-source key + = truncation 9 pass / 2 fail 9 pass / 2 fail exact
D1 replant (8ce32b1:src/cli/index.ts) 0 pass / 16 fail 0 pass / 16 fail exact, and non-vacuous
D3 replant (comparePath=undefined + proveGit off) 20 pass / 13 fail 20 pass / 13 fail exact
containment disabled 12 pass / 3 fail 12 pass / 3 fail exact
broadening the subtraction to drop .env 14 pass / 2 fail 18 pass / 4 fail (my scope: source-excludes + git-history-source) caught; scope differs

The D1 replant is not an import error. Every failure is a real assertion (toContain, toBe), and
the captured output shows the old code performed a backup when asked for help — archive
bkp_20260728004308_h83vbt-fixture.tgz, "dryRun": false, manifest written. Defect 1 independently
reproduced.

The secrets guard fails closed through the real CLI, not just in tests. With excludesGitDir
broadened onto .env:

sources add <p> --include-git-history --home <fixture>      rc=1
  "Refusing to build the \"default-with-git-history\" exclude list: it would drop the secrets
   pattern(s) \".env\", so credentials in the tree would be archived in plaintext."

That is the strongest safety claim in the PR and it holds under mutation.

2. Non-widening — measured independently, and end-to-end

All cells re-measured through the real CLI, --home <fixture> on the same command line, redirection
grepped out of the init output first. Read back from the stored config.json, not from stdout:

source recorded profile patterns .git present secrets patterns kept
(nothing) default 12 yes 7/7
--include-git-history default-with-git-history 11 no 7/7
--profile default default 12 yes 7/7
--profile default --include-git-history default-with-git-history 11 no 7/7
--profile preservation ± modifier preservation 0 no 0/7 (by design, notice fires)

All four refusal cells reproduce at rc=1, including --profile default-with-git-history"Invalid
source exclude profile … Expected one of default, preservation."

End-to-end, which is what actually mattersrun on the --profile default --include-git-history
source, then tar tzf:

src/  src/app.js  src/.git/  src/.git/HEAD          <- history captured
                                                    <- src/.env is ABSENT

The composition captures the .git the caller asked for and does not widen onto the .env. Ruling:
the subtractive-modifier reading is right and the rejected refusal is wrong. --include-git-history
alone already resolved to default-minus-.git in shipped main, and the two invocations produce
identical 11-pattern lists as measured — so refusing the explicit spelling would make naming the
default
a breaking change. Recording it as default-with-git-history rather than default is also
right: the stored list is 11 patterns and sources add prints the label.

3. logicalRefName against the real GitHub API

git ls-remote https://github.com/hasna/backup.git → 37 refs (10 refs/heads/*, 27
refs/pull/*, 0 tags). Counts have grown since your measurement (35 / 26). Ran proveGit against both
a real --mirror clone and a real working clone:

mirror:        ok=true  37/37 matched  0 missing  0 advisory
working-clone: ok=true  10/37 matched  0 history refs missing  27 advisory

Correct. Then I planted your disclosed first-version bug (return undefined for unrecognized
namespaces) and re-measured against the real API:

mirror:        ok=true  10/37 matched  27 advisory     <- a faithful mirror now reads as a working clone

Two things worth recording. First, ok stays true under the bug — it is an under-report, not a
false failure, so no ok-assertion could ever catch it. Second, the behavioural tests do not catch
it either
: stageUpstreamAndClone() builds a local upstream with no refs/pull/*, so under the plant
verify-git-refs.test.ts went 9 pass / 1 fail and the single failure was the direct unit assertion
at :107. The mirror and working-clone tests both stayed green. Your docblock at :102-104 is honest
about this, and the unit test does pin it — but a git update-ref refs/pull/1/head <sha> on the staged
upstream would make the pin behavioural and match the standard the rest of this suite sets.

4. --deep is not wired into anything — confirmed

  • Only loop that calls the verb: /home/hasna/.hasna/loops/scripts/charter/backup-offsite.sh:202
    backup verify latest, plain, no --deep.
  • crontab -l: one backup entry, backup_s3_freshness_check.sh. No --deep.
  • doctor(home) takes no flags (src/cli/index.ts:119-121) and its command list is
    commandNames() — verb names only (src/cli/spec.ts:433-435).

And the finding it is gated on is real: on a default-profile source,
verify <id> --deeprc=1, ok=false, sourceFiles=3 restoredFiles=1,
missing=['.env','.git/HEAD'] — red purely because the excludes did their job.


BLOCKING — fix before merge

B1. restore apply --help documents a refusal the code does not implement

The spec's apply subcommand ships this text (src/cli/spec.ts:362-368):

--allow-insecure-target Write into a group/world-readable --target anyway. Refused without this.
--target is refused if it is group- or world-readable, or if it sits inside the backup home.

Measured, with no --allow-insecure-target:

chmod 0777 <t>
restore apply latest --target <t> --yes --home <fixture>     rc=0
  <t>                        777
  <t>/s4/src4/creds.env      664      <- restored, world-readable, from a preservation archive

The code passes exposure: "warn" at src/backup.ts:764-773. The help text on that verb states the
verify --deep rule instead. This is a CLI printing a false statement about what it refuses — the
defect class this PR exists to close — and it is introduced here.

B2. The restore apply warning never reaches stderr

stderr was empty for the run above. The notices exist only in the stdout JSON warnings[].

  • verify --deep prints them: src/cli/index.ts:733-734console.error('TARGET: …').
  • restore apply does not: case "restore" is print(await handleRestore(...)); return;
    (src/cli/index.ts:220-222) — no stderr emission at all, unlike case "sources" and case "run".

Ruling on the asymmetry: keep warn, fix the warning. Your 0755 argument is correct — refusing the
primary restore path would train everyone to pass the override, which is worse. But as shipped, "warn"
means silent on the human channel while the help promises a refusal. That combination is weaker than
either option you were choosing between. Fix: emit the notices to stderr from case "restore", and
make the apply help say warn-not-refuse.


Non-blocking findings

N1 — "--profile preservation remains the only way to archive secrets, and it still says so out
loud" is false as measured.
The notice condition is literally
excludeProfile === "preservation" (src/config.ts:520), and the constant is
PRESERVATION_SECRETS_WARNING. So:

sources add <tree with .env> --exclude '.gi?' --home <fixture>    rc=0
  stderr: (nothing about credentials)
  run + tar tzf:  src/  src/app.js  src/.env      <- plaintext .env archived, silently

R2-3 was closed for the restore/verify target notices (which do now name what the archive held) but
not at registration, which is the higher-leverage place — a source registered this way then runs on
a schedule forever. The body claim should be retracted in a comment.

N2 — the glob-bypass provenance claim is true, but your proof does not establish it, and the severity
is worse than filed.
git diff d32e5cb..HEAD -- src/git.ts is indeed 0 lines (also 0 vs b2c4809
and 8ce32b1) — but the refusal site is src/config.ts:624, a file this PR does change, so the
src/git.ts diff alone does not prove it. I established it by running the invocation at both refs:

d32e5cb:  sources add <p> --exclude '.gi?' --include-git-history   rc=0  excludes=['.gi?']
2350364:  same                                                    rc=0  excludes=['.gi?'] profile=explicit

Pre-existing, confirmed. Severity to carry into the follow-up: the resulting archive lacks the .git
the operator explicitly asked for and contains the plaintext .env, at rc=0, with no warning.

N3 — "none introduced by this PR" is wrong for finding 1. git cat-file -e b2c4809:src/verification.ts
ABSENT. verify --deep and its red-by-default behaviour are wholly introduced here. The
mitigation is real and I confirmed it (opt-in, unwired), so this is not a blocker — but left standing,
that provenance line would let someone wire --deep into a loop believing the red is pre-existing noise.

N4 — a test introduced here writes a backup home into the CWD under regression.
tests/cli-repeated-flags.test.ts:186 probes every declared flag with a repeat, and for --home it
passes the relative values one/two with no fixture home (homeArgs = [] at :189). Under the
shipped refusal nothing is written. Under PLANT 2 the CLI created a real backup home in the test
process CWD — I observed two/config.json, two/manifests, two/restore-plans, two/tmp appear in
the repo root. The audit built to catch "the CLI used a home you did not name" itself names a home by
relative path, and its containment depends on the assertion it is testing. One-line fix:
join(tempRoot, "one") / join(tempRoot, "two").

N5 — the corrected baseline is itself one rebase stale. Measured by building each ref:
d32e5cb = 100 pass / 8 files, but the base this PR now lands on, b2c4809, = 109 pass / 9
files
. So the delta is 109 → 206 (+97), not 100 → 206. Same error class as the one the round-2
comment corrected: a baseline quoted from a superseded base.

N6 — proveGit is exported without normalizing its remote. normalizeGitRemote is applied only in
verifyBackups (src/verification.ts:77), so proveGit(root, scratch, "hasna/backup") hard-fails with
ls-remote … does not appear to be a git repository. It does fail closed (ok=false, message names the
failure), which is the right direction. Low severity.

Merge order vs #24

#22 first, then #24. Six files overlap: CHANGELOG.md, docs/COMMANDS.md, src/backup.ts,
src/cli/index.ts, src/runtime.ts, src/types.ts. #24 adds a reconcile verb reading --adopt,
--no-hold, --destination. #22 makes flags a closed allowlist with a drift scanner that names
undeclared flags — proven, since PLANT 1 produced 3 failing tests naming exactly the missing
declarations. Landing #22 first makes #24's rebase machine-checked: it must declare reconcile and its
flags in src/cli/spec.ts or tests/cli-spec-drift.test.ts fails it. The reverse order leaves #22
rebasing over a verb its allowlist would refuse.

Live catalog

Control per the brief — no entry disappeared, never a count:

before/after:  73 manifests   comm -23 → 0 LOST   comm -13 → 0 ADDED
config.json    sha256 60ffe3b3a191e47da58db9c58489cebc40cb7498ebafb725b87d04eb24eaf325  (unchanged)
               mtime 2026-07-28 03:21:34 +0300  (unchanged across my session)
runs.jsonl     mtime 2026-07-27 17:14:40 +0300  (unchanged)
swarm-final-archive-20260727 present in config.json

No backup run against real sources, no prune --apply, nothing deleted, expired, moved or renamed.
Every command carried an explicit --home <fixture> on the same command line.

Method caveat worth publishing, because it nearly produced a false alarm here: my first comparison
reported "73 lost" purely because the before snapshot used relative paths (cd $HOME/.hasna/backup && find manifests) and the after used absolute. comm compares strings. Take both snapshots the same
way or the control invents a catastrophe.

On the fingerprint: I can neither confirm nor refute the rehabilitated
7e4d2912f84e55ee369ad7fd1dfcd5c2.
The directory now holds 73 manifests, not the 62 it was taken
over, so any digest must differ — the relative-path recipe gives 46b406627bfa11a41927cfe525a7f99c
today. I state that rather than claim agreement I cannot show. It is also the point you made: a digest
over a directory a scheduled loop writes to is not a control.

… help true

`restore apply --help` shipped "--allow-insecure-target  Write into a
group/world-readable --target anyway. Refused without this." and "--target is
refused if it is group- or world-readable", while `applyRestore` passed
`exposure: "warn"`. Measured with no override:

    chmod 0777 <t>
    restore apply latest --target <t> --yes    rc=0
    <t>/s/src/creds.env                        mode 664, stderr EMPTY

Two defects in one: the help states a refusal the code does not perform on a
credential path, and the warning that replaces the refusal reached no human
channel — `verify --deep`, `sources` and `run` all print theirs.

`warn` is kept: the 0755 argument stands, since refusing the primary restore
path would train everyone to pass the override. The warning is fixed instead.

- The exposure policy per verb is one constant, `TARGET_EXPOSURE`, which the
  guard call sites pass AND the help text is generated from, so prose in a
  `notes` array can no longer drift from an argument at a call site.
- `case "restore"` prints every warning the result carries on stderr:
  `restore:` lines, plus `TARGET:` for the target notices, the same prefix
  `verify --deep` uses. Notices are returned separately from the payload so
  `--contract` cannot silence them.
- `RestoreApplyResult.targetNotices` mirrors `DeepVerifyResult`; empty on a run
  that wrote nothing, which has not measured the target's mode.
- `--allow-insecure-target` was inert on this verb — the warn path never read
  it — and now records the acceptance in the notice.

tests/restore-target-help.test.ts reads the help text and the measured
behaviour in ONE test for both verbs, because the defect was a
documentation/code divergence; the asymmetry itself is pinned separately as a
decision. Suite: 206 -> 211 pass, 0 fail, 18 files.
… just preservation

The notice's condition was `excludeProfile === "preservation"`, so the claim
that preservation is the only way to archive secrets — and the only way that
says so out loud — was false. Measured on a tree holding a `.env`:

    sources add <path> --exclude '.gi?'   rc=0   notices: []
    run; tar tzf <archive>                src/  src/app.js  src/.env

Registration is the higher-leverage place for this than the restore/verify
target notices already fixed: a source registered this way runs on a schedule
forever and nothing asks again.

The condition is now the EFFECTIVE exclude list. Coverage is measured with the
same matcher the exclude report uses, against one representative filename per
secrets pattern, because a pattern is not a filename — `*.pem` never matches
the literal string `*.pem`. Using the matcher rather than `includes()` also
keeps the notice honest in the other direction: `--exclude '*'` drops
everything and is correctly not warned about, and a warning layer that fires
on lists that are fine is one operators learn to skip.

`SECRETS_PATTERN_EXAMPLES` is keyed by the patterns themselves, so adding one
to `SECRETS_EXCLUDES` without a representative is a compile error rather than
a pattern that silently stops being checked. `PRESERVATION_SECRETS_WARNING`
(introduced by this PR, absent from `main`) is replaced by the generated
notice; `runtime.ts` re-exports the generator.

Suite: 211 -> 218 pass, 0 fail, 19 files.
…t nothing was created

The audit built to catch "the CLI used a home you did not name" probed `--home`
with the RELATIVE values "one" and "two". Those resolve against the test
process's cwd, so the single regression this test exists to detect writes
`two/config.json`, `two/manifests`, `two/restore-plans` and `two/tmp` into the
repository root — measured under a mutation plant.

Probe values are now absolute paths under the test's own temp root, and the
walk asserts neither was created. The exit-code assertion alone cannot see a
refusal that resolved the home first, which is the shape that matters here.

Verified by planting the collapse (disabling the repeat refusal): the audit
fails with 93 collapsed invocations named, and nothing lands outside the temp
root.
#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.
@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Corrections to claims in this PR's body and release notes

Three statements this PR made are wrong. Recorded here rather than by editing the body or amending a
pushed commit, so the original text and the correction are both attributable. Each was re-measured on
this machine (UTC+0300) rather than restated from the review.

1. "None of these findings is introduced by this PR" is wrong for verify --deep

git cat-file -e b2c4809:src/verification.ts   -> ABSENT
git cat-file -e d32e5cb:src/verification.ts   -> ABSENT
git cat-file -e 2350364:src/verification.ts   -> present

src/verification.ts does not exist at the base of this branch. verify --deep, and its
red-by-default behaviour on a default-profile source, are wholly introduced here.
The mitigation is
real and independently confirmed — the verb is opt-in and wired into nothing — but left standing, that
provenance line would let someone wire --deep into a loop believing the red is pre-existing noise.

2. The glob-bypass IS pre-existing, but the proof given for it does not establish that, and the severity is worse than filed

The claim was justified with "src/git.ts is 0 lines changed". That diff is indeed empty
(git diff --stat d32e5cb..2350364 -- src/git.ts -> no output), but the refusal site is
src/config.ts:624, and this PR changes src/config.ts by 233 insertions / 16 deletions.
The
unchanged predicate does not prove the unchanged behaviour of a call site in a file that did change.

Established the only way that does prove it — by running the invocation at both refs, on a tree holding
a .env and a .git:

d32e5cb:  sources add <p> --exclude '.gi?' --include-git-history   rc=0  excludes=['.gi?']  notices=0
          tar tzf <archive>:  src/  src/.env  src/app.js
HEAD:     same                                                     rc=0  excludes=['.gi?']  notices=1
          tar tzf <archive>:  src/  src/.env  src/app.js

Pre-existing, confirmed. Severity for the follow-up: both failures at once — the archive lacks the
.git the operator explicitly asked for and holds the plaintext .env, at rc=0. The .git half is
still open (excludesGitDir uses the internal matcher, which does not resolve .gi?). The credentials
half is now spoken aloud at HEAD: notices=0 -> notices=1, which is the registration-notice fix in
this push.

3. "--profile preservation remains the only way to archive secrets, and it still says so out loud" was false

The notice's condition was literally excludeProfile === "preservation". Measured on 2350364:

sources add <tree with .env> --exclude '.gi?'    rc=0    notices: []    stderr: nothing about credentials
run + tar tzf:  src/  src/app.js  src/.env

Retracted. An explicit --exclude list that omits the secrets patterns archives them too, silently.
Fixed in this push and corrected in CHANGELOG.md and docs/COMMANDS.md, both of which carried the same
sentence.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Round 3's two blocking defects are fixed, plus the registration notice and the --home audit

Head 40a3cd6, on a fresh clone at $HOME/.hasna/repos/worktrees/backup/cc893efa-b1b2-warning-fix
(the shared checkout is a hollow .git skeleton). All timestamps UTC+0300.

bun test --timeout 60000        rc=0   242 pass / 0 fail / 20 files / 1728 expect()
bun run typecheck               rc=0
bun run contracts:conformance   rc=0
bun run build                   rc=0
CI "Bun check"                  pass (32s)

Baseline for the delta is b2c4809 = 109 pass / 9 files, so this PR is now 109 -> 242 (+133).
grep -c '(pass)' and grep -c '✓' both return 0 on a green run here (FORCE_COLOR=3, no per-test
markers), so counts are from the summary line and failures from .

B1 — the help promised a refusal the code did not perform

Reproduced first, on 2350364, before touching anything:

chmod 0777 <t>
restore apply latest --target <t> --yes --home <fixture>    rc=0
<t>                                                          777
<t>/s/src/creds.env                                          664
stderr                                                       0 bytes

Kept warn, fixed the warning, per the ruling. The reason the two could disagree is that the policy
lived as a string literal at each guard call site while the help was prose in a notes array, with
nothing reading both. It is now one constant:

  • TARGET_EXPOSURE (src/lib/restore-target.ts) — deepVerify: "refuse", restoreApply: "warn".
  • applyRestore and resolveTarget pass it instead of a literal.
  • The --allow-insecure-target summary and the --target notes are generated from it, so
    restore apply --help now says "Accept a group/world-readable --target. This command WARNS and
    proceeds either way; it does not refuse."
    and "--target is REFUSED if it sits inside the backup
    home. If it is group- or world-readable this command WARNS on stderr and PROCEEDS ..."
    , while
    verify --help keeps its refusal wording.

One more thing that fell out of reading the flag: --allow-insecure-target was inert on
restore apply
— the warn branch never read it. It now records the acceptance in the notice, so the
flag means the same thing on both verbs.

B2 — and the warning never reached stderr

case "restore" now prints every warning the result carries: restore: lines for the plan/dry-run
warnings, TARGET: for the target notices — the same prefix verify --deep uses. handleRestore
returns the payload and the notices separately, so --contract (which replaces the payload with a
contract document that has no warnings) cannot silence them. RestoreApplyResult.targetNotices
mirrors DeepVerifyResult, and is empty on a run that wrote nothing, because the guard runs immediately
before the first write and a dry run has not measured the target's mode.

Measured after:

restore apply latest --target <0777 t> --yes --home <fixture>   rc=0
stderr:  TARGET: --target <t> is mode 777: other users can read the restored copy, including any
         credentials the archive carried. `chmod 700 <t>` if that is not intended.
         TARGET: --target <t> holds a real restored copy of the data and is NOT removed ...

The tests, and proof they are not vacuous

tests/restore-target-help.test.ts (5 tests). The one that matters reads the help text and the
measured behaviour together
, for both verbs, with every expectation derived from TARGET_EXPOSURE
because the defect was a documentation/code divergence and only a test that reads both can catch one.
The asymmetry itself is pinned separately as a decision
(expect(TARGET_EXPOSURE).toEqual({ deepVerify: "refuse", restoreApply: "warn" })), since a test driven
by a constant cannot also police that constant. The suite runs the CLI as a subprocess so stderr is
a real file descriptor, not a captured console.error.

Both halves planted and re-measured, in throwaway copies:

plant result
revert the case "restore" stderr emission 4 fail / 1 pass, every failure a real assertion (Received: ""), no import errors
hand-write the old apply help back in, behaviour untouched 1 fail / 4 pass, on the help/behaviour agreement test

Non-blocking findings, also in this push

N1 — the registration-time notice now fires on the effective exclude list. sources add measures
which secrets patterns the resolved list fails to drop, using the matcher the exclude report uses
against one representative filename per pattern, and names them. --exclude '.gi?' on a tree with a
.env went notices: 0 -> notices: 1. It correctly does not fire for --exclude '*', which
covers credentials another way — an over-firing warning is the failure mode that gets warnings ignored.
Adding a pattern to SECRETS_EXCLUDES without a representative is now a compile error.
tests/source-secrets-notice.test.ts (7 tests); planting the old profile-name condition fails it on the
.gi? case specifically.

The relative---home audit is fixed. tests/cli-repeated-flags.test.ts probed --home with the
relative values one/two, so the one regression that audit exists to detect writes
two/config.json, two/manifests, two/restore-plans and two/tmp into the repository root. Probe
values are absolute paths under the test's own temp root, and the walk now asserts neither was created —
the exit-code assertion cannot see a refusal that resolved the home first. Verified by planting the
collapse: 93 collapsed invocations named, nothing written outside the temp root.

Left alone, deliberately

--deep is still wired into nothing, and this push does not wire it in. The subtractive-modifier
composition is untouched. The .git half of the glob bypass is untouched — see the corrections comment.

#24 is integrated, and its interaction was machine-caught

bad7d52 landed first, so this branch merges main rather than rebasing onto it: the review record
cites 2350364 and the two commits before it, and rewriting those SHAs would leave three rounds of
evidence pointing at commits the PR no longer lists.

Checked 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.

Live catalog

Control is "no entry disappeared", never a count, and one path form throughout
(cd $HOME/.hasna/backup && find manifests -type f):

manifests    73 entries
config.json  sha256 60ffe3b3a191e47da58db9c58489cebc40cb7498ebafb725b87d04eb24eaf325
             mtime 2026-07-28 03:21:34 +0300   (identical to round 3's record)
runs.jsonl   mtime 2026-07-27 17:14:40 +0300   (unchanged)

Every command I ran carried an explicit --home <fixture> on the same command line. No backup run
against real sources, no prune --apply, nothing deleted, expired, moved or renamed.

Not merged. Ready for a final adversarial pass.

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