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

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

@@ -38,18 +38,28 @@ 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. Report findings with concrete remediation guidance for all `FAIL` or `PENDING` items.
-7. Return an overall status:
+7. Report findings with concrete remediation guidance for all `FAIL` or `PENDING` items.
+8. 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

@@ -142,16 +142,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

@@ -65,6 +65,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
@@ -117,6 +130,65 @@ 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).
 
+### 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.
+
+### 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
@@ -297,6 +369,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,9 @@
   "[rust]": {
     "editor.formatOnSave": true
   },
+  "[markdown]": {
+    "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",

docs/copilot-pr-reviews/pr-2174-copilot-suggestions.md

@@ -0,0 +1,48 @@
+---
+semantic-links:
+  skill-links:
+    - process-copilot-suggestions
+  related-artifacts:
+    - .github/skills/dev/pr-reviews/process-copilot-suggestions/SKILL.md
+    - docs/issues/open/2149-1347-add-focused-udp-server-package-tests/performance-evidence.md
+    - docs/issues/open/2149-1347-add-focused-udp-server-package-tests/test-refactor-plans/README.md
+    - packages/udp-server/src/server/request_buffer.rs
+---
+
+<!-- cspell:disable -->
+
+<!-- skill-link: process-copilot-suggestions -->
+
+# PR #2174 Copilot Suggestions Tracking
+
+Source: Copilot PR review threads for https://github.com/torrust/torrust-tracker/pull/2174
+
+Status legend:
+
+- `action`: code/docs change applied
+- `no-action`: suggestion reviewed; no code change needed
+- `resolved`: thread resolved in PR
+
+## Processing Log
+
+- 2026-09-09: Started processing six Copilot suggestions after rebasing the draft PR.
+- 2026-09-09: Completed all six suggestions. Three received focused action commits, and three
+  were resolved as already addressed or intentionally declined with a documented rationale.
+
+## Suggestions
+
+| # | Thread ID | Path | URL | Suggestion Summary | Decision | Reply URL | Status | Thread State |
+| --- | --- | --- | --- | --- | --- | --- | --- | --- |
+| 1 | `PRRT_kwDOGp2yqc6goHu9` | `packages/udp-server/src/server/request_buffer.rs` | [thread](https://github.com/torrust/torrust-tracker/pull/2174#discussion_r3967777870) | Bound task-completion waits so a cleanup regression cannot hang CI. | action: added one-second absolute cleanup bound in `1ef8589b`. | [reply](https://github.com/torrust/torrust-tracker/pull/2174#discussion_r3968614576) | DONE | RESOLVED |
+| 2 | `PRRT_kwDOGp2yqc6goHvj` | `docs/issues/open/2149-1347-add-focused-udp-server-package-tests/performance-evidence.md` | [thread](https://github.com/torrust/torrust-tracker/pull/2174#discussion_r3967777918) | Update stale request-buffer implementation status. | no-action: duplicate suggestion addressed in `01cde544`. | [reply](https://github.com/torrust/torrust-tracker/pull/2174#discussion_r3968226077) | DONE | RESOLVED |
+| 3 | `PRRT_kwDOGp2yqc6goHv9` | `docs/issues/open/2149-1347-add-focused-udp-server-package-tests/performance-evidence.md` | [thread](https://github.com/torrust/torrust-tracker/pull/2174#discussion_r3967777950) | Update stale request-buffer implementation status. | action: corrected test-only completion/deferred benchmark status in `01cde544`. | [reply](https://github.com/torrust/torrust-tracker/pull/2174#discussion_r3968216396) | DONE | RESOLVED |
+| 4 | `PRRT_kwDOGp2yqc6goHwW` | `docs/issues/open/2149-1347-add-focused-udp-server-package-tests/test-refactor-plans/README.md` | [thread](https://github.com/torrust/torrust-tracker/pull/2174#discussion_r3967777992) | Mark completed handler-dispatch plan as complete. | no-action: already addressed in `9c05e359`. | [reply](https://github.com/torrust/torrust-tracker/pull/2174#discussion_r3968220747) | DONE | RESOLVED |
+| 5 | `PRRT_kwDOGp2yqc6goHwn` | `packages/udp-server/src/server/request_buffer.rs` | [thread](https://github.com/torrust/torrust-tracker/pull/2174#discussion_r3967778018) | Avoid direct ring-buffer mutation in tests where public behavior can express setup. | no-action: `force_push` is the test Act; direct insertion remains controlled Arrange mechanics. | [reply](https://github.com/torrust/torrust-tracker/pull/2174#discussion_r3968617981) | DONE | RESOLVED |
+| 6 | `PRRT_kwDOGp2yqc6goHwz` | `packages/udp-server/src/server/request_buffer.rs` | [thread](https://github.com/torrust/torrust-tracker/pull/2174#discussion_r3967778042) | Avoid hard-coded active-request capacity in test setup. | action: derive the retained count from actual buffer capacity in `1ef8589b`. | [reply](https://github.com/torrust/torrust-tracker/pull/2174#discussion_r3968621167) | DONE | RESOLVED |
+
+## Notes
+
+- Process every thread individually: reply before resolving it.
+- Record the action/no-action decision, rationale, and reply URL in this audit log.
+- R6 in `test-refactor-plans/request-buffer-tests.md` records the approved rationale and prose-first
+  design review for request-buffer suggestions 1, 5, and 6.