Skip to content

feat: rename NullTool to ShellTool and add install_script support#2691

Draft
noahd1 wants to merge 1 commit intomainfrom
rename-null-tool-to-shell-tool-with-install-script
Draft

feat: rename NullTool to ShellTool and add install_script support#2691
noahd1 wants to merge 1 commit intomainfrom
rename-null-tool-to-shell-tool-with-install-script

Conversation

@noahd1
Copy link
Member

@noahd1 noahd1 commented Feb 12, 2026

Summary

  • Rename NullTool to ShellTool and add install_script field to PluginDef
  • When install_script is set, ShellTool creates the tool directory and runs the script via sh during tool setup; when unset, preserves existing no-op behavior
  • Install script paths from source plugin.toml files are resolved to absolute paths during source parsing; relative paths in inline qlty.toml definitions are resolved against the project root at install time

Test plan

  • Unit tests for ShellTool: setup with install script, no-op without script, failing script, idempotent setup
  • Integration test (install_script) demonstrating a plugin that uses an install script to create a checker used by the driver
  • cargo check passes
  • All existing unit tests pass

🤖 Generated with Claude Code

Rename NullTool to ShellTool and add an install_script field to
PluginDef so that plugins without a runtime, GitHub release, or
download can run a shell script during tool setup.

When install_script is set, ShellTool creates the tool directory and
runs the script via `sh`. The script runs with cwd set to the tool
install directory. When install_script is not set, the existing no-op
behavior is preserved.

Install script paths from source plugin.toml files are resolved to
absolute paths during source parsing. Relative paths in inline
qlty.toml definitions are resolved against the project root at
install time.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Member

@brynary brynary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff! Added comments

pub tab_column_width: usize,

#[serde(default)]
pub install_script: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is on PluginDef, I think it implies that any plugin (including non-ShellTool-based) can define an install_script -- which probably makes sense as a useful hook but we would need to define the semantics / ordering behavior.

To make sure we don't paint ourselves into a corner, we should probably map out all the hooks we intend to have and how they fit together

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we could block install_script for any non-ShellTool on the basis that our regular installation is the one true install (e.g. npm install and force those plugins to use a before_install_script or after_install_script as hooks to be clear what order things run in). That may be less prone to plugin author errors.


Plugin Result Targets Time Debug File
greeter Success 1 target [..]s .qlty/out/invoke-[..].yaml
greeter Success 1 target [..]s .qlty/out/invoke-[..].yaml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the purposes of integration testing it would be nice if we can strength the tests...

Overall, we could use more visibility into the install process. This just shows data about what happens post-install during runtime. I think this is a pre-existing deficiency.

The debug files (invocation files) are the gold standard, so ideally we would write out a debug file for the install_script and be able to assert against it (or snapshot it)

#!/bin/sh
echo '#!/bin/sh' > greet.sh
echo 'grep -q "hello" "$1" && exit 0 || exit 1' >> greet.sh
chmod +x greet.sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

config_version = "0"

[plugins.definitions.greeter]
file_types = ["shell"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works because shell will cause it to self-lint but is a bit obscure. Might warrant a comment.


[[plugin]]
name = "greeter"
version = "1.0.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think version is optional but we should confirm. Certainly it's not doing anything here.

if let Some(ref script_path) = self.plugin.install_script {
let script_path =
std::fs::canonicalize(script_path).unwrap_or_else(|_| PathBuf::from(script_path));
self.run_command(duct::cmd!("sh", script_path))?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this logic is lifted from elsewhere it would make want to think about extracting duplication to avoid subtle bugs later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants