Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions docs/copilot-pr-reviews/pr-2211-copilot-suggestions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
---
semantic-links:
skill-links:
- process-copilot-suggestions
related-artifacts:
- .github/skills/dev/pr-reviews/process-copilot-suggestions/SKILL.md
---

<!-- cspell:disable -->

<!-- skill-link: process-copilot-suggestions -->

# PR #2211 Copilot Suggestions Tracking

Source: Copilot PR review threads for https://github.com/torrust/torrust-tracker/pull/2211

Status legend:

- `action`: code/docs change applied
- `no-action`: suggestion reviewed; no code change needed
- `resolved`: thread resolved in PR

## Workflow

1. Download all review threads (including resolved/outdated state and thread IDs).
2. Add one row per thread in the Suggestions table.
3. Process suggestions one by one:
- decide `action` or `no-action`
- if `action`, apply change and validate
- if needed, commit changes
- reply on the PR thread with the fix commit and outcome, or the no-action rationale
- resolve the PR thread

4. Set `Thread State` to `resolved` once resolved in PR.

## Processing Log

- 2026-09-14: Started processing suggestions.
- 2026-09-14: Completed processing suggestions.

## Suggestions

| # | Thread ID | Path | URL | Suggestion Summary | Decision | Reply URL | Status | Thread State |
| - | -------------------------- | -------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------- | -------- | --------- | ------ | ------------ |
| 1 | `PRRT_kwDOGp2yqc6iFL3q` | `docs/issues/open/2185-2003-triage-advisory-external-link-check-findings/ISSUE.md` | https://github.com/torrust/torrust-tracker/pull/2211#discussion_r4004512828 | Add the C3 hosted run to AC5 acceptance evidence. | action | https://github.com/torrust/torrust-tracker/pull/2211#discussion_r4004593868 | DONE | RESOLVED |
| 2 | `PRRT_kwDOGp2yqc6iFL4F` | `docs/issues/open/2185-2003-triage-advisory-external-link-check-findings/external-link-baseline.md` | https://github.com/torrust/torrust-tracker/pull/2211#discussion_r4004512869 | Remove completed C3 from the deferred-work range and update reason. | action | https://github.com/torrust/torrust-tracker/pull/2211#discussion_r4004601832 | DONE | RESOLVED |

## Notes

- Keep this file as an audit log of review handling for the PR.
- Prefer concise decisions with explicit rationale.
- If no code changes are needed, explain why in `Decision`.
- Reply on every PR suggestion thread before resolving it so the decision is visible to reviewers.
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`.
| ID | Status | Task | Notes / Expected Output |
| --- | ----------- | ----------------------------------------- | ---------------------------------------------------------------------------------------------------------------- |
| T1 | DONE | Preserve and classify the baseline | `external-link-baseline.md` maps all 461 report errors to nine recurring categories and dispositions. |
| T2 | IN_PROGRESS | Repair clearly stale references | Repaired C3's 14 unavailable package-specific docs.rs links; hosted verification is pending. |
| T2 | IN_PROGRESS | Repair clearly stale references | C3's 14 unavailable package-specific docs.rs links are hosted-verified; C4-C6 investigation and repair remain. |
| T3 | DONE | Add justified narrow exclusions | C1/C9 and C2 online-only rules are verified on merged upstream hosted runs. |
| T4 | DONE | Revalidate hosted signal | Hosted run 34616458439 excluded C1/C9 and C2 while retaining visible unrelated failures and its report artifact. |
| T5 | TODO | Document operations and review completion | Triage procedure, residual risks, acceptance evidence, and independent review are updated from observed results. |
Expand Down Expand Up @@ -134,11 +134,12 @@ A category that needs no repository change is recorded in issue-local evidence w
- 2026-09-11 09:25 UTC - Copilot - Added online-only `exclude_loopback = true` for C2. With explicit online configuration, a three-link boundary test excluded `127.0.0.1` and `localhost` while checking `https://www.rust-lang.org/` successfully. Hosted verification remains pending.
- 2026-09-11 15:38 UTC - Copilot - After PR #2202 merged, [run 34616458439](https://github.com/torrust/torrust-tracker/actions/runs/34616458439) completed on merged revision `f6df96bf`: it visibly failed with 44 remaining errors, excluded 495 links, uploaded a retained 1,533-byte report artifact, and contained no loopback (`localhost` or `127.0.0.1`) or C1/C9 `#discussion_r` URLs. Unrelated `404`, `403`, FSF transport, issue-comment, pull-request-review, and other missing-fragment failures remained visible.
- 2026-09-11 16:45 UTC - Copilot - Verified the 14 C3 package manifests each inherit `documentation = https://docs.rs/crate/torrust-tracker/`; that target returned HTTP 200 and redirects to `latest`. Replaced only the 14 unavailable package-specific docs.rs links with the shared tracker documentation target. Hosted workflow verification remains pending.
- 2026-09-14 09:51 UTC - Copilot - After PR #2208 merged, [run 34829466145](https://github.com/torrust/torrust-tracker/actions/runs/34829466145) completed on merged revision `952911af`: it visibly failed with 31 unrelated errors, retained a 1,357-byte report artifact through 2026-09-28, and contained none of C3's 14 replaced package-specific docs.rs URLs. The lower count than C2's 44 is not directly comparable because PR #2207 archived issue specifications from the checked document set.

## Acceptance Criteria

- [x] AC1: An issue-local baseline records the exact hosted run, revision, summary counts, and a disposition for every distinct failing URL or recurring failure pattern.
- [ ] AC2: Each repair changes only a verified stale reference and records why its replacement target is correct. C3 repair evidence is recorded; hosted verification remains pending.
- [ ] AC2: Each repair changes only a verified stale reference and records why its replacement target is correct. C3 target and hosted verification are recorded; remaining repair slices must be verified before AC2 can be completed.
- [x] AC3: Each added exclusion is online-only, narrowly scoped to a documented durable false-positive category, and does not suppress unrelated external-link failures.
- [x] AC4: The advisory workflow remains scheduled/manual, visibly fails for remaining external-link failures, and continues to upload its Markdown report on failure.
- [x] AC5: At least one hosted rerun after each remediation slice records the resulting counts and explains material differences from the prior run.
Expand Down Expand Up @@ -166,7 +167,7 @@ Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`.
| --- | ------------------------------ | ------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | ------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| M1 | Reproduce baseline | Manually dispatch `external-link-check.yaml` on `develop`; download `lychee-external-link-report`. | Report is available even when Lychee fails; baseline counts and categories can be reviewed. | DONE | [Run 34347690674](https://github.com/torrust/torrust-tracker/actions/runs/34347690674), revision `7abc30b2`, 461 errors, 0 timeouts. |
| M2 | Classify durable failures | Review all report entries and group by URL/pattern, response type, owning document, and proposed disposition. | Every baseline failure has a traceable disposition; no broad host-level suppression is proposed. | DONE | `external-link-baseline.md`; independent reconciliation passed on 2026-09-10. |
| M3 | Verify reference repairs | Check each changed target using the appropriate authoritative source, then run local validation. | Replacement reference is correct and offline local-link validation remains clean. | TODO | Focused commands and review evidence. |
| M3 | Verify reference repairs | Check each changed target using the appropriate authoritative source, then run local validation. | Replacement reference is correct and offline local-link validation remains clean. | DONE | C3: Cargo metadata and docs.rs confirmed the shared target; [run 34829466145](https://github.com/torrust/torrust-tracker/actions/runs/34829466145) contains none of the 14 replaced URLs and retains unrelated errors. |
Comment thread
josecelano marked this conversation as resolved.
| M4 | Verify exclusion boundaries | Dispatch the hosted workflow after adding a proposed exclusion. | The intended durable false-positive category is absent, while representative unrelated external failures remain visible and the report uploads. | DONE | C1/C9: [run 34578523069](https://github.com/torrust/torrust-tracker/actions/runs/34578523069) on `427b0c93` excluded 476 links and retained its report. C2: [run 34616458439](https://github.com/torrust/torrust-tracker/actions/runs/34616458439) on `f6df96bf` excluded 495 links, left 44 errors, retained a 1,533-byte artifact, and contained no loopback URLs. |
| M5 | Distinguish transient failures | Re-run a newly observed timeout, 403, or other potentially transient result once. | The record distinguishes a persistent failure from a transient response before an exclusion or repair decision. | TODO | Pair of hosted-run URLs and comparison. |

Expand All @@ -175,10 +176,10 @@ Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`.
| AC ID | Status (`TODO`/`DONE`) | Evidence |
| ----- | ---------------------- | ------------------------------------------------------------------ |
| AC1 | DONE | `external-link-baseline.md` from run 34347690674. |
| AC2 | TODO | Reviewed reference-repair commits and target evidence. |
| AC2 | TODO | C3 target and hosted verification are recorded; remaining repair slices must be verified. |
| AC3 | DONE | C1/C9 and C2 hosted runs show each exact exclusion remains narrow. |
| AC4 | DONE | Run 34578523069 failed visibly and uploaded its report artifact. |
| AC5 | DONE | Runs 34578523069 and 34616458439 record each exclusion slice. |
| AC5 | DONE | Runs 34578523069, 34616458439, and 34829466145 record each completed remediation slice. |
| AC6 | TODO | Updated documentation and reviewer confirmation. |

## Risks and Trade-offs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,17 @@ semantic-links:
- Follow-up actions:
- Merge the C3 slice, manually dispatch the advisory workflow, retain its report artifact, and record the run URL, revision, counts, absence of all 14 replaced URLs, remaining failures, and upload outcome.
- Independently review that hosted evidence before completing C3-related issue criteria.

### 2026-09-14 10:00 UTC - Task Reviewer

- Invocation scope: Read-only review of the uncommitted C3 hosted-verification evidence in `ISSUE.md`, `external-link-baseline.md`, the supplied downloaded report, GitHub Actions run 34829466145, and `.github/workflows/external-link-check.yaml`.
- Evidence: Run 34829466145 completed on `952911af2321e848ac95ad183e8e8b96d8fd354a` with the expected visible `Check External Links` failure. `Upload Lychee Report` succeeded under the workflow's unconditional upload policy. The retained `lychee-external-link-report` artifact is 1,357 bytes, is not expired, and expires at `2026-09-28T09:51:01Z`. The supplied report contains none of C3's 14 retired package-specific docs.rs targets while retaining representative unrelated FSF transport, Docker fragment, GitHub comment/review-fragment, stale-reference, third-party `403`, and Star History fragment failures.
- Findings:
- Resolved: The merged hosted evidence verifies the C3 repair boundary without adding an exclusion or suppressing unrelated failures. M3 may remain DONE.
- Correction required: The supplied report summary and its individual entries both show 31 errors, not 24. Correct the C3 count in `ISSUE.md` and `external-link-baseline.md`.
- Correction required: AC2 must remain pending for remaining repair slices, but its wording must no longer state that C3 hosted verification is pending.
- Pending: Keep T2 and AC2 pending, and keep all issue-wide completion, quality, manual-verification, acceptance-review, and implementation-completion-review criteria pending.
- Verdict: REVIEW WARNED.
- Follow-up actions:
- Correct the C3 error count and stale AC2-hosted-verification wording.
- Preserve this review entry as append-only evidence; do not close issue #2185.
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ The following nine categories cover all 461 report errors. C1 and C9 share one p

- **Pattern:** `https://docs.rs/torrust-*` and `https://docs.rs/bittorrent-udp-protocol` returning `404`.
- **Disposition:** Repaired. These package README links pointed at unavailable individual crate pages; the corresponding workspace packages inherit the shared `https://docs.rs/crate/torrust-tracker/latest` documentation target.
- **Next action:** Complete local verification: all 14 affected package manifests report the shared target through Cargo metadata, and docs.rs returned HTTP 200 for it. Verify removal from a hosted report after this repair slice merges.
- **Next action:** Complete. All 14 affected package manifests report the shared target through Cargo metadata, docs.rs returned HTTP 200 for it, and hosted [run 34829466145](https://github.com/torrust/torrust-tracker/actions/runs/34829466145) contained none of the replaced URLs while retaining unrelated errors and its report artifact.

### C4: Stale repository-controlled GitHub links — 3 occurrences

Expand Down Expand Up @@ -147,6 +147,12 @@ The report recorded 1,816 total checks, 1,288 successful checks, 26 redirects, 4

The report still contained visible `404`, `403`, local loopback, GitHub issue-comment and pull-request-review fragments, third-party missing-fragment, and rate-limited diagnostics. This proves the C1/C9 rule suppresses the intended dynamic review-comment-anchor pattern without hiding unrelated categories. The remaining 52 errors are the next triage input; their exact count is not directly comparable to the 461-error baseline because the checked document set changed between revisions.

### C3 repair verification

[External Link Check run 34829466145](https://github.com/torrust/torrust-tracker/actions/runs/34829466145) ran after PR #2208 merged on revision `952911af2321e848ac95ad183e8e8b96d8fd354a`. The `Check External Links` step failed visibly with exit code 2 and `Upload Lychee Report` succeeded. The retained `lychee-external-link-report` artifact is 1,357 bytes and expires on 2026-09-28.

The downloaded report contains 31 errors and no occurrence of any of C3's 14 replaced package-specific docs.rs URLs. It retains unrelated FSF transport, Docker missing-fragment, GitHub issue-comment and pull-request-review-anchor, stale-reference, third-party `403`, and Star History fragment errors. The count is not directly comparable to C2's 44 errors because PR #2207 archived issue specifications that were part of C2's checked document set. This verifies the C3 repair without adding an exclusion or hiding remaining external-link failures.

## Deferred Work

C3-C9 are intentionally deferred until C1 is independently reviewed and its exclusion boundary is validated. This prevents the first configuration change from mixing clearly uncheckable dynamic anchors with potentially stale or transient external URLs.
C4-C9 remain deferred until each category's affected references or transient behavior is verified individually. This keeps future repairs and any potential exclusions narrowly scoped and prevents mixing stale-reference, missing-fragment, and transient/access-controlled findings in one change.