TASK-007: CI test for public-header hygiene

etr · claude · etr · commit 601855966c7b · 2026-05-03T10:41:59.000+02:00
Add a two-layer header-hygiene gate that locks in the "no backend
headers leak through &lt;httpserver.hpp&gt;" invariant from PRD-HDR-REQ-001..003.

Layer 1 -- compile/runtime sentinel (test/unit/header_hygiene_test.cpp):
  Includes only &lt;httpserver.hpp&gt;, then checks well-known include-guard
  macros (MHD_VERSION, _PTHREAD_H{,_}, GNUTLS_GNUTLS_H, _SYS_SOCKET_H{,_},
  _SYS_UIO_H{,_}). At runtime it prints the leaked headers and exits 1.
  Per-target CPPFLAGS overrides AM_CPPFLAGS so HTTPSERVER_COMPILATION
  and the build-tree -I src/httpserver/ entries are NOT in scope --
  mimics a real consumer translation unit.

Layer 2 -- preprocessor grep against staged install (`make check-hygiene`):
  Stages `make install DESTDIR=$(CHECK_HYGIENE_STAGE)` to a clean tree,
  preprocesses test/headers/consumer_umbrella_no_backend.cpp using ONLY
  -I$(CHECK_HYGIENE_STAGE)$(includedir), then greps cpp line markers
  for forbidden backend headers. HEADER_HYGIENE_STRICT controls
  fatality (default no -&gt; informational; yes -&gt; hard fail at TASK-020).

Both gates are wired into `make check`:
- header_hygiene runs as a check_PROGRAMS test, marked XFAIL_TESTS
  until M5 lands and the umbrella is clean. Automake's XPASS-as-error
  default is the explicit signal for TASK-020 to remove the marker.
- check-hygiene runs via check-local; in non-strict mode it prints an
  EXPECTED-FAIL banner with diagnostics and exits 0 so `make check`
  stays green during M2-M5 while keeping leak progress visible.

CI surface: new header-hygiene matrix entry in verify-build.yml runs
`make check-hygiene` as a focused, named GitHub Actions check.

TASK-020.md updated with explicit M5 close-out steps (delete
XFAIL_TESTS line + flip HEADER_HYGIENE_STRICT default).

Verified locally on macOS/aarch64 with gnutls 3.x, libmicrohttpd 1.0.5,
Apple Clang 15+: 24 tests / 23 PASS / 1 XFAIL (header_hygiene); the
sentinel correctly reports microhttpd, pthread, gnutls, sys/socket,
sys/uio leaks; check-hygiene reports EXPECTED-FAIL on staged install
(webserver.hpp still references private detail header until TASK-014).

Co-Authored-By: Claude Opus 4.7 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/.github/workflows/verify-build.yml b/.github/workflows/verify-build.yml
@@ -253,6 +253,24 @@ jobs:
             debug: debug
             coverage: nocoverage
             shell: bash
+          # TASK-007: dedicated header-hygiene gate. Runs `make check-hygiene`
+          # (preprocesses <httpserver.hpp> against the staged install and greps
+          # for forbidden backend headers). Surfaces this gate as its own named
+          # GitHub Actions check so reviewers see header-hygiene status
+          # independently of the broader `make check` log. Until M5 lands the
+          # check is informational (HEADER_HYGIENE_STRICT defaults to "no");
+          # TASK-020 flips it to strict.
+          - test-group: extra
+            os: ubuntu-latest
+            os-type: ubuntu
+            build-type: header-hygiene
+            compiler-family: gcc
+            c-compiler: gcc-14
+            cc-compiler: g++-14
+            debug: nodebug
+            coverage: nocoverage
+            linking: dynamic
+            shell: bash
           - test-group: basic
             os: windows-latest
             os-type: windows
@@ -634,6 +652,17 @@ jobs:
         make check;
       if: ${{ matrix.build-type != 'iwyu' && matrix.compiler-family != 'arm-cross' }}
 
+    - name: Run header-hygiene check
+      # TASK-007: dedicated public-header hygiene gate. Runs the
+      # preprocessor-grep target (Layer 2) against a staged install and
+      # reports any forbidden backend headers reaching <httpserver.hpp>.
+      # Currently informational (HEADER_HYGIENE_STRICT=no) -- TASK-020
+      # flips this to strict when M5 closes the umbrella.
+      run: |
+        cd build
+        make check-hygiene
+      if: ${{ matrix.build-type == 'header-hygiene' }}
+
     - name: Print tests results
       shell: bash
       run: |
diff --git a/Makefile.am b/Makefile.am
@@ -40,7 +40,8 @@ endif
 
 EXTRA_DIST = libhttpserver.pc.in $(DX_CONFIG) scripts/extract-release-notes.sh scripts/validate-version.sh \
 	test/headers/consumer_direct.cpp test/headers/consumer_detail.cpp test/headers/consumer_umbrella.cpp \
-	test/headers/consumer_post_umbrella.cpp
+	test/headers/consumer_post_umbrella.cpp \
+	test/headers/consumer_umbrella_no_backend.cpp
 
 # ---------------------------------------------------------------------------
 # Header-hygiene checks (TASK-002)
@@ -163,9 +164,83 @@ check-install-layout:
 	@rm -rf $(CHECK_INSTALL_STAGE)
 	@echo "  PASS: staged install layout is clean"
 
-check-local: check-headers check-install-layout
+# ---------------------------------------------------------------------------
+# Header-hygiene preprocessor gate (TASK-007).
+#
+# This is the preprocessor-grep half of the TASK-007 enforcement (the
+# compile-time half lives as `header_hygiene` in test/Makefile.am).
+#
+# Procedure:
+#   1. Stage `make install DESTDIR=$(CHECK_HYGIENE_STAGE)` to get a
+#      pristine public include tree -- exactly what packagers and
+#      downstream consumers see.
+#   2. Preprocess test/headers/consumer_umbrella_no_backend.cpp using
+#      ONLY -I$(CHECK_HYGIENE_STAGE)$(includedir) plus $(CPPFLAGS) (so
+#      e.g. /opt/homebrew/include is on the search path -- the grep
+#      below NEEDS to resolve <microhttpd.h> if the umbrella pulls it
+#      in, otherwise we couldn't detect the leak).
+#   3. Grep the cpp output for `# <line> "<file>"` line markers that
+#      name any forbidden backend header. The line-marker filter
+#      avoids false positives from substrings in code or comments.
+#
+# HEADER_HYGIENE_STRICT controls whether a leak is fatal:
+#   - "no"  (default until M5): leaks are reported as EXPECTED-FAIL
+#           and exit 0. This keeps `make check` green during M2-M5
+#           while making M2-M5 progress visible in CI logs.
+#   - "yes" (TASK-020 close-out): leaks are fatal. Set this from the
+#           command line (`make check-hygiene HEADER_HYGIENE_STRICT=yes`)
+#           or flip the default below.
+#
+# Cross-reference: keep HEADER_HYGIENE_FORBIDDEN in sync with the
+# #ifdef ladder in test/unit/header_hygiene_test.cpp.
+# ---------------------------------------------------------------------------
+
+HEADER_HYGIENE_FORBIDDEN = microhttpd\.h|pthread\.h|gnutls/gnutls\.h|sys/socket\.h|sys/uio\.h
+CHECK_HYGIENE_STAGE      = $(abs_top_builddir)/.hygiene-stage
+CHECK_HYGIENE_CXX        = $(CXX) -std=c++20 -E -I$(CHECK_HYGIENE_STAGE)$(includedir) $(CPPFLAGS)
+HEADER_HYGIENE_STRICT   ?= no
+
+check-hygiene:
+	@echo "=== check-hygiene: <httpserver.hpp> must not transitively include backend headers ==="
+	@rm -rf $(CHECK_HYGIENE_STAGE)
+	@$(MAKE) $(AM_MAKEFLAGS) install DESTDIR=$(CHECK_HYGIENE_STAGE) >check-hygiene-install.log 2>&1 || { \
+	    echo "FAIL: staged install failed"; cat check-hygiene-install.log; \
+	    rm -f check-hygiene-install.log; rm -rf $(CHECK_HYGIENE_STAGE); exit 1; }
+	@rm -f check-hygiene-install.log
+	@status=0; \
+	if ! $(CHECK_HYGIENE_CXX) $(top_srcdir)/test/headers/consumer_umbrella_no_backend.cpp >check-hygiene.i 2>check-hygiene.err; then \
+	    if test "$(HEADER_HYGIENE_STRICT)" = "yes"; then \
+	        echo "FAIL: preprocessor failed"; cat check-hygiene.err; \
+	        status=1; \
+	    else \
+	        echo "EXPECTED-FAIL (informational until M5): preprocessor failed against staged install."; \
+	        echo "  This is expected while M2-M5 are in flight (e.g. webserver.hpp still"; \
+	        echo "  references private detail headers that aren't shipped)."; \
+	        echo "  Tail of preprocessor diagnostics:"; \
+	        sed 's/^/    /' check-hygiene.err | tail -10; \
+	    fi; \
+	else \
+	    leaks=`grep -hE '^# [0-9]+ "[^"]*($(HEADER_HYGIENE_FORBIDDEN))"' check-hygiene.i | awk '{print $$3}' | sort -u`; \
+	    if test -n "$$leaks"; then \
+	        if test "$(HEADER_HYGIENE_STRICT)" = "yes"; then \
+	            echo "FAIL: forbidden headers leaked through <httpserver.hpp>:"; \
+	            echo "$$leaks"; \
+	            status=1; \
+	        else \
+	            echo "EXPECTED-FAIL (informational until M5): forbidden headers currently leak through <httpserver.hpp>:"; \
+	            echo "$$leaks"; \
+	        fi; \
+	    else \
+	        echo "  PASS: no forbidden headers reached the consumer TU"; \
+	    fi; \
+	fi; \
+	rm -f check-hygiene.i check-hygiene.err; \
+	rm -rf $(CHECK_HYGIENE_STAGE); \
+	exit $$status
+
+check-local: check-headers check-install-layout check-hygiene
 
-.PHONY: check-headers check-install-layout
+.PHONY: check-headers check-install-layout check-hygiene
 
 MOSTLYCLEANFILES = $(DX_CLEANFILES) *.gcda *.gcno *.gcov
 DISTCLEANFILES = DIST_REVISION
diff --git a/specs/tasks/M1-foundation/TASK-007.md b/specs/tasks/M1-foundation/TASK-007.md
@@ -0,0 +1,51 @@
+### TASK-007: CI test for public-header hygiene
+
+**Milestone:** M1 - Foundation
+**Component:** CI / Test infrastructure
+**Estimate:** S
+
+**Goal:**
+Lock in the "no backend headers leak through `<httpserver.hpp>`" invariant with a CI gate so a future commit can't silently regress it.
+
+**Action Items:**
+- [x] Add a test program `test/header_hygiene.cpp` containing only `#include <httpserver.hpp>` and `int main(){}`. *(Implemented as `test/unit/header_hygiene_test.cpp` for test-tree symmetry; `test/headers/consumer_umbrella_no_backend.cpp` is the parallel source consumed by the preprocessor-grep target.)*
+- [x] In `Makefile.am`, build it without `-I` flags pointing at libmicrohttpd / pthread / gnutls headers (use only the installed-header path). *(Per-target `header_hygiene_CPPFLAGS = -I$(top_srcdir)/src $(CPPFLAGS)` overrides `AM_CPPFLAGS`, dropping `-DHTTPSERVER_COMPILATION` and `-I$(top_srcdir)/src/httpserver/`. The preprocessor-grep target uses ONLY the staged `DESTDIR` install include path.)*
+- [x] Run `g++ -E test/header_hygiene.cpp -I<install-prefix>/include` and `grep -E 'microhttpd\.h|pthread\.h|gnutls/gnutls\.h|sys/socket\.h|sys/uio\.h'` — expect zero matches. *(See `check-hygiene` in top-level `Makefile.am`. Today the grep finds matches; that's the EXPECTED-FAIL state until M5.)*
+- [x] Wire the check into `make check` (or a dedicated `make hygiene` target invoked by CI). *(Both: the runtime sentinel `header_hygiene` runs as part of `make check` (XFAIL until M5); the preprocessor-grep `check-hygiene` runs via `check-local` and also stands alone as a target for CI.)*
+- [x] Add a CI job that fails if any of the forbidden headers appear in the preprocessed output. *(Added `header-hygiene` matrix entry in `.github/workflows/verify-build.yml` running `make check-hygiene`. Currently informational; flips to fatal at TASK-020 by setting `HEADER_HYGIENE_STRICT=yes`.)*
+
+**Dependencies:**
+- Blocked by: TASK-002
+- Blocks: None (informational gate; will fail until M2-M5 land, that's expected and intended)
+
+**Acceptance Criteria:**
+- `grep -lE 'microhttpd\.h|pthread\.h|gnutls\.h|sys/socket\.h' src/httpserver/*.hpp` returns no results once M2-M5 land (PRD §3.1 acceptance).
+- The hygiene test is invoked by `make check` and fails loudly when violated.
+- Typecheck passes.
+
+**Related Requirements:** PRD-HDR-REQ-001..003
+**Related Decisions:** §9 testing item 1
+
+**Status:** Done (informational gate landed; full enforcement at TASK-020)
+
+---
+
+**Implementation Notes (TASK-007 close-out):**
+
+- **Strategy:** Option (c) from the plan -- "implement the test machinery now, mark it XFAIL until M5." Rejected (a) "leave `make check` red" (would block every PR for weeks); rejected (b) "narrow the grep to today's leaks" (encodes a binary invariant as a moving target, four chances to forget).
+- **Two layers of enforcement, both wired into `make check`:**
+  - *Layer 1 (compile-time sentinel):* `test/unit/header_hygiene_test.cpp` includes `<httpserver.hpp>` then checks well-known include-guard macros (`MHD_VERSION`, `_PTHREAD_H{,_}`, `GNUTLS_GNUTLS_H`, `_SYS_SOCKET_H{,_}`, `_SYS_UIO_H{,_}`). At runtime it prints the leaked headers and exits 1. Marked `XFAIL_TESTS` in `test/Makefile.am` so `make check` stays green.
+  - *Layer 2 (preprocessor grep):* `make check-hygiene` in the top-level `Makefile.am` stages `make install DESTDIR=$(CHECK_HYGIENE_STAGE)` and preprocesses `test/headers/consumer_umbrella_no_backend.cpp` against ONLY the staged include path, then greps cpp line markers for forbidden headers. Default `HEADER_HYGIENE_STRICT=no` makes it informational; flipping to `yes` makes it fatal.
+- **CI:** dedicated `header-hygiene` matrix entry in `.github/workflows/verify-build.yml` invokes `make check-hygiene` so the gate surfaces as its own GitHub Actions check.
+- **`<sys/uio.h>` rationale:** PRD-HDR-REQ-001..003 don't name `<sys/uio.h>` directly, but TASK-004 introduced `iovec_entry` specifically to avoid exposing it. Listing it here is a hardening assertion that TASK-004's intent isn't regressed.
+- **Why preprocessor-grep currently fails ahead of leak detection:** the staged install does not ship `details/` headers (per TASK-002); `webserver.hpp` still references `httpserver/details/http_endpoint.hpp` until TASK-014's PIMPL split. The `check-hygiene` recipe treats this preprocessor failure as EXPECTED-FAIL in informational mode, with diagnostics so M2-M5 progress remains visible.
+
+**M5 close-out (TASK-020 owner: zero ambiguity):**
+
+When TASK-020 makes `<httpserver.hpp>` clean of backend headers:
+
+1. Run `make check-hygiene HEADER_HYGIENE_STRICT=yes` from the build dir -- confirm exit 0 and `PASS: no forbidden headers reached the consumer TU`.
+2. Run `make check` -- expect Automake to report `XPASS: header_hygiene` (treated as a hard error by default), confirming the sentinel now passes.
+3. In `test/Makefile.am`, delete the line `XFAIL_TESTS = header_hygiene` and the comment block above it. Re-run `make check` -- expect `PASS: header_hygiene` and overall green.
+4. In `Makefile.am`, change `HEADER_HYGIENE_STRICT ?= no` to `HEADER_HYGIENE_STRICT ?= yes` (or remove the conditional and inline the strict path). Re-run `make check` to confirm `check-hygiene` is green.
+5. Mark this task `Status: Done (full enforcement)` and tick the M5 acceptance criterion (`grep -lE '...' src/httpserver/*.hpp` returns no results).
diff --git a/specs/tasks/M3-request/TASK-020.md b/specs/tasks/M3-request/TASK-020.md
@@ -0,0 +1,31 @@
+### TASK-020: Final public-header backend-include sweep
+
+**Milestone:** M3 - Webserver internal & Request Refactor
+**Component:** Public headers (sweep)
+**Estimate:** S
+
+**Goal:**
+Verify and lock the "no backend headers in public surface" invariant after PIMPL splits and accessor refactors land, removing any straggler includes that survived earlier tasks.
+
+**Action Items:**
+- [ ] `grep -lE 'microhttpd\.h|pthread\.h|gnutls/gnutls\.h|sys/socket\.h|sys/uio\.h' src/httpserver/*.hpp`. Each file that turns up: route the include into the corresponding `details/*_impl.hpp` or `.cpp` file.
+- [ ] Verify after the sweep that the grep returns zero results.
+- [ ] Ensure the hygiene CI test from TASK-007 now passes. **Specifically:**
+  - [ ] In `test/Makefile.am`, delete the line `XFAIL_TESTS = header_hygiene` (and the explanatory comment block above it). After this edit, `make check` should report `PASS: header_hygiene` -- not `XFAIL` and not `XPASS`.
+  - [ ] In `Makefile.am`, change `HEADER_HYGIENE_STRICT ?= no` to `HEADER_HYGIENE_STRICT ?= yes` (or remove the conditional and inline the strict-mode path). Verify `make check-hygiene` exits 0 with `PASS: no forbidden headers reached the consumer TU`.
+  - [ ] Run `make check-hygiene HEADER_HYGIENE_STRICT=yes` from the build dir as a final smoke check.
+
+**Dependencies:**
+- Blocked by: TASK-014, TASK-015, TASK-019
+- Blocks: None (gating outcome that the rest of the project relies on)
+
+**Acceptance Criteria:**
+- `grep -lE 'microhttpd\.h|pthread\.h|gnutls\.h|sys/socket\.h' src/httpserver/*.hpp` returns no results (PRD §3.1 acceptance).
+- A test program containing only `#include <httpserver.hpp>` and `int main(){}` compiles without `-I` to libmicrohttpd / pthread / gnutls (PRD §3.1 acceptance).
+- TASK-007's hygiene test (red until now) goes green.
+- Typecheck passes.
+
+**Related Requirements:** PRD-HDR-REQ-001, PRD-HDR-REQ-002, PRD-HDR-REQ-003
+**Related Decisions:** §2.2, §5.5
+
+**Status:** Not Started
diff --git a/test/Makefile.am b/test/Makefile.am
@@ -26,7 +26,7 @@ LDADD += -lcurl
 
 AM_CPPFLAGS = -I$(top_srcdir)/src -I$(top_srcdir)/src/httpserver/ -DHTTPSERVER_COMPILATION
 METASOURCES = AUTO
-check_PROGRAMS = basic file_upload http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication deferred http_resource http_response create_webserver new_response_types daemon_info uri_log feature_unavailable header_hygiene_iovec iovec_entry iovec_response http_method constants
+check_PROGRAMS = basic file_upload http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication deferred http_resource http_response create_webserver new_response_types daemon_info uri_log feature_unavailable header_hygiene_iovec header_hygiene iovec_entry iovec_response http_method constants
 
 MOSTLYCLEANFILES = *.gcda *.gcno *.gcov
 
@@ -53,6 +53,18 @@ uri_log_SOURCES = unit/uri_log_test.cpp
 uri_log_LDADD = $(LDADD) -lmicrohttpd
 feature_unavailable_SOURCES = unit/feature_unavailable_test.cpp
 header_hygiene_iovec_SOURCES = unit/header_hygiene_iovec_test.cpp
+# header_hygiene: TASK-007 sentinel TU. Mimics a true consumer:
+# - per-target CPPFLAGS overrides AM_CPPFLAGS so HTTPSERVER_COMPILATION
+#   and the build-tree -I src/httpserver/ entries are NOT in scope (a
+#   real consumer wouldn't have either). Only -I$(top_srcdir)/src is
+#   passed so <httpserver.hpp> resolves.
+# - LDADD is overridden to empty: this is a pure-compile assertion, the
+#   `int main(){}` body has no library dependencies.
+# Currently in XFAIL_TESTS (see below); flips to PASS when M5 lands and
+# the umbrella is free of backend-header leakage.
+header_hygiene_SOURCES = unit/header_hygiene_test.cpp
+header_hygiene_CPPFLAGS = -I$(top_srcdir)/src $(CPPFLAGS)
+header_hygiene_LDADD =
 iovec_entry_SOURCES = unit/iovec_entry_test.cpp
 iovec_response_SOURCES = unit/iovec_response_test.cpp
 http_method_SOURCES = unit/http_method_test.cpp
@@ -69,6 +81,16 @@ endif
 
 TESTS = $(check_PROGRAMS)
 
+# header_hygiene is expected to fail until M5 (TASK-014/015/019/020) lands and
+# <httpserver.hpp> stops transitively pulling in <microhttpd.h>, <pthread.h>,
+# <gnutls/gnutls.h>, <sys/socket.h>, and <sys/uio.h>. Automake's XFAIL_TESTS
+# mechanism marks the failure as "expected" so the suite stays green, and --
+# importantly -- when the umbrella becomes clean and the test starts passing,
+# Automake reports XPASS and treats it as a hard error. That XPASS is the
+# explicit signal for TASK-020 to remove this line. Do NOT silently delete the
+# XFAIL until the umbrella is clean.
+XFAIL_TESTS = header_hygiene
+
 @VALGRIND_CHECK_RULES@
 VALGRIND_SUPPRESSIONS_FILES = libhttpserver.supp
 EXTRA_DIST = libhttpserver.supp
diff --git a/test/headers/consumer_umbrella_no_backend.cpp b/test/headers/consumer_umbrella_no_backend.cpp
@@ -0,0 +1,36 @@
+/*
+     This file is part of libhttpserver
+     Copyright (C) 2011-2026 Sebastiano Merlino
+
+     This library is free software; you can redistribute it and/or
+     modify it under the terms of the GNU Lesser General Public
+     License as published by the Free Software Foundation; either
+     version 2.1 of the License, or (at your option) any later version.
+
+     This library is distributed in the hope that it will be useful,
+     but WITHOUT ANY WARRANTY; without even the implied warranty of
+     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+     Lesser General Public License for more details.
+
+     You should have received a copy of the GNU Lesser General Public
+     License along with this library; if not, write to the Free Software
+     Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301
+     USA
+*/
+
+// TASK-007: consumer source used by the `make check-hygiene` target.
+//
+// The top-level Makefile.am preprocesses this file against ONLY the
+// staged install include path (DESTDIR=$(CHECK_HYGIENE_STAGE)) plus the
+// system $(CPPFLAGS), then greps the cpp output for `# <line> "..."`
+// markers that name forbidden backend headers. If any appear, the
+// umbrella has transitively pulled them in.
+//
+// We deliberately include NO standard-library headers here. Even
+// <cstdio> can pull in libc internals that on some platforms touch
+// <sys/uio.h>, which would produce false positives for the grep that
+// is checking <httpserver.hpp> hygiene specifically.
+
+#include <httpserver.hpp>
+
+int main() { return 0; }
diff --git a/test/unit/header_hygiene_test.cpp b/test/unit/header_hygiene_test.cpp
