ci: add packaging release gates with artifact smoke checks#21
Merged
nsalvacao merged 1 commit intocandidate/lote2-audit-2026-03-16from Mar 16, 2026
Merged
Conversation
- build sdist/wheel in CI\n- validate artifact set deterministically\n- install built wheel and smoke test imports/entrypoints\n- run CI on candidate PR branches Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
🔍 AI Code Review# Code Review Summary
---
### 🔵 CI Workflow (.github/workflows/ci.yml)
- Added `candidate/**` branches to trigger CI on push and PR — good for feature gating.
- Added `package-release-gates` job:
- Builds sdist and wheel with fixed `SOURCE_DATE_EPOCH` for reproducibility — best practice.
- Validates exactly one sdist and one wheel artifact exist.
- Smoke tests install and import of built wheel, runs help commands — good sanity checks.
No issues found here.
---
### 🔵 Formatter Changes (src/crawler/formatter.py)
- Changed `_embed_raw` to remove unused `cli_name` parameter — cleaner signature.
- Fixed call site to pass `data.get("commands", {})` instead of `data.get("tree", {})` — likely a bug fix.
- Type hinting improved for `_embed_raw`.
No security or bug concerns.
---
### 🟡 Pipeline Changes (src/crawler/pipeline.py)
- Added `shutil` and `sys` imports (sys unused, consider removing).
- Introduced `RootCLIBinaryNotFoundError` exception for missing root CLI binary.
- Added pre-check with `shutil.which(cli_name)` to fast-fail if root CLI binary not found — improves UX and avoids locale-dependent stderr parsing.
- Added `_is_missing_root_cli_binary` helper to detect missing binary from stderr/stdout content.
- Raises `RootCLIBinaryNotFoundError` if binary missing detected after help pattern detection.
- Minor code style improvements.
**Warnings:**
- `sys` imported but unused — remove to keep clean.
- The heuristic in `_is_missing_root_cli_binary` relies on error message substrings; consider localization or edge cases where messages differ.
- Raising a custom exception is good, but ensure callers handle `RootCLIBinaryNotFoundError` gracefully to avoid unhandled crashes.
---
# Summary
- 🔴 **No critical security issues found.**
- 🟡 Minor warnings on unused imports and heuristic detection robustness.
- 🔵 Best practices applied in CI and error handling.
---
# Recommendations
- Remove unused `sys` import.
- Ensure `RootCLIBinaryNotFoundError` is caught and handled properly in CLI or higher-level code.
- Consider extending `_is_missing_root_cli_binary` to handle localized error messages or different shells.🤖 gpt-4.1-mini · 2133 tokens · GitHub Models free tier · 0 premium requests |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:\n- run CI for PRs targeting candidate branches\n- add package-release-gates job to build sdist and wheel using python -m build\n- enforce deterministic artifact expectations (exactly one sdist and one wheel)\n- install built wheel and run import/entrypoint smoke checks\n\nValidation:\n- local build of sdist and wheel succeeded\n- local clean-venv install from built wheel succeeded\n- local smoke checks for imports and CLI help commands passed