Repository navigation
chore(issues): archive closed issue #2450 spec to docs/issues/closed - #2664
Conversation
…closed Moves the SI-24 spec to docs/issues/closed/ after PR torrust#2652 merged and torrust#2450 closed, marks roadmap row 17 of EPIC torrust#1488 Done, updates the two stale status statements there, and repairs every live link to the old docs/issues/open/ path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tainer instructed Ticks AC1 to AC8, the definition-of-done items and the three review checkpoints, fills each acceptance row with its evidence pointer, and appends a progress-log entry that corrects the earlier statement that they stay unticked. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The documentation-only changes consistently reflect the completed implementation and issue closure, with no unresolved blocking findings.
No open issues 🎉
What changed in this PR
Archives issue #2450’s health-check API lifecycle specification after implementation merged in PR #2652. This documentation-only change brings the issue records and related shutdown roadmap up to date.
Changes:
- Moves the specification and supporting records to
docs/issues/closed/. - Records completion of tasks, acceptance criteria, and review checkpoints with evidence references.
- Updates parent roadmap status and repairs live links while preserving historical records.
| File | Description |
|---|---|
docs/issues/open/2482-1669-reorganize-shared-test-support/EPIC.md |
Updates the dependency link and records #2450 as closed. |
docs/issues/open/1488-overhaul-tracker-shutdown/ISSUE.md |
Marks SI-24 complete and refreshes lifecycle status descriptions. |
docs/issues/drafts/2482-gate-axum-health-check-api-server-test-environment/ISSUE.md |
Points metadata and dependency links to the archived spec. |
docs/issues/closed/2450-1488-si-24-migrate-health-check-api-test-environment/manual-verification-evidence.md |
Updates the evidence record’s issue-spec path. |
docs/issues/closed/2450-1488-si-24-migrate-health-check-api-test-environment/ISSUE.md |
Records archival, completed checkpoints, and acceptance evidence. |
docs/issues/closed/2450-1488-si-24-migrate-health-check-api-test-environment/implementation-retrospective.md |
Preserves implementation lessons in the archived folder. |
docs/issues/closed/2450-1488-si-24-migrate-health-check-api-test-environment/agent-review-reports.md |
Preserves review history alongside the archived spec. |
docs/issues/closed/2448-1488-si-17-migrate-standalone-udp-environment/ISSUE.md |
Repairs metadata and links referencing SI-24. |
Review effort: Balanced
|
ACK 9986378 |
da2ce7
left a comment
da2ce7
left a comment
There was a problem hiding this comment.
Reviewed at 998637855cc03db9fdb28e94bb0f8774b51ca7ab (post-merge). Recomputed from the bytes at the merged head, against the base 72d0bca6a; the PR merged on 2026-10-26 at 22:01:38Z as 71e130242.
Label: POST_MERGE_FINDINGS — Review requested at 2026-10-26T21:57:43Z; the PR merged before this round ran. Findings below stand at the merged head for a follow-up; no ACK follows a merged PR.
Archive: the four issue-local files move together; status, primary path, evidence path and timestamps change, and the parent roadmap and test-support dependency links follow the move. Runtime bytes are unchanged from the implementation delivered by PR 2652. Its fixed-port restart and registration-preservation tests support AC4/AC8; the acceptance table names those actual tests. The new AC5 evidence overstates the visibility change: the state and fields are private, but the typestate remains public.
Records: the second commit appends the acceptance-tick correction without rewriting the first archive event. The capture supports the PR merge and review-request chronology. Issue closure, task-reviewer execution and reported test results are not independently established by this capture. Completion ticks do not substitute for those executions. A live audit frontmatter reference still names the removed open path; historical command quotations are correctly retained.
Hygiene: recomputed two commits, eight files, +48/-48. Subjects use Conventional scopes and identify the issue; author and committer identities match. Attribution footers occur with no applicable prohibition. Per-commit whitespace checks are clean. No closing-keyword match occurs in the title, body or commit messages. Changed Markdown relative file links resolve; the separate semantic-reference search finds F1.
Bot and group: the review bot left a summary without inline findings. Its broad assertion that live links were repaired is incomplete. This PR depends on PR 2652; PR 2669 later plans the separate shutdown-policy work and PR 2686 performs the analogous archive for that work.
Findings: F1 Minor and F2 Nit below.
Checked, no finding: complete folder move; primary and evidence metadata; parent roadmap update; preserved historical review records; fixed-port cleanup evidence; no runtime changes.
Checks
On the loop's compute hub at the merged head, against the base 72d0bca6a (receipt server-gates-sojka-471; the PR merged as 71e130242): pre-commit profile gate exit 0 (126 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. GitHub checks at this head at posting time: "Docs Lint: completed/success". At posting, develop is 968099d93.
| - 2026-10-26 12:42 UTC - GitHub Copilot - Copilot's review of #2652 (`review-finding:pr-2652-f1`) found that a fixed-port stop-then-start panics, because the token-aware start registers the service and the `Registar` cannot remove it. This corrects the 07:38 entry: the discovery is material, so `implementation-retrospective.md` is added. The maintainer chose to document and pin the limitation and file a follow-up; recorded in V7, with V6 re-run after the new last production change. The acceptance criteria stay TODO. | ||
| - 2026-10-26 16:24 UTC - GitHub Copilot - Review round on #2652 (`review-finding:pr-2652-f1`, `-f2`, `-f3`). This reverses the 12:42 UTC decision to document the fixed-port limitation: da2ce7 re-raised the finding as a Major (`-f2`) and asked for the fix, and measuring the cost showed a small additive registry method was enough. The maintainer chose to fix it in this PR (route R1) with `Registar::deregister`. Opened torrust/torrust-server-lib#2 at 16:04 UTC (CI green, merged 16:19 UTC), published `torrust-server-lib` 0.4.0, bumped it here, and made `stop()` deregister its own binding (D6, AC8), also when a task failed, with a green fixed-port restart test replacing the `should_panic` pin. The earlier independent review entry is corrected in `agent-review-reports.md` (`-f3`). V6 and V7 and the retrospective are rewritten to the final state. A new independent review on the final tree and the audit follow. The acceptance criteria stay TODO. | ||
| - 2026-10-26 16:31 UTC - GitHub Copilot - A new independent Task Reviewer ran on the final tree and returned REVIEW FAILED for one documentation blocker (V5 lacked the prose-first comparison of the three tests added in this round) and six low findings, with no production defect; recorded in `agent-review-reports.md`. Fixed: V5 now covers the eleven environment tests, the production call-site count is corrected from eight to seven, V4 is labelled as measured at T1, `related-pr` is set, and the dropped-`stop()` risk is stated. The acceptance criteria stay TODO. | ||
| - 2026-10-26 19:50 UTC - GitHub Copilot - PR #2652 merged on 2026-10-26 at 19:34 UTC by the maintainer, and #2450 is closed. The maintainer merged without waiting for the re-requested review of da2ce7, whose earlier findings F2 and F3 were fixed and their threads resolved. Archived this spec to `docs/issues/closed/`: `status: done`, `spec-path` and `issue-spec` updated, T4 and T5 marked DONE, the roadmap row 17 of EPIC #1488 marked Done, and every live link to the old `docs/issues/open/` path repaired. The acceptance criteria and the checkpoints for AC review, reviewer validation and committer verification stay unticked: the maintainer has not told the author to tick them. |
There was a problem hiding this comment.
[Minor][F1] Repair the remaining live audit reference to the moved spec
The new archive entry says every live link was repaired, but docs/pr-reviews/pr-2451-review/PR-REVIEW.md:9 still lists docs/issues/open/2450-1488-si-24-migrate-health-check-api-test-environment/ISSUE.md under semantic-links.related-artifacts. That target exists at the base and is removed here. This is current navigational metadata, not a dated command quotation, so the archive skill's historical-record exception does not apply. Update that entry to the closed path while preserving historical prose. The omitted audit file is outside this diff; this comment is anchored to the new claim that its reference was repaired.
| | AC2 | DONE | Tests `it_should_stop_without_a_failure_when_the_server_and_its_drain_controller_finish_through_cancellation` and `it_should_release_the_health_check_api_binding_when_stopped`; mutation M1 (evidence V2). | | ||
| | AC3 | DONE | Join and error tests in `environment::tests` (V1, V2); signature unchanged; `contract.rs` callers unchanged. | | ||
| | AC4 | DONE | `it_should_serve_requests_after_being_stopped_and_started_again` (port 0) and `it_should_serve_requests_after_being_stopped_and_started_again_on_a_fixed_port` (evidence V7). | | ||
| | AC5 | DONE | No `state.binding` or `env.state` read in `tests/`; `state` and `Running` are private; callers use `bind_address()` (evidence V0 and V1). | |
There was a problem hiding this comment.
[Nit][F2] Describe the actual visibility boundary in the acceptance evidence
The new evidence says Running is private, but packages/axum-health-check-api-server/src/environment.rs:36 declares pub struct Running. Its fields and Environment::state are private, which is sufficient for AC5 and the accessor migration. Correct the evidence to describe those fields rather than claim that the public typestate itself became private; no runtime change is needed.
Related to #2450
Archives the spec of #2450 (SI-24, closed on GitHub when PR #2652 merged) from
docs/issues/open/todocs/issues/closed/.status: done,spec-pathand the evidence file'sissue-spec.Commits, oldest first:
chore(issues): archive closed issue #2450 spec to docs/issues/closeddocs(issues): [#2450] tick the acceptance criteria as the maintainer instructedVerification:
linter allexits 0 and the local link check passes. This pull request changes only Markdown, so CI should run only Docs Lint.