unworked validation sweep: tick remaining 337 minor items across 33 files

Every remaining unchecked finding across the older sweep files
(task-003 through task-044, plus manual-validation, plus the
task-016/017/018/019 iter0 reports) already carries an explicit
disposition annotation in its body — Status: wontfix / deferred /
fixed-in-batch, Resolution: ..., or an explicit scope-deferral note
referencing a downstream task that has since been completed.

This commit ticks all 337 checkboxes so the backlog state finally
mirrors the actual disposition embedded in each item. Banners added
to the task-016/018/019 iter0 files explain that those tasks are now
Done and most observations have been superseded by subsequent work.

Verification:
  $ for f in specs/unworked_review_issues/*.md; do
      grep -c '^[0-9]\+\. \[ \]' "$f"
    done | sort -u
  # Output: 0

Net result of this multi-commit sweep (across all five commits):
- 7 unworked-review files closed end-to-end with actual code changes
  (task-053..057, plus task-055 carry-over).
- The remaining ~60 files now show no unchecked findings; every item
  has a recorded disposition.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: etr

specs/unworked_review_issues/2026-05-01_152911_task-003.md

@@ -73,7 +73,7 @@
     *Recommendation:* Replace `const std::runtime_error* base = &e; msg = base->what();` with `msg = e.what();`. The static_assert above already verifies the inheritance relationship, so the explicit upcast here is redundant.
     *Status:* fixed in this batch — the entire test was replaced (see item 1); the `base` pointer variable is gone.
 
-14. [ ] **housekeeper** | `/Users/etr/progs/libhttpserver/specs/tasks/_index.md:88` | task-not-marked-complete
+14. [x] **housekeeper** | `/Users/etr/progs/libhttpserver/specs/tasks/_index.md:88` | task-not-marked-complete
     TASK-003 status in _index.md still shows 'In Progress' rather than 'Done', though the prompt notes the merge step will update this.
     *Recommendation:* Confirm the merge step updates the _index.md status to 'Done' for TASK-003 after validation passes.
     *Status:* deferred — handled by the main-branch merge step per the original instruction.
@@ -88,7 +88,7 @@
     *Recommendation:* For call sites that always pass string literals (feature names and build-flag macros), accepting const char* is both safe and cheaper. If string_view is preferred for generality, add a brief doc comment stating both arguments must remain valid for the duration of the constructor call, and consider a clang-tidy lifetime-profile annotation or a deleted rvalue-ref overload to prevent accidental temporaries.
     *Status:* fixed in this batch — added Doxygen `@param` doc comments stating both views must remain valid for the duration of the constructor call.
 
-17. [ ] **spec-alignment-checker** | `/Users/etr/progs/libhttpserver/.worktrees/TASK-003/src/httpserver.hpp:30` | specification-gap
+17. [x] **spec-alignment-checker** | `/Users/etr/progs/libhttpserver/.worktrees/TASK-003/src/httpserver.hpp:30` | specification-gap
     httpserver.hpp still gates `basic_auth_fail_response.hpp` behind `#ifdef HAVE_BAUTH` and `websocket_handler.hpp` behind `#ifdef HAVE_WEBSOCKET`. TASK-003 action item 4 says to 'apply the gate from TASK-002', which means the TASK-002 include-guard pattern should be in place. The file correctly includes `feature_unavailable.hpp` unconditionally, but the residual HAVE_* guards in this umbrella header are a pre-existing condition not introduced by this task. This is a minor note rather than a defect introduced by TASK-003.
     *Recommendation:* This is a pre-existing condition outside the scope of TASK-003. A future task (per PRD-FLG-REQ-001) should remove the remaining HAVE_* guards from the public umbrella header. No change required to approve TASK-003.
     *Status:* deferred — reviewer explicitly says no change required to approve TASK-003; pre-existing condition for a future task.

specs/unworked_review_issues/2026-05-02_230828_task-005.md

@@ -58,7 +58,7 @@
     *Recommendation:* Either add a static_assert that count_ < 32 (not <= 32, since bit 32 of a uint32_t is UB), or add an explicit mask/clamp in method_bit. The existing static_assert at line 237 uses <= 32 which technically allows count_ == 32 (UB territory). Tightening it to < 32 would remove the ambiguity.
     *Status:* fixed in this batch — static_assert tightened to `< 32`; method_bit comment updated to accurately describe the invariant.
 
-11. [ ] **code-quality-reviewer** | `test/unit/http_method_test.cpp:154` | test-coverage
+11. [x] **code-quality-reviewer** | `test/unit/http_method_test.cpp:154` | test-coverage
     The clean-code principle 'one assert per test' is violated throughout the test suite. Most LT_BEGIN_AUTO_TEST blocks contain multiple LT_CHECK calls testing distinct behaviors (e.g., test 5 checks get present, post present, and put absent in a single test). This makes it harder to identify exactly which assertion failed on a test failure.
     *Recommendation:* Split multi-assertion tests into individual focused tests, each with a name that describes the single behavior under test. For example, split 'bitwise_or_two_enumerators_yields_set_with_both' into 'bitwise_or_includes_first_operand', 'bitwise_or_includes_second_operand', and 'bitwise_or_excludes_third_method'. This is a low-priority trade-off against test verbosity so keep it in mind for future expansion.
     *Status:* deferred — the framework makes per-assertion test splitting very verbose; the reviewer's own note calls it "low-priority trade-off against test verbosity". Acceptable at current granularity.
@@ -68,7 +68,7 @@
     *Recommendation:* Remove the empty set_up() and tear_down() bodies if the test framework allows omitting them. If the framework requires them, add a brief comment explaining they are intentionally empty (no per-test state).
     *Status:* fixed in this batch — the littletest CRTP base requires these methods; added comment explaining they are required by the framework even when empty.
 
-13. [ ] **code-quality-reviewer** | `test/unit/http_method_test.cpp:154` | test-coverage
+13. [x] **code-quality-reviewer** | `test/unit/http_method_test.cpp:154` | test-coverage
     Several runtime tests contain multiple independent assertions (e.g. test 5 checks contains(get), contains(post), and !contains(put); test 10 chains three compound-assignment operations with five distinct checks). The clean-code Tests rule recommends one assert per test to keep failure messages pinpointed.
     *Recommendation:* Split multi-assertion tests into focused single-behavior tests, e.g. separate 'bitwise_or_includes_left_operand', 'bitwise_or_includes_right_operand', and 'bitwise_or_excludes_absent_method'. This also improves the granularity of failure messages from LT_CHECK.
     *Status:* deferred — same rationale as item 11; low-priority verbosity trade-off. Duplicate recommendation.
@@ -83,7 +83,7 @@
     *Recommendation:* Remove the duplicate static_assert from the test file; the production header's assert fires in every TU that includes httpserver.hpp and is sufficient. Retain only the test-file-specific asserts (underlying type pin, bits field type, contiguity of count_) that add coverage beyond what the header already checks.
     *Status:* partially addressed — the assertion is documented as intentional in the AC block with a comment; bound tightened to `< 32` to match header.
 
-16. [ ] **code-simplifier** | `src/httpserver/http_method.hpp:116` | naming
+16. [x] **code-simplifier** | `src/httpserver/http_method.hpp:116` | naming
     The switch arms in to_string construct std::string_view via its explicit single-argument constructor (e.g. std::string_view{"GET"}) rather than the more idiomatic string literal suffix ("GET"sv) available since C++17, which is the minimum required standard for this library. The explicit constructor form is correct and readable, but the sv suffix is the established C++17 idiom and would be slightly more concise and consistent with modern C++17 style.
     *Recommendation:* Optionally replace `std::string_view{"GET"}` with `"GET"sv` (and similarly for each arm) after adding `using namespace std::string_view_literals;` or `using std::literals::string_view_literals::operator""sv;` at the top of the function or file. This is purely a style preference and should only be applied if it matches the style used elsewhere in the codebase.
     *Status:* wontfix — cosmetic/style preference, no functional impact; purely stylistic; array literal form acceptable, low priority
@@ -123,17 +123,17 @@
     *Recommendation:* Replace the switch with a static constexpr std::array<std::string_view, static_cast<std::size_t>(http_method::count_)> keyed by static_cast<std::uint8_t>(m), with a bounds check returning {} for out-of-range inputs.
     *Status:* already addressed — to_string already uses a constexpr array implementation.
 
-24. [ ] **performance-reviewer** | `src/httpserver/http_method.hpp:62` | missing-caching
+24. [x] **performance-reviewer** | `src/httpserver/http_method.hpp:62` | missing-caching
     method_bit(m) involves a runtime shift whose shift amount is the uint8_t cast of m. For the 9 current methods the shift amount is 0–8, all within a single-byte range that most CPUs can compute in one instruction. However, if this function is called at a non-constexpr runtime site (e.g. looking up a parsed method from a network request) the shift is free but the cast chain (enum -> uint8_t -> uint32_t -> shift) adds two widening moves on some ABIs. Marking the caller sites that already have the integer value directly (e.g. after a parse step that produces uint8_t) to pass the method enum avoids double-conversion and is already satisfied by the current design — this is just a note that the design is correct.
     *Recommendation:* No change needed; the current design is already optimal for the stated hot path.
     *Status:* deferred — reviewer explicitly says no change needed; noted for completeness.
 
-25. [ ] **performance-reviewer** | `src/httpserver/http_method.hpp:67` | algorithmic-complexity
+25. [x] **performance-reviewer** | `src/httpserver/http_method.hpp:67` | algorithmic-complexity
     valid_method_mask() recomputes (1 << count_) - 1 on every call. Because count_ is a compile-time constant the compiler will constant-fold this, but the function is called in every complement and set_all operation. Marking the result consteval or caching it as a constexpr variable at namespace scope would make the intent explicit and remove any residual risk of non-constant evaluation in debug builds.
     *Recommendation:* Add a constexpr constant: inline constexpr std::uint32_t k_valid_method_mask = detail::valid_method_mask(); and replace all call sites.
     *Status:* deferred — the function is constexpr and constant-folded by the compiler; a micro-optimization that changes the public API surface. Defer to a later cleanup pass.
 
-26. [ ] **security-reviewer** | `src/httpserver/http_method.hpp:116` | insecure-design
+26. [x] **security-reviewer** | `src/httpserver/http_method.hpp:116` | insecure-design
     to_string() is the only direction provided (enum -> wire token). There is no from_string() or validate() primitive, so downstream parsing code will inevitably write ad-hoc string comparisons or unsafe static_casts from integer indices to produce an http_method value.
     *Recommendation:* Consider adding a constexpr std::optional<http_method> from_string(std::string_view) noexcept in this header as a companion to to_string().
     *Status:* deferred — from_string() is out of scope for TASK-005 (not in the task spec). Tracked for a future task.
@@ -143,27 +143,27 @@
     *Recommendation:* Add a guard inside method_bit() itself or change the static_assert to count_ < 32 (strictly less).
     *Status:* fixed in this batch — static_assert tightened to `< 32` and method_bit comment updated. Same fix as items 9/10.
 
-28. [ ] **security-reviewer** | `src/httpserver/http_method.hpp:79` | insecure-design
+28. [x] **security-reviewer** | `src/httpserver/http_method.hpp:79` | insecure-design
     The method_set::bits field is public and mutable with no validation, allowing callers to directly inject arbitrary bitmask values.
     *Recommendation:* Consider making bits private and providing a named constructor or factory (e.g. static constexpr method_set from_bits(uint32_t) noexcept that masks with valid_method_mask()).
     *Status:* deferred — aggregate design is intentional (standard layout, trivially copyable); making bits private changes the public API. Design decision requiring a broader discussion.
 
-29. [ ] **spec-alignment-checker** | `/Users/etr/progs/libhttpserver/.worktrees/TASK-005/src/httpserver/http_method.hpp:null` | specification-gap
+29. [x] **spec-alignment-checker** | `/Users/etr/progs/libhttpserver/.worktrees/TASK-005/src/httpserver/http_method.hpp:null` | specification-gap
     PRD-HDL-REQ-006 requires 'webserver::route(http_method, path, handler)' as a registration entry point. TASK-005 only delivers the http_method enum and method_set bitmask primitive.
     *Recommendation:* No action required for TASK-005. Downstream tasks (TASK-021 et al.) must implement webserver::route().
     *Status:* deferred — no action required per reviewer.
 
-30. [ ] **test-quality-reviewer** | `test/unit/http_method_test.cpp:163` | redundant-test
+30. [x] **test-quality-reviewer** | `test/unit/http_method_test.cpp:163` | redundant-test
     Runtime test `set_then_contains_runtime` (test 1) directly mirrors the AC #1 static_assert at line 35-37.
     *Recommendation:* Remove the runtime test and rely on the static_assert, or broaden the runtime test to cover a scenario the static_assert cannot.
     *Status:* deferred — the runtime test documents intent in the LT suite output and catches dynamic-only runtime failures. Minor redundancy acceptable.
 
-31. [ ] **test-quality-reviewer** | `test/unit/http_method_test.cpp:254` | multiple-concerns
+31. [x] **test-quality-reviewer** | `test/unit/http_method_test.cpp:254` | multiple-concerns
     Test `compound_assign_or_equals_with_enumerator` chains |=, &=, and ^= in sequence within one test body. If the &= step silently misbehaves, the ^= assertion may mask it, and the name only mentions `|=`.
     *Recommendation:* Split into three tests: one for |=, one for &=, one for ^=.
     *Status:* deferred — framework verbosity makes splitting burdensome; the combined test is a regression guard. A new test (14) explicitly tests method_set RHS overloads; this test covers the http_method RHS overloads.
 
-32. [ ] **test-quality-reviewer** | `test/unit/http_method_test.cpp:272` | naming-convention
+32. [x] **test-quality-reviewer** | `test/unit/http_method_test.cpp:272` | naming-convention
     Test `to_string_returns_uppercase_wire_tokens` contains 9 assertions for 9 different methods in a single test body.
     *Recommendation:* This is acceptable as-is given the framework's lack of parametrize support.
     *Status:* wontfix — cosmetic/style preference, no functional impact; reviewer says acceptable as-is; already addressed in TASK-027