envoy.github.release: three correctness fixes (version regex, status-before-stream, exception chain)

[Copilot]

Bundles three high-priority correctness bugs from the py/envoy.github.release code review.

Fixes

8.3 — Version regex (HIGH)

_version_re = r"v(\w+)" — \w doesn't match ., so v1.19.0 parsed as Version("1") via sub, silently collapsing every minor into the same bucket in GithubReleaseManager.latest.

# Before
_version_re = r"v(\w+)"
_version = self.version_re.sub(r"\1", version)   # "v1.19.0" → wrong

# After
_version_re = r"^v(\d+\.\d+\.\d+.*)$"
m = self.version_re.fullmatch(version)
return packaging.version.Version(m.group(1)) if m else None

4.2 — Status check before streaming

GithubReleaseAssetsFetcher.save streamed the response body to disk before checking download.status, writing 404 HTML to the asset path and only then reporting an error. Status is now checked first; the writer is never opened on a non-2xx response. The returned dict shape (name, outfile, optional error) is unchanged.

6.1 — Exception chain on concurrent push

raise e.args[0] → raise e.args[0] from e in GithubRelease.push so the ConcurrentError context is preserved in tracebacks.

Tests

• test_release_manager_parse_version: updated to mock fullmatch instead of sub
• test_release_manager_parse_version_regex: new parametrized integration test (v1.19.0, v0.0.1, pre-releases, v1, notaversion)
• test_release_manager_latest_minors: new integration test asserting ["v1.18.0", "v1.19.0", "v1.19.1", "v2.0.0"] produces correct per-minor buckets
• test_fetcher_save: asserts stream.writer is not called on non-200 responses
• test_release_push: asserts exc_info.value.__cause__ is the original ConcurrentError

Original prompt

Bundle the remaining high-priority correctness findings from the code review in PR #4479 (py/envoy.distribution.release/REVIEW.md) into a single PR. All three target py/envoy.github.release. No behavioural changes beyond the bug fixes; add tests for each.

1. Fix version regex (finding 8.3 — HIGH risk)

File: py/envoy.github.release/envoy/github/release/manager.py (around L25)

Currently:

_version_re = r"v(\w+)"

Python's \w does not match ., so when parse_version("v1.19.0") runs:

_version = self.version_re.sub(r"\1", version)  # "v1.19.0" -> "1"
return packaging.version.Version(_version)       # Version("1")

…every release under a given major collapses to Version("1"), breaking the latest dict in GithubReleaseManager.latest — any "find latest minor" flow in FetchCommand silently returns the wrong release. This is a real production correctness bug.

Fix: use a proper semver-ish pattern with re.fullmatch semantics. Replace the _version_re with something like:

_version_re = r"^v(\d+\.\d+\.\d+.*)$"

…and update parse_version to use fullmatch and extract group 1 directly (rather than sub(r"\1", ...) which silently passes the original string through on non-match). On non-match, log the warning and return None (preserving current behaviour for unparseable tags). Make sure the existing _version_format = "v{version}" round-trip still works.

Tests: add a parametrized test in py/envoy.github.release/tests/test_manager.py for parse_version covering at minimum:

• v1.19.0 → Version("1.19.0")
• v0.0.1 → Version("0.0.1")
• v1.2.3-rc1 / v1.2.3.dev0 → correct Version (or None if you choose to reject pre-releases — document the choice)
• v1 → None (or whatever the chosen regex implies — assert explicitly)
• notaversion → None
• Also add/extend a test for GithubReleaseManager.latest that exercises tags like ["v1.18.0", "v1.19.0", "v1.19.1", "v2.0.0"] and asserts that the resulting latest dict contains the right Version objects under keys "1.18", "1.19", "2.0", "1.18.0", "1.19.0", "1.19.1", "2.0.0" — i.e. that minors are no longer collapsed.

2. Check HTTP status before streaming the download body (finding 4.2)

File: py/envoy.github.release/envoy/github/release/assets.py GithubReleaseAssetsFetcher.save (around L55–L71)

Currently the response body is streamed to the output file via stream.writer before download.status is checked, so a 404 HTML body is written to the user's asset path and only afterwards reported as an error.

Fix: check download.status first; if non-2xx, call self.fail(...) with the message and return the error result without opening/creating the outfile. Only stream to disk on success.

Keep the returned dict shape identical (name, outfile, optional error) so callers in release.py:fetch() and format_response are unaffected. Note: on the error path, outfile should still be set to the planned path (so existing call sites don't KeyError), but the file must NOT have been created.

Tests: update/extend py/envoy.github.release/tests/test_assets.py to:

• assert that on a non-200 response, stream.writer is not called (and no file is created on disk in a tmp_path test)
• assert that the returned dict still has error set and name/outfile keys present
• keep the existing happy-path test passing

3. Preserve exception chain on concurrent push (finding 6.1)

File: py/envoy.github.release/envoy/github/release/release.py (around L200–L201) in GithubRelease.push:

except ConcurrentError as e:
    raise e.args[0]

Fix: raise e.args[0] from e so the ConcurrentError context is preserved in the traceback.

Tests: update the corresponding test in py/envoy.github.release/tests/test_release.py for GithubRelease.push to assert the raised exception's __cause__ is the original ConcurrentError instance.

General

• Do not touch packaging metadata, dep bounds, or anything else outside the three findings above. Keep the diff tight.
• Add a brief note to py/envoy.distribution.release/REVIEW.md marking findings 4.2, 6.1, and 8.3 as resolved (e.g., a small "Resolved in follow-up #" annotation next to each), OR — preferred — delete those three findings from the Findings sections and remove their rows from the "Recommended follow-up PRs" table. Pick one approach and apply consistently.
• Run the existing test suites for both py/envoy.github.release and py/envoy.distribution.release and ensure they pass.
• Sign-off commits as usual.

Context / prior PRs

These already landed and resolved other review findings — do not duplicate their work:

• py/deps: Bump distribution package deps #4477 — distribution package deps bump
• py/envoy.distribution.release: Cleanups #4483 — envoy.distribution.release cleanups (metadata, typing, deps)
• py/envoy.distribution.release: Code cleanups #4490 — envoy.distribution.release code cleanups (dead code, typing migration, no-op test fixes, setup.cfg url...

This pull request was created from Copilot chat.

[netlify]

✅ Deploy Preview for nifty-bassi-e26446 ready!

Name	Link
🔨 Latest commit	744208c
🔍 Latest deploy log	https://app.netlify.com/projects/nifty-bassi-e26446/deploys/6a01d419d0e9d80008d5c94a
😎 Deploy Preview	https://deploy-preview-4495--nifty-bassi-e26446.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.