Add install-whitaker shared action - #361
Open
leynos wants to merge 3 commits into
Open
Conversation
Cache and run `whitaker-installer`, preferring cargo-binstall when it is available and falling back to a locked Cargo build otherwise. Document the action and cover its cache, preferred installation path, and fallback behaviour with deterministic contract tests.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary
WalkthroughChangesWhitaker installation
Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant CargoBinstall
participant Cargo
participant WhitakerInstaller
GitHubActions->>GitHubActions: Restore installer and cargo-binstall caches
GitHubActions->>CargoBinstall: Check cargo binstall availability
CargoBinstall-->>GitHubActions: Report availability
GitHubActions->>CargoBinstall: Install whitaker-installer when available
GitHubActions->>Cargo: Install whitaker-installer as fallback
GitHubActions->>WhitakerInstaller: Install the Whitaker Dylint suite
Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 4 warnings)
✅ Passed checks (14 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Match the repository's aligned Markdown table style so the action guide passes the Markdown lint gate.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Keep coverage-enabled validation from leaving an untracked `.coverage` artefact after the Markdown and spelling gates run.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
buzzybee-df12
marked this pull request as ready for review
July 21, 2026 17:19
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 @.github/actions/install-whitaker/action.yml:
- Around line 28-36: Update the installer setup around the command -v
whitaker-installer check to prepend ${CARGO_HOME:-$HOME/.cargo}/bin to PATH
before resolving or invoking whitaker-installer. Preserve the existing
installation fallback logic and ensure both restored and newly installed
binaries are discoverable by the final whitaker-installer command.
In @.github/actions/install-whitaker/README.md:
- Around line 22-27: Update the Install Whitaker usage example to use the
mandated local action reference `./.github/actions/install-whitaker@v1` instead
of the repository-qualified path, while preserving the surrounding setup steps.
- Around line 16-18: Replace the prose under the Outputs section with the
required outputs table format, including a row explicitly stating that this
action exposes no outputs. Keep the existing README structure and document style
otherwise unchanged.
In @.github/actions/install-whitaker/tests/test_action.py:
- Around line 136-174: Add failure-path coverage alongside
test_installs_with_cargo_binstall_when_available,
test_falls_back_to_cargo_install, and test_reuses_cached_installer by extending
the command stubs to fail cargo binstall, cargo install, and whitaker-installer
independently. Assert each scenario exits non-zero and writes actionable stderr,
while preserving the existing success and cached-installer contracts.
- Around line 102-109: Update the subprocess.run call in the test helper to
explicitly pass check=False, preserving the existing returncode-based assertions
and all other invocation options.
In `@README.md`:
- Line 17: Update the “Install Whitaker” catalogue row to show its published
latest major version instead of “unreleased” once v1 is available; if it remains
unpublished, remove the row. Keep the table limited to published actions and
their latest major versions.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 67485f07-b88d-4695-8922-39400b84b222
📒 Files selected for processing (6)
.github/actions/install-whitaker/CHANGELOG.md.github/actions/install-whitaker/README.md.github/actions/install-whitaker/action.yml.github/actions/install-whitaker/tests/test_action.py.gitignoreREADME.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/polythene(auto-detected)
Comment on lines
+28
to
+36
| if ! command -v whitaker-installer >/dev/null 2>&1; then | ||
| if cargo binstall --version >/dev/null 2>&1; then | ||
| cargo binstall --no-confirm --locked "whitaker-installer@${WHITAKER_INSTALLER_VERSION}" | ||
| else | ||
| echo "cargo-binstall unavailable; building whitaker-installer from crates.io" | ||
| cargo install --locked whitaker-installer --version "${WHITAKER_INSTALLER_VERSION}" | ||
| fi | ||
| fi | ||
| whitaker-installer |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Export Cargo’s bin directory before resolving the installer.
Add ${CARGO_HOME:-$HOME/.cargo}/bin to PATH before command -v and the final invocation. A runner can provide cargo without exposing ~/.cargo/bin, causing both a restored installer and a newly installed installer to remain undiscoverable.
Proposed fix
set -euo pipefail
+ export PATH="${CARGO_HOME:-$HOME/.cargo}/bin:${PATH}"
if ! command -v whitaker-installer >/dev/null 2>&1; then📝 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.
Suggested change
| if ! command -v whitaker-installer >/dev/null 2>&1; then | |
| if cargo binstall --version >/dev/null 2>&1; then | |
| cargo binstall --no-confirm --locked "whitaker-installer@${WHITAKER_INSTALLER_VERSION}" | |
| else | |
| echo "cargo-binstall unavailable; building whitaker-installer from crates.io" | |
| cargo install --locked whitaker-installer --version "${WHITAKER_INSTALLER_VERSION}" | |
| fi | |
| fi | |
| whitaker-installer | |
| set -euo pipefail | |
| export PATH="${CARGO_HOME:-$HOME/.cargo}/bin:${PATH}" | |
| if ! command -v whitaker-installer >/dev/null 2>&1; then | |
| if cargo binstall --version >/dev/null 2>&1; then | |
| cargo binstall --no-confirm --locked "whitaker-installer@${WHITAKER_INSTALLER_VERSION}" | |
| else | |
| echo "cargo-binstall unavailable; building whitaker-installer from crates.io" | |
| cargo install --locked whitaker-installer --version "${WHITAKER_INSTALLER_VERSION}" | |
| fi | |
| fi | |
| whitaker-installer |
🤖 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 @.github/actions/install-whitaker/action.yml around lines 28 - 36, Update the
installer setup around the command -v whitaker-installer check to prepend
${CARGO_HOME:-$HOME/.cargo}/bin to PATH before resolving or invoking
whitaker-installer. Preserve the existing installation fallback logic and ensure
both restored and newly installed binaries are discoverable by the final
whitaker-installer command.
Comment on lines
+16
to
+18
| ## Outputs | ||
|
|
||
| This action has no outputs. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document outputs in the required table format.
Replace the prose statement with an outputs table, including a row that states that no outputs are exposed. As per coding guidelines, each action README must include “input and output tables”.
Triage: [type:docstyle]
🤖 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 @.github/actions/install-whitaker/README.md around lines 16 - 18, Replace the
prose under the Outputs section with the required outputs table format,
including a row explicitly stating that this action exposes no outputs. Keep the
existing README structure and document style otherwise unchanged.
Sources: Coding guidelines, Path instructions
Comment on lines
+22
to
+27
| ```yaml | ||
| - name: Set up Rust | ||
| uses: leynos/shared-actions/.github/actions/setup-rust@v1 | ||
|
|
||
| - name: Install Whitaker | ||
| uses: leynos/shared-actions/.github/actions/install-whitaker@v1 |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the mandated local action reference in the usage example.
Replace the repository-qualified reference with uses: ./.github/actions/install-whitaker@v1. As per coding guidelines, each action README must include “a usage example using uses: ./.github/actions/<name>@<major>”.
Triage: [type:docstyle]
🤖 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 @.github/actions/install-whitaker/README.md around lines 22 - 27, Update the
Install Whitaker usage example to use the mandated local action reference
`./.github/actions/install-whitaker@v1` instead of the repository-qualified
path, while preserving the surrounding setup steps.
Sources: Coding guidelines, Path instructions
Comment on lines
+102
to
+109
| return subprocess.run( # noqa: S603,TID251 - exercise the Bash fragment. | ||
| [bash, "-c", _install_script()], | ||
| cwd=tmp_path, | ||
| env=env, | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=30, | ||
| ) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Declare the intentional non-raising subprocess behaviour.
Set check=False because these tests inspect returncode directly. This resolves the reported Ruff warning without changing the failure assertions.
Proposed fix
capture_output=True,
+ check=False,
text=True,📝 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.
Suggested change
| return subprocess.run( # noqa: S603,TID251 - exercise the Bash fragment. | |
| [bash, "-c", _install_script()], | |
| cwd=tmp_path, | |
| env=env, | |
| capture_output=True, | |
| text=True, | |
| timeout=30, | |
| ) | |
| return subprocess.run( # noqa: S603,TID251 - exercise the Bash fragment. | |
| [bash, "-c", _install_script()], | |
| cwd=tmp_path, | |
| env=env, | |
| capture_output=True, | |
| check=False, | |
| text=True, | |
| timeout=30, | |
| ) |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 102-102: subprocess.run without explicit check argument
Add explicit check=False
(PLW1510)
🤖 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 @.github/actions/install-whitaker/tests/test_action.py around lines 102 -
109, Update the subprocess.run call in the test helper to explicitly pass
check=False, preserving the existing returncode-based assertions and all other
invocation options.
Source: Linters/SAST tools
Comment on lines
+136
to
+174
| def test_installs_with_cargo_binstall_when_available(tmp_path: Path) -> None: | ||
| """cargo-binstall should be preferred when its subcommand is available.""" | ||
| result = _run_install_script(tmp_path, binstall_available=True) | ||
|
|
||
| assert result.returncode == 0, result.stderr | ||
| assert (tmp_path / "cargo.log").read_text(encoding="utf-8").splitlines() == [ | ||
| "binstall --version", | ||
| "binstall --no-confirm --locked whitaker-installer@0.2.6", | ||
| ] | ||
| assert (tmp_path / "installer.log").read_text(encoding="utf-8") == ( | ||
| "suite installed\n" | ||
| ) | ||
|
|
||
|
|
||
| def test_falls_back_to_cargo_install(tmp_path: Path) -> None: | ||
| """Cargo should build whitaker-installer when cargo-binstall is unavailable.""" | ||
| result = _run_install_script(tmp_path, binstall_available=False) | ||
|
|
||
| assert result.returncode == 0, result.stderr | ||
| assert (tmp_path / "cargo.log").read_text(encoding="utf-8").splitlines() == [ | ||
| "binstall --version", | ||
| "install --locked whitaker-installer --version 0.2.6", | ||
| ] | ||
| assert "cargo-binstall unavailable" in result.stdout | ||
|
|
||
|
|
||
| def test_reuses_cached_installer(tmp_path: Path) -> None: | ||
| """A restored installer should avoid both Cargo installation paths.""" | ||
| result = _run_install_script( | ||
| tmp_path, | ||
| binstall_available=False, | ||
| installer_present=True, | ||
| ) | ||
|
|
||
| assert result.returncode == 0, result.stderr | ||
| assert not (tmp_path / "cargo.log").exists() | ||
| assert (tmp_path / "installer.log").read_text(encoding="utf-8") == ( | ||
| "suite installed\n" | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add failure-path contract tests.
Extend the stubs to fail cargo binstall, cargo install, and whitaker-installer, then assert that the action exits non-zero with actionable stderr. Current tests cover only successful acquisition and reuse. As per coding guidelines, action tests “must cover the happy path, common edge cases, and failure modes with clear error messages”.
🤖 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 @.github/actions/install-whitaker/tests/test_action.py around lines 136 -
174, Add failure-path coverage alongside
test_installs_with_cargo_binstall_when_available,
test_falls_back_to_cargo_install, and test_reuses_cached_installer by extending
the command stubs to fail cargo binstall, cargo install, and whitaker-installer
independently. Assert each scenario exits non-zero and writes actionable stderr,
while preserving the existing success and cached-installer contracts.
Sources: Coding guidelines, Learnings
| | Export Cargo metadata | `.github/actions/export-cargo-metadata` | v1 | | ||
| | Export Postgres URL | `.github/actions/export-postgres-url` | v1 | | ||
| | Generate coverage | `.github/actions/generate-coverage` | v1 | | ||
| | Install Whitaker | `.github/actions/install-whitaker` | unreleased | |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Publish the v1 label or omit this unreleased action from the catalogue.
Replace unreleased with the published latest major version once released; otherwise remove the row until publication. The table must list published actions and their latest major versions.
Triage: [type:docstyle]
🧰 Tools
🪛 LanguageTool
[uncategorized] ~17-~17: The official name of this software platform is spelled with a capital “H”.
Context: ... | | Install Whitaker | .github/actions/install-whitaker ...
(GITHUB)
🤖 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 `@README.md` at line 17, Update the “Install Whitaker” catalogue row to show
its published latest major version instead of “unreleased” once v1 is available;
if it remains unpublished, remove the row. Keep the table limited to published
actions and their latest major versions.
Sources: Coding guidelines, Path instructions
This was referenced Jul 21, 2026
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
1 participant
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
This branch adds a reusable
install-whitakercomposite action based on the installation flow used by Frankie CI. It cacheswhitaker-installer, prefers cargo-binstall when available, falls back tocargo install --locked, and installs the Whitaker Dylint suite.The installer version is configurable through
installer-version, with0.2.6as the source-compatible default.Review walkthrough
Validation
make check-fmt: passedmake typecheck: passedmake lint: passedinstall-whitakertests: 4 passedmake test: all four new tests passed within the full suite; the repository-wide command remained non-zero with 1 unrelated failure, 948 passes, 14 skips, and 46 unrelated setup errors. The existing failures were a transient.hypothesis/tmpcopy race in a Linux-packages test and missing repository-root release scripts expected byrelease-to-pypi-uvtests.References
https://lody.ai/leynos/sessions/fb9d4255-76f9-427d-a587-89983e2d88c9