From 121c10ff53208f2ea1db402e69a66f84e536c0d8 Mon Sep 17 00:00:00 2001 From: Andrei Lavrenov Date: Mon, 1 Jun 2026 22:36:46 +0200 Subject: [PATCH 1/2] fix: anchor exclude dirs to absolute paths so bandit honors --exclude MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit collect_exclude_dirs() documents itself as returning "absolute paths" but returned str(scan_root / p), which is relative whenever scan_root is. The Python security detector derives scan_root from os.path.commonpath of the file list (scan_root_from_files), so it is Path(".") for any scan launched with a relative --path. bandit matches --exclude against the absolute paths it walks, so a relative entry like ".worktrees" matched nothing and bandit re-scanned excluded directories — producing thousands of phantom B101 "assert detected" findings from worktree copies of a repo, even though ".worktrees" was in the exclude config. The file-discovery detectors were unaffected (they filter an already-relative file list via matches_exclusion's path-component match), and ruff tolerates relative excludes; only bandit, which receives a directory root and does its own walk, surfaced the bug. Anchor a relative scan_root to its absolute form (matching the .resolve() bandit already applies to its own scan target). Already-absolute roots are left byte-stable, so existing callers and the test_returns_absolute_paths expectation are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- desloppify/base/discovery/source.py | 7 +++++++ .../tests/detectors/test_external_adapters.py | 17 +++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/desloppify/base/discovery/source.py b/desloppify/base/discovery/source.py index 25e2ce934..d6da5f743 100644 --- a/desloppify/base/discovery/source.py +++ b/desloppify/base/discovery/source.py @@ -149,6 +149,13 @@ def collect_exclude_dirs( if "*" not in pat: patterns.add(pat) patterns.update(p for p in resolved_exclusions if p and "*" not in p) + # External tools (e.g. bandit) match --exclude against absolute walked paths, + # so a relative scan_root — commonpath is often Path(".") — would make every + # exclude silently match nothing. Anchor relative roots to an absolute path + # (matching the .resolve() bandit applies to its own scan target); leave + # already-absolute roots byte-stable so callers' paths are unchanged. + if not scan_root.is_absolute(): + scan_root = scan_root.resolve() return [str(scan_root / p) for p in sorted(patterns) if p] diff --git a/desloppify/tests/detectors/test_external_adapters.py b/desloppify/tests/detectors/test_external_adapters.py index eb5aec350..cd0c44f87 100644 --- a/desloppify/tests/detectors/test_external_adapters.py +++ b/desloppify/tests/detectors/test_external_adapters.py @@ -1010,3 +1010,20 @@ def test_deduplicates(self, tmp_path): result = collect_exclude_dirs(tmp_path) node_entries = [p for p in result if p.endswith("/node_modules")] assert len(node_entries) == 1 + + def test_relative_scan_root_yields_absolute_paths(self): + """A relative scan_root must still yield absolute exclude dirs. + + ``scan_root_from_files`` derives the root from ``os.path.commonpath``, + which is relative (often ``Path('.')``) when scanning with a relative + ``--path``. bandit matches ``--exclude`` against *absolute* walked paths, + so a relative entry like ``.worktrees`` silently excludes nothing — the + bug behind ~2000 phantom B101 findings from worktree copies. + """ + with patch( + "desloppify.base.discovery.source.get_exclusions", + return_value=(".worktrees",), + ): + result = collect_exclude_dirs(Path(".")) + assert all(Path(p).is_absolute() for p in result), result + assert any(p.endswith("/.worktrees") for p in result), result From aa6bce54e124354e5f2e6eed37a4dd0f285c3f5b Mon Sep 17 00:00:00 2001 From: Andrei Lavrenov Date: Thu, 28 May 2026 16:51:54 +0200 Subject: [PATCH 2/2] chore(ci): unblock CI by gating bash tests on tree-sitter; sort review prompt glob MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## The problem CI has been red on `main` since 2026-04-06, and on every PR since, because of 5 pre-existing test failures that are unrelated to feature work. They block green builds without any way to fix them inside a feature PR. Two distinct root causes: 1. Three bash tests in `desloppify/tests/lang/common/test_bash_unused_imports.py` call `detect_unused_imports`, which parses via tree-sitter. When `tree-sitter-language-pack` is not installed (the case in the `tests-core` job, but not `tests-full`), the function returns `[]`. The tests then assert specific findings and fail. 2. Two review tests in `desloppify/tests/review/test_review_commands.py` and its integration counterpart use `list(runs_dir.glob("*/prompts/batch-*.md"))` without sorting. The glob order is filesystem-dependent: on macOS it happens to return `batch-1.md` (the batch with `historical_issue_focus`) first, on Linux it returns `batch-2.md` first. The assertion `"Previously flagged issues" in prompt_text` then fails on Linux because that string only appears in `batch-1.md`. ## The solution Two small fixes: 1. Gate the bash test file on tree-sitter availability with the existing pattern from `desloppify/tests/lang/common/test_treesitter.py`: ```python pytestmark = pytest.mark.skipif( not is_available(), reason="tree-sitter-language-pack not installed", ) ``` Without tree-sitter the tests skip cleanly; with it they pass as before. 2. Wrap the review test's glob in `sorted(...)`: ```python prompt_files = sorted(runs_dir.glob("*/prompts/batch-*.md")) ``` This makes the test deterministic across operating systems. `batch-1.md` always sorts first; the assertion that depends on its content is reliable. ## Why this is in the same PR These two test fixes are independent of the exclude-dir anchoring in this PR (#616) — they are pre-existing `main` failures that could be cherry-picked back to `main` separately. Bundling them here lets the PR actually go green without waiting for a separate maintenance PR. The fix was originally authored on the framework-support branch; see the cherry-pick trailer below. (cherry picked from commit 9baba25e494b8638d6883dabd409c7d7c571283b) Co-Authored-By: Claude Opus 4.8 (1M context) --- desloppify/tests/lang/common/test_bash_unused_imports.py | 8 ++++++++ desloppify/tests/review/review_commands_cases.py | 5 ++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/desloppify/tests/lang/common/test_bash_unused_imports.py b/desloppify/tests/lang/common/test_bash_unused_imports.py index c74dee941..d41cba215 100644 --- a/desloppify/tests/lang/common/test_bash_unused_imports.py +++ b/desloppify/tests/lang/common/test_bash_unused_imports.py @@ -4,6 +4,14 @@ import textwrap +import pytest + +from desloppify.languages._framework.treesitter import is_available + +pytestmark = pytest.mark.skipif( + not is_available(), reason="tree-sitter-language-pack not installed" +) + def _detect(tmp_path, contents: str): from desloppify.languages._framework.treesitter.analysis.unused_imports import ( diff --git a/desloppify/tests/review/review_commands_cases.py b/desloppify/tests/review/review_commands_cases.py index 0158e4fd8..2b9c0c2e1 100644 --- a/desloppify/tests/review/review_commands_cases.py +++ b/desloppify/tests/review/review_commands_cases.py @@ -873,7 +873,10 @@ def test_do_run_batches_dry_run_generates_packet_and_prompts( assert len(packet_files) == 1 blind_packet = tmp_path / ".desloppify" / "review_packet_blind.json" assert blind_packet.exists() - prompt_files = list(runs_dir.glob("*/prompts/batch-*.md")) + # Sort: prompt files are named batch-1.md, batch-2.md; glob order is + # filesystem-dependent (differs Linux vs macOS) and the assertions + # below assume batch-1 (high_level_elegance, with historical_focus) first. + prompt_files = sorted(runs_dir.glob("*/prompts/batch-*.md")) assert len(prompt_files) == 2 prompt_text = prompt_files[0].read_text() assert "Blind packet:" in prompt_text