sneak/vaultik
1
0
Fork 0
Code Issues 19 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

snapshot list must list remote snapshots without requiring the private key #64

New Issue
Closed
opened 2026-08-09 03:41:14 +02:00 by clawbot · 3 comments
clawbot commented 2026-08-09 03:41:14 +02:00
Collaborator
Copy Link

vaultik snapshot list never reads the destination store on a correctly
configured host, contradicting both the README and vaultik's core threat
model.

Evidence

ListSnapshots (internal/vaultik/snapshot.go:471) builds its table
entirely from the local SQLite index via
v.Repositories.Snapshots.ListRecent (line 474). Every row goes through
snapshotInfoFromLocal (snapshot.go:574-603), which hardcodes
LocallyTracked: true (line 602). The only remote access is
reportRemoteDrift, and it is gated:

// internal/vaultik/snapshot.go:504-508
if v.Config.AgeSecretKey == "" {
    return nil
}
v.reportRemoteDrift(localSnaps)

The whole premise of vaultik (README:3-7) is that the backed-up host has
no private key. So on a properly configured production host,
AgeSecretKey is empty and snapshot list never contacts the
destination at all.

This contradicts:

  • README:178-182 — "Show every snapshot known to the destination store
    … The uncompressed and 'new chunk' columns show <remote only> for
    snapshots not in the local index."
  • The function's own doc comment (snapshot.go:468-470) — "If remote
    listing fails (unmounted volume, permission denied, network), we
    degrade to local-only with a warning. List never fails just because the
    destination is unreachable." This describes behavior that does not
    exist.

Consequence: const remoteOnlyCell = "<remote only>" (snapshot.go:663)
and its consumer at line 668 are unreachable dead code, and a user
who has lost their local index cannot see their own backups.

Secondary bug in the same function: snapshot.go:562 prints

v.UI.Infof("Run 'vaultik snapshot cleanup' to remove stale local records.")

There is no snapshot cleanup subcommand. NewSnapshotCommand
(internal/cli/snapshot.go:50-55) registers only create, list,
purge, verify, remove, restore. The correct advice per
README:223-232 is vaultik prune. Note that CleanupLocalSnapshots
exists on *Vaultik but is wired to no command — determine whether the
command was removed and the message orphaned, or the command was never
added.

Definition of done

  1. snapshot list enumerates snapshots from the destination store,
    merged with the local index, and does so without requiring
    age_secret_key    . Listing must work on a host holding only the
    public key.
  2. Snapshots present remotely but absent locally appear in the table with
    <remote only> in the uncompressed and new-chunk columns, making
    remoteOnlyCell reachable. Snapshots present locally but not remotely
    are still surfaced as drift.
  3. Remote listing failure (unreachable, permission denied, unmounted)
    degrades to local-only with a warning and a zero exit code, exactly
    as the existing doc comment promises. snapshot list must not fail
    because the destination is unreachable.
  4. --json output reflects the same merged view, with an explicit field
    distinguishing locally-tracked from remote-only entries.
  5. The vaultik snapshot cleanup string is gone: either wire up a real
    command backed by CleanupLocalSnapshots, or change the message to
    name vaultik prune. No user-facing message may name a nonexistent
    command. Decide and state which was chosen.
  6. Tests: a case with a remote-only snapshot asserting <remote only>
    rendering; a case with an unreachable destination asserting graceful
    local-only degradation and exit 0; a case asserting remote listing
    happens with age_secret_key unset.
  7. README:178-182 and the function doc comment match the implemented
    behavior.
  8. make check green.
`vaultik snapshot list` never reads the destination store on a correctly configured host, contradicting both the README and vaultik's core threat model. ## Evidence `ListSnapshots` (`internal/vaultik/snapshot.go:471`) builds its table entirely from the local SQLite index via `v.Repositories.Snapshots.ListRecent` (line 474). Every row goes through `snapshotInfoFromLocal` (`snapshot.go:574-603`), which hardcodes `LocallyTracked: true` (line 602). The only remote access is `reportRemoteDrift`, and it is gated: ```go // internal/vaultik/snapshot.go:504-508 if v.Config.AgeSecretKey == "" { return nil } v.reportRemoteDrift(localSnaps) ``` The whole premise of vaultik (README:3-7) is that the backed-up host has **no private key**. So on a properly configured production host, `AgeSecretKey` is empty and `snapshot list` never contacts the destination at all. This contradicts: - README:178-182 — "Show every snapshot known to the destination store … The uncompressed and 'new chunk' columns show `<remote only>` for snapshots not in the local index." - The function's own doc comment (`snapshot.go:468-470`) — "If remote listing fails (unmounted volume, permission denied, network), we degrade to local-only with a warning. List never fails just because the destination is unreachable." This describes behavior that does not exist. Consequence: `const remoteOnlyCell = "<remote only>"` (`snapshot.go:663`) and its consumer at line 668 are **unreachable dead code**, and a user who has lost their local index cannot see their own backups. Secondary bug in the same function: `snapshot.go:562` prints ```go v.UI.Infof("Run 'vaultik snapshot cleanup' to remove stale local records.") ``` There is no `snapshot cleanup` subcommand. `NewSnapshotCommand` (`internal/cli/snapshot.go:50-55`) registers only `create`, `list`, `purge`, `verify`, `remove`, `restore`. The correct advice per README:223-232 is `vaultik prune`. Note that `CleanupLocalSnapshots` exists on `*Vaultik` but is wired to no command — determine whether the command was removed and the message orphaned, or the command was never added. ## Definition of done 1. `snapshot list` enumerates snapshots from the **destination store**, merged with the local index, and does so **without requiring `age_secret_key`**. Listing must work on a host holding only the public key. 2. Snapshots present remotely but absent locally appear in the table with `<remote only>` in the uncompressed and new-chunk columns, making `remoteOnlyCell` reachable. Snapshots present locally but not remotely are still surfaced as drift. 3. Remote listing failure (unreachable, permission denied, unmounted) degrades to local-only **with a warning** and a zero exit code, exactly as the existing doc comment promises. `snapshot list` must not fail because the destination is unreachable. 4. `--json` output reflects the same merged view, with an explicit field distinguishing locally-tracked from remote-only entries. 5. The `vaultik snapshot cleanup` string is gone: either wire up a real command backed by `CleanupLocalSnapshots`, or change the message to name `vaultik prune`. No user-facing message may name a nonexistent command. Decide and state which was chosen. 6. Tests: a case with a remote-only snapshot asserting `<remote only>` rendering; a case with an unreachable destination asserting graceful local-only degradation and exit 0; a case asserting remote listing happens with `age_secret_key` unset. 7. README:178-182 and the function doc comment match the implemented behavior. 8. `make check` green.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:41:14 +02:00
clawbot commented 2026-08-09 04:57:23 +02:00
Author
Collaborator
Copy Link

Manager note — implementation guidance, plus one constraint that was not
obvious when I filed this.

The naming constraint

I checked whether a remote-only snapshot can even be named without the
private key. It cannot, fully — but it can be described usefully.

RemoteSnapshotKey (internal/snapshot/remotekey.go:35) is
hex(SHA256(SHA256("vaultik|" + id))), one-way, and the manifest's
snapshot_id field stores that hash, not the human ID. The human ID
(<hostname>_<name>_<timestamp>) exists only in the local index and in
the encrypted db.zst.age. So for a snapshot absent from the local
index, hostname and snapshot name are not recoverable on a host
holding only the public key. Do not attempt to recover them, and do not
add anything to remote storage to make them recoverable — that would
undo a deliberate privacy property (see #81).

What is available without the private key, from the unencrypted
metadata/<remote-key>/manifest.json.zst
(internal/snapshot/manifest.go:15-28): timestamp, blob_count,
total_compressed_size, and per-blob hashes and sizes.

That is enough to make this feature genuinely useful, and it maps cleanly
onto the README's existing promise: a remote-only row can show a real
timestamp and a real compressed size, with <remote only> in exactly the
columns that require the local index (uncompressed size and new-chunk
count). Identify such rows by their remote key — truncated for display,
with the full key available in --json — and make it visually obvious
that the human name is unavailable rather than blank or fabricated.

So this issue is implementable as scoped and is not blocked on #81.

Ordering

#81 asks whether to encrypt the manifest. If that were answered "encrypt",
this feature would need the private key and the design would change
materially. I recommended keeping the manifest readable partly for this
reason. Whoever implements this should read the manifest through a single
helper so that a future change to #81 has one call site to update, not
several.

Additional requirements beyond the definition of done

  1. Do not regress the no-private-key property. snapshot list must work
    with age_secret_key unset — that is the whole point of this issue.
    Add a test that fails if the code ever requires it again.
  2. Remote listing must be paginated/bounded sensibly. A destination with
    thousands of snapshots must not be read entirely into memory or make
    one request per snapshot if the backend can list a prefix.
  3. Coordinate with fix/sync-snapshot-cleanup (see #71) — that stale
    branch touches syncWithRemote, the same path. Read its diff before
    starting; if it is still correct, fold it in rather than colliding.
  4. reportRemoteDrift currently exists only to warn about mismatches.
    Once listing is merged, decide explicitly whether it still has a
    distinct job or whether it collapses into the merged view, and say
    which in the PR.
  5. remoteOnlyCell (snapshot.go:663) and its LocallyTracked == false
    branch become reachable. Confirm they render correctly rather than
    assuming.
Manager note — implementation guidance, plus one constraint that was not obvious when I filed this. ## The naming constraint I checked whether a remote-only snapshot can even be *named* without the private key. It cannot, fully — but it can be described usefully. `RemoteSnapshotKey` (`internal/snapshot/remotekey.go:35`) is `hex(SHA256(SHA256("vaultik|" + id)))`, one-way, and the manifest's `snapshot_id` field stores that **hash**, not the human ID. The human ID (`<hostname>_<name>_<timestamp>`) exists only in the local index and in the encrypted `db.zst.age`. So for a snapshot absent from the local index, hostname and snapshot name are **not recoverable** on a host holding only the public key. Do not attempt to recover them, and do not add anything to remote storage to make them recoverable — that would undo a deliberate privacy property (see #81). What **is** available without the private key, from the unencrypted `metadata/<remote-key>/manifest.json.zst` (`internal/snapshot/manifest.go:15-28`): `timestamp`, `blob_count`, `total_compressed_size`, and per-blob hashes and sizes. That is enough to make this feature genuinely useful, and it maps cleanly onto the README's existing promise: a remote-only row can show a real timestamp and a real compressed size, with `<remote only>` in exactly the columns that require the local index (uncompressed size and new-chunk count). Identify such rows by their remote key — truncated for display, with the full key available in `--json` — and make it visually obvious that the human name is unavailable rather than blank or fabricated. So this issue is **implementable as scoped** and is not blocked on #81. ## Ordering #81 asks whether to encrypt the manifest. If that were answered "encrypt", this feature would need the private key and the design would change materially. I recommended keeping the manifest readable partly for this reason. Whoever implements this should read the manifest through a single helper so that a future change to #81 has one call site to update, not several. ## Additional requirements beyond the definition of done 1. Do not regress the no-private-key property. `snapshot list` must work with `age_secret_key` unset — that is the whole point of this issue. Add a test that fails if the code ever requires it again. 2. Remote listing must be paginated/bounded sensibly. A destination with thousands of snapshots must not be read entirely into memory or make one request per snapshot if the backend can list a prefix. 3. Coordinate with `fix/sync-snapshot-cleanup` (see #71) — that stale branch touches `syncWithRemote`, the same path. Read its diff before starting; if it is still correct, fold it in rather than colliding. 4. `reportRemoteDrift` currently exists only to warn about mismatches. Once listing is merged, decide explicitly whether it still has a distinct job or whether it collapses into the merged view, and say which in the PR. 5. `remoteOnlyCell` (`snapshot.go:663`) and its `LocallyTracked == false` branch become reachable. Confirm they render correctly rather than assuming.
clawbot commented 2026-08-09 05:01:19 +02:00
Author
Collaborator
Copy Link

Implementation plan for this issue, on branch fix-snapshot-list-remote.

Findings before writing code

Stale branch fix/sync-snapshot-cleanup (tip 332ea26). Its only
change is v.Repositories.Snapshots.Delete(...) →
v.deleteSnapshotFromLocalDB(...) inside syncWithRemote. That change
is already present on main (internal/vaultik/snapshot.go:1186),
having landed independently through the deleteSnapshotFromLocalDB
error-propagation work. There is nothing to fold in and nothing to
collide with; the branch is redundant and can be deleted under #71.

CleanupLocalSnapshots is not orphaned. It is wired to
vaultik prune at internal/vaultik/prune.go:82, and the comment there
states explicitly that snapshot cleanup was folded into prune. So
the snapshot cleanup string is a leftover from a removed command, not
a missing one.

Design

Merged listing. ListSnapshots builds one merged set:

  1. Local completed snapshots → SnapshotInfo{LocallyTracked: true}
    as today, plus the derived RemoteSnapshotKey.
  2. One prefix listing of metadata/ via the existing streaming
    listAllRemoteSnapshotKeys — a single prefix request, not one
    per snapshot.
  3. For each remote key with no local counterpart, read
    metadata/<key>/manifest.json.zst and emit
    SnapshotInfo{LocallyTracked: false} carrying the manifest
    timestamp and total_compressed_size.

None of this touches AgeSecretKey; the manifest is unencrypted, so
listing works on a host holding only the public key. The
AgeSecretKey == "" early return is deleted.

Naming constraint honored. No attempt to recover hostname or
snapshot name for remote-only rows, and nothing new written to remote
storage. The identifier column shows
<remote only:<first 12 hex of key>>, which is visibly not a human
ID. --json carries the full 64-char key in a new remote_key field.
<remote only> fills the uncompressed and new-chunk columns, exactly
as README:178-182 already promises.

Bounding (comment req 2). The listing is streamed, one prefix
request. Manifests are fetched only for keys absent from the local
index, with bounded concurrency and a hard cap on the number of
remote-only rows; past the cap the table is truncated with a warning
rather than growing unbounded.

Single manifest reader (comment "Ordering"). All manifest reads go
through the existing downloadManifestByKey. internal/vaultik/verify.go
and internal/vaultik/info.go still open and decode manifests inline;
both get routed through that helper so #81 has exactly one call site to
change.

reportRemoteDrift (comment req 4): collapses. Its remote-only half
is fully subsumed by the merged table, which now shows the actual rows
instead of a bare count. Its local-only half stays meaningful but no
longer needs its own remote listing — it becomes a small reporter fed
from the already-computed merge, so snapshot list makes exactly one
remote listing per invocation.

Degradation (DoD 3). Remote listing failure is a warning plus
local-only output and a zero exit code. In --json mode the warning
goes to the log rather than stdout so the JSON stays parseable.

snapshot cleanup string (DoD 5): renamed to vaultik prune. Not
wiring a new command: prune already runs CleanupLocalSnapshots as
its first pass, and adding snapshot cleanup back would re-create the
duplicate entry point that the 2026-07-02 CLI consolidation removed.

Tests

  • Remote-only snapshot renders <remote only> in both
    local-index-dependent columns, with the truncated key in the
    identifier column and the real manifest timestamp/size.
  • Remote listing happens with age_secret_key unset — asserts the
    remote store was actually read and the remote-only row is present.
    This is the regression guard for the whole issue.
  • Unreachable destination: warning emitted, local rows still printed,
    ListSnapshots returns nil.
  • Local-only snapshot surfaced as drift, with the hint naming
    vaultik prune.
  • --json merged view: locally_tracked and full remote_key.

README:178-182, the ListSnapshots doc comment, and TODO.md updated
in the same commit. Gate is script/cibuild.

Implementation plan for this issue, on branch `fix-snapshot-list-remote`. ## Findings before writing code **Stale branch `fix/sync-snapshot-cleanup` (tip `332ea26`).** Its only change is `v.Repositories.Snapshots.Delete(...)` → `v.deleteSnapshotFromLocalDB(...)` inside `syncWithRemote`. That change is **already present on `main`** (`internal/vaultik/snapshot.go:1186`), having landed independently through the `deleteSnapshotFromLocalDB` error-propagation work. There is nothing to fold in and nothing to collide with; the branch is redundant and can be deleted under #71. **`CleanupLocalSnapshots` is not orphaned.** It is wired to `vaultik prune` at `internal/vaultik/prune.go:82`, and the comment there states explicitly that `snapshot cleanup` was folded into `prune`. So the `snapshot cleanup` string is a leftover from a removed command, not a missing one. ## Design **Merged listing.** `ListSnapshots` builds one merged set: 1. Local completed snapshots → `SnapshotInfo{LocallyTracked: true}` as today, plus the derived `RemoteSnapshotKey`. 2. One prefix listing of `metadata/` via the existing streaming `listAllRemoteSnapshotKeys` — a single prefix request, not one per snapshot. 3. For each remote key with no local counterpart, read `metadata/<key>/manifest.json.zst` and emit `SnapshotInfo{LocallyTracked: false}` carrying the manifest `timestamp` and `total_compressed_size`. None of this touches `AgeSecretKey`; the manifest is unencrypted, so listing works on a host holding only the public key. The `AgeSecretKey == ""` early return is deleted. **Naming constraint honored.** No attempt to recover hostname or snapshot name for remote-only rows, and nothing new written to remote storage. The identifier column shows `<remote only:<first 12 hex of key>>`, which is visibly not a human ID. `--json` carries the full 64-char key in a new `remote_key` field. `<remote only>` fills the uncompressed and new-chunk columns, exactly as README:178-182 already promises. **Bounding (comment req 2).** The listing is streamed, one prefix request. Manifests are fetched only for keys absent from the local index, with bounded concurrency and a hard cap on the number of remote-only rows; past the cap the table is truncated with a warning rather than growing unbounded. **Single manifest reader (comment "Ordering").** All manifest reads go through the existing `downloadManifestByKey`. `internal/vaultik/verify.go` and `internal/vaultik/info.go` still open and decode manifests inline; both get routed through that helper so #81 has exactly one call site to change. **`reportRemoteDrift` (comment req 4): collapses.** Its remote-only half is fully subsumed by the merged table, which now shows the actual rows instead of a bare count. Its local-only half stays meaningful but no longer needs its own remote listing — it becomes a small reporter fed from the already-computed merge, so `snapshot list` makes exactly one remote listing per invocation. **Degradation (DoD 3).** Remote listing failure is a warning plus local-only output and a zero exit code. In `--json` mode the warning goes to the log rather than stdout so the JSON stays parseable. **`snapshot cleanup` string (DoD 5): renamed to `vaultik prune`.** Not wiring a new command: `prune` already runs `CleanupLocalSnapshots` as its first pass, and adding `snapshot cleanup` back would re-create the duplicate entry point that the 2026-07-02 CLI consolidation removed. ## Tests - Remote-only snapshot renders `<remote only>` in both local-index-dependent columns, with the truncated key in the identifier column and the real manifest timestamp/size. - Remote listing happens with `age_secret_key` unset — asserts the remote store was actually read and the remote-only row is present. This is the regression guard for the whole issue. - Unreachable destination: warning emitted, local rows still printed, `ListSnapshots` returns nil. - Local-only snapshot surfaced as drift, with the hint naming `vaultik prune`. - `--json` merged view: `locally_tracked` and full `remote_key`. README:178-182, the `ListSnapshots` doc comment, and `TODO.md` updated in the same commit. Gate is `script/cibuild`.
clawbot commented 2026-08-09 07:35:21 +02:00
Author
Collaborator
Copy Link

Closed by PR #83, merged to main as 50e20b4.

Verified against the merged main with the test cache explicitly
defeated, not trusted from the PR:

$ GOFLAGS=-count=1 make check; echo "EXIT=$?"
EXIT=0

0 issues. from the pinned linter, 14 packages ok, 0 (cached), 0
FAIL.

snapshot list now enumerates the destination store merged with the
local index, with no age_secret_key gate — the property this issue
existed for. A regression test asserts the key is empty, counts prefix
listings, and asserts no .age key is ever read, so reintroducing the
gate fails the build on two independent assertions.

Resolutions for the open questions:

  • snapshot cleanup: the command was removed and the message
    orphaned, not never-added. internal/vaultik/prune.go:80-82 calls
    CleanupLocalSnapshots and its comment says so — it is prune's first
    pass, not dead code. Re-adding the command would restore the duplicate
    entry point the 2026-07-02 consolidation deliberately removed, so the
    hint now names vaultik prune.
  • reportRemoteDrift collapsed into the merged view as
    reportListDrift. Net effect: the destination is listed exactly once
    per invocation.
  • Remote-only naming: hostname and snapshot name are genuinely
    unrecoverable without the private key, so those rows are identified by
    abbreviated remote key with the real timestamp and compressed size from
    the manifest. Nothing new is written remotely and the ID is never
    fabricated — the privacy property in #81 is untouched.

Two bugs were found and fixed during review that were not in the
original scope:

  1. Timezone drift. scanSnapshotRows omitted .UTC() where its two
    siblings had it, so on a non-UTC host the same snapshot rendered a
    different time depending on whether it was locally tracked — in the
    same column, with no indication why. Fixed at the scanner, the single
    point where the value enters the domain, and the duplicate loop in
    GetIncompleteByHostname folded onto it so three readers now share one
    normalization point. The regression test pins time.Local to +07:13,
    since every existing fixture was UTC and would not have caught it.
  2. Silent --json truncation. The 1000-row cap and unreadable-manifest
    count were reported in table mode but not JSON, so a machine consumer
    past 1000 remote-only snapshots got a truncated document with no
    signal. Now reported on stderr in both modes; the JSON shape is
    unchanged.

Two follow-ups filed rather than fixed drive-by: #82 (the logger writes to
stdout, so warnings corrupt --json — this PR carries a local workaround
to be removed when that lands) and #84 (a Vaultik.UI doc comment
misdescribing --cron).

Closed by PR #83, merged to `main` as `50e20b4`. Verified against the merged `main` with the test cache explicitly defeated, not trusted from the PR: ``` $ GOFLAGS=-count=1 make check; echo "EXIT=$?" EXIT=0 ``` `0 issues.` from the pinned linter, 14 packages `ok`, **0 `(cached)`**, 0 `FAIL`. `snapshot list` now enumerates the destination store merged with the local index, with **no `age_secret_key` gate** — the property this issue existed for. A regression test asserts the key is empty, counts prefix listings, and asserts no `.age` key is ever read, so reintroducing the gate fails the build on two independent assertions. Resolutions for the open questions: - **`snapshot cleanup`**: the command was removed and the message orphaned, not never-added. `internal/vaultik/prune.go:80-82` calls `CleanupLocalSnapshots` and its comment says so — it is `prune`'s first pass, not dead code. Re-adding the command would restore the duplicate entry point the 2026-07-02 consolidation deliberately removed, so the hint now names `vaultik prune`. - **`reportRemoteDrift`** collapsed into the merged view as `reportListDrift`. Net effect: the destination is listed exactly once per invocation. - **Remote-only naming**: hostname and snapshot name are genuinely unrecoverable without the private key, so those rows are identified by abbreviated remote key with the real timestamp and compressed size from the manifest. Nothing new is written remotely and the ID is never fabricated — the privacy property in #81 is untouched. Two bugs were found and fixed during review that were **not** in the original scope: 1. **Timezone drift.** `scanSnapshotRows` omitted `.UTC()` where its two siblings had it, so on a non-UTC host the same snapshot rendered a different time depending on whether it was locally tracked — in the same column, with no indication why. Fixed at the scanner, the single point where the value enters the domain, and the duplicate loop in `GetIncompleteByHostname` folded onto it so three readers now share one normalization point. The regression test pins `time.Local` to +07:13, since every existing fixture was UTC and would not have caught it. 2. **Silent `--json` truncation.** The 1000-row cap and unreadable-manifest count were reported in table mode but not JSON, so a machine consumer past 1000 remote-only snapshots got a truncated document with no signal. Now reported on stderr in both modes; the JSON shape is unchanged. Two follow-ups filed rather than fixed drive-by: #82 (the logger writes to stdout, so warnings corrupt `--json` — this PR carries a local workaround to be removed when that lands) and #84 (a `Vaultik.UI` doc comment misdescribing `--cron`).
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No Label
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/vaultik#64
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?