Record the stale-branch triage and advance TODO.md (closes #71) #95

clawbot · 2026-08-09T10:21:00+02:00

clawbot commented

2026-08-09 10:21:00 +02:00

Issue #71 was mostly investigation and branch deletion. Fourteen of the
fifteen stale branches are now deleted from origin; the remaining one,
feature/daemon-mode, is deferred to #94 pending an owner decision and
was not touched.

The triage resolved entirely to deletions, so no product code changes
here. This PR is the TODO.md Workflow step for the issue, which the
definition of done asks for explicitly if no other PR is opened.

What is in the diff

TODO.md only, 37 insertions and 2 deletions:

Next Step moves into Completed Steps with the triage record.
The top Future Step ("Define remaining scope for a first tagged
release and cut v0.1.0.") is promoted to Next Step, leaving Future
Steps empty and marked as such rather than silently blank.

The full per-branch disposition with evidence is recorded on issue #71
rather than duplicated here.

The stale TODO entry needed no fix

The definition of done calls for correcting TODO.md:51-53, which named
add-godoc-to-cli-package (gone) and feature/pluggable-storage-backend
(merged). That text is already absent from main — e496aa3 removed it.
I did not re-fix it; git log -S add-godoc-to-cli-package -- TODO.md
returns e496aa3 and the original fc56b0c, and nothing else.

The two branches this issue existed for

Neither was landed, because in both cases main has already moved past
them by a decision recorded in the tracker. Neither is a bug that still
exists, so neither owed a regression test.

fix/ctime-scanner-population would repopulate a field that no
longer exists. 1c72a37 ("Remove all ctime usage and storage (#55)")
deleted the ctime column, the File.CTime field and every use, per the
decision on PR #48, closing #54. grep -rn 'CTime\|ctime' --include=*.go
over main returns nothing at all.

The data-loss concern raised in the issue does not apply: change
detection compares size, mtime, mode, uid and gid
(internal/snapshot/scanner.go:1165-1171), exactly matching
ARCHITECTURE.md:353. ctime never participated, so a wrong ctime
could not cause a changed file to be silently skipped.

fix/sql-injection-whitelist would revert a deliberate decision.
bfd7334 ("fix: replace table name allowlist with regex sanitization")
replaced that exact allow-list with validTableNameRe on review feedback
from PR #32; #7 is closed. Landing the branch would also break main:
its allow-list is {files, chunks, blobs} and main calls
getTableCount("snapshots") at snapshot.go:1544, whose error is
discarded into _, so the count would silently become 0.

For the record, the remaining exposure on main is nil rather than
merely small: getTableCount is unexported, all seven call sites pass
string literals, and ^[a-z0-9_]+$ admits no quote, space, semicolon or
parenthesis. The repeatPlaceholder helper cited in the issue emits only
bound ? placeholders and interpolates no values.

Verification

script/cibuild, exit 0, on this branch.

Checked against the forgery criteria in #93 rather than trusting the exit
code: 14 ok packages, zero (cached) markers, test stage 49.9s
wall and lint stage 36.6s. The CHECK_EPOCH guard steps report DONE,
not CACHED, so the check layers genuinely executed. Lint ran in the
container, so the host-lint voidness of #88 does not apply.

make fmt is a no-op here — script/fmt runs go fmt ./... only, and
this PR touches no Go files. Prose wrapping matches the surrounding file.

The gomodguard deprecation warning appears in the lint output; it is
already tracked as #90 and is not addressed here.

Issue #71 was mostly investigation and branch deletion. Fourteen of the fifteen stale branches are now deleted from `origin`; the remaining one, `feature/daemon-mode`, is deferred to #94 pending an owner decision and was not touched. The triage resolved entirely to deletions, so **no product code changes here**. This PR is the `TODO.md` Workflow step for the issue, which the definition of done asks for explicitly if no other PR is opened. ## What is in the diff `TODO.md` only, 37 insertions and 2 deletions: - Next Step moves into Completed Steps with the triage record. - The top Future Step ("Define remaining scope for a first tagged release and cut v0.1.0.") is promoted to Next Step, leaving Future Steps empty and marked as such rather than silently blank. The full per-branch disposition with evidence is recorded on issue #71 rather than duplicated here. ## The stale TODO entry needed no fix The definition of done calls for correcting `TODO.md:51-53`, which named `add-godoc-to-cli-package` (gone) and `feature/pluggable-storage-backend` (merged). That text is already absent from `main` — `e496aa3` removed it. I did not re-fix it; `git log -S add-godoc-to-cli-package -- TODO.md` returns `e496aa3` and the original `fc56b0c`, and nothing else. ## The two branches this issue existed for Neither was landed, because in both cases `main` has already moved past them by a decision recorded in the tracker. Neither is a bug that still exists, so neither owed a regression test. **`fix/ctime-scanner-population`** would repopulate a field that no longer exists. `1c72a37` ("Remove all ctime usage and storage (#55)") deleted the `ctime` column, the `File.CTime` field and every use, per the decision on PR #48, closing #54. `grep -rn 'CTime\|ctime' --include=*.go` over `main` returns nothing at all. The data-loss concern raised in the issue does not apply: change detection compares size, mtime, mode, uid and gid (`internal/snapshot/scanner.go:1165-1171`), exactly matching `ARCHITECTURE.md:353`. `ctime` never participated, so a wrong `ctime` could not cause a changed file to be silently skipped. **`fix/sql-injection-whitelist`** would revert a deliberate decision. `bfd7334` ("fix: replace table name allowlist with regex sanitization") replaced that exact allow-list with `validTableNameRe` on review feedback from PR #32; #7 is closed. Landing the branch would also break `main`: its allow-list is `{files, chunks, blobs}` and `main` calls `getTableCount("snapshots")` at `snapshot.go:1544`, whose error is discarded into `_`, so the count would silently become 0. For the record, the remaining exposure on `main` is nil rather than merely small: `getTableCount` is unexported, all seven call sites pass string literals, and `^[a-z0-9_]+$` admits no quote, space, semicolon or parenthesis. The `repeatPlaceholder` helper cited in the issue emits only bound `?` placeholders and interpolates no values. ## Verification `script/cibuild`, exit 0, on this branch. Checked against the forgery criteria in #93 rather than trusting the exit code: 14 `ok` packages, **zero** `(cached)` markers, test stage 49.9s wall and lint stage 36.6s. The `CHECK_EPOCH` guard steps report `DONE`, not `CACHED`, so the check layers genuinely executed. Lint ran in the container, so the host-lint voidness of #88 does not apply. `make fmt` is a no-op here — `script/fmt` runs `go fmt ./...` only, and this PR touches no Go files. Prose wrapping matches the surrounding file. The `gomodguard` deprecation warning appears in the lint output; it is already tracked as #90 and is not addressed here.

clawbot added 1 commit 2026-08-09 10:21:00 +02:00

Record the stale-branch triage and advance TODO.md (closes #71 )

check / check (pull_request) Successful in 2m12s

Details

7e49b483d8

Fourteen of the fifteen stale remote branches are deleted; the
per-branch disposition and its evidence live on issue #71. No product
code changes -- the triage resolved to deletions, so this commit is the
TODO.md Workflow step for the issue.

Two branches were the reason the issue existed, and both turned out to
be closed questions that main had already moved past:

fix/ctime-scanner-population would repopulate a field that no longer
exists. 1c72a37 removed the ctime column, the File.CTime field and
every use (#54/#55). Change detection compares size, mtime, mode, uid
and gid only, matching what ARCHITECTURE.md documents, so ctime never
participated and the silently-skipped-file data-loss risk that made
this a 1.0 item does not exist.

fix/sql-injection-whitelist would revert bfd7334, which deliberately
replaced that allow-list with regex sanitisation on review feedback,
and would break getTableCount("snapshots") because its allow-list omits
that table.

Containment was established with two-dot tip diffs, tree-hash equality
and git cherry rather than three-dot merge-base diffs, which replay
everything that landed on main after a branch diverged and make old
branches look like they hold unlanded work.

feature/daemon-mode is untouched, deferred to #94 pending an owner
decision. The stale TODO.md entry named in the issue needed no fix;
e496aa3 had already removed it.

clawbot referenced this pull request

2026-08-09 10:21:45 +02:00

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

clawbot commented

2026-08-09 10:22:06 +02:00

What I built

One file changed, TODO.md, 37 insertions / 2 deletions. No product
code. The triage this PR records resolved entirely to branch deletions,
so there was nothing to implement.

Next Step moved to the top of Completed Steps with the triage record.
Top Future Step promoted to Next Step; Future Steps left explicitly
empty rather than blank.

Fourteen branches were deleted from origin. The per-branch inventory
with evidence is on #71 (comment 49071). Final remote state is
origin/main, origin/feature/daemon-mode (deferred to #94, untouched),
and this PR's branch.

Why no regression test

The definition of done requires a regression test for any code that
lands. No code lands. Both candidate fix branches were determined not to
describe bugs that exist on main:

ctime was removed from the codebase entirely by 1c72a37 (#54/#55);
grep -rn 'CTime\|ctime' --include=*.go over main returns nothing,
and change detection compares only size, mtime, mode, uid, gid.
the SQL allow-list branch would revert bfd7334, which replaced that
allow-list with regex sanitisation on review feedback (#7 closed), and
would break getTableCount("snapshots").

A test asserting either would be a test of a decision, not of behaviour.

How I verified

script/cibuild on this branch, exit 0.

Per #93 I did not treat the exit code as sufficient. All three
independent signals check out:

ok count: 14 packages ok, 4 [no test files], 0 FAIL.
(cached) markers: grep -c '(cached)' over the full build log
returns 0.
Wall time: test stage 49.9s, lint stage 36.6s, fmt-check 2.6s —
plausible for -race on a cold cache, not instant replay.

Additionally the CHECK_EPOCH guard steps (RUN [ -n "…" ] || exit 1)
report DONE, not CACHED, in both the lint and builder stages, so the
check layers genuinely executed rather than being served from the layer
cache — the failure mode #85 and #91 were about.

Docker cache handling: I scoped nothing globally and ran no
docker builder prune. The cold dependency layers came back as CACHED
and were reused; only the epoch-guarded check layers re-ran.

Lint ran inside the container, so the host-side voidness of #88 does not
apply — the output cites no paths outside the build context and contains
no parallel golangci-lint is running.

make fmt is a no-op for this change: script/fmt runs go fmt ./...
only and this PR touches no Go files. The repo has no markdown
formatter wired into script/fmt, so prose wrapping was matched to the
surrounding file by hand.

Noted, not addressed

The lint stage emits The linter 'gomodguard' is deprecated (since v2.12.0). Already tracked as #90; out of scope here and left alone.

## What I built One file changed, `TODO.md`, 37 insertions / 2 deletions. No product code. The triage this PR records resolved entirely to branch deletions, so there was nothing to implement. - Next Step moved to the top of Completed Steps with the triage record. - Top Future Step promoted to Next Step; Future Steps left explicitly empty rather than blank. Fourteen branches were deleted from `origin`. The per-branch inventory with evidence is on #71 (comment 49071). Final remote state is `origin/main`, `origin/feature/daemon-mode` (deferred to #94, untouched), and this PR's branch. ## Why no regression test The definition of done requires a regression test for any code that lands. No code lands. Both candidate fix branches were determined not to describe bugs that exist on `main`: - `ctime` was removed from the codebase entirely by `1c72a37` (#54/#55); `grep -rn 'CTime\|ctime' --include=*.go` over `main` returns nothing, and change detection compares only size, mtime, mode, uid, gid. - the SQL allow-list branch would revert `bfd7334`, which replaced that allow-list with regex sanitisation on review feedback (#7 closed), and would break `getTableCount("snapshots")`. A test asserting either would be a test of a decision, not of behaviour. ## How I verified `script/cibuild` on this branch, **exit 0**. Per #93 I did not treat the exit code as sufficient. All three independent signals check out: - **`ok` count**: 14 packages `ok`, 4 `[no test files]`, 0 `FAIL`. - **`(cached)` markers**: `grep -c '(cached)'` over the full build log returns **0**. - **Wall time**: test stage 49.9s, lint stage 36.6s, fmt-check 2.6s — plausible for `-race` on a cold cache, not instant replay. Additionally the `CHECK_EPOCH` guard steps (`RUN [ -n "…" ] || exit 1`) report `DONE`, not `CACHED`, in both the lint and builder stages, so the check layers genuinely executed rather than being served from the layer cache — the failure mode #85 and #91 were about. Docker cache handling: I scoped nothing globally and ran no `docker builder prune`. The cold dependency layers came back as `CACHED` and were reused; only the epoch-guarded check layers re-ran. Lint ran inside the container, so the host-side voidness of #88 does not apply — the output cites no paths outside the build context and contains no `parallel golangci-lint is running`. `make fmt` is a no-op for this change: `script/fmt` runs `go fmt ./...` only and this PR touches no Go files. The repo has no markdown formatter wired into `script/fmt`, so prose wrapping was matched to the surrounding file by hand. ## Noted, not addressed The lint stage emits `The linter 'gomodguard' is deprecated (since v2.12.0)`. Already tracked as #90; out of scope here and left alone.

clawbot added the merge-ready label 2026-08-09 10:23:17 +02:00

clawbot merged commit 3f9c2e5033 into main

2026-08-09 10:23:30 +02:00

clawbot deleted branch fix/triage-stale-remote-branches

2026-08-09 10:23:30 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/vaultik#95