Conversation
Reviewer's GuideThis PR adds a new composite GitHub Action for releasing Python packages via uv, enriches scripting standards and documentation, and refactors several existing action scripts and tests for consistency and robustness. The Sequence diagram for the release-to-pypi-uv composite action workflowsequenceDiagram
participant Workflow
participant Action
participant Scripts
participant PyPI
participant GitHubAPI
participant Summary
Workflow->>Action: Start release-to-pypi-uv
Action->>Scripts: determine_release.py (resolve tag/version)
Action->>Scripts: confirm_release.py (if confirmation required)
Action->>Scripts: check_github_release.py (validate release)
Scripts->>GitHubAPI: GET /repos/:repo/releases/tags/:tag
GitHubAPI-->>Scripts: Release info
Action->>Scripts: validate_toml_versions.py (check TOML versions)
Action->>Scripts: uv build (build dists)
Action->>Scripts: publish_release.py (publish)
Scripts->>PyPI: uv publish
Action->>Scripts: write_summary.py (write summary)
Scripts->>Summary: Update step summary
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Reviews pausedUse the following commands to manage reviews:
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit
WalkthroughAdd a composite GitHub Action "Release to PyPI (uv)" with supporting Typer scripts for tag resolution, manual confirmation, GitHub Release verification (with retries), TOML version validation, publishing via uv, and step-summary writing; include docs, changelog, tests, CmdMox-based test refactors, CI steps, and dev-tooling updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Runner as GitHub Actions Runner
participant Action as Release-to-PyPI (composite)
participant Scripts as uv-run scripts
participant GH as GitHub API
participant PyPI as PyPI (uv Trusted Publishing)
Runner->>Action: Trigger action (inputs incl. python-version)
Action->>Scripts: determine_release (tag/env)
Scripts-->>Action: resolved tag, version
alt require-confirmation == true
Action->>Scripts: confirm_release(expected="release <tag>", confirm=input)
Scripts-->>Action: OK or exit(error)
end
Action->>Scripts: check_github_release(tag, token, repo)
Scripts->>GH: GET /repos/{repo}/releases/tags/{tag} (with retries)
GH-->>Scripts: JSON (ensure not draft/prerelease)
Scripts-->>Action: OK
Action->>Scripts: validate_toml_versions(version, pattern, fail-on-dynamic)
Scripts-->>Action: OK or exit(error)
Action->>Action: setup uv & cache
Action->>Scripts: uv python install "${{ inputs.python-version }}"
Action->>Scripts: build dists (sdist/wheel)
Action->>Scripts: publish_release(index?)
Scripts->>PyPI: Publish (Trusted Publishing OIDC)
PyPI-->>Scripts: success
Action->>Scripts: write_summary(tag, index, environment)
Scripts-->>Action: appended summary
Action-->>Runner: outputs tag, version
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `.github/actions/release-to-pypi-uv/scripts/check_github_release.py:25-26` </location>
<code_context>
+
+def _fetch_release(repo: str, tag: str, token: str) -> dict[str, object]:
+ api = f"https://api.github.com/repos/{repo}/releases/tags/{tag}"
+ request = urllib.request.Request(
+ api,
+ headers={
+ "Authorization": f"Bearer {token}",
+ "Accept": "application/vnd.github+json",
+ "X-GitHub-Api-Version": "2022-11-28",
+ "User-Agent": "release-to-pypi-action",
+ },
+ )
+
</code_context>
<issue_to_address>
**suggestion:** No retry logic for transient network errors.
Adding retries with exponential backoff will help handle temporary network issues and make the API requests more reliable.
```suggestion
import time
def _fetch_release(repo: str, tag: str, token: str) -> dict[str, object]:
api = f"https://api.github.com/repos/{repo}/releases/tags/{tag}"
request = urllib.request.Request(
api,
headers={
"Authorization": f"Bearer {token}",
"Accept": "application/vnd.github+json",
"X-GitHub-Api-Version": "2022-11-28",
"User-Agent": "release-to-pypi-action",
},
)
max_attempts = 5
backoff_factor = 1.5
for attempt in range(1, max_attempts + 1):
try:
with urllib.request.urlopen(request) as response:
return json.load(response)
except (urllib.error.URLError, urllib.error.HTTPError) as e:
if attempt == max_attempts:
raise
sleep_time = backoff_factor ** attempt
time.sleep(sleep_time)
```
</issue_to_address>
### Comment 2
<location> `.github/actions/release-to-pypi-uv/scripts/determine_release.py:43-48` </location>
<code_context>
+ )
+ raise typer.Exit(1)
+
+ if not re.fullmatch(r"v[0-9].*", resolved_tag):
+ typer.echo(
+ f"::error::Tag must start with 'v' (e.g. v1.2.3), got '{resolved_tag}'.",
</code_context>
<issue_to_address>
**suggestion:** Tag format regex may allow invalid semantic versions.
The current regex matches tags like 'v1abc', which aren't valid semantic versions. Use a stricter pattern such as 'v\d+\.\d+\.\d+' to ensure proper semantic versioning.
```suggestion
if not re.fullmatch(r"v\d+\.\d+\.\d+", resolved_tag):
typer.echo(
f"::error::Tag must be a valid semantic version (e.g. v1.2.3), got '{resolved_tag}'.",
err=True,
)
raise typer.Exit(1)
```
</issue_to_address>
### Comment 3
<location> `.github/actions/release-to-pypi-uv/tests/test_determine_release.py:46-57` </location>
<code_context>
+ return out
+
+
+def test_resolves_tag_from_ref(tmp_path: Path) -> None:
+ env = base_env(tmp_path)
+ env["GITHUB_REF_TYPE"] = "tag"
+ env["GITHUB_REF_NAME"] = "v1.2.3"
+
+ script = Path(__file__).resolve().parents[1] / "scripts" / "determine_release.py"
+ result = run_script(script, env=env)
+
+ assert result.returncode == 0, result.stderr
+ outputs = read_outputs(tmp_path)
+ assert outputs["tag"] == "v1.2.3"
+ assert outputs["version"] == "1.2.3"
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for edge cases such as missing environment variables and invalid input formats.
Tests should also cover scenarios with missing or empty environment variables and tags with unexpected formats to verify error handling.
```suggestion
def test_resolves_tag_from_ref(tmp_path: Path) -> None:
env = base_env(tmp_path)
env["GITHUB_REF_TYPE"] = "tag"
env["GITHUB_REF_NAME"] = "v1.2.3"
script = Path(__file__).resolve().parents[1] / "scripts" / "determine_release.py"
result = run_script(script, env=env)
assert result.returncode == 0, result.stderr
outputs = read_outputs(tmp_path)
assert outputs["tag"] == "v1.2.3"
assert outputs["version"] == "1.2.3"
def test_missing_github_ref_type(tmp_path: Path) -> None:
env = base_env(tmp_path)
# env["GITHUB_REF_TYPE"] is intentionally missing
env["GITHUB_REF_NAME"] = "v1.2.3"
script = Path(__file__).resolve().parents[1] / "scripts" / "determine_release.py"
result = run_script(script, env=env)
assert result.returncode != 0
assert "GITHUB_REF_TYPE" in result.stderr or "missing" in result.stderr.lower()
def test_missing_github_ref_name(tmp_path: Path) -> None:
env = base_env(tmp_path)
env["GITHUB_REF_TYPE"] = "tag"
# env["GITHUB_REF_NAME"] is intentionally missing
script = Path(__file__).resolve().parents[1] / "scripts" / "determine_release.py"
result = run_script(script, env=env)
assert result.returncode != 0
assert "GITHUB_REF_NAME" in result.stderr or "missing" in result.stderr.lower()
def test_empty_github_ref_name(tmp_path: Path) -> None:
env = base_env(tmp_path)
env["GITHUB_REF_TYPE"] = "tag"
env["GITHUB_REF_NAME"] = ""
script = Path(__file__).resolve().parents[1] / "scripts" / "determine_release.py"
result = run_script(script, env=env)
assert result.returncode != 0
assert "GITHUB_REF_NAME" in result.stderr or "empty" in result.stderr.lower()
def test_invalid_tag_format(tmp_path: Path) -> None:
env = base_env(tmp_path)
env["GITHUB_REF_TYPE"] = "tag"
env["GITHUB_REF_NAME"] = "release-1.2.3" # Unexpected format
script = Path(__file__).resolve().parents[1] / "scripts" / "determine_release.py"
result = run_script(script, env=env)
assert result.returncode != 0
assert "tag format" in result.stderr.lower() or "invalid" in result.stderr.lower()
def test_malformed_version_tag(tmp_path: Path) -> None:
env = base_env(tmp_path)
env["GITHUB_REF_TYPE"] = "tag"
env["GITHUB_REF_NAME"] = "v1.2" # Malformed version string
script = Path(__file__).resolve().parents[1] / "scripts" / "determine_release.py"
result = run_script(script, env=env)
assert result.returncode != 0
assert "version" in result.stderr.lower() or "invalid" in result.stderr.lower()
```
</issue_to_address>
### Comment 4
<location> `.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py:34-48` </location>
<code_context>
+ )
+
+
+def test_passes_when_versions_match(tmp_path: Path) -> None:
+ project = tmp_path / "pkg"
+ project.mkdir()
+ (project / "pyproject.toml").write_text(
+ """
+[project]
+name = "demo"
+version = "1.0.0"
+""".strip()
+ )
+
+ result = _run(tmp_path, version="1.0.0")
+
+ assert result.returncode == 0, result.stderr
+ assert "all versions match 1.0.0" in result.stdout
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for TOML files with missing [project] section and for multiple TOML files.
Adding these tests will help verify the script's behavior when TOML files are missing the [project] section and when handling multiple files with mixed validity.
```suggestion
def test_passes_when_versions_match(tmp_path: Path) -> None:
project = tmp_path / "pkg"
project.mkdir()
(project / "pyproject.toml").write_text(
"""
[project]
name = "demo"
version = "1.0.0"
""".strip()
)
result = _run(tmp_path, version="1.0.0")
assert result.returncode == 0, result.stderr
assert "all versions match 1.0.0" in result.stdout
def test_fails_when_project_section_missing(tmp_path: Path) -> None:
project = tmp_path / "pkg"
project.mkdir()
(project / "pyproject.toml").write_text(
"""
[tool.poetry]
name = "demo"
version = "1.0.0"
""".strip()
)
result = _run(tmp_path, version="1.0.0")
assert result.returncode != 0
assert "missing [project] section" in result.stderr.lower() or "could not find [project]" in result.stderr.lower()
def test_multiple_toml_files_mixed_validity(tmp_path: Path) -> None:
# Valid TOML
valid_project = tmp_path / "pkg_valid"
valid_project.mkdir()
(valid_project / "pyproject.toml").write_text(
"""
[project]
name = "demo"
version = "1.0.0"
""".strip()
)
# Invalid TOML (missing [project])
invalid_project = tmp_path / "pkg_invalid"
invalid_project.mkdir()
(invalid_project / "pyproject.toml").write_text(
"""
[tool.poetry]
name = "demo"
version = "1.0.0"
""".strip()
)
# Another valid TOML with matching version
another_valid = tmp_path / "pkg_another_valid"
another_valid.mkdir()
(another_valid / "pyproject.toml").write_text(
"""
[project]
name = "demo2"
version = "1.0.0"
""".strip()
)
result = _run(tmp_path, version="1.0.0")
# Should fail due to one invalid TOML
assert result.returncode != 0
assert "missing [project] section" in result.stderr.lower() or "could not find [project]" in result.stderr.lower()
```
</issue_to_address>
### Comment 5
<location> `.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py:68-77` </location>
<code_context>
+ assert "version '1.0.1' != tag version '1.0.0'" in result.stderr
+
+
+def test_dynamic_version_failure(tmp_path: Path) -> None:
+ project = tmp_path / "pkg"
+ project.mkdir()
+ (project / "pyproject.toml").write_text(
+ """
+[project]
+name = "demo"
+dynamic = ["version"]
+""".strip()
+ )
+
+ result = _run(tmp_path, version="1.0.0", fail_dynamic="true")
+
+ assert result.returncode == 1
+ assert "dynamic 'version'" in result.stderr
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for dynamic version with fail_dynamic set to false.
Adding this test will confirm that the script behaves as expected when fail_dynamic is false, ensuring it emits a notice and skips the version check.
</issue_to_address>
### Comment 6
<location> `.github/actions/release-to-pypi-uv/scripts/check_github_release.py:75` </location>
<code_context>
+ raise RuntimeError(f"{path}: failed to parse: {exc}") from exc
+
+
+def main(
+ version: str = VERSION_OPTION,
+ pattern: str = PATTERN_OPTION,
</code_context>
<issue_to_address>
**issue (review_instructions):** Add behavioural and unit tests for check_github_release.py to demonstrate correct operation and error handling.
This script introduces logic for verifying GitHub Release status, including error handling for draft and prerelease states. You must provide both behavioural and unit tests to cover success, draft, prerelease, and API failure scenarios.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*`
**Instructions:**
For any new feature or change to an existing feature, both behavioural *and* unit tests are required.
</details>
</issue_to_address>
### Comment 7
<location> `.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py:59` </location>
<code_context>
+ raise RuntimeError(f"{path}: failed to parse: {exc}") from exc
+
+
+def main(
+ version: str = VERSION_OPTION,
+ pattern: str = PATTERN_OPTION,
</code_context>
<issue_to_address>
**issue (review_instructions):** Add behavioural and unit tests for validate_toml_versions.py to verify version checks and error handling.
This script validates pyproject.toml versions and handles dynamic version errors. You must provide both behavioural and unit tests to cover matching, mismatching, missing, and dynamic version cases.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*`
**Instructions:**
For any new feature or change to an existing feature, both behavioural *and* unit tests are required.
</details>
</issue_to_address>
### Comment 8
<location> `.github/actions/release-to-pypi-uv/scripts/determine_release.py:26` </location>
<code_context>
+ fh.write(f"version={version}\n")
+
+
+def main(tag: str | None = TAG_OPTION, github_output: Path = GITHUB_OUTPUT_OPTION) -> None:
+ ref_type = os.getenv("GITHUB_REF_TYPE", "")
+ ref_name = os.getenv("GITHUB_REF_NAME", "")
</code_context>
<issue_to_address>
**issue (review_instructions):** Add behavioural and unit tests for determine_release.py to cover tag resolution and error conditions.
This script resolves release tags and versions, including error handling for missing or malformed tags. You must provide both behavioural and unit tests for all code paths, including valid and invalid inputs.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*`
**Instructions:**
For any new feature or change to an existing feature, both behavioural *and* unit tests are required.
</details>
</issue_to_address>
### Comment 9
<location> `.github/actions/release-to-pypi-uv/scripts/publish_release.py:24` </location>
<code_context>
+INDEX_OPTION = typer.Option("", envvar="INPUT_UV_INDEX")
+
+
+def main(index: str = INDEX_OPTION) -> None:
+ if index:
+ typer.echo(f"Publishing with uv to index '{index}'")
</code_context>
<issue_to_address>
**issue (review_instructions):** Add behavioural and unit tests for publish_release.py to verify publishing logic and error handling.
This script publishes distributions using uv, optionally to a custom index. You must provide both behavioural and unit tests to cover publishing to default and custom indexes, and error scenarios from run_cmd.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*`
**Instructions:**
For any new feature or change to an existing feature, both behavioural *and* unit tests are required.
</details>
</issue_to_address>
### Comment 10
<location> `.github/actions/release-to-pypi-uv/scripts/write_summary.py:20` </location>
<code_context>
+ raise RuntimeError(f"{path}: failed to parse: {exc}") from exc
+
+
+def main(
+ version: str = VERSION_OPTION,
+ pattern: str = PATTERN_OPTION,
</code_context>
<issue_to_address>
**issue (review_instructions):** Add behavioural and unit tests for write_summary.py to verify summary output and error handling.
This script writes a release summary to a file. You must provide both behavioural and unit tests to cover correct output and error conditions (e.g., file write failures).
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*`
**Instructions:**
For any new feature or change to an existing feature, both behavioural *and* unit tests are required.
</details>
</issue_to_address>
### Comment 11
<location> `.github/actions/release-to-pypi-uv/scripts/confirm_release.py:16` </location>
<code_context>
+CONFIRM_OPTION = typer.Option("", envvar="INPUT_CONFIRM")
+
+
+def main(expected: str = EXPECTED_OPTION, confirm: str = CONFIRM_OPTION) -> None:
+ if confirm != expected:
+ typer.echo(
</code_context>
<issue_to_address>
**issue (review_instructions):** Add behavioural and unit tests for confirm_release.py to verify confirmation logic and error handling.
This script validates a manual confirmation string. You must provide both behavioural and unit tests to cover correct and incorrect confirmation inputs and error handling.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*`
**Instructions:**
For any new feature or change to an existing feature, both behavioural *and* unit tests are required.
</details>
</issue_to_address>
### Comment 12
<location> `.github/actions/release-to-pypi-uv/README.md:3` </location>
<code_context>
+# Release to PyPI (uv)
+
+Build and publish Python distributions via [uv](https://github.com/astral-sh/uv) with GitHub's
+trusted publishing flow.
+
</code_context>
<issue_to_address>
**issue (review_instructions):** This paragraph exceeds the 80 column wrapping limit for Markdown text.
Please wrap this paragraph so that no line exceeds 80 columns, as per the documentation standards.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Paragraphs and bullets must be wrapped to 80 columns
</details>
</issue_to_address>
### Comment 13
<location> `.github/actions/release-to-pypi-uv/tests/test_determine_release.py:19` </location>
<code_context>
cwd=env.get("PWD", None),
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace `dict.get(x, None)` with `dict.get(x)` ([`remove-none-from-default-get`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/remove-none-from-default-get))
```suggestion
cwd=env.get("PWD"),
```
<br/><details><summary>Explanation</summary>When using a dictionary's `get` method you can specify a default to return if
the key is not found. This defaults to `None`, so it is unnecessary to specify
`None` if this is the required behaviour. Removing the unnecessary argument
makes the code slightly shorter and clearer.
</details>
</issue_to_address>
### Comment 14
<location> `.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py:91` </location>
<code_context>
def main(
version: str = VERSION_OPTION,
pattern: str = PATTERN_OPTION,
fail_on_dynamic: str = FAIL_ON_DYNAMIC_OPTION,
) -> None:
files = list(_iter_files(pattern))
if not files:
typer.echo(f"::warning::No TOML files matched pattern {pattern}")
return
literal_version_errors: list[str] = []
dynamic_errors: list[str] = []
checked = 0
fail_dynamic = _parse_bool(fail_on_dynamic)
for path in files:
try:
data = _load_toml(path)
except RuntimeError as exc:
typer.echo(f"::error::{exc}", err=True)
return
project = data.get("project")
if not isinstance(project, dict):
continue
checked += 1
dynamic = project.get("dynamic")
dynamic_set = {str(item) for item in dynamic} if isinstance(dynamic, (list, tuple)) else set()
if "version" in dynamic_set:
message = f"{path}: uses dynamic 'version' (PEP 621)."
if fail_dynamic:
dynamic_errors.append(message + " Set fail-on-dynamic-version=false to allow.")
else:
typer.echo(f"::notice::{message} Skipping version check.")
continue
toml_version = project.get("version")
if toml_version is None:
literal_version_errors.append(
f"{path}: missing [project].version and not marked dynamic"
)
continue
if str(toml_version) != version:
literal_version_errors.append(
f"{path}: [project].version '{toml_version}' != tag version '{version}'"
)
if dynamic_errors or literal_version_errors:
for error in (*dynamic_errors, *literal_version_errors):
typer.echo(f"::error::{error}", err=True)
raise typer.Exit(1)
typer.echo(f"Checked {checked} PEP 621 project file(s); all versions match {version}.")
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Use f-string instead of string concatenation ([`use-fstring-for-concatenation`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-fstring-for-concatenation/))
- Split conditional into multiple branches ([`split-or-ifs`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/split-or-ifs/))
- Merge duplicate blocks in conditional ([`merge-duplicate-blocks`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/merge-duplicate-blocks/))
- Remove redundant conditional ([`remove-redundant-if`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-redundant-if/))
```suggestion
dynamic_errors.append(f"{message} Set fail-on-dynamic-version=false to allow.")
```
</issue_to_address>
### Comment 15
<location> `.github/actions/release-to-pypi-uv/tests/test_determine_release.py:12-20` </location>
<code_context>
def run_script(script: Path, *, env: dict[str, str]) -> subprocess.CompletedProcess[str]:
cmd = ["uv", "run", "--script", str(script)]
result = subprocess.run( # noqa: S603
cmd,
capture_output=True,
encoding="utf-8",
errors="replace",
env=env,
check=False,
cwd=env.get("PWD", None),
)
return result
</code_context>
<issue_to_address>
**issue (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/inline-immediately-returned-variable/))
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
.github/actions/release-to-pypi-uv/CHANGELOG.md(1 hunks).github/actions/release-to-pypi-uv/README.md(1 hunks).github/actions/release-to-pypi-uv/action.yml(1 hunks).github/actions/release-to-pypi-uv/scripts/check_github_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/confirm_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/determine_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/publish_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/scripts/write_summary.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_determine_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py(1 hunks)README.md(1 hunks)docs/scripting-standards.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{README.md,docs/**}
📄 CodeRabbit inference engine (.rules/python-00.md)
Colocate documentation near reusable packages: keep README.md or docs/ and include usage examples
Files:
README.mddocs/scripting-standards.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
README.mddocs/scripting-standards.md
🧬 Code graph analysis (6)
.github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
.github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-30)
.github/actions/release-to-pypi-uv/scripts/write_summary.py (3)
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
main(75-87).github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
main(26-53).github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
main(24-30)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
shellstub.py (1)
env(169-174)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (3)
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
main(75-87).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (1)
main(59-113).github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-30)
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
main(26-53)
.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (1)
shellstub.py (1)
env(169-174)
🔍 Remote MCP Ref
Additional review facts & quick checks (concise)
- Composite action outputs: mapping action outputs to a step's outputs (e.g.
outputs: tag: ${{ steps.resolve.outputs.tag }}) is supported for composite actions. - Setting step outputs from a script: writing lines
name=valueto the file at $GITHUB_OUTPUT is the documented way to set outputs; determine_release's append-to-GITHUB_OUTPUT matches that pattern. - Job summary: appending text to the path in $GITHUB_STEP_SUMMARY is the documented way to add a job summary; write_summary's append usage matches docs.
- GitHub Release check: GET /repos/{owner}/{repo}/releases/tags/{tag} returns 404 if no release; handling 404/drafts/prerelease as check_github_release.py does is appropriate. Ensure the GH token used by the script has repo/read-release access and that the request sets the expected Accept/Authorization headers.
- PEP 621 / dynamic versions: projects that declare
dynamic = ["version"]will not have a static[project].version; validate_toml_versions'fail-on-dynamicoption matches the expected handling pattern (fail vs skip). (Behaviour echoed by the added tests in this PR; confirm policy for dynamic-version projects in your org.)
Reviewer action items (concrete):
- Verify the composite action consumers will run on runners that install Python 3.13 and
uv(the action usesastral-sh/setup-uvand scripts calluv run --script). Ensure the setup action is available/pinned in workflows. - Confirm
cmd_utils.run_cmd(imported by publish_release.py) exists in the repo and properly masks secrets / checks return codes. - Confirm the action's use of the GH token and any PyPI tokens (via
uv publish) follows your org's secret/permissions policy and that logs will not expose credentials.
Tooling note: I fetched GitHub docs and related references while preparing these checks; see tool calls used below for source verification.
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Sourcery review
- GitHub Check: python-tests (windows-latest)
- GitHub Check: python-tests (ubuntu-latest)
🔇 Additional comments (7)
README.md (1)
12-12: Add action entry: LGTM.Register the new action in the table correctly.
.github/actions/release-to-pypi-uv/CHANGELOG.md (1)
1-8: Changelog entry reads cleanly.Date and versioning look correct; scope matches the implementation.
docs/scripting-standards.md (1)
1-80: Standards read crisply and follow repository guidelines.Headings, en‑GB spelling, and neutral voice comply with house style.
.github/actions/release-to-pypi-uv/scripts/confirm_release.py (1)
12-25: Confirmation gate: LGTM.Enforce a strict match, emit a GitHub annotation, and exit non‑zero on failure.
.github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
24-30: Publish flow: LGTM post‑import fix.Command selection and messaging are appropriate; delegate exit‑code handling to run_cmd.
Run the checks below to ensure the import now resolves and that
run_cmdenforces non‑zero exit handling and masks secrets.#!/bin/bash set -euo pipefail echo "Confirm cmd_utils location relative to publish_release.py…" PUBLISH_DIR=$(dirname ".github/actions/release-to-pypi-uv/scripts/publish_release.py") test -f "$PUBLISH_DIR/cmd_utils.py" && echo "Found: $PUBLISH_DIR/cmd_utils.py" || { echo "Missing cmd_utils.py next to publish_release.py"; exit 1; } echo "Inspect run_cmd signature and behaviour…" rg -nC3 'def\s+run_cmd\s*\(' "$PUBLISH_DIR/cmd_utils.py" || true.github/actions/release-to-pypi-uv/action.yml (1)
40-50: Surface resolve outputs robustly.Mapping outputs to step outputs is correct. Keep step id stable; changing it is a breaking change for consumers relying on action outputs.
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
24-31: Clear GITHUB_REF_TYPE and GITHUB_REF_NAME in base_env
Reset these variables in base_env to avoid CI-provided refs flipping test_resolves_tag_from_input. Apply:merged["GITHUB_REF_TYPE"] = "" merged["GITHUB_REF_NAME"] = ""
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/release-to-pypi-uv/action.yml (1)
40-97: Add branding for Marketplace discoverability.Surface icon and colour to improve UX when browsing actions.
Apply at the top-level:
name: Release to PyPI (uv) description: Build and publish Python distributions to PyPI using uv. +branding: + icon: package + color: purple
♻️ Duplicate comments (2)
.github/actions/release-to-pypi-uv/README.md (2)
47-52: Fix invalid local action reference in usage.Path-based actions must not include a ref. Either remove the ref for local usage or switch to the repo path for external consumption.
Apply one of:
- - name: Build and publish - uses: ./.github/actions/release-to-pypi-uv@v1 + - name: Build and publish + uses: ./.github/actions/release-to-pypi-uvor
- - name: Build and publish - uses: ./.github/actions/release-to-pypi-uv@v1 + - name: Build and publish + uses: leynos/shared-actions/.github/actions/release-to-pypi-uv@v1
25-26: Document token wiring for the GitHub Release check.State explicitly that the action sets GH_TOKEN from github.token internally, so users do not need to pass GH_TOKEN in their workflow.
Apply this addition under “Required permissions”:
> **Required permissions**: set the job to `permissions: contents: read` and `permissions: id-token: write` so uv Trusted Publishing can exchange an OIDC token with PyPI. + +Note: This action wires `GH_TOKEN` from `github.token` for the GitHub Release check; no additional env mapping is required in your workflow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/actions/release-to-pypi-uv/CHANGELOG.md(1 hunks).github/actions/release-to-pypi-uv/README.md(1 hunks).github/actions/release-to-pypi-uv/action.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: python-tests (windows-latest)
- GitHub Check: python-tests (ubuntu-latest)
- GitHub Check: Sourcery review
🔇 Additional comments (4)
.github/actions/release-to-pypi-uv/CHANGELOG.md (1)
3-13: Changelog entries read cleanly and match the shipped surface.Ship v1.0.1 note aligns with README/action.yml updates about required permissions; initial feature list is scoped and accurate.
.github/actions/release-to-pypi-uv/action.yml (3)
3-5: Good: call out required OIDC permissions in metadata.Keep this comment block; it prevents a common Trusted Publishing failure mode.
65-71: Good: wire GH_TOKEN and repository for the GitHub Release check.Leave the internal env wiring; this removes friction for consumers.
43-51: Mark resolved — setup-uv v6.4.3 pinned to commit e92bafb6253dcd438e0484186d7669ea7a8ca1cc (exact match)
v6.4.3 resolves to e92bafb6253dcd438e0484186d7669ea7a8ca1cc; no change required.
|
@coderabbitai pause |
✅ Actions performedReviews paused. |
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
43-49: Support SemVer prerelease/build metadata if policy allows.Current regex blocks tags like v1.2.3-rc.1 or v1.2.3+build. Enable full SemVer or confirm strict MAJOR.MINOR.PATCH is intended.
Apply if prereleases/builds should pass:
- if not re.fullmatch(r"v\d+\.\d+\.\d+", resolved_tag): + if not re.fullmatch( + r"v\d+\.\d+\.\d+(?:-[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?", + resolved_tag, + ):.github/actions/release-to-pypi-uv/action.yml (2)
3-5: Call out permissions in README usage too; keep OIDC requirements front-and-centre.Mirror these exact permissions in the action README usage snippet so users don’t miss them at job level.
Quote the PyPI docs that state id-token: write is mandatory for Trusted Publishing, and whether contents: read is recommended/required for related steps.(Refs: PyPI Trusted Publishing docs and Marketplace pages corroborate id-token: write; contents: read is commonly recommended when the workflow reads repo content.) (docs.pypi.org)
47-55: Propagate interpreter via setup-uv and key the cache by Python version.Set python-version on setup-uv so UV_PYTHON is exported to subsequent steps; include the Python version in the cache-suffix to avoid cross-version cache collisions. (github.com)
File: .github/actions/release-to-pypi-uv/action.yml Lines: 47-55
- name: Setup uv # v6.4.3 uses: astral-sh/setup-uv@e92bafb6253dcd438e0484186d7669ea7a8ca1cc with: + python-version: ${{ inputs.python-version }} cache-dependency-glob: | **/pyproject.toml **/uv.lock - cache-suffix: action-${{ github.action_ref || github.sha }} + cache-suffix: action-${{ github.action_ref || github.sha }}-py${{ inputs.python-version }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
.github/actions/release-to-pypi-uv/CHANGELOG.md(1 hunks).github/actions/release-to-pypi-uv/README.md(1 hunks).github/actions/release-to-pypi-uv/action.yml(1 hunks).github/actions/release-to-pypi-uv/scripts/check_github_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/determine_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/publish_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/scripts/write_summary.py(1 hunks).github/actions/release-to-pypi-uv/tests/_helpers.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_check_github_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_confirm_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_determine_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_publish_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_write_summary.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
.github/actions/release-to-pypi-uv/tests/test_publish_release.py (1)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(13-21)
.github/actions/release-to-pypi-uv/tests/test_confirm_release.py (1)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
base_env(23-30)
.github/actions/release-to-pypi-uv/scripts/write_summary.py (2)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
main(26-53).github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
main(53-59)
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(13-21).github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
main(97-109)
.github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
.github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-38)
.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (1)
shellstub.py (1)
env(169-174)
.github/actions/release-to-pypi-uv/tests/test_write_summary.py (1)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(13-21)
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (1)
read(25-26)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (3)
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
main(97-109).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (1)
main(68-122).github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-38)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
shellstub.py (1)
env(169-174)
.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (1)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
main(26-53)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (29)
.github/actions/release-to-pypi-uv/CHANGELOG.md (1)
3-9: Ship these entries as‑is.Content, dates, and scope read cleanly; no action needed.
.github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
20-38: Keep the Markdown summary; it’s tidy and idempotent.Heading, bullets, and prefix logic are sound.
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
37-71: Leave the retry/timeout and 403/404 handling as implemented.Backoff, headers, and actionable errors are appropriate.
.github/actions/release-to-pypi-uv/tests/test_publish_release.py (1)
22-46: Retain these tests; behaviour coverage is crisp.Default vs custom index calls are asserted correctly.
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (7)
15-27: LGTM! Clean context manager implementation.The
DummyResponseclass properly implements the context manager protocol and correctly encodes the JSON payload.
34-44: LGTM! Well-structured success test.The test correctly validates the published release scenario with proper output assertion.
46-57: LGTM! Draft release handling validated.The test properly checks that draft releases trigger an error with the expected message.
59-70: LGTM! Prerelease validation covered.The test correctly verifies that prereleases are rejected with appropriate error messaging.
72-89: LGTM! 404 error handling tested.The test properly simulates and validates the missing release scenario with HTTPError.
91-111: LGTM! Permission error handling verified.The test correctly validates 403 Forbidden responses and checks for the permission error message.
113-129: LGTM! Retry logic thoroughly tested.The test effectively validates the retry mechanism with URLError, ensuring three attempts are made before success.
.github/actions/release-to-pypi-uv/tests/test_write_summary.py (3)
18-32: LGTM! Core functionality validated.The test properly verifies that the summary is written with the correct format and content for the default index case.
34-48: LGTM! Append behaviour tested.The test correctly validates that existing content is preserved and only one header is added when appending to an existing file.
50-59: LGTM! Error propagation verified.The test appropriately checks that IO errors are raised when writing to non-existent paths.
.github/actions/release-to-pypi-uv/scripts/publish_release.py (2)
17-44: LGTM! Path resolution logic is comprehensive.The
_extend_sys_pathfunction properly handles both GitHub Action and local execution contexts, with appropriate error handling for missing parent directories.
53-60: LGTM! Publishing logic correctly handles index parameter.The main function appropriately branches based on whether an index is provided and uses clear messaging for each case.
.github/actions/release-to-pypi-uv/tests/test_confirm_release.py (3)
11-26: LGTM! Helper function properly configured.The
run_confirmfunction correctly sets up the environment and executes the script with appropriate subprocess parameters.
28-33: LGTM! Success case validated.The test properly verifies that matching confirmation strings result in success with the expected output.
35-39: LGTM! Failure case tested.The test correctly validates that mismatched confirmation strings result in failure with the appropriate error message.
.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (8)
13-32: LGTM! Test runner well-configured.The
_runhelper function properly sets up the environment with PYTHONPATH and required variables for script execution.
34-49: LGTM! Version match validation covered.The test correctly verifies that matching versions pass validation with the expected success message.
51-66: LGTM! Version mismatch detection tested.The test properly validates that version mismatches are caught and reported with the correct error message.
68-83: LGTM! Dynamic version failure case covered.The test correctly verifies that dynamic versions trigger failure when the fail flag is set to true.
85-94: LGTM! Parse error handling validated.The test properly checks that invalid TOML syntax triggers an error with the appropriate message.
96-111: LGTM! Dynamic version allowance tested.The test correctly validates that dynamic versions are permitted when the fail flag is false, with appropriate notice in stdout.
113-127: LGTM! Missing project section handling verified.The test properly confirms that TOML files without a [project] section are ignored and don't cause failures.
129-153: LGTM! Multiple file validation tested.The test effectively validates that mismatches are detected across multiple TOML files in different directories.
.github/actions/release-to-pypi-uv/action.yml (1)
48-55: Pin the setup-uv ref to the intended release tag or confirm the SHA.Either pin to a tag (e.g. v6.4.3) or verify the SHA matches that release to keep supply‑chain hygiene tight.
Does tag v6.4.3 of astral-sh/setup-uv correspond to commit e92bafb6253dcd438e0484186d7669ea7a8ca1cc?.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
45-57: LGTM: test matrix robustly locks the resolve paths and error modes.Keep this coverage; it nails happy paths and key failures (bad tag, missing env, malformed version).
Also applies to: 59-70, 72-82, 84-95, 97-107, 109-119, 121-131, 133-143
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
.github/actions/release-to-pypi-uv/README.md (1)
52-54: Forward the python-version input to setup-uv.The README shows
setup-uvwith apython-versioninput, but the composite action doesn't wire this through. Add the input mapping to ensure the specified Python version is actually installed.Apply this diff to the action.yml:
- uses: astral-sh/setup-uv@e92bafb6253dcd438e0484186d7669ea7a8ca1cc with: + python-version: ${{ inputs.python-version }} cache-dependency-glob: | **/pyproject.toml **/uv.lock.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
10-20: Add uv availability check to prevent test failures.The test suite will crash with
FileNotFoundErrorifuvisn't installed. Add a skip condition to handle environments whereuvis unavailable.Apply this diff:
+import shutil import os import subprocess from pathlib import Path +import pytest +# Skip tests if uv is not available +pytestmark = pytest.mark.skipif(shutil.which("uv") is None, reason="uv not available") + def run_script(script: Path, *, env: dict[str, str]) -> subprocess.CompletedProcess[str]:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
.github/actions/release-to-pypi-uv/README.md(1 hunks).github/actions/release-to-pypi-uv/action.yml(1 hunks).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/tests/_helpers.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_determine_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py(1 hunks)docs/cmd-mox-users-guide.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{README.md,docs/**}
📄 CodeRabbit inference engine (.rules/python-00.md)
Colocate documentation near reusable packages: keep README.md or docs/ and include usage examples
Files:
docs/cmd-mox-users-guide.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/cmd-mox-users-guide.md
🧬 Code graph analysis (3)
.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (1)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
main(26-53)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
shellstub.py (1)
env(169-174)
.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(21-29).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (2)
main(75-129)_parse_bool(49-55)
🔍 Remote MCP
Additional context for PR #90 review
-
Astral-sh/setup-uv pin
The composite action’saction.ymlpinsastral-sh/setup-uvto version v6.4.3, which was published on July 23, 2025 and includes a bug-fix for handling relative paths beginning with dots. This confirms the referenced version exists and is up-to-date for the action’s setup step. (github.com) -
Composite action permissions behavior
According to GitHub’s documentation, composite actions cannot declare or override job-level permissions in their metadata; any required permissions (e.g.id-token: writefor trusted publishing) must be set in the calling workflow. Reviewers should verify that workflows invoking this action include the necessarypermissionsblock for GitHub token forwarding and environment protection. (docs.github.com)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Sourcery review
- GitHub Check: python-tests (windows-latest)
- GitHub Check: python-tests (ubuntu-latest)
- GitHub Check: build-release (windows-latest, x86_64-pc-windows-gnu)
- GitHub Check: build-release (windows-latest, aarch64-pc-windows-gnu)
🔇 Additional comments (16)
docs/cmd-mox-users-guide.md (1)
1-6: Open with a level‑1 heading; heading progression is correct.Keep this structure; it complies with the docs rules (H1 start, no wrapped headings, single‑level increments).
.github/actions/release-to-pypi-uv/tests/_helpers.py (2)
10-18: LGTM!The path resolution logic correctly handles both GitHub Actions runtime (
GITHUB_ACTION_PATH) and local development scenarios. The fallback calculation properly navigates the directory structure to locate the repository root and scripts directory.
21-29: LGTM!The module loading implementation is robust, with proper error handling for missing specs or loaders. The consistent module naming convention (
release_to_pypi_uv_{name}) aids debugging..github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (6)
1-6: LGTM!The uv script shebang and metadata are correctly configured for Python 3.13+ with the typer dependency. The script description clearly explains its purpose.
17-22: LGTM!The Typer options are properly configured with environment variable bindings that match the composite action's environment setup. The defaults align with the action inputs.
24-33: LGTM!The skip list comprehensively covers common directories that should be excluded from TOML discovery, including VCS, virtualenv, build artifacts, and development tools.
49-55: LGTM!The boolean parsing function handles None values and empty strings correctly, using a comprehensive set of truthy values for robust environment variable interpretation.
102-110: LGTM!The dynamic version handling logic correctly distinguishes between failure and notice modes based on the
fail_on_dynamicflag, providing clear feedback in both cases.
124-127: LGTM!The error aggregation and reporting correctly exits with code 1 when any validation errors are found, ensuring proper CI failure propagation.
.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (4)
12-14: LGTM!The module fixture correctly uses the helper to load the target script module for testing.
84-105: LGTM!The parameterised test comprehensively covers various truthy string representations, ensuring robust boolean parsing across different input formats.
139-159: LGTM!The parameterised test for falsey variants ensures the boolean parser correctly handles all expected false-like values, including edge cases like empty strings.
201-227: LGTM!The mixed validity test properly exercises the error aggregation logic when multiple TOML files have different validation outcomes, ensuring all errors are collected and reported.
.github/actions/release-to-pypi-uv/action.yml (1)
1-5: LGTM!The action metadata clearly documents the required permissions for OIDC trusted publishing in the comment, helping users understand the necessary workflow configuration.
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (2)
46-57: LGTM!The test correctly validates tag resolution from GitHub ref environment variables and verifies both the exit code and output parsing.
85-95: LGTM!The negative test case properly exercises the error path when no tag is provided and the workflow isn't running on a tag ref, validating both exit code and error message.
|
@coderabbitai please create a new GitHub issue for the following: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@leynos I'll create a GitHub issue for this podman timeout failure in the rust-build-release action. 🏁 Script executed: Length of output: 50 @leynos I've created GitHub issue #97 to track the podman timeout failure in the rust-build-release action. The issue includes:
You can view the issue at: #97 The issue has been assigned to you as requested. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
pyproject.toml (2)
24-34: Expose dev deps via PEP 621[project.optional-dependencies]while keeping uv groups.Keep uv’s
[dependency-groups]for local workflows, and add a canonical PEP 621 extra so metadata remains portable and tooling‑friendly.Append:
+[project.optional-dependencies] +dev = [ + "lxml-stubs>=0.5.1", + "pytest>=8.0,<9.0", + "pyyaml>=6.0,<7.0", + "ty>=0.0.1a20", + "uuid6>=2025.0.1", + "cmd-mox@git+https://github.com/leynos/cmd-mox.git@<commit-sha>", +]
111-120: Treat Pyright warnings as CI errors.Tighten severities to align with “strict + fail on warnings”.
Apply:
[tool.pyright] typeCheckingMode = "strict" reportUnknownVariableType = "error" reportUnknownParameterType = "error" reportUnknownMemberType = "error" -reportMissingTypeStubs = "warning" +reportMissingTypeStubs = "error" pythonVersion = "3.12" venvPath = "." venv = ".venv".github/actions/rust-build-release/tests/test_target_install.py (3)
22-42: Fix NameError: add cmd_mox fixture to test signature.The test uses cmd_mox but does not request the fixture, causing a NameError at import/run time.
-@CMD_MOX_UNSUPPORTED -def test_skips_target_install_when_cross_available( - main_module: ModuleType, - cross_module: ModuleType, - module_harness: HarnessFactory, -) -> None: +@CMD_MOX_UNSUPPORTED +def test_skips_target_install_when_cross_available( + main_module: ModuleType, + cross_module: ModuleType, + module_harness: HarnessFactory, + cmd_mox, +) -> None:
64-80: Fix NameError: add cmd_mox fixture to test signature.The test calls _register_rustup_toolchain_stub(cmd_mox, …) without declaring cmd_mox.
-@CMD_MOX_UNSUPPORTED -def test_errors_when_target_unsupported_without_cross( - main_module: ModuleType, - cross_module: ModuleType, - module_harness: HarnessFactory, - capsys: pytest.CaptureFixture[str], -) -> None: +@CMD_MOX_UNSUPPORTED +def test_errors_when_target_unsupported_without_cross( + main_module: ModuleType, + cross_module: ModuleType, + module_harness: HarnessFactory, + cmd_mox, + capsys: pytest.CaptureFixture[str], +) -> None:
101-121: Fix NameError: add cmd_mox fixture to test signature.The test uses cmd_mox later (replay/verify) but does not request it.
-@CMD_MOX_UNSUPPORTED -def test_falls_back_to_cargo_when_cross_container_fails( - main_module: ModuleType, - cross_module: ModuleType, - module_harness: HarnessFactory, -) -> None: +@CMD_MOX_UNSUPPORTED +def test_falls_back_to_cargo_when_cross_container_fails( + main_module: ModuleType, + cross_module: ModuleType, + module_harness: HarnessFactory, + cmd_mox, +) -> None:docs/cmd-mox-users-guide.md (1)
1-241: Remove all second-person pronouns
- Change “commands in your tests” (line 4) to “commands in tests”
- Change “In your
conftest.py” (line 15) to “Inconftest.py”- Change “run your code” (line 32) to “run the code”
- Change “when you need to manage verification manually” (line 173) to “when manual verification management is required”
- Change “When you want to intercept a command” (line 190) to “To intercept a command”
♻️ Duplicate comments (5)
docs/cmd-mox-users-guide.md (5)
15-22: Remove second‑person pronoun in heading lead‑in.Replace “In your …” with neutral phrasing.
Apply this diff:
-In your `conftest.py`: +In a project’s `conftest.py`:
83-90: Eliminate “You …” phrasing to meet the no 2nd‑person rule.Rephrase the lead‑in sentence.
Apply this diff:
-You can match arguments more flexibly using comparators: +Arguments can be matched more flexibly using comparators:
190-192: Remove second person and fix comma after “for example”.Use an infinitive construction and punctuate “for example”.
Apply this diff:
-When you want to intercept a command without configuring a double—for example to -ensure it is treated as unexpected—register it explicitly: +To intercept a command without configuring a double—for example, to ensure it is +treated as unexpected—register it explicitly:
219-219: Use en‑GB‑oxendict “‑ize” spelling.Swap “emphasises” → “emphasizes”.
Apply this diff:
-- `times_called(count)` – alias for `times` that emphasises spy call counts. +- `times_called(count)` – alias for `times` that emphasizes spy call counts.
3-4: Remove second‑person pronoun per docs style guide.Rewrite to avoid “your”.
Apply this diff:
-CmdMox provides a fluent API for mocking, stubbing and spying on external -commands in your tests. This guide shows common patterns for everyday use. +CmdMox provides a fluent API for mocking, stubbing and spying on external +commands in tests. This guide shows common patterns for everyday use.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.github/actions/rust-build-release/tests/conftest.py(0 hunks).github/actions/rust-build-release/tests/test_cross_install.py(6 hunks).github/actions/rust-build-release/tests/test_smoke.py(2 hunks).github/actions/rust-build-release/tests/test_target_install.py(5 hunks).github/actions/rust-build-release/tests/test_utils.py(3 hunks)conftest.py(1 hunks)docs/cmd-mox-users-guide.md(1 hunks)pyproject.toml(1 hunks)
💤 Files with no reviewable changes (1)
- .github/actions/rust-build-release/tests/conftest.py
🧰 Additional context used
📓 Path-based instructions (7)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For Python code, follow the Python Code Style, Context Managers, Generators, Return Patterns, and Typing guidelines under .rules/
**/*.py: Use snake_case.py for file names; names should reflect contents (e.g., http_client.py, task_queue.py)
Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix internal or non-exported helpers with a single underscore (_)
Use TypedDict or dataclass for structured data; prefer @DataClass(slots=True) for internal-only usage
Be explicit with return types (e.g., -> None, -> str) for all public functions and class methods
Favor immutability: prefer tuples to lists and MappingProxyType for read-only mappings; document any frozendict dependency
Use '# pyright: ignore' sparingly and include an explanation when used
Avoid side effects at import time; modules should not modify global state or perform actions on import
Use docstrings in NumPy format for public functions, classes, and modules
Explain tricky code with inline comments for non-obvious logic or decisions
**/*.py: Prefer @contextmanager from contextlib for straightforward procedural setup/teardown
Implement class-based context managers with enter and exit when lifecycle/state is complex or stateful
Choose @contextmanager when control flow is linear and no persistent state is required
Choose a class-based context manager when there is internal state, lifecycle-tied methods, or the need for re-entry/advanced features
Use context managers for file or network resource handling
Use context managers for lock acquisition and release
Use context managers for temporary environment changes (e.g., os.chdir, patch, tempfile)
Use context managers for logging scope control or tracing
Use context managers for transaction control in databases or services
Prefer with open(...) over manual try/finally when working with files
**/*.py: Prefer generators over complex loop logic; refactor complex for-loops into...
Files:
conftest.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep cyclomatic complexity ≤ 12
- Follow single responsibility and CQRS (command/query segregation)
- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.
Files:
conftest.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.rules/python-00.md)
**/*.{py,pyi}: Use typing everywhere and maintain full static type coverage with Pyright
Avoid Any; prefer precise types (TypeVar, Protocol, Literal, Union); use typing.cast[...] only when necessary with justification; use object for unknown-but-opaque values
Files:
conftest.py
**/pyproject.toml
📄 CodeRabbit inference engine (AGENTS.md)
For Python projects, follow the Python Project Configuration guidance in .rules/python-pyproject.md
Files:
pyproject.toml
pyproject.toml
📄 CodeRabbit inference engine (.rules/python-00.md)
pyproject.toml: Enable Ruff and configure it (linting, fixers, formatting) via pyproject.toml
Configure Pyright in strict mode and treat all warnings as CI errors
Use pyproject.toml to configure Ruff, Pyright, and Pytest
Use Ruff for formatting (let Ruff handle whitespace and formatting entirely)
pyproject.toml: Use PEP 621[project]as the single source of project metadata and runtime dependencies (avoid separate setup.py)
Include mandatory[project]fields:nameandversion
Provide recommended[project]metadata:description,readme,requires-python,license,authors,keywords,classifiers
Declare runtimedependenciesin PEP 508 syntax under[project]
Prefer exact or bounded version ranges for dependencies (e.g.,requests>=2.25,<3.0)
Use[project.optional-dependencies]to separate groups likedevanddocs
Expose CLIs via[project.scripts]and GUIs via[project.gui-scripts]; register plugins via[project.entry-points.'group.name']
Declare a PEP 517[build-system]withrequires = ["setuptools>=61.0", "wheel"]andbuild-backend = "setuptools.build_meta"when using setuptools
If omitting[build-system], set[tool.uv].package = trueto ensure your project is built/installed by uv
Add[tool.uv]configuration as needed; commonlypackage = true
Setrequires-pythonin[project]to declare supported interpreter versions
Use valid Trove classifiers in[project].classifiers
Follow semantic versioning forversion(e.g., 1.2.3)
Use dynamic fields (e.g.,dynamic = ["version"]) sparingly and only if supported by the build backend
Minimize build constraints; omit[build-system]if editable installs aren’t needed (or explicitly set[tool.uv].package = true)
Files:
pyproject.toml
{pyproject.toml,uv.lock}
📄 CodeRabbit inference engine (.rules/python-pyproject.md)
Maintain lockfile discipline: after changing
[project]fields ordependencies, runuv sync/lockto updateuv.lock
Files:
pyproject.toml
{README.md,docs/**}
📄 CodeRabbit inference engine (.rules/python-00.md)
Colocate documentation near reusable packages: keep README.md or docs/ and include usage examples
Files:
docs/cmd-mox-users-guide.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/cmd-mox-users-guide.md
🧬 Code graph analysis (3)
.github/actions/rust-build-release/tests/test_cross_install.py (4)
conftest.py (4)
_register_cross_version_stub(19-35)_register_docker_info_stub(47-53)_register_rustup_toolchain_stub(38-44)cmd_mox(61-64).github/actions/rust-build-release/tests/conftest.py (3)
module_harness(106-115)patch_shutil_which(89-91)patch_run_cmd(78-87).github/actions/rust-build-release/src/cross_manager.py (1)
ensure_cross(164-249).github/actions/rust-build-release/src/main.py (1)
main(32-166)
.github/actions/rust-build-release/tests/test_target_install.py (3)
conftest.py (4)
_register_cross_version_stub(19-35)_register_docker_info_stub(47-53)_register_rustup_toolchain_stub(38-44)cmd_mox(61-64).github/actions/rust-build-release/tests/conftest.py (2)
patch_shutil_which(89-91)patch_attr(97-99).github/actions/rust-build-release/src/main.py (1)
main(32-166)
.github/actions/rust-build-release/tests/test_utils.py (2)
.github/actions/rust-build-release/tests/conftest.py (1)
utils_module(212-214)conftest.py (1)
cmd_mox(61-64)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-release (windows-latest, x86_64-pc-windows-gnu)
- GitHub Check: build-release (windows-latest, aarch64-pc-windows-gnu)
- GitHub Check: python-tests (windows-latest)
- GitHub Check: python-tests (ubuntu-latest)
- GitHub Check: Sourcery review
🔇 Additional comments (7)
pyproject.toml (2)
108-110: Confirmtool.uv.package = falseis intentional.Guidelines call for
package = truewhen omitting[build-system]. If this project should be installable (even for tests), flip it; otherwise, confirm the non‑packaged layout is deliberate.Apply if packaging is desired:
-[tool.uv] -package = false +[tool.uv] +package = true
1-120: Regenerate and commit uv.lock with cmd-mox pinned to an immutable VCS ref.The automated check failed in the sandbox (uv not installed); run the following locally or in CI:
uv lock rg -n 'cmd-mox' uv.lock || { echo "ERROR: cmd-mox not present in uv.lock" >&2; exit 1; } rg -nP 'cmd-mox@git\+https://github\.com/leynos/cmd-mox\.git@([0-9a-f]{7,40}|v?[0-9][^"]*)' uv.lock || { echo "ERROR: cmd-mox not pinned to an immutable ref in uv.lock" >&2; exit 1; }Commit the updated uv.lock alongside pyproject.toml.
conftest.py (1)
12-16: Gate cmd‑mox on Windows early.Keep this skip mark; it prevents flakiness on unsupported hosts and documents the constraint in one place.
.github/actions/rust-build-release/tests/test_utils.py (1)
39-63: Adopt cmd‑mox spy with replay/verify boundaries.This is the right pattern; it asserts the validated path and the single invocation cleanly.
.github/actions/rust-build-release/tests/test_target_install.py (1)
37-43: Align stub registration with declared replay/verify.Replay/verify calls are correct; ensure cmd_mox is injected per above so these lines execute.
#!/bin/bash # Verify the three target_install tests declare `cmd_mox` in their signatures. rg -nP 'def\s+(test_skips_target_install_when_cross_available|test_errors_when_target_unsupported_without_cross|test_falls_back_to_cargo_when_cross_container_fails)\(' .github/actions/rust-build-release/tests/test_target_install.py -n -C0.github/actions/rust-build-release/tests/test_cross_install.py (1)
25-35: Adopt cmd‑mox across install/upgrade paths.Good use of stubbed cross version outputs with replay/verify; keeps install flows deterministic.
docs/cmd-mox-users-guide.md (1)
226-227: Fix the broken relative link in docs/cmd-mox-users-guide.mdVerification found no file at docs/python-native-command-mocking-design.md.
File: docs/cmd-mox-users-guide.md lines 226–227
Refer to the [design document](./python-native-command-mocking-design.md) for the full table of methods and examples.Action: Add docs/python-native-command-mocking-design.md or update the href to the correct relative path from docs/.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
.github/actions/release-to-pypi-uv/README.md(1 hunks).github/actions/release-to-pypi-uv/action.yml(1 hunks).github/actions/release-to-pypi-uv/tests/test_action_python_version.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_confirm_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_determine_release.py(1 hunks)conftest.py(1 hunks)docs/cmd-mox-users-guide.md(1 hunks)docs/python-native-command-mocking-design.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For Python code, follow the Python Code Style, Context Managers, Generators, Return Patterns, and Typing guidelines under .rules/
**/*.py: Use snake_case.py for file names; names should reflect contents (e.g., http_client.py, task_queue.py)
Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix internal or non-exported helpers with a single underscore (_)
Use TypedDict or dataclass for structured data; prefer @DataClass(slots=True) for internal-only usage
Be explicit with return types (e.g., -> None, -> str) for all public functions and class methods
Favor immutability: prefer tuples to lists and MappingProxyType for read-only mappings; document any frozendict dependency
Use '# pyright: ignore' sparingly and include an explanation when used
Avoid side effects at import time; modules should not modify global state or perform actions on import
Use docstrings in NumPy format for public functions, classes, and modules
Explain tricky code with inline comments for non-obvious logic or decisions
**/*.py: Prefer @contextmanager from contextlib for straightforward procedural setup/teardown
Implement class-based context managers with enter and exit when lifecycle/state is complex or stateful
Choose @contextmanager when control flow is linear and no persistent state is required
Choose a class-based context manager when there is internal state, lifecycle-tied methods, or the need for re-entry/advanced features
Use context managers for file or network resource handling
Use context managers for lock acquisition and release
Use context managers for temporary environment changes (e.g., os.chdir, patch, tempfile)
Use context managers for logging scope control or tracing
Use context managers for transaction control in databases or services
Prefer with open(...) over manual try/finally when working with files
**/*.py: Prefer generators over complex loop logic; refactor complex for-loops into...
Files:
conftest.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep cyclomatic complexity ≤ 12
- Follow single responsibility and CQRS (command/query segregation)
- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.
Files:
conftest.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.rules/python-00.md)
**/*.{py,pyi}: Use typing everywhere and maintain full static type coverage with Pyright
Avoid Any; prefer precise types (TypeVar, Protocol, Literal, Union); use typing.cast[...] only when necessary with justification; use object for unknown-but-opaque values
Files:
conftest.py
{README.md,docs/**}
📄 CodeRabbit inference engine (.rules/python-00.md)
Colocate documentation near reusable packages: keep README.md or docs/ and include usage examples
Files:
docs/python-native-command-mocking-design.mddocs/cmd-mox-users-guide.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/python-native-command-mocking-design.mddocs/cmd-mox-users-guide.md
🧬 Code graph analysis (2)
.github/actions/release-to-pypi-uv/tests/test_confirm_release.py (2)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
base_env(28-36)shellstub.py (1)
env(169-174)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
shellstub.py (1)
env(169-174)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: python-tests (ubuntu-latest)
- GitHub Check: python-tests (windows-latest)
- GitHub Check: build-release (windows-latest, x86_64-pc-windows-gnu)
- GitHub Check: build-release (windows-latest, aarch64-pc-windows-gnu)
🔇 Additional comments (35)
.github/actions/release-to-pypi-uv/tests/test_action_python_version.py (5)
1-9: LGTM! Well-structured test module for action inputs and step validation.The module correctly uses module-level docstring, modern Python annotations, and appropriate imports for YAML parsing and path handling.
11-13: Good helper design with proper path resolution.The helper function correctly resolves the relative path to action.yml and uses safe YAML loading with explicit encoding.
16-22: Comprehensive validation of python-version input metadata.The test properly verifies both the default value and description content, ensuring the action exposes the expected interface.
25-31: Proper behavioural test for step parameter forwarding.The test correctly validates that the Setup uv step receives the python-version input through template substitution.
34-43: Effective validation of Python installation step.The test ensures the Install Python step uses the correct command with input substitution. This complements the action.yml validation.
.github/actions/release-to-pypi-uv/README.md (4)
1-5: LGTM! Clear and concise action description.The README effectively introduces the action's purpose and links to the relevant uv project.
6-21: Comprehensive input documentation with good defaults.The table format clearly presents all inputs with appropriate descriptions and sensible defaults. The python-version input is properly documented.
23-31: Helpful explanation of token forwarding behaviour.The note about GITHUB_TOKEN forwarding is particularly useful, preventing users from adding redundant environment mappings.
35-62: Well-structured usage example with proper permissions.The workflow example includes all necessary elements: concurrency control, required permissions for trusted publishing, and proper step configuration. The local action reference is appropriate for the action's own documentation.
.github/actions/release-to-pypi-uv/tests/test_confirm_release.py (4)
1-10: Good test module structure with proper imports and dependencies.The module correctly imports from the shared conftest and uses the REQUIRES_UV marker for appropriate test gating.
16-30: Well-designed test helper with proper environment setup.The helper function correctly configures the environment, uses the shared base_env, and properly handles subprocess execution with appropriate parameters.
33-37: Effective positive path test coverage.The success test properly validates both the exit code and expected output message for matching confirmation strings.
40-44: Good negative path test for failure scenarios.The failure test correctly validates error handling when confirmation strings don't match, checking both exit code and error output.
conftest.py (6)
1-11: Good module documentation and import structure.The docstring clearly describes the purpose, and imports are appropriately organised with standard library modules first.
13-20: Proper platform-specific test markers and UV detection.The Windows skip marker and UV availability check provide appropriate test gating. The module registration pattern is clever for cross-file imports.
23-28: Simple and effective UV requirement fixture.The fixture properly skips tests when UV is unavailable, preventing spurious failures in environments without the tool.
31-47: Flexible cross version stub with multiple output modes.The helper supports both simple string and iterable outputs with proper deque handling for sequence simulation. The implementation correctly handles empty sequences.
50-66: Clean helper functions for external tool stubs.Both rustup and docker stub helpers follow consistent patterns and return shim paths appropriately. The pragma comments correctly mark coverage exclusions.
68-76: Proper platform-conditional plugin loading.The conditional import of cmd_mox plugin and Windows-specific fixture provide appropriate cross-platform compatibility.
.github/actions/release-to-pypi-uv/action.yml (5)
1-36: Comprehensive action metadata with proper input definitions.The action name, description, and permissions documentation are clear. All inputs are properly defined with appropriate defaults and descriptions. The python-version input correctly sets UV_PYTHON for the rest of the workflow, matching the documented behaviour of astral-sh/setup-uv.
47-55: Proper uv setup with python-version forwarding.The setup step correctly passes the python-version input to astral-sh/setup-uv and includes appropriate cache configuration with action-specific suffix.
56-76: Well-orchestrated workflow steps with proper environment passing.The tag resolution, confirmation, and GitHub Release validation steps are properly sequenced with appropriate conditional logic and environment variable passing.
77-86: Move Python installation before script execution.The Python installation step occurs after several uv script executions, which may cause failures if the required Python version isn't available. Move this step immediately after the uv setup.
87-102: Proper build and publish sequence with comprehensive summary.The build, publish, and summary steps are well-configured with appropriate environment variables. The summary step uses
if: always()to ensure it runs regardless of previous step outcomes..github/actions/release-to-pypi-uv/tests/test_determine_release.py (6)
1-12: Good test module setup with proper gating.The module correctly imports required dependencies and uses the REQUIRES_UV marker for appropriate test execution control.
15-25: Well-designed script execution helper.The helper properly configures subprocess execution with UV run, captures output appropriately, and uses environment-specified working directory.
28-36: Robust environment setup with proper path handling.The base_env function correctly sets up PYTHONPATH, output files, and encoding while preserving existing environment variables.
39-48: Simple and effective output parser.The output reader properly handles the GitHub Actions output format with key=value pairs, including appropriate file existence checks.
51-75: Comprehensive positive path test coverage.Both test cases properly validate tag resolution from different sources (GitHub ref vs input) and verify correct tag/version output parsing.
78-148: Thorough negative path testing for error handling.The error test cases comprehensively cover various failure scenarios: invalid tags, missing inputs, empty values, and malformed versions. Each test properly validates both exit codes and error messages.
docs/python-native-command-mocking-design.md (1)
1-12: Solid structure and scope.Headings, sequencing, and contracts read cleanly and align with repository guidelines.
docs/cmd-mox-users-guide.md (4)
6-10: Link resolves and cross‑refs are correct.The related‑documents link targets an existing file and path is valid.
20-27: Remove second‑person pronoun in heading.Align with previous guidance from earlier review.
-In your `conftest.py`: +In a project’s `conftest.py`:
88-96: Eliminate “You …” phrasing.Rephrase to neutral voice.
-You can match arguments more flexibly using comparators: +Arguments can be matched more flexibly using comparators:
3-5: Remove second‑person pronoun.Rewrite to avoid “your”.
-CmdMox provides a fluent API for mocking, stubbing and spying on external -commands in your tests. This guide shows common patterns for everyday use. +CmdMox provides a fluent API for mocking, stubbing and spying on external +commands in tests. This guide shows common patterns for everyday use.
There was a problem hiding this comment.
Actionable comments posted: 17
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
.github/actions/release-to-pypi-uv/scripts/check_github_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/confirm_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/determine_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/publish_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/scripts/write_summary.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_check_github_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_confirm_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_determine_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_publish_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_write_summary.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(21-29).github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
main(97-125)
.github/actions/release-to-pypi-uv/scripts/write_summary.py (2)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
main(26-69).github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
main(53-66)
.github/actions/release-to-pypi-uv/tests/test_write_summary.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(21-29).github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-51)
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (1)
read(25-26)
.github/actions/release-to-pypi-uv/tests/test_publish_release.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(21-29).github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
main(53-66)
.github/actions/release-to-pypi-uv/tests/test_confirm_release.py (1)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
base_env(42-62)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
shellstub.py (1)
env(169-174)
.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(21-29).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (2)
main(75-146)_parse_bool(49-55)
.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (1)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
main(26-69)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (3)
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
main(97-125).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (1)
main(75-146).github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-51)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Sourcery review
- GitHub Check: python-tests (windows-latest)
- GitHub Check: python-tests (ubuntu-latest)
- GitHub Check: build-release (windows-latest, aarch64-pc-windows-gnu)
- GitHub Check: build-release (windows-latest, x86_64-pc-windows-gnu)
🔇 Additional comments (8)
.github/actions/release-to-pypi-uv/tests/test_write_summary.py (1)
25-48: LGTM: exercises default index label and basic markdown.The assertions cover the heading and default index rendering.
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (1)
175-203: LGTM: robust transient-failure path coverage.Good use of URLError simulation and sleep monkeypatch to validate retry behaviour and success message.
.github/actions/release-to-pypi-uv/tests/test_confirm_release.py (1)
49-61: LGTM: positive path validated end‑to‑end via uv.The test asserts exit code and message precisely.
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
55-63: LGTM: environment bootstrap is clean and respects prior PYTHONPATH.The base environment is constructed safely for uv runs.
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
59-65: Decide whether pre‑release/build metadata should be accepted.If pre‑release tags like v1.2.3-rc.1 (or build metadata) are valid, extend the regex; otherwise add a brief comment stating only stable releases are allowed.
- if not re.fullmatch(r"v\d+\.\d+\.\d+", resolved_tag): + if not re.fullmatch(r"v\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?", resolved_tag):.github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
40-45: Improve readability with a blank line after the heading.Add an empty line so bullets don’t run into the heading.
- heading = "## Release summary\n" + heading = "## Release summary\n\n".github/actions/release-to-pypi-uv/tests/test_publish_release.py (1)
73-92: LGTM: error propagation is correctThe test asserts that exceptions from run_cmd bubble up; this aligns with expected behaviour.
.github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
61-67: LGTM: command construction is correctThe arguments passed to uv are correct for default and custom index flows.
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
Makefile (1)
44-46: Validate the same rules in check-fmt to keep CI parity with fmt.Augment check-fmt to fail when docstring/import issues exist without modifying files.
Add the following command under check-fmt:
uvx ruff check --select $(RUFF_FIX_RULES)CRUSH.md (3)
5-6: Fix malformed bold markup and line break.Remove the stray backslash-escaped asterisk and use a standard GFM line break.
Apply this diff:
-> **Audience**: Contributors and maintainers of this repository.\\ -> \*\***Scope**: Standards and workflows for creating, versioning, testing, releasing and deprecating GitHub Actions that live **inside this monorepo**. +> **Audience**: Contributors and maintainers of this repository. +> **Scope**: Standards and workflows for creating, versioning, testing, releasing and deprecating GitHub Actions that live inside this monorepo.
111-121: Fix the broken README link.Avoid
http://README.md. Use a neutral reference or a relative link that actually resolves.Apply one of these diffs (preferred first):
-- Each action [**README.md**](http://README.md) must contain: +- Each action `README.md` must contain:or
-- Each action [**README.md**](http://README.md) must contain: +- Each action [README.md](./README.md) must contain:
148-151: Replace curly quotes with ASCII quotes in${{ github.action_path }}example.Smart quotes will break copy–paste in YAML/composite actions.
Apply this diff:
-- **Composite actions & path context** – When referencing sibling scripts, use `“${{ github.action_path }}”` for portability. +- **Composite actions & path context** – When referencing sibling scripts, use `"${{ github.action_path }}"` for portability..github/actions/setup-windows-gnu/action.yml (3)
29-45: Add download retries to harden network flakiness.Retry the llvm-mingw download with exponential backoff to reduce transient failures.
- try { - Invoke-WebRequest -Uri $url -OutFile $archive -ErrorAction Stop - } catch { - Write-Error "Failed to download llvm-mingw archive from $url" - throw - } + $attempts = 5 + for ($i = 1; $i -le $attempts; $i++) { + try { + Invoke-WebRequest -Uri $url -OutFile $archive -ErrorAction Stop + break + } catch { + if ($i -eq $attempts) { + Write-Error "Failed to download llvm-mingw archive from $url after $attempts attempts" + throw + } + $delay = [math]::Min([int][math]::Pow(2, $i), 30) + Write-Warning "Download failed (attempt $i). Retrying in ${delay}s..." + Start-Sleep -Seconds $delay + } + }
56-61: Export helpful env vars (root, bin, CC/CXX) for downstream tools.Emit LLVM paths and sane defaults for aarch64 Windows GNU to streamline usage in build steps.
$binPath | Out-File -FilePath $Env:GITHUB_PATH -Encoding utf8 -Append + Add-Content -Path $Env:GITHUB_ENV -Value "LLVM_MINGW_ROOT=$toolRoot" + Add-Content -Path $Env:GITHUB_ENV -Value "LLVM_MINGW_BIN=$binPath" + # Toolchain hints (useful for Autotools/CMake; override as needed) + Add-Content -Path $Env:GITHUB_ENV -Value "CC_aarch64_pc_windows_gnu=aarch64-w64-mingw32-clang" + Add-Content -Path $Env:GITHUB_ENV -Value "CXX_aarch64_pc_windows_gnu=aarch64-w64-mingw32-clang++"
65-73: Don’t hard-require aarch64; gate it behind an input.Avoid failing jobs that only need x86_64 by making the aarch64 check optional.
Add an input and wire it into the verification:
inputs: llvm-mingw-version: @@ llvm-mingw-sha256: description: SHA-256 checksum for the llvm-mingw archive matching the selected version required: false default: "bd88084d7a3b95906fa295453399015a1fdd7b90a38baa8f78244bd234303737" + require-aarch64: + description: Fail if no aarch64 MinGW compiler is available + required: false + default: "true"- if ! command -v aarch64-w64-mingw32-gcc >/dev/null 2>&1 \ - && ! command -v aarch64-w64-mingw32-clang >/dev/null 2>&1; then - echo "::error::No aarch64 MinGW compiler found (neither GCC nor clang)" >&2 - exit 1 - fi + require_a64="${{ inputs.require-aarch64 }}" + if ! command -v aarch64-w64-mingw32-gcc >/dev/null 2>&1 \ + && ! command -v aarch64-w64-mingw32-clang >/dev/null 2>&1; then + if [ "${require_a64}" = "true" ]; then + echo "::error::No aarch64 MinGW compiler found (neither GCC nor clang)" >&2 + exit 1 + else + echo "::warning::No aarch64 MinGW compiler found; proceeding because require-aarch64=false" >&2 + fi + fi.github/actions/rust-build-release/action.yml (1)
93-101: Use NUL delimiter correctly with mapfilemapfile -d '' is ambiguous across shells. Use an explicit NUL delimiter to robustly consume find -print0.
Apply this diff:
- mapfile -d '' -t man_matches < <( + mapfile -d $'\0' -t man_matches < <( find "target/${{ inputs.target }}/release/build" \ -path "*/out/${{ inputs.bin-name }}.1" \ -type f -print0 ).github/actions/rust-build-release/tests/test_runtime.py (1)
224-258: Add an import‑time override test for PROBE_TIMEOUTVerify that RUNTIME_PROBE_TIMEOUT influences the timeout passed to run_validated at import time.
Apply:
+def test_probe_timeout_env_override(module_harness: HarnessFactory, monkeypatch) -> None: + """Respect RUNTIME_PROBE_TIMEOUT when importing the module.""" + monkeypatch.setenv("RUNTIME_PROBE_TIMEOUT", "2") + # Re-load the module after setting the env var + from .conftest import _load_module + module = _load_module("runtime.py", "rbr_runtime_reloaded", deps=(("typer", "Typer"),)) + harness = module_harness(module) + + harness.patch_shutil_which(lambda name: "/usr/bin/rustc") + harness.patch_attr("ensure_allowed_executable", lambda path, allowed: path) + + captured: dict[str, object] = {} + def fake_run(executable: str, args: list[str], *, allowed_names: tuple[str, ...], **kwargs: object): + captured.update(kwargs) + return subprocess.CompletedProcess([executable, *args], 0, stdout="host: x86_64-unknown-linux-gnu\n") + + harness.monkeypatch.setattr(module, "run_validated", fake_run) + module.detect_host_target() + assert captured.get("timeout") == 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (26)
.github/actions/generate-coverage/tests/test_scripts.py(3 hunks).github/actions/release-to-pypi-uv/scripts/check_github_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/determine_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/scripts/write_summary.py(1 hunks).github/actions/release-to-pypi-uv/tests/_helpers.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_action_python_version.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_check_github_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_confirm_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_determine_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_publish_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_write_summary.py(1 hunks).github/actions/rust-build-release/action.yml(2 hunks).github/actions/rust-build-release/src/runtime.py(5 hunks).github/actions/rust-build-release/tests/test_cross_install.py(7 hunks).github/actions/rust-build-release/tests/test_runtime.py(3 hunks).github/actions/rust-build-release/tests/test_target_install.py(5 hunks).github/actions/rust-build-release/tests/test_utils.py(3 hunks).github/actions/setup-windows-gnu/action.yml(2 hunks).github/workflows/rust-toy-app.yml(1 hunks).gitignore(1 hunks)CRUSH.md(1 hunks)Makefile(1 hunks)conftest.py(1 hunks)shellstub.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For Python code, follow the Python Code Style, Context Managers, Generators, Return Patterns, and Typing guidelines under .rules/
**/*.py: Use snake_case.py for file names; names should reflect contents (e.g., http_client.py, task_queue.py)
Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix internal or non-exported helpers with a single underscore (_)
Use TypedDict or dataclass for structured data; prefer @DataClass(slots=True) for internal-only usage
Be explicit with return types (e.g., -> None, -> str) for all public functions and class methods
Favor immutability: prefer tuples to lists and MappingProxyType for read-only mappings; document any frozendict dependency
Use '# pyright: ignore' sparingly and include an explanation when used
Avoid side effects at import time; modules should not modify global state or perform actions on import
Use docstrings in NumPy format for public functions, classes, and modules
Explain tricky code with inline comments for non-obvious logic or decisions
**/*.py: Prefer @contextmanager from contextlib for straightforward procedural setup/teardown
Implement class-based context managers with enter and exit when lifecycle/state is complex or stateful
Choose @contextmanager when control flow is linear and no persistent state is required
Choose a class-based context manager when there is internal state, lifecycle-tied methods, or the need for re-entry/advanced features
Use context managers for file or network resource handling
Use context managers for lock acquisition and release
Use context managers for temporary environment changes (e.g., os.chdir, patch, tempfile)
Use context managers for logging scope control or tracing
Use context managers for transaction control in databases or services
Prefer with open(...) over manual try/finally when working with files
**/*.py: Prefer generators over complex loop logic; refactor complex for-loops into...
Files:
shellstub.pyconftest.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep cyclomatic complexity ≤ 12
- Follow single responsibility and CQRS (command/query segregation)
- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.
Files:
shellstub.pyconftest.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.rules/python-00.md)
**/*.{py,pyi}: Use typing everywhere and maintain full static type coverage with Pyright
Avoid Any; prefer precise types (TypeVar, Protocol, Literal, Union); use typing.cast[...] only when necessary with justification; use object for unknown-but-opaque values
Files:
shellstub.pyconftest.py
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
CRUSH.md
🧬 Code graph analysis (15)
.github/actions/release-to-pypi-uv/tests/test_write_summary.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(21-31).github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-51)
.github/actions/release-to-pypi-uv/tests/test_confirm_release.py (1)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
base_env(29-49)
.github/actions/release-to-pypi-uv/tests/test_publish_release.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(21-31).github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
main(53-66)
.github/actions/rust-build-release/tests/test_utils.py (2)
.github/actions/rust-build-release/tests/conftest.py (1)
utils_module(212-214)conftest.py (1)
cmd_mox(68-70)
.github/actions/release-to-pypi-uv/scripts/write_summary.py (2)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
main(26-55).github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
main(53-66)
.github/actions/rust-build-release/tests/test_runtime.py (2)
.github/actions/rust-build-release/tests/conftest.py (4)
runtime_module(186-192)module_harness(106-115)patch_shutil_which(89-91)patch_attr(97-99).github/actions/rust-build-release/src/runtime.py (2)
runtime_available(22-83)detect_host_target(86-124)
.github/actions/rust-build-release/tests/test_target_install.py (2)
conftest.py (4)
_register_cross_version_stub(29-44)_register_docker_info_stub(55-60)_register_rustup_toolchain_stub(47-52)cmd_mox(68-70).github/actions/rust-build-release/src/main.py (1)
main(32-166)
.github/actions/rust-build-release/tests/test_cross_install.py (3)
conftest.py (4)
_register_cross_version_stub(29-44)_register_docker_info_stub(55-60)_register_rustup_toolchain_stub(47-52)cmd_mox(68-70).github/actions/rust-build-release/tests/conftest.py (3)
module_harness(106-115)patch_shutil_which(89-91)patch_run_cmd(78-87).github/actions/rust-build-release/src/main.py (1)
main(32-166)
.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(21-31).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (2)
main(75-154)_parse_bool(49-55)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (3)
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
main(103-131).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (1)
main(75-154).github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-51)
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (1)
read(29-30)
.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (1)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
main(26-55)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
.github/actions/generate-coverage/tests/test_scripts.py (1)
run_script(30-48)
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(21-31).github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
main(103-131)
.github/actions/generate-coverage/tests/test_scripts.py (2)
.github/actions/generate-coverage/scripts/coverage_parsers.py (1)
get_line_coverage_percent_from_cobertura(32-89).github/actions/generate-coverage/scripts/run_rust.py (1)
get_line_coverage_percent_from_cobertura(120-149)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Sourcery review
- GitHub Check: python-tests (windows-latest)
- GitHub Check: python-tests (ubuntu-latest)
- GitHub Check: build-release (windows-latest, x86_64-pc-windows-gnu)
- GitHub Check: build-release (windows-latest, aarch64-pc-windows-gnu)
🔇 Additional comments (47)
.github/actions/release-to-pypi-uv/tests/test_publish_release.py (3)
51-61: LGTM: error propagation path is exercised correctlyKeep this as-is.
10-10: Insert SCRIPTS_DIR into sys.path, not REPO_ROOT, to avoid cmd_utils shadowingImport SCRIPTS_DIR and prepend that path instead of the repo root.
-from ._helpers import REPO_ROOT, load_script_module +from ._helpers import SCRIPTS_DIR, load_script_module @@ - if str(REPO_ROOT) not in module.sys.path: # type: ignore[attr-defined] - module.sys.path.insert(0, str(REPO_ROOT)) # type: ignore[attr-defined] + if str(SCRIPTS_DIR) not in module.sys.path: # type: ignore[attr-defined] + module.sys.path.insert(0, str(SCRIPTS_DIR)) # type: ignore[attr-defined]Also applies to: 16-17
21-48: Parametrise default/custom index cases to remove duplicationReplace the two near-identical tests with one parametrised test.
-def test_publish_default_index( - monkeypatch: pytest.MonkeyPatch, publish_module: typ.Any -) -> None: - calls: list[list[str]] = [] - - def fake_run_cmd(args: list[str], **_: object) -> None: - calls.append(args) - - monkeypatch.setattr(publish_module, "run_cmd", fake_run_cmd) - - publish_module.main(index="") - - assert calls == [["uv", "publish"]] - - -def test_publish_custom_index( - monkeypatch: pytest.MonkeyPatch, publish_module: typ.Any -) -> None: - calls: list[list[str]] = [] - - def fake_run_cmd(args: list[str], **_: object) -> None: - calls.append(args) - - monkeypatch.setattr(publish_module, "run_cmd", fake_run_cmd) - - publish_module.main(index="testpypi") - - assert calls == [["uv", "publish", "--index", "testpypi"]] +@pytest.mark.parametrize( + "index,expected", + [ + ("", [["uv", "publish"]]), + ("testpypi", [["uv", "publish", "--index", "testpypi"]]), + ], +) +def test_publish_index_variants( + monkeypatch: pytest.MonkeyPatch, + publish_module: typ.Any, + index: str, + expected: list[list[str]], +) -> None: + calls: list[list[str]] = [] + + def fake_run_cmd(args: list[str], **_: object) -> None: + calls.append(args) + + monkeypatch.setattr(publish_module, "run_cmd", fake_run_cmd) + publish_module.main(index=index) + assert calls == expectedCRUSH.md (3)
57-76: Versioning and tagging guidance reads well.SemVer rules, tag format, and mutable major tags are clearly specified and align with common GitHub Actions practice.
1-1: Start the document with a level-1 heading; fix filename/title mismatch.Comply with the Markdown guideline: documents must start with an H1. The bare
AGENTS.mdline is not a heading and does not match the file’s name.Apply this diff:
-AGENTS.md +# AgentsLikely an incorrect or invalid review comment.
162-170: Resolve: referenced .rules files exist.
Record verification output: OK for every target.shellstub.py (1)
9-12: Defer type-only imports — keep cabc/Path under TYPE_CHECKING; verify no runtime evaluation of annotations.File: shellstub.py (lines 9–12)
if typ.TYPE_CHECKING:
import collections.abc as cabc
from pathlib import PathEnsure nothing in the repo calls typing.get_type_hints (or similar) at runtime; such calls require those names in module globals. Run the targeted search below and paste the output.
#!/bin/bash set -euo pipefail PAT='(get_type_hints|typing\.get_type_hints|from\s+typing\s+import\s+get_type_hints|inspect\.signature|__annotations__\s*=)' if command -v rg >/dev/null 2>&1; then rg -n -C2 --hidden --glob '!**/.git/**' --glob '!**/venv/**' "$PAT" || true elif command -v grep >/dev/null 2>&1; then grep -nR -E -C2 --exclude-dir=.git --exclude-dir=venv "$PAT" . || true else python - <<'PY' import os,re p=re.compile(r'(get_type_hints|typing\.get_type_hints|from\s+typing\s+import\s+get_type_hints|inspect\.signature|__annotations__\s*=)') for root,dirs,files in os.walk('.'): dirs[:] = [d for d in dirs if d not in ('.git','venv','env','.venv')] for f in files: if not f.endswith('.py'): continue path=os.path.join(root,f) try: with open(path,'r',encoding='utf-8') as fh: for i,line in enumerate(fh,1): if p.search(line): print(f"{path}:{i}:{line.rstrip()}") except: pass PY fi.github/actions/setup-windows-gnu/action.yml (2)
17-18: Good clarification on aarch64 cross-linkers.Keep the intent explicit near the MSYS2 step.
10-12: Validate the llvm-mingw defaults and keep them fresh.Ensure the default SHA-256 matches the default version asset and plan for periodic bumps or drop the defaults to force explicit inputs.
Fetch the GitHub release “20250910” for mstorsjo/llvm-mingw and verify the SHA-256 of asset “llvm-mingw-20250910-ucrt-x86_64.zip” equals bd88084d7a3b95906fa295453399015a1fdd7b90a38baa8f78244bd234303737..github/actions/generate-coverage/tests/test_scripts.py (5)
467-472: LGTMZero‑lines LCOV case is correctly asserted to 0.00.
551-556: LGTMRoot‑totals Cobertura fallback is validated correctly.
559-565: LGTMZero‑totals Cobertura case handled as 0.00.
567-574: LGTMMalformed XML raises typer.Exit as expected.
661-668: LGTMMissing Cobertura file error path is covered and asserts exit code.
.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (4)
1-5: Pin Typer dependency in script metadataAvoid unbounded deps in script headers to reduce upstream breakage.
Apply this diff:
# /// script # requires-python = ">=3.13" -# dependencies = ["typer"] +# dependencies = ["typer>=0.12,<0.13"] # ///
24-33: Broaden SKIP_PARTS to cut IO on large reposSkip common caches/coverage artefacts.
Apply this diff:
SKIP_PARTS = { @@ ".mypy_cache", + ".pytest_cache", + ".cache", + "htmlcov", }
38-46: Stream and sort discovery with iglob for determinism and lower memoryIterate with glob.iglob and sort for stable order.
Apply this diff:
-def _iter_files(pattern: str) -> typ.Iterable[Path]: - candidates = [Path(p) for p in glob.glob(pattern, recursive=True)] - for path in candidates: +def _iter_files(pattern: str) -> typ.Iterable[Path]: + for candidate in sorted(glob.iglob(pattern, recursive=True)): + path = Path(candidate) if not path.is_file(): continue parts = set(path.parts) if parts & SKIP_PARTS: continue yield path
17-23: Gate “no matches” behaviour behind INPUT_FAIL_ON_EMPTYFail explicitly on zero matches when requested; keep warn‑and‑continue default.
Apply this diff:
PATTERN_OPTION = typer.Option("**/pyproject.toml", envvar="INPUT_TOML_GLOB") FAIL_ON_DYNAMIC_OPTION = typer.Option( "false", envvar="INPUT_FAIL_ON_DYNAMIC_VERSION", ) +FAIL_ON_EMPTY_OPTION = typer.Option( + "false", + envvar="INPUT_FAIL_ON_EMPTY", +) @@ -def main( +def main( version: str = VERSION_OPTION, pattern: str = PATTERN_OPTION, - fail_on_dynamic: str = FAIL_ON_DYNAMIC_OPTION, + fail_on_dynamic: str = FAIL_ON_DYNAMIC_OPTION, + fail_on_empty: str = FAIL_ON_EMPTY_OPTION, ) -> None: @@ - files = list(_iter_files(pattern)) - if not files: - typer.echo(f"::warning::No TOML files matched pattern {pattern}") - return + files = list(_iter_files(pattern)) + if not files: + if _parse_bool(fail_on_empty): + typer.echo(f"::error::No TOML files matched pattern {pattern}", err=True) + raise typer.Exit(1) + typer.echo(f"::warning::No TOML files matched pattern {pattern}") + returnUpdate the composite action inputs and README to document INPUT_FAIL_ON_EMPTY.
~~~web Add an action input named "fail-on-empty" (envvar INPUT_FAIL_ON_EMPTY) defaulting to "false" in .github/actions/release-to-pypi-uv/action.yml and document it in the README’s inputs table.Also applies to: 97-101 </blockquote></details> <details> <summary>.github/actions/release-to-pypi-uv/tests/test_write_summary.py (1)</summary><blockquote> `18-26`: **Exercise of defaults and append behaviour is sound.** Invoke the CLI with an empty index to hit the “pypi (default)” branch and write to a new file; this is the right coverage. </blockquote></details> <details> <summary>.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (1)</summary><blockquote> `38-52`: **Happy path coverage is correct.** Mock urlopen, run main, and assert the success message; this validates the green path cleanly. </blockquote></details> <details> <summary>.github/actions/release-to-pypi-uv/tests/test_confirm_release.py (1)</summary><blockquote> `15-31`: **Drive the script via uv with a realistic env.** Build the environment, call uv run --script, and plumb stdout/stderr with encoding; this is the right shape for an end‑to‑end CLI test. </blockquote></details> <details> <summary>.github/actions/release-to-pypi-uv/scripts/write_summary.py (1)</summary><blockquote> `39-51`: **Emit a clean, Markdown‑friendly summary.** Compute the index label, prepend a newline only when appending to non‑empty files, and write a single heading with bullets; this meets the readability goal and matches tests. </blockquote></details> <details> <summary>.github/actions/rust-build-release/tests/test_utils.py (1)</summary><blockquote> `38-62`: **Adopt cmd‑mox stubs and verify invocation fidelity.** Use the shimmed docker.exe, replay/verify around run_validated, and assert args[0] equals the validated path; this improves determinism and removes brittle monkeypatching. </blockquote></details> <details> <summary>conftest.py (1)</summary><blockquote> `12-19`: **Provide portable markers and UV gating.** Expose CMD_MOX_UNSUPPORTED and REQUIRES_UV, alias the module as shared_actions_conftest, and add require_uv; this cleanly centralises test controls. </blockquote></details> <details> <summary>.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)</summary><blockquote> `20-24`: **Future‑proof GitHub output writes (multiline syntax).** Write outputs using the multiline form to avoid edge cases if values ever contain special characters. Update tests that parse GITHUB_OUTPUT accordingly. Apply this diff: ```diff def _emit_outputs(dest: Path, tag: str, version: str) -> None: - with dest.open("a", encoding="utf-8") as fh: - fh.write(f"tag={tag}\n") - fh.write(f"version={version}\n") + with dest.open("a", encoding="utf-8") as fh: + for key, val in (("tag", tag), ("version", version)): + fh.write(f"{key}<<__EOF__\n{val}\n__EOF__\n") ``` </blockquote></details> <details> <summary>.github/actions/release-to-pypi-uv/tests/_helpers.py (1)</summary><blockquote> `12-19`: **Ignore the off‑by‑one claim — keep REPO_ROOT as-is.** Keep REPO_ROOT computed from _action_root.parents[2]. Verification shows SCRIPTS_DIR = /home/jailuser/git/.github/actions/release-to-pypi-uv/scripts and REPO_ROOT = /home/jailuser/git; _action_root.parents[2] == SCRIPTS_DIR.parents[3] == repository root (parents[1] is '.github'). Do not apply the proposed diff. > Likely an incorrect or invalid review comment. </blockquote></details> <details> <summary>.github/actions/rust-build-release/tests/test_runtime.py (3)</summary><blockquote> `38-61`: **Treat runtime discovery timeouts as unavailable — LGTM** The timeout path is exercised cleanly and asserts the correct False outcome. --- `128-156`: **Gate podman on security probe timeout — LGTM** Correctly models a timeout during the security check as “unavailable”. --- `194-222`: **Fallback to default triple on rustc timeout — LGTM** The test validates graceful degradation on slow/hung rustc. </blockquote></details> <details> <summary>.github/actions/rust-build-release/tests/test_target_install.py (3)</summary><blockquote> `22-60`: **Adopt cmd‑mox for tool discovery — LGTM** Stub registration and which‑patching are tidy; replay/verify brackets execution correctly, and the assertions prove cross usage. --- `65-99`: **Error when target unsupported without cross — LGTM** The test isolates the failure path and asserts the emitted error reliably. --- `104-141`: **Fallback to cargo when cross container fails — LGTM** The side‑effect with return code 125 is appropriate and the final assertions lock the cargo fallback. </blockquote></details> <details> <summary>.github/actions/rust-build-release/tests/test_cross_install.py (7)</summary><blockquote> `26-47`: **Install cross when missing — LGTM** The staged which responses model first‑run install accurately and assert the correct cargo install args. --- `81-111`: **Upgrade outdated cross — LGTM** The rolling version stub and assertions cover the upgrade path cleanly. --- `113-135`: **Use cached cross — LGTM** The absence of calls validates the no‑op behaviour when version is sufficient. --- `137-172`: **Install prebuilt cross on Windows — LGTM** The test enforces the release path and avoids cargo install on Windows hosts. --- `351-391`: **Install cross even without container runtimes — LGTM** The flow ensures installation proceeds and that the build falls back to cargo with the correct toolchain spec. --- `393-427`: **Fallback to git when crates.io fails — LGTM** The two‑attempt flow and assertions on --git/--tag are robust. --- `429-462`: **Fallback to cargo when runtime unusable — LGTM** The docker info non‑zero exit models an unusable runtime and asserts cargo usage definitively. </blockquote></details> <details> <summary>.github/actions/release-to-pypi-uv/tests/test_determine_release.py (2)</summary><blockquote> `14-26`: **Use explicit working directory — LGTM** Running via uv with cwd from env stabilises relative paths; keep check=False to assert return codes manually. --- `29-49`: **Harden base env for uv runs — LGTM** PYTHONPATH composition and PWD/IO encoding setup are appropriate for hermetic script execution. </blockquote></details> <details> <summary>.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (2)</summary><blockquote> `51-67`: **Tighten the success assertion** Assert the deterministic success line to lock the checked count. Apply: ```diff - captured = capsys.readouterr() - assert "all versions match 1.0.0" in captured.out + captured = capsys.readouterr() + assert "Checked 1 PEP 621 project file(s); all versions match 1.0.0." in captured.out ``` --- `254-283`: **Add a failure test for missing [project].version** Exercise the non‑dynamic missing version error path. Apply: ```diff @@ def test_missing_project_section_is_ignored( @@ assert captured.err == "" + + +def test_fails_when_project_version_missing( + project_root: Path, + module: typ.Any, + capsys: pytest.CaptureFixture[str], +) -> None: + _write_pyproject( + project_root / "pkg", + """ +[project] +name = "demo" +""", + ) + with pytest.raises(module.typer.Exit): + _invoke_main(module, version="1.0.0") + captured = capsys.readouterr() + assert "missing [project].version and not marked dynamic" in captured.err ``` </blockquote></details> <details> <summary>.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)</summary><blockquote> `47-70`: **Handle HTTP 401 unauthorised explicitly** Surface invalid/expired tokens with an actionable message instead of falling into the generic branch. Apply: ```diff except urllib.error.HTTPError as exc: # pragma: no cover - network failure path detail = ( exc.read().decode("utf-8", errors="ignore") if hasattr(exc, "read") else "" ) if exc.code == 404: raise GithubReleaseError( f"No GitHub release found for tag {tag}. Create and publish the release first." ) from exc + if exc.code == 401: + msg = "GitHub API returned 401 Unauthorized. Ensure GH_TOKEN is valid and not expired." + context = detail or exc.reason + raise GithubReleaseError(f"{msg} ({context})") from exc if exc.code == 403: msg = ( "GitHub token lacks permission to read releases or has expired. " "Ensure the workflow is using GITHUB_TOKEN with contents:read scope." ) context = detail or exc.reason raise GithubReleaseError(f"{msg} ({context})") from exc ``` </blockquote></details> <details> <summary>.github/workflows/rust-toy-app.yml (3)</summary><blockquote> `40-43`: **Replace Out-File with Add-Content to avoid writing a UTF‑8 BOM to $GITHUB_ENV.** Out-File can emit a UTF‑8 BOM on file creation in some PowerShell versions; use Add-Content -Encoding utf8 to append without a BOM and keep the step idempotent. File: .github/workflows/rust-toy-app.yml (lines 40–43) ~~~diff - shell: pwsh - run: | - "LLVM_MINGW_VERSION=20250910" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append - "LLVM_MINGW_SHA256=bd88084d7a3b95906fa295453399015a1fdd7b90a38baa8f78244bd234303737" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append + shell: pwsh + run: | + Add-Content -Path $env:GITHUB_ENV -Value 'LLVM_MINGW_VERSION=20250910' -Encoding utf8 + Add-Content -Path $env:GITHUB_ENV -Value 'LLVM_MINGW_SHA256=bd88084d7a3b95906fa295453399015a1fdd7b90a38baa8f78244bd234303737' -Encoding utf8Run this on a Windows runner after the step to verify no BOM (previous attempt failed because GITHUB_ENV was not set):
python - <<'PY' import os, sys p = os.environ.get('GITHUB_ENV') if not p: print('GITHUB_ENV not set') sys.exit(2) with open(p, 'rb') as f: b = f.read(3) print('BOM present' if b == b'\xef\xbb\xbf' else 'No BOM') PY
51-55: Harden rust-build-release against Podman detection timeouts — verify action inputs or add a knobRun the diagnostic below from the repository root and paste the full output.
#!/bin/bash set -euo pipefail echo "PWD: $(pwd)" echo echo "=== List .github and .github/actions ===" ls -la .github || true ls -la .github/actions || true echo if [ -d ".github/actions/rust-build-release" ]; then echo "FOUND: .github/actions/rust-build-release" find .github/actions/rust-build-release -type f -maxdepth 5 -print echo for f in .github/actions/rust-build-release/action.yml .github/actions/rust-build-release/action.yaml; do if [ -f "$f" ]; then echo "---- $f ----" sed -n '1,240p' "$f" || true fi done echo echo "---- Search for timeout/podman/docker/runtime inputs or strings ----" rg -n --hidden -uu -S -C3 -e 'inputs:' -e 'timeout' -e 'container' -e 'runtime' -e 'podman' -e 'docker' -e 'TimeoutExpired' -e 'subprocess\.run' -e 'timeout=' .github/actions/rust-build-release || true echo if [ -d ".github/actions/rust-build-release/src" ]; then echo "---- Head of source files (up to 10) ----" find .github/actions/rust-build-release/src -type f | head -n 10 | while read -r file; do echo "==== $file ====" sed -n '1,120p' "$file" || true done fi else echo "NOT FOUND: .github/actions/rust-build-release" echo "Searching whole repo for rust-build-release, podman, timeout occurrences..." rg -n --hidden -uu -S -C3 -e 'rust-build-release' -e 'podman' -e 'docker' -e 'TimeoutExpired' -e 'subprocess\.run' -e 'timeout=' || true fiIf the action exposes an input to disable runtime detection or set a timeout, set that input on the rust-build-release step in .github/workflows/rust-toy-app.yml for ubuntu-latest. If no input exists, add inputs to action.yml (e.g., runtime_detection: 'skip'|'auto' and runtime_detection_timeout: ), default them to 'skip' in CI, and wire them into the workflow to avoid spurious Podman timeouts.
42-43: Validate llvm-mingw version/sha256 pair before toolchain setup.Assert LLVM_MINGW_SHA256 matches the downloaded llvm-mingw release asset and fail the job on mismatch to guard against supply‑chain drift.
Check .github/workflows/rust-toy-app.yml (lines 42–43) and verify the custom action at .github/actions/setup-windows-gnu enforces the checksum:
# Confirm action directory exists and search for checksum enforcement if [ -d ".github/actions/setup-windows-gnu" ]; then rg -uu -n -C2 -g ".github/actions/setup-windows-gnu/**" -e 'sha256' -e 'Get-FileHash' -e 'sha256sum' -e 'shasum' -e 'Verify' -e 'checksum' rg -uu -n -g ".github/actions/setup-windows-gnu/**" -e 'inputs:' -e 'llvm-mingw-sha256' else echo "MISSING: .github/actions/setup-windows-gnu directory" fiIf checksum enforcement is absent, add a preflight step immediately before “Configure Windows GNU toolchains” that downloads the 20250910 asset and verifies the SHA256 with Get-FileHash; fail fast on mismatch. Provide the asset filename pattern and I will prepare a patch.
leynos
force-pushed
the
python-lib-release-action
branch
from
2401e23 to
def8d17
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
CRUSH.md (3)
111-121: Fix the README link and simplify formatting.Use a relative link and drop bold inside the link text to keep Markdown clean and portable.
Apply:
-- Each action [**README.md**](http://README.md) must contain: +- Each action [README.md](README.md) must contain:
148-149: Replace smart quotes with ASCII quotes for copy‑paste reliability.Smart quotes around expressions break usage when copied from docs.
Apply:
-- **Composite actions & path context** – When referencing sibling scripts, use “${{ github.action_path }}” for portability. +- **Composite actions & path context** – When referencing sibling scripts, use "${{ github.action_path }}" for portability.
89-91: Clarifyactguidance for macOS.
nektos/actruns Linux containers and does not execute macOS runners. Recommend using it forubuntu-latestlocally and gate macOS smoke tests to GitHub‑hosted jobs.Apply:
-- **Integration smoke test**: Run via `nektos/act` in CI for the matrix `ubuntu-latest`, `macos-latest` (where feasible). +- **Integration smoke test**: Run `nektos/act` locally for `ubuntu-latest`. Execute `macos-latest` smoke tests on GitHub‑hosted runners..github/actions/rust-build-release/tests/test_cross_install.py (1)
464-487: Fix stream assertion: warning is written to stderr, not stdout.Read from err and assert the exact message to lock the contract.
- out = capsys.readouterr().out.lower() - assert "warning" in out + io = capsys.readouterr() + msg = io.err.lower() + assert "warning" in msg + assert "cross install failed; continuing without cross" in msg
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
.github/actions/generate-coverage/tests/test_scripts.py(3 hunks).github/actions/release-to-pypi-uv/CHANGELOG.md(1 hunks).github/actions/release-to-pypi-uv/README.md(1 hunks).github/actions/release-to-pypi-uv/action.yml(1 hunks).github/actions/release-to-pypi-uv/scripts/check_github_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/confirm_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/determine_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/publish_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/scripts/write_summary.py(1 hunks).github/actions/release-to-pypi-uv/tests/_helpers.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_action_python_version.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_check_github_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_confirm_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_determine_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_publish_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_write_summary.py(1 hunks).github/actions/rust-build-release/tests/conftest.py(0 hunks).github/actions/rust-build-release/tests/test_cross_install.py(7 hunks).github/actions/rust-build-release/tests/test_smoke.py(2 hunks).github/actions/rust-build-release/tests/test_target_install.py(5 hunks).github/actions/rust-build-release/tests/test_utils.py(3 hunks).gitignore(1 hunks)CRUSH.md(1 hunks)Makefile(1 hunks)README.md(1 hunks)conftest.py(1 hunks)docs/cmd-mox-users-guide.md(1 hunks)docs/python-native-command-mocking-design.md(1 hunks)docs/scripting-standards.md(1 hunks)pyproject.toml(1 hunks)
💤 Files with no reviewable changes (1)
- .github/actions/rust-build-release/tests/conftest.py
🧰 Additional context used
📓 Path-based instructions (7)
{README.md,docs/**}
📄 CodeRabbit inference engine (.rules/python-00.md)
Colocate documentation near reusable packages: keep README.md or docs/ and include usage examples
Files:
docs/python-native-command-mocking-design.mddocs/scripting-standards.mddocs/cmd-mox-users-guide.mdREADME.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/python-native-command-mocking-design.mddocs/scripting-standards.mdCRUSH.mddocs/cmd-mox-users-guide.mdREADME.md
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For Python code, follow the Python Code Style, Context Managers, Generators, Return Patterns, and Typing guidelines under .rules/
**/*.py: Use snake_case.py for file names; names should reflect contents (e.g., http_client.py, task_queue.py)
Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix internal or non-exported helpers with a single underscore (_)
Use TypedDict or dataclass for structured data; prefer @DataClass(slots=True) for internal-only usage
Be explicit with return types (e.g., -> None, -> str) for all public functions and class methods
Favor immutability: prefer tuples to lists and MappingProxyType for read-only mappings; document any frozendict dependency
Use '# pyright: ignore' sparingly and include an explanation when used
Avoid side effects at import time; modules should not modify global state or perform actions on import
Use docstrings in NumPy format for public functions, classes, and modules
Explain tricky code with inline comments for non-obvious logic or decisions
**/*.py: Prefer @contextmanager from contextlib for straightforward procedural setup/teardown
Implement class-based context managers with enter and exit when lifecycle/state is complex or stateful
Choose @contextmanager when control flow is linear and no persistent state is required
Choose a class-based context manager when there is internal state, lifecycle-tied methods, or the need for re-entry/advanced features
Use context managers for file or network resource handling
Use context managers for lock acquisition and release
Use context managers for temporary environment changes (e.g., os.chdir, patch, tempfile)
Use context managers for logging scope control or tracing
Use context managers for transaction control in databases or services
Prefer with open(...) over manual try/finally when working with files
**/*.py: Prefer generators over complex loop logic; refactor complex for-loops into...
Files:
conftest.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep cyclomatic complexity ≤ 12
- Follow single responsibility and CQRS (command/query segregation)
- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.
Files:
conftest.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.rules/python-00.md)
**/*.{py,pyi}: Use typing everywhere and maintain full static type coverage with Pyright
Avoid Any; prefer precise types (TypeVar, Protocol, Literal, Union); use typing.cast[...] only when necessary with justification; use object for unknown-but-opaque values
Files:
conftest.py
**/pyproject.toml
📄 CodeRabbit inference engine (AGENTS.md)
For Python projects, follow the Python Project Configuration guidance in .rules/python-pyproject.md
Files:
pyproject.toml
pyproject.toml
📄 CodeRabbit inference engine (.rules/python-00.md)
pyproject.toml: Enable Ruff and configure it (linting, fixers, formatting) via pyproject.toml
Configure Pyright in strict mode and treat all warnings as CI errors
Use pyproject.toml to configure Ruff, Pyright, and Pytest
Use Ruff for formatting (let Ruff handle whitespace and formatting entirely)
pyproject.toml: Use PEP 621[project]as the single source of project metadata and runtime dependencies (avoid separate setup.py)
Include mandatory[project]fields:nameandversion
Provide recommended[project]metadata:description,readme,requires-python,license,authors,keywords,classifiers
Declare runtimedependenciesin PEP 508 syntax under[project]
Prefer exact or bounded version ranges for dependencies (e.g.,requests>=2.25,<3.0)
Use[project.optional-dependencies]to separate groups likedevanddocs
Expose CLIs via[project.scripts]and GUIs via[project.gui-scripts]; register plugins via[project.entry-points.'group.name']
Declare a PEP 517[build-system]withrequires = ["setuptools>=61.0", "wheel"]andbuild-backend = "setuptools.build_meta"when using setuptools
If omitting[build-system], set[tool.uv].package = trueto ensure your project is built/installed by uv
Add[tool.uv]configuration as needed; commonlypackage = true
Setrequires-pythonin[project]to declare supported interpreter versions
Use valid Trove classifiers in[project].classifiers
Follow semantic versioning forversion(e.g., 1.2.3)
Use dynamic fields (e.g.,dynamic = ["version"]) sparingly and only if supported by the build backend
Minimize build constraints; omit[build-system]if editable installs aren’t needed (or explicitly set[tool.uv].package = true)
Files:
pyproject.toml
{pyproject.toml,uv.lock}
📄 CodeRabbit inference engine (.rules/python-pyproject.md)
Maintain lockfile discipline: after changing
[project]fields ordependencies, runuv sync/lockto updateuv.lock
Files:
pyproject.toml
🧬 Code graph analysis (15)
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(22-32).github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
main(103-131)
.github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
.github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-51)
.github/actions/release-to-pypi-uv/tests/test_confirm_release.py (1)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
base_env(29-49)
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (1)
read(30-31)
.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (1)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
main(26-55)
.github/actions/rust-build-release/tests/test_utils.py (2)
.github/actions/rust-build-release/tests/conftest.py (1)
utils_module(212-214)conftest.py (1)
cmd_mox(68-70)
.github/actions/release-to-pypi-uv/scripts/write_summary.py (2)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
main(26-55).github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
main(53-66)
.github/actions/rust-build-release/tests/test_target_install.py (3)
conftest.py (4)
_register_cross_version_stub(29-44)_register_docker_info_stub(55-60)_register_rustup_toolchain_stub(47-52)cmd_mox(68-70).github/actions/rust-build-release/tests/conftest.py (3)
module_harness(106-115)patch_shutil_which(89-91)patch_attr(97-99).github/actions/rust-build-release/src/main.py (1)
main(56-193)
.github/actions/generate-coverage/tests/test_scripts.py (2)
.github/actions/generate-coverage/scripts/coverage_parsers.py (1)
get_line_coverage_percent_from_cobertura(32-89).github/actions/generate-coverage/scripts/run_rust.py (1)
get_line_coverage_percent_from_cobertura(120-149)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (3)
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
main(103-131).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (1)
main(75-154).github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-51)
.github/actions/release-to-pypi-uv/tests/test_write_summary.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(22-32).github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-51)
.github/actions/release-to-pypi-uv/tests/test_publish_release.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(22-32).github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
main(53-66)
.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(22-32).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (2)
main(75-154)_parse_bool(49-55)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
.github/actions/generate-coverage/tests/test_scripts.py (1)
run_script(30-48)
.github/actions/rust-build-release/tests/test_cross_install.py (3)
conftest.py (4)
_register_cross_version_stub(29-44)_register_docker_info_stub(55-60)_register_rustup_toolchain_stub(47-52)cmd_mox(68-70).github/actions/rust-build-release/tests/conftest.py (2)
patch_shutil_which(89-91)patch_run_cmd(78-87).github/actions/rust-build-release/src/cross_manager.py (1)
ensure_cross(164-249)
🪛 GitHub Actions: CI
.github/actions/rust-build-release/tests/test_smoke.py
[error] 1-1: Test failure: test_action_builds_release_binary_and_manpage[x86_64-pc-windows-msvc] XPASS(strict) Known failure on Windows; see #93. Process completed with exit code 1.
[error] 1-1: Test failure: test_action_builds_release_binary_and_manpage[x86_64-pc-windows-gnu] XPASS(strict) Known failure on Windows; see #93. Process completed with exit code 1.
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (56)
CRUSH.md (1)
162-170: Verify referenced.rules/documents exist.Automated check failed in this environment; run the following locally and report missing files.
#!/bin/sh missing=0 for p in \ .rules/python-00.md \ .rules/python-context-managers.md \ .rules/python-generators.md \ .rules/python-pyproject.md \ .rules/python-return.md \ .rules/python-typing.md; do if [ -f "$p" ]; then echo "OK: $p" else echo "MISSING: $p" missing=1 fi done exit $missingREADME.md (1)
12-12: Document the new action entry.Entry reads well and matches the path/version.
docs/scripting-standards.md (1)
1-80: Adopt scripting standards doc.Content meets the docs style (headings, en‑GB, no second person) and provides actionable guidance.
.gitignore (1)
159-160: Anchor Crush artefact ignore to the repository root.Prevent overmatching nested directories by anchoring the pattern.
-.crush/ +/.crush/Also anchor the adjacent uv entries as previously flagged (apply near Lines 156–157):
/uv.lock /.uv/Makefile (1)
42-42: Parameterise Ruff auto-fix selection.Drive rule selection via a variable to avoid churn in recipes.
- uvx ruff check --select D202,I001 --fix + uvx ruff check --select $(RUFF_FIX_RULES) --fixAdd near the top:
RUFF_FIX_RULES ?= D,IUpdate the target description (Line 40) to: “Format Python files and auto-fix selected lint rules”.
pyproject.toml (1)
31-31: Pin the VCS dependency to an immutable revision.Float-freeze cmd-mox for reproducible CI; use a tag or commit SHA. If private, ensure credentials are wired in CI.
- "cmd-mox@git+https://github.com/leynos/cmd-mox.git", + "cmd-mox@git+https://github.com/leynos/cmd-mox.git@<commit-sha>",Enable strict Pyright per repository standards (absent at present):
[tool.pyright] typeCheckingMode = "strict" reportUnknownParameterType = true reportUnknownMemberType = true reportMissingTypeStubs = "error" pythonVersion = "3.12" venvPath = "." venv = ".venv"docs/cmd-mox-users-guide.md (1)
42-43: Use en‑GB “behaviour”.Align with repository docs style.
-1. **Record** – describe each expected command call, including its arguments - and behavior. +1. **Record** – describe each expected command call, including its arguments + and behaviour.docs/python-native-command-mocking-design.md (3)
29-31: Use en‑GB “behaviour”.Match the mandated spelling in design docs.
- deterministic behavior. + deterministic behaviour.
90-92: Switch to Oxford “‑ize” form.Use initializes per en‑GB‑oxendict.
-manager initialises the controller. +manager initializes the controller.
107-107: Use en‑GB “behaviour”.Align the final occurrence.
- identical behavior given the same expectations and inputs. + identical behaviour given the same expectations and inputs..github/actions/release-to-pypi-uv/scripts/confirm_release.py (1)
31-36: Avoid leaking the confirmation phrase; normalise input before compare.Do not echo the expected phrase into logs and tolerate trivial whitespace issues.
- if confirm != expected: - typer.echo( - f"::error::Confirmation failed. Set the 'confirm' input to: {expected}", - err=True, - ) + # Normalise whitespace to reduce accidental mismatches + expected = " ".join(expected.split()) + confirm = " ".join(confirm.split()) + if confirm != expected: + typer.echo("::error::Confirmation failed. Set the 'confirm' input to the expected phrase.", err=True) raise typer.Exit(1).github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
20-52: Emit summary looks good; interface is clear.Retain as-is. The newline prefix prevents header collisions in step summaries, and the env-mapped Typer options make the CLI predictable.
.github/actions/generate-coverage/tests/test_scripts.py (5)
467-472: LCOV zero‑lines test reads clean.
551-556: Cobertura root totals test is correct and matches parser behaviour.
559-564: Cobertura zero totals case exercised properly.
567-574: Malformed XML failure path asserted appropriately.
661-668: Missing file exit path asserted with exit code check..github/actions/release-to-pypi-uv/tests/test_publish_release.py (3)
52-65: Tighten failure assertion by matching the error message.Assert the raised message to lock the contract.
- with pytest.raises(DummyError): - publish_module.main(index="") + with pytest.raises(DummyError, match="uv publish failed"): + publish_module.main(index="")
11-19: Prevent unintended module shadowing; insert scripts dir not repo root.Import SCRIPTS_DIR and prepend it to sys.path to avoid picking unrelated cmd_utils modules.
-from ._helpers import REPO_ROOT, load_script_module +from ._helpers import SCRIPTS_DIR, load_script_module @@ def fixture_publish_module() -> ModuleType: module = load_script_module("publish_release") - if str(REPO_ROOT) not in module.sys.path: # type: ignore[attr-defined] - module.sys.path.insert(0, str(REPO_ROOT)) # type: ignore[attr-defined] + if str(SCRIPTS_DIR) not in module.sys.path: # type: ignore[attr-defined] + module.sys.path.insert(0, str(SCRIPTS_DIR)) # type: ignore[attr-defined] return module
22-49: Parametrise default/custom index tests to remove duplication.Replace the two near‑identical tests with one parametrised test.
-def test_publish_default_index( - monkeypatch: pytest.MonkeyPatch, publish_module: ModuleType -) -> None: - calls: list[list[str]] = [] - - def fake_run_cmd(args: list[str], **_: object) -> None: - calls.append(args) - - monkeypatch.setattr(publish_module, "run_cmd", fake_run_cmd) - - publish_module.main(index="") - - assert calls == [["uv", "publish"]] - - -def test_publish_custom_index( - monkeypatch: pytest.MonkeyPatch, publish_module: ModuleType -) -> None: - calls: list[list[str]] = [] - - def fake_run_cmd(args: list[str], **_: object) -> None: - calls.append(args) - - monkeypatch.setattr(publish_module, "run_cmd", fake_run_cmd) - - publish_module.main(index="testpypi") - - assert calls == [["uv", "publish", "--index", "testpypi"]] +@pytest.mark.parametrize( + "index,expected", + [ + ("", [["uv", "publish"]]), + ("testpypi", [["uv", "publish", "--index", "testpypi"]]), + ], +) +def test_publish_index_variants( + monkeypatch: pytest.MonkeyPatch, + publish_module: ModuleType, + index: str, + expected: list[list[str]], +) -> None: + calls: list[list[str]] = [] + + def fake_run_cmd(args: list[str], **_: object) -> None: + calls.append(args) + + monkeypatch.setattr(publish_module, "run_cmd", fake_run_cmd) + publish_module.main(index=index) + assert calls == expected.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
20-24: Future‑proof GITHUB_OUTPUT writes using multiline syntax.Guard against future values containing special characters by switching to the Actions multiline output format.
def _emit_outputs(dest: Path, tag: str, version: str) -> None: - with dest.open("a", encoding="utf-8") as fh: - fh.write(f"tag={tag}\n") - fh.write(f"version={version}\n") + with dest.open("a", encoding="utf-8") as fh: + for key, val in (("tag", tag), ("version", version)): + fh.write(f"{key}<<__EOF__\n{val}\n__EOF__\n").github/actions/rust-build-release/tests/test_smoke.py (1)
65-69: Deduplicate Windows target checks.Collapse the repeated endswith logic.
- if target.endswith("-pc-windows-gnu") or target.endswith("-pc-windows-msvc"): - marks.append(WINDOWS_ONLY) - if target.endswith("-pc-windows-gnu") or target.endswith("-pc-windows-msvc"): - marks.append(WINDOWS_KNOWN_FAILURE) + if target.endswith(("-pc-windows-gnu", "-pc-windows-msvc")): + marks.extend((WINDOWS_ONLY, WINDOWS_KNOWN_FAILURE)).github/actions/release-to-pypi-uv/tests/test_check_github_release.py (1)
138-159: Add a failing‑after‑retries test.Exercise the terminal retry path and assert non‑zero exit with an error message.
Apply this diff (append to the end of the file):
+def test_retries_then_fail(monkeypatch, module, capsys) -> None: + def failing_urlopen(request, timeout: float = 30): # noqa: ANN001 + raise module.urllib.error.URLError("temporary") + + monkeypatch.setattr(module.urllib.request, "urlopen", failing_urlopen) + monkeypatch.setattr(module.time, "sleep", lambda _: None) + + with pytest.raises(module.typer.Exit): + module.main(tag="v1.0.0", token="token", repo="owner/repo") + + captured = capsys.readouterr() + assert "temporary" in captured.err or "fetch" in captured.err.github/actions/release-to-pypi-uv/CHANGELOG.md (1)
3-23: LGTM — entries read clearly and match the shipped surface..github/actions/release-to-pypi-uv/README.md (1)
30-33: LGTM — usage, permissions, and token wiring are correct..github/actions/release-to-pypi-uv/tests/test_confirm_release.py (1)
34-46: LGTM — happy‑path acceptance is clear and stable..github/actions/release-to-pypi-uv/tests/test_write_summary.py (1)
29-33: Assert the environment bullet as well.Tighten the first test to verify the environment line is written.
Apply this diff:
content = summary_path.read_text(encoding="utf-8") assert "## Release summary" in content assert "- Released tag: v1.2.3" in content assert "- Publish index: pypi (default)" in content + assert "- Environment: pypi" in content.github/actions/release-to-pypi-uv/scripts/publish_release.py (2)
36-41: Remove dead truthiness check in sys.path extension.Path objects are always truthy; keep only the existence test.
Apply this diff:
- for candidate in candidates: - if not candidate: - continue - if not candidate.exists(): + for candidate in candidates: + if not candidate.exists(): continue
50-70: Decouple CLI options from the programmatic API and normalise index.Avoid Typer options in function defaults and strip whitespace before use.
Apply this diff:
-INDEX_OPTION = typer.Option("", envvar="INPUT_UV_INDEX") +INDEX_OPTION = typer.Option( + "", + envvar="INPUT_UV_INDEX", + help="Optional index name or URL for uv publish.", +) @@ -def main(index: str = INDEX_OPTION) -> None: +def main(index: str = "") -> None: @@ - if index: + index = index.strip() + if index: typer.echo(f"Publishing with uv to index '{index}'") run_cmd(["uv", "publish", "--index", index]) else: typer.echo("Publishing with uv to default index (PyPI)") run_cmd(["uv", "publish"]) - -if __name__ == "__main__": - typer.run(main) +def cli(index: str = INDEX_OPTION) -> None: + """CLI entrypoint.""" + main(index=index) + +if __name__ == "__main__": + typer.run(cli).github/actions/release-to-pypi-uv/tests/test_action_python_version.py (1)
32-37: Relax brittle equality on the install step.Assert substring presence to tolerate harmless quoting/layout changes.
Apply this diff:
- assert install_step["run"] == 'uv python install "${{ inputs.python-version }}"' + assert 'uv python install "${{ inputs.python-version }}"' in install_step["run"].github/actions/release-to-pypi-uv/tests/_helpers.py (1)
13-19: Fix REPO_ROOT when GITHUB_ACTION_PATH is set.parents[2] resolves to “.github”, not the repository root. Use parents[3] for parity with the else branch.
Apply this diff:
if _ACTION_PATH: _action_root = Path(_ACTION_PATH).resolve() SCRIPTS_DIR = _action_root / "scripts" - REPO_ROOT = _action_root.parents[2] + REPO_ROOT = _action_root.parents[3] else: SCRIPTS_DIR = Path(__file__).resolve().parents[1] / "scripts" REPO_ROOT = SCRIPTS_DIR.parents[3].github/actions/rust-build-release/tests/test_utils.py (1)
38-47: Adopt cmd‑mox spy; LGTM.Use of CMD_MOX_UNSUPPORTED, environment.shim_dir, spy/replay/verify is correct and consistent with repo fixtures.
conftest.py (2)
12-20: Expose shared test utilities; LGTM.Windows guard, uv gating, and module export via sys.modules are tidy and pragmatic.
29-61: Stub helpers look solid; LGTM.Deque‑based multi‑response handler and shim path returns align with cmd‑mox patterns.
.github/actions/rust-build-release/tests/test_target_install.py (7)
22-63: Cross path with container probe; LGTM.Harness usage, which‑patching, and cmd‑mox replay/verify produce deterministic outcomes.
65-101: Error path without cross; LGTM.Assertions verify user‑visible error and proper Exit on unsupported target.
104-141: Fallback to cargo on container error; LGTM.Simulated cross failure via return code 125 and correct fallback are covered.
151-201: Windows host probe decisions; LGTM.Probe suppression for Windows targets and positive probe for non‑Windows targets are well isolated.
256-275: Unit coverage for should_probe_container; LGTM.Parameterised expectations are clear and minimal.
278-323: Linker configuration prefers toolchain GCC; LGTM.Environment export behaviour is correctly asserted.
332-389: Cross linkers on PATH; LGTM.Selective which patching and env var checks read cleanly.
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
47-70: Handle HTTP 401 and 429 explicitly; improve operator feedback and resilience.Map 401 unauthorised to an actionable message and respect 429 Retry‑After for backoff. This was requested previously and appears missing again.
except urllib.error.HTTPError as exc: # pragma: no cover - network failure path detail = ( exc.read().decode("utf-8", errors="ignore") if hasattr(exc, "read") else "" ) if exc.code == 404: raise GithubReleaseError( f"No GitHub release found for tag {tag}. Create and publish the release first." ) from exc + if exc.code == 401: + msg = "GitHub API returned 401 Unauthorized. Ensure GH_TOKEN is valid and not expired." + context = detail or exc.reason + raise GithubReleaseError(f"{msg} ({context})") from exc if exc.code == 403: msg = ( "GitHub token lacks permission to read releases or has expired. " "Ensure the workflow is using GITHUB_TOKEN with contents:read scope." ) context = detail or exc.reason raise GithubReleaseError(f"{msg} ({context})") from exc + if exc.code == 429: + # Respect rate-limit signalling and increase backoff. + retry_after = 0.0 + try: + header = exc.headers.get("Retry-After") if hasattr(exc, "headers") else None # type: ignore[truthy-bool] + if header: + retry_after = float(header) + except Exception: + retry_after = 0.0 + if attempt < max_attempts: + sleep_for = max(delay, retry_after) + time.sleep(sleep_for) + delay *= backoff_factor + continue if attempt == max_attempts: raise GithubReleaseError( f"GitHub API request failed with status {exc.code}: {detail or exc.reason}" ) from exc time.sleep(delay) delay *= backoff_factor.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (5)
4-5: Pin Typer dependency in script metadata.Align dependency policy with other scripts.
-# dependencies = ["typer"] +# dependencies = ["typer>=0.12,<0.13"]
24-33: Expand skip set to cut IO on large repos.Skip common caches/artefacts during discovery.
SKIP_PARTS = { ".git", ".venv", "venv", "node_modules", "dist", "build", ".direnv", ".mypy_cache", + ".pytest_cache", + ".cache", + "__pycache__", + ".tox", + "htmlcov", }
38-46: Stream and stabilise file discovery.Use iglob and sort for determinism and lower memory.
-def _iter_files(pattern: str) -> typ.Iterable[Path]: - candidates = [Path(p) for p in glob.glob(pattern, recursive=True)] - for path in candidates: +def _iter_files(pattern: str) -> typ.Iterable[Path]: + for candidate in sorted(glob.iglob(pattern, recursive=True)): + path = Path(candidate) if not path.is_file(): continue parts = set(path.parts) if parts & SKIP_PARTS: continue yield path
17-23: Gate “no matches” behaviour behind a flag.Enable failing the step when nothing matches.
VERSION_OPTION = typer.Option(..., envvar="RESOLVED_VERSION") PATTERN_OPTION = typer.Option("**/pyproject.toml", envvar="INPUT_TOML_GLOB") FAIL_ON_DYNAMIC_OPTION = typer.Option( "false", envvar="INPUT_FAIL_ON_DYNAMIC_VERSION", ) +FAIL_ON_EMPTY_OPTION = typer.Option( + "false", + envvar="INPUT_FAIL_ON_EMPTY", +)
75-101: Implement FAIL_ON_EMPTY in control flow.Emit error and exit non‑zero when requested.
-def main( +def main( version: str = VERSION_OPTION, pattern: str = PATTERN_OPTION, - fail_on_dynamic: str = FAIL_ON_DYNAMIC_OPTION, + fail_on_dynamic: str = FAIL_ON_DYNAMIC_OPTION, + fail_on_empty: str = FAIL_ON_EMPTY_OPTION, ) -> None: @@ - files = list(_iter_files(pattern)) - if not files: - typer.echo(f"::warning::No TOML files matched pattern {pattern}") - return + files = list(_iter_files(pattern)) + if not files: + if _parse_bool(fail_on_empty): + typer.echo(f"::error::No TOML files matched pattern {pattern}", err=True) + raise typer.Exit(1) + typer.echo(f"::warning::No TOML files matched pattern {pattern}") + return.github/actions/rust-build-release/tests/test_cross_install.py (2)
164-171: Windows prebuilt path validation looks good.The test exercises the release installer, validates execution, and avoids cargo. No issues.
351-357: Add coverage for podman timeout during runtime detection.Simulate subprocess.TimeoutExpired from
podman infoand assert the tool falls back tocargo(nocrossinvocation). Add the test to .github/actions/rust-build-release/tests/test_cross_install.py (e.g. test_falls_back_to_cargo_when_podman_times_out).Run the repository search below and paste the output to confirm runtime detection catches subprocess.TimeoutExpired; if no handler exists, add handling and the test:
rg -n -C2 --hidden -S -g '!**/node_modules/**' -g '!**/dist/**' \ 'subprocess\.TimeoutExpired|TimeoutExpired|podman info|podman' .If the search returns no matches, update the runtime detection to catch subprocess.TimeoutExpired raised by
podman infoand include the new test..github/actions/release-to-pypi-uv/action.yml (3)
47-56: Good: setup-uv pinned and wired to python-version with caching.Pinning the action and enabling dependency cache is solid.
70-77: Surface both GH_TOKEN and GITHUB_TOKEN for downstream tools.Already exported; keep this for compatibility. No change required.
84-90: Drop the explicit Python install step or move it immediately after setup-uv.File: .github/actions/release-to-pypi-uv/action.yml (lines 84–90)
- - name: Install Python - run: uv python install "${{ inputs.python-version }}" - shell: bashVerify that setup-uv activates the requested interpreter. Run the check below in a runner with uv available; if it prints the requested version/executable, remove the step; otherwise move it immediately after setup-uv.
#!/bin/bash set -eu # Run this on a runner that has `uv` available (e.g., your GitHub Actions runner). uv --version uv run --script - <<'PY' import sys print("sys.version:", sys.version) print("sys.executable:", sys.executable) PY.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (3)
66-68: Tighten success assertion to include checked count.Assert the full, deterministic success message.
- captured = capsys.readouterr() - assert "all versions match 1.0.0" in captured.out + captured = capsys.readouterr() + assert "Checked 1 PEP 621 project file(s); all versions match 1.0.0." in captured.out
320-324: Optionally count all mismatch messages to lock processing of multiple files.Strengthen determinism by asserting at least one mismatch was reported per offending file.
- assert "!= tag version" in captured.err + assert "!= tag version" in captured.err + assert captured.err.count("!= tag version") >= 1
255-284: Add failure case for missing [project].version.Exercise the branch where version is absent and not marked dynamic.
@@ def test_missing_project_section_is_ignored( @@ assert captured.err == "" + + +def test_fails_when_project_version_missing( + project_root: Path, + module: ModuleType, + capsys: pytest.CaptureFixture[str], +) -> None: + _write_pyproject( + project_root / "pkg", + """ +[project] +name = "demo" +""", + ) + with pytest.raises(module.typer.Exit): + _invoke_main(module, version="1.0.0") + captured = capsys.readouterr() + assert "missing [project].version" in captured.err.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
117-134: Provide the repository or release-docs linkPaste the repo URL or the action README/release-docs snippet that defines allowed tag formats; if the repo is private, paste determine_release.py or the relevant parsing code.
Do the release docs for <INSERT_REPO_URL_HERE> state that pre-release (-rc) and build metadata (+build) tags are allowed for this action?
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.github/actions/rust-build-release/tests/test_cross_install.py (4)
28-58: Assert no git fallback during initial installHarden the test to prove the crates.io path was used (and not the git fallback).
Apply this diff:
@@ install = next( cmd for cmd in harness.calls if cmd[:3] == ["cargo", "install", "cross"] ) assert "--locked" in install idx = install.index("--version") assert install[idx + 1] == "0.2.5" + # Prove we did not take the git fallback path + assert "--git" not in install + assert "--tag" not in install
83-113: Strengthen upgrade assertions to avoid false positivesPin that an actual versioned reinstall path was taken and git fallback was not.
Apply this diff:
@@ assert "--locked" in install idx = install.index("--version") assert install[idx + 1] == "0.2.5" + # Ensure upgrade used crates.io, not the git fallback + assert "--git" not in install + assert "--tag" not in install
139-174: Capture the release version argument on WindowsRecord the version passed to
install_cross_releaseto assert the contract.Apply this diff:
@@ - release_called = {"value": False} - - def fake_release(version: str) -> bool: - release_called["value"] = True - return True + release_call_args: list[str] = [] + + def fake_release(version: str) -> bool: + release_call_args.append(version) + return True @@ - assert release_called["value"] is True + assert release_call_args == ["0.2.5"]
353-393: Prove no container runtime invocation leaked throughAugment assertions to guarantee no
docker/podmancommands were attempted when both are unavailable.Apply this diff:
@@ assert install[idx + 1] == "0.2.5" build_cmd = app_env.calls[-1] assert build_cmd[0] == "cargo" assert build_cmd[1] == f"+{default_toolchain}-x86_64-unknown-linux-gnu" + # Ensure no container runtime calls were attempted + assert all(cmd[0] not in {"docker", "podman"} for cmd in app_env.calls) + assert all(cmd[0] not in {"docker", "podman"} for cmd in cross_env.calls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
.github/actions/release-to-pypi-uv/scripts/determine_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/tests/__init__.py(1 hunks).github/actions/release-to-pypi-uv/tests/_helpers.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_check_github_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_confirm_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_determine_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_publish_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_write_summary.py(1 hunks).github/actions/rust-build-release/src/main.py(0 hunks).github/actions/rust-build-release/tests/test_cross_install.py(6 hunks).github/actions/rust-build-release/tests/test_target_install.py(5 hunks).github/actions/rust-build-release/tests/test_utils.py(3 hunks)conftest.py(1 hunks)
💤 Files with no reviewable changes (1)
- .github/actions/rust-build-release/src/main.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.rules/python-return.md)
**/*.py: R501 — Avoid explicitreturn Noneif it's the only return in a function
R502 — Avoid implicitNonein functions that may return a value; ensure all branches explicitly return a value if any branch does
R503 — Add an explicit return at the end; don't rely on implicitNone
R504 — Avoid redundant variable assignment beforereturn; inline the expression unless the variable is meaningfully reused
R505 — Eliminate unnecessaryelseafter areturn
R506 — Eliminate unnecessaryelseafter araise
R507 — Eliminate unnecessaryelseafter abreak
R508 — Eliminate unnecessaryelseafter acontinue
**/*.py: Use PEP 695 bracketed class-level type parameters for generic class declarations (class Box[T])
Use typing.Self in fluent/builder-style APIs to indicate methods return the same instance
Use @typing.override on methods that override a superclass method (PEP 698)
Use typing.TypeIs[T] for custom runtime type guards that narrow types (PEP 742)
Provide default TypeVar values (PEP 696) for ergonomic generics when appropriate
Prefer builtin generics (list, dict, tuple, etc.) over typing.List, typing.Dict, etc. (PEP 585)
Use PEP 604 union syntax A | B and A | None instead of Optional[A]
Define type aliases using the type keyword (type StrDict = dict[str, str])
When targeting Python < 3.12, use typing.TypeAlias and add "# noqa: UP040" to suppress ruff warning
Place type alias definitions after the import block
Add from future import annotations in modules using type annotations to defer evaluation
Use if typing.TYPE_CHECKING to guard imports needed only for static typing
Use standard aliases: import datetime as dt and collections.abc as cabc
**/*.py: Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix non-exported helpers or internal APIs with a single underscore (_)
Use typing everywhere and maintain full static type coverage with Pyright
Use TypedDict or datacl...
Files:
conftest.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep cyclomatic complexity ≤ 12
- Follow single responsibility and CQRS (command/query segregation)
- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.
Files:
conftest.py
🧬 Code graph analysis (11)
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(21-31).github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
main(103-131)
.github/actions/release-to-pypi-uv/tests/test_write_summary.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(21-31).github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-51)
.github/actions/release-to-pypi-uv/tests/test_publish_release.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(21-31).github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
main(53-66)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (3)
.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (1)
main(74-153).github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-51).github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
main(103-131)
.github/actions/release-to-pypi-uv/tests/test_confirm_release.py (1)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
base_env(30-39)
.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (1)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
main(26-70)
.github/actions/rust-build-release/tests/test_utils.py (2)
conftest.py (7)
CmdMox(46-61)cmd_mox(114-116)spy(54-55)with_args(29-30)returns(32-40)replay(57-58)verify(60-61).github/actions/rust-build-release/tests/conftest.py (1)
utils_module(212-214)
.github/actions/rust-build-release/tests/test_target_install.py (2)
conftest.py (7)
_register_cross_version_stub(71-87)_register_docker_info_stub(99-106)_register_rustup_toolchain_stub(90-96)CmdMox(46-61)cmd_mox(114-116)replay(57-58)verify(60-61).github/actions/rust-build-release/tests/conftest.py (3)
module_harness(106-115)patch_shutil_which(89-91)patch_attr(97-99)
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (1)
.github/actions/generate-coverage/tests/test_scripts.py (1)
run_script(30-48)
.github/actions/rust-build-release/tests/test_cross_install.py (2)
conftest.py (7)
_register_cross_version_stub(71-87)_register_docker_info_stub(99-106)_register_rustup_toolchain_stub(90-96)CmdMox(46-61)cmd_mox(114-116)replay(57-58)verify(60-61).github/actions/rust-build-release/tests/conftest.py (3)
module_harness(106-115)patch_shutil_which(89-91)patch_run_cmd(78-87)
.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(21-31).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (2)
main(74-153)_parse_bool(48-54)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Sourcery review
- GitHub Check: python-tests (ubuntu-latest)
- GitHub Check: python-tests (windows-latest)
- GitHub Check: build-release (windows-latest, x86_64-pc-windows-gnu)
- GitHub Check: build-release (windows-latest, aarch64-pc-windows-gnu)
🔇 Additional comments (33)
.github/actions/release-to-pypi-uv/tests/__init__.py (1)
1-1: LGTM — simple, focused package docstring..github/actions/release-to-pypi-uv/tests/test_check_github_release.py (1)
176-176: Cover terminal retry failure.Add a test where all attempts raise URLError and the CLI exits non‑zero.
Apply this diff:
@@ assert "GitHub Release 'ok' is published." in captured.out + +def test_retries_then_fail( + monkeypatch: pytest.MonkeyPatch, + module: ModuleType, + capsys: pytest.CaptureFixture[str], +) -> None: + """Exit with an error after exhausting retries on transient failures.""" + def failing_urlopen(request: typ.Any, timeout: float = 30) -> typ.Any: # noqa: ANN401 + raise module.urllib.error.URLError("temporary") + + monkeypatch.setattr(module.urllib.request, "urlopen", failing_urlopen) + monkeypatch.setattr(module.time, "sleep", lambda _s: None) + + with pytest.raises(module.typer.Exit): + module.main(tag="v1.0.0", token="token", repo="owner/repo") + + captured = capsys.readouterr() + assert "temporary" in captured.err or "fetch" in captured.err.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
12-19: LGTM — resolve action/script paths robustly.Path derivation and explicit module loading look sound.
.github/actions/release-to-pypi-uv/tests/test_confirm_release.py (1)
35-49: LGTM — exercise success and failure paths via uv run.The helper isolates environment and asserts correct CLI outcomes.
.github/actions/release-to-pypi-uv/scripts/determine_release.py (2)
60-66: Confirm tag policy (pre‑releases/build metadata).If tags like v1.2.3-rc.1 or v1.2.3+build.5 must be accepted, expand the regex; otherwise document that only vMAJOR.MINOR.PATCH is permitted.
20-24: Future‑proof GITHUB_OUTPUT writes using multiline syntax.Guard against special characters/newlines in values.
Apply this diff:
def _emit_outputs(dest: Path, tag: str, version: str) -> None: - with dest.open("a", encoding="utf-8") as fh: - fh.write(f"tag={tag}\n") - fh.write(f"version={version}\n") + with dest.open("a", encoding="utf-8") as fh: + for key, val in (("tag", tag), ("version", version)): + fh.write(f"{key}<<__EOF__\n{val}\n__EOF__\n").github/actions/release-to-pypi-uv/tests/test_publish_release.py (3)
16-18: Avoid accidental import shadowing — insert SCRIPTS_DIR on sys.path, not REPO_ROOT.Prevent picking up unrelated cmd_utils from the repo root.
Apply this diff:
-from ._helpers import REPO_ROOT, load_script_module +from ._helpers import SCRIPTS_DIR, load_script_module @@ - if str(REPO_ROOT) not in module.sys.path: # type: ignore[attr-defined] - module.sys.path.insert(0, str(REPO_ROOT)) # type: ignore[attr-defined] + if str(SCRIPTS_DIR) not in module.sys.path: # type: ignore[attr-defined] + module.sys.path.insert(0, str(SCRIPTS_DIR)) # type: ignore[attr-defined]Also applies to: 9-9
21-35: Remove duplication — parametrise default/custom index tests.Collapse the two tests into one parametrised case to reduce drift.
Apply this diff:
-def test_publish_default_index( - monkeypatch: pytest.MonkeyPatch, publish_module: ModuleType -) -> None: - """Invoke ``uv publish`` without an index when none is provided.""" - calls: list[list[str]] = [] - - def fake_run_cmd(args: list[str], **_: object) -> None: - calls.append(args) - - monkeypatch.setattr(publish_module, "run_cmd", fake_run_cmd) - - publish_module.main(index="") - - assert calls == [["uv", "publish"]] - - -def test_publish_custom_index( - monkeypatch: pytest.MonkeyPatch, publish_module: ModuleType -) -> None: - """Add the ``--index`` flag when a custom index value is supplied.""" - calls: list[list[str]] = [] - - def fake_run_cmd(args: list[str], **_: object) -> None: - calls.append(args) - - monkeypatch.setattr(publish_module, "run_cmd", fake_run_cmd) - - publish_module.main(index="testpypi") - - assert calls == [["uv", "publish", "--index", "testpypi"]] +@pytest.mark.parametrize( + "index,expected", + [ + ("", [["uv", "publish"]]), + ("testpypi", [["uv", "publish", "--index", "testpypi"]]), + ], +) +def test_publish_index_variants( + monkeypatch: pytest.MonkeyPatch, publish_module: ModuleType, index: str, expected: list[list[str]] +) -> None: + """Invoke ``uv publish`` with default and custom indices.""" + calls: list[list[str]] = [] + + def fake_run_cmd(args: list[str], **_: object) -> None: + calls.append(args) + + monkeypatch.setattr(publish_module, "run_cmd", fake_run_cmd) + publish_module.main(index=index) + assert calls == expectedAlso applies to: 37-50
66-67: Tighten the failure contract — assert the error message.Match the raised message to ensure the correct failure surfaces.
Apply this diff:
-with pytest.raises(DummyError): +with pytest.raises(DummyError, match="uv publish failed"): publish_module.main(index="").github/actions/release-to-pypi-uv/tests/test_write_summary.py (1)
33-36: Assert the environment bullet too.Verify the full summary content.
Apply this diff:
content = summary_path.read_text(encoding="utf-8") assert "## Release summary" in content assert "- Released tag: v1.2.3" in content assert "- Publish index: pypi (default)" in content + assert "- Environment: pypi" in content.github/actions/rust-build-release/tests/test_utils.py (1)
60-63: Attach assertion messages to aid triage.Make failures self‑describing.
Apply this diff:
- assert isinstance(result, subprocess.CompletedProcess) - assert result.args[0] == str(exe_path) - assert result.stdout == "ok" - assert spy.call_count == 1 + assert isinstance(result, subprocess.CompletedProcess), "run_validated must return CompletedProcess" + assert result.args[0] == str(exe_path), f"unexpected executable path: {result.args[0]!r}" + assert result.stdout == "ok", f"unexpected stdout: {result.stdout!r}" + assert spy.call_count == 1, f"unexpected docker info call count: {spy.call_count}"conftest.py (2)
14-21: Keep the UV gate and Windows skip mark.The platform guard and REQUIRES_UV fixture wiring are clean and avoid spurious failures.
109-116: Retain plugin injection pattern.The conditional pytest_plugins injection is sound and keeps Windows builds green.
.github/actions/rust-build-release/tests/test_target_install.py (4)
10-15: Import the cmd‑mox types and helpers centrally.Centralising via shared_actions_conftest reads well and avoids duplication.
Also applies to: 21-22
26-67: Exercise cross‑available fallback path thoroughly.The run_cmd side effect, toolchain stub, and which mapping correctly drive the path to use cross after rustup failure. Replay/verify bracketing is correct.
69-106: Assert the error branch when cross is unavailable.The test pins rustup only and forces the add‑target failure, then asserts typer.Exit with a precise error. Good coverage.
108-145: Validate container failure fallback to cargo.Raising exit 125 for cross and asserting cargo usage mirrors real‑world behaviour. Keep this.
.github/actions/release-to-pypi-uv/tests/test_determine_release.py (2)
9-12: Skip when uv is absent.The module‑level marker prevents environment‑specific flakes.
14-27: Retain the script runner and env harness.Encapsulating the UV call and stable base env keeps tests readable and reproducible.
Also applies to: 30-40, 42-53
.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (3)
37-45: Stream and sort glob results to stabilise order.Avoid building a list and stabilise logs.
-def _iter_files(pattern: str) -> typ.Iterable[Path]: - candidates = [Path(p) for p in glob.glob(pattern, recursive=True)] - for path in candidates: +def _iter_files(pattern: str) -> typ.Iterable[Path]: + for candidate in sorted(glob.iglob(pattern, recursive=True)): + path = Path(candidate) if not path.is_file(): continue parts = set(path.parts) if parts & SKIP_PARTS: continue yield path
23-32: Broaden the skip set to cut IO and noise.Skip common caches/artefacts for faster deterministic scans.
SKIP_PARTS = { ".git", ".venv", "venv", "node_modules", "dist", "build", ".direnv", ".mypy_cache", + ".pytest_cache", + ".cache", + "htmlcov", }
2-5: Pin Typer in script metadata.Avoid unbounded dependency drift by pinning Typer to the tested minor.
# /// script # requires-python = ">=3.13" -# dependencies = ["typer"] +# dependencies = ["typer>=0.12,<0.13"] # ///.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (8)
266-266: Add a “no matches” test to lock warning behaviour.Exercise the early‑return path when the glob matches nothing.
+def test_warns_when_no_files_match( + project_root: Path, module: ModuleType, capsys: pytest.CaptureFixture[str] +) -> None: + _invoke_main(module, version="1.0.0", pattern="nope/**/pyproject.toml") + captured = capsys.readouterr() + assert "::warning::No TOML files matched pattern" in captured.out
29-33: Write TOML with UTF‑8 and create parent dirs to avoid flakiness.Force UTF‑8 and allow nested dirs to be created for future tests (e.g. .venv/pkg).
def _write_pyproject(base: Path, content: str) -> None: """Create a ``pyproject.toml`` file populated with the provided content.""" - base.mkdir() - (base / "pyproject.toml").write_text(content.strip()) + base.mkdir(parents=True, exist_ok=True) + (base / "pyproject.toml").write_text(content.strip(), encoding="utf-8")
55-59: Assert the full success message (incl. checked count).Lock the message shape to catch regressions.
captured = capsys.readouterr() -assert "all versions match 1.0.0" in captured.out +assert "Checked 1 PEP 621 project file(s); all versions match 1.0.0." in captured.out
125-132: Write the malformed TOML with UTF‑8.Eliminate locale‑dependent behaviour on CI.
(target / "pyproject.toml").write_text("this is not TOML") +# Be explicit about encoding to avoid platform defaults +(target / "pyproject.toml").write_text("this is not TOML", encoding="utf-8")
226-254: Add a test for missing [project].version (not dynamic).Cover the explicit error branch for absent literal versions.
def test_multiple_toml_files_mixed_validity( @@ assert "!= tag version" in captured.err + + +def test_fails_when_project_version_missing( + project_root: Path, + module: ModuleType, + capsys: pytest.CaptureFixture[str], +) -> None: + _write_pyproject( + project_root / "pkg_no_version", + """ +[project] +name = "demo" +""", + ) + with pytest.raises(module.typer.Exit): + _invoke_main(module, version="1.0.0") + captured = capsys.readouterr() + assert "missing [project].version" in captured.err
29-33: Add ignored‑directories test (.venv/.mypy_cache) if the walker skips them.Prove that discovery honours the ignore list.
+def test_skips_files_in_ignored_directories( + project_root: Path, module: ModuleType, capsys: pytest.CaptureFixture[str] +) -> None: + _write_pyproject(project_root / ".venv" / "pkg", """ +[project] +name = "ignored" +version = "9.9.9" +""") + _write_pyproject(project_root / ".mypy_cache" / "pkg", """ +[project] +name = "ignored2" +version = "9.9.9" +""") + _invoke_main(module, version="1.0.0") + captured = capsys.readouterr() + assert "::warning::No TOML files matched pattern" in captured.outConfirm that validate_toml_versions._iter_files excludes .venv and .mypy_cache. If not, implement the skip list before adding this test.
226-254: Assert only one mismatch is reported.Tighten expectations for error fan‑out.
captured = capsys.readouterr() assert "!= tag version" in captured.err +assert captured.err.count("!= tag version") == 1
220-224: Also assert the zero‑checked success message.Prove that non‑PEP‑621 files are ignored and still produce the deterministic summary.
captured = capsys.readouterr() assert captured.err == "" +assert "Checked 0 PEP 621 project file(s); all versions match 1.0.0." in captured.out.github/actions/rust-build-release/tests/test_cross_install.py (3)
115-137: Cached cross path — LGTMAvoids install and only verifies version; exactly the behaviour we want.
12-17: Approve shared cmd‑mox helpers — audit passedUnify imports via shared_actions_conftest and tighten types — LGTM. Automated audit confirms every test that accepts
cmd_moxis decorated with@CMD_MOX_UNSUPPORTED.
431-464: Add regression test for podman timeout → cargo fallbackAdd an integration test in .github/actions/rust-build-release/tests/test_cross_install.py that simulates subprocess.TimeoutExpired for the "podman info" probe and asserts main(... ) invokes cargo and not cross. Runtime unit tests already exercise timeout behaviour (see .github/actions/rust-build-release/tests/test_runtime.py:test_podman_security_timeout_treated_as_unavailable); add this cross_install-level test to cover the end-to-end fallback path.
Run only the new test locally to verify:
pytest -q -k "podman and timeout"
|
@coderabbitai pause |
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
.github/actions/generate-coverage/scripts/run_rust.py(1 hunks).github/actions/generate-coverage/tests/test_scripts.py(3 hunks).github/actions/release-to-pypi-uv/scripts/check_github_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/determine_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/publish_release.py(1 hunks).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/tests/_helpers.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_check_github_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_publish_release.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py(1 hunks).github/actions/release-to-pypi-uv/tests/test_write_summary.py(1 hunks).github/actions/rust-build-release/tests/test_target_install.py(6 hunks).github/actions/rust-build-release/tests/test_utils.py(3 hunks).github/workflows/ci.yml(1 hunks)conftest.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
**/*.py: Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix non-exported helpers or internal APIs with a single underscore (_)
Use typing everywhere and maintain full static type coverage with Pyright
Use TypedDict or dataclass for structured data; for internal-only data classes prefer @DataClass(slots=True)
Avoid Any; prefer precise types (TypeVar, Protocol, Literal, Union) and use typing.cast only when necessary with justification; use object for unknown opaque values
Be explicit with return types for all public functions and class methods (e.g., -> None, -> str)
Favor immutability: prefer tuples to lists and MappingProxyType for read-only mappings; document any frozendict dependency
Avoid side effects at import time—modules should not modify global state or perform actions on import
Use NumPy-style docstrings for public functions, classes, and modules
Explain tricky or non-obvious code with inline comments
**/*.py: Prefer context managers to encapsulate setup/teardown for resources (files, locks, connections, transactions) instead of manual try/finally
Use contextlib.contextmanager for straightforward, linear setup/teardown flows
Implement class-based context managers by defining enter and exit when resource management is stateful or spans multiple methods (e.g., re-entry or advanced features)
Use with open(...) for file handling instead of open() with manual try/finally
**/*.py: Suffix all concrete exception classes with "Error" (Ruff N818)
Avoid raising bare Exception; prefer specific built-ins (e.g., ValueError, TypeError) or domain-specific exceptions (TRY003/TRY004)
Preserve causal chains using "raise ... from ..." when wrapping lower-level errors (TRY201/TRY200)
Avoid blind except handlers like "except:" or "except Exception:"; catch only actionable exceptions (BLE001)
Use try/except/else with else for the happy path when appropriate (TRY300)
Construct exceptio...
Files:
conftest.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep cyclomatic complexity ≤ 12
- Follow single responsibility and CQRS (command/query segregation)
- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.
Files:
conftest.py
🧬 Code graph analysis (10)
.github/actions/release-to-pypi-uv/tests/test_write_summary.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(24-35).github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-51)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
.github/actions/generate-coverage/scripts/run_rust.py (1)
main(343-388)
.github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
.github/actions/release-to-pypi-uv/scripts/write_summary.py (1)
main(20-51)
.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (1)
.github/actions/release-to-pypi-uv/scripts/determine_release.py (1)
main(26-71)
.github/actions/rust-build-release/tests/test_utils.py (2)
conftest.py (7)
CmdMox(55-70)cmd_mox(132-134)spy(63-64)with_args(32-33)returns(35-43)replay(66-67)verify(69-70).github/actions/rust-build-release/tests/conftest.py (1)
utils_module(212-214)
.github/actions/release-to-pypi-uv/tests/test_publish_release.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(24-35).github/actions/release-to-pypi-uv/scripts/publish_release.py (1)
main(50-63)
.github/actions/rust-build-release/tests/test_target_install.py (3)
conftest.py (7)
_register_cross_version_stub(89-105)_register_docker_info_stub(117-124)_register_rustup_toolchain_stub(108-114)CmdMox(55-70)cmd_mox(132-134)replay(66-67)verify(69-70).github/actions/rust-build-release/tests/conftest.py (3)
module_harness(106-115)patch_shutil_which(89-91)patch_attr(97-99).github/actions/rust-build-release/src/main.py (1)
main(54-191)
.github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (1)
read(38-40)
.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(24-35).github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (2)
main(76-155)_parse_bool(47-53)
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (2)
.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
load_script_module(24-35).github/actions/release-to-pypi-uv/scripts/check_github_release.py (1)
main(119-147)
🔍 Remote MCP Deepwiki, Ref
Search attempts — results
- Tried to fetch the PR/page and run documentation searches; requests failed with HTTP 402 (Request failed).
- Tried Deepwiki repo index for leynos/shared-actions; returned “Repository not found” (suggested indexing).
Result: I could not retrieve additional external pages beyond the PR context you provided.
Concise, actionable reviewer checklist (from the provided PR context)
- Security/permissions: confirm the action’s required workflow permissions and that forwarding GITHUB_TOKEN to scripts (GH_TOKEN) is limited to the minimal scopes needed for trusted publishing.
- Tag/version resolution: verify determine_release enforces vX.Y.Z tags, strips leading "v" correctly, and writes GITHUB_OUTPUT as expected.
- TOML/version validation: confirm validate_toml_versions correctly handles PEP 621 dynamic vs static versions and that the fail-on-dynamic flag behaves as tested.
- Release checks: review check_github_release exponential-backoff/retry logic and error handling for 403/404/other HTTP errors (ensure error messages and exit codes are suitable).
- Publish path: ensure publish_release invokes uv publish with --index when provided and that the composite action’s uv setup/“python-version” input is correctly forwarded to the install step (tests were added to validate this).
- Tests/CI dependencies: note pyproject adds cmd-mox as a git dev-dependency and tests use REQUIRES_UV/HAS_UV markers — confirm CI runners can install these (and that uv is available when required).
- Known CI failure: CI logs (in PR comments) show rust-build-release timing out on
/usr/bin/podman info(subprocess TimeoutExpired). There is an issue (#97) created describing the failure and mitigations — recommend adding TimeoutExpired handling, fallback logic, or a longer timeout for container runtime detection. - Docs & changelog: README/action.yml/CHANGELOG additions should be checked for accuracy (inputs/outputs defaults, example usage, and changelog dates/entries).
If you want me to retry external lookups (PR page, issue #97, or action.yml) I can re-run searches — current attempts failed as shown above.
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: python-tests (ubuntu-latest)
- GitHub Check: python-tests (windows-latest)
- GitHub Check: build-release (windows-latest, aarch64-pc-windows-gnu)
- GitHub Check: build-release (windows-latest, x86_64-pc-windows-gnu)
- GitHub Check: Sourcery review
🔇 Additional comments (31)
.github/actions/generate-coverage/tests/test_scripts.py (3)
231-233: Use local variable for exception message — OKRaising with a pre-bound
messageis fine and keeps the regex assertion intact.
513-515: Pre-bindingPermissionErrormessage — OKThe change is neutral and maintains clarity without altering behaviour.
682-684: Pre-bindingPermissionErrormessage — OKThe refactor is safe and consistent with the surrounding tests.
.github/workflows/ci.yml (2)
38-41: Keep action pinning; verify the SHA maps to v2.0.2.Pinning
oven-sh/setup-bunis good. Confirm the commit SHA actually corresponds to tag v2.0.2 and is not revoked.Does commit 735343b667d3e6f658f44d0eca948eb6282f2b76 correspond to oven-sh/setup-bun v2.0.2, and is it the current recommended pin?
46-53: Ensure fmt/lint run through the project's pinneduvruntime (avoid host-tool drift).CI calls
make check-fmt/make lint, but the Makefile invokesuvx(e.g.uvx ruff format --check,uvx ruff check). Verify thatuvxexecutes the pinneduvenvironment; if it does not, update the Makefile targets to use explicituv run ...so the CI uses the locked toolchain..github/actions/rust-build-release/tests/test_utils.py (2)
40-53: Adopt CmdMox correctly — good swap from monkeypatch.The switch to CmdMox spies and environment shims is sound and aligns with the new harness.
47-47: Attach assertion messages to aid triage.Add explicit messages to these asserts per the repo’s testing guidelines.
Apply:
- assert shim_dir is not None + assert shim_dir is not None, "cmd-mox shim_dir must be initialised" - assert isinstance(result, subprocess.CompletedProcess) - assert result.args[0] == str(exe_path) - assert result.stdout == "ok" - assert spy.call_count == 1 + assert isinstance(result, subprocess.CompletedProcess), ( + f"expected subprocess.CompletedProcess, got {type(result)}" + ) + assert result.args[0] == str(exe_path), ( + f"unexpected executable path: expected {exe_path}, got {result.args[0]}" + ) + assert result.stdout == "ok", f"unexpected stdout: expected 'ok', got {result.stdout!r}" + assert spy.call_count == 1, f"unexpected spy call count: expected 1, got {spy.call_count}"Also applies to: 62-65
.github/actions/rust-build-release/tests/test_target_install.py (3)
26-67: Exercise cross‑path and docker probe — looks correct.Stubbing rustup/cross/docker via CmdMox and validating cross build invocation is correct.
108-145: Validate cargo fallback on container error — good coverage.The 125 exit code path is covered and asserts the cargo fallback correctly.
1-1: Handle podman/docker timeouts in runtime detection to unblock CI.Catch subprocess.TimeoutExpired in runtime_available(...) and treat it as “runtime not available”; optionally increase the timeout and emit a warning. This prevents the hard failure observed in PR #90 logs.
Apply in the action code (reference path shown; adjust to the actual module):
--- a/.github/actions/rust-build-release/src/utils.py +++ b/.github/actions/rust-build-release/src/utils.py @@ -def runtime_available(name: str) -> bool: - """Return True if container runtime responds to 'info'.""" - run_validated( - name, - ["info"], - allowed_names=(name, f"{name}.exe"), - timeout=10, - check=True, - capture_output=True, - text=True, - ) - return True +def runtime_available(name: str) -> bool: + """Return True if container runtime responds to 'info' promptly.""" + try: + run_validated( + name, + ["info"], + allowed_names=(name, f"{name}.exe"), + timeout=20, + check=True, + capture_output=True, + text=True, + ) + return True + except subprocess.TimeoutExpired: + typer.echo(f"::warning::{name} info timed out; treating as unavailable", err=True) + return False + except (subprocess.CalledProcessError, UnexpectedExecutableError): + return FalseLikely an incorrect or invalid review comment.
.github/actions/release-to-pypi-uv/tests/test_check_github_release.py (1)
168-192: Cover terminal retry failure path.Add a test that exhausts retries (URLError every time) and asserts exit(1) with an error message.
Apply:
@@ def test_retries_then_success( @@ assert "GitHub Release 'ok' is published." in captured.out + +def test_retries_then_fail( + monkeypatch: pytest.MonkeyPatch, + module: ModuleType, + capsys: pytest.CaptureFixture[str], + fake_token: str, +) -> None: + """Fail after exhausting retries on transient errors.""" + monkeypatch.setattr( + module.urllib.request, + "urlopen", + lambda *_args, **_kwargs: (_ for _ in ()).throw(module.urllib.error.URLError("temporary")), + ) + monkeypatch.setattr(module.time, "sleep", lambda _: None) + + with pytest.raises(module.typer.Exit): + module.main(tag="v1.0.0", token=fake_token, repo="owner/repo") + + captured = capsys.readouterr() + assert "temporary" in captured.err or "fetch" in captured.err.github/actions/release-to-pypi-uv/tests/_helpers.py (1)
15-22: Resolve action paths robustly — LGTM.Derive SCRIPTS_DIR and REPO_ROOT correctly for both GITHUB_ACTION_PATH and local runs.
.github/actions/release-to-pypi-uv/scripts/determine_release.py (2)
60-67: Confirm tag policy (pre‑releases/build metadata).Decide whether v1.2.3‑rc.1 / v1.2.3+build.5 should be accepted; if yes, expand the regex; if no, document the restriction in the error message and README.
20-24: Future‑proof GITHUB_OUTPUT writes (multiline).Use the delimiter form to avoid edge cases with special characters.
Apply:
def _emit_outputs(dest: Path, tag: str, version: str) -> None: - with dest.open("a", encoding="utf-8") as fh: - fh.write(f"tag={tag}\n") - fh.write(f"version={version}\n") + with dest.open("a", encoding="utf-8") as fh: + for key, val in (("tag", tag), ("version", version)): + fh.write(f"{key}<<__EOF__\n{val}\n__EOF__\n").github/actions/release-to-pypi-uv/tests/test_publish_release.py (3)
12-21: Avoid sys.path shadowing; insert scripts directory, not repo root.Prevent accidental imports by adding SCRIPTS_DIR to sys.path.
Apply:
-from ._helpers import REPO_ROOT, load_script_module +from ._helpers import SCRIPTS_DIR, load_script_module @@ - module = load_script_module("publish_release") - if str(REPO_ROOT) not in module.sys.path: # type: ignore[attr-defined] - module.sys.path.insert(0, str(REPO_ROOT)) # type: ignore[attr-defined] + module = load_script_module("publish_release") + if str(SCRIPTS_DIR) not in module.sys.path: # type: ignore[attr-defined] + module.sys.path.insert(0, str(SCRIPTS_DIR)) # type: ignore[attr-defined] return module
24-54: Remove duplication; parametrise default/custom index tests.Merge the two tests into one parametrised case.
Apply:
-def test_publish_default_index( - monkeypatch: pytest.MonkeyPatch, publish_module: ModuleType -) -> None: - """Invoke ``uv publish`` without an index when none is provided.""" - calls: list[list[str]] = [] - - def fake_run_cmd(args: list[str], **_: object) -> None: - calls.append(args) - - monkeypatch.setattr(publish_module, "run_cmd", fake_run_cmd) - - publish_module.main(index="") - - assert calls == [["uv", "publish"]] - - -def test_publish_custom_index( - monkeypatch: pytest.MonkeyPatch, publish_module: ModuleType -) -> None: - """Add the ``--index`` flag when a custom index value is supplied.""" - calls: list[list[str]] = [] - - def fake_run_cmd(args: list[str], **_: object) -> None: - calls.append(args) - - monkeypatch.setattr(publish_module, "run_cmd", fake_run_cmd) - - publish_module.main(index="testpypi") - - assert calls == [["uv", "publish", "--index", "testpypi"]] +@pytest.mark.parametrize( + "index,expected", + [ + ("", [["uv", "publish"]]), + ("testpypi", [["uv", "publish", "--index", "testpypi"]]), + ], +) +def test_publish_index_variants( + monkeypatch: pytest.MonkeyPatch, publish_module: ModuleType, index: str, expected: list[list[str]] +) -> None: + """Invoke ``uv publish`` with default and custom indices.""" + calls: list[list[str]] = [] + monkeypatch.setattr(publish_module, "run_cmd", lambda args, **__: calls.append(args)) + publish_module.main(index=index) + assert calls == expected
56-71: Assert the error message to tighten the contract.Verify the propagated message.
Apply:
- with pytest.raises(DummyError): + with pytest.raises(DummyError, match="uv publish failed"): publish_module.main(index="").github/actions/release-to-pypi-uv/tests/test_write_summary.py (1)
22-39: Assert the environment bullet as well.Tighten the assertions by checking the environment line.
Apply:
content = summary_path.read_text(encoding="utf-8") assert "## Release summary" in content assert "- Released tag: v1.2.3" in content assert "- Publish index: pypi (default)" in content + assert "- Environment: pypi" in content.github/actions/release-to-pypi-uv/scripts/publish_release.py (2)
33-37: Remove dead truthiness check on Path.Drop the always‑true guard; keep the existence check.
Apply:
- for candidate in candidates: - if not candidate: - continue + for candidate in candidates: if not candidate.exists(): continue
47-67: Decouple CLI from API and normalise input.Keep Typer types out of the function signature and strip whitespace on index.
Apply:
-INDEX_OPTION = typer.Option("", envvar="INPUT_UV_INDEX") +INDEX_OPTION = typer.Option( + "", + envvar="INPUT_UV_INDEX", + help="Optional index name or URL for uv publish.", +) -def main(index: str = INDEX_OPTION) -> None: +def main(index: str = "") -> None: @@ - if index: + index = index.strip() + if index: typer.echo(f"Publishing with uv to index '{index}'") run_cmd(["uv", "publish", "--index", index]) else: typer.echo("Publishing with uv to default index (PyPI)") run_cmd(["uv", "publish"]) - -if __name__ == "__main__": - typer.run(main) +def cli(index: str = INDEX_OPTION) -> None: + """CLI entrypoint.""" + main(index=index) + +if __name__ == "__main__": + typer.run(cli).github/actions/release-to-pypi-uv/scripts/check_github_release.py (3)
2-5: Pin Typer in script metadata.Pin Typer to a tested minor to avoid upstream breakage.
# /// script # requires-python = ">=3.13" -# dependencies = ["typer"] +# dependencies = ["typer>=0.12,<0.13"] # ///
64-71: Handle HTTP 401 explicitly.Surface unauthorised tokens with a clear, actionable message.
if exc.code == 404: message = ( "No GitHub release found for tag " f"{tag}. Create and publish the release first." ) raise GithubReleaseError(message) from exc + if exc.code == 401: + message = "GitHub API returned 401 Unauthorized. Ensure GH_TOKEN is valid and not expired." + raise GithubReleaseError(message) from exc if exc.code == 403: permission_message = ( "GitHub token lacks permission to read releases or has expired. " "Use a token with contents:read scope." )
27-41: Support GitHub Enterprise and unify User-Agent+import os @@ -def _fetch_release(repo: str, tag: str, token: str) -> dict[str, object]: - api = f"https://api.github.com/repos/{repo}/releases/tags/{tag}" +def _fetch_release(repo: str, tag: str, token: str) -> dict[str, object]: + api_base = os.getenv("GITHUB_API_URL", "https://api.github.com").rstrip("/") + api = f"{api_base}/repos/{repo}/releases/tags/{tag}" @@ - "User-Agent": "release-to-pypi-action", + "User-Agent": "release-to-pypi-uv",Scan repository for remaining hard-coded api.github.com URLs and mismatched User-Agent strings and replace them.
conftest.py (1)
32-46: Return Self from fluent doubles.Modernise the protocol to use typing.Self for fluent API.
-from __future__ import annotations +from __future__ import annotations @@ -class CmdDouble(typ.Protocol): +from typing import Self + +class CmdDouble(typ.Protocol): @@ - def with_args(self, *args: str) -> CmdDouble: + def with_args(self, *args: str) -> Self: """Set the expected argv for the double.""" @@ - ) -> CmdDouble: + ) -> Self: """Provide canned output for the command invocation.""" @@ - def runs(self, handler: cabc.Callable[[object], tuple[str, str, int]]) -> CmdDouble: + def runs(self, handler: cabc.Callable[[object], tuple[str, str, int]]) -> Self: """Execute a handler when the double is invoked.""".github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py (3)
2-5: Pin Typer in script metadata.Align dependencies with a safe, tested range.
# /// script # requires-python = ">=3.13" -# dependencies = ["typer"] +# dependencies = ["typer>=0.12,<0.13"] # ///
22-31: Broaden SKIP_PARTS to cut IO noise.Skip common caches/artefacts in mono‑repos.
SKIP_PARTS = { ".git", ".venv", "venv", "node_modules", "dist", "build", ".direnv", ".mypy_cache", + "__pycache__", + ".pytest_cache", + ".tox", + ".cache", + "htmlcov", }
36-45: Stream and stabilise discovery.Use iglob + sorted for deterministic, memory‑friendly walks.
-def _iter_files(pattern: str) -> typ.Iterable[Path]: - root = Path() - for path in root.glob(pattern): +def _iter_files(pattern: str) -> typ.Iterable[Path]: + import glob + root = Path() + for candidate in sorted(glob.iglob(str(root / pattern), recursive=True)): + path = Path(candidate) if not path.is_file(): continue parts = set(path.parts) if parts & SKIP_PARTS: continue yield path.github/actions/release-to-pypi-uv/tests/test_validate_toml_versions.py (4)
29-33: Write files with explicit encoding.Eliminate locale‑dependent flakiness.
def _write_pyproject(base: Path, content: str) -> None: """Create a ``pyproject.toml`` file populated with the provided content.""" base.mkdir() - (base / "pyproject.toml").write_text(content.strip()) + (base / "pyproject.toml").write_text(content.strip(), encoding="utf-8")
35-40: Allow None for omitted flags.Model “flag omitted entirely” by accepting None.
-from types import ModuleType +from types import ModuleType @@ -def _invoke_main(module: ModuleType, **kwargs: str) -> None: +def _invoke_main(module: ModuleType, **kwargs: typ.Any) -> None: """Invoke ``module.main`` with defaults tailored for the tests.""" kwargs.setdefault("pattern", "**/pyproject.toml") kwargs.setdefault("fail_on_dynamic", "false") module.main(**kwargs)
202-203: Pass None to truly omit the flag.Match the test’s intent and rely on _parse_bool(None) → False.
- _invoke_main(module, version="1.0.0", fail_on_dynamic="") + _invoke_main(module, version="1.0.0", fail_on_dynamic=None)
229-257: Strengthen multi-file assertions and add “no matches” coverage.Assert deterministic success text and cover empty-match scenarios. Append the following tests:
@@ def test_multiple_toml_files_mixed_validity( @@ assert "!= tag version" in captured.err + + +def test_warns_when_no_files_match(project_root: Path, module: ModuleType, capsys: pytest.CaptureFixture[str]) -> None: + _invoke_main(module, version="1.0.0") + captured = capsys.readouterr() + assert "::warning::No TOML files matched pattern" in captured.out + + +def test_fails_when_no_files_match_and_flag_enabled( + project_root: Path, + module: ModuleType, + capsys: pytest.CaptureFixture[str], +) -> None: + with pytest.raises(module.typer.Exit): + _invoke_main(module, version="1.0.0", fail_on_empty="true") + captured = capsys.readouterr() + assert "::error::No TOML files matched pattern" in captured.err
✅ Actions performedReviews paused. |
leynos
force-pushed
the
python-lib-release-action
branch
2 times, most recently
from
075fda7 to
2f73d93
Compare
* Use cmd-mox for rust build command tests * Guard cmd-mox integration on Windows * Refactor cmd-mox helpers into shared conftest * Apply cmd-mox fake which simplification
* Mark Windows smoke tests as xfail * Tighten Windows smoke test expectations
* Address review comments for release to PyPI action * Add tests and docs for python-version handling
* Fix cmd_mox fixture usage and expose dev extras * Remove pyproject optional dev extra * Remove Pyright configuration
* Add toolchain-specific cargo cache key and clean shellstub imports (#88) * Address review feedback for shellstub imports * Fix MSYS2 install comments breaking pacman (#89) * Fix MSYS2 install comments breaking pacman * Fix Windows llvm-mingw metadata export * Handle runtime detection timeouts * Fix rust host detection timeout and Windows bin path (#91) * Fix rust host detection timeout and Windows bin path * Add timeout coverage for rust host detection * Unify runtime probe timeout configuration * test: capture rustc probe kwargs in timeout test * Remove inline comments from Windows GNU MSYS2 package list (#99) * Remove inline MSYS2 install comments * Skip nfpm steps on Windows builds * Handle nfpm packaging only for Linux * Fix Windows invocation of rust-build-release (#102) * Silence type-check import lints for release action * Adds ignore rule for Crush agent artifacts Ignores files generated by the Crush AI agent to prevent them from being tracked in version control. Adds a symlink to AGENTS.md for discovery by Crush * Format code after rebase validation to satisfy repo style checks. 💘 Generated with Crush Co-Authored-By: Crush <crush@charm.land> * fmt: extend fmt target to run ruff check --select D202,I001 --fix; apply resulting fixes 💘 Generated with Crush Co-Authored-By: Crush <crush@charm.land> --------- Co-authored-by: Payton McIntosh <pmcintosh@df12.net> Co-authored-by: Crush <crush@charm.land>
* Add missing docstrings for release-to-pypi-uv tests * Simplify cmd-mox typing and streamline docstrings * Apply formatting * chore: remove unused imports across scripts and tests * style(_helpers): alphabetise __all__ tuple * tests: drop superfluous parentheses from @pytest.fixture usage --------- Co-authored-by: Payton McIntosh <pmcintosh@df12.net>
* Scope type-checking imports * Fix lint warnings across release scripts * Clarify cargo stream capture error * ci: add lint and format checks * Install action-validator in CI lint workflow * Install bun validator packages in CI
* Address review feedback * Add CLI coverage for publish and validate scripts * Add deterministic validation tests and bunx validator checks
* Fix Windows xfail marker removal for pytest 8 * Tighten Windows xfail marker filtering
* Handle runtime probe timeouts and expand release tests * Add _probe_runtime coverage and assert timeout warnings
* Expand pyproject skip list handling * Strengthen TOML and runtime timeout tests * Assert cargo fallback logs podman failure * Parameterize skip-directory regression
* Refine skip directory regression parameterization * Document skip directories and DRY timeout assertions * Parametrize publish index test * Refactor runtime timeout tests * Rebase onto origin/python-lib-release-action and resolve conflicts - Resolve merge in validate_toml_versions tests by standardizing module loading - Register dynamically loaded scripts in sys.modules to support reload semantics - Make toolchain triple test robust to host arch by selecting matching target - Run formatting, lint, typecheck, and tests to validate integration 💘 Generated with Crush Co-Authored-By: Crush <crush@charm.land> --------- Co-authored-by: Crush <crush@charm.land>
* Improve release validation jitter handling * Adjust runtime fallback host triples per platform * Make release validation deterministic and configurable
…preserving branch intent; drop uv.lock in favor of main; fix tests and typing/lint issues; all tests pass and linters clean
* Close cargo pipes when missing * Ensure cargo pipes close on all paths * Ensure guard closes cargo pipes before exiting
* Handle release script auth failure and multiline outputs * Harden windows toolchain setup and extend runtime tests * Handle release auth errors and tweak toolchain retries
* Fix cross install warning assertion * Harden cross install harness checks * Sanitize runtime probe timeout and guard actions * Refine runtime probe helpers and add timeout tests * Fix runtime probe lint findings * Share echo recorder fixture across runtime tests
…ts from main while preserving branch improvements; ensure formatting passes and all tests/linters are green. 💘 Generated with Crush Co-Authored-By: Crush <crush@charm.land>
leynos
force-pushed
the
python-lib-release-action
branch
from
dbbd359 to
fb721b7
Compare
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- This PR is very large and touches many areas—consider splitting it into smaller, domain-focused PRs (e.g. action metadata, helper script implementations, and tests) to make review and maintenance easier.
- The exponential backoff and jitter logic in setup-windows-gnu’s YAML is quite complex—consider extracting it into a shared script or template to keep the action definition concise.
- Tests for runtime probe timeouts have a lot of repeated setup—extracting a fixture or helper for patching
run_validatedon timeout would reduce duplication and improve readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- This PR is very large and touches many areas—consider splitting it into smaller, domain-focused PRs (e.g. action metadata, helper script implementations, and tests) to make review and maintenance easier.
- The exponential backoff and jitter logic in setup-windows-gnu’s YAML is quite complex—consider extracting it into a shared script or template to keep the action definition concise.
- Tests for runtime probe timeouts have a lot of repeated setup—extracting a fixture or helper for patching `run_validated` on timeout would reduce duplication and improve readability.
## Individual Comments
### Comment 1
<location> `.github/actions/setup-windows-gnu/action.yml:173` </location>
<code_context>
if ! command -v x86_64-w64-mingw32-gcc >/dev/null 2>&1; then
echo "::warning::x86_64 MinGW GCC not found" >&2
fi
+ require_a64="${{ inputs.require-aarch64 }}"
if ! command -v aarch64-w64-mingw32-gcc >/dev/null 2>&1 \
&& ! command -v aarch64-w64-mingw32-clang >/dev/null 2>&1; then
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Shell quoting for require_aarch64 may be fragile.
Double-quoting the GitHub input may not handle all cases, such as unexpected whitespace or non-boolean values. Use a stricter parsing method to ensure reliable boolean handling in bash.
</issue_to_address>
### Comment 2
<location> `.github/actions/generate-coverage/scripts/run_rust.py:160` </location>
<code_context>
+ stream.close()
+
+
def _run_cargo(args: list[str]) -> str:
"""Run ``cargo`` with ``args`` streaming output and return ``stdout``."""
typer.echo(f"$ cargo {shlex.join(args)}")
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the platform-specific output pumping logic into a helper function to make `_run_cargo` flatter and more focused.
Consider extracting the two big “pump” branches (win vs posix) into a helper so that `_run_cargo` becomes flat and focused on orchestration (spawn → pump → wait → cleanup). For example:
```python
def _pump_cargo_output(proc: subprocess.Popen[str]) -> list[str]:
"""Pump proc.stdout/stderr to console, return collected stdout lines."""
stdout_lines: list[str] = []
if os.name == "nt":
thread_excs: list[Exception] = []
def _win_pump(src: TextIO, out: TextIO, collect: bool):
try:
for line in iter(src.readline, ""):
out.write(line); out.flush()
if collect:
stdout_lines.append(line.rstrip("\r\n"))
except Exception as ex:
thread_excs.append(ex)
threads = [
threading.Thread(target=_win_pump, args=(proc.stdout, sys.stdout, True), daemon=True),
threading.Thread(target=_win_pump, args=(proc.stderr, sys.stderr, False), daemon=True),
]
for t in threads: t.start()
for t in threads: t.join()
if thread_excs:
proc.kill(); proc.wait()
raise thread_excs[0]
else:
sel = selectors.DefaultSelector()
sel.register(proc.stdout, selectors.EVENT_READ, "out")
sel.register(proc.stderr, selectors.EVENT_READ, "err")
try:
while sel.get_map():
for key, _ in sel.select():
line = key.fileobj.readline()
if not line:
sel.unregister(key.fileobj)
continue
if key.data == "out":
typer.echo(line, nl=False)
stdout_lines.append(line.rstrip("\r\n"))
else:
typer.echo(line, err=True, nl=False)
finally:
sel.close()
return stdout_lines
```
Then shrink `_run_cargo` to:
```python
def _run_cargo(args: list[str]) -> str:
typer.echo(f"$ cargo {shlex.join(args)}")
proc = cargo[args].popen(...)
# early stream‐check + cleanup omitted for brevity
try:
stdout_lines = _pump_cargo_output(proc)
ret = proc.wait()
if ret != 0:
typer.echo(f"cargo ... failed with code {ret}", err=True)
raise typer.Exit(code=ret or 1)
return "\n".join(stdout_lines)
finally:
_safe_close_text_stream(proc.stdout)
_safe_close_text_stream(proc.stderr)
```
This removes two nested `if/else` layers inside `_run_cargo`, collapses common cleanup, and keeps all behavior identical.
</issue_to_address>
### Comment 3
<location> `.github/actions/release-to-pypi-uv/scripts/check_github_release.py:56` </location>
<code_context>
def _fetch_release(repo: str, tag: str, token: str) -> dict[str, object]:
api = f"https://api.github.com/repos/{repo}/releases/tags/{tag}"
parsed = urllib.parse.urlsplit(api)
if parsed.scheme != "https": # pragma: no cover - defensive guard
message = f"Unsupported URL scheme '{parsed.scheme}' for GitHub API request."
raise GithubReleaseError(message)
request = urllib.request.Request( # noqa: S310 - https scheme enforced above
api,
headers={
"Authorization": f"Bearer {token}",
"Accept": "application/vnd.github+json",
"X-GitHub-Api-Version": "2022-11-28",
"User-Agent": "release-to-pypi-action",
},
)
max_attempts = 5
backoff_factor = 1.5
delay = 1.0
payload: str | None = None
for attempt in range(1, max_attempts + 1):
try:
with urllib.request.urlopen(request, timeout=30) as response: # noqa: S310
payload = response.read().decode("utf-8")
break
except urllib.error.HTTPError as exc: # pragma: no cover - network failure path
detail = (
exc.read().decode("utf-8", errors="ignore")
if hasattr(exc, "read")
else ""
)
match exc.code:
case 401:
context = detail or exc.reason
message = (
"GitHub rejected the token (401 Unauthorized). "
"Verify that GH_TOKEN is correct and has not expired."
)
if context:
message = f"{message} ({context})"
raise GithubReleaseError(message) from exc
case 403:
permission_message = (
"GitHub token lacks permission to read releases "
"or has expired. "
"Use a token with contents:read scope."
)
context = detail or exc.reason
message = f"{permission_message} ({context})"
raise GithubReleaseError(message) from exc
case 404:
message = (
"No GitHub release found for tag "
f"{tag}. Create and publish the release first."
)
raise GithubReleaseError(message) from exc
case _ if attempt == max_attempts:
failure_reason = detail or exc.reason
message = (
"GitHub API request failed with status "
f"{exc.code}: {failure_reason}"
)
raise GithubReleaseError(message) from exc
case _:
retry_after = None
if hasattr(exc, "headers") and exc.headers is not None:
retry_after = exc.headers.get("Retry-After")
if retry_after:
with contextlib.suppress(Exception):
delay = float(retry_after)
_sleep_with_jitter(delay)
delay *= backoff_factor
except urllib.error.URLError as exc: # pragma: no cover - network failure path
if attempt == max_attempts:
message = f"Failed to reach GitHub API: {exc.reason}"
raise GithubReleaseError(message) from exc
_sleep_with_jitter(delay)
delay *= backoff_factor
else: # pragma: no cover - loop exhausted without break
message = "GitHub API request failed after retries."
raise GithubReleaseError(message)
try:
return json.loads(payload or "")
except json.JSONDecodeError as exc: # pragma: no cover - unexpected payload
message = "GitHub API returned invalid JSON"
raise GithubReleaseError(message) from exc
</code_context>
<issue_to_address>
**issue (code-quality):** Low code quality found in \_fetch\_release - 21% ([`low-code-quality`](https://docs.sourcery.ai/Reference/Default-Rules/comments/low-code-quality/))
<br/><details><summary>Explanation</summary>The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines.
- Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.</details>
</issue_to_address>
### Comment 4
<location> `.github/actions/release-to-pypi-uv/scripts/publish_release.py:34-35` </location>
<code_context>
def _extend_sys_path() -> None:
candidates: list[Path] = []
action_path_env = os.getenv("GITHUB_ACTION_PATH")
if action_path_env:
action_path = Path(action_path_env).resolve()
candidates.append(action_path / "scripts")
with contextlib.suppress(IndexError):
candidates.append(action_path.parents[2])
else:
script_path = Path(__file__).resolve()
scripts_dir = script_path.parent
candidates.append(scripts_dir)
with contextlib.suppress(IndexError):
candidates.append(scripts_dir.parents[3])
for candidate in candidates:
if not candidate.exists():
continue
path_str = str(candidate)
if path_str not in sys.path:
sys.path.insert(0, path_str)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if action_path_env := os.getenv("GITHUB_ACTION_PATH"):
```
</issue_to_address>
### Comment 5
<location> `.github/actions/release-to-pypi-uv/scripts/validate_toml_versions.py:80-82` </location>
<code_context>
def _parse_bool(value: str | None) -> bool:
if value is None:
return False
normalized = value.strip().lower()
if not normalized:
return False
return normalized in TRUTHY_STRINGS
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Lift code into else after jump in control flow ([`reintroduce-else`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/reintroduce-else/))
- Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
```suggestion
return False if not normalized else normalized in TRUTHY_STRINGS
```
</issue_to_address>
### Comment 6
<location> `.github/actions/release-to-pypi-uv/tests/_helpers.py:14` </location>
<code_context>
"""Test helpers for release-to-pypi-uv action scripts."""
from __future__ import annotations
import importlib.util
import os
import sys
import typing as typ
from pathlib import Path
if typ.TYPE_CHECKING: # pragma: no cover - imported for annotations only
from types import ModuleType
_ACTION_PATH = os.environ.get("GITHUB_ACTION_PATH")
if _ACTION_PATH:
_action_root = Path(_ACTION_PATH).resolve()
SCRIPTS_DIR = _action_root / "scripts"
REPO_ROOT = _action_root.parents[2]
else:
SCRIPTS_DIR = Path(__file__).resolve().parents[1] / "scripts"
REPO_ROOT = SCRIPTS_DIR.parents[3]
</code_context>
<issue_to_address>
**issue (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
</issue_to_address>
### Comment 7
<location> `.github/actions/rust-build-release/src/main.py:143` </location>
<code_context>
@app.command()
def main(
target: str = typer.Argument("", help="Target triple to build"),
toolchain: str = typer.Option(
DEFAULT_TOOLCHAIN,
envvar="RBR_TOOLCHAIN",
help="Rust toolchain version",
),
) -> None:
"""Build the project for *target* using *toolchain*."""
if not target:
target = os.environ.get("RBR_TARGET", "")
if not target:
env_rbr_target = os.environ.get("RBR_TARGET", "<unset>")
env_input_target = os.environ.get("INPUT_TARGET", "<unset>")
env_github_ref = os.environ.get("GITHUB_REF", "<unset>")
typer.echo(
"::error:: no build target specified; "
"set input 'target' or env RBR_TARGET\n"
f"RBR_TARGET={env_rbr_target} "
f"INPUT_TARGET={env_input_target} "
f"GITHUB_REF={env_github_ref}",
err=True,
)
raise typer.Exit(1)
rustup_path = shutil.which("rustup")
if rustup_path is None:
typer.echo("::error:: rustup not found", err=True)
raise typer.Exit(1)
try:
rustup_exec = ensure_allowed_executable(rustup_path, ("rustup", "rustup.exe"))
except UnexpectedExecutableError:
typer.echo("::error:: unexpected rustup executable", err=True)
raise typer.Exit(1) from None
installed_names = _list_installed_toolchains(rustup_exec)
toolchain_name = _resolve_toolchain_name(toolchain, target, installed_names)
if not toolchain_name:
try:
run_cmd(
[
rustup_exec,
"toolchain",
"install",
toolchain,
"--profile",
"minimal",
"--no-self-update",
]
)
except subprocess.CalledProcessError:
typer.echo(
f"::error:: failed to install toolchain '{toolchain}'",
err=True,
)
typer.echo(
f"::error:: requested toolchain '{toolchain}' not installed",
err=True,
)
raise typer.Exit(1) from None
installed_names = _list_installed_toolchains(rustup_exec)
toolchain_name = _resolve_toolchain_name(toolchain, target, installed_names)
if not toolchain_name:
# Fallback: any installed variant that starts with the channel name.
channel_prefix = f"{toolchain}-"
toolchain_name = next(
(
name
for name in installed_names
if name == toolchain or name.startswith(channel_prefix)
),
"",
)
if not toolchain_name:
# Accept host-architecture-suffixed installations of the requested channel
channel_prefix = f"{toolchain}-"
toolchain_name = next(
(name for name in installed_names if name.startswith(channel_prefix)), ""
)
if not toolchain_name:
typer.echo(
f"::error:: requested toolchain '{toolchain}' not installed",
err=True,
)
raise typer.Exit(1)
target_installed = True
try:
run_cmd([rustup_exec, "target", "add", "--toolchain", toolchain_name, target])
except subprocess.CalledProcessError:
typer.echo(
f"::warning:: toolchain '{toolchain_name}' does not support "
f"target '{target}'; continuing",
err=True,
)
target_installed = False
configure_windows_linkers(toolchain_name, target, rustup_exec)
cross_path, cross_version = ensure_cross("0.2.5")
docker_present = False
podman_present = False
if should_probe_container(sys.platform, target):
docker_present = _probe_runtime("docker")
podman_present = _probe_runtime("podman")
has_container = docker_present or podman_present
use_cross = cross_path is not None and has_container
cargo_toolchain_spec = f"+{toolchain_name}"
cross_toolchain_spec = cargo_toolchain_spec
if use_cross:
cross_toolchain_name = _toolchain_channel(toolchain_name)
if (
cross_toolchain_name != toolchain_name
and cross_toolchain_name not in installed_names
):
try:
run_cmd(
[
rustup_exec,
"toolchain",
"install",
cross_toolchain_name,
"--profile",
"minimal",
"--no-self-update",
]
)
except subprocess.CalledProcessError:
typer.echo(
"::warning:: failed to install sanitized toolchain; using cargo",
err=True,
)
use_cross = False
else:
installed_names = _list_installed_toolchains(rustup_exec)
if use_cross:
cross_toolchain_spec = f"+{cross_toolchain_name}"
if not use_cross and not target_installed:
typer.echo(
f"::error:: toolchain '{toolchain_name}' does not support "
f"target '{target}'",
err=True,
)
raise typer.Exit(1)
if not use_cross:
if cross_path is None:
typer.echo("cross missing; using cargo")
elif not has_container:
typer.echo(
f"cross ({cross_version}) requires a container runtime; using cargo "
f"(docker={docker_present}, podman={podman_present})"
)
else:
typer.echo(f"Building with cross ({cross_version})")
build_cmd = [
"cross" if use_cross else "cargo",
cross_toolchain_spec if use_cross else cargo_toolchain_spec,
"build",
"--release",
"--target",
target,
]
try:
run_cmd(build_cmd)
except subprocess.CalledProcessError as exc:
if use_cross and exc.returncode in CROSS_CONTAINER_ERROR_CODES:
typer.echo(
"::warning:: cross failed to start a container; retrying with cargo",
err=True,
)
fallback_cmd = [
"cargo",
cargo_toolchain_spec,
"build",
"--release",
"--target",
target,
]
run_cmd(fallback_cmd)
else:
raise
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Hoist conditional out of nested conditional ([`hoist-if-from-if`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/hoist-if-from-if/))
- Swap if/else branches ([`swap-if-else-branches`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/swap-if-else-branches/))
- Merge else clause's nested if statement into elif ([`merge-else-if-into-elif`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/merge-else-if-into-elif/))
- Low code quality found in main - 15% ([`low-code-quality`](https://docs.sourcery.ai/Reference/Default-Rules/comments/low-code-quality/))
<br/><details><summary>Explanation</summary>
The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines.
- Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.</details>
</issue_to_address>
### Comment 8
<location> `.github/actions/rust-build-release/tests/test_cross_install.py:500-502` </location>
<code_context>
@CMD_MOX_UNSUPPORTED
def test_falls_back_to_cargo_when_podman_unusable(
main_module: ModuleType,
cross_module: ModuleType,
module_harness: HarnessFactory,
cmd_mox: CmdMox,
) -> None:
"""Falls back to cargo when podman exists but is unusable."""
cross_env = module_harness(cross_module)
app_env = module_harness(main_module)
default_toolchain = main_module.DEFAULT_TOOLCHAIN
rustup_stdout = f"{default_toolchain}-x86_64-unknown-linux-gnu\n"
cross_path = _register_cross_version_stub(cmd_mox)
rustup_path = _register_rustup_toolchain_stub(cmd_mox, rustup_stdout)
podman_path = _register_podman_info_stub(cmd_mox, exit_code=1)
def fake_which(name: str) -> str | None:
if name == "podman":
return podman_path
if name == "cross":
return cross_path
if name == "rustup":
return rustup_path
return None
cross_env.patch_shutil_which(fake_which)
app_env.patch_shutil_which(fake_which)
cmd_mox.replay()
main_module.main("x86_64-unknown-linux-gnu", default_toolchain)
cmd_mox.verify()
assert any(cmd[0] == "cargo" for cmd in app_env.calls)
assert all(cmd[0] != "cross" for cmd in app_env.calls)
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Lift code into else after jump in control flow ([`reintroduce-else`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/reintroduce-else/))
- Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
```suggestion
return rustup_path if name == "rustup" else None
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if ! command -v x86_64-w64-mingw32-gcc >/dev/null 2>&1; then | ||
| echo "::warning::x86_64 MinGW GCC not found" >&2 | ||
| fi | ||
| require_a64="${{ inputs.require-aarch64 }}" |
There was a problem hiding this comment.
suggestion (bug_risk): Shell quoting for require_aarch64 may be fragile.
Double-quoting the GitHub input may not handle all cases, such as unexpected whitespace or non-boolean values. Use a stricter parsing method to ensure reliable boolean handling in bash.
| stream.close() | ||
|
|
||
|
|
||
| def _run_cargo(args: list[str]) -> str: |
There was a problem hiding this comment.
issue (complexity): Consider extracting the platform-specific output pumping logic into a helper function to make _run_cargo flatter and more focused.
Consider extracting the two big “pump” branches (win vs posix) into a helper so that _run_cargo becomes flat and focused on orchestration (spawn → pump → wait → cleanup). For example:
def _pump_cargo_output(proc: subprocess.Popen[str]) -> list[str]:
"""Pump proc.stdout/stderr to console, return collected stdout lines."""
stdout_lines: list[str] = []
if os.name == "nt":
thread_excs: list[Exception] = []
def _win_pump(src: TextIO, out: TextIO, collect: bool):
try:
for line in iter(src.readline, ""):
out.write(line); out.flush()
if collect:
stdout_lines.append(line.rstrip("\r\n"))
except Exception as ex:
thread_excs.append(ex)
threads = [
threading.Thread(target=_win_pump, args=(proc.stdout, sys.stdout, True), daemon=True),
threading.Thread(target=_win_pump, args=(proc.stderr, sys.stderr, False), daemon=True),
]
for t in threads: t.start()
for t in threads: t.join()
if thread_excs:
proc.kill(); proc.wait()
raise thread_excs[0]
else:
sel = selectors.DefaultSelector()
sel.register(proc.stdout, selectors.EVENT_READ, "out")
sel.register(proc.stderr, selectors.EVENT_READ, "err")
try:
while sel.get_map():
for key, _ in sel.select():
line = key.fileobj.readline()
if not line:
sel.unregister(key.fileobj)
continue
if key.data == "out":
typer.echo(line, nl=False)
stdout_lines.append(line.rstrip("\r\n"))
else:
typer.echo(line, err=True, nl=False)
finally:
sel.close()
return stdout_linesThen shrink _run_cargo to:
def _run_cargo(args: list[str]) -> str:
typer.echo(f"$ cargo {shlex.join(args)}")
proc = cargo[args].popen(...)
# early stream‐check + cleanup omitted for brevity
try:
stdout_lines = _pump_cargo_output(proc)
ret = proc.wait()
if ret != 0:
typer.echo(f"cargo ... failed with code {ret}", err=True)
raise typer.Exit(code=ret or 1)
return "\n".join(stdout_lines)
finally:
_safe_close_text_stream(proc.stdout)
_safe_close_text_stream(proc.stderr)This removes two nested if/else layers inside _run_cargo, collapses common cleanup, and keeps all behavior identical.
| """Raised when the GitHub release is not ready for publishing.""" | ||
|
|
||
|
|
||
| def _fetch_release(repo: str, tag: str, token: str) -> dict[str, object]: |
There was a problem hiding this comment.
issue (code-quality): Low code quality found in _fetch_release - 21% (low-code-quality)
Explanation
The quality score for this function is below the quality threshold of 25%.This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines. - Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.
| action_path_env = os.getenv("GITHUB_ACTION_PATH") | ||
| if action_path_env: |
There was a problem hiding this comment.
suggestion (code-quality): Use named expression to simplify assignment and conditional (use-named-expression)
| action_path_env = os.getenv("GITHUB_ACTION_PATH") | |
| if action_path_env: | |
| if action_path_env := os.getenv("GITHUB_ACTION_PATH"): |
| if not normalized: | ||
| return False | ||
| return normalized in TRUTHY_STRINGS |
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
| if not normalized: | |
| return False | |
| return normalized in TRUTHY_STRINGS | |
| return False if not normalized else normalized in TRUTHY_STRINGS |
| if typ.TYPE_CHECKING: # pragma: no cover - imported for annotations only | ||
| from types import ModuleType | ||
|
|
||
| _ACTION_PATH = os.environ.get("GITHUB_ACTION_PATH") |
There was a problem hiding this comment.
issue (code-quality): Use named expression to simplify assignment and conditional (use-named-expression)
|
|
||
|
|
||
| @app.command() | ||
| def main( |
There was a problem hiding this comment.
issue (code-quality): We've found these issues:
- Hoist conditional out of nested conditional (
hoist-if-from-if) - Swap if/else branches (
swap-if-else-branches) - Merge else clause's nested if statement into elif (
merge-else-if-into-elif) - Low code quality found in main - 15% (
low-code-quality)
Explanation
The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines. - Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.
| if name == "rustup": | ||
| return rustup_path | ||
| return None |
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
| if name == "rustup": | |
| return rustup_path | |
| return None | |
| return rustup_path if name == "rustup" else None |
1 participant
Summary
Testing
Summary by Sourcery
Introduce a new composite action ‘release-to-pypi-uv’ to streamline release workflows with uv, bolster error handling and retries in existing actions, integrate cmd-mox for more robust command‐doubles testing, enforce lint/formatting in CI, and enrich documentation and tests across the repository
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests: