[josecelano]

Closes #2450

Migrates the health-check API test Environment to the token lifecycle (SI-24, EPIC #1488 roadmap row 17), following the #2449 pattern.

What the change does: private state, Stopped and Running structs, one cancellation token held as one DropGuard, and a stop() that disarms and cancels before its first await, joins both tasks and returns an error naming every failing task. A new bind_address() accessor replaces the 8 field reads in the contract tests.

Review round: Copilot and da2ce7 found that a stop-then-start on a fixed port panicked with DuplicateBinding, because the token-aware start registers the service and Registar could not remove it. This PR now fixes it: stop() removes the environment's own registration with Registar::deregister, which is new in torrust-server-lib 0.4.0 (torrust/torrust-server-lib#2, merged and published). The decision to first document the limitation was reversed; see the evidence file, section V7. The audit is docs/pr-reviews/pr-2652-review/PR-REVIEW.md.

Evidence and reviews: manual-verification-evidence.md (V0 to V7, mutation proofs), agent-review-reports.md (an earlier review entry, its correction, and a review of the final tree) and implementation-retrospective.md. Acceptance criteria AC1 to AC8 are left for the maintainer to tick.

Head to merge: the current head of this branch, docs(pr-reviews): [#2450] record the review audit of PR 2652. Any later push will be announced in a comment on this pull request.

Commits, oldest first:

• refactor(axum-health-check-api-server): [#2450] migrate the health-check API test environment to the token lifecycle
• docs(issues): [#2450] record T0 baseline, T1 mutation proofs and M1/M2 evidence
• docs(shutdown-process): [#2450] record the health-check test environment token lifecycle
• docs(issues): [#2450] record the prose-first comparison and the post-change checks
• test(axum-health-check-api-server): [#2450] assert the stop result in the binding-release test
• docs(issues): [#2450] record the independent review, resolve its findings and update the spec status
• test(axum-health-check-api-server): [#2450] document and pin the fixed-port restart limitation
• docs(issues): [#2450] record the fixed-port restart limitation and its retrospective
• docs(issues): [#2450] correct the independent review entry after the fixed-port finding
• chore(deps): [#2450] bump torrust-server-lib to 0.4.0
• fix(axum-health-check-api-server): [#2450] deregister the health-check test environment on stop
• docs(issues): [#2450] record the registry deregistration and update the spec to the final design
• docs(issues): [#2450] record the prose-first comparison of the new tests and fix the review findings
• docs(issues): [#2450] record the independent review of the final tree
• docs(pr-reviews): [#2450] record the review audit of PR 2652

Files touched:

• Cargo.lock
• Cargo.toml
• docs/features/shutdown-process/task-inventory.md
• docs/issues/open/2450-1488-si-24-migrate-health-check-api-test-environment/ISSUE.md
• docs/issues/open/2450-1488-si-24-migrate-health-check-api-test-environment/agent-review-reports.md
• docs/issues/open/2450-1488-si-24-migrate-health-check-api-test-environment/implementation-retrospective.md
• docs/issues/open/2450-1488-si-24-migrate-health-check-api-test-environment/manual-verification-evidence.md
• docs/pr-reviews/pr-2652-review/PR-REVIEW.md
• packages/axum-health-check-api-server/Cargo.toml
• packages/axum-health-check-api-server/src/environment.rs
• packages/axum-health-check-api-server/src/server.rs
• packages/axum-health-check-api-server/tests/server/contract.rs
• packages/axum-http-server/Cargo.toml
• packages/axum-rest-api-server/Cargo.toml
• packages/axum-server/Cargo.toml
• packages/udp-server/Cargo.toml

[Copilot]

🟡 Changes recommended

Fixed-port restarts retain the previous service registration, causing duplicate registration and a startup panic.

1 open issue

• Prevent duplicate health binding on fixed-port restart · New

What changed in this PR

Migrates the health-check API test environment to the cancellation-token lifecycle for #2450, advancing the tracker shutdown overhaul.

Changes:

• Replaces legacy halt signaling with token cancellation and joins both owned tasks.
• Encapsulates lifecycle state, adds bind_address(), and updates contract-test callers.
• Adds eight lifecycle tests and records verification, review evidence, and inventory updates.

File	Description
packages/​axum-health-check-api-server/​tests/​server/​contract.rs	Replaces eight direct binding reads with the accessor.
packages/​axum-health-check-api-server/​src/​environment.rs	Implements token-based lifecycle management and tests.
docs/​issues/​open/​2450-1488-si-24-migrate-health-check-api-test-environment/​manual-verification-evidence.md	Records test results, mutation checks, and verification evidence.
docs/​issues/​open/​2450-1488-si-24-migrate-health-check-api-test-environment/​ISSUE.md	Updates implementation progress and verification status.
docs/​issues/​open/​2450-1488-si-24-migrate-health-check-api-test-environment/​agent-review-reports.md	Records the independent review and its resolutions.
docs/​features/​shutdown-process/​task-inventory.md	Documents the migrated environment’s task ownership and shutdown.

_{Review effort: Balanced}

[da2ce7]

Reviewed at f0656bc1d2c2cb289b9756f12315e1d84e8dabe3. Recomputed from the bytes at this head, against the merge base 5124c66e2; git merge-tree against develop 4b81147e9 exited 0 with one result tree and no conflicts.

Against the spec: cancellation, joined errors, encapsulation, metadata, accessor migration and the inventory are implemented. AC4 is not satisfied for a fixed nonzero port. T4 and all AC checkboxes remain open; the recorded design-checkpoint ordering exception is explicit.

The new environment cancels before awaiting and joins both tasks on a completed stop. Drop and abandoned stop request cancellation but detach the handles. Unlike the REST environment, stop retains its registry, including the health-check registration. Socket release does not remove that entry.

The tests: all eight new tests bound their waits. The restart case uses port 0; its mutations can fail during the initial stop, so they do not establish fixed-port restart. The binding-release test now requires a successful stop. Mutation dependencies were inspected; historical executions and timings were not independently reproduced.

The records: inventory changes match the lifecycle. The independent-review entry asserts all criteria are met but omits the template's inputs, evidence, explicit verdict field and follow-up actions. Its blanket same-commit resolution claim conflicts with the earlier test correction. Acceptance remains for the maintainer.

Hygiene: six files, +487/-86; all six subjects have suitable Conventional Commit scopes and the issue tag, identities match, and all per-commit whitespace checks pass. Attribution footers are present; no tree rule found forbids them. The prescribed branch-id check prints nothing. The description's closing-pattern scan matches only issue 2450. Relative targets resolve; dictionary comparison found no clear new spelling defect.

The bot: agree with its open fixed-port finding. The absent author-owned audit record is an observed fact, not a finding before the thread is processed.

Findings: F2 Major (restart, re-raise); F3 Minor (misleading and incomplete review record). REQUEST_CHANGES.

Checked, no finding: one token/one guard, both error branches and task joins, accessor call sites, bounded new-test waits, changed links, commit scope and whitespace, clean textual merge. Build and lint execution belongs to the orchestrator; none is claimed here.

Checks

On the loop's compute hub at this head, against the merge base 5124c66e2, the #2647 merge (receipt server-gates-sojka-466; develop has since moved to 4b81147e9, the #2649 merge): pre-commit profile gate exit 0 (85 s); linter markdown, linter cspell, linter lychee exit 0; frontmatter-validator --all: 16 errors / 4 warnings at this head and 16 / 4 at the base re-measured, the same 20 diagnostics by path and category, none in the files this PR touches. cargo test -p torrust-tracker-axum-health-check-api-server: 11 lib tests and 8 integration tests pass at this head against 3 lib tests at the base, measured in a separate target directory; cargo test -p torrust-tracker-axum-rest-api-server: 14 lib and 58 integration tests pass at both. The 8 environment tests pass in each of three consecutive runs (about 1.0 s each). cargo clippy over both packages with all targets and features and -D warnings: exit 0. On the merge of this head into develop 4b81147e9 (tree bc88539ae), both packages' tests pass with --locked (91 tests). GitHub checks at this head at posting time: "Publish (Development): completed/skipped; Publish (Release): completed/skipped; Context: completed/success; Package Coverage Regression: completed/success; Package Coverage (torrust-tracker-axum-health-check-api-server): completed/success; Docker E2E: completed/skipped; Generate Coverage Report: completed/success; Discover Package Coverage: completed/success; Unit (stable): completed/success; Documented Clippy Allows: completed/success; Unit (nightly): completed/success; Workspace Packaging: completed/success; Build on ubuntu-latest (stable): completed/success; Build on ubuntu-latest (nightly): completed/success; Build on windows-latest (nightly): completed/success; Test (Docker) (release): completed/success; Build on macos-latest (nightly): completed/success; Layer Boundary Bans: completed/success; Build on macos-latest (stable): completed/success; Build on windows-latest (stable): completed/success; Docs Lint: completed/success". At posting, develop is 4b81147e9.

[josecelano]

Review round 1 is processed and pushed in two pushes (the second only adds the audit). Head to merge: docs(pr-reviews): [#2450] record the review audit of PR 2652. The earlier heads, including the one da2ce7 reviewed, are superseded; please do not merge them.

Findings: F1 (Copilot, Major) and F2 (da2ce7, Major, re-raise of F1) are fixed by fix(axum-health-check-api-server): [#2450] deregister the health-check test environment on stop, with Registar::deregister from torrust-server-lib 0.4.0 (torrust/torrust-server-lib#2, merged and published). F3 (da2ce7, Minor) is fixed by docs(issues): [#2450] correct the independent review entry after the fixed-port finding. All three threads have a reply and are resolved, and validate-audit-record.py --pr-number 2652 --base torrust/develop exits 0 with 3 rows.

Two things for the maintainer: the acceptance criteria AC1 to AC8 stay unticked until you say so, and a Task Reviewer review of the final tree is recorded in agent-review-reports.md (it failed on one documentation point, fixed in docs(issues): [#2450] record the prose-first comparison of the new tests and fix the review findings).

Dismissed by the maintainer. F2 and F3 were fixed and their threads resolved in head 6c0d9cd (torrust-server-lib 0.4.0 Registar::deregister, stop() deregisters its own binding, review record corrected). CI is green. Not waiting for the re-review.

[josecelano]

ACK 6c0d9cd

[da2ce7]

Reviewed at 6c0d9cd80a294e5d64e5675fc4a62820b35887e5 (post-merge). Recomputed from the bytes at the merged head, against the base 5124c66e2; the PR merged on 2026-10-26 at 19:34:27Z as 72d0bca6a.

Label: POST_MERGE_FINDINGS — Review requested at 2026-10-26T17:32:46Z; the PR merged before this round ran. Findings below stand at the merged head for a follow-up; no ACK follows a merged PR.

The change: compared the earlier reviewed tree first. Commit cb8c34fd7 fixes F1/F2: environment.rs:126-129 joins both tasks, deregisters the exact binding retained by server.rs:184, then propagates failure. Other owners' registrations survive. The guard cancels on ordinary drop; drop or cancellation of the stop future cannot promise registry cleanup, as the spec now states.

Tests: the nonzero-port restart at environment.rs:278 would fail without deregistration; the preservation and failed-task cases at lines 310 and 431 catch clearing the registry or moving cleanup after ?. The ephemeral restart alone would not establish that contract. The eleven environment tests cover cancellation, binding release, restart, joined failure aggregation and cleanup; the eight contract tests change only their address accessor. Runtime and mutation results recorded by the PR were not rerun in this read-only lane.

Records and bot: agree with the review bot's F1 at the earlier tree; the merged head addresses it and F2 together. Commit 70b2141e5 answers F3 with an appended correction, preserving the earlier entry and distinguishing its unsupported completion conclusion from the later review. The audit retains three rows, F2 as RE_RAISE_OF:F1, all FIXED/RESOLVED; all three reply IDs belong to the correct source threads and both resolution subjects identify actual fixes. Captured threads are resolved. The log's reply times agree with the capture and precede the audit commit. Execution counts and external dependency publication remain reported evidence, not independently reproduced facts.

Hygiene: recomputed 15 commits, 16 files, +1065/-100. Subjects are Conventional with scope and issue tags, author and committer identities match, and per-commit whitespace checks are clean. Attribution footers occur; no applicable prohibition was found. The body contains the intended closing reference to issue 2450. Changed Markdown relative file links resolve. The dependency version changes consistently to 0.4.0; the library implementation is outside this snapshot.

Group relationship: the archive in PR 2664 moves this specification and supporting records; PR 2678 later adapts this environment to the drain-duration API using the shared defaults while retaining its registry cleanup.

Findings: none new.

Checked, no finding: fixed-port restart; caller-owned registry entries; cleanup before failed-task return; F3 correction; audit IDs, relationships, reply targets and resolution references. Acceptance ticks remain explicitly reserved for the maintainer.

Checks

On the loop's compute hub at the merged head, against the base 5124c66e2 (receipt server-gates-sojka-468; the PR merged as 72d0bca6a): pre-commit profile gate exit 0 (153 s); linter markdown, linter cspell, linter lychee exit 0; frontmatter-validator --all: 16 errors / 4 warnings at the merged head and 16 / 4 at the base re-measured, the same 20 diagnostics by path and category; the audit validator over docs/pr-reviews/pr-2652-review/PR-REVIEW.md with the review comments and commit list captured at gate time: ok, 3 rows, 7 log entries, 0 failures; cargo test at the merged head: torrust-tracker-axum-health-check-api-server 22 passed, 0 failed (exit 0, 56 s; 11 at the base); torrust-tracker-axum-http-server 110 passed, 0 failed (exit 0, 24 s; 110 at the base); torrust-tracker-axum-rest-api-server 72 passed, 0 failed (exit 0, 19 s; 72 at the base); torrust-tracker-axum-server 6 passed, 0 failed (exit 0, 21 s; 6 at the base); torrust-tracker-udp-server 252 passed, 0 failed, 1 ignored (exit 0, 36 s; 252 at the base); cargo clippy over torrust-tracker-axum-health-check-api-server torrust-tracker-axum-http-server torrust-tracker-axum-rest-api-server torrust-tracker-axum-server torrust-tracker-udp-server with all targets and features and -D warnings: exit 0, 0 diagnostics. GitHub checks at this head at posting time: "Publish (Development): completed/skipped; Publish (Release): completed/skipped; Context: completed/success; Package Coverage Regression: completed/success; Docker E2E: completed/skipped; Documented Clippy Allows: completed/success; Workspace Packaging: completed/success; Layer Boundary Bans: completed/success; Unit (stable): completed/success; Unit (nightly): completed/success; Test (Docker) (release): completed/success; Package Coverage (torrust-tracker-axum-server): completed/success; Package Coverage (torrust-tracker-axum-rest-api-server): completed/success; Package Coverage (torrust-tracker-axum-health-check-api-server): completed/success; Package Coverage (torrust-tracker-udp-server): completed/success; Package Coverage (torrust-tracker-axum-http-server): completed/success; Generate Coverage Report: completed/success; Discover Package Coverage: completed/success; Build on ubuntu-latest (stable): completed/success; Build on windows-latest (stable): completed/success; Build on macos-latest (stable): completed/success; Build on windows-latest (nightly): completed/success; Build on macos-latest (nightly): completed/success; Build on ubuntu-latest (nightly): completed/success; Docs Lint: completed/success". At posting, develop is 968099d93.