fix(store): gate install_from_url, the last ungated route in - #510
Merged
Merged
Conversation
Sideloading a plugin from a URL validated required manifest fields, warned on version-field and schema problems, and then installed whatever it had found -- never once comparing the core version. With this, all three routes in refuse on the same rule: install_plugin (#431/#433), the git-pull update path (the commit before this one), and now sideloading. The argument for gating it is not the same as for the other two, and worth stating. An automatic store update is something that happens TO a user; a sideload is something they chose. But the floor is not advice about intent -- it is a statement that the plugin cannot run on this core. Letting it through produces exactly the silent PluginState.ERROR at load that the store gate exists to prevent, and someone who pasted a URL is no better placed to diagnose that than someone who pressed Update. Checked after the download, like the others, because the manifest is the only place the floor is written. Placed before the move into plugins/, so the existing `finally` removes the temp tree and nothing half-installed survives a refusal. The reason string is returned to the caller, so the web UI shows the same actionable message the store path already gives. TestSideloadGate pins the three cases the other routes pin: refuses a floor above this core and leaves nothing behind, still allows a compatible plugin (the guard against a gate that refuses everything and passes the first test), and does not block a 2.0.0 floor on a core reporting an untrustworthy version -- the v3.1.0-reports-1.0.0 population, who must not be locked out. TestInstallGate's docstring said this route was still ungated. It no longer is, so it now points at TestSideloadGate rather than describing a gap. Verified the tests bite: removing the check fails the refusal case. Full suite 3723 passed, 6 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Up to standards ✅🟢 Issues
|
ChuckBuilds
added a commit
that referenced
this pull request
Sep 2, 2026
This PR was opened to say B6 was deliberately held. It has since run, so the document said the opposite of the truth -- which is the exact failure it was written to fix: "a plan that misreports which phase it is in is worse than no plan". Merges current main first (53 commits), which brings in #508's corrections to this same file, then replaces the hold with what happened. **Why the hold lifted is worth recording, because the stated gate was never met.** It asked for evidence of 3.2.0 uptake, and that evidence could not arrive: the core updates by `git pull --rebase`, so release-asset counts cannot measure it, and no store-side telemetry exists. What changed is that the risk the gate protected against was closed directly -- the store now refuses a plugin whose floor exceeds the running core on all three routes in: install_plugin (#431/#433), update_plugin's git branch (#508), and install_from_url (#510). A pre-3.2.0 user cannot receive a sunset plugin at all, so they keep the version they run. Refusal replaced the bundled copy, which is what the copy stood in for. Records what shipped (eight plugins, ~5,800 lines, plugins #346/#349/#350/#351) and the two findings worth carrying to the next module: baseball's fallback was the only one holding orchestration logic the core lacked, and two tests had been leaning on the guard -- soccer's stubbed `src` in a way that shadowed the core, so it had been exercising the frozen copy rather than the shipping class since B5. The remaining-work list is replaced too. Its first item was "nothing on the critical path, B6 is waiting on calendar time", which is no longer true. What remains: hardware soaks (with a note to check the rig's display_mode first, or a board in switch mode tells you nothing about the scroll code), cutting 3.3.0 -- not required by B6, whose floors are 3.2.0, but calendar 1.2.3 floors at 3.3.0 and is un-installable until it exists -- and reconsidering the modules held back during the sunset. Keeps the pre-B6 sections as history. The reasoning still applies to the next module; it is just no longer in force for this one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
This was referenced Sep 2, 2026
ChuckBuilds
added a commit
that referenced
this pull request
Sep 2, 2026
…511) #510 shows as merged, but into fix/gate-git-pull-updates -- #508's branch -- rather than main. #508 reached main first, so the sideload gate was left behind on a branch. Same failure as plugins #350/#351, which merged into each other's bases; worth knowing the pattern, because GitHub reports these as MERGED and `gh pr list` shows nothing outstanding. main today has two of the three routes gated: install_plugin (#431/#433) and update_plugin's git branch (#508). install_from_url validates required manifest fields and then installs whatever it found, never comparing the core version. Cherry-picked unchanged from the orphaned branch -- it applies to main with no conflict. TestSideloadGate pins the three cases the other routes pin: refuses a floor above this core leaving nothing behind, still allows a compatible plugin (the guard against a gate that refuses everything), and does not block a 2.0.0 floor on a core reporting an untrustworthy version. Full suite 3725 passed, 6 skipped. Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Stacked on #508 — base it on
mainand retarget once that lands. The diff against #508 is just this.Sideloading a plugin from a URL validated required manifest fields, warned on version-field and schema problems, and then installed whatever it found — never once comparing the core version. With this, all three routes in refuse on the same rule:
install_plugin(#431/#433), the git-pull update path (#508), and now sideloading.Why gate it at all
The argument is genuinely different from the other two, and worth saying out loud. An automatic store update is something that happens to a user; a sideload is something they chose, so "they didn't ask for this" doesn't apply.
But the floor is not advice about intent — it is a statement that the plugin cannot run on this core. Letting it through produces exactly the silent
PluginState.ERRORat load that the store gate exists to prevent, and someone who pasted a URL is no better placed to diagnose that than someone who pressed Update.Placement
Checked after the download, like the others, because the manifest is the only place the floor is written. Placed before the move into
plugins/, so the existingfinallyremoves the temp tree and nothing half-installed survives a refusal.The
reasonstring is returned to the caller rather than only logged, so the web UI shows the same actionable message the store path already gives ("…requires LEDMatrix X, but this system is running Y. Update LEDMatrix first, then install it.").Tests
TestSideloadGatepins the same three cases the other routes pin:2.0.0floor on a core reporting an untrustworthy version — the v3.1.0-reports-1.0.0population, who must not be locked outTestInstallGate's docstring said this route was still ungated. It no longer is, so it now points atTestSideloadGaterather than describing a gap.Verified the tests bite: removing the check fails the refusal case with
assert True is False.Full suite
3723 passed, 6 skipped.🤖 Generated with Claude Code
https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9