README accuracy sweep: architecture omissions, undocumented state field, overclaimed MAINTENANCE_MODE and /metrics #108

Open
opened 2026-08-09 03:40:07 +02:00 by clawbot · 0 comments
Collaborator

A full claim-by-claim audit of README.md against the code at origin/main (9347a28) found the document to be largely accurate — the HTTP route table, the environment-variable names and defaults, the state-file JSON field names, and most of the monitoring lifecycle all check out exactly. This issue collects the residual drift that is documentation-only.

Substantive functional gaps found by the same audit are tracked separately in #104, #105, #106, and #107 and are explicitly not part of this issue.

Items to fix

1. DNSWATCHER_MAINTENANCE_MODE is overclaimed. The README (README.md:251) says "Enable maintenance mode". The only thing it does is get copied into the maintenanceMode field of the healthcheck JSON response (internal/healthcheck/healthcheck.go:73). It does not pause the watcher, does not skip checks, does not suppress notifications, and does not change the healthcheck Status, which is unconditionally "ok" (healthcheck.go:67). Reword to describe what it actually is: a flag surfaced in the healthcheck payload for external consumers to observe.

2. /metrics implies application instrumentation that does not exist. The route serves promhttp.Handler() against the default registry (internal/server/routes.go:58-61), and there is not a single promauto-registered application metric anywhere in the codebase. So /metrics exposes generic Go runtime and process metrics only — no checks-performed counter, no notifications-sent counter, nothing dnswatcher-specific. The README's "Prometheus metrics (optional)" reads as though there is app-level instrumentation. Say plainly what is exposed.

3. The Architecture tree omits real files. README.md:189-208 is missing:

  • internal/config/classify.go — the Public Suffix List classification logic that the README's own domain-vs-hostname feature bullets (README.md:44-45,56-57) depend on.
  • static/static.go — the //go:embed css package that implements the "all assets embedded in the binary" claim (README.md:168-170). The whole static/ package is absent from the tree.
  • Also worth adding: internal/notify/history.go (the 100-entry alert ring buffer backing the dashboard's "Recent alerts"), internal/notify/retry.go (backoff/retry), internal/watcher/interfaces.go, and the individual handler files dashboard.go / status.go / healthcheck.go.

Nothing listed in the tree is fictional — every entry exists. This is purely about omissions.

4. The state-file error field is undocumented. Both NameserverRecordState (internal/state/state.go:44-49) and CertificateState (:104-112) carry Error string \json:"error,omitempty"`, and it is populated on certificate failures (internal/watcher/watcher.go:731`). It appears in the persisted JSON but appears nowhere in the README's State File Format example or field description.

5. Notification delivery mechanics are undocumented. Neither the retry behaviour (internal/notify/retry.go — up to 5 retries with 1s-60s exponential backoff and jitter, applied to all three backends) nor the 100-entry in-memory alert history (internal/notify/history.go) is mentioned in the Notifications section, though the latter is what the dashboard's "Recent alerts" panel reads from.

6. Wildcard CORS on the public API is undocumented. Middleware.CORS() (internal/middleware/middleware.go:172-187) sets Access-Control-Allow-Origin: * across the API including /api/v1/status. That is a deliberate and policy-permitted choice for a public read-only API, but it is a fact about the HTTP surface that belongs in the README. Coordinate with #100, which changes the CORS configuration — document the end state, not the current one, and rebase if #100 lands first.

7. Backward-compatible PortState loading is undocumented. internal/state/state.go:64-101 special-cases loading old single-hostname state files into the current multi-hostname format. Worth a sentence in State File Format, since it is a compatibility guarantee operators may rely on.

Definition of done

  1. All seven items above corrected in README.md.
  2. No claim is added that is not verified against the code — check each edit against the file it describes.
  3. Markdown is formatted with the repo's own tooling: run make fmt and include the result in the commit. Do not hand-wrap.
  4. make check green; TODO.md updated in the same commit.

The finishing commit's title must end with (closes #N) referencing this issue.

Sequencing note

This issue touches README sections that #100, #104, #105, and #106 also touch. Land it after those where they overlap, or expect to rebase. Do not preemptively document behaviour those issues have not implemented yet — the README must describe what the code does at the moment this merges.

Process note

This is a documentation-only change (README.md plus TODO.md, zero changes to code, config, scripts, CI, or build files). Per repo policy it therefore skips adversarial review and goes straight to the merge gate once make check is green. If the implementation ends up touching any non-documentation file, that exemption is void and it goes through full review.

A full claim-by-claim audit of `README.md` against the code at `origin/main` (`9347a28`) found the document to be largely accurate — the HTTP route table, the environment-variable names and defaults, the state-file JSON field names, and most of the monitoring lifecycle all check out exactly. This issue collects the residual drift that is **documentation-only**. Substantive functional gaps found by the same audit are tracked separately in #104, #105, #106, and #107 and are explicitly **not** part of this issue. ## Items to fix **1. `DNSWATCHER_MAINTENANCE_MODE` is overclaimed.** The README (`README.md:251`) says "Enable maintenance mode". The only thing it does is get copied into the `maintenanceMode` field of the healthcheck JSON response (`internal/healthcheck/healthcheck.go:73`). It does not pause the watcher, does not skip checks, does not suppress notifications, and does not change the healthcheck `Status`, which is unconditionally `"ok"` (`healthcheck.go:67`). Reword to describe what it actually is: a flag surfaced in the healthcheck payload for external consumers to observe. **2. `/metrics` implies application instrumentation that does not exist.** The route serves `promhttp.Handler()` against the default registry (`internal/server/routes.go:58-61`), and there is not a single `promauto`-registered application metric anywhere in the codebase. So `/metrics` exposes generic Go runtime and process metrics only — no checks-performed counter, no notifications-sent counter, nothing dnswatcher-specific. The README's "Prometheus metrics (optional)" reads as though there is app-level instrumentation. Say plainly what is exposed. **3. The Architecture tree omits real files.** `README.md:189-208` is missing: - `internal/config/classify.go` — the Public Suffix List classification logic that the README's own domain-vs-hostname feature bullets (`README.md:44-45,56-57`) depend on. - `static/static.go` — the `//go:embed css` package that implements the "all assets embedded in the binary" claim (`README.md:168-170`). The whole `static/` package is absent from the tree. - Also worth adding: `internal/notify/history.go` (the 100-entry alert ring buffer backing the dashboard's "Recent alerts"), `internal/notify/retry.go` (backoff/retry), `internal/watcher/interfaces.go`, and the individual handler files `dashboard.go` / `status.go` / `healthcheck.go`. Nothing listed in the tree is fictional — every entry exists. This is purely about omissions. **4. The state-file `error` field is undocumented.** Both `NameserverRecordState` (`internal/state/state.go:44-49`) and `CertificateState` (`:104-112`) carry `Error string \`json:"error,omitempty"\``, and it is populated on certificate failures (`internal/watcher/watcher.go:731`). It appears in the persisted JSON but appears nowhere in the README's State File Format example or field description. **5. Notification delivery mechanics are undocumented.** Neither the retry behaviour (`internal/notify/retry.go` — up to 5 retries with 1s-60s exponential backoff and jitter, applied to all three backends) nor the 100-entry in-memory alert history (`internal/notify/history.go`) is mentioned in the Notifications section, though the latter is what the dashboard's "Recent alerts" panel reads from. **6. Wildcard CORS on the public API is undocumented.** `Middleware.CORS()` (`internal/middleware/middleware.go:172-187`) sets `Access-Control-Allow-Origin: *` across the API including `/api/v1/status`. That is a deliberate and policy-permitted choice for a public read-only API, but it is a fact about the HTTP surface that belongs in the README. **Coordinate with #100**, which changes the CORS configuration — document the end state, not the current one, and rebase if #100 lands first. **7. Backward-compatible `PortState` loading is undocumented.** `internal/state/state.go:64-101` special-cases loading old single-hostname state files into the current multi-hostname format. Worth a sentence in State File Format, since it is a compatibility guarantee operators may rely on. ## Definition of done 1. All seven items above corrected in `README.md`. 2. No claim is added that is not verified against the code — check each edit against the file it describes. 3. Markdown is formatted with the repo's own tooling: run `make fmt` and include the result in the commit. Do not hand-wrap. 4. `make check` green; `TODO.md` updated in the same commit. The finishing commit's title must end with ` (closes #N)` referencing this issue. ## Sequencing note This issue touches README sections that #100, #104, #105, and #106 also touch. Land it **after** those where they overlap, or expect to rebase. Do not preemptively document behaviour those issues have not implemented yet — the README must describe what the code does at the moment this merges. ## Process note This is a documentation-only change (`README.md` plus `TODO.md`, zero changes to code, config, scripts, CI, or build files). Per repo policy it therefore skips adversarial review and goes straight to the merge gate once `make check` is green. If the implementation ends up touching any non-documentation file, that exemption is void and it goes through full review.
clawbot added this to the 1.0 milestone 2026-08-09 03:40:07 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/dnswatcher#108