Resolve python tools off PATH, not just as importable modules
1.1.0 reported an installed ruff as missing and skipped the lint. Its resolution order ended at `python -m <tool>`, guarded by an `import <tool>` probe, but ruff ships as a standalone Rust binary and is never importable. The probe failed, the check was recorded as skipped, and the gate reported a coverage gap that did not exist. This also regressed 1.0.2, which called a bare `ruff` and let PATH resolve it. Restore that path, ordered after the project venv and uv so a project-local tool still wins, and before `python -m` so a binary is found even when a same-named module is not importable. Verified by injecting an unused import into audit-terraform and confirming the guard blocks with `ruff check .`. The new regression test fails against the 1.1.0 resolution order and passes against this one. Tests: 10 passing (bash-guard), 197 (audit-code), 106 (audit-terraform), ruff clean across both skills.
This commit is contained in:
@@ -220,11 +220,24 @@ function pyTool(dir, tool, args) {
|
||||
const venv = join(dir, '.venv', 'bin', tool);
|
||||
if (existsSync(venv)) return [venv, args];
|
||||
if (existsSync(join(dir, 'uv.lock')) && pyprojectHasProject(dir)) return ['uv', ['run', tool, ...args]];
|
||||
// PATH before `python -m`: ruff ships as a standalone binary and is never importable,
|
||||
// so probing for a module would declare an installed ruff missing.
|
||||
if (onPath(tool)) return [tool, args];
|
||||
if (pyModuleAvailable(tool)) return ['python', ['-m', tool, ...args]];
|
||||
skipped.push({ dir, tool });
|
||||
return null;
|
||||
}
|
||||
|
||||
function onPath(tool) {
|
||||
try {
|
||||
execFileSync(tool, ['--version'], { stdio: 'ignore' });
|
||||
return true;
|
||||
} catch (e) {
|
||||
// A non-zero exit still proves the binary exists; only ENOENT means absent.
|
||||
return e.code !== 'ENOENT';
|
||||
}
|
||||
}
|
||||
|
||||
function pyprojectHasProject(dir) {
|
||||
try { return /^\[project\]/m.test(readFileSync(join(dir, 'pyproject.toml'), 'utf8')); }
|
||||
catch { return false; }
|
||||
|
||||
@@ -96,4 +96,11 @@ with tempfile.TemporaryDirectory() as tmp:
|
||||
r = run_guard(bash("jj git push --bookmark main"), cwd=tmp)
|
||||
check("passing nested test allows the push", decision(r) != "deny", f"got {decision(r)!r}")
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
skill = nested_repo(tmp, "def test_passes():\n assert True\n")
|
||||
open(os.path.join(skill, "scripts.py"), "w").write("import os\n")
|
||||
r = run_guard(bash("jj git push --bookmark main"), cwd=tmp)
|
||||
check("lint failure blocks the push", decision(r) == "deny", f"got {decision(r)!r}")
|
||||
check("ruff is resolved off PATH, not skipped", "ruff" in reason(r), f"got {reason(r)!r}")
|
||||
|
||||
sys.exit(1 if failures else 0)
|
||||
|
||||
Reference in New Issue
Block a user