config: stop startup on an invalid DNS or TLS interval (closes #177)

DNSWATCHER_DNS_INTERVAL and DNSWATCHER_TLS_INTERVAL were parsed with
time.ParseDuration and silently replaced by the default when that failed,
so a value like 5 or 1d gave hourly checks with no hint why, and zero or
negative values were accepted. Both now go through parseInterval, which
returns an error naming the variable and the value, and startup stops the
same way it does for invalid targets. An unset or empty variable still gets
its default from setupViper. The three tests that pinned the old fallback
are replaced. The README says what a valid value looks like.

Model: opus-5-5

Dockerfile

@@ -34,8 +34,11 @@ COPY . .
 # Run the tests - build fails if any test fails
 RUN make test
 
-# Build the binary
+# Build the binary. .dockerignore leaves out .git, so `git describe` in
-RUN make build
+# the Makefile cannot find the version here: script/docker passes it as
+# --build-arg VERSION, and a build that passes none reports `dev`.
+ARG VERSION=dev
+RUN make build VERSION="${VERSION}"
 
 # Runtime stage
 # alpine 3.21, 2026-02-28

Makefile

@@ -1,6 +1,8 @@
 .PHONY: all bootstrap setup build lint fmt fmt-check test check clean hooks docker
 
 BINARY := dnswatcher
+# `make build VERSION=...` overrides this; the Dockerfile does so, as the
+# image has no .git to describe.
 VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev")
 LDFLAGS := -X main.Version=$(VERSION)
 

README.md

@@ -488,7 +488,8 @@ them. We provide:
 - `script/check` — run test, lint, and fmt-check
 - `script/docker` — build the Docker image tagged via `script/projectname`, with
   `--no-cache-filter=lint,builder` so the lint stage and the builder stage,
-  which runs the tests, run on every invocation
+  which runs the tests, run on every invocation, and with the version from
+  `git describe` passed as `--build-arg VERSION`
 - `script/cibuild` — CI entrypoint: `docker build` with
   `--no-cache-filter=lint,builder`, so the lint stage and the builder stage,
   which runs the tests, run on every invocation, because a cached build lints
@@ -510,11 +511,14 @@ make clean      # Remove build artifacts
 
 ### Build-Time Variables
 
-Version is injected via `-ldflags`:
+`make build` sets the version with `-ldflags "-X main.Version=..."`, taking
+it from `git describe --tags --always --dirty`, or from `VERSION` when given
+on the command line (`make build VERSION=1.2.3`). The version appears in the
+startup log and in the health check response.
 
-```sh
+The Docker image has no `.git`, so the `Dockerfile` takes the version as
-go build -ldflags "-X main.Version=$(git describe --tags --always)" ./cmd/dnswatcher
+`--build-arg VERSION`. `make docker` passes it; a plain `docker build`
-```
+passes none, and that image reports `dev`.
 
 ---
 

TODO.md

@@ -21,7 +21,13 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104
 # Completed Steps
 
 - 2026-10-01: a `DNSWATCHER_DNS_INTERVAL` or `DNSWATCHER_TLS_INTERVAL` that is
-  not a positive duration stops startup instead of being ignored (closes #177).
+  not a positive duration stops startup; empty means the default (closes #177).
+- 2026-10-01: the image built by `make docker` reports the `git describe`
+  version, not `dev`, and the startup log now shows it (closes #109).
+- 2026-10-01: two notify shutdown tests always release the delivery they hold,
+  so a drain that returns early fails them instead of hanging (closes #176).
+- 2026-10-01: `script/install-precommit` asks git for the repository's git
+  directory, so `make hooks` also works where `.git` is a file (closes #129).
 - 2026-10-01: `TODO.md` brought up to date: open issues listed by URL, every
   Completed Steps entry cut to at most two lines (closes #146).
 - 2026-10-01: wildcard CORS now applies only to the public routes, not to
@@ -93,7 +99,6 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104
   https://git.eeqj.de/sneak/dnswatcher/issues/107
 - rate limit on `/metrics` Basic Auth:
   https://git.eeqj.de/sneak/dnswatcher/issues/101
-- images report version `dev`: https://git.eeqj.de/sneak/dnswatcher/issues/109
 - trial run of the finished image:
   https://git.eeqj.de/sneak/dnswatcher/issues/149
 - 1.0 readiness: run it with a real config and read the logs:
@@ -101,12 +106,8 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104
 - `goimports` in `make fmt-check`, Markdown formatting:
   https://git.eeqj.de/sneak/dnswatcher/issues/119
 - final state save at shutdown: https://git.eeqj.de/sneak/dnswatcher/issues/114
-- `internal/notify` shutdown tests hang when a drain returns early:
-  https://git.eeqj.de/sneak/dnswatcher/issues/176
 - README accuracy sweep: https://git.eeqj.de/sneak/dnswatcher/issues/108
 - README sections required by policy:
   https://git.eeqj.de/sneak/dnswatcher/issues/173
-- `script/install-precommit` in a linked worktree:
-  https://git.eeqj.de/sneak/dnswatcher/issues/129
 - fixed root server order: https://git.eeqj.de/sneak/dnswatcher/issues/138
 - review toward 1.0: https://git.eeqj.de/sneak/dnswatcher/issues/144

cmd/dnswatcher/main.go

@@ -63,6 +63,7 @@ func main() {
 				return n
 			},
 		),
+		fx.Invoke(func(l *logger.Logger) { l.Identify() }),
 		fx.Invoke(func(*server.Server, *watcher.Watcher) {}),
 	).Run()
 }

internal/config/config.go

@@ -29,7 +29,8 @@ var ErrNoTargets = errors.New(
 )
 
 // ErrInvalidInterval is returned when DNSWATCHER_DNS_INTERVAL or
-// DNSWATCHER_TLS_INTERVAL is set to something other than a positive duration.
+// DNSWATCHER_TLS_INTERVAL is set but is not a positive duration. An empty
+// value counts as unset and means the default.
 var ErrInvalidInterval = errors.New(
 	"interval must be a positive duration such as 30m or 1h",
 )

internal/notify/shutdown_test.go

@@ -143,6 +143,12 @@ func TestDrainWaitsForInFlightDelivery(t *testing.T) {
 	srv := blockingNtfyServer(entered, release, &served)
 	defer srv.Close()
 
+	// srv.Close waits for the handler, so release it however the
+	// test ends; otherwise a drain that returns early hangs the
+	// package instead of failing this test.
+	releaseHandler := sync.OnceFunc(func() { close(release) })
+	defer releaseHandler()
+
 	topicURL, _ := url.Parse(srv.URL)
 
 	svc := notify.NewTestService(http.DefaultTransport)
@@ -167,9 +173,7 @@ func TestDrainWaitsForInFlightDelivery(t *testing.T) {
 	// delay alone.
 	start := time.Now()
 
-	timer := time.AfterFunc(inFlightHold, func() {
+	timer := time.AfterFunc(inFlightHold, releaseHandler)
-		close(release)
-	})
 	defer timer.Stop()
 
 	ctx, cancel := context.WithTimeout(
@@ -268,7 +272,7 @@ func TestDrainBoundedByContextDeadline(t *testing.T) {
 	// all never returns here (the delivery is parked in a backoff
 	// that never fires), so an unbounded drain must fail this
 	// test promptly instead of hanging the package until the test
-	// binary's 30s timeout.
+	// binary's -timeout.
 	returned := make(chan struct{})
 
 	go func() {
@@ -447,6 +451,11 @@ func TestNewRegistersDrainingStopHook(t *testing.T) {
 	srv := blockingNtfyServer(entered, release, &served)
 	defer srv.Close()
 
+	// As in TestDrainWaitsForInFlightDelivery: release the handler
+	// however the test ends, before srv.Close waits for it.
+	releaseHandler := sync.OnceFunc(func() { close(release) })
+	defer releaseHandler()
+
 	lifecycle := &recordingLifecycle{}
 	svc := newNotifyService(t, lifecycle, srv.URL)
 
@@ -472,9 +481,7 @@ func TestNewRegistersDrainingStopHook(t *testing.T) {
 		t.Fatal("delivery never reached the endpoint")
 	}
 
-	timer := time.AfterFunc(inFlightHold, func() {
+	timer := time.AfterFunc(inFlightHold, releaseHandler)
-		close(release)
-	})
 	defer timer.Stop()
 
 	ctx, cancel := context.WithTimeout(

script/docker

@@ -12,7 +12,15 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
 
 main() {
     cd "$ROOT"
-    docker build --no-cache-filter=lint,builder -t "$("$SCRIPT_DIR/projectname")" .
+    # Own line: a failing command substitution inside an argument does
+    # not trip `set -e`, so the inline form degrades silently to an
+    # empty constant. VERSION is computed here because .dockerignore
+    # excludes .git, so `git describe` in a build stage cannot find it.
+    version="$(git describe --tags --always --dirty 2>/dev/null || true)"
+    [ -n "$version" ] || version="unknown"
+    docker build --no-cache-filter=lint,builder \
+        --build-arg VERSION="$version" \
+        -t "$("$SCRIPT_DIR/projectname")" .
 }
 
 main "$@"

script/install-precommit

@@ -7,7 +7,20 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 
 main() {
     cd "$ROOT"
-    hook=".git/hooks/pre-commit"
+    # Stop if this directory is not the top of its own git checkout, for
+    # example a copy inside another repository, whose hook must not be
+    # replaced.
+    if [ "$(git rev-parse --show-toplevel)" != "$ROOT" ]; then
+        echo "install-precommit: $ROOT is not the top of a git checkout" >&2
+        exit 1
+    fi
+    # Ask git for the repository's own git directory: .git is a file, not
+    # a directory, in some checkouts (for example a clone made with
+    # --separate-git-dir). core.hooksPath is deliberately not followed, so
+    # the hook is never written outside this repository.
+    hooks="$(git rev-parse --git-common-dir)/hooks"
+    mkdir -p "$hooks"
+    hook="$hooks/pre-commit"
     printf '#!/bin/sh\nset -e\nscript/precommit\n' > "$hook"
     chmod +x "$hook"
     echo "pre-commit hook installed: runs script/precommit"