chore: readability comment sweep + WIP routing/cookie refactor checkpoint

Phase 1 of the feature/v2.0 readability pass, plus the in-progress
routing (exact-tier-first lookup) and cookie-mirror refactor that was
already in the working tree. Bundled as a single known-good checkpoint
before the Phase 2 mechanical renames.

Phase 1 (comment-only, verified against a pre-sweep snapshot):
  - stripped provenance breadcrumbs (TASK-/DR-/finding-# / spec cites)
    from comment text, keeping the behavioral rationale; refs remain
    only inside string literals (static_assert / [[deprecated]]), which
    are code.
  - fixed actively-misleading comments (any_hooks_ memory-ordering
    contract, hook_handle detach()/ctor discriminator, methods_allowed_
    TOCTOU overclaim, matched_path_template, use_regex default, etc.).
  - added missing invariant comments (route-tier self-match caveat,
    route_cache hash-sync, request-lifecycle state machine,
    ip_representation shape invariants, path-normalization chain, ...).

Build green (make -j4, exit 0); full suite 109/109 prior to checkpoint.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: etr

src/cookie.cpp

@@ -18,7 +18,7 @@
      USA
 */
 
-// TASK-064: implementation of the structured cookie value type.
+// Implementation of the structured cookie value type.
 // See src/httpserver/cookie.hpp for the public contract.
 
 #include "httpserver/cookie.hpp"

src/create_test_request.cpp

@@ -32,7 +32,7 @@ namespace httpserver {
 
 namespace {
 
-// TASK-016: heap-deleter used for the test-request impl. The live-request
+// Heap-deleter used for the test-request impl. The live-request
 // constructor in http_request.cpp uses an internal-linkage delete_impl_heap
 // of the same shape; reproducing it here avoids exposing it across TUs.
 // Both implementations call operator delete, matching v1 lifetime exactly.
@@ -103,7 +103,7 @@ http_request create_test_request::build() {
     req.impl_->tls_enabled_local = _tls_enabled;
 #endif  // HAVE_GNUTLS
 
-    // TASK-057: propagate the test-builder opt-in into the impl. The
+    // Propagate the test-builder opt-in into the impl. The
     // webserver-dispatch path does the equivalent assignment in
     // webserver_impl::requests_answer_first_step.
     req.set_expose_credentials_in_logs(_expose_credentials_in_logs);

src/create_webserver.cpp

@@ -39,7 +39,7 @@
 
 namespace httpserver {
 
-// TASK-034 / PRD-FLG-REQ-001: defined here (not as a default member
+// Defined here (not as a default member
 // initializer in the header) so the public header carries no
 // #ifdef HAVE_BAUTH. The library was compiled with the right
 // HAVE_BAUTH state; the consumer TU's HAVE_BAUTH is irrelevant.
@@ -51,7 +51,7 @@ bool create_webserver::basic_auth_default() noexcept {
 #endif
 }
 
-// PRD-FLG-REQ-001: same pattern as basic_auth_default(). Returns true
+// Same pattern as basic_auth_default(). Returns true
 // on HAVE_DAUTH-on builds (preserving historical behaviour) and false
 // on HAVE_DAUTH-off builds so an unmodified builder doesn't trip the
 // feature_unavailable guard in webserver::webserver().

src/detail/body.cpp

@@ -44,8 +44,6 @@ namespace detail {
 // Layout-pinning static_asserts for iovec_entry → MHD_IoVec / struct iovec.
 // These asserts live next to the cast site (iovec_body::materialize below)
 // to catch platform layout drift at compile time rather than at runtime.
-// iovec_response.cpp was removed in TASK-013 (the subclass hierarchy was
-// replaced by http_response value-type factories in TASK-010).
 //
 // The POSIX `struct iovec` asserts are gated on !_WIN32 (no <sys/uio.h> on
 // MSYS2/mingw); the MHD_IoVec asserts run everywhere because that's the
@@ -98,7 +96,7 @@ MHD_Response* empty_body::materialize() {
 MHD_Response* string_body::materialize() {
     // _static variant: accepts const void* directly, no const_cast needed,
     // and documents the same PERSISTENT ownership semantics — the buffer is
-    // owned by *this and outlives the MHD_Response (TASK-009). Requires
+    // owned by *this and outlives the MHD_Response. Requires
     // MHD >= 0x00097701, well below the project minimum 0x01000000.
     return MHD_create_response_from_buffer_static(
         content_.size(),
@@ -109,7 +107,7 @@ MHD_Response* string_body::materialize() {
 // file_body — opens the file and fstat's it at construction so size() is
 // accurate immediately.  materialize() uses fstat's st_size; it never calls
 // lseek(), so the fd's read position remains at 0 when handed to
-// MHD_create_response_from_fd (security-reviewer-iter1-1 / CWE-367).
+// MHD_create_response_from_fd (CWE-367).
 // ---------------------------------------------------------------------------
 file_body::file_body(std::string path) noexcept
     : path_(std::move(path)) {
@@ -128,7 +126,7 @@ file_body::file_body(std::string path) noexcept
     }
 
     // Use fstat's st_size directly — no lseek, no TOCTOU, no fd-position
-    // side-effect (security-reviewer-iter1-1 / performance-reviewer-iter1-4).
+    // side-effect.
     //
     // CWE-190 guard: on 32-bit platforms std::size_t is 32 bits while off_t
     // can be 64 bits; a file larger than 4 GiB would silently truncate via
@@ -234,8 +232,8 @@ MHD_Response* pipe_body::materialize() {
 // ---------------------------------------------------------------------------
 ssize_t deferred_body::trampoline(void* cls, std::uint64_t pos,
                                   char* buf, std::size_t max) {
-    // Guard against null cls or empty producer_ (security-reviewer-iter1-3 /
-    // CWE-476). MHD's callback mechanism does not catch C++ exceptions, so
+    // Guard against null cls or empty producer_ (CWE-476).
+    // MHD's callback mechanism does not catch C++ exceptions, so
     // throwing std::bad_function_call here would call std::terminate().
     // Return MHD_CONTENT_READER_END_WITH_ERROR instead.
     auto* self = static_cast<deferred_body*>(cls);
@@ -248,13 +246,13 @@ ssize_t deferred_body::trampoline(void* cls, std::uint64_t pos,
 MHD_Response* deferred_body::materialize() {
     // Block size 1024 mirrors v1 deferred_response::get_raw_response_helper.
     // Free-callback is nullptr because *this owns producer_ and outlives the
-    // MHD_Response (TASK-009 enforces this via http_response's lifetime).
+    // MHD_Response (http_response's lifetime enforces this).
     return MHD_create_response_from_callback(
         MHD_SIZE_UNKNOWN, 1024, &deferred_body::trampoline, this, nullptr);
 }
 
 // ---------------------------------------------------------------------------
-// digest_challenge_body — TASK-062 / RFC 7616.
+// digest_challenge_body — RFC 7616 Digest auth challenge.
 //
 // Returns a body-only MHD_Response carrying the "access denied" payload.
 // The WWW-Authenticate header itself is NOT attached here -- the dispatch

src/detail/hook_phase_dispatch.cpp

@@ -20,13 +20,12 @@
 
 // hook_phase_dispatch.cpp -- per-phase hook firing (the fire_* family).
 //
-// Carved out of src/hook_handle.cpp in TASK-086 to keep both translation
-// units under the project per-file LOC ceiling (FILE_LOC_MAX in
+// Split out of src/hook_handle.cpp to keep both translation units under
+// the project per-file LOC ceiling (FILE_LOC_MAX in
 // scripts/check-file-size.sh). hook_handle.cpp retains the hook_handle
 // lifetime ops (move/dtor/detach/remove and the per-phase erase
 // templates); this file holds the dispatch helpers and every
-// detail::webserver_impl::fire_* member. No behaviour change: the bodies
-// are moved verbatim.
+// detail::webserver_impl::fire_* member.
 
 #include <exception>
 #include <optional>
@@ -44,20 +43,13 @@
 
 namespace httpserver {
 
-// ---- fire_* (TASK-046) ---------------------------------------------------
+// ---- fire_*: lifecycle phases --------------------------------------------
 //
-// Per the plan: snapshot the phase vector under a shared_lock, release
-// the lock, iterate the snapshot inside a try/catch routed through
-// log_dispatch_error. Mirrors the TASK-027 route-cache promotion pattern
-// (shared_lock + atomic gate). Reentrancy: a hook may call
-// ws.add_hook() / handle.remove() because we no longer hold the table
-// lock by the time the user code runs.
-
-// (Previously an unused kHookSnapshotReserve constant lived here.
-// The thread_local snapshot buffers below are now sized lazily on
-// first use; the constant was dead code as of TASK-048's perf rework.
-// Removed in TASK-049 to silence -Wunused-const-variable under
-// --enable-debug.)
+// Snapshot the phase vector under a shared_lock, release the lock,
+// iterate the snapshot inside a try/catch routed through
+// log_dispatch_error. Reentrancy: a hook may call ws.add_hook() /
+// handle.remove() because we no longer hold the table lock by the time
+// the user code runs.
 
 // fire_hooks_for_phase: shared dispatch template for all void-returning
 // lifecycle hook phases. Snapshots the caller-supplied vector under a
@@ -68,12 +60,12 @@ namespace httpserver {
 // callers are noexcept and will std::terminate on uncaught throws; the
 // inner catches ensure no exception can propagate.
 //
-// TASK-048 perf: the snapshot vector is thread_local so the per-template-
+// Perf: the snapshot vector is thread_local so the per-template-
 // instantiation per-thread buffer is reused across calls — no heap
 // allocation after the first request on each thread (warm path).
 
 // One-allocation log helpers shared by fire_hooks_for_phase and
-// fire_short_circuit_hooks_for_phase (TASK-049 review). Free helpers
+// fire_short_circuit_hooks_for_phase. Free helpers
 // (not lambdas captured per-instance) so the two templates' inner
 // catch blocks can stay one-liners without divergent strings.
 static void log_hook_threw(detail::webserver_impl* impl,
@@ -144,7 +136,7 @@ void detail::webserver_impl::fire_connection_closed(
     fire_hooks_for_phase(this, hooks_connection_closed_, ctx, "connection_closed");
 }
 
-// ---- fire_* (TASK-047) ---------------------------------------------------
+// ---- fire_*: pre-handler short-circuit phases ----------------------------
 //
 // Short-circuit-capable firing helpers. Mirrors fire_hooks_for_phase but:
 //   - the entry's std::function returns hook_action,
@@ -153,9 +145,9 @@ void detail::webserver_impl::fire_connection_closed(
 //     extracted http_response.
 //
 // A throwing hook is caught + logged and treated as if it had returned
-// hook_action::pass() -- same DR-009 §5.2 routing as the void variant.
+// hook_action::pass() -- same exception routing as the void variant.
 //
-// TASK-048 perf: thread_local snapshot buffer (same rationale as
+// Perf: thread_local snapshot buffer (same rationale as
 // fire_hooks_for_phase above).
 template <typename Ctx>
 static std::optional<::httpserver::http_response>
@@ -208,7 +200,7 @@ detail::webserver_impl::fire_body_chunk(
         this, hooks_body_chunk_, ctx, "body_chunk");
 }
 
-// ---- fire_* (TASK-048) ---------------------------------------------------
+// ---- fire_*: route_resolved + before_handler -----------------------------
 
 void detail::webserver_impl::fire_route_resolved(
     const ::httpserver::route_resolved_ctx& ctx) noexcept {
@@ -222,7 +214,7 @@ detail::webserver_impl::fire_before_handler(
         this, hooks_before_handler_, ctx, "before_handler");
 }
 
-// ---- fire_* (TASK-049) ---------------------------------------------------
+// ---- fire_*: handler_exception -------------------------------------------
 //
 // handler_exception is the only short-circuit-capable phase whose ctx is
 // passed as `const&` (the user cannot mutate the in-flight exception or
@@ -235,10 +227,16 @@ detail::webserver_impl::fire_before_handler(
 // try/catch -- with an extra tail that invokes the alias slot after the
 // user vector is exhausted.
 //
-// Structural differences preventing template reuse (findings #12, #13):
+// Structural differences vs fire_short_circuit_hooks_for_phase:
 //   1. ctx is `const Ctx&` (not `Ctx&`) -- the template takes mutable.
 //   2. The alias tail (handler_exception_alias_) sits AFTER the user
 //      vector and has its own throw-containment block.
+//   3. No empty-vector early-return under the lock: the template bails
+//      out before copying when the phase vector is empty; here the
+//      snapshot copy runs unconditionally. Behaviourally irrelevant --
+//      copying an empty vector into the already-cleared thread_local
+//      buffer allocates nothing, and control must fall through to the
+//      alias tail regardless (it fires even with zero user hooks).
 // If the template is extended to support const-ctx or alias-tail in a
 // future task, fire_handler_exception should be collapsed into it and
 // the rationale comment updated accordingly.
@@ -248,7 +246,7 @@ detail::webserver_impl::fire_handler_exception(
     using EntryVec = std::vector<phase_entry<
         ::httpserver::hook_action(
             const ::httpserver::handler_exception_ctx&)>>;
-    // Per-thread cost note (finding #30): this thread_local EntryVec is a
+    // Per-thread cost note: this thread_local EntryVec is a
     // SECOND per-thread snapshot buffer in addition to the one inside
     // fire_short_circuit_hooks_for_phase. Both buffers are warm after the
     // first invocation on a given thread (no heap allocation on subsequent
@@ -280,13 +278,13 @@ detail::webserver_impl::fire_handler_exception(
     }
     // Tail: invoke the alias slot, if any. Read without synchronisation:
     // the slot is single-writer-at-construction, immutable after
-    // webserver::start() (DR-012 / §4.10).
+    // webserver::start().
     //
     // Throw containment: a throwing alias is logged with the legacy
-    // "internal_error_handler threw" prefix so the DR-009 §5.2 point 4
-    // log contract (and its tests in basic.cpp) is preserved verbatim
-    // even though the call site has moved from
-    // run_internal_error_handler_safely into the hook chain.
+    // "internal_error_handler threw" prefix so the error-log contract
+    // (and its tests in basic.cpp) is preserved verbatim even though
+    // the call site has moved from run_internal_error_handler_safely
+    // into the hook chain.
     if (handler_exception_alias_) {
         try {
             auto action = handler_exception_alias_(ctx);
@@ -305,7 +303,7 @@ detail::webserver_impl::fire_handler_exception(
     return std::nullopt;
 }
 
-// ---- fire_* (TASK-050) ---------------------------------------------------
+// ---- fire_*: post-handler phases -----------------------------------------
 //
 // after_handler is the post-handler short-circuit. Returns engaged
 // optional iff a hook short-circuited with respond_with(); the caller
@@ -320,13 +318,13 @@ detail::webserver_impl::fire_after_handler(
 }
 
 // response_sent: void-returning user hooks, then the log_access_alias_
-// slot. Same alias-tail pattern as fire_handler_exception (TASK-049).
+// slot. Same alias-tail pattern as fire_handler_exception.
 void detail::webserver_impl::fire_response_sent(
     const ::httpserver::response_sent_ctx& ctx) noexcept {
     fire_hooks_for_phase(this, hooks_response_sent_, ctx, "response_sent");
     // Tail: invoke the alias slot, if any. Read without synchronisation:
     // the slot is single-writer-at-construction, immutable after
-    // webserver::start() (DR-012 / §4.10).
+    // webserver::start().
     if (log_access_alias_) {
         try {
             log_access_alias_(ctx);

src/detail/http_endpoint.cpp

@@ -164,10 +164,19 @@ bool http_endpoint::operator <(const http_endpoint& b) const {
 bool http_endpoint::match(const http_endpoint& url) const {
     if (!reg_compiled) throw std::invalid_argument("Cannot run match. Regex suppressed.");
 
+    // Family (prefix) rule: a family endpoint matches when the FIRST N
+    // request segments match the registered pattern, where N is the
+    // pattern's own segment count (url_pieces.size()). A request with
+    // fewer segments than the pattern can never satisfy that prefix
+    // rule, so it falls through to plain full-string matching below
+    // (which also handles every non-family endpoint).
     if (!family_url || url.url_pieces.size() < url_pieces.size()) {
         return regex_match(url.url_complete, re_url_normalized);
     }
 
+    // Rebuild the request path truncated to the pattern's first N
+    // segments in `nn`, so the prefix can be regex-matched against
+    // re_url_normalized (which matches whole strings, not prefixes).
     string nn = "/";
     bool first = true;
     for (unsigned int i = 0; i < url_pieces.size(); i++) {