TASK-046: fire connection_opened / accept_decision / connection_closed

Wires the three connection-level lifecycle phases of the hook bus into
the existing MHD callback sites. Closes long-standing feature request
#332 (banned-IP log entry).

Production code
- accept_ctx extended from {peer} to {peer, accepted, reason}; `reason`
  is a std::optional<std::string_view> pointing at a string literal
  ("banned" / "not-allowed") with static storage duration.
- Three new noexcept fire_* helpers on webserver_impl (declared in the
  dispatch sibling header, defined in src/hook_handle.cpp): each takes
  a shared_lock, snapshots the phase vector with reserve(8), releases
  the lock, then iterates with try/catch routed through
  log_dispatch_error (DR-009 §5.2). Mirrors TASK-027's route-cache
  promotion pattern.
- connection_notify + policy_callback split out of
  webserver_callbacks.cpp into a new webserver_callbacks_lifecycle.cpp
  TU. The original would have overshot FILE_LOC_MAX after the firing-
  site code landed. webserver_callbacks.cpp shrinks to 432 lines.
- MHD_OPTION_NOTIFY_CONNECTION closure pointer switched from nullptr to
  the owning webserver* so connection_notify can reach
  impl_->any_hooks_ / fire_connection_opened / fire_connection_closed.
- policy_callback gains decision-derivation logic (accept_ctx.reason);
  extracted into anon-ns classify_decision() helper to stay under the
  CCN gate.
- All three firing sites are gated by a relaxed atomic load on
  any_hooks_[phase] so the zero-hook path stays one branch + one atomic
  load (PRD-HOOK-REQ-008).
- accept_decision's throwing-hook semantics are a structural
  guarantee: fire_accept_decision returns void and `decision` is
  captured in a local before the fire call.

Pre-existing build fix
- src/detail/webserver_dispatch.cpp was missing `using std::map` and
  `using httpserver::http::http_utils` directives (left out of the
  TASK-15f8083 7-way split). Added so fresh worktree builds succeed.

Tests (+4)
- test/unit/hooks_accept_ctx_shape_test.cpp: compile-time pin for the
  extended accept_ctx shape.
- test/integ/hooks_connection_lifecycle.cpp: drives one curl round-trip
  and asserts all three lifecycle hooks fire with valid peer + correct
  decision/reason; pins lifecycle ordering (closed is last; opened OR
  accept is first — MHD callback order is platform-dependent).
- test/integ/hooks_accept_decision_banned.cpp: ACCEPT policy +
  block_ip("127.0.0.1") -> hook observes accepted=false reason="banned".
- test/integ/hooks_accept_decision_throwing.cpp: two sub-tests pin that
  a throwing accept_decision hook does not flip the decision (banned
  still rejected; unbanned still accepted).
- test/integ/hooks_no_firing.cpp narrowed: still asserts zero
  invocations on the eight phases TASK-047..051 will wire; the three
  lifecycle phases are now expected to fire.

Example
- examples/banned_ip_log.cpp demonstrates the solution to issue #332:
  ACCEPT policy + block_ip + accept_decision hook logging every
  rejection to stderr with peer + reason. Wired into examples/Makefile.am.

Docs
- RELEASE_NOTES.md: one-line note under "What's new" describing the
  M5 hook bus landing.

Verification
- 55/55 tests pass (was 51, +4 new).
- check-file-size, check-examples, check-readme, check-release-notes,
  check-doxygen, check-install-layout, check-hygiene, check-duplication
  all pass. check-complexity surfaces only pre-existing TASK-045
  warnings (hook_phase::to_string, hook_handle::remove).
- cpplint clean on all modified/new files.
- Debug build (-Werror -Wextra -pedantic) compiles and tests pass.

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

Author: etr

RELEASE_NOTES.md

@@ -101,6 +101,13 @@ v1.x is end-of-life on the day v2.0 ships.
   uses `method_set`.
 - **`httpserver::constants` namespace.** `constexpr` replacements for
   every removed `#define`.
+- **Lifecycle hook bus (M5).** `ws.add_hook(hook_phase, std::function)`
+  registers observation/mutation hooks on the request/connection
+  lifecycle. The three connection-level phases — `connection_opened`,
+  `accept_decision`, `connection_closed` — fire on every connection;
+  `accept_decision` exposes `{peer, accepted, reason}` (closes #332,
+  see `examples/banned_ip_log.cpp`). The other eight phases land in
+  later M5 tasks.
 
 ## What's renamed (v1 → v2)
 

examples/Makefile.am

@@ -19,7 +19,7 @@
 LDADD = $(top_builddir)/src/libhttpserver.la
 AM_CPPFLAGS = -I$(top_srcdir)/src -I$(top_srcdir)/src/httpserver/
 METASOURCES = AUTO
-noinst_PROGRAMS = hello_world shared_state service custom_error allowing_disallowing_methods handlers hello_with_get_arg args_processing setting_headers custom_access_log minimal_https minimal_file_response minimal_deferred url_registration minimal_ip_ban benchmark_select benchmark_threads benchmark_nodelay deferred_with_accumulator file_upload file_upload_with_callback empty_response_example iovec_response_example pipe_response_example daemon_info external_event_loop turbo_mode binary_buffer_response
+noinst_PROGRAMS = hello_world shared_state service custom_error allowing_disallowing_methods handlers hello_with_get_arg args_processing setting_headers custom_access_log minimal_https minimal_file_response minimal_deferred url_registration minimal_ip_ban banned_ip_log benchmark_select benchmark_threads benchmark_nodelay deferred_with_accumulator file_upload file_upload_with_callback empty_response_example iovec_response_example pipe_response_example daemon_info external_event_loop turbo_mode binary_buffer_response
 
 hello_world_SOURCES = hello_world.cpp
 shared_state_SOURCES = shared_state.cpp
@@ -37,6 +37,7 @@ minimal_deferred_SOURCES = minimal_deferred.cpp
 deferred_with_accumulator_SOURCES = deferred_with_accumulator.cpp
 url_registration_SOURCES = url_registration.cpp
 minimal_ip_ban_SOURCES = minimal_ip_ban.cpp
+banned_ip_log_SOURCES = banned_ip_log.cpp
 benchmark_select_SOURCES = benchmark_select.cpp
 benchmark_threads_SOURCES = benchmark_threads.cpp
 benchmark_nodelay_SOURCES = benchmark_nodelay.cpp

examples/banned_ip_log.cpp

@@ -0,0 +1,56 @@
+// Demonstrates the solution to issue #332: log every banned-IP rejection.
+//
+// Configure the daemon with an ACCEPT default policy, block the IPs you
+// want denied via ws.block_ip(), and register a connection-level
+// `accept_decision` hook that logs every rejection to stderr.
+//
+// The hook is observation-only: throwing from it does not flip the
+// decision (the daemon still rejects the connection per the policy
+// callback's return), so it is safe to do I/O here.
+//
+// Run, then attempt to connect from one of the blocked IPs to see
+// stderr show a "[BANNED] ..." line per attempt.
+
+#include <httpserver.hpp>
+
+#include <functional>
+#include <iostream>
+#include <memory>
+#include <string>
+
+namespace hs = httpserver;
+
+class hello_resource : public hs::http_resource {
+ public:
+    hs::http_response render_get(const hs::http_request&) override {
+        return hs::http_response::string("Hello, World!");
+    }
+};
+
+int main() {
+    hs::webserver ws{hs::create_webserver(8080)
+                         .default_policy(hs::http::http_utils::ACCEPT)};
+
+    // Block whatever client IP you want denied. The shipping example
+    // uses an RFC-1918 placeholder; swap with your client's address.
+    ws.block_ip("10.0.0.1");
+
+    auto h = ws.add_hook(hs::hook_phase::accept_decision,
+        std::function<void(const hs::accept_ctx&)>(
+            [](const hs::accept_ctx& ctx) {
+                if (!ctx.accepted) {
+                    std::cerr << "[BANNED] peer=" << ctx.peer.to_string()
+                              << " reason="
+                              << (ctx.reason.has_value()
+                                      ? std::string(*ctx.reason)
+                                      : std::string("unspecified"))
+                              << std::endl;
+                }
+            }));
+
+    auto resource = std::make_shared<hello_resource>();
+    ws.register_path("/hello", resource);
+
+    ws.start(true);   // blocking
+    return 0;
+}

specs/tasks/M5-routing-lifecycle/TASK-046.md

@@ -8,11 +8,23 @@
 Wire the three connection-level phases into the existing MHD callback sites. Closes long-standing feature request #332 (banned-IP log entry).
 
 **Action Items:**
-- [ ] In `webserver_impl::connection_notify` (`webserver.cpp:1295`), fire `connection_opened` on `MHD_CONNECTION_NOTIFY_STARTED` and `connection_closed` on `MHD_CONNECTION_NOTIFY_CLOSED`. Context: `peer_address`, the `connection_state*` already allocated on STARTED.
-- [ ] In `webserver_impl::policy_callback` (`webserver.cpp:1450`), at the bottom (after the YES/NO decision is fixed), fire `accept_decision` with `{peer_address, bool accepted, optional<std::string_view> reason}`. `reason` is `"banned"` / `"not-allowed"` / `std::nullopt` for ACCEPT.
-- [ ] Use the per-phase `any_hooks_` short-circuit: a relaxed atomic load, branch out if zero. The body of each firing site is a single `if (impl_->any_hooks_[hook_phase::X].load(std::memory_order_relaxed)) impl_->fire_X(...);` to keep the inline code path tiny.
-- [ ] `fire_*` helpers take a `shared_lock` on `hook_table_mutex_`, copy the phase vector into a small `std::vector` on the stack (typical N is small; reserve(8) is fine), release the lock, iterate. Mirror the TASK-027 pattern for route-cache promotion.
-- [ ] Catch any exception thrown by a hook callable; route it through the existing `log_dispatch_error` helper (DR-009 §5.2). For `accept_decision` specifically, a throwing hook does NOT change the accept/reject decision — the decision was made before the hook fired.
+- [x] In `webserver_impl::connection_notify`, fire `connection_opened` on `MHD_CONNECTION_NOTIFY_STARTED` and `connection_closed` on `MHD_CONNECTION_NOTIFY_CLOSED`. Lives in the new `src/detail/webserver_callbacks_lifecycle.cpp` (split out of `webserver_callbacks.cpp` to stay under FILE_LOC_MAX). Closure pointer for `MHD_OPTION_NOTIFY_CONNECTION` switched from `nullptr` to `parent` (the owning `webserver*`) so the callback can reach `impl_->any_hooks_`.
+- [x] In `webserver_impl::policy_callback` (also in `webserver_callbacks_lifecycle.cpp`), at the bottom (after the YES/NO decision is fixed), fire `accept_decision` with `{peer_address, bool accepted, optional<std::string_view> reason}`. `reason` is `"banned"` / `"not-allowed"` / `std::nullopt` for ACCEPT. Decision derivation extracted into anon-ns helper `classify_decision` to stay under CCN.
+- [x] Use the per-phase `any_hooks_` short-circuit: a relaxed atomic load, branch out if zero. Inline pattern: `if (impl->any_hooks_[hook_phase::X].load(std::memory_order_relaxed)) impl->fire_X(ctx);` — context is constructed only inside the if-body so the zero-hook path costs one atomic load.
+- [x] `fire_*` helpers take a `shared_lock` on `hook_table_mutex_`, snapshot the phase vector into a stack-local `std::vector<phase_entry<Sig>>` with `reserve(8)`, release the lock, iterate with try/catch. Implemented as three `noexcept` members of `webserver_impl` in `src/hook_handle.cpp`. Mirrors the TASK-027 route-cache promotion pattern.
+- [x] Catch any exception thrown by a hook callable; route it through `log_dispatch_error` with a `"hook[<phase>] threw: ..."` prefix. Non-`std::exception` caught via `catch (...)`. For `accept_decision` specifically, the structural guarantee holds: `fire_accept_decision` returns void and `decision` is captured in a local before the fire call, so a throwing hook cannot reach the `return decision;` branch.
+
+**Public-header change:** `accept_ctx` extended to `{peer_address peer, bool accepted, std::optional<std::string_view> reason}` (TASK-045 had only `peer`). `reason` always references a string literal with static storage duration.
+
+**New tests (4):**
+- `test/unit/hooks_accept_ctx_shape_test.cpp` — compile-time pin for the extended shape.
+- `test/integ/hooks_connection_lifecycle.cpp` — drives a curl round-trip and asserts the three lifecycle hooks fire (accepted=true, valid peer).
+- `test/integ/hooks_accept_decision_banned.cpp` — blocks `127.0.0.1`, asserts hook observes accepted=false reason="banned" (closes #332).
+- `test/integ/hooks_accept_decision_throwing.cpp` — two sub-tests pinning that a throwing hook does not flip the decision.
+
+**New example:** `examples/banned_ip_log.cpp` — minimal program demonstrating the solution to issue #332.
+
+**Updated tests:** `test/integ/hooks_no_firing.cpp` narrowed: still asserts zero invocations on the eight phases TASK-047..051 will wire, but lets the three lifecycle phases fire (they must, per the new tests).
 
 **Dependencies:**
 - Blocked by: TASK-045
@@ -30,4 +42,4 @@ Wire the three connection-level phases into the existing MHD callback sites. Clo
 **Related Decisions:** DR-012, §4.10
 **Resolves:** Issue #332
 
-**Status:** Not Started
+**Status:** Done

src/Makefile.am

@@ -25,7 +25,7 @@ lib_LTLIBRARIES = libhttpserver.la
 # builds. The WS-off branch in websocket_handler.cpp provides stub
 # definitions (every member throws feature_unavailable except is_valid()
 # which returns false).
-libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp http_utils.cpp file_info.cpp http_request.cpp http_request_auth.cpp http_response.cpp create_webserver.cpp create_test_request.cpp websocket_handler.cpp hook_handle.cpp detail/http_endpoint.cpp detail/body.cpp detail/ip_representation.cpp detail/http_request_impl.cpp detail/http_request_impl_tls.cpp detail/webserver_setup.cpp detail/webserver_register.cpp detail/webserver_routes.cpp detail/webserver_callbacks.cpp detail/webserver_websocket.cpp detail/webserver_dispatch.cpp detail/webserver_request.cpp
+libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp http_utils.cpp file_info.cpp http_request.cpp http_request_auth.cpp http_response.cpp create_webserver.cpp create_test_request.cpp websocket_handler.cpp hook_handle.cpp detail/http_endpoint.cpp detail/body.cpp detail/ip_representation.cpp detail/http_request_impl.cpp detail/http_request_impl_tls.cpp detail/webserver_setup.cpp detail/webserver_register.cpp detail/webserver_routes.cpp detail/webserver_callbacks.cpp detail/webserver_callbacks_lifecycle.cpp detail/webserver_websocket.cpp detail/webserver_dispatch.cpp detail/webserver_request.cpp
 # noinst_HEADERS: shipped in the tarball but NEVER installed under $prefix/include.
 # Detail headers (httpserver/detail/*.hpp) live here so they cannot leak to
 # downstream consumers — the public surface comes in through <httpserver.hpp>.

src/detail/webserver_callbacks.cpp

@@ -129,34 +129,11 @@ void webserver_impl::request_completed(void *cls, struct MHD_Connection *connect
     }
 }
 
-void webserver_impl::connection_notify(void* cls, struct MHD_Connection* connection,
-                                       void** socket_context,
-                                       enum MHD_ConnectionNotificationCode toe) {
-    std::ignore = cls;
-    std::ignore = connection;
-
-    switch (toe) {
-        case MHD_CONNECTION_NOTIFY_STARTED:
-            // Allocate the per-connection state (and its embedded arena)
-            // on connection start. The new is the only heap allocation
-            // tied to a connection's lifetime; afterwards every request
-            // on this connection draws its impl out of the arena.
-            *socket_context = new detail::connection_state();
-            break;
-        case MHD_CONNECTION_NOTIFY_CLOSED:
-            // MHD ordering guarantee: NOTIFY_COMPLETED fires before
-            // NOTIFY_CLOSED for the same connection. By the time we reach
-            // this branch, request_completed has already called reset_arena()
-            // and the modded_request has already been deleted -- so the
-            // connection_state is no longer referenced by any live object.
-            // (security-reviewer-iter1-4: documents the invariant that
-            // prevents the concurrent request_completed + NOTIFY_CLOSED
-            // race described in CWE-362.)
-            delete static_cast<detail::connection_state*>(*socket_context);
-            *socket_context = nullptr;
-            break;
-    }
-}
+// connection_notify (NOTIFY_STARTED / NOTIFY_CLOSED) and policy_callback
+// live in webserver_callbacks_lifecycle.cpp (TASK-046 split). Both
+// callbacks fire lifecycle hooks and reach into <sys/socket.h> via the
+// peer-address adapter; isolating them keeps this TU under the project
+// FILE_LOC_MAX gate.
 
 #ifdef HAVE_GNUTLS
 // MHD_PskServerCredentialsCallback signature:
@@ -289,28 +266,6 @@ int webserver_impl::sni_cert_callback_func(void* cls,
 
 namespace detail {
 
-MHD_Result webserver_impl::policy_callback(void *cls, const struct sockaddr* addr, socklen_t addrlen) {
-    // Parameter needed to respect MHD interface, but not needed here.
-    std::ignore = addrlen;
-
-    const auto ws = static_cast<webserver*>(cls);
-
-    if (!ws->ban_system_enabled) return MHD_YES;
-
-    auto* impl = ws->impl_.get();
-    std::shared_lock bans_lock(impl->bans_mutex);
-    std::shared_lock allowances_lock(impl->allowances_mutex);
-    const bool is_banned = impl->bans.count(ip_representation(addr));
-    const bool is_allowed = impl->allowances.count(ip_representation(addr));
-
-    if ((ws->default_policy == http_utils::ACCEPT && is_banned && !is_allowed) ||
-        (ws->default_policy == http_utils::REJECT && (!is_allowed || is_banned))) {
-        return MHD_NO;
-    }
-
-    return MHD_YES;
-}
-
 void* webserver_impl::uri_log(void* cls, const char* uri, struct MHD_Connection *con) {
     // Parameter needed to respect MHD interface, but not needed here.
     std::ignore = cls;