Support immutable nightly binary installation - #135
Conversation
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe release workflow no longer injects ChangesRelease and installation flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Installer as install.sh
participant GitHubAPI as GitHub releases API
participant Asset as nightly release asset
Installer->>GitHubAPI: Request release page
GitHubAPI-->>Installer: Return prerelease objects
Installer->>Installer: Select matching nightly-* tag
Installer->>Asset: Download resolved nightly binary
Asset-->>Installer: Return binary
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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 `@install.sh`:
- Around line 56-85: Update the nightly-release parsing in the version-selection
loop to require the enclosing release record’s prerelease field to be true
before accepting a matching nightly tag. Preserve pagination and empty-result
handling, and add a fixture covering a non-prerelease nightly-shaped tag before
a valid nightly prerelease.
In `@tests/Unit/ApplicationInfoTest.php`:
- Around line 57-61: Remove the eval-based source rewriting from
packagedCommitTakesPrecedenceOverStaleReleaseMetadata(). Test resolveVersion()
through a fixture or class configuration that directly supplies the packaged
commit, or update resolveVersion() to accept that value as a testable parameter,
while preserving the assertion that the packaged commit takes precedence over
stale release metadata.
In `@tests/Unit/Packaging/ConfigurationPackagingTest.php`:
- Line 474: Update the release-workflow assertions in the relevant packaging
test to verify that the `YIIPRESS_COMMIT` override is absent in every form,
including both the YAML environment syntax and command assignments such as
`${GITHUB_SHA}`. Prefer asserting that `YIIPRESS_COMMIT` does not occur anywhere
in the workflow, while preserving the existing workflow validation.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3aee4b96-b88c-49ca-87c4-36f192677242
📒 Files selected for processing (8)
.github/workflows/release.ymldocs/binaries-phar-docker.mdinstall.shroadmap.mdsrc/ApplicationInfo.phptests/Unit/ApplicationInfoTest.phptests/Unit/Packaging/ConfigurationPackagingTest.phptests/Unit/Packaging/InstallerTest.php
There was a problem hiding this comment.
Pull request overview
Adds support for resolving and installing the newest immutable “nightly” binary, and adjusts version reporting so packaged/nightly artifacts can prefer an embedded commit SHA over stale Composer metadata.
Changes:
- Extend
install.shand installer tests to resolveYIIPRESS_VERSION=nightlyvia the GitHub Releases API with pagination. - Update application version resolution logic so packaged builds can report an embedded commit SHA reliably.
- Align release workflow assertions/docs/roadmap with the new nightly + version-reporting behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
install.sh |
Adds nightly resolution via GitHub API before downloading assets. |
tests/Unit/Packaging/InstallerTest.php |
Stubs paginated GitHub API responses and adds nightly installer coverage. |
src/ApplicationInfo.php |
Makes embedded COMMIT take precedence over Composer-provided version/reference. |
tests/Unit/ApplicationInfoTest.php |
Adds a regression test for embedded commit precedence over stale metadata. |
.github/workflows/release.yml |
Stops passing YIIPRESS_COMMIT for tagged releases. |
tests/Unit/Packaging/ConfigurationPackagingTest.php |
Updates assertions to match the release workflow changes. |
docs/binaries-phar-docker.md |
Documents nightly install usage and the updated version-reporting behavior. |
roadmap.md |
Marks “install newest nightly via YIIPRESS_VERSION=nightly” as completed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/Unit/Packaging/InstallerTest.php (2)
79-88: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a second valid nightly candidate to test newest selection.
The page-two fixture has only one
prerelease: truenightly. The assertion at Line 182 can pass without proving newest-candidate selection. Add an older valid nightly afternightly-42-1-abcdef123456.Proposed fixture change
- "[{\"prerelease\":false,\"draft\":false,\"tag_name\":\"nightly-41-1-deadbeef1234\"}," - . "{\"prerelease\":true,\"draft\":false,\"tag_name\":\"nightly-42-1-abcdef123456\"}]\n", + "[{\"prerelease\":false,\"draft\":false,\"tag_name\":\"nightly-40-1-deadbeef1234\"}," + . "{\"prerelease\":true,\"draft\":false,\"tag_name\":\"nightly-42-1-abcdef123456\"}," + . "{\"prerelease\":true,\"draft\":false,\"tag_name\":\"nightly-41-1-deadbeef1234\"}]\n",Also applies to: 174-183
🤖 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/Unit/Packaging/InstallerTest.php` around lines 79 - 88, Add a second valid, older nightly release entry to the page-two fixture in the installer test, placing it after nightly-42-1-abcdef123456 while keeping it prerelease and non-draft. Preserve the existing assertions so the test verifies the newer nightly candidate is selected.
184-196: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAssert that discovery stops after page 2.
The assertions at Lines 186-187 confirm requests for pages 1 and 2, but they do not reject page 3. A regression could continue pagination after finding a match and still pass this test.
Proposed assertion
self::assertStringContainsString('/repos/test/engine/releases?per_page=100&page=2', $curlLog); + self::assertStringNotContainsString( + '/repos/test/engine/releases?per_page=100&page=3', + $curlLog, + );🤖 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/Unit/Packaging/InstallerTest.php` around lines 184 - 196, Extend the curlLog assertions in the installer test to verify that no request is made for the releases endpoint with page=3. Keep the existing positive assertions for pages 1 and 2 and the download URLs unchanged.
🤖 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.
Nitpick comments:
In `@tests/Unit/Packaging/InstallerTest.php`:
- Around line 79-88: Add a second valid, older nightly release entry to the
page-two fixture in the installer test, placing it after
nightly-42-1-abcdef123456 while keeping it prerelease and non-draft. Preserve
the existing assertions so the test verifies the newer nightly candidate is
selected.
- Around line 184-196: Extend the curlLog assertions in the installer test to
verify that no request is made for the releases endpoint with page=3. Keep the
existing positive assertions for pages 1 and 2 and the download URLs unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 42b8308f-ee90-480c-a6d8-8dddbfacdfae
📒 Files selected for processing (5)
install.shsrc/ApplicationInfo.phptests/Unit/ApplicationInfoTest.phptests/Unit/Packaging/ConfigurationPackagingTest.phptests/Unit/Packaging/InstallerTest.php
🚧 Files skipped from review as they are similar to previous changes (4)
- install.sh
- src/ApplicationInfo.php
- tests/Unit/Packaging/ConfigurationPackagingTest.php
- tests/Unit/ApplicationInfoTest.php
# Conflicts: # install.sh # tests/Unit/Packaging/InstallerTest.php
Summary
Tests
make test CLI_ARGS='tests/Unit/ApplicationInfoTest.php'(4 tests, 12 assertions)Summary by CodeRabbit
YIIPRESS_VERSION=nightly.