Triage the 15 stale remote branches; two may contain unlanded fixes #71
Reference in New Issue
Block a user
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?
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— deleteConfirmed via
git branch -r --merged origin/main:add-compressstream-regression-testfeature/implement-prune-flag-on-snapshot-createfeature/pluggable-storage-backendfix/issue-25,fix/issue-26,fix/issue-27,fix/issue-28,fix/issue-29fix/restore-error-handlingAlso delete
golangci-v2.12.2— its tree hash is identical tomain(
0f36f0b99599b63018da6cd3b0a62e2046eb3c16), so it contributes nothing.Unmerged — need a decision each
fix/ctime-scanner-population25860c0ARCHITECTURE.md:353says 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-whitelist3e282afinternal/database/database.go:420has arepeatPlaceholder(n)helper, confirming dynamic query construction exists. Security-adjacent; evaluate before 1.0.fix/sync-snapshot-cleanup332ea26feature/daemon-mode87acc05daemoncommand onmain, README documents none. Unshipped scope — decide 1.0, post-1.0, or delete.feature/restore-progress-baree161fbmainalready has restore progress viaprintRestoreProgressandinternal/snapshot/progress.go. Likely superseded — verify, then delete.Definition of done
rebased onto
mainand landed via its own PR; converted into atracked 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.
fix/ctime-scanner-populationandfix/sql-injection-whitelistareassessed on their merits against current
main— read the diff anddetermine 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.
golangci-v2.12.2are deleted fromorigin.git branch -rafterwards shows onlymain,HEAD, and any branchwith an open PR.
TODO.md:51-53is corrected: it namesadd-godoc-to-cli-package(no longer exists) and
feature/pluggable-storage-backend(alreadymerged), so the entry is stale in both halves.
git branch -r --merged origin/mainat the time of deletion.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-modeis different: it is unshipped product scope. Tip87acc05, a long-running daemon/watcher mode with filesystem-changebatching. No
daemoncommand exists onmainand the README documentsnone, so nothing today depends on it.
Options:
the design is not lost, and revisit after the tag.
main, bring it up to current interfaces, addthe CLI surface, document it, and test it — a substantial work unit
that would meaningfully extend the path to a tag.
My recommendation is option 1. The 1.0.0 milestone as scoped is
already carrying a
snapshot listcorrectness fix (#64), a releasepipeline 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
sneakfor the daemon-mode call only.Manager note — unblocking this and dispatching it.
feature/daemon-modewas 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-modealone until #94 is answered — do notdelete it, do not rebase it.
One branch is already resolved
fix/sync-snapshot-cleanup(tip332ea26) is redundant — confirmedtwice, independently. It changes exactly one line,
v.Repositories.Snapshots.Delete(...)→v.deleteSnapshotFromLocalDB(...)in
syncWithRemote, and that change is already onmainatinternal/vaultik/snapshot.go:1186, having landed through thedeleteSnapshotFromLocalDBerror-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.2held 126 files of unpushedwork 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-populationandfix/sql-injection-whitelistare thereason this is a 1.0 item rather than housekeeping. Neither may be deleted
without first reading its diff and determining against current
mainwhether 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-populationdeserves the most care: if ctimeparticipates 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
builds are cold. Do not run
docker builder prune; scopeinvalidation with
--no-cache/--no-cache-filter.parallel golangci-lint is runningand cites no paths outside yourworktree.
okcount, zero(cached)markers, and plausible wall time (#93). Any one of thosealone is forgeable.
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, andgit branch -r --merged origin/main. No three-dot / merge-base diff wasused as evidence.
git worktree add --detach /tmp/impl-71 origin/mainat
50816b7; the shared clone's working tree is untouched.git list_pull_requests state=openreturns[], so no branch herebacks 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 eachdeletion, not from the list in this issue.
2. Delete
golangci-v2.12.2Its tree hash is
0f36f0b99599b63018da6cd3b0a62e2046eb3c16. That is nolonger
main's tree —mainhas advanced toe5cf4017…— so the checkrecorded in this issue has gone stale and needed redoing rather than
repeating. It still holds:
main's commitcc58583("Updategolangci-lint to v2.12.2 with canonical config (#62)") has tree
0f36f0b9…, byte-identical to the branch tip. The branch landedsquashed; its two
git cherry+commits are the unsquashed originals.3. Delete
fix/sync-snapshot-cleanupRe-confirmed as instructed. The one-line change is on
main— the callis now
v.deleteSnapshotFromLocalDB(snapshotIDStr)insidesyncWithRemote(internal/vaultik/snapshot.go:955; the functionbegins at
:913). Line 1186 cited in the manager note has moved withmain, but the content is present.4. The two that matter
Both were read against current
main. Neither describes a bug thatstill exists, and in both cases
mainhas moved past the branch by adecision already recorded in the tracker — so neither will be landed.
Full evidence goes in the disposition comment; the short form:
fix/ctime-scanner-population—ctimeno longer exists in thiscodebase.
1c72a37("Remove all ctime usage and storage (#55)")deleted the column, the
File.CTimefield and every use, per thedecision on PR #48, closing #54.
grep -rn 'CTime\|ctime'over theGo sources returns nothing. Change detection compares size, mtime,
mode, uid, gid (
internal/snapshot/scanner.go:1165-1171), exactlymatching
ARCHITECTURE.md:353— ctime does not participate, sothe silent-skip data-loss risk this issue raised does not exist.
fix/sql-injection-whitelist— the allow-list it adds wasalready tried and deliberately removed.
bfd7334("fix: replacetable name allowlist with regex sanitization") replaced it with
validTableNameReper review feedback on PR #32; #7 is closed.Landing this branch would also break
main, whose allow-list wouldreject
getTableCount("snapshots")(snapshot.go:1544).5. Delete
feature/restore-progress-barSuperseded on every point it claims, and now actively regressive — it
deletes
internal/blobgen/compress_test.go, the #28 regression testthat landed separately.
6. Leave
feature/daemon-modealoneDeferred to #94, awaiting the owner decision. Not deleted, not rebased,
not touched.
7.
TODO.mdThe stale text named in the definition of done is already gone:
e496aa3removed theadd-godoc-to-cli-package/feature/pluggable-storage-backendentry. I will not re-fix it. I willopen one small PR applying the
TODO.mdWorkflow section for thisissue — move the Next Step into Completed Steps with the triage record,
promote the top Future Step — verified with
script/cibuild.Branch disposition inventory
Fourteen branches deleted, one deferred.
git branch -rafter the work:plus
origin/fix/triage-stale-remote-branches, which backs PR #95 andis therefore allowed by the definition of done.
Method
Every containment claim below rests on two-dot tip diffs, tree-hash
equality,
git cherry, andgit merge-base --is-ancestor. Nothree-dot / merge-base diff was used as evidence anywhere. Merged status
was re-derived immediately before each
git push --delete, not readfrom the list in the issue body — which mattered, see
golangci-v2.12.2below.
list_pull_requests state=openreturned[]before deletion, so nodeletion closed a PR.
Group 1 — merged, deleted (9)
Re-verified at deletion time. All nine:
git merge-base --is-ancestor origin/<branch> origin/maintrue, andgit cherry origin/main origin/<branch>reporting zero+commits.add-compressstream-regression-test2e7356dfeature/implement-prune-flag-on-snapshot-create76f4421feature/pluggable-storage-backend899448efix/issue-251e2ecedfix/issue-262f249e3fix/issue-27162d76bfix/issue-28441c441fix/issue-299c66674fix/restore-error-handling8c4ea2bNote these all show large two-dot diffs against
mainpurely becausethey are old ancestors; ancestry, not diff size, is the containment test.
golangci-v2.12.2— landed squashed, deletedThe 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: thebranch tree is still
0f36f0b99599b63018da6cd3b0a62e2046eb3c16, butmainhas advanced and its tree is nowe5cf4017…. Repeating therecorded check would have failed and could have been misread as the
branch holding work.
The conclusion survives in a stronger form:
main's commitcc58583("Update golangci-lint to v2.12.2 with canonical config (#62)") has tree
0f36f0b99599b63018da6cd3b0a62e2046eb3c16— byte-identical to thebranch tip — and
cc58583is an ancestor ofmain. The branch landedsquashed; its two
git cherry+commits are the unsquashed originals.Nothing unlanded. Deleted.
Group 2 —
fix/sync-snapshot-cleanup, redundant, deletedRe-confirmed as instructed. The single line is on
main:inside
syncWithRemote, which begins at:913. The line number 1186from the manager note has moved as
mainadvanced, but the content ispresent. Deleted.
Group 3 — the two that mattered
Both were read in full against current
main. Neither describes a bugthat still exists, and in both cases
mainhas moved past the branchby 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, deletedThe branch replaces
CTime: info.ModTime()with a platform-specificfileCTime(info)incheckFileInMemory. That field no longer exists.1c72a37"Remove all ctime usage and storage (#55)" (2026-03-20,ancestor of
main) removed thectimecolumn fromschema.sql, theCTimefield fromFile, every INSERT/SELECT/scan target ininternal/database/files.go, the scanner assignment, and the docs —explicitly per the decision on PR #48, closing #54.
grep -rn 'CTime\|ctime' --include=*.goovermainreturns nothing.grep -rn ctimeover*.sqland*.mdlikewise returns nothing.#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
mainisinternal/snapshot/scanner.go:1165-1171:Size, mtime, mode, uid, gid — exactly the five fields
ARCHITECTURE.md:353lists, with no discrepancy between doc and code.
ctimedoes notparticipate and never did, so a wrong
ctimecould not cause a changedfile to be silently skipped. Landing this branch would reintroduce a
column that was deliberately removed.
fix/sql-injection-whitelist(3e282af) — would revert a decision, deletedThe tip is a merge commit; the only real commit is
bb4b9b5("fix: usewhitelist for SQL table names in getTableCount (closes #7)"), which
replaces
validTableNameRewith an allow-list of{files, chunks, blobs}.That is backwards relative to
main. Commitbfd7334, "fix: replacetable 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.maincallsgetTableCount("snapshots")at
snapshot.go:1544;snapshotsis not in the branch's allow-list, andthe call site discards the error into
_, sosnapshotCountBeforewouldsilently become 0.
Exposure assessment on
main, since the issue asked for one: nil, notmerely small.
getTableCountis unexported with all seven call sitespassing string literals (
snapshots,files,chunks,blobs), and^[a-z0-9_]+$admits no quote, whitespace, semicolon or parenthesis, sono payload is expressible.
repeatPlaceholder(n)— cited in the issue asevidence of dynamic query construction — emits only bound
?placeholders (
internal/database/database.go:418-426); its two callersin
file_chunks.go:196andchunk_files.go:165bind values asparameters and interpolate nothing. That is the correct pattern.
feature/restore-progress-bar(ee161fb) — superseded and now regressive, deletedSuperseded on every point it claims, and it contains nothing
mainlacks:
mainhasprintRestoreProgress(
internal/vaultik/restore.go:487, called at:445) plusinternal/snapshot/progress.go;mainhasinternal/vaultik/blobcache.gowithblobDiskCacheandevictLRU();blob_fetch_stub.go— already absent frommain.It has additionally become actively harmful: its diff deletes
internal/blobgen/compress_test.go, the #28 double-close regression testthat 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
mainremaining on the remote.TODO.mdDefinition-of-done item 5 asks for
TODO.md:51-53to be corrected. Italready is —
e496aa3("Finish the lint remediation: script/cibuildexits 0 (closes #61)") removed the entry naming
add-godoc-to-cli-packageandfeature/pluggable-storage-backend.git log -S add-godoc-to-cli-package -- TODO.mdreturns onlye496aa3and the original
fc56b0c. I did not re-fix it.PR #95 applies the
TODO.mdWorkflow step for this issue instead. Itchanges
TODO.mdonly; no product code changed anywhere in this triage.Verified with
script/cibuildexit 0 — 14okpackages, zero(cached)markers, 49.9s test wall time, and
CHECK_EPOCHguard steps reportingDONErather thanCACHED.