Skip to content

refactor: [#2801] remove the legacy shutdown API and library OS signals - #2807

Merged
josecelano merged 23 commits into
torrust:developfrom
josecelano-bot:2801-remove-legacy-shutdown-api
Nov 11, 2026
Merged

josecelano merged 23 commits into
torrust:developfrom
josecelano-bot:2801-remove-legacy-shutdown-api

Conversation

@josecelano-bot

@josecelano-bot josecelano-bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Implements #2801 (SI-19, roadmap row 19 of EPIC #1488): the breaking removal of what SI-18 (#2751) deprecated. Closes #2801.

Summary

  • Removes the Halted-based shutdown API: axum-server's graceful_shutdown; the legacy start, start_with_health_check, stop, Running states, aliases (RunningHttpServer, RunningApiServer, RunningUdpServer), Running::new, halt_task, rx_halt and RunningServer of the HTTP, REST API, health-check API and UDP servers; the legacy UDP launcher adapter (Launcher::run_with_graceful_shutdown, Spawner::spawn_launcher, LaunchRequest); and the root HaltSignal and NestedServerTask.
  • Removes the last library-level OS-signal subscription (global_shutdown_signal in the UDP launcher). Only executables (src/main.rs, src/console/profiling.rs, the standalone examples) and the tracker_client check monitor subscribe to signals.
  • Bumps torrust-server-lib to 0.6.0 (published; removal in Remove the legacy shutdown API deprecated in 0.5.0 and release 0.6.0 torrust-server-lib#5 and PR feat(signals)!: remove legacy shutdown API and release 0.6.0 torrust-server-lib#6) in every Cargo.toml, and drops the dependency from axum-server, which no longer uses it.
  • Removes the tests that only covered removed paths. Each package keeps the token-aware tests for the same behavior; the token-path drain test of the HTTP server asserts that the server keeps its bind address.
  • Updates the migration guide, the task inventory and the shutdown-process README, and records the verification in manual-verification-evidence.md.
  • Out of scope, left in place: the now-unused public Error::Stop, StartupNotification and StartupNotificationDropped variants of the HTTP and REST servers (not deprecated), and the supervisor consolidation (a separate follow-up).

Verification

  • cargo check --workspace --all-targets: warning-free. cargo test --workspace before the review rounds: 3947 passed, 0 failed. The review rounds since then delete one duplicate HTTP test and add one owner-abort test (manager); the package tests of the six changed packages passed again at the second round head (245 passed; the run recorded in review 5845706883).
  • A search for the removed symbols in .rs and .toml files finds nothing.
  • SIGINT and SIGTERM sent to the direct PID of the tracker and of the four standalone examples: each exits 0 in about 0.1 s with one shutdown sequence and no listener left.
  • Container: the image builds with its 1778 in-image tests passing (a first build failed once on a flaky health-check API test that passes on rerun, recorded in the evidence); docker stop -t 35 and SIGINT exit 0 in about 0.2 s.
  • linter all: markdown, cspell, links, YAML, TOML, rustfmt and shellcheck passed on the last run.

The evidence is in docs/issues/open/2801-1488-si-19-remove-legacy-shutdown-api/manual-verification-evidence.md.

🤖 Generated with Claude Code

josecelano-bot and others added 5 commits November 10, 2026 15:37
…n API

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…own API

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ump torrust-server-lib to 0.6.0

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…cy shutdown API

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…legacy shutdown API removal

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

No open issues 🎉

What changed in this PR

Removes the deprecated legacy Halted-based shutdown APIs and the last library-level OS-signal subscription, completing SI-19 / #2801 and standardizing shutdown on injected CancellationTokens.

Changes:

  • Deleted legacy shutdown types/APIs (HaltSignal, NestedServerTask, legacy start/stop paths, legacy UDP launcher adapter, and axum-server’s legacy graceful_shutdown).
  • Removed library-level OS-signal handling (notably the UDP launcher global_shutdown_signal path), leaving signals to executables/CLI tools.
  • Bumped torrust-server-lib to 0.6.0 across the workspace and updated docs/evidence to reflect verification.
File Description
src/​bootstrap/​jobs/​manager.rs Removes legacy HaltSignal / NestedServerTask code and associated tests.
packages/​udp-server/​src/​server/​states.rs Removes legacy Running/start/stop state machine; keeps token-aware lifecycle.
packages/​udp-server/​src/​server/​spawner.rs Removes legacy LaunchRequest and deprecated launcher spawning adapter.
packages/​udp-server/​src/​server/​mod.rs Drops tests that only exercised removed legacy start/stop paths.
packages/​udp-server/​src/​server/​launcher.rs Removes legacy graceful-shutdown adapter and global_shutdown_signal usage; deletes legacy-only tests.
packages/​udp-server/​Cargo.toml Bumps torrust-server-lib to 0.6.0.
packages/​axum-server/​src/​signals.rs Removes deprecated graceful_shutdown that depended on Halted; keeps cancellation-based shutdown.
packages/​axum-server/​Cargo.toml Drops torrust-server-lib dependency (no longer needed after legacy shutdown removal).
packages/​axum-rest-api-server/​src/​server.rs Removes legacy Halted-based start/stop and running state; uses cancellation-based shutdown.
packages/​axum-rest-api-server/​Cargo.toml Bumps torrust-server-lib to 0.6.0.
packages/​axum-http-server/​src/​server.rs Removes legacy start/stop APIs; updates tests to validate behavior via cancellation path.
packages/​axum-http-server/​Cargo.toml Bumps torrust-server-lib to 0.6.0.
packages/​axum-health-check-api-server/​src/​server.rs Removes deprecated start/RunningServer legacy API and tests; keeps token-aware runtime.
packages/​axum-health-check-api-server/​Cargo.toml Bumps torrust-server-lib to 0.6.0.
docs/​issues/​open/​2801-1488-si-19-remove-legacy-shutdown-api/​manual-verification-evidence.md Records completed verification evidence (searches, tests, signal behavior, container stop).
docs/​issues/​open/​2801-1488-si-19-remove-legacy-shutdown-api/​ISSUE.md Marks tasks/acceptance criteria as DONE and logs implementation progress.
docs/​features/​shutdown-process/​task-inventory.md Updates shutdown-process inventory to reflect SI-19 removals.
docs/​features/​shutdown-process/​legacy-api-migration.md Updates migration guide to reflect removed symbols and torrust-server-lib 0.6.0 removal.
docs/​features/​shutdown-process/​README.md Updates wording to reflect legacy symbols are removed (not merely deprecated).
Cargo.toml Bumps workspace torrust-server-lib to 0.6.0.

Review effort: Lite

da2ce7
da2ce7 previously approved these changes Nov 10, 2026

@da2ce7 da2ce7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at d8165422d2d4883321ae6cc83cfa75631ff9b7fd. Recomputed from the bytes at this head, against the merge base f0d0ea09b; git merge-tree against develop c2b5d96ef writes a tree with no conflicts, and the nine files develop changed since the merge base do not overlap the 21 this PR changes.

Process: the governing skills are open-pull-request (title, body, issue link, commit-id rule), commit-changes (Conventional subjects) and create-issue (the spec and evidence files edited here). The title is Conventional with [#2801], and Closes #2801 is the only closing keyword, correct for an implementation PR.

The change against its claim: all 27 shutdown #[deprecated] attributes present at the merge base are removed and none remain; no .rs or .toml file names a removed symbol; every import left behind is still used; torrust-server-lib is 0.6.0 in the five manifests that keep it and is dropped from axum-server, which no longer uses it; the lock agrees. Only executables and the client check monitor subscribe to signals. The removed tests exercised removed paths only. The spec and evidence overstate some results (F1-F4, F9, F10), and some rustdoc and one test lag the removal (F5-F7).

Consistency: every link, anchor and backticked path in the five changed Markdown files resolves; the spec frontmatter meets the strict issue profile; the dictionary is unchanged; nothing is moved or renamed.

Hygiene: five Conventional subjects with [#2801]; author equals committer on each; no commit has a body; each ends with one attribution footer that credits an AI model, which no rule in the tree forbids (the provenance discussion records no outcome); git diff --check is clean; the skill's PR-branch commit-id check prints nothing.

Findings: F1-F7 Minor, F8-F10 Nit, F11 Suggestion; none blocks, because each is repairable after merge and the merged code is correct.

Checked, no finding: the evidence's first two searches reproduce at this head; the EPIC rows it cites are Done; docs/containers.md#stopping-the-tracker gives 30 s minimum, so the recorded 35 s stop timeout fits; no released tag contains HaltSignal or NestedServerTask; the remaining start_with_health_check hits are test-environment methods on the token path; futures is still used where its import was dropped.

Checks

On the loop's compute node at this head, against the merge base f0d0ea09b (receipt server-gates-kranich-608): the repository's pre-commit gate exit 0 (80 s), clippy-allow-reasons against the base exit 0, and the tree left clean; linter markdown, linter cspell, linter lychee (offline links) exit 0; frontmatter-validator --all showed no new error against the base. The build resolves torrust-server-lib 0.6.0. Package tests at this head: cargo test -p torrust-tracker -p torrust-tracker-axum-health-check-api-server -p torrust-tracker-axum-http-server -p torrust-tracker-axum-rest-api-server -p torrust-tracker-axum-server -p torrust-tracker-udp-server exit 0 (68 s, 245 tests passed, none failed). Confirmed at this head: no shutdown #[deprecated] remains, no Rust or manifest file names Halted, HaltSignal, NestedServerTask, shutdown_signal_with_message or global_shutdown_signal, and only UdpError::Bind and UdpError::Registration are still constructed (F6). 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; Package Coverage (torrust-tracker-axum-http-server): completed/success; Package Coverage (torrust-tracker-udp-server): completed/success; Package Coverage (torrust-tracker-axum-server): completed/success; Package Coverage (wait-for-pr-review): completed/success; Package Coverage (torrust-tracker): completed/success; Package Coverage (torrust-tracker-axum-rest-api-server): completed/success; Docs Lint: completed/success; Build on ubuntu-latest (stable): completed/success; Build on windows-latest (nightly): completed/success; Build on macos-latest (nightly): completed/cancelled; Build on ubuntu-latest (nightly): completed/success; Build on windows-latest (stable): completed/success; Build on macos-latest (stable): completed/cancelled; Test (Docker) (release): completed/success; Discover Package Coverage: completed/success; Generate Coverage Report: completed/success; Docker E2E: completed/skipped; Layer Boundary Bans: completed/success; Unit (stable): completed/success; Documented Clippy Allows: completed/success; Unit (nightly): completed/success; Workspace Packaging: completed/success; Shipped Features: completed/success". At posting, develop is c2b5d96ef.

Comment thread packages/axum-http-server/src/server.rs Resolved
Comment thread Cargo.lock Resolved
@da2ce7

da2ce7 commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

ACK d816542 — all 27 shutdown deprecations are removed with no remaining Rust or manifest reference, consistent imports, dependencies and lock, and a conflict-free merge with develop

josecelano-bot and others added 3 commits November 10, 2026 16:14
…he docs left by the removal

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
da2ce7
da2ce7 previously approved these changes Nov 10, 2026

@da2ce7 da2ce7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at f742998c29d69694ab2a169e6e08a4541dcf5169. Recomputed from the bytes at this head, against the previous round's head d8165422d and the merge base f0d0ea09b; git merge-tree against develop c2b5d96ef writes a tree with no conflicts, and the nine files develop changed since the merge base do not overlap the 22 this PR changes.

What moved: three commits fast-forward d8165422d (11 files, +378/-82). Every other path is byte-identical, so earlier conclusions on those paths stand.

Previous findings:

  • F1 fixed: nothing imports torrust_server_lib::signals.
  • F2 fixed: row 19 is left to the archive step, as row 18 was.
  • F3 fixed: the row is IN_PROGRESS, and the approval is traced to the merged start gate.
  • F4 fixed: searches 1 to 3 reproduce, and the script is inlined.
  • F5 fixed: the drain test still asserts bind_to.
  • F6, F7 fixed: the rustdoc matches the code.
  • F9 fixed.
  • F10 partly fixed (re-raised).
  • F11 fixed: the new test checks that both children are aborted.
  • F8 stays as accepted.

The new commits: the code commit's claims hold. The audit passes the validator's checks offline, and the commit-id check prints nothing. The new evidence block is not raw output (F12), and two Solutions overstate (F14).

Hygiene: Conventional subjects, and author equals committer on all three. Two commits end with an attribution footer and one has none; no rule in the tree requires or forbids one. git diff --check flags one trailing space (F12).

Findings: F12 Minor; F10 re-raise, F13 to F15 Nit; F16 Suggestion. None blocks.

Checked, no finding: the frontmatter validator does refuse a staged legacy-shape draft. The guide's new link resolves. The unbuilt UdpError variants were built only in the removed start and stop. The stamps match the forge times.

Checks

On the loop's compute node at this head, against the merge base f0d0ea09b (receipt server-gates-kranich-611): the repository's pre-commit gate exit 0 (70 s), clippy-allow-reasons against the base exit 0, and the tree left clean; linter markdown, linter cspell, linter lychee (offline links) exit 0; frontmatter-validator --all showed no new error against the base; the repository's audit validator on pr-2807-review exit 0 (11 rows, 3 log entries, no failures). Package tests at this head: cargo test -p torrust-tracker -p torrust-tracker-axum-health-check-api-server -p torrust-tracker-axum-http-server -p torrust-tracker-axum-rest-api-server -p torrust-tracker-axum-server -p torrust-tracker-udp-server exit 0 (67 s, 245 passed, none failed; the round removes the duplicate test of F5 and adds the owner-abort test of F11, so the count is unchanged). GitHub checks at this head at posting time: "Package Coverage Regression: completed/success; Package Coverage (torrust-tracker-axum-http-server): completed/success; Package Coverage (torrust-tracker): completed/success; Package Coverage (torrust-tracker-axum-health-check-api-server): completed/success; Package Coverage (wait-for-pr-review): completed/success; Package Coverage (torrust-tracker-axum-rest-api-server): completed/success; Package Coverage (torrust-tracker-udp-server): completed/success; Package Coverage (torrust-tracker-axum-server): completed/success; Build on windows-latest (nightly): completed/success; Build on windows-latest (stable): completed/success; Build on macos-latest (nightly): in_progress/pending; Build on macos-latest (stable): completed/success; Build on ubuntu-latest (nightly): completed/success; Build on ubuntu-latest (stable): completed/success; Docker E2E: completed/skipped; Test (Docker) (release): in_progress/pending; Unit (stable): completed/success; Workspace Packaging: completed/success; Documented Clippy Allows: completed/success; Unit (nightly): in_progress/pending; Shipped Features: completed/success; Layer Boundary Bans: completed/success; Discover Package Coverage: completed/success; Generate Coverage Report: completed/success; Docs Lint: completed/success". At posting, develop is c2b5d96ef.

@da2ce7

da2ce7 commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

ACK f742998 — the previous round's fixes, the new owner-abort test and the corrected rustdoc hold at this head, the audit record passes the validator's checks, and the merge into develop is conflict-free

josecelano-bot and others added 2 commits November 10, 2026 16:46
…full output

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ations

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@josecelano

Copy link
Copy Markdown
Member

ACK f742998

josecelano-bot and others added 5 commits November 10, 2026 16:51
…emoved API

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ion branch

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… spec

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… the 0.6.0 publication

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ecord the second round

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@josecelano-bot

josecelano-bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor Author

Consolidated response for review rounds 4 and 5 of da2ce7. Round 4 (review 5846656529): F20 (audit F21) FIXED, resolved by this response: the PR description was edited, its Summary says the token-path drain test of the HTTP server asserts that the server keeps its bind address, and its Verification says the 3947-test workspace run was made before the review rounds and what changed since. F21 (audit F22) NO_ACTION, as for F15: no change, and the branch is not rewritten; resolved by the reply #2807 (comment). Round 5 (review 5846951116): F22 (audit F23) FIXED, resolved by this response, which now calls the first review above "Round 4" (it opens "Consolidated response for review rounds 4 and 5 of da2ce7"). F23 (audit F24) FIXED, resolved by this response: the PR description names the source of the 245-test run, the run recorded in review 5845706883.

…e audit

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@josecelano

Copy link
Copy Markdown
Member

ACk 703e342

da2ce7
da2ce7 previously approved these changes Nov 10, 2026

@da2ce7 da2ce7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 703e34251149277d992b7edb63530e6d3a6731bb. Recomputed from the bytes at this head, against the previous round's head be0fd0dbf and the merge base f0d0ea09b; git merge-tree against develop 4a46f98b2 writes a tree with no conflicts, and the 15 files develop changed since the merge base do not overlap the 22 this PR changes.

What moved: one docs-only commit, 703e342, fast-forwards be0fd0dbf (the audit only, +31/-1). Every other path is byte-identical, so earlier conclusions on those paths stand.

Previous findings:

  • F20 fixed: the description now says the token-path drain test keeps the bind address (server.rs:681). Its Verification says the 3947-test run came before the review rounds, which deleted one test and added one; the bytes agree.
  • F21 left as is by decision, and recorded as the audit's F22 the way F15 was. Not blocking.

The new commit: the review, the two replies, the response and both thread states match the forge record. Mapping F20 and F21 to F21 and F22 follows the skill's collision rule. Both log stamps take their events' minutes and are no later than the commit. The audit passes the validator's checks offline (22 rows, 9 log entries), and the commit-id check prints nothing.

Hygiene: Conventional subject that matches the diff, author equals committer, no body, and one attribution footer, which no rule in the tree requires or forbids. git diff --check is clean.

Findings: F22 and F23, both Nit. None blocks. Both IDs collide with the audit's, so the skill's collision rule gives them the next audit-local IDs.

Checked, no finding: the description's six packages match the PR's six changed manifests, and no .rs or .toml file changed after the round-2 head. The F22 reply has the exact form the skill prescribes. The Test 1 searches and the five greps the audit names still print what their rows say.

Checks

On the loop's compute node at this head, against the merge base f0d0ea09b (receipt server-gates-kranich-628): the repository's pre-commit gate exit 0, clippy-allow-reasons against the base exit 0, and the tree left clean; linter markdown, linter cspell, linter lychee (offline links) exit 0; frontmatter-validator --all showed no new error against the base; the repository's audit validator on pr-2807-review exit 0 (22 rows, 9 log entries, no failures). This round changes only the audit record, so the package tests at f742998c2 (245 passed) still describe the code. GitHub checks at this head at posting time: "Package Coverage (torrust-tracker-axum-health-check-api-server): queued/pending; Package Coverage (wait-for-pr-review): queued/pending; Package Coverage (torrust-tracker-axum-server): queued/pending; Package Coverage (torrust-tracker): queued/pending; Package Coverage (torrust-tracker-axum-http-server): queued/pending; Package Coverage (torrust-tracker-axum-rest-api-server): queued/pending; Package Coverage (torrust-tracker-udp-server): queued/pending; Package Coverage (torrust-tracker-axum-server): queued/pending; Package Coverage (wait-for-pr-review): queued/pending; Package Coverage (torrust-tracker-axum-http-server): queued/pending; Package Coverage (torrust-tracker-udp-server): queued/pending; Package Coverage (torrust-tracker-axum-health-check-api-server): queued/pending; Package Coverage (torrust-tracker): queued/pending; Package Coverage (torrust-tracker-axum-rest-api-server): queued/pending; Test (Docker) (release): queued/pending; Publish (Development): completed/cancelled; Publish (Release): completed/cancelled; Context: completed/cancelled; Docker E2E: completed/skipped; Generate Coverage Report: in_progress/pending; Discover Package Coverage: completed/success; Docs Lint: queued/pending; Docs Lint: queued/pending; Unit (nightly): queued/pending; Workspace Packaging: queued/pending; Documented Clippy Allows: completed/success; Shipped Features: completed/success; Unit (stable): in_progress/pending; Layer Boundary Bans: completed/success; Generate Coverage Report: in_progress/pending; Discover Package Coverage: completed/success; Build on windows-latest (nightly): in_progress/pending; Build on windows-latest (stable): completed/success; Build on ubuntu-latest (stable): queued/pending; Build on ubuntu-latest (nightly): queued/pending; Build on macos-latest (nightly): completed/success; Build on ubuntu-latest (nightly): queued/pending; Build on macos-latest (nightly): queued/pending; Build on windows-latest (stable): completed/success; Build on macos-latest (stable): in_progress/pending; Build on windows-latest (nightly): completed/success; Build on ubuntu-latest (stable): in_progress/pending; Build on macos-latest (stable): completed/success; Test (Docker) (release): completed/cancelled; Docker E2E: completed/skipped; Documented Clippy Allows: completed/cancelled; Unit (stable): completed/cancelled; Shipped Features: completed/cancelled; Unit (nightly): completed/cancelled; Layer Boundary Bans: completed/cancelled; Workspace Packaging: completed/cancelled". At posting, develop is 4a46f98b2.

@da2ce7

da2ce7 commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

ACK 703e342 — the round-4 audit rows, replies and log match the forge record, the audit record passes the validator's checks, every other path is byte-identical to the previous head, and the merge into develop is conflict-free

… audit

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
da2ce7
da2ce7 previously approved these changes Nov 10, 2026

@da2ce7 da2ce7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 3b4361208195c357c82f37948b213e2353f9adce. Recomputed from the bytes at this head, against the previous round's head 703e34251 and the merge base f0d0ea09b; git merge-tree against develop 4a46f98b2 writes a tree with no conflicts, and the 15 files develop changed since the merge base do not overlap the 22 this PR changes.

What moved: one docs-only commit, 3b43612, fast-forwards 703e34251 (the audit only, +31/-1). Every other path is byte-identical, so earlier conclusions on those paths stand.

Previous findings:

  • F22 fixed: the conversation response now opens "Fourth review round" (edited 18:11:23).
  • F23 fixed: the description's Verification names review 5845706883 as the source of the 245-test run, and that review records it at the round-2 head.

The new commit: the review, the two replies, the edited response and both thread states match the forge record. Mapping F22 and F23 to F23 and F24 follows the skill's collision rule. Both log stamps take their events' minutes and are no later than the commit. The audit passes the validator's checks offline (24 rows, 11 log entries), and the commit-id check prints nothing.

Hygiene: Conventional subject that matches the diff, author equals committer, no body, and one attribution footer, which no rule in the tree requires or forbids. git diff --check is clean.

Findings: F24, Nit. It does not block. Its ID collides with the audit's F24, so the skill's collision rule gives it the next audit-local ID.

Checked, no finding: "Six reviews" counts the summary review and the five rounds. Every new URL is on the thread or comment it names. The Test 1 searches and the five greps the audit names still print what their rows say.

Checks

On the loop's compute node at this head, against the merge base f0d0ea09b (receipt server-gates-kranich-634): the repository's pre-commit gate exit 0 (59 s), clippy-allow-reasons against the base exit 0, and the tree left clean; linter markdown, linter cspell, linter lychee (offline links) exit 0; frontmatter-validator --all showed no new error against the base; the repository's audit validator on pr-2807-review exit 0 (24 rows, 11 log entries, no failures). This round changes only the audit record, so the package tests at f742998c2 (245 passed) still describe the code. GitHub checks at this head at posting time: "". At posting, develop is 4a46f98b2.

@da2ce7

da2ce7 commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

ACK 3b43612 — the round-5 audit rows, replies and log match the forge record and pass the validator's checks, every other path is byte-identical to the previous head, and the merge into develop is conflict-free

@josecelano

Copy link
Copy Markdown
Member

ACK 3b43612

… audit

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
da2ce7
da2ce7 previously approved these changes Nov 10, 2026

@da2ce7 da2ce7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 5a15d702694b3fce876794dddc844d738158f644. Recomputed from the bytes at this head, against the previous round's head 3b4361208 and the merge base f0d0ea09b; git merge-tree against develop 4a46f98b2 writes a tree with no conflicts, and the 15 files develop changed since the merge base do not overlap the 22 this PR changes.

What moved: one docs-only commit, 5a15d70, fast-forwards 3b4361208 (the audit only, +18/-2). Every other path is byte-identical, so earlier conclusions on those paths stand.

Previous findings:

  • F24 fixed: the response (edited 19:18:01) gives each of the four findings its disposition and resolution reference and cites the reply discussion_r4528458638, and the F22 row now carries the response URL.

The new commit: the round-6 review, the reply, the edited response and the thread state match the forge record. "Seven reviews" counts the summary review and the six rounds, and mapping F24 to F25 follows the skill's collision rule. Both log stamps take their events' minutes and are no later than the commit. Five reviews had a FIXED finding before this round, so the round limit applied before the sixth push of fixes, as the log says. The audit passes the validator's checks offline (25 rows, 13 log entries), and the commit-id check prints nothing.

Hygiene: Conventional subject that matches the diff, author equals committer, no body, and one attribution footer, which no rule in the tree requires or forbids. git diff --check is clean.

Findings: F25, Nit. It does not block. Its ID collides with the audit's F25, so the skill's collision rule gives it the next audit-local ID.

Checked, no finding: every new URL is on the thread or comment it names. The Test 1 searches and the five greps the audit names still print what their rows say. The Findings paragraph's "asked by the maintainer" and the log's "the maintainer was asked" read in opposite directions; the decision has no forge record, and no rule asks for one.

Checks

On the loop's compute node at this head, against the merge base f0d0ea09b (receipt server-gates-kranich-642): the repository's pre-commit gate exit 0 (50 s), clippy-allow-reasons against the base exit 0, and the tree left clean; the repository's audit validator on pr-2807-review exit 0 (25 rows, 13 log entries, no failures). Code is byte-identical to the previous round's head. GitHub checks at this head at posting time: "Test (Docker) (release): queued/pending; Docker E2E: completed/skipped; Unit (stable): queued/pending; Workspace Packaging: queued/pending; Layer Boundary Bans: queued/pending; Shipped Features: queued/pending; Unit (nightly): queued/pending; Documented Clippy Allows: completed/success; Docs Lint: completed/success; Generate Coverage Report: queued/pending; Build on ubuntu-latest (nightly): queued/pending; Discover Package Coverage: queued/pending; Build on windows-latest (nightly): in_progress/pending; Build on macos-latest (nightly): completed/success; Build on macos-latest (stable): in_progress/pending; Build on ubuntu-latest (stable): in_progress/pending; Build on windows-latest (stable): in_progress/pending". At posting, develop is 4a46f98b2.

@da2ce7

da2ce7 commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

ACK 5a15d70 — the round-6 audit rows, reply and log match the forge record and pass the validator's checks, every other path is byte-identical to the previous head, and the merge into develop is conflict-free

…he audit

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

@da2ce7 da2ce7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 5103f88ddc1095378c8acf96d179ddad1ad52a00. Recomputed from the bytes at this head, against the previous round's head 5a15d7026 and the merge base f0d0ea09b; git merge-tree against develop 4a46f98b2 writes a tree with no conflicts, and the 15 files develop changed since the merge base do not overlap the 22 this PR changes.

What moved: one docs-only commit, 5103f88, fast-forwards 5a15d7026 (the audit only, +18/-2). Every other path is byte-identical, so earlier conclusions on those paths stand.

Previous findings:

  • F25 fixed: the response's Round 5 sentence (edited 21:22:44) and the F23 Solution now say what the response says for round 4 and how it opens. No text of the author on the PR still carries "Fourth review round", except the round-7 reply that quotes it.

The new commit: the round-7 review, the reply, both edits and the thread state match the forge record. "Eight reviews" counts the summary review and the seven rounds, and mapping F25 to F26 follows the collision rule. The F23 Solution's "until the edit of F25" matches the captures. The git grep the F26 row names prints its four quotations. Both log minutes are right and no later than the commit. The audit passes the validator's checks offline (26 rows, 15 log entries), and the commit-id check prints nothing.

Hygiene: Conventional subject that matches the diff, author equals committer in name and address (the commit date is 116 s after the author date), no body, and one attribution footer, which no rule in the tree requires or forbids. git diff --check is clean.

Findings: F26, Nit. It does not block. Its ID collides with the audit's F26, so the collision rule gives it the next audit-local ID.

Checked, no finding: every new URL is on the thread or comment it names. The round-7 decision has no forge record, as for round 6, and no rule asks for one. The F26 resolution reference is the response, as for F25, although part of the fix is in the audit.

Checks

On the loop's compute node at this head, against the merge base f0d0ea09b (receipt server-gates-kranich-656): the repository's pre-commit gate exit 0 (40 s), clippy-allow-reasons against the base exit 0, and the tree left clean; the repository's audit validator on pr-2807-review exit 0 (26 rows, 15 log entries, no failures). Code is byte-identical to the previous round's head. GitHub checks at this head at posting time: "Package Coverage (torrust-tracker-axum-server): queued/pending; Package Coverage (torrust-tracker-udp-server): queued/pending; Package Coverage (torrust-tracker-axum-health-check-api-server): in_progress/pending; Package Coverage (torrust-tracker): queued/pending; Package Coverage (torrust-tracker-axum-http-server): queued/pending; Package Coverage (wait-for-pr-review): in_progress/pending; Package Coverage (torrust-tracker-axum-rest-api-server): in_progress/pending; Docker E2E: completed/skipped; Unit (stable): queued/pending; Unit (nightly): queued/pending; Shipped Features: in_progress/pending; Layer Boundary Bans: completed/success; Workspace Packaging: completed/success; Documented Clippy Allows: completed/success; Generate Coverage Report: in_progress/pending; Discover Package Coverage: completed/success; Docs Lint: completed/success; Build on macos-latest (stable): completed/success; Build on macos-latest (nightly): completed/success; Build on ubuntu-latest (stable): completed/success; Build on windows-latest (nightly): in_progress/pending; Build on ubuntu-latest (nightly): completed/success; Build on windows-latest (stable): completed/success; Test (Docker) (release): in_progress/pending". At posting, develop is 4a46f98b2.

- Reviewer finding ID: F25
- Source URL: https://github.com/torrust/torrust-tracker/pull/2807#discussion_r4529587172
- Concern: After the consolidating edit, the response no longer opens "Fourth review round", but its Round 5 sentence and the F23 Solution still say it does.
- Solution: The Round 5 sentence of the response, the F23 Solution and the F22 reply now say what the response says for round 4: it calls review 5846656529 "Round 4" and opens "Consolidated response for review rounds 4 and 5 of da2ce7".

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Nit][F26] This Solution calls the edited reply "the F22 reply", but by the audit's own IDs it is the F23 reply

The reply edited at 21:22:46 is discussion_r4528688756, the Reply URL of the audit's F23 (the reviewer's F22). The audit's F22 reply, discussion_r4528458638, was not edited: its updated_at is still 17:50:24 and its body is unchanged. This sentence names one finding twice, as "the F23 Solution" and "the F22 reply", while the rest of the audit's prose uses audit IDs ("the F22 row" at :396). The verification at :410 and the 21:22 log entry at :431 repeat "the F22 reply", and that entry gives the reply's edit the response's stamp, 21:22:44, two seconds before the reply's updated_at of 21:22:46; process-pr-review takes a log stamp from the event's own recorded time (SKILL.md:120-122). The round-7 reply on this thread says "the F22 reply" too. Say "the F23 reply" in the three places, and stamp the reply's edit 21:22:46 in the log.

@da2ce7

da2ce7 commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

ACK 5103f88 — the round-7 audit entries match the forge record apart from one non-blocking nit and pass the validator's checks, every other path is byte-identical to the previous head, and the merge into develop is conflict-free

@josecelano

Copy link
Copy Markdown
Member

ACK 5103f88

@josecelano
josecelano merged commit ee06933 into torrust:develop Nov 11, 2026
28 checks passed

This branch was previously deployed

1 inactive deployment
coverage — 5103f88d Deployed Nov 10, 2026 by josecelano-bot via Generate Coverage Report #3034
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove legacy shutdown API and library OS signals

4 participants