From d12329053e0da318842bd17094576b61c36aa146 Mon Sep 17 00:00:00 2001 From: Cameron Garnham Date: Tue, 8 Sep 2026 21:04:11 +0200 Subject: [PATCH 1/6] docs(issues): draft docker e2e job package flag specification --- .../docker-e2e-job-package-flag/ISSUE.md | 191 ++++++++++++++++++ 1 file changed, 191 insertions(+) create mode 100644 docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md diff --git a/docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md b/docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md new file mode 100644 index 000000000..dba96398c --- /dev/null +++ b/docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md @@ -0,0 +1,191 @@ +--- +doc-type: issue +issue-type: bug +status: draft +priority: p1 +epic: null +github-issue: null +spec-path: docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md +branch: "{issue-number}-docker-e2e-job-package-flag" +related-pr: null +last-updated-utc: 2026-09-08 21:30 +semantic-links: + skill-links: + - create-issue + related-artifacts: + - .github/workflows/testing.yaml + - .github/workflows/container.yaml + - .yamllint-ci.yml + - packages/e2e-tools/Cargo.toml + - docs/issues/closed/1854-1840-workflow-performance-container-test-gating/ISSUE.md + - docs/issues/open/1840-improve-pr-workflow-performance-epic/EPIC.md +--- + + + +# Issue #[To be assigned] - Docker E2E Job in `testing.yaml` Fails on Every Feature-Branch Push + +## Goal + +Restore the `Docker E2E` job in `.github/workflows/testing.yaml` to a state where its four E2E runner steps actually execute, by naming the package that owns the binaries on each `cargo run` invocation exactly as `.github/workflows/container.yaml` already does, so that feature-branch pushes and pull requests targeting branches other than `develop` and `main` regain the E2E coverage the job exists to provide. + +## Background + +The `docker-e2e` job in `.github/workflows/testing.yaml` (lines 124-197) exists to give E2E coverage to the events that `.github/workflows/container.yaml` does not cover. Its `if:` guard (lines 133-139) skips the job on pull requests whose base is `develop` or `main`, and on pushes to `refs/heads/develop`, `refs/heads/main`, and `refs/heads/releases/*`, because `container.yaml` builds the same image and runs the same E2E tests for exactly those events. The job's comment (lines 125-129) states this and cites issue #1854. What remains after the guard is precisely the feature-branch case: a push to any in-repo branch that is not `develop`, `main`, or a release branch, and any pull request targeting some other branch. + +That job cannot reach its E2E tests. Its four runner steps invoke the binaries without naming the package that owns them: + +- line 185, step `run-tracker-e2e-tests`: `cargo run --bin e2e_tests_runner -- ...` +- line 189, step `run-qbittorrent-e2e-test-sqlite3`: `cargo run --bin qbittorrent_e2e_runner -- ...` +- line 193, step `run-qbittorrent-e2e-test-mysql`: `cargo run --bin qbittorrent_e2e_runner -- ...` +- line 197, step `run-qbittorrent-e2e-test-postgresql`: `cargo run --bin qbittorrent_e2e_runner -- ...` + +Both binaries live in the `torrust-tracker-e2e-tools` package, at `packages/e2e-tools/src/bin/e2e_tests_runner.rs` and `packages/e2e-tools/src/bin/qbittorrent_e2e_runner.rs`. The workspace root is itself a package: the root `Cargo.toml` opens with a `[package]` section declaring `name = "torrust-tracker"` and `default-run = "torrust-tracker"`, and the `[workspace]` table sets `members` without setting `default-members`. Run from the workspace root without `-p`, `cargo run --bin ` therefore resolves the target against the root package alone, where neither binary exists, and the step fails before any tracker image is exercised. The failure reported from CI logs is `error: no bin target named 'e2e_tests_runner' in default-run packages`, with exit code 101. + +The same four invocations in `.github/workflows/container.yaml` (lines 117, 123, 127, 131) carry `-p torrust-tracker-e2e-tools`, which is why that workflow is green while this one is not. + +The divergence has a precise origin, and it is not the guard. Commit `c1b6c5466` ("feat(ci): eliminate duplicate E2E tests and add test binary landscape analysis", 2026-06-03) added only the `if:` guard and its explanatory comment to `testing.yaml`, twelve added lines and no deletions; the runner steps already existed there and predate it. The break arrived the next day in commit `c47173f53` ("ci(container): slim nextest archive by extracting e2e and benchmark packages", 2026-06-04), which moved `src/bin/e2e_tests_runner.rs`, `src/bin/qbittorrent_e2e_runner.rs`, and `src/bin/profiling.rs` into the new `packages/e2e-tools` package as pure renames and touched neither workflow. At that moment both workflows were broken in the same way. Commit `2d1ce246b` ("ci(container): address Copilot review issues after T13/T14 extraction", 2026-06-04) then repaired `container.yaml` alone: its own message records fixing the four `cargo run --bin` invocations to include `-p torrust-tracker-e2e-tools` "so Cargo resolves the binaries after they were extracted out of `src/bin/` into the new e2e-tools package", and wrapping the long `e2e_tests_runner` step in a YAML `>-` block scalar to stay inside the yamllint line-length limit. The identical four invocations in `testing.yaml` were not part of that change and have carried the defect ever since. + +What the guard did contribute is the silence. Because `docker-e2e` never runs on pushes to `develop` or on pull requests targeting `develop`, upstream CI has never executed the broken steps, so `develop` has stayed green while the job has failed on every feature-branch push since June 2026. The failure surfaces only where nobody routinely looks: it has been reported on the dependency-update pull requests #2055 and #2106, whose branch pushes do trigger the job, and observed on the `da2ce7/torrust-tracker` fork's push of the `merge-tool-symlink-exceptions-spec` branch on 2026-09-08. Those observations are recorded here as reported evidence; they were not reproduced while drafting this specification, which had no GitHub access. + +One detail of the repair is load-bearing rather than cosmetic. `.yamllint-ci.yml` sets `line-length: max: 200`, and `linter all` runs yamllint over the workflow files. Line 185 is currently 184 characters; adding `-p torrust-tracker-e2e-tools ` costs 26 more and would take it to 210, over the limit. The other three lines are 153, 151, and 156 characters and land at 179, 177, and 182. The first step must therefore be wrapped in a `>-` block scalar, which is exactly the shape `container.yaml` already uses for the same step and for the same reason. + +## Scope + +### In Scope + +- Add `-p torrust-tracker-e2e-tools` to the four `cargo run` invocations in the `docker-e2e` job of `.github/workflows/testing.yaml`, at lines 185, 189, 193, and 197, keeping every other argument of each step byte-for-byte as it is today. +- Wrap the `run-tracker-e2e-tests` step's command in a YAML `>-` block scalar so the added flag does not push the line past the 200-character yamllint limit, matching the form `container.yaml` uses for the same step. +- Extend the job's existing comment with one sentence recording why the package flag is required: the workspace root is itself a package, so `--bin` alone resolves only against the root package's targets. +- Verify the fix by pushing the branch to a fork or another in-repo feature branch, where the guard admits the job, and recording the passing run URL in this specification. + +### Out of Scope + +- Changing when the `docker-e2e` job runs. The `if:` guard and the trigger design stay exactly as they are. +- Deduplicating `docker-e2e` against the E2E steps in `container.yaml`, or deciding which of the two workflows should own E2E coverage. +- Any change to the E2E runners themselves, to `packages/e2e-tools`, or to the tracker image build. +- Any change to `.github/workflows/container.yaml`. It is already correct and is the reference this fix matches. +- Repairing the stale `src/bin/` inventory in `AGENTS.md`, which still lists `e2e_tests_runner` and `profiling` as root-package binaries and omits `e2e-tools` from the package catalog. It is the same drift from `c47173f53` and is worth its own issue. +- Any general CI policy about jobs that can only fail where nobody looks. The Design and Ownership Review notes the question; this issue does not answer it. + +## Architectural Decisions + +- Related ADRs: `docs/adrs/20260603000000_keep_unit_tests_inside_container_build.md` documents the E2E and unit-test split these workflows implement; it is context, not a constraint this change touches. +- ADRs to create: None required. The defect is a missing package flag on four command lines, and the correct value is already fixed by the package that owns the binaries; there is no design choice to record. + +## Design and Ownership Review + +Not applicable in the template's usual sense: this issue adds no child processes, asynchronous I/O, readiness waits, resource cleanup, or test fixtures. Every change is confined to one workflow file, and the behavior it restores is entirely the behavior `container.yaml` already demonstrates with the same commands against the same binaries. + +Two ownership observations are worth carrying forward without widening this issue. First, the four invocations exist twice, in `testing.yaml` and in `container.yaml`, with no mechanism keeping them in step; this defect is what that duplication costs, and a later issue could decide whether one workflow should own the commands. Second, the job's silence on `develop` is a property of its `if:` guard rather than of this fix: the guard is doing what it was designed to do, and it is also what let a broken job stay broken for three months. Whether any job should be allowed to fail only on events that upstream CI never runs is a question worth asking separately; it is noted here, not scoped here. + +## Implementation Plan + +Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. + +| ID | Status | Task | Notes / Expected Output | +| --- | ------ | ------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| T1 | TODO | Add the package flag and amend the comment | The four `cargo run` lines in the `docker-e2e` job of `.github/workflows/testing.yaml` carry `-p torrust-tracker-e2e-tools`; the `run-tracker-e2e-tests` step uses a `>-` block scalar; the job comment gains one sentence explaining the flag; `linter all` exits `0`. | +| T2 | TODO | Verify on a feature-branch push | The branch is pushed to a fork or another in-repo feature branch, the `Docker E2E` job of `testing.yaml` runs, its four runner steps execute and pass, and the run URL is recorded as evidence for M1 and AC2. | + +## Commit Points + +| Task | Coherent change set | Commit policy | +| ---- | --------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | +| T1 | The workflow edit: four `cargo run` lines plus the block scalar and the comment sentence, in one file. | Commit after `linter all` passes. One signed Conventional Commit, scope `ci`. | +| T2 | Evidence only: progress-log entry and manual-scenario rows updated with the run URL, no workflow change. | Commit after the run completes, or fold into the pull request description if no spec update is warranted. | + +## Progress Tracking + +### Workflow Checkpoints + +- [x] Folder-style spec drafted in `docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md` +- [ ] Spec reviewed and approved by user/maintainer +- [ ] GitHub issue created and issue number added to this spec +- [ ] (Optional, recommended for complex issues) Spec-only PR merged into `develop` before implementation +- [ ] Implementation completed +- [ ] Automatic verification completed (`linter all`, relevant tests, and any pre-push checks) +- [ ] Manual verification scenarios executed and recorded (status + evidence) +- [ ] Acceptance criteria reviewed after implementation and updated with evidence +- [ ] Evidence-based implementation completion review recorded: issue-local retrospective created for material discoveries, or progress log states why none was needed +- [ ] Reviewer validated acceptance criteria and updated checkboxes +- [ ] Independent reviewer reports recorded in issue-local `agent-review-reports.md` when reviewers received this folder-style specification +- [ ] Committer verified spec progress is up to date before commit +- [ ] Issue closed and spec moved from `docs/issues/open/` to `docs/issues/closed/` + +### Progress Log + +- 2026-09-08 21:30 UTC - Spec author - Drafted this specification against `develop` at `e4db63d5`. Verified the four invocation line numbers in both workflows, the `if:` guard range, the binaries' location under `packages/e2e-tools/src/bin/`, and the root `Cargo.toml` `[package]`/`default-run` declaration. Traced the divergence to `c47173f53` (package extraction) and `2d1ce246b` (which repaired `container.yaml` only), not to `c1b6c5466`, which added only the guard. Measured the four line lengths against the 200-character limit in `.yamllint-ci.yml` and found that line 185 needs a `>-` block scalar. CI observations for #2055, #2106, and the `merge-tool-symlink-exceptions-spec` push are recorded as reported evidence; no GitHub access was available while drafting. + +## Acceptance Criteria + +- [ ] AC1: The four `cargo run` invocations in the `docker-e2e` job of `.github/workflows/testing.yaml` each carry `-p torrust-tracker-e2e-tools`, and every other argument of each step is unchanged. +- [ ] AC2: On a push to a feature branch, the `Docker E2E` job of `testing.yaml` runs, its four runner steps reach the E2E runners rather than failing on target resolution, and the job completes successfully. Evidence is the run URL. +- [ ] AC3: `.github/workflows/container.yaml` is unchanged by this issue. +- [ ] AC4: The `docker-e2e` job comment records why the package flag is required. +- [ ] `linter all` exits with code `0` +- [ ] Relevant tests pass +- [ ] Manual verification scenarios are executed and documented (status + evidence) +- [ ] Acceptance criteria are re-reviewed after implementation and reflect actual behavior +- [ ] Documentation is updated when behavior/workflow changes + +## Verification Plan + +### Automatic Checks + +- `linter all` — this covers the YAML change through yamllint (`.yamllint-ci.yml`), including the 200-character line-length rule that motivates the `>-` block scalar on the first step, and covers this specification through markdownlint, cspell, and lychee. +- No Rust tests are affected; the change touches no code. + +### Manual Verification Scenarios + +Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`. + +A pull request targeting `develop` cannot verify this fix: the `if:` guard skips `docker-e2e` for exactly that event, so the job never runs and produces no evidence. Verification must come from an event the guard admits. + +| ID | Scenario | Command/Steps | Expected Result | Status | Evidence | +| --- | ------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ----------------------------------------------------- | +| M1 | The job runs and passes on a feature-branch push | Push the fix branch to a fork or to an in-repo branch that is not `develop`, `main`, or `releases/*`, then open the `Testing` workflow run for that push. | The `Docker E2E` job runs; steps `run-tracker-e2e-tests`, `run-qbittorrent-e2e-test-sqlite3`, `run-qbittorrent-e2e-test-mysql`, and `run-qbittorrent-e2e-test-postgresql` all execute the runners and succeed; the job is green. | TODO | Run URL of the `Docker E2E` job | +| M2 | Behavior on `develop` is unchanged | Open the pull request for this fix against `develop` and inspect its `Testing` workflow run. | The `Docker E2E` job is skipped, as it is today, because `container.yaml` covers the event. No new job appears and no existing job changes status. | TODO | Run URL of the pull request's `Testing` workflow | + +Notes: + +- Manual verification is mandatory even when automated tests pass. +- If a scenario fails, record the failure and diagnosis in the progress log before proceeding. + +### Acceptance Verification + +| AC ID | Status (`TODO`/`DONE`) | Evidence | +| ----- | ---------------------- | --------------------------------------------------- | +| AC1 | TODO | Diff of `.github/workflows/testing.yaml` | +| AC2 | TODO | M1 run URL | +| AC3 | TODO | Pull request file list | +| AC4 | TODO | Diff of `.github/workflows/testing.yaml` | + +## Risks and Trade-offs + +- No material risk to the change itself. The flag value is not a guess: `container.yaml` runs the same four binaries with the same flag against the same package on every push to `develop`, so the invocation form is already proven in CI. +- The line-length rule is the only way this edit can go wrong mechanically, and `linter all` catches it before the change leaves the working tree. +- Verification depends on an event upstream CI does not produce, so the passing run URL must be captured deliberately rather than expected to appear on the pull request. M1 exists for that reason. +- Restoring the job means feature-branch pushes will start spending E2E time they have not spent since June 2026. That is the job's intended cost, not a regression, and the job already carries `timeout-minutes: 90`. + +## Implementation Completion Review + +After implementation, compare the result with this specification. Record invalidated assumptions, material design changes, unexpected validation findings, and reusable lessons. + +- Retrospective: `Not yet assessed` +- A retrospective is likely unnecessary for a four-line workflow fix. It becomes warranted if the feature-branch run reveals that the runner steps fail for some further reason once target resolution succeeds, since that would mean the job has been hiding more than one defect. +- If no retrospective is needed, add a concise progress-log entry explaining why the work had no material discovery. +- When an independent reviewer receives this folder-style specification, it records its result in `agent-review-reports.md` using `docs/templates/AGENT-REVIEW-REPORTS.md`. + +## References + +- Origin of the job's `if:` guard: #1854, spec at `docs/issues/closed/1854-1840-workflow-performance-container-test-gating/ISSUE.md` +- Related EPIC: #1840, spec at `docs/issues/open/1840-improve-pr-workflow-performance-epic/EPIC.md` +- Reported failing runs: #2055, #2106 +- Commit `c1b6c5466` — added the `if:` guard and comment to the `docker-e2e` job +- Commit `c47173f53` — moved the E2E binaries into `packages/e2e-tools`, breaking both workflows +- Commit `2d1ce246b` — added `-p torrust-tracker-e2e-tools` to `container.yaml` only +- `.github/workflows/testing.yaml` — the `docker-e2e` job, lines 124-197 +- `.github/workflows/container.yaml` — the correct invocations, lines 117, 123, 127, 131 +- `packages/e2e-tools/` — package `torrust-tracker-e2e-tools`, owner of both binaries +- `.yamllint-ci.yml` — the 200-character line-length rule From c554118b83189fd331abcf2688c1804d54796d5d Mon Sep 17 00:00:00 2001 From: Cameron Garnham Date: Tue, 8 Sep 2026 21:10:31 +0200 Subject: [PATCH 2/6] docs(issues): fix code span spacing in the docker e2e spec --- docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md b/docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md index dba96398c..8f9431557 100644 --- a/docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md +++ b/docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md @@ -48,7 +48,7 @@ The divergence has a precise origin, and it is not the guard. Commit `c1b6c5466` What the guard did contribute is the silence. Because `docker-e2e` never runs on pushes to `develop` or on pull requests targeting `develop`, upstream CI has never executed the broken steps, so `develop` has stayed green while the job has failed on every feature-branch push since June 2026. The failure surfaces only where nobody routinely looks: it has been reported on the dependency-update pull requests #2055 and #2106, whose branch pushes do trigger the job, and observed on the `da2ce7/torrust-tracker` fork's push of the `merge-tool-symlink-exceptions-spec` branch on 2026-09-08. Those observations are recorded here as reported evidence; they were not reproduced while drafting this specification, which had no GitHub access. -One detail of the repair is load-bearing rather than cosmetic. `.yamllint-ci.yml` sets `line-length: max: 200`, and `linter all` runs yamllint over the workflow files. Line 185 is currently 184 characters; adding `-p torrust-tracker-e2e-tools ` costs 26 more and would take it to 210, over the limit. The other three lines are 153, 151, and 156 characters and land at 179, 177, and 182. The first step must therefore be wrapped in a `>-` block scalar, which is exactly the shape `container.yaml` already uses for the same step and for the same reason. +One detail of the repair is load-bearing rather than cosmetic. `.yamllint-ci.yml` sets `line-length: max: 200`, and `linter all` runs yamllint over the workflow files. Line 185 is currently 184 characters; adding `-p torrust-tracker-e2e-tools` together with the space separating it from the following argument costs 26 more characters and would take it to 210, over the limit. The other three lines are 153, 151, and 156 characters and land at 179, 177, and 182. The first step must therefore be wrapped in a `>-` block scalar, which is exactly the shape `container.yaml` already uses for the same step and for the same reason. ## Scope From 85b5f2d9cc66a4f2011a9fabee9a7f8af0ca5e1b Mon Sep 17 00:00:00 2001 From: Cameron Garnham Date: Thu, 10 Sep 2026 10:13:08 +0200 Subject: [PATCH 3/6] docs(issues): add issue specification for #2179 The reviewed draft now has a GitHub issue, so the specification for the broken Docker E2E job moves out of the drafts folder into `docs/issues/open/` under its assigned number, and its frontmatter, heading, workflow checkpoints and progress log record that number together with the spec-only delivery branch. Both evidence claims were recomputed against `develop` at 89d45145 before the move: the four `cargo run` invocations in the `docker-e2e` job still carry no package flag at lines 185, 189, 193 and 197, measuring 184, 153, 151 and 156 characters, so the block-scalar requirement on the first step stands; and the symptom is now cited from an observed fork run rather than only from reported ones. The out-of-scope note on the stale `src/bin/` inventory names #2190, which has since taken ownership of those references. --- .../ISSUE.md | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) rename docs/issues/{drafts/docker-e2e-job-package-flag => open/2179-docker-e2e-job-package-flag}/ISSUE.md (89%) diff --git a/docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md b/docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md similarity index 89% rename from docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md rename to docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md index 8f9431557..36717a15d 100644 --- a/docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md +++ b/docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md @@ -1,14 +1,14 @@ --- doc-type: issue issue-type: bug -status: draft +status: open priority: p1 epic: null -github-issue: null -spec-path: docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md -branch: "{issue-number}-docker-e2e-job-package-flag" +github-issue: 2179 +spec-path: docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md +branch: "2179-docker-e2e-job-package-flag-spec" related-pr: null -last-updated-utc: 2026-09-08 21:30 +last-updated-utc: 2026-09-10 08:15 semantic-links: skill-links: - create-issue @@ -23,7 +23,7 @@ semantic-links: -# Issue #[To be assigned] - Docker E2E Job in `testing.yaml` Fails on Every Feature-Branch Push +# Issue #2179 - Docker E2E job in testing.yaml fails on every feature-branch push: cargo run lacks the e2e-tools package flag ## Goal @@ -46,7 +46,7 @@ The same four invocations in `.github/workflows/container.yaml` (lines 117, 123, The divergence has a precise origin, and it is not the guard. Commit `c1b6c5466` ("feat(ci): eliminate duplicate E2E tests and add test binary landscape analysis", 2026-06-03) added only the `if:` guard and its explanatory comment to `testing.yaml`, twelve added lines and no deletions; the runner steps already existed there and predate it. The break arrived the next day in commit `c47173f53` ("ci(container): slim nextest archive by extracting e2e and benchmark packages", 2026-06-04), which moved `src/bin/e2e_tests_runner.rs`, `src/bin/qbittorrent_e2e_runner.rs`, and `src/bin/profiling.rs` into the new `packages/e2e-tools` package as pure renames and touched neither workflow. At that moment both workflows were broken in the same way. Commit `2d1ce246b` ("ci(container): address Copilot review issues after T13/T14 extraction", 2026-06-04) then repaired `container.yaml` alone: its own message records fixing the four `cargo run --bin` invocations to include `-p torrust-tracker-e2e-tools` "so Cargo resolves the binaries after they were extracted out of `src/bin/` into the new e2e-tools package", and wrapping the long `e2e_tests_runner` step in a YAML `>-` block scalar to stay inside the yamllint line-length limit. The identical four invocations in `testing.yaml` were not part of that change and have carried the defect ever since. -What the guard did contribute is the silence. Because `docker-e2e` never runs on pushes to `develop` or on pull requests targeting `develop`, upstream CI has never executed the broken steps, so `develop` has stayed green while the job has failed on every feature-branch push since June 2026. The failure surfaces only where nobody routinely looks: it has been reported on the dependency-update pull requests #2055 and #2106, whose branch pushes do trigger the job, and observed on the `da2ce7/torrust-tracker` fork's push of the `merge-tool-symlink-exceptions-spec` branch on 2026-09-08. Those observations are recorded here as reported evidence; they were not reproduced while drafting this specification, which had no GitHub access. +What the guard did contribute is the silence. Because `docker-e2e` never runs on pushes to `develop` or on pull requests targeting `develop`, upstream CI has never executed the broken steps, so `develop` has stayed green while the job has failed on every feature-branch push since June 2026. The failure surfaces only where nobody routinely looks: it has been reported on the dependency-update pull requests #2055 and #2106, whose branch pushes do trigger the job, and it is observed directly on the `da2ce7/torrust-tracker` fork, most recently in the `Testing` run for the push of `2175-merge-tool-symlink-exceptions` at `4bff469e` on 2026-09-10, where the `Docker E2E` job builds the tracker image successfully and then fails at step `Run E2E Tests`, leaving the three qBittorrent steps skipped ([run 34444943476](https://github.com/da2ce7/torrust-tracker/actions/runs/34444943476/job/102767609212)). The #2055 and #2106 observations are carried here as reported evidence and were not re-checked. One detail of the repair is load-bearing rather than cosmetic. `.yamllint-ci.yml` sets `line-length: max: 200`, and `linter all` runs yamllint over the workflow files. Line 185 is currently 184 characters; adding `-p torrust-tracker-e2e-tools` together with the space separating it from the following argument costs 26 more characters and would take it to 210, over the limit. The other three lines are 153, 151, and 156 characters and land at 179, 177, and 182. The first step must therefore be wrapped in a `>-` block scalar, which is exactly the shape `container.yaml` already uses for the same step and for the same reason. @@ -65,7 +65,7 @@ One detail of the repair is load-bearing rather than cosmetic. `.yamllint-ci.yml - Deduplicating `docker-e2e` against the E2E steps in `container.yaml`, or deciding which of the two workflows should own E2E coverage. - Any change to the E2E runners themselves, to `packages/e2e-tools`, or to the tracker image build. - Any change to `.github/workflows/container.yaml`. It is already correct and is the reference this fix matches. -- Repairing the stale `src/bin/` inventory in `AGENTS.md`, which still lists `e2e_tests_runner` and `profiling` as root-package binaries and omits `e2e-tools` from the package catalog. It is the same drift from `c47173f53` and is worth its own issue. +- Repairing the stale `src/bin/` inventory in `AGENTS.md`, which still lists `e2e_tests_runner` and `profiling` as root-package binaries and omits `e2e-tools` from the package catalog. It is the same drift from `c47173f53`, and #2190 has since taken ownership of the five live references that carry it. - Any general CI policy about jobs that can only fail where nobody looks. The Design and Ownership Review notes the question; this issue does not answer it. ## Architectural Decisions @@ -99,9 +99,9 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. ### Workflow Checkpoints -- [x] Folder-style spec drafted in `docs/issues/drafts/docker-e2e-job-package-flag/ISSUE.md` -- [ ] Spec reviewed and approved by user/maintainer -- [ ] GitHub issue created and issue number added to this spec +- [x] Folder-style spec drafted and moved to `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` +- [x] Spec reviewed and approved by user/maintainer +- [x] GitHub issue [#2179](https://github.com/torrust/torrust-tracker/issues/2179) created and issue number added to this spec - [ ] (Optional, recommended for complex issues) Spec-only PR merged into `develop` before implementation - [ ] Implementation completed - [ ] Automatic verification completed (`linter all`, relevant tests, and any pre-push checks) @@ -116,6 +116,8 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. ### Progress Log - 2026-09-08 21:30 UTC - Spec author - Drafted this specification against `develop` at `e4db63d5`. Verified the four invocation line numbers in both workflows, the `if:` guard range, the binaries' location under `packages/e2e-tools/src/bin/`, and the root `Cargo.toml` `[package]`/`default-run` declaration. Traced the divergence to `c47173f53` (package extraction) and `2d1ce246b` (which repaired `container.yaml` only), not to `c1b6c5466`, which added only the guard. Measured the four line lengths against the 200-character limit in `.yamllint-ci.yml` and found that line 185 needs a `>-` block scalar. CI observations for #2055, #2106, and the `merge-tool-symlink-exceptions-spec` push are recorded as reported evidence; no GitHub access was available while drafting. +- 2026-09-10 08:15 UTC - Spec author - Re-verified both evidence facts before opening the issue. Against `develop` at `89d45145` the four `cargo run` invocations in the `docker-e2e` job still carry no `-p` flag at lines 185, 189, 193, and 197, measuring 184, 153, 151, and 156 characters, so the block-scalar requirement on the first step holds unchanged. The symptom is live on the `da2ce7/torrust-tracker` fork: in the `Testing` run for `4bff469e` the `Docker E2E` job fails at step `Run E2E Tests` after a successful image build, with the three qBittorrent steps skipped - https://github.com/da2ce7/torrust-tracker/actions/runs/34444943476/job/102767609212 +- 2026-09-10 08:15 UTC - Spec author - GitHub issue #2179 created from the reviewed draft; specification moved to `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` - https://github.com/torrust/torrust-tracker/issues/2179 ## Acceptance Criteria @@ -182,6 +184,7 @@ After implementation, compare the result with this specification. Record invalid - Origin of the job's `if:` guard: #1854, spec at `docs/issues/closed/1854-1840-workflow-performance-container-test-gating/ISSUE.md` - Related EPIC: #1840, spec at `docs/issues/open/1840-improve-pr-workflow-performance-epic/EPIC.md` - Reported failing runs: #2055, #2106 +- Observed failing run: the `Docker E2E` job of the fork's `Testing` run for `4bff469e`, 2026-09-10 - https://github.com/da2ce7/torrust-tracker/actions/runs/34444943476/job/102767609212 - Commit `c1b6c5466` — added the `if:` guard and comment to the `docker-e2e` job - Commit `c47173f53` — moved the E2E binaries into `packages/e2e-tools`, breaking both workflows - Commit `2d1ce246b` — added `-p torrust-tracker-e2e-tools` to `container.yaml` only From b13e062b3c4d27a2edc75de6b4fae1b8524cb810 Mon Sep 17 00:00:00 2001 From: Cameron Garnham Date: Thu, 10 Sep 2026 10:44:15 +0200 Subject: [PATCH 4/6] docs(issues): [#2179] correct three review findings in the specification Qualify the "every push" evidence, correct the line-length arithmetic, and route manual verification through an issue-local evidence file. Both `testing.yaml` and `container.yaml` carry `paths-ignore` entries for `**/*.md` and `project-words.txt` on `push` and `pull_request`, so a push whose changed files are all documentation starts no run at all. The specification claimed the broken job fails on every feature-branch push, and that `container.yaml` proves the invocation form on every push to `develop`; both hold only for pushes carrying a non-documentation change. The title, the Background paragraph and both Risks entries now say so. The inserted reference form `-p torrust-tracker-e2e-tools` plus the space separating it from the following argument is 29 characters, not 26. The four `cargo run` lines therefore land at 213, 182, 180 and 185, not 210, 179, 177 and 182. Line 185 still exceeds the 200-character limit in `.yamllint-ci.yml`, so the `>-` block scalar the repair depends on is unchanged; only the documented numbers were wrong. The `create-issue` workflow requires manual-verification evidence in an issue-local `manual-verification-evidence.md` recording the steps, commands, output and outcome. The specification asked only for a run URL, which preserves neither what was done nor what was observed once the run's logs expire. T2, the in-scope bullet, the commit point, the checkpoint, the acceptance criterion, both manual scenarios and the AC2 evidence row now name that file, with the run URL carried inside it as the reference to the run. --- .../2179-docker-e2e-job-package-flag/ISSUE.md | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md b/docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md index 36717a15d..679bfc2dd 100644 --- a/docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md +++ b/docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md @@ -8,7 +8,7 @@ github-issue: 2179 spec-path: docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md branch: "2179-docker-e2e-job-package-flag-spec" related-pr: null -last-updated-utc: 2026-09-10 08:15 +last-updated-utc: 2026-09-10 08:42 semantic-links: skill-links: - create-issue @@ -19,11 +19,12 @@ semantic-links: - packages/e2e-tools/Cargo.toml - docs/issues/closed/1854-1840-workflow-performance-container-test-gating/ISSUE.md - docs/issues/open/1840-improve-pr-workflow-performance-epic/EPIC.md + - docs/templates/MANUAL-VERIFICATION-EVIDENCE.md --- -# Issue #2179 - Docker E2E job in testing.yaml fails on every feature-branch push: cargo run lacks the e2e-tools package flag +# Issue #2179 - Docker E2E job in testing.yaml fails on feature-branch pushes: cargo run lacks the e2e-tools package flag ## Goal @@ -46,9 +47,9 @@ The same four invocations in `.github/workflows/container.yaml` (lines 117, 123, The divergence has a precise origin, and it is not the guard. Commit `c1b6c5466` ("feat(ci): eliminate duplicate E2E tests and add test binary landscape analysis", 2026-06-03) added only the `if:` guard and its explanatory comment to `testing.yaml`, twelve added lines and no deletions; the runner steps already existed there and predate it. The break arrived the next day in commit `c47173f53` ("ci(container): slim nextest archive by extracting e2e and benchmark packages", 2026-06-04), which moved `src/bin/e2e_tests_runner.rs`, `src/bin/qbittorrent_e2e_runner.rs`, and `src/bin/profiling.rs` into the new `packages/e2e-tools` package as pure renames and touched neither workflow. At that moment both workflows were broken in the same way. Commit `2d1ce246b` ("ci(container): address Copilot review issues after T13/T14 extraction", 2026-06-04) then repaired `container.yaml` alone: its own message records fixing the four `cargo run --bin` invocations to include `-p torrust-tracker-e2e-tools` "so Cargo resolves the binaries after they were extracted out of `src/bin/` into the new e2e-tools package", and wrapping the long `e2e_tests_runner` step in a YAML `>-` block scalar to stay inside the yamllint line-length limit. The identical four invocations in `testing.yaml` were not part of that change and have carried the defect ever since. -What the guard did contribute is the silence. Because `docker-e2e` never runs on pushes to `develop` or on pull requests targeting `develop`, upstream CI has never executed the broken steps, so `develop` has stayed green while the job has failed on every feature-branch push since June 2026. The failure surfaces only where nobody routinely looks: it has been reported on the dependency-update pull requests #2055 and #2106, whose branch pushes do trigger the job, and it is observed directly on the `da2ce7/torrust-tracker` fork, most recently in the `Testing` run for the push of `2175-merge-tool-symlink-exceptions` at `4bff469e` on 2026-09-10, where the `Docker E2E` job builds the tracker image successfully and then fails at step `Run E2E Tests`, leaving the three qBittorrent steps skipped ([run 34444943476](https://github.com/da2ce7/torrust-tracker/actions/runs/34444943476/job/102767609212)). The #2055 and #2106 observations are carried here as reported evidence and were not re-checked. +What the guard did contribute is the silence. Because `docker-e2e` never runs on pushes to `develop` or on pull requests targeting `develop`, upstream CI has never executed the broken steps, so `develop` has stayed green while the job has failed since June 2026 on every feature-branch push that starts a `Testing` run at all. The qualification is the workflow's own path filter: `testing.yaml` carries `paths-ignore` entries for `**/*.md` and `project-words.txt` on both `push` and `pull_request`, so a push whose changed files are all documentation starts no run and produces no failure. Every feature-branch push that carries a non-documentation change reaches the broken steps. The failure surfaces only where nobody routinely looks: it has been reported on the dependency-update pull requests #2055 and #2106, whose branch pushes do trigger the job, and it is observed directly on the `da2ce7/torrust-tracker` fork, most recently in the `Testing` run for the push of `2175-merge-tool-symlink-exceptions` at `4bff469e` on 2026-09-10, where the `Docker E2E` job builds the tracker image successfully and then fails at step `Run E2E Tests`, leaving the three qBittorrent steps skipped ([run 34444943476](https://github.com/da2ce7/torrust-tracker/actions/runs/34444943476/job/102767609212)). The #2055 and #2106 observations are carried here as reported evidence and were not re-checked. -One detail of the repair is load-bearing rather than cosmetic. `.yamllint-ci.yml` sets `line-length: max: 200`, and `linter all` runs yamllint over the workflow files. Line 185 is currently 184 characters; adding `-p torrust-tracker-e2e-tools` together with the space separating it from the following argument costs 26 more characters and would take it to 210, over the limit. The other three lines are 153, 151, and 156 characters and land at 179, 177, and 182. The first step must therefore be wrapped in a `>-` block scalar, which is exactly the shape `container.yaml` already uses for the same step and for the same reason. +One detail of the repair is load-bearing rather than cosmetic. `.yamllint-ci.yml` sets `line-length: max: 200`, and `linter all` runs yamllint over the workflow files. Line 185 is currently 184 characters; inserting `-p torrust-tracker-e2e-tools` together with the space separating it from the following argument adds 29 characters and would take it to 213, over the limit. The other three lines are 153, 151, and 156 characters and land at 182, 180, and 185. The first step must therefore be wrapped in a `>-` block scalar, which is exactly the shape `container.yaml` already uses for the same step and for the same reason. ## Scope @@ -57,7 +58,7 @@ One detail of the repair is load-bearing rather than cosmetic. `.yamllint-ci.yml - Add `-p torrust-tracker-e2e-tools` to the four `cargo run` invocations in the `docker-e2e` job of `.github/workflows/testing.yaml`, at lines 185, 189, 193, and 197, keeping every other argument of each step byte-for-byte as it is today. - Wrap the `run-tracker-e2e-tests` step's command in a YAML `>-` block scalar so the added flag does not push the line past the 200-character yamllint limit, matching the form `container.yaml` uses for the same step. - Extend the job's existing comment with one sentence recording why the package flag is required: the workspace root is itself a package, so `--bin` alone resolves only against the root package's targets. -- Verify the fix by pushing the branch to a fork or another in-repo feature branch, where the guard admits the job, and recording the passing run URL in this specification. +- Verify the fix by pushing the branch to a fork or another in-repo feature branch, where the guard admits the job, and recording the verification in an issue-local `manual-verification-evidence.md` created from `docs/templates/MANUAL-VERIFICATION-EVIDENCE.md`: the steps performed, the commands run, the observed job and step output, and the outcome, with the passing run URL carried there as the reference to the run. ### Out of Scope @@ -86,14 +87,14 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. | ID | Status | Task | Notes / Expected Output | | --- | ------ | ------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | T1 | TODO | Add the package flag and amend the comment | The four `cargo run` lines in the `docker-e2e` job of `.github/workflows/testing.yaml` carry `-p torrust-tracker-e2e-tools`; the `run-tracker-e2e-tests` step uses a `>-` block scalar; the job comment gains one sentence explaining the flag; `linter all` exits `0`. | -| T2 | TODO | Verify on a feature-branch push | The branch is pushed to a fork or another in-repo feature branch, the `Docker E2E` job of `testing.yaml` runs, its four runner steps execute and pass, and the run URL is recorded as evidence for M1 and AC2. | +| T2 | TODO | Verify on a feature-branch push | The branch is pushed to a fork or another in-repo feature branch, the `Docker E2E` job of `testing.yaml` runs, and its four runner steps execute and pass. Issue-local `manual-verification-evidence.md` is created or updated with section V1: the steps performed, the commands run, the observed step output, the outcome, and the run URL as the reference. M1 and AC2 cite that section. | ## Commit Points | Task | Coherent change set | Commit policy | | ---- | --------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | | T1 | The workflow edit: four `cargo run` lines plus the block scalar and the comment sentence, in one file. | Commit after `linter all` passes. One signed Conventional Commit, scope `ci`. | -| T2 | Evidence only: progress-log entry and manual-scenario rows updated with the run URL, no workflow change. | Commit after the run completes, or fold into the pull request description if no spec update is warranted. | +| T2 | Evidence only: issue-local `manual-verification-evidence.md`, progress-log entry, and manual-scenario rows, no workflow change. | Commit after the run completes. One signed Conventional Commit, scope `docs`. | ## Progress Tracking @@ -105,7 +106,7 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. - [ ] (Optional, recommended for complex issues) Spec-only PR merged into `develop` before implementation - [ ] Implementation completed - [ ] Automatic verification completed (`linter all`, relevant tests, and any pre-push checks) -- [ ] Manual verification scenarios executed and recorded (status + evidence) +- [ ] Manual verification scenarios executed and recorded in issue-local `manual-verification-evidence.md` - [ ] Acceptance criteria reviewed after implementation and updated with evidence - [ ] Evidence-based implementation completion review recorded: issue-local retrospective created for material discoveries, or progress log states why none was needed - [ ] Reviewer validated acceptance criteria and updated checkboxes @@ -118,6 +119,7 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. - 2026-09-08 21:30 UTC - Spec author - Drafted this specification against `develop` at `e4db63d5`. Verified the four invocation line numbers in both workflows, the `if:` guard range, the binaries' location under `packages/e2e-tools/src/bin/`, and the root `Cargo.toml` `[package]`/`default-run` declaration. Traced the divergence to `c47173f53` (package extraction) and `2d1ce246b` (which repaired `container.yaml` only), not to `c1b6c5466`, which added only the guard. Measured the four line lengths against the 200-character limit in `.yamllint-ci.yml` and found that line 185 needs a `>-` block scalar. CI observations for #2055, #2106, and the `merge-tool-symlink-exceptions-spec` push are recorded as reported evidence; no GitHub access was available while drafting. - 2026-09-10 08:15 UTC - Spec author - Re-verified both evidence facts before opening the issue. Against `develop` at `89d45145` the four `cargo run` invocations in the `docker-e2e` job still carry no `-p` flag at lines 185, 189, 193, and 197, measuring 184, 153, 151, and 156 characters, so the block-scalar requirement on the first step holds unchanged. The symptom is live on the `da2ce7/torrust-tracker` fork: in the `Testing` run for `4bff469e` the `Docker E2E` job fails at step `Run E2E Tests` after a successful image build, with the three qBittorrent steps skipped - https://github.com/da2ce7/torrust-tracker/actions/runs/34444943476/job/102767609212 - 2026-09-10 08:15 UTC - Spec author - GitHub issue #2179 created from the reviewed draft; specification moved to `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` - https://github.com/torrust/torrust-tracker/issues/2179 +- 2026-09-10 08:42 UTC - Spec author - Applied three review findings from the specification pull request, each recomputed against `develop` at `89d45145` first. The "every push" claims are qualified: both `testing.yaml` and `container.yaml` ignore pushes whose changed files are all `**/*.md` or `project-words.txt`, so those start no run, and the title, the Background paragraph, and both Risks entries now say so. The line-length arithmetic is corrected: the inserted `-p torrust-tracker-e2e-tools` plus its separating space is 29 characters, not 26, so the four lines land at 213, 182, 180, and 185 rather than 210, 179, 177, and 182; line 185 still exceeds the 200-character limit and still needs the `>-` block scalar, so the repair shape is unchanged. Manual verification now produces an issue-local `manual-verification-evidence.md` from `docs/templates/MANUAL-VERIFICATION-EVIDENCE.md`, as `create-issue` requires, with the run URL carried there as a reference rather than standing in for the evidence. ## Acceptance Criteria @@ -127,7 +129,7 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. - [ ] AC4: The `docker-e2e` job comment records why the package flag is required. - [ ] `linter all` exits with code `0` - [ ] Relevant tests pass -- [ ] Manual verification scenarios are executed and documented (status + evidence) +- [ ] Manual verification scenarios are executed and documented in issue-local `manual-verification-evidence.md` - [ ] Acceptance criteria are re-reviewed after implementation and reflect actual behavior - [ ] Documentation is updated when behavior/workflow changes @@ -146,12 +148,13 @@ A pull request targeting `develop` cannot verify this fix: the `if:` guard skips | ID | Scenario | Command/Steps | Expected Result | Status | Evidence | | --- | ------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ----------------------------------------------------- | -| M1 | The job runs and passes on a feature-branch push | Push the fix branch to a fork or to an in-repo branch that is not `develop`, `main`, or `releases/*`, then open the `Testing` workflow run for that push. | The `Docker E2E` job runs; steps `run-tracker-e2e-tests`, `run-qbittorrent-e2e-test-sqlite3`, `run-qbittorrent-e2e-test-mysql`, and `run-qbittorrent-e2e-test-postgresql` all execute the runners and succeed; the job is green. | TODO | Run URL of the `Docker E2E` job | -| M2 | Behavior on `develop` is unchanged | Open the pull request for this fix against `develop` and inspect its `Testing` workflow run. | The `Docker E2E` job is skipped, as it is today, because `container.yaml` covers the event. No new job appears and no existing job changes status. | TODO | Run URL of the pull request's `Testing` workflow | +| M1 | The job runs and passes on a feature-branch push | Push the fix branch to a fork or to an in-repo branch that is not `develop`, `main`, or `releases/*`, then open the `Testing` workflow run for that push. | The `Docker E2E` job runs; steps `run-tracker-e2e-tests`, `run-qbittorrent-e2e-test-sqlite3`, `run-qbittorrent-e2e-test-mysql`, and `run-qbittorrent-e2e-test-postgresql` all execute the runners and succeed; the job is green. | TODO | `manual-verification-evidence.md` section V1, carrying the run URL of the `Docker E2E` job | +| M2 | Behavior on `develop` is unchanged | Open the pull request for this fix against `develop` and inspect its `Testing` workflow run. | The `Docker E2E` job is skipped, as it is today, because `container.yaml` covers the event. No new job appears and no existing job changes status. | TODO | `manual-verification-evidence.md` section V2, carrying the run URL of the pull request's `Testing` workflow | Notes: - Manual verification is mandatory even when automated tests pass. +- Create `manual-verification-evidence.md` in this issue folder from `docs/templates/MANUAL-VERIFICATION-EVIDENCE.md` when executing these scenarios, and record there the actual prerequisites, steps, commands, job and step output, and outcome. A run URL is a reference to that evidence, not the evidence itself: the run's logs expire and the URL alone preserves neither what was done nor what was observed. - If a scenario fails, record the failure and diagnosis in the progress log before proceeding. ### Acceptance Verification @@ -159,16 +162,16 @@ Notes: | AC ID | Status (`TODO`/`DONE`) | Evidence | | ----- | ---------------------- | --------------------------------------------------- | | AC1 | TODO | Diff of `.github/workflows/testing.yaml` | -| AC2 | TODO | M1 run URL | +| AC2 | TODO | `manual-verification-evidence.md` section V1 (M1) | | AC3 | TODO | Pull request file list | | AC4 | TODO | Diff of `.github/workflows/testing.yaml` | ## Risks and Trade-offs -- No material risk to the change itself. The flag value is not a guess: `container.yaml` runs the same four binaries with the same flag against the same package on every push to `develop`, so the invocation form is already proven in CI. +- No material risk to the change itself. The flag value is not a guess: `container.yaml` runs the same four binaries with the same flag against the same package on every push to `develop` that its own `paths-ignore` filter admits, so the invocation form is already proven in CI. - The line-length rule is the only way this edit can go wrong mechanically, and `linter all` catches it before the change leaves the working tree. -- Verification depends on an event upstream CI does not produce, so the passing run URL must be captured deliberately rather than expected to appear on the pull request. M1 exists for that reason. -- Restoring the job means feature-branch pushes will start spending E2E time they have not spent since June 2026. That is the job's intended cost, not a regression, and the job already carries `timeout-minutes: 90`. +- Verification depends on an event upstream CI does not produce, so the run and the evidence recorded from it must be captured deliberately rather than expected to appear on the pull request. M1 exists for that reason. +- Restoring the job means feature-branch pushes that carry a non-documentation change, the ones the workflow's `paths-ignore` filter admits, will start spending E2E time they have not spent since June 2026. That is the job's intended cost, not a regression, and the job already carries `timeout-minutes: 90`. ## Implementation Completion Review From 1d920d5c812f781e3909753dc226219032c395e0 Mon Sep 17 00:00:00 2001 From: Cameron Garnham Date: Thu, 10 Sep 2026 10:46:36 +0200 Subject: [PATCH 5/6] docs(review): document PR #2194 copilot suggestions audit --- .../pr-2194-copilot-suggestions.md | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 docs/copilot-pr-reviews/pr-2194-copilot-suggestions.md diff --git a/docs/copilot-pr-reviews/pr-2194-copilot-suggestions.md b/docs/copilot-pr-reviews/pr-2194-copilot-suggestions.md new file mode 100644 index 000000000..c7a99d47c --- /dev/null +++ b/docs/copilot-pr-reviews/pr-2194-copilot-suggestions.md @@ -0,0 +1,46 @@ +--- +semantic-links: + skill-links: + - process-copilot-suggestions + related-artifacts: + - .github/skills/dev/pr-reviews/process-copilot-suggestions/SKILL.md +--- + + + + + +# PR #2194 Copilot Suggestions Tracking + +Source: Copilot PR review threads for + +Status legend: + +- `action`: code/docs change applied +- `no-action`: suggestion reviewed; no code change needed +- `resolved`: thread resolved in PR + +## Processing Log + +- 2026-09-10 08:35 UTC: Started processing suggestions (three unresolved threads, all on `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md`, the only file the pull request adds). +- 2026-09-10 08:38 UTC: Recomputed all three claims against `develop` at `89d45145` before deciding any of them. The path filters were read from the `on:` blocks of both workflows, the inserted string was measured and the four line lengths recomputed from the workflow file itself, and the manual-evidence requirement was read from the `create-issue` skill. All three claims hold. +- 2026-09-10 08:44 UTC: Accepted all three and applied them in commit `b13e062b`: the "every push" claims are qualified by the path filters, the line-length arithmetic is corrected to 29 characters and 213, 182, 180, 185, and manual verification now produces an issue-local `manual-verification-evidence.md`. +- 2026-09-10 08:46 UTC: Gated commit `b13e062b` on the build server: `git diff --check` over the branch range against `develop` reports no whitespace defects, and `linter all` exits `0` in 18.2 s with markdown, link, YAML, TOML, spelling, clippy, formatting and shell checks all green. + +## Suggestions + +| # | Thread ID | Path | URL | Suggestion Summary | Decision | Reply URL | Status | Thread State | +| --- | ----------------------- | -------------------------------------------------------- | ----------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- | ------- | ------------ | +| 1 | `PRRT_kwDOGp2yqc6g_vID` | `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` | | Both workflows ignore pushes whose changed files are only Markdown or `project-words.txt`, so the job does not fail on literally every feature-branch push. | action — commit `b13e062b`. The premise checks out in both workflows' `on:` blocks at `89d45145`: each carries `paths-ignore` for `**/*.md` and `project-words.txt` on `push` and on `pull_request`, and a push whose changed files all match starts no run, so there is no job to fail. The Background paragraph now states the qualification and its cause, the title drops the quantifier, and the second flagged claim, that `container.yaml` proves the invocation form on every push to `develop`, is qualified the same way. The `container.yaml` half of the report is in fact stronger than stated: that workflow's `push` trigger is also restricted to `develop`, `main` and `releases/**/*`, so it never runs on a feature-branch push at all. | pending | APPLIED | OPEN | +| 2 | `PRRT_kwDOGp2yqc6g_vIX` | `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` | | The character arithmetic is off by three: the inserted reference form adds 29 characters, so the lengths become 213, 182, 180 and 185. | action — commit `b13e062b`. The reported numbers are exactly right. the reference form, `-p torrust-tracker-e2e-tools` followed by the separating space, measures 29 characters, and the four lines of `.github/workflows/testing.yaml` at `89d45145` measure 184, 153, 151 and 156, so they land at 213, 182, 180 and 185. The specification said 26 and 210, 179, 177, 182. The conclusion the numbers support is unaffected, because 213 still exceeds the 200-character limit in `.yamllint-ci.yml` and the other three still fall inside it, so the `>-` block scalar on the first step stands. The same wrong figure in the pull request description is corrected alongside. | pending | APPLIED | OPEN | +| 3 | `PRRT_kwDOGp2yqc6g_vIh` | `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` | | `create-issue` requires manual-verification evidence in an issue-local `manual-verification-evidence.md`; a passing run URL alone does not preserve it. | action — commit `b13e062b`. The requirement is in the `create-issue` skill verbatim and in the issue template's own rows, and this specification did not meet it: it asked for a run URL and nothing else. T2, the in-scope verification bullet, the T2 commit point, the workflow checkpoint, the manual-verification acceptance criterion, both manual scenarios and the AC2 evidence row now name the issue-local file and the section each scenario writes, with the run URL carried there as the reference to the run rather than standing in for the evidence. The template is added to the specification's related artifacts. | pending | APPLIED | OPEN | + +## Notes + +- Keep this file as an audit log of review handling for the PR. +- Reply on every PR suggestion thread before resolving it so the decision is visible to reviewers. The replies for this round are posted after this commit, so the `Reply URL`, `Status` and `Thread State` columns are filled in a follow-up commit once each thread carries its reply. +- Suggestion 1 evidence: `testing.yaml` at `89d45145` declares `on: push: paths-ignore: ["**/*.md", "project-words.txt"]` and the same list under `pull_request`; `container.yaml` declares the identical two entries under both events, and additionally restricts `push` to branches `develop`, `main` and `releases/**/*` and `pull_request` to `develop` and `main`. A push whose changed files all match a `paths-ignore` pattern does not start the workflow, so the `docker-e2e` job cannot fail on it. The specification's own evidence run remains valid: the `4bff469e` push it cites carried workflow and source changes, not documentation alone. +- Suggestion 2 evidence: `len('-p torrust-tracker-e2e-tools ')` is 29, the flag being 28 characters and the separating space one more. Lines 185, 189, 193 and 197 of `.github/workflows/testing.yaml` at `89d45145` measure 184, 153, 151 and 156 characters, which the specification already recorded correctly, so the sums are 213, 182, 180 and 185. Against `line-length: max: 200` in `.yamllint-ci.yml` only the first exceeds the limit, which is what the block-scalar requirement rests on. +- Suggestion 3 evidence: the `create-issue` skill requires "manual verification scenarios that describe real human-oriented feature use or bug reproduction, with status tracked in the spec and actual commands, output, and relevant logs recorded in issue-local `manual-verification-evidence.md`". `docs/templates/ISSUE.md` carries the same requirement in its workflow checkpoints, its acceptance criteria, its manual-scenario evidence cells and its notes, and `docs/templates/MANUAL-VERIFICATION-EVIDENCE.md` is the file's template, with sections for environment, steps performed, observed result and conclusion. +- Follow-up outside this pull request: GitHub issue #2179 still carries the unqualified title "Docker E2E job in testing.yaml fails on every feature-branch push"; the specification heading no longer does, and the issue title needs the same edit to stay in step. +- Validation: `git diff --check` over the branch range against `develop` reports no whitespace defects, and `linter all` exits `0` on the build server, first at commit `b13e062b` and then again at the branch tip carrying this record, so each commit is green on its own. The run covers this file and the specification through markdownlint, lychee, cspell, YAML, TOML and the shell checks. From da4ffb79a57958046b44d24ebd6ff034b71702f9 Mon Sep 17 00:00:00 2001 From: Cameron Garnham Date: Thu, 10 Sep 2026 10:52:37 +0200 Subject: [PATCH 6/6] docs(review): record PR #2194 copilot reply links --- docs/copilot-pr-reviews/pr-2194-copilot-suggestions.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/copilot-pr-reviews/pr-2194-copilot-suggestions.md b/docs/copilot-pr-reviews/pr-2194-copilot-suggestions.md index c7a99d47c..a1eefe7f5 100644 --- a/docs/copilot-pr-reviews/pr-2194-copilot-suggestions.md +++ b/docs/copilot-pr-reviews/pr-2194-copilot-suggestions.md @@ -26,19 +26,21 @@ Status legend: - 2026-09-10 08:38 UTC: Recomputed all three claims against `develop` at `89d45145` before deciding any of them. The path filters were read from the `on:` blocks of both workflows, the inserted string was measured and the four line lengths recomputed from the workflow file itself, and the manual-evidence requirement was read from the `create-issue` skill. All three claims hold. - 2026-09-10 08:44 UTC: Accepted all three and applied them in commit `b13e062b`: the "every push" claims are qualified by the path filters, the line-length arithmetic is corrected to 29 characters and 213, 182, 180, 185, and manual verification now produces an issue-local `manual-verification-evidence.md`. - 2026-09-10 08:46 UTC: Gated commit `b13e062b` on the build server: `git diff --check` over the branch range against `develop` reports no whitespace defects, and `linter all` exits `0` in 18.2 s with markdown, link, YAML, TOML, spelling, clippy, formatting and shell checks all green. +- 2026-09-10 09:05 UTC: Pushed the two commits and replied on each thread with its decision and evidence, resolving each immediately after its reply, and posted a summary comment naming this record - +- 2026-09-10 09:05 UTC: Re-fetched the pull request's review threads and confirmed all three are resolved and none remains open. ## Suggestions | # | Thread ID | Path | URL | Suggestion Summary | Decision | Reply URL | Status | Thread State | | --- | ----------------------- | -------------------------------------------------------- | ----------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- | ------- | ------------ | -| 1 | `PRRT_kwDOGp2yqc6g_vID` | `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` | | Both workflows ignore pushes whose changed files are only Markdown or `project-words.txt`, so the job does not fail on literally every feature-branch push. | action — commit `b13e062b`. The premise checks out in both workflows' `on:` blocks at `89d45145`: each carries `paths-ignore` for `**/*.md` and `project-words.txt` on `push` and on `pull_request`, and a push whose changed files all match starts no run, so there is no job to fail. The Background paragraph now states the qualification and its cause, the title drops the quantifier, and the second flagged claim, that `container.yaml` proves the invocation form on every push to `develop`, is qualified the same way. The `container.yaml` half of the report is in fact stronger than stated: that workflow's `push` trigger is also restricted to `develop`, `main` and `releases/**/*`, so it never runs on a feature-branch push at all. | pending | APPLIED | OPEN | -| 2 | `PRRT_kwDOGp2yqc6g_vIX` | `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` | | The character arithmetic is off by three: the inserted reference form adds 29 characters, so the lengths become 213, 182, 180 and 185. | action — commit `b13e062b`. The reported numbers are exactly right. the reference form, `-p torrust-tracker-e2e-tools` followed by the separating space, measures 29 characters, and the four lines of `.github/workflows/testing.yaml` at `89d45145` measure 184, 153, 151 and 156, so they land at 213, 182, 180 and 185. The specification said 26 and 210, 179, 177, 182. The conclusion the numbers support is unaffected, because 213 still exceeds the 200-character limit in `.yamllint-ci.yml` and the other three still fall inside it, so the `>-` block scalar on the first step stands. The same wrong figure in the pull request description is corrected alongside. | pending | APPLIED | OPEN | -| 3 | `PRRT_kwDOGp2yqc6g_vIh` | `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` | | `create-issue` requires manual-verification evidence in an issue-local `manual-verification-evidence.md`; a passing run URL alone does not preserve it. | action — commit `b13e062b`. The requirement is in the `create-issue` skill verbatim and in the issue template's own rows, and this specification did not meet it: it asked for a run URL and nothing else. T2, the in-scope verification bullet, the T2 commit point, the workflow checkpoint, the manual-verification acceptance criterion, both manual scenarios and the AC2 evidence row now name the issue-local file and the section each scenario writes, with the run URL carried there as the reference to the run rather than standing in for the evidence. The template is added to the specification's related artifacts. | pending | APPLIED | OPEN | +| 1 | `PRRT_kwDOGp2yqc6g_vID` | `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` | | Both workflows ignore pushes whose changed files are only Markdown or `project-words.txt`, so the job does not fail on literally every feature-branch push. | action — commit `b13e062b`. The premise checks out in both workflows' `on:` blocks at `89d45145`: each carries `paths-ignore` for `**/*.md` and `project-words.txt` on `push` and on `pull_request`, and a push whose changed files all match starts no run, so there is no job to fail. The Background paragraph now states the qualification and its cause, the title drops the quantifier, and the second flagged claim, that `container.yaml` proves the invocation form on every push to `develop`, is qualified the same way. The `container.yaml` half of the report is in fact stronger than stated: that workflow's `push` trigger is also restricted to `develop`, `main` and `releases/**/*`, so it never runs on a feature-branch push at all. | | DONE | RESOLVED | +| 2 | `PRRT_kwDOGp2yqc6g_vIX` | `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` | | The character arithmetic is off by three: the inserted reference form adds 29 characters, so the lengths become 213, 182, 180 and 185. | action — commit `b13e062b`. The reported numbers are exactly right. the reference form, `-p torrust-tracker-e2e-tools` followed by the separating space, measures 29 characters, and the four lines of `.github/workflows/testing.yaml` at `89d45145` measure 184, 153, 151 and 156, so they land at 213, 182, 180 and 185. The specification said 26 and 210, 179, 177, 182. The conclusion the numbers support is unaffected, because 213 still exceeds the 200-character limit in `.yamllint-ci.yml` and the other three still fall inside it, so the `>-` block scalar on the first step stands. The same wrong figure in the pull request description is corrected alongside. | | DONE | RESOLVED | +| 3 | `PRRT_kwDOGp2yqc6g_vIh` | `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` | | `create-issue` requires manual-verification evidence in an issue-local `manual-verification-evidence.md`; a passing run URL alone does not preserve it. | action — commit `b13e062b`. The requirement is in the `create-issue` skill verbatim and in the issue template's own rows, and this specification did not meet it: it asked for a run URL and nothing else. T2, the in-scope verification bullet, the T2 commit point, the workflow checkpoint, the manual-verification acceptance criterion, both manual scenarios and the AC2 evidence row now name the issue-local file and the section each scenario writes, with the run URL carried there as the reference to the run rather than standing in for the evidence. The template is added to the specification's related artifacts. | | DONE | RESOLVED | ## Notes - Keep this file as an audit log of review handling for the PR. -- Reply on every PR suggestion thread before resolving it so the decision is visible to reviewers. The replies for this round are posted after this commit, so the `Reply URL`, `Status` and `Thread State` columns are filled in a follow-up commit once each thread carries its reply. +- Reply on every PR suggestion thread before resolving it so the decision is visible to reviewers. All three threads were replied to before being resolved, one at a time, and the round is summarised in a pull request comment that names this record - - Suggestion 1 evidence: `testing.yaml` at `89d45145` declares `on: push: paths-ignore: ["**/*.md", "project-words.txt"]` and the same list under `pull_request`; `container.yaml` declares the identical two entries under both events, and additionally restricts `push` to branches `develop`, `main` and `releases/**/*` and `pull_request` to `develop` and `main`. A push whose changed files all match a `paths-ignore` pattern does not start the workflow, so the `docker-e2e` job cannot fail on it. The specification's own evidence run remains valid: the `4bff469e` push it cites carried workflow and source changes, not documentation alone. - Suggestion 2 evidence: `len('-p torrust-tracker-e2e-tools ')` is 29, the flag being 28 characters and the separating space one more. Lines 185, 189, 193 and 197 of `.github/workflows/testing.yaml` at `89d45145` measure 184, 153, 151 and 156 characters, which the specification already recorded correctly, so the sums are 213, 182, 180 and 185. Against `line-length: max: 200` in `.yamllint-ci.yml` only the first exceeds the limit, which is what the block-scalar requirement rests on. - Suggestion 3 evidence: the `create-issue` skill requires "manual verification scenarios that describe real human-oriented feature use or bug reproduction, with status tracked in the spec and actual commands, output, and relevant logs recorded in issue-local `manual-verification-evidence.md`". `docs/templates/ISSUE.md` carries the same requirement in its workflow checkpoints, its acceptance criteria, its manual-scenario evidence cells and its notes, and `docs/templates/MANUAL-VERIFICATION-EVIDENCE.md` is the file's template, with sections for environment, steps performed, observed result and conclusion.