infra: pypi trusted publishing ci#245
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 17 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughAdds a GitHub Actions "Publish" workflow triggered by tag pushes, updates package Changes
Sequence Diagram(s)sequenceDiagram
participant Tag as "Git Tag (v*)"
participant GH as "GitHub Actions"
participant Runner as "Runner (ubuntu-latest)"
participant Tests as "CI Smoke Tests"
participant PyPI as "PyPI (uv publish)"
Tag->>GH: push tag triggers workflow
GH->>Runner: start publish job (pypi env)
Runner->>Runner: checkout repo
Runner->>Runner: install setup-uv & uv\nsetup Python 3.12\nuv build
Runner->>Runner: verify tag version == temoa.__version__
Runner->>Tests: run smoke tests against wheel and sdist (tests/smoke_test.py)
Tests-->>Runner: test results
Runner->>PyPI: uv publish (using id-token)
PyPI-->>Runner: publish result
Runner-->>GH: job completes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
e261834 to
e10e0f9
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI Agents
In @.github/workflows/publish.yml:
- Around line 31-32: Add a pre-publish verification step before the "Publish"
run that extracts the tag version from GITHUB_REF (e.g. strip "refs/tags/v"),
reads the package version by running python -c "import temoa;
print(temoa.__version__)", compares the two, and fails the job (exit 1) with a
clear error if they differ; if they match, allow the workflow to continue to the
existing uv publish step and emit a success message.
- Around line 27-30: Add a timeout to the two smoke test workflow steps ("Smoke
test (wheel)" and "Smoke test (source distribution)") so a hung CLI won't stall
the run; update each job step to include a timeout-minutes (e.g.,
timeout-minutes: 10) alongside the existing name and run keys to ensure the
subprocess in tests/smoke_test.py is bounded.
In @tests/smoke_test.py:
- Around line 17-21: The test_help function uses subprocess.run without a
timeout which can hang; update the subprocess.run call in test_help to include a
timeout (match the value used in test_cli, e.g., timeout=10) and optionally
catch subprocess.TimeoutExpired to fail the test with a clear message; target
the subprocess.run invocation inside test_help.
- Around line 5-7: The test_import test currently only asserts temoa.__version__
is not None; strengthen it to ensure temoa.__version__ is a non-empty string by
checking its type and that its length is > 0 (e.g., assert
isinstance(temoa.__version__, str) and assert temoa.__version__.strip() != ""),
referencing the test_import function and temoa.__version__ to locate and update
the assertion.
- Around line 9-15: The subprocess.run call in test_cli can hang indefinitely;
add a timeout (e.g., timeout=10) to the subprocess.run invocation in the
test_cli function and handle subprocess.TimeoutExpired by failing the test
(raise or pytest.fail) with a clear message; update the call from
subprocess.run([...], capture_output=True, text=True) to include timeout=10 and
wrap it in try/except subprocess.TimeoutExpired to provide a deterministic test
failure if the CLI is unresponsive.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/publish.ymltemoa/__about__.pytests/smoke_test.py
🧰 Additional context used
🪛 Ruff (0.14.10)
tests/smoke_test.py
11-11: Starting a process with a partial executable path
(S607)
19-19: Starting a process with a partial executable path
(S607)
29-29: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (5)
.github/workflows/publish.yml (2)
3-7: LGTM! Appropriate trigger configuration.The tag-based trigger for version releases follows conventional patterns for publishing workflows.
10-16: LGTM! Correct permissions for trusted publishing.The
id-token: writepermission is properly configured for OIDC-based trusted publishing to PyPI.temoa/__about__.py (1)
3-3: LGTM! Version bump aligns with release workflow.The version change from development (
4.0.0a1.dev20251201) to alpha (4.0.0a1) is appropriate for the new publishing workflow targeting stable releases.tests/smoke_test.py (2)
1-3: LGTM! Imports are appropriate for smoke tests.The direct import of
temoais correct for this use case, as the workflow installs the package before running tests.
23-31: LGTM! Exception handling is appropriate for smoke tests.The bare
except Exceptionis acceptable in this context, as the smoke test needs to catch and report any failure. The static analysis warning (BLE001) can be safely ignored here.
e10e0f9 to
edd743a
Compare
edd743a to
591772f
Compare
591772f to
d1ca3a6
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
tests/smoke_test.py (2)
9-15: 🧹 Nitpick | 🔵 TrivialAdd explicit
check=Falseto document intent and add timeout.Since you're manually checking
returncode, adding explicitcheck=Falsedocuments this intent and silences the linter warning (PLW1510). The timeout concern from prior feedback also remains applicable.🔧 Proposed fix
def test_cli() -> None: print("Running temoa --version CLI command...") - result = subprocess.run(["temoa", "--version"], capture_output=True, text=True) + result = subprocess.run(["temoa", "--version"], capture_output=True, text=True, check=False, timeout=30) print(f"CLI output: {result.stdout.strip()}") assert result.returncode == 0 assert "Temoa Version:" in result.stdout assert temoa.__version__ in result.stdout,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/smoke_test.py` around lines 9 - 15, In test_cli update the subprocess.run call inside test_cli to pass explicit check=False and a reasonable timeout (e.g., timeout=10) so the test documents that return codes are checked manually and cannot hang; locate the subprocess.run invocation in the test_cli function and add these kwargs while keeping capture_output=True and text=True unchanged.
17-21: 🧹 Nitpick | 🔵 TrivialSame recommendations apply: explicit
check=Falseand timeout.🔧 Proposed fix
def test_help() -> None: print("Running temoa --help CLI command...") - result = subprocess.run(["temoa", "--help"], capture_output=True, text=True) + result = subprocess.run(["temoa", "--help"], capture_output=True, text=True, check=False, timeout=30) assert result.returncode == 0 assert "The Temoa Project" in result.stdout,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/smoke_test.py` around lines 17 - 21, The subprocess.run invocation in test_help (function test_help) should explicitly pass check=False and a reasonable timeout to avoid hanging tests; update the call to subprocess.run(["temoa", "--help"], capture_output=True, text=True, check=False, timeout=30) so the test explicitly opts out of raising on nonzero exit and fails fast on hangs while keeping the existing assertions on result.returncode and result.stdout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/smoke_test.py`:
- Around line 9-15: In test_cli update the subprocess.run call inside test_cli
to pass explicit check=False and a reasonable timeout (e.g., timeout=10) so the
test documents that return codes are checked manually and cannot hang; locate
the subprocess.run invocation in the test_cli function and add these kwargs
while keeping capture_output=True and text=True unchanged.
- Around line 17-21: The subprocess.run invocation in test_help (function
test_help) should explicitly pass check=False and a reasonable timeout to avoid
hanging tests; update the call to subprocess.run(["temoa", "--help"],
capture_output=True, text=True, check=False, timeout=30) so the test explicitly
opts out of raising on nonzero exit and fails fast on hangs while keeping the
existing assertions on result.returncode and result.stdout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 45d3af42-a00d-437e-8f01-3d66a1cb33ed
📒 Files selected for processing (3)
.github/workflows/publish.ymltemoa/__about__.pytests/smoke_test.py
914d391 to
4c3f0ed
Compare
1d65fd6 to
dcb4d18
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/publish.yml:
- Around line 19-21: Replace the mutable action tags with immutable commit SHAs:
change uses: actions/checkout@v6 and uses: astral-sh/setup-uv@v7 to use the
full-length commit SHAs for those repositories, and add a short comment (e.g.,
"# pin to SHA for supply-chain integrity (was v6/v7)") beside each for version
reference; ensure the SHAs are current and verified before committing.
In `@tests/smoke_test.py`:
- Line 11: Replace the bare "temoa" string passed to subprocess.run with an
absolute executable path resolved via shutil.which("temoa") (or shutil.which and
Path.resolve) and fail the test early with a clear error if which returns None;
update both subprocess.run calls (the one assigning result and the one later at
line ~19) to use the resolved path variable instead of the literal "temoa" to
prevent PATH hijacking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b59d04fa-7964-410f-b710-173a588782a3
📒 Files selected for processing (3)
.github/workflows/publish.ymltemoa/__about__.pytests/smoke_test.py
dcb4d18 to
c4d76ac
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
.github/workflows/publish.yml (3)
19-21:⚠️ Potential issue | 🟠 MajorPin third-party actions to immutable SHAs.
Using mutable major tags (
@v6,@v7) weakens workflow supply-chain integrity.🔧 Proposed fix
- name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@<FULL_LENGTH_COMMIT_SHA> # v6 - name: Install uv - uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@<FULL_LENGTH_COMMIT_SHA> # v7🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish.yml around lines 19 - 21, Replace mutable version tags with immutable commit SHAs for the third-party actions: change uses: actions/checkout@v6 to the corresponding pinned SHA (e.g., actions/checkout@<commit-sha>) and change uses: astral-sh/setup-uv@v7 to its pinned SHA (e.g., astral-sh/setup-uv@<commit-sha>); update both uses entries in the workflow to the exact commit hashes for the current tested versions and ensure the SHAs are recorded instead of version tags to harden supply-chain integrity.
31-32:⚠️ Potential issue | 🟠 MajorVerify git tag version matches package version before publish.
Without this guard, a mismatched tag can publish incorrectly versioned artifacts.
🔧 Proposed fix
+ - name: Verify version matches tag + run: | + TAG_VERSION="${GITHUB_REF#refs/tags/v}" + PACKAGE_VERSION=$(uv run python -c "import temoa; print(temoa.__version__)") + if [ "$TAG_VERSION" != "$PACKAGE_VERSION" ]; then + echo "Error: tag version ($TAG_VERSION) != package version ($PACKAGE_VERSION)" + exit 1 + fi - name: Publish run: uv publish🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish.yml around lines 31 - 32, Add a pre-publish guard that verifies the Git tag matches the package version before the existing "Publish" step that runs "uv publish": implement a new step prior to the "Publish" step which extracts the tag from GITHUB_REF, reads the version from package.json (or the package manifest your publisher uses), compares them, and fails the job with a clear error if they differ so "uv publish" only runs when tag === package version.
27-30:⚠️ Potential issue | 🟡 MinorAdd step-level timeouts to smoke tests.
These steps can hang if the CLI blocks; bound them with
timeout-minutes.🔧 Proposed fix
- name: Smoke test (wheel) + timeout-minutes: 5 run: uv run --isolated --no-project --with dist/*.whl tests/smoke_test.py - name: Smoke test (source distribution) + timeout-minutes: 5 run: uv run --isolated --no-project --with dist/*.tar.gz tests/smoke_test.py🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish.yml around lines 27 - 30, The two GitHub Actions steps named "Smoke test (wheel)" and "Smoke test (source distribution)" lack per-step timeouts and can hang; add a step-level timeout by adding the timeout-minutes key to each step (e.g., timeout-minutes: 10) so the "uv run" commands are bounded and the workflow fails fast if the CLI blocks.tests/smoke_test.py (1)
11-11:⚠️ Potential issue | 🟠 MajorHarden subprocess calls: resolve executable path and add timeouts.
Both CLI invocations still use a partial path (
"temoa") and no timeout, which keeps S607/PLW1510 risk and can hang CI indefinitely.🔧 Proposed fix
+import shutil import subprocess import sys import temoa @@ +def _temoa_bin() -> str: + path = shutil.which("temoa") + if path is None: + raise AssertionError("temoa executable not found on PATH") + return path + def test_cli() -> None: print("Running temoa --version CLI command...") - result = subprocess.run(["temoa", "--version"], capture_output=True, text=True) + result = subprocess.run( + [_temoa_bin(), "--version"], + capture_output=True, + text=True, + check=False, + timeout=30, + ) @@ def test_help() -> None: print("Running temoa --help CLI command...") - result = subprocess.run(["temoa", "--help"], capture_output=True, text=True) + result = subprocess.run( + [_temoa_bin(), "--help"], + capture_output=True, + text=True, + check=False, + timeout=30, + )Also applies to: 19-19
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/smoke_test.py` at line 11, The subprocess.run call assigns to result but uses a bare executable name ("temoa") and no timeout; update the invocation(s) that call subprocess.run (the lines creating result from subprocess.run(["temoa", ...]) and the similar call later) to first resolve the executable via shutil.which("temoa") and raise/assert if not found, then call subprocess.run with the resolved path and a sensible timeout argument (e.g., timeout=10) plus check returncode; ensure both occurrences are changed and preserve capture_output=True and text=True.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/smoke_test.py`:
- Around line 23-31: The top-level try/except in the script entrypoint should be
removed so exceptions from test_import, test_cli, and test_help propagate with
full tracebacks; delete the try/except block (and the sys.exit(1) failure path
and its simple print) and leave the calls to test_import(), test_cli(),
test_help() and the success print("\n✅ Smoke test passed!") so Python's default
error handling surfaces failures in CI.
---
Duplicate comments:
In @.github/workflows/publish.yml:
- Around line 19-21: Replace mutable version tags with immutable commit SHAs for
the third-party actions: change uses: actions/checkout@v6 to the corresponding
pinned SHA (e.g., actions/checkout@<commit-sha>) and change uses:
astral-sh/setup-uv@v7 to its pinned SHA (e.g., astral-sh/setup-uv@<commit-sha>);
update both uses entries in the workflow to the exact commit hashes for the
current tested versions and ensure the SHAs are recorded instead of version tags
to harden supply-chain integrity.
- Around line 31-32: Add a pre-publish guard that verifies the Git tag matches
the package version before the existing "Publish" step that runs "uv publish":
implement a new step prior to the "Publish" step which extracts the tag from
GITHUB_REF, reads the version from package.json (or the package manifest your
publisher uses), compares them, and fails the job with a clear error if they
differ so "uv publish" only runs when tag === package version.
- Around line 27-30: The two GitHub Actions steps named "Smoke test (wheel)" and
"Smoke test (source distribution)" lack per-step timeouts and can hang; add a
step-level timeout by adding the timeout-minutes key to each step (e.g.,
timeout-minutes: 10) so the "uv run" commands are bounded and the workflow fails
fast if the CLI blocks.
In `@tests/smoke_test.py`:
- Line 11: The subprocess.run call assigns to result but uses a bare executable
name ("temoa") and no timeout; update the invocation(s) that call subprocess.run
(the lines creating result from subprocess.run(["temoa", ...]) and the similar
call later) to first resolve the executable via shutil.which("temoa") and
raise/assert if not found, then call subprocess.run with the resolved path and a
sensible timeout argument (e.g., timeout=10) plus check returncode; ensure both
occurrences are changed and preserve capture_output=True and text=True.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2ecf0521-2271-48cd-ba50-2bcd9b4fc476
📒 Files selected for processing (3)
.github/workflows/publish.ymltemoa/__about__.pytests/smoke_test.py
63ae043 to
2c70af4
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/publish.yml:
- Around line 28-29: Replace the brittle shell parsing that sets VERSION using
grep/cut with a small Python-based extraction so VERSION is read robustly from
temoa/__about__.py regardless of single vs double quotes or spacing; update the
step that assigns VERSION (and leaves TAG extraction as-is) to run a Python
one-liner which imports ast, reads/parses __about__.py, and prints the
__version__ value so the VERSION variable gets the exact string or fails with a
clear error.
In `@scripts/bump_version.py`:
- Around line 54-66: In bump_version(), when handling prerelease branches (the
block using pre_map, target_pre and new['pre_type']/new['pre_num']), add logic
to prevent lowering prerelease precedence (e.g., rc -> a or b -> a). Define an
explicit ordinal list like ['a','b','rc'] (map 'alpha'->'a','beta'->'b'),
compare the current new['pre_type'] ordinal to the target_pre ordinal; if
current exists and target_pre ordinal is lower than current, treat this as a
version-increasing change by incrementing new['patch'], setting
new['pre_type']=target_pre and new['pre_num']=1 (instead of just switching
types), otherwise keep the existing behavior for same-type increments or
first-time prereleases.
In `@tests/smoke_test.py`:
- Around line 14-16: Extract the duplicated shutil.which('temoa') guard into a
single helper function (e.g., get_temoa_path or find_executable) and replace
both occurrences in tests/smoke_test.py with calls to that helper; the helper
should call shutil.which('temoa'), raise the same RuntimeError('temoa executable
not found in PATH') when absent, and return the path when found so both the
earlier temoa_path usage and the later copy (previously lines 14–16 and 28–30)
use the unified logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8772930a-0b4f-4b77-bdc1-0b2aa82fea73
📒 Files selected for processing (5)
.github/workflows/publish.yml.gitignorescripts/bump_version.pytemoa/__about__.pytests/smoke_test.py
2c70af4 to
5c24fa1
Compare
Adds CI workflow for uploading tagged
vcommits to pypi when enabled through trusted publishing.Also adds a simple smoke_test to ensure the build functions as intended
Also adds a bump script for convenience.
Summary by CodeRabbit