script/bootstrap credited the goimports pin to script/fmt-check, which
runs gofmt only; the header now credits script/fmt. script/cibuild
still described the Dockerfile as running make check, which stopped
being true once linting moved to its own stage. The docker-missing
warning in script/bootstrap is one sentence instead of three fragments
each carrying the bootstrap: prefix. Dockerfile.lint now states the
residual risk of skipping golangci-lint config verify: unknown
top-level keys in .golangci.yml are ignored silently, so a mistyped key
lints clean and applies nothing.
Comment and message text only; no behaviour changes.
The repository had no licence file at all, which makes publicly readable
code all-rights-reserved by default: nobody may legally use it. That is a
1.0 blocker rather than a nicety, and `LICENSE` was the only file from
`REPO_POLICIES.md`'s required minimum still missing here.
The choice is standing org policy rather than a per-repo call: any public
repo lacking a licence gets MIT, while a private repo with no licence is
already all-rights-reserved and needs nothing. `sneak/dnswatcher` is
public, so MIT.
`LICENSE` carries the canonical MIT text byte-for-byte with only the
copyright line filled in; no clauses added, removed, reworded, or
reflowed. `README.md`'s first line now names the licence, which the
Description requirement in `REPO_POLICIES.md` calls for, and the License
section states MIT and points at the file instead of recording the
decision as pending.
`script/test` did not pass `-count=1`, so on an unchanged tree Go
served the whole suite from its test cache: exit 0 in ~0.2s with every
package marked `(cached)` and not one DNS query made. This repo's suite
exists to exercise live resolution on every run (`TESTING.md`), so that
green asserted nothing — and it is exactly the green used as evidence
that a flakiness fix works, since "run it a few times" stops being
runs after the first.
`-count=1` now disables caching on every invocation.
The conditional verbose rerun that `REPO_POLICIES.md` mandates was
missing at the same spot and is added here rather than left broken: the
primary run had been unconditionally `-v`, which is the failure mode
the policy exists to prevent (unreadable CI and `docker build` logs on
success). Tests now run quiet, and only a failure triggers the `-v`
rerun. The rerun carries `-count=1` too, so it cannot replay a cached
copy of the failure it is meant to diagnose, and its exit status is
discarded in favour of a forced 1: the first failure already proved the
suite broken, so a flake that passes the second time must not turn the
build green.
`-timeout 90s` is untouched. It is a deliberate backstop that must
strictly exceed the 60s hard cap on suite duration.
No special-casing for the Docker build, which also reaches this script
via `RUN make test`: a fresh container's test cache is empty, so
`-count=1` changes nothing there and carving out an exception would
only create a second code path that could drift.
Verified: three back-to-back `make test` runs on an unchanged tree,
zero `(cached)` markers, ~4.0-4.5s wall each (was ~0.2s cached),
comfortably inside the 20s target with `-race` and `-cover` both still
working and coverage percentages unchanged. The rerun-and-still-fail
path was exercised against a purpose-built flaky test that fails once
then passes: quiet failure, verbose rerun that genuinely re-executed,
exit 1 regardless. `make check` green.
Rework of the unit at #93
(commit 9cb2c2b), against the review at
#136 (comment).
Review of 9cb2c2b found the quorum assertions could not fail on a
class of wrong answer. Each test banned exactly one bad status —
_AllReturnOK banned only nxdomain, _NXDomainFromAllNS banned only ok
— so resolver.StatusNoData passed both. nodata is a wrong answer, not
silence, and answeredCount counted it as answered, so it did not even
trigger a retry; with a quorum of 3 of 4 a single wrong nameserver
slid through undetected. That is assertion-loosening beyond what the
quorum change requires.
Tolerance is now a closed allowlist rather than a blocklist of one
status. unsanctionedStatuses() reports every per-nameserver result
whose status the caller did not explicitly sanction: ok/timeout/error
for the all-OK test, nxdomain/timeout/error for the NXDOMAIN test.
Silence (timeout, error) is the only thing quorum exists to tolerate;
any other status, including one added to the resolver later, fails by
name. answeredCount is likewise an allowlist of ok/nxdomain/nodata, so
an unknown status counts as silence and can only cause a retry and
then a loud failure, never a quiet pass.
Two harness tests cover the regression directly: three OK plus one
nodata (quorum satisfied, no nxdomain present — the input that used
to pass) is now reported as unsanctioned, and an unknown status is
neither counted as answered nor tolerated.
Verified by re-running the reviewer's probe: queryEachNS patched to
force one of google.com's four nameservers to return StatusNoData
turns both tests red, naming the offending nameserver and status —
--- FAIL: TestQueryAllNameservers_AllReturnOK (1.12s)
Should be empty, but was [ns1.google.com.=nodata]
every nameserver must answer OK or not answer at all:
ns1.google.com.=nodata ns2.google.com.=ok ns3.google.com.=ok
ns4.google.com.=ok
--- FAIL: TestQueryAllNameservers_NXDomainFromAllNS (1.34s)
Should be empty, but was [ns1.google.com.=nodata]
every nameserver must report NXDOMAIN or not answer at all:
ns1.google.com.=nodata ns2.google.com.=nxdomain
ns3.google.com.=nxdomain ns4.google.com.=nxdomain
— and green with the probe reverted. Also fixes the review's nit: the
per-attempt deadline assertion had no lower bound, so it passed for a
deadline far shorter than intended.
No production code changed; DNS is still never mocked.
The resolver's live-DNS tests failed nondeterministically, a different
subset each run. Three structural causes, all test-side:
- Burst fan-out. Every test in the package is parallel and the build
hosts have many cores, so all ~35 iterative resolutions started at
the same instant and, because queryServers walks rootServerList() in
fixed order, hit the same root server within milliseconds. Root
servers rate-limit that.
- No retry anywhere. One dropped UDP packet in a delegation chain
failed a test outright.
- Unanimity assertions. TestQueryAllNameservers_AllReturnOK and
_NXDomainFromAllNS required every one of a domain's nameservers to
answer, with no tolerance for one being slow.
New internal/resolver/livedns_test.go addresses each: a package-wide
gate bounds how many live resolutions are in flight at once, every
live operation gets three attempts with exponential backoff and its
own deadline, and multi-nameserver assertions now need a strict
majority rather than unanimity. The retry predicate is deliberately
transport-level -- "did a nameserver answer at all" -- never the
assertion under test, so a resolver that answers incorrectly still
fails on the first attempt. A nameserver that stays silent is
tolerated; one that answers wrongly is not.
livedns_harness_test.go tests that machinery directly: quorum
arithmetic, status counting, the gate's concurrency bound, per-attempt
deadlines, and recovery from a transient failure. It touches no DNS.
Nothing is mocked, faked, stubbed, recorded, skipped or build-tagged,
and production resolver behaviour is unchanged.
Test caps move to the new org-wide values ruled at prompts issue 41:
60s hard cap, 20s target, 90s -timeout backstop. REPO_POLICIES.md is
re-vendored byte-identical from sneak/prompts rather than hand-edited,
which also picks up the golangci-lint paragraph this copy had drifted
behind on. TESTING.md's stale 30-second target follows to 60.
#93
golangci-lint is no longer installed or run on the host. script/lint is
now a thin wrapper that builds the new root Dockerfile.lint, which COPYs
the repo into the digest-pinned golangci/golangci-lint:v2.12.2 image and
lints as a build step, so a successful build is a clean lint. This works
even where the docker daemon is remote and bind mounts are impossible.
Dockerfile.lint is split into a deps stage (base image, go mod download)
and a lint stage (source copy, linter run). script/lint passes
--no-cache-filter=lint so the lint stage executes on every invocation:
caching is explicitly waived for linting, and a cached build lints
nothing. The deps stage stays cached and no global cache invalidation is
performed. --progress=plain keeps the linter's own output visible.
golangci-lint config verify is deliberately omitted: it fetches its JSON
schema over a live, unpinned HTTPS call, which would make linting
network-dependent and defeat hash-pinning.
script/bootstrap no longer installs golangci-lint and warns instead when
docker is absent. The goimports install stays, since script/fmt and
script/fmt-check still run it on the host.
The root Dockerfile ran make check in its builder stage, which would now
recurse into script/lint and shell out to docker build with no daemon
available. It gains its own lint stage on the same pinned image, invoked
directly, with the builder depending on it via COPY --from=lint and
running make fmt-check, make test and make build.
Updates golangci-lint to v2.12.2 and sets `.golangci.yml` to the org-standard v2-schema config already deployed across the org's repos. The config change is owner-authorized (see #96 (comment) and #96 (comment)); the same file is being landed as canonical via prompts PR #24 (sneak/prompts#24).
## Changes
- **Commit-pinned installs**: golangci-lint pinned to commit `c0d3ddc9cf3faa61a4e378e879ece580256d76e5` (v2.12.2, released 2026-05-06) in `Dockerfile` and `script/bootstrap`.
- **`.golangci.yml` set to the org-standard v2 config** (sha256 `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`), byte-identical to the file used across the org's other repos. Settings live under `linters.settings`, so the `lll`/`funlen`/`cyclop`/`dupl` thresholds are actually applied (under the old hybrid file, v2 silently ignored the top-level `linters-settings` block).
- **Lint fixes** required by the now-active thresholds:
- `goconst`: shared constants for repeated status/priority/DNS-fixture strings in `internal/watcher/watcher.go` and the notify, state, and watcher tests
- `dupl`: consolidated duplicated ntfy/slack HTTP-error tests and SendNotification endpoint-error tests behind shared helpers in `internal/notify/delivery_test.go`
- `lll`: wrapped long test table entries and comments in `internal/config/classify_test.go`, `internal/notify/history_test.go`, `internal/state/state_test.go`, `internal/watcher/watcher_test.go`; shortened one inline nolint justification in `internal/notify/retry.go`
- **`TODO.md`**: Completed Steps entry updated in the same commit.
- Rebased onto current `main` (`f79cd98`); the branch is one clean commit.
## Notes
- v2.12 deprecates the `gomodguard` linter in favor of `gomodguard_v2`. The org-standard config does not disable the deprecated linter, so golangci-lint may emit an informational deprecation warning; this is accepted by the owner and does not affect the exit status (this exact config+code combination was CI-green at `dea7e44`).
## Verification
- `make check` exits 0 (fmt-check, tests, lint)
- `make lint`: 0 issues; no deprecation warning surfaced in the runs performed
- sha256 of `.golangci.yml` at HEAD verified equal to `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`
Co-authored-by: sneak <sneak@sneak.berlin>
Reviewed-on: #96
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
