From 91a6963bc69688830d2844caf0fa35f02c84345a Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 11 May 2026 13:06:11 +0100 Subject: [PATCH 1/5] fix(tracker-client): format unrecognized UDP responses clearly --- .../src/console/clients/udp/app.rs | 6 ++ .../src/console/clients/udp/mod.rs | 58 ++++++++++++++++--- packages/tracker-client/src/udp/mod.rs | 34 ++++++++++- 3 files changed, 88 insertions(+), 10 deletions(-) diff --git a/console/tracker-client/src/console/clients/udp/app.rs b/console/tracker-client/src/console/clients/udp/app.rs index 09d452483..5e1013427 100644 --- a/console/tracker-client/src/console/clients/udp/app.rs +++ b/console/tracker-client/src/console/clients/udp/app.rs @@ -64,6 +64,12 @@ //! } //! ``` //! +//! Unrecognized UDP response: +//! +//! ```text +//! Error: Unrecognized UDP tracker response. Expected a valid UDP response, got: [0, 0, 0, 1] +//! ``` +//! //! You can use an URL with instead of the socket address. For example: //! //! ```text diff --git a/console/tracker-client/src/console/clients/udp/mod.rs b/console/tracker-client/src/console/clients/udp/mod.rs index 21f026a79..43d232cac 100644 --- a/console/tracker-client/src/console/clients/udp/mod.rs +++ b/console/tracker-client/src/console/clients/udp/mod.rs @@ -18,23 +18,35 @@ pub enum Error { #[error("Failed to send a connection request, with error: {err}")] UnableToSendConnectionRequest { err: udp::Error }, - #[error("Failed to receive a connect response, with error: {err}")] - UnableToReceiveConnectResponse { err: udp::Error }, + #[error("{err}")] + UnableToReceiveConnectResponse { + #[source] + err: udp::Error, + }, #[error("Failed to send a announce request, with error: {err}")] UnableToSendAnnounceRequest { err: udp::Error }, - #[error("Failed to receive a announce response, with error: {err}")] - UnableToReceiveAnnounceResponse { err: udp::Error }, + #[error("{err}")] + UnableToReceiveAnnounceResponse { + #[source] + err: udp::Error, + }, #[error("Failed to send a scrape request, with error: {err}")] UnableToSendScrapeRequest { err: udp::Error }, - #[error("Failed to receive a scrape response, with error: {err}")] - UnableToReceiveScrapeResponse { err: udp::Error }, + #[error("{err}")] + UnableToReceiveScrapeResponse { + #[source] + err: udp::Error, + }, - #[error("Failed to receive a response, with error: {err}")] - UnableToReceiveResponse { err: udp::Error }, + #[error("{err}")] + UnableToReceiveResponse { + #[source] + err: udp::Error, + }, #[error("Failed to get local address for connection: {err}")] UnableToGetLocalAddr { err: udp::Error }, @@ -48,3 +60,33 @@ impl From for String { value.to_string() } } + +#[cfg(test)] +mod tests { + use std::io; + use std::sync::Arc; + + use bittorrent_tracker_client::udp; + + use super::Error; + + #[test] + fn it_should_display_the_inner_udp_parse_error_for_announce_responses() { + // Arrange + let inner_error = udp::Error::UnableToParseResponse { + err: Arc::new(io::Error::new(io::ErrorKind::Other, "failed to fill whole buffer")), + response: vec![0, 0, 0, 1], + }; + + let error = Error::UnableToReceiveAnnounceResponse { err: inner_error }; + + // Act + let message = error.to_string(); + + // Assert + assert_eq!( + message, + "Unrecognized UDP tracker response. Expected a valid UDP response, got: [0, 0, 0, 1]" + ); + } +} diff --git a/packages/tracker-client/src/udp/mod.rs b/packages/tracker-client/src/udp/mod.rs index 57924b964..7694fb88b 100644 --- a/packages/tracker-client/src/udp/mod.rs +++ b/packages/tracker-client/src/udp/mod.rs @@ -57,8 +57,12 @@ pub enum Error { #[error("Failed to get data from request: {request:?}, with error: {err:?}")] UnableToWriteDataFromRequest { err: Arc, request: Request }, - #[error("Failed to parse response: {response:?}, with error: {err:?}")] - UnableToParseResponse { err: Arc, response: Vec }, + #[error("Unrecognized UDP tracker response. Expected a valid UDP response, got: {response:?}")] + UnableToParseResponse { + #[source] + err: Arc, + response: Vec, + }, } impl From for DynError { @@ -66,3 +70,29 @@ impl From for DynError { Arc::new(Box::new(e)) } } + +#[cfg(test)] +mod tests { + use std::io; + use std::sync::Arc; + + use super::Error; + + #[test] + fn it_should_display_unrecognized_udp_tracker_response_without_debug_noise() { + // Arrange + let error = Error::UnableToParseResponse { + err: Arc::new(io::Error::new(io::ErrorKind::Other, "failed to fill whole buffer")), + response: vec![0, 0, 0, 1], + }; + + // Act + let message = error.to_string(); + + // Assert + assert_eq!( + message, + "Unrecognized UDP tracker response. Expected a valid UDP response, got: [0, 0, 0, 1]" + ); + } +} From 82b8935aaf2177ca6093fabda95a5e9e417940ca Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 11 May 2026 13:25:00 +0100 Subject: [PATCH 2/5] docs(issue-671): add manual verification plan and tracker results --- ...ker-client-print-unrecognized-responses.md | 59 +++++++++++++++++++ project-words.txt | 3 + 2 files changed, 62 insertions(+) diff --git a/docs/issues/open/671-udp-tracker-client-print-unrecognized-responses.md b/docs/issues/open/671-udp-tracker-client-print-unrecognized-responses.md index 00291c3f0..011bc4296 100644 --- a/docs/issues/open/671-udp-tracker-client-print-unrecognized-responses.md +++ b/docs/issues/open/671-udp-tracker-client-print-unrecognized-responses.md @@ -108,6 +108,65 @@ UnableToReceiveAnnounceResponse { err: udp::Error }, In `console/tracker-client/src/console/clients/udp/app.rs`, add an example showing what the error output looks like when an unrecognized response is received. +## Manual Verification + +This section is a living test plan and result log for validating the implementation against real +UDP trackers. + +### Goal + +- Confirm that the CLI prints a clean, readable error when a UDP tracker returns bytes that cannot + be parsed into a known response. +- Confirm whether the issue can be reproduced with real-world public trackers from the newtrackon + UDP list. +- If all sampled trackers return valid responses, record that outcome here and switch to the + fallback plan described later in the issue discussion. + +### Step 1: Collect stable UDP trackers + +- Query the newtrackon UDP endpoint: +- Record the returned tracker list used for the verification run. +- Note the date, time, and any filtering applied before testing. + +### Step 2: Probe each tracker with a sample request + +- Send a representative UDP request to each tracker in the sampled list. +- Record whether the tracker returns a valid UDP response or an unrecognized payload. +- For invalid responses, record the raw bytes exactly as printed by the CLI. + +### Step 3: Record results + +Use this table to track progress and outcomes: + +| Tracker | Sample request | Result | Notes | +| ------------------------------------------ | --------------------------------------------------- | ------ | --------------------------------- | +| `udp://tracker.dler.com:6969/announce` | `announce 9c38422213e30bff212b30c360d26f9a02136422` | valid | Returned announce JSON with peers | +| `udp://tracker.tryhackx.org:6969/announce` | `announce 9c38422213e30bff212b30c360d26f9a02136422` | valid | Returned announce JSON with peers | +| `udp://tracker.fnix.net:6969/announce` | `announce 9c38422213e30bff212b30c360d26f9a02136422` | valid | Returned announce JSON | +| `udp://evan.im:6969/announce` | `announce 9c38422213e30bff212b30c360d26f9a02136422` | valid | Returned announce JSON | + +Observed on 2026-05-11. + +### Step 4: Decide next action + +- The sampled newtrackon trackers returned valid UDP responses. +- No malformed payload has been observed yet, so the real-tracker path is currently not enough to + exercise the unrecognized-response display branch. + +### Step 5: Local invalid-response verification + +If the public trackers stay valid, use a local tracker instance to force a malformed UDP response +and verify the CLI output end-to-end. + +1. Change the code of the UDP tracker in the local code so it returns a deliberately malformed + UDP payload. +2. Run the UDP tracker locally. +3. Make the request to the locally running tracker with the UDP tracker client. +4. Verify the client cannot parse the response and prints useful information, including the + malformed bytes, so the user can understand what happened. + +Record the observed output here, including the exact raw bytes if the malformed payload is hit. + ## Acceptance Criteria - [ ] Running the client against a tracker that returns an invalid packet produces output diff --git a/project-words.txt b/project-words.txt index 5c16f0ed6..3a712b8da 100644 --- a/project-words.txt +++ b/project-words.txt @@ -69,6 +69,7 @@ datetime dbip dbname debuginfo +dler Deque Dihc Dijke @@ -88,6 +89,7 @@ fdbased fdget filesd finalises +fnix flamegraph formatjson fput @@ -218,6 +220,7 @@ recognised recompiles referer Registar +tryhackx repomix repr reqs From 752ec19351e7c76583c35a17e966c6a96e7e6af9 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 11 May 2026 13:31:03 +0100 Subject: [PATCH 3/5] docs(issue-671): record local malformed-response verification --- ...ker-client-print-unrecognized-responses.md | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/docs/issues/open/671-udp-tracker-client-print-unrecognized-responses.md b/docs/issues/open/671-udp-tracker-client-print-unrecognized-responses.md index 011bc4296..a5825328b 100644 --- a/docs/issues/open/671-udp-tracker-client-print-unrecognized-responses.md +++ b/docs/issues/open/671-udp-tracker-client-print-unrecognized-responses.md @@ -165,7 +165,36 @@ and verify the CLI output end-to-end. 4. Verify the client cannot parse the response and prints useful information, including the malformed bytes, so the user can understand what happened. -Record the observed output here, including the exact raw bytes if the malformed payload is hit. +Observed local verification on 2026-05-11: + +Tracker start command (with a temporary local patch applied in the UDP server +send path to force payload `[0, 0, 0, 1]`): + +```bash +cargo run +``` + +Client probe command: + +```bash +target/debug/udp_tracker_client announce \ + udp://127.0.0.1:6969/announce \ + 9c38422213e30bff212b30c360d26f9a02136422 +``` + +Observed client output: + +```text +Error: Unrecognized UDP tracker response. Expected a valid UDP response, + got: [0, 0, 0, 1] + +Caused by: + 0: Unrecognized UDP tracker response. Expected a valid UDP response, + got: [0, 0, 0, 1] + 1: invalid data +``` + +Result: malformed bytes are visible in CLI output as required. ## Acceptance Criteria From e582ec3902188f7922ceb1d0d099ae351139336f Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 11 May 2026 13:38:03 +0100 Subject: [PATCH 4/5] docs(issue-671): mark acceptance criteria complete --- ...-udp-tracker-client-print-unrecognized-responses.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/issues/open/671-udp-tracker-client-print-unrecognized-responses.md b/docs/issues/open/671-udp-tracker-client-print-unrecognized-responses.md index a5825328b..aefe1e932 100644 --- a/docs/issues/open/671-udp-tracker-client-print-unrecognized-responses.md +++ b/docs/issues/open/671-udp-tracker-client-print-unrecognized-responses.md @@ -198,14 +198,14 @@ Result: malformed bytes are visible in CLI output as required. ## Acceptance Criteria -- [ ] Running the client against a tracker that returns an invalid packet produces output +- [x] Running the client against a tracker that returns an invalid packet produces output matching: `Error: Unrecognized UDP tracker response. Expected a valid UDP response, got: [...]` -- [ ] Running the client against a well-behaved tracker still prints the JSON response and +- [x] Running the client against a well-behaved tracker still prints the JSON response and exits `0` -- [ ] `linter all` exits with code `0` -- [ ] `cargo machete` reports no unused dependencies -- [ ] All existing tests pass +- [x] `linter all` exits with code `0` +- [x] `cargo machete` reports no unused dependencies +- [x] All existing tests pass ## Key Files From a74545343709e62a3f9262cbfc97246ff9c78324 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 11 May 2026 13:44:16 +0100 Subject: [PATCH 5/5] docs(workflow): enforce PR body accuracy checks --- .github/agents/github-operator.agent.md | 9 ++++++--- .../dev/git-workflow/open-pull-request/SKILL.md | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/.github/agents/github-operator.agent.md b/.github/agents/github-operator.agent.md index cb06dcb91..beaa8af31 100644 --- a/.github/agents/github-operator.agent.md +++ b/.github/agents/github-operator.agent.md @@ -39,9 +39,10 @@ Do not jump directly to raw API calls if a dedicated MCP or CLI command covers t 2. Read any local specification or context file needed to perform the task correctly. 3. Load the relevant repository skill when one exists. 4. Choose the highest-level GitHub interface that can perform the task safely. -5. Execute the operation with the minimum number of calls needed. -6. Verify the result by reading the updated GitHub object or returned URL. -7. Report only the outcome and key identifiers back to the caller. +5. For PR descriptions, reconcile the proposed body with the actual branch diff and commit list before applying updates. +6. Execute the operation with the minimum number of calls needed. +7. Verify the result by reading the updated GitHub object or returned URL. +8. Report only the outcome and key identifiers back to the caller. ## Repository Guidance @@ -58,6 +59,7 @@ Do not jump directly to raw API calls if a dedicated MCP or CLI command covers t - Do not assume the visible issue number is the same identifier required by a GitHub API. - For sub-issue linking, remember that the REST API expects the child issue's internal GitHub ID, not its visible issue number. +- Do not claim PR implementation changes that are not present in the current HEAD diff. - Do not mix GitHub task execution with unrelated code changes. - If a PR review comment requires code changes, stop after identifying the actionable request and hand control back to the caller or a code-focused agent. @@ -70,3 +72,4 @@ When finishing a task, return: 1. What was changed or verified 2. The key GitHub identifiers or URLs 3. Any blockers, permissions issues, or follow-up needed +4. For PR body updates, a short evidence line showing the checked commit range and changed files diff --git a/.github/skills/dev/git-workflow/open-pull-request/SKILL.md b/.github/skills/dev/git-workflow/open-pull-request/SKILL.md index 04074a383..2ea285f67 100644 --- a/.github/skills/dev/git-workflow/open-pull-request/SKILL.md +++ b/.github/skills/dev/git-workflow/open-pull-request/SKILL.md @@ -22,6 +22,8 @@ Before opening a PR: - [ ] Branch is pushed to your fork remote - [ ] Commits are GPG signed (`git log --show-signature -n 1`) - [ ] All pre-commit checks passed (`linter all`, `cargo machete`, tests) +- [ ] PR body claims are aligned with the actual commit range (`origin/develop..HEAD`) +- [ ] If manual verification used temporary local-only patches, PR body explicitly says they are not included > Important: always open the PR in the **upstream repository**, not in your fork. > Resolve upstream from `Cargo.toml` (`repository = "https://github.com/torrust/torrust-tracker"`) and use that value for `gh pr create --repo ...`. @@ -42,6 +44,11 @@ PR body must include: - Validation performed - Issue link (`Closes #`) +PR body must not include: + +- Claims about code changes that are not present in the branch diff +- Ambiguous wording that mixes temporary local verification patches with committed implementation + ## Option A (Preferred): GitHub CLI ```bash @@ -76,6 +83,15 @@ When MCP pull request management tools are available, create the PR with: - [ ] Head branch is correct - [ ] CI workflows started - [ ] Issue linked in description +- [ ] PR body still matches branch diff and commit history after final rebases/edits + +Quick body-accuracy verification: + +```bash +gh pr view --repo / --json body +git diff --name-only origin/develop...HEAD +git log --oneline origin/develop..HEAD +``` ## Troubleshooting