sfdupes/TODO.md

# Workflow

- take an issue from the `1.0.0` milestone on the tracker; work not
  yet on the tracker gets filed as an issue first
- branch (from `main`)
- do the work, with tests, in small focused commits
- record it at the top of Completed Steps (`TODO.md` changes in the
  same commit as the work)
- push the branch and open a PR whose title ends with
  ` (closes #N)`
- an independent review gates the merge; every finding is addressed
  or explicitly rebutted on the PR
- merge to `main` once the review passes

# Status

- pre-1.0
- the Gitea tracker is authoritative for the pre-1.0 backlog: the
  open issues under the `1.0.0` milestone are what remains before
  the tag, and this file records history and process, not the queue

# Next Step

- take the next issue from the `1.0.0` milestone on the tracker:
  https://git.eeqj.de/sneak/sfdupes/milestone/17 — the milestone is
  the source of truth for what is left before 1.0.0. Individual
  issues are deliberately not restated here; a copy in this file
  drifts out of date the moment the tracker moves

# Completed Steps

- run all linting in Docker via `Dockerfile.lint` and `script/lint`
  (2026-08-10, branch `next`, closes
  https://git.eeqj.de/sneak/sfdupes/issues/46): per the owner ruling, the
  linter runs inside a container invoked through the `script/`
  entrypoint and is never installed on a host. 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. `script/bootstrap` loses the `go install`,
  the pin constants, the version parser and `verify_golangci_lint`
  outright rather than hardening them — with nothing linting on the
  host, the `$GOPATH/bin` versus `PATH` problem that motivated them has
  no subject — and now warns rather than fails when `docker` is absent.
  Two traps handled. A lint build on an unchanged tree returns success
  in well under a second having run no linter, which is
  https://git.eeqj.de/sneak/sfdupes/issues/32 and
  https://git.eeqj.de/sneak/sfdupes/issues/39 again, so
  `Dockerfile.lint` carries `ARG CHECK_EPOCH` referenced
  inside every gate `RUN` (BuildKit hashes the expanded command, not
  the declaration) and `script/lint` passes `"$(date +%s)-$$"` — the
  PID matters because two lint runs land inside the same second easily.
  And nothing inside an image build may shell out to docker, so the
  main `Dockerfile`'s lint stage now 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 (`make`, not
  the scripts bare, because the Makefile's `export CGO_ENABLED = 0`
  only reaches what it invokes). `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, and dropping it without replacing the edge would have ended
  fail-fast linting silently under a still-green build. That is
  canonical `REPO_POLICIES.md:107`'s ordering edge, restored.
  `ENV PATH=/home/builder/go/bin:$PATH` is gone 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 the 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
  https://git.eeqj.de/sneak/sfdupes/issues/42 made a build failure. Its
  replacement is one new `script/verify-lint-image-pin`,
  run as a gate in both files, which compares the two references to
  each other and deliberately restates neither: a hardcoded expected
  digest would be a third copy and the same drift one file further out.
  `golangci-lint config verify` is included per the ruling, and 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 from an embedded schema and makes no network
  call of its own. The README scopes that to the gate steps rather
  than to linting as a whole: `Dockerfile.lint` runs `go mod download`
  above them, so a cold cache still needs the network and only a warm
  one lints offline. Verified: `make lint` green with every `PATH`
  directory containing a `golangci-lint` removed
  (`/home/user/go/bin`, `/home/user/.local/bin`, `/usr/local/bin`;
  `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, at 42.2s and 41.8s wall clock — the no-cache rule was not
  weakened to shorten that. Negative control: a planted
  `var unusedIssue46Sentinel = 1` failed `script/lint` with
  `report.go:173:5: var unusedIssue46Sentinel is unused (unused)`, and
  failed `make docker` at `[lint 9/9]` with the build stage stopped at
  `[builder 3/12]` — `COPY --from=lint`, `script/bootstrap`, the test
  gate and `make build` all zero occurrences — then reverted clean. The
  drift guard fails on a tag-only disagreement, on a digest-only
  disagreement, and on an unreadable reference, naming both sides.
  `make docker` green in 5m35s with all six gates executing under one
  epoch (lint 37.6s, test 25.2s reporting
  `ok sneak.berlin/go/sfdupes 1.938s coverage: 88.5%`, not `(cached)`).
  The non-root quirk still holds: in the builder image with the Go test
  cache off, `--user 0:0` fails `TestScanHardlinkRunFailsTogether`
  (exit 1) where the unprivileged user passes (exit 0). Noted for
  follow-up, not fixed here: `golangci-lint` warns that the
  `gomodguard` linter is deprecated since v2.12.0 in favour of
  `gomodguard_v2`.

- install the Docker build stage's prerequisites by running
  `script/bootstrap` instead of `apk add --no-cache make` inline
  (2026-08-09, branch `dockerfile-bootstrap`, closes #42): canonical
  `REPO_POLICIES.md:97` requires it, and the inline install left the
  build stage maintaining its own notion of the toolchain — exactly
  the divergence #24 exists to close, one layer down. The stage now
  copies `script/` plus `go.mod`/`go.sum` and runs `script/bootstrap`,
  which ends in `go mod download`, so the separate invocation of that
  is gone. `COPY --from=lint /usr/bin/golangci-lint` stays, and moves
  above the bootstrap layer. It is the only edge making this stage
  depend on the lint stage, so deleting it as redundant would end
  fail-fast linting silently. Letting bootstrap install its own linter
  here would have reintroduced the second toolchain and paid for a
  from-source build of it. What makes the two stages provably one
  toolchain rather than two that happen to agree is a new
  `script/verify-linter-pin`, run in the build stage on the binary
  that arrives from the lint stage, before bootstrap: it fails the
  build naming both versions unless that binary is the version
  `script/bootstrap` pins. Bootstrap's own check could not serve that
  purpose — it reinstalls its pin from source and then verifies
  whatever `PATH` resolves, so drift self-heals silently and a lint
  stage image bumped on its own would lint at the new version while
  `make check` ran at the old one, green. The linter version is pinned
  in two independent places (the lint stage image digest and
  `GOLANGCI_LINT_VERSION`) and nothing else keeps them in sync, so a
  half-applied bump is now a build failure. The pin is read out of
  `script/bootstrap`, which stays the single source of truth; a pin
  that cannot be read is a hard failure, not a skip. The check needs
  no `CHECK_EPOCH`: its only inputs are the copied binary and
  `script/`, so Docker invalidates the layer exactly when a cached
  result would stop being true, and it is documented with the other
  entrypoints in the README. `$GOPATH/bin` joins `PATH` because
  that is where bootstrap's `go install` lands and bootstrap verifies
  its installs against what `PATH` resolves — nothing in the image is
  shadowed by it, the directory does not exist until bootstrap runs.
  Everything added sits above `ARG CHECK_EPOCH`, and the `chown` and
  `USER builder` still precede `make check`. Verified: the guard fails
  the build with both versions named when the lint stage's linter is
  faked to a different version, and an unmodified build still passes
  it; bootstrap runs clean under Alpine's `sh` and its `apk` branch,
  installing `git` and `make` and finding the copied
  linter already at the pin; a second build served the bootstrap and
  dependency layers `CACHED` while both gates ran with a fresh epoch;
  a planted `unused` finding failed the build at the lint gate in
  48.9s with the build stage's `make check` never starting; and the
  suite run in the image as `--user 0:0` fails
  `TestScanHardlinkRunFailsTogether`, so the drop to the unprivileged
  user is still load-bearing. That last check needs the Go test cache
  disabled — the first attempt reported `ok ... (cached)` as root,
  reusing the result the build-time run had left in the shared cache,
  which would have read as a pass. Build wall time, on a shared host
  running many concurrent builds and so noisy: 2m13s on an unchanged
  tree, 2m17s and 4m29s for two builds after a source change, 5m14s
  cold. Only the cold one breaches the policy ceiling, and not because
  of this change — `chown -R builder:builder /src /home/builder` walks
  the module cache and re-runs on every source change, and it alone
  varied between 77s and 210s across those four builds, which is also
  the whole spread in the totals. The same cold measurement against
  `main` is 5m03s with a 209s `chown`. Filed as #43
- bust the Docker layer cache for the gate steps, so `script/cibuild`
  and `script/docker` cannot report a green they did not earn
  (2026-08-09, branch `cibuild-cache-bust`, closes #32): both scripts
  were bare `docker build` invocations with no cache control, and the
  `Dockerfile` copies the tree before running its gates, so on an
  unchanged tree Docker served those layers from cache and the build
  exited 0 having executed nothing. That is not hypothetical here —
  every merge this repo has done is a non-fast-forward merge of an
  undiverged branch, so each merge commit's tree is byte-identical to
  the branch head's and each merge CI run was almost certainly a full
  cache hit; and PR #31's reviewer found `make docker` returning
  success as a 17-layer cache hit, catching it only by being
  suspicious. The fix is `ARG CHECK_EPOCH` with the scripts passing
  `--build-arg CHECK_EPOCH="$(date +%s)"`. Two details make or break
  it. `ARG` is scoped per stage and this `Dockerfile` has three gates
  across two — `make fmt-check` and `make lint` in the lint stage,
  `make check` in the build stage — so a single declaration would have
  left one stage silently cacheable; it is declared in both. And
  BuildKit hashes the expanded command, not the declaration, so a
  declared-but-unreferenced `ARG` invalidates nothing: each gate `RUN`
  echoes the epoch, which also puts the value in the build log as
  evidence the layer really ran. Placement is below the dependency
  layers on purpose — a build that goes cold every time would be a
  different bug, not a fix. Verified by running each script twice back
  to back on an unchanged tree under `BUILDKIT_PROGRESS=plain`: all
  three gates executed on all four runs, each with a fresh epoch in
  the log (`script/cibuild` 78.8s then 61.1s; `script/docker` 61.1s
  then 53.4s), and twelve steps were still served `CACHED` in the
  steady state — both `go mod download`s, `apk add`, `adduser`, the
  `chown`, every `go.mod`/`go.sum` and source copy, the linter copy
  out of the lint stage, and the binary copy into the runtime stage.
  The lint stage still gates the build stage: with a deliberate
  `unused` finding planted in the tree, the build failed at
  `make lint` in 36.1s and the build-stage `make check` never started.
  The build stage also still drops to the unprivileged `builder` user
  before `make check`, which the suite depends on rather than merely
  prefers: forcing the same image to run the tests as root fails
  `TestScanHardlinkRunFailsTogether`, because root reads straight
  through the `chmod(0)` the test uses to prove hard links are read
  once. This is the local fix only; propagating it to the canonical
  templates is `prompts` #26
- check the installed golangci-lint version in `script/bootstrap`
  instead of only its presence (2026-08-09, branch
  `bootstrap-version-check`, closes #24): `missing golangci-lint` meant
  any linter already on `PATH` satisfied the check, so the pin was never
  consulted and the v2.12.2 bump from #3 was inert on every host that
  already had one — this host ran v2.10.1 against a v2.12.2 pin,
  `make check` went green, and `make docker` then rejected the same
  commit with findings the local gate never saw. The version now lives
  in one place, `GOLANGCI_LINT_VERSION`, with the `go install` module
  ref derived from it so a bump cannot half-apply; a
  `golangci_lint_version` helper parses `golangci-lint --version`
  (taking the field after the word `version` and tolerating an optional
  leading `v`, which the module ref carries and the binary's output does
  not), and any version that is not the pin — older, newer, absent or
  unparseable — is reinstalled. The install is then verified against the
  binary `PATH` actually resolves: `go install` writes into `GOBIN` (or
  `GOPATH/bin`) while `make lint` runs whichever `golangci-lint` comes
  first on `PATH`, so a wrong-version one sitting ahead of it — nix,
  apt, brew, apk, or the `/usr/local/bin` copy the `Dockerfile` builder
  stage makes — would swallow the install and leave the local gate
  disagreeing with CI under an affirmative `bootstrap complete`.
  Bootstrap now re-reads the effective version after installing and, on
  a mismatch, prints both paths and both versions to stderr and exits
  non-zero instead of claiming success; it does not reorder anyone's
  `PATH` or delete their binary. The `--version` call keeps its stderr
  connected, so a present-but-broken binary says why rather than
  reinstalling forever in silence, and is bounded by `timeout(1)` where
  that exists, so a wedged binary cannot hang bootstrap. `git`, `make`
  and `go` keep their presence-only checks and now say why in a
  comment: they are host package-manager tools the repo deliberately
  does not pin, with `go.mod` governing the language version and the
  digest-pinned images covering reproducible builds. Verified on this
  host by bootstrapping from v2.10.1 to v2.12.2 and running it again to
  a no-op, plus stub runs of the real script under `dash` covering a
  thirteen-input parse matrix (absent, older, newer, host-style,
  image-style, leading-`v`, stderr-only, empty, non-zero exit, impostor
  binary, `(devel)`, trailing `version`), a shadowed install that must
  exit non-zero, an install destination not on `PATH` at all, `GOBIN`
  set, and a wedged binary that must hit the timeout; `make check` and
  `make lint` are clean at v2.12.2, so v2.10.1 was not hiding any
  findings on `main`
- unwind the hash worker pool on the error path (2026-08-09, branch
  `hash-pool-cleanup`, closes #6): `hashPhase` used to return the
  moment `recordRun` failed and abandon the pool — the feeder parked
  forever on a full `jobs` channel and every worker on a full
  `results` channel. That only stopped being invisible when #4 landed
  and `runScan` began unwinding instead of calling `os.Exit`. The
  pool is now an owned, context-aware `hashPool`: every blocking send
  in the feeder and the workers selects on `ctx.Done()`, `jobs` is
  closed on every path out, and `hashPhase` defers `pool.stop()`,
  which cancels and then drains `results` until the last goroutine
  has exited — draining is what frees a worker already parked on a
  send. `ctx` is threaded from `cmd.Context()` through `runScan`,
  `syncScan`, both worker pools and the whole database layer (it is
  the first parameter everywhere), so #5 can hand this path a signal
  and needs to add nothing else. The walk pool never leaked, because
  `walkPhase` always drains its events to close, but it has the same
  unbounded-send shape and #5 will give it an early return, so it
  gets the same treatment plus a `ctx.Err()` guard after the walk: a
  cancelled walk yields a partial size census, and every file it never
  reached looks vanished to the update phase. That phase's own
  `BeginTx` fails on the same cancelled context before deleting
  anything, so the guard is defence in depth rather than the only
  barrier — but it is the one that survives #5 deciding an interrupted
  scan may commit what it has. Tests drive `run(scan)` against a
  database whose insert trigger aborts, and assert both that the scan
  fails instead of hanging and that `runtime.NumGoroutine()` polls
  back to its pre-scan baseline; a second set cancels a scan part-way
  through the walk — deterministically, by counting the scan's own
  consultations of `ctx.Done()` rather than racing a timer — and
  asserts that it stops at the guard holding a partial census and a
  still-populated record index, with every record intact. The
  remaining cancellation branches of both pools are covered by direct
  tests of `sendEvent`, the walk workers, `dispatchDirs`,
  `feedHashJobs`, `hashWorker` and `hashPhase`
- guarantee the database is closed on every fatal exit path
  (2026-08-09, branch `db-close-on-fatal`, closes #4): `fatalf` and
  its `os.Exit(1)` are gone, so the deferred `db.Close()` — and with
  it the SQLite WAL checkpoint — now actually runs when a subcommand
  fails; `runScan`, `runReport`, `runTrees`, `loadRecords` and
  `resolveRoots` return errors instead. The single exit point is `run`
  in `main.go`: it maps a `fatalError` (anything a subcommand
  returned) to exit 1 and cobra's own argument and flag errors to exit
  2, which keeps a runtime failure from being reported as a usage
  error or printing the usage text. New `main_test.go` drives the CLI
  in-process and asserts the exit codes from README §Error handling
  plus the stdout/stderr split, including that a fatal error raised
  after the database is open leaves no `-wal`/`-shm` sidecar behind
  for `scan`, `report` or `trees`
- update golangci-lint to v2.12.2 with the canonical config
  (2026-08-09, branch `golangci-v2.12.2`, merged as `38a01bd`,
  closes #3): bumped the pinned linter in the `Dockerfile` lint
  stage and `script/bootstrap` from v2.12.1 to v2.12.2, and replaced
  `.golangci.yml` with the canonical file — the linter settings
  (`lll`, `funlen`, `cyclop`, `dupl` thresholds) now live under
  `linters.settings` per the v2 schema, so they are actually
  applied; no new lint findings surfaced
- convert Makefile targets to scripts-to-rule-them-all `script/`
  entrypoints like the other managed repos (2026-07-26, commit
  `3abeacf`, closes #1): all 12 `script/` entrypoints exist
  (`bootstrap`, `setup`, `projectname`, `test`, `lint`, `fmt`,
  `fmt-check`, `check`, `docker`, `cibuild`, `precommit`,
  `install-precommit`) and every Makefile target is now a thin shim
  over them, matching the other managed repos
- make the binary the default Make target (2026-07-24, branch
  `make-default-target`): plain `make` now builds `sfdupes`
  (previously it ran `check` plus `build`); `make build` remains as
  an alias
- scan-wide phases, concurrent operands, batched updates (2026-07-24,
  branch `scan-wide-phases`): all operands seed the shared walk pool
  and every pass runs once over the whole scan, so totals and ETAs
  are scan-global; the per-operand walk/hash/update cycles and their
  stderr announcements are gone; the update pass commits in batched
  transactions — the filesystem is authoritative and the database an
  eventually-consistent reflection, so scan-level atomicity is not
  required
- split the stat pass back out of the walk (2026-07-24, branch
  `parallel-phases`): 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 and the
  stat pass lstats them with per-file workers, restoring the exact
  total/ETA stat bar
- announce each operand on stderr before its passes (2026-07-24,
  branch `scan-operand-progress`): with per-operand walk/hash/update
  cycles, a multi-operand run (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 20M files were being skipped
- parallel walk (2026-07-24, branch `parallel-walk`): the walk pass
  was a single goroutine and took hours at ~20M files on a busy pool
  (observed: 22M files in 4h on a ZFS server); it is now a
  per-directory worker-pool traversal that records size/mtime during
  the walk (folding away the separate stat pass, halving metadata
  I/O), and each `PATH` operand commits in its own transaction so an
  interrupted scan keeps completed operands

- persistent scan database (2026-07-24, branch `persistent-database`):
  `scan` now maintains a SQLite database (`modernc.org/sqlite`, pure
  Go, cgo stays disabled) keyed by absolute path that survives between
  runs — a rescan hashes only new or changed files (by mtime/size),
  deletes records for files vanished from under the scanned operands,
  and leaves records outside them untouched, so `scan` can be cronned
  daily; `report` and `trees` read the database (no positional
  arguments) instead of a scan stream. Database at
  `/var/lib/sfdupes/db.sqlite`, overridable via `SFDUPES_DATABASE`;
  WAL journaling plus a single-transaction update keep a report run
  during a scan safe
- add the `origin` remote (`git@git.eeqj.de:sneak/sfdupes.git`), tag
  `v0.0.1`, and push `main` plus tags (2026-07-23)
- `scan` CLI rework (2026-07-23, branch `scan-required-paths`): required
  `PATH...` operands via cobra flags replacing the `/srv` `-root`
  default; new `-x`/`--one-file-system` flag (GNU convention) to stop
  at filesystem boundaries, which are crossed by default
- bring the repo into full policy compliance (2026-07-23, branch
  `repo-policy-compliance`; checklist below)
- `git init` with README-only first commit; code baseline committed on
  `main` (2026-07-22)
- implement `scan`, `report`, and `trees` subcommands (pre-git history)

# Future Steps

- possible later features (explicitly out of scope per README):
  full-content verification of candidates, removal-script helpers

# Repo Policy Compliance

Audited 2026-07-22 against `REPO_POLICIES.md` (2026-07-06), the existing
repo checklist, and the Go styleguide. Code is already gofmt-clean, so no
standalone formatting commit is needed.

- [x] `.gitignore` missing — the compiled `sfdupes` binary and
      `files.dat` sit untracked in the tree; needs OS/editor/Go
      artifacts plus secrets patterns
- [x] `.editorconfig` missing
- [x] `LICENSE` missing and README has no License section (MIT assumed
      from house convention — user to confirm)
- [x] `REPO_POLICIES.md` missing from repo root
- [x] `.golangci.yml` missing (install canonical copy); code must then
      pass `make lint` (150 findings fixed; `make lint` is clean)
- [x] `Makefile` lacks required targets `test`, `lint`, `fmt`,
      `fmt-check`, `docker`, `hooks`; `check` currently depends on
      `build`, which writes the binary (`make check` must not modify
      files)
- [x] no tests — `go test ./...` has nothing to run; policy requires
      real tests with a 30-second timeout and the conditional `-v`
      rerun pattern (suite covers parsing, grouping, digests,
      suppression, hashing, and the scan pipeline; 64% coverage)
- [x] `Dockerfile` missing — Go multistage with hash-pinned images:
      fail-fast lint stage, build stage running `make check`
- [x] `.dockerignore` missing
- [x] `.gitea/workflows/check.yml` missing (`docker build .` on push,
      checkout action pinned by commit SHA)
- [x] README lacks required sections: Description first line
      (name/purpose/category/license/author), Getting Started,
      Rationale, TODO, License, Author
- [x] README non-goal "no git repository setup and no CI" is stale now
      that the repo is under git with CI
- [x] pre-commit hook not installed (`make hooks` once the target
      exists)

Accepted divergences (no action):

- flat single-package layout with `.go` files in the repo root — fine
  for a small single-binary tool per the Go styleguide; the tracker
  audit agrees
- `go test` runs without `-race` — the repo mandates `CGO_ENABLED=0`
  (pure-Go builds) and the race detector requires cgo