feat: wire F4 provisioning evidence gates into packaging#126
Conversation
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces F4 linter provisioning evidence tracking across the packaging pipeline. A new ChangesF4 Linter Provisioning Evidence
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/build_pyinstaller_linux.py`:
- Line 41: The unconditional F4 hook import in build_pyinstaller_linux.py is
affecting shared packaging flows like package_appimage and
build_and_package_slackware. Move the
run_or_record_f4_linter_provisioning_for_artifacts step out of the shared helper
and into the release-level entry point, or guard it with a top-level
flag/sentinel so only the intended PyInstaller path runs it. Update the
build_pyinstaller_linux helper so its behavior stays reusable without triggering
duplicate evidence work for linux-pyinstaller and linux-tarball.
In `@scripts/build-and-package-windows.ps1`:
- Around line 57-104: The Invoke-F4LinterEvidenceHook function assigns to
$profile, which collides with the PowerShell automatic variable and is flagged
by PSAvoidAssignmentToAutomaticVariable. Rename that local variable to a
non-reserved name such as $linterProfile, then update the value selection logic
and the $provisionArgs array entry that passes the profile option so the
behavior stays unchanged.
In `@tests/packaging/test_f4_linter_packaging_hooks.py`:
- Around line 178-187: Add a parallel path traversal rejection test for
build_provisioning_command, since it uses the same canonical_artifact_entry_id
validation as build_verification_command but lacks direct coverage. Reuse the
existing test pattern in
test_invalid_artifact_id_is_rejected_before_command_construction, and invoke
build_provisioning_command with a "../deb" artifact_entry_id plus a temporary
provisioning/evidence path, asserting it raises ValueError before any command
construction occurs.
🪄 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: 74a03503-6098-4fe3-8100-29a3ce9b36b9
📒 Files selected for processing (21)
.github/workflows/release.ymldocker/build-linux-deb.Dockerfiledocker/build-linux-rpm.Dockerfilescripts/build-and-package-windows.ps1scripts/build_and_package_arch.pyscripts/build_and_package_deb.pyscripts/build_and_package_freebsd.pyscripts/build_and_package_macos.pyscripts/build_and_package_opensuse_rpm.pyscripts/build_and_package_rpm.pyscripts/build_and_package_slackware.pyscripts/build_docker.pyscripts/build_freebsd_pkg.pyscripts/build_freebsd_port.pyscripts/build_pyinstaller_linux.pyscripts/f4_linter_packaging.pyscripts/package_appimage.pyscripts/publish_pypi.pyscripts/verify_release_assets.pysrc/ecli/extensions/linters/core/provisioning.pytests/packaging/test_f4_linter_packaging_hooks.py
| import sys | ||
| from pathlib import Path | ||
|
|
||
| from f4_linter_packaging import run_or_record_f4_linter_provisioning_for_artifacts |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP --type=py -B2 -A2 'build_pyinstaller_linux\.py' scripts/Repository: SSobol77/ecli
Length of output: 1637
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== callers of build_pyinstaller_linux.py =='
rg -n "build_pyinstaller_linux\.py" scripts/ || true
echo
echo '== relevant section of scripts/build_pyinstaller_linux.py =='
sed -n '1,220p' scripts/build_pyinstaller_linux.py
echo
echo '== relevant sections of scripts/package_appimage.py and scripts/build_and_package_slackware.py =='
sed -n '90,130p' scripts/package_appimage.py
echo '---'
sed -n '145,170p' scripts/build_and_package_slackware.pyRepository: SSobol77/ecli
Length of output: 7663
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== definition of run_or_record_f4_linter_provisioning_for_artifacts =='
rg -n "def run_or_record_f4_linter_provisioning_for_artifacts|run_or_record_f4_linter_provisioning_for_artifacts\(" -S .
echo
echo '== build_pyinstaller_linux.py around the hook =='
sed -n '120,170p' scripts/build_pyinstaller_linux.py
echo
echo '== callers of build_pyinstaller_linux.py =='
rg -n "build_pyinstaller_linux\.py" scripts/Repository: SSobol77/ecli
Length of output: 2167
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== f4 helper definition =='
sed -n '430,540p' scripts/f4_linter_packaging.py
echo
echo '== build_and_package_deb.py around F4 call =='
sed -n '320,360p' scripts/build_and_package_deb.py
echo
echo '== build_and_package_rpm.py around F4 call =='
sed -n '420,460p' scripts/build_and_package_rpm.py
echo
echo '== build_and_package_macos.py around F4 call =='
sed -n '560,600p' scripts/build_and_package_macos.pyRepository: SSobol77/ecli
Length of output: 5644
Move the F4 hook out of the shared PyInstaller build helper
build_pyinstaller_linux.py is reused by scripts/package_appimage.py and scripts/build_and_package_slackware.py, so the unconditional F4 step now runs for those packaging flows too and duplicates linux-pyinstaller/linux-tarball evidence work. Gate it behind a top-level flag/sentinel or move it to the release-level entry point.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/build_pyinstaller_linux.py` at line 41, The unconditional F4 hook
import in build_pyinstaller_linux.py is affecting shared packaging flows like
package_appimage and build_and_package_slackware. Move the
run_or_record_f4_linter_provisioning_for_artifacts step out of the shared helper
and into the release-level entry point, or guard it with a top-level
flag/sentinel so only the intended PyInstaller path runs it. Update the
build_pyinstaller_linux helper so its behavior stays reusable without triggering
duplicate evidence work for linux-pyinstaller and linux-tarball.
| def test_invalid_artifact_id_is_rejected_before_command_construction( | ||
| f4_packaging: ModuleType, | ||
| tmp_path: Path, | ||
| ) -> None: | ||
| with pytest.raises(ValueError): | ||
| f4_packaging.build_verification_command( | ||
| tmp_path, | ||
| artifact_entry_id="../deb", | ||
| evidence_dir=tmp_path / "evidence", | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
Add path traversal test for build_provisioning_command too.
The existing test validates that build_verification_command rejects "../deb", but build_provisioning_command uses the same canonical_artifact_entry_id validation and is not independently tested for path traversal rejection. Adding a parallel test closes this security-relevant coverage gap.
♻️ Proposed additional test
def test_invalid_artifact_id_is_rejected_before_command_construction(
f4_packaging: ModuleType,
tmp_path: Path,
) -> None:
with pytest.raises(ValueError):
f4_packaging.build_verification_command(
tmp_path,
artifact_entry_id="../deb",
evidence_dir=tmp_path / "evidence",
)
+
+ with pytest.raises(ValueError):
+ f4_packaging.build_provisioning_command(
+ tmp_path,
+ artifact_entry_id="../deb",
+ target_dir=tmp_path / "target",
+ evidence_dir=tmp_path / "evidence",
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_invalid_artifact_id_is_rejected_before_command_construction( | |
| f4_packaging: ModuleType, | |
| tmp_path: Path, | |
| ) -> None: | |
| with pytest.raises(ValueError): | |
| f4_packaging.build_verification_command( | |
| tmp_path, | |
| artifact_entry_id="../deb", | |
| evidence_dir=tmp_path / "evidence", | |
| ) | |
| def test_invalid_artifact_id_is_rejected_before_command_construction( | |
| f4_packaging: ModuleType, | |
| tmp_path: Path, | |
| ) -> None: | |
| with pytest.raises(ValueError): | |
| f4_packaging.build_verification_command( | |
| tmp_path, | |
| artifact_entry_id="../deb", | |
| evidence_dir=tmp_path / "evidence", | |
| ) | |
| with pytest.raises(ValueError): | |
| f4_packaging.build_provisioning_command( | |
| tmp_path, | |
| artifact_entry_id="../deb", | |
| target_dir=tmp_path / "target", | |
| evidence_dir=tmp_path / "evidence", | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/packaging/test_f4_linter_packaging_hooks.py` around lines 178 - 187,
Add a parallel path traversal rejection test for build_provisioning_command,
since it uses the same canonical_artifact_entry_id validation as
build_verification_command but lacks direct coverage. Reuse the existing test
pattern in test_invalid_artifact_id_is_rejected_before_command_construction, and
invoke build_provisioning_command with a "../deb" artifact_entry_id plus a
temporary provisioning/evidence path, asserting it raises ValueError before any
command construction occurs.
|
1 participant
Summary
This PR wires F4 linter provisioning evidence gates into ECLI packaging and release flows.
It adds a shared packaging helper for artifact-specific F4 evidence generation and verification, then connects that helper to the existing packaging scripts and release verification path.
This PR is intentionally scoped as evidence-gate wiring. It does not claim concrete non-dry-run installer/download/bundling implementation for external linter binaries. Real non-dry-run provisioning remains explicit opt-in via
ECLI_F4_LINTER_PROVISIONING_MODE=provisionand is left for the next implementation PR.What changed
Added
scripts/f4_linter_packaging.pyshell=TrueWired F4 evidence hooks into packaging scripts:
Extended release verification:
scripts/verify_release_assets.py --f4-evidence-dir <dir>Added packaging tests:
Artifact policy
Full evidence hook:
linux-pyinstallerlinux-tarballdebrpmopensuse-rpmarch-pkgbuildslackware-txzappimagefreebsd-pkgfreebsd-ports-chrootmacos-appmacos-dmgwindows-portable-exewindows-nsis-installerdocker-deb-helperdocker-rpm-helpergha-release-contractConstrained/minimal:
pypi-wheelpypi-sdistNix policy:
nix-flakenixos-packageExplicit non-scope
This PR does not implement concrete external linter binary installation, downloading, vendoring, or bundling.
No changes were made to:
src/ecli/core/Ecli.pysrc/ecli/ui/src/ecli/diagnostics/service.pyValidation
Passed locally:
Observed results:
Safety checks:
Next PR
Next implementation PR should handle concrete non-dry-run installer/download/bundling mechanisms per artifact family and package-manager policy.
Summary by CodeRabbit
New Features
Bug Fixes