Auto-install plugins via env var & tag :latest on all releases#973
Open
bigjoedata wants to merge 1 commit into
Open
Auto-install plugins via env var & tag :latest on all releases#973bigjoedata wants to merge 1 commit into
bigjoedata wants to merge 1 commit into
Conversation
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:
📝 WalkthroughWalkthroughThe release workflow selects Docker ChangesRelease Docker tagging
Plugin auto-installation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant load_plugins
participant urllib_request
participant zipfile_ZipFile
participant user_plugins_directory
load_plugins->>urllib_request: Download configured plugin ZIP
load_plugins->>zipfile_ZipFile: Validate and extract archive
load_plugins->>user_plugins_directory: Install verified plugin root
user_plugins_directory-->>load_plugins: Skip populated target
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: 7
🤖 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/workflows/release.yml:
- Around line 39-41: Update the image-tag generation logic in the release
workflow so the latest tag is emitted only for stable versions; make the version
check branch mutually exclusive, ensuring versions containing a hyphen emit only
the preview tag while stable versions emit only latest.
In `@plugins/__init__.py`:
- Around line 1194-1209: Update the auto-install block around target_dir and
archive extraction to stage the plugin under user_path, require exactly one
extracted plugin root containing plugin.json, and remove any empty placeholder
target before installation. Atomically rename the validated staged directory
into the previously absent target, ensuring failed or interrupted installs
cannot leave a populated target that future runs skip, and only log success
after the rename completes.
- Line 1198: Update the auto-install logging statement in the plugin
installation flow to stop emitting the complete URL. Log only plugin_name, or
sanitize url by removing credentials, query parameters, and other sensitive
access details before passing it to log.info.
- Around line 1186-1192: Update the URL-versus-repository detection in the
plugin auto-install flow to accept URL inputs only when they use the exact https
scheme and include a non-empty host; do not treat http:// or malformed URLs as
valid. Preserve repository shorthand handling for non-URL inputs, and derive
plugin_name from validated HTTPS URLs through the existing logic.
- Around line 1186-1189: Update the repository handling in the auto-install flow
around the item-to-archive URL construction to avoid hardcoding the main branch.
Support an explicitly supplied ref when present; otherwise resolve the
repository’s default branch before constructing the archive URL, while
preserving plugin_name derivation from the repository path.
- Line 1202: Update the auto-install download flow around
urllib.request.urlretrieve to avoid unbounded startup downloads: use a
timeout-enabled request, validate the HTTP response, and stream the archive
while enforcing a maximum download size before writing to zip_path. Preserve the
existing FEEDBACK_AUTO_INSTALL_PLUGINS behavior for valid, bounded downloads.
- Around line 1203-1205: Update the ZIP extraction flow around
zip_ref.extractall() to validate every archive member first: resolve each target
path beneath extract_path to prevent traversal, reject symlink and special-file
entries, and enforce a maximum cumulative uncompressed size. Only call
extractall() after all members pass validation.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d0830c5-18e0-4ae0-92da-35e82d1e098c
📒 Files selected for processing (2)
.github/workflows/release.ymlplugins/__init__.py
bigjoedata
force-pushed
the
feature/auto-install-and-latest-tag
branch
from
July 15, 2026 04:26
ffb4a62 to
9efed05
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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/workflows/release.yml:
- Around line 39-43: Add a new entry to the [Unreleased] section of CHANGELOG.md
documenting the release Docker tag behavior change implemented by the workflow’s
version-based preview/latest tagging logic.
In `@plugins/__init__.py`:
- Around line 1174-1175: Update CHANGELOG.md’s [Unreleased] section to document
the automatic downloading of custom plugins configured through
FEEDBACK_AUTO_INSTALL_PLUGINS, using the changelog’s existing format.
- Around line 1200-1220: Validate plugin_name after both URL and
repository-shorthand parsing, before constructing target_dir. Reject empty names
and the unsafe values "." or "..", log the invalid input, and continue without
creating or using a target directory.
- Around line 1287-1295: Update the installation flow around root_dir and the
atomic shutil.move to read and parse root_dir / "plugin.json" before publishing.
Require the parsed manifest to be a JSON object with a non-empty string id, and
reject invalid or malformed manifests before removing the zip or moving the
target directory.
- Around line 1266-1269: Update the ZIP validation loop over zip_ref.infolist()
to enforce a maximum number of archive members before extraction, rejecting
archives that exceed the configured cap while preserving the existing cumulative
uncompressed-size check.
- Line 1182: Remove the function-local import os from load_plugins and rely on
the module-level import instead, so the earlier os access at line 1170 does not
raise UnboundLocalError regardless of auto-installation settings.
- Around line 1239-1243: Update the archive download flow around the
urllib.request.Request and urlopen call to use a redirect handler that rejects
any redirect whose Location is not HTTPS before following it. Preserve HTTPS
redirects and the existing timeout and response handling, and ensure the handler
is installed on the opener used for this fetch.
- Around line 1174-1176: The auto-install flow around
FEEDBACK_AUTO_INSTALL_PLUGINS currently does nothing when no plugin directory
variable is configured. Make FEEDBACK_AUTO_INSTALL_PLUGINS sufficient to enable
installation by selecting a writable default target directory, or fail fast with
a clear configuration error when no target is available; preserve the existing
behavior when FEEDBACK_PLUGINS_DIR or SLOPSMITH_PLUGINS_DIR is set.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 584c951f-1a86-4962-a0ff-71c6e9f8a44f
📒 Files selected for processing (2)
.github/workflows/release.ymlplugins/__init__.py
| import shutil | ||
| import tempfile | ||
| from urllib.parse import urlparse | ||
| import os |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Remove the function-local import os; it crashes every plugin load.
Python treats os as local throughout load_plugins, so Line 1170 accesses it before assignment and raises UnboundLocalError, even when auto-installation is disabled.
Proposed fix
- import os📝 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
| import os |
🤖 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 `@plugins/__init__.py` at line 1182, Remove the function-local import os from
load_plugins and rely on the module-level import instead, so the earlier os
access at line 1170 does not raise UnboundLocalError regardless of
auto-installation settings.
bigjoedata
force-pushed
the
feature/auto-install-and-latest-tag
branch
from
July 15, 2026 04:43
9efed05 to
bd78276
Compare
Contributor
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 `@plugins/__init__.py`:
- Around line 1187-1193: Redact plugin URLs across all failure logging paths in
SafeRedirectHandler.redirect_request and the related validation/download
handling at the referenced locations. Replace raw item, newurl, and other URL
values in warnings or propagated errors with the derived plugin name or a
sanitized URL removing userinfo, query, and fragment, while preserving the
existing failure behavior.
- Around line 1238-1240: The target reservation in load_plugins() must be atomic
across concurrent or re-entrant passes. Replace the check-then-shutil.move flow
around target_dir with an exclusive per-target lock or same-filesystem rename
that fails if the destination is created concurrently; when that occurs, treat a
populated target_dir as already installed and skip it, including the
corresponding logic at the other installation site.
- Around line 1195-1196: Keep target preparation in the per-plugin auto-install
failure isolation: move user_path.mkdir and target_dir.iterdir operations inside
the guarded OSError handling. Explicitly detect and handle target_dir file or
symlink collisions, logging the failure and continuing with bundled plugin
loading instead of aborting all plugins.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 87583e05-c63d-4690-9d36-815723424d8f
📒 Files selected for processing (3)
.github/workflows/release.ymlCHANGELOG.mdplugins/__init__.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/release.yml
bigjoedata
force-pushed
the
feature/auto-install-and-latest-tag
branch
from
July 15, 2026 04:53
bd78276 to
fa0355d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/__init__.py (1)
1356-1362: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider preserving a traceback for unexpected auto-install failures.
log.warning("Failed to auto-install plugin %r: %s", plugin_name, err_msg)only logs the exception's string message, with no stack trace. That's fine for expectedValueErrors raised by the validation code above, but for a genuine bug (AttributeError,TypeError, etc.) this makes root-causing failures from server logs much harder, unlike the route-loading failure path elsewhere in this function which useslog.exception(...).♻️ Suggested tweak
- log.warning("Failed to auto-install plugin %r: %s", plugin_name, err_msg) + log.warning("Failed to auto-install plugin %r: %s", plugin_name, err_msg, exc_info=not isinstance(e, ValueError))🤖 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 `@plugins/__init__.py` around lines 1356 - 1362, Update the auto-install exception handler around the plugin installation flow to preserve traceback details for unexpected failures, matching the existing route-loading failure path’s use of log.exception. Retain URL sanitization and the current warning context, while ensuring expected validation errors remain handled appropriately.
🤖 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 `@plugins/__init__.py`:
- Around line 1320-1342: Remove the function-local import json from
load_plugins() and reuse the existing module-level json import. Keep the
manifest parsing with json.load unchanged so later json.loads(...) calls cannot
be treated as referencing an uninitialized local variable.
---
Nitpick comments:
In `@plugins/__init__.py`:
- Around line 1356-1362: Update the auto-install exception handler around the
plugin installation flow to preserve traceback details for unexpected failures,
matching the existing route-loading failure path’s use of log.exception. Retain
URL sanitization and the current warning context, while ensuring expected
validation errors remain handled appropriately.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dc7f22df-e1e8-4ad7-bd52-7da618f7fc4e
📒 Files selected for processing (3)
.github/workflows/release.ymlCHANGELOG.mdplugins/__init__.py
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/release.yml
- CHANGELOG.md
bigjoedata
force-pushed
the
feature/auto-install-and-latest-tag
branch
from
July 15, 2026 06:40
fa0355d to
d7355c7
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@plugins/__init__.py`:
- Around line 1209-1243: Update the non-URL repository handling in the
auto_plugins loop to build GitHub archive URLs with the explicit refs path,
using refs/heads for branch-style references or the appropriate refs/tags form
for tags, so refs containing slashes such as release/2.0 resolve correctly. Keep
the existing repository shorthand parsing and plugin-name derivation unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 317fc924-0b7d-400d-a335-c080bf4d0c6b
📒 Files selected for processing (3)
.github/workflows/release.ymlCHANGELOG.mdplugins/__init__.py
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/release.yml
- CHANGELOG.md
bigjoedata
force-pushed
the
feature/auto-install-and-latest-tag
branch
from
July 15, 2026 11:25
d7355c7 to
0f42b90
Compare
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.
This PR introduces two features:
Summary by CodeRabbit
:previewfor version tags containing-, and:latestfor stable versions (kept mutually exclusive).FEEDBACK_AUTO_INSTALL_PLUGINS; if no plugin directory is provided, it defaults toCONFIG_DIR/plugins.