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

Triage the 15 stale remote branches; two may contain unlanded fixes #71

New Issue
Closed
opened 2026-08-09 03:43:10 +02:00 by clawbot · 4 comments
clawbot commented 2026-08-09 03:43:10 +02:00
Collaborator
Copy Link
Copy Source

The repo carries 15 stale remote branches. Nine are fully merged and are
pure clutter; six are unmerged, and two of those look like real
correctness fixes that never landed. That second group is the reason
this is a 1.0 item and not housekeeping.

Merged into origin/main — delete

Confirmed via git branch -r --merged origin/main:

  • add-compressstream-regression-test
  • feature/implement-prune-flag-on-snapshot-create
  • feature/pluggable-storage-backend
  • fix/issue-25, fix/issue-26, fix/issue-27, fix/issue-28,
    fix/issue-29
  • fix/restore-error-handling

Also delete golangci-v2.12.2 — its tree hash is identical to main
(0f36f0b99599b63018da6cd3b0a62e2046eb3c16), so it contributes nothing.

Unmerged — need a decision each

Branch Tip Assessment
fix/ctime-scanner-population 25860c0 "populate ctime from actual filesystem stats instead of mtime fallback". ARCHITECTURE.md:353 says dedup compares size, mtime, mode, uid, gid. If ctime is meant to participate in change detection, this is a dedup-correctness bug — wrong ctime means changed files silently skipped, i.e. data loss in a backup tool. Evaluate first, highest priority of the six.
fix/sql-injection-whitelist 3e282af Identifier allow-listing for dynamically built SQL. internal/database/database.go:420 has a repeatPlaceholder(n) helper, confirming dynamic query construction exists. Security-adjacent; evaluate before 1.0.
fix/sync-snapshot-cleanup 332ea26 "use deleteSnapshotFromLocalDB in syncWithRemote" — touches the exact code path #64 rewrites. Coordinate with #64 rather than landing independently.
feature/daemon-mode 87acc05 A daemon/watcher mode with filesystem-change batching. No daemon command on main, README documents none. Unshipped scope — decide 1.0, post-1.0, or delete.
feature/restore-progress-bar ee161fb main already has restore progress via printRestoreProgress and internal/snapshot/progress.go. Likely superseded — verify, then delete.

Definition of done

  1. Each of the six unmerged branches is resolved to exactly one of:
    rebased onto main and landed via its own PR; converted into a
    tracked issue with its diff summarized (if the intent is right but the
    code is stale); or deleted with a one-line rationale recorded on this
    issue.
  2. fix/ctime-scanner-population and fix/sql-injection-whitelist are
    assessed on their merits against current main — read the diff and
    determine whether the underlying bug still exists today. Do not delete
    either without that determination written down. If the bug is real,
    land the fix with a regression test.
  3. All nine merged branches plus golangci-v2.12.2 are deleted from
    origin.
  4. git branch -r afterwards shows only main, HEAD, and any branch
    with an open PR.
  5. TODO.md:51-53 is corrected: it names add-godoc-to-cli-package
    (no longer exists) and feature/pluggable-storage-backend (already
    merged), so the entry is stale in both halves.
  6. No branch is deleted without first confirming merged status via
    git branch -r --merged origin/main at the time of deletion.
The repo carries 15 stale remote branches. Nine are fully merged and are pure clutter; six are unmerged, and **two of those look like real correctness fixes that never landed**. That second group is the reason this is a 1.0 item and not housekeeping. ## Merged into `origin/main` — delete Confirmed via `git branch -r --merged origin/main`: - `add-compressstream-regression-test` - `feature/implement-prune-flag-on-snapshot-create` - `feature/pluggable-storage-backend` - `fix/issue-25`, `fix/issue-26`, `fix/issue-27`, `fix/issue-28`, `fix/issue-29` - `fix/restore-error-handling` Also delete `golangci-v2.12.2` — its tree hash is identical to `main` (`0f36f0b99599b63018da6cd3b0a62e2046eb3c16`), so it contributes nothing. ## Unmerged — need a decision each | Branch | Tip | Assessment | | --- | --- | --- | | `fix/ctime-scanner-population` | `25860c0` | "populate ctime from actual filesystem stats instead of mtime fallback". `ARCHITECTURE.md:353` says dedup compares size, mtime, mode, uid, gid. If ctime is meant to participate in change detection, this is a **dedup-correctness bug** — wrong ctime means changed files silently skipped, i.e. data loss in a backup tool. **Evaluate first, highest priority of the six.** | | `fix/sql-injection-whitelist` | `3e282af` | Identifier allow-listing for dynamically built SQL. `internal/database/database.go:420` has a `repeatPlaceholder(n)` helper, confirming dynamic query construction exists. Security-adjacent; evaluate before 1.0. | | `fix/sync-snapshot-cleanup` | `332ea26` | "use deleteSnapshotFromLocalDB in syncWithRemote" — touches the exact code path #64 rewrites. Coordinate with #64 rather than landing independently. | | `feature/daemon-mode` | `87acc05` | A daemon/watcher mode with filesystem-change batching. No `daemon` command on `main`, README documents none. Unshipped scope — decide 1.0, post-1.0, or delete. | | `feature/restore-progress-bar` | `ee161fb` | `main` already has restore progress via `printRestoreProgress` and `internal/snapshot/progress.go`. Likely superseded — verify, then delete. | ## Definition of done 1. Each of the six unmerged branches is resolved to exactly one of: rebased onto `main` and landed via its own PR; converted into a tracked issue with its diff summarized (if the intent is right but the code is stale); or deleted with a one-line rationale recorded on this issue. 2. `fix/ctime-scanner-population` and `fix/sql-injection-whitelist` are assessed **on their merits against current `main`** — read the diff and determine whether the underlying bug still exists today. Do not delete either without that determination written down. If the bug is real, land the fix with a regression test. 3. All nine merged branches plus `golangci-v2.12.2` are deleted from `origin`. 4. `git branch -r` afterwards shows only `main`, `HEAD`, and any branch with an open PR. 5. `TODO.md:51-53` is corrected: it names `add-godoc-to-cli-package` (no longer exists) and `feature/pluggable-storage-backend` (already merged), so the entry is stale in both halves. 6. No branch is deleted without first confirming merged status via `git branch -r --merged origin/main` at the time of deletion.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:43:10 +02:00
clawbot commented 2026-08-09 03:44:52 +02:00
Author
Collaborator
Copy Link
Copy Source

One item in this issue is a scope decision I should not make alone:
feature/daemon-mode.

Five of the six unmerged branches are triageable on technical merit —
does the bug still exist, is the fix still correct against current main.
feature/daemon-mode is different: it is unshipped product scope. Tip
87acc05, a long-running daemon/watcher mode with filesystem-change
batching. No daemon command exists on main and the README documents
none, so nothing today depends on it.

Options:

  1. Post-1.0. Delete the branch, file an issue capturing the intent so
    the design is not lost, and revisit after the tag.
  2. In 1.0. Rebase onto main, bring it up to current interfaces, add
    the CLI surface, document it, and test it — a substantial work unit
    that would meaningfully extend the path to a tag.
  3. Drop entirely. Delete the branch and file nothing.

My recommendation is option 1. The 1.0.0 milestone as scoped is
already carrying a snapshot list correctness fix (#64), a release
pipeline that does not currently work (#65), a security review (#73), and
two zero-coverage packages (#66, #73). Adding a new always-on execution
mode expands the surface to be audited right when the goal is to stop
expanding it. A daemon is also exactly the kind of feature that benefits
from being designed against a shipped, tagged baseline.

The other five branches proceed on technical merit without needing this
answer, so this is not blocking the rest of the issue. Assigning to
sneak for the daemon-mode call only.

One item in this issue is a scope decision I should not make alone: **`feature/daemon-mode`.** Five of the six unmerged branches are triageable on technical merit — does the bug still exist, is the fix still correct against current `main`. `feature/daemon-mode` is different: it is unshipped product scope. Tip `87acc05`, a long-running daemon/watcher mode with filesystem-change batching. No `daemon` command exists on `main` and the README documents none, so nothing today depends on it. Options: 1. **Post-1.0.** Delete the branch, file an issue capturing the intent so the design is not lost, and revisit after the tag. 2. **In 1.0.** Rebase onto `main`, bring it up to current interfaces, add the CLI surface, document it, and test it — a substantial work unit that would meaningfully extend the path to a tag. 3. **Drop entirely.** Delete the branch and file nothing. My recommendation is **option 1**. The 1.0.0 milestone as scoped is already carrying a `snapshot list` correctness fix (#64), a release pipeline that does not currently work (#65), a security review (#73), and two zero-coverage packages (#66, #73). Adding a new always-on execution mode expands the surface to be audited right when the goal is to stop expanding it. A daemon is also exactly the kind of feature that benefits from being designed against a shipped, tagged baseline. The other five branches proceed on technical merit without needing this answer, so this is not blocking the rest of the issue. Assigning to `sneak` for the daemon-mode call only.
sneak was assigned by clawbot 2026-08-09 03:44:55 +02:00
clawbot commented 2026-08-09 10:13:17 +02:00
Author
Collaborator
Copy Link
Copy Source

Manager note — unblocking this and dispatching it.

feature/daemon-mode was the only item here needing an owner decision,
so I have split it into #94 (assigned to sneak, recommendation:
post-1.0). Everything else in this issue is resolvable on technical merit,
so this issue is now actionable and reassigned to clawbot.

Leave feature/daemon-mode alone until #94 is answered — do not
delete it, do not rebase it.

One branch is already resolved

fix/sync-snapshot-cleanup (tip 332ea26) is redundant — confirmed
twice, independently. It changes exactly one line,
v.Repositories.Snapshots.Delete(...) → v.deleteSnapshotFromLocalDB(...)
in syncWithRemote, and that change is already on main at
internal/vaultik/snapshot.go:1186, having landed through the
deleteSnapshotFromLocalDB error-propagation work (ddc23f8 / 597b560).
The three-dot diff still shows it only because the merge base predates
both. Nothing to fold, nothing to collide with — safe to delete.

This is worth generalising to the rest of the triage: a three-dot diff
is not evidence a branch contains unlanded work. That artifact is what
made me briefly believe golangci-v2.12.2 held 126 files of unpushed
work earlier in this session; its tree hash turned out to be identical to
main's. Compare tips (two-dot) or tree hashes, not merge-base diffs.

Reminder on the two that matter

fix/ctime-scanner-population and fix/sql-injection-whitelist are the
reason this is a 1.0 item rather than housekeeping. Neither may be deleted
without first reading its diff and determining against current main
whether the underlying bug still exists. If it does, land the fix with a
regression test rather than deleting the branch. Write the determination
down either way.

fix/ctime-scanner-population deserves the most care: if ctime
participates in change detection, a wrong ctime means changed files are
silently skipped — data loss in a backup tool, and the kind of bug that
leaves no trace until a restore.

Environment notes for whoever picks this up

  • The shared BuildKit cache was destroyed earlier by another session, so
    builds are cold. Do not run docker builder prune; scope
    invalidation with --no-cache / --no-cache-filter.
  • Host-side lint runs may be VOID (#88) — retry unless the output has no
    parallel golangci-lint is running and cites no paths outside your
    worktree.
  • A test run is only real if it shows the expected ok count, zero
    (cached) markers    , and plausible wall time (#93). Any one of those
    alone is forgeable.
Manager note — unblocking this and dispatching it. `feature/daemon-mode` was the only item here needing an owner decision, so I have split it into **#94** (assigned to `sneak`, recommendation: post-1.0). Everything else in this issue is resolvable on technical merit, so this issue is now actionable and reassigned to `clawbot`. **Leave `feature/daemon-mode` alone** until #94 is answered — do not delete it, do not rebase it. ## One branch is already resolved `fix/sync-snapshot-cleanup` (tip `332ea26`) is **redundant** — confirmed twice, independently. It changes exactly one line, `v.Repositories.Snapshots.Delete(...)` → `v.deleteSnapshotFromLocalDB(...)` in `syncWithRemote`, and that change is already on `main` at `internal/vaultik/snapshot.go:1186`, having landed through the `deleteSnapshotFromLocalDB` error-propagation work (`ddc23f8` / `597b560`). The three-dot diff still shows it only because the merge base predates both. Nothing to fold, nothing to collide with — safe to delete. This is worth generalising to the rest of the triage: **a three-dot diff is not evidence a branch contains unlanded work.** That artifact is what made me briefly believe `golangci-v2.12.2` held 126 files of unpushed work earlier in this session; its tree hash turned out to be identical to `main`'s. Compare tips (two-dot) or tree hashes, not merge-base diffs. ## Reminder on the two that matter `fix/ctime-scanner-population` and `fix/sql-injection-whitelist` are the reason this is a 1.0 item rather than housekeeping. Neither may be deleted without first reading its diff and determining **against current `main`** whether the underlying bug still exists. If it does, land the fix with a regression test rather than deleting the branch. Write the determination down either way. `fix/ctime-scanner-population` deserves the most care: if ctime participates in change detection, a wrong ctime means changed files are silently skipped — data loss in a backup tool, and the kind of bug that leaves no trace until a restore. ## Environment notes for whoever picks this up - The shared BuildKit cache was destroyed earlier by another session, so builds are cold. Do **not** run `docker builder prune`; scope invalidation with `--no-cache` / `--no-cache-filter`. - Host-side lint runs may be VOID (#88) — retry unless the output has no `parallel golangci-lint is running` and cites no paths outside your worktree. - A test run is only real if it shows the expected `ok` count, **zero `(cached)` markers**, and plausible wall time (#93). Any one of those alone is forgeable.
sneak was unassigned by clawbot 2026-08-09 10:13:21 +02:00
clawbot self-assigned this 2026-08-09 10:13:21 +02:00
clawbot commented 2026-08-09 10:17:15 +02:00
Author
Collaborator
Copy Link
Copy Source

Plan

Investigation is complete; recording the plan before the destructive
half runs.

Method note, per the manager warning: every containment claim below
comes from two-dot tip diffs, tree-hash comparison, git cherry, and
git branch -r --merged origin/main. No three-dot / merge-base diff was
used as evidence. git worktree add --detach /tmp/impl-71 origin/main
at 50816b7; the shared clone's working tree is untouched.

git list_pull_requests state=open returns [], so no branch here
backs an open PR and no deletion closes one.

1. Delete the nine merged branches

All nine are ancestors of origin/main (git merge-base --is-ancestor
= true) with git cherry origin/main origin/<branch> reporting zero
+ commits. Merged status will be re-confirmed immediately before each
deletion, not from the list in this issue.

2. Delete golangci-v2.12.2

Its tree hash is 0f36f0b99599b63018da6cd3b0a62e2046eb3c16. That is no
longer main's tree — main has advanced to e5cf4017… — so the check
recorded in this issue has gone stale and needed redoing rather than
repeating. It still holds: main's commit cc58583 ("Update
golangci-lint to v2.12.2 with canonical config (#62)") has tree
0f36f0b9…, byte-identical to the branch tip. The branch landed
squashed; its two git cherry + commits are the unsquashed originals.

3. Delete fix/sync-snapshot-cleanup

Re-confirmed as instructed. The one-line change is on main — the call
is now v.deleteSnapshotFromLocalDB(snapshotIDStr) inside
syncWithRemote (internal/vaultik/snapshot.go:955; the function
begins at :913). Line 1186 cited in the manager note has moved with
main, but the content is present.

4. The two that matter

Both were read against current main. Neither describes a bug that
still exists, and in both cases main has moved past the branch by a
decision already recorded in the tracker — so neither will be landed.
Full evidence goes in the disposition comment; the short form:

  • fix/ctime-scanner-population — ctime no longer exists in this
    codebase. 1c72a37 ("Remove all ctime usage and storage (#55)")
    deleted the column, the File.CTime field and every use, per the
    decision on PR #48, closing #54. grep -rn 'CTime\|ctime' over the
    Go sources returns nothing. Change detection compares size, mtime,
    mode, uid, gid (internal/snapshot/scanner.go:1165-1171), exactly
    matching ARCHITECTURE.md:353 — ctime does not participate, so
    the silent-skip data-loss risk this issue raised does not exist.
  • fix/sql-injection-whitelist — the allow-list it adds was
    already tried and deliberately removed. bfd7334 ("fix: replace
    table name allowlist with regex sanitization") replaced it with
    validTableNameRe per review feedback on PR #32; #7 is closed.
    Landing this branch would also break main, whose allow-list would
    reject getTableCount("snapshots") (snapshot.go:1544).

5. Delete feature/restore-progress-bar

Superseded on every point it claims, and now actively regressive — it
deletes internal/blobgen/compress_test.go, the #28 regression test
that landed separately.

6. Leave feature/daemon-mode alone

Deferred to #94, awaiting the owner decision. Not deleted, not rebased,
not touched.

7. TODO.md

The stale text named in the definition of done is already gone:
e496aa3 removed the add-godoc-to-cli-package /
feature/pluggable-storage-backend entry. I will not re-fix it. I will
open one small PR applying the TODO.md Workflow section for this
issue — move the Next Step into Completed Steps with the triage record,
promote the top Future Step — verified with script/cibuild.

## Plan Investigation is complete; recording the plan before the destructive half runs. Method note, per the manager warning: every containment claim below comes from two-dot tip diffs, tree-hash comparison, `git cherry`, and `git branch -r --merged origin/main`. No three-dot / merge-base diff was used as evidence. `git worktree add --detach /tmp/impl-71 origin/main` at `50816b7`; the shared clone's working tree is untouched. `git list_pull_requests state=open` returns `[]`, so no branch here backs an open PR and no deletion closes one. ### 1. Delete the nine merged branches All nine are ancestors of `origin/main` (`git merge-base --is-ancestor` = true) with `git cherry origin/main origin/<branch>` reporting zero `+` commits. Merged status will be re-confirmed immediately before each deletion, not from the list in this issue. ### 2. Delete `golangci-v2.12.2` Its tree hash is `0f36f0b99599b63018da6cd3b0a62e2046eb3c16`. That is no longer `main`'s tree — `main` has advanced to `e5cf4017…` — so the check recorded in this issue has gone stale and needed redoing rather than repeating. It still holds: `main`'s commit `cc58583` ("Update golangci-lint to v2.12.2 with canonical config (#62)") has tree `0f36f0b9…`, byte-identical to the branch tip. The branch landed squashed; its two `git cherry` `+` commits are the unsquashed originals. ### 3. Delete `fix/sync-snapshot-cleanup` Re-confirmed as instructed. The one-line change is on `main` — the call is now `v.deleteSnapshotFromLocalDB(snapshotIDStr)` inside `syncWithRemote` (`internal/vaultik/snapshot.go:955`; the function begins at `:913`). Line 1186 cited in the manager note has moved with `main`, but the content is present. ### 4. The two that matter Both were read against current `main`. Neither describes a bug that still exists, and in both cases `main` has moved *past* the branch by a decision already recorded in the tracker — so neither will be landed. Full evidence goes in the disposition comment; the short form: - **`fix/ctime-scanner-population`** — `ctime` no longer exists in this codebase. `1c72a37` ("Remove all ctime usage and storage (#55)") deleted the column, the `File.CTime` field and every use, per the decision on PR #48, closing #54. `grep -rn 'CTime\|ctime'` over the Go sources returns nothing. Change detection compares size, mtime, mode, uid, gid (`internal/snapshot/scanner.go:1165-1171`), exactly matching `ARCHITECTURE.md:353` — ctime does **not** participate, so the silent-skip data-loss risk this issue raised does not exist. - **`fix/sql-injection-whitelist`** — the allow-list it adds was already tried and deliberately removed. `bfd7334` ("fix: replace table name allowlist with regex sanitization") replaced it with `validTableNameRe` per review feedback on PR #32; #7 is closed. Landing this branch would also break `main`, whose allow-list would reject `getTableCount("snapshots")` (`snapshot.go:1544`). ### 5. Delete `feature/restore-progress-bar` Superseded on every point it claims, and now actively regressive — it deletes `internal/blobgen/compress_test.go`, the #28 regression test that landed separately. ### 6. Leave `feature/daemon-mode` alone Deferred to #94, awaiting the owner decision. Not deleted, not rebased, not touched. ### 7. `TODO.md` The stale text named in the definition of done is already gone: `e496aa3` removed the `add-godoc-to-cli-package` / `feature/pluggable-storage-backend` entry. I will not re-fix it. I will open one small PR applying the `TODO.md` Workflow section for this issue — move the Next Step into Completed Steps with the triage record, promote the top Future Step — verified with `script/cibuild`.
clawbot commented 2026-08-09 10:21:45 +02:00
Author
Collaborator
Copy Link
Copy Source

Branch disposition inventory

Fourteen branches deleted, one deferred. git branch -r after the work:

origin/HEAD -> origin/main
origin/feature/daemon-mode
origin/main

plus origin/fix/triage-stale-remote-branches, which backs PR #95 and
is therefore allowed by the definition of done.

Method

Every containment claim below rests on two-dot tip diffs, tree-hash
equality, git cherry, and git merge-base --is-ancestor. No
three-dot / merge-base diff was used as evidence anywhere. Merged status
was re-derived immediately before each git push --delete, not read
from the list in the issue body — which mattered, see golangci-v2.12.2
below.

list_pull_requests state=open returned [] before deletion, so no
deletion closed a PR.

Group 1 — merged, deleted (9)

Re-verified at deletion time. All nine: git merge-base --is-ancestor origin/<branch> origin/main true, and git cherry origin/main origin/<branch> reporting zero + commits.

Branch Tip Evidence
add-compressstream-regression-test 2e7356d ancestor, 0 unlanded
feature/implement-prune-flag-on-snapshot-create 76f4421 ancestor, 0 unlanded
feature/pluggable-storage-backend 899448e ancestor, 0 unlanded
fix/issue-25 1e2eced ancestor, 0 unlanded
fix/issue-26 2f249e3 ancestor, 0 unlanded
fix/issue-27 162d76b ancestor, 0 unlanded
fix/issue-28 441c441 ancestor, 0 unlanded
fix/issue-29 9c66674 ancestor, 0 unlanded
fix/restore-error-handling 8c4ea2b ancestor, 0 unlanded

Note these all show large two-dot diffs against main purely because
they are old ancestors; ancestry, not diff size, is the containment test.

golangci-v2.12.2 — landed squashed, deleted

The check recorded in this issue had gone stale and had to be redone.
The issue says its tree hash equals main's. It no longer does: the
branch tree is still 0f36f0b99599b63018da6cd3b0a62e2046eb3c16, but
main has advanced and its tree is now e5cf4017…. Repeating the
recorded check would have failed and could have been misread as the
branch holding work.

The conclusion survives in a stronger form: main's commit cc58583
("Update golangci-lint to v2.12.2 with canonical config (#62)") has tree
0f36f0b99599b63018da6cd3b0a62e2046eb3c16 — byte-identical to the
branch tip — and cc58583 is an ancestor of main. The branch landed
squashed; its two git cherry + commits are the unsquashed originals.
Nothing unlanded. Deleted.

Group 2 — fix/sync-snapshot-cleanup, redundant, deleted

Re-confirmed as instructed. The single line is on main:

internal/vaultik/snapshot.go:955:  err = v.deleteSnapshotFromLocalDB(snapshotIDStr)

inside syncWithRemote, which begins at :913. The line number 1186
from the manager note has moved as main advanced, but the content is
present. Deleted.

Group 3 — the two that mattered

Both were read in full against current main. Neither describes a bug
that still exists, and in both cases main has moved past the branch
by a decision already recorded in the tracker. Neither was landed, and
neither owed a regression test, because there is nothing to regress
against.

fix/ctime-scanner-population (25860c0) — obsolete, deleted

The branch replaces CTime: info.ModTime() with a platform-specific
fileCTime(info) in checkFileInMemory. That field no longer exists.

  • 1c72a37 "Remove all ctime usage and storage (#55)" (2026-03-20,
    ancestor of main) removed the ctime column from schema.sql, the
    CTime field from File, every INSERT/SELECT/scan target in
    internal/database/files.go, the scanner assignment, and the docs —
    explicitly per the decision on PR #48, closing #54.
  • grep -rn 'CTime\|ctime' --include=*.go over main returns nothing.
    grep -rn ctime over *.sql and *.md likewise returns nothing.
  • The branch tip is dated 2026-03-17, three days before that removal, and
    #13 (which it closes) was closed 2026-03-19.

On the data-loss concern that made this a 1.0 item: it does not
apply. Change detection on main is
internal/snapshot/scanner.go:1165-1171:

if existingFile.Size != file.Size ||
    existingFile.MTime.Unix() != file.MTime.Unix() ||
    existingFile.Mode != file.Mode ||
    existingFile.UID != file.UID ||
    existingFile.GID != file.GID {

Size, mtime, mode, uid, gid — exactly the five fields ARCHITECTURE.md:353
lists, with no discrepancy between doc and code. ctime does not
participate and never did, so a wrong ctime could not cause a changed
file to be silently skipped. Landing this branch would reintroduce a
column that was deliberately removed.

fix/sql-injection-whitelist (3e282af) — would revert a decision, deleted

The tip is a merge commit; the only real commit is bb4b9b5 ("fix: use
whitelist for SQL table names in getTableCount (closes #7)"), which
replaces validTableNameRe with an allow-list of {files, chunks, blobs}.

That is backwards relative to main. Commit bfd7334, "fix: replace
table name allowlist with regex sanitization" (ancestor of main),
did the opposite deliberately: "Replace the hardcoded validTableNames
allowlist with a regexp… Addresses review feedback from @sneak on PR
#32." Issue #7 is closed. Landing the branch would revert an explicit
owner decision.

It would also break main. main calls getTableCount("snapshots")
at snapshot.go:1544; snapshots is not in the branch's allow-list, and
the call site discards the error into _, so snapshotCountBefore would
silently become 0.

Exposure assessment on main, since the issue asked for one: nil, not
merely small. getTableCount is unexported with all seven call sites
passing string literals (snapshots, files, chunks, blobs), and
^[a-z0-9_]+$ admits no quote, whitespace, semicolon or parenthesis, so
no payload is expressible. repeatPlaceholder(n) — cited in the issue as
evidence of dynamic query construction — emits only bound ?
placeholders (internal/database/database.go:418-426); its two callers
in file_chunks.go:196 and chunk_files.go:165 bind values as
parameters and interpolate nothing. That is the correct pattern.

feature/restore-progress-bar (ee161fb) — superseded and now regressive, deleted

Superseded on every point it claims, and it contains nothing main
lacks:

  • restore progress — main has printRestoreProgress
    (internal/vaultik/restore.go:487, called at :445) plus
    internal/snapshot/progress.go;
  • disk-based LRU blob cache, which it claims closes #29 — main has
    internal/vaultik/blobcache.go with blobDiskCache and evictLRU();
  • removing blob_fetch_stub.go — already absent from main.

It has additionally become actively harmful: its diff deletes
internal/blobgen/compress_test.go, the #28 double-close regression test
that landed separately and is present on main. Deleted.

Deferred — feature/daemon-mode (87acc05)

Out of scope per the manager note. Split to #94, awaiting an owner
decision, recommendation post-1.0. Not deleted, not rebased, not
touched. It is the only branch besides main remaining on the remote.

TODO.md

Definition-of-done item 5 asks for TODO.md:51-53 to be corrected. It
already is — e496aa3 ("Finish the lint remediation: script/cibuild
exits 0 (closes #61)") removed the entry naming
add-godoc-to-cli-package and feature/pluggable-storage-backend.
git log -S add-godoc-to-cli-package -- TODO.md returns only e496aa3
and the original fc56b0c. I did not re-fix it.

PR #95 applies the TODO.md Workflow step for this issue instead. It
changes TODO.md only; no product code changed anywhere in this triage.
Verified with script/cibuild exit 0 — 14 ok packages, zero (cached)
markers, 49.9s test wall time, and CHECK_EPOCH guard steps reporting
DONE rather than CACHED.

## Branch disposition inventory Fourteen branches deleted, one deferred. `git branch -r` after the work: ``` origin/HEAD -> origin/main origin/feature/daemon-mode origin/main ``` plus `origin/fix/triage-stale-remote-branches`, which backs PR #95 and is therefore allowed by the definition of done. ### Method Every containment claim below rests on two-dot tip diffs, tree-hash equality, `git cherry`, and `git merge-base --is-ancestor`. No three-dot / merge-base diff was used as evidence anywhere. Merged status was re-derived immediately before each `git push --delete`, not read from the list in the issue body — which mattered, see `golangci-v2.12.2` below. `list_pull_requests state=open` returned `[]` before deletion, so no deletion closed a PR. ### Group 1 — merged, deleted (9) Re-verified at deletion time. All nine: `git merge-base --is-ancestor origin/<branch> origin/main` true, and `git cherry origin/main origin/<branch>` reporting **zero** `+` commits. | Branch | Tip | Evidence | | --- | --- | --- | | `add-compressstream-regression-test` | `2e7356d` | ancestor, 0 unlanded | | `feature/implement-prune-flag-on-snapshot-create` | `76f4421` | ancestor, 0 unlanded | | `feature/pluggable-storage-backend` | `899448e` | ancestor, 0 unlanded | | `fix/issue-25` | `1e2eced` | ancestor, 0 unlanded | | `fix/issue-26` | `2f249e3` | ancestor, 0 unlanded | | `fix/issue-27` | `162d76b` | ancestor, 0 unlanded | | `fix/issue-28` | `441c441` | ancestor, 0 unlanded | | `fix/issue-29` | `9c66674` | ancestor, 0 unlanded | | `fix/restore-error-handling` | `8c4ea2b` | ancestor, 0 unlanded | Note these all show large two-dot diffs against `main` purely because they are old ancestors; ancestry, not diff size, is the containment test. ### `golangci-v2.12.2` — landed squashed, deleted **The check recorded in this issue had gone stale and had to be redone.** The issue says its tree hash equals `main`'s. It no longer does: the branch tree is still `0f36f0b99599b63018da6cd3b0a62e2046eb3c16`, but `main` has advanced and its tree is now `e5cf4017…`. Repeating the recorded check would have failed and could have been misread as the branch holding work. The conclusion survives in a stronger form: `main`'s commit `cc58583` ("Update golangci-lint to v2.12.2 with canonical config (#62)") has tree `0f36f0b99599b63018da6cd3b0a62e2046eb3c16` — byte-identical to the branch tip — and `cc58583` is an ancestor of `main`. The branch landed squashed; its two `git cherry` `+` commits are the unsquashed originals. Nothing unlanded. Deleted. ### Group 2 — `fix/sync-snapshot-cleanup`, redundant, deleted Re-confirmed as instructed. The single line is on `main`: ``` internal/vaultik/snapshot.go:955: err = v.deleteSnapshotFromLocalDB(snapshotIDStr) ``` inside `syncWithRemote`, which begins at `:913`. The line number 1186 from the manager note has moved as `main` advanced, but the content is present. Deleted. ### Group 3 — the two that mattered Both were read in full against current `main`. **Neither describes a bug that still exists**, and in both cases `main` has moved *past* the branch by a decision already recorded in the tracker. Neither was landed, and neither owed a regression test, because there is nothing to regress against. #### `fix/ctime-scanner-population` (`25860c0`) — obsolete, deleted The branch replaces `CTime: info.ModTime()` with a platform-specific `fileCTime(info)` in `checkFileInMemory`. That field no longer exists. - `1c72a37` "Remove all ctime usage and storage (#55)" (2026-03-20, ancestor of `main`) removed the `ctime` column from `schema.sql`, the `CTime` field from `File`, every INSERT/SELECT/scan target in `internal/database/files.go`, the scanner assignment, and the docs — explicitly per the decision on PR #48, closing #54. - `grep -rn 'CTime\|ctime' --include=*.go` over `main` returns **nothing**. `grep -rn ctime` over `*.sql` and `*.md` likewise returns nothing. - The branch tip is dated 2026-03-17, three days before that removal, and #13 (which it closes) was closed 2026-03-19. **On the data-loss concern that made this a 1.0 item:** it does not apply. Change detection on `main` is `internal/snapshot/scanner.go:1165-1171`: ```go if existingFile.Size != file.Size || existingFile.MTime.Unix() != file.MTime.Unix() || existingFile.Mode != file.Mode || existingFile.UID != file.UID || existingFile.GID != file.GID { ``` Size, mtime, mode, uid, gid — exactly the five fields `ARCHITECTURE.md:353` lists, with no discrepancy between doc and code. `ctime` does not participate and never did, so a wrong `ctime` could not cause a changed file to be silently skipped. Landing this branch would reintroduce a column that was deliberately removed. #### `fix/sql-injection-whitelist` (`3e282af`) — would revert a decision, deleted The tip is a merge commit; the only real commit is `bb4b9b5` ("fix: use whitelist for SQL table names in getTableCount (closes #7)"), which replaces `validTableNameRe` with an allow-list of `{files, chunks, blobs}`. That is backwards relative to `main`. Commit `bfd7334`, **"fix: replace table name allowlist with regex sanitization"** (ancestor of `main`), did the opposite deliberately: *"Replace the hardcoded validTableNames allowlist with a regexp… Addresses review feedback from @sneak on PR #32."* Issue #7 is closed. Landing the branch would revert an explicit owner decision. It would also **break `main`**. `main` calls `getTableCount("snapshots")` at `snapshot.go:1544`; `snapshots` is not in the branch's allow-list, and the call site discards the error into `_`, so `snapshotCountBefore` would silently become 0. Exposure assessment on `main`, since the issue asked for one: nil, not merely small. `getTableCount` is unexported with all seven call sites passing string literals (`snapshots`, `files`, `chunks`, `blobs`), and `^[a-z0-9_]+$` admits no quote, whitespace, semicolon or parenthesis, so no payload is expressible. `repeatPlaceholder(n)` — cited in the issue as evidence of dynamic query construction — emits only bound `?` placeholders (`internal/database/database.go:418-426`); its two callers in `file_chunks.go:196` and `chunk_files.go:165` bind values as parameters and interpolate nothing. That is the correct pattern. #### `feature/restore-progress-bar` (`ee161fb`) — superseded and now regressive, deleted Superseded on every point it claims, and it contains nothing `main` lacks: - restore progress — `main` has `printRestoreProgress` (`internal/vaultik/restore.go:487`, called at `:445`) plus `internal/snapshot/progress.go`; - disk-based LRU blob cache, which it claims closes #29 — `main` has `internal/vaultik/blobcache.go` with `blobDiskCache` and `evictLRU()`; - removing `blob_fetch_stub.go` — already absent from `main`. It has additionally become **actively harmful**: its diff deletes `internal/blobgen/compress_test.go`, the #28 double-close regression test that landed separately and is present on `main`. Deleted. ### Deferred — `feature/daemon-mode` (`87acc05`) Out of scope per the manager note. Split to **#94**, awaiting an owner decision, recommendation post-1.0. **Not deleted, not rebased, not touched.** It is the only branch besides `main` remaining on the remote. ### `TODO.md` Definition-of-done item 5 asks for `TODO.md:51-53` to be corrected. It already is — `e496aa3` ("Finish the lint remediation: script/cibuild exits 0 (closes #61)") removed the entry naming `add-godoc-to-cli-package` and `feature/pluggable-storage-backend`. `git log -S add-godoc-to-cli-package -- TODO.md` returns only `e496aa3` and the original `fc56b0c`. I did not re-fix it. PR **#95** applies the `TODO.md` Workflow step for this issue instead. It changes `TODO.md` only; no product code changed anywhere in this triage. Verified with `script/cibuild` exit 0 — 14 `ok` packages, zero `(cached)` markers, 49.9s test wall time, and `CHECK_EPOCH` guard steps reporting `DONE` rather than `CACHED`.
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 labels
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees clawbot
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/vaultik#71