Commit Graph

61 Commits

Author SHA1 Message Date
beb865ae68 Run all linting in Docker via Dockerfile.lint (closes #46)
All checks were successful
check / check (push) Successful in 1m5s
Per the owner ruling, the linter runs inside a container invoked
through the script/ entrypoint and is never installed on a host. A new
root Dockerfile.lint COPYs the repo into the digest-pinned
golangci/golangci-lint:v2.12.2 image and runs `golangci-lint config
verify` and `golangci-lint run` as build steps, so a successful build
IS a clean lint. script/lint is reduced to building it, which also
works when the docker daemon is remote and bind mounts are impossible.

script/bootstrap loses the `go install`, the pin constants, the
version parser and verify_golangci_lint outright rather than being
hardened: with nothing linting on the host, the $GOPATH/bin versus
PATH shadowing problem those existed to diagnose has no subject. It
keeps the git/make/go presence checks and `go mod download`, and warns
rather than fails when docker is absent.

Two traps.

A lint build on an unchanged tree returns success in well under a
second having run no linter, which is #32 and #39 over again. Caching
is waived by ruling, so Dockerfile.lint carries ARG CHECK_EPOCH
referenced inside every gate RUN -- BuildKit hashes the expanded
command, not the declaration, so a declared but unreferenced ARG
invalidates nothing -- and script/lint passes "$(date +%s)-$$". The
PID is in that value because two lint runs land inside the same second
easily and a bare epoch would cache the second one.

Nothing inside an image build may shell out to docker. The main
Dockerfile's lint stage therefore invokes golangci-lint directly
instead of `make lint`, and its build stage runs `make test` and
`make fmt-check` instead of the `make check` aggregate, which reaches
script/lint. Those two remain `make` invocations rather than the bare
scripts because the Makefile's `export CGO_ENABLED = 0` only applies
to what it invokes, and today's `make check` gets it.

COPY --from=lint /usr/bin/golangci-lint is replaced by
COPY --from=lint /src/go.sum /dev/null. The copied binary was the only
edge forcing BuildKit to finish linting before the build stage starts;
dropping it without replacing the edge would have ended fail-fast
linting silently under a still-green build. That no-op copy is the
ordering edge canonical REPO_POLICIES.md prescribes. Nothing in the
build stage runs the linter any more, so the binary itself is not
wanted there, and ENV PATH=/home/builder/go/bin:$PATH goes with the
`go install` that justified it.

script/verify-linter-pin is retired -- deleted along with its README
entry -- because both of its subjects ceased to exist in this same
change: it compared a linter binary against GOLANGCI_LINT_VERSION in
script/bootstrap, and there is now neither a binary crossing between
stages nor a version pin in bootstrap. The drift it guarded has not
gone away, it has moved. The linter is still pinned twice, now as the
FROM line of Dockerfile.lint and the FROM line of the Dockerfile lint
stage, with nothing syncing them, which is exactly what #42 made a
build failure. Its replacement is one new script/verify-lint-image-pin
that compares those two references to each other and deliberately
restates neither pin: a hardcoded expected digest would be a third
copy and the same drift one file further out. It runs as a gate in
both files, so `make lint`, `make check` and `make docker` all catch
drift, and an unreadable reference is a hard failure rather than a
vacuous pass.

`golangci-lint config verify` is included per the ruling. The concern
about its unpinned live HTTPS schema fetch was measured rather than
assumed: under --network none the pinned binary both passes a valid
config and rejects an invalid one with the jsonschema error, so it
validates against a schema it embeds and linting needs no network
beyond pulling the image. The README states that rather than a
requirement that does not exist.

Verified. `make lint` green with every PATH directory containing a
golangci-lint removed and `command -v golangci-lint` empty. Two
consecutive script/lint runs on an untouched tree both executed the
linter, 27.7s and 28.7s in the lint step under distinct epochs with
the COPY layer CACHED above them. A planted unused variable failed
script/lint with that exact finding and failed `make docker` at the
lint stage with the build stage stopped before its COPY --from=lint,
then reverted clean. The drift guard fails on tag-only, digest-only
and unreadable-reference cases, naming both sides. `make check` green;
`make docker` green in 5m35s with all six gates executing and the test
gate reporting real coverage rather than a cached ok. In the builder
image with the Go test cache off, --user 0:0 still fails
TestScanHardlinkRunFailsTogether where the unprivileged user passes,
so the non-root quirk is intact.
2026-08-10 12:53:03 +00:00
e6a91711b0 Merge branch 'dockerfile-bootstrap': bootstrap the builder stage, guard the linter pin (closes #42)
All checks were successful
check / check (push) Successful in 1m19s
The build stage installed prerequisites inline instead of running
script/bootstrap, which canonical REPO_POLICIES forbids. It now copies
script/ and the manifests and runs bootstrap.

The first attempt claimed that reordering COPY --from=lint made the
build fail loudly on linter drift. It did the opposite: bootstrap
reinstalled to GOPATH/bin, which ENV PATH placed ahead of the copied
binary, so verification passed by construction and lint and build
stages could silently run different linters. Rather than soften the
claim, the guarantee is now implemented. script/verify-linter-pin runs
against the copied binary before bootstrap and fails, naming both
versions, unless it matches the pin read out of script/bootstrap.

Reviewed independently. The reviewer reproduced the negative control in
both directions, including one the PR did not claim - bumping the pin
alone also fails the build - and established that no version output or
mangled pin file yields a silent pass.
2026-08-09 17:43:57 +02:00
clawbot
5ca68804ac Fail the Docker build when the lint stage's linter is not the pin
All checks were successful
check / check (push) Successful in 1m23s
The reordered COPY --from=lint did not make the two stages provably one
toolchain, as the Dockerfile comment, the previous commit message and
TODO.md all claimed. script/bootstrap compares its pin against whatever
PATH resolves, and $GOPATH/bin sits ahead of /usr/local/bin, so any
drift was absorbed: bootstrap rebuilt the pinned version from source,
verified that, and the build went green with the lint stage having
linted at one version and make check having run at another. Bumping the
lint stage image without touching the pin was enough to produce it.

New script/verify-linter-pin fails, naming both versions, unless a given
golangci-lint binary is exactly the version script/bootstrap pins. The
build stage runs it on the binary copied out of the lint stage,
immediately after the copy and before bootstrap, so no reinstall can
satisfy it. The pin is read out of script/bootstrap, which stays its
single source of truth; a pin that cannot be read is a hard failure
rather than a skip. The check takes no CHECK_EPOCH because its only
inputs are the copied binary and script/, so Docker invalidates the
layer exactly when a cached result would stop being true.

The linter version is pinned independently in the lint stage's image
digest and in GOLANGCI_LINT_VERSION, with nothing keeping them in sync;
a half-applied bump is now a build failure instead of a silent split.

ENV PATH keeps $GOPATH/bin, but its comment no longer claims a reinstall
is the reason: bootstrap must be able to run and verify what it
installs, and nothing in this image is shadowed by the entry.

Verified: with the lint stage's linter faked to 2.11.0 after the gates
had really run, the build fails at verify-linter-pin naming 2.11.0 and
2.12.2, with bootstrap and the check gate never reached; an unmodified
make docker is green with all three gates run on a fresh epoch and real
test results. A planted unused finding still fails at the lint stage
with gate check absent from the log; the image still fails
TestScanHardlinkRunFailsTogether under --user 0:0 and passes as uid
1000, both with the Go test cache disabled; and a second build serves
bootstrap, the verify layer and the dependency layers CACHED while the
gates go cold.
2026-08-09 15:29:13 +00:00
3a183aa64b Run script/bootstrap in the Docker build stage (closes #42)
All checks were successful
check / check (push) Successful in 1m16s
Canonical REPO_POLICIES.md:97 requires Dockerfiles to install
development prerequisites by running script/bootstrap rather than
duplicating installs inline. The build stage did the opposite: an
inline `apk add --no-cache make` and its own `go mod download`, so it
maintained a second, independent notion of the toolchain — the
local-versus-CI divergence #24 exists to close, reintroduced one layer
down.

The stage now copies script/ plus go.mod/go.sum and runs
script/bootstrap, which ends in `go mod download`.

COPY --from=lint /usr/bin/golangci-lint is kept and moved above the
bootstrap layer. It is the only edge making this stage depend on the
lint stage, so removing it as redundant would silently stop the build
gating on lint. Copying it first also puts it on PATH before bootstrap
runs, so bootstrap's version check compares the lint stage's linter
against the pin on every build: the two stages are now provably one
toolchain rather than two that happen to agree, and bootstrap does not
pay for a from-source build of its own linter.

$GOPATH/bin joins PATH so that if the copied binary ever stops matching
the pin, bootstrap's reinstall lands somewhere PATH resolves instead of
failing its own verification.

All of it sits above ARG CHECK_EPOCH, and the chown and USER builder
still precede make check.
2026-08-09 14:51:27 +00:00
47fd4e8def Merge branch 'refresh-repo-policies': re-vendor REPO_POLICIES.md (closes #20)
All checks were successful
check / check (push) Successful in 1m7s
The vendored policy copy had drifted to 368 lines against the canonical
408 while still declaring last_modified: 2026-07-06, so nothing about
it signalled staleness. Agents read this file to learn the rules, and
the drift has already caused real defects: the missing scripts-to-rule-
them-all section is why script/fmt lost its Markdown pass (#19) and why
the README had no Entrypoints section (#21).

Replaced wholesale with the canonical copy, sha256
117dde7f148ed3cd693b333312f6345a0a0ee84fadbbe5cca559ca6fed4a1775.
Verified byte-identical by diff; REPO_POLICIES.md is the only file
touched.

The drift turned out to be bidirectional: the vendored copy also held a
repo-memory paragraph the canonical file does not have, so re-vendoring
removes it. That is the correct action for a vendored copy — if the
paragraph should exist it belongs upstream — but it is the one part of
this diff that is not a pure restoration, and it is flagged for a
decision rather than buried.

Docs-only, so the adversarial review was skipped per the standing
exception. make check green.
2026-08-09 12:00:54 +02:00
99d757c31d Re-vendor REPO_POLICIES.md from the canonical copy (closes #20)
All checks were successful
check / check (push) Successful in 1m14s
The vendored copy had drifted to 368 lines against the canonical 408
while still declaring `last_modified: 2026-07-06`, so nothing about the
file signalled that it was stale. Agents working in this repo read the
vendored copy to learn the rules, which makes a silent 40-line gap a
source of real defects rather than untidiness: two have already been
traced to exactly this drift, `script/fmt` having dropped the prettier
Markdown pass (#19, still open) and the README having no Entrypoints
section (#21, since fixed).

Restored by copying the canonical file wholesale — no hand-editing, no
partial merge, no local adaptation, because it is a vendored copy and
its value comes from matching upstream byte for byte. What comes back:

- the entire Scripts to Rule Them All section, including the POSIX sh
  requirement, the repo-root discovery idiom, and the division between
  the standard's canonical scripts and our four extensions
- `bootstrap`, `setup` and `hooks` in the required Makefile target list
- the `script/precommit` paragraph and how the pre-commit hook is wired
  through `script/install-precommit`
- the README **Entrypoints** section requirement
- the Dockerfile bootstrap-layer guidance and the `script/cibuild`
  wording in the Gitea Actions bullet

Verified: `diff` against the canonical file is empty, sha256 is
117dde7f148ed3cd693b333312f6345a0a0ee84fadbbe5cca559ca6fed4a1775, and
`REPO_POLICIES.md` is the only changed path. `make check` is green.

No `TODO.md` entry accompanies this commit. The repo convention is to
record the work in the same commit, but the issue's definition of done
restricts the change to `REPO_POLICIES.md` alone; that scope discipline
is what lets a docs-only change skip adversarial review.
2026-08-09 09:57:43 +00:00
a5fa600c98 Merge branch 'readme-entrypoints': document the entrypoints, drop the tooling line (closes #21)
All checks were successful
check / check (push) Successful in 1m10s
Removes the tooling attribution sentence from the Description, adds the
required Entrypoints section documenting all twelve script/ entrypoints
written from reading each script, corrects the stale Build section that
still claimed the Makefile was the single source of truth after every
target became a shim, and fixes the scan summary example, whose figures
did not sum to the stated total.

Docs-only (README.md alone), so the adversarial review was skipped per
the standing exception. Scope verified independently: git diff
--name-only reports README.md and nothing else. make check green.
2026-08-09 09:53:49 +02:00
9322e8ddee README: drop the tooling attribution, document the entrypoints (closes #21)
All checks were successful
check / check (push) Successful in 1m1s
Four independent README defects, one docs-only pass. README.md is the
only file touched: the issue's definition of done restricts the change
to the README, so TODO.md is deliberately not updated here.

Remove the trailing attribution sentence from the Description. The
paragraph above it already carries authorship and licence, so nothing
is lost, and the naming it carried is not wanted in this repo's output.

Add the required Entrypoints section. House policy wants a README
section that opens by stating adherence to Scripts to Rule Them All,
links the standard, and documents every script/ entrypoint; grep for
"script/" in the README previously returned nothing, though all twelve
entrypoints exist and the Gitea workflow runs script/cibuild. Each
entry is written from reading the script, not from its name. Two
behaviours are called out because they are unusual and were hard-won:
script/bootstrap version-checks golangci-lint against its pin and
verifies the install is the copy PATH resolves rather than a shadowed
one, and script/docker and script/cibuild pass a fresh CHECK_EPOCH so
the Dockerfile's gate layers cannot be served from cache, which is
what keeps a green build from being one that ran nothing.

Fix the stale Build section. Every Makefile target became a thin shim
into script/, so the "Makefile is the single source of truth" claim is
no longer true; point at script/ instead. Document make bootstrap and
make setup, which exist and were undocumented. Drop the make clean
clause about a legacy local files.dat, a file nothing produces any
more; the remaining files.dat references in Makefile, .gitignore and
.dockerignore are #22 and are untouched.

Fix the scan summary example, which showed 123456 files seen against
components summing to 123400. removed is deliberately excluded from
the total, as the following paragraph says and as scan.go implements,
so the total is what was wrong.
2026-08-09 07:51:31 +00:00
a102b8fb06 Merge branch 'cibuild-cache-bust': make the Docker gates actually execute (closes #32)
All checks were successful
check / check (push) Successful in 1m12s
script/cibuild and script/docker were bare docker build invocations, so
on an unchanged tree Docker served the gate layers from cache: no
tests, no lint, exit 0. Confirmed live three times by reviewers, once
as a 17-layer total cache hit.

ARG CHECK_EPOCH is now declared in both stages, since ARG is scoped per
stage and this Dockerfile gates in two of them, and is interpolated
into each gate RUN so BuildKit's command hash actually changes. Both
scripts pass a fresh epoch. Dependency layers stay cached.

Reviewed independently. The reviewer re-established its own cache
baseline after a host-wide BuildKit prune landed mid-review, then
tested the counterfactual: holding the epoch constant reproduces
exactly the 17-layer false green this fixes. It also confirmed the
lint stage still gates the build stage by planting a finding, and that
the non-root drop remains load-bearing by showing the hardlink
permission test fails when the same image runs as root.
2026-08-09 09:47:02 +02:00
clawbot
964fc29ed3 Bust the Docker layer cache for the gate steps (closes #32)
All checks were successful
check / check (push) Successful in 1m50s
script/cibuild and script/docker were bare docker build invocations
with no cache control, and the Dockerfile copies the tree before
running its gates. On an unchanged tree Docker served those layers
from cache, so the gates never executed and the build still exited 0.
A merge commit here has a tree byte-identical to the branch head it
merges, so every merge CI run was almost certainly a full cache hit,
and PR #31's reviewer caught make docker returning success as a
17-layer cache hit that proved nothing.

Declare ARG CHECK_EPOCH in both stages and have the scripts pass
--build-arg CHECK_EPOCH="$(date +%s)". ARG is scoped per stage and
this Dockerfile has three gates across two of them (make fmt-check and
make lint in the lint stage, make check in the build stage), so one
declaration would have left a stage silently cacheable. BuildKit
hashes the expanded command rather than the declaration, so each gate
RUN echoes the epoch: an unreferenced ARG invalidates nothing, and the
echo doubles as evidence in the build log that the layer really ran.

Both declarations sit below the dependency layers, so the pinned base
images, go mod download, apk add and the source copies keep their
cache and only the gates go cold. The build-stage declaration sits
after USER, so the drop to the unprivileged builder user still happens
before make check and the chmod(0) permission tests stay real.
2026-08-09 07:04:35 +00:00
b8ebe5f578 Merge branch 'bootstrap-version-check': check the linter version, not just presence (closes #24)
All checks were successful
check / check (push) Successful in 4s
script/bootstrap installed the pinned golangci-lint only when the
command was absent, so a version bump was inert on any host that
already had the tool. This one ran v2.10.1 against a v2.12.2 pin: local
make check went green while make docker rejected the same commit with
six goconst findings.

Bootstrap now compares the installed version against the pin, which
lives in exactly one place, and reinstalls on mismatch. It then
verifies the install actually took effect: go install writes to
GOPATH/bin, but the binary make lint runs is whatever PATH resolves, so
a wrong-version linter shadowing it earlier on PATH would otherwise
leave bootstrap printing success having changed nothing. On mismatch it
now names both paths on stderr and exits non-zero.

The first review caught that shadowing case. Re-reviewed independently
by a fresh reviewer, who rebuilt the reproduction from scratch across
seven PATH layouts, ran a 20-row version-parse matrix confirming no
input yields a false match, and forced a cold Docker build after
finding the cached one executed nothing. Confirmed along the way that
v2.10.1 was hiding no findings on main.
2026-08-09 08:38:36 +02:00
clawbot
9d06c13777 Verify the golangci-lint install actually took effect
All checks were successful
check / check (push) Successful in 1m31s
`go install` writes into GOBIN (or GOPATH/bin), but the linter `make
lint` runs is whichever golangci-lint PATH resolves first. On a host
where a wrong-version binary sits ahead of that directory — a nix
profile, apt, brew, apk, a tarball in /usr/local/bin, or the
/usr/local/bin copy the Dockerfile builder stage makes — the install
landed behind the shadow, changed nothing the gate uses, and bootstrap
still printed "bootstrap complete" and exited 0. That leaves the local
gate linting against a different ruleset than CI while affirmatively
claiming otherwise, and every subsequent run reinstalls forever, so the
second run is never a no-op.

After installing, re-read the effective version. On a mismatch print
the resolved binary, the install directory and both versions to stderr
and exit non-zero. Do not reorder PATH or remove anyone's binary:
diagnose and stop.

Also:

- stop discarding `golangci-lint --version` stderr, so a present but
  broken binary (missing shared library, wrong architecture) says why
  instead of silently yielding the empty string and reinstalling on
  every run forever. Only stdout is parsed, so the parse matrix is
  unchanged.
- bound the `--version` call with timeout(1) where it exists, since
  bootstrap now executes a binary it previously only located and a
  wedged one would otherwise hang the script. Hosts without timeout(1)
  run it unbounded, as before.
- use X.Y.Z in the parsing comment so the pinned version stays a single
  literal in the script.
2026-08-09 06:19:37 +00:00
clawbot
9e924721e6 Check the golangci-lint version in bootstrap, not just presence (closes #24)
All checks were successful
check / check (push) Successful in 1m48s
script/bootstrap installed the pinned linter only when the command was
absent, so on any host that already had some golangci-lint the pin was
never consulted and a version bump was inert forever. That is how a
host running v2.10.1 against a v2.12.2 pin got a green `make check`
while `make docker` rejected the same commit: the local gate was
linting with a different ruleset than CI, and the disagreement only
surfaced after a push.

The version is now a single value, GOLANGCI_LINT_VERSION, with the
`go install` module ref derived from it, so a future bump cannot
half-apply. A golangci_lint_version helper parses the installed
version out of `golangci-lint --version` (the field after the word
"version", with an optional leading "v" stripped, since the module ref
carries one and the binary's output does not) and yields the empty
string when the tool is absent or unreadable. Any version that is not
the pin -- older, newer, absent or unparseable -- is reinstalled, so a
first run upgrades and a second is a no-op.

git, make and go keep their presence-only checks: they come from the
host package manager, the repo pins no system toolchain versions, and
go.mod governs the language version. That is now stated in a comment
next to them rather than left ambiguous beside a tool that is
version-checked.
2026-08-09 05:52:55 +00:00
076d82231b Merge branch 'hash-pool-cleanup': unwind the hash worker pool on error (closes #6)
All checks were successful
check / check (push) Successful in 5s
hashPhase returned early on a recordRun error, abandoning the feeder
goroutine and the hash workers, which parked forever on channel sends.
The pool is now owned: every blocking send selects on ctx.Done(), and
hashPhase defers a stop() that cancels and then drains results. ctx is
threaded from cmd.Context() through runScan, syncScan, both pools and
the database layer, as contextcheck requires.

The walk pool gets the same treatment. It does not leak today, because
walkPhase has no early return, but #5 introduces one. That change also
required a ctx.Err() guard after the walk: a cancelled walk yields a
partial size census, and without the guard the update phase reads every
unreached file as vanished and deletes its record.

First review failed this: the test covering that guard was vacuous,
cancelling before the walk began. Replaced with a deterministic
mid-walk cancellation. Re-reviewed independently by a fresh reviewer,
who reproduced both falsification checks, measured the census across
four parallelism settings, mutation-verified all nine cancellation
branches, ran 120 randomised mid-flight cancellations over 18,000 files
with zero records lost, and forced a cold Docker build after finding
the cached one proved nothing. Coverage 88.5%.
2026-08-09 07:46:43 +02:00
clawbot
1a38570301 Cover the post-walk cancellation guard with a test that reaches it
All checks were successful
check / check (push) Successful in 1m32s
TestSyncScanCancelledWalkKeepsRecords handed syncScan a context that
was already cancelled. loadIndex is the first thing syncScan does, and
its QueryContext fails on that context, so the scan returned before
startWalk was ever called: no walk ran, no pool started, no write path
was reachable, and all three of the test's assertions held for the
wrong reason. The post-walk ctx.Err() guard, which is the highest-stakes
line in the change, had no coverage at all — a panic in its body, or
deleting it outright, left the suite green.

Replace it with TestSyncScanCancelledMidWalkKeepsRecords, which cancels
during the walk and so reaches the guard holding a genuinely partial
census and a still-populated record index. The cancellation is driven
by the scan's own progress rather than by a timer: walkClock is a
context that cancels itself once its Done method has been consulted a
set number of times, and since every blocking channel operation in the
walk selects on Done — one consultation per event, a couple per
directory, against the index load's fixed three — a threshold set to a
quarter of the fixture's file count lands the cancellation deep inside
the walk on every run. The census settles at around 380 of 2000 files,
leaving some 1600 records that a complete-looking census would have
handed to the update phase as deletions.

The already-cancelled case is kept, renamed to what it actually tests
and with its goroutine assertion dropped, since nothing that could leak
is ever started.

Direct tests cover the remaining cancellation branches of both pools:
sendEvent abandoning a blocked send, walk workers dropping queued
directories, a walk worker abandoning its subdirectory hand-off,
dispatchDirs closing jobs on its way out, feedHashJobs doing the same,
hashWorker dropping queued runs, and hashPhase leaving its result loop.
Each is deterministic — the channels involved are unbuffered, unread or
pre-filled, so the cancellation case is the only one that can be ready.

Also correct two overstated claims. The hashLeakFiles comment described
a mechanism that does not occur: the surplus is absorbed exactly by the
two pool channels plus the workers in flight, so the feeder drains and
exits, and what an abandoned pool leaves parked is the workers and the
goroutine waiting on them. And the guard is defence in depth, not the
sole barrier against data loss: the update phase's BeginTx fails on the
same cancelled context before deleting anything today. The guard is
what keeps that true once an interrupted scan is allowed to commit what
it has.
2026-08-09 05:12:43 +00:00
1399249957 Unwind the hash worker pool instead of abandoning it (closes #6)
All checks were successful
check / check (push) Successful in 1m2s
hashPhase returned the moment recordRun failed and left the pool
running: the feeder parked forever on a full jobs channel and every
worker on a full results channel. Until #4 landed the process exited
before that mattered; now that runScan returns an error and unwinds,
the goroutines are a real leak.

The pool is now an owned hashPool. Its context is derived from the
scan's, every blocking send in the feeder and the workers selects on
ctx.Done(), the feeder closes jobs on every path out so the workers'
range always terminates, and hashPhase defers pool.stop(), which
cancels and then drains results until the last goroutine has exited.
Draining is the half that matters: a worker already parked on a send
cannot observe the cancellation until a receiver frees it.

ctx comes from cmd.Context() and is threaded through runScan,
syncScan, both worker pools and the database layer as the first
parameter throughout, so graceful interrupt handling has a path to
hook into rather than a pool to rewrite.

The walk pool never leaked, because walkPhase always drains its
events to close, but it has the same unbounded-send shape and gets
the same treatment, together with a ctx.Err() guard after the walk: a
cancelled walk leaves a partial size census, and the update phase
would read every file it never reached as vanished and delete its
record.

Tests drive the scan entry point against a database whose insert
trigger aborts, with a fixture large enough that the failure lands
partway through the hash phase with more runs queued than either pool
channel can hold, and assert that the scan fails instead of hanging
and that runtime.NumGoroutine polls back to its pre-scan baseline.
2026-08-09 03:00:01 +00:00
2a055c0104 Merge branch 'db-close-on-fatal': close the database on every exit path (closes #4)
All checks were successful
check / check (push) Successful in 5s
fatalf called os.Exit(1), which skips deferred functions, so every
defer db.Close() was dead on the fatal path and the SQLite WAL was
never checkpointed. fatalf is gone; runScan, runReport and runTrees
return errors, and a single run(args, stderr) int in main.go is the
only exit point, which also makes these paths testable in-process.

Exit-code behaviour is unchanged: 0 success, 1 fatal, 2 usage. A runE
adapter keeps cobra from reclassifying runtime failures as usage errors
or printing usage text alongside them.

Independently reviewed. The reviewer verified all 15 documented exit
paths against binaries built from both base and head, confirmed the
sidecar WAL files no longer survive a fatal exit, and mutation-tested
the new assertions by reintroducing the defect to confirm they fail.
Coverage 64% to 86.9%. make check and make docker both green.
2026-08-09 04:39:28 +02:00
73841c9989 Guarantee the database is closed on every fatal exit path (closes #4)
All checks were successful
check / check (push) Successful in 57s
fatalf called os.Exit(1), which does not run deferred functions, so
every defer db.Close() was dead on the fatal path: the SQLite WAL was
left uncheckpointed and the -wal/-shm sidecars were left for the next
process to recover. It also made those paths impossible to exercise
in-process.

fatalf is gone. runScan, runReport, runTrees, loadRecords and
resolveRoots return their errors, so the deferred close always runs,
and the only exit point is run() in main.go.

Mapping errors to exit codes needs care: cobra prints the error and
the command's usage text for anything RunE returns, and main mapped
every Execute() error to exit 2. A runtime failure is not a usage
problem, so the runE adapter silences both for the subcommands and
marks their errors fatalError; run() reports a fatalError as
"sfdupes: ..." on stderr and exits 1, and leaves everything else --
cobra's own argument, flag and unknown-command errors, which cobra has
already reported with its usage text -- on exit 2. The bare
"sfdupes" invocation still prints usage and exits 2.

Exit codes and message text are unchanged: 0 on success even with
per-file warnings, 1 fatal, 2 usage, per README section "Error
handling and exit codes". Everything on stdout is still data only.

main_test.go drives the CLI in-process and covers all three: a fatal
error raised after the database is open (a database with no files
table) closes it and leaves no -wal or -shm behind for scan, report
and trees; a nonexistent PATH operand is fatal, not usage, and prints
no usage text; the usage errors still exit 2; and a scan that skipped
an unreadable file still exits 0.
2026-08-09 02:27:07 +00:00
ce6d29dffb Merge branch 'todo-next-step-stale': point TODO.md at the tracker (closes #27)
All checks were successful
check / check (push) Successful in 11s
TODO.md's Next Step still named the scripts-to-rule-them-all conversion,
which landed in 3abeacf on 2026-07-26 without recording itself in
Completed Steps, so the file pointed the next contributor at finished
work. Next Step now names the 1.0.0 milestone on the tracker, the
scaffold gains its missing Completed Steps entry, and Workflow matches
the issue-branch-review-merge process actually in use.

Docs-only change (TODO.md only), so the adversarial review was skipped
per the standing exception. make check green.
2026-08-09 04:05:04 +02:00
a7295750ef Point TODO.md at the tracker and record landed work (closes #27)
All checks were successful
check / check (push) Successful in 2m20s
The Next Step section still named the scripts-to-rule-them-all
conversion, which landed in 3abeacf on 2026-07-26 and closed #1
without recording itself in Completed Steps. TODO.md therefore
pointed the next contributor at finished work.

The backlog is no longer file-shaped: the Gitea tracker is
authoritative, with the open issues under the 1.0.0 milestone
defining what remains before the tag. Next Step now names the
milestone instead of restating a single issue that will drift, and
Status says so explicitly.

Workflow is reconciled with how work actually happens now: take an
issue, branch, implement with tests, record the result in the same
commit, open a PR titled "... (closes #N)", pass an independent
review, merge.

The scripts-to-rule-them-all conversion gains its missing Completed
Steps entry, and the golangci-lint entry now records its merge
commit 38a01bd and issue #3 rather than only the branch date.

Docs-only: TODO.md is the only file touched.
2026-08-09 02:03:23 +00:00
38a01bd27c Merge branch 'golangci-v2.12.2': golangci-lint v2.12.2 and canonical config (closes #3)
Some checks failed
check / check (push) Has been cancelled
Bumps the pinned golangci-lint from v2.12.1 to v2.12.2 in the Dockerfile
lint stage and script/bootstrap, and replaces .golangci.yml with the
canonical org-standard file (sha256
021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb).

The material change is that the lll, funlen, cyclop and dupl thresholds
move from the v1-style top-level linters-settings key, which
golangci-lint v2 silently ignores, to linters.settings, where they are
actually enforced. Independent review proved the migration claim with a
controlled experiment and confirmed the code passes the now-live
thresholds with zero findings.

Reviewed independently; make check and make docker both green.
2026-08-09 03:57:09 +02:00
814bdada2b Update golangci-lint to v2.12.2 with canonical config
All checks were successful
check / check (push) Successful in 1m5s
Bump the pinned golangci-lint from v2.12.1 to v2.12.2 in the
Dockerfile lint stage (tagged, digest-pinned Debian image) and in
script/bootstrap (go install ref). Replace .golangci.yml with the
canonical config: linter settings (lll, funlen, cyclop, dupl) move
under linters.settings per the v2 schema so they are actually
applied, and the redundant issues.exclude-use-default key is
dropped. No new lint findings surfaced; make check is green.
2026-08-07 16:46:10 +00:00
3abeacf8ee Add scripts-to-rule-them-all scaffold (refs #1)
All checks were successful
check / check (push) Successful in 6s
Bring the repo into conformance with the scripts-to-rule-them-all
(STRTA) scaffold. The real logic that lived inline in the Makefile now
lives in POSIX-sh entrypoints under script/, and the Makefile's standard
targets are thin @script/NAME shims.

- script/: bootstrap, setup, projectname, test, lint, fmt, fmt-check,
  check, docker, precommit, install-precommit, cibuild. All are
  executable #!/bin/sh entrypoints; the go mod tidy guard from the old
  inline hooks recipe moved into script/precommit.
- Makefile: the nine standard targets (bootstrap, setup, test, lint,
  fmt, fmt-check, check, docker, hooks) are now thin shims; the
  repo-specific sfdupes/build/clean targets and the CGO_ENABLED export
  are preserved.
- .gitea/workflows/check.yml: run script/cibuild instead of a bare
  docker build.
- Dockerfile: run make check (and the build) as an unprivileged builder
  user rather than root. We should never build or run as root, and doing
  so also lets the permission-denied tests run legitimately: root
  bypasses the chmod(0) that TestScanHardlinkRunFailsTogether relies on,
  which made the in-image make check fail. HOME and the Go caches point
  at the user's home so go build/test and golangci-lint can write.

make check passes locally and docker build . is green (the in-image
non-root make check passes, including the hardlink permission test).
2026-07-26 23:23:06 +07:00
b5f6faa00e Merge branch 'inode-runs': inode-ordered hashing, hard links read once
Some checks failed
check / check (push) Failing after 47s
2026-07-25 14:36:56 +07:00
b14b735c88 Hash in inode order, read hard links once, never open empty files
Sort the hash queue by (device, inode) so reads proceed in inode
order, which minimizes seeking on spinning disks. Paths that are hard
links to the same inode form one run: the run is read once and every
path shares the result, so link farms (rsync --link-dest backups)
cost one read per inode instead of one per path. A run that fails to
read skips all of its paths.

Zero-length files have constant head/tail hashes; return them without
opening the file.

The hash progress total now counts actual reads (runs, not paths).
Hard-linked paths still appear in reports as duplicates — their
content is identical — though they share storage; noted in README.
2026-07-25 14:36:55 +07:00
67bde6226d Merge branch 'load-progress': never look hung during startup
All checks were successful
check / check (push) Successful in 1m9s
2026-07-25 14:28:13 +07:00
57efa64f18 Show progress while loading the record index
On a database with tens of millions of records, indexing the existing
rows before the walk takes real single-core time with no output,
which is indistinguishable from a hang. Give the load its own
spinner, and render every phase display the moment the phase starts
instead of waiting for its first completed item.
2026-07-25 14:28:11 +07:00
d4b43ebb30 Merge branch 'size-census': hash only files with shared sizes
All checks were successful
check / check (push) Successful in 1m8s
2026-07-25 06:06:30 +07:00
b62b4f297f Stat in the walk, hash only shared sizes, flush batches mid-scan
Restructure scan into three phases: walk+stat, hash, update.

The stat pass is folded into the walk workers: each regular file is
lstatted as its directory is read, while the metadata is hot. The
walk builds a scan-wide size census (walked files plus records
outside the scan roots), and unchanged already-hashed files resolve
during the walk without further work.

Only files whose size at least one other file shares are ever read:
a size-unique file cannot be a duplicate, so it is recorded without
hashes (head and tail empty). When a later scan makes its size
shared, the file is hashed then, even if otherwise unchanged. report
excludes unhashed records; trees gives them a never-matching
signature so a tree containing one never compares equal to another.

Hashed records are committed in batched transactions while the hash
phase runs, so an interrupted scan keeps everything hashed so far
and the next run resumes cheaply. The hash phase total is exact,
giving a meaningful ETA.

Memory drops accordingly: the existing-record index holds only path,
size, mtime, and a hashed flag (no hash values); the walk carries one
small record per candidate file; overlapping operands are pruned up
front instead of deduplicating every walked path in a scan-wide set.
Files no bigger than one chunk are hashed with a single read.
2026-07-25 06:06:22 +07:00
340bdbe39e Merge branch 'make-default-target': plain make builds the binary
All checks were successful
check / check (push) Successful in 1m12s
2026-07-24 11:21:18 +07:00
a1c3b852c3 Make the binary the default Make target
All checks were successful
check / check (push) Successful in 4s
Plain make now builds sfdupes (previously the default was all =
check + build); make build remains as an alias, so the Dockerfile
and existing habits keep working. The sfdupes target is phony: go
build's own cache decides what to recompile.
2026-07-24 11:21:16 +07:00
9f03eb3e2a Merge branch 'scan-wide-phases': scan-wide phases, concurrent operands, batched updates
All checks were successful
check / check (push) Successful in 1m8s
2026-07-24 10:26:54 +07:00
3ecf73c80a Make phases scan-wide, walk operands concurrently, batch updates
All checks were successful
check / check (push) Successful in 5s
All PATH operands belong to a single scan: every operand seeds the
shared walk worker pool, and each pass (walk, stat, hash, update)
runs exactly once over the whole scan, so pass totals, percentages,
and ETAs are scan-global. The per-operand walk/hash/update cycles and
their stderr operand announcements are gone; duplicate paths from
overlapping operands are deduplicated before stat.

The update pass now commits in batched transactions (10k changes per
batch) instead of one scan-wide transaction: the filesystem is
authoritative and the database is an eventually-consistent reflection
of it, so scan-level atomicity buys nothing, while batches keep the
WAL small and let concurrent reports observe progress.
2026-07-24 10:26:52 +07:00
732fc351d7 Merge branch 'parallel-phases': sequential phases, parallelism within each
All checks were successful
check / check (push) Successful in 1m9s
2026-07-24 08:12:49 +07:00
1e7a519608 Split the stat pass back out of the walk
All checks were successful
check / check (push) Successful in 4s
Phases are strictly sequential again — walk, stat, hash, update per
operand — with parallelism only inside each phase. The walk
enumerates paths with per-directory workers (no lstat of file
entries); the stat pass lstats every collected path with per-file
workers, restoring its exact-total/ETA progress bar and per-file
parallelism inside wide flat directories.
2026-07-24 08:12:47 +07:00
09ff9b5f30 Merge branch 'scan-operand-progress': announce operands on stderr
All checks were successful
check / check (push) Successful in 1m15s
2026-07-24 07:52:55 +07:00
a0f0050ada Announce each operand on stderr before its passes
All checks were successful
check / check (push) Successful in 6s
With per-operand walk/hash/update cycles, a multi-operand invocation
(e.g. scan /srv/*) showed pass totals that looked like the whole
run's: an operator watching operand 3 of 14 hash 300k files concluded
the other 20M files were being skipped. Print the operand path and
its position before each cycle.
2026-07-24 07:52:54 +07:00
6a15b879de Merge branch 'parallel-walk': parallel per-directory walk, per-operand commits
All checks were successful
check / check (push) Successful in 1m10s
2026-07-24 07:44:28 +07:00
dced5cf0d2 Parallelize the walk and commit per operand
All checks were successful
check / check (push) Successful in 5s
Replace the single-goroutine WalkDir traversal with a per-directory
worker pool: workers read directories concurrently and lstat entries
while each directory is fresh in cache, recording size and mtime
during the walk. This folds the separate stat pass away (halving
metadata I/O per run) and overlaps metadata latency, which dominated
on busy pools — a sequential walk of a ~22M-file tree was observed
taking over 4 hours.

Each PATH operand now loads its scope, walks, hashes, and commits in
its own transaction, so an interrupted scan keeps every operand
completed so far; a later overlapping operand sees the records
committed by earlier ones and reuses them unchanged.
2026-07-24 07:44:18 +07:00
3ebf98940a Specify parallel per-directory walk and per-operand commits
The walk pass is a single goroutine; on a busy ZFS pool it manages
only a few thousand directory entries per second and takes hours at
~20M files. Respecify it as a worker-pool traversal that reads
directories concurrently and records size/mtime during the walk,
folding away the separate stat pass and halving metadata I/O. Each
PATH operand now commits in its own transaction so an interrupted
scan keeps the operands completed so far.
2026-07-24 06:28:12 +07:00
abea945730 Merge branch 'persistent-database': persistent SQLite scan database
All checks were successful
check / check (push) Successful in 1m2s
2026-07-24 03:08:58 +07:00
d8fbcb32c2 Implement persistent SQLite scan database
All checks were successful
check / check (push) Successful in 3s
scan now synchronizes a database that survives between runs
(SFDUPES_DATABASE, default /var/lib/sfdupes/db.sqlite) instead of
emitting a stream: operands are resolved to absolute paths, unchanged
files (same size, mtime not newer than recorded) are never re-read, new
and changed files are hashed, and records under the scanned operands
that were not verified this run are deleted; records outside the
operands are untouched. All changes commit in a single transaction, and
WAL journaling with a busy timeout keeps a report run during a cron
scan safe.

report and trees read the database (no positional arguments); the
NUL-terminated stream format, its parser, and the malformed-record
handling are gone. The driver is modernc.org/sqlite (pure Go), so
builds keep cgo disabled.
2026-07-24 03:08:49 +07:00
e0d578a707 Specify persistent SQLite scan database in README; plan in TODO.md
scan will maintain a persistent database of file signatures
(default /var/lib/sfdupes/db.sqlite, overridable via
SFDUPES_DATABASE) that survives between runs; rescans hash only new
or changed files (mtime/size) and remove records for vanished files,
so scan can be cronned daily. report and trees will read the
database instead of a scan stream.
2026-07-24 02:54:08 +07:00
90c9ef3546 Record remote setup and v0.0.1 tag in TODO.md
All checks were successful
check / check (push) Successful in 40s
2026-07-23 09:08:28 +07:00
13b9839e73 Disable background-session worktree isolation for this repo 2026-07-23 09:06:24 +07:00
4b1c3cbf70 Make scan paths required operands; add -x/--one-file-system
All checks were successful
check / check (push) Successful in 4s
scan now takes one or more PATH operands (directories or regular
files) via cobra flags instead of the -root flag with its /srv
default; invoking scan with no operand is a usage error and a
nonexistent operand is fatal. Filesystem boundaries are crossed by
default; the new -x/--one-file-system flag (GNU du/rsync convention)
stops the walk at each operand's filesystem, implemented by comparing
lstat device IDs with build-tagged helpers for darwin's int32 Dev.
Verified against a real mounted disk image: default crosses, -x does
not, --one-file-system is identical to -x.
v0.0.1
2026-07-23 08:55:17 +07:00
3391207f8d Check off completed repo policy compliance items in TODO.md 2026-07-23 07:59:12 +07:00
5b20171dbd Install pre-commit hook via make hooks; resolve shared hooks dir
The hooks target uses git rev-parse --git-common-dir so it works from
both the main checkout and linked worktrees.
2026-07-23 06:45:02 +07:00
375233fff4 Restructure README with required policy sections
Adds Description (name/purpose/category/license/author), Getting
Started, Rationale, TODO, License, and Author sections; the full
normative specification is preserved under Design. The stale non-goal
about having no git repository or CI is removed, and the Build section
now documents the Makefile targets.
2026-07-23 06:44:32 +07:00
b4d013cb34 Add hash-pinned Dockerfile, .dockerignore, and Gitea CI workflow
Multistage build per policy: a fail-fast lint stage on the pinned
golangci-lint image runs fmt-check and lint, the builder stage reuses
its linter binary (which also forces stage ordering), runs make check,
and builds; the runtime stage is pinned alpine with just the binary.
CI runs docker build . on push with the checkout action pinned by
commit SHA. All image references pinned by sha256 digest with
version/date comments.
2026-07-23 06:42:11 +07:00