-
Notifications
You must be signed in to change notification settings - Fork 7
Basic plugin system allowing hooks #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@bstancil can you help check |
|
Quick notes about the superpowers claude skills plugin:
here's the current patch to get claude consider skills |
b4f7775 to
7036672
Compare
…db_type extraction
…ontainer_filename
…owers before we have proper hooks for post-agent-install Also have git otherwise Claude get pretty confused.
Remove the --use-mcp CLI flag and use_mcp parameter throughout the codebase. The used_mcp field in trial results is now derived from whether the dbt-mcp plugin actually ran during the trial, determined by PluginRegistry.did_plugin_run(). Changes: - Remove --use-mcp from both CLI commands (ade run and run_harness.py) - Remove use_mcp parameter from Harness and AbstractInstalledAgent - Add plugins_run tracking set to PluginRegistry - Add did_plugin_run() method to check if specific plugin executed - Update trial result creation to derive used_mcp from plugin execution - Add validation to error out when non-existing plugins are specified 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The SetupOrchestrator created during trial execution was missing the enabled_plugins parameter, causing plugins to never run. Store enabled_plugins as instance variable and pass to both orchestrator instantiations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
7c0c2d6 to
411dc9a
Compare
bstancil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aiight, finally catching up to this. Three comments:
-
Nitpick - for the CLI, i'd prefer the convention of spaces between params. can we do that, where it'd be
--plugins dbt-mcp superpowers -
For the logging, since this is still early, I'm ok breaking the current logging fuction so that there's not a "used_mcp" value and instead there's a "plugins" value, and that's a list of the plugins that got used. So you could derive the dbt MCP one from that.
-
Passing the plugins as a param in the setup orchestrator feels like wrong pattern here. all the other details like db, trial, etc, are handed via the run variant and the variant config. It seems like we should set the plugins that are used in that way, and then just reference that in the same way that we reference the other run params.
|
@clkao - We're getting closer to getting more folks involved in this, so wanted to see where you are on this and the plugin approach? I've fully migrated things over to the CLI, so this should be a bit simpler now. But there might be more people poking around after this (https://www.getdbt.com/resources/webinars/analytics-data-engineer-bench), as a heads up. |
Add a basic plugin system that allows hooking into pre/post setup and agent runs. Closes #39.
You can try with ade run ... --plugins superpowers
follow up once this is merged:
For reference: claude conversation for this PR