feat(integrations): external plugin sources (git/local) in plugins:#44
Open
thad0ctor wants to merge 6 commits into
Open
feat(integrations): external plugin sources (git/local) in plugins:#44thad0ctor wants to merge 6 commits into
plugins:#44thad0ctor wants to merge 6 commits into
Conversation
Allow each `plugins:` entry to be either a dotted class path (unchanged) or a mapping that points at an external git repo or local directory. Axolotl clones/resolves the source into a self-ignoring cache dir, adds it to sys.path, optionally installs the plugin + its deps, then loads it by dotted class path — leaving the Axolotl checkout and install untouched. - PluginSpec schema (cls/source/ref/subdir/pip_install/update) + plugin_cache_dir - provisioning.py: git clone/update, local resolve, filelock-guarded for multi-process launches, pip editable/requirements install modes - normalize plugins to list[str] before validation so no downstream consumer changes (registration loops, spectrum args, builders) - cache dir self-ignores; .axolotl_plugins/ added to repo .gitignore - docs: custom_integrations "External plugin sources" section + agent doc - tests: 14 cases incl. a real local git-clone-and-load
|
Worried about impact? Review this PR in Change Stack to explore blast radius before you approve or request changes. Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdd external plugin source support to Axolotl training configs: users specify plugins via simple dotted class paths or structured specs with git/local sources, optional installation modes, and subdirectory selection. The implementation includes git cloning with file-locked caching, pip install dispatch, and normalized config resolution; comprehensive documentation and test suite accompany the feature. ChangesExternal Plugin Provisioning Feature
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 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 `@tests/integrations/test_plugin_provisioning.py`:
- Around line 17-20: The call to subprocess.run in the helper function _git uses
a partial executable "git" which triggers security lint S607; change _git to
resolve the absolute git binary with shutil.which("git") and pass that absolute
path as the first element of the command list to subprocess.run, and if
shutil.which returns None raise a clear RuntimeError (or pytest.skip) so tests
fail fast rather than invoking a partial path.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3af7fdb6-2fb3-4354-ab4d-f84c5305aff7
📒 Files selected for processing (10)
.gitignoreAGENTS.mddocs/agents/plugins.mddocs/custom_integrations.qmdpyproject.tomlsrc/axolotl/cli/agent_docs/__init__.pysrc/axolotl/cli/config.pysrc/axolotl/integrations/provisioning.pysrc/axolotl/utils/schemas/config.pytests/integrations/test_plugin_provisioning.py
|
📖 Documentation Preview: Deployed on Netlify from commit 079f8a8 |
Use shutil.which to pass an absolute git executable to subprocess.run in the test helper, skipping the test if git is unavailable. Addresses CodeRabbit review on PR #44.
…dir traversal Addresses agent review of PR #44: - axolotl.utils.config.prepare_plugins (public, used by tests + docs) now also calls provision_plugins, so external/dict plugin entries work through that entry point and not only axolotl.cli.config. Previously a dict entry reaching it crashed in PluginManager.register (dict.rsplit). - clamp PluginSpec.subdir to within the source root; reject '..'/absolute subdir that would inject an arbitrary directory onto sys.path. - tests: utils.config provisioning path, relative + absolute subdir escape.
… clone Second-round agent review: - git fetch was followed by a no-op checkout, so update: true never advanced an already-checked-out branch (default branch or branch ref). Fast-forward to the fetched remote tip after fetching. - only attempt the fast-forward when HEAD is on a branch; a tag/SHA ref leaves HEAD detached and immutable, so there is nothing to advance (avoids a spurious, swallowed 'not on a branch' error). - tests: update advances the default branch; update with a tag ref stays pinned and does not raise.
cls is now optional. When a source is given without cls, Axolotl imports the
package(s) under the source (or its subdir) and uses the single BasePlugin
subclass found, so a config can be just:
plugins:
- source: https://github.com/org/my-plugin.git
ref: v1.0.0
Convention for discoverable plugins: an importable package whose top-level
__init__ exports exactly one BasePlugin subclass. Discovery skips tests/, docs/,
examples/, build/, dist/; zero or multiple matches raise and ask for an explicit
cls (which remains the override). A PluginSpec now requires at least one of
cls/source.
- tests: discovery (root + subdir), multiple/zero-match errors, tests-dir skip,
cls-or-source validation
- docs: convention + discovery in custom_integrations.qmd and agent plugins doc
cls now accepts a list, so several plugin classes can be loaded from a single
source (cloned once) without repeating source/ref:
plugins:
- source: https://github.com/org/multi-plugin.git
ref: v1.0.0
cls: [multi_pkg.AlphaPlugin, multi_pkg.BetaPlugin]
String -> one plugin; list -> several; empty list/omitted -> discover the one.
- tests: cls list (alone + mixed with string entries), empty-list falls back to
discovery, validation accepts a list
- docs: multi-plugin example + cls field note in both docs
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
Adds the ability to load a plugin straight from an external git repo or local directory declared in the config, without modifying the Axolotl checkout or install. Each
plugins:entry may now be either a dotted class path (unchanged) or a mapping describing where the plugin lives.Axolotl clones/resolves the source into a self-ignoring cache dir (
./.axolotl_plugins/by default, override viaplugin_cache_dir:/AXOLOTL_PLUGIN_CACHE_DIR), adds it tosys.path, optionally installs the plugin + its deps, then loads it by dotted class path.Design
PluginSpecschema:cls/source/ref/subdir/pip_install/update; new top-levelplugin_cache_dir. Field widened tolist[str | PluginSpec].provisioning.py: git clone/update, local resolve,filelock-guarded so multi-process / multi-GPU launches sharing one cache don't race;pip_installsupportseditable(pip install -e, falling back to requirements if not a package) andrequirements(pip install -r).provision_plugins()rewritescfg["plugins"]to a flatlist[str]before validation, so every downstream consumer (the registration loops,spectrum/args.py's substring check, the builders) is untouched. Backward compatible..axolotl_plugins/also added to repo.gitignore.filelockdeclared inpyproject.toml(now a direct import).pip_installonly ever touches the plugin + its deps — never the Axolotl install.Docs
docs/custom_integrations.qmd: new "External plugin sources" section (field table, cache dir, security note).docs/agents/plugins.md: new agent reference + registeredpluginstopic (axolotl agent-docs plugins) +AGENTS.mdentry.config-reference.qmdis auto-generated from the schema and picks up the new fields.Tests
tests/integrations/test_plugin_provisioning.py— 14 cases: str-noop, local source, subdir, mixed entries, missing path, cache self-ignore, env override, a real local git clone-and-load + idempotent reuse, andpip_installdispatch (mocked).Manual verification
Validated end-to-end against a real example plugin (config args + a
TrainerCallback) cloned from a live GitHub repo:https://github.com/thad0ctor/axolotl-example-plugin — reviewers can point a config at it to exercise this.
Scope note
This loads self-contained plugins that hook in only through the public
BasePluginAPI. Plugins that require edits to Axolotl core are out of scope by design.Summary by CodeRabbit
New Features
Documentation
Tests
Chores