Fix the one-click installer finding no assets in any release - #6
Conversation
ExtractAssets ended the "assets" array at the first "]" in the payload. Every release built by the workflow is uploaded by "github-actions[bot]", and that bracket sits inside a string a few hundred characters into the first asset, so the array body was truncated mid-object, no asset survived the brace matcher, and the installer refused every release with "has no SqlPilot ZIP asset attached". Verified against the live v1.0.0 and v1.1.1 payloads. Scan the array with a depth counter that skips over string literals instead of regex-matching it. Confirmed end to end: the installer now downloads and installs into SSMS 18, 20 and 22. The self-update banner was unaffected because it only reads scalars. Adds tests/SqlPilot.Installer.Tests to cover the parsing directly -- a silent failure there means the installer downloads nothing, which is how this shipped.
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe installer now scans GitHub release asset arrays without stopping at ChangesInstaller asset parsing
Priority: ⚪ Not assessed Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (4 skipped: 4 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
1 participant
The one-click installer has never been able to install any release. It fails with
Release vX.Y.Z has no SqlPilot ZIP asset attached.while the release plainly has the ZIP.Cause
GitHubReleaseClient.ExtractAssetsfound the"assets"array with a non-greedy regex that ended at the first]. Every release built by the workflow is uploaded bygithub-actions[bot], and that bracket sits inside a string a few hundred characters into the first asset object — so the array body was cut off mid-object, the brace matcher found zero complete objects, andInstallEnginehad nothing to download. Reproduced against the live v1.0.0 and v1.1.1 payloads.GitHubReleaseClient.csis byte-identical to the v1.1.1 tag, so every shipped installer has this defect.The self-update banner kept working because it only reads scalar fields.
Fix
Scan the array with a depth counter that skips over string literals instead of regex-matching it. Verified end to end: the real installer now downloads the v1.0.0 payload and installs into SSMS 18, 20 and 22.
Tests
New
tests/SqlPilot.Installer.Tests(xUnit, FluentAssertions pinned<8.0like the core tests) covering thegithub-actions[bot]case, URL/size extraction, picking the.zipover the.vsixthat sorts first in v1.1.1, empty assets, and not reading past the end of the array. With the old algorithm restored, 4 of the 5 fail — the one that passes is the empty-assets case, which genuinely worked before. Added toSqlPilot.slnso CI builds it.Summary by CodeRabbit
Bug Fixes
Tests
Documentation