diff --git a/SKILL-QUICK.md b/SKILL-QUICK.md index f7944813..1fa77d35 100755 --- a/SKILL-QUICK.md +++ b/SKILL-QUICK.md @@ -32,6 +32,7 @@ $CLI list --limit 5 --offset 10 --format compact # → paginated + co | `--limit N` / `--offset N` | Pagination on list-type commands (`list`, `search`, `trace`, `symbols`, `outline`). Default limit=20 (issue #17). `--top N` is an alias for `--limit N --offset 0` | | `--deep` | Enable LSP-enhanced deep analysis (requires language server; check with `lsp-status`) | | `--db-path PATH` | Custom SQLite database path (default: `.codelens/codelens.db`) | +| `--diff-base REF` | Git ref (branch/tag/SHA/`HEAD~1`) to diff against. Only findings from files changed relative to REF are reported. Empty diff → early exit. Useful for CI PR checks. Works on all analysis commands (issue #157) | ### Lite Mode Per Command diff --git a/SKILL.md b/SKILL.md index 77b8be13..5ccdfd59 100755 --- a/SKILL.md +++ b/SKILL.md @@ -336,5 +336,13 @@ CodeLens v8.0+ adds 7 major capability pillars over v7.x: 5. **Cross-File Dataflow Engine** (`dataflow` v2) — Workspace-wide call graph with import resolution (`from/import`, `require` destructuring) and bidirectional taint propagation. 6. **OWASP Top 10 + Compliance Mapping** — 89 rules total (A01-A10 + PCI-DSS requirements 1-12 + HIPAA 45 CFR § 164.312). 7. **CI/CD Quality Gate** (`check` command) — Exits non-zero on failure, SARIF output for GitHub Advanced Security / VS Code. +8. **Diff-Scoped Analysis** (`--diff-base REF` flag, issue #157) — Global flag that restricts any analysis command to only report findings from files changed relative to a git ref (branch/tag/SHA/`HEAD~1`). Empty diff → early exit with clear message. Invalid ref → clear error + non-zero exit. Useful for CI PR checks where only NEW findings introduced by the PR should fail the gate. + +```bash +# CI PR check — only findings from files changed vs main +codelens check . --diff-base origin/main --format sarif > codelens.sarif +codelens taint . --diff-base origin/main +codelens secrets . --diff-base HEAD~1 +``` v8.1 follows up with F1 benchmark improvements (avg F1 0.803 → 0.872), circular engine depth fixes (F1 0.667 → 1.000), dead-code engine fixes (F1 0.800 → 0.952), and AST taint depth enhancements (return-value propagation, scope-hierarchical TaintState, branch condition refinement). diff --git a/scripts/codelens.py b/scripts/codelens.py index 437d3f79..1c88a516 100755 --- a/scripts/codelens.py +++ b/scripts/codelens.py @@ -907,6 +907,13 @@ def main(): sub.add_argument("--db-path", default=None, metavar="PATH", help="Custom path for SQLite database file") + # Issue #157: --diff-base on every subparser so it works both + # before and after the subcommand (matches --db-path / --format pattern). + if "diff_base" not in existing_dests: + sub.add_argument("--diff-base", default=None, metavar="REF", + help="Git ref to diff against — only findings from " + "changed files are reported (issue #157)") + # Global format option (works before subcommand) # Default: "ai" if CODELENS_AI_MODE is set (for AI consumers), else "json" _default_format = "ai" if os.environ.get("CODELENS_AI_MODE", "").lower() in ("1", "true", "yes") else "json" @@ -914,6 +921,14 @@ def main(): help=f"Output format (default: {_default_format}. Set CODELENS_AI_MODE=1 for ai default. compact = token-efficient single-char keys)") parser.add_argument("--db-path", default=None, help="Custom path for SQLite database (default: .codelens/codelens.db)") + # Issue #157: --diff-base restricts analysis to files changed + # relative to . Pre-filter layer: commands still scan the full + # workspace, but findings from unchanged files are filtered out of + # the result. Empty diff → early exit with a clear message. + parser.add_argument("--diff-base", default=None, metavar="REF", + help="Git ref (branch/tag/SHA/HEAD~1) to diff against. " + "Only findings from files changed relative to REF " + "are reported. Useful for CI PR checks. (issue #157)") # ─── Parse and dispatch ───────────────────────────── @@ -943,6 +958,7 @@ def main(): global_deep = False global_disable_suppression = False global_ignore_pattern = None + global_diff_base = None # issue #157 i = 1 while i < len(sys.argv): @@ -985,6 +1001,12 @@ def main(): global_ignore_pattern = sys.argv[i + 1] elif arg.startswith('--codelens-ignore-pattern='): global_ignore_pattern = arg.split('=', 1)[1] + # Issue #157: --diff-base (space form) + elif arg == '--diff-base' and i + 1 < len(sys.argv): + global_diff_base = sys.argv[i + 1] + # Issue #157: --diff-base= (equals form) + elif arg.startswith('--diff-base='): + global_diff_base = arg.split('=', 1)[1] i += 1 args = parser.parse_args() @@ -1034,6 +1056,53 @@ def main(): if workspace != (getattr(args, 'workspace', None) or ""): print(f"[CodeLens] Auto-detected workspace: {workspace}", file=sys.stderr) + # ─── Issue #157: --diff-base ─────────────────────────── + # Build the DiffScope once, before command execution. If the diff is + # empty, early-exit with a clear message. The scope is attached to + # ``args`` so commands that want to do in-engine pre-filtering can + # access it (none do yet — this is a post-filter layer for now). + diff_scope = None + # Resolve --diff-base: global pre-parse value or subparser value. + # argparse stores --diff-base as ``diff_base`` on the subparser too, + # but since it's a global flag, the pre-parse value is authoritative. + diff_base_ref = global_diff_base or getattr(args, 'diff_base', None) + if diff_base_ref: + from diff_scope import DiffScope, DiffScopeError + try: + diff_scope = DiffScope.from_ref(workspace, diff_base_ref) + except DiffScopeError as exc: + error_result = { + "status": "error", + "command": args.command, + "error": str(exc), + "error_type": "diff_scope_error", + "suggestion": ( + "Ensure the workspace is a git repository and the ref " + "exists. Use `git rev-parse --verify ` to check." + ), + } + print(format_output(error_result, args.format, args.command), file=sys.stderr) + sys.exit(1) + if diff_scope.is_empty: + # Empty diff → early exit per issue #157 DoD + empty_result = { + "status": "ok", + "command": args.command, + "message": f"No changed files relative to {diff_base_ref!r}", + "diff_scope": diff_scope.summary(), + "stats": {}, + "findings": [], + } + print(format_output(empty_result, args.format, args.command, workspace)) + sys.exit(0) + # Attach to args so commands can opt-in to in-engine pre-filtering + args.diff_scope = diff_scope + print( + f"[CodeLens] --diff-base {diff_base_ref!r}: {diff_scope.changed_count} " + f"file(s) in scope", + file=sys.stderr, + ) + # ─── Auto-setup: if command needs registry and none exists, bootstrap it ──── # Commands that need a registry to work meaningfully _REGISTRY_COMMANDS = { @@ -1276,6 +1345,59 @@ def main(): except Exception as e: logger.warning(f"Suppression processing failed: {e}", exc_info=True) + # ─── Issue #157: --diff-base post-filter ── + # Drop findings from files not in the changed-file allowlist. This + # is a post-filter layer — commands still scan the full workspace, + # but findings from unchanged files are removed before output. + # Commands that produce graph data (trace, impact, circular, scan) + # are NOT filtered because their results are structural (node/edge + # graphs) rather than file-keyed findings — filtering them would + # silently corrupt the graph. + if diff_scope is not None and isinstance(result, dict) and args.command in ( + "secrets", "smell", "complexity", "dead-code", "debug-leak", + "circular", "taint", "vuln-scan", "check", "analyze", + "missing-refs", "side-effect", "perf-hint", "regex-audit", + "a11y", "css-deep", "dataflow", "stack-trace", "config-drift", + "ownership", "test-map", + ): + _FILTER_KEYS = ( + "findings", "leaks", "hints", "issues", "violations", + "matches", "chains", "results", + ) + total_before = 0 + total_after = 0 + for key in _FILTER_KEYS: + val = result.get(key) + if isinstance(val, list): + before = len(val) + result[key] = diff_scope.filter_findings(val) + total_before += before + total_after += len(result[key]) + elif isinstance(val, dict): + # Category-keyed (dead-code by_category, smell by_category) + for sub_key, sub_val in val.items(): + if isinstance(sub_val, list): + before = len(sub_val) + val[sub_key] = diff_scope.filter_findings(sub_val) + total_before += before + total_after += len(val[sub_key]) + # Also filter the flat ``findings`` list that some commands + # (e.g., ``check``) produce at the top level. + if "findings" in result and isinstance(result["findings"], list): + # Already filtered above if ``findings`` is in _FILTER_KEYS, + # but ``check`` stores them under ``findings`` — covered. + pass + # Attach diff_scope summary so consumers can see what was filtered + result["diff_scope"] = diff_scope.summary() + result["diff_scope"]["findings_before_filter"] = total_before + result["diff_scope"]["findings_after_filter"] = total_after + if total_before != total_after: + print( + f"[CodeLens] --diff-base: {total_before - total_after} " + f"finding(s) from unchanged files filtered out", + file=sys.stderr, + ) + # ─── Format and print output ── # Some commands (doctor issue #64 Phase 1, sessions issue #64 # Phase 2) print their own human-readable output directly and diff --git a/scripts/diff_scope.py b/scripts/diff_scope.py new file mode 100644 index 00000000..2c68528f --- /dev/null +++ b/scripts/diff_scope.py @@ -0,0 +1,312 @@ +# @WHO: scripts/diff_scope.py +# @WHAT: DiffScope — git-diff-based file allowlist for --diff-base flag (issue #157) +# @PART: utils +# @ENTRY: DiffScope.from_ref() +""" +DiffScope — restrict CodeLens analysis to git-changed files only. + +Issue #157: ``--diff-base `` global flag. When set, CodeLens should +only report findings from files that changed relative to ````. This +matters in CI: a PR check should flag only NEW issues introduced by the +PR, not pre-existing issues in unchanged files. + +Design +------ +DiffScope is a thin wrapper around ``git_aware.get_changed_files()`` (and +``get_untracked_files()``). It validates the ref, computes the changed-file +set, and exposes: + +- ``DiffScope.from_ref(workspace, ref)`` — factory; returns a DiffScope or + raises ``DiffScopeError`` on invalid ref / not-a-git-repo +- ``scope.changed_files`` — frozenset of relative paths +- ``scope.is_empty`` — True if the diff is empty (caller should early-exit) +- ``scope.allows(path)`` — True if ``path`` is in the changed-file set +- ``scope.filter_findings(findings, file_key=...)`` — drop findings whose + file is not in the changed-file set + +The class is intentionally pure (no I/O beyond the one-time git diff call +in the factory). ``filter_findings`` handles both relative and absolute +file paths — engines are inconsistent (secrets uses rel_path, check uses +absolute file_path for rule-engine findings), so the filter normalizes both +to relative paths before comparing. + +@FLOW: DIFF_SCOPE_FILTER +@CALLS: git_aware.get_changed_files() -> List[str] +@CALLS: git_aware.get_untracked_files() -> List[str] +@MUTATES: none (pure utility — reads git, returns data) +""" + +from __future__ import annotations + +import os +from typing import Any, Dict, FrozenSet, Iterable, List, Optional + +# git_aware is in the same scripts/ directory; the CLI adds scripts/ to +# sys.path before importing. Lazy import so this module can be imported +# in test contexts where git_aware isn't on the path yet. + + +class DiffScopeError(Exception): + """Raised when a DiffScope cannot be constructed. + + Common causes: + - The workspace is not a git repository + - The ref does not exist (typo, wrong branch name) + - Git is not installed + """ + + +class DiffScope: + """An immutable allowlist of git-changed files for one workspace. + + Construct via :meth:`from_ref`. Once constructed, the instance is + safe to share across commands — the changed-file set is captured at + construction time and does not change. + """ + + __slots__ = ("_workspace", "_changed_files", "_base_ref") + + def __init__(self, workspace: str, changed_files: Iterable[str]) -> None: + self._workspace = os.path.abspath(workspace) + self._base_ref: Optional[str] = None + # Normalize: relative paths, forward slashes for cross-platform compare. + # Handle BOTH separators (os.sep + altsep) so backslash paths from + # Windows are normalized to forward slashes on any platform. + normalized = set() + for p in changed_files: + if not p: + continue + # Store as relative path with OS-native separators + if os.path.isabs(p): + try: + p = os.path.relpath(p, self._workspace) + except ValueError: + # On Windows, relpath across drives raises — keep as-is + pass + # Normalize to forward slashes for stable comparison. + # Replace both os.sep and os.altsep (Windows: \ and /). + p = p.replace("\\", "/") + if os.altsep and os.altsep != "/": + p = p.replace(os.altsep, "/") + normalized.add(p) + self._changed_files: FrozenSet[str] = frozenset(normalized) + + @property + def workspace(self) -> str: + """Absolute path to the workspace root.""" + return self._workspace + + @property + def changed_files(self) -> FrozenSet[str]: + """FrozenSet of changed file paths (relative, forward-slash).""" + return self._changed_files + + @property + def is_empty(self) -> bool: + """True if no files changed relative to the base ref. + + Callers should check this and early-exit with a clear message + rather than running analysis that would produce zero findings. + """ + return len(self._changed_files) == 0 + + @property + def changed_count(self) -> int: + """Number of changed files.""" + return len(self._changed_files) + + def allows(self, path: str) -> bool: + """Return True if ``path`` is in the changed-file allowlist. + + Handles both absolute and relative paths. Paths are normalized + to relative + forward-slash before comparison so the check works + cross-platform. + + Args: + path: A file path (absolute or relative to workspace). + + Returns: + True if the path is in the changed-file set. + """ + if not path: + return False + # Normalize to relative + forward slash (same logic as __init__) + p = path + if os.path.isabs(p): + try: + p = os.path.relpath(p, self._workspace) + except ValueError: + # Windows cross-drive — fall through with original + pass + p = p.replace("\\", "/") + if os.altsep and os.altsep != "/": + p = p.replace(os.altsep, "/") + return p in self._changed_files + + def filter_findings( + self, + findings: List[Dict[str, Any]], + file_keys: Optional[List[str]] = None, + ) -> List[Dict[str, Any]]: + """Drop findings whose file is not in the changed-file allowlist. + + Engines are inconsistent about the key name for the file path + (``file``, ``path``, ``defined_in``, ``file_path``) and about + whether it's absolute or relative. This function tries a list + of keys in order and uses the first one present on each finding. + + Findings with no recognizable file key are KEPT — they may be + workspace-level findings (e.g., "no .gitignore found") that + shouldn't be filtered out by a file-based diff scope. + + Args: + findings: List of finding dicts. + file_keys: Optional list of keys to try (default: ``file``, + ``path``, ``defined_in``, ``file_path``). + + Returns: + New list containing only findings from changed files (or + findings with no file key). + """ + if file_keys is None: + file_keys = ["file", "path", "defined_in", "file_path"] + + kept: List[Dict[str, Any]] = [] + for f in findings: + if not isinstance(f, dict): + kept.append(f) + continue + file_path: Optional[str] = None + for key in file_keys: + val = f.get(key) + if isinstance(val, str) and val: + file_path = val + break + if file_path is None: + # No file key — keep workspace-level findings + kept.append(f) + continue + if self.allows(file_path): + kept.append(f) + return kept + + def summary(self) -> Dict[str, Any]: + """Return a dict summary suitable for embedding in command output. + + Commands should add this to their result dict under a + ``diff_scope`` key so consumers (CI, agents) can see which files + were in scope. + """ + return { + "base_ref": self._base_ref, + "changed_files": sorted(self._changed_files), + "changed_count": self.changed_count, + "workspace": self._workspace, + } + + # ─── Factory ───────────────────────────────────────────── + + @classmethod + def from_ref( + cls, + workspace: str, + ref: str, + include_untracked: bool = True, + ) -> "DiffScope": + """Construct a DiffScope by diffing HEAD against ``ref``. + + Args: + workspace: Path to the workspace root (must be a git repo). + ref: Git ref to diff against (branch name, tag, SHA, ``HEAD~1``, + ``origin/main``, etc.). + include_untracked: If True (default), also include untracked + files (newly created files not yet ``git add``-ed). These + are part of the working-tree changes and should be in scope. + + Returns: + A DiffScope instance. + + Raises: + DiffScopeError: If the workspace is not a git repo, git is + unavailable, or ``ref`` does not exist. + """ + if not ref: + raise DiffScopeError("--diff-base requires a non-empty ref argument") + + workspace = os.path.abspath(workspace) + if not os.path.isdir(workspace): + raise DiffScopeError( + f"Workspace does not exist or is not a directory: {workspace}" + ) + + # Lazy import so this module can be imported in test contexts + try: + from git_aware import get_changed_files, get_untracked_files + except ImportError as exc: + raise DiffScopeError( + f"git_aware module unavailable — cannot compute diff: {exc}" + ) from exc + + # Validate the ref BEFORE calling get_changed_files. + # get_changed_files returns [] on invalid ref (same as "no changes"), + # which would silently produce an empty scope. We need to distinguish + # "invalid ref" from "valid ref with no changes". + _validate_git_ref(workspace, ref) + + changed = get_changed_files(workspace, since_sha=ref) + + if include_untracked: + untracked = get_untracked_files(workspace) + # Untracked files are returned as absolute paths by get_untracked_files + changed = list(changed) + [ + os.path.relpath(p, workspace) if os.path.isabs(p) else p + for p in untracked + ] + + scope = cls(workspace, changed) + # Attach the base ref so summary() can report it + scope._base_ref = ref + return scope + + +# ─── Internal helpers ──────────────────────────────────────── + + +def _validate_git_ref(workspace: str, ref: str) -> None: + """Verify that ``ref`` exists in the git repo at ``workspace``. + + Raises ``DiffScopeError`` if git is unavailable, the workspace is not + a git repo, or ``ref`` does not resolve to a valid commit. + + Args: + workspace: Absolute path to workspace root. + ref: Git ref (branch, tag, SHA, HEAD~1, etc.). + """ + import subprocess + + try: + result = subprocess.run( + ["git", "rev-parse", "--verify", ref], + cwd=workspace, + capture_output=True, + text=True, + timeout=10, + ) + except FileNotFoundError as exc: + raise DiffScopeError( + f"git command not found — cannot validate ref {ref!r}: {exc}" + ) from exc + except subprocess.TimeoutExpired as exc: + raise DiffScopeError( + f"git rev-parse timed out validating ref {ref!r}: {exc}" + ) from exc + except Exception as exc: + raise DiffScopeError( + f"Unexpected error validating ref {ref!r}: {exc}" + ) from exc + + if result.returncode != 0: + stderr = (result.stderr or "").strip() + raise DiffScopeError( + f"Invalid git ref {ref!r}: {stderr or 'ref does not exist'}" + ) diff --git a/tests/test_diff_scope.py b/tests/test_diff_scope.py new file mode 100644 index 00000000..0e59e01c --- /dev/null +++ b/tests/test_diff_scope.py @@ -0,0 +1,443 @@ +"""Tests for scripts/diff_scope.py — issue #157. + +Tests the DiffScope class and its factory from_ref(), including: +- Construction with explicit file lists +- Path normalization (absolute ↔ relative, OS separators) +- filter_findings with various file key names +- from_ref against real temporary git repos +- Error cases: invalid ref, empty ref, non-git directory, missing workspace +- Empty diff detection +- Summary dict structure +""" + +import os +import shutil +import subprocess +import sys +import tempfile +import unittest + +SCRIPT_DIR = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "scripts") +sys.path.insert(0, SCRIPT_DIR) + +from diff_scope import DiffScope, DiffScopeError + + +# ─── 1. Direct construction ────────────────────────────────── + + +class TestConstruction(unittest.TestCase): + """DiffScope constructed directly with a file list (no git).""" + + def test_basic_construction(self): + scope = DiffScope("/tmp/work", ["src/a.py", "src/b.py"]) + self.assertEqual(scope.workspace, "/tmp/work") + self.assertEqual(scope.changed_count, 2) + self.assertFalse(scope.is_empty) + + def test_absolute_paths_normalized_to_relative(self): + scope = DiffScope("/tmp/work", ["/tmp/work/src/a.py"]) + # Stored as relative with forward slashes + self.assertIn("src/a.py", scope.changed_files) + + def test_empty_file_list_is_empty_scope(self): + scope = DiffScope("/tmp/work", []) + self.assertTrue(scope.is_empty) + self.assertEqual(scope.changed_count, 0) + + def test_empty_strings_in_list_are_skipped(self): + scope = DiffScope("/tmp/work", ["", "src/a.py", ""]) + self.assertEqual(scope.changed_count, 1) + + def test_paths_normalized_to_forward_slash(self): + """Backslash separators (Windows) normalized to forward slash.""" + scope = DiffScope("/tmp/work", ["src\\nested\\a.py"]) + self.assertIn("src/nested/a.py", scope.changed_files) + + def test_changed_files_is_frozenset(self): + scope = DiffScope("/tmp/work", ["src/a.py"]) + self.assertIsInstance(scope.changed_files, frozenset) + + def test_workspace_is_absolute(self): + scope = DiffScope("relative/path", ["a.py"]) + self.assertTrue(os.path.isabs(scope.workspace)) + + +# ─── 2. allows() ───────────────────────────────────────────── + + +class TestAllows(unittest.TestCase): + """DiffScope.allows() path-matching logic.""" + + def setUp(self): + self.scope = DiffScope("/tmp/work", ["src/a.py", "src/b.py"]) + + def test_relative_path_in_set(self): + self.assertTrue(self.scope.allows("src/a.py")) + + def test_absolute_path_in_set(self): + self.assertTrue(self.scope.allows("/tmp/work/src/a.py")) + + def test_relative_path_not_in_set(self): + self.assertFalse(self.scope.allows("src/c.py")) + + def test_empty_path(self): + self.assertFalse(self.scope.allows("")) + + def test_none_path(self): + self.assertFalse(self.scope.allows(None)) # type: ignore[arg-type] + + def test_backslash_path_matches(self): + """Windows-style backslash path should match forward-slash entry.""" + scope = DiffScope("/tmp/work", ["src/a.py"]) + self.assertTrue(scope.allows("src\\a.py")) + + +# ─── 3. filter_findings() ──────────────────────────────────── + + +class TestFilterFindings(unittest.TestCase): + """DiffScope.filter_findings() drops findings from unchanged files.""" + + def setUp(self): + self.scope = DiffScope("/tmp/work", ["src/a.py", "src/b.py"]) + + def test_filters_by_file_key(self): + findings = [ + {"file": "src/a.py", "msg": "keep"}, + {"file": "src/unchanged.py", "msg": "drop"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 1) + self.assertEqual(kept[0]["msg"], "keep") + + def test_filters_by_path_key(self): + findings = [ + {"path": "src/a.py", "msg": "keep"}, + {"path": "src/unchanged.py", "msg": "drop"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 1) + + def test_filters_by_defined_in_key(self): + findings = [ + {"defined_in": "src/a.py", "msg": "keep"}, + {"defined_in": "src/unchanged.py", "msg": "drop"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 1) + + def test_filters_by_file_path_key(self): + findings = [ + {"file_path": "src/a.py", "msg": "keep"}, + {"file_path": "src/unchanged.py", "msg": "drop"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 1) + + def test_absolute_file_path_matches_relative_entry(self): + findings = [ + {"file": "/tmp/work/src/a.py", "msg": "keep"}, + {"file": "/tmp/work/src/unchanged.py", "msg": "drop"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 1) + + def test_findings_without_file_key_are_kept(self): + """Workspace-level findings (no file key) should be kept.""" + findings = [ + {"msg": "no .gitignore found", "severity": "low"}, + {"file": "src/a.py", "msg": "keep"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 2) + + def test_empty_file_value_treated_as_no_file(self): + """A finding with file="" should be kept (treated as no file key).""" + findings = [ + {"file": "", "msg": "keep"}, + {"file": "src/a.py", "msg": "keep"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 2) + + def test_empty_findings_list(self): + kept = self.scope.filter_findings([]) + self.assertEqual(kept, []) + + def test_non_dict_findings_kept(self): + """Non-dict entries (strings, None) are kept — caller's responsibility.""" + findings = ["raw string", None, {"file": "src/a.py", "msg": "keep"}] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 3) + + def test_custom_file_keys_override_defaults(self): + """When custom file_keys are specified, only those keys are checked. + + A finding with a ``file`` key (default key) but no ``custom_key`` + should be KEPT (treated as no-file because the custom key isn't + present). + """ + findings = [ + {"custom_key": "src/a.py", "msg": "keep"}, + {"custom_key": "src/unchanged.py", "msg": "drop"}, + {"file": "src/a.py", "msg": "keep (no custom_key → treated as no-file)"}, + ] + kept = self.scope.filter_findings(findings, file_keys=["custom_key"]) + self.assertEqual(len(kept), 2) + msgs = {f["msg"] for f in kept} + self.assertIn("keep", msgs) + self.assertIn("keep (no custom_key → treated as no-file)", msgs) + + def test_does_not_mutate_input(self): + findings = [{"file": "src/a.py"}, {"file": "src/unchanged.py"}] + original = [dict(f) for f in findings] + self.scope.filter_findings(findings) + self.assertEqual(findings, original) + + +# ─── 4. summary() ──────────────────────────────────────────── + + +class TestSummary(unittest.TestCase): + """DiffScope.summary() returns a dict suitable for embedding in output.""" + + def test_summary_structure(self): + scope = DiffScope("/tmp/work", ["src/a.py", "src/b.py"]) + s = scope.summary() + self.assertIn("base_ref", s) + self.assertIn("changed_files", s) + self.assertIn("changed_count", s) + self.assertIn("workspace", s) + + def test_summary_changed_files_sorted(self): + scope = DiffScope("/tmp/work", ["src/b.py", "src/a.py"]) + s = scope.summary() + self.assertEqual(s["changed_files"], ["src/a.py", "src/b.py"]) + + def test_summary_base_ref_none_when_not_from_ref(self): + scope = DiffScope("/tmp/work", ["src/a.py"]) + self.assertIsNone(scope.summary()["base_ref"]) + + +# ─── 5. from_ref() against real git repos ─────────────────── + + +class TestFromRefRealGit(unittest.TestCase): + """DiffScope.from_ref() against real temporary git repositories.""" + + def setUp(self): + """Create a temp git repo with a couple of commits.""" + self.tmpdir = tempfile.mkdtemp(prefix="codelens_diffscope_test_") + self._run("git", "init", cwd=self.tmpdir) + self._run("git", "config", "user.email", "test@test.com", cwd=self.tmpdir) + self._run("git", "config", "user.name", "test", cwd=self.tmpdir) + # Initial commit + self._write("file1.py", "print('hello')\n") + self._write("file2.py", "print('world')\n") + self._run("git", "add", ".", cwd=self.tmpdir) + self._run("git", "commit", "-m", "initial", cwd=self.tmpdir) + self.first_sha = self._run("git", "rev-parse", "HEAD", cwd=self.tmpdir).strip() + + def tearDown(self): + shutil.rmtree(self.tmpdir, ignore_errors=True) + + def _run(self, *cmd, cwd=None): + return subprocess.run( + cmd, cwd=cwd, capture_output=True, text=True, check=True + ).stdout + + def _write(self, name, content): + with open(os.path.join(self.tmpdir, name), "w") as f: + f.write(content) + + def test_from_ref_with_no_changes(self): + """HEAD vs HEAD (no uncommitted changes) → empty diff.""" + scope = DiffScope.from_ref(self.tmpdir, "HEAD") + self.assertTrue(scope.is_empty) + self.assertEqual(scope.changed_count, 0) + + def test_from_ref_with_uncommitted_change(self): + """HEAD vs working tree (uncommitted edit) → 1 changed file.""" + self._write("file1.py", "print('changed')\n") + scope = DiffScope.from_ref(self.tmpdir, "HEAD") + self.assertFalse(scope.is_empty) + self.assertEqual(scope.changed_count, 1) + self.assertIn("file1.py", scope.changed_files) + + def test_from_ref_with_new_untracked_file(self): + """Untracked files should be included by default.""" + self._write("file3.py", "print('new')\n") + scope = DiffScope.from_ref(self.tmpdir, "HEAD") + self.assertIn("file3.py", scope.changed_files) + + def test_from_ref_exclude_untracked(self): + """include_untracked=False excludes untracked files.""" + self._write("file3.py", "print('new')\n") + scope = DiffScope.from_ref(self.tmpdir, "HEAD", include_untracked=False) + self.assertNotIn("file3.py", scope.changed_files) + self.assertTrue(scope.is_empty) + + def test_from_ref_against_first_commit(self): + """Diffing HEAD against the first commit should show all post-initial changes.""" + # Make a second commit + self._write("file3.py", "print('third')\n") + self._run("git", "add", ".", cwd=self.tmpdir) + self._run("git", "commit", "-m", "second", cwd=self.tmpdir) + scope = DiffScope.from_ref(self.tmpdir, self.first_sha) + self.assertIn("file3.py", scope.changed_files) + + def test_from_ref_sets_base_ref_in_summary(self): + scope = DiffScope.from_ref(self.tmpdir, "HEAD") + self.assertEqual(scope.summary()["base_ref"], "HEAD") + + def test_from_ref_head_tilde_1(self): + """HEAD~1 syntax works.""" + # Make a second commit + self._write("file3.py", "print('third')\n") + self._run("git", "add", ".", cwd=self.tmpdir) + self._run("git", "commit", "-m", "second", cwd=self.tmpdir) + scope = DiffScope.from_ref(self.tmpdir, "HEAD~1") + self.assertIn("file3.py", scope.changed_files) + self.assertEqual(scope.summary()["base_ref"], "HEAD~1") + + def test_from_ref_branch_name(self): + """Branch name as ref works (detects default branch — main or master).""" + # Detect the default branch created by `git init` + branch_out = subprocess.run( + ["git", "branch", "--show-current"], + cwd=self.tmpdir, capture_output=True, text=True, check=True, + ) + default_branch = branch_out.stdout.strip() + self.assertTrue(default_branch, "git should have a current branch") + scope = DiffScope.from_ref(self.tmpdir, default_branch) + # current_branch vs HEAD (no uncommitted changes) → empty + self.assertTrue(scope.is_empty) + self.assertEqual(scope.summary()["base_ref"], default_branch) + + +# ─── 6. from_ref() error cases ────────────────────────────── + + +class TestFromRefErrors(unittest.TestCase): + """DiffScope.from_ref() raises DiffScopeError on bad input.""" + + def test_invalid_ref_raises(self): + with tempfile.TemporaryDirectory() as tmpdir: + subprocess.run(["git", "init"], cwd=tmpdir, capture_output=True, check=True) + subprocess.run(["git", "config", "user.email", "t@t.com"], cwd=tmpdir, capture_output=True, check=True) + subprocess.run(["git", "config", "user.name", "t"], cwd=tmpdir, capture_output=True, check=True) + with open(os.path.join(tmpdir, "f.py"), "w") as f: + f.write("x = 1\n") + subprocess.run(["git", "add", "."], cwd=tmpdir, capture_output=True, check=True) + subprocess.run(["git", "commit", "-m", "init"], cwd=tmpdir, capture_output=True, check=True) + with self.assertRaises(DiffScopeError) as ctx: + DiffScope.from_ref(tmpdir, "nonexistent-ref-xyz") + self.assertIn("nonexistent-ref-xyz", str(ctx.exception)) + + def test_empty_ref_raises(self): + with tempfile.TemporaryDirectory() as tmpdir: + with self.assertRaises(DiffScopeError) as ctx: + DiffScope.from_ref(tmpdir, "") + self.assertIn("non-empty", str(ctx.exception)) + + def test_nonexistent_workspace_raises(self): + with self.assertRaises(DiffScopeError) as ctx: + DiffScope.from_ref("/nonexistent/path/xyz", "HEAD") + self.assertIn("does not exist", str(ctx.exception)) + + def test_non_git_directory_raises(self): + """A directory that exists but is not a git repo should raise.""" + with tempfile.TemporaryDirectory() as tmpdir: + with self.assertRaises(DiffScopeError): + DiffScope.from_ref(tmpdir, "HEAD") + + +# ─── 7. CLI integration (subprocess) ──────────────────────── + + +class TestCliIntegration(unittest.TestCase): + """End-to-end CLI tests via subprocess — --diff-base flag works.""" + + @classmethod + def setUpClass(cls): + """Use the CodeLens repo itself as the test workspace.""" + cls.codelens_repo = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + cls.cli = os.path.join(cls.codelens_repo, "scripts", "codelens.py") + + def _run_cli(self, *args): + env = os.environ.copy() + env["PYTHONUTF8"] = "1" + env.pop("CODELENS_AI_MODE", None) # ensure json default + proc = subprocess.run( + [sys.executable, self.cli] + list(args), + capture_output=True, text=True, env=env, timeout=60, + cwd=self.codelens_repo, + ) + return proc + + def test_diff_base_flag_in_help(self): + proc = self._run_cli("--help") + self.assertIn("--diff-base", proc.stdout) + + def test_invalid_ref_exits_nonzero(self): + proc = self._run_cli("secrets", "tests/fixtures", "--diff-base", "nonexistent-ref-xyz") + self.assertNotEqual(proc.returncode, 0) + self.assertIn("diff_scope_error", proc.stderr + proc.stdout) + + def test_valid_ref_produces_diff_scope_in_output(self): + """--diff-base HEAD~1 should add a diff_scope key to the JSON output.""" + proc = self._run_cli("secrets", "tests/fixtures", "--diff-base", "HEAD~1") + # Find JSON in output (skip stderr hint lines) + import json + out = proc.stdout + json_start = out.find("{") + self.assertGreater(json_start, -1, "No JSON in output") + data = json.loads(out[json_start:]) + self.assertIn("diff_scope", data) + self.assertIn("changed_count", data["diff_scope"]) + self.assertIn("findings_before_filter", data["diff_scope"]) + self.assertIn("findings_after_filter", data["diff_scope"]) + + def test_empty_diff_early_exit(self): + """--diff-base HEAD in a clean repo → early exit with 'No changed files' message.""" + with tempfile.TemporaryDirectory() as tmpdir: + subprocess.run(["git", "init"], cwd=tmpdir, capture_output=True, check=True) + subprocess.run(["git", "config", "user.email", "t@t.com"], cwd=tmpdir, capture_output=True, check=True) + subprocess.run(["git", "config", "user.name", "t"], cwd=tmpdir, capture_output=True, check=True) + with open(os.path.join(tmpdir, "f.py"), "w") as f: + f.write("x = 1\n") + subprocess.run(["git", "add", "."], cwd=tmpdir, capture_output=True, check=True) + subprocess.run(["git", "commit", "-m", "init"], cwd=tmpdir, capture_output=True, check=True) + + env = os.environ.copy() + env["PYTHONUTF8"] = "1" + env.pop("CODELENS_AI_MODE", None) + proc = subprocess.run( + [sys.executable, self.cli, "secrets", tmpdir, "--diff-base", "HEAD"], + capture_output=True, text=True, env=env, timeout=60, + cwd=self.codelens_repo, + ) + self.assertEqual(proc.returncode, 0) + import json + out = proc.stdout + json_start = out.find("{") + self.assertGreater(json_start, -1) + data = json.loads(out[json_start:]) + self.assertEqual(data["status"], "ok") + self.assertIn("No changed files", data.get("message", "")) + self.assertEqual(data["diff_scope"]["changed_count"], 0) + + def test_diff_base_before_subcommand(self): + """--diff-base works both before and after the subcommand.""" + proc = self._run_cli("--diff-base", "HEAD~1", "secrets", "tests/fixtures") + import json + out = proc.stdout + json_start = out.find("{") + self.assertGreater(json_start, -1) + data = json.loads(out[json_start:]) + self.assertIn("diff_scope", data) + + +if __name__ == "__main__": + unittest.main()