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.
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.
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.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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 andwas not touched.
The triage resolved entirely to deletions, so no product code changes
here. This PR is the
TODO.mdWorkflow step for the issue, which thedefinition of done asks for explicitly if no other PR is opened.
What is in the diff
TODO.mdonly, 37 insertions and 2 deletions: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 namedadd-godoc-to-cli-package(gone) andfeature/pluggable-storage-backend(merged). That text is already absent from
main—e496aa3removed it.I did not re-fix it;
git log -S add-godoc-to-cli-package -- TODO.mdreturns
e496aa3and the originalfc56b0c, and nothing else.The two branches this issue existed for
Neither was landed, because in both cases
mainhas already moved pastthem by a decision recorded in the tracker. Neither is a bug that still
exists, so neither owed a regression test.
fix/ctime-scanner-populationwould repopulate a field that nolonger exists.
1c72a37("Remove all ctime usage and storage (#55)")deleted the
ctimecolumn, theFile.CTimefield and every use, per thedecision on PR #48, closing #54.
grep -rn 'CTime\|ctime' --include=*.goover
mainreturns 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 matchingARCHITECTURE.md:353.ctimenever participated, so a wrongctimecould not cause a changed file to be silently skipped.
fix/sql-injection-whitelistwould revert a deliberate decision.bfd7334("fix: replace table name allowlist with regex sanitization")replaced that exact allow-list with
validTableNameReon review feedbackfrom PR #32; #7 is closed. Landing the branch would also break
main:its allow-list is
{files, chunks, blobs}andmaincallsgetTableCount("snapshots")atsnapshot.go:1544, whose error isdiscarded into
_, so the count would silently become 0.For the record, the remaining exposure on
mainis nil rather thanmerely small:
getTableCountis unexported, all seven call sites passstring literals, and
^[a-z0-9_]+$admits no quote, space, semicolon orparenthesis. The
repeatPlaceholderhelper cited in the issue emits onlybound
?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
okpackages, zero(cached)markers, test stage 49.9swall and lint stage 36.6s. The
CHECK_EPOCHguard steps reportDONE,not
CACHED, so the check layers genuinely executed. Lint ran in thecontainer, so the host-lint voidness of #88 does not apply.
make fmtis a no-op here —script/fmtrunsgo fmt ./...only, andthis PR touches no Go files. Prose wrapping matches the surrounding file.
The
gomodguarddeprecation warning appears in the lint output; it isalready tracked as #90 and is not addressed here.
What I built
One file changed,
TODO.md, 37 insertions / 2 deletions. No productcode. The triage this PR records resolved entirely to branch deletions,
so there was nothing to implement.
empty rather than blank.
Fourteen branches were deleted from
origin. The per-branch inventorywith 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:ctimewas removed from the codebase entirely by1c72a37(#54/#55);grep -rn 'CTime\|ctime' --include=*.goovermainreturns nothing,and change detection compares only size, mtime, mode, uid, gid.
bfd7334, which replaced thatallow-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/cibuildon this branch, exit 0.Per #93 I did not treat the exit code as sufficient. All three
independent signals check out:
okcount: 14 packagesok, 4[no test files], 0FAIL.(cached)markers:grep -c '(cached)'over the full build logreturns 0.
plausible for
-raceon a cold cache, not instant replay.Additionally the
CHECK_EPOCHguard steps (RUN [ -n "…" ] || exit 1)report
DONE, notCACHED, in both the lint and builder stages, so thecheck 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 asCACHEDand 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 fmtis a no-op for this change:script/fmtrunsgo 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 thesurrounding 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.