Skip to content

ci: run the 3.5.0 testsuite over loopback TCP on every pull request - #7408

Merged
oferchen merged 1 commit into
masterfrom
ci/wire-350-tcp-legs
Aug 20, 2026
Merged

ci: run the 3.5.0 testsuite over loopback TCP on every pull request#7408
oferchen merged 1 commit into
masterfrom
ci/wire-350-tcp-legs

Conversation

@oferchen

Copy link
Copy Markdown
Owner

Problem

The 3.5.0 conformance suite has four legs - {nonroot,root} x {pipe,tcp} - but
only the two pipe legs ran on pull requests. The tcp legs existed solely as
standalone badge workflows on workflow_dispatch + cron, so no pull request
ever validated them: a regression could only surface the next morning, already
merged.

The two transports are not redundant. The pipe leg uses upstream's secure
default - a stdio socketpair with no listening socket - so it never reaches
accept(), the hosts allow/hosts deny decision, or peer-address
resolution. Those are precisely the surfaces the 3.5.0 daemon fixes live on,
which is the axis a pipe-only gate cannot see.

Change

Call the existing reusable workflow a second time with transport: tcp under
job id upstream-testsuite-tcp, and add the matching pair of ci-skip stubs.

Published contexts go from 2 to 4:

pipe tcp
non-root upstream-testsuite / upstream testsuite upstream-testsuite-tcp / upstream testsuite
root upstream-testsuite / upstream testsuite (root) upstream-testsuite-tcp / upstream testsuite (root)

The tcp legs pass --daemon-tests-only, which is the pairing upstream ships
the option for, so they select the 155 transport-observing tests rather than
all 345 and carry their own manifests sized to match. The driver derives that
expected size from upstream's own select_daemon_tests(), so a short manifest
is still refused.

ci-skip.yml reuses the same stub workflow rather than a copy - the job id
supplies the differing prefix, and one file stays the owner of "which contexts
the testsuite publishes". A copy is what drifts when a third leg is added.

Both halves ship in one commit deliberately: a new context in ci.yml without
its skip-path stand-in leaves any doc-only pull request waiting forever on a
context that never arrives.

Verification

Validated by extracting the committed YAML back out with yaml.safe_load and
computing the published contexts from both files, rather than eyeballing the
diff:

  • ci.yml and ci-skip.yml publish the same 4 contexts, exact string match.
  • Non-vacuity proven: renaming the skip job to ...-TYPO makes the parity
    check report a mismatch, so it can actually fail.
  • No job name: in the reusable workflow is an expression - a skipped job would
    otherwise report the raw expression text and the context would never arrive.
  • Every with: key the caller passes is a declared input of the callee.
  • All four manifest paths exist; the tcp pair is 155 rows vs the pipe pair's
    345, confirming the daemon-only set is wired and not a copy of the pipe files.

Scope

Not registered as required status checks - that is a separate repo-admin
action. Until then these report without blocking, which still replaces "never
measured on a pull request" with "visible on every pull request".

The four 3.5.0 legs are {nonroot,root} x {pipe,tcp}, but only the two pipe
legs ran on pull requests. The tcp legs existed solely as standalone badge
workflows on workflow_dispatch + cron, so no pull request ever validated
them and a regression could only surface the next morning, already merged.

The transports are not redundant. The pipe leg uses upstream's secure
default - a stdio socketpair with no listening socket - so it never reaches
accept(), the hosts allow/hosts deny decision, or peer-address resolution.
Those are the surfaces the 3.5.0 daemon fixes live on, which is exactly the
axis a pipe-only gate cannot see.

Call the existing reusable workflow a second time with transport: tcp under
job id `upstream-testsuite-tcp`, so it reports as
`upstream-testsuite-tcp / upstream testsuite` and `... (root)`. The tcp legs
pass --daemon-tests-only and therefore select 155 transport-observing tests
rather than all 345, and carry their own manifests sized to match; the
driver derives that expected size from upstream's own select_daemon_tests(),
so a short manifest is still refused.

ci-skip.yml gains the matching pair from the SAME stub workflow rather than
a copy - the job id supplies the differing prefix. Both halves ship in one
commit: a new context in ci.yml without its skip-path stand-in leaves any
doc-only pull request waiting forever on a context that never arrives.

Not yet registered as required status checks; that is a separate repo-admin
action. Until then these report without blocking, which still replaces
"never measured on a pull request" with "visible on every pull request".
@github-actions github-actions Bot added the ci label Aug 20, 2026
@oferchen
oferchen merged commit eb5d591 into master Aug 20, 2026
61 checks passed
@oferchen
oferchen deleted the ci/wire-350-tcp-legs branch August 20, 2026 20:57
oferchen added a commit that referenced this pull request Aug 24, 2026
…7408)

The four 3.5.0 legs are {nonroot,root} x {pipe,tcp}, but only the two pipe
legs ran on pull requests. The tcp legs existed solely as standalone badge
workflows on workflow_dispatch + cron, so no pull request ever validated
them and a regression could only surface the next morning, already merged.

The transports are not redundant. The pipe leg uses upstream's secure
default - a stdio socketpair with no listening socket - so it never reaches
accept(), the hosts allow/hosts deny decision, or peer-address resolution.
Those are the surfaces the 3.5.0 daemon fixes live on, which is exactly the
axis a pipe-only gate cannot see.

Call the existing reusable workflow a second time with transport: tcp under
job id `upstream-testsuite-tcp`, so it reports as
`upstream-testsuite-tcp / upstream testsuite` and `... (root)`. The tcp legs
pass --daemon-tests-only and therefore select 155 transport-observing tests
rather than all 345, and carry their own manifests sized to match; the
driver derives that expected size from upstream's own select_daemon_tests(),
so a short manifest is still refused.

ci-skip.yml gains the matching pair from the SAME stub workflow rather than
a copy - the job id supplies the differing prefix. Both halves ship in one
commit: a new context in ci.yml without its skip-path stand-in leaves any
doc-only pull request waiting forever on a context that never arrives.

Not yet registered as required status checks; that is a separate repo-admin
action. Until then these report without blocking, which still replaces
"never measured on a pull request" with "visible on every pull request".
oferchen added a commit that referenced this pull request Sep 1, 2026
* docs: align the upstream reference and testsuite gate to 3.5.0

The gate moved to the rsync 3.5.0 Python corpus on 2026-08-19 (#7387) and the
TCP legs followed on 2026-08-20 (#7408), but README.md and SECURITY.md still
described a 3.4.4-gated world and carried outcome figures that no longer match
the committed manifests.

Every number below is counted from tools/ci/upstream-3.5.0-expect.*.txt with

  awk '!/^#/ && NF {c[$NF]++; t++} END {print t, c["pass"], c["fail"], c["skip"]}'

  leg              pass  fail  skip  corpus   was
  non-root, pipe    254     6    85     345   252/8/89 of 349
  root, pipe        282     7    56     345   280/9/60 of 349
  non-root, tcp     101    21    33     155   100/22/36 of 158
  root, tcp         113    27    15     155   112/28/18 of 158

The corpus column was the durable error: it was already 345/155 when #7533 last
refreshed these tables, so the skip and corpus columns were carried forward
untouched while only the pass/fail pair was re-measured. Both docs now name the
command that produces the row, so the next refresh is a re-run rather than a
transcription. The divergence unions fall the same way: 7 distinct tests across
the two full-corpus legs, not 9, and 32 across all four, not 34.

Other corrections:

- The 3.4.4 shell corpus is not run by any workflow. _interop.yml states
  explicitly that it does not drive it, and run_upstream_testsuite.sh defaults
  to UPSTREAM_VERSION=3.5.0. "Against 3.4.4 all tests pass and the roster is
  empty" was reporting an empty roster as a measurement.
- The interop version lists in two README sections were hand copies of
  versions= in tools/ci/run_interop.sh, which is what drifted last time. They
  now point at the script; the per-version protocol table stays, since it
  carries columns the script does not.
- The nightly 3.5.0dev tracker badge is removed from the badge row. Every other
  badge there is a gate, and "3.5.0dev" one glyph from "3.5.0" read as a fifth
  one. The workflow and its explanatory paragraph both stay - it tracks the
  moving RsyncProject master, which the release gate cannot see - but its header
  comment claimed the required gate was still pinned to 3.4.4, which is false.

CHANGELOG: the Unreleased section stopped at #7522. Added the 51 pull requests
merged since, grouped into the existing sections and derived from their titles
via git log v0.6.4..origin/master.

* docs: name the one dispatch-only workflow that still pins 3.4.4
oferchen added a commit that referenced this pull request Sep 4, 2026
… them (#7661)

`README.md` ships the exact `awk` command that reads a leg's expected-outcome
manifest and prints its pass/fail/skip counts. Running it disagrees with every
row of the table directly beneath it.

| row | documented | re-derived |
|---|---|---|
| non-root, pipe | 257 / 3 / 85 | **259 / 1 / 85** |
| root, pipe | 286 / 3 / 56 | **288 / 1 / 56** |
| non-root, tcp | 101 / 21 / 33 | **108 / 14 / 33** |
| root, tcp | 113 / 27 / 15 | **120 / 20 / 15** |

`SECURITY.md` carries the same four rows and the same two summary figures, so
both files drifted together. Corrected throughout, along with:

- **"3 of 345 tests currently diverge"** across the full-corpus legs. It is
  **one** - `filter-merge-content-echo` - and the README names it now rather
  than leaving the reader to count.
- **"29 across all four"** distinct failures. It is **23**, and across
  **five** manifests, not four: the README's own glob
  `tools/ci/upstream-3.5.0-expect.*.txt` has always matched the macOS file its
  prose excluded.
- **The macOS leg was missing from both tables.** It runs on every PR as
  `upstream-testsuite-macos` (`ci.yml`), on the full 345-cell corpus, with its
  own committed manifest. It is the only leg that can observe a
  platform-conditional divergence - one of its three remaining failures *skips*
  on Linux, so it had never executed in this repository's CI before the leg
  existed. Added as a fifth row, with its non-required status stated.
- **"the required checks being the two stdio-pipe legs."** All four Linux legs
  have been required contexts since #7408 wired the TCP pair into PR CI; the
  ruleset returns ten contexts, not eight.
- **`proxy protocol hosts` described as "not yet implemented"** and "still
  under audit" in two places. It shipped in #7648: parsed into a
  `ProxyProtocolPolicy` that mirrors upstream's `allow_proxy_protocol_peer()`,
  rejecting every peer when the trusted list is empty or unset, and warning at
  startup on the combination upstream warns about.
- **The `MAX_PROXY_LINE_BYTES` citation** pointed at `connect/proxy.rs:344`.
  The constant moved and, more usefully, stopped being a typed literal: it is
  now `PROXY_BUF_SIZE - 1`, so the doc records the derivation rather than a
  line number that will drift again.

The two macOS-leg rationale comments in the workflows carried the same pre-fix
counts and a failure list six entries out of date. Both are recounted from the
manifests.

`CHANGELOG.md` stopped at #7632, leaving the 26 PRs merged since then
unrecorded. Added under Security / Fixed / Testing and CI / Documentation.

## Why the numbers were wrong in a way reading could not catch

Every figure here is the outcome column of a committed manifest. The previous
values were transcribed once and then maintained by hand, so they decayed as
fixes landed - and a reader checking the table against the prose beside it
would find them perfectly consistent with each other.

The check that finds this class is re-running the derivation, not re-reading
the text, so that is what was done: a script recomputes all five legs and both
summary figures from `tools/ci/upstream-3.5.0-expect*.txt` and asserts the two
documents contain the results, with the superseded strings blacklisted so a
partial edit cannot pass. It also caught a live error in this changeset -
#7659 landed mid-review and flipped `operator-path-partial-dir-daemon`, taking
macOS from 236/4 to 237/3 and the distinct-failure count from 24 to 23. The
figures here are derived from master with that merge in place.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant