test(udp-server): [#2149] add focused UDP server tests

.github/agents/task-reviewer.agent.md

@@ -38,22 +38,32 @@ pull request is opened.
    - Acceptance criteria list
    - Claimed implementation scope
 2. Inspect relevant diffs/files and run focused checks as needed.
-3. Validate each acceptance criterion explicitly as one of:
+3. When changed tests are in scope, apply the `Test Design` checklist from
+   `.github/skills/dev/task-reviews/review-task/SKILL.md` to every changed test. Report each
+   violated item as a repository-convention finding with concrete remediation; do not pass the
+   review while a test fixture is a parameter bag or hides the causal state, production Act, or
+   independently specified expected result. Require recorded evidence of the mandatory prose-first
+   Arrange-Act-Assert comparison; do not pass a review when a changed test's code has not been
+   compared against its temporary prose specification, or when redundant prose remains without an
+   irreducible-context rationale. Assess helper quality by whether it gives a coherent action,
+   capability, or state a meaningful name and aligns the caller's abstraction level; do not flag a
+   helper solely because it has a single caller.
+4. Validate each acceptance criterion explicitly as one of:
    - `PASS` - implemented and verified
    - `FAIL` - not implemented or incorrect
    - `PENDING` - partial/unclear or missing evidence
-4. If the issue spec contains checklist items, mark only verified `PASS` items as done.
-5. Review the completion-review evidence. Require an issue-local
+5. If the issue spec contains checklist items, mark only verified `PASS` items as done.
+6. Review the completion-review evidence. Require an issue-local
    `implementation-retrospective.md` when implementation revealed reusable
    lessons, material design changes, or meaningful deviations from the original
    plan. Otherwise require a concise issue progress-log entry explaining why no
    retrospective was needed.
-6. Confirm that mandatory manual scenarios were executed against the finished
+7. Confirm that mandatory manual scenarios were executed against the finished
    artifact and recorded in `manual-verification-evidence.md` with actual
    commands or interactions, observed output, relevant logs, and conclusions.
    Do not accept automated test or disposable-script output as manual evidence.
-7. Report findings with concrete remediation guidance for all `FAIL` or `PENDING` items.
-8. Return an overall status:
+8. Report findings with concrete remediation guidance for all `FAIL` or `PENDING` items.
+9. Return an overall status:
    - `REVIEW PASSED` when all required criteria pass and no blocking issues remain.
    - `REVIEW FAILED` when any required criterion fails or blocking issues remain.
 

.github/skills/dev/planning/create-issue/SKILL.md

@@ -144,16 +144,36 @@ For testing or coverage-focused issue specs, also require:
 
 - an issue-local, human-readable coverage-evidence document when coverage is measured;
 - the exact reproducible coverage command and a statement of what paths and code types it includes;
-- aggregate baseline/current values **and** per-file coverage plus prioritized uncovered functions,
-  regions, or behavior gaps; and
+- separate aggregate/global and unit-only baseline/current tables, plus per-file coverage and
+  prioritized uncovered functions, regions, or behavior gaps. Aggregate/global coverage tracks all
+  selected test levels; unit-only coverage tracks the primary package-local objective. Do not infer
+  sufficient unit coverage from aggregate, integration, example, or end-to-end results. Record
+  integration-only results separately when they inform an ownership decision; and
 - a policy to retain concise Markdown evidence rather than raw generated JSON, LCOV, or HTML
   artifacts unless the artifact itself has a documented human-review purpose.
 
-When the plan adds or changes tests, include a progressive test-development loop: make the smallest
-behavior-focused increment, review its design and focused validation before the next test-producing
-task, and stop for maintainer review after the final increment before final verification, commit, or
-pull request. Direct test authors to the `write-unit-test` skill and the test refactoring-pattern
-catalog when applicable.
+For package-testing work, require a feasible focused unit test to be assessed before accepting
+higher-level coverage as sufficient. Integration, example, root, or end-to-end coverage may retain
+a distinct contract, but must not be used to decline a package-owned unit test that is deterministic
+and readable at the unit boundary. A documented no-unit-test decision must state why the behavior
+cannot be protected appropriately by a unit test or why the higher-level boundary is demonstrably
+clearer and more maintainable.
+
+When the plan adds or changes tests, include a progressive test-development loop: use the
+`write-unit-test` skill; make the smallest behavior-focused increment; and, after it passes focused
+validation, perform and record an explicit design review before maintainer review and commit. The
+review must confirm the test exposes the one causal initial-state difference, its fixture owns only
+incidental mechanics, and the production Act plus independently specified expected result remain
+visible. Make the review enforceable with the mandatory prose-first Arrange-Act-Assert comparison:
+write temporary prose for each section, refactor until the code expresses it, remove redundant prose,
+and record the result in task evidence or a file-local plan. Complete this review for every
+test-producing subtask before starting the next one. Stop for maintainer review after the final
+increment before final verification, commit, or pull request. Direct test authors to the test
+refactoring-pattern catalog when applicable. Require the test-design review to judge helper
+boundaries by meaningful named actions and abstraction-level alignment, not caller count; a
+single-use helper is valid when it hides only incidental mechanics. Use the independent Task
+Reviewer for the final pre-PR review of the completed issue, not as a mandatory reviewer for every
+subtask.
 
 During implementation, create an ADR when an important architectural decision
 emerges, even if the issue draft did not anticipate it. Link the ADR from the

.github/skills/dev/task-reviews/review-task/SKILL.md

@@ -51,6 +51,29 @@ an issue/task is complete and ready to be pushed.
 - [ ] Docs updates are present when behavior changed.
 - [ ] New terms are added to `project-words.txt` when needed.
 
+### Test Design
+
+When the reviewed changes add or modify tests, inspect each changed test against
+`.github/skills/dev/testing/write-unit-test/SKILL.md` and report a finding for every unchecked
+item below:
+
+- [ ] The test's name states one observable behavior and relevant condition.
+- [ ] Arrange makes the causal initial-state difference visible.
+- [ ] Any builder or scenario fixture is named for that state and owns only incidental mechanics;
+      it is not a parameter bag mirroring the production call.
+- [ ] Every helper names a coherent action, capability, or state and keeps the caller at one
+      abstraction level. Do not treat a single-use helper as a defect solely because it has one
+      caller; flag it only when it is vague, hides behavior, or mixes responsibilities.
+- [ ] The production Act remains visible in the test body.
+- [ ] Expected results are independently specified and assertions remain visible.
+- [ ] The test does not duplicate a better-owned protocol, domain, integration, or end-to-end
+      contract.
+- [ ] Execution is deterministic: no uncontrolled I/O, wall-clock dependency, sleep, polling, or
+      shared mutable state is introduced.
+- [ ] The test evidence records a prose-first Arrange-Act-Assert comparison, or the reviewer
+      records why it was not applicable. The final code expresses the temporary prose; redundant
+      comments were removed and retained comments provide irreducible context.
+
 ### Spec Hygiene
 
 - [ ] Only verified checklist items are marked done.

.github/skills/dev/testing/write-unit-test/SKILL.md

@@ -73,6 +73,19 @@ Acceptable reasons to defer or avoid direct unit tests include:
 If a feature is hard to test, treat that as design feedback first and improve testability when
 practical.
 
+### Coverage Attribution Is Unit-First
+
+For package-owned behavior, treat unit-only coverage as the primary measurement and aggregate/global
+coverage as a separate broad-progress measurement. An aggregate report can include unit,
+integration, example, or end-to-end binaries; it cannot prove that a source seam has adequate unit
+protection. Record unit-only and integration-only measurements separately when coverage informs a
+test-boundary decision.
+
+Do not reject a feasible focused unit test because an integration, example, or end-to-end test
+already executes the behavior. Decline a unit test only when it cannot protect the behavior at an
+appropriate boundary, or when a higher-level contract is demonstrably clearer and more maintainable;
+record that rationale in the issue-local evidence.
+
 ### Lifecycle Fixture Design Review
 
 When a test fixture manages a child process, asynchronous I/O, network
@@ -125,6 +138,102 @@ components, or derive an expected outcome using production code under test. For
 constraints and example, see
 [Scenario fixtures for causal initial state](../../../../../docs/testing/refactoring-patterns/scenario-fixtures-for-causal-initial-state.md).
 
+### Reveal Behavioral Data; Hide Collaborator Mechanics
+
+Trace every value that crosses from Arrange into the Act or Assert. Keep a value visible in the test
+body when it selects the behavior under test, establishes a causal initial state, or independently
+specifies an expected result. Its use in the Act or Assert must make that relationship readable.
+Hide only ordinary valid collaborator-construction mechanics that do not vary the selected behavior,
+such as locks, reference-counted handles, default dependency construction, or required repository
+setup.
+
+For example, a banning-handler gauge test keeps an `unrelated_client_ip` visible when it establishes
+the pre-existing tracked-IP state, keeps the event's `cookie_error_client_ip` visible where it enters
+the event context, and keeps `expected_distinct_client_ip_total` visible before the Act and in the
+Assert. A state-named test context may hide its `Arc<RwLock<BanService>>` and `Repository` setup.
+Do not hide the relevant IPs or expected total inside that context.
+
+During review, ask: **“Can the reader follow every value that makes the Act behave differently or
+sets the expected result from its Arrange origin to its Act/Assert use?”** If not, expose that value
+or rename/refocus the scenario. Also ask: **“Does this value merely make an ordinary collaborator
+valid?”** If yes, it belongs in focused setup rather than the test narrative.
+
+### Name Coherent Actions at One Abstraction Level
+
+Use a helper when it gives a coherent sequence of setup or transport actions a meaningful name and
+keeps the caller at one readable abstraction level. A helper does **not** require multiple callers:
+`start_ephemeral_udp_tracker()` can be justified by naming one complete ordinary setup action even
+when one contract test currently uses it.
+
+Judge a helper by semantic value, not reuse count. Keep it when its name expresses a capability or
+state relevant to the test and it hides only incidental mechanics. Reject it when it merely moves
+code away behind a vague name such as `setup()`, becomes a parameter bag, hides the causal state,
+production Act, or expected result, or mixes unrelated responsibilities. See
+[Named helpers for abstraction-level alignment](../../../../../docs/testing/refactoring-patterns/named-helpers-for-abstraction-level-alignment.md)
+for selection criteria and examples.
+
+### Anti-Pattern: Duplicated Fixture-Derived Expectations
+
+Do not extract a second helper that manually reconstructs a representation already derived from a
+fixture when that representation is not independently under test. For example, a test that passes a
+`ConnectionContext` to production code should not separately hard-code every metric label expected
+from that context merely to add one causal label such as `request_kind=connect`. The fixture and
+expectation become coupled by hidden duplication: an unrelated fixture change makes the test fail
+with stale expected details.
+
+Instead, derive fixture-owned details from the exact fixture value used by the Act, and specify only
+the test's causal input or independently asserted result in the test body. In the metric example,
+create `LabelSet::from(connection_context.clone())` and visibly add `request_kind=connect`. Add a
+separate focused test when conversion of the fixture into its derived representation is itself the
+behavior under test.
+
+During prose-first review, ask: **“If this fixture changes, should this test fail?”** If no, derive
+the incidental expectation from the fixture. If yes, keep the relevant fixture value and its
+assertion visibly connected in the test prose; use a scenario or builder if several coordinated
+values establish that causal state.
+
+### Review Test-Code Smells Before Finishing
+
+Before requesting maintainer review for a test-producing increment, inspect the final test against
+these smells. A smell is a prompt to improve the design, not an automatic rule: keep the clearest
+test when an alternative would weaken its behavioral contract or diagnostic value.
+
+| Smell                           | Review question                                                                                     | Preferred response                                                                                                                                         |
+| ------------------------------- | --------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| Complex Arrange                 | Can a reader name the causal initial state without reconstructing setup plumbing?                   | Use inline values, a readable builder, or a narrowly named scenario fixture. Keep causal input visible and move only coordinated incidental mechanics.     |
+| Hidden behavioral data coupling | Can the reader trace every causal input and expected value from Arrange into its Act or Assert use? | Keep behavior-selecting inputs, causal pre-existing state, and independent expected values visible; hide only ordinary collaborator construction.          |
+| Multiple assertions             | Do the assertions specify one complete observable result or unrelated behaviors?                    | Use one higher-level semantic assertion when it preserves the full result and diagnostic clarity; otherwise split the test so each has one reason to fail. |
+| Hidden fixture coupling         | Would an unrelated fixture change fail this test?                                                   | Derive incidental expectations from the exact fixture used by the Act; keep independently specified causal values visible.                                 |
+| Hidden production Act           | Can the reader identify the production behavior under test directly?                                | Keep the production invocation visible; do not move it into setup or assertion helpers.                                                                    |
+| Production-derived expectation  | Is the expected result calculated by code that the test is meant to verify?                         | Construct the expected result independently; move only mechanical comparisons into a semantic assertion helper.                                            |
+
+Record material refactoring decisions from this review in the file-local plan or task evidence.
+
+### Verify Intent with Prose-First AAA
+
+Before considering any new or materially refactored test ready for maintainer review, make its
+intent explicit and verify that the final code communicates it. This is mandatory for every
+test-producing increment:
+
+1. Write temporary normal-prose **Arrange**, **Act**, and **Assert** paragraphs above the test.
+   State the causal initial state, the production action, and independently specified observable
+   result; do not describe implementation mechanics without explaining their behavioral purpose.
+2. Repeat each paragraph above the corresponding `// Arrange`, `// Act`, or `// Assert` code
+   section.
+3. Compare the code with each paragraph. Refactor names, setup, builders, scenario fixtures, the
+   visible Act, or assertions until the code itself expresses the paragraph.
+4. Remove prose that is redundant once the code communicates the intent. Retain only essential
+   context that cannot be expressed clearly in code without disproportionate complexity or a
+   misleading abstraction.
+5. Record the completed prose-first comparison in the task evidence or file-local test plan before
+   maintainer review and commit.
+
+The temporary prose is the test's specification, not permanent commentary. A parameter bag, an
+opaque fixture, a hidden Act, or an assertion derived through production code is evidence that the
+code has not yet expressed its specification. See
+[Prose-first Arrange-Act-Assert verification](../../../../../docs/testing/refactoring-patterns/prose-first-arrange-act-assert-verification.md)
+for a repository example.
+
 ## Phase 1: Basic Unit Test
 
 ### Naming Convention
@@ -305,6 +414,7 @@ establishes a reusable pattern for future tests.
 
 - [ ] Test name uses `it_should_` prefix
 - [ ] Test follows AAA pattern with comments (`// Arrange`, `// Act`, `// Assert`)
+- [ ] Temporary prose-first AAA specification was compared with the code; redundant prose was removed
 - [ ] No `std::time::SystemTime::now()` in production code — use the `CurrentClock` type alias instead
 - [ ] No shared mutable state between tests
 - [ ] Behaviour coverage is maximized with maintainable tests

.vscode/settings.json

@@ -2,6 +2,12 @@
   "[rust]": {
     "editor.formatOnSave": true
   },
+  "[markdown]": {
+    // Editor Markdown formatters conflict with the markdownlint rules enforced
+    // by `linter all` (for example table padding and hard-wrap style), so
+    // Markdown is formatted only by the lint gate, not on save.
+    "editor.formatOnSave": false
+  },
   "[ignore]": {
     "rust-analyzer.cargo.extraEnv": {
       "RUSTFLAGS": "-Z profile -C codegen-units=1 -C inline-threshold=0 -C link-dead-code -C overflow-checks=off -C panic=abort -Z panic_abort_tests",